scheduler: fix pod update when pod creation is before quota creation (#…

…2177) Signed-off-by: chuanyun.lcy <[email protected]> Co-authored-by: chuanyun.lcy <[email protected]>
koordinator-sh · Sep 2, 2024 · 4d89c84 · 4d89c84
1 parent 6c740d0
commit 4d89c84
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 2 deletions.
diff --git a/pkg/scheduler/plugins/elasticquota/core/group_quota_manager.go b/pkg/scheduler/plugins/elasticquota/core/group_quota_manager.go
@@ -749,8 +749,9 @@ func (gqm *GroupQuotaManager) OnPodAdd(quotaName string, pod *v1.Pod) {
 	gqm.hierarchyUpdateLock.RLock()
 	defer gqm.hierarchyUpdateLock.RUnlock()
 
+	// if the quotaInfo is nil or include the pod, skip it.
 	quotaInfo := gqm.getQuotaInfoByNameNoLock(quotaName)
-	if quotaInfo != nil && quotaInfo.IsPodExist(pod) {
+	if quotaInfo == nil || quotaInfo.IsPodExist(pod) {
 		return
 	}
 
@@ -774,6 +775,14 @@ func (gqm *GroupQuotaManager) OnPodUpdate(newQuotaName, oldQuotaName string, new
 		}
 
 		if !shouldBeIgnored(newPod) {
+			if quotaInfo.IsPodExist(newPod) {
+				gqm.updatePodRequestNoLock(newQuotaName, oldPod, newPod)
+			} else {
+				// it's means the pod creation is before quota creation.
+				gqm.updatePodCacheNoLock(newQuotaName, newPod, true)
+				gqm.updatePodRequestNoLock(newQuotaName, nil, newPod)
+			}
+
 			isAssigned := gqm.getPodIsAssignedNoLock(newQuotaName, newPod)
 			if isAssigned {
 				// reserve phase will assign the pod. Just update it.
@@ -786,7 +795,6 @@ func (gqm *GroupQuotaManager) OnPodUpdate(newQuotaName, oldQuotaName string, new
 					gqm.updatePodUsedNoLock(newQuotaName, nil, newPod)
 				}
 			}
-			gqm.updatePodRequestNoLock(newQuotaName, oldPod, newPod)
 		} else {
 			if quotaInfo.IsPodExist(oldPod) {
 				// remove the old resource.

diff --git a/pkg/scheduler/plugins/elasticquota/core/group_quota_manager_test.go b/pkg/scheduler/plugins/elasticquota/core/group_quota_manager_test.go
@@ -2022,3 +2022,50 @@ func TestUpdateQuotaInternalNoLock_ParenstSelf(t *testing.T) {
 	assert.Equal(t, createResourceList(25, 25), qi1.CalculateInfo.NonPreemptibleRequest)
 	assert.Equal(t, createResourceList(50, 20), qi1.CalculateInfo.Request)
 }
+
+func TestUpdatePod_WhenQuotaCreateAfterPodCreate(t *testing.T) {
+	gqm := NewGroupQuotaManagerForTest()
+	gqm.scaleMinQuotaEnabled = true
+	gqm.UpdateClusterTotalResource(createResourceList(50, 50))
+
+	// create pod first
+	pod1 := schetesting.MakePod().Name("1").Obj()
+	pod1.Spec.Containers = []v1.Container{
+		{
+			Resources: v1.ResourceRequirements{
+				Requests: createResourceList(10, 10),
+			},
+		},
+	}
+	gqm.OnPodAdd("1", pod1)
+
+	// create quota later
+	eq1 := createQuota("1", extension.RootQuotaName, 40, 40, 20, 20)
+	gqm.UpdateQuota(eq1, false)
+
+	quotaInfo := gqm.GetQuotaInfoByName(eq1.Name)
+	assert.Equal(t, 0, len(quotaInfo.GetPodCache()))
+	assert.Equal(t, v1.ResourceList{}, quotaInfo.GetRequest())
+	assert.Equal(t, v1.ResourceList{}, quotaInfo.GetUsed())
+
+	// update pod.
+	newPod1 := pod1.DeepCopy()
+	newPod1.Labels = map[string]string{"aaa": "aaa"}
+
+	gqm.OnPodUpdate("1", "1", newPod1, pod1)
+
+	quotaInfo = gqm.GetQuotaInfoByName(eq1.Name)
+	assert.Equal(t, 1, len(quotaInfo.GetPodCache()))
+	assert.Equal(t, createResourceList(10, 10), quotaInfo.GetRequest())
+	assert.Equal(t, v1.ResourceList{}, quotaInfo.GetUsed())
+
+	// update again
+	newPod2 := newPod1.DeepCopy()
+	newPod2.Spec.NodeName = "node1"
+	gqm.OnPodUpdate("1", "1", newPod2, newPod1)
+	quotaInfo = gqm.GetQuotaInfoByName(eq1.Name)
+	assert.Equal(t, 1, len(quotaInfo.GetPodCache()))
+	assert.Equal(t, createResourceList(10, 10), quotaInfo.GetRequest())
+	assert.Equal(t, createResourceList(10, 10), quotaInfo.GetUsed())
+
+}