diff --git a/.dockerignore b/.dockerignore new file mode 100644 index 000000000..315675727 --- /dev/null +++ b/.dockerignore @@ -0,0 +1,17 @@ +# Build context for python/Containerfile is the repo root. +# Keep .git — hatch-vcs metadata hooks call `git rev-parse` for URLs/version. +rust/target/ +**/target/ +python/.venv/ +**/.venv/ +**/__pycache__/ +**/.pytest_cache/ +**/.mypy_cache/ +**/.ruff_cache/ +e2e/testdata/ +.github/ +docs/_build/ +controller/bin/ +*.qcow2 +*.img +*.raw diff --git a/.github/actions/load-e2e-artifacts/action.yaml b/.github/actions/load-e2e-artifacts/action.yaml index 6d7247857..8ebd5f87c 100644 --- a/.github/actions/load-e2e-artifacts/action.yaml +++ b/.github/actions/load-e2e-artifacts/action.yaml @@ -27,6 +27,18 @@ runs: name: exporterset-controller-image-${{ inputs.arch }} path: /tmp/artifacts + - name: Download exporter image + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 + with: + name: exporter-image-${{ inputs.arch }} + path: /tmp/artifacts + + - name: Download qemu-runtime image + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 + with: + name: qemu-runtime-image-${{ inputs.arch }} + path: /tmp/artifacts + - name: Download python wheels uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 with: @@ -39,5 +51,7 @@ runs: docker load < /tmp/artifacts/controller-image.tar docker load < /tmp/artifacts/operator-image.tar docker load < /tmp/artifacts/exporterset-controller-image.tar + docker load < /tmp/artifacts/exporter-image.tar + docker load < /tmp/artifacts/qemu-runtime-image.tar mkdir -p controller/deploy/operator/dist cp /tmp/artifacts/operator-install.yaml controller/deploy/operator/dist/install.yaml diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index 7fcbdec27..f817e9271 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -172,6 +172,77 @@ jobs: path: /tmp/exporterset-controller-image.tar retention-days: 1 + build-exporter-image: + needs: changes + if: needs.changes.outputs.should_run == 'true' || github.event_name == 'workflow_dispatch' + strategy: + matrix: + include: ${{ fromJson(needs.changes.outputs.e2e-matrix) }} + runs-on: ${{ matrix.os }} + timeout-minutes: 45 + steps: + - name: Checkout repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + fetch-depth: 0 + + - name: Cache exporter image + id: cache + uses: actions/cache@caa296126883cff596d87d8935842f9db880ef25 # v5 + with: + path: /tmp/exporter-image.tar + # Bust when exporter image inputs change: Containerfile, Python sources, + # lockfiles, or embedded jumpstarter-exec (rust/). + key: exporter-image-${{ matrix.arch }}-${{ hashFiles('python/Containerfile', 'python/packages/**/*.py', 'python/**/pyproject.toml', 'python/uv.lock', 'rust/jumpstarter-exec/**', 'controller/Makefile') }} + + - name: Build exporter image + if: steps.cache.outputs.cache-hit != 'true' + run: | + make -C controller docker-build-exporter + docker save quay.io/jumpstarter-dev/jumpstarter:latest -o /tmp/exporter-image.tar + + - name: Upload exporter image + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: exporter-image-${{ matrix.arch }} + path: /tmp/exporter-image.tar + retention-days: 1 + + build-qemu-runtime-image: + needs: changes + if: needs.changes.outputs.should_run == 'true' || github.event_name == 'workflow_dispatch' + strategy: + matrix: + include: ${{ fromJson(needs.changes.outputs.e2e-matrix) }} + runs-on: ${{ matrix.os }} + timeout-minutes: 30 + steps: + - name: Checkout repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + fetch-depth: 0 + + - name: Cache qemu-runtime image + id: cache + uses: actions/cache@caa296126883cff596d87d8935842f9db880ef25 # v5 + with: + path: /tmp/qemu-runtime-image.tar + # Bust when QEMU runtime inputs change. + key: qemu-runtime-image-${{ matrix.arch }}-${{ hashFiles('controller/Makefile', 'controller/Containerfile.qemu-runtime') }} + + - name: Build qemu-runtime image + if: steps.cache.outputs.cache-hit != 'true' + run: | + make -C controller docker-build-qemu-runtime + docker save quay.io/jumpstarter-dev/virtual/qemu-runtime:latest -o /tmp/qemu-runtime-image.tar + + - name: Upload qemu-runtime image + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: qemu-runtime-image-${{ matrix.arch }} + path: /tmp/qemu-runtime-image.tar + retention-days: 1 + build-python-wheels: needs: changes if: needs.changes.outputs.should_run == 'true' || github.event_name == 'workflow_dispatch' @@ -215,12 +286,14 @@ jobs: # =========================================================================== e2e-tests: - needs: [changes, build-controller-image, build-operator-image, build-exporterset-controller-image, build-python-wheels] + needs: [changes, build-controller-image, build-operator-image, build-exporterset-controller-image, build-exporter-image, build-qemu-runtime-image, build-python-wheels] strategy: matrix: include: ${{ fromJson(needs.changes.outputs.e2e-matrix) }} runs-on: ${{ matrix.os }} - timeout-minutes: 60 + # Includes ExporterSet QEMU coverage (TCG); flash/boot still skipped until #924. + # Job > Ginkgo suite timeout (60m in e2e/lib/common.sh) for setup/log upload. + timeout-minutes: 70 steps: - name: Checkout repository uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 @@ -320,7 +393,7 @@ jobs: artifact-name: e2e-logs-compat-old-controller e2e-compat-old-client: - needs: [changes, build-controller-image, build-operator-image, build-exporterset-controller-image, build-python-wheels] + needs: [changes, build-controller-image, build-operator-image, build-exporterset-controller-image, build-exporter-image, build-qemu-runtime-image, build-python-wheels] # Skip on PRs — compat tests run in merge queue and workflow_dispatch. if: >- github.event_name != 'pull_request' diff --git a/Makefile b/Makefile index bf4055d61..7dcd2818c 100644 --- a/Makefile +++ b/Makefile @@ -36,11 +36,12 @@ help: @echo " make docs-test - Run documentation tests" @echo "" @echo "End-to-end testing:" - @echo " make e2e-setup - Setup e2e test environment (one-time)" - @echo " make e2e-run - Run e2e tests (requires e2e-setup first)" - @echo " make e2e - Same as e2e-run" - @echo " make e2e-full - Full setup + run (for CI or first time)" - @echo " make e2e-clean - Clean up e2e test environment (delete cluster, certs, etc.)" + @echo " make e2e-setup - Setup e2e test environment (one-time)" + @echo " make e2e-run - Run full e2e suite (includes ExporterSet QEMU)" + @echo " make e2e - Same as e2e-run" + @echo " make e2e-exporterset-qemu - Run ExporterSet QEMU e2e only" + @echo " make e2e-full - Full setup + run (for CI or first time)" + @echo " make e2e-clean - Clean up e2e test environment (delete cluster, certs, etc.)" @echo "" @echo "Per-project targets:" @echo " make build- - Build specific project" @@ -198,6 +199,12 @@ e2e-run: @echo "Running e2e tests..." @bash e2e/run-e2e.sh +# Focused local run of ExporterSet QEMU e2e (also covered by make e2e-run / CI). +.PHONY: e2e-exporterset-qemu +e2e-exporterset-qemu: + @echo "Running ExporterSet QEMU e2e tests..." + @GINKGO_LABEL_FILTER=exporterset-qemu bash e2e/run-e2e.sh + # Convenience alias for running e2e tests .PHONY: e2e e2e: e2e-run diff --git a/controller/Containerfile.qemu-runtime b/controller/Containerfile.qemu-runtime index 52ccae8bb..664b75427 100644 --- a/controller/Containerfile.qemu-runtime +++ b/controller/Containerfile.qemu-runtime @@ -26,6 +26,9 @@ RUN dnf install --setopt=install_weak_deps=False -y \ virtiofsd && \ dnf clean all +# Default image USER is non-root; ExporterSet QEMU pods override target-runtime +# to runAsUser 0 via Pod securityContext so QEMU can access devices and shared +# volume paths owned by the exporter container. USER 65532:65532 ENTRYPOINT ["/shared/jumpstarter-exec"] diff --git a/controller/hack/deploy_vars b/controller/hack/deploy_vars index a021ca5b0..ffa707286 100755 --- a/controller/hack/deploy_vars +++ b/controller/hack/deploy_vars @@ -8,6 +8,8 @@ BASEDOMAIN=${BASEDOMAIN:-"jumpstarter.${IP}.nip.io"} IMG=${IMG:-quay.io/jumpstarter-dev/jumpstarter-controller:latest} OPERATOR_IMG=${OPERATOR_IMG:-$(make -C deploy/operator --no-print-directory -s print-img 2>/dev/null || echo "quay.io/jumpstarter-dev/jumpstarter-operator:latest")} EXPORTER_SET_CONTROLLER_IMG=${EXPORTER_SET_CONTROLLER_IMG:-quay.io/jumpstarter-dev/jumpstarter-exporterset-controller:latest} +EXPORTER_IMG=${EXPORTER_IMG:-quay.io/jumpstarter-dev/jumpstarter:latest} +QEMU_RUNTIME_IMG=${QEMU_RUNTIME_IMG:-quay.io/jumpstarter-dev/virtual/qemu-runtime:latest} # Determine endpoints based on NETWORKING_MODE and CLUSTER_TYPE if [ "${NETWORKING_MODE}" == "ingress" ]; then @@ -46,4 +48,6 @@ export IMAGE_TAG export IMG export OPERATOR_IMG export EXPORTER_SET_CONTROLLER_IMG +export EXPORTER_IMG +export QEMU_RUNTIME_IMG diff --git a/controller/hack/deploy_with_operator.sh b/controller/hack/deploy_with_operator.sh index ff9f0b2a6..308737b6f 100755 --- a/controller/hack/deploy_with_operator.sh +++ b/controller/hack/deploy_with_operator.sh @@ -33,6 +33,18 @@ fi load_image "${IMG}" load_image "${OPERATOR_IMG}" load_image "${EXPORTER_SET_CONTROLLER_IMG}" +# Exporter + QEMU runtime images are required for ExporterSet QEMU e2e / samples. +# Missing images are skipped so plain controller deploys still work. +if container_image_exists "${EXPORTER_IMG}"; then + load_image "${EXPORTER_IMG}" +else + echo -e "${YELLOW}Skipping load of exporter image (not present locally): ${EXPORTER_IMG}${NC}" +fi +if container_image_exists "${QEMU_RUNTIME_IMG}"; then + load_image "${QEMU_RUNTIME_IMG}" +else + echo -e "${YELLOW}Skipping load of qemu-runtime image (not present locally): ${QEMU_RUNTIME_IMG}${NC}" +fi # Deploy the operator echo -e "${GREEN}Deploying Jumpstarter operator ...${NC}" diff --git a/controller/hack/sample-x86_64-kind.yaml b/controller/hack/sample-x86_64-kind.yaml index bd935cc52..53228d2c1 100644 --- a/controller/hack/sample-x86_64-kind.yaml +++ b/controller/hack/sample-x86_64-kind.yaml @@ -72,3 +72,4 @@ spec: arch: x86_64 smp: 1 mem: 1G + disk_size: 2G diff --git a/controller/hack/utils b/controller/hack/utils index 0b142398c..49fa1a91e 100755 --- a/controller/hack/utils +++ b/controller/hack/utils @@ -19,6 +19,7 @@ export K3S_KUBECONFIG=${K3S_KUBECONFIG:-/etc/rancher/k3s/k3s.yaml} # Color codes for terminal output export GREEN='\033[0;32m' +export YELLOW='\033[1;33m' export NC='\033[0m' # No Color # Get external IP address @@ -76,6 +77,20 @@ load_image() { esac } +# Return 0 if the image exists in the local container tool store. +container_image_exists() { + local image=$1 + local tool=${CONTAINER_TOOL:-} + if [ -z "${tool}" ]; then + if command -v podman >/dev/null 2>&1; then + tool=podman + else + tool=docker + fi + fi + ${tool} image inspect "${image}" >/dev/null 2>&1 +} + create_cluster() { _require_valid_cluster_type case "${CLUSTER_TYPE}" in diff --git a/controller/internal/exporterset/exporterconfig.go b/controller/internal/exporterset/exporterconfig.go index 3d7d6e545..09e5df7ea 100644 --- a/controller/internal/exporterset/exporterconfig.go +++ b/controller/internal/exporterset/exporterconfig.go @@ -44,10 +44,14 @@ const ( // configVolumeName is the volume name for the ExporterConfig Secret. configVolumeName = "exporter-config" - // configMountPath is where the ExporterConfig Secret is mounted. - configMountPath = "/etc/jumpstarter/exporters" + // ExporterConfigMountPath is where the ExporterConfig Secret is mounted + // inside the exporter container (QEMU provisioner and injectConfigVolume). + ExporterConfigMountPath = "/etc/jumpstarter/exporters" - // exporterContainerName is the init-container name in the sidecar Pod. + // configMountPath is the unexported alias used within this package. + configMountPath = ExporterConfigMountPath + + // exporterContainerName is the main container that runs jmp run. exporterContainerName = "exporter" ) diff --git a/controller/internal/exporterset/provisioners/qemu/enrich_test.go b/controller/internal/exporterset/provisioners/qemu/enrich_test.go index 019207d9b..7f06bb936 100644 --- a/controller/internal/exporterset/provisioners/qemu/enrich_test.go +++ b/controller/internal/exporterset/provisioners/qemu/enrich_test.go @@ -265,11 +265,29 @@ func TestEnrichExporterExport_defaultsFromMergedParameters(t *testing.T) { if got := config["smp"]; got != float64(4) { t.Errorf("smp = %v, want 4", got) } - if got := config["mem"]; got != "4Gi" { - t.Errorf("mem = %v, want 4Gi", got) + if got := config["mem"]; got != "4G" { + t.Errorf("mem = %v, want 4G", got) } - if got := config["disk_size"]; got != "40Gi" { - t.Errorf("disk_size = %v, want 40Gi", got) + if got := config["disk_size"]; got != "40G" { + t.Errorf("disk_size = %v, want 40G", got) + } +} + +func TestNormalizeQemuSize(t *testing.T) { + cases := []struct { + in, want string + }{ + {"10Gi", "10G"}, + {"512Mi", "512M"}, + {"1Ti", "1T"}, + {"2G", "2G"}, + {"128M", "128M"}, + } + for _, tc := range cases { + got := normalizeQemuSize(tc.in) + if got != tc.want { + t.Errorf("normalizeQemuSize(%q) = %v, want %q", tc.in, got, tc.want) + } } } diff --git a/controller/internal/exporterset/provisioners/qemu/qemu.go b/controller/internal/exporterset/provisioners/qemu/qemu.go index 5b540f41d..9d1180947 100644 --- a/controller/internal/exporterset/provisioners/qemu/qemu.go +++ b/controller/internal/exporterset/provisioners/qemu/qemu.go @@ -66,9 +66,15 @@ const ( // the QEMU runtime container. launcherSocketPath = "/shared/launcher.sock" - // configMountPath is where the ExporterConfig Secret is mounted - // inside the exporter sidecar. - configMountPath = "/etc/jumpstarter/exporters" + // exporterNonRootUID is the UID for the exporter main container. + // The runtime sidecar runs as root so it can read exporter-created + // paths on the shared volume without world-writable permissions. + exporterNonRootUID int64 = 65532 + + // exporterConfigPath is the jmp run config path. Must match + // exporterset.ExporterConfigMountPath + "/" + exporterConfigKey + // (cannot import the parent package — test import cycle). + exporterConfigPath = "/etc/jumpstarter/exporters/config.yaml" // QEMU driver type for identification during enrichment. qemuDriverType = "jumpstarter_driver_qemu.driver.Qemu" @@ -134,12 +140,17 @@ func (p *Provisioner) resolveImageSpec(spec *virtualtargetv1alpha1.ImageSpec, de // using the native sidecar pattern (KEP-753): // // - copy-jumpstarter-exec (regular init container) copies the -// jumpstarter-exec binary onto the shared volume and exits. -// - Exporter sidecar (init container with restartPolicy: Always) -// starts next and drains last; registers with the controller. -// - QEMU runtime (main container) runs the virtual machine. -// - Shared emptyDir volume for Unix socket communication -// (QMP, serial console, launcher socket). +// jumpstarter-exec binary from the exporter image onto the +// shared volume and exits. +// - target-runtime (native sidecar, restartPolicy: Always) starts +// next so launcher.sock is ready before the exporter; runs +// jumpstarter-exec serve / QEMU. +// - exporter (main container) runs `jmp run` — default kubectl logs +// target; when it exits (exitOnLeaseEnd / ExitAndReplace), +// Kubernetes terminates sidecars and the Pod completes. +// Pod restartPolicy is Never so a clean exporter exit is not +// restarted in-place (ExporterSet replaces the instance instead). +// - Shared emptyDir for Unix sockets (QMP, serial, launcher) and disk. // // The caller (reconciler) is responsible for setting // OwnerReferences on the Pod and injecting the config volume. @@ -153,6 +164,9 @@ func (p *Provisioner) RenderPod( ) (*corev1.Pod, error) { restartAlways := corev1.ContainerRestartPolicyAlways sizeLimit := resource.MustParse(sharedVolumeSizeLimit) + runAsRoot := int64(0) + runAsExporter := exporterNonRootUID + exporterNonRoot := true var exporterSpec, runtimeSpec *virtualtargetv1alpha1.ImageSpec if images != nil { @@ -190,6 +204,9 @@ func (p *Provisioner) RenderPod( pod := &corev1.Pod{ ObjectMeta: podMeta, Spec: corev1.PodSpec{ + // Never: ExitAndReplace relies on exporter (main) exit completing + // the Pod. Always would restart jmp run in-place and skip recycle. + RestartPolicy: corev1.RestartPolicyNever, InitContainers: []corev1.Container{ { Name: "copy-jumpstarter-exec", @@ -208,16 +225,19 @@ func (p *Provisioner) RenderPod( }, }, { - Name: "exporter", - Image: exporterImage, - ImagePullPolicy: exporterPullPolicy, + // Native sidecar: starts before the main exporter so + // launcher.sock exists when jmp run begins. Torn down + // automatically when the exporter (main) container exits. + // Runs as root so QEMU can use KVM devices and read + // exporter-owned paths on the shared volume. + Name: "target-runtime", + Image: runtimeImage, + ImagePullPolicy: runtimePullPolicy, RestartPolicy: &restartAlways, - Command: []string{"jmp", "run", "--exporter-config", configMountPath + "/config.yaml"}, - Env: []corev1.EnvVar{ - { - Name: "JUMPSTARTER_LAUNCHER_SOCKET", - Value: launcherSocketPath, - }, + Env: runtimeEnv, + SecurityContext: &corev1.SecurityContext{ + RunAsUser: &runAsRoot, + RunAsNonRoot: boolPtr(false), }, VolumeMounts: []corev1.VolumeMount{ { @@ -229,10 +249,23 @@ func (p *Provisioner) RenderPod( }, Containers: []corev1.Container{ { - Name: "target-runtime", - Image: runtimeImage, - ImagePullPolicy: runtimePullPolicy, - Env: runtimeEnv, + Name: "exporter", + Image: exporterImage, + ImagePullPolicy: exporterPullPolicy, + Command: []string{ + "jmp", "run", "--exporter-config", + exporterConfigPath, + }, + Env: []corev1.EnvVar{ + { + Name: "JUMPSTARTER_LAUNCHER_SOCKET", + Value: launcherSocketPath, + }, + }, + SecurityContext: &corev1.SecurityContext{ + RunAsUser: &runAsExporter, + RunAsNonRoot: &exporterNonRoot, + }, VolumeMounts: []corev1.VolumeMount{ { Name: sharedVolumeName, @@ -264,8 +297,13 @@ func (p *Provisioner) RenderPod( pod.Spec.Tolerations = append([]corev1.Toleration(nil), vtc.Spec.Scheduling.Tolerations...) } if vtc.Spec.Scheduling.Resources != nil { - // Apply resource requirements to target-runtime - pod.Spec.Containers[0].Resources = *vtc.Spec.Scheduling.Resources.DeepCopy() + // CPU/memory belong on the runtime sidecar (where QEMU runs). + for i := range pod.Spec.InitContainers { + if pod.Spec.InitContainers[i].Name == "target-runtime" { + pod.Spec.InitContainers[i].Resources = *vtc.Spec.Scheduling.Resources.DeepCopy() + break + } + } } } @@ -392,10 +430,33 @@ func setDefault(config map[string]interface{}, key string, params map[string]int } if val != nil { + // Kubernetes resource quantities use binary suffixes (Gi, Mi); + // the QEMU driver expects qemu-img style sizes (G, M). + if key == "disk_size" || key == "mem" { + val = normalizeQemuSize(val) + } config[key] = val } } +// normalizeQemuSize converts Kubernetes binary quantity strings (e.g. "10Gi") +// to the form expected by the QEMU driver / qemu-img (e.g. "10G"). +func normalizeQemuSize(v interface{}) interface{} { + s, ok := v.(string) + if !ok || len(s) < 2 { + return v + } + if s[len(s)-1] != 'i' { + return v + } + switch s[len(s)-2] { + case 'K', 'M', 'G', 'T', 'k', 'm', 'g', 't': + return s[:len(s)-1] + default: + return v + } +} + func splitDot(s string) []string { result := make([]string, 0, 2) start := 0 @@ -414,6 +475,10 @@ func mustJSON(v interface{}) *apiextensionsv1.JSON { return &apiextensionsv1.JSON{Raw: raw} } +func boolPtr(v bool) *bool { + return &v +} + // Cleanup handles teardown of QEMU-based exporter instances. // For in-cluster QEMU, this is a no-op since deleting the Pod // (via OwnerReference cascade) handles cleanup. diff --git a/controller/internal/exporterset/provisioners/qemu/qemu_test.go b/controller/internal/exporterset/provisioners/qemu/qemu_test.go index defcef855..1b4b09364 100644 --- a/controller/internal/exporterset/provisioners/qemu/qemu_test.go +++ b/controller/internal/exporterset/provisioners/qemu/qemu_test.go @@ -111,16 +111,31 @@ func TestRenderPod_copiesMetadataAndAppliesDefaults(t *testing.T) { if copyInit.RestartPolicy != nil { t.Errorf("copy-jumpstarter-exec RestartPolicy = %v, want nil (one-shot init)", copyInit.RestartPolicy) } - exporterInit := pod.Spec.InitContainers[1] - if exporterInit.Name != "exporter" { - t.Errorf("InitContainers[1].Name = %q, want exporter", exporterInit.Name) + runtimeInit := pod.Spec.InitContainers[1] + if runtimeInit.Name != "target-runtime" { + t.Errorf("InitContainers[1].Name = %q, want target-runtime", runtimeInit.Name) } - if exporterInit.RestartPolicy == nil || *exporterInit.RestartPolicy != corev1.ContainerRestartPolicyAlways { - t.Errorf("exporter RestartPolicy = %v, want Always", exporterInit.RestartPolicy) + if runtimeInit.RestartPolicy == nil || *runtimeInit.RestartPolicy != corev1.ContainerRestartPolicyAlways { + t.Errorf("target-runtime RestartPolicy = %v, want Always", runtimeInit.RestartPolicy) } - if len(pod.Spec.Containers) != 1 || pod.Spec.Containers[0].Name != "target-runtime" { + if runtimeInit.SecurityContext == nil || runtimeInit.SecurityContext.RunAsUser == nil || + *runtimeInit.SecurityContext.RunAsUser != 0 { + t.Errorf("target-runtime RunAsUser = %v, want 0", runtimeInit.SecurityContext) + } + if len(pod.Spec.Containers) != 1 || pod.Spec.Containers[0].Name != "exporter" { t.Errorf("unexpected containers: %#v", pod.Spec.Containers) } + exporter := pod.Spec.Containers[0] + if exporter.SecurityContext == nil || exporter.SecurityContext.RunAsUser == nil || + *exporter.SecurityContext.RunAsUser != exporterNonRootUID { + t.Errorf("exporter RunAsUser = %v, want %d", exporter.SecurityContext, exporterNonRootUID) + } + if exporter.SecurityContext.RunAsNonRoot == nil || !*exporter.SecurityContext.RunAsNonRoot { + t.Errorf("exporter RunAsNonRoot = %v, want true", exporter.SecurityContext.RunAsNonRoot) + } + if pod.Spec.RestartPolicy != corev1.RestartPolicyNever { + t.Errorf("RestartPolicy = %q, want Never (ExitAndReplace)", pod.Spec.RestartPolicy) + } } func TestRenderPod_clonesSchedulingFromVTC(t *testing.T) { @@ -173,11 +188,11 @@ func TestRenderPod_clonesSchedulingFromVTC(t *testing.T) { t.Errorf("VTC Tolerations mutated: got %q", got) } - gotCPU := pod.Spec.Containers[0].Resources.Requests[corev1.ResourceCPU] + gotCPU := pod.Spec.InitContainers[1].Resources.Requests[corev1.ResourceCPU] if !gotCPU.Equal(cpu) { t.Errorf("CPU request = %v, want %v", gotCPU, cpu) } - pod.Spec.Containers[0].Resources.Requests[corev1.ResourceCPU] = resource.MustParse("1") + pod.Spec.InitContainers[1].Resources.Requests[corev1.ResourceCPU] = resource.MustParse("1") if got := vtc.Spec.Scheduling.Resources.Requests[corev1.ResourceCPU]; !got.Equal(cpu) { t.Errorf("VTC Resources mutated: got %v", got) } @@ -209,7 +224,7 @@ func TestRenderPod_injectsJumpstarterExecLogFields(t *testing.T) { t.Errorf("Pod.GenerateName = %q, want empty when exporter is provided", pod.GenerateName) } - env := pod.Spec.Containers[0].Env + env := pod.Spec.InitContainers[1].Env var got string for _, e := range env { if e.Name == "JUMPSTARTER_EXEC_LOG_FIELDS" { @@ -285,11 +300,11 @@ func TestRenderPod_usesResolvedImages(t *testing.T) { if pod.Spec.InitContainers[0].Image != wantExporter { t.Errorf("copy-jumpstarter-exec image = %q, want %q", pod.Spec.InitContainers[0].Image, wantExporter) } - if pod.Spec.InitContainers[1].Image != wantExporter { - t.Errorf("exporter image = %q, want %q", pod.Spec.InitContainers[1].Image, wantExporter) + if pod.Spec.InitContainers[1].Image != wantRuntime { + t.Errorf("target-runtime image = %q, want %q", pod.Spec.InitContainers[1].Image, wantRuntime) } - if pod.Spec.Containers[0].Image != wantRuntime { - t.Errorf("target-runtime image = %q, want %q", pod.Spec.Containers[0].Image, wantRuntime) + if pod.Spec.Containers[0].Image != wantExporter { + t.Errorf("exporter image = %q, want %q", pod.Spec.Containers[0].Image, wantExporter) } } @@ -333,14 +348,17 @@ func TestRenderPod_imageOverrideFromSpec(t *testing.T) { if pod.Spec.InitContainers[0].ImagePullPolicy != corev1.PullAlways { t.Errorf("copy-jumpstarter-exec pullPolicy = %q, want Always", pod.Spec.InitContainers[0].ImagePullPolicy) } - if pod.Spec.InitContainers[1].Image != wantExporter { - t.Errorf("exporter image = %q, want %q", pod.Spec.InitContainers[1].Image, wantExporter) + if pod.Spec.InitContainers[1].Image != wantRuntime { + t.Errorf("target-runtime image = %q, want %q", pod.Spec.InitContainers[1].Image, wantRuntime) + } + if pod.Spec.InitContainers[1].ImagePullPolicy != corev1.PullIfNotPresent { + t.Errorf("target-runtime pullPolicy = %q, want IfNotPresent (default)", pod.Spec.InitContainers[1].ImagePullPolicy) } - if pod.Spec.Containers[0].Image != wantRuntime { - t.Errorf("target-runtime image = %q, want %q", pod.Spec.Containers[0].Image, wantRuntime) + if pod.Spec.Containers[0].Image != wantExporter { + t.Errorf("exporter image = %q, want %q", pod.Spec.Containers[0].Image, wantExporter) } - if pod.Spec.Containers[0].ImagePullPolicy != corev1.PullIfNotPresent { - t.Errorf("target-runtime pullPolicy = %q, want IfNotPresent (default)", pod.Spec.Containers[0].ImagePullPolicy) + if pod.Spec.Containers[0].ImagePullPolicy != corev1.PullAlways { + t.Errorf("exporter pullPolicy = %q, want Always", pod.Spec.Containers[0].ImagePullPolicy) } } @@ -369,7 +387,10 @@ func TestRenderPod_partialImageOverride(t *testing.T) { if pod.Spec.InitContainers[0].Image != wantExporter { t.Errorf("copy-jumpstarter-exec image = %q, want %q", pod.Spec.InitContainers[0].Image, wantExporter) } - if pod.Spec.Containers[0].Image != wantRuntime { - t.Errorf("target-runtime image = %q, want %q", pod.Spec.Containers[0].Image, wantRuntime) + if pod.Spec.InitContainers[1].Image != wantRuntime { + t.Errorf("target-runtime image = %q, want %q", pod.Spec.InitContainers[1].Image, wantRuntime) + } + if pod.Spec.Containers[0].Image != wantExporter { + t.Errorf("exporter image = %q, want %q", pod.Spec.Containers[0].Image, wantExporter) } } diff --git a/controller/internal/exporterset/reconciler.go b/controller/internal/exporterset/reconciler.go index 5b2e04cc2..09f0af47e 100644 --- a/controller/internal/exporterset/reconciler.go +++ b/controller/internal/exporterset/reconciler.go @@ -204,7 +204,8 @@ func (r *ExporterSetReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, err } - // Clean up drained (disabled+unleased) exporters before computing state. + // Clean up drained (disabled+unleased) exporters and ExitAndReplace + // terminal pods (Succeeded/Failed) before computing pool state. if deleted, err := r.cleanupDisabledExporters(ctx, &exporterSet, ownedExporters); err != nil { return ctrl.Result{}, err } else if deleted { @@ -220,6 +221,20 @@ func (r *ExporterSetReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, nil } + if deleted, err := r.cleanupTerminalExporters(ctx, &exporterSet, ownedExporters); err != nil { + return ctrl.Result{}, err + } else if deleted { + if err := r.reconcileStatusCounts(ctx, &exporterSet); err != nil { + return ctrl.Result{}, err + } + r.reconcileConditions(&exporterSet) + if err := r.Status().Update(ctx, &exporterSet); err != nil { + return requeueConflict(logger, err) + } + r.emitConditionEvents(&exporterSet, prevAvailable, prevProgressing, prevDegraded, prevScalingLimited) + return ctrl.Result{}, nil + } + state := computePoolState(ownedExporters) mergedParameters, err := deepMergeParameters(vtc.Spec.Parameters, exporterSet.Spec.Parameters) @@ -540,11 +555,11 @@ func (r *ExporterSetReconciler) readCredentialToken( return string(token), nil } -// listPodsByExporter returns a set of Exporter names that already have a Pod. -func (r *ExporterSetReconciler) listPodsByExporter( +// listPodsGroupedByExporter maps Exporter names to their owned Pods. +func (r *ExporterSetReconciler) listPodsGroupedByExporter( ctx context.Context, es *virtualtargetv1alpha1.ExporterSet, -) (map[string]struct{}, error) { +) (map[string][]corev1.Pod, error) { var podList corev1.PodList if err := r.List(ctx, &podList, client.InNamespace(es.Namespace), @@ -553,11 +568,13 @@ func (r *ExporterSetReconciler) listPodsByExporter( return nil, fmt.Errorf("list Pods for ExporterSet %s: %w", es.Name, err) } - byExporter := make(map[string]struct{}, len(podList.Items)) + byExporter := make(map[string][]corev1.Pod) for i := range podList.Items { - for _, ref := range podList.Items[i].OwnerReferences { + pod := podList.Items[i] + for _, ref := range pod.OwnerReferences { if ref.Kind == kindExporter && ref.Controller != nil && *ref.Controller { - byExporter[ref.Name] = struct{}{} + byExporter[ref.Name] = append(byExporter[ref.Name], pod) + break } } } @@ -565,6 +582,23 @@ func (r *ExporterSetReconciler) listPodsByExporter( return byExporter, nil } +// listPodsByExporter returns the set of Exporter names that already have a Pod. +// Derived from listPodsGroupedByExporter so reconcile does not List twice. +func (r *ExporterSetReconciler) listPodsByExporter( + ctx context.Context, + es *virtualtargetv1alpha1.ExporterSet, +) (map[string]struct{}, error) { + grouped, err := r.listPodsGroupedByExporter(ctx, es) + if err != nil { + return nil, err + } + byExporter := make(map[string]struct{}, len(grouped)) + for name := range grouped { + byExporter[name] = struct{}{} + } + return byExporter, nil +} + // injectConfigVolume adds the config Secret as a volume and mounts it in the // "exporter" init-container. Returns false if that container isn't found. func injectConfigVolume(pod *corev1.Pod, exporterName string) bool { @@ -577,16 +611,24 @@ func injectConfigVolume(pod *corev1.Pod, exporterName string) bool { }, }) + mount := corev1.VolumeMount{ + Name: configVolumeName, + MountPath: configMountPath, + ReadOnly: true, + } + + // Exporter is the main container (default logs / ExitAndReplace lifecycle). + for i := range pod.Spec.Containers { + if pod.Spec.Containers[i].Name == exporterContainerName { + pod.Spec.Containers[i].VolumeMounts = append(pod.Spec.Containers[i].VolumeMounts, mount) + return true + } + } + + // Backward-compatible: older Pod templates ran exporter as a native sidecar. for i := range pod.Spec.InitContainers { if pod.Spec.InitContainers[i].Name == exporterContainerName { - pod.Spec.InitContainers[i].VolumeMounts = append( - pod.Spec.InitContainers[i].VolumeMounts, - corev1.VolumeMount{ - Name: configVolumeName, - MountPath: configMountPath, - ReadOnly: true, - }, - ) + pod.Spec.InitContainers[i].VolumeMounts = append(pod.Spec.InitContainers[i].VolumeMounts, mount) return true } } @@ -653,6 +695,73 @@ func (r *ExporterSetReconciler) cleanupDisabledExporters( return deleted, nil } +// cleanupTerminalExporters deletes unleased exporters whose Pod has reached a +// terminal phase (Succeeded or Failed). This is the ExitAndReplace recycle +// path: exitOnLeaseEnd completes the Pod, then the controller deletes the +// Exporter (cascading the Pod) so scale-up can refill minAvailableReplicas. +// Without this, Offline/Succeeded instances inflate replicas, block warm-buffer +// refill at maxReplicas, and leave Completed Pods behind. +func (r *ExporterSetReconciler) cleanupTerminalExporters( + ctx context.Context, + es *virtualtargetv1alpha1.ExporterSet, + exporters []jumpstarterdevv1alpha1.Exporter, +) (bool, error) { + logger := log.FromContext(ctx) + + podsByExporter, err := r.listPodsGroupedByExporter(ctx, es) + if err != nil { + return false, err + } + + deleted := false + for i := range exporters { + exp := &exporters[i] + if exp.Status.LeaseRef != nil { + continue + } + + pods := podsByExporter[exp.Name] + if !allPodsTerminal(pods) { + continue + } + + if err := r.Provisioner.Cleanup(ctx, es, exp); err != nil { + return deleted, fmt.Errorf("unable to cleanup Exporter %s: %w", exp.Name, err) + } + + if err := r.Delete(ctx, exp); err != nil && !apierrors.IsNotFound(err) { + return deleted, fmt.Errorf("unable to delete Exporter %s: %w", exp.Name, err) + } + + logger.Info("deleted Exporter after terminal Pod (ExitAndReplace)", + "exporter", exp.Name, "pods", len(pods)) + + if r.Recorder != nil { + r.Recorder.Eventf(es, corev1.EventTypeNormal, "Recycle", + "Deleted Exporter %s after terminal Pod", exp.Name) + } + deleted = true + } + + return deleted, nil +} + +func allPodsTerminal(pods []corev1.Pod) bool { + if len(pods) == 0 { + return false + } + for i := range pods { + if !isTerminalPodPhase(pods[i].Status.Phase) { + return false + } + } + return true +} + +func isTerminalPodPhase(phase corev1.PodPhase) bool { + return phase == corev1.PodSucceeded || phase == corev1.PodFailed +} + // reconcileScaleUp evaluates the three scale-up rules in priority order and // creates one Exporter if any rule fires. Returns true if a scale-up was // attempted (caller should skip scale-down for this cycle). diff --git a/controller/internal/exporterset/reconciler_test.go b/controller/internal/exporterset/reconciler_test.go index bcfa043c3..b9b65eda8 100644 --- a/controller/internal/exporterset/reconciler_test.go +++ b/controller/internal/exporterset/reconciler_test.go @@ -198,7 +198,10 @@ func makePod(name string, phase corev1.PodPhase) *corev1.Pod { ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: nsDefault, - Labels: map[string]string{"exporterset": "demo-set"}, + Labels: map[string]string{ + "exporterset": "demo-set", + labelExporterSetName: "demo-set", + }, // Pods are owned by Exporters, not ExporterSets directly. OwnerReferences: []metav1.OwnerReference{{ APIVersion: "jumpstarter.dev/v1alpha1", @@ -580,6 +583,131 @@ func TestReconcile_scaleDown_doesNotDeleteLeasedDisabledExporter(t *testing.T) { } } +func TestReconcile_exitAndReplace_deletesSucceededUnleasedExporter(t *testing.T) { + es := makeExporterSet(func(es *virtualtargetv1alpha1.ExporterSet) { + es.Spec.MinReplicas = 0 + es.Spec.MaxReplicas = 1 + es.Spec.MinAvailableReplicas = 1 + es.Spec.RecycleStrategy = virtualtargetv1alpha1.RecycleStrategyExitAndReplace + }) + + // Offline exporter with Succeeded Pod (exitOnLeaseEnd completed). + r, c := newReconciler(t, + es, makeVTC(), + makeExporter("exp-dead", false, false, true), + makePod("exp-dead", corev1.PodSucceeded), + ) + + reconcileOnce(t, r) + + exporters := listExporters(t, c) + if len(exporters) != 0 { + t.Fatalf("expected Succeeded exporter deleted for ExitAndReplace, got %d", len(exporters)) + } +} + +func TestReconcile_exitAndReplace_deletesFailedUnleasedExporter(t *testing.T) { + es := makeExporterSet(func(es *virtualtargetv1alpha1.ExporterSet) { + es.Spec.MinReplicas = 0 + es.Spec.MaxReplicas = 1 + es.Spec.MinAvailableReplicas = 1 + }) + + r, c := newReconciler(t, + es, makeVTC(), + makeExporter("exp-crash", false, false, true), + makePod("exp-crash", corev1.PodFailed), + ) + + reconcileOnce(t, r) + + exporters := listExporters(t, c) + if len(exporters) != 0 { + t.Fatalf("expected Failed exporter deleted, got %d", len(exporters)) + } +} + +func TestReconcile_exitAndReplace_keepsRunningExporter(t *testing.T) { + es := makeExporterSet(func(es *virtualtargetv1alpha1.ExporterSet) { + es.Spec.MinReplicas = 0 + es.Spec.MaxReplicas = 1 + es.Spec.MinAvailableReplicas = 1 + }) + + r, c := newReconciler(t, + es, makeVTC(), + makeExporter("exp-live", true, false, true), + makePod("exp-live", corev1.PodRunning), + ) + + reconcileOnce(t, r) + + exporters := listExporters(t, c) + if len(exporters) != 1 { + t.Fatalf("expected Running exporter kept, got %d", len(exporters)) + } +} + +func TestReconcile_exitAndReplace_keepsLeasedTerminalExporter(t *testing.T) { + es := makeExporterSet(func(es *virtualtargetv1alpha1.ExporterSet) { + es.Spec.MinReplicas = 0 + es.Spec.MaxReplicas = 1 + es.Spec.MinAvailableReplicas = 1 + }) + + r, c := newReconciler(t, + es, makeVTC(), + makeExporter("exp-leased-dead", false, true, true), + makePod("exp-leased-dead", corev1.PodSucceeded), + ) + + reconcileOnce(t, r) + + exporters := listExporters(t, c) + if len(exporters) != 1 { + t.Fatalf("expected leased terminal exporter kept until lease clears, got %d", len(exporters)) + } +} + +func TestReconcile_exitAndReplace_maxReplicasRefillsAfterDelete(t *testing.T) { + caCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: caConfigMapName, + Namespace: nsDefault, + }, + Data: map[string]string{ + caConfigMapKey: "-----BEGIN CERTIFICATE-----\ntest\n-----END CERTIFICATE-----\n", + }, + } + es := makeExporterSet(func(es *virtualtargetv1alpha1.ExporterSet) { + es.Spec.MinReplicas = 0 + es.Spec.MaxReplicas = 1 + es.Spec.MinAvailableReplicas = 1 + }) + + r, c := newReconciler(t, + es, makeVTC(), caCM, + makeExporter("exp-dead", false, false, true), + makePod("exp-dead", corev1.PodSucceeded), + ) + + // First reconcile: delete terminal exporter (frees the maxReplicas slot). + reconcileOnce(t, r) + if got := len(listExporters(t, c)); got != 0 { + t.Fatalf("after cleanup: expected 0 exporters, got %d", got) + } + + // Second reconcile: warm buffer scale-up creates a replacement. + reconcileOnce(t, r) + exporters := listExporters(t, c) + if len(exporters) != 1 { + t.Fatalf("after refill: expected 1 exporter, got %d", len(exporters)) + } + if exporters[0].Name == "exp-dead" { + t.Fatal("replacement should be a new Exporter, not exp-dead") + } +} + func TestReconcile_scaleDown_respectsMinReplicas(t *testing.T) { cooldown := 1 * time.Second es := makeExporterSet(func(es *virtualtargetv1alpha1.ExporterSet) { diff --git a/docs/source/contributing/jeps/JEP-0014-virtual-scalable-exporters.md b/docs/source/contributing/jeps/JEP-0014-virtual-scalable-exporters.md index bef51a9a8..cc80c5da4 100644 --- a/docs/source/contributing/jeps/JEP-0014-virtual-scalable-exporters.md +++ b/docs/source/contributing/jeps/JEP-0014-virtual-scalable-exporters.md @@ -108,7 +108,7 @@ VirtualTargetClass ←── referenced by ── ExporterSet │ ▼ Exporter ──► Pod - (exporter sidecar + target runtime) + (exporter main + target-runtime sidecar) ``` - **`VirtualTargetClass`** — **namespaced** configuration for a backend @@ -292,22 +292,23 @@ unresolved; see *Unresolved Questions*. ```yaml # rendered by qemu.jumpstarter.dev provisioner spec: + restartPolicy: Never # exporter exit completes the Pod (ExitAndReplace) initContainers: - - name: exporter # native sidecar (starts first, drains last) - restartPolicy: Always + - name: copy-jumpstarter-exec # one-shot: stage binary onto shared volume image: quay.io/jumpstarter-dev/jumpstarter:latest - containers: - - name: target-runtime # QEMU/Cuttlefish — independent image + - name: target-runtime # native sidecar (starts before exporter) + restartPolicy: Always image: quay.io/jumpstarter-dev/virtual/qemu-runtime:latest volumeMounts: - - name: os - mountPath: /os + - name: shared + mountPath: /shared + containers: + - name: exporter # main — default logs; exit tears Pod down + image: quay.io/jumpstarter-dev/jumpstarter:latest + volumeMounts: - name: shared mountPath: /shared volumes: - - name: os - image: - reference: registry.example.com/os/rpi4:latest # OS as OCI artifact - name: shared emptyDir: {} ``` @@ -316,15 +317,18 @@ Benefits: - **Independent release cadence** — exporter, runtime, and OS image version independently. -- **Fault isolation** — exporter survives target-runtime crashes and can drain - or report failure. +- **Natural teardown** — exporter is the main container, so ExitAndReplace is + simply process exit; Kubernetes terminates the runtime sidecar automatically. +- **Default logs** — `kubectl logs` shows the exporter without `-c`. +- **Socket ready first** — runtime sidecar starts before the exporter so + `launcher.sock` exists when `jmp run` begins. - **Standard interfaces** — drivers attach over virtio (serial/SPI/CAN/GPIO) or Unix sockets on shared volumes; same driver code works physical + virtual. - **Unprivileged Pods** — virtio-backed guests avoid privileged containers when the host supports it. -The exporter sidecar communicates with the target-runtime container via Unix -sockets on a shared `emptyDir` volume (QMP for QEMU control, serial console, +The exporter (main container) communicates with the target-runtime sidecar via +Unix sockets on a shared `emptyDir` volume (QMP for QEMU control, serial console, launcher socket for dynamic argv). API-backed provisioners (`corellium`, `ec2`) and off-cluster provisioners (`qemu-baremetal.jumpstarter.dev`) skip the in-cluster runtime container — see *External and Off-Cluster Provisioning*. @@ -371,7 +375,7 @@ VirtualTargetClass ←── referenced by ── ExporterSet │ ▼ Exporter ──► Pod - (exporter sidecar + QEMU runtime) + (exporter main + QEMU runtime sidecar) ``` Homogeneous QEMU pools configure **`VirtualTargetClass` + `ExporterSet` only**. @@ -505,11 +509,12 @@ spec: - Create an `Exporter` CR with labels from `spec.template.metadata` and drivers from `spec.template.spec`. - Render a Kubernetes Pod (sidecar pattern): - - **Exporter sidecar** (native sidecar, `restartPolicy: Always`) — starts - first, registers with `jumpstarter-controller`. - - **QEMU runtime container** — baseline virt machine from merged + - **QEMU runtime** (native sidecar, `restartPolicy: Always`) — starts + first so `launcher.sock` is ready; baseline virt machine from merged `parameters` (CPU, memory, firmware blob); **empty disk** ready for user flash at lease time. + - **Exporter** (main container) — registers with `jumpstarter-controller`; + default logs; exit drives ExitAndReplace Pod completion. - Exporter talks to runtime via Unix sockets on a shared `emptyDir` (QMP, serial, launcher). - Apply scheduling from `VirtualTargetClass.scheduling` to the Pod. @@ -559,7 +564,7 @@ jmp lease -l board=rpi4,virtual=true **Result:** User holds an active lease on `rpi4-virtual-aaa`. Pool still maintains warm capacity via background scale-up. -#### Phase 4 — User session: flash, boot, test (user + exporter sidecar) +#### Phase 4 — User session: flash, boot, test (user + exporter) The warm pool provides **instant lease assignment**; image selection happens **after** lease — same workflow as a physical bench (DD-7). The pool does not @@ -575,12 +580,12 @@ with env() as client: # ... run tests ... ``` -**Exporter sidecar actions:** +**Exporter actions:** - `storage.flash` writes the image to shared storage (or tells QEMU runtime via QMP/`blockdev-add`). - `power.on` sends QEMU start via QMP or launcher socket on shared volume. -- Serial/network drivers proxy to the QEMU runtime container. +- Serial/network drivers proxy to the QEMU runtime sidecar. **Controller actions:** None during the session (lease is held). @@ -601,10 +606,12 @@ jmp delete-lease # or lease TTL expires 1. Observe exporter is unleased; update `availableReplicas` / `leasedReplicas`. 2. Apply `recycleStrategy`: - - **ExitAndReplace (default):** exporter sidecar exits after cleanup → Pod - terminates → controller deletes `Exporter` CR → creates a fresh replacement - with empty baseline storage to maintain `minAvailableReplicas` (next lessee - flashes again). + - **ExitAndReplace (default):** exporter (main) exits after cleanup → Pod + reaches `Succeeded`/`Failed` → controller deletes the unleased `Exporter` + CR (OwnerReference cascade removes the Completed Pod) → warm-buffer + scale-up creates a fresh replacement with empty baseline storage to + maintain `minAvailableReplicas` (next lessee flashes again). With + `maxReplicas` tight, delete must happen before create. - **InPlaceReuse:** exporter resets QEMU state in place → same Pod returns to Ready without restart (lessee may re-flash before next session). 3. If `availableReplicas > minAvailableReplicas` for longer than @@ -824,10 +831,10 @@ spec: limits: devices.kubevirt.io/kvm: "1" images: # optional; overrides default container images - exporter: # exporter sidecar image + exporter: # exporter (main container) image image: # e.g. quay.io/jumpstarter-dev/jumpstarter:v0.9.0 imagePullPolicy: - runtime: # provisioner-specific runtime image + runtime: # provisioner-specific runtime sidecar image image: # e.g. quay.io/jumpstarter-dev/virtual/qemu-runtime:v0.9.0 imagePullPolicy: ``` @@ -870,8 +877,8 @@ Both `VirtualTargetClass` and `ExporterSet` expose an `spec.images` field with typed sub-fields for overriding the default container images used by the provisioner: -- **`images.exporter`** — the exporter sidecar container image. -- **`images.runtime`** — the provisioner-specific runtime container image +- **`images.exporter`** — the exporter (main container) image. +- **`images.runtime`** — the provisioner-specific runtime sidecar image (e.g. QEMU runtime for `qemu.jumpstarter.dev`). Each sub-field is an `ImageSpec` with: @@ -1414,10 +1421,15 @@ it straightforward to correlate Pods, Exporters, and their logs. **`exitOnLeaseEnd` derivation:** The `exitOnLeaseEnd` flag in the generated `ExporterConfig` is derived from `ExporterSet.spec.recycleStrategy`: -- `ExitAndReplace` (default) → `exitOnLeaseEnd: true` — the exporter process - exits when its lease ends, causing the Pod to terminate and be replaced. +- `ExitAndReplace` (default) → `exitOnLeaseEnd: true` — the exporter (main + container) exits when its lease ends. The Pod uses `restartPolicy: Never` + so that exit completes the Pod (rather than restarting `jmp run` in place); + Kubernetes then terminates the `target-runtime` native sidecar. The + ExporterSet controller replaces the instance. The exporter also best-effort + calls `jumpstarter-exec shutdown` for a clean runtime exit before process + death. - `InPlaceReuse` → `exitOnLeaseEnd: false` — the exporter stays running and - transitions back to available. + transitions back to available (runtime sidecar is left alone). ### Component Interaction @@ -1447,14 +1459,17 @@ it straightforward to correlate Pods, Exporters, and their logs. ## Test Plan - - ### Unit Tests Unit tests should meet the project test coverage requirements. ### Integration Tests -- End-to-end lease lifecycle with QEMU provisioner in a test cluster +- Kind e2e suite labeled `exporterset-qemu` (`e2e/test/exporterset_qemu_test.go`, + part of `make e2e-run` / CI `e2e-tests`): apply kind-friendly + `VirtualTargetClass` + `ExporterSet`, wait for Exporter/Pod Ready, lease, + flash Alpine UEFI tiny, expect a serial-console boot marker. Flash/boot is + skipped until guest-disk capacity lands (#924); control-plane coverage runs + today. Use `make e2e-exporterset-qemu` for a focused local run. - Mixed physical/virtual lease orchestration - Provisioner failure and recovery scenarios - Parameter deep-merge and provisioner-side validation @@ -1647,8 +1662,8 @@ flash-at-lease workflow (DD-7). - [ ] Deployment-style status + `scale` subresource - [ ] Watch Leases and Exporters for scaling decisions - [ ] Add `exporterSets` section to `Jumpstarter` operator CR -- [ ] Integration test: deploy `ExporterSet`, lease, flash, boot, release, - observe scaling +- [x] Integration test: deploy `ExporterSet`, lease, flash, boot, release, + observe scaling (`exporterset-qemu` e2e; flash/boot gated on #924 storage) ### Phase 3: External / off-cluster provisioning diff --git a/e2e/dex.values.yaml b/e2e/dex.values.yaml index c2a22055a..2b09c420d 100644 --- a/e2e/dex.values.yaml +++ b/e2e/dex.values.yaml @@ -38,6 +38,10 @@ config: hash: "$2a$10$2b2cU8CPhOTaGrs1HRQuAueS7JTT5ZHsHSzYiFPm1leZck7Mc8T4W" # password username: "test-exporter-hooks" userID: "c6ed6f40-6689-6a7e-c64c-55d0a9a5871f" + - email: "test-client-exporterset-qemu@example.com" + hash: "$2a$10$2b2cU8CPhOTaGrs1HRQuAueS7JTT5ZHsHSzYiFPm1leZck7Mc8T4W" # password + username: "test-client-exporterset-qemu" + userID: "d7fe7051-7790-7b8f-d75d-66e1b0b69820" connectors: - name: kubernetes type: oidc diff --git a/e2e/lib/common.sh b/e2e/lib/common.sh index d0d6a9aca..9ee3b6a42 100644 --- a/e2e/lib/common.sh +++ b/e2e/lib/common.sh @@ -71,7 +71,15 @@ run_ginkgo() { local label_filter="${1:-}" shift || true - local flags=(-v --show-node-events --trace --timeout 30m) + local timeout="30m" + # Full suite and ExporterSet QEMU runs need TCG headroom (flash/boot when enabled). + # "!exporterset-qemu" also matches the positive substring, so check negation first. + if [[ "${label_filter}" != *"!exporterset-qemu"* ]] && \ + { [[ -z "${label_filter}" ]] || [[ "${label_filter}" == *"exporterset-qemu"* ]]; }; then + timeout="60m" + fi + + local flags=(-v --show-node-events --trace --timeout "${timeout}") if [ -n "$label_filter" ]; then flags+=(--label-filter "$label_filter") fi diff --git a/e2e/manifests/exporterset-qemu-kind.yaml b/e2e/manifests/exporterset-qemu-kind.yaml new file mode 100644 index 000000000..0fd027aa3 --- /dev/null +++ b/e2e/manifests/exporterset-qemu-kind.yaml @@ -0,0 +1,64 @@ +# Sample VirtualTargetClass + ExporterSet for exporterset-qemu e2e tests. +# +# Kind/TCG friendly (no KVM). Applied by e2e/test/exporterset_qemu_test.go. +# Images default to locally loaded :latest tags used by Kind e2e setup. +--- +apiVersion: virtualtarget.jumpstarter.dev/v1alpha1 +kind: VirtualTargetClass +metadata: + name: qemu-x86-64-e2e + namespace: jumpstarter-lab +spec: + provisioner: qemu.jumpstarter.dev + bindingMode: Immediate + reclaimPolicy: Delete + scheduling: + resources: + requests: + cpu: "1" + memory: 1Gi + images: + exporter: + image: quay.io/jumpstarter-dev/jumpstarter:latest + imagePullPolicy: IfNotPresent + runtime: + image: quay.io/jumpstarter-dev/virtual/qemu-runtime:latest + imagePullPolicy: IfNotPresent + parameters: + arch: x86_64 + machineType: q35 + resources: + cpu: 1 + memory: 1Gi + storage: 10Gi +--- +apiVersion: virtualtarget.jumpstarter.dev/v1alpha1 +kind: ExporterSet +metadata: + name: x86-64-virtual-e2e + namespace: jumpstarter-lab +spec: + minReplicas: 0 + maxReplicas: 1 + minAvailableReplicas: 1 + scaleDownCooldown: 5m + recycleStrategy: ExitAndReplace + virtualTargetClassName: qemu-x86-64-e2e + selector: + matchLabels: + board: x86-64-virtual-e2e + template: + metadata: + labels: + board: x86-64-virtual-e2e + arch: x86_64 + virtual: "true" + spec: + drivers: + - name: qemu + type: jumpstarter_driver_qemu.driver.Qemu + config: + arch: x86_64 + smp: 1 + mem: 1G + disk_size: 10G diff --git a/e2e/run-e2e.sh b/e2e/run-e2e.sh index e6be477f8..3713d0842 100755 --- a/e2e/run-e2e.sh +++ b/e2e/run-e2e.sh @@ -2,12 +2,17 @@ # Jumpstarter End-to-End Test Runner # This script runs the e2e test suite (assumes setup-e2e.sh was run first) # -# The tests are implemented using Go + Ginkgo. Label filters can be used to -# run specific subsets: -# --label-filter "core" - run core tests only -# --label-filter "hooks" - run hooks tests only -# --label-filter "direct-listener" - run direct-listener tests only -# --label-filter "!operator-only" - skip operator-specific tests +# The tests are implemented using Go + Ginkgo. By default the full suite runs, +# including ExporterSet QEMU (label exporterset-qemu). Label filters select +# subsets: +# --label-filter "core" - run core tests only +# --label-filter "hooks" - run hooks tests only +# --label-filter "direct-listener" - run direct-listener tests only +# --label-filter "exporterset-qemu" - ExporterSet QEMU only (needs qemu images) +# --label-filter "!exporterset-qemu" - skip ExporterSet QEMU +# --label-filter "!operator-only" - skip operator-specific tests +# +# Override with GINKGO_LABEL_FILTER for focused local runs. set -euo pipefail diff --git a/e2e/scripts/ensure-qemu-guest-image.sh b/e2e/scripts/ensure-qemu-guest-image.sh new file mode 100755 index 000000000..0b1c53081 --- /dev/null +++ b/e2e/scripts/ensure-qemu-guest-image.sh @@ -0,0 +1,46 @@ +#!/usr/bin/env bash +# Ensure the Alpine UEFI tiny guest image is available for exporterset-qemu e2e. +# +# Resolution order: +# 1. JUMPSTARTER_E2E_QEMU_IMAGE (absolute path to an existing image — printed and exited) +# 2. Existing file at e2e/testdata/ +# 3. Copy from python/packages/jumpstarter-driver-qemu/images/ if present +# 4. Download from ALPINE_IMAGE_URL +# +# Prints the absolute image path on stdout (last line). + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +REPO_ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)" +TESTDATA_DIR="${REPO_ROOT}/e2e/testdata" +IMAGE_NAME="${JUMPSTARTER_E2E_QEMU_IMAGE_NAME:-nocloud_alpine-3.22.4-x86_64-uefi-tiny-r0.qcow2}" +DEST="${TESTDATA_DIR}/${IMAGE_NAME}" +# Pinned Alpine nocloud UEFI tiny image (~128Mi). Override with JUMPSTARTER_E2E_QEMU_IMAGE_URL. +ALPINE_IMAGE_URL="${JUMPSTARTER_E2E_QEMU_IMAGE_URL:-https://dl-cdn.alpinelinux.org/alpine/v3.22/releases/cloud/${IMAGE_NAME}}" +QEMU_PKG_IMAGE="${REPO_ROOT}/python/packages/jumpstarter-driver-qemu/images/${IMAGE_NAME}" + +if [ -n "${JUMPSTARTER_E2E_QEMU_IMAGE:-}" ]; then + echo "${JUMPSTARTER_E2E_QEMU_IMAGE}" + exit 0 +fi + +mkdir -p "${TESTDATA_DIR}" + +if [ -f "${DEST}" ]; then + echo "${DEST}" + exit 0 +fi + +if [ -f "${QEMU_PKG_IMAGE}" ]; then + echo "Copying guest image from ${QEMU_PKG_IMAGE}" >&2 + cp -f "${QEMU_PKG_IMAGE}" "${DEST}" + echo "${DEST}" + exit 0 +fi + +echo "Downloading Alpine guest image from ${ALPINE_IMAGE_URL}" >&2 +tmp="${DEST}.partial" +curl -fL --retry 3 --retry-delay 2 -o "${tmp}" "${ALPINE_IMAGE_URL}" +mv "${tmp}" "${DEST}" +echo "${DEST}" diff --git a/e2e/scripts/qemu_flash_boot.py b/e2e/scripts/qemu_flash_boot.py new file mode 100755 index 000000000..0e6c9e71a --- /dev/null +++ b/e2e/scripts/qemu_flash_boot.py @@ -0,0 +1,106 @@ +#!/usr/bin/env python3 +# Copyright 2026 The Jumpstarter Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Flash a guest image to a leased ExporterSet QEMU target and wait for boot. + +Intended to run under `jmp shell`, which sets JUMPSTARTER_HOST so env() works: + + jmp shell --client … --selector board=… -- \\ + python3 e2e/scripts/qemu_flash_boot.py /path/to/image.qcow2 + +Verification is serial-console based (Alpine boot markers). SSH/vsock are +intentionally not required for this minimal smoke path. +""" + +from __future__ import annotations + +import argparse +import os +import sys + +import pexpect +from jumpstarter_driver_network.adapters import PexpectAdapter + +from jumpstarter.utils.env import env + +DEFAULT_BOOT_MARKERS = ( + "login:", + "Welcome to Alpine", + "Alpine Linux", + "localhost login:", +) + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("image", help="Path or URL to the guest disk image to flash") + parser.add_argument( + "--timeout", + type=int, + default=int(os.environ.get("JUMPSTARTER_E2E_CONSOLE_TIMEOUT", "900")), + help="Seconds to wait for a console boot marker (default: 900)", + ) + parser.add_argument( + "--disk-size", + default=os.environ.get("JUMPSTARTER_E2E_DISK_SIZE", "2G"), + help="Disk size passed to qemu.set_disk_size before power on", + ) + parser.add_argument( + "--skip-flash", + action="store_true", + help="Skip flashing (use an already-written root disk)", + ) + args = parser.parse_args() + + markers = list(DEFAULT_BOOT_MARKERS) + + with env() as client: + qemu = client.qemu + + if not args.skip_flash: + print(f"flashing {args.image!r}...", flush=True) + qemu.flasher.flash(args.image) + print("flash complete", flush=True) + + if args.disk_size: + print(f"set_disk_size({args.disk_size!r})", flush=True) + qemu.set_disk_size(args.disk_size) + + print("power on...", flush=True) + qemu.power.on() + + print(f"waiting up to {args.timeout}s for console markers {markers!r}...", flush=True) + try: + with PexpectAdapter(client=qemu.console) as p: + p.logfile = sys.stdout.buffer + idx = p.expect(markers + [pexpect.TIMEOUT, pexpect.EOF], timeout=args.timeout) + if idx >= len(markers): + print("FAILED: timed out / EOF waiting for Alpine boot marker", flush=True) + try: + qemu.power.off() + except Exception as exc: # noqa: BLE001 - best-effort cleanup + print(f"power off after failure also failed: {exc}", flush=True) + return 1 + print(f"OK: matched marker {markers[idx]!r}", flush=True) + finally: + print("power off...", flush=True) + qemu.power.off() + + print("done", flush=True) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/e2e/setup-e2e.sh b/e2e/setup-e2e.sh index 1eebb24ec..83ca2bdbe 100755 --- a/e2e/setup-e2e.sh +++ b/e2e/setup-e2e.sh @@ -338,6 +338,11 @@ main() { install_e2e_tools echo "" + + log_info "Prefetching Alpine guest image for ExporterSet QEMU..." + bash "$SCRIPT_DIR/scripts/ensure-qemu-guest-image.sh" > /dev/null + log_info "✓ QEMU guest image ready" + echo "" deploy_dex echo "" diff --git a/e2e/test/auth_logging_test.go b/e2e/test/auth_logging_test.go index 898e60c9b..26c96c794 100644 --- a/e2e/test/auth_logging_test.go +++ b/e2e/test/auth_logging_test.go @@ -50,7 +50,6 @@ var _ = Describe("Auth Failure Logging E2E Tests", Label("auth-logging"), Ordere var ( tracker *ProcessTracker - ns string tmpDir string exporterConfigPath string ) @@ -73,19 +72,14 @@ var _ = Describe("Auth Failure Logging E2E Tests", Label("auth-logging"), Ordere BeforeAll(func() { tracker = NewProcessTracker() - ns = Namespace() var err error tmpDir, err = os.MkdirTemp("", "jmp-e2e-authlog-*") Expect(err).NotTo(HaveOccurred()) exporterConfigPath = filepath.Join(tmpDir, exporterName+".yaml") - MustJmp("admin", "create", "client", "-n", ns, clientName, - "--unsafe", "--save") - - MustJmp("admin", "create", "exporter", "-n", ns, exporterName, - "--out", exporterConfigPath, - "--label", "example.com/board=authlog") + CreateLegacyClient(clientName) + CreateLegacyExporter(exporterName, exporterConfigPath, "example.com/board=authlog") // Give the exporter mock drivers so `jmp run` passes config // validation and reaches the controller registration step. @@ -95,8 +89,8 @@ var _ = Describe("Auth Failure Logging E2E Tests", Label("auth-logging"), Ordere AfterAll(func() { tracker.StopAll() - _, _ = Jmp("admin", "delete", "client", "--namespace", ns, clientName, "--delete") - _, _ = Jmp("admin", "delete", "exporter", "--namespace", ns, exporterName, "--delete") + DeleteClient(clientName) + DeleteExporter(exporterName) tracker.Cleanup() if tmpDir != "" { _ = os.RemoveAll(tmpDir) @@ -108,10 +102,7 @@ var _ = Describe("Auth Failure Logging E2E Tests", Label("auth-logging"), Ordere }) AfterEach(func() { - if CurrentSpecReport().Failed() { - tracker.DumpLogs(250) - DumpControllerLogs(250) - } + DumpOnFailure(250, tracker.DumpLogs) }) It("controller logs client authentication failures with the peer address", func() { diff --git a/e2e/test/exit_on_lease_end_test.go b/e2e/test/exit_on_lease_end_test.go index 09cee69ff..4ed04aa9f 100644 --- a/e2e/test/exit_on_lease_end_test.go +++ b/e2e/test/exit_on_lease_end_test.go @@ -27,22 +27,17 @@ import ( var _ = Describe("Exit On Lease End E2E Tests", Label("exit-on-lease-end"), Ordered, func() { var ( tracker *ProcessTracker - ns string exporterConfigPath string ) BeforeAll(func() { tracker = NewProcessTracker() - ns = Namespace() exporterConfigPath = SystemExporterConfigPath("test-exporter-exit-on-lease-end") - // Create client and exporter using legacy (token) auth — no OIDC/dex dependency. - MustJmp("admin", "create", "client", "-n", ns, "test-client-exit-on-lease-end", - "--unsafe", "--save") - - MustJmp("admin", "create", "exporter", "-n", ns, "test-exporter-exit-on-lease-end", - "--out", exporterConfigPath, - "--label", "example.com/board=exit-on-lease-end") + // Legacy (token) auth — no OIDC/dex dependency. + CreateLegacyClient("test-client-exit-on-lease-end") + CreateLegacyExporter("test-exporter-exit-on-lease-end", exporterConfigPath, + "example.com/board=exit-on-lease-end") // Merge the base exporter drivers + exitOnLeaseEnd overlay overlayPath := filepath.Join(RepoRoot(), "e2e", "exporters", "exporter-exit-on-lease-end.yaml") @@ -52,9 +47,8 @@ var _ = Describe("Exit On Lease End E2E Tests", Label("exit-on-lease-end"), Orde AfterAll(func() { tracker.StopAll() - // Clean up CRDs - _, _ = Jmp("admin", "delete", "client", "--namespace", ns, "test-client-exit-on-lease-end", "--delete") - _, _ = Jmp("admin", "delete", "exporter", "--namespace", ns, "test-exporter-exit-on-lease-end", "--delete") + DeleteClient("test-client-exit-on-lease-end") + DeleteExporter("test-exporter-exit-on-lease-end") tracker.Cleanup() }) @@ -64,13 +58,13 @@ var _ = Describe("Exit On Lease End E2E Tests", Label("exit-on-lease-end"), Orde }) AfterEach(func() { - if CurrentSpecReport().Failed() { - tracker.DumpLogs(250) - DumpControllerLogs(250) - } - // Stop any running exporter between tests + DumpOnFailure(250, tracker.DumpLogs) + // Stop any running exporter between tests and wait until it is gone. tracker.StopAll() - time.Sleep(time.Second) + Eventually(func() bool { + return tracker.IsProcessRunning() + }, 10*time.Second, 100*time.Millisecond).Should(BeFalse(), + "exporter process should stop after StopAll") }) It("exporter exits after serving one lease", func() { diff --git a/e2e/test/exporterset_qemu_test.go b/e2e/test/exporterset_qemu_test.go new file mode 100644 index 000000000..9784c01d8 --- /dev/null +++ b/e2e/test/exporterset_qemu_test.go @@ -0,0 +1,257 @@ +/* +Copyright 2026. The Jumpstarter Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + https://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package e2e + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "time" + + . "github.com/onsi/ginkgo/v2" //nolint:revive + . "github.com/onsi/gomega" //nolint:revive +) + +const ( + exporterSetQemuClientName = "test-client-exporterset-qemu" + exporterSetQemuSelector = "board=x86-64-virtual-e2e" + exporterSetQemuManifest = "e2e/manifests/exporterset-qemu-kind.yaml" +) + +var _ = Describe("ExporterSet QEMU E2E Tests", Label("exporterset-qemu"), Ordered, func() { + var ( + ns string + manifest string + imagePath string + script string + ) + + BeforeAll(func() { + ns = Namespace() + manifest = filepath.Join(RepoRoot(), exporterSetQemuManifest) + script = filepath.Join(RepoRoot(), "e2e", "scripts", "qemu_flash_boot.py") + + By("ensuring Alpine guest image is available") + out := MustRunCmd("bash", filepath.Join(RepoRoot(), "e2e", "scripts", "ensure-qemu-guest-image.sh")) + lines := strings.Split(strings.TrimSpace(out), "\n") + imagePath = lines[len(lines)-1] + Expect(imagePath).NotTo(BeEmpty()) + Expect(imagePath).To(BeAnExistingFile()) + + By("waiting for exporterset-controller Deployment") + WaitForDeploymentAvailable("component=exporterset-controller", 5*time.Minute) + + By("creating and logging in e2e client") + EnsureOIDCClient(exporterSetQemuClientName) + + By("applying ExporterSet QEMU kind manifest") + MustKubectl("apply", "-f", manifest) + }) + + AfterAll(func() { + By("cleaning up ExporterSet resources and client") + _, _ = Kubectl("delete", "--ignore-not-found", "-f", manifest) + DeleteClient(exporterSetQemuClientName) + }) + + AfterEach(func() { + DumpOnFailure(250, DumpExporterSetQemuLogs) + }) + + It("brings an Exporter Online with a Ready Pod", func() { + By("waiting for ExporterSet to create an exporter") + var exporterName string + Eventually(func() string { + out, _ := Kubectl("-n", ns, "get", "exporter", + "-l", exporterSetQemuSelector, + "-o", "jsonpath={.items[0].metadata.name}") + exporterName = out + return out + }, 5*time.Minute, 5*time.Second).ShouldNot(BeEmpty()) + + By(fmt.Sprintf("waiting for exporter %s Online/Registered/Available", exporterName)) + WaitForExporter(exporterName) + + By("waiting for Pod Ready") + Eventually(func() string { + out, _ := Kubectl("-n", ns, "get", "pod", exporterName, + "-o", "jsonpath={.status.phase}") + return out + }, 5*time.Minute, 5*time.Second).Should(Equal("Running")) + + Eventually(func() string { + out, _ := Kubectl("-n", ns, "get", "pod", exporterName, + "-o", "jsonpath={.status.containerStatuses[*].ready}") + return out + }, 5*time.Minute, 5*time.Second).Should(ContainSubstring("true")) + }) + + It("leases, flashes Alpine, and boots to a console login marker", func() { + By("waiting for a Running pod so we can read shared volume SizeLimit") + Eventually(func() string { + out, _ := Kubectl("-n", ns, "get", "pod", + "-l", exporterSetQemuSelector, + "--field-selector=status.phase=Running", + "-o", "jsonpath={.items[0].metadata.name}") + return out + }, 2*time.Minute, 5*time.Second).ShouldNot(BeEmpty()) + + sizeLimit, _ := Kubectl("-n", ns, "get", "pod", + "-l", exporterSetQemuSelector, + "--field-selector=status.phase=Running", + "-o", "jsonpath={.items[0].spec.volumes[?(@.name==\"shared\")].emptyDir.sizeLimit}") + // Without the storage follow-up (#924), SizeLimit stays at 100Mi and + // flashing Alpine evicts the Pod. Skip until capacity is available. + if sizeLimit == "" || sizeLimit == "100Mi" { + Skip(fmt.Sprintf("shared emptyDir SizeLimit=%q is too small for Alpine flash; needs #924 storage work", sizeLimit)) + } + + By("running flash+boot helper under jmp shell") + // Long timeout: Kind uses TCG emulation without KVM. + cmd := JmpCmd( + "shell", + "--client", exporterSetQemuClientName, + "--selector", exporterSetQemuSelector, + "--duration", "1h", + "--", + "python3", script, + "--timeout", "900", + "--disk-size", "2G", + imagePath, + ) + cmd.Env = append(os.Environ(), "JUMPSTARTER_GRPC_INSECURE=1") + out, err := cmd.CombinedOutput() + GinkgoWriter.Write(out) + Expect(err).NotTo(HaveOccurred(), "qemu_flash_boot.py failed: %s", string(out)) + Expect(string(out)).To(ContainSubstring("OK: matched marker")) + }) + + It("power cycles QEMU then rotates the Pod/Exporter and stays responsive", func() { + By("recording the current Running Pod name and UID") + var oldName, oldUID string + Eventually(func(g Gomega) { + out, err := Kubectl("-n", ns, "get", "pod", + "-l", exporterSetQemuSelector, + "--field-selector=status.phase=Running", + "-o", "jsonpath={.items[0].metadata.name}") + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(out).NotTo(BeEmpty()) + oldName = out + uid, err := Kubectl("-n", ns, "get", "pod", oldName, + "-o", "jsonpath={.metadata.uid}") + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(uid).NotTo(BeEmpty()) + oldUID = uid + }, 2*time.Minute, 5*time.Second).Should(Succeed()) + + By("running j qemu power on / power off under jmp shell") + // One lease: start QEMU via the runtime sidecar, stop it, then release + // so exitOnLeaseEnd completes the Pod and ExitAndReplace recycles it. + MustJmp("shell", "--client", exporterSetQemuClientName, + "--selector", exporterSetQemuSelector, + "--duration", "5m", + "--", "sh", "-c", "j qemu power on && j qemu power off") + + By("waiting for the old Pod/Exporter to be deleted and a single replacement Running") + Eventually(func(g Gomega) { + // Old Completed instance must be gone (controller deletes Exporter → cascade Pod). + _, err := Kubectl("-n", ns, "get", "pod", oldName) + g.Expect(err).To(HaveOccurred(), "old Pod %s should be deleted after ExitAndReplace", oldName) + + _, err = Kubectl("-n", ns, "get", "exporter", oldName) + g.Expect(err).To(HaveOccurred(), "old Exporter %s should be deleted after ExitAndReplace", oldName) + + podNames, err := Kubectl("-n", ns, "get", "pod", + "-l", exporterSetQemuSelector, + "--field-selector=status.phase=Running", + "-o", "jsonpath={range .items[*]}{.metadata.name}{' '}{end}") + g.Expect(err).NotTo(HaveOccurred()) + names := strings.Fields(strings.TrimSpace(podNames)) + g.Expect(names).To(HaveLen(1), "expected exactly one Running Pod, got %v", names) + + uid, err := Kubectl("-n", ns, "get", "pod", names[0], + "-o", "jsonpath={.metadata.uid}") + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(uid).NotTo(Equal(oldUID), "replacement Pod should have a new UID") + + ready, err := Kubectl("-n", ns, "get", "pod", names[0], + "-o", "jsonpath={.status.containerStatuses[*].ready}") + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(ready).To(ContainSubstring("true")) + + exporters, err := Kubectl("-n", ns, "get", "exporter", + "-l", exporterSetQemuSelector, + "-o", "jsonpath={range .items[*]}{.metadata.name}{' '}{end}") + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(strings.Fields(strings.TrimSpace(exporters))).To(HaveLen(1), + "expected exactly one Exporter after recycle, got %q", exporters) + }, 5*time.Minute, 5*time.Second).Should(Succeed()) + + By("waiting for the replacement exporter to become Available") + var exporterName string + Eventually(func() string { + out, _ := Kubectl("-n", ns, "get", "exporter", + "-l", exporterSetQemuSelector, + "-o", "jsonpath={.items[0].metadata.name}") + exporterName = out + return out + }, 2*time.Minute, 5*time.Second).ShouldNot(BeEmpty()) + WaitForExporter(exporterName) + + By("verifying the replacement still responds to qemu power on/off") + MustJmp("shell", "--client", exporterSetQemuClientName, + "--selector", exporterSetQemuSelector, + "--duration", "5m", + "--", "sh", "-c", "j qemu power on && j qemu power off") + }) +}) + +// DumpExporterSetQemuLogs prints recent logs from exporterset-controller and +// virtual QEMU pods for failure diagnosis. +func DumpExporterSetQemuLogs(maxLines int) { + ns := Namespace() + _, _ = fmt.Fprintf(GinkgoWriter, "=== ExporterSet / QEMU pod logs (last %d lines) ===\n", maxLines) + + out, _ := Kubectl("-n", ns, "get", "pods", + "-l", exporterSetQemuSelector, + "-o", "jsonpath={range .items[*]}{.metadata.name}{'\\n'}{end}") + for _, name := range strings.Split(strings.TrimSpace(out), "\n") { + if name == "" { + continue + } + _, _ = fmt.Fprintf(GinkgoWriter, "--- pod/%s exporter (main) ---\n", name) + logs, _ := Kubectl("-n", ns, "logs", name, "-c", "exporter", + "--tail", fmt.Sprintf("%d", maxLines)) + _, _ = fmt.Fprintln(GinkgoWriter, logs) + _, _ = fmt.Fprintf(GinkgoWriter, "--- pod/%s target-runtime (sidecar) ---\n", name) + logs, _ = Kubectl("-n", ns, "logs", name, "-c", "target-runtime", + "--tail", fmt.Sprintf("%d", maxLines)) + _, _ = fmt.Fprintln(GinkgoWriter, logs) + } + + out, _ = Kubectl("-n", ns, "get", "deploy", + "-o", "jsonpath={range .items[*]}{.metadata.name}{'\\n'}{end}") + for _, name := range strings.Split(strings.TrimSpace(out), "\n") { + if !strings.Contains(name, "exporterset") { + continue + } + _, _ = fmt.Fprintf(GinkgoWriter, "--- deploy/%s ---\n", name) + logs, _ := Kubectl("-n", ns, "logs", "deploy/"+name, "--tail", fmt.Sprintf("%d", maxLines)) + _, _ = fmt.Fprintln(GinkgoWriter, logs) + } +} diff --git a/e2e/test/hooks_test.go b/e2e/test/hooks_test.go index 14b2e7e56..66340d831 100644 --- a/e2e/test/hooks_test.go +++ b/e2e/test/hooks_test.go @@ -29,7 +29,6 @@ import ( var _ = Describe("Hooks E2E Tests", Label("hooks"), Ordered, func() { var ( tracker *ProcessTracker - ns string exporterConfigPath string ) @@ -64,34 +63,19 @@ var _ = Describe("Hooks E2E Tests", Label("hooks"), Ordered, func() { BeforeAll(func() { tracker = NewProcessTracker() - ns = Namespace() exporterConfigPath = SystemExporterConfigPath("test-exporter-hooks") - // Create client and exporter for hooks tests - MustJmp("admin", "create", "client", "-n", ns, "test-client-hooks", - "--unsafe", "--nointeractive", "--oidc-username", "dex:test-client-hooks") - - MustJmp("admin", "create", "exporter", "-n", ns, "test-exporter-hooks", - "--nointeractive", "--oidc-username", "dex:test-exporter-hooks", - "--label", "example.com/board=hooks") - - MustJmp("login", "--client", "test-client-hooks", - "--endpoint", Endpoint(), "--namespace", ns, "--name", "test-client-hooks", - "--issuer", "https://dex.dex.svc.cluster.local:5556", - "--username", "test-client-hooks@example.com", "--password", "password", "--unsafe") - - MustJmp("login", "--exporter-config", SystemExporterConfigPath("test-exporter-hooks"), - "--endpoint", Endpoint(), "--namespace", ns, "--name", "test-exporter-hooks", - "--issuer", "https://dex.dex.svc.cluster.local:5556", - "--username", "test-exporter-hooks@example.com", "--password", "password") + CreateOIDCClient("test-client-hooks") + CreateOIDCExporter("test-exporter-hooks", "example.com/board=hooks") + LoginOIDCClient("test-client-hooks") + LoginOIDCExporter("test-exporter-hooks") }) AfterAll(func() { tracker.StopAll() - // Clean up CRDs - _, _ = Jmp("admin", "delete", "client", "--namespace", ns, "test-client-hooks", "--delete") - _, _ = Jmp("admin", "delete", "exporter", "--namespace", ns, "test-exporter-hooks", "--delete") + DeleteClient("test-client-hooks") + DeleteExporter("test-exporter-hooks") tracker.Cleanup() }) diff --git a/e2e/test/utils.go b/e2e/test/utils.go index 58777e2a8..5088e3d64 100644 --- a/e2e/test/utils.go +++ b/e2e/test/utils.go @@ -41,6 +41,9 @@ const ( exporterPollPeriod = 500 * time.Millisecond exporterPostDelay = 2 * time.Second exporterProcessWait = 2 * time.Second + + // DexIssuer is the in-cluster Dex OIDC issuer used by e2e login helpers. + DexIssuer = "https://dex.dex.svc.cluster.local:5556" ) // --- Environment helpers --- @@ -227,6 +230,99 @@ func MustJmp(args ...string) string { return out } +// --- Client / exporter provisioning helpers --- + +// CreateOIDCClient creates a Jumpstarter client CR with Dex OIDC identity. +func CreateOIDCClient(name string) { + MustJmp("admin", "create", "client", "-n", Namespace(), name, + "--unsafe", "--nointeractive", + "--oidc-username", "dex:"+name) +} + +// LoginOIDCClient performs jmp login for a Dex-backed client (password "password"). +func LoginOIDCClient(name string) { + ns := Namespace() + MustJmp("login", "--client", name, + "--endpoint", Endpoint(), "--namespace", ns, "--name", name, + "--issuer", DexIssuer, + "--username", name+"@example.com", "--password", "password", "--unsafe") +} + +// EnsureOIDCClient deletes any existing client of this name, recreates it, and logs in. +func EnsureOIDCClient(name string) { + DeleteClient(name) + CreateOIDCClient(name) + LoginOIDCClient(name) +} + +// CreateLegacyClient creates a client with a controller-issued token (--save). +func CreateLegacyClient(name string) { + MustJmp("admin", "create", "client", "-n", Namespace(), name, "--unsafe", "--save") +} + +// DeleteClient best-effort deletes a client CR and its local credentials. +func DeleteClient(name string) { + _, _ = Jmp("admin", "delete", "client", "--namespace", Namespace(), name, "--delete") +} + +// CreateOIDCExporter creates an exporter CR with Dex OIDC identity and optional labels. +func CreateOIDCExporter(name string, labels ...string) { + args := []string{ + "admin", "create", "exporter", "-n", Namespace(), name, + "--nointeractive", "--oidc-username", "dex:" + name, + } + for _, label := range labels { + args = append(args, "--label", label) + } + MustJmp(args...) +} + +// LoginOIDCExporter performs jmp login for a Dex-backed exporter config. +func LoginOIDCExporter(name string) { + ns := Namespace() + MustJmp("login", "--exporter-config", SystemExporterConfigPath(name), + "--endpoint", Endpoint(), "--namespace", ns, "--name", name, + "--issuer", DexIssuer, + "--username", name+"@example.com", "--password", "password") +} + +// CreateLegacyExporter creates an exporter with token auth, writing config to outPath. +func CreateLegacyExporter(name, outPath string, labels ...string) { + args := []string{"admin", "create", "exporter", "-n", Namespace(), name, "--out", outPath} + for _, label := range labels { + args = append(args, "--label", label) + } + MustJmp(args...) +} + +// DeleteExporter best-effort deletes an exporter CR and its local credentials. +func DeleteExporter(name string) { + _, _ = Jmp("admin", "delete", "exporter", "--namespace", Namespace(), name, "--delete") +} + +// DumpOnFailure dumps controller logs (and optional extras) when the current +// Ginkgo spec failed. Intended for AfterEach hooks. +func DumpOnFailure(maxLines int, extras ...func(int)) { + if !CurrentSpecReport().Failed() { + return + } + DumpControllerLogs(maxLines) + for _, extra := range extras { + extra(maxLines) + } +} + +// WaitForDeploymentAvailable waits until a Deployment matching labelSelector is Available. +func WaitForDeploymentAvailable(labelSelector string, timeout time.Duration) { + ns := Namespace() + EventuallyWithOffset(1, func() error { + _, err := Kubectl("-n", ns, "wait", "--timeout=60s", + "--for=condition=Available", + "deployment", "-l", labelSelector) + return err + }, timeout, 5*time.Second).Should(Succeed()) +} + // Kubectl runs a kubectl command and returns the output. func Kubectl(args ...string) (string, error) { return RunCmd("kubectl", args...) diff --git a/e2e/testdata/.gitignore b/e2e/testdata/.gitignore new file mode 100644 index 000000000..19145ef20 --- /dev/null +++ b/e2e/testdata/.gitignore @@ -0,0 +1,3 @@ +# Guest images for ExporterSet QEMU e2e (downloaded / copied by ensure-qemu-guest-image.sh). +* +!.gitignore diff --git a/python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver.py b/python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver.py index f58c704bf..b346a17e1 100644 --- a/python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver.py +++ b/python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver.py @@ -483,7 +483,10 @@ def __post_init__(self): @property def _work_dir(self) -> str: if self.launcher_socket: - return "/shared" + # Sidecar: QEMU only sees the shared volume. Derive from the + # socket path so production (/shared/launcher.sock) and tests + # (tmpdir/shared/launcher.sock) both place cidata correctly. + return str(Path(self.launcher_socket).parent) return self._tmp_dir.name @property @@ -538,9 +541,13 @@ def validate_partition( return path def cidata(self) -> TemporaryDirectory: - tmp = TemporaryDirectory() - + # In sidecar mode QEMU runs in the runtime container and can only + # see paths on the shared volume — never the exporter's /tmp. + # Runtime runs as root (pod securityContext); TemporaryDirectory's + # default 0o700 is fine for cross-container read access. + tmp = TemporaryDirectory(dir=self._work_dir) path = Path(tmp.name) + (path / "meta-data").write_text( yaml.safe_dump( { diff --git a/python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver_test.py b/python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver_test.py index 3532b0416..c1e6c6b47 100644 --- a/python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver_test.py +++ b/python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver_test.py @@ -205,6 +205,34 @@ def test_set_memory_size_invalid(): driver.set_memory_size("invalid") +def test_cidata_uses_tmp_by_default(): + """Local mode keeps cloud-init vvfat content under a system temp dir.""" + driver = Qemu() + cidata = driver.cidata() + try: + assert Path(cidata.name).exists() + assert (Path(cidata.name) / "meta-data").is_file() + assert (Path(cidata.name) / "user-data").is_file() + assert not str(cidata.name).startswith("/shared") + finally: + cidata.cleanup() + + +def test_cidata_uses_shared_work_dir_with_launcher_socket(tmp_path): + """Sidecar mode must place cidata on the shared volume so QEMU can read it.""" + shared = tmp_path / "shared" + shared.mkdir() + # Real _work_dir derives from the launcher socket parent directory. + driver = Qemu(launcher_socket=str(shared / "launcher.sock")) + cidata = driver.cidata() + try: + assert Path(cidata.name).is_relative_to(shared) + assert (Path(cidata.name) / "meta-data").is_file() + assert (Path(cidata.name) / "user-data").is_file() + finally: + cidata.cleanup() + + # OCI Flash Tests diff --git a/python/packages/jumpstarter/jumpstarter/exporter/exporter.py b/python/packages/jumpstarter/jumpstarter/exporter/exporter.py index f19726a23..4312eabfe 100644 --- a/python/packages/jumpstarter/jumpstarter/exporter/exporter.py +++ b/python/packages/jumpstarter/jumpstarter/exporter/exporter.py @@ -54,6 +54,77 @@ grpc.StatusCode.UNIMPLEMENTED, }) +# Sidecar launcher socket injected by the ExporterSet QEMU provisioner. +_LAUNCHER_SOCKET_ENV = "JUMPSTARTER_LAUNCHER_SOCKET" +_DEFAULT_JMP_EXEC = "/shared/jumpstarter-exec" + + +def shutdown_runtime_sidecar( + *, + socket_path: str | None = None, + binary: str | None = None, + timeout: float = 10.0, +) -> bool: + """Ask jumpstarter-exec serve (runtime container PID 1) to exit. + + Required for ExitAndReplace: the runtime sidecar uses + ``restartPolicy: Always``, so exiting the exporter alone leaves the + runtime container running. Shutting down ``jumpstarter-exec serve`` + lets the Pod complete so the ExporterSet controller replaces it. + + Returns True if a shutdown was attempted, False if no launcher socket + is configured (non-sidecar / InPlaceReuse hosts). + + Callers on the async event loop must offload this via + ``await anyio.to_thread.run_sync(shutdown_runtime_sidecar)``. + """ + import os + import subprocess + from pathlib import Path + + sock = socket_path or os.environ.get(_LAUNCHER_SOCKET_ENV) + if not sock: + return False + + exec_bin = Path(binary) if binary else Path(_DEFAULT_JMP_EXEC) + if not exec_bin.is_file(): + # Fall back to the binary next to the socket (same shared volume). + candidate = Path(sock).parent / "jumpstarter-exec" + if candidate.is_file(): + exec_bin = candidate + else: + logger.warning( + "jumpstarter-exec binary not found at %s or %s; cannot shut down runtime", + _DEFAULT_JMP_EXEC, + candidate, + ) + return False + + cmd = [str(exec_bin), "shutdown", "--socket", sock] + logger.info("Shutting down runtime sidecar via %s", " ".join(cmd)) + try: + result = subprocess.run(cmd, check=False, capture_output=True, text=True, timeout=timeout) + except OSError as e: + # FileNotFoundError, PermissionError, and other pre-exec failures. + logger.warning("jumpstarter-exec not executable at %s: %s", exec_bin, e) + return False + except subprocess.TimeoutExpired: + logger.warning("jumpstarter-exec shutdown timed out after %ss", timeout) + return False + + if result.returncode != 0: + logger.warning( + "jumpstarter-exec shutdown exited %s: stdout=%r stderr=%r", + result.returncode, + result.stdout, + result.stderr, + ) + return False + + logger.info("Runtime sidecar shutdown acknowledged") + return True + + async def _standalone_shutdown_waiter(): """Wait forever; used so serve_standalone_tcp can be cancelled by stop().""" @@ -1093,6 +1164,7 @@ async def serve(self): # noqa: C901 if self.exit_on_lease_end and previous_leased: logger.info("Exporter configured to exit after lease, shutting down") + await anyio.to_thread.run_sync(shutdown_runtime_sidecar) self._stop_requested = True if self._stop_requested: @@ -1100,6 +1172,11 @@ async def serve(self): # noqa: C901 break self._previous_leased = current_leased + if self.exit_on_lease_end: + # Ensure the runtime container exits whenever this exporter is + # configured for ExitAndReplace (covers hook on_failure=exit and + # other stop paths that skip the lease-end branch above). + await anyio.to_thread.run_sync(shutdown_runtime_sidecar) self._tg = None self._status_drain_active = False clear_log_context() diff --git a/python/packages/jumpstarter/jumpstarter/exporter/exporter_test.py b/python/packages/jumpstarter/jumpstarter/exporter/exporter_test.py index 849bb8f3b..193cbbb97 100644 --- a/python/packages/jumpstarter/jumpstarter/exporter/exporter_test.py +++ b/python/packages/jumpstarter/jumpstarter/exporter/exporter_test.py @@ -1148,15 +1148,18 @@ async def fake_session(): return exporter -def _wire_status_stream(exporter, statuses): +def _wire_status_stream(exporter, statuses, sent: Event | None = None): """Replace _retry_stream with a function that feeds statuses into tx. The stream sends the provided statuses then waits indefinitely (until cancelled), matching production behavior where status streams are long-lived. + If ``sent`` is provided, it is set after all statuses have been queued. """ async def fake_retry_stream(name, factory, tx, **kwargs): for s in statuses: await tx.send(s) + if sent is not None: + sent.set() # Don't close - wait until task group cancels us (matches production) await anyio.sleep_forever() @@ -1183,7 +1186,9 @@ async def test_serve_stops_after_lease_ends(self): ]) _wire_handle_lease(exporter) - await exporter.serve() + with patch("jumpstarter.exporter.exporter.shutdown_runtime_sidecar") as shutdown: + await exporter.serve() + shutdown.assert_called() assert exporter._stop_requested is True @@ -1191,37 +1196,179 @@ async def test_serve_not_triggered_on_startup(self): """serve() does NOT set _stop_requested on startup when no lease has been served yet (previous_leased is False).""" exporter = _make_serve_exporter(exit_on_lease_end=True) + statuses_sent = Event() _wire_status_stream(exporter, [ MagicMock(leased=False, lease_name="", client_name=""), - ]) + ], sent=statuses_sent) - async with create_task_group() as tg: - tg.start_soon(exporter.serve) - # Give serve time to process the status - await anyio.sleep(0.1) - # Check that _stop_requested was NOT set - assert exporter._stop_requested is False - # Clean exit - tg.cancel_scope.cancel() + with patch("jumpstarter.exporter.exporter.shutdown_runtime_sidecar"): + async with create_task_group() as tg: + tg.start_soon(exporter.serve) + await statuses_sent.wait() + # Yield so serve() can process the queued status. + await anyio.sleep(0) + assert exporter._stop_requested is False + tg.cancel_scope.cancel() async def test_serve_continues_when_disabled(self): """serve() does NOT set _stop_requested after lease ends when exit_on_lease_end is False — the exporter loops for next lease.""" exporter = _make_serve_exporter(exit_on_lease_end=False) + statuses_sent = Event() _wire_status_stream(exporter, [ MagicMock(leased=True, lease_name="test-lease", client_name="test"), MagicMock(leased=False, lease_name="", client_name=""), - ]) + ], sent=statuses_sent) _wire_handle_lease(exporter) - async with create_task_group() as tg: - tg.start_soon(exporter.serve) - # Give serve time to process both statuses - await anyio.sleep(0.2) - # Check that _stop_requested was NOT set (exit_on_lease_end is False) - assert exporter._stop_requested is False - # Clean exit - tg.cancel_scope.cancel() + with patch("jumpstarter.exporter.exporter.shutdown_runtime_sidecar") as shutdown: + async with create_task_group() as tg: + tg.start_soon(exporter.serve) + await statuses_sent.wait() + # Wait until the lease→unleased transition has been applied. + with anyio.fail_after(2): + while not exporter._started or exporter._lease_context is not None: + await anyio.sleep(0.01) + assert exporter._stop_requested is False + shutdown.assert_not_called() + tg.cancel_scope.cancel() + + + +class TestShutdownRuntimeSidecar: + def test_noop_without_socket_env(self, monkeypatch): + from jumpstarter.exporter.exporter import shutdown_runtime_sidecar + + monkeypatch.delenv("JUMPSTARTER_LAUNCHER_SOCKET", raising=False) + assert shutdown_runtime_sidecar() is False + + def test_runs_shutdown_command(self, monkeypatch, tmp_path): + from jumpstarter.exporter.exporter import shutdown_runtime_sidecar + + sock = tmp_path / "launcher.sock" + sock.write_text("") # path only; connect is mocked via subprocess + binary = tmp_path / "jumpstarter-exec" + binary.write_text("#!/bin/sh\nexit 0\n") + binary.chmod(0o755) + + monkeypatch.setenv("JUMPSTARTER_LAUNCHER_SOCKET", str(sock)) + + with patch("subprocess.run") as run: + run.return_value = MagicMock(returncode=0, stdout="", stderr="") + assert shutdown_runtime_sidecar(binary=str(binary)) is True + run.assert_called_once() + args = run.call_args.args[0] + assert args[0] == str(binary) + assert args[1] == "shutdown" + assert "--socket" in args + assert str(sock) in args + + def test_falls_back_to_binary_beside_socket(self, monkeypatch, tmp_path): + """When the default path is missing, use jumpstarter-exec next to the socket.""" + from pathlib import Path + + from jumpstarter.exporter.exporter import shutdown_runtime_sidecar + + sock = tmp_path / "launcher.sock" + sock.write_text("") + binary = tmp_path / "jumpstarter-exec" + binary.write_text("#!/bin/sh\nexit 0\n") + binary.chmod(0o755) + + monkeypatch.setenv("JUMPSTARTER_LAUNCHER_SOCKET", str(sock)) + + real_is_file = Path.is_file + + def is_file_no_default(self): + # Treat the packaged default path as absent so we exercise fallback. + if str(self) == "/shared/jumpstarter-exec": + return False + return real_is_file(self) + + with ( + patch.object(Path, "is_file", is_file_no_default), + patch("subprocess.run") as run, + ): + run.return_value = MagicMock(returncode=0, stdout="", stderr="") + assert shutdown_runtime_sidecar() is True + assert run.call_args.args[0][0] == str(binary) + + monkeypatch.delenv("JUMPSTARTER_LAUNCHER_SOCKET", raising=False) + with ( + patch.object(Path, "is_file", is_file_no_default), + patch("subprocess.run") as run, + ): + run.return_value = MagicMock(returncode=0, stdout="", stderr="") + assert shutdown_runtime_sidecar(socket_path=str(sock)) is True + assert run.call_args.args[0][0] == str(binary) + + def test_missing_binary_returns_false(self, monkeypatch, tmp_path): + from jumpstarter.exporter.exporter import shutdown_runtime_sidecar + + sock = tmp_path / "launcher.sock" + sock.write_text("") + monkeypatch.setenv("JUMPSTARTER_LAUNCHER_SOCKET", str(sock)) + + missing = tmp_path / "does-not-exist" + assert shutdown_runtime_sidecar(binary=str(missing)) is False + + def test_file_not_found_returns_false(self, monkeypatch, tmp_path): + from jumpstarter.exporter.exporter import shutdown_runtime_sidecar + + sock = tmp_path / "launcher.sock" + sock.write_text("") + binary = tmp_path / "jumpstarter-exec" + binary.write_text("#!/bin/sh\nexit 0\n") + binary.chmod(0o755) + monkeypatch.setenv("JUMPSTARTER_LAUNCHER_SOCKET", str(sock)) + + with patch("subprocess.run", side_effect=FileNotFoundError): + assert shutdown_runtime_sidecar(binary=str(binary)) is False + + def test_permission_error_returns_false(self, monkeypatch, tmp_path): + from jumpstarter.exporter.exporter import shutdown_runtime_sidecar + + sock = tmp_path / "launcher.sock" + sock.write_text("") + binary = tmp_path / "jumpstarter-exec" + binary.write_text("#!/bin/sh\nexit 0\n") + binary.chmod(0o755) + monkeypatch.setenv("JUMPSTARTER_LAUNCHER_SOCKET", str(sock)) + + with patch("subprocess.run", side_effect=PermissionError("denied")): + assert shutdown_runtime_sidecar(binary=str(binary)) is False + + def test_timeout_returns_false(self, monkeypatch, tmp_path): + import subprocess + + from jumpstarter.exporter.exporter import shutdown_runtime_sidecar + + sock = tmp_path / "launcher.sock" + sock.write_text("") + binary = tmp_path / "jumpstarter-exec" + binary.write_text("#!/bin/sh\nexit 0\n") + binary.chmod(0o755) + monkeypatch.setenv("JUMPSTARTER_LAUNCHER_SOCKET", str(sock)) + + with patch( + "subprocess.run", + side_effect=subprocess.TimeoutExpired(cmd=["jumpstarter-exec"], timeout=1), + ): + assert shutdown_runtime_sidecar(binary=str(binary), timeout=1.0) is False + + def test_nonzero_exit_returns_false(self, monkeypatch, tmp_path): + from jumpstarter.exporter.exporter import shutdown_runtime_sidecar + + sock = tmp_path / "launcher.sock" + sock.write_text("") + binary = tmp_path / "jumpstarter-exec" + binary.write_text("#!/bin/sh\nexit 0\n") + binary.chmod(0o755) + monkeypatch.setenv("JUMPSTARTER_LAUNCHER_SOCKET", str(sock)) + + with patch("subprocess.run") as run: + run.return_value = MagicMock(returncode=1, stdout="", stderr="boom") + assert shutdown_runtime_sidecar(binary=str(binary)) is False class TestContextPropagation: diff --git a/rust/jumpstarter-exec/src/client.rs b/rust/jumpstarter-exec/src/client.rs index b6708bccc..d713ddd0c 100644 --- a/rust/jumpstarter-exec/src/client.rs +++ b/rust/jumpstarter-exec/src/client.rs @@ -47,6 +47,20 @@ pub fn exec(socket_path: &str, argv: Vec) -> std::io::Result { } }); + wait_for_exit(reader) +} + +/// Ask a running `serve` process to exit (used for ExitAndReplace recycle). +pub fn shutdown(socket_path: &str) -> std::io::Result { + let stream = UnixStream::connect(socket_path)?; + let reader = BufReader::new(stream.try_clone()?); + let writer: Arc> = Arc::new(Mutex::new(stream)); + + send(&writer, &ClientMessage::Shutdown)?; + wait_for_exit(reader) +} + +fn wait_for_exit(reader: BufReader) -> std::io::Result { let mut exit_code = 1; for line in reader.lines() { let line = line?; diff --git a/rust/jumpstarter-exec/src/main.rs b/rust/jumpstarter-exec/src/main.rs index 135b66fb1..a08177a1a 100644 --- a/rust/jumpstarter-exec/src/main.rs +++ b/rust/jumpstarter-exec/src/main.rs @@ -73,6 +73,18 @@ fn main() { } } } + "shutdown" => { + let rest = &args[2..]; + let socket = + parse_option(rest, "--socket").unwrap_or_else(|| DEFAULT_SOCKET.to_string()); + match jumpstarter_exec::client::shutdown(&socket) { + Ok(code) => process::exit(code), + Err(e) => { + eprintln!("jumpstarter-exec shutdown: {e}"); + process::exit(1); + } + } + } other => { eprintln!("jumpstarter-exec: unknown subcommand '{other}'"); usage(); @@ -82,7 +94,7 @@ fn main() { } fn usage() { - eprintln!("Usage: jumpstarter-exec [options]"); + eprintln!("Usage: jumpstarter-exec [options]"); eprintln!(); eprintln!("Subcommands:"); eprintln!(" serve [--socket ] [--debug] [--log-format json|text]"); @@ -90,6 +102,8 @@ fn usage() { eprintln!(" Listen for exec requests (JSON logs by default)"); eprintln!(" exec [--socket ] -- [...]"); eprintln!(" Execute a command remotely"); + eprintln!(" shutdown [--socket ]"); + eprintln!(" Ask serve to exit (ExitAndReplace / exitOnLeaseEnd)"); eprintln!(" version Print version and exit"); eprintln!(); eprintln!("Log context (JEP-0013 persistent fields on every line):"); diff --git a/rust/jumpstarter-exec/src/protocol.rs b/rust/jumpstarter-exec/src/protocol.rs index 7d5a7ff46..996894eb3 100644 --- a/rust/jumpstarter-exec/src/protocol.rs +++ b/rust/jumpstarter-exec/src/protocol.rs @@ -37,6 +37,13 @@ pub enum ClientMessage { #[serde(default)] cwd: Option, }, + /// Ask the serve process to exit (tear down the runtime container). + /// + /// Used by the exporter sidecar on `exitOnLeaseEnd` / ExitAndReplace so + /// the main container leaves and Kubernetes replaces the Pod. Without + /// this, a native sidecar with `restartPolicy: Always` would simply + /// restart while `jumpstarter-exec serve` kept running. + Shutdown, /// Stdin data for the running child process (base64-encoded). Stdin { data: String }, /// Close the child's stdin. @@ -101,6 +108,7 @@ mod tests { #[test] fn client_variants_roundtrip() { let cases: Vec<(&str, ClientMessage)> = vec![ + ("Shutdown", ClientMessage::Shutdown), ( "Stdin", ClientMessage::Stdin { diff --git a/rust/jumpstarter-exec/src/server.rs b/rust/jumpstarter-exec/src/server.rs index 341aaa972..a7f75c0e2 100644 --- a/rust/jumpstarter-exec/src/server.rs +++ b/rust/jumpstarter-exec/src/server.rs @@ -1,8 +1,11 @@ -use std::io::{BufRead, BufReader, Read, Write}; +use std::collections::HashSet; +use std::io::{BufRead, BufReader, ErrorKind, Read, Write}; use std::os::unix::net::{UnixListener, UnixStream}; use std::process::{Command, Stdio}; +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex}; use std::thread; +use std::time::Duration; use base64::{engine::general_purpose::STANDARD, Engine as _}; use serde_json::{json, Value}; @@ -12,6 +15,7 @@ use crate::protocol::{ClientMessage, ServerMessage}; extern "C" { fn kill(pid: i32, sig: i32) -> i32; + fn umask(mask: u32) -> u32; } /// Options for `serve`. @@ -35,6 +39,13 @@ impl Default for ServeOptions { } } +struct RuntimeState { + shutdown: AtomicBool, + /// PIDs of in-flight Exec children so Shutdown can SIGTERM them + /// before the process returns (avoids zombies when we are PID 1). + children: Mutex>, +} + /// Listen on `socket_path` with default options (JSON logs, debug off). pub fn serve(socket_path: &str) -> std::io::Result<()> { serve_with(socket_path, ServeOptions::default()) @@ -43,6 +54,20 @@ pub fn serve(socket_path: &str) -> std::io::Result<()> { /// Listen on `socket_path` with the given options. pub fn serve_with(socket_path: &str, opts: ServeOptions) -> std::io::Result<()> { let log = Arc::new(Logger::new(opts.log_format, opts.debug, opts.log_fields)); + let state = Arc::new(RuntimeState { + shutdown: AtomicBool::new(false), + children: Mutex::new(HashSet::new()), + }); + + // Shared-volume sidecar pattern: the exporter often runs as a different + // UID than this process (e.g. runtime root + exporter 65532). Clear the + // umask so Exec children (QEMU) create QMP/serial/VNC sockets that the + // peer can connect to (mode 0777). The listen socket is then tightened + // to 0666 below — still cross-UID, but not executable. + #[cfg(unix)] + unsafe { + umask(0); + } if std::path::Path::new(socket_path).exists() { if UnixStream::connect(socket_path).is_ok() { @@ -53,33 +78,59 @@ pub fn serve_with(socket_path: &str, opts: ServeOptions) -> std::io::Result<()> std::fs::remove_file(socket_path)?; } let listener = UnixListener::bind(socket_path)?; + listener.set_nonblocking(true)?; #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; - std::fs::set_permissions(socket_path, std::fs::Permissions::from_mode(0o660))?; + std::fs::set_permissions(socket_path, std::fs::Permissions::from_mode(0o666))?; } log.info( "listening", &[("socket", json!(socket_path)), ("debug", json!(opts.debug))], ); - for stream in listener.incoming() { - match stream { - Ok(s) => { + while !state.shutdown.load(Ordering::SeqCst) { + match listener.accept() { + Ok((s, _)) => { let log = Arc::clone(&log); + let state = Arc::clone(&state); + let socket = socket_path.to_string(); thread::spawn(move || { - if let Err(e) = handle_connection(s, Arc::clone(&log)) { + if let Err(e) = handle_connection(s, Arc::clone(&log), &socket, state) { log.error("connection error", &[("error", json!(e.to_string()))]); } }); } + Err(e) if e.kind() == ErrorKind::WouldBlock => { + thread::sleep(Duration::from_millis(50)); + } Err(e) => log.error("accept error", &[("error", json!(e.to_string()))]), } } + + // Best-effort: signal any remaining Exec children and give their + // handler threads a moment to wait()/reap before we return. + let leftover: Vec = state.children.lock().unwrap().iter().copied().collect(); + for &pid in &leftover { + unsafe { + kill(pid as i32, 15); + } + } + if !leftover.is_empty() { + thread::sleep(Duration::from_millis(200)); + } + + let _ = std::fs::remove_file(socket_path); + log.info("shutdown complete", &[]); Ok(()) } -fn handle_connection(stream: UnixStream, log: Arc) -> std::io::Result<()> { +fn handle_connection( + stream: UnixStream, + log: Arc, + socket_path: &str, + state: Arc, +) -> std::io::Result<()> { let reader = BufReader::new(stream.try_clone()?); let writer: Arc> = Arc::new(Mutex::new(stream)); let mut lines = reader.lines(); @@ -91,11 +142,40 @@ fn handle_connection(stream: UnixStream, log: Arc) -> std::io::Result<() let msg: ClientMessage = serde_json::from_str(&first_line).map_err(|e| io_err(&format!("invalid message: {e}")))?; - let (argv, env, cwd) = match msg { - ClientMessage::Exec { argv, env, cwd } => (argv, env, cwd), - _ => return Err(io_err("first message must be Exec")), - }; + match msg { + ClientMessage::Shutdown => { + log.info("shutdown requested", &[("socket", json!(socket_path))]); + // Acknowledge before stopping the accept loop so the client + // observes success. Do not process::exit — that skips Drop + // and can leave Exec children as zombies when we are PID 1. + send(&writer, &ServerMessage::Exit { code: Some(0) })?; + drop(writer); + let _ = std::fs::remove_file(socket_path); + let pids: Vec = state.children.lock().unwrap().iter().copied().collect(); + for pid in pids { + unsafe { + kill(pid as i32, 15); + } + } + state.shutdown.store(true, Ordering::SeqCst); + Ok(()) + } + ClientMessage::Exec { argv, env, cwd } => { + handle_exec(argv, env, cwd, lines, writer, log, state) + } + _ => Err(io_err("first message must be Exec or Shutdown")), + } +} +fn handle_exec( + argv: Vec, + env: Vec<(String, String)>, + cwd: Option, + lines: std::io::Lines>, + writer: Arc>, + log: Arc, + state: Arc, +) -> std::io::Result<()> { if argv.is_empty() { send( &writer, @@ -144,6 +224,7 @@ fn handle_connection(stream: UnixStream, log: Arc) -> std::io::Result<() })?; let pid = child.id(); + state.children.lock().unwrap().insert(pid); log.info( "exec started", &[("pid", json!(pid)), ("argv", json!(argv))], @@ -169,7 +250,7 @@ fn handle_connection(stream: UnixStream, log: Arc) -> std::io::Result<() let child_stdin = Arc::new(Mutex::new(child_stdin)); let stdin_ref = Arc::clone(&child_stdin); let log_in = Arc::clone(&log); - let reaped = Arc::new(std::sync::atomic::AtomicBool::new(false)); + let reaped = Arc::new(AtomicBool::new(false)); let reaped_ref = Arc::clone(&reaped); let _reader_handle = thread::spawn(move || { for line in lines { @@ -203,7 +284,7 @@ fn handle_connection(stream: UnixStream, log: Arc) -> std::io::Result<() *stdin_ref.lock().unwrap() = None; } ClientMessage::Signal { signal } => { - if reaped_ref.load(std::sync::atomic::Ordering::Acquire) { + if reaped_ref.load(Ordering::Acquire) { break; } log_in.debug("signal", &[("pid", json!(pid)), ("signal", json!(signal))]); @@ -212,13 +293,14 @@ fn handle_connection(stream: UnixStream, log: Arc) -> std::io::Result<() _ => {} } } - if !reaped_ref.load(std::sync::atomic::Ordering::Acquire) { + if !reaped_ref.load(Ordering::Acquire) { unsafe { kill(pid as i32, 15) }; // SIGTERM on client disconnect } }); let status = child.wait()?; - reaped.store(true, std::sync::atomic::Ordering::Release); + reaped.store(true, Ordering::Release); + state.children.lock().unwrap().remove(&pid); let _ = stdout_handle.join(); let _ = stderr_handle.join(); @@ -267,7 +349,7 @@ fn forward_output( break; } } - Err(ref e) if e.kind() == std::io::ErrorKind::Interrupted => continue, + Err(ref e) if e.kind() == ErrorKind::Interrupted => continue, Err(_) => break, } } diff --git a/rust/jumpstarter-exec/tests/integration.rs b/rust/jumpstarter-exec/tests/integration.rs index bd5484723..4301ae085 100644 --- a/rust/jumpstarter-exec/tests/integration.rs +++ b/rust/jumpstarter-exec/tests/integration.rs @@ -47,6 +47,20 @@ fn start_server_process() -> (Child, TempDir, String) { } assert!(sock.exists(), "server socket never appeared"); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mode = std::fs::metadata(&sock) + .expect("stat socket") + .permissions() + .mode() + & 0o777; + assert_eq!( + mode, 0o666, + "listen socket must be world-accessible for cross-UID sidecar peers, got {mode:#o}" + ); + } + (child, dir, path) } @@ -439,6 +453,39 @@ fn e2e_echo() { server.kill().ok(); } +#[test] +fn e2e_shutdown_exits_serve() { + let (mut server, _dir, sock) = start_server_process(); + + let status = Command::new(binary_path()) + .args(["shutdown", "--socket", &sock]) + .status() + .expect("failed to run jumpstarter-exec shutdown"); + assert!(status.success(), "shutdown should exit 0"); + + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5); + loop { + match server.try_wait() { + Ok(Some(wait)) => { + assert!( + wait.success(), + "serve should exit 0 after shutdown, got {wait}" + ); + return; + } + Ok(None) if std::time::Instant::now() < deadline => { + std::thread::sleep(std::time::Duration::from_millis(50)); + } + Ok(None) => { + let _ = server.kill(); + let _ = server.wait(); + panic!("serve did not exit within 5s after shutdown"); + } + Err(e) => panic!("try_wait failed: {e}"), + } + } +} + #[test] fn e2e_stderr() { let (mut server, _dir, sock) = start_server_process();