DRA: add DRAControlPlaneController feature gate for "classic DRA"

In the API, the effect of the feature gate is that alpha fields get dropped on
create. They get preserved during updates if already set. The
PodSchedulingContext registration is *not* restricted by the feature gate.
This enables deleting stale PodSchedulingContext objects after disabling
the feature gate.

The scheduler checks the new feature gate before setting up an informer for
PodSchedulingContext objects and when deciding whether it can schedule a
pod. If any claim depends on a control plane controller, the scheduler bails
out, leading to:

    Status:       Pending
    ...
      Warning  FailedScheduling             73s   default-scheduler  0/1 nodes are available: resourceclaim depends on disabled DRAControlPlaneController feature. no new claims to deallocate, preemption: 0/1 nodes are available: 1 Preemption is not helpful for scheduling.

The rest of the changes prepare for testing the new feature separately from
"structured parameters". The goal is to have base "dra" jobs which just enable
and test those, then "classic-dra" jobs which add DRAControlPlaneController.

Author: pohly

pkg/features/kube_features.go

@@ -220,7 +220,17 @@ const (
 	// alpha: v1.26
 	//
 	// Enables support for resources with custom parameters and a lifecycle
-	// that is independent of a Pod.
+	// that is independent of a Pod. Resource allocation is done by a DRA driver's
+	// "control plane controller" in cooperation with the scheduler.
+	DRAControlPlaneController featuregate.Feature = "DRAControlPlaneController"
+
+	// owner: @pohly
+	// kep: http://kep.k8s.io/4381
+	// alpha: v1.29
+	//
+	// Enables support for resources with custom parameters and a lifecycle
+	// that is independent of a Pod. Resource allocation is done by the scheduler
+	// based on "structured parameters".
 	DynamicResourceAllocation featuregate.Feature = "DynamicResourceAllocation"
 
 	// owner: @harche
@@ -1044,6 +1054,8 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 
 	DevicePluginCDIDevices: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.33
 
+	DRAControlPlaneController: {Default: false, PreRelease: featuregate.Alpha},
+
 	DynamicResourceAllocation: {Default: false, PreRelease: featuregate.Alpha},
 
 	EventedPLEG: {Default: false, PreRelease: featuregate.Alpha},

pkg/registry/resource/deviceclass/strategy.go

@@ -23,9 +23,11 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/apiserver/pkg/storage/names"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/kubernetes/pkg/api/legacyscheme"
 	"k8s.io/kubernetes/pkg/apis/resource"
 	"k8s.io/kubernetes/pkg/apis/resource/validation"
+	"k8s.io/kubernetes/pkg/features"
 )
 
 // deviceClassStrategy implements behavior for DeviceClass objects
@@ -43,6 +45,7 @@ func (deviceClassStrategy) NamespaceScoped() bool {
 func (deviceClassStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) {
 	class := obj.(*resource.DeviceClass)
 	class.Generation = 1
+	dropDisabledFields(class, nil)
 }
 
 func (deviceClassStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList {
@@ -65,6 +68,8 @@ func (deviceClassStrategy) PrepareForUpdate(ctx context.Context, obj, old runtim
 	class := obj.(*resource.DeviceClass)
 	oldClass := old.(*resource.DeviceClass)
 
+	dropDisabledFields(class, oldClass)
+
 	// Any changes to the spec increment the generation number.
 	if !apiequality.Semantic.DeepEqual(oldClass.Spec, class.Spec) {
 		class.Generation = oldClass.Generation + 1
@@ -83,3 +88,22 @@ func (deviceClassStrategy) WarningsOnUpdate(ctx context.Context, obj, old runtim
 func (deviceClassStrategy) AllowUnconditionalUpdate() bool {
 	return true
 }
+
+// dropDisabledFields removes fields which are covered by the optional DRAControlPlaneController feature gate.
+func dropDisabledFields(newClass, oldClass *resource.DeviceClass) {
+	if utilfeature.DefaultFeatureGate.Enabled(features.DRAControlPlaneController) {
+		// No need to drop anything.
+		return
+	}
+
+	if oldClass == nil {
+		// Always drop on create.
+		newClass.Spec.SuitableNodes = nil
+		return
+	}
+
+	// Drop on update only if not already set.
+	if oldClass.Spec.SuitableNodes == nil {
+		newClass.Spec.SuitableNodes = nil
+	}
+}

pkg/registry/resource/deviceclass/strategy_test.go

@@ -19,18 +19,42 @@ package deviceclass
 import (
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
+	featuregatetesting "k8s.io/component-base/featuregate/testing"
+	"k8s.io/kubernetes/pkg/apis/core"
 	"k8s.io/kubernetes/pkg/apis/resource"
+	"k8s.io/kubernetes/pkg/features"
 )
 
-var deviceClass = &resource.DeviceClass{
+var obj = &resource.DeviceClass{
+	ObjectMeta: metav1.ObjectMeta{
+		Name:       "valid-class",
+		Generation: 1,
+	},
+}
+
+var objWithGatedFields = &resource.DeviceClass{
 	ObjectMeta: metav1.ObjectMeta{
-		Name: "valid-class",
+		Name:       "valid-class",
+		Generation: 1,
+	},
+	Spec: resource.DeviceClassSpec{
+		SuitableNodes: &core.NodeSelector{
+			NodeSelectorTerms: []core.NodeSelectorTerm{{
+				MatchExpressions: []core.NodeSelectorRequirement{{
+					Key:      "foo",
+					Operator: core.NodeSelectorOpExists,
+				}},
+			}},
+		},
 	},
 }
 
-func TestClassStrategy(t *testing.T) {
+func TestStrategy(t *testing.T) {
 	if Strategy.NamespaceScoped() {
 		t.Errorf("DeviceClass must not be namespace scoped")
 	}
@@ -39,42 +63,135 @@ func TestClassStrategy(t *testing.T) {
 	}
 }
 
-func TestClassStrategyCreate(t *testing.T) {
+func TestStrategyCreate(t *testing.T) {
 	ctx := genericapirequest.NewDefaultContext()
-	deviceClass := deviceClass.DeepCopy()
 
-	Strategy.PrepareForCreate(ctx, deviceClass)
-	errs := Strategy.Validate(ctx, deviceClass)
-	if len(errs) != 0 {
-		t.Errorf("unexpected error validating for create %v", errs)
+	testcases := map[string]struct {
+		obj                    *resource.DeviceClass
+		controlPlaneController bool
+		expectValidationError  bool
+		expectObj              *resource.DeviceClass
+	}{
+		"simple": {
+			obj:       obj,
+			expectObj: obj,
+		},
+		"validation-error": {
+			obj: func() *resource.DeviceClass {
+				obj := obj.DeepCopy()
+				obj.Name = "%#@$%$"
+				return obj
+			}(),
+			expectValidationError: true,
+		},
+		"drop-fields": {
+			obj:                    objWithGatedFields,
+			controlPlaneController: false,
+			expectObj:              obj,
+		},
+		"keep-fields": {
+			obj:                    objWithGatedFields,
+			controlPlaneController: true,
+			expectObj:              objWithGatedFields,
+		},
+	}
+
+	for name, tc := range testcases {
+		t.Run(name, func(t *testing.T) {
+			featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DRAControlPlaneController, tc.controlPlaneController)
+
+			obj := tc.obj.DeepCopy()
+			Strategy.PrepareForCreate(ctx, obj)
+			if errs := Strategy.Validate(ctx, obj); len(errs) != 0 {
+				if !tc.expectValidationError {
+					t.Fatalf("unexpected validation errors: %q", errs)
+				}
+				return
+			} else if tc.expectValidationError {
+				t.Fatal("expected validation error(s), got none")
+			}
+			if warnings := Strategy.WarningsOnCreate(ctx, obj); len(warnings) != 0 {
+				t.Fatalf("unexpected warnings: %q", warnings)
+			}
+			Strategy.Canonicalize(obj)
+			assert.Equal(t, tc.expectObj, obj)
+		})
 	}
 }
 
-func TestClassStrategyUpdate(t *testing.T) {
-	t.Run("no-changes-okay", func(t *testing.T) {
-		ctx := genericapirequest.NewDefaultContext()
-		deviceClass := deviceClass.DeepCopy()
-		newClass := deviceClass.DeepCopy()
-		newClass.ResourceVersion = "4"
-
-		Strategy.PrepareForUpdate(ctx, newClass, deviceClass)
-		errs := Strategy.ValidateUpdate(ctx, newClass, deviceClass)
-		if len(errs) != 0 {
-			t.Errorf("unexpected validation errors: %v", errs)
-		}
-	})
-
-	t.Run("name-change-not-allowed", func(t *testing.T) {
-		ctx := genericapirequest.NewDefaultContext()
-		deviceClass := deviceClass.DeepCopy()
-		newClass := deviceClass.DeepCopy()
-		newClass.Name = "valid-class-2"
-		newClass.ResourceVersion = "4"
-
-		Strategy.PrepareForUpdate(ctx, newClass, deviceClass)
-		errs := Strategy.ValidateUpdate(ctx, newClass, deviceClass)
-		if len(errs) == 0 {
-			t.Errorf("expected a validation error")
-		}
-	})
+func TestStrategyUpdate(t *testing.T) {
+	ctx := genericapirequest.NewDefaultContext()
+
+	testcases := map[string]struct {
+		oldObj                 *resource.DeviceClass
+		newObj                 *resource.DeviceClass
+		controlPlaneController bool
+		expectValidationError  bool
+		expectObj              *resource.DeviceClass
+	}{
+		"no-changes-okay": {
+			oldObj:    obj,
+			newObj:    obj,
+			expectObj: obj,
+		},
+		"name-change-not-allowed": {
+			oldObj: obj,
+			newObj: func() *resource.DeviceClass {
+				obj := obj.DeepCopy()
+				obj.Name += "-2"
+				return obj
+			}(),
+			expectValidationError: true,
+		},
+		"drop-fields": {
+			oldObj:                 obj,
+			newObj:                 objWithGatedFields,
+			controlPlaneController: false,
+			expectObj:              obj,
+		},
+		"keep-fields": {
+			oldObj:                 obj,
+			newObj:                 objWithGatedFields,
+			controlPlaneController: true,
+			expectObj: func() *resource.DeviceClass {
+				obj := objWithGatedFields.DeepCopy()
+				// Spec changes -> generation gets bumped.
+				obj.Generation++
+				return obj
+			}(),
+		},
+		"keep-existing-fields": {
+			oldObj:                 objWithGatedFields,
+			newObj:                 objWithGatedFields,
+			controlPlaneController: false,
+			expectObj:              objWithGatedFields,
+		},
+	}
+
+	for name, tc := range testcases {
+		t.Run(name, func(t *testing.T) {
+			featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DRAControlPlaneController, tc.controlPlaneController)
+			oldObj := tc.oldObj.DeepCopy()
+			newObj := tc.newObj.DeepCopy()
+			newObj.ResourceVersion = "4"
+
+			Strategy.PrepareForUpdate(ctx, newObj, oldObj)
+			if errs := Strategy.ValidateUpdate(ctx, newObj, oldObj); len(errs) != 0 {
+				if !tc.expectValidationError {
+					t.Fatalf("unexpected validation errors: %q", errs)
+				}
+				return
+			} else if tc.expectValidationError {
+				t.Fatal("expected validation error(s), got none")
+			}
+			if warnings := Strategy.WarningsOnUpdate(ctx, newObj, oldObj); len(warnings) != 0 {
+				t.Fatalf("unexpected warnings: %q", warnings)
+			}
+			Strategy.Canonicalize(newObj)
+
+			expectObj := tc.expectObj.DeepCopy()
+			expectObj.ResourceVersion = "4"
+			assert.Equal(t, expectObj, newObj)
+		})
+	}
 }

pkg/registry/resource/resourceclaim/strategy.go

@@ -28,9 +28,11 @@ import (
 	"k8s.io/apiserver/pkg/registry/generic"
 	"k8s.io/apiserver/pkg/storage"
 	"k8s.io/apiserver/pkg/storage/names"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/kubernetes/pkg/api/legacyscheme"
 	"k8s.io/kubernetes/pkg/apis/resource"
 	"k8s.io/kubernetes/pkg/apis/resource/validation"
+	"k8s.io/kubernetes/pkg/features"
 	"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
 )
 
@@ -65,6 +67,8 @@ func (resourceclaimStrategy) PrepareForCreate(ctx context.Context, obj runtime.O
 	claim := obj.(*resource.ResourceClaim)
 	// Status must not be set by user on create.
 	claim.Status = resource.ResourceClaimStatus{}
+
+	dropDisabledFields(claim, nil)
 }
 
 func (resourceclaimStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList {
@@ -87,6 +91,8 @@ func (resourceclaimStrategy) PrepareForUpdate(ctx context.Context, obj, old runt
 	newClaim := obj.(*resource.ResourceClaim)
 	oldClaim := old.(*resource.ResourceClaim)
 	newClaim.Status = oldClaim.Status
+
+	dropDisabledFields(newClaim, oldClaim)
 }
 
 func (resourceclaimStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList {
@@ -127,6 +133,8 @@ func (resourceclaimStatusStrategy) PrepareForUpdate(ctx context.Context, obj, ol
 	oldClaim := old.(*resource.ResourceClaim)
 	newClaim.Spec = oldClaim.Spec
 	metav1.ResetObjectMetaForStatus(&newClaim.ObjectMeta, &oldClaim.ObjectMeta)
+
+	dropDisabledFields(newClaim, oldClaim)
 }
 
 func (resourceclaimStatusStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList {
@@ -163,3 +171,36 @@ func toSelectableFields(claim *resource.ResourceClaim) fields.Set {
 	fields := generic.ObjectMetaFieldsSet(&claim.ObjectMeta, true)
 	return fields
 }
+
+// dropDisabledFields removes fields which are covered by the optional DRAControlPlaneController feature gate.
+func dropDisabledFields(newClaim, oldClaim *resource.ResourceClaim) {
+	if utilfeature.DefaultFeatureGate.Enabled(features.DRAControlPlaneController) {
+		// No need to drop anything.
+		return
+	}
+
+	if oldClaim == nil {
+		// Always drop on create. There's no status yet, so nothing to do there.
+		newClaim.Spec.Controller = ""
+		return
+	}
+
+	// Drop on (status) update only if not already set.
+	if oldClaim.Spec.Controller == "" {
+		newClaim.Spec.Controller = ""
+	}
+	// If the claim is handled by a control plane controller, allow
+	// setting it also in the status. Stripping that field would be bad.
+	if oldClaim.Spec.Controller == "" &&
+		newClaim.Status.Allocation != nil &&
+		oldClaim.Status.Allocation == nil &&
+		(oldClaim.Status.Allocation == nil || oldClaim.Status.Allocation.Controller == "") {
+		newClaim.Status.Allocation.Controller = ""
+	}
+	// If there is an existing allocation which used a control plane controller, then
+	// allow requesting its deallocation.
+	if !oldClaim.Status.DeallocationRequested &&
+		(newClaim.Status.Allocation == nil || newClaim.Status.Allocation.Controller == "") {
+		newClaim.Status.DeallocationRequested = false
+	}
+}