Deep Review: 20260521-153126-pr-361

- **status.kubernetesPort**: The host TCP port allocated for the k3s API server (`7441 + instance.Index()` by default). Set by the App reconciler on the first reconcile after `spec.kubernetes.enabled` becomes `true`, and cleared when `spec.kubernetes.enabled` is set back to `false` so that a fresh port is resolved on the next enable. The `KUBERNETES_PORT` Lima template param is set to this value; Lima's identity port-forward rule binds the same port on the host and forwards it to the guest.

- **status.conditions**: Multiple controllers write here. The App controller mirrors `Created` and `Running` from the owned `LimaVM`, computes `Settled`, and the engine controller writes `ContainerEngineReady`. All writers use `retry.RetryOnConflict` with a re-Get so concurrent status updates do not 409.

  | Type                   | Status    | Reason           | Description                                                       |
  |------------------------|-----------|------------------|-------------------------------------------------------------------|
  | `Created`              | `Unknown` | `Pending`        | LimaVM controller has started reconciliation                      |
  | `Created`              | `True`    | `Created`        | Lima instance created on disk and ready                           |
  | `Created`              | `False`   | `CreateFailed`   | Lima instance creation failed (see `message` for details)         |
  | `Running`              | `Unknown` | `Reconciling`    | Verifying instance state (e.g. after controller restart)          |
  | `Running`              | `True`    | `Started`        | Lima instance is running                                          |
  | `Running`              | `False`   | `Stopped`        | Lima instance is stopped                                          |
  | `Running`              | `False`   | `Starting`       | Lima instance is starting up                                      |
  | `Running`              | `False`   | `StartFailed`    | Lima instance failed to start                                     |
  | `Running`              | `False`   | `StopFailed`     | Lima instance failed to stop cleanly                              |
  | `ContainerEngineReady` | `True`    | `Connected`      | Engine controller has connected to Docker and completed full sync |
  | `ContainerEngineReady` | `True`    | `NotApplicable`  | Mirroring is not implemented for the current backend (e.g. `containerd`); forced `True` so `rdd set` can finish waiting |
  | `ContainerEngineReady` | `False`   | `ConnectFailed`  | Engine controller failed to connect to Docker                     |
  | `ContainerEngineReady` | `False`   | `Stopped`        | The VM is stopped; the engine watcher is not running              |
  | `Settled`              | `True`    | `Settled`        | Reconcile chain has caught up with the current spec               |
  | `Settled`              | `False`   | `WaitingForLimaVM` | The App has no `Running` condition yet (nothing mirrored from LimaVM) |
  | `Settled`              | `False`   | `WaitingForEngine` | Engine controller has not yet written `ContainerEngineReady`    |
  | `Settled`              | `False`   | `EngineStale`    | Engine controller has not yet observed the current generation     |
  | `Settled`              | `False`   | *(forwarded)*    | Forwarded from the blocking `Running` or `ContainerEngineReady` reason |

  `Running=True` means the Lima guest has finished booting and SSH is reachable. It says nothing about the container engine socket; consumers that depend on the engine (container/image/volume mirrors, `docker` against the forwarded socket) must also check `ContainerEngineReady`, which flips to `True` only after the engine controller has connected to the socket and completed its initial full sync.

  The `Created` and `Running` conditions are mirrored from LimaVM, so their `lastTransitionTime` reflects the LimaVM transition rather than the copy — the timestamp is meaningful for staleness checks. The engine reconciler stamps `ContainerEngineReady.observedGeneration` with the App's `metadata.generation` at the time it writes the condition; if the App's generation advances between the reconciler's read and write, the stamp reflects the write-time generation rather than the observed one. The App reconciler computes `Settled` from the mirrored `Running` and the engine-written `ContainerEngineReady`, and stamps its own `observedGeneration` with the `metadata.generation` observed when it computed the condition, not the generation at write time. The retry-on-conflict loop re-reads on 409, so a successful write always carries the current generation. `rdd set` waits for `Settled.status=True` with `observedGeneration >= post-patch metadata.generation`, so stale snapshots cannot prematurely satisfy the wait.

Deleting the `App` resource triggers the finalizer to stop and delete the owned LimaVM (and wait for the LimaVM controller to complete its own cleanup before removing the App finalizer).

AppKubernetesReasonMergeFailed is a new user-visible condition reason (pkg/apis/app/v1alpha1/app_types.go:99-101), but the status-condition table in api_app.md does not list KubernetesReady at all — neither the existing Ready/Probing/NotRunning/NotApplicable reasons nor the new MergeFailed. Because computeSettledCondition forwards the blocking Kubernetes reason (pkg/controllers/app/app/controllers/app_controller.go:572-575), users may see MergeFailed surface on Settled too, with no documentation behind either condition.

	// AppKubernetesReasonProbing means the controller is still waiting for
	// the k3s API server to respond.
	AppKubernetesReasonProbing = "Probing"

	// AppKubernetesReasonMergeFailed means the k3s API server is reachable
	// but merging the instance kubeconfig into ~/.kube/config failed.
	AppKubernetesReasonMergeFailed = "MergeFailed"
)

const (
	// EngineReasonStopped is set on ContainerEngineReady when the engine has
	// stopped and all mirror resources have been cleaned up.

		settled.Message = settledMessageWaitingForKubernetes
	case kubernetesEnabled && app.Spec.Kubernetes.Enabled && kubeCond.ObservedGeneration < app.Generation:
		settled.Status = metav1.ConditionFalse
		settled.Reason = v1alpha1.AppSettledReasonKubernetesStale
		settled.Message = settledMessageKubernetesStale
	case kubernetesEnabled && app.Spec.Kubernetes.Enabled && kubeCond.Status != metav1.ConditionTrue:
		settled.Status = metav1.ConditionFalse
		settled.Reason = kubeCond.Reason
		settled.Message = kubeCond.Message
	default:
		settled.Status = metav1.ConditionTrue
		settled.Reason = v1alpha1.AppSettledReasonSettled
		settled.Message = settledMessageSettled
	}

Fix: add KubernetesReady rows covering Ready, Probing, MergeFailed, NotRunning, and NotApplicable to the table, and extend the Settled (forwarded) row to mention KubernetesReady alongside Running/ContainerEngineReady.

			return ctrl.Result{}, condErr
		}
		return ctrl.Result{RequeueAfter: kubeProbeRequeue}, nil
	}

	// k3s is healthy — merge the context into ~/.kube/config.
	if err := r.manageKubeContext(ctx); err != nil {
		if condErr := r.setKubeCondition(ctx, &app,
			metav1.ConditionFalse,
			appv1alpha1.AppKubernetesReasonMergeFailed,
			fmt.Sprintf("Failed to merge kubeconfig: %v", err),
		); condErr != nil {
			return ctrl.Result{}, condErr
		}
		return ctrl.Result{RequeueAfter: kubeProbeRequeue}, nil
	}

	return ctrl.Result{}, r.setKubeCondition(ctx, &app,
		metav1.ConditionTrue,
		appv1alpha1.AppKubernetesReasonReady,
		"Kubernetes API server is ready",

kube_context_test.go covers only the helper functions; no test drives Reconcile on the new branch. The headline behavior change of commit 8ca6e6f — manageKubeContext returns err → KubernetesReady=False/MergeFailed → requeue at kubeProbeRequeue — therefore has zero coverage, and the condition propagates into Settled so a regression slips into rdd set waits silently.

Fix: add either a Reconcile-level unit test using a fake client and a t.Setenv("KUBECONFIG", ...) pointed at an unwritable destination (or instance.K3sConfig() pointed at a missing file), or a BATS case under bats/tests/32-app-controllers/kube-context.bats that makes ~/.kube read-only and asserts KubernetesReady.reason == MergeFailed. A unit test catches the wire-up regression at lowest cost; a BATS case adds end-to-end confidence.

	// k3s is healthy — merge the context into ~/.kube/config.
	if err := r.manageKubeContext(ctx); err != nil {
		if condErr := r.setKubeCondition(ctx, &app,
			metav1.ConditionFalse,
			appv1alpha1.AppKubernetesReasonMergeFailed,
			fmt.Sprintf("Failed to merge kubeconfig: %v", err),
		); condErr != nil {
			return ctrl.Result{}, condErr
		}
		return ctrl.Result{RequeueAfter: kubeProbeRequeue}, nil
	}

createReplaceKubeContext can return before any write — load instance kubeconfig (pkg/controllers/app/kubernetes/controllers/kube_context.go:45), instance kubeconfig has no cluster entry (line 55), instance kubeconfig has no user entry (line 64). In those cases no merge has been attempted, yet the condition message — surfaced via kubectl get app -o yaml and forwarded into Settled at app_controller.go:573-575 — claims a merge failure. The internal log.Error at kube_reconciler.go:208 separately says "Failed to create Kubernetes context", so the two messages disagree about what happened.

// its cluster/user/context entries to contextName, and merges them into
// ~/.kube/config.
func createReplaceKubeContext(contextName, srcPath string) error {
	src, err := clientcmd.LoadFromFile(srcPath)
	if err != nil {
		return fmt.Errorf("load instance kubeconfig: %w", err)
	}

	// Extract the single cluster and user (k3s names them "default").
	var cluster *clientcmdapi.Cluster
	for _, c := range src.Clusters {

	case kubernetesEnabled && app.Spec.Kubernetes.Enabled && kubeCond.ObservedGeneration < app.Generation:
		settled.Status = metav1.ConditionFalse
		settled.Reason = v1alpha1.AppSettledReasonKubernetesStale
		settled.Message = settledMessageKubernetesStale
	case kubernetesEnabled && app.Spec.Kubernetes.Enabled && kubeCond.Status != metav1.ConditionTrue:
		settled.Status = metav1.ConditionFalse
		settled.Reason = kubeCond.Reason
		settled.Message = kubeCond.Message
	default:
		settled.Status = metav1.ConditionTrue
		settled.Reason = v1alpha1.AppSettledReasonSettled
		settled.Message = settledMessageSettled
	}

func (r *KubernetesReconciler) manageKubeContext(ctx context.Context) error {
	contextName := instance.Name()
	log := logf.FromContext(ctx).WithName("kube-context")

	if err := createReplaceKubeContext(contextName, instance.K3sConfig()); err != nil {
		log.Error(err, "Failed to create Kubernetes context", "context", contextName)
		return fmt.Errorf("create Kubernetes context %q: %w", contextName, err)
	}

	r.contextMu.Lock()
	if r.contextProbeCancel != nil {

	// contextProbeCancel cancels the in-flight current-context probe goroutine.
	contextProbeCancel context.CancelFunc
	// contextProbeGen detects superseded goroutines.
	contextProbeGen int
	// contextProbeWg lets removeKubeContext wait for any probe to finish.
	contextProbeWg sync.WaitGroup
}

// Reconcile drives the KubernetesReady condition and ~/.kube/config lifecycle.
func (r *KubernetesReconciler) Reconcile(ctx context.Context, _ ctrl.Request) (ctrl.Result, error) {
	log := logf.FromContext(ctx)

	var app appv1alpha1.App
	if err := r.Get(ctx, client.ObjectKey{Name: appName}, &app); err != nil {
		return ctrl.Result{}, client.IgnoreNotFound(err)
	}

	running := apimeta.IsStatusConditionTrue(app.Status.Conditions, appv1alpha1.AppConditionRunning)

	log.V(1).Info("reconcile entered",

Fix:

-            fmt.Sprintf("Failed to merge kubeconfig: %v", err),
+            fmt.Sprintf("Failed to publish Kubernetes context: %v", err),

Or align with the inner log.Error wording ("Failed to create Kubernetes context").

// WatchEventLogger returns a predicate that logs every watch event the
// controller receives, before workqueue dispatch. Logs at V(1), so default
// verbosity stays quiet.
func WatchEventLogger(name string) predicate.Predicate {
	log := logf.Log.WithName(name + ".watch")
	return predicate.Funcs{
		CreateFunc: func(e event.CreateEvent) bool {
			log.V(1).Info("create",
				"name", e.Object.GetName(),
				"generation", e.Object.GetGeneration(),
				"resourceVersion", e.Object.GetResourceVersion(),
			)
			return true
		},
		UpdateFunc: func(e event.UpdateEvent) bool {
			log.V(1).Info("update",
				"name", e.ObjectNew.GetName(),
				"oldGeneration", e.ObjectOld.GetGeneration(),
				"newGeneration", e.ObjectNew.GetGeneration(),
				"oldResourceVersion", e.ObjectOld.GetResourceVersion(),
				"newResourceVersion", e.ObjectNew.GetResourceVersion(),
			)
			return true
		},
		DeleteFunc: func(e event.DeleteEvent) bool {
			log.V(1).Info("delete", "name", e.Object.GetName())
			return true
		},
		GenericFunc: func(e event.GenericEvent) bool {
			log.V(1).Info("generic", "name", e.Object.GetName())
			return true
		},
	}
}

The standard cache-driven watch source always populates Object, so For(&App{}) never delivers a nil event in practice; the same pattern lived in app_controller.go for months without crashing. However, this PR exports the helper into a shared package, widening the call surface to any future caller (custom event source, synthetic GenericEvent for triggering, test harness). Controller-runtime's own ResourceVersionChangedPredicate nil-checks ObjectOld/ObjectNew defensively for exactly that reason. Calling .GetName() on a nil object panics; controller-runtime recovers and requeues, but the next event panics again.

Fix:

 CreateFunc: func(e event.CreateEvent) bool {
+    if e.Object == nil {
+        return false
+    }
     log.V(1).Info("create",

Apply the same pattern to UpdateFunc (e.ObjectOld == nil || e.ObjectNew == nil), DeleteFunc, and GenericFunc.

			appv1alpha1.AppKubernetesReasonMergeFailed,
			fmt.Sprintf("Failed to merge kubeconfig: %v", err),
		); condErr != nil {
			return ctrl.Result{}, condErr
		}
		return ctrl.Result{RequeueAfter: kubeProbeRequeue}, nil
	}

	return ctrl.Result{}, r.setKubeCondition(ctx, &app,
		metav1.ConditionTrue,
		appv1alpha1.AppKubernetesReasonReady,

When the merge fails for a deterministic reason (read-only ~/.kube/config, missing parent dir the user revoked), the controller fires every 5s and manageKubeContext re-logs Failed to create Kubernetes context at Error level each time (line 208). setKubeCondition short-circuits identical reason/message pairs (SetStatusCondition returns changed=false), so there is no API churn, but the log noise pegs the user's terminal for as long as the underlying state persists. Returning the error (return ctrl.Result{}, fmt.Errorf(...)) would let controller-runtime's rate-limited workqueue apply exponential backoff. The current shape matches the pre-existing probe-failure paths at lines 108 and 119, so this is a consistency suggestion; if you keep the constant interval, add a comment naming the rationale (fast recovery once the transient clears).

			appv1alpha1.AppKubernetesReasonProbing,
			"Waiting for k3s API server",
		); condErr != nil {
			return ctrl.Result{}, condErr
		}
		return ctrl.Result{RequeueAfter: kubeProbeRequeue}, nil
	}
	if !healthy {
		log.V(1).Info("k3s API server not yet healthy")
		if condErr := r.setKubeCondition(ctx, &app,
			metav1.ConditionFalse,

func (r *KubernetesReconciler) manageKubeContext(ctx context.Context) error {
	contextName := instance.Name()
	log := logf.FromContext(ctx).WithName("kube-context")

	if err := createReplaceKubeContext(contextName, instance.K3sConfig()); err != nil {
		log.Error(err, "Failed to create Kubernetes context", "context", contextName)
		return fmt.Errorf("create Kubernetes context %q: %w", contextName, err)
	}

	r.contextMu.Lock()
	if r.contextProbeCancel != nil {