Merge pull request #28991 from ZTE-PaaS/zhangke-patch-008

Automatic merge from submit-queue

optimize conditions of ServiceReplenishmentUpdateFunc to replenish service

Originally, the  replenishQuota method didn't focus on the third parameter object even if others transfered to it, i think the function is not efficient and perfect. then i use the third param to get MatchResources, it will be more exact. for example, if the old pod was quota tracked and the new was not, the replenishQuota only focus on usage resource of the old pod, still if  the third parameter object is nil, the process will be same as before

pkg/controller/resourcequota/replenishment_controller.go

@@ -59,7 +59,7 @@ func PodReplenishmentUpdateFunc(options *ReplenishmentControllerOptions) func(ol
 		oldPod := oldObj.(*api.Pod)
 		newPod := newObj.(*api.Pod)
 		if core.QuotaPod(oldPod) && !core.QuotaPod(newPod) {
-			options.ReplenishmentFunc(options.GroupKind, newPod.Namespace, newPod)
+			options.ReplenishmentFunc(options.GroupKind, newPod.Namespace, oldPod)
 		}
 	}
 }
@@ -219,13 +219,13 @@ func (r *replenishmentControllerFactory) NewController(options *ReplenishmentCon
 	return result, nil
 }
 
-// ServiceReplenishmentUpdateFunc will replenish if the old service was quota tracked but the new is not
+// ServiceReplenishmentUpdateFunc will replenish if the service was quota tracked has changed service type
 func ServiceReplenishmentUpdateFunc(options *ReplenishmentControllerOptions) func(oldObj, newObj interface{}) {
 	return func(oldObj, newObj interface{}) {
 		oldService := oldObj.(*api.Service)
 		newService := newObj.(*api.Service)
-		if core.QuotaServiceType(oldService) || core.QuotaServiceType(newService) {
-			options.ReplenishmentFunc(options.GroupKind, newService.Namespace, newService)
+		if core.GetQuotaServiceType(oldService) != core.GetQuotaServiceType(newService) {
+			options.ReplenishmentFunc(options.GroupKind, newService.Namespace, nil)
 		}
 	}
 }

pkg/controller/resourcequota/replenishment_controller_test.go

@@ -118,4 +118,38 @@ func TestServiceReplenishmentUpdateFunc(t *testing.T) {
 	if mockReplenish.namespace != oldService.Namespace {
 		t.Errorf("Unexpected namespace %v", mockReplenish.namespace)
 	}
+
+	mockReplenish = &testReplenishment{}
+	options = ReplenishmentControllerOptions{
+		GroupKind:         api.Kind("Service"),
+		ReplenishmentFunc: mockReplenish.Replenish,
+		ResyncPeriod:      controller.NoResyncPeriodFunc,
+	}
+	oldService = &api.Service{
+		ObjectMeta: api.ObjectMeta{Namespace: "test", Name: "mysvc"},
+		Spec: api.ServiceSpec{
+			Type: api.ServiceTypeNodePort,
+			Ports: []api.ServicePort{{
+				Port:       80,
+				TargetPort: intstr.FromInt(80),
+			}},
+		},
+	}
+	newService = &api.Service{
+		ObjectMeta: api.ObjectMeta{Namespace: "test", Name: "mysvc"},
+		Spec: api.ServiceSpec{
+			Type: api.ServiceTypeNodePort,
+			Ports: []api.ServicePort{{
+				Port:       81,
+				TargetPort: intstr.FromInt(81),
+			}}},
+	}
+	updateFunc = ServiceReplenishmentUpdateFunc(&options)
+	updateFunc(oldService, newService)
+	if mockReplenish.groupKind == api.Kind("Service") {
+		t.Errorf("Unexpected group kind %v", mockReplenish.groupKind)
+	}
+	if mockReplenish.namespace == oldService.Namespace {
+		t.Errorf("Unexpected namespace %v", mockReplenish.namespace)
+	}
 }

pkg/quota/evaluator/core/services.go

@@ -74,3 +74,14 @@ func QuotaServiceType(service *api.Service) bool {
 	}
 	return false
 }
+
+//GetQuotaServiceType returns ServiceType if the service type is eligible to track against a quota, nor return ""
+func GetQuotaServiceType(service *api.Service) api.ServiceType {
+	switch service.Spec.Type {
+	case api.ServiceTypeNodePort:
+		return api.ServiceTypeNodePort
+	case api.ServiceTypeLoadBalancer:
+		return api.ServiceTypeLoadBalancer
+	}
+	return api.ServiceType("")
+}