fix vpa status current update

Author: luomingmeng

pkg/controller/vpa/util/resource.go

@@ -337,46 +337,84 @@ func GetVPAResourceStatusWithCurrent(vpa *apis.KatalystVerticalPodAutoscaler, po
 		return nil, nil, err
 	}
 
+	updateContainerResourcesCurrent := func(targetResourceNames map[consts.ContainerName][]core.ResourceName,
+		containerName consts.ContainerName,
+		target core.ResourceList, current *core.ResourceList,
+		specResource core.ResourceList,
+	) {
+		// if pod apply strategy is 'Pod', we not need update container current,
+		// because each pod has different recommendation.
+		if vpa.Spec.UpdatePolicy.PodApplyStrategy == apis.PodApplyStrategyStrategyPod {
+			*current = nil
+			return
+		}
+
+		_, ok := targetResourceNames[containerName]
+		if !ok {
+			*current = target.DeepCopy()
+			resourceNames := make([]core.ResourceName, 0, len(target))
+			for resourceName := range target {
+				resourceNames = append(resourceNames, resourceName)
+			}
+			sort.SliceStable(resourceNames, func(i, j int) bool {
+				return string(resourceNames[i]) < string(resourceNames[j])
+			})
+			targetResourceNames[containerName] = resourceNames
+		}
+
+		specCurrent := cropResourcesWithBounds(specResource, nil, nil, targetResourceNames[containerName])
+		if !native.ResourcesEqual(target, specCurrent) &&
+			(native.ResourcesEqual(target, *current) ||
+				!native.ResourcesLess(*current, specCurrent, targetResourceNames[containerName])) {
+			*current = specCurrent
+		}
+	}
+
 	for containerName := range containerResources {
 		// get container resource current requests
 		if containerResources[containerName].Requests != nil {
+			targetResourceNames := make(map[consts.ContainerName][]core.ResourceName)
 			func(pods []*core.Pod) {
 				for _, pod := range pods {
 					for _, container := range pod.Spec.Containers {
 						if container.Name != string(containerName) {
 							continue
 						}
 
-						containerResources[containerName].Requests.Current = container.Resources.Requests
-						// if some container current requests doesn't equal to target, return it immediately
-						if !native.ResourcesEqual(containerResources[containerName].Requests.Target, container.Resources.Requests) {
-							return
-						}
+						updateContainerResourcesCurrent(targetResourceNames, containerName, containerResources[containerName].Requests.Target,
+							&containerResources[containerName].Requests.Current, container.Resources.Requests)
 					}
 				}
 			}(pods)
 		}
 
 		// get container resource current limits
 		if containerResources[containerName].Limits != nil {
+			targetResourceNames := make(map[consts.ContainerName][]core.ResourceName)
 			func(pods []*core.Pod) {
 				for _, pod := range pods {
 					for _, container := range pod.Spec.Containers {
 						if container.Name != string(containerName) {
 							continue
 						}
 
-						containerResources[containerName].Limits.Current = container.Resources.Limits
-						// if some container current limits doesn't equal to target, return it immediately
-						if !native.ResourcesEqual(containerResources[containerName].Limits.Target, container.Resources.Limits) {
-							return
-						}
+						updateContainerResourcesCurrent(targetResourceNames, containerName, containerResources[containerName].Limits.Target,
+							&containerResources[containerName].Limits.Current, container.Resources.Limits)
 					}
 				}
 			}(pods)
 		}
 	}
 
+	getPodContainerResourcesCurrent := func(target core.ResourceList, specResource core.ResourceList) core.ResourceList {
+		resourceNames := make([]core.ResourceName, 0, len(target))
+		for resourceName := range target {
+			resourceNames = append(resourceNames, resourceName)
+		}
+
+		return cropResourcesWithBounds(specResource, nil, nil, resourceNames)
+	}
+
 	for podContainerName := range podResources {
 		podName, containerName, err := native.ParsePodContainerName(podContainerName)
 		if err != nil {
@@ -396,7 +434,7 @@ func GetVPAResourceStatusWithCurrent(vpa *apis.KatalystVerticalPodAutoscaler, po
 							continue
 						}
 
-						podResources[podContainerName].Requests.Current = container.Resources.Requests
+						podResources[podContainerName].Requests.Current = getPodContainerResourcesCurrent(podResources[podContainerName].Requests.Target, container.Resources.Requests)
 						return
 					}
 				}
@@ -415,7 +453,8 @@ func GetVPAResourceStatusWithCurrent(vpa *apis.KatalystVerticalPodAutoscaler, po
 						if container.Name != containerName {
 							continue
 						}
-						podResources[podContainerName].Limits.Current = container.Resources.Limits
+
+						podResources[podContainerName].Limits.Current = getPodContainerResourcesCurrent(podResources[podContainerName].Limits.Target, container.Resources.Limits)
 						return
 					}
 				}

pkg/controller/vpa/util/resource_test.go

@@ -422,10 +422,7 @@ func testGetVPAResourceStatusWithCurrentAndIndexer(t *testing.T, podIndexers cac
 				{
 					ContainerName: pointer.String("c1"),
 					Limits: &apis.ContainerResourceList{
-						Current: v1.ResourceList{
-							v1.ResourceMemory: resource.MustParse("4Gi"),
-							v1.ResourceCPU:    resource.MustParse("4"),
-						},
+						Current:        v1.ResourceList{},
 						Target:         v1.ResourceList{},
 						UncappedTarget: v1.ResourceList{},
 						LowerBound: v1.ResourceList{

pkg/controller/vpa/vpa_status.go

@@ -324,22 +324,21 @@ func (vs *vpaStatusController) syncVPA(key string) error {
 // setRecommendationAppliedCondition set vpa recommendation applied condition by checking all pods whether
 // are updated to the expected resources in their annotations
 func (vs *vpaStatusController) setRecommendationAppliedCondition(vpa *apis.KatalystVerticalPodAutoscaler, pods []*v1.Pod) error {
-	allPodSpecUpdated := func() bool {
-		for _, pod := range pods {
-			if !katalystutil.CheckPodSpecUpdated(pod) {
-				return false
-			}
+	failedCount := 0
+	for _, pod := range pods {
+		if !katalystutil.CheckPodSpecUpdated(pod) {
+			failedCount += 1
 		}
-		return true
-	}()
+	}
 
-	if allPodSpecUpdated {
+	if failedCount == 0 {
 		err := util.SetVPAConditions(vpa, apis.RecommendationApplied, v1.ConditionTrue, util.VPAConditionReasonPodSpecUpdated, "")
 		if err != nil {
 			return err
 		}
 	} else {
-		err := util.SetVPAConditions(vpa, apis.RecommendationApplied, v1.ConditionFalse, util.VPAConditionReasonPodSpecNoUpdate, "")
+		msg := fmt.Sprintf("failed to update %d pods, total %d pods", failedCount, len(pods))
+		err := util.SetVPAConditions(vpa, apis.RecommendationApplied, v1.ConditionFalse, util.VPAConditionReasonPodSpecNoUpdate, msg)
 		if err != nil {
 			return err
 		}

pkg/util/native/resources.go

@@ -145,6 +145,23 @@ func ResourcesEqual(a, b v1.ResourceList) bool {
 	return true
 }
 
+// ResourcesLess checks whether the given resources a are less than b
+func ResourcesLess(a, b v1.ResourceList, resourceNameList []v1.ResourceName) bool {
+	for _, name := range resourceNameList {
+		quantityA, okA := a[name]
+		quantityB, okB := b[name]
+		if okA != okB {
+			return okA
+		} else if okA {
+			if quantityA.Cmp(quantityB) < 0 {
+				return true
+			}
+		}
+	}
+
+	return false
+}
+
 // AddResources sums up two ResourceList, and returns the summed as results.
 func AddResources(a, b v1.ResourceList) v1.ResourceList {
 	res := make(v1.ResourceList)