AEP-8026: per-vpa-component-configuration

[omerap12]

What type of PR is this?

/kind documentation
/kind feature
/kind api-change

What this PR does / why we need it:

AEP for #7650

[omerap12]

/assign @raywainman

[k8s-triage-robot]

This PR may require API review.

If so, when the changes are ready, complete the pre-review checklist and request an API review.

Status of requested reviews is tracked in the API Review project.

[raywainman]

I really like this @omerap12, thanks for putting this together!

[raywainman]

Will we still do some basic validating in the code?

[omerap12]

Yeah, e2e tests will be included: 5b89607

[adrianmoisey]

Do these API changes need to be feature-gated and go through the process of:

1. Default off
2. Default on
3. GA

I don't think the VPA project really promises a downgrade path, but Kubernetes does. If we use what Kubernetes has defined, then we may need to do this.

[omerap12]

Do these API changes need to be feature-gated and go through the process of:

1. Default off
2. Default on
3. GA

I don't think the VPA project really promises a downgrade path, but Kubernetes does. If we use what Kubernetes has defined, then we may need to do this.

I thought you are on vacation!
But I don't think we need to do the all feature gate process , maybe I'm wrong here..
@raywainman @voelzmo wdyt?

[adrianmoisey]

I thought you are on vacation!

I'm easing back into open source over the next week or so, and playing catch up.

But I don't think we need to do the all feature gate process , maybe I'm wrong here..

I think it depends on what our promise is to the user. If we promise the ability to downgrade by 1 version, then we need to do the feature-gating
If we don't promise anything, then we're good and don't need to feature-gate.

In the past it seems as though we loosely adopt what Kubernetes promises (ability to downgrade), but that doesn't seem to be a formal decision.

[omerap12]

I think it depends on what our promise is to the user. If we promise the ability to downgrade by 1 version, then we need to do the feature-gating
If we don't promise anything, then we're good and don't need to feature-gate.

What does downgrade means in this context? cause you are just adding fields..

[adrianmoisey]

I think it depends on what our promise is to the user. If we promise the ability to downgrade by 1 version, then we need to do the feature-gating
If we don't promise anything, then we're good and don't need to feature-gate.

What does downgrade means in this context? cause you are just adding fields..

Something like this is what I'm thinking:

1. Someone upgrades the VPA to a version including this feature (CRDs and all components upgraded)
2. They then use one of these fields for a VPA
3. They downgrade. I assume the CRD is not downgraded... only a VPA component, to a version that doesn't support the new feature

Their previously-set VPA setting (oomBumpUpRatio, for example) no longer works.

It's not a big deal, but it's how Kubernetes rolls out new features.

[omerap12]

I think it depends on what our promise is to the user. If we promise the ability to downgrade by 1 version, then we need to do the feature-gating
If we don't promise anything, then we're good and don't need to feature-gate.

What does downgrade means in this context? cause you are just adding fields..

Something like this is what I'm thinking:

1. Someone upgrades the VPA to a version including this feature (CRDs and all components upgraded)
2. They then use one of these fields for a VPA
3. They downgrade. I assume the CRD is not downgraded... only a VPA component, to a version that doesn't support the new feature

Their previously-set VPA setting (oomBumpUpRatio, for example) no longer works.

It's not a big deal, but it's how Kubernetes rolls out new features.

I understand. Right now, if someone updates the VPA and sets a different oomBumpUpRatio than the global setting, and then later downgrades, it will go back to the global value.
I can add a feature gate if you think it's needed.

[adrianmoisey]

I understand. Right now, if someone updates the VPA and sets a different oomBumpUpRatio than the global setting, and then later downgrades, it will go back to the global value.
I can add a feature gate if you think it's needed.

Yeah, the downgrade isn't a big problem with this change.
I think the big question is if we want to align with what Kubernetes does, or if we're happy with not supporting a super clean downgrade.

I think the VPA is a lower risk than Kubernetes, so we could get away not worrying about a downgrade, but, someone may expect the same from the VPA as they do from Kubernetes. So I'm on the fence with this one.
I've added an item to the sig-autoscaling meeting (for the 12th May) to discuss it

[omerap12]

@adrianmoisey PTAL :)

[adrianmoisey]

/lgtm
/hold

I'd really love more eyes on this, hence holding it

[omerap12]

cc @kubernetes/sig-autoscaling-leads
For more context: #7650

[raywainman]

I'm curious why you want to expose this and not also expose the count?

Do we just assume that 8 intervals is good and we can just adjust the time interval instead to suit the need?

I'm curious how much success you've had in tweaking this in your own deployment.

[omerap12]

There's no specific reason — the list above just includes what I thought were the most useful fields to expose. I can also expose the count field since it seems straightforward. We'll continue exposing new fields based on user requirements, as mentioned here: https://github.com/kubernetes/autoscaler/pull/8026/files#diff-b666c5599d02971edb0c08090acb86761b7c726c97c4c6c1bbf00ff5883a2364R190

[raywainman]

Overall LGTM, just a couple comments. Apologies for the delay here team...

[adrianmoisey]

/lgtm

[raywainman]

/lgtm

Modulo the discussion on Quantity vs String for the OomBumpUpRatio which can be figured out during implementation.

[adrianmoisey]

/lgtm

[omerap12]

Ping @raywainman for final approval.

[raywainman]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: omerap12, raywainman

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• vertical-pod-autoscaler/enhancements/OWNERS [raywainman]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[omerap12]

/unhold
Thanks folks!