From 02b215ff4a3d50cf9396c8dc562dd5be585d7486 Mon Sep 17 00:00:00 2001 From: Tzif Date: Sun, 9 Aug 2026 11:52:02 +0300 Subject: [PATCH 1/3] OSAC-3162: add GpuSpec to ComputeInstance CRD Add GpuSpec struct (PciDeviceSelector, ResourceName, Count) and optional Gpu field to ComputeInstanceSpec, following the existing ImageSpec/DiskSpec nested struct pattern with kubebuilder validation markers. Includes unit tests (struct-level) and envtest CRD validation tests (valid GPU, no GPU, omitempty, count boundaries, empty string rejection). Regenerated CRD manifests and synced Helm charts. Assisted-by: Claude Code Signed-off-by: Tzif --- .../api/v1alpha1/computeinstance_types.go | 23 ++++++ .../v1alpha1/computeinstance_types_test.go | 24 ++++++ .../api/v1alpha1/zz_generated.deepcopy.go | 20 +++++ .../osac.openshift.io_computeinstances.yaml | 25 ++++++ .../osac.openshift.io_computeinstances.yaml | 25 ++++++ .../computeinstance_validation_test.go | 79 +++++++++++++++++++ 6 files changed, 196 insertions(+) diff --git a/osac-operator/api/v1alpha1/computeinstance_types.go b/osac-operator/api/v1alpha1/computeinstance_types.go index 6d4d7a116c..de7b752766 100644 --- a/osac-operator/api/v1alpha1/computeinstance_types.go +++ b/osac-operator/api/v1alpha1/computeinstance_types.go @@ -60,6 +60,25 @@ type DiskSpec struct { StorageTier string `json:"storageTier,omitempty"` } +// GpuSpec defines GPU passthrough configuration resolved from the InstanceType. +type GpuSpec struct { + // PciDeviceSelector is the PCI vendor:device ID (e.g., "10DE:20B0"). + // +kubebuilder:validation:Required + // +kubebuilder:validation:MinLength=1 + PciDeviceSelector string `json:"pciDeviceSelector"` + + // ResourceName is the Kubernetes device plugin resource (e.g., "nvidia.com/A100"). + // +kubebuilder:validation:Required + // +kubebuilder:validation:MinLength=1 + ResourceName string `json:"resourceName"` + + // Count is the number of GPU devices of this type. + // +kubebuilder:validation:Required + // +kubebuilder:validation:Minimum=1 + // +kubebuilder:validation:Maximum=16 + Count int32 `json:"count"` +} + // RunStrategyType defines valid VM run strategies // +kubebuilder:validation:Enum=Always;Halted type RunStrategyType string @@ -137,6 +156,10 @@ type ComputeInstanceSpec struct { // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="additionalDisks is immutable" AdditionalDisks []DiskSpec `json:"additionalDisks,omitempty"` + // Gpu defines GPU passthrough configuration resolved from the InstanceType. + // +optional + Gpu *GpuSpec `json:"gpu,omitempty"` + // RunStrategy controls VM running state (MUTABLE) // +kubebuilder:validation:Required RunStrategy RunStrategyType `json:"runStrategy"` diff --git a/osac-operator/api/v1alpha1/computeinstance_types_test.go b/osac-operator/api/v1alpha1/computeinstance_types_test.go index 0c0971fb70..73f0b2f4c7 100644 --- a/osac-operator/api/v1alpha1/computeinstance_types_test.go +++ b/osac-operator/api/v1alpha1/computeinstance_types_test.go @@ -106,6 +106,30 @@ var _ = Describe("ComputeInstanceSpec", func() { Expect(spec.UserDataSecretRef.Name).To(Equal("my-cloud-init")) }) + It("should support GPU spec", func() { + spec := v1alpha1.ComputeInstanceSpec{ + TemplateID: "test-template", + Image: v1alpha1.ImageSpec{ + SourceType: v1alpha1.ImageSourceTypeRegistry, + SourceRef: "test-image:latest", + }, + Cores: 8, + MemoryGiB: 64, + BootDisk: v1alpha1.DiskSpec{SizeGiB: 100}, + Gpu: &v1alpha1.GpuSpec{ + PciDeviceSelector: "10DE:20B0", + ResourceName: "nvidia.com/A100", + Count: 2, + }, + RunStrategy: v1alpha1.RunStrategyAlways, + } + + Expect(spec.Gpu).ToNot(BeNil()) + Expect(spec.Gpu.PciDeviceSelector).To(Equal("10DE:20B0")) + Expect(spec.Gpu.ResourceName).To(Equal("nvidia.com/A100")) + Expect(spec.Gpu.Count).To(Equal(int32(2))) + }) + It("should support SSH key", func() { sshKey := "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQ..." spec := v1alpha1.ComputeInstanceSpec{ diff --git a/osac-operator/api/v1alpha1/zz_generated.deepcopy.go b/osac-operator/api/v1alpha1/zz_generated.deepcopy.go index 101281d8b7..96d02c02d1 100644 --- a/osac-operator/api/v1alpha1/zz_generated.deepcopy.go +++ b/osac-operator/api/v1alpha1/zz_generated.deepcopy.go @@ -270,6 +270,11 @@ func (in *ComputeInstanceSpec) DeepCopyInto(out *ComputeInstanceSpec) { *out = make([]DiskSpec, len(*in)) copy(*out, *in) } + if in.Gpu != nil { + in, out := &in.Gpu, &out.Gpu + *out = new(GpuSpec) + **out = **in + } if in.UserDataSecretRef != nil { in, out := &in.UserDataSecretRef, &out.UserDataSecretRef *out = new(corev1.LocalObjectReference) @@ -690,6 +695,21 @@ func (in *ExternalIPStatus) DeepCopy() *ExternalIPStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *GpuSpec) DeepCopyInto(out *GpuSpec) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GpuSpec. +func (in *GpuSpec) DeepCopy() *GpuSpec { + if in == nil { + return nil + } + out := new(GpuSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ImageSpec) DeepCopyInto(out *ImageSpec) { *out = *in diff --git a/osac-operator/charts/operator-crds/templates/osac.openshift.io_computeinstances.yaml b/osac-operator/charts/operator-crds/templates/osac.openshift.io_computeinstances.yaml index 308688a23c..e52af3259f 100644 --- a/osac-operator/charts/operator-crds/templates/osac.openshift.io_computeinstances.yaml +++ b/osac-operator/charts/operator-crds/templates/osac.openshift.io_computeinstances.yaml @@ -121,6 +121,31 @@ spec: x-kubernetes-validations: - message: cores is immutable rule: self == oldSelf + gpu: + description: Gpu defines GPU passthrough configuration resolved from + the InstanceType. + properties: + count: + description: Count is the number of GPU devices of this type. + format: int32 + maximum: 16 + minimum: 1 + type: integer + pciDeviceSelector: + description: PciDeviceSelector is the PCI vendor:device ID (e.g., + "10DE:20B0"). + minLength: 1 + type: string + resourceName: + description: ResourceName is the Kubernetes device plugin resource + (e.g., "nvidia.com/A100"). + minLength: 1 + type: string + required: + - count + - pciDeviceSelector + - resourceName + type: object guestOSFamily: description: |- GuestOSFamily specifies the guest operating system family for the VM. diff --git a/osac-operator/config/crd/bases/osac.openshift.io_computeinstances.yaml b/osac-operator/config/crd/bases/osac.openshift.io_computeinstances.yaml index 87ab960c07..7dc898b27d 100644 --- a/osac-operator/config/crd/bases/osac.openshift.io_computeinstances.yaml +++ b/osac-operator/config/crd/bases/osac.openshift.io_computeinstances.yaml @@ -119,6 +119,31 @@ spec: x-kubernetes-validations: - message: cores is immutable rule: self == oldSelf + gpu: + description: Gpu defines GPU passthrough configuration resolved from + the InstanceType. + properties: + count: + description: Count is the number of GPU devices of this type. + format: int32 + maximum: 16 + minimum: 1 + type: integer + pciDeviceSelector: + description: PciDeviceSelector is the PCI vendor:device ID (e.g., + "10DE:20B0"). + minLength: 1 + type: string + resourceName: + description: ResourceName is the Kubernetes device plugin resource + (e.g., "nvidia.com/A100"). + minLength: 1 + type: string + required: + - count + - pciDeviceSelector + - resourceName + type: object guestOSFamily: description: |- GuestOSFamily specifies the guest operating system family for the VM. diff --git a/osac-operator/internal/controller/computeinstance_validation_test.go b/osac-operator/internal/controller/computeinstance_validation_test.go index 96847ba3b5..98540d4e65 100644 --- a/osac-operator/internal/controller/computeinstance_validation_test.go +++ b/osac-operator/internal/controller/computeinstance_validation_test.go @@ -488,6 +488,85 @@ var _ = Describe("ComputeInstance CEL Validation", func() { }) }) + Describe("GPU validation", func() { + It("should allow creation with valid GPU spec", func() { + instance := createValidInstance("test-gpu-valid") + instance.Spec.Gpu = &osacv1alpha1.GpuSpec{ + PciDeviceSelector: "10DE:20B0", + ResourceName: "nvidia.com/A100", + Count: 1, + } + + Expect(k8sClient.Create(ctx, instance)).To(Succeed()) + }) + + It("should allow creation without GPU spec", func() { + instance := createValidInstance("test-gpu-none") + + Expect(k8sClient.Create(ctx, instance)).To(Succeed()) + }) + + It("should omit gpu from JSON when not set (omitempty)", func() { + instance := createValidInstance("test-gpu-omitempty") + Expect(k8sClient.Create(ctx, instance)).To(Succeed()) + + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + Expect(instance.Spec.Gpu).To(BeNil()) + }) + + It("should reject GPU with count below minimum", func() { + instance := createValidInstance("test-gpu-count-zero") + instance.Spec.Gpu = &osacv1alpha1.GpuSpec{ + PciDeviceSelector: "10DE:20B0", + ResourceName: "nvidia.com/A100", + Count: 0, + } + + err := k8sClient.Create(ctx, instance) + Expect(err).To(HaveOccurred()) + Expect(apierrors.IsInvalid(err)).To(BeTrue()) + }) + + It("should reject GPU with count above maximum", func() { + instance := createValidInstance("test-gpu-count-max") + instance.Spec.Gpu = &osacv1alpha1.GpuSpec{ + PciDeviceSelector: "10DE:20B0", + ResourceName: "nvidia.com/A100", + Count: 17, + } + + err := k8sClient.Create(ctx, instance) + Expect(err).To(HaveOccurred()) + Expect(apierrors.IsInvalid(err)).To(BeTrue()) + }) + + It("should reject GPU with empty pciDeviceSelector", func() { + instance := createValidInstance("test-gpu-empty-pci") + instance.Spec.Gpu = &osacv1alpha1.GpuSpec{ + PciDeviceSelector: "", + ResourceName: "nvidia.com/A100", + Count: 1, + } + + err := k8sClient.Create(ctx, instance) + Expect(err).To(HaveOccurred()) + Expect(apierrors.IsInvalid(err)).To(BeTrue()) + }) + + It("should reject GPU with empty resourceName", func() { + instance := createValidInstance("test-gpu-empty-rn") + instance.Spec.Gpu = &osacv1alpha1.GpuSpec{ + PciDeviceSelector: "10DE:20B0", + ResourceName: "", + Count: 1, + } + + err := k8sClient.Create(ctx, instance) + Expect(err).To(HaveOccurred()) + Expect(apierrors.IsInvalid(err)).To(BeTrue()) + }) + }) + Describe("MaxItems validation", func() { It("should reject creating ComputeInstance with more than 8 networkAttachments", func() { instance := createValidInstance("test-max-attachments") From f9c1c6f50de626f70f17a9f0f2db6e3776539ee5 Mon Sep 17 00:00:00 2001 From: Tzif Date: Wed, 12 Aug 2026 10:00:17 +0300 Subject: [PATCH 2/3] OSAC-3162: add immutability rule and fix optional marker for GPU field Add XValidation immutability rule to the Gpu field, consistent with all other hardware fields (cores, memoryGiB, bootDisk, etc.). Fix the optionality marker from // +optional to // +kubebuilder:validation:Optional to match codebase convention. Signed-off-by: Tzif Assisted-by: Claude Code Signed-off-by: Tzif --- .../api/v1alpha1/computeinstance_types.go | 3 ++- .../osac.openshift.io_computeinstances.yaml | 3 +++ .../osac.openshift.io_computeinstances.yaml | 3 +++ .../computeinstance_validation_test.go | 18 ++++++++++++++++++ 4 files changed, 26 insertions(+), 1 deletion(-) diff --git a/osac-operator/api/v1alpha1/computeinstance_types.go b/osac-operator/api/v1alpha1/computeinstance_types.go index de7b752766..3b64b8d9cf 100644 --- a/osac-operator/api/v1alpha1/computeinstance_types.go +++ b/osac-operator/api/v1alpha1/computeinstance_types.go @@ -157,7 +157,8 @@ type ComputeInstanceSpec struct { AdditionalDisks []DiskSpec `json:"additionalDisks,omitempty"` // Gpu defines GPU passthrough configuration resolved from the InstanceType. - // +optional + // +kubebuilder:validation:Optional + // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="gpu is immutable" Gpu *GpuSpec `json:"gpu,omitempty"` // RunStrategy controls VM running state (MUTABLE) diff --git a/osac-operator/charts/operator-crds/templates/osac.openshift.io_computeinstances.yaml b/osac-operator/charts/operator-crds/templates/osac.openshift.io_computeinstances.yaml index e52af3259f..f304357413 100644 --- a/osac-operator/charts/operator-crds/templates/osac.openshift.io_computeinstances.yaml +++ b/osac-operator/charts/operator-crds/templates/osac.openshift.io_computeinstances.yaml @@ -146,6 +146,9 @@ spec: - pciDeviceSelector - resourceName type: object + x-kubernetes-validations: + - message: gpu is immutable + rule: self == oldSelf guestOSFamily: description: |- GuestOSFamily specifies the guest operating system family for the VM. diff --git a/osac-operator/config/crd/bases/osac.openshift.io_computeinstances.yaml b/osac-operator/config/crd/bases/osac.openshift.io_computeinstances.yaml index 7dc898b27d..a74d1ace99 100644 --- a/osac-operator/config/crd/bases/osac.openshift.io_computeinstances.yaml +++ b/osac-operator/config/crd/bases/osac.openshift.io_computeinstances.yaml @@ -144,6 +144,9 @@ spec: - pciDeviceSelector - resourceName type: object + x-kubernetes-validations: + - message: gpu is immutable + rule: self == oldSelf guestOSFamily: description: |- GuestOSFamily specifies the guest operating system family for the VM. diff --git a/osac-operator/internal/controller/computeinstance_validation_test.go b/osac-operator/internal/controller/computeinstance_validation_test.go index 98540d4e65..c729d3bade 100644 --- a/osac-operator/internal/controller/computeinstance_validation_test.go +++ b/osac-operator/internal/controller/computeinstance_validation_test.go @@ -565,6 +565,24 @@ var _ = Describe("ComputeInstance CEL Validation", func() { Expect(err).To(HaveOccurred()) Expect(apierrors.IsInvalid(err)).To(BeTrue()) }) + + It("should reject changing gpu", func() { + instance := createValidInstance("test-gpu-immutable") + instance.Spec.Gpu = &osacv1alpha1.GpuSpec{ + PciDeviceSelector: "10DE:20B0", + ResourceName: "nvidia.com/A100", + Count: 1, + } + Expect(k8sClient.Create(ctx, instance)).To(Succeed()) + + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + + instance.Spec.Gpu.Count = 2 + err := k8sClient.Update(ctx, instance) + Expect(err).To(HaveOccurred()) + Expect(apierrors.IsInvalid(err)).To(BeTrue()) + Expect(err.Error()).To(ContainSubstring("gpu is immutable")) + }) }) Describe("MaxItems validation", func() { From 57887832bdbbfd4fcd276f6d81da2b72dd661637 Mon Sep 17 00:00:00 2001 From: Tzif Date: Wed, 12 Aug 2026 17:25:27 +0300 Subject: [PATCH 3/3] OSAC-3162: use spec-level has() rule for GPU immutability MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move GPU immutability from field-level self == oldSelf to spec-level has() check to also block absent-to-present transitions. Add test for the absent→present case. Assisted-by: Claude Code Signed-off-by: Tzif --- .../api/v1alpha1/computeinstance_types.go | 2 +- .../osac.openshift.io_computeinstances.yaml | 7 ++++--- .../osac.openshift.io_computeinstances.yaml | 7 ++++--- .../computeinstance_validation_test.go | 17 +++++++++++++++++ 4 files changed, 26 insertions(+), 7 deletions(-) diff --git a/osac-operator/api/v1alpha1/computeinstance_types.go b/osac-operator/api/v1alpha1/computeinstance_types.go index 3b64b8d9cf..2a111686f7 100644 --- a/osac-operator/api/v1alpha1/computeinstance_types.go +++ b/osac-operator/api/v1alpha1/computeinstance_types.go @@ -107,6 +107,7 @@ type NetworkAttachment struct { } // ComputeInstanceSpec defines the desired state of ComputeInstance +// +kubebuilder:validation:XValidation:rule="has(self.gpu) == has(oldSelf.gpu) && (!has(self.gpu) || self.gpu == oldSelf.gpu)",message="gpu is immutable" type ComputeInstanceSpec struct { // TemplateID is the unique identifier of the compute instance template to use when creating this compute instance // +kubebuilder:validation:Required @@ -158,7 +159,6 @@ type ComputeInstanceSpec struct { // Gpu defines GPU passthrough configuration resolved from the InstanceType. // +kubebuilder:validation:Optional - // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="gpu is immutable" Gpu *GpuSpec `json:"gpu,omitempty"` // RunStrategy controls VM running state (MUTABLE) diff --git a/osac-operator/charts/operator-crds/templates/osac.openshift.io_computeinstances.yaml b/osac-operator/charts/operator-crds/templates/osac.openshift.io_computeinstances.yaml index f304357413..bf322fb400 100644 --- a/osac-operator/charts/operator-crds/templates/osac.openshift.io_computeinstances.yaml +++ b/osac-operator/charts/operator-crds/templates/osac.openshift.io_computeinstances.yaml @@ -146,9 +146,6 @@ spec: - pciDeviceSelector - resourceName type: object - x-kubernetes-validations: - - message: gpu is immutable - rule: self == oldSelf guestOSFamily: description: |- GuestOSFamily specifies the guest operating system family for the VM. @@ -309,6 +306,10 @@ spec: - runStrategy - templateID type: object + x-kubernetes-validations: + - message: gpu is immutable + rule: has(self.gpu) == has(oldSelf.gpu) && (!has(self.gpu) || self.gpu + == oldSelf.gpu) status: description: status defines the observed state of ComputeInstance properties: diff --git a/osac-operator/config/crd/bases/osac.openshift.io_computeinstances.yaml b/osac-operator/config/crd/bases/osac.openshift.io_computeinstances.yaml index a74d1ace99..99d07e73d7 100644 --- a/osac-operator/config/crd/bases/osac.openshift.io_computeinstances.yaml +++ b/osac-operator/config/crd/bases/osac.openshift.io_computeinstances.yaml @@ -144,9 +144,6 @@ spec: - pciDeviceSelector - resourceName type: object - x-kubernetes-validations: - - message: gpu is immutable - rule: self == oldSelf guestOSFamily: description: |- GuestOSFamily specifies the guest operating system family for the VM. @@ -307,6 +304,10 @@ spec: - runStrategy - templateID type: object + x-kubernetes-validations: + - message: gpu is immutable + rule: has(self.gpu) == has(oldSelf.gpu) && (!has(self.gpu) || self.gpu + == oldSelf.gpu) status: description: status defines the observed state of ComputeInstance properties: diff --git a/osac-operator/internal/controller/computeinstance_validation_test.go b/osac-operator/internal/controller/computeinstance_validation_test.go index c729d3bade..c2e3ef0deb 100644 --- a/osac-operator/internal/controller/computeinstance_validation_test.go +++ b/osac-operator/internal/controller/computeinstance_validation_test.go @@ -583,6 +583,23 @@ var _ = Describe("ComputeInstance CEL Validation", func() { Expect(apierrors.IsInvalid(err)).To(BeTrue()) Expect(err.Error()).To(ContainSubstring("gpu is immutable")) }) + + It("should reject adding gpu after creation", func() { + instance := createValidInstance("test-gpu-add-after") + Expect(k8sClient.Create(ctx, instance)).To(Succeed()) + + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed()) + + instance.Spec.Gpu = &osacv1alpha1.GpuSpec{ + PciDeviceSelector: "10DE:20B0", + ResourceName: "nvidia.com/A100", + Count: 1, + } + err := k8sClient.Update(ctx, instance) + Expect(err).To(HaveOccurred()) + Expect(apierrors.IsInvalid(err)).To(BeTrue()) + Expect(err.Error()).To(ContainSubstring("gpu is immutable")) + }) }) Describe("MaxItems validation", func() {