diff --git a/charts/gpu-base-operator/templates/role.yaml b/charts/gpu-base-operator/templates/role.yaml index c9a4999..53d9d80 100644 --- a/charts/gpu-base-operator/templates/role.yaml +++ b/charts/gpu-base-operator/templates/role.yaml @@ -165,11 +165,14 @@ rules: - resource.k8s.io resources: - deviceclasses + - resourceclaimtemplates + - resourceslices verbs: - create - delete - get - list + - patch - update - watch - apiGroups: @@ -180,19 +183,6 @@ rules: - get - list - watch -- apiGroups: - - resource.k8s.io - resources: - - resourceclaimtemplates - - resourceslices - verbs: - - create - - delete - - get - - list - - patch - - update - - watch - apiGroups: - security.openshift.io resources: diff --git a/config/deployments/deployments.go b/config/deployments/deployments.go index 23ff2a5..7466825 100644 --- a/config/deployments/deployments.go +++ b/config/deployments/deployments.go @@ -32,8 +32,10 @@ import ( ) const ( - vfioExpression = `device.attributes["gpu.intel.com"].driver == 'vfio-pci'` - xeVfioExpression = `device.attributes["gpu.intel.com"].driver == 'xe-vfio-pci'` + vfioDriverExpression = `device.attributes["gpu.intel.com"].driver == 'vfio-pci'` + xeVfioDriverExpression = `device.attributes["gpu.intel.com"].driver == 'xe-vfio-pci'` + i915DriverExpressions = `device.attributes["gpu.intel.com"].driver == 'i915'` + xeDriverExpressions = `device.attributes["gpu.intel.com"].driver == 'xe'` ) // XPU Manager @@ -94,8 +96,18 @@ func DynamicResourceAllocationServiceAccount() *core.ServiceAccount { //go:embed dra/device-class.yaml var contentDRADC []byte -func DynamicResourceAllocationDeviceClass() *resv1.DeviceClass { - return getDeviceClass(contentDRADC).DeepCopy() +func DynamicResourceAllocationDeviceClass(limitToKMDs bool) *resv1.DeviceClass { + dc := getDeviceClass(contentDRADC).DeepCopy() + + if limitToKMDs { + dc.Spec.Selectors = append(dc.Spec.Selectors, resv1.DeviceSelector{ + CEL: &resv1.CELDeviceSelector{ + Expression: fmt.Sprintf("%s || %s", i915DriverExpressions, xeDriverExpressions), + }, + }) + } + + return dc } //go:embed dra/device-class-vfio.yaml @@ -108,7 +120,7 @@ func DynamicResourceAllocationDeviceClassVfio(limitToVfio bool) *resv1.DeviceCla if limitToVfio { dc.Spec.Selectors = append(dc.Spec.Selectors, resv1.DeviceSelector{ CEL: &resv1.CELDeviceSelector{ - Expression: fmt.Sprintf("%s || %s", vfioExpression, xeVfioExpression), + Expression: fmt.Sprintf("%s || %s", vfioDriverExpression, xeVfioDriverExpression), }, }) } diff --git a/config/deployments/deployments_test.go b/config/deployments/deployments_test.go index 41f790d..1dafb33 100644 --- a/config/deployments/deployments_test.go +++ b/config/deployments/deployments_test.go @@ -18,6 +18,7 @@ import ( "testing" core "k8s.io/api/core/v1" + resv1 "k8s.io/api/resource/v1" ) const ( @@ -86,17 +87,27 @@ func TestDynamicResourceAllocationServiceAccount(t *testing.T) { } } -func TestDynamicResourceAllocationDeviceClass(t *testing.T) { - dc := DynamicResourceAllocationDeviceClass() - if dc == nil { - t.Error("DynamicResourceAllocationDeviceClass returned nil") - } -} - -func TestDynamicResourceAllocationDeviceClassVfio(t *testing.T) { - dc := DynamicResourceAllocationDeviceClassVfio(false) - if dc == nil { - t.Error("DynamicResourceAllocationDeviceClassVfio returned nil") +func TestDeviceClassDriverSelectors(t *testing.T) { + for _, tc := range []struct { + name string + get func(bool) *resv1.DeviceClass + limit bool + selectors int + }{ + {"DeviceClass without driver limit", DynamicResourceAllocationDeviceClass, false, 1}, + {"DeviceClass with driver limit", DynamicResourceAllocationDeviceClass, true, 2}, + {"vfio DeviceClass without driver limit", DynamicResourceAllocationDeviceClassVfio, false, 1}, + {"vfio DeviceClass with driver limit", DynamicResourceAllocationDeviceClassVfio, true, 2}, + } { + t.Run(tc.name, func(t *testing.T) { + dc := tc.get(tc.limit) + if dc == nil { + t.Fatal("device class getter returned nil") + } + if len(dc.Spec.Selectors) != tc.selectors { + t.Errorf("expected %d selector(s), got %d", tc.selectors, len(dc.Spec.Selectors)) + } + }) } } @@ -532,3 +543,34 @@ func TestDRAClusterRole_NoWildcardsAndNoSecrets(t *testing.T) { } } } + +func TestUnmarshalFailurePanics(t *testing.T) { + for _, tc := range []struct { + name string + get func([]byte) + }{ + {"getService", func(b []byte) { _ = getService(b) }}, + {"getServiceAccount", func(b []byte) { _ = getServiceAccount(b) }}, + {"getDaemonset", func(b []byte) { _ = getDaemonset(b) }}, + {"getClusterRole", func(b []byte) { _ = getClusterRole(b) }}, + {"getClusterRoleBinding", func(b []byte) { _ = getClusterRoleBinding(b) }}, + {"getAdmissionPolicy", func(b []byte) { _ = getAdmissionPolicy(b) }}, + {"getAdmissionPolicyBinding", func(b []byte) { _ = getAdmissionPolicyBinding(b) }}, + {"getDeviceClass", func(b []byte) { _ = getDeviceClass(b) }}, + {"getResourceClaimTemplate", func(b []byte) { _ = getResourceClaimTemplate(b) }}, + {"getNodeFeatureRule", func(b []byte) { _ = getNodeFeatureRule(b) }}, + {"getServiceMonitor", func(b []byte) { _ = getServiceMonitor(b) }}, + {"getJob", func(b []byte) { _ = getJob(b) }}, + {"getOTelConfig", func(b []byte) { _ = getOTelConfig(b) }}, + } { + t.Run(tc.name, func(t *testing.T) { + defer func() { + if r := recover(); r == nil { + t.Errorf("expected panic when %s unmarshals invalid YAML", tc.name) + } + }() + + tc.get([]byte("[")) + }) + } +} diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index eb7206a..73cda25 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -165,11 +165,14 @@ rules: - resource.k8s.io resources: - deviceclasses + - resourceclaimtemplates + - resourceslices verbs: - create - delete - get - list + - patch - update - watch - apiGroups: @@ -180,19 +183,6 @@ rules: - get - list - watch -- apiGroups: - - resource.k8s.io - resources: - - resourceclaimtemplates - - resourceslices - verbs: - - create - - delete - - get - - list - - patch - - update - - watch - apiGroups: - security.openshift.io resources: diff --git a/internal/controller/clusterpolicy_controller.go b/internal/controller/clusterpolicy_controller.go index 8ac81ad..71b1555 100644 --- a/internal/controller/clusterpolicy_controller.go +++ b/internal/controller/clusterpolicy_controller.go @@ -159,7 +159,7 @@ func addIfMissing(slice *[]string, s string) { // +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterrolebindings,verbs=get;list;create;delete;watch // +kubebuilder:rbac:groups=admissionregistration.k8s.io,resources=validatingadmissionpolicies,verbs=get;list;create;delete // +kubebuilder:rbac:groups=admissionregistration.k8s.io,resources=validatingadmissionpolicybindings,verbs=get;list;create;delete -// +kubebuilder:rbac:groups=resource.k8s.io,resources=deviceclasses,verbs=get;list;create;delete;watch;update +// +kubebuilder:rbac:groups=resource.k8s.io,resources=deviceclasses,verbs=get;list;create;delete;watch;update;patch // +kubebuilder:rbac:groups=resource.k8s.io,resources=resourceclaims,verbs=get;list;watch // +kubebuilder:rbac:groups=resource.k8s.io,resources=resourceslices,verbs=get;list;watch;create;update;patch;delete diff --git a/internal/controller/dra_controller.go b/internal/controller/dra_controller.go index c7af483..66bb805 100644 --- a/internal/controller/dra_controller.go +++ b/internal/controller/dra_controller.go @@ -84,11 +84,13 @@ func (r *DRAReconciler) createDependencyComponentsIfMissing(ctx context.Context, crb.RoleRef.Name = objName objects = append(objects, crb) - // Device classes - objects = append(objects, deployments.DynamicResourceAllocationDeviceClass()) - - // Device Class for VFIO and configure it based on the ManageBinding setting in the CR. + // Device classes are configured via ManageBinding setting in the CR. mb := cp.Spec.DynamicResourceAllocationSpec.ManageBinding + + // DRM device class + objects = append(objects, deployments.DynamicResourceAllocationDeviceClass(!mb)) + + // VFIO device class objects = append(objects, deployments.DynamicResourceAllocationDeviceClassVfio(!mb)) // Validating admission policy @@ -120,6 +122,22 @@ func (r *DRAReconciler) createDependencyComponentsIfMissing(ctx context.Context, return nil } +func (r *DRAReconciler) ensureDrmDeviceClass(ctx context.Context, manageBinding bool) { + desired := deployments.DynamicResourceAllocationDeviceClass(!manageBinding) + + selectors := desired.Spec.Selectors + + if ret, err := controllerutil.CreateOrPatch(ctx, r.Client, desired, func() error { + desired.Spec.Selectors = selectors + + return nil + }); err != nil { + klog.Error(err, "unable to create or patch DRM device class") + } else { + klog.V(4).Infof("DRM device class %s %s", desired.Name, ret) + } +} + func (r *DRAReconciler) ensureVfioDeviceClass(ctx context.Context, manageBinding bool) { desired := deployments.DynamicResourceAllocationDeviceClassVfio(!manageBinding) @@ -131,7 +149,6 @@ func (r *DRAReconciler) ensureVfioDeviceClass(ctx context.Context, manageBinding return nil }); err != nil { klog.Error(err, "unable to create or patch VFIO device class") - return } else { klog.V(4).Infof("VFIO device class %s %s", desired.Name, ret) } @@ -472,7 +489,10 @@ func (r *DRAReconciler) Reconcile(ctx context.Context, cp *v1alpha.ClusterPolicy return ctrl.Result{}, err } - r.ensureVfioDeviceClass(ctx, cp.Spec.DynamicResourceAllocationSpec.ManageBinding) + manageBinding := cp.Spec.DynamicResourceAllocationSpec.ManageBinding + + r.ensureDrmDeviceClass(ctx, manageBinding) + r.ensureVfioDeviceClass(ctx, manageBinding) ds := r.buildDraDaemonset(cp) diff --git a/internal/controller/dra_controller_test.go b/internal/controller/dra_controller_test.go index c4f4fa6..e4ed5c2 100644 --- a/internal/controller/dra_controller_test.go +++ b/internal/controller/dra_controller_test.go @@ -223,6 +223,7 @@ var _ = Describe("ClusterPolicy Controller for DRA", func() { for _, dc := range dcList.Items { switch dc.Name { case "gpu.intel.com": + Expect(dc.Spec.Selectors).To(HaveLen(2)) case "gpu-vfio.intel.com": Expect(dc.Spec.Selectors).To(HaveLen(2)) default: @@ -247,6 +248,7 @@ var _ = Describe("ClusterPolicy Controller for DRA", func() { for _, dc := range dcList.Items { switch dc.Name { case "gpu.intel.com": + Expect(dc.Spec.Selectors).To(HaveLen(1)) case "gpu-vfio.intel.com": Expect(dc.Spec.Selectors).To(HaveLen(1)) default: