Merge pull request kubernetes#126807 from pohly/dra-resourceslice-update

DRA scheduler: ResourceSlice update

Author: k8s-ci-robot

pkg/scheduler/eventhandlers.go

@@ -514,7 +514,7 @@ func addAllEventHandlers(
 			}
 			handlers = append(handlers, handlerRegistration)
 		case framework.PodSchedulingContext:
-			if utilfeature.DefaultFeatureGate.Enabled(features.DynamicResourceAllocation) {
+			if utilfeature.DefaultFeatureGate.Enabled(features.DRAControlPlaneController) {
 				if handlerRegistration, err = informerFactory.Resource().V1alpha3().PodSchedulingContexts().Informer().AddEventHandler(
 					buildEvtResHandler(at, framework.PodSchedulingContext, "PodSchedulingContext"),
 				); err != nil {
@@ -529,6 +529,15 @@ func addAllEventHandlers(
 				)
 				handlers = append(handlers, handlerRegistration)
 			}
+		case framework.ResourceSlice:
+			if utilfeature.DefaultFeatureGate.Enabled(features.DynamicResourceAllocation) {
+				if handlerRegistration, err = informerFactory.Resource().V1alpha3().ResourceSlices().Informer().AddEventHandler(
+					buildEvtResHandler(at, framework.ResourceSlice, "ResourceSlice"),
+				); err != nil {
+					return err
+				}
+				handlers = append(handlers, handlerRegistration)
+			}
 		case framework.DeviceClass:
 			if utilfeature.DefaultFeatureGate.Enabled(features.DynamicResourceAllocation) {
 				if handlerRegistration, err = informerFactory.Resource().V1alpha3().DeviceClasses().Informer().AddEventHandler(

pkg/scheduler/eventhandlers_test.go

@@ -216,6 +216,7 @@ func TestAddAllEventHandlers(t *testing.T) {
 		name                   string
 		gvkMap                 map[framework.GVK]framework.ActionType
 		enableDRA              bool
+		enableClassicDRA       bool
 		expectStaticInformers  map[reflect.Type]bool
 		expectDynamicInformers map[schema.GroupVersionResource]bool
 	}{
@@ -234,6 +235,7 @@ func TestAddAllEventHandlers(t *testing.T) {
 			gvkMap: map[framework.GVK]framework.ActionType{
 				framework.PodSchedulingContext: framework.Add,
 				framework.ResourceClaim:        framework.Add,
+				framework.ResourceSlice:        framework.Add,
 				framework.DeviceClass:          framework.Add,
 			},
 			expectStaticInformers: map[reflect.Type]bool{
@@ -244,19 +246,41 @@ func TestAddAllEventHandlers(t *testing.T) {
 			expectDynamicInformers: map[schema.GroupVersionResource]bool{},
 		},
 		{
-			name: "DRA events enabled",
+			name: "some DRA events enabled",
 			gvkMap: map[framework.GVK]framework.ActionType{
 				framework.PodSchedulingContext: framework.Add,
 				framework.ResourceClaim:        framework.Add,
+				framework.ResourceSlice:        framework.Add,
 				framework.DeviceClass:          framework.Add,
 			},
 			enableDRA: true,
+			expectStaticInformers: map[reflect.Type]bool{
+				reflect.TypeOf(&v1.Pod{}):                    true,
+				reflect.TypeOf(&v1.Node{}):                   true,
+				reflect.TypeOf(&v1.Namespace{}):              true,
+				reflect.TypeOf(&resourceapi.ResourceClaim{}): true,
+				reflect.TypeOf(&resourceapi.ResourceSlice{}): true,
+				reflect.TypeOf(&resourceapi.DeviceClass{}):   true,
+			},
+			expectDynamicInformers: map[schema.GroupVersionResource]bool{},
+		},
+		{
+			name: "all DRA events enabled",
+			gvkMap: map[framework.GVK]framework.ActionType{
+				framework.PodSchedulingContext: framework.Add,
+				framework.ResourceClaim:        framework.Add,
+				framework.ResourceSlice:        framework.Add,
+				framework.DeviceClass:          framework.Add,
+			},
+			enableDRA:        true,
+			enableClassicDRA: true,
 			expectStaticInformers: map[reflect.Type]bool{
 				reflect.TypeOf(&v1.Pod{}):                           true,
 				reflect.TypeOf(&v1.Node{}):                          true,
 				reflect.TypeOf(&v1.Namespace{}):                     true,
 				reflect.TypeOf(&resourceapi.PodSchedulingContext{}): true,
 				reflect.TypeOf(&resourceapi.ResourceClaim{}):        true,
+				reflect.TypeOf(&resourceapi.ResourceSlice{}):        true,
 				reflect.TypeOf(&resourceapi.DeviceClass{}):          true,
 			},
 			expectDynamicInformers: map[schema.GroupVersionResource]bool{},
@@ -320,6 +344,7 @@ func TestAddAllEventHandlers(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DynamicResourceAllocation, tt.enableDRA)
+			featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DRAControlPlaneController, tt.enableClassicDRA)
 			logger, ctx := ktesting.NewTestContext(t)
 			ctx, cancel := context.WithCancel(ctx)
 			defer cancel()

pkg/scheduler/framework/plugins/dynamicresources/dynamicresources.go

@@ -398,6 +398,8 @@ func (pl *dynamicResources) EventsToRegister(_ context.Context) ([]framework.Clu
 		{Event: framework.ClusterEvent{Resource: framework.Node, ActionType: framework.Add | framework.UpdateNodeLabel | framework.UpdateNodeTaint}},
 		// A pod might be waiting for a class to get created or modified.
 		{Event: framework.ClusterEvent{Resource: framework.DeviceClass, ActionType: framework.Add | framework.Update}},
+		// Adding or updating a ResourceSlice might make a pod schedulable because new resources became available.
+		{Event: framework.ClusterEvent{Resource: framework.ResourceSlice, ActionType: framework.Add | framework.Update}, QueueingHintFn: pl.isSchedulableAfterResourceSliceChange},
 	}
 
 	if pl.podSchedulingContextLister != nil {
@@ -445,7 +447,7 @@ func (pl *dynamicResources) isSchedulableAfterClaimChange(logger klog.Logger, po
 		// This is not an unexpected error: we know that
 		// foreachPodResourceClaim only returns errors for "not
 		// schedulable".
-		logger.V(4).Info("pod is not schedulable", "pod", klog.KObj(pod), "claim", klog.KObj(modifiedClaim), "reason", err.Error())
+		logger.V(6).Info("pod is not schedulable after resource claim change", "pod", klog.KObj(pod), "claim", klog.KObj(modifiedClaim), "reason", err.Error())
 		return framework.QueueSkip, nil
 	}
 
@@ -491,6 +493,38 @@ func (pl *dynamicResources) isSchedulableAfterClaimChange(logger klog.Logger, po
 	return framework.Queue, nil
 }
 
+// isSchedulableAfterResourceSliceChange is invoked for add and update slice events reported by
+// an informer. Such changes can make an unschedulable pod schedulable when the pod requests a device
+// and the change adds a suitable device.
+//
+// For the sake of faster execution and avoiding code duplication, isSchedulableAfterResourceSliceChange
+// only checks whether the pod uses claims. All of the more detailed checks are done in the scheduling
+// attempt.
+//
+// The delete claim event will not invoke it, so newObj will never be nil.
+func (pl *dynamicResources) isSchedulableAfterResourceSliceChange(logger klog.Logger, pod *v1.Pod, oldObj, newObj interface{}) (framework.QueueingHint, error) {
+	_, modifiedSlice, err := schedutil.As[*resourceapi.ResourceSlice](oldObj, newObj)
+	if err != nil {
+		// Shouldn't happen.
+		return framework.Queue, fmt.Errorf("unexpected object in isSchedulableAfterResourceSliceChange: %w", err)
+	}
+
+	if err := pl.foreachPodResourceClaim(pod, nil); err != nil {
+		// This is not an unexpected error: we know that
+		// foreachPodResourceClaim only returns errors for "not
+		// schedulable".
+		logger.V(6).Info("pod is not schedulable after resource slice change", "pod", klog.KObj(pod), "resourceSlice", klog.KObj(modifiedSlice), "reason", err.Error())
+		return framework.QueueSkip, nil
+	}
+
+	// We could check what got changed in the slice, but right now that's likely to be
+	// about the spec (there's no status yet...).
+	// We could check whether all claims use classic DRA, but that doesn't seem worth it.
+	// Let's assume that changing the slice may make the pod schedulable.
+	logger.V(5).Info("ResourceSlice change might make pod schedulable", "pod", klog.KObj(pod), "resourceSlice", klog.KObj(modifiedSlice))
+	return framework.Queue, nil
+}
+
 // isSchedulableAfterPodSchedulingContextChange is invoked for all
 // PodSchedulingContext events reported by an informer. It checks whether that
 // change made a previously unschedulable pod schedulable (updated) or a new

pkg/scheduler/framework/types.go

@@ -125,6 +125,7 @@ const (
 	StorageClass          GVK = "storage.k8s.io/StorageClass"
 	PodSchedulingContext  GVK = "PodSchedulingContext"
 	ResourceClaim         GVK = "ResourceClaim"
+	ResourceSlice         GVK = "ResourceSlice"
 	DeviceClass           GVK = "DeviceClass"
 
 	// WildCard is a special GVK to match all resources.

staging/src/k8s.io/dynamic-resource-allocation/structured/allocator.go

@@ -208,7 +208,7 @@ func (a *Allocator) Allocate(ctx context.Context, node *v1.Node) (finalResult []
 			alloc.requestData[requestIndices{claimIndex: claimIndex, requestIndex: requestIndex}] = requestData
 			numDevices += requestData.numDevices
 		}
-		alloc.logger.Info("Checked claim", "claim", klog.KObj(claim), "numDevices", numDevices)
+		alloc.logger.V(6).Info("Checked claim", "claim", klog.KObj(claim), "numDevices", numDevices)
 
 		// Check that we don't end up with too many results.
 		if numDevices > resourceapi.AllocationResultsMaxSize {