Deep Review: 20260412-031335-pr-295

		events.ActionUnPause,
		events.ActionRestart:
		log.V(1).Info("Container event", "action", msg.Action, "id", msg.Actor.ID)
		return w.syncContainer(ctx, msg.Actor.ID)

	case events.ActionDestroy:
		log.V(1).Info("Container destroyed", "id", msg.Actor.ID)
		return w.removeMirrorResource(ctx, &containersv1alpha1.Container{}, msg.Actor.ID)

	default:
		return nil
	}
}

All three reviewers caught this. When Docker emits an untag event, msg.Actor.ID is the image ID hash, not the tag. removeImageByTag searches for status.repoTag == <hash> at pkg/controllers/app/engine/controllers/sync_images.go:185, which never matches. The per-tag Image mirror survives indefinitely, until a watcher restart runs fullSync again.

// reconcileImageByID re-inspects an image and reconciles every K8s Image
// mirror whose status.id matches. Tags still present are re-applied; K8s
// resources for tags that are no longer present are deleted. If the
// image has been fully removed, all mirrors with that status.id are
// deleted instead.
//
// This is the path for events that carry an image ID but not the tag
// name — notably Docker's untag events, where the event payload only
// contains the image ID (see handleImageEvent).
func (w *dockerWatcher) reconcileImageByID(ctx context.Context, id string) error {
	result, err := w.cli.ImageInspect(ctx, id)

I verified the upstream Moby contract: daemon/images/image_delete.go:134,191,272 all call LogImageEvent(ctx, imgID.String(), imgID.String(), events.ActionUnTag), so both Actor.ID and Attributes["name"] are the image ID hash — the tag that was removed is not propagated through the event payload at all. Gemini proposed fixing this by reading msg.Actor.Attributes["name"], which is also wrong for this reason.

Fix (Codex's proposal): treat untag as a per-image reconciliation, not a direct tag delete. Re-inspect the image by ID, then delete any K8s Image objects whose status.id matches but whose status.repoTag is no longer in the fresh RepoTags. If the inspect returns NotFound, fall back to removeImagesByID.

 case events.ActionUnTag:
-    return w.removeImageByTag(ctx, msg.Actor.ID)
+    return w.syncImageAndPruneStaleTags(ctx, msg.Actor.ID)

// iteration starts from a fresh copy via DeepCopyObject and writes only
// that copy, so caller state is never observed.
func (w *dockerWatcher) removeMirrorResource(ctx context.Context, obj client.Object, name string) error {
	key := client.ObjectKey{Name: name, Namespace: apiNamespace}
	retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error {
		latest := obj.DeepCopyObject().(client.Object)
		if err := w.k8s.Get(ctx, key, latest); err != nil {
			if apierrors.IsNotFound(err) {
				return nil
			}
			return err
		}
		if !removeFinalizer(latest, mirrorFinalizer) {
			return nil
		}
		return w.k8s.Update(ctx, latest)
	})
	if retryErr != nil {
		return fmt.Errorf("failed to remove finalizer from %s: %w", name, retryErr)
	}

The comment at docker_watcher.go:268-271 carefully explains that ContainerStop handles paused/restarting containers natively, and the BATS test patching spec.state=created stops a paused container at bats/tests/32-app-controllers/engine.bats:200-220 covers that direction. The symmetric case (desired=running, actual=paused) falls into ContainerStart, which Docker rejects for paused containers with "container is paused, unpause before starting". The user's intent is silently dropped and there is no test for this direction.

//
// The obj parameter is used as an empty object template: callers may
// pass either a zero-valued struct (e.g. &containersv1alpha1.Volume{})
// or a slice element from a prior List; in both cases each retry
// iteration starts from a fresh copy via DeepCopyObject and writes only
// that copy, so caller state is never observed.
func (w *dockerWatcher) removeMirrorResource(ctx context.Context, obj client.Object, name string) error {
	key := client.ObjectKey{Name: name, Namespace: apiNamespace}
	retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error {
		latest := obj.DeepCopyObject().(client.Object)
		if err := w.k8s.Get(ctx, key, latest); err != nil {
			if apierrors.IsNotFound(err) {
				return nil
			}

    run -0 docker inspect test-state --format '{{.Id}}'
    cid=${output}

    rdd ctl wait --for=jsonpath='{.status.status}'=running \
        --namespace="${NAMESPACE}" container/"${cid}" --timeout=30s

    rdd ctl patch container "${cid}" --namespace="${NAMESPACE}" \
        --type=merge -p '{"spec":{"state":"created"}}'

    rdd ctl wait --for=jsonpath='{.status.status}'=exited \
        --namespace="${NAMESPACE}" container/"${cid}" --timeout=30s

    run -0 docker inspect test-state --format '{{.State.Status}}'
    assert_output "exited"
}

@test "patching spec.state=running restarts Docker container" {
    run -0 docker inspect test-state --format '{{.Id}}'
    cid=${output}

    rdd ctl patch container "${cid}" --namespace="${NAMESPACE}" \
        --type=merge -p '{"spec":{"state":"running"}}'

    rdd ctl wait --for=jsonpath='{.status.status}'=running \
        --namespace="${NAMESPACE}" container/"${cid}" --timeout=30s

    run -0 docker inspect test-state --format '{{.State.Status}}'
    assert_output "running"
}

@test "patching spec.state=created stops a paused container" {

Fix: dispatch ContainerUnpause when actual == paused and desired == running, and add a BATS test mirroring the existing stop-paused test.

 switch desired {
 case containersv1alpha1.ContainerStatusRunning:
+    if actual == containersv1alpha1.ContainerStatusPaused {
+        log.Info("Unpausing container", "id", c.Name)
+        _, err := w.cli.ContainerUnpause(ctx, c.Name, dockerclient.ContainerUnpauseOptions{})
+        return err
+    }
     log.Info("Starting container", "id", c.Name)
     _, err := w.cli.ContainerStart(ctx, c.Name, dockerclient.ContainerStartOptions{})
     return err

		k8s:           k8s,
		cancel:        watchCancel,
		done:          make(chan struct{}),
		reconcileChan: reconcileChan,
	}

	// Capture a Docker-relative timestamp before fullSync begins. The
	// run() goroutine passes this to the event stream as the Since
	// filter, so any mutation that happens while fullSync is snapshotting
	// state is replayed once the stream opens. Without this, the window
	// between the List-based snapshot and the event subscription is a
	// blind spot during VM startup.
	since := strconv.FormatInt(time.Now().Unix(), 10)

	if err := w.fullSync(watchCtx); err != nil {
		watchCancel()
		cli.Close()
		return nil, fmt.Errorf("failed to perform initial sync: %w", err)

newDockerWatcher() runs fullSync before run() subscribes to the Docker event stream at docker_watcher.go:108-110. Any container/image/volume mutation in Docker during that window is invisible — it happened after the sync snapshotted state, but before the event stream was open. This is especially plausible during VM startup where dockerd is actively accepting work as the App's Running condition flips.

	}
}

// run is the main watcher goroutine that processes Docker events. The
// since parameter is the "Since" filter passed to the Docker events
// endpoint, captured before the initial fullSync so events that
// happened during the sync window are replayed once the stream opens.
func (w *dockerWatcher) run(ctx context.Context, since string) {
	log := logf.FromContext(ctx).WithName("docker-watcher")
	defer close(w.done)
	// Recover from panics in event handling so an unexpected Docker
	// event shape cannot crash the whole app-controller process. The
	// engine reconciler notices the dead watcher via alive() on the

After startup, Reconcile() marks the engine connected and returns ctrl.Result{}, nil at engine_reconciler.go:143-146, so there is no immediate second sync to close the gap.

		return ctrl.Result{}, r.setEngineCondition(ctx, &app, terminalStatus, terminalReason, terminalMessage)
	}

	if !watcherRunning {
		log.Info("App is running, starting Docker watcher")
		if err := r.startWatcher(ctx); err != nil {
			log.Error(err, "Failed to start Docker watcher")
			_ = r.setEngineCondition(ctx, &app, metav1.ConditionFalse, "ConnectFailed", err.Error())
			return ctrl.Result{}, err
		}
		if err := r.setEngineCondition(ctx, &app, metav1.ConditionTrue, "Connected", "Container engine synced"); err != nil {
			return ctrl.Result{}, err
		}
		return ctrl.Result{}, nil

Fix: open the event stream before or in parallel with fullSync, buffer events until the sync finishes, then replay. Alternatively, record a Since timestamp before fullSync starts and subscribe to the event stream with Since set to that timestamp so post-snapshot mutations are replayed after the sync.

		r.watcher = nil
		watcherRunning = false
	}
	r.watcherMu.Unlock()

	// A watcher that dies while the App is still running is treated as a
	// transient disconnect: log it, clear the watcher pointer, and fall
	// through to the normal reconcile flow. If wantWatcher is still true
	// below, the reconciler starts a fresh watcher whose fullSync
	// reconciles any drift in place — downstream clients see no churn
	// because the existing mirror resources keep their identity. An
	// actual stop or backend change is handled by the !wantWatcher
	// branch below, which does sweep the mirrors.
	if watcherDied {
		log.Info("Docker watcher died, will attempt to reconnect")
	}

	// The watcher should only run when the App is Running with the moby
	// backend. In every other state (stopped, containerd, both) we stop
	// the watcher and sweep mirror resources. The sweep is gated on the
	// current ContainerEngineReady reason: once the condition reflects
	// the terminal state, cleanup would be a no-op against an empty
	// namespace, so we short-circuit to avoid four empty List calls per

Any transient error on the Docker event stream (docker_watcher.go:117-123 — e.g. user ran systemctl restart docker inside the guest) causes run() to return, flips the watcher to !alive on the next reconcile, and then cleanupMirrorResources deletes every Container/Image/Volume/ContainerNamespace in rancher-desktop. The subsequent reconcile restarts the watcher and fullSync recreates them, but every downstream client (front end, rdd ctl get, anything watching) sees the whole universe blink DELETED → ADDED with fresh UIDs. A single Docker blip is as disruptive as an intentional VM shutdown.

	defer close(w.done)
	// Recover from panics in event handling so an unexpected Docker
	// event shape cannot crash the whole app-controller process. The
	// engine reconciler notices the dead watcher via alive() on the
	// next reconcile and starts a fresh one.
	defer func() {
		if r := recover(); r != nil {
			log.Error(nil, "panic in Docker watcher goroutine", "recovered", r)
		}
	}()

	eventFilter := dockerclient.Filters{}.
		Add("type", string(events.ContainerEventType)).
		Add("type", string(events.ImageEventType)).
		Add("type", string(events.VolumeEventType))

	result := w.cli.Events(ctx, dockerclient.EventsListOptions{

Fix: split "VM stopped / engine backend changed" (sweep everything — current behavior) from "transient watcher failure" (stop the watcher, leave mirror resources in place, attempt a bounded retry; on success fullSync reconciles drift in place). Only sweep if reconnect fails after a timeout. This preserves resource identity across transient outages and avoids flooding downstream watchers.

	}

	return errors.Join(errs...)
}

// syncContainer inspects a single container and creates/updates the K8s resource.
// NotFound is treated as success: the container raced a concurrent delete
// between List and Inspect, and the stale K8s mirror will be pruned by
// syncAllContainers' remove-stale step later in the same sync.
func (w *dockerWatcher) syncContainer(ctx context.Context, id string) error {
	result, err := w.cli.ContainerInspect(ctx, id, dockerclient.ContainerInspectOptions{})
	if cerrdefs.IsNotFound(err) {
		return nil
	}
	if err != nil {
		return fmt.Errorf("failed to inspect container %s: %w", id, err)
	}

During fullSync, the controller lists all containers/images/volumes and then Inspects each by ID. If a resource is deleted in Docker between the List and Inspect calls (a routine TOCTOU), Inspect returns a NotFound error. That error propagates through syncAllContainers → fullSync → newDockerWatcher, which then tears down the watcher (watchCancel(), cli.Close()) and returns ConnectFailed. The engine reconciler then flips the App condition to ConnectFailed and waits for the next retry — over nothing more than a single concurrent docker rm.

Fix: swallow NotFound as success in syncContainer, syncImageFromSummary, and syncVolume — the stale K8s mirror will be swept in the "remove stale" step later in the sync.

 result, err := w.cli.ContainerInspect(ctx, id, dockerclient.ContainerInspectOptions{})
 if err != nil {
+    if cerrdefs.IsNotFound(err) {
+        return nil
+    }
     return fmt.Errorf("failed to inspect container %s: %w", id, err)
 }

					WithRepoTag(tag).
					WithNamespace(containerNamespace),
			); err != nil {
				errs = append(errs, err)
			}
		}
	} else {
		// Dangling image (no tags).
		name := sanitizeKubernetesObjectName(inspect.ID)
		names = append(names, name)
		if err := w.applyImage(ctx,
			containersv1alpha1apply.Image(name, apiNamespace).
				WithFinalizers(mirrorFinalizer),
			imageStatusFromInspect(inspect),
		); err != nil {
			errs = append(errs, err)
		}
	}

	return names, errors.Join(errs...)
}

// imageStatusFromInspect builds a fresh ImageStatus apply config from a
// Docker inspect response. Returning a new value per call avoids the
// aliasing trap of a shallow struct copy — slice/map WithX calls on
// one "copy" would otherwise mutate the backing memory shared with
// other callers.
func imageStatusFromInspect(inspect mobyimage.InspectResponse) *containersv1alpha1apply.ImageStatusApplyConfiguration {
	statusApply := containersv1alpha1apply.ImageStatus().
		WithID(inspect.ID).
		WithRepoDigests(inspect.RepoDigests...).
		WithArchitecture(inspect.Architecture).
		WithOS(inspect.Os).
		WithSize(inspect.Size).
		WithLabels(inspect.Config.Labels)

	if t, err := time.Parse(time.RFC3339Nano, inspect.Created); err == nil {
		statusApply.WithCreatedAt(metav1.NewTime(t))
	}
	return statusApply
}

When an image that was dangling (K8s mirror name <id>) is subsequently tagged, applyImageFromInspect creates the new tagged mirror <id>-<taghash> but never deletes the pre-existing dangling mirror. Both survive until the next fullSync at watcher restart.

This is the mirror image of I1: tags and the dangling form should transition cleanly in both directions, but neither direction is currently handled incrementally.

Fix: inside the len(inspect.RepoTags) > 0 branch, proactively remove the dangling resource:

w.removeMirrorResource(ctx, &containersv1alpha1.Image{}, sanitizeKubernetesObjectName(inspect.ID))

// the original spec.state patch) and then sit forever.
func (r *EngineReconciler) reconcileContainerSpecs(ctx context.Context) error {
	r.watcherMu.Lock()
	w := r.watcher
	r.watcherMu.Unlock()
	if w == nil {
		return nil
	}

	var containers containersv1alpha1.ContainerList
	if err := r.List(ctx, &containers, client.InNamespace(apiNamespace)); err != nil {
		return err
	}

	var errs []error
	for i := range containers.Items {
		c := &containers.Items[i]
		if c.DeletionTimestamp != nil {
			continue
		}
		if err := w.reconcileContainerState(ctx, c); err != nil {

reconcileContainerState returns raw Docker API errors, but reconcileContainerSpecs only logs them and returns success. The reconcile then falls through to return ctrl.Result{}, nil, so a failed start/stop gets exactly one attempt: the user patch that changed spec.state produced the only watch event, and no requeue is requested when Docker rejects the operation. The mismatch sits forever until some unrelated event triggers a reconcile.

Gemini and Codex both flagged this as a correctness issue (Gemini at Critical, Codex at Important); Claude flagged it as Suggestion S5 pointing out the asymmetry with the per-item errors.Join pattern used in processContainerFinalizers (engine_reconciler.go:334-356). I settled on Important — the retry never happens unless another reconcile fires, but the reconcile loop's overall safety net (watch events from later Docker activity) provides an eventual fallback.

	if err := r.processContainerFinalizers(ctx, w); err != nil {
		return err
	}
	if err := r.processImageFinalizers(ctx, w); err != nil {
		return err
	}
	return r.processVolumeFinalizers(ctx, w)
}

// processContainerFinalizers deletes the Docker-side container for every
// K8s Container pending deletion and only strips the mirror finalizer
// when the Docker delete succeeds. Per-item errors are collected so one
// stuck container does not block the rest; the reconciler retries the
// remaining stuck items on the next reconcile.
func (r *EngineReconciler) processContainerFinalizers(ctx context.Context, w *dockerWatcher) error {
	var containers containersv1alpha1.ContainerList
	if err := r.List(ctx, &containers, client.InNamespace(apiNamespace)); err != nil {
		return err
	}
	var errs []error
	for i := range containers.Items {
		c := &containers.Items[i]
		if c.DeletionTimestamp == nil || !hasFinalizer(c, mirrorFinalizer) {
			continue
		}
		if err := w.deleteContainer(ctx, c.Name); err != nil {
			errs = append(errs, fmt.Errorf("failed to delete container %s from Docker: %w", c.Name, err))
			continue
		}
		if removeFinalizer(c, mirrorFinalizer) {
			// NotFound is benign: a concurrent Docker destroy event may
			// already have stripped the finalizer and deleted the
			// mirror between our List and Update.

Fix: collect per-container errors with errors.Join and return them so controller-runtime requeues with backoff, matching the existing finalizer pattern.

+    var errs []error
     for i := range containers.Items {
         ...
         if err := w.reconcileContainerState(ctx, c); err != nil {
-            logf.FromContext(ctx).Error(err, "Failed to reconcile container state", "container", c.Name)
+            errs = append(errs, fmt.Errorf("container %s: %w", c.Name, err))
         }
     }
-    return nil
+    return errors.Join(errs...)

	// current ContainerEngineReady reason: once the condition reflects
	// the terminal state, cleanup would be a no-op against an empty
	// namespace, so we short-circuit to avoid four empty List calls per
	// unrelated reconcile. On failure, the condition stays pending and
	// the next requeue re-tries the sweep.
	wantWatcher := running && engineIsDocker
	if !wantWatcher {
		if watcherRunning {
			log.Info("Stopping Docker watcher",
				"running", running, "engine", app.Spec.ContainerEngine.Name)
			r.stopWatcher()
		}
		terminalReason := "Stopped"
		terminalStatus := metav1.ConditionFalse
		terminalMessage := "Container engine stopped"
		if running && !engineIsDocker {
			terminalReason = "NotApplicable"
			terminalStatus = metav1.ConditionTrue
			terminalMessage = "Engine mirroring is only supported with the moby backend"
		}
		current := meta.FindStatusCondition(app.Status.Conditions, conditionContainerEngineReady)
		alreadyClean := !watcherDied && current != nil && current.Reason == terminalReason && current.Status == terminalStatus
		if !alreadyClean {
			if err := r.cleanupMirrorResources(ctx); err != nil {
				log.Error(err, "Failed to clean up mirror resources")
				return ctrl.Result{}, err
			}
		}
		return ctrl.Result{}, r.setEngineCondition(ctx, &app, terminalStatus, terminalReason, terminalMessage)
	}

The comment defends unconditional cleanup ("so transient errors are retried on the next requeue"), but in the common "VM stopped" and "containerd backend" steady states every incoming App event triggers four List calls (Container/Image/Volume/ContainerNamespace), each returning empty. Since the reconciler also Watches Container/Image/Volume as trigger sources (engine_reconciler.go:421-423), any update on any of them in rancher-desktop cascades into four list calls per event.

}

// SetupWithManager sets up the controller with the Manager.
func (r *EngineReconciler) SetupWithManager(mgr ctrl.Manager) error {
	r.reconcileChan = make(chan event.GenericEvent, 1)

	// Map any container/image/volume event to a reconcile of the App singleton.
	enqueueApp := handler.EnqueueRequestsFromMapFunc(
		func(_ context.Context, _ client.Object) []reconcile.Request {
			return []reconcile.Request{{NamespacedName: types.NamespacedName{Name: appName}}}
		},
	)

Fix: gate cleanup on a state change — only sweep on transition into the not-wanted state (watcher was running, or a dedicated "CleanupPending" bit is set). If ContainerEngineReady already reflects the terminal state and observedGeneration == app.Generation, short-circuit with no List calls.

// WithSpec(unknown), the engine-controller field owner would keep
// reasserting spec.state=unknown, clobbering any user patch to
// running/created. Gating the WithSpec call behind a Get — so spec is
// written exactly once, on creation — keeps the invariant "subsequent
// syncs never touch spec" without needing a second field owner.
func (w *dockerWatcher) applyContainer(ctx context.Context, inspect mobycontainer.InspectResponse) error {
	namespace, name := parseContainerName(inspect.Name)

	// Create the resource if it doesn't exist. spec.state is always set to
	// "unknown" on creation — meaning the engine mirrors Docker state without
	// expressing intent. The user can later set it to "running" or "created"
	// to control the container; the reconciler ignores "unknown".
	//
	// Deliberately omit ForceOwnership on the create apply: if a user
	// patch to spec.state landed in the window between the Get above
	// and this Apply, ForceOwnership would silently clobber it. Without
	// the flag, the Apply succeeds on a fresh resource (no other owner
	// for spec.state yet) and fails loudly on a concurrent conflict
	// (the Get-NotFound observation was racy).
	var existing containersv1alpha1.Container
	err := w.k8s.Get(ctx, client.ObjectKey{Name: inspect.ID, Namespace: apiNamespace}, &existing)
	if apierrors.IsNotFound(err) {

The Get-then-Apply pattern (explained by the comment) is the right idea — spec must be written exactly once to avoid fighting user intent. But the Get is racy: a concurrent user patch spec.state=running that lands between the Get (returning NotFound) and the Apply is clobbered by the Apply, because ForceOwnership explicitly steals spec.state from whoever wrote it last. The window is small but real: in practice the first Docker create event fires very close to when rdd-ctl-driven user patches might arrive for pre-existing resources after a crash recovery.

Fix: drop ForceOwnership on the initial create apply, or use a typed Create call with explicit metav1.CreateOptions instead of SSA. The narrower fix is to remove client.ForceOwnership from the create path and leave it on the status-only subsequent apply.

				return true, nil
			}
		}
		return false, nil
	}

	condition := func(event watch.Event) (bool, error) {
		obj, ok := event.Object.(*unstructured.Unstructured)
		if !ok {
			return false, nil
		}
		return satisfied(obj), nil
	}

	if _, err := watchtools.UntilWithSync(ctx, lw, &unstructured.Unstructured{}, precondition, condition); err != nil {
		if errors.Is(err, context.DeadlineExceeded) {
			return errors.New("timed out waiting for App state")

When the user interrupts rdd set mid-wait with Ctrl+C, ctx is cancelled and watchtools returns context.Canceled. The CLI reports "timed out waiting for App state", which is false — the user cancelled. A user debugging why rdd set "timed out" after 2 seconds will go hunting in VM logs for the symptom instead of realizing they pressed Ctrl+C.

Fix: distinguish the two cases.

-    if errors.Is(err, context.DeadlineExceeded) || errors.Is(err, context.Canceled) {
+    if errors.Is(err, context.DeadlineExceeded) {
         return errors.New("timed out waiting for App state")
     }
+    if errors.Is(err, context.Canceled) {
+        return errors.New("wait cancelled")
+    }
     return fmt.Errorf("failed to watch App: %w", err)

//
// TODO: changes that trigger a VM restart without changing "running" (e.g.
// containerEngine.name) are not waited on. This requires a "Reconciled"
// condition with observedGeneration on the App resource so the CLI can
// detect when the full reconcile chain has settled.
//
// Known hazard until the "Reconciled" condition lands: `rdd set
// containerEngine.name=<other> running=true` on an already-running VM
// returns immediately because the stale ContainerEngineReady=True from
// the previous backend satisfies the precondition check before the
// reconciler has observed the new spec. Users who switch backends
// on a running VM should either stop first (`rdd set running=false`)
// or re-query ContainerEngineReady after the set completes to confirm
// the backend has finished transitioning.
func waitForDesiredState(ctx context.Context, config *rest.Config, properties map[string]string, timeout time.Duration) error {
	runningVal, ok := properties["running"]
	if !ok {
		return nil
	}

	ctx, cancel := context.WithTimeout(ctx, timeout)
	defer cancel()

	if runningVal == "true" {

rdd set containerEngine.name=containerd running=true on an already-running VM races with the stale cached ContainerEngineReady=True/Connected from the moby backend. The precondition check at set.go:489-495 fires on the stale state and returns immediately, leaving the user with a half-restarted VM even though rdd set succeeded.

		return fmt.Errorf("failed to create dynamic client: %w", err)
	}

	lw := &cache.ListWatch{
		ListFunc: func(options metav1.ListOptions) (runtime.Object, error) {
			options.FieldSelector = "metadata.name=app"
			return dynClient.Resource(appGVR).List(ctx, options)
		},
		WatchFunc: func(options metav1.ListOptions) (watch.Interface, error) {
			options.FieldSelector = "metadata.name=app"
			return dynClient.Resource(appGVR).Watch(ctx, options)
		},
	}

	precondition := func(store cache.Store) (bool, error) {
		for _, item := range store.List() {
			if obj, ok := item.(*unstructured.Unstructured); ok && satisfied(obj) {

The test at bats/tests/32-app-controllers/engine.bats:266-274 explicitly documents this limitation: "Stop first so there is no stale True/Connected from moby to satisfy the CLI wait below before the engine reconciler has run." That comment is the confession — the code works around a race the test harness itself can't avoid without stopping first.

    rdd ctl wait --for=jsonpath='{.status.status}'=paused \
        --namespace="${NAMESPACE}" container/"${cid}" --timeout=30s

    rdd ctl patch container "${cid}" --namespace="${NAMESPACE}" \
        --type=merge -p '{"spec":{"state":"running"}}'

    rdd ctl wait --for=jsonpath='{.status.status}'=running \
        --namespace="${NAMESPACE}" container/"${cid}" --timeout=30s

    run -0 docker inspect test-unpause --format '{{.State.Status}}'
    assert_output "running"

    docker rm --force test-unpause
}

# --- Cleanup on shutdown ---

@test "stopping VM removes all mirror resources" {
    # Make sure we have at least one resource to verify cleanup.

The author's TODO at set.go:440-443 acknowledges the root cause and proposes a future Reconciled condition with observedGeneration. Until that lands: either narrow the documented behavior ("waits for engine readiness only when running is the only property"), or skip the precondition shortcut when properties include anything besides running. At minimum, document the hazard in the TODO itself so future maintainers know it's a real user-visible issue, not just a polish item.

// container engine to be ready. If running=false was set, it waits for the
// App's Running condition to go to False — i.e. the VM has actually
// stopped, which is stricter than "container engine disconnected".
// Other property changes do not wait.
//
// TODO: changes that trigger a VM restart without changing "running" (e.g.
// containerEngine.name) are not waited on. This requires a "Reconciled"
// condition with observedGeneration on the App resource so the CLI can
// detect when the full reconcile chain has settled.
//
// Known hazard until the "Reconciled" condition lands: `rdd set
// containerEngine.name=<other> running=true` on an already-running VM
// returns immediately because the stale ContainerEngineReady=True from
// the previous backend satisfies the precondition check before the

		log.V(1).Info("Image deleted", "id", msg.Actor.ID)
		return w.removeImagesByID(ctx, msg.Actor.ID)

	default:
		return nil
	}
}

// handleVolumeEvent processes volume events.
func (w *dockerWatcher) handleVolumeEvent(ctx context.Context, msg events.Message) error {
	log := logf.FromContext(ctx).WithName("docker-watcher")

	switch msg.Action {
	case events.ActionCreate:
		log.V(1).Info("Volume created", "name", msg.Actor.ID)
		return w.syncVolume(ctx, msg.Actor.ID)

	case events.ActionDestroy:
		log.V(1).Info("Volume destroyed", "name", msg.Actor.ID)
		return w.removeMirrorResource(ctx, &containersv1alpha1.Volume{},
			volumeK8sName(msg.Actor.ID))

	default:
		return nil
	}
}

// removeMirrorResource removes the finalizer from a mirror resource and deletes
// it. This is used when Docker has already deleted the object.

	// Remove stale K8s containers.
	var k8sContainers containersv1alpha1.ContainerList
	if err := w.k8s.List(ctx, &k8sContainers, client.InNamespace(apiNamespace)); err != nil {
		return fmt.Errorf("failed to list K8s containers: %w", err)
	}
	for i := range k8sContainers.Items {
		c := &k8sContainers.Items[i]
		if !dockerIDs[c.Name] {
			log.V(1).Info("Removing stale container", "id", c.Name)
			if err := w.removeMirrorResource(ctx, c, c.Name); err != nil {
				errs = append(errs, err)
			}
		}
	}

	return errors.Join(errs...)

		return err
	}

	var errs []error
	for _, item := range items {
		obj := item.(client.Object)
		key := client.ObjectKeyFromObject(obj)
		kind := obj.GetObjectKind().GroupVersionKind().Kind

		retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error {
			latest := obj.DeepCopyObject().(client.Object)

// (e.g., ~/.rd2/docker.sock). This is the host-side socket that Lima
// port-forwards from the guest's /var/run/docker.sock.
//
// TODO: use ShortDir() once the Lima template derives the socket path
// from the instance suffix instead of hardcoding ".rd2". The two
// hardcodes (here and in pkg/controllers/app/app/lima-template.yaml)
// must move together — either both parameterized on RDD_INSTANCE or
// neither — or the watcher will open a different socket from the one
// Lima is port-forwarding to, and mirroring will silently diverge per
// instance. Until then, multi-instance BATS runs cannot share the
// same host and the engine.bats test has a parallel TODO at line
// 12-14.
var DockerSocket = sync.OnceValue(func() string {
	home, err := os.UserHomeDir()
	if err != nil {
		panic(fmt.Errorf("could not get home directory: %w", err))
	}
	dir := filepath.Join(home, ".rd2")
	_ = os.MkdirAll(dir, 0o700)

  - 0
  guestSocket: /var/run/docker.sock
  hostPortRange:
  - 0
  - 0
  hostSocket: "{{.Home}}/.rd2/docker.sock"
provision:
- mode: system
  script: |
    #!/bin/bash

// DockerSocket returns the path to the Docker socket for this instance
// (e.g., ~/.rd2/docker.sock). This is the host-side socket that Lima
// port-forwards from the guest's /var/run/docker.sock.
//
// TODO: use ShortDir() once the Lima template derives the socket path
// from the instance suffix instead of hardcoding ".rd2". The two
// hardcodes (here and in pkg/controllers/app/app/lima-template.yaml)
// must move together — either both parameterized on RDD_INSTANCE or
// neither — or the watcher will open a different socket from the one
// Lima is port-forwarding to, and mirroring will silently diverge per
// instance. Until then, multi-instance BATS runs cannot share the

# Engine controller tests — verify that the engine controller mirrors Docker
# containers, images, and volumes into K8s resources, and that K8s deletions
# and spec.state changes are forwarded to Docker.

NAMESPACE="rancher-desktop"
# TODO: use .rd${RDD_INSTANCE} once the Lima template derives the socket path
# from the instance suffix instead of hardcoding ".rd2".
DOCKER_HOST="unix://${HOME}/.rd2/docker.sock"
export DOCKER_HOST

local_setup_file() {
    # The Docker socket access pattern used by these tests is not yet wired
    # up for Windows/WSL2.

		name := sanitizeKubernetesObjectName(inspect.ID)
		names = append(names, name)
		if err := w.applyImage(ctx,
			containersv1alpha1apply.Image(name, apiNamespace).
				WithFinalizers(mirrorFinalizer),
			imageStatusFromInspect(inspect),
		); err != nil {
			errs = append(errs, err)
		}
	}

	return names, errors.Join(errs...)
}

// imageStatusFromInspect builds a fresh ImageStatus apply config from a
// Docker inspect response. Returning a new value per call avoids the
// aliasing trap of a shallow struct copy — slice/map WithX calls on
// one "copy" would otherwise mutate the backing memory shared with
// other callers.
func imageStatusFromInspect(inspect mobyimage.InspectResponse) *containersv1alpha1apply.ImageStatusApplyConfiguration {
	statusApply := containersv1alpha1apply.ImageStatus().

	// Start watcher goroutine to track hostagent lifecycle and reap the process.
	// The watcher enqueues reconciles as phase transitions occur.
	r.startWatcher(ctx, limaVM.Name, limaVM.Namespace, haCmd, inst.Dir, begin)

	// On Unix, SetGroup() sets Setpgid=true which makes the hostagent a
	// new process group leader, so pgid == pid. The "pgid" log field
	// reflects that Unix-only semantics and is consumed by
	// bats-with-timeout.sh for post-mortem leak detection (attributing
	// orphaned qemu grandchildren back to their hostagent ancestor).
	// On Windows, SetGroup() sets CREATE_NEW_PROCESS_GROUP instead —
	// a different console-control-event concept — and this log field
	// will need to be renamed or recalculated when Windows CI arrives.
	logger.Info("Hostagent started, watcher active",
		"instance", limaVM.Name,
		"pid", haCmd.Process.Pid,
		"pgid", haCmd.Process.Pid)

# --- K8s deletion removes Docker object ---

@test "deleting Container resource removes Docker container" {
    docker run -d --name test-delete busybox sleep 3600

    run -0 docker inspect test-delete --format '{{.Id}}'
    cid=${output}

    rdd ctl wait --for=create --namespace="${NAMESPACE}" \
        container/"${cid}" --timeout=30s

 run -0 rdd ctl get image --namespace="${NAMESPACE}" \
     --field-selector "status.repoTag=busybox:latest" -o name
+refute_output ""
 image_ref=${output}
-[ -n "${image_ref}" ]

	var containers containersv1alpha1.ContainerList
	if err := r.List(ctx, &containers, client.InNamespace(apiNamespace)); err != nil {
		return err
	}
	var errs []error
	for i := range containers.Items {
		c := &containers.Items[i]
		if c.DeletionTimestamp == nil || !hasFinalizer(c, mirrorFinalizer) {
			continue
		}
		if err := w.deleteContainer(ctx, c.Name); err != nil {
			errs = append(errs, fmt.Errorf("failed to delete container %s from Docker: %w", c.Name, err))
			continue
		}
		if removeFinalizer(c, mirrorFinalizer) {

## Engine Mirroring

The engine controller (`pkg/controllers/app/engine/`) watches the `App` resource
for the `Running` condition.  When the VM is running with the `moby` backend,
the controller:

1. Connects to the Docker engine via the host socket.
2. Creates the `rancher-desktop` K8s namespace and the `moby`
   `ContainerNamespace` resource.
3. Lists all containers, images, and volumes and creates corresponding K8s
   resources.
4. Watches the Docker event stream for create, update, and delete events.

Containerd mirroring is not implemented yet; with `containerEngine.name=containerd`

	// current ContainerEngineReady reason: once the condition reflects
	// the terminal state, cleanup would be a no-op against an empty
	// namespace, so we short-circuit to avoid four empty List calls per
	// unrelated reconcile. On failure, the condition stays pending and
	// the next requeue re-tries the sweep.
	wantWatcher := running && engineIsDocker
	if !wantWatcher {
		if watcherRunning {
			log.Info("Stopping Docker watcher",
				"running", running, "engine", app.Spec.ContainerEngine.Name)
			r.stopWatcher()
		}
		terminalReason := "Stopped"
		terminalStatus := metav1.ConditionFalse
		terminalMessage := "Container engine stopped"
		if running && !engineIsDocker {
			terminalReason = "NotApplicable"
			terminalStatus = metav1.ConditionTrue
			terminalMessage = "Engine mirroring is only supported with the moby backend"
		}
		current := meta.FindStatusCondition(app.Status.Conditions, conditionContainerEngineReady)
		alreadyClean := !watcherDied && current != nil && current.Reason == terminalReason && current.Status == terminalStatus
		if !alreadyClean {
			if err := r.cleanupMirrorResources(ctx); err != nil {
				log.Error(err, "Failed to clean up mirror resources")
				return ctrl.Result{}, err
			}
		}
		return ctrl.Result{}, r.setEngineCondition(ctx, &app, terminalStatus, terminalReason, terminalMessage)

-2. Creates the `rancher-desktop` K8s namespace and the appropriate
-   `ContainerNamespace` resource (`moby` for dockerd, engine-specific
-   namespaces for containerd).
+2. Creates the `rancher-desktop` K8s namespace and the `moby`
+   `ContainerNamespace` resource when the App uses the moby backend.
+   Containerd mirroring is not implemented yet; the controller reports
+   `ContainerEngineReady` with reason `NotApplicable` in that mode.

Delete the Image object; the finalizer will untag the image. If all tags of an
image are removed, _and_ it is not in use by a container (running or not), then
the image itself is removed.