webhook: support elasticquota enable update resource key

Signed-off-by: lijunxin <[email protected]>

pkg/features/features.go

@@ -68,6 +68,11 @@ const (
 	// to belong to the users and will not be preempted back.
 	ElasticQuotaGuaranteeUsage featuregate.Feature = "ElasticQuotaGuaranteeUsage"
 
+	// ElasticQuotaEnableUpdateResourceKey allows to update resource key in standard operation
+	// when delete resource type: from child to parent
+	// when add resource type: from parent to child
+	ElasticQuotaEnableUpdateResourceKey featuregate.Feature = "ElasticQuotaEnableUpdateResourceKey"
+
 	// DisableDefaultQuota disable default quota.
 	DisableDefaultQuota featuregate.Feature = "DisableDefaultQuota"
 
@@ -94,6 +99,7 @@ var defaultFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{
 	MultiQuotaTree:                         {Default: false, PreRelease: featuregate.Alpha},
 	ElasticQuotaIgnorePodOverhead:          {Default: false, PreRelease: featuregate.Alpha},
 	ElasticQuotaGuaranteeUsage:             {Default: false, PreRelease: featuregate.Alpha},
+	ElasticQuotaEnableUpdateResourceKey:    {Default: false, PreRelease: featuregate.Alpha},
 	DisableDefaultQuota:                    {Default: false, PreRelease: featuregate.Alpha},
 	SupportParentQuotaSubmitPod:            {Default: false, PreRelease: featuregate.Alpha},
 	EnableQuotaAdmission:                   {Default: false, PreRelease: featuregate.Alpha},

pkg/scheduler/plugins/elasticquota/core/group_quota_manager.go

@@ -563,24 +563,33 @@ func (gqm *GroupQuotaManager) updateOneGroupSharedWeightNoLock(quotaInfo *QuotaI
 	gqm.runtimeQuotaCalculatorMap[quotaInfo.ParentName].updateOneGroupSharedWeight(quotaInfo)
 }
 
+// updateResourceKeyNoLock based on quotaInfo.CalculateInfo.Max of self
+// Note: RootQuotaName need to be updated as allResourceKeys
 func (gqm *GroupQuotaManager) updateResourceKeyNoLock() {
 	// collect all dimensions
-	resourceKeys := make(map[v1.ResourceName]struct{})
+	allResourceKeys := make(map[v1.ResourceName]struct{})
 	for quotaName, quotaInfo := range gqm.quotaInfoMap {
-		if quotaName == extension.DefaultQuotaName || quotaName == extension.SystemQuotaName {
+		if quotaName == extension.RootQuotaName || quotaName == extension.DefaultQuotaName || quotaName == extension.SystemQuotaName {
 			continue
 		}
+		resourceKeys := make(map[v1.ResourceName]struct{})
 		for resName := range quotaInfo.CalculateInfo.Max {
+			allResourceKeys[resName] = struct{}{}
 			resourceKeys[resName] = struct{}{}
 		}
-	}
-
-	if !reflect.DeepEqual(resourceKeys, gqm.resourceKeys) {
-		gqm.resourceKeys = resourceKeys
-		for _, runtimeQuotaCalculator := range gqm.runtimeQuotaCalculatorMap {
+		// update right now
+		if runtimeQuotaCalculator, ok := gqm.runtimeQuotaCalculatorMap[quotaName]; ok && runtimeQuotaCalculator != nil && !reflect.DeepEqual(resourceKeys, runtimeQuotaCalculator.resourceKeys) {
 			runtimeQuotaCalculator.updateResourceKeys(resourceKeys)
 		}
 	}
+
+	if !reflect.DeepEqual(allResourceKeys, gqm.resourceKeys) {
+		gqm.resourceKeys = allResourceKeys
+	}
+	// in case RootQuota-resourceKey is nil
+	if runtimeQuotaCalculator, ok := gqm.runtimeQuotaCalculatorMap[extension.RootQuotaName]; ok && runtimeQuotaCalculator != nil && !reflect.DeepEqual(allResourceKeys, runtimeQuotaCalculator.resourceKeys) {
+		runtimeQuotaCalculator.updateResourceKeys(allResourceKeys)
+	}
 }
 
 func (gqm *GroupQuotaManager) GetAllQuotaNames() map[string]struct{} {
@@ -1019,7 +1028,6 @@ func (gqm *GroupQuotaManager) updateQuotaInternalNoLock(newQuotaInfo, oldQuotaIn
 
 	// update resource keys
 	gqm.updateResourceKeyNoLock()
-
 	oldMin := v1.ResourceList{}
 	if oldQuotaInfo != nil {
 		oldMin = oldQuotaInfo.CalculateInfo.Min

pkg/scheduler/plugins/elasticquota/core/runtime_quota_calculator_test.go

@@ -31,6 +31,13 @@ import (
 	"github.com/koordinator-sh/koordinator/apis/thirdparty/scheduler-plugins/pkg/apis/scheduling/v1alpha1"
 )
 
+const (
+	TestNode1 = "node1"
+	TestNode2 = "node2"
+	TestNode3 = "node3"
+	TestNode4 = "node4"
+)
+
 func TestQuotaInfo_GetLimitRequest(t *testing.T) {
 	max := createResourceList(100, 10000)
 	req := createResourceList(1000, 1000)
@@ -129,29 +136,134 @@ func createElasticQuota() *v1alpha1.ElasticQuota {
 	return eQ
 }
 
-func TestRuntimeQuotaCalculator_Iteration4AdjustQuota(t *testing.T) {
-	qtw := NewRuntimeQuotaCalculator("testTreeName")
-	resourceKey := make(map[corev1.ResourceName]struct{})
-	cpu := corev1.ResourceCPU
-	resourceKey[cpu] = struct{}{}
-	qtw.updateResourceKeys(resourceKey)
-	qtw.quotaTree[cpu].insert("node1", 40, 5, 10, 0, true)
-	qtw.quotaTree[cpu].insert("node2", 60, 20, 15, 0, true)
-	qtw.quotaTree[cpu].insert("node3", 50, 40, 20, 0, true)
-	qtw.quotaTree[cpu].insert("node4", 80, 70, 15, 0, true)
-	qtw.totalResource = corev1.ResourceList{}
-	qtw.totalResource[corev1.ResourceCPU] = *resource.NewMilliQuantity(100, resource.DecimalSI)
-	qtw.calculateRuntimeNoLock()
-	if qtw.globalRuntimeVersion == 0 {
-		t.Error("error")
+func TestRuntimeQuotaCalculator_IterationAdjustQuota(t *testing.T) {
+	type quotaNodeInfo = struct {
+		groupName         string
+		sharedWeight      int64
+		request           int64
+		min               int64
+		guarantee         int64
+		allowLentResource bool
 	}
-	if qtw.quotaTree[cpu].quotaNodes["node1"].runtimeQuota != 5 ||
-		qtw.quotaTree[cpu].quotaNodes["node2"].runtimeQuota != 20 ||
-		qtw.quotaTree[cpu].quotaNodes["node3"].runtimeQuota != 35 ||
-		qtw.quotaTree[cpu].quotaNodes["node4"].runtimeQuota != 40 {
-		t.Error("error")
+	node1 := &quotaNodeInfo{
+		groupName:         TestNode1,
+		sharedWeight:      40,
+		request:           5,
+		min:               10,
+		guarantee:         0,
+		allowLentResource: true,
+	}
+	node2 := &quotaNodeInfo{
+		groupName:         TestNode2,
+		sharedWeight:      60,
+		request:           20,
+		min:               15,
+		guarantee:         0,
+		allowLentResource: true,
+	}
+	node3 := &quotaNodeInfo{
+		groupName:         TestNode3,
+		sharedWeight:      50,
+		request:           40,
+		min:               20,
+		guarantee:         0,
+		allowLentResource: true,
+	}
+	node4 := &quotaNodeInfo{
+		groupName:         TestNode4,
+		sharedWeight:      80,
+		request:           70,
+		min:               15,
+		guarantee:         0,
+		allowLentResource: true,
+	}
+	node4_1 := &quotaNodeInfo{
+		groupName:         TestNode4,
+		sharedWeight:      0,
+		request:           70,
+		min:               15,
+		guarantee:         0,
+		allowLentResource: true,
+	}
+	node4_2 := &quotaNodeInfo{
+		groupName:         TestNode4,
+		sharedWeight:      0,
+		request:           70,
+		min:               15,
+		guarantee:         45,
+		allowLentResource: true,
 	}
 
+	testCases := []struct {
+		name              string
+		totalResource     corev1.ResourceList
+		nodes             []*quotaNodeInfo
+		expectedRuntimeMp map[string]map[corev1.ResourceName]int64
+	}{
+		{
+			name: "case1-no-guarantee",
+			totalResource: corev1.ResourceList{
+				corev1.ResourceCPU: *resource.NewMilliQuantity(100, resource.DecimalSI),
+			},
+			nodes: []*quotaNodeInfo{node1, node2, node3, node4},
+			expectedRuntimeMp: map[string]map[corev1.ResourceName]int64{
+				TestNode1: {corev1.ResourceCPU: 5},
+				TestNode2: {corev1.ResourceCPU: 20},
+				TestNode3: {corev1.ResourceCPU: 35},
+				TestNode4: {corev1.ResourceCPU: 40},
+			},
+		},
+		{
+			name: "case2-node4.sharedWeight=0",
+			totalResource: corev1.ResourceList{
+				corev1.ResourceCPU: *resource.NewMilliQuantity(100, resource.DecimalSI),
+			},
+			nodes: []*quotaNodeInfo{node1, node2, node3, node4_1},
+			expectedRuntimeMp: map[string]map[corev1.ResourceName]int64{
+				TestNode1: {corev1.ResourceCPU: 5},
+				TestNode2: {corev1.ResourceCPU: 20},
+				TestNode3: {corev1.ResourceCPU: 40},
+				TestNode4: {corev1.ResourceCPU: 15},
+			},
+		},
+		{
+			name: "case3-node4.guarantee>min",
+			totalResource: corev1.ResourceList{
+				corev1.ResourceCPU: *resource.NewMilliQuantity(100, resource.DecimalSI),
+			},
+			nodes: []*quotaNodeInfo{node1, node2, node3, node4_2},
+			expectedRuntimeMp: map[string]map[corev1.ResourceName]int64{
+				TestNode1: {corev1.ResourceCPU: 5},
+				TestNode2: {corev1.ResourceCPU: 20},
+				TestNode3: {corev1.ResourceCPU: 30},
+				TestNode4: {corev1.ResourceCPU: 45},
+			},
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			qtw := NewRuntimeQuotaCalculator("testTreeName")
+			resourceKey := make(map[corev1.ResourceName]struct{})
+			for key, value := range tc.totalResource {
+				if !value.IsZero() {
+					resourceKey[key] = struct{}{}
+				}
+			}
+			qtw.updateResourceKeys(resourceKey)
+			qtw.totalResource = tc.totalResource
+			for _, node := range tc.nodes {
+				for resKey := range resourceKey {
+					qtw.quotaTree[resKey].insert(node.groupName, node.sharedWeight, node.request, node.min, node.guarantee, node.allowLentResource)
+				}
+			}
+			qtw.calculateRuntimeNoLock()
+			for node, rq := range tc.expectedRuntimeMp {
+				for resKey, q := range rq {
+					assert.Equal(t, q, qtw.quotaTree[resKey].quotaNodes[node].runtimeQuota)
+				}
+			}
+		})
+	}
 }
 
 func createQuotaInfoWithRes(name string, max, min corev1.ResourceList) *QuotaInfo {

pkg/webhook/elasticquota/quota_topology_check.go

@@ -68,7 +68,7 @@ func (qt *quotaTopology) validateQuotaSelfItem(quota *v1alpha1.ElasticQuota) err
 }
 
 // validateQuotaTopology checks the quotaInfo's topology with its parent and its children.
-// oldQuotaInfo is null wben validate a new create request, and is the current quotaInfo when validate a update request.
+// oldQuotaInfo is null when validate a new create request, and is the current quotaInfo when validate a update request.
 func (qt *quotaTopology) validateQuotaTopology(oldQuotaInfo, newQuotaInfo *QuotaInfo, oldNamespaces []string) error {
 	if newQuotaInfo.Name == extension.RootQuotaName {
 		return nil
@@ -91,8 +91,8 @@ func (qt *quotaTopology) validateQuotaTopology(oldQuotaInfo, newQuotaInfo *Quota
 		return err
 	}
 
-	if err := qt.checkSubAndParentGroupMaxQuotaKeySame(newQuotaInfo); err != nil {
-		return err
+	if err := qt.checkSubAndParentGroupQuotaKey(newQuotaInfo, utilfeature.DefaultFeatureGate.Enabled(features.ElasticQuotaEnableUpdateResourceKey)); err != nil {
+		return fmt.Errorf("failed to check sub and parent group quotaKey")
 	}
 
 	if err := qt.checkMinQuotaValidate(newQuotaInfo); err != nil {
@@ -180,15 +180,32 @@ func (qt *quotaTopology) checkParentQuotaInfo(quotaName, parentName string) erro
 	return nil
 }
 
-func (qt *quotaTopology) checkSubAndParentGroupMaxQuotaKeySame(quotaInfo *QuotaInfo) error {
+// checkSubAndParentGroupQuotaKey check the quotaInfo's quota with its parent and its children
+//
+//	while enableResourceTypeUpdate=false, the quotaInfo's max quota key must be same as its children and parent's quota key
+//	while enableResourceTypeUpdate=true, the quotaInfo's max quota key only need be included in its parent's quota key
+//
+// the quotaInfo's min quota key only need be included in its parent's quota key no matter when
+func (qt *quotaTopology) checkSubAndParentGroupQuotaKey(quotaInfo *QuotaInfo, enableUpdateResourceKey bool) error {
 	if quotaInfo.Name == extension.RootQuotaName {
 		return nil
 	}
 	if quotaInfo.ParentName != extension.RootQuotaName {
 		parentInfo := qt.quotaInfoMap[quotaInfo.ParentName]
-		if !checkQuotaKeySame(parentInfo.CalculateInfo.Max, quotaInfo.CalculateInfo.Max) {
-			return fmt.Errorf("checkSubAndParentGroupMaxQuotaKeySame failed: %v's key is not the same with %v",
-				quotaInfo.ParentName, quotaInfo.Name)
+		if enableUpdateResourceKey {
+			if !checkQuotaKeyIncluded(parentInfo.CalculateInfo.Max, quotaInfo.CalculateInfo.Max) {
+				return fmt.Errorf("checkSubAndParentGroupQuotaKey failed: %v's max keys are not all included in %v's",
+					quotaInfo.Name, quotaInfo.ParentName)
+			}
+		} else {
+			if !checkQuotaKeySame(parentInfo.CalculateInfo.Max, quotaInfo.CalculateInfo.Max) {
+				return fmt.Errorf("checkSubAndParentGroupQuotaKey failed: %v's max keys are not the same with %v's",
+					quotaInfo.ParentName, quotaInfo.Name)
+			}
+		}
+		if !checkQuotaKeyIncluded(parentInfo.CalculateInfo.Min, quotaInfo.CalculateInfo.Min) {
+			return fmt.Errorf("checkSubAndParentGroupQuotaKey failed: %v's min keys are not all included in %v's",
+				quotaInfo.Name, quotaInfo.ParentName)
 		}
 	}
 
@@ -199,9 +216,20 @@ func (qt *quotaTopology) checkSubAndParentGroupMaxQuotaKeySame(quotaInfo *QuotaI
 
 	for name := range children {
 		if child, exist := qt.quotaInfoMap[name]; exist {
-			if !checkQuotaKeySame(quotaInfo.CalculateInfo.Max, child.CalculateInfo.Max) {
-				return fmt.Errorf("checkSubAndParentGroupMaxQuotaKeySame failed: %v's key is not the same with %v",
-					quotaInfo.Name, name)
+			if enableUpdateResourceKey {
+				if !checkQuotaKeyIncluded(quotaInfo.CalculateInfo.Max, child.CalculateInfo.Max) {
+					return fmt.Errorf("checkSubAndParentGroupQuotaKey failed: %v's max keys are not all included in %v's",
+						name, quotaInfo.Name)
+				}
+			} else {
+				if !checkQuotaKeySame(quotaInfo.CalculateInfo.Max, child.CalculateInfo.Max) {
+					return fmt.Errorf("checkSubAndParentGroupQuotaKey failed: %v's max keys are not the same with %v's",
+						name, quotaInfo.Name)
+				}
+			}
+			if !checkQuotaKeyIncluded(quotaInfo.CalculateInfo.Min, child.CalculateInfo.Min) {
+				return fmt.Errorf("checkSubAndParentGroupQuotaKey failed: %v's min keys are not all included in %v's",
+					name, quotaInfo.Name)
 			}
 		} else {
 			return fmt.Errorf("internal error: quotaInfoMap and quotaTree information out of sync, losed :%v", name)
@@ -224,7 +252,6 @@ func (qt *quotaTopology) checkMinQuotaValidate(newQuotaInfo *QuotaInfo) error {
 		return nil
 	}
 
-	// check brothers' minquota sum
 	if newQuotaInfo.ParentName != extension.RootQuotaName {
 		childMinSumNotIncludeSelf, err := qt.getChildMinQuotaSumExceptSpecificChild(newQuotaInfo.ParentName, newQuotaInfo.Name)
 		if err != nil {
@@ -344,6 +371,16 @@ func checkQuotaKeySame(parent, child v1.ResourceList) bool {
 	return true
 }
 
+// checkQuotaKeyIncluded will check whether the parent quota includes all keys of child quota.
+func checkQuotaKeyIncluded(parent, child v1.ResourceList) bool {
+	for k := range child {
+		if _, ok := parent[k]; !ok {
+			return false
+		}
+	}
+	return true
+}
+
 func (qt *quotaTopology) checkGuaranteedForMin(quotaInfo *QuotaInfo) error {
 	if quotaInfo.AllowForceUpdate {
 		return nil