recovery: add the OpenShift SCC for recovery Jobs

internal/controller/gpurecoveryplan_const.go

@@ -68,6 +68,9 @@ const (
 	// reflashStagingDir is where the fw-copy initContainer stages the firmware inside the Job's emptyDir.
 	reflashStagingDir = "/update"
 
+	// recoveryResourcePart is this component's segment in the OpenShift SCC/Role/Binding/SA name
+	recoveryResourcePart = "gpu-recovery"
+
 	// maxStatusMessages is the maximum number of entries kept in status.messages.
 	maxStatusMessages = 50
 

internal/controller/gpurecoveryplan_controller.go

@@ -68,6 +68,10 @@ type GPURecoveryPlanReconciler struct {
 // +kubebuilder:rbac:groups="",resources=pods,verbs=get;list;watch
 // +kubebuilder:rbac:groups=resource.k8s.io,resources=resourceclaims,verbs=get;list;watch
 
+// On OpenShift the recovery Jobs run under an SCC of their own; the operator has to be able to
+// create it and to grant its use.
+// +kubebuilder:rbac:groups=security.openshift.io,resources=securitycontextconstraints,verbs=create;delete;get;list;watch;use;update
+
 // Reconcile is the main reconciliation loop for GPURecoveryPlan.
 //
 // The loop is triggered by a change to a GPURecoveryPlan (an admin adding an approval, or the
@@ -119,6 +123,15 @@ func (r *GPURecoveryPlanReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		return ctrl.Result{}, fmt.Errorf("syncRecoveryEventsFromSlices: %w", err)
 	}
 
+	// Apply SCCs if in OpenShift
+	if r.Opts.OpenShift {
+		if err := r.ensureOpenShiftResources(ctx, plan.Name); err != nil {
+			appendMessage(plan, fmt.Sprintf("Failed to ensure OpenShift SCC resources: %v", err))
+
+			return ctrl.Result{}, fmt.Errorf("ensureOpenShiftResources: %w", err)
+		}
+	}
+
 	// Move every event an admin has approved forward: into a node drain for a reset, or straight
 	// into a recovery Job for anything that does not need the node emptied.
 	r.processApprovals(ctx, plan)
@@ -245,6 +258,11 @@ func (r *GPURecoveryPlanReconciler) handleFinalizer(ctx context.Context, plan *i
 		// and the taint carries the plan's own name, which nothing else knows to look for.
 		r.releaseAllDrainTaints(ctx, plan)
 
+		// Remove any SCCs as the plan is going away.
+		if r.Opts.OpenShift {
+			r.cleanupOpenShiftResources(ctx, plan.Name)
+		}
+
 		controllerutil.RemoveFinalizer(plan, recoveryPlanFinalizer)
 
 		if err := r.Update(ctx, plan); err != nil {
@@ -910,9 +928,47 @@ func (r *GPURecoveryPlanReconciler) prepareRecoveryJob(job *batch.Job, plan *int
 		job.Spec.Template.Spec.ImagePullSecrets = []core.LocalObjectReference{{Name: r.Opts.SecretName}}
 	}
 
+	// On OpenShift the pod has to run under the ServiceAccount bound to the recovery SCC
+	if r.Opts.OpenShift {
+		_, _, _, saName := buildOpenShiftNames(plan.Name, recoveryResourcePart)
+		job.Spec.Template.Spec.ServiceAccountName = saName
+	}
+
 	return jobName
 }
 
+// ensureOpenShiftResources creates the SCC, ClusterRole, ClusterRoleBinding and ServiceAccount that
+// let recovery Job pods run privileged on OpenShift.
+func (r *GPURecoveryPlanReconciler) ensureOpenShiftResources(ctx context.Context, planName string) error {
+	sccName, roleName, bindingName, saName := buildOpenShiftNames(planName, recoveryResourcePart)
+
+	if err := createServiceAccount(ctx, r.Client, saName, r.Opts.Namespace); err != nil {
+		return fmt.Errorf("failed to ensure recovery ServiceAccount: %w", err)
+	}
+
+	if err := ensureSCC(ctx, r.Client, buildRecoverySCC(sccName)); err != nil {
+		return fmt.Errorf("failed to ensure recovery SCC: %w", err)
+	}
+
+	if err := createSCCRole(ctx, r.Client, roleName, sccName); err != nil {
+		return fmt.Errorf("failed to ensure recovery SCC ClusterRole: %w", err)
+	}
+
+	if err := createSCCRoleBinding(ctx, r.Client, bindingName, roleName, saName, r.Opts.Namespace); err != nil {
+		return fmt.Errorf("failed to ensure recovery SCC ClusterRoleBinding: %w", err)
+	}
+
+	return nil
+}
+
+// cleanupOpenShiftResources removes the SCC quadruple when the plan is deleted. The objects are
+// cluster-scoped — or, for the ServiceAccount, in the operator namespace — and not owned by the
+// plan, so they are not garbage-collected with it.
+func (r *GPURecoveryPlanReconciler) cleanupOpenShiftResources(ctx context.Context, planName string) {
+	sccName, roleName, bindingName, saName := buildOpenShiftNames(planName, recoveryResourcePart)
+	deleteOpenShiftSCCResources(ctx, r.Client, sccName, roleName, bindingName, saName, r.Opts.Namespace)
+}
+
 // createRecoveryJob creates the Job that carries out the event's recovery and moves the event to
 // in-progress.
 func (r *GPURecoveryPlanReconciler) createRecoveryJob(ctx context.Context, plan *intelv1a1.GPURecoveryPlan, evt *intelv1a1.RecoveryEvent) error {

internal/controller/gpurecoveryplan_controller_test.go

@@ -28,9 +28,11 @@ import (
 	batch "k8s.io/api/batch/v1"
 	core "k8s.io/api/core/v1"
 	policy "k8s.io/api/policy/v1"
+	rbac "k8s.io/api/rbac/v1"
 	resv1 "k8s.io/api/resource/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/apimachinery/pkg/util/validation"
@@ -225,6 +227,16 @@ func newTestReconcilerVerifying(v ContentImageVerifier) *GPURecoveryPlanReconcil
 	}
 }
 
+// newOpenShiftTestReconciler is newTestReconciler with OpenShift detection forced on, so the SCC
+// paths can be driven without a real OpenShift cluster. envtest loads the SCC CRD from
+// internal/controller/testdata/scc-crd.yaml, so the objects are really created and read back.
+func newOpenShiftTestReconciler() *GPURecoveryPlanReconciler {
+	r := newTestReconciler()
+	r.Opts.OpenShift = true
+
+	return r
+}
+
 // reconcilePlan runs a single reconcile cycle for the named GPURecoveryPlan.
 func reconcilePlan(ctx context.Context, name string) (reconcile.Result, error) {
 	return newTestReconciler().Reconcile(ctx, reconcile.Request{
@@ -3755,6 +3767,240 @@ var _ = Describe("GPURecoveryPlan Controller", func() {
 		})
 	})
 
+	// Recovery Jobs are privileged, run as root and mount hostPath /sys. On OpenShift the default
+	// restricted SCC rejects that at admission, so without an SCC of their own the recovery never
+	// happens — and the event is already marked in-progress against a pod that never runs.
+	Context("OpenShift SCC handling", func() {
+		// sccNames returns the SCC/Role/Binding/SA quadruple for a plan and registers cleanup for
+		// all four: they are cluster-scoped (bar the SA) and outlive the plan otherwise.
+		sccNames := func(planName string) (string, string, string, string) {
+			sccName, roleName, bindingName, saName := buildOpenShiftNames(planName, recoveryResourcePart)
+
+			DeferCleanup(func() {
+				deleteOpenShiftSCCResources(context.Background(), k8sClient,
+					sccName, roleName, bindingName, saName, "default")
+			})
+
+			return sccName, roleName, bindingName, saName
+		}
+
+		getSCC := func(sccName string) (*unstructured.Unstructured, error) {
+			scc := &unstructured.Unstructured{}
+			scc.SetAPIVersion(sccAPIVersion)
+			scc.SetKind(sccKind)
+
+			return scc, k8sClient.Get(ctx, types.NamespacedName{Name: sccName}, scc)
+		}
+
+		// planWithEvent builds a plan carrying one event in waiting-approval, ready for a direct
+		// createRecoveryJob call.
+		planWithEvent := func(planName, evtID string, rt intelv1a1.RecoveryType) *intelv1a1.GPURecoveryPlan {
+			reason := reasonWedged
+			if rt == intelv1a1.RecoveryTypeReflash {
+				reason = reasonSurvivability
+			}
+
+			return &intelv1a1.GPURecoveryPlan{
+				ObjectMeta: metav1.ObjectMeta{Name: planName},
+				Spec: intelv1a1.GPURecoveryPlanSpec{
+					DefaultResetType: intelv1a1.RecoveryTypeSlot,
+					DeviceID:         "0xabcd",
+					MaxRetries:       3,
+					XpuSmi:           intelv1a1.XpuSmiSpec{Image: "local/xpusmi:devel"},
+				},
+				Status: intelv1a1.GPURecoveryPlanStatus{
+					Events: []intelv1a1.RecoveryEvent{{
+						ID:           evtID,
+						NodeName:     "node01",
+						GPUBDF:       "0000:02:00.0",
+						Reason:       reason,
+						RecoveryType: intelv1a1.RecoveryTypeSpec{Type: rt},
+						State:        intelv1a1.RecoveryEventStateWaitingApproval,
+						LastUpdated:  ptr.To(metav1.Now()),
+					}},
+				},
+			}
+		}
+
+		expectJobServiceAccount := func(jobName, want string) {
+			job := &batch.Job{}
+			Expect(k8sClient.Get(ctx, types.NamespacedName{Name: jobName, Namespace: "default"}, job)).To(Succeed())
+			DeferCleanup(func() {
+				_ = k8sClient.Delete(context.Background(), job)
+			})
+
+			Expect(job.Spec.Template.Spec.ServiceAccountName).To(Equal(want))
+		}
+
+		It("should create the SCC quadruple on reconcile", func() {
+			r := newOpenShiftTestReconciler()
+			key := types.NamespacedName{Name: "plan-openshift-scc"}
+			sccName, roleName, bindingName, saName := sccNames(key.Name)
+
+			createPlanForOwnerRef(&intelv1a1.GPURecoveryPlan{
+				ObjectMeta: metav1.ObjectMeta{Name: key.Name},
+				Spec: intelv1a1.GPURecoveryPlanSpec{
+					DefaultResetType: intelv1a1.RecoveryTypeSlot,
+					DeviceID:         "0xabcd",
+					MaxRetries:       3,
+				},
+			})
+
+			// The first reconcile only adds the finalizer; the second runs the phases.
+			for range 2 {
+				_, err := r.Reconcile(ctx, reconcile.Request{NamespacedName: key})
+				Expect(err).NotTo(HaveOccurred())
+			}
+
+			scc, err := getSCC(sccName)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(scc.Object["allowPrivilegedContainer"]).To(BeTrue())
+
+			role := &rbac.ClusterRole{}
+			Expect(k8sClient.Get(ctx, types.NamespacedName{Name: roleName}, role)).To(Succeed())
+			Expect(role.Rules[0].Verbs).To(ContainElement("use"))
+			Expect(role.Rules[0].ResourceNames).To(ContainElement(sccName),
+				"the Role must grant use of this plan's own SCC, not of every SCC in the cluster")
+
+			binding := &rbac.ClusterRoleBinding{}
+			Expect(k8sClient.Get(ctx, types.NamespacedName{Name: bindingName}, binding)).To(Succeed())
+			Expect(binding.Subjects).To(ConsistOf(rbac.Subject{
+				Kind: "ServiceAccount", Name: saName, Namespace: "default",
+			}))
+
+			Expect(k8sClient.Get(ctx,
+				types.NamespacedName{Name: saName, Namespace: "default"}, &core.ServiceAccount{})).To(Succeed())
+		})
+
+		It("should not create any SCC resources on a plain Kubernetes cluster", func() {
+			r := newTestReconciler()
+			key := types.NamespacedName{Name: "plan-vanilla-k8s"}
+			sccName, _, _, saName := sccNames(key.Name)
+
+			createPlanForOwnerRef(&intelv1a1.GPURecoveryPlan{
+				ObjectMeta: metav1.ObjectMeta{Name: key.Name},
+				Spec: intelv1a1.GPURecoveryPlanSpec{
+					DefaultResetType: intelv1a1.RecoveryTypeSlot,
+					DeviceID:         "0xabcd",
+					MaxRetries:       3,
+				},
+			})
+
+			for range 2 {
+				_, err := r.Reconcile(ctx, reconcile.Request{NamespacedName: key})
+				Expect(err).NotTo(HaveOccurred())
+			}
+
+			_, err := getSCC(sccName)
+			Expect(err).To(Satisfy(errors.IsNotFound))
+			Expect(k8sClient.Get(ctx, types.NamespacedName{Name: saName, Namespace: "default"},
+				&core.ServiceAccount{})).To(Satisfy(errors.IsNotFound),
+				"a cluster without security.openshift.io must not get OpenShift-only objects")
+		})
+
+		// Every reconcile calls it, so it has to be a no-op once the objects are there.
+		It("should be idempotent across reconciles", func() {
+			r := newOpenShiftTestReconciler()
+			sccNames("plan-scc-idem")
+
+			for range 3 {
+				Expect(r.ensureOpenShiftResources(ctx, "plan-scc-idem")).To(Succeed())
+			}
+		})
+
+		// SCC admission picks the constraint from the pod's ServiceAccount. A Job created without
+		// one falls back to restricted and is refused for being privileged.
+		It("should run reset Job pods under the SCC ServiceAccount", func() {
+			r := newOpenShiftTestReconciler()
+			_, _, _, saName := sccNames("plan-scc-job")
+
+			p := planWithEvent("plan-scc-job", "evt-scc-job", intelv1a1.RecoveryTypeSBR)
+			createPlanForOwnerRef(p)
+
+			Expect(r.createRecoveryJob(ctx, p, &p.Status.Events[0])).To(Succeed())
+			expectJobServiceAccount("recovery-evt-scc-job-0", saName)
+		})
+
+		// The reflash Job is built from the other template and by another function, so the SA has
+		// to be asserted on both: prepareRecoveryJob is what they share, and it is where it is set.
+		It("should run reflash Job pods under the SCC ServiceAccount too", func() {
+			r := newOpenShiftTestReconciler()
+			_, _, _, saName := sccNames("plan-scc-reflash")
+
+			p := planWithEvent("plan-scc-reflash", "evt-scc-reflash", intelv1a1.RecoveryTypeReflash)
+			p.Spec.Firmware = &intelv1a1.FirmwareSpec{
+				Source: intelv1a1.FirmwareSource{
+					ContainerSource: &intelv1a1.ContainerFirmwareSource{Name: "registry/fw:v1"},
+				},
+				File: "gfx.bin",
+			}
+			createPlanForOwnerRef(p)
+
+			Expect(r.createRecoveryJob(ctx, p, &p.Status.Events[0])).To(Succeed())
+			expectJobServiceAccount("recovery-evt-scc-reflash-0", saName)
+		})
+
+		It("should not set a ServiceAccount on a plain Kubernetes cluster", func() {
+			r := newTestReconciler()
+
+			p := planWithEvent("plan-no-sa", "evt-no-sa", intelv1a1.RecoveryTypeSBR)
+			createPlanForOwnerRef(p)
+
+			Expect(r.createRecoveryJob(ctx, p, &p.Status.Events[0])).To(Succeed())
+			expectJobServiceAccount("recovery-evt-no-sa-0", "")
+		})
+
+		// The quadruple is cluster-scoped (bar the SA) and not owned by the plan, so nothing
+		// garbage-collects it. A stale SCC left behind keeps granting privileges to a
+		// ServiceAccount name a later, unrelated plan of the same name would reuse.
+		It("should delete the SCC quadruple when the plan is deleted", func() {
+			r := newOpenShiftTestReconciler()
+			key := types.NamespacedName{Name: "plan-scc-delete"}
+			sccName, roleName, bindingName, saName := sccNames(key.Name)
+
+			p := &intelv1a1.GPURecoveryPlan{
+				ObjectMeta: metav1.ObjectMeta{Name: key.Name, Finalizers: []string{recoveryPlanFinalizer}},
+				Spec: intelv1a1.GPURecoveryPlanSpec{
+					DefaultResetType: intelv1a1.RecoveryTypeSlot,
+					DeviceID:         "0xabcd",
+					MaxRetries:       3,
+				},
+			}
+			Expect(k8sClient.Create(ctx, p)).To(Succeed())
+
+			_, err := r.Reconcile(ctx, reconcile.Request{NamespacedName: key})
+			Expect(err).NotTo(HaveOccurred())
+
+			_, err = getSCC(sccName)
+			Expect(err).NotTo(HaveOccurred())
+
+			Expect(k8sClient.Delete(ctx, p)).To(Succeed())
+
+			// The deletion path runs first and removes the finalizer, so the object is gone once
+			// this reconcile returns.
+			_, err = r.Reconcile(ctx, reconcile.Request{NamespacedName: key})
+			Expect(err).NotTo(HaveOccurred())
+
+			_, err = getSCC(sccName)
+			Expect(err).To(Satisfy(errors.IsNotFound))
+			Expect(k8sClient.Get(ctx, types.NamespacedName{Name: roleName},
+				&rbac.ClusterRole{})).To(Satisfy(errors.IsNotFound))
+			Expect(k8sClient.Get(ctx, types.NamespacedName{Name: bindingName},
+				&rbac.ClusterRoleBinding{})).To(Satisfy(errors.IsNotFound))
+			Expect(k8sClient.Get(ctx, types.NamespacedName{Name: saName, Namespace: "default"},
+				&core.ServiceAccount{})).To(Satisfy(errors.IsNotFound))
+		})
+
+		// Two plans in one cluster must not share an SCC: deleting one would otherwise strip the
+		// other's recovery Jobs of the grounds they were admitted on, mid-reset.
+		It("should name the SCC quadruple per plan", func() {
+			sccA, roleA, bindingA, saA := buildOpenShiftNames("plan-a", recoveryResourcePart)
+			sccB, roleB, bindingB, saB := buildOpenShiftNames("plan-b", recoveryResourcePart)
+
+			Expect([]string{sccA, roleA, bindingA, saA}).NotTo(ContainElements(sccB, roleB, bindingB, saB))
+		})
+	})
+
 	Context("Reconcile: long node names still produce a creatable Job", func() {
 		It("should create the recovery Job for a node name well over the limit", func() {
 			r := newTestReconciler()