Deep Review: 20260417-165817-pr-323

		http.Error(w, "Invalid container ID", http.StatusBadRequest)
		return
	}

	// TODO: Figure out containerd
	reader, err := r.watcher.cli.ContainerLogs(
		req.Context(),
		containerID,
		client.ContainerLogsOptions{
			ShowStdout: true,
			ShowStderr: true,
			Follow:     true,
		},
	)
	if err != nil {
		switch {
		case errdefs.IsNotFound(err):
			log.V(5).Info("Container not found", "container", containerID)
			http.Error(w, "Container not found", http.StatusNotFound)
		case errdefs.IsInvalidArgument(err):
			log.V(5).Info("Invalid argument", "error", err)
			http.Error(w, "Invalid argument", http.StatusBadRequest)
		default:
			log.V(5).Info("Failed to get container logs", "error", err)
			http.Error(w, "Failed to get container logs", http.StatusInternalServerError)
		}
		return
	}

	upgrader := websocket.Upgrader{}
	conn, err := upgrader.Upgrade(w, req, nil)
	if err != nil {
		log.V(5).Info("Failed to upgrade to WebSocket", "error", err)
		return
	}
	defer conn.Close()
	defer func() {
		message := websocket.FormatCloseMessage(websocket.CloseNormalClosure, "Closing connection")
		err := conn.WriteControl(websocket.CloseMessage, message, time.Now().Add(time.Second))
		if err != nil {
			log.V(5).Info("Failed to close WebSocket", "error", err)
		}
	}()

	writer, err := conn.NextWriter(websocket.TextMessage)
	if err != nil {
		log.V(5).Info("Failed to get next writer", "error", err)
		return
	}
	defer writer.Close()

	_, err = stdcopy.StdCopy(writer, writer, reader)
	if err != nil {
		log.V(5).Info("Failed to copy container logs", "error", err)
		// At this point we already sent the data, so we can't send HTTP status.
	}
}

The moby client's ContainerLogs contract states the caller must close the returned io.ReadCloser, but HandleLogs never calls reader.Close() — not on the Upgrade-failure path (lines 61-64) and not on the normal-return path. Once upgrader.Upgrade hijacks the connection, http.Server no longer cancels req.Context() when the remote TCP connection drops, so a client that drops the socket does not cancel the Docker request either, and the response body stays open until the Docker daemon evicts it.

Gorilla WebSocket additionally requires the server to run a concurrent conn.ReadMessage() loop to observe client-sent close frames. HandleLogs only writes, so on a clean client close frame StdCopy blocks forever on reader.Read for an idle container — a permanent goroutine + Docker-connection leak per request.

Fix: derive a cancellable context, defer reader.Close() right after the error check, and run a reader goroutine to convert WebSocket closure into context cancellation.

ctx, cancel := context.WithCancel(req.Context())
defer cancel()

reader, err := watcher.cli.ContainerLogs(ctx, ...)
if err != nil { ... return }
defer reader.Close()

conn, err := upgrader.Upgrade(w, req, nil)
if err != nil { ... return }
defer conn.Close()

go func() {
    defer cancel()
    for {
        if _, _, err := conn.ReadMessage(); err != nil {
            return
        }
    }
}()

		},
	)
	if err != nil {
		switch {
		case errdefs.IsNotFound(err):
			log.V(5).Info("Container not found", "container", containerID)
			http.Error(w, "Container not found", http.StatusNotFound)
		case errdefs.IsInvalidArgument(err):
			log.V(5).Info("Invalid argument", "error", err)
			http.Error(w, "Invalid argument", http.StatusBadRequest)
		default:
			log.V(5).Info("Failed to get container logs", "error", err)
			http.Error(w, "Failed to get container logs", http.StatusInternalServerError)
		}
		return
	}

	upgrader := websocket.Upgrader{}
	conn, err := upgrader.Upgrade(w, req, nil)
	if err != nil {
		log.V(5).Info("Failed to upgrade to WebSocket", "error", err)
		return
	}
	defer conn.Close()
	defer func() {
		message := websocket.FormatCloseMessage(websocket.CloseNormalClosure, "Closing connection")
		err := conn.WriteControl(websocket.CloseMessage, message, time.Now().Add(time.Second))
		if err != nil {
			log.V(5).Info("Failed to close WebSocket", "error", err)
		}
	}()

	writer, err := conn.NextWriter(websocket.TextMessage)
	if err != nil {
		log.V(5).Info("Failed to get next writer", "error", err)
		return
	}
	defer writer.Close()

	_, err = stdcopy.StdCopy(writer, writer, reader)
	if err != nil {
		log.V(5).Info("Failed to copy container logs", "error", err)
		// At this point we already sent the data, so we can't send HTTP status.
	}
}

Every error branch logs at V(5), which requires -v=5/RDD_LOG_LEVEL=trace to surface. RDD runs at v=0 by default, so "container not found", Docker 5xx passthrough, WebSocket upgrade failures, and the post-upgrade copy failure are all invisible in rdd.stderr.log. The post-upgrade paths are the only channels where the client does not already see an HTTP body (once the connection is hijacked, http.Error cannot run), so a "logs stopped streaming" report has no server-side breadcrumb to investigate.

Keep V(5) for client-input validation ("Invalid container ID"), but raise the rest to log.Error / log.Info:

log.Error(err, "Failed to get container logs", "container", containerID)
http.Error(w, "Failed to get container logs", http.StatusInternalServerError)

The mock controller uses the same V(5) pattern, but it exists to not spam the test fixture; the real engine path deserves diagnosable failures.

// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: SUSE LLC
// SPDX-FileCopyrightText: The Rancher Desktop Authors

// Package engine registers the engine controller. The engine controller
// mirrors Docker container engine state (containers, images, volumes)
// into `Container`, `Image`, and `Volume` resources in the
// containers.rancherdesktop.io API group, and forwards user-initiated
// deletions back to the Docker engine.
package engine

import (
	"runtime"

	ctrl "sigs.k8s.io/controller-runtime"

	appv1alpha1 "github.com/rancher-sandbox/rancher-desktop-daemon/pkg/apis/app/v1alpha1"
	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"

Go randomises map iteration, and slices.Collect preserves that order. The result feeds EnabledPassthroughs in the discovery ConfigMap (manager.go:331-337, manager.go:346). Today the map has one key so the order is stable by accident; as soon as a second endpoint (exec, stats) is added, the serialised JSON will shuffle on every restart and produce spurious SSA updates on the discovery ConfigMap, which in turn wakes every watcher downstream. This is a latent, one-line fix now; after the second endpoint it becomes a noisy observable.

// registerDiscovery registers the controller manager information in the cluster.
func (scm *SharedControllerManager) registerDiscovery(ctx context.Context) error {
	// Get controller names, filtering out builtin controllers.
	// Builtin controllers are internal system controllers and should not be registered in discovery.
	var controllerNames []string
	passthroughEndpoints := make(map[string][]string)
	for _, registration := range scm.registrations {
		if registration.GetAPIGroup() != "builtin" {
			controllerNames = append(controllerNames, registration.GetName())
		}
		if httpController, ok := registration.(base.PassthroughController); ok {
			passthroughEndpoints[registration.GetName()] = httpController.GetPassthroughEndpoints()
		}
	}

	return scm.discovery.RegisterControllerManager(ctx, ControllerManagerInput{
		HealthPort:          scm.healthPort,
		MetricsPort:         scm.metricsPort,
		PassthroughPort:     scm.passthroughPort,
		EnabledControllers:  controllerNames,
		EnabledPassthroughs: passthroughEndpoints,
	})
}

// installControllerCRDs installs all controller CRDs in parallel for better performance.
func (scm *SharedControllerManager) installControllerCRDs(ctx context.Context) error {

func (c *controller) GetPassthroughEndpoints() []string {
    keys := slices.Collect(maps.Keys(c.passthroughs))
    slices.Sort(keys)
    return keys
}

The design doc says:

> An endpoint at /passthrough/containers/logs will speak WebSocket; messages are one way, text only, with each line being one message.

Two mismatches with the PR: (a) the controller registers under its name engine (controller.go:64-65), and the passthrough server mounts each endpoint at /{controller}/{endpoint} (manager.go:475-478), so the actual route is /passthrough/engine/logs, not /passthrough/containers/logs. Any consumer built from the doc will hit 404. (b) "one message per line" is the behaviour of the mock, not of the new Docker implementation (see C2). Once C2 is fixed, the message-shape sentence will be accurate again for TextMessage; the URL line needs updating regardless.

//go:embed crd.yaml
var appCRD string

const (
	// appValidatorWebhookName is the name used for the App validating webhook.
	appValidatorWebhookName = "app-validator.app.rancherdesktop.io"
	// appValidatorConfigName is the name of the App ValidatingWebhookConfiguration.
	appValidatorConfigName = "app-validator"
)

// controller implements the base.Controller interface for app.
type controller struct {

	for _, registration := range scm.registrations {
		if httpController, ok := registration.(base.PassthroughController); ok {
			hasPassthroughServers = true
			for _, endpoint := range httpController.GetPassthroughEndpoints() {
				handler := httpController.GetPassthroughHandler(endpoint)
				prefix := fmt.Sprintf("/%s/%s", registration.GetName(), endpoint)
				log.V(2).Info("Registering passthrough endpoint",
					"controller", registration.GetName(), "endpoint", prefix)
				mux.Handle(prefix+"/", http.StripPrefix(prefix, handler))
			}
		}
	}

	if !hasPassthroughServers {