Deep Review: 20260512-133636-pr-350

}

// waitMirrorResourcesGone polls until all four mirror resource types
// are empty in the API server's watch cache. It returns an error if
// the context is cancelled before the list drains.
func (r *EngineReconciler) waitMirrorResourcesGone(ctx context.Context) error {
	log := logf.FromContext(ctx)
	for {
		remaining, err := r.countMirrorResources(ctx)
		if err != nil {
			return fmt.Errorf("error verifying mirror resources gone: %w", err)
		}
		if remaining == 0 {
			return nil
		}
		log.V(1).Info("Waiting for mirror resources to disappear from watch cache",
			"remaining", remaining)
		select {
		case <-ctx.Done():
			return fmt.Errorf("timed out waiting for %d mirror resources to be deleted: %w",
				remaining, ctx.Err())
		case <-time.After(200 * time.Millisecond):
		}
	}
}

// countMirrorResources returns the total number of mirror resources
// still visible in the API server for all four types.
func (r *EngineReconciler) countMirrorResources(ctx context.Context) (int, error) {
	lists := []client.ObjectList{
		&containersv1alpha1.ContainerList{},
		&containersv1alpha1.VolumeList{},
		&containersv1alpha1.ImageList{},
		&containersv1alpha1.ContainerNamespaceList{},
	}
	var total int
	for _, list := range lists {
		if err := r.apiReader.List(ctx, list, client.InNamespace(r.apiNamespace)); err != nil {
			return 0, err
		}
		items, err := meta.ExtractList(list)
		if err != nil {
			return 0, err
		}
		total += len(items)
	}
	return total, nil
}

// deleteAllOfType lists and removes every resource of the given kind
// in r.apiNamespace. Engine is authoritative for Container, Image, and
// Volume in the App's namespace: this loop deletes every object, not
// just engine-owned mirrors. Coexistence with another writer to these

Two problems compound. First, pkg/service/controllers/manager.go:143-153 sets no ReconciliationTimeout, so the ctx passed in is the manager's long-lived stop context. The loop exits only when every mirror disappears or the manager shuts down. A Container/Image/Volume created between stopWatcher() and the snapshot list in deleteAllOfType, a user-added finalizer that is not mirrorFinalizer (only that one is stripped at engine_reconciler.go:593), or a storage hiccup leaves an object visible forever. Because App is a cluster-scoped singleton with at most one in-flight Reconcile, a stuck loop blocks every subsequent state transition — rdd set running=true afterwards never runs.

	if err != nil {
		return fmt.Errorf("failed to resolve passthrough port: %w", err)
	}

	// Create the shared controller-runtime manager
	managerOptions := ctrl.Options{
		Scheme: managerScheme,
		Metrics: server.Options{
			BindAddress: "127.0.0.1:" + strconv.Itoa(scm.metricsPort),
		},
		HealthProbeBindAddress: "127.0.0.1:" + strconv.Itoa(scm.healthPort),
		LeaderElection:         false, // RDD controllers are single-instance
		// Limit graceful shutdown time to avoid blocking external controller exit.
		// Default is 30s which is too long when control plane disappears.
		GracefulShutdownTimeout: ptr.To(10 * time.Second),
	}

	// Only configure webhook server if controllers require it
	if scm.hasWebhookControllers() {
		// Use instance TLS directory for webhook certificates (persistent storage)
		webhookCertDir := instance.TLSDir()

				if apierrors.IsNotFound(err) {
					return nil
				}
				return err
			}
			if !controllerutil.RemoveFinalizer(latest, mirrorFinalizer) {
				return nil
			}
			return r.Update(ctx, latest)
		})
		if retryErr != nil {

Second, the loop's stated purpose (the function comment) is to "Block until the API server's watch cache reflects the deletions". apiReader is the manager's GetAPIReader() — it bypasses every cache and issues a quorum etcd read (no resourceVersion=0). DELETE commits to etcd synchronously, so apiReader.List returns 0 immediately. The user-facing rdd ctl get reads with resourceVersion=0 from the API server's watch cache, which lags etcd by the goroutine that processes the watch event. The poll exits before the cache it claims to drain is drained; the test passes today because the other fixes in this commit (the ObservedGeneration guard on alreadyClean, plus reads via apiReader in deleteAllOfType) close the actual race.

Fix: bound the wait with context.WithTimeout, and switch countMirrorResources to the controller-runtime cache (r.List) so the loop drains a cache downstream of the API server's watch cache.

 func (r *EngineReconciler) waitMirrorResourcesGone(ctx context.Context) error {
     log := logf.FromContext(ctx)
+    ctx, cancel := context.WithTimeout(ctx, 30*time.Second)
+    defer cancel()
     for {

 func (r *EngineReconciler) countMirrorResources(ctx context.Context) (int, error) {
     ...
     for _, list := range lists {
-        if err := r.apiReader.List(ctx, list, client.InNamespace(r.apiNamespace)); err != nil {
+        if err := r.List(ctx, list, client.InNamespace(r.apiNamespace)); err != nil {
             return 0, err
         }

Note: r.List reads the controller's informer cache, which is populated only for types the controller Watches. Container, Image, and Volume are watched; ContainerNamespace is not (see I2 below), so this fix should land alongside watching ContainerNamespace or accept the small risk that one ContainerNamespace slips by on the rare delete-during-stop window.

package main

import (
	"os"

	// Import app controller packages to trigger init() functions.
	_ "github.com/rancher-sandbox/rancher-desktop-daemon/pkg/controllers/containers"
	"github.com/rancher-sandbox/rancher-desktop-daemon/pkg/external"
)

func main() {
	os.Exit(external.RunControllers("containers"))
}

// pkg/controllers/containers/controller.go
import (
    // Import controllers to register them.
    _ "github.com/rancher-sandbox/rancher-desktop-daemon/pkg/controllers/containers/container"
    _ "github.com/rancher-sandbox/rancher-desktop-daemon/pkg/controllers/containers/containernamespace"
    _ "github.com/rancher-sandbox/rancher-desktop-daemon/pkg/controllers/containers/image"
    _ "github.com/rancher-sandbox/rancher-desktop-daemon/pkg/controllers/containers/volume"
)

The engine controller registers itself in pkg/controllers/app/engine/controller.go:25-26 via init() and declares the containers API group (pkg/controllers/app/engine/controller.go:59-60). The embedded service imports it at pkg/service/cmd/service.go:59, but the standalone containers-controller aggregator imports only the four container-CRD packages. --controllers help text at pkg/service/controllers/options.go:18 advertises containers,-engine, implying engine is part of the containers group, yet bin/containers-controller silently omits it. The repo context flags exactly this shape: "a missing import in either is a silent drop with no startup error."

	containersv1alpha1 "github.com/rancher-sandbox/rancher-desktop-daemon/pkg/apis/containers/v1alpha1"
	"github.com/rancher-sandbox/rancher-desktop-daemon/pkg/controllers/app/engine/controllers"
	"github.com/rancher-sandbox/rancher-desktop-daemon/pkg/controllers/base"
)

func init() {
	base.RegisterController(newController())
}

// apiGroup is "containers" because the reconciler watches
// Container, Image, and Volume from that group and needs
// their CRDs at startup. Grouping engine with its dependencies

func (c *controller) GetName() string {
	return appv1alpha1.EngineControllerName
}

func (c *controller) GetAPIGroup() string {
	return apiGroup
}

func (c *controller) GetCRDData() string {
	return ""
}

	"github.com/rancher-sandbox/rancher-desktop-daemon/pkg/cli/help"
	// Import controller packages to trigger init() functions for embedded mode.
	_ "github.com/rancher-sandbox/rancher-desktop-daemon/pkg/controllers/app/app"
	_ "github.com/rancher-sandbox/rancher-desktop-daemon/pkg/controllers/app/demo"
	_ "github.com/rancher-sandbox/rancher-desktop-daemon/pkg/controllers/app/engine"
	"github.com/rancher-sandbox/rancher-desktop-daemon/pkg/controllers/base"
	// Import built-in system controllers.
	_ "github.com/rancher-sandbox/rancher-desktop-daemon/pkg/controllers/builtin/namespace"
	// Import controller packages to trigger init() functions for embedded mode.
	_ "github.com/rancher-sandbox/rancher-desktop-daemon/pkg/controllers/containers"

// Shared between the embedded service and any standalone binary that
// surfaces the same flag so the two cannot drift.
const ControllersFlagUsage = "Controllers to enable. Use '*' for all, " +
	"or a comma-separated list of controller or API-group names. " +
	"Groups: rdd, app, containers, lima. " +
	"Prefix a name with '-' to exclude it, e.g. '*,-demo' or 'containers,-engine'."

// Options holds the controller configuration options.
type Options struct {
	Controllers string // Controller selection specification (--controllers flag)
}

The gap pre-dates this PR (engine has always lived under pkg/controllers/app/engine while declaring group containers), but Windows support makes engine reachable on every platform now and amplifies the consequence: a user who runs bin/containers-controller standalone on Windows gets Container/Image/Volume CRDs but no mirroring.

Fix: blank-import the engine package from the containers aggregator so every consumer of controllers/containers gets the full group.

 import (
     // Import controllers to register them.
+    _ "github.com/rancher-sandbox/rancher-desktop-daemon/pkg/controllers/app/engine"
     _ "github.com/rancher-sandbox/rancher-desktop-daemon/pkg/controllers/containers/container"
     _ "github.com/rancher-sandbox/rancher-desktop-daemon/pkg/controllers/containers/containernamespace"
     _ "github.com/rancher-sandbox/rancher-desktop-daemon/pkg/controllers/containers/image"
     _ "github.com/rancher-sandbox/rancher-desktop-daemon/pkg/controllers/containers/volume"
 )

}

// probeNamedDockerContext pings the Docker endpoint for the named context
// within dockerContextProbeTimeout and returns true if the endpoint is healthy.
//
// When we cannot determine whether the context is healthy — because the store
// is unreadable, the endpoint metadata is missing or malformed, the scheme is
// not tcp/unix (e.g. SSH), or the TLS config cannot be loaded — we return true
// (assume healthy). This conservatism prevents RDD from clobbering a context
// that is in the middle of being edited or migrated.
func probeNamedDockerContext(ctx context.Context, name string) bool {
	log := logf.FromContext(ctx).WithName("docker-context").WithValues("context", name)

	s, err := newContextStore()
	if err != nil {
		log.Error(err, "Cannot open context store; assuming context healthy")
		return true
	}
	md, err := s.GetMetadata(name)
	if err != nil {
		if errdefs.IsNotFound(err) {
			return false
		}
		log.Error(err, "Cannot read context metadata; assuming context healthy")
		return true
	}
	epMeta, err := docker.EndpointFromContext(md)
	if err != nil {
		log.Error(err, "Cannot decode docker endpoint; assuming context healthy")
		return true
	}
	scheme, _, _ := strings.Cut(epMeta.Host, "://")
	switch scheme {
	case "unix", "tcp", "npipe":
	default:
		log.Info("Non-tcp/unix endpoint scheme; assuming context healthy", "scheme", scheme)
		return true
	}
	ep, err := docker.WithTLSData(s, name, epMeta)
	if err != nil {
		log.Error(err, "Cannot load TLS data; assuming context healthy")
		return true

-// not tcp/unix (e.g. SSH), or the TLS config cannot be loaded — we return true
+// not tcp/unix/npipe (e.g. SSH), or the TLS config cannot be loaded — we return true
 ...
-        log.Info("Non-tcp/unix endpoint scheme; assuming context healthy", "scheme", scheme)
+        log.Info("Unprobed endpoint scheme; assuming context healthy", "scheme", scheme)

    #
    # On Windows, rdd.exe is a native binary: it interprets /tmp/... as a
    # path from the drive root (C:\tmp\...) rather than MSYS2's /tmp which
    # maps to a different location. cygpath -m produces a mixed-format path
    # (C:/msys64/...) that both native Windows processes and MSYS2 agree on.
    if is_windows; then
        DOCKER_CONFIG="$(cygpath -m "${BATS_FILE_TMPDIR}/docker-config")"
    else
        DOCKER_CONFIG="${BATS_FILE_TMPDIR}/docker-config"
    fi
    export DOCKER_CONFIG
    mkdir -p "${DOCKER_CONFIG}"

    # Deliberately skip setup_rdd_control_plane here: `rdd set` internally
    # calls ensureServiceRunning, which is exactly the CLI path we want to

 if is_windows; then
-    DOCKER_CONFIG="$(cygpath -m "${BATS_FILE_TMPDIR}/docker-config")"
+    run -0 cygpath -m "${BATS_FILE_TMPDIR}/docker-config"
+    DOCKER_CONFIG=${output}
 else
     DOCKER_CONFIG="${BATS_FILE_TMPDIR}/docker-config"
 fi

		//     "Connected/M+1" condition — written while the VM was still
		//     running on an earlier reconcile that saw the spec change but
		//     not yet the stopped VM — would incorrectly satisfy the wait,
		//     causing `rdd set running=false` to return before mirror
		//     resources (Containers, Images, Volumes) are deleted.
		engineSettled := engineCond != nil &&
			engineCond.ObservedGeneration >= app.Generation &&
			(engineCond.Reason == "Stopped" || engineCond.Reason == "NotApplicable")
		if !engineSettled {
			settled.Status = metav1.ConditionFalse
			settled.Reason = v1alpha1.AppSettledReasonEngineStale
			settled.Message = settledMessageEngineStale
		} else {

// engine_reconciler.go
terminalReason := "Stopped"
...
if running && !engineIsDocker {
    terminalReason = "NotApplicable"
...
if err := r.setEngineCondition(ctx, &app, metav1.ConditionTrue, "Connected", "Container engine synced"); err != nil {

// length constraints.
var LimaHome = sync.OnceValue(func() string {
	return filepath.Join(ShortDir(), "lima")
})

// 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.
// On Windows, returns the named pipe path (\\.\pipe\docker_engine).
var DockerSocket = sync.OnceValue(func() string {
	if runtime.GOOS == "windows" {
		return `\\.\pipe\docker_engine`
	}
	return filepath.Join(ShortDir(), "docker.sock")
})

// DockerEndpoint returns the full Docker endpoint URL for this instance.
// On Windows this is a named-pipe URL (npipe:////./pipe/docker_engine);
// on Unix it is a unix-socket URL (unix:///path/to/docker.sock).
var DockerEndpoint = sync.OnceValue(func() string {

)

const (
	// DockerPipeName is the well-known Windows named pipe that Docker CLI
	// and Docker Desktop use to reach dockerd.
	DockerPipeName = `\\.\pipe\docker_engine`

	// VsockForwardPort is the AF_VSOCK port that rdd-guest listens on inside
	// the Lima VM.  Must match the constant in cmd/rdd-guest/main.go.
	VsockForwardPort uint32 = 6660
)

-// 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.
-// On Windows, returns the named pipe path (\\.\pipe\docker_engine).
+// DockerSocket returns the path to the Docker socket. On Unix this is the
+// per-instance host-side socket that Lima port-forwards from the guest's
+// /var/run/docker.sock (e.g., ~/.rd2/docker.sock). On Windows it is the
+// well-known named pipe \\.\pipe\docker_engine — process-global, so at
+// most one RDD instance can serve Docker on Windows at a time.

    assert_file_exists "${meta_file}"

    run -0 jq -r '.Name' "${meta_file}"
    assert_output "${context_name}"

    run -0 jq -r '.Endpoints.docker.Host' "${meta_file}"
    if is_windows; then
        assert_output "npipe:////./pipe/docker_engine"
    else
        run -0 rdd service paths docker_socket
        local socket_path=${output}
        run -0 jq -r '.Endpoints.docker.Host' "${meta_file}"
        assert_output "unix://${socket_path}"
    fi
}

@test "moby engine sets currentContext when no healthy context exists" {
    local context_name="rancher-desktop-${RDD_INSTANCE}"

-    run -0 jq -r '.Endpoints.docker.Host' "${meta_file}"
     if is_windows; then
+        run -0 jq -r '.Endpoints.docker.Host' "${meta_file}"
         assert_output "npipe:////./pipe/docker_engine"
     else
         run -0 rdd service paths docker_socket
-        local socket_path=${output}
+        socket_path=${output}
         run -0 jq -r '.Endpoints.docker.Host' "${meta_file}"
         assert_output "unix://${socket_path}"
     fi