Add --in-place-skip-disruption-budget flag

Signed-off-by: Omer Aplatony <[email protected]>

Author: omerap12

vertical-pod-autoscaler/docs/flags.md

@@ -146,6 +146,7 @@ This document is auto-generated from the flag definitions in the VPA updater cod
 | `eviction-tolerance` | float |  0.5 | Fraction of replica count that can be evicted for update, if more than one pod can be evicted.  |
 | `feature-gates` | mapStringBool |  | A set of key=value pairs that describe feature gates for alpha/experimental features. Options are:<br>AllAlpha=true\|false (ALPHA - default=false)<br>AllBeta=true\|false (BETA - default=false)<br>InPlaceOrRecreate=true\|false (BETA - default=true)<br>PerVPAConfig=true\|false (ALPHA - default=false) |
 | `ignored-vpa-object-namespaces` | string |  | A comma-separated list of namespaces to ignore when searching for VPA objects. Leave empty to avoid ignoring any namespaces. These namespaces will not be cleaned by the garbage collector. |
+| `in-place-skip-disruption-budget` |  |  | If true, VPA updater skips disruption budget checks for in-place pod updates when all containers have NotRequired resize policy or no policy. Disruption budgets are still respected when any container has RestartContainer resize policy. |
 | `in-recommendation-bounds-eviction-lifetime-threshold` |  |  12h0m0s | duration   Pods that live for at least that long can be evicted even if their request is within the [MinRecommended...MaxRecommended] range  |
 | `kube-api-burst` | float |  100 | QPS burst limit when making requests to Kubernetes apiserver  |
 | `kube-api-qps` | float |  50 | QPS limit when making requests to Kubernetes apiserver  |
@@ -174,4 +175,3 @@ This document is auto-generated from the flag definitions in the VPA updater cod
 | `v,` |  | : 4 | , --v Level                                                         set the log level verbosity  (default 4) |
 | `vmodule` | moduleSpec |  | comma-separated list of pattern=N settings for file-filtered logging |
 | `vpa-object-namespace` | string |  | Specifies the namespace to search for VPA objects. Leave empty to include all namespaces. If provided, the garbage collector will only clean this namespace. |
-

vertical-pod-autoscaler/pkg/updater/logic/updater.go

@@ -93,6 +93,7 @@ func NewUpdater(
 	evictionRateBurst int,
 	evictionToleranceFraction float64,
 	useAdmissionControllerStatus bool,
+	inPlaceSkipDisruptionBudget bool,
 	statusNamespace string,
 	recommendationProcessor vpa_api_util.RecommendationProcessor,
 	evictionAdmission priority.PodEvictionAdmission,
@@ -111,6 +112,7 @@ func NewUpdater(
 		minReplicasForEviction,
 		evictionToleranceFraction,
 		patchCalculators,
+		inPlaceSkipDisruptionBudget,
 	)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create restriction factory: %v", err)

vertical-pod-autoscaler/pkg/updater/main.go

@@ -75,6 +75,13 @@ var (
 	useAdmissionControllerStatus = flag.Bool("use-admission-controller-status", true,
 		"If true, updater will only evict pods when admission controller status is valid.")
 
+	inPlaceSkipDisruptionBudget = flag.Bool(
+		"in-place-skip-disruption-budget",
+		false,
+		"If true, VPA updater skips disruption budget checks for in-place pod updates when all containers have NotRequired resize policy or no policy. "+
+			"Disruption budgets are still respected when any container has RestartContainer resize policy.",
+	)
+
 	namespace = os.Getenv("NAMESPACE")
 )
 
@@ -217,6 +224,7 @@ func run(healthCheck *metrics.HealthCheck, commonFlag *common.CommonFlags) {
 		*evictionRateBurst,
 		*evictionToleranceFraction,
 		*useAdmissionControllerStatus,
+		*inPlaceSkipDisruptionBudget,
 		admissionControllerStatusNamespace,
 		vpa_api_util.NewCappingRecommendationProcessor(limitRangeCalculator),
 		priority.NewScalingDirectionPodEvictionAdmission(),

vertical-pod-autoscaler/pkg/updater/restriction/pods_eviction_restriction_test.go

@@ -54,7 +54,7 @@ func TestEvictTooFewReplicas(t *testing.T) {
 	}
 
 	basicVpa := getBasicVpa()
-	factory, err := getRestrictionFactory(&rc, nil, nil, nil, 10, 0.5, nil, nil, nil)
+	factory, err := getRestrictionFactory(&rc, nil, nil, nil, 10, 0.5, nil, nil, nil, false)
 	assert.NoError(t, err)
 	creatorToSingleGroupStatsMap, podToReplicaCreatorMap, err := factory.GetCreatorMaps(pods, basicVpa)
 	assert.NoError(t, err)
@@ -94,7 +94,7 @@ func TestEvictionTolerance(t *testing.T) {
 	}
 
 	basicVpa := getBasicVpa()
-	factory, err := getRestrictionFactory(&rc, nil, nil, nil, 2 /*minReplicas*/, tolerance, nil, nil, nil)
+	factory, err := getRestrictionFactory(&rc, nil, nil, nil, 2 /*minReplicas*/, tolerance, nil, nil, nil, false)
 	assert.NoError(t, err)
 	creatorToSingleGroupStatsMap, podToReplicaCreatorMap, err := factory.GetCreatorMaps(pods, basicVpa)
 	assert.NoError(t, err)
@@ -138,7 +138,7 @@ func TestEvictAtLeastOne(t *testing.T) {
 	}
 
 	basicVpa := getBasicVpa()
-	factory, err := getRestrictionFactory(&rc, nil, nil, nil, 2, tolerance, nil, nil, nil)
+	factory, err := getRestrictionFactory(&rc, nil, nil, nil, 2, tolerance, nil, nil, nil, false)
 	assert.NoError(t, err)
 	creatorToSingleGroupStatsMap, podToReplicaCreatorMap, err := factory.GetCreatorMaps(pods, basicVpa)
 	assert.NoError(t, err)
@@ -230,7 +230,7 @@ func TestEvictEmitEvent(t *testing.T) {
 			pods = append(pods, p.pod)
 		}
 		clock := baseclocktest.NewFakeClock(time.Time{})
-		factory, err := getRestrictionFactory(&rc, nil, nil, nil, 2, testCase.evictionTolerance, clock, map[string]time.Time{}, nil)
+		factory, err := getRestrictionFactory(&rc, nil, nil, nil, 2, testCase.evictionTolerance, clock, map[string]time.Time{}, nil, false)
 		assert.NoError(t, err)
 		creatorToSingleGroupStatsMap, podToReplicaCreatorMap, err := factory.GetCreatorMaps(pods, testCase.vpa)
 		assert.NoError(t, err)
@@ -257,3 +257,45 @@ func TestEvictEmitEvent(t *testing.T) {
 		}
 	}
 }
