[VPA] Allow in-place updates when replica group is below minReplicas (InPlaceOrRecreate); block eviction fallback

[ortherion]

Which component are you using?:
/area vertical-pod-autoscaler

Is your feature request designed to solve a problem? If so describe the problem this feature should solve.:

With VPA updateMode: InPlaceOrRecreate, replica groups that have fewer live pods than minReplicas (e.g. a single pod when --min-replicas=2) never get recommendations applied. The updater skips such groups in GetCreatorMaps (if actual < required { continue }), so they are not in the maps and neither in-place nor eviction is considered. Users with single-replica or small replica sets cannot receive VPA updates without lowering minReplicas or increasing replica count, even though in-place resize does not reduce the number of pods and would be safe.

Describe the solution you'd like.:

• When livePods < minReplicas, still include the replica group in the updater maps and mark it with a flag (e.g. belowMinReplicas in group stats).
• Eviction (recreate): keep it blocked for groups below minReplicas — in the eviction path (e.g. PodsEvictionRestrictionImpl.CanEvict), return false when the group is below minReplicas and log it.
• In-place: allow in-place resize for these groups — the in-place restriction logic does not consider the “below minReplicas” flag, so in-place updates can proceed when the node supports it.
• If in-place is not possible (e.g. resize Infeasible), do not fall back to eviction when below minReplicas; only log and skip.

Result: with InPlaceOrRecreate, single-replica and small replica sets can receive VPA recommendations only via in-place resize; eviction/recreate remains forbidden when below minReplicas.

Describe any alternative solutions you've considered.:

• Removing the actual < required check entirely: would allow eviction for groups below minReplicas and reduce availability, so rejected.
• Per-VPA minReplicas=1 for singletons: users can set spec.updatePolicy.minReplicas: 1 today, but then eviction would also be allowed for that one pod; the goal is to allow only in-place for below-minReplicas groups, not eviction.
• New update mode (e.g. “InPlaceOnly”): would add API surface and overlap with “allow in-place below minReplicas for InPlaceOrRecreate”; the proposed change is a small, backward-compatible fix in updater restriction logic.

Additional context.:

• Affected code: vertical-pod-autoscaler/pkg/updater/restriction/ (factory, eviction restriction, in-place restriction).
• In-place resize does not change the number of pods, so allowing it below minReplicas does not reduce availability; only eviction/recreate needs to stay blocked.
• Implementation adds a boolean (e.g. belowMinReplicas) to group stats, set in GetCreatorMaps when actual < required, and checked only in the eviction path.

[adrianmoisey]

Thanks for the issue.

My concern with this is that in-place updating a pod isn't guaranteed to be without disruption. It's possible that decreasing a pod's memory limit could cause it to OOM.

I think if we did accept a feature like this, it would need to be an opt-in from the user, and not the default.

We do have InPlace coming soon (targeting VPA 1.6 or 1.7), would the combination of InPlace plus minReplicas = 1 be the same thing?

The only limitation I can think of is if the pod is controlled with an HPA that could scale to zero, but in that scenario I'd expect a short disruption is OK (ie: InPlaceOrRecrate + minReplicas = 1 should be sufficient)

/cc @maxcao13 @omerap12

[adrianmoisey]

Oh, we fixed this in #8987

[maxcao13]

I actually don't think that PR that omer put in does skip minReplicas, but originally in my mind I envisioned overriding minReplicas would fit as a use case for that flag - I think we might have just missed it. Though I am only assuming from taking a quick look at the code, I haven't actually tested this. The minReplicas check in the GetCreatorMaps function, which is a bit earlier in the logic than where inplacedisruptionbudgetcheck occurs:

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

Line 215 in daa94fb

klog.V(2).InfoS("Too few replicas", "kind", creator.Kind, "object", klog.KRef(creator.Namespace, creator.Name), "livePods", actual, "requiredPods", required, "globalMinReplicas", f.minReplicas)

But if we didn't fix it in that PR, I agree we should adjust the flag to skip minReplica checks.

[adrianmoisey]

Dang, you're right. I read the doc and didn't look a the code.
Let's get that updated
/triage accepted

[omerap12]

Yeah, we probably missed this. I didn’t test it, but after reading the code I’m pretty sure we did.
I think we can just do:

for creator, replicas := range livePods {
	actual := len(replicas)
	if actual < required && !f.inPlaceSkipDisruptionBudget { // this is the diff
		klog.V(2).InfoS("Too few replicas",
			"kind", creator.Kind,
			"object", klog.KRef(creator.Namespace, creator.Name),
			"livePods", actual,
			"requiredPods", required,
			"globalMinReplicas", f.minReplicas,
		)
		continue
	}
}

This should fix the problem.
Later we already have logic to decide if we really need to skip these pods (when all containers have NotRequired resize policy):
https://github.com/kubernetes/autoscaler/blob/master/vertical-pod-autoscaler/pkg/updater/restriction/pods_inplace_restriction.go#L94-L98

[adrianmoisey]

Cool, I can handle this quickly, since I'd like to get this fixed before VPA 1.6 (which I really want to get out asap)
/assign