# Deep Review: 20260325-112652-pr-188

| | |
|---|---|
| **Date** | 2026-03-25 11:26 |
| **Repo** | [rancher-sandbox/rancher-desktop-daemon](https://github.com/rancher-sandbox/rancher-desktop-daemon) |
| **Round** | 3 |
| **Author** | [@mook-as](https://github.com/mook-as) |
| **PR** | [#188](https://github.com/rancher-sandbox/rancher-desktop-daemon/pull/188) — mock: namespaces: ensure we have all namespaces |
| **Branch** | `mook-as/container-api/mock-namespace` |
| **Commits** | `4c931b2` API: containers: make pid optional<br>`e9fa971` mock controller: Create in `rancher-desktop` namespace<br>`c75bc1a` mock: namespaces: ensure we have all namespaces |
| **Reviewers** | Claude Opus 4.6, Codex GPT 5.4, ~~Gemini 3.1 Pro~~ (429 capacity) |
| **Verdict** | **Merge with fixes** — two regressions in test correctness; dead watch code should be removed |
| **Wall-clock time** | `20 min 5 s` |

### Executive Summary

This PR moves mock controller resources from the `default` Kubernetes namespace to a dedicated `rancher-desktop` namespace, adds a `kubeNamespaceReconciler` to create that namespace, extends `containerNamespaceReconciler` to derive container namespaces from volume data, and makes the `Pid` field on containers optional. The core namespace migration is correct, but the new `kubeNamespaceReconciler` contains a dead `Watches` clause that creates a false impression of self-healing, and the BATS tests regress by using scalar variables where arrays are needed.

---

## Consolidated Review

### Critical Issues

None.

---

### Important Issues

1. **Dead `Watches` clause in `kubeNamespaceReconciler`** — `pkg/controllers/mock/kube_namespace_reconciler.go:63-80` (important, regression) [Claude Opus 4.6, Codex GPT 5.4]

```go
return ctrl.NewControllerManagedBy(mgr).
    For(&corev1.Namespace{}).
    Named("mock-kube-namespace-reconciler").
    Watches(
        &corev1.Namespace{},
        handler.EnqueueRequestForOwner(
            mgr.GetScheme(),
            mgr.GetRESTMapper(),
            &corev1.Namespace{},
            handler.OnlyControllerOwner(),
        )).
    WithEventFilter(predicate.NewPredicateFuncs(func(object client.Object) bool {
        if _, ok := object.(*corev1.Namespace); ok {
            return object.GetName() == mockNamespaceName
        }
        return false
    }))
```

The `WithEventFilter` at line 75 applies to all event sources. Both `For` and `Watches` target `corev1.Namespace`, so the type assertion at line 76 always succeeds and the name check at line 77 always compares against `mockNamespaceName` (`"rdd-mocks"`). Events on the owned `rancher-desktop` namespace are rejected before `EnqueueRequestForOwner` runs.

Other mock reconcilers avoid this problem because their primary and secondary types differ (e.g., `corev1.Namespace` vs. `containersv1alpha1.ContainerNamespace`), so the type assertion distinguishes the two event streams. The `kubeNamespaceReconciler` is unique in watching `corev1.Namespace` for both.

Practical impact is low: the reconciler fires once on `rdd-mocks` creation and creates `rancher-desktop`. If `rancher-desktop` is later deleted, nothing recreates it until a restart. Nobody deletes it during normal operation, but the dead code misleads future readers into thinking the controller self-heals.

Fix: Remove the `Watches` clause. It cannot work with a shared-type predicate, and removing it eliminates the false impression of self-healing.

```diff
 func (r *kubeNamespaceReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&corev1.Namespace{}).
 		Named("mock-kube-namespace-reconciler").
-		Watches(
-			&corev1.Namespace{},
-			handler.EnqueueRequestForOwner(
-				mgr.GetScheme(),
-				mgr.GetRESTMapper(),
-				&corev1.Namespace{},
-				handler.OnlyControllerOwner(),
-			)).
 		WithEventFilter(predicate.NewPredicateFuncs(func(object client.Object) bool {
```

2. **BATS tests use `"${var[@]}"` on scalar variables** — `bats/tests/34-containers-controllers/containers-mock.bats:28-32,51-55` (important, regression) [Claude Opus 4.6, Codex GPT 5.4]

```bash
run -0 jq_output '.[].Id'
containers=${output}

rdd ctl wait --for=create --namespace="${NAMESPACE}" container "${containers[@]}" --timeout=30s
```

`containers` and `volumes` are assigned via `var=${output}` (plain string, not an array). In bash, `"${var[@]}"` on a non-array expands as a single quoted word. With the current single-entry test data this works by accident, but adding a second entry to `containers.json` or `volumes.json` would pass a newline-embedded string as one argument instead of separate arguments. The original code (replaced in commit `e9fa971`) handled multiple entries with a `while IFS= read -r` loop.

Fix: Declare proper arrays.

```diff
-    containers=${output}
-
-    rdd ctl wait --for=create --namespace="${NAMESPACE}" container "${containers[@]}" --timeout=30s
+    readarray -t containers <<<"${output}"
+    rdd ctl wait --for=create --namespace="${NAMESPACE}" container "${containers[@]}" --timeout=30s
```

Same fix for volumes at lines 53-55.

---

### Suggestions

1. **`assert_line` in images test is a no-op** — `bats/tests/34-containers-controllers/containers-mock.bats:43-44` (suggestion, regression) [Claude Opus 4.6]

```bash
rdd ctl wait --for=create --namespace="${NAMESPACE}" image \
    --field-selector "status.repoTag=${image}" --timeout=30s \
    --output=jsonpath='{.items[*].status.repoTag}'
assert_line "${image}"
```

The `rdd ctl wait` call runs directly (not via `run`), so `$output` still holds the jq output from line 39. Since `$image` comes from iterating that same jq output, `assert_line "${image}"` is true by construction. The `--output=jsonpath=...` result goes to stdout but is never captured.

The test is still valid: `--for=create` with `--field-selector "status.repoTag=${image}"` verifies the image exists with the correct repoTag. The `assert_line` is redundant.

Fix: Either remove the `assert_line` and `--output` flag, or wrap the wait in `run -0` to capture and verify the API response.

2. **Design doc still references `default` namespace** — `docs/design/api_containers.md:55,279,300` (suggestion, gap) [Codex GPT 5.4]

The design doc describes `Container` and `Volume` objects in the `default` namespace. If `rancher-desktop` is now the intended namespace, the doc should be updated to match.

---

### Design Observations

**Concerns**

- **Startup race between namespace creation and resource creation** (in-scope) [Claude Opus 4.6] — Previously, resources lived in `default` (always present). Now they target `rancher-desktop`, created by `kubeNamespaceReconciler`. All mock reconcilers trigger concurrently on the `rdd-mocks` event. If container/image/volume reconcilers run before `kubeNamespaceReconciler` completes, their Apply calls fail with a namespace-not-found error. Controller-runtime requeues them and the next attempt succeeds, so this is self-healing. But it produces transient error logs at startup that could confuse debugging. A simpler approach: create `rancher-desktop` directly in `createNamespace()` at `mock_controller.go:119`, alongside `rdd-mocks`, eliminating the race entirely.

- **Second namespace-lifecycle path** (future) [Codex GPT 5.4] — A separate `kubeNamespaceReconciler` creates a parallel namespace-management path alongside the startup bootstrap in `mock_controller.go:68-105`. Folding `rancher-desktop` creation into the existing bootstrap flow would remove the watch/predicate coupling that caused Finding 1.

**Strengths**

- The `getVolumeName` abstraction at `volume_reconciler.go:42-44` cleanly separates the volume-to-namespace mapping. When volume namespacing evolves, only this function changes. [Claude Opus 4.6]
- Using `errors.Join` for error aggregation across the namespace loop (`container_namespace_reconciler.go:80`) is the right approach: one failed namespace does not block the others. [Claude Opus 4.6]
- Moving mock objects into `rancher-desktop` aligns with the intended containers API direction. [Codex GPT 5.4]

---

### Testing Assessment

Untested scenarios ranked by risk:

1. **Multiple test data entries** — The `"${var[@]}"` expansion will break with more than one container or volume. Currently masked by single-entry test data.
2. **Deletion and re-creation of `rancher-desktop` namespace** — The dead `Watches` clause means the controller will not self-heal. Low priority for a mock controller.

---

### Documentation Assessment

`docs/design/api_containers.md` still documents `Container` and `Volume` objects in the `default` namespace (lines 55, 279, 300). This should be updated if `rancher-desktop` is the intended namespace going forward.

### Commit Structure

Commit `4c931b2` ("API: containers: make pid optional") is an independent API change unrelated to the namespace work. It would be cleaner in a separate PR, but it is small and self-contained enough that bundling is not a practical concern. The namespace commits (`c75bc1a` then `e9fa971`) build on each other logically.

### Unresolved Feedback

The PR author's [comment](https://github.com/rancher-sandbox/rancher-desktop-daemon/pull/188#issuecomment-3948232496) states resources should use `rancher-desktop` instead of `default`. Commit `e9fa971` addresses this.

---

## Agent Performance Retro

### [Claude] Opus 4.6

- **Duration**: 8 min 54 s
- **Unique contributions**: Identified the `assert_line` no-op (Suggestion 1). Provided detailed explanation of why `WithEventFilter` affects all event sources when both `For` and `Watches` share a type. Noted the startup race design concern with a concrete fix location.
- **Accuracy**: All findings verified with `git blame` and code tracing. No false positives.
- **Depth**: Traced into other mock reconcilers to explain why they avoid the predicate problem (different primary/secondary types). Traced the `try` helper to explain why `$output` is stale.
- **Coverage**: All 9 files reviewed. No coverage misses.
- **Signal-to-noise**: High. Three findings, all actionable, no filler.

### [Codex] GPT 5.4

- **Duration**: 5 min 23 s
- **Unique contributions**: Flagged the `docs/design/api_containers.md` documentation gap (Suggestion 2). Traced into the builtin namespace controller to show the downstream impact of namespace deletion.
- **Accuracy**: All findings verified. No false positives.
- **Depth**: Traced into `builtin/namespace/controllers/namespace_controller.go` to show the cascade effect of namespace deletion. Cited specific line ranges for downstream retry loops.
- **Coverage**: All 9 files reviewed. No coverage misses.
- **Signal-to-noise**: High. Two findings plus one documentation gap, all actionable.

### [Gemini] 3.1 Pro

Failed with HTTP 429 (MODEL_CAPACITY_EXHAUSTED). No review produced.

### Summary

| Metric | Claude Opus 4.6 | Codex GPT 5.4 | Gemini 3.1 Pro |
|--------|-----------------|----------------|----------------|
| Duration | 8:54 | 5:23 | N/A (429) |
| Critical | 0 | 0 | — |
| Important | 2 | 1 | — |
| Suggestion | 1 | 1 | — |
| Design observations | 3 | 2 | — |
| False positives | 0 | 0 | — |
| Unique insights | 1 | 1 | — |
| Files reviewed | 9 | 9 | — |
| Coverage misses | 0 | 0 | — |

Both agents converged on the same two core issues (dead `Watches`, BATS scalar expansion), providing strong confirmation. Claude went deeper on the controller-runtime mechanics and caught the `assert_line` no-op. Codex was faster and caught the documentation gap. Gemini was unavailable due to capacity exhaustion.

---

## Appendix: Original Reviews

### Claude Opus 4.6

Now I have a thorough understanding of all the changes. Let me compile the review.

---

## Review: PR #188 — mock: namespaces: ensure we have all namespaces

| Attribute | Value |
|-----------|-------|
| **Risk** | Low |
| **Verdict** | Approve with suggestions |

### Executive Summary

This PR moves mock controller resources from the `default` Kubernetes namespace to a dedicated `rancher-desktop` namespace, adds a new `kubeNamespaceReconciler` to create that namespace, and extends the `containerNamespaceReconciler` to derive container namespaces from volume data. The `Pid` field on containers is also made optional (separate concern). The core changes are correct and self-consistent, but the new `kubeNamespaceReconciler` has dead code in its watch setup, and the BATS tests use incorrect array expansion syntax that works by accident with single-entry test data.

### Findings

#### Critical Issues

None.

#### Important Issues

1. **`kubeNamespaceReconciler` Watches clause is dead code** — `kube_namespace_reconciler.go:67-80` (important, regression)

```go
Watches(
    &corev1.Namespace{},
    handler.EnqueueRequestForOwner(
        mgr.GetScheme(),
        mgr.GetRESTMapper(),
        &corev1.Namespace{},
        handler.OnlyControllerOwner(),
    )).
WithEventFilter(predicate.NewPredicateFuncs(func(object client.Object) bool {
    if _, ok := object.(*corev1.Namespace); ok {
        return object.GetName() == mockNamespaceName
    }
    return false
}))
```

The `WithEventFilter` at line 75 applies to **all** event sources — both `For` and `Watches`. When the owned `rancher-desktop` namespace is created/updated/deleted, the event object is the `rancher-desktop` Namespace. The predicate checks `object.GetName() == mockNamespaceName` (line 77), which is `"rancher-desktop" == "rdd-mocks"` → false. The event is rejected before `EnqueueRequestForOwner` ever runs.

Every other mock reconciler (`containerNamespaceReconciler` at line 101-108, `containerReconciler`, `volumeReconciler`, `imageReconciler`) works around this because primary and secondary types differ (e.g., `corev1.Namespace` vs `containersv1alpha1.ContainerNamespace`), allowing the predicate to distinguish them with type assertions. The `kubeNamespaceReconciler` is unique in having both `For` and `Watches` on `corev1.Namespace`, making the predicate unable to distinguish primary from secondary events.

Practical impact is low for a mock controller: the reconciler fires once on `rdd-mocks` creation and creates `rancher-desktop`. If `rancher-desktop` is later deleted, the reconciler won't recreate it — but nobody deletes it during normal operation.

Fix: Remove the `Watches` clause entirely, since it cannot work with a shared-type predicate. This removes the false impression of self-healing behavior.

```diff
 func (r *kubeNamespaceReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&corev1.Namespace{}).
 		Named("mock-kube-namespace-reconciler").
-		Watches(
-			&corev1.Namespace{},
-			handler.EnqueueRequestForOwner(
-				mgr.GetScheme(),
-				mgr.GetRESTMapper(),
-				&corev1.Namespace{},
-				handler.OnlyControllerOwner(),
-			)).
 		WithEventFilter(predicate.NewPredicateFuncs(func(object client.Object) bool {
```

2. **BATS tests use `"${var[@]}"` on non-array variables** — `containers-mock.bats:30-32,53-55` (important, regression)

```bash
containers=${output}
# ...
rdd ctl wait --for=create --namespace="${NAMESPACE}" container "${containers[@]}" --timeout=30s
```

`containers` and `volumes` are assigned via `var=${output}` (plain string assignment, lines 30 and 53). In bash, `"${containers[@]}"` on a non-array variable is identical to `"${containers}"` — it expands as a single quoted word. With the current single-entry test data, this works because there's only one container ID and one volume name.

If additional entries are added to `containers.json` or `volumes.json`, `output` would contain multiple newline-separated values. The `"${var[@]}"` expansion would pass them as a single argument with embedded newlines. `kubectl wait` would interpret this as one resource name containing a newline, which would fail.

The original code (replaced in commit `e9fa971`) properly handled multiple entries with a `while IFS= read -r` loop. Git blame confirms this was introduced by this PR at lines 32 and 55.

Fix: Either restore the loop or properly declare arrays.

```diff
-    containers=${output}
-
-    rdd ctl wait --for=create --namespace="${NAMESPACE}" container "${containers[@]}" --timeout=30s
+    readarray -t containers <<<"${output}"
+    rdd ctl wait --for=create --namespace="${NAMESPACE}" container "${containers[@]}" --timeout=30s
```

Same fix for volumes at line 53-55.

#### Suggestions

1. **`assert_line` in images test is now a no-op** — `containers-mock.bats:43-44` (suggestion, regression)

```bash
rdd ctl wait --for=create --namespace="${NAMESPACE}" image --field-selector "status.repoTag=${image}" --timeout=30s --output=jsonpath='{.items[*].status.repoTag}'
assert_line "${image}"
```

The old `try` helper used `run_e` internally (`utils.bash:32`), which called BATS's `run --separate-stderr`, updating `$output` to the actual kubectl response. The new code calls `rdd ctl wait` directly (not via `run`), so `$output` is still the jq output from `run -0 jq_output '.[].RepoTags.[]'` at line 39. Since `$image` comes from iterating over that same jq output, `assert_line "${image}"` is true by construction. The `--output=jsonpath=...` output goes to stdout but is never captured or verified.

The test is still valid — `--for=create` with `--field-selector "status.repoTag=${image}"` is a strong assertion that the image exists with the correct repoTag. The `assert_line` is just redundant.

Fix: Either remove the `assert_line` and `--output` flag, or wrap the wait in `run -0` to actually verify the API response.

### Design Observations

**Concerns**

- **Startup race between namespace creation and resource creation** (in-scope) — Previously, resources lived in `default` (always exists). Now they're in `rancher-desktop`, created by the `kubeNamespaceReconciler`. All mock reconcilers trigger concurrently on the `rdd-mocks` creation event. If container/image/volume reconcilers run before `kubeNamespaceReconciler` completes, their Apply calls will fail with a namespace-not-found error. Controller-runtime requeues them, and the next attempt succeeds — this is self-healing. But it produces transient error logs at startup that could be confusing during debugging. One possible mitigation: create the `rancher-desktop` namespace directly in `createNamespace()` (alongside `rdd-mocks`) at `mock_controller.go:119`, eliminating the race entirely.

**Strengths**

- The `getVolumeName` abstraction at `volume_reconciler.go:42-44` cleanly separates the volume→namespace mapping from both the volume reconciler and the namespace reconciler. When volume namespacing evolves, only this function needs to change.
- Using `errors.Join` for error aggregation across the namespace loop (`container_namespace_reconciler.go:80`) is the right approach — one failed namespace doesn't block the others.

### Testing Assessment

Test coverage exists for all the mock resource types (containers, images, volumes). Untested scenarios ranked by risk:

1. **Multiple test data entries** — The `"${var[@]}"` syntax will break. Currently masked by single-entry test data.
2. **Deletion and re-creation of `rancher-desktop` namespace** — The `kubeNamespaceReconciler` won't self-heal (dead `Watches`). Not covered and not expected to be covered for a mock controller.

### Documentation Assessment

No documentation gaps. The constants `apiNamespace` and `containerNamespace` at `mock_controller.go:26-28` have clear comments distinguishing the Kubernetes namespace from the container namespace.

### Commit Structure

Commit `4c931b2` ("API: containers: make pid optional") is an independent API change unrelated to the namespace work in the other two commits. It would be cleaner in a separate PR, but it's small enough that bundling isn't a practical concern. The namespace commits (`c75bc1a` then `e9fa971`) build on each other logically.

### Unresolved Feedback

The PR author's comment ([link](https://github.com/rancher-sandbox/rancher-desktop-daemon/pull/188#issuecomment-3948232496)) states resources should use `rancher-desktop` instead of `default`. This is addressed by commit `e9fa971`.

### Coverage Summary

| File | Status |
|------|--------|
| `bats/tests/34-containers-controllers/containers-mock.bats` | **Finding 2**, **Suggestion 1** |
| `pkg/apis/containers/v1alpha1/container_types.go` | Reviewed, no issues |
| `pkg/controllers/containers/container/crd.yaml` | Reviewed, no issues — consistent with type change |
| `pkg/controllers/mock/container_namespace_reconciler.go` | Reviewed, no issues |
| `pkg/controllers/mock/container_reconciler.go` | Reviewed, no issues — `apiNamespace` swap is straightforward |
| `pkg/controllers/mock/image_reconciler.go` | Reviewed, no issues — `apiNamespace` swap is straightforward |
| `pkg/controllers/mock/kube_namespace_reconciler.go` | **Finding 1** |
| `pkg/controllers/mock/mock_controller.go` | Reviewed, no issues |
| `pkg/controllers/mock/volume_reconciler.go` | Reviewed, no issues |

### Codex GPT 5.4

**Verdict**: Changes requested

### Executive Summary
This PR moves the mock containers API objects into the `rancher-desktop` Kubernetes namespace and adds a dedicated reconciler to create that namespace. The main regression is that the new namespace reconciler does not actually watch the owned `rancher-desktop` namespace after creation, so deleting that namespace leaves the mock controllers stuck retrying forever; the new BATS shortcuts also stop validating multiple containers or volumes correctly.

### Findings

**Critical Issues**  
None.

**Important Issues**

1. **`rancher-desktop` is not recreated after deletion** — [pkg/controllers/mock/kube_namespace_reconciler.go:63-80](/Users/jan/git/rancher-desktop-daemon/pkg/controllers/mock/kube_namespace_reconciler.go#L63) (important, regression)

```go
return ctrl.NewControllerManagedBy(mgr).
    For(&corev1.Namespace{}).
    Named("mock-kube-namespace-reconciler").
    Watches(
        &corev1.Namespace{},
        handler.EnqueueRequestForOwner(...)).
    WithEventFilter(predicate.NewPredicateFuncs(func(object client.Object) bool {
        if _, ok := object.(*corev1.Namespace); ok {
            return object.GetName() == mockNamespaceName
```

At [pkg/controllers/mock/kube_namespace_reconciler.go:49-60](/Users/jan/git/rancher-desktop-daemon/pkg/controllers/mock/kube_namespace_reconciler.go#L49) the new reconciler creates `rancher-desktop` and then returns `ctrl.Result{}, nil`, so future repairs depend on watch events. But the global predicate at [pkg/controllers/mock/kube_namespace_reconciler.go:75-80](/Users/jan/git/rancher-desktop-daemon/pkg/controllers/mock/kube_namespace_reconciler.go#L75) only admits `Namespace` events whose name is `rdd-mocks`; controller-runtime applies that predicate to all watched objects, so delete/update events for the owned `rancher-desktop` namespace never requeue the owner. If `rancher-desktop` is deleted, the builtin namespace controller removes everything in it at [pkg/controllers/builtin/namespace/controllers/namespace_controller.go:51-121](/Users/jan/git/rancher-desktop-daemon/pkg/controllers/builtin/namespace/controllers/namespace_controller.go#L51) and [pkg/controllers/builtin/namespace/controllers/namespace_controller.go:126-153](/Users/jan/git/rancher-desktop-daemon/pkg/controllers/builtin/namespace/controllers/namespace_controller.go#L126); the mock data reconcilers then keep retrying writes into the missing namespace at [pkg/controllers/mock/container_reconciler.go:81-114](/Users/jan/git/rancher-desktop-daemon/pkg/controllers/mock/container_reconciler.go#L81), [pkg/controllers/mock/image_reconciler.go:112-159](/Users/jan/git/rancher-desktop-daemon/pkg/controllers/mock/image_reconciler.go#L112), and [pkg/controllers/mock/volume_reconciler.go:79-110](/Users/jan/git/rancher-desktop-daemon/pkg/controllers/mock/volume_reconciler.go#L79), but nothing recreates the namespace until restart or another `rdd-mocks` event.

Fix: do not use a global predicate that filters out the owned namespace. Either add per-watch predicates so the owner watch accepts both `rdd-mocks` and `rancher-desktop`, or explicitly requeue until [pkg/controllers/mock/kube_namespace_reconciler.go:49-55](/Users/jan/git/rancher-desktop-daemon/pkg/controllers/mock/kube_namespace_reconciler.go#L49) can confirm the namespace exists.

**Suggestions**

1. **The new BATS shortcuts collapse multiple names into one argument** — [bats/tests/34-containers-controllers/containers-mock.bats:28-32](/Users/jan/git/rancher-desktop-daemon/bats/tests/34-containers-controllers/containers-mock.bats#L28) (suggestion, regression)

```bash
run -0 jq_output '.[].Id'
containers=${output}

rdd ctl wait --for=create --namespace="${NAMESPACE}" container "${containers[@]}" --timeout=30s
```

The same pattern appears at [bats/tests/34-containers-controllers/containers-mock.bats:51-55](/Users/jan/git/rancher-desktop-daemon/bats/tests/34-containers-controllers/containers-mock.bats#L51). `containers` and `volumes` are scalars, not arrays, so quoted `"${containers[@]}"` / `"${volumes[@]}"` expands to one multiline argument in bash. With the current fixture this still passes because there is only one container and one volume, but the test no longer proves that waiting for multiple resources works, which is exactly what this PR is trying to strengthen.

Fix: keep the per-line loop, or populate a real array first, e.g. `mapfile -t containers < <(jq -r '.[].Id' ...)` and then pass `"${containers[@]}"`.

### Design Observations
**Concerns**
- A separate `kubeNamespaceReconciler` adds a second namespace-lifecycle path next to [pkg/controllers/mock/mock_controller.go:68-105](/Users/jan/git/rancher-desktop-daemon/pkg/controllers/mock/mock_controller.go#L68). Folding `rancher-desktop` bootstrap into the existing startup/bootstrap flow, or sharing a helper with the data reconcilers, would remove the watch/predicate coupling that caused Finding 1. `(future)`

**Strengths**
- Moving mock `Container`, `Image`, `Volume`, and `ContainerNamespace` objects into `rancher-desktop` matches the intended containers API direction better than the previous `default` placement. `(in-scope)`

### Testing Assessment
1. There is no coverage for deleting `namespace/rancher-desktop` and verifying that the mock controller recreates it and repopulates child objects.
2. The updated BATS file no longer exercises multiple container or volume names because the new scalar expansion only passes one argument.

### Documentation Assessment
`docs/design/api_containers.md` still documents `Container` and `Volume` objects in `default` at [docs/design/api_containers.md:55](/Users/jan/git/rancher-desktop-daemon/docs/design/api_containers.md#L55), [docs/design/api_containers.md:279](/Users/jan/git/rancher-desktop-daemon/docs/design/api_containers.md#L279), and [docs/design/api_containers.md:300](/Users/jan/git/rancher-desktop-daemon/docs/design/api_containers.md#L300). This should be updated if `rancher-desktop` is now the intended namespace.

### Commit Structure
`4c931b2` (`API: containers: make pid optional`) is a separate API-schema change from the mock namespace work. It is self-contained, but it broadens the PR scope and would be easier to review as a separate PR or a clearly motivated dependency commit.

### Coverage Summary
- `bats/tests/34-containers-controllers/containers-mock.bats` — Finding 1
- `pkg/apis/containers/v1alpha1/container_types.go` — Reviewed, no issues
- `pkg/controllers/containers/container/crd.yaml` — Reviewed, no issues
- `pkg/controllers/mock/container_namespace_reconciler.go` — Reviewed, no issues
- `pkg/controllers/mock/container_reconciler.go` — Reviewed, no issues
- `pkg/controllers/mock/image_reconciler.go` — Reviewed, no issues
- `pkg/controllers/mock/kube_namespace_reconciler.go` — Finding 1
- `pkg/controllers/mock/mock_controller.go` — Reviewed, no issues
- `pkg/controllers/mock/volume_reconciler.go` — Reviewed, no issues

### Gemini 3.1 Pro

Failed with HTTP 429 (MODEL_CAPACITY_EXHAUSTED). No review produced.