Deep Review: 20260320-133532-msys2-ci

	}

	switch inst.Status {
	case limatype.StatusRunning, limatype.StatusBroken:
		// Orphaned hostagent from before controller restart. Kill it so the
		// next reconcile can start with a watcher.
		logger.Info("Found orphaned hostagent, killing it", "status", inst.Status)
		if err := r.killOrphanedHostagent(ctx, inst); err != nil {
			logger.Error(err, "Failed to kill orphaned hostagent")
			return ctrl.Result{}, err
		}
		return ctrl.Result{RequeueAfter: time.Second}, nil

// killOrphanedHostagent terminates an orphaned hostagent (one running without a
// watcher, typically after a controller restart).
func (r *LimaVMReconciler) killOrphanedHostagent(ctx context.Context, inst *limatype.Instance) error {
	stopInstanceForcibly(ctx, log.FromContext(ctx), inst)
	return nil
}

Before this branch, killOrphanedHostagent attempted limainstance.StopGracefully with a 30-second timeout before falling back to limainstance.StopForcibly. The new code goes straight to stopInstanceForcibly, which sends SIGKILL without a graceful phase. Every controller restart with a running orphan now hard-kills the guest, risking filesystem corruption.

Verified via git diff upstream/main...HEAD: the old implementation called StopGracefully then StopForcibly; the new code calls only stopInstanceForcibly.

Fix: restore graceful-then-force for orphan recovery. On Windows, where StopGracefully cannot deliver signals to orphaned processes, skip to force-stop.

func stopInstanceForcibly(ctx context.Context, logger logr.Logger, inst *limatype.Instance) {
	for _, pid := range []int{inst.DriverPID, inst.HostAgentPID} {
		if pid > 0 {
			if err := process.KillTree(ctx, pid); err != nil {
				logger.V(1).Info("Failed to kill process tree", "pid", pid, "error", err)
			}
		}
	}
	// On WSL2, terminate the distro so store.Inspect reports StatusStopped.
	if inst.VMType == limatype.WSL2 {
		distroName := "lima-" + inst.Name
		// Best-effort with a timeout: wsl.exe can hang if the WSL
		// subsystem is degraded; don't block the caller.
		wslCtx, cancel := context.WithTimeout(ctx, 10*time.Second)
		defer cancel()
		if err := exec.CommandContext(wslCtx, "wsl.exe", "--terminate", distroName).Run(); err != nil {
			logger.V(1).Info("Failed to terminate WSL2 distro", "distro", distroName, "error", err)
		}
	}

Lima's upstream StopForcibly (at lima/pkg/instance/stop.go:119-129) iterates inst.AdditionalDisks, inspects each disk, and calls disk.Unlock(). The new stopInstanceForcibly omits this step entirely. After any forced stop or delete, in_use_by locks remain, blocking later reuse of those disks.

Fix: port the additional-disk unlock loop from Lima's StopForcibly into stopInstanceForcibly, after process cleanup and before returning.

// setVerbosityFromEnv sets klog verbosity from RDD_LOG_LEVEL as a default.
// Called before flag.Parse() so that an explicit -v flag on the command line
// takes precedence.
func setVerbosityFromEnv() {
	level := strings.TrimSpace(os.Getenv("RDD_LOG_LEVEL"))
	switch strings.ToLower(level) {
	case "trace":
		_ = flag.Set("v", "4")
	case "debug":
		_ = flag.Set("v", "2")
	}
}

The RDD CLI supports six log levels; external controllers handle only trace and debug. Setting RDD_LOG_LEVEL=info or RDD_LOG_LEVEL=warn is silently ignored — operators expect quieter output and get none. klog's numeric verbosity can only increase detail, so mapping warn/error to lower output is impossible through -v alone.

Fix: add a default case that logs when a non-empty, unrecognized level is set, so operators see why their setting has no effect:

default:
    if level != "" {
        klog.Infof("RDD_LOG_LEVEL=%q not supported for klog (only trace, debug); using default verbosity", level)
    }

@test "graceful service stop terminates hostagent" {
    local ha_pid
    ha_pid=$(get_hostagent_pid "${VM_NAME}")
    assert [ -n "${ha_pid}" ]
    assert_process_alive "${ha_pid}"

    rdd svc stop

    # On Windows, child process handles (wsl.exe) keep the hostagent PID visible
    # in the process table long after termination. The next test ("restart service
    # after graceful shutdown") verifies the hostagent is functionally dead.
    if is_unix; then
        try --max 15 --delay 1 --until-fail -- assert_process_alive "${ha_pid}"
    fi
}

The "graceful service stop terminates hostagent" test skips its key assertion on Windows. The follow-up test succeeds regardless of whether graceful shutdown or orphan recovery cleaned up the hostagent. On Windows, process.Kill calls TerminateProcess, which bypasses Go's signal handlers — shutdownAllHostagents never runs. The test cannot distinguish the two paths.

Fix: add a Windows-specific check (e.g., query rdd ctl get limavm status) to verify the hostagent was stopped by the service, not by orphan recovery. Or document this as a known Windows limitation and remove the misleading comment in the next test.

func ensureSSHKeys() error {
	configDir, err := dirnames.LimaConfigDir()
	if err != nil {
		return err
	}
	privPath := filepath.Join(configDir, filenames.UserPrivateKey)
	if _, err := os.Stat(privPath); err == nil {
		return nil
	}
	if err := os.MkdirAll(configDir, 0o700); err != nil {
		return fmt.Errorf("could not create %q: %w", configDir, err)
	}
	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
	defer cancel()
	cmd := exec.CommandContext(ctx, "ssh-keygen", "-t", "ed25519", "-q", "-N", "",
		"-C", "lima", "-f", privPath)
	if out, err := cmd.CombinedOutput(); err != nil {
		return fmt.Errorf("failed to generate SSH key: %s: %w", out, err)
	}
	return nil
}

If ssh-keygen crashes or is interrupted, it may leave a partial private key. On the next run, os.Stat(privPath) succeeds, ensureSSHKeys returns nil, and Lima fails to start with a corrupt key. Recovery requires manual deletion.

Fix: generate into a temporary path and rename on success, or verify the keypair is usable before returning.

	// gracefulShutdownTimeout is the time to wait for a hostagent to exit
	// after sending SIGINT before falling back to SIGKILL.
	gracefulShutdownTimeout = 30 * time.Second
)

This comment references Unix signal names, but the code now uses process.Interrupt (CTRL_BREAK on Windows) and process.KillTree (taskkill on Windows). The comment predates this PR but describes behavior this PR changed.

Fix: update to platform-agnostic language: "time to wait for a hostagent to exit after requesting graceful shutdown before falling back to forced termination."