From 3a72f52874010bbd8b315fbfb9c6ce40c9c47ab5 Mon Sep 17 00:00:00 2001 From: lijunxin Date: Tue, 14 Jan 2025 17:00:33 +0800 Subject: [PATCH] webhook: support elasticquota enable update resource key Signed-off-by: lijunxin --- pkg/features/features.go | 6 + .../elasticquota/core/group_quota_manager.go | 24 +- .../core/group_quota_manager_test.go | 50 +++- .../core/runtime_quota_calculator.go | 6 +- .../core/runtime_quota_calculator_test.go | 152 ++++++++-- pkg/webhook/elasticquota/quota_topology.go | 41 ++- .../elasticquota/quota_topology_check.go | 71 ++++- .../elasticquota/quota_topology_test.go | 276 +++++++++++++----- 8 files changed, 506 insertions(+), 120 deletions(-) diff --git a/pkg/features/features.go b/pkg/features/features.go index 763e8ea50..96d2aa1d8 100644 --- a/pkg/features/features.go +++ b/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}, diff --git a/pkg/scheduler/plugins/elasticquota/core/group_quota_manager.go b/pkg/scheduler/plugins/elasticquota/core/group_quota_manager.go index 18e1d8e06..4803945d2 100644 --- a/pkg/scheduler/plugins/elasticquota/core/group_quota_manager.go +++ b/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 diff --git a/pkg/scheduler/plugins/elasticquota/core/group_quota_manager_test.go b/pkg/scheduler/plugins/elasticquota/core/group_quota_manager_test.go index 7508579a7..c1d9a18a2 100644 --- a/pkg/scheduler/plugins/elasticquota/core/group_quota_manager_test.go +++ b/pkg/scheduler/plugins/elasticquota/core/group_quota_manager_test.go @@ -40,7 +40,8 @@ import ( ) const ( - GigaByte = 1024 * 1048576 + GigaByte = 1024 * 1048576 + ExtendedResourceKeyXCPU = "x-cpu" ) func TestGroupQuotaManager_QuotaAdd(t *testing.T) { @@ -151,6 +152,7 @@ func TestGroupQuotaManager_UpdateQuota(t *testing.T) { } func TestGroupQuotaManager_UpdateQuotaInternalAndRequest(t *testing.T) { + // add resource to node gqm := NewGroupQuotaManagerForTest() deltaRes := createResourceList(96, 160*GigaByte) gqm.UpdateClusterTotalResource(deltaRes) @@ -160,23 +162,61 @@ func TestGroupQuotaManager_UpdateQuotaInternalAndRequest(t *testing.T) { AddQuotaToManager(t, gqm, "test1", extension.RootQuotaName, 96, 160*GigaByte, 50, 80*GigaByte, true, false) - // test1 request[120, 290] runtime == maxQuota + // request[120, 290] > maxQuota, runtime == maxQuota request := createResourceList(120, 290*GigaByte) gqm.updateGroupDeltaRequestNoLock("test1", request, request, 0) runtime := gqm.RefreshRuntime("test1") - assert.Equal(t, deltaRes, runtime) + expectCurrentRuntime := deltaRes + assert.Equal(t, expectCurrentRuntime, runtime) + // update resourceKey quota1 := CreateQuota("test1", extension.RootQuotaName, 64, 100*GigaByte, 60, 90*GigaByte, true, false) quota1.Labels[extension.LabelQuotaIsParent] = "false" err := gqm.UpdateQuota(quota1, false) assert.Nil(t, err) quotaInfo := gqm.GetQuotaInfoByName("test1") assert.Equal(t, createResourceList(64, 100*GigaByte), quotaInfo.CalculateInfo.Max) + runtime = gqm.RefreshRuntime("test1") + expectCurrentRuntime = createResourceList(64, 100*GigaByte) + assert.Equal(t, expectCurrentRuntime, runtime) + // added max ExtendedResourceKeyXCPU without node resource added + // runtime.ExtendedResourceKeyXCPU = 0 + request[ExtendedResourceKeyXCPU] = *resource.NewQuantity(80, resource.DecimalSI) + gqm.updateGroupDeltaRequestNoLock("test1", request, request, 0) + xCPUQuantity := resource.NewQuantity(100, resource.DecimalSI) + quota1.Spec.Max[ExtendedResourceKeyXCPU] = *xCPUQuantity + maxJson, err := json.Marshal(quota1.Spec.Max) + assert.Nil(t, err) + quota1.Annotations[extension.AnnotationSharedWeight] = string(maxJson) + gqm.UpdateQuota(quota1, false) + quotaInfo = gqm.quotaInfoMap["test1"] + assert.True(t, quotaInfo != nil) + assert.Equal(t, *xCPUQuantity, quotaInfo.CalculateInfo.Max[ExtendedResourceKeyXCPU]) runtime = gqm.RefreshRuntime("test1") - assert.Equal(t, createResourceList(64, 100*GigaByte), runtime) -} + expectCurrentRuntime[ExtendedResourceKeyXCPU] = resource.Quantity{Format: resource.DecimalSI} + assert.Equal(t, expectCurrentRuntime, runtime) + // add ExtendedResourceKeyXCPU to node resource + deltaRes[ExtendedResourceKeyXCPU] = *xCPUQuantity + gqm.UpdateClusterTotalResource(deltaRes) + runtime = gqm.RefreshRuntime("test1") + expectCurrentRuntime[ExtendedResourceKeyXCPU] = *resource.NewQuantity(80, resource.DecimalSI) + assert.Equal(t, expectCurrentRuntime, runtime) + + // delete max ExtendedResourceKeyXCPU + delete(quota1.Spec.Max, ExtendedResourceKeyXCPU) + maxJson, err = json.Marshal(quota1.Spec.Max) + assert.Nil(t, err) + quota1.Annotations[extension.AnnotationSharedWeight] = string(maxJson) + gqm.UpdateQuota(quota1, false) + quotaInfo = gqm.quotaInfoMap["test1"] + assert.True(t, quotaInfo != nil) + assert.Equal(t, resource.Quantity{}, quotaInfo.CalculateInfo.Max[ExtendedResourceKeyXCPU]) + runtime = gqm.RefreshRuntime("test1") + delete(expectCurrentRuntime, ExtendedResourceKeyXCPU) + assert.Equal(t, expectCurrentRuntime, runtime) +} func TestGroupQuotaManager_DeleteOneGroup(t *testing.T) { gqm := NewGroupQuotaManagerForTest() gqm.UpdateClusterTotalResource(createResourceList(1000, 1000*GigaByte)) diff --git a/pkg/scheduler/plugins/elasticquota/core/runtime_quota_calculator.go b/pkg/scheduler/plugins/elasticquota/core/runtime_quota_calculator.go index 2a1cc31bd..7ed7bd993 100644 --- a/pkg/scheduler/plugins/elasticquota/core/runtime_quota_calculator.go +++ b/pkg/scheduler/plugins/elasticquota/core/runtime_quota_calculator.go @@ -21,6 +21,7 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" + "k8s.io/apiserver/pkg/util/flowcontrol/format" "k8s.io/klog/v2" "github.com/koordinator-sh/koordinator/pkg/util" @@ -127,6 +128,7 @@ func (qt *quotaTree) redistribution(totalResource int64) { if node.request > min { // if a node's request > autoScaleMin, the node needs adjustQuota // the node's runtime is autoScaleMin + klog.Infof("[jxtest] node.request: %v,min: %v,node.sharedWeight: %v", node.request, min, node.sharedWeight) needAdjustQuotaNodes = append(needAdjustQuotaNodes, node) totalSharedWeight += node.sharedWeight node.runtimeQuota = min @@ -139,6 +141,7 @@ func (qt *quotaTree) redistribution(totalResource int64) { node.runtimeQuota = min } } + klog.Infof("[jxtest] toPartitionResource: %v", toPartitionResource) toPartitionResource -= node.runtimeQuota } @@ -152,7 +155,7 @@ func (qt *quotaTree) iterationForRedistribution(totalRes, totalSharedWeight int6 // if totalSharedWeight is not larger than 0, no need to iterate anymore. return } - + klog.Infof("[jxtest] iterationForRedistribution") needAdjustQuotaNodes := make([]*quotaNode, 0) toPartitionResource, needAdjustTotalSharedWeight := int64(0), int64(0) for _, node := range nodes { @@ -493,6 +496,7 @@ func (qtw *RuntimeQuotaCalculator) calculateRuntimeNoLock() { //lock outside for resKey := range qtw.resourceKeys { totalResourcePerKey := *qtw.totalResource.Name(resKey, resource.DecimalSI) + klog.Infof("[jxtest] key: %v,q:%v", resKey, format.ToJSON(totalResourcePerKey)) qtw.quotaTree[resKey].redistribution(getQuantityValue(totalResourcePerKey, resKey)) } } diff --git a/pkg/scheduler/plugins/elasticquota/core/runtime_quota_calculator_test.go b/pkg/scheduler/plugins/elasticquota/core/runtime_quota_calculator_test.go index 9fe8d05b3..069a405cc 100644 --- a/pkg/scheduler/plugins/elasticquota/core/runtime_quota_calculator_test.go +++ b/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 := "aNodeInfo{ + groupName: TestNode1, + sharedWeight: 40, + request: 5, + min: 10, + guarantee: 0, + allowLentResource: true, + } + node2 := "aNodeInfo{ + groupName: TestNode2, + sharedWeight: 60, + request: 20, + min: 15, + guarantee: 0, + allowLentResource: true, + } + node3 := "aNodeInfo{ + groupName: TestNode3, + sharedWeight: 50, + request: 40, + min: 20, + guarantee: 0, + allowLentResource: true, + } + node4 := "aNodeInfo{ + groupName: TestNode4, + sharedWeight: 80, + request: 70, + min: 15, + guarantee: 0, + allowLentResource: true, + } + node4_1 := "aNodeInfo{ + groupName: TestNode4, + sharedWeight: 0, + request: 70, + min: 15, + guarantee: 0, + allowLentResource: true, + } + node4_2 := "aNodeInfo{ + 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 { diff --git a/pkg/webhook/elasticquota/quota_topology.go b/pkg/webhook/elasticquota/quota_topology.go index 0015a027e..efae3d20f 100644 --- a/pkg/webhook/elasticquota/quota_topology.go +++ b/pkg/webhook/elasticquota/quota_topology.go @@ -195,7 +195,7 @@ func (qt *quotaTopology) ValidDeleteQuota(quota *v1alpha1.ElasticQuota) error { return nil } -// fillQuotaDefaultInformation fills quota with default information if not configure +// fillQuotaDefaultInformation fills quota with default information if not be configured func (qt *quotaTopology) fillQuotaDefaultInformation(quota *v1alpha1.ElasticQuota) error { if quota.Name == extension.RootQuotaName { return nil @@ -235,8 +235,20 @@ func (qt *quotaTopology) fillQuotaDefaultInformation(quota *v1alpha1.ElasticQuot if sharedWeight, exist := quota.Annotations[extension.AnnotationSharedWeight]; !exist || len(sharedWeight) == 0 { quota.Annotations[extension.AnnotationSharedWeight] = string(maxQuota) klog.V(5).Infof("fill quota %v sharedWeight as max", quota.Name) + } else { + sharedWeightRL := make(corev1.ResourceList) + err = json.Unmarshal([]byte(sharedWeight), &sharedWeightRL) + if err != nil { + return fmt.Errorf("fillDefaultQuotaInfo unmarshal sharedWeight failed:%v", err) + } + if fixedSharedWeight(sharedWeightRL, quota.Spec.Max) { + fixedSharedWeightRL, err := json.Marshal(&sharedWeightRL) + if err != nil { + return fmt.Errorf("fillDefaultQuotaInfo marshal fixedSharedWeight max failed:%v", err) + } + quota.Annotations[extension.AnnotationSharedWeight] = string(fixedSharedWeightRL) + } } - return nil } @@ -286,3 +298,28 @@ func (qt *quotaTopology) getQuotaInfo(name, namespace string) *QuotaInfo { } return nil } + +// fixedSharedWeight keep keys in sharedWeight and maxQuota same +// if key in maxQuota not included in sharedWeight, add key/value in sharedWeight +// if key in sharedWeight not included in maxQuota, delete key/value in sharedWeight +// if fixed, return true +func fixedSharedWeight(sharedWeight, maxQuota corev1.ResourceList) bool { + fixed := false + for key, value := range maxQuota { + if _, ok := sharedWeight[key]; !ok { + sharedWeight[key] = value + fixed = true + } + } + toDeleted := make([]corev1.ResourceName, 0) + for key := range sharedWeight { + if _, ok := maxQuota[key]; !ok { + toDeleted = append(toDeleted, key) + } + } + for _, key := range toDeleted { + fixed = true + delete(sharedWeight, key) + } + return fixed +} diff --git a/pkg/webhook/elasticquota/quota_topology_check.go b/pkg/webhook/elasticquota/quota_topology_check.go index ec3bbaba9..c55cc68ba 100644 --- a/pkg/webhook/elasticquota/quota_topology_check.go +++ b/pkg/webhook/elasticquota/quota_topology_check.go @@ -47,6 +47,7 @@ func (qt *quotaTopology) validateQuotaSelfItem(quota *v1alpha1.ElasticQuota) err } var sharedRatio v1.ResourceList + // 1.check if sharewight is equal to if quota.Annotations[extension.AnnotationSharedWeight] != "" { if err := json.Unmarshal([]byte(quota.Annotations[extension.AnnotationSharedWeight]), &sharedRatio); err != nil { return err @@ -57,10 +58,15 @@ func (qt *quotaTopology) validateQuotaSelfItem(quota *v1alpha1.ElasticQuota) err } } - // minQuota <= maxQuota + // 1. check if all key in min are included in max + // 2. check if all quantities in min <= that in max for key, val := range quota.Spec.Min { - if maxVal, exist := quota.Spec.Max[key]; !exist || maxVal.Cmp(val) == -1 { - return fmt.Errorf("%v min :%v > max,%v", quota.Name, quota.Spec.Min, quota.Spec.Max) + if maxVal, exist := quota.Spec.Max[key]; exist { + if maxVal.Cmp(val) == -1 { + return fmt.Errorf("resourceKey %v of quota %v min :%v > max,%v", key, quota.Name, quota.Spec.Min, quota.Spec.Max) + } + } else { + return fmt.Errorf("resourceKey %v of quota %v is included in min, which is not included in max", key, quota.Name) } } @@ -68,7 +74,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 +97,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 +186,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 +222,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 +258,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 +377,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 diff --git a/pkg/webhook/elasticquota/quota_topology_test.go b/pkg/webhook/elasticquota/quota_topology_test.go index a5f9baccc..0a12dfbf2 100644 --- a/pkg/webhook/elasticquota/quota_topology_test.go +++ b/pkg/webhook/elasticquota/quota_topology_test.go @@ -65,12 +65,12 @@ func TestQuotaTopology_basicItemCheck(t *testing.T) { err: nil, }, { - name: "max <0", + name: "max < 0", quota: MakeQuota("temp").Max(MakeResourceList().CPU(-1).Mem(1048576).Obj()).Obj(), err: fmt.Errorf("%v quota.Spec.Max's value < 0, in dimensions :%v", "temp", "[cpu]"), }, { - name: "min <0", + name: "min < 0", quota: MakeQuota("temp").Min(MakeResourceList().CPU(-1).Mem(1048576).Obj()). Max(MakeResourceList().CPU(1).Mem(1048576).Obj()).Obj(), err: fmt.Errorf("%v quota.Spec.Min's value < 0, in dimensions :%v", "temp", "[cpu]"), @@ -79,19 +79,18 @@ func TestQuotaTopology_basicItemCheck(t *testing.T) { name: "min dimension larger than max", quota: MakeQuota("temp").Min(MakeResourceList().CPU(1).Mem(1048576).Obj()). Max(MakeResourceList().CPU(10).Obj()).Obj(), - err: fmt.Errorf("%v min :%v > max,%v", "temp", - MakeResourceList().CPU(1).Mem(1048576).Obj(), MakeResourceList().CPU(10).Obj()), + err: fmt.Errorf("resourceKey %v of quota %v is included in min, which is not included in max", "memory", "temp"), }, { name: "min > max", quota: MakeQuota("temp").Min(MakeResourceList().CPU(12).Obj()). Max(MakeResourceList().CPU(10).Obj()).Obj(), - err: fmt.Errorf("%v min :%v > max,%v", "temp", + err: fmt.Errorf("resourceKey %v of quota %v min :%v > max,%v", "cpu", "temp", MakeResourceList().CPU(12).Obj(), MakeResourceList().CPU(10).Obj()), }, { - name: "min dimension larger than max", - quota: MakeQuota("temp").sharedWeight(MakeResourceList().CPU(-1).Mem(1048576).Obj()).Obj(), + name: "annotation sharedWeight < 0", + quota: MakeQuota("temp").sharedWeight(MakeResourceList().CPU(-1).Mem(1048576).Obj()).Max(MakeResourceList().CPU(0).Mem(1048576).Obj()).Obj(), err: fmt.Errorf("%v quota.Annotation[%v]'s value < 0, in dimension :%v", "temp", extension.AnnotationSharedWeight, "[cpu]"), }, } @@ -100,108 +99,245 @@ func TestQuotaTopology_basicItemCheck(t *testing.T) { qt := newFakeQuotaTopology() qt.fillQuotaDefaultInformation(tt.quota) err := qt.validateQuotaSelfItem(tt.quota) - assert.Equal(t, err, tt.err) + assert.Equal(t, tt.err, err) }) } } -func TestQuotaTopology_fillDefaultQuotaInfo(t *testing.T) { - qt := newFakeQuotaTopology() - quota := MakeQuota("temp2").Max(MakeResourceList().CPU(120).Mem(1048576).Obj()).Obj() - quota.Labels = nil - quota.Annotations = nil - err := qt.fillQuotaDefaultInformation(quota) - assert.Nil(t, err) - assert.Equal(t, extension.RootQuotaName, quota.Labels[extension.LabelQuotaParent]) - maxQuota, _ := json.Marshal(quota.Spec.Max) - assert.Equal(t, string(maxQuota), quota.Annotations[extension.AnnotationSharedWeight]) - - qt.OnQuotaAdd(quota) - - quota = MakeQuota("temp2-bu1").ParentName("temp2").Max(MakeResourceList().CPU(120).Mem(1048576).Obj()).Obj() - err = qt.fillQuotaDefaultInformation(quota) - assert.Nil(t, err) - assert.Equal(t, "temp2", quota.Labels[extension.LabelQuotaParent]) - assert.Equal(t, string(maxQuota), quota.Annotations[extension.AnnotationSharedWeight]) -} - -func TestQuotaTopology_fillDefaultQuotaInfoWithTreeID(t *testing.T) { - qt := newFakeQuotaTopology() - quota := MakeQuota("temp2").Max(MakeResourceList().CPU(120).Mem(1048576).Obj()).TreeID("tree-1").Obj() - err := qt.fillQuotaDefaultInformation(quota) - assert.Nil(t, err) - assert.Equal(t, extension.RootQuotaName, quota.Labels[extension.LabelQuotaParent]) - assert.Equal(t, "tree-1", quota.Labels[extension.LabelQuotaTreeID]) - maxQuota, _ := json.Marshal(quota.Spec.Max) - assert.Equal(t, string(maxQuota), quota.Annotations[extension.AnnotationSharedWeight]) - - qt.OnQuotaAdd(quota) - - quota = MakeQuota("temp2-bu1").ParentName("temp2").Max(MakeResourceList().CPU(120).Mem(1048576).Obj()).Obj() - err = qt.fillQuotaDefaultInformation(quota) - assert.Nil(t, err) - assert.Equal(t, "temp2", quota.Labels[extension.LabelQuotaParent]) - assert.Equal(t, "tree-1", quota.Labels[extension.LabelQuotaTreeID]) - assert.Equal(t, string(maxQuota), quota.Annotations[extension.AnnotationSharedWeight]) +func TestQuotaTopology_fillQuotaDefaultInformation(t *testing.T) { + type quotaInfo struct { + initOne *v1alpha1.ElasticQuota + expectLabelQuotaParent string + expectAnnotationSharedWeight string + expectedLabelQuotaTreeID string + } + testCase := []struct { + name string + quotas []*quotaInfo + }{ + { + name: "parent quota without treeID", + quotas: []*quotaInfo{ + { + initOne: MakeQuota("temp2").Max(MakeResourceList().CPU(120).Mem(1048576).Obj()).Obj(), + expectLabelQuotaParent: extension.RootQuotaName, + expectAnnotationSharedWeight: "{\"cpu\":\"120\",\"memory\":\"1048576\"}", + expectedLabelQuotaTreeID: "", + }, + { + initOne: MakeQuota("temp2-bu1").ParentName("temp2").Max(MakeResourceList().CPU(120).Mem(1048576).Obj()).Obj(), + expectLabelQuotaParent: "temp2", + expectAnnotationSharedWeight: "{\"cpu\":\"120\",\"memory\":\"1048576\"}", + expectedLabelQuotaTreeID: "", + }, + }, + }, + { + name: "parent quota with treeID", + quotas: []*quotaInfo{ + { + initOne: MakeQuota("temp2").Max(MakeResourceList().CPU(120).Mem(1048576).Obj()).TreeID("tree-1").Obj(), + expectLabelQuotaParent: extension.RootQuotaName, + expectAnnotationSharedWeight: "{\"cpu\":\"120\",\"memory\":\"1048576\"}", + expectedLabelQuotaTreeID: "tree-1", + }, + { + initOne: MakeQuota("temp2-bu1").ParentName("temp2").Max(MakeResourceList().CPU(120).Mem(1048576).Obj()).Obj(), + expectLabelQuotaParent: "temp2", + expectAnnotationSharedWeight: "{\"cpu\":\"120\",\"memory\":\"1048576\"}", + expectedLabelQuotaTreeID: "tree-1", + }, + }, + }, + { + name: "quota with annotation SharedWeight, SharedWeight.keys == maxQuota.keys", + quotas: []*quotaInfo{ + { + initOne: MakeQuota("temp2").sharedWeight(MakeResourceList().CPU(0).Mem(0).Obj()).Max(MakeResourceList().CPU(120).Mem(1048576).Obj()).TreeID("tree-1").Obj(), + expectLabelQuotaParent: extension.RootQuotaName, + expectAnnotationSharedWeight: "{\"cpu\":\"0\",\"memory\":\"0\"}", + expectedLabelQuotaTreeID: "tree-1", + }, + }, + }, + { + name: "quota with annotation SharedWeight, SharedWeight.keys > maxQuota.keys", + quotas: []*quotaInfo{ + { + initOne: MakeQuota("temp2").sharedWeight(MakeResourceList().CPU(0).Mem(0).Obj()).Max(MakeResourceList().Mem(1048576).Obj()).TreeID("tree-1").Obj(), + expectLabelQuotaParent: extension.RootQuotaName, + expectAnnotationSharedWeight: "{\"memory\":\"0\"}", + expectedLabelQuotaTreeID: "tree-1", + }, + }, + }, + { + name: "quota with annotation SharedWeight, SharedWeight.keys < maxQuota.keys", + quotas: []*quotaInfo{ + { + initOne: MakeQuota("temp2").sharedWeight(MakeResourceList().CPU(0).Obj()).Max(MakeResourceList().CPU(120).Mem(1048576).Obj()).TreeID("tree-1").Obj(), + expectLabelQuotaParent: extension.RootQuotaName, + expectAnnotationSharedWeight: "{\"cpu\":\"0\",\"memory\":\"1048576\"}", + expectedLabelQuotaTreeID: "tree-1", + }, + }, + }, + { + name: "quota with annotation SharedWeight, len(SharedWeight.keys) == len(maxQuota.keys), SharedWeight.keys != maxQuota.keys", + quotas: []*quotaInfo{ + { + initOne: MakeQuota("temp2").sharedWeight(MakeResourceList().CPU(0).Obj()).Max(MakeResourceList().Mem(1048576).Obj()).TreeID("tree-1").Obj(), + expectLabelQuotaParent: extension.RootQuotaName, + expectAnnotationSharedWeight: "{\"memory\":\"1048576\"}", + expectedLabelQuotaTreeID: "tree-1", + }, + }, + }, + } + for _, tt := range testCase { + t.Run(tt.name, func(t *testing.T) { + qt := newFakeQuotaTopology() + for _, info := range tt.quotas { + quota := info.initOne + err := qt.fillQuotaDefaultInformation(quota) + assert.Nil(t, err) + assert.Equal(t, info.expectLabelQuotaParent, quota.Labels[extension.LabelQuotaParent]) + assert.Equal(t, info.expectAnnotationSharedWeight, quota.Annotations[extension.AnnotationSharedWeight]) + assert.Equal(t, info.expectedLabelQuotaTreeID, quota.Labels[extension.LabelQuotaTreeID]) + qt.OnQuotaAdd(quota) + } + }) + } } - func TestQuotaTopology_checkSubAndParentGroupMaxQuotaKeySame(t *testing.T) { tests := []struct { - name string - parQuota *v1alpha1.ElasticQuota - quota *v1alpha1.ElasticQuota - subQuota *v1alpha1.ElasticQuota - err error - eraseSub bool + name string + parQuota *v1alpha1.ElasticQuota + quota *v1alpha1.ElasticQuota + subQuota *v1alpha1.ElasticQuota + enableResourceTypeUpdate bool + err error + eraseSub bool }{ { name: "no tree", quota: MakeQuota("temp").Max(MakeResourceList().CPU(120).Mem(1048576).Obj()).IsParent(true).Obj(), subQuota: MakeQuota("temp-bu1").ParentName("temp"). Max(MakeResourceList().CPU(120).Mem(1048576).Obj()).IsParent(false).Obj(), - err: nil, + enableResourceTypeUpdate: false, + err: nil, }, { name: "parent is root", quota: MakeQuota("temp-bu1").ParentName(extension.RootQuotaName). Max(MakeResourceList().CPU(120).Mem(1048576).Obj()).IsParent(false).Obj(), - err: nil, + enableResourceTypeUpdate: false, + err: nil, }, { - name: "parent's key size > child's key size", + name: "child's max has different dimension with its,but be included, enableResourceTypeUpdate = false", quota: MakeQuota("temp").Max(MakeResourceList().CPU(10).Mem(120).Obj()).IsParent(true).Obj(), subQuota: MakeQuota("temp-bu1").ParentName("temp"). Max(MakeResourceList().CPU(120).Obj()).IsParent(false).Obj(), - err: fmt.Errorf("error"), + enableResourceTypeUpdate: false, + err: fmt.Errorf("error"), }, { - name: "size same, dimension is different", - quota: MakeQuota("temp").Max(MakeResourceList().Mem(120).Obj()).IsParent(true).Obj(), + name: "child's max has same dimension with its,but be included, enableResourceTypeUpdate = false", + quota: MakeQuota("temp").Max(MakeResourceList().CPU(10).Obj()).IsParent(true).Obj(), subQuota: MakeQuota("temp-bu1").ParentName("temp"). Max(MakeResourceList().CPU(120).Obj()).IsParent(false).Obj(), - err: fmt.Errorf("error"), + enableResourceTypeUpdate: false, + err: nil, }, { - name: "child's key size > parent's key size", - quota: MakeQuota("temp").Max(MakeResourceList().CPU(10).Obj()).IsParent(true).Obj(), + name: "child's max has different dimension with its,but be included, enableResourceTypeUpdate = true", + quota: MakeQuota("temp").Max(MakeResourceList().CPU(10).Mem(120).Obj()).IsParent(true).Obj(), subQuota: MakeQuota("temp-bu1").ParentName("temp"). - Max(MakeResourceList().CPU(120).Mem(120).Obj()).IsParent(false).Obj(), - err: fmt.Errorf("error"), + Max(MakeResourceList().CPU(120).Obj()).IsParent(false).Obj(), + enableResourceTypeUpdate: true, + err: nil, + }, + { + name: "child's max has different dimension with its,not be included, enableResourceTypeUpdate = true", + quota: MakeQuota("temp").Max(MakeResourceList().Mem(120).Obj()).IsParent(true).Obj(), + subQuota: MakeQuota("temp-bu1").ParentName("temp"). + Max(MakeResourceList().CPU(120).Obj()).IsParent(false).Obj(), + enableResourceTypeUpdate: true, + err: fmt.Errorf("error"), + }, + { + name: "its max has different dimension with parent's,but be included, enableResourceTypeUpdate = false", + parQuota: MakeQuota("temp").Max(MakeResourceList().CPU(10).Mem(120).Obj()).IsParent(true).Obj(), + quota: MakeQuota("temp-bu1").ParentName("temp"). + Max(MakeResourceList().CPU(120).Obj()).IsParent(false).Obj(), + enableResourceTypeUpdate: false, + err: fmt.Errorf("error"), }, { - name: "quotaInfo not satisfy", + name: "its max has same dimension with parent's,but be included, enableResourceTypeUpdate = false", parQuota: MakeQuota("temp").Max(MakeResourceList().CPU(10).Obj()).IsParent(true).Obj(), quota: MakeQuota("temp-bu1").ParentName("temp"). - Max(MakeResourceList().CPU(120).Mem(120).Obj()).IsParent(false).Obj(), - err: fmt.Errorf("error"), + Max(MakeResourceList().CPU(120).Obj()).IsParent(false).Obj(), + enableResourceTypeUpdate: false, + err: nil, + }, + { + name: "its max has different dimension with parent's,but be included, enableResourceTypeUpdate = true", + parQuota: MakeQuota("temp").Max(MakeResourceList().CPU(10).Mem(120).Obj()).IsParent(true).Obj(), + quota: MakeQuota("temp-bu1").ParentName("temp"). + Max(MakeResourceList().CPU(120).Obj()).IsParent(false).Obj(), + enableResourceTypeUpdate: true, + err: nil, + }, + { + name: "its max has different dimension with parent's,not be included, enableResourceTypeUpdate = true", + parQuota: MakeQuota("temp").Max(MakeResourceList().Mem(120).Obj()).IsParent(true).Obj(), + quota: MakeQuota("temp-bu1").ParentName("temp"). + Max(MakeResourceList().CPU(120).Obj()).IsParent(false).Obj(), + enableResourceTypeUpdate: true, + err: fmt.Errorf("error"), + }, + { + name: "its min has different dimension with parent's,but be included, enableResourceTypeUpdate = false", + parQuota: MakeQuota("temp").Min(MakeResourceList().CPU(10).Mem(120).Obj()).IsParent(true).Obj(), + quota: MakeQuota("temp-bu1").ParentName("temp"). + Min(MakeResourceList().CPU(120).Obj()).IsParent(false).Obj(), + enableResourceTypeUpdate: false, + err: nil, + }, + { + name: "its min has different dimension with parent's,not be all included, enableResourceTypeUpdate = false", + parQuota: MakeQuota("temp").Min(MakeResourceList().CPU(10).Obj()).IsParent(true).Obj(), + quota: MakeQuota("temp-bu1").ParentName("temp"). + Min(MakeResourceList().CPU(120).Mem(120).Obj()).IsParent(false).Obj(), + enableResourceTypeUpdate: false, + err: fmt.Errorf("error"), + }, + { + name: "its min has different dimension with parent's,but be all included, enableResourceTypeUpdate = true", + parQuota: MakeQuota("temp").Min(MakeResourceList().CPU(10).Mem(120).Obj()).IsParent(true).Obj(), + quota: MakeQuota("temp-bu1").ParentName("temp"). + Min(MakeResourceList().CPU(120).Obj()).IsParent(false).Obj(), + enableResourceTypeUpdate: true, + err: nil, + }, + { + name: "its min has different dimension with parent's,not be included, enableResourceTypeUpdate = true", + parQuota: MakeQuota("temp").Min(MakeResourceList().CPU(10).Obj()).IsParent(true).Obj(), + quota: MakeQuota("temp-bu1").ParentName("temp"). + Min(MakeResourceList().CPU(120).Mem(120).Obj()).IsParent(false).Obj(), + enableResourceTypeUpdate: true, + err: fmt.Errorf("error"), }, { name: "bug", quota: MakeQuota("temp").Max(MakeResourceList().CPU(120).Mem(1048576).Obj()).IsParent(true).Obj(), subQuota: MakeQuota("temp-bu1").ParentName("temp"). Max(MakeResourceList().CPU(120).Mem(1048576).Obj()).IsParent(false).Obj(), - err: fmt.Errorf("error"), - eraseSub: true, + enableResourceTypeUpdate: false, + err: fmt.Errorf("error"), + eraseSub: true, }, } for _, tt := range tests { @@ -214,7 +350,7 @@ func TestQuotaTopology_checkSubAndParentGroupMaxQuotaKeySame(t *testing.T) { if tt.eraseSub { delete(qt.quotaInfoMap, tt.subQuota.Name) } - err := qt.checkSubAndParentGroupMaxQuotaKeySame(quotaInfo) + err := qt.checkSubAndParentGroupQuotaKey(quotaInfo, tt.enableResourceTypeUpdate) if (tt.err != nil && err == nil) || (tt.err == nil && err != nil) { t.Errorf("error") }