From 29726bf0dff63e074a91b0ac72cdd4d732eeabbd Mon Sep 17 00:00:00 2001 From: Miguel Angel Ajo Pelayo Date: Wed, 29 Jul 2026 14:23:45 +0200 Subject: [PATCH 1/2] feat: add guest disk storage for QEMU ExporterSet pods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Provision flashable /disk via PVC when storageClassName is set, otherwise emptyDir with ephemeral-storage accounting so lease → flash → boot can work. Co-authored-by: Cursor --- .../v1alpha1/exporterset_types.go | 6 + .../api/virtualtarget/v1alpha1/storage.go | 30 +++++ .../virtualtarget/v1alpha1/storage_test.go | 49 ++++++++ .../v1alpha1/virtualtargetclass_types.go | 8 ++ .../v1alpha1/zz_generated.deepcopy.go | 5 + ...altarget.jumpstarter.dev_exportersets.yaml | 6 + ....jumpstarter.dev_virtualtargetclasses.yaml | 8 ++ .../controller/jumpstarter/exporterset.go | 5 + .../jumpstarter/exporterset_test.go | 11 ++ controller/hack/sample-x86_64-kind.yaml | 6 + controller/hack/sample-x86_64.yaml | 6 + controller/internal/exporterset/disk/disk.go | 114 ++++++++++++++++++ .../internal/exporterset/disk/disk_test.go | 84 +++++++++++++ .../exporterset/provisioners/qemu/qemu.go | 66 +++++++++- .../provisioners/qemu/qemu_test.go | 112 ++++++++++++++++- controller/internal/exporterset/reconciler.go | 62 ++++++++++ .../jumpstarter_driver_qemu/driver.py | 26 +++- 17 files changed, 591 insertions(+), 13 deletions(-) create mode 100644 controller/api/virtualtarget/v1alpha1/storage.go create mode 100644 controller/api/virtualtarget/v1alpha1/storage_test.go create mode 100644 controller/internal/exporterset/disk/disk.go create mode 100644 controller/internal/exporterset/disk/disk_test.go diff --git a/controller/api/virtualtarget/v1alpha1/exporterset_types.go b/controller/api/virtualtarget/v1alpha1/exporterset_types.go index d71d710d0..d779ec981 100644 --- a/controller/api/virtualtarget/v1alpha1/exporterset_types.go +++ b/controller/api/virtualtarget/v1alpha1/exporterset_types.go @@ -133,6 +133,12 @@ type ExporterSetSpec struct { // +optional Images *ImageOverrides `json:"images,omitempty"` + // StorageClassName overrides VirtualTargetClass.spec.storageClassName for + // guest disk volumes. When nil, the class value is used. When set to an + // empty string, forces emptyDir even if the class names a StorageClass. + // +optional + StorageClassName *string `json:"storageClassName,omitempty"` + // Selector defines the label selector for matching exporters owned by this set. Selector metav1.LabelSelector `json:"selector"` diff --git a/controller/api/virtualtarget/v1alpha1/storage.go b/controller/api/virtualtarget/v1alpha1/storage.go new file mode 100644 index 000000000..6716aac85 --- /dev/null +++ b/controller/api/virtualtarget/v1alpha1/storage.go @@ -0,0 +1,30 @@ +/* +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 + + http://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 v1alpha1 + +// EffectiveStorageClassName returns the StorageClass to use for guest disk +// volumes. ExporterSet.spec.storageClassName overrides the class when set +// (including the empty string to force emptyDir). +func EffectiveStorageClassName(vtc *VirtualTargetClass, es *ExporterSet) string { + if es != nil && es.Spec.StorageClassName != nil { + return *es.Spec.StorageClassName + } + if vtc != nil { + return vtc.Spec.StorageClassName + } + return "" +} diff --git a/controller/api/virtualtarget/v1alpha1/storage_test.go b/controller/api/virtualtarget/v1alpha1/storage_test.go new file mode 100644 index 000000000..47d6169e7 --- /dev/null +++ b/controller/api/virtualtarget/v1alpha1/storage_test.go @@ -0,0 +1,49 @@ +/* +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 + + http://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 v1alpha1 + +import ( + "testing" +) + +func TestEffectiveStorageClassName(t *testing.T) { + empty := "" + override := "es-sc" + + tests := []struct { + name string + vtc string + es *string + want string + }{ + {name: "both empty", want: ""}, + {name: "vtc only", vtc: "vtc-sc", want: "vtc-sc"}, + {name: "es override", vtc: "vtc-sc", es: &override, want: "es-sc"}, + {name: "es clears to emptyDir", vtc: "vtc-sc", es: &empty, want: ""}, + {name: "es only", es: &override, want: "es-sc"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + vtc := &VirtualTargetClass{Spec: VirtualTargetClassSpec{StorageClassName: tt.vtc}} + es := &ExporterSet{Spec: ExporterSetSpec{StorageClassName: tt.es}} + if got := EffectiveStorageClassName(vtc, es); got != tt.want { + t.Errorf("EffectiveStorageClassName() = %q, want %q", got, tt.want) + } + }) + } +} diff --git a/controller/api/virtualtarget/v1alpha1/virtualtargetclass_types.go b/controller/api/virtualtarget/v1alpha1/virtualtargetclass_types.go index e0f3de6bb..2d2dc99a3 100644 --- a/controller/api/virtualtarget/v1alpha1/virtualtargetclass_types.go +++ b/controller/api/virtualtarget/v1alpha1/virtualtargetclass_types.go @@ -141,6 +141,14 @@ type VirtualTargetClassSpec struct { // ExporterSet-level images take precedence over these class-level defaults. // +optional Images *ImageOverrides `json:"images,omitempty"` + + // StorageClassName selects the StorageClass for the guest disk PVC. + // When empty, the provisioner uses an emptyDir volume sized from + // parameters.resources.storage and sets ephemeral-storage + // requests/limits so the scheduler accounts for local disk usage. + // ExporterSet.spec.storageClassName can override this value. + // +optional + StorageClassName string `json:"storageClassName,omitempty"` } // +kubebuilder:object:root=true diff --git a/controller/api/virtualtarget/v1alpha1/zz_generated.deepcopy.go b/controller/api/virtualtarget/v1alpha1/zz_generated.deepcopy.go index a8485d19b..ed7971c0f 100644 --- a/controller/api/virtualtarget/v1alpha1/zz_generated.deepcopy.go +++ b/controller/api/virtualtarget/v1alpha1/zz_generated.deepcopy.go @@ -168,6 +168,11 @@ func (in *ExporterSetSpec) DeepCopyInto(out *ExporterSetSpec) { *out = new(ImageOverrides) (*in).DeepCopyInto(*out) } + if in.StorageClassName != nil { + in, out := &in.StorageClassName, &out.StorageClassName + *out = new(string) + **out = **in + } in.Selector.DeepCopyInto(&out.Selector) in.Template.DeepCopyInto(&out.Template) } diff --git a/controller/deploy/operator/config/crd/bases/virtualtarget.jumpstarter.dev_exportersets.yaml b/controller/deploy/operator/config/crd/bases/virtualtarget.jumpstarter.dev_exportersets.yaml index 36c740db3..a428f8f3b 100644 --- a/controller/deploy/operator/config/crd/bases/virtualtarget.jumpstarter.dev_exportersets.yaml +++ b/controller/deploy/operator/config/crd/bases/virtualtarget.jumpstarter.dev_exportersets.yaml @@ -186,6 +186,12 @@ spec: type: object type: object x-kubernetes-map-type: atomic + storageClassName: + description: |- + StorageClassName overrides VirtualTargetClass.spec.storageClassName for + guest disk volumes. When nil, the class value is used. When set to an + empty string, forces emptyDir even if the class names a StorageClass. + type: string template: description: Template defines the exporter template for instances created by this set. diff --git a/controller/deploy/operator/config/crd/bases/virtualtarget.jumpstarter.dev_virtualtargetclasses.yaml b/controller/deploy/operator/config/crd/bases/virtualtarget.jumpstarter.dev_virtualtargetclasses.yaml index 598887b6c..01e191e78 100644 --- a/controller/deploy/operator/config/crd/bases/virtualtarget.jumpstarter.dev_virtualtargetclasses.yaml +++ b/controller/deploy/operator/config/crd/bases/virtualtarget.jumpstarter.dev_virtualtargetclasses.yaml @@ -262,6 +262,14 @@ spec: type: object type: array type: object + storageClassName: + description: |- + StorageClassName selects the StorageClass for the guest disk PVC. + When empty, the provisioner uses an emptyDir volume sized from + parameters.resources.storage and sets ephemeral-storage + requests/limits so the scheduler accounts for local disk usage. + ExporterSet.spec.storageClassName can override this value. + type: string required: - provisioner type: object diff --git a/controller/deploy/operator/internal/controller/jumpstarter/exporterset.go b/controller/deploy/operator/internal/controller/jumpstarter/exporterset.go index 46a842cc3..7e14d2505 100644 --- a/controller/deploy/operator/internal/controller/jumpstarter/exporterset.go +++ b/controller/deploy/operator/internal/controller/jumpstarter/exporterset.go @@ -522,6 +522,11 @@ func exporterSetPolicyRules() []rbacv1.PolicyRule { Resources: []string{"pods"}, Verbs: []string{"get", "list", "watch", "create", "update", "patch", "delete"}, }, + { + APIGroups: []string{""}, + Resources: []string{"persistentvolumeclaims"}, + Verbs: []string{"get", "list", "watch", "create", "update", "patch", "delete"}, + }, { APIGroups: []string{""}, Resources: []string{"events"}, diff --git a/controller/deploy/operator/internal/controller/jumpstarter/exporterset_test.go b/controller/deploy/operator/internal/controller/jumpstarter/exporterset_test.go index e58351745..033a6bb57 100644 --- a/controller/deploy/operator/internal/controller/jumpstarter/exporterset_test.go +++ b/controller/deploy/operator/internal/controller/jumpstarter/exporterset_test.go @@ -181,6 +181,17 @@ var _ = Describe("exporterSetPolicyRules", func() { Fail("no rule found granting full CRUD on pods") }) + It("should grant full CRUD on persistentvolumeclaims", func() { + for _, rule := range rules { + if containsString(rule.APIGroups, "") && + containsString(rule.Resources, "persistentvolumeclaims") { + Expect(rule.Verbs).To(ContainElements("get", "list", "watch", "create", "update", "patch", "delete")) + return + } + } + Fail("no rule found granting full CRUD on persistentvolumeclaims") + }) + It("should grant full CRUD on exporters", func() { for _, rule := range rules { if containsString(rule.APIGroups, "jumpstarter.dev") && diff --git a/controller/hack/sample-x86_64-kind.yaml b/controller/hack/sample-x86_64-kind.yaml index 53228d2c1..c8387f4fa 100644 --- a/controller/hack/sample-x86_64-kind.yaml +++ b/controller/hack/sample-x86_64-kind.yaml @@ -8,6 +8,12 @@ # acceleration, which is fine for a functional/architecture smoke test # but much slower than a real x86_64+KVM cluster. # +# Guest disk uses emptyDir sized from parameters.resources.storage (no +# storageClassName). The provisioner also sets ephemeral-storage +# requests/limits so the scheduler accounts for local disk. To use a +# PVC instead, set spec.storageClassName on the VirtualTargetClass or +# override it on the ExporterSet. +# # For real deployments with KVM acceleration, use sample-x86_64.yaml # on a cluster that exposes /dev/kvm via the kubevirt device plugin # and taints its KVM-capable nodes with jumpstarter.dev/kvm. diff --git a/controller/hack/sample-x86_64.yaml b/controller/hack/sample-x86_64.yaml index 3e78fbb00..baaf7bd4a 100644 --- a/controller/hack/sample-x86_64.yaml +++ b/controller/hack/sample-x86_64.yaml @@ -7,6 +7,10 @@ # This creates: # - A VirtualTargetClass for x86_64 QEMU VMs with KVM acceleration # - An ExporterSet that manages a pool of virtual exporters +# +# Guest disk: omit storageClassName to use emptyDir (with ephemeral-storage +# accounting), or set storageClassName to provision a per-exporter PVC +# mounted at /disk. ExporterSet.spec.storageClassName overrides the class. --- apiVersion: virtualtarget.jumpstarter.dev/v1alpha1 kind: VirtualTargetClass @@ -17,6 +21,7 @@ spec: provisioner: qemu.jumpstarter.dev bindingMode: Immediate reclaimPolicy: Delete + # storageClassName: "your-storage-class" # optional; omit → emptyDir scheduling: nodeSelector: kubernetes.io/arch: amd64 @@ -50,6 +55,7 @@ spec: scaleDownCooldown: 5m recycleStrategy: ExitAndReplace virtualTargetClassName: qemu-x86-64 + # storageClassName: "override-sc" # optional override ("" forces emptyDir) selector: matchLabels: board: x86-64-virtual diff --git a/controller/internal/exporterset/disk/disk.go b/controller/internal/exporterset/disk/disk.go new file mode 100644 index 000000000..dfa8ba8b5 --- /dev/null +++ b/controller/internal/exporterset/disk/disk.go @@ -0,0 +1,114 @@ +/* +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 + + http://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 disk provides shared helpers for guest-disk volume provisioning +// used by ExporterSet provisioners and the reconciler. +package disk + +import ( + "fmt" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +const ( + // VolumeName is the Pod volume name for guest disk storage. + VolumeName = "disk" + + // MountPath is where guest disk storage is mounted in exporter and runtime. + MountPath = "/disk" + + // DefaultSize is used when parameters.resources.storage is unset. + DefaultSize = "10Gi" + + pvcNamePrefix = "disk-" +) + +// PVCName returns the per-exporter PersistentVolumeClaim name. +func PVCName(exporterName string) string { + return pvcNamePrefix + exporterName +} + +// SizeFromParameters reads parameters.resources.storage, defaulting to DefaultSize. +func SizeFromParameters(params map[string]interface{}) (resource.Quantity, error) { + raw := DefaultSize + if params != nil { + if resources, ok := params["resources"].(map[string]interface{}); ok { + switch v := resources["storage"].(type) { + case string: + if v != "" { + raw = v + } + case float64: + // JSON numbers land as float64; treat as Gi if unitless is awkward — + // require string quantities in the API. + return resource.Quantity{}, fmt.Errorf("parameters.resources.storage must be a string quantity (e.g. \"10Gi\"), got number %v", v) + case nil: + // use default + default: + return resource.Quantity{}, fmt.Errorf("parameters.resources.storage must be a string quantity (e.g. \"10Gi\"), got %T", v) + } + } + } + + qty, err := resource.ParseQuantity(raw) + if err != nil { + return resource.Quantity{}, fmt.Errorf("parse parameters.resources.storage %q: %w", raw, err) + } + return qty, nil +} + +// BuildPVC constructs a guest-disk PVC owned by the given exporter metadata. +func BuildPVC(namespace, exporterName, storageClassName string, size resource.Quantity, labels map[string]string) *corev1.PersistentVolumeClaim { + return &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: PVCName(exporterName), + Namespace: namespace, + Labels: labels, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{ + corev1.ReadWriteOnce, + }, + StorageClassName: &storageClassName, + Resources: corev1.VolumeResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceStorage: size, + }, + }, + }, + } +} + +// SetEphemeralStorage ensures requests and limits include ephemeral-storage +// equal to size (used when guest disk is backed by emptyDir). +func SetEphemeralStorage(resources *corev1.ResourceRequirements, size resource.Quantity) { + if resources.Requests == nil { + resources.Requests = corev1.ResourceList{} + } + if resources.Limits == nil { + resources.Limits = corev1.ResourceList{} + } + // Only set when unset so explicit scheduling.resources win. + if _, ok := resources.Requests[corev1.ResourceEphemeralStorage]; !ok { + resources.Requests[corev1.ResourceEphemeralStorage] = size.DeepCopy() + } + if _, ok := resources.Limits[corev1.ResourceEphemeralStorage]; !ok { + resources.Limits[corev1.ResourceEphemeralStorage] = size.DeepCopy() + } +} diff --git a/controller/internal/exporterset/disk/disk_test.go b/controller/internal/exporterset/disk/disk_test.go new file mode 100644 index 000000000..22e2d1deb --- /dev/null +++ b/controller/internal/exporterset/disk/disk_test.go @@ -0,0 +1,84 @@ +/* +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 + + http://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 disk + +import ( + "testing" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" +) + +func TestPVCName(t *testing.T) { + if got := PVCName("exp-1"); got != "disk-exp-1" { + t.Errorf("PVCName() = %q, want disk-exp-1", got) + } +} + +func TestSizeFromParameters(t *testing.T) { + qty, err := SizeFromParameters(nil) + if err != nil { + t.Fatalf("nil params: %v", err) + } + if !qty.Equal(resource.MustParse(DefaultSize)) { + t.Errorf("default = %v, want %s", qty, DefaultSize) + } + + qty, err = SizeFromParameters(map[string]interface{}{ + "resources": map[string]interface{}{"storage": "15Gi"}, + }) + if err != nil { + t.Fatalf("15Gi: %v", err) + } + if !qty.Equal(resource.MustParse("15Gi")) { + t.Errorf("got %v, want 15Gi", qty) + } + + _, err = SizeFromParameters(map[string]interface{}{ + "resources": map[string]interface{}{"storage": 10.0}, + }) + if err == nil { + t.Fatal("expected error for numeric storage") + } +} + +func TestSetEphemeralStorage(t *testing.T) { + size := resource.MustParse("10Gi") + res := corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("1"), + }, + } + SetEphemeralStorage(&res, size) + if !res.Requests[corev1.ResourceEphemeralStorage].Equal(size) { + t.Errorf("request = %v, want %v", res.Requests[corev1.ResourceEphemeralStorage], size) + } + if !res.Limits[corev1.ResourceEphemeralStorage].Equal(size) { + t.Errorf("limit = %v, want %v", res.Limits[corev1.ResourceEphemeralStorage], size) + } + if !res.Requests[corev1.ResourceCPU].Equal(resource.MustParse("1")) { + t.Error("cpu request should be preserved") + } + + // Does not overwrite existing ephemeral-storage. + custom := resource.MustParse("1Gi") + res.Requests[corev1.ResourceEphemeralStorage] = custom + SetEphemeralStorage(&res, size) + if !res.Requests[corev1.ResourceEphemeralStorage].Equal(custom) { + t.Errorf("should preserve explicit ephemeral-storage, got %v", res.Requests[corev1.ResourceEphemeralStorage]) + } +} diff --git a/controller/internal/exporterset/provisioners/qemu/qemu.go b/controller/internal/exporterset/provisioners/qemu/qemu.go index 9043e1d26..f1a49c280 100644 --- a/controller/internal/exporterset/provisioners/qemu/qemu.go +++ b/controller/internal/exporterset/provisioners/qemu/qemu.go @@ -30,6 +30,7 @@ import ( jumpstarterdevv1alpha1 "github.com/jumpstarter-dev/jumpstarter/controller/api/v1alpha1" virtualtargetv1alpha1 "github.com/jumpstarter-dev/jumpstarter/controller/api/virtualtarget/v1alpha1" + "github.com/jumpstarter-dev/jumpstarter/controller/internal/exporterset/disk" corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -154,7 +155,10 @@ func (p *Provisioner) resolveImageSpec(spec *virtualtargetv1alpha1.ImageSpec, de // 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. +// - Shared emptyDir for Unix sockets (QMP, serial, launcher). +// - Guest disk volume at /disk (ephemeral PVC when +// parameters.storage.storageClassName is set, otherwise sized +// emptyDir with ephemeral-storage requests/limits). // // The caller (reconciler) is responsible for setting // OwnerReferences on the Pod and injecting the config volume. @@ -172,6 +176,11 @@ func (p *Provisioner) RenderPod( runAsExporter := exporterNonRootUID exporterNonRoot := true + diskSize, err := disk.SizeFromParameters(mergedParameters) + if err != nil { + return nil, err + } + var exporterSpec, runtimeSpec *virtualtargetv1alpha1.ImageSpec if images != nil { exporterSpec = images.Exporter @@ -194,6 +203,11 @@ func (p *Provisioner) RenderPod( }) } + diskMount := corev1.VolumeMount{ + Name: disk.VolumeName, + MountPath: disk.MountPath, + } + podMeta := metav1.ObjectMeta{ Namespace: exporterSet.Namespace, Labels: maps.Clone(exporterSet.Spec.Template.Metadata.Labels), @@ -248,6 +262,7 @@ func (p *Provisioner) RenderPod( Name: sharedVolumeName, MountPath: sharedMountPath, }, + diskMount, }, }, }, @@ -275,6 +290,7 @@ func (p *Provisioner) RenderPod( Name: sharedVolumeName, MountPath: sharedMountPath, }, + diskMount, }, }, }, @@ -291,6 +307,11 @@ func (p *Provisioner) RenderPod( }, } + storageClass := virtualtargetv1alpha1.EffectiveStorageClassName(vtc, exporterSet) + if err := attachDiskVolume(pod, exporter, storageClass, diskSize); err != nil { + return nil, err + } + // Apply scheduling from VirtualTargetClass. // Clone maps and slices to avoid mutating the VTC's fields. if vtc.Spec.Scheduling != nil { @@ -311,9 +332,52 @@ func (p *Provisioner) RenderPod( } } + // emptyDir guest disks consume node ephemeral storage — ensure the + // scheduler and kubelet account for it on containers that mount /disk. + if storageClass == "" { + disk.SetEphemeralStorage(&pod.Spec.Containers[0].Resources, diskSize) + for i := range pod.Spec.InitContainers { + if pod.Spec.InitContainers[i].Name == "exporter" { + disk.SetEphemeralStorage(&pod.Spec.InitContainers[i].Resources, diskSize) + } + } + } + return pod, nil } +// attachDiskVolume appends the guest disk volume. When storageClass is set, +// the volume references a PVC that the reconciler creates; otherwise an +// emptyDir sized to diskSize is used. +func attachDiskVolume( + pod *corev1.Pod, + exporter *jumpstarterdevv1alpha1.Exporter, + storageClass string, + diskSize resource.Quantity, +) error { + vol := corev1.Volume{Name: disk.VolumeName} + if storageClass != "" { + if exporter == nil { + return fmt.Errorf("disk PVC requires an Exporter to derive the claim name") + } + claimName := disk.PVCName(exporter.Name) + vol.VolumeSource = corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: claimName, + }, + } + } else { + size := diskSize.DeepCopy() + vol.VolumeSource = corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{ + SizeLimit: &size, + }, + } + } + pod.Spec.Volumes = append(pod.Spec.Volumes, vol) + return nil +} + // EnrichExporterExport injects QEMU-specific driver configuration: // - Forces launcher_socket on the QEMU driver entry // - Defaults arch/smp/mem/disk_size from mergedParameters if not set diff --git a/controller/internal/exporterset/provisioners/qemu/qemu_test.go b/controller/internal/exporterset/provisioners/qemu/qemu_test.go index 8edf567e5..a09eda6df 100644 --- a/controller/internal/exporterset/provisioners/qemu/qemu_test.go +++ b/controller/internal/exporterset/provisioners/qemu/qemu_test.go @@ -98,17 +98,45 @@ func assertRenderPodMetadata(t *testing.T, pod *corev1.Pod, exporterSet *virtual func assertRenderPodSharedVolume(t *testing.T, pod *corev1.Pod) { t.Helper() - // Only shared volume — config volume is injected by the reconciler. - if len(pod.Spec.Volumes) != 1 { - t.Fatalf("expected 1 volume (shared), got %d", len(pod.Spec.Volumes)) + // Shared emptyDir + guest disk emptyDir (default size). Config volume is + // injected by the reconciler. + if len(pod.Spec.Volumes) != 2 { + t.Fatalf("expected 2 volumes (shared + disk), got %d", len(pod.Spec.Volumes)) } - if pod.Spec.Volumes[0].EmptyDir == nil { - t.Fatal("expected shared emptyDir volume at index 0") + if pod.Spec.Volumes[0].Name != sharedVolumeName || pod.Spec.Volumes[0].EmptyDir == nil { + t.Fatalf("expected shared emptyDir volume at index 0, got %#v", pod.Spec.Volumes[0]) } wantLimit := resource.MustParse(sharedVolumeSizeLimit) if pod.Spec.Volumes[0].EmptyDir.SizeLimit == nil || !pod.Spec.Volumes[0].EmptyDir.SizeLimit.Equal(wantLimit) { - t.Errorf("SizeLimit = %v, want %v", pod.Spec.Volumes[0].EmptyDir.SizeLimit, wantLimit) + t.Errorf("shared SizeLimit = %v, want %v", pod.Spec.Volumes[0].EmptyDir.SizeLimit, wantLimit) + } + if pod.Spec.Volumes[1].Name != "disk" || pod.Spec.Volumes[1].EmptyDir == nil { + t.Fatalf("expected disk emptyDir volume at index 1, got %#v", pod.Spec.Volumes[1]) + } + wantDisk := resource.MustParse("10Gi") + if pod.Spec.Volumes[1].EmptyDir.SizeLimit == nil || + !pod.Spec.Volumes[1].EmptyDir.SizeLimit.Equal(wantDisk) { + t.Errorf("disk SizeLimit = %v, want %v", pod.Spec.Volumes[1].EmptyDir.SizeLimit, wantDisk) + } + + ephemeral := pod.Spec.Containers[0].Resources.Requests[corev1.ResourceEphemeralStorage] + if !ephemeral.Equal(wantDisk) { + t.Errorf("runtime ephemeral-storage request = %v, want %v", ephemeral, wantDisk) + } + exporterEphemeral := pod.Spec.InitContainers[1].Resources.Requests[corev1.ResourceEphemeralStorage] + if !exporterEphemeral.Equal(wantDisk) { + t.Errorf("exporter ephemeral-storage request = %v, want %v", exporterEphemeral, wantDisk) + } + + hasDiskMount := false + for _, m := range pod.Spec.Containers[0].VolumeMounts { + if m.Name == "disk" && m.MountPath == "/disk" { + hasDiskMount = true + } + } + if !hasDiskMount { + t.Error("target-runtime missing /disk mount") } } @@ -421,3 +449,75 @@ func TestRenderPod_partialImageOverride(t *testing.T) { t.Errorf("exporter image = %q, want %q", pod.Spec.Containers[0].Image, wantExporter) } } + +func TestRenderPod_diskPVCWhenStorageClassSet(t *testing.T) { + exporterSet := &virtualtargetv1alpha1.ExporterSet{ + ObjectMeta: metav1.ObjectMeta{Name: "demo-set", Namespace: "default"}, + Spec: virtualtargetv1alpha1.ExporterSetSpec{ + StorageClassName: ptr("fast-ssd"), + }, + } + vtc := &virtualtargetv1alpha1.VirtualTargetClass{ + Spec: virtualtargetv1alpha1.VirtualTargetClassSpec{ + Provisioner: ProvisionerName, + StorageClassName: "ignored-because-override", + }, + } + exporter := &jumpstarterdevv1alpha1.Exporter{ + ObjectMeta: metav1.ObjectMeta{Name: "demo-exporter", Namespace: "default"}, + } + params := map[string]interface{}{ + "resources": map[string]interface{}{ + "storage": "20Gi", + }, + } + + pod, err := New("dev").RenderPod(context.Background(), exporterSet, vtc, params, nil, exporter) + if err != nil { + t.Fatalf("RenderPod() error = %v", err) + } + + var diskVol *corev1.Volume + for i := range pod.Spec.Volumes { + if pod.Spec.Volumes[i].Name == "disk" { + diskVol = &pod.Spec.Volumes[i] + break + } + } + if diskVol == nil || diskVol.PersistentVolumeClaim == nil { + t.Fatalf("expected disk PVC volume, got %#v", diskVol) + } + if diskVol.PersistentVolumeClaim.ClaimName != "disk-demo-exporter" { + t.Errorf("ClaimName = %q, want disk-demo-exporter", diskVol.PersistentVolumeClaim.ClaimName) + } + if _, ok := pod.Spec.Containers[0].Resources.Requests[corev1.ResourceEphemeralStorage]; ok { + t.Error("PVC mode should not set ephemeral-storage for guest disk") + } +} + +func TestRenderPod_diskEmptyDirUsesParamSize(t *testing.T) { + exporterSet := &virtualtargetv1alpha1.ExporterSet{ + ObjectMeta: metav1.ObjectMeta{Name: "demo-set", Namespace: "default"}, + } + vtc := &virtualtargetv1alpha1.VirtualTargetClass{ + Spec: virtualtargetv1alpha1.VirtualTargetClassSpec{Provisioner: ProvisionerName}, + } + params := map[string]interface{}{ + "resources": map[string]interface{}{ + "storage": "7Gi", + }, + } + + pod, err := New("dev").RenderPod(context.Background(), exporterSet, vtc, params, nil, nil) + if err != nil { + t.Fatalf("RenderPod() error = %v", err) + } + + want := resource.MustParse("7Gi") + diskVol := pod.Spec.Volumes[1] + if diskVol.EmptyDir == nil || diskVol.EmptyDir.SizeLimit == nil || !diskVol.EmptyDir.SizeLimit.Equal(want) { + t.Errorf("disk SizeLimit = %v, want %v", diskVol.EmptyDir, want) + } +} + +func ptr(s string) *string { return &s } diff --git a/controller/internal/exporterset/reconciler.go b/controller/internal/exporterset/reconciler.go index 8491e875b..3408f8890 100644 --- a/controller/internal/exporterset/reconciler.go +++ b/controller/internal/exporterset/reconciler.go @@ -57,6 +57,7 @@ import ( jumpstarterdevv1alpha1 "github.com/jumpstarter-dev/jumpstarter/controller/api/v1alpha1" virtualtargetv1alpha1 "github.com/jumpstarter-dev/jumpstarter/controller/api/virtualtarget/v1alpha1" + "github.com/jumpstarter-dev/jumpstarter/controller/internal/exporterset/disk" ) const ( @@ -459,6 +460,7 @@ func (r *ExporterSetReconciler) syncConfigSecret( // createExporterPod issues a Pod for a single Exporter that has credentials // but no Pod yet. The config Secret must already exist (syncConfigSecret). +// When a StorageClass is configured, the guest-disk PVC is ensured first. func (r *ExporterSetReconciler) createExporterPod( ctx context.Context, es *virtualtargetv1alpha1.ExporterSet, @@ -469,6 +471,10 @@ func (r *ExporterSetReconciler) createExporterPod( ) error { logger := log.FromContext(ctx) + if err := r.ensureDiskPVC(ctx, es, vtc, mergedParameters, exp); err != nil { + return err + } + pod, err := r.Provisioner.RenderPod(ctx, es, vtc, mergedParameters, images, exp) if err != nil { return fmt.Errorf("render Pod for %s: %w", exp.Name, err) @@ -508,6 +514,62 @@ func (r *ExporterSetReconciler) createExporterPod( return nil } +// ensureDiskPVC creates the guest-disk PVC when a StorageClass is configured. +// No-op for emptyDir mode. The PVC is owned by the Exporter so ExitAndReplace +// cascade deletes it with the instance. +func (r *ExporterSetReconciler) ensureDiskPVC( + ctx context.Context, + es *virtualtargetv1alpha1.ExporterSet, + vtc *virtualtargetv1alpha1.VirtualTargetClass, + mergedParameters map[string]interface{}, + exp *jumpstarterdevv1alpha1.Exporter, +) error { + storageClass := virtualtargetv1alpha1.EffectiveStorageClassName(vtc, es) + if storageClass == "" { + return nil + } + + size, err := disk.SizeFromParameters(mergedParameters) + if err != nil { + return fmt.Errorf("disk size for %s: %w", exp.Name, err) + } + + labels := maps.Clone(es.Spec.Template.Metadata.Labels) + if labels == nil { + labels = make(map[string]string) + } + labels[labelExporterSetName] = es.Name + + desired := disk.BuildPVC(exp.Namespace, exp.Name, storageClass, size, labels) + + existing := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: desired.Name, + Namespace: desired.Namespace, + }, + } + + _, err = controllerutil.CreateOrUpdate(ctx, r.Client, existing, func() error { + if err := ctrl.SetControllerReference(exp, existing, r.Scheme); err != nil { + return fmt.Errorf("set owner on disk PVC for %s: %w", exp.Name, err) + } + // Spec is immutable after creation for most fields; only set on create. + if existing.CreationTimestamp.IsZero() { + existing.Labels = desired.Labels + existing.Spec = desired.Spec + } else if existing.Labels == nil { + existing.Labels = desired.Labels + } else { + existing.Labels[labelExporterSetName] = es.Name + } + return nil + }) + if err != nil { + return fmt.Errorf("ensure disk PVC for %s: %w", exp.Name, err) + } + return nil +} + // readCABundle fetches PEM CA data. If VTC specifies a CABundleConfigMapRef, // that is used. Otherwise falls back to the CertManager-generated ConfigMap. func (r *ExporterSetReconciler) readCABundle( 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 b346a17e1..d13d4ef5c 100644 --- a/python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver.py +++ b/python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver.py @@ -285,13 +285,15 @@ async def on(self) -> None: # noqa: C901 for device in devices: cmdline += ["-device", device] - if bios.exists(): + if bios.exists() or self.parent._runtime_firmware_path(bios): cmdline += [ "-bios", str(bios), ] - if ovmf_code.exists() and ovmf_vars.exists(): + if (ovmf_code.exists() or self.parent._runtime_firmware_path(ovmf_code)) and ( + ovmf_vars.exists() or self.parent._runtime_firmware_path(ovmf_vars) + ): cmdline += [ "-drive", f"file={ovmf_code},if=pflash,format=raw,unit=0,readonly=on", @@ -482,6 +484,7 @@ def __post_init__(self): @property def _work_dir(self) -> str: + """Directory for sockets and jumpstarter-exec in sidecar mode.""" if self.launcher_socket: # Sidecar: QEMU only sees the shared volume. Derive from the # socket path so production (/shared/launcher.sock) and tests @@ -489,6 +492,13 @@ def _work_dir(self) -> str: return str(Path(self.launcher_socket).parent) return self._tmp_dir.name + @property + def _disk_dir(self) -> str: + """Directory for flashable guest disk images (root, bios, …).""" + if self.launcher_socket: + return "/disk" + return self._tmp_dir.name + @property def _pty(self) -> str: return str(Path(self._work_dir) / "pty") @@ -518,6 +528,10 @@ def _wrap_command(self, cmd: list[str]) -> list[str]: def _cid(self) -> int: return randbits(32) + def _runtime_firmware_path(self, path: Path) -> bool: + """True when path is a default firmware path that lives in the runtime image.""" + return self.launcher_socket is not None and path in self.default_partitions.values() + def validate_partition( self, partition: str | None = None, @@ -525,13 +539,13 @@ def validate_partition( ) -> Path: match partition: case "root" | None: - path = Path(self._work_dir) / "root" + path = Path(self._disk_dir) / "root" case "OVMF_CODE.fd": - path = Path(self._work_dir) / "OVMF_CODE.fd" + path = Path(self._disk_dir) / "OVMF_CODE.fd" case "OVMF_VARS.fd": - path = Path(self._work_dir) / "OVMF_VARS.fd" + path = Path(self._disk_dir) / "OVMF_VARS.fd" case "bios": - path = Path(self._work_dir) / "bios" + path = Path(self._disk_dir) / "bios" case _: raise ValueError(f"invalid partition name: {partition}") From f429112f44fa645d5b7bd21516ec1718a42e843f Mon Sep 17 00:00:00 2001 From: Miguel Angel Ajo Pelayo Date: Tue, 1 Sep 2026 11:40:54 +0200 Subject: [PATCH 2/2] refactor: guest disk via parameters.storage and ephemeral volumes Move StorageClass off the CRDs into merged parameters so provisioners that do not need disks stay untouched, and bind PVCs to the Pod with generic ephemeral volumes so ExitAndReplace cleans them up. Co-authored-by: Cursor --- .github/workflows/e2e.yaml | 2 +- .../v1alpha1/exporterset_types.go | 6 - .../api/virtualtarget/v1alpha1/storage.go | 30 ---- .../virtualtarget/v1alpha1/storage_test.go | 49 ------ .../v1alpha1/virtualtargetclass_types.go | 8 - .../v1alpha1/zz_generated.deepcopy.go | 5 - ...altarget.jumpstarter.dev_exportersets.yaml | 6 - ....jumpstarter.dev_virtualtargetclasses.yaml | 8 - .../controller/jumpstarter/exporterset.go | 5 - .../jumpstarter/exporterset_test.go | 11 -- controller/hack/sample-x86_64-kind.yaml | 8 +- controller/hack/sample-x86_64.yaml | 16 +- controller/internal/exporterset/disk/disk.go | 142 +++++++++++++++--- .../internal/exporterset/disk/disk_test.go | 95 ++++++++++-- .../exporterset/provisioners/qemu/qemu.go | 62 ++------ .../provisioners/qemu/qemu_test.go | 70 +++++---- controller/internal/exporterset/reconciler.go | 62 -------- .../JEP-0014-virtual-scalable-exporters.md | 54 +++++-- e2e/README.md | 2 +- e2e/test/exporterset_qemu_test.go | 18 --- .../jumpstarter_driver_qemu/driver.py | 7 +- .../jumpstarter_driver_qemu/driver_test.py | 17 +++ 22 files changed, 338 insertions(+), 345 deletions(-) delete mode 100644 controller/api/virtualtarget/v1alpha1/storage.go delete mode 100644 controller/api/virtualtarget/v1alpha1/storage_test.go diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index 848f2e81f..254dc8419 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -302,7 +302,7 @@ jobs: matrix: include: ${{ fromJson(needs.changes.outputs.e2e-matrix) }} runs-on: ${{ matrix.os }} - # Includes ExporterSet QEMU coverage (TCG); flash/boot still skipped until #924. + # Includes ExporterSet QEMU coverage (TCG), including flash/boot. # Job > Ginkgo suite timeout (60m in e2e/lib/common.sh) for setup/log upload. timeout-minutes: 70 steps: diff --git a/controller/api/virtualtarget/v1alpha1/exporterset_types.go b/controller/api/virtualtarget/v1alpha1/exporterset_types.go index d779ec981..d71d710d0 100644 --- a/controller/api/virtualtarget/v1alpha1/exporterset_types.go +++ b/controller/api/virtualtarget/v1alpha1/exporterset_types.go @@ -133,12 +133,6 @@ type ExporterSetSpec struct { // +optional Images *ImageOverrides `json:"images,omitempty"` - // StorageClassName overrides VirtualTargetClass.spec.storageClassName for - // guest disk volumes. When nil, the class value is used. When set to an - // empty string, forces emptyDir even if the class names a StorageClass. - // +optional - StorageClassName *string `json:"storageClassName,omitempty"` - // Selector defines the label selector for matching exporters owned by this set. Selector metav1.LabelSelector `json:"selector"` diff --git a/controller/api/virtualtarget/v1alpha1/storage.go b/controller/api/virtualtarget/v1alpha1/storage.go deleted file mode 100644 index 6716aac85..000000000 --- a/controller/api/virtualtarget/v1alpha1/storage.go +++ /dev/null @@ -1,30 +0,0 @@ -/* -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 - - http://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 v1alpha1 - -// EffectiveStorageClassName returns the StorageClass to use for guest disk -// volumes. ExporterSet.spec.storageClassName overrides the class when set -// (including the empty string to force emptyDir). -func EffectiveStorageClassName(vtc *VirtualTargetClass, es *ExporterSet) string { - if es != nil && es.Spec.StorageClassName != nil { - return *es.Spec.StorageClassName - } - if vtc != nil { - return vtc.Spec.StorageClassName - } - return "" -} diff --git a/controller/api/virtualtarget/v1alpha1/storage_test.go b/controller/api/virtualtarget/v1alpha1/storage_test.go deleted file mode 100644 index 47d6169e7..000000000 --- a/controller/api/virtualtarget/v1alpha1/storage_test.go +++ /dev/null @@ -1,49 +0,0 @@ -/* -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 - - http://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 v1alpha1 - -import ( - "testing" -) - -func TestEffectiveStorageClassName(t *testing.T) { - empty := "" - override := "es-sc" - - tests := []struct { - name string - vtc string - es *string - want string - }{ - {name: "both empty", want: ""}, - {name: "vtc only", vtc: "vtc-sc", want: "vtc-sc"}, - {name: "es override", vtc: "vtc-sc", es: &override, want: "es-sc"}, - {name: "es clears to emptyDir", vtc: "vtc-sc", es: &empty, want: ""}, - {name: "es only", es: &override, want: "es-sc"}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - vtc := &VirtualTargetClass{Spec: VirtualTargetClassSpec{StorageClassName: tt.vtc}} - es := &ExporterSet{Spec: ExporterSetSpec{StorageClassName: tt.es}} - if got := EffectiveStorageClassName(vtc, es); got != tt.want { - t.Errorf("EffectiveStorageClassName() = %q, want %q", got, tt.want) - } - }) - } -} diff --git a/controller/api/virtualtarget/v1alpha1/virtualtargetclass_types.go b/controller/api/virtualtarget/v1alpha1/virtualtargetclass_types.go index 2d2dc99a3..e0f3de6bb 100644 --- a/controller/api/virtualtarget/v1alpha1/virtualtargetclass_types.go +++ b/controller/api/virtualtarget/v1alpha1/virtualtargetclass_types.go @@ -141,14 +141,6 @@ type VirtualTargetClassSpec struct { // ExporterSet-level images take precedence over these class-level defaults. // +optional Images *ImageOverrides `json:"images,omitempty"` - - // StorageClassName selects the StorageClass for the guest disk PVC. - // When empty, the provisioner uses an emptyDir volume sized from - // parameters.resources.storage and sets ephemeral-storage - // requests/limits so the scheduler accounts for local disk usage. - // ExporterSet.spec.storageClassName can override this value. - // +optional - StorageClassName string `json:"storageClassName,omitempty"` } // +kubebuilder:object:root=true diff --git a/controller/api/virtualtarget/v1alpha1/zz_generated.deepcopy.go b/controller/api/virtualtarget/v1alpha1/zz_generated.deepcopy.go index ed7971c0f..a8485d19b 100644 --- a/controller/api/virtualtarget/v1alpha1/zz_generated.deepcopy.go +++ b/controller/api/virtualtarget/v1alpha1/zz_generated.deepcopy.go @@ -168,11 +168,6 @@ func (in *ExporterSetSpec) DeepCopyInto(out *ExporterSetSpec) { *out = new(ImageOverrides) (*in).DeepCopyInto(*out) } - if in.StorageClassName != nil { - in, out := &in.StorageClassName, &out.StorageClassName - *out = new(string) - **out = **in - } in.Selector.DeepCopyInto(&out.Selector) in.Template.DeepCopyInto(&out.Template) } diff --git a/controller/deploy/operator/config/crd/bases/virtualtarget.jumpstarter.dev_exportersets.yaml b/controller/deploy/operator/config/crd/bases/virtualtarget.jumpstarter.dev_exportersets.yaml index a428f8f3b..36c740db3 100644 --- a/controller/deploy/operator/config/crd/bases/virtualtarget.jumpstarter.dev_exportersets.yaml +++ b/controller/deploy/operator/config/crd/bases/virtualtarget.jumpstarter.dev_exportersets.yaml @@ -186,12 +186,6 @@ spec: type: object type: object x-kubernetes-map-type: atomic - storageClassName: - description: |- - StorageClassName overrides VirtualTargetClass.spec.storageClassName for - guest disk volumes. When nil, the class value is used. When set to an - empty string, forces emptyDir even if the class names a StorageClass. - type: string template: description: Template defines the exporter template for instances created by this set. diff --git a/controller/deploy/operator/config/crd/bases/virtualtarget.jumpstarter.dev_virtualtargetclasses.yaml b/controller/deploy/operator/config/crd/bases/virtualtarget.jumpstarter.dev_virtualtargetclasses.yaml index 01e191e78..598887b6c 100644 --- a/controller/deploy/operator/config/crd/bases/virtualtarget.jumpstarter.dev_virtualtargetclasses.yaml +++ b/controller/deploy/operator/config/crd/bases/virtualtarget.jumpstarter.dev_virtualtargetclasses.yaml @@ -262,14 +262,6 @@ spec: type: object type: array type: object - storageClassName: - description: |- - StorageClassName selects the StorageClass for the guest disk PVC. - When empty, the provisioner uses an emptyDir volume sized from - parameters.resources.storage and sets ephemeral-storage - requests/limits so the scheduler accounts for local disk usage. - ExporterSet.spec.storageClassName can override this value. - type: string required: - provisioner type: object diff --git a/controller/deploy/operator/internal/controller/jumpstarter/exporterset.go b/controller/deploy/operator/internal/controller/jumpstarter/exporterset.go index 7e14d2505..46a842cc3 100644 --- a/controller/deploy/operator/internal/controller/jumpstarter/exporterset.go +++ b/controller/deploy/operator/internal/controller/jumpstarter/exporterset.go @@ -522,11 +522,6 @@ func exporterSetPolicyRules() []rbacv1.PolicyRule { Resources: []string{"pods"}, Verbs: []string{"get", "list", "watch", "create", "update", "patch", "delete"}, }, - { - APIGroups: []string{""}, - Resources: []string{"persistentvolumeclaims"}, - Verbs: []string{"get", "list", "watch", "create", "update", "patch", "delete"}, - }, { APIGroups: []string{""}, Resources: []string{"events"}, diff --git a/controller/deploy/operator/internal/controller/jumpstarter/exporterset_test.go b/controller/deploy/operator/internal/controller/jumpstarter/exporterset_test.go index 033a6bb57..e58351745 100644 --- a/controller/deploy/operator/internal/controller/jumpstarter/exporterset_test.go +++ b/controller/deploy/operator/internal/controller/jumpstarter/exporterset_test.go @@ -181,17 +181,6 @@ var _ = Describe("exporterSetPolicyRules", func() { Fail("no rule found granting full CRUD on pods") }) - It("should grant full CRUD on persistentvolumeclaims", func() { - for _, rule := range rules { - if containsString(rule.APIGroups, "") && - containsString(rule.Resources, "persistentvolumeclaims") { - Expect(rule.Verbs).To(ContainElements("get", "list", "watch", "create", "update", "patch", "delete")) - return - } - } - Fail("no rule found granting full CRUD on persistentvolumeclaims") - }) - It("should grant full CRUD on exporters", func() { for _, rule := range rules { if containsString(rule.APIGroups, "jumpstarter.dev") && diff --git a/controller/hack/sample-x86_64-kind.yaml b/controller/hack/sample-x86_64-kind.yaml index c8387f4fa..e6265f73f 100644 --- a/controller/hack/sample-x86_64-kind.yaml +++ b/controller/hack/sample-x86_64-kind.yaml @@ -9,10 +9,10 @@ # but much slower than a real x86_64+KVM cluster. # # Guest disk uses emptyDir sized from parameters.resources.storage (no -# storageClassName). The provisioner also sets ephemeral-storage -# requests/limits so the scheduler accounts for local disk. To use a -# PVC instead, set spec.storageClassName on the VirtualTargetClass or -# override it on the ExporterSet. +# parameters.storage.storageClassName). The provisioner also sets +# ephemeral-storage requests/limits so the scheduler accounts for local +# disk. To use a PVC instead, set parameters.storage.storageClassName +# on the VirtualTargetClass (ExporterSet parameters deep-merge over it). # # For real deployments with KVM acceleration, use sample-x86_64.yaml # on a cluster that exposes /dev/kvm via the kubevirt device plugin diff --git a/controller/hack/sample-x86_64.yaml b/controller/hack/sample-x86_64.yaml index baaf7bd4a..a52eec461 100644 --- a/controller/hack/sample-x86_64.yaml +++ b/controller/hack/sample-x86_64.yaml @@ -8,9 +8,11 @@ # - A VirtualTargetClass for x86_64 QEMU VMs with KVM acceleration # - An ExporterSet that manages a pool of virtual exporters # -# Guest disk: omit storageClassName to use emptyDir (with ephemeral-storage -# accounting), or set storageClassName to provision a per-exporter PVC -# mounted at /disk. ExporterSet.spec.storageClassName overrides the class. +# Guest disk: sized emptyDir at /disk from parameters.resources.storage +# (with ephemeral-storage accounting). To use a StorageClass, set +# parameters.storage.storageClassName — the provisioner renders a generic +# ephemeral PVC whose lifetime follows the Pod (ExitAndReplace). +# ExporterSet.spec.parameters.storage deep-merges over the class. --- apiVersion: virtualtarget.jumpstarter.dev/v1alpha1 kind: VirtualTargetClass @@ -21,7 +23,6 @@ spec: provisioner: qemu.jumpstarter.dev bindingMode: Immediate reclaimPolicy: Delete - # storageClassName: "your-storage-class" # optional; omit → emptyDir scheduling: nodeSelector: kubernetes.io/arch: amd64 @@ -42,6 +43,9 @@ spec: cpu: 2 memory: 2Gi storage: 20Gi + # storage: + # storageClassName: "your-storage-class" + # accessModes: ["ReadWriteOnce"] --- apiVersion: virtualtarget.jumpstarter.dev/v1alpha1 kind: ExporterSet @@ -55,7 +59,9 @@ spec: scaleDownCooldown: 5m recycleStrategy: ExitAndReplace virtualTargetClassName: qemu-x86-64 - # storageClassName: "override-sc" # optional override ("" forces emptyDir) + # parameters: + # storage: + # storageClassName: "override-sc" # override class; "" forces emptyDir selector: matchLabels: board: x86-64-virtual diff --git a/controller/internal/exporterset/disk/disk.go b/controller/internal/exporterset/disk/disk.go index dfa8ba8b5..884d280a4 100644 --- a/controller/internal/exporterset/disk/disk.go +++ b/controller/internal/exporterset/disk/disk.go @@ -15,7 +15,20 @@ limitations under the License. */ // Package disk provides shared helpers for guest-disk volume provisioning -// used by ExporterSet provisioners and the reconciler. +// used by ExporterSet provisioners. +// +// Size comes from parameters.resources.storage (JEP-0014). Kubernetes +// backend config comes from parameters.storage: +// +// parameters: +// resources: +// storage: 20Gi +// storage: +// storageClassName: gp3 # omit or "" → sized emptyDir +// accessModes: ["ReadWriteOnce"] +// +// When storageClassName is set, the volume is a generic ephemeral PVC +// (volumeClaimTemplate) so its lifetime follows the Pod (ExitAndReplace). package disk import ( @@ -23,7 +36,6 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) const ( @@ -35,13 +47,67 @@ const ( // DefaultSize is used when parameters.resources.storage is unset. DefaultSize = "10Gi" - - pvcNamePrefix = "disk-" ) -// PVCName returns the per-exporter PersistentVolumeClaim name. -func PVCName(exporterName string) string { - return pvcNamePrefix + exporterName +// Spec is the resolved guest-disk volume configuration. +type Spec struct { + Size resource.Quantity + StorageClassName string + AccessModes []corev1.PersistentVolumeAccessMode +} + +// UsePVC reports whether the disk is backed by an ephemeral PVC. +func (s Spec) UsePVC() bool { + return s.StorageClassName != "" +} + +// Mount returns the VolumeMount for /disk. +func Mount() corev1.VolumeMount { + return corev1.VolumeMount{ + Name: VolumeName, + MountPath: MountPath, + } +} + +// FromParameters reads disk size and optional storage backend from merged +// ExporterSet/VirtualTargetClass parameters. +func FromParameters(params map[string]interface{}) (Spec, error) { + size, err := SizeFromParameters(params) + if err != nil { + return Spec{}, err + } + spec := Spec{ + Size: size, + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, + } + if params == nil { + return spec, nil + } + storage, ok := params["storage"].(map[string]interface{}) + if !ok { + if params["storage"] != nil { + return Spec{}, fmt.Errorf("parameters.storage must be an object, got %T", params["storage"]) + } + return spec, nil + } + + switch v := storage["storageClassName"].(type) { + case string: + spec.StorageClassName = v + case nil: + // omit → emptyDir + default: + return Spec{}, fmt.Errorf("parameters.storage.storageClassName must be a string, got %T", v) + } + + if raw, exists := storage["accessModes"]; exists && raw != nil { + modes, err := parseAccessModes(raw) + if err != nil { + return Spec{}, err + } + spec.AccessModes = modes + } + return spec, nil } // SizeFromParameters reads parameters.resources.storage, defaulting to DefaultSize. @@ -55,8 +121,6 @@ func SizeFromParameters(params map[string]interface{}) (resource.Quantity, error raw = v } case float64: - // JSON numbers land as float64; treat as Gi if unitless is awkward — - // require string quantities in the API. return resource.Quantity{}, fmt.Errorf("parameters.resources.storage must be a string quantity (e.g. \"10Gi\"), got number %v", v) case nil: // use default @@ -73,26 +137,35 @@ func SizeFromParameters(params map[string]interface{}) (resource.Quantity, error return qty, nil } -// BuildPVC constructs a guest-disk PVC owned by the given exporter metadata. -func BuildPVC(namespace, exporterName, storageClassName string, size resource.Quantity, labels map[string]string) *corev1.PersistentVolumeClaim { - return &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: PVCName(exporterName), - Namespace: namespace, - Labels: labels, - }, - Spec: corev1.PersistentVolumeClaimSpec{ - AccessModes: []corev1.PersistentVolumeAccessMode{ - corev1.ReadWriteOnce, - }, - StorageClassName: &storageClassName, - Resources: corev1.VolumeResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceStorage: size, +// Volume builds the guest-disk Pod volume from spec. +func Volume(spec Spec) corev1.Volume { + vol := corev1.Volume{Name: VolumeName} + if spec.UsePVC() { + sc := spec.StorageClassName + vol.VolumeSource = corev1.VolumeSource{ + Ephemeral: &corev1.EphemeralVolumeSource{ + VolumeClaimTemplate: &corev1.PersistentVolumeClaimTemplate{ + Spec: corev1.PersistentVolumeClaimSpec{ + AccessModes: spec.AccessModes, + StorageClassName: &sc, + Resources: corev1.VolumeResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceStorage: spec.Size, + }, + }, + }, }, }, + } + return vol + } + size := spec.Size.DeepCopy() + vol.VolumeSource = corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{ + SizeLimit: &size, }, } + return vol } // SetEphemeralStorage ensures requests and limits include ephemeral-storage @@ -112,3 +185,22 @@ func SetEphemeralStorage(resources *corev1.ResourceRequirements, size resource.Q resources.Limits[corev1.ResourceEphemeralStorage] = size.DeepCopy() } } + +func parseAccessModes(v interface{}) ([]corev1.PersistentVolumeAccessMode, error) { + items, ok := v.([]interface{}) + if !ok { + return nil, fmt.Errorf("parameters.storage.accessModes must be a list of strings, got %T", v) + } + if len(items) == 0 { + return nil, fmt.Errorf("parameters.storage.accessModes must not be empty") + } + out := make([]corev1.PersistentVolumeAccessMode, 0, len(items)) + for _, item := range items { + s, ok := item.(string) + if !ok || s == "" { + return nil, fmt.Errorf("parameters.storage.accessModes must be a list of strings, got %T", item) + } + out = append(out, corev1.PersistentVolumeAccessMode(s)) + } + return out, nil +} diff --git a/controller/internal/exporterset/disk/disk_test.go b/controller/internal/exporterset/disk/disk_test.go index 22e2d1deb..a06482af5 100644 --- a/controller/internal/exporterset/disk/disk_test.go +++ b/controller/internal/exporterset/disk/disk_test.go @@ -23,32 +23,63 @@ import ( "k8s.io/apimachinery/pkg/api/resource" ) -func TestPVCName(t *testing.T) { - if got := PVCName("exp-1"); got != "disk-exp-1" { - t.Errorf("PVCName() = %q, want disk-exp-1", got) +func TestFromParameters_defaults(t *testing.T) { + spec, err := FromParameters(nil) + if err != nil { + t.Fatalf("nil params: %v", err) + } + if !spec.Size.Equal(resource.MustParse(DefaultSize)) { + t.Errorf("size = %v, want %s", spec.Size, DefaultSize) + } + if spec.UsePVC() { + t.Error("expected emptyDir when storageClassName is unset") + } + if len(spec.AccessModes) != 1 || spec.AccessModes[0] != corev1.ReadWriteOnce { + t.Errorf("accessModes = %v, want [ReadWriteOnce]", spec.AccessModes) } } -func TestSizeFromParameters(t *testing.T) { - qty, err := SizeFromParameters(nil) +func TestFromParameters_storageClassAndSize(t *testing.T) { + spec, err := FromParameters(map[string]interface{}{ + "resources": map[string]interface{}{"storage": "15Gi"}, + "storage": map[string]interface{}{ + "storageClassName": "gp3", + "accessModes": []interface{}{"ReadWriteOnce", "ReadWriteMany"}, + }, + }) if err != nil { - t.Fatalf("nil params: %v", err) + t.Fatalf("FromParameters: %v", err) + } + if !spec.Size.Equal(resource.MustParse("15Gi")) { + t.Errorf("size = %v, want 15Gi", spec.Size) + } + if spec.StorageClassName != "gp3" { + t.Errorf("storageClassName = %q, want gp3", spec.StorageClassName) } - if !qty.Equal(resource.MustParse(DefaultSize)) { - t.Errorf("default = %v, want %s", qty, DefaultSize) + if !spec.UsePVC() { + t.Error("expected PVC when storageClassName is set") } + if len(spec.AccessModes) != 2 { + t.Fatalf("accessModes = %v", spec.AccessModes) + } +} - qty, err = SizeFromParameters(map[string]interface{}{ - "resources": map[string]interface{}{"storage": "15Gi"}, +func TestFromParameters_emptyStorageClassForcesEmptyDir(t *testing.T) { + spec, err := FromParameters(map[string]interface{}{ + "storage": map[string]interface{}{ + "storageClassName": "", + }, }) if err != nil { - t.Fatalf("15Gi: %v", err) + t.Fatalf("FromParameters: %v", err) } - if !qty.Equal(resource.MustParse("15Gi")) { - t.Errorf("got %v, want 15Gi", qty) + if spec.UsePVC() { + t.Error("empty storageClassName should force emptyDir") } +} - _, err = SizeFromParameters(map[string]interface{}{ +func TestFromParameters_rejectsNumericStorage(t *testing.T) { + _, err := FromParameters(map[string]interface{}{ "resources": map[string]interface{}{"storage": 10.0}, }) if err == nil { @@ -56,6 +87,41 @@ func TestSizeFromParameters(t *testing.T) { } } +func TestVolume_emptyDir(t *testing.T) { + spec := Spec{Size: resource.MustParse("7Gi")} + vol := Volume(spec) + if vol.Name != VolumeName { + t.Errorf("name = %q, want %s", vol.Name, VolumeName) + } + if vol.EmptyDir == nil || vol.EmptyDir.SizeLimit == nil { + t.Fatalf("expected sized emptyDir, got %#v", vol) + } + if !vol.EmptyDir.SizeLimit.Equal(spec.Size) { + t.Errorf("SizeLimit = %v, want %v", vol.EmptyDir.SizeLimit, spec.Size) + } +} + +func TestVolume_ephemeralPVC(t *testing.T) { + sc := "fast-ssd" + spec := Spec{ + Size: resource.MustParse("20Gi"), + StorageClassName: sc, + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, + } + vol := Volume(spec) + if vol.Ephemeral == nil || vol.Ephemeral.VolumeClaimTemplate == nil { + t.Fatalf("expected ephemeral volumeClaimTemplate, got %#v", vol) + } + claim := vol.Ephemeral.VolumeClaimTemplate.Spec + if claim.StorageClassName == nil || *claim.StorageClassName != sc { + t.Errorf("StorageClassName = %v, want %s", claim.StorageClassName, sc) + } + got := claim.Resources.Requests[corev1.ResourceStorage] + if !got.Equal(spec.Size) { + t.Errorf("storage request = %v, want %v", got, spec.Size) + } +} + func TestSetEphemeralStorage(t *testing.T) { size := resource.MustParse("10Gi") res := corev1.ResourceRequirements{ @@ -74,7 +140,6 @@ func TestSetEphemeralStorage(t *testing.T) { t.Error("cpu request should be preserved") } - // Does not overwrite existing ephemeral-storage. custom := resource.MustParse("1Gi") res.Requests[corev1.ResourceEphemeralStorage] = custom SetEphemeralStorage(&res, size) diff --git a/controller/internal/exporterset/provisioners/qemu/qemu.go b/controller/internal/exporterset/provisioners/qemu/qemu.go index f1a49c280..7c6c90a44 100644 --- a/controller/internal/exporterset/provisioners/qemu/qemu.go +++ b/controller/internal/exporterset/provisioners/qemu/qemu.go @@ -176,7 +176,7 @@ func (p *Provisioner) RenderPod( runAsExporter := exporterNonRootUID exporterNonRoot := true - diskSize, err := disk.SizeFromParameters(mergedParameters) + diskSpec, err := disk.FromParameters(mergedParameters) if err != nil { return nil, err } @@ -203,10 +203,7 @@ func (p *Provisioner) RenderPod( }) } - diskMount := corev1.VolumeMount{ - Name: disk.VolumeName, - MountPath: disk.MountPath, - } + diskMount := disk.Mount() podMeta := metav1.ObjectMeta{ Namespace: exporterSet.Namespace, @@ -307,10 +304,7 @@ func (p *Provisioner) RenderPod( }, } - storageClass := virtualtargetv1alpha1.EffectiveStorageClassName(vtc, exporterSet) - if err := attachDiskVolume(pod, exporter, storageClass, diskSize); err != nil { - return nil, err - } + pod.Spec.Volumes = append(pod.Spec.Volumes, disk.Volume(diskSpec)) // Apply scheduling from VirtualTargetClass. // Clone maps and slices to avoid mutating the VTC's fields. @@ -332,13 +326,19 @@ func (p *Provisioner) RenderPod( } } - // emptyDir guest disks consume node ephemeral storage — ensure the - // scheduler and kubelet account for it on containers that mount /disk. - if storageClass == "" { - disk.SetEphemeralStorage(&pod.Spec.Containers[0].Resources, diskSize) + if diskSpec.UsePVC() { + // fsGroup so the non-root exporter can write the ephemeral claim. + if pod.Spec.SecurityContext == nil { + pod.Spec.SecurityContext = &corev1.PodSecurityContext{} + } + pod.Spec.SecurityContext.FSGroup = &runAsExporter + } else { + // emptyDir guest disks consume node ephemeral storage — ensure the + // scheduler and kubelet account for it on containers that mount /disk. + disk.SetEphemeralStorage(&pod.Spec.Containers[0].Resources, diskSpec.Size) for i := range pod.Spec.InitContainers { - if pod.Spec.InitContainers[i].Name == "exporter" { - disk.SetEphemeralStorage(&pod.Spec.InitContainers[i].Resources, diskSize) + if pod.Spec.InitContainers[i].Name == runtimeContainerName { + disk.SetEphemeralStorage(&pod.Spec.InitContainers[i].Resources, diskSpec.Size) } } } @@ -346,38 +346,6 @@ func (p *Provisioner) RenderPod( return pod, nil } -// attachDiskVolume appends the guest disk volume. When storageClass is set, -// the volume references a PVC that the reconciler creates; otherwise an -// emptyDir sized to diskSize is used. -func attachDiskVolume( - pod *corev1.Pod, - exporter *jumpstarterdevv1alpha1.Exporter, - storageClass string, - diskSize resource.Quantity, -) error { - vol := corev1.Volume{Name: disk.VolumeName} - if storageClass != "" { - if exporter == nil { - return fmt.Errorf("disk PVC requires an Exporter to derive the claim name") - } - claimName := disk.PVCName(exporter.Name) - vol.VolumeSource = corev1.VolumeSource{ - PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ - ClaimName: claimName, - }, - } - } else { - size := diskSize.DeepCopy() - vol.VolumeSource = corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{ - SizeLimit: &size, - }, - } - } - pod.Spec.Volumes = append(pod.Spec.Volumes, vol) - return nil -} - // EnrichExporterExport injects QEMU-specific driver configuration: // - Forces launcher_socket on the QEMU driver entry // - Defaults arch/smp/mem/disk_size from mergedParameters if not set diff --git a/controller/internal/exporterset/provisioners/qemu/qemu_test.go b/controller/internal/exporterset/provisioners/qemu/qemu_test.go index a09eda6df..da6e3c395 100644 --- a/controller/internal/exporterset/provisioners/qemu/qemu_test.go +++ b/controller/internal/exporterset/provisioners/qemu/qemu_test.go @@ -22,6 +22,7 @@ import ( jumpstarterdevv1alpha1 "github.com/jumpstarter-dev/jumpstarter/controller/api/v1alpha1" virtualtargetv1alpha1 "github.com/jumpstarter-dev/jumpstarter/controller/api/virtualtarget/v1alpha1" + "github.com/jumpstarter-dev/jumpstarter/controller/internal/exporterset/disk" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -111,7 +112,7 @@ func assertRenderPodSharedVolume(t *testing.T, pod *corev1.Pod) { !pod.Spec.Volumes[0].EmptyDir.SizeLimit.Equal(wantLimit) { t.Errorf("shared SizeLimit = %v, want %v", pod.Spec.Volumes[0].EmptyDir.SizeLimit, wantLimit) } - if pod.Spec.Volumes[1].Name != "disk" || pod.Spec.Volumes[1].EmptyDir == nil { + if pod.Spec.Volumes[1].Name != disk.VolumeName || pod.Spec.Volumes[1].EmptyDir == nil { t.Fatalf("expected disk emptyDir volume at index 1, got %#v", pod.Spec.Volumes[1]) } wantDisk := resource.MustParse("10Gi") @@ -122,22 +123,20 @@ func assertRenderPodSharedVolume(t *testing.T, pod *corev1.Pod) { ephemeral := pod.Spec.Containers[0].Resources.Requests[corev1.ResourceEphemeralStorage] if !ephemeral.Equal(wantDisk) { - t.Errorf("runtime ephemeral-storage request = %v, want %v", ephemeral, wantDisk) + t.Errorf("exporter ephemeral-storage request = %v, want %v", ephemeral, wantDisk) } - exporterEphemeral := pod.Spec.InitContainers[1].Resources.Requests[corev1.ResourceEphemeralStorage] - if !exporterEphemeral.Equal(wantDisk) { - t.Errorf("exporter ephemeral-storage request = %v, want %v", exporterEphemeral, wantDisk) + runtimeEphemeral := pod.Spec.InitContainers[1].Resources.Requests[corev1.ResourceEphemeralStorage] + if !runtimeEphemeral.Equal(wantDisk) { + t.Errorf("runtime ephemeral-storage request = %v, want %v", runtimeEphemeral, wantDisk) } - hasDiskMount := false - for _, m := range pod.Spec.Containers[0].VolumeMounts { - if m.Name == "disk" && m.MountPath == "/disk" { - hasDiskMount = true + assertDiskMount(t, runtimeContainerName, pod.Spec.InitContainers[1].VolumeMounts) + assertDiskMount(t, "exporter", pod.Spec.Containers[0].VolumeMounts) + for _, m := range pod.Spec.InitContainers[0].VolumeMounts { + if m.Name == disk.VolumeName { + t.Error("copy-jumpstarter-exec should not mount /disk") } } - if !hasDiskMount { - t.Error("target-runtime missing /disk mount") - } } func assertRenderPodContainers(t *testing.T, pod *corev1.Pod) { @@ -193,6 +192,16 @@ func assertSharedMount(t *testing.T, name string, mounts []corev1.VolumeMount) { t.Errorf("%s missing VolumeMount %s -> %s; got %#v", name, sharedVolumeName, sharedMountPath, mounts) } +func assertDiskMount(t *testing.T, name string, mounts []corev1.VolumeMount) { + t.Helper() + for _, m := range mounts { + if m.Name == disk.VolumeName && m.MountPath == disk.MountPath { + return + } + } + t.Errorf("%s missing VolumeMount %s -> %s; got %#v", name, disk.VolumeName, disk.MountPath, mounts) +} + func TestRenderPod_clonesSchedulingFromVTC(t *testing.T) { cpu := resource.MustParse("500m") mem := resource.MustParse("512Mi") @@ -450,49 +459,54 @@ func TestRenderPod_partialImageOverride(t *testing.T) { } } -func TestRenderPod_diskPVCWhenStorageClassSet(t *testing.T) { +func TestRenderPod_diskEphemeralWhenStorageClassSet(t *testing.T) { exporterSet := &virtualtargetv1alpha1.ExporterSet{ ObjectMeta: metav1.ObjectMeta{Name: "demo-set", Namespace: "default"}, - Spec: virtualtargetv1alpha1.ExporterSetSpec{ - StorageClassName: ptr("fast-ssd"), - }, } vtc := &virtualtargetv1alpha1.VirtualTargetClass{ Spec: virtualtargetv1alpha1.VirtualTargetClassSpec{ - Provisioner: ProvisionerName, - StorageClassName: "ignored-because-override", + Provisioner: ProvisionerName, }, } - exporter := &jumpstarterdevv1alpha1.Exporter{ - ObjectMeta: metav1.ObjectMeta{Name: "demo-exporter", Namespace: "default"}, - } params := map[string]interface{}{ "resources": map[string]interface{}{ "storage": "20Gi", }, + "storage": map[string]interface{}{ + "storageClassName": "fast-ssd", + }, } - pod, err := New("dev").RenderPod(context.Background(), exporterSet, vtc, params, nil, exporter) + pod, err := New("dev").RenderPod(context.Background(), exporterSet, vtc, params, nil, nil) if err != nil { t.Fatalf("RenderPod() error = %v", err) } var diskVol *corev1.Volume for i := range pod.Spec.Volumes { - if pod.Spec.Volumes[i].Name == "disk" { + if pod.Spec.Volumes[i].Name == disk.VolumeName { diskVol = &pod.Spec.Volumes[i] break } } - if diskVol == nil || diskVol.PersistentVolumeClaim == nil { - t.Fatalf("expected disk PVC volume, got %#v", diskVol) + if diskVol == nil || diskVol.Ephemeral == nil || diskVol.Ephemeral.VolumeClaimTemplate == nil { + t.Fatalf("expected disk ephemeral volume, got %#v", diskVol) + } + claim := diskVol.Ephemeral.VolumeClaimTemplate.Spec + if claim.StorageClassName == nil || *claim.StorageClassName != "fast-ssd" { + t.Errorf("StorageClassName = %v, want fast-ssd", claim.StorageClassName) } - if diskVol.PersistentVolumeClaim.ClaimName != "disk-demo-exporter" { - t.Errorf("ClaimName = %q, want disk-demo-exporter", diskVol.PersistentVolumeClaim.ClaimName) + want := resource.MustParse("20Gi") + if !claim.Resources.Requests[corev1.ResourceStorage].Equal(want) { + t.Errorf("storage request = %v, want %v", claim.Resources.Requests[corev1.ResourceStorage], want) } if _, ok := pod.Spec.Containers[0].Resources.Requests[corev1.ResourceEphemeralStorage]; ok { t.Error("PVC mode should not set ephemeral-storage for guest disk") } + if pod.Spec.SecurityContext == nil || pod.Spec.SecurityContext.FSGroup == nil || + *pod.Spec.SecurityContext.FSGroup != exporterNonRootUID { + t.Errorf("FSGroup = %v, want %d", pod.Spec.SecurityContext, exporterNonRootUID) + } } func TestRenderPod_diskEmptyDirUsesParamSize(t *testing.T) { @@ -519,5 +533,3 @@ func TestRenderPod_diskEmptyDirUsesParamSize(t *testing.T) { t.Errorf("disk SizeLimit = %v, want %v", diskVol.EmptyDir, want) } } - -func ptr(s string) *string { return &s } diff --git a/controller/internal/exporterset/reconciler.go b/controller/internal/exporterset/reconciler.go index 3408f8890..8491e875b 100644 --- a/controller/internal/exporterset/reconciler.go +++ b/controller/internal/exporterset/reconciler.go @@ -57,7 +57,6 @@ import ( jumpstarterdevv1alpha1 "github.com/jumpstarter-dev/jumpstarter/controller/api/v1alpha1" virtualtargetv1alpha1 "github.com/jumpstarter-dev/jumpstarter/controller/api/virtualtarget/v1alpha1" - "github.com/jumpstarter-dev/jumpstarter/controller/internal/exporterset/disk" ) const ( @@ -460,7 +459,6 @@ func (r *ExporterSetReconciler) syncConfigSecret( // createExporterPod issues a Pod for a single Exporter that has credentials // but no Pod yet. The config Secret must already exist (syncConfigSecret). -// When a StorageClass is configured, the guest-disk PVC is ensured first. func (r *ExporterSetReconciler) createExporterPod( ctx context.Context, es *virtualtargetv1alpha1.ExporterSet, @@ -471,10 +469,6 @@ func (r *ExporterSetReconciler) createExporterPod( ) error { logger := log.FromContext(ctx) - if err := r.ensureDiskPVC(ctx, es, vtc, mergedParameters, exp); err != nil { - return err - } - pod, err := r.Provisioner.RenderPod(ctx, es, vtc, mergedParameters, images, exp) if err != nil { return fmt.Errorf("render Pod for %s: %w", exp.Name, err) @@ -514,62 +508,6 @@ func (r *ExporterSetReconciler) createExporterPod( return nil } -// ensureDiskPVC creates the guest-disk PVC when a StorageClass is configured. -// No-op for emptyDir mode. The PVC is owned by the Exporter so ExitAndReplace -// cascade deletes it with the instance. -func (r *ExporterSetReconciler) ensureDiskPVC( - ctx context.Context, - es *virtualtargetv1alpha1.ExporterSet, - vtc *virtualtargetv1alpha1.VirtualTargetClass, - mergedParameters map[string]interface{}, - exp *jumpstarterdevv1alpha1.Exporter, -) error { - storageClass := virtualtargetv1alpha1.EffectiveStorageClassName(vtc, es) - if storageClass == "" { - return nil - } - - size, err := disk.SizeFromParameters(mergedParameters) - if err != nil { - return fmt.Errorf("disk size for %s: %w", exp.Name, err) - } - - labels := maps.Clone(es.Spec.Template.Metadata.Labels) - if labels == nil { - labels = make(map[string]string) - } - labels[labelExporterSetName] = es.Name - - desired := disk.BuildPVC(exp.Namespace, exp.Name, storageClass, size, labels) - - existing := &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: desired.Name, - Namespace: desired.Namespace, - }, - } - - _, err = controllerutil.CreateOrUpdate(ctx, r.Client, existing, func() error { - if err := ctrl.SetControllerReference(exp, existing, r.Scheme); err != nil { - return fmt.Errorf("set owner on disk PVC for %s: %w", exp.Name, err) - } - // Spec is immutable after creation for most fields; only set on create. - if existing.CreationTimestamp.IsZero() { - existing.Labels = desired.Labels - existing.Spec = desired.Spec - } else if existing.Labels == nil { - existing.Labels = desired.Labels - } else { - existing.Labels[labelExporterSetName] = es.Name - } - return nil - }) - if err != nil { - return fmt.Errorf("ensure disk PVC for %s: %w", exp.Name, err) - } - return nil -} - // readCABundle fetches PEM CA data. If VTC specifies a CABundleConfigMapRef, // that is used. Otherwise falls back to the CertManager-generated ConfigMap. func (r *ExporterSetReconciler) readCABundle( 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 cc80c5da4..af871b27a 100644 --- a/docs/source/contributing/jeps/JEP-0014-virtual-scalable-exporters.md +++ b/docs/source/contributing/jeps/JEP-0014-virtual-scalable-exporters.md @@ -164,10 +164,10 @@ spec: cpu: 4 memory: 4Gi storage: 16Gi + # storage: + # storageClassName: gp3 # omit → sized emptyDir at /disk ``` -**Example: ExporterSet (generic scaling resource)** - ```yaml apiVersion: virtualtarget.jumpstarter.dev/v1alpha1 kind: ExporterSet @@ -296,21 +296,32 @@ spec: initContainers: - name: copy-jumpstarter-exec # one-shot: stage binary onto shared volume image: quay.io/jumpstarter-dev/jumpstarter:latest + volumeMounts: + - name: shared + mountPath: /shared - name: target-runtime # native sidecar (starts before exporter) restartPolicy: Always image: quay.io/jumpstarter-dev/virtual/qemu-runtime:latest volumeMounts: - name: shared mountPath: /shared + - name: disk + mountPath: /disk containers: - name: exporter # main — default logs; exit tears Pod down image: quay.io/jumpstarter-dev/jumpstarter:latest volumeMounts: - name: shared mountPath: /shared + - name: disk + mountPath: /disk volumes: - name: shared - emptyDir: {} + emptyDir: + sizeLimit: 100Mi # sockets / cidata / jumpstarter-exec + - name: disk + emptyDir: + sizeLimit: 16Gi # guest disk; or ephemeral volumeClaimTemplate ``` Benefits: @@ -582,8 +593,8 @@ with env() as client: **Exporter actions:** -- `storage.flash` writes the image to shared storage (or tells QEMU runtime via - QMP/`blockdev-add`). +- `storage.flash` writes the image to `/disk` on the guest-disk volume (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 sidecar. @@ -954,6 +965,28 @@ firmware: # unchanged — set did not specify firmware digest: sha256:abc... ``` +**Guest disk (`qemu.jumpstarter.dev`):** `parameters.resources.storage` is the +guest disk size (also mapped to the QEMU driver's `disk_size`). Optional +`parameters.storage` selects the Kubernetes volume backend: + +```yaml +parameters: + resources: + storage: 16Gi + storage: + storageClassName: gp3 # omit or "" → sized emptyDir + accessModes: ["ReadWriteOnce"] # default ReadWriteOnce +``` + +When `storageClassName` is set, the provisioner attaches a [generic ephemeral +volume](https://kubernetes.io/docs/concepts/storage/ephemeral-volumes/#generic-ephemeral-volumes) +(`volumeClaimTemplate`) at `/disk` so the claim is created and deleted with the +Pod (ExitAndReplace). When it is omitted, `/disk` is a sized `emptyDir` and +the provisioner sets `ephemeral-storage` requests/limits for scheduler +accounting. Unix sockets stay on the separate 100Mi `/shared` volume. +ExporterSet `parameters.storage` deep-merges over the class; an empty +`storageClassName` on the set forces emptyDir. + **Status subresource (ExporterSet):** ```yaml @@ -1467,9 +1500,9 @@ Unit tests should meet the project test coverage requirements. - 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. + flash Alpine UEFI tiny, expect a serial-console boot marker. Guest disk is a + sized emptyDir (or ephemeral PVC when `parameters.storage.storageClassName` is + set). 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 @@ -1663,7 +1696,7 @@ flash-at-lease workflow (DD-7). - [ ] Watch Leases and Exporters for scaling decisions - [ ] Add `exporterSets` section to `Jumpstarter` operator CR - [x] Integration test: deploy `ExporterSet`, lease, flash, boot, release, - observe scaling (`exporterset-qemu` e2e; flash/boot gated on #924 storage) + observe scaling (`exporterset-qemu` e2e) ### Phase 3: External / off-cluster provisioning @@ -1721,6 +1754,9 @@ claim CRDs. - 2026-07-28: Made `DriverConfig.name` mandatory (no longer derived from type); removed `wait-for-binary.sh` script in favor of direct `jumpstarter-exec` entrypoint in `qemu-runtime` container; updated all examples +- 2026-09-01: Guest disk at `/disk` via `parameters.resources.storage` (size) + and optional `parameters.storage.storageClassName` (generic ephemeral PVC or + sized emptyDir); sockets remain on `/shared` ## References diff --git a/e2e/README.md b/e2e/README.md index e3e656ab4..d8d858702 100644 --- a/e2e/README.md +++ b/e2e/README.md @@ -163,7 +163,7 @@ arch detected via `qemu-guest-arch.sh`; Alpine guest image ensured via | Test Name | Steps | Pass Check | |---|---|---| | brings an Exporter Online with a Ready Pod | wait for ExporterSet-created Exporter, wait Online/Registered/Available, wait Pod Running+Ready | Pod ready; `target-runtime` container has the expected `qemu-system-*` binary | -| leases, flashes Alpine, and boots to a console login marker | (skipped if shared emptyDir `sizeLimit` is empty or `100Mi`, pending #924) run `qemu_flash_boot.py` under `jmp shell --duration 1h` | script output contains "OK: matched marker" | +| leases, flashes Alpine, and boots to a console login marker | run `qemu_flash_boot.py` under `jmp shell --duration 1h` | script output contains "OK: matched marker" | | power cycles QEMU then rotates the Pod/Exporter and stays responsive | record old Pod name/UID; `j qemu power on/off` inside shell, verify qemu binary is the running process; wait old Pod+Exporter deleted, one new Pod/Exporter running w/ new UID; re-run `j qemu power on/off` | ExitAndReplace produced exactly one new, ready, differently-UID'd Pod/Exporter that still answers power commands | --- diff --git a/e2e/test/exporterset_qemu_test.go b/e2e/test/exporterset_qemu_test.go index 36975de64..5cc6dd1bf 100644 --- a/e2e/test/exporterset_qemu_test.go +++ b/e2e/test/exporterset_qemu_test.go @@ -159,24 +159,6 @@ var _ = Describe("ExporterSet QEMU E2E Tests", Label("exporterset-qemu"), Ordere }) 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 { - return KubectlQuery("-n", ns, "get", "pod", - "-l", guest.Selector, - "--field-selector=status.phase=Running", - "-o", "jsonpath={.items[0].metadata.name}") - }, 2*time.Minute, qemuPollPeriod).ShouldNot(BeEmpty()) - - sizeLimit := KubectlQuery("-n", ns, "get", "pod", - "-l", guest.Selector, - "--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( 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 d13d4ef5c..2cba7ecc4 100644 --- a/python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver.py +++ b/python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver.py @@ -496,7 +496,12 @@ def _work_dir(self) -> str: def _disk_dir(self) -> str: """Directory for flashable guest disk images (root, bios, …).""" if self.launcher_socket: - return "/disk" + work = Path(self._work_dir) + # Production sidecar: sockets on /shared, guest disk on /disk. + # Tests use a tmp shared dir — keep disks next to sockets. + if work == Path("/shared"): + return "/disk" + return str(work) return self._tmp_dir.name @property 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 bb7e133b0..777595825 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 @@ -206,6 +206,23 @@ def test_set_memory_size_invalid(): driver.set_memory_size("invalid") +def test_disk_dir_uses_tmp_by_default(): + driver = Qemu() + assert driver._disk_dir == driver._tmp_dir.name + + +def test_disk_dir_stays_with_shared_in_tests(tmp_path): + shared = tmp_path / "shared" + shared.mkdir() + driver = Qemu(launcher_socket=str(shared / "launcher.sock")) + assert driver._disk_dir == str(shared) + + +def test_disk_dir_is_slash_disk_in_production_sidecar(): + driver = Qemu(launcher_socket="/shared/launcher.sock") + assert driver._disk_dir == "/disk" + + def test_cidata_uses_tmp_by_default(): """Local mode keeps cloud-init vvfat content under a system temp dir.""" driver = Qemu()