volumebinding: scheduler queueing hints - StorageClass
This commit is contained in:
		| @@ -25,6 +25,7 @@ import ( | ||||
|  | ||||
| 	v1 "k8s.io/api/core/v1" | ||||
| 	storagev1 "k8s.io/api/storage/v1" | ||||
| 	apiequality "k8s.io/apimachinery/pkg/api/equality" | ||||
| 	apierrors "k8s.io/apimachinery/pkg/api/errors" | ||||
| 	"k8s.io/apimachinery/pkg/runtime" | ||||
| 	corelisters "k8s.io/client-go/listers/core/v1" | ||||
| @@ -98,7 +99,7 @@ func (pl *VolumeBinding) EventsToRegister() []framework.ClusterEventWithHint { | ||||
| 		// Pods may fail because of missing or mis-configured storage class | ||||
| 		// (e.g., allowedTopologies, volumeBindingMode), and hence may become | ||||
| 		// schedulable upon StorageClass Add or Update events. | ||||
| 		{Event: framework.ClusterEvent{Resource: framework.StorageClass, ActionType: framework.Add | framework.Update}}, | ||||
| 		{Event: framework.ClusterEvent{Resource: framework.StorageClass, ActionType: framework.Add | framework.Update}, QueueingHintFn: pl.isSchedulableAfterStorageClassChange}, | ||||
|  | ||||
| 		// We bind PVCs with PVs, so any changes may make the pods schedulable. | ||||
| 		{Event: framework.ClusterEvent{Resource: framework.PersistentVolumeClaim, ActionType: framework.Add | framework.Update}, QueueingHintFn: pl.isSchedulableAfterPersistentVolumeClaimChange}, | ||||
| @@ -195,6 +196,38 @@ func (pl *VolumeBinding) isSchedulableAfterPersistentVolumeClaimChange(logger kl | ||||
| 	return framework.QueueSkip, nil | ||||
| } | ||||
|  | ||||
| // isSchedulableAfterStorageClassChange checks whether an StorageClass event might make a Pod schedulable or not. | ||||
| // Any StorageClass addition and a StorageClass update to allowedTopologies | ||||
| // might make a Pod schedulable. | ||||
| // Note that an update to volume binding mode is not allowed and we don't have to consider while examining the update event. | ||||
| func (pl *VolumeBinding) isSchedulableAfterStorageClassChange(logger klog.Logger, pod *v1.Pod, oldObj, newObj interface{}) (framework.QueueingHint, error) { | ||||
| 	oldSC, newSC, err := util.As[*storagev1.StorageClass](oldObj, newObj) | ||||
| 	if err != nil { | ||||
| 		return framework.Queue, err | ||||
| 	} | ||||
|  | ||||
| 	logger = klog.LoggerWithValues( | ||||
| 		logger, | ||||
| 		"Pod", klog.KObj(pod), | ||||
| 		"StorageClass", klog.KObj(newSC), | ||||
| 	) | ||||
|  | ||||
| 	if oldSC == nil { | ||||
| 		// No further filtering can be made for a creation event, | ||||
| 		// and we just always return Queue. | ||||
| 		logger.V(5).Info("A new StorageClass was created, which could make a Pod schedulable") | ||||
| 		return framework.Queue, nil | ||||
| 	} | ||||
|  | ||||
| 	if !apiequality.Semantic.DeepEqual(newSC.AllowedTopologies, oldSC.AllowedTopologies) { | ||||
| 		logger.V(5).Info("StorageClass got an update in AllowedTopologies", "AllowedTopologies", newSC.AllowedTopologies) | ||||
| 		return framework.Queue, nil | ||||
| 	} | ||||
|  | ||||
| 	logger.V(5).Info("StorageClass was updated, but it doesn't make this pod schedulable") | ||||
| 	return framework.QueueSkip, nil | ||||
| } | ||||
|  | ||||
| // podHasPVCs returns 2 values: | ||||
| // - the first one to denote if the given "pod" has any PVC defined. | ||||
| // - the second one to return any error if the requested PVC is illegal. | ||||
|   | ||||
| @@ -1093,3 +1093,91 @@ func TestIsSchedulableAfterPersistentVolumeClaimChange(t *testing.T) { | ||||
| 		}) | ||||
| 	} | ||||
| } | ||||
|  | ||||
| func TestIsSchedulableAfterStorageClassChange(t *testing.T) { | ||||
| 	table := []struct { | ||||
| 		name      string | ||||
| 		pod       *v1.Pod | ||||
| 		oldSC     interface{} | ||||
| 		newSC     interface{} | ||||
| 		pvcLister tf.PersistentVolumeClaimLister | ||||
| 		err       bool | ||||
| 		expect    framework.QueueingHint | ||||
| 	}{ | ||||
| 		{ | ||||
| 			name:  "When a new StorageClass is created, it returns Queue", | ||||
| 			pod:   makePod("pod-a").Pod, | ||||
| 			oldSC: nil, | ||||
| 			newSC: &storagev1.StorageClass{ | ||||
| 				ObjectMeta: metav1.ObjectMeta{ | ||||
| 					Name: "sc-a", | ||||
| 				}, | ||||
| 			}, | ||||
| 			err:    false, | ||||
| 			expect: framework.Queue, | ||||
| 		}, | ||||
| 		{ | ||||
| 			name: "When the AllowedTopologies are changed, it returns Queue", | ||||
| 			pod:  makePod("pod-a").Pod, | ||||
| 			oldSC: &storagev1.StorageClass{ | ||||
| 				ObjectMeta: metav1.ObjectMeta{ | ||||
| 					Name: "sc-a", | ||||
| 				}, | ||||
| 			}, | ||||
| 			newSC: &storagev1.StorageClass{ | ||||
| 				ObjectMeta: metav1.ObjectMeta{ | ||||
| 					Name: "sc-a", | ||||
| 				}, | ||||
| 				AllowedTopologies: []v1.TopologySelectorTerm{ | ||||
| 					{ | ||||
| 						MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ | ||||
| 							{ | ||||
| 								Key:    "kubernetes.io/hostname", | ||||
| 								Values: []string{"node-a"}, | ||||
| 							}, | ||||
| 						}, | ||||
| 					}, | ||||
| 				}, | ||||
| 			}, | ||||
| 			err:    false, | ||||
| 			expect: framework.Queue, | ||||
| 		}, | ||||
| 		{ | ||||
| 			name: "When there are no changes to the StorageClass, it returns QueueSkip", | ||||
| 			pod:  makePod("pod-a").Pod, | ||||
| 			oldSC: &storagev1.StorageClass{ | ||||
| 				ObjectMeta: metav1.ObjectMeta{ | ||||
| 					Name: "sc-a", | ||||
| 				}, | ||||
| 			}, | ||||
| 			newSC: &storagev1.StorageClass{ | ||||
| 				ObjectMeta: metav1.ObjectMeta{ | ||||
| 					Name: "sc-a", | ||||
| 				}, | ||||
| 			}, | ||||
| 			err:    false, | ||||
| 			expect: framework.QueueSkip, | ||||
| 		}, | ||||
| 		{ | ||||
| 			name:   "type conversion error", | ||||
| 			oldSC:  new(struct{}), | ||||
| 			newSC:  new(struct{}), | ||||
| 			err:    true, | ||||
| 			expect: framework.Queue, | ||||
| 		}, | ||||
| 	} | ||||
|  | ||||
| 	for _, item := range table { | ||||
| 		t.Run(item.name, func(t *testing.T) { | ||||
| 			pl := &VolumeBinding{PVCLister: item.pvcLister} | ||||
| 			logger, _ := ktesting.NewTestContext(t) | ||||
| 			qhint, err := pl.isSchedulableAfterStorageClassChange(logger, item.pod, item.oldSC, item.newSC) | ||||
| 			if (err != nil) != item.err { | ||||
| 				t.Errorf("isSchedulableAfterStorageClassChange failed - got: %q", err) | ||||
| 			} | ||||
| 			if qhint != item.expect { | ||||
| 				t.Errorf("QHint does not match: %v, want: %v", qhint, item.expect) | ||||
| 			} | ||||
| 		}) | ||||
| 	} | ||||
| } | ||||
|   | ||||
		Reference in New Issue
	
	Block a user
	 bells17
					bells17