HPA: Use capabilites & align manifests.

[Gacko]

What this PR does / why we need it:

This PR provides a way to determine the correct apiVersion based on the clusters capabilities instead of a configurable value. Since the manifest, especially the .spec, works with autoscaling/v2beta2 and higher only, it does not make sense to give users the impression of being able to configure an older version anyway.

Additionally it implements annotations for the default backend's HPA. They exist in the values.yaml but are not used in the manifest.

Last but not least this PR aligns the templating between the controller's and the default backend's HPA.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• CVE Report (Scanner found CVE and adding report)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation only

Which issue/s this PR fixes

How Has This Been Tested?

Checklist:

• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I've read the CONTRIBUTION guide
• I have added unit and/or e2e tests to cover my changes.
• All new and existing tests passed.
• Added Release Notes.

Does my pull request need a release note?

Any user-visible or operator-visible change qualifies for a release note. This could be a:

• CLI change
• API change
• UI change
• configuration schema change
• behavioral change
• change in non-functional attributes such as efficiency or availability, availability of a new platform
• a warning about a deprecation
• fix of a previous Known Issue
• fix of a vulnerability (CVE)

No release notes are required for changes to the following:

• Tests
• Build infrastructure
• Fixes for unreleased bugs

For more tips on writing good release notes, check out the Release Notes Handbook

HPA: Use capabilites & align manifests.

[k8s-ci-robot]

@Gacko: This issue is currently awaiting triage.

If Ingress contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[k8s-ci-robot]

Hi @Gacko. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[tao12345666333]

Thanks for your contribution.

It is indeed possible to reduce a configuration item, but it will be a breaking change.
cc @rikatz @strongjz @longwuyuan

/hold

[Gacko]

Would it be better / non-breaking if the configuration is just optional and we set the default to empty string?

[Gacko]

@strongjz @rikatz: Would you mind reviewing and eventually merging this? :)

[Gacko]

@strongjz Since you just merged this, would this PR also be eligible for merging? 😉

[strongjz]

It's on my watch list. CI is running.

[Gacko]

Hm, CI fails due to timeouts, right? At least the failing tests do not seem to be related to my changes.

Anyway: Thank you very much!

[Gacko]

CI is flaky af... 😑

[tao12345666333]

/retest

[olyhuta]

Hi
Is it in progress still?
Can somebody provide please a little bit more clarity. I tried to change manually version here https://github.com/kubernetes/ingress-nginx/blob/main/charts/ingress-nginx/values.yaml#L329 to the older autoscaling/v2beta2 but it hasn't been deployed and changes didn't take effect, so wondering if v2 version is the only version that can be deployed via helm

[Gacko]

Hey @olyhuta!

This PR is not merged, yet. The current main or latest release is still using the configurable API version. Your API server might store it as v2 if you create it as v2beta2. IIRC v2beta1 has already been discontinued. Anyway: The way the HPA manifest looks right now, it could possibly already be incompatible with v2beta2...

v2beta2 uses targetAverageUtilization while v2 uses a separate target object.

Which version are you referencing to? There has been a change on main recently, so as long as you're referring to a release and not to main you should be safe.

Kind regards
Marco

[olyhuta]

Hey @Gacko , gotcha, thank you ✋ Yeah, me bad, made some mistakes during deployment so it didn't pick up changes,
but sorted it out, trying to reference to v2beta2 so it workable solution for me at this moment

[cpanato]

retrigger the failed jobs, if that fails again I would say to rebase this PR to get a fresh code from main

lgtm

[Gacko]

Rebased.

[cpanato]

looks good
/lgtm

[Gacko]

@cpanato Can I simply rebase my PR(s) to include latest changes from main or does this rather interfere with your approval and merging process?

[cpanato]

@cpanato Can I simply rebase my PR(s) to include latest changes from main or does this rather interfere with your approval and merging process?

it removes the lgtm, thats is ok is part of the process

[Gacko]

@cpanato Could you have a look here? The PR is up-to-date and tests are green. :)

[strongjz]

/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cpanato, Gacko, strongjz

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:

• charts/ingress-nginx/OWNERS [cpanato,strongjz]

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

[cpanato]

/unhold