+
+// This test ensures that in-place-skip-disruption-budget only affects in-place
+// updates and does not bypass eviction tolerance when performing pod evictions.
+func TestEvictTooFewReplicasWithInPlaceSkipDisruptionBudget(t *testing.T) {
+	replicas := int32(5)
+	livePods := 5
+
+	rc := apiv1.ReplicationController{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "rc",
+			Namespace: "default",
+		},
+		TypeMeta: metav1.TypeMeta{
+			Kind: "ReplicationController",
+		},
+		Spec: apiv1.ReplicationControllerSpec{
+			Replicas: &replicas,
+		},
+	}
+
+	pods := make([]*apiv1.Pod, livePods)
+	for i := range pods {
+		pods[i] = test.Pod().WithName(getTestPodName(i)).WithCreator(&rc.ObjectMeta, &rc.TypeMeta).Get()
+	}
+
+	basicVpa := getBasicVpa()
+	// factory with inPlaceSkipDisruptionBudget on
+	factory, err := getRestrictionFactory(&rc, nil, nil, nil, 10, 0.5, nil, nil, nil, true)
+	assert.NoError(t, err)
+	creatorToSingleGroupStatsMap, podToReplicaCreatorMap, err := factory.GetCreatorMaps(pods, basicVpa)
+	assert.NoError(t, err)
+	eviction := factory.NewPodsEvictionRestriction(creatorToSingleGroupStatsMap, podToReplicaCreatorMap)
+
+	for _, pod := range pods {
+		assert.False(t, eviction.CanEvict(pod))
+	}
+
+	for _, pod := range pods {
+		err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder())
+		assert.Error(t, err, "Error expected")
+	}
+}

vertical-pod-autoscaler/pkg/updater/restriction/pods_inplace_restriction.go

@@ -67,6 +67,7 @@ type PodsInPlaceRestrictionImpl struct {
 	patchCalculators             []patch.Calculator
 	clock                        clock.Clock
 	lastInPlaceAttemptTimeMap    map[string]time.Time
+	inPlaceSkipDisruptionBudget  bool
 }
 
 // CanInPlaceUpdate checks if pod can be safely updated
@@ -89,6 +90,13 @@ func (ip *PodsInPlaceRestrictionImpl) CanInPlaceUpdate(pod *apiv1.Pod) utils.InP
 				}
 				return utils.InPlaceDeferred
 			}
+			if ip.inPlaceSkipDisruptionBudget {
+				if utils.IsNonDisruptiveResize(pod) {
+					klog.V(4).InfoS("in-place-skip-disruption-budget enabled, skipping disruption budget check for in-place update")
+					return utils.InPlaceApproved
+				}
+				klog.V(4).InfoS("in-place-skip-disruption-budget enabled, but pod has RestartContainer resize policy", "pod", klog.KObj(pod))
+			}
 			if singleGroupStats.isPodDisruptable() {
 				return utils.InPlaceApproved
 			}