Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump HorizontalPodAutoscaler apiVersion to v2 #5130

Merged
merged 1 commit into from
Oct 31, 2022

Conversation

vdemeester
Copy link
Member

@vdemeester vdemeester commented Jul 13, 2022

Changes

Before this, we get a warning when applying the HPA:

Warning: autoscaling/v2beta1 HorizontalPodAutoscaler is deprecated in v1.22+, unavailable in v1.25+; use autoscaling/v2 HorizontalPodAutoscaler

Signed-off-by: Vincent Demeester vdemeest@redhat.com

Closes #5128
Takes over #4896

/cc @afrittoli @abayer @imjasonh @lbernick @jerop @pritidesai

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

  • Has Docs included if any changes are user facing
  • Has Tests included if any functionality added or changed
  • Follows the commit message standard
  • Meets the Tekton contributor standards (including
    functionality, content, code)
  • Has a kind label. You can add one by adding a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
  • Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings)
  • Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

Webhook HPA uses autoscaling/v2 instead of the deprecated autoscaling/v2beta1. This also brings the minimum kubernetes version to v1.23.0

@tekton-robot tekton-robot requested a review from abayer July 13, 2022 07:52
@tekton-robot tekton-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Jul 13, 2022
@tekton-robot tekton-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jul 13, 2022
Copy link
Member

@afrittoli afrittoli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve

@tekton-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: afrittoli

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:

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

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 13, 2022
@vdemeester
Copy link
Member Author

/kind misc

@tekton-robot tekton-robot added the kind/misc Categorizes issue or PR as a miscellaneuous one. label Jul 13, 2022
@vdemeester
Copy link
Member Author

vdemeester commented Jul 13, 2022

Alright, this fails because apiVersion: autoscaling/v2, kind: HorizontalPodAutoscaler is available starting with 1.23. So v2beta1 is, weirdly enough, deprecated in 1.22+ but the v2 is only available in 1.23.

Bumping to autoscaling/v2 would effectively mean we require a minimum version of kubernetes to be 1.23. We, most likely, will have to do this before or at the same time as kubernetes 1.25 goes out or is "widely" available.

/cc @imjasonh @afrittoli @abayer @tektoncd/core-maintainers

See https://kubernetes.io/docs/tasks/run-application/horizontal-pod-autoscale-walkthrough/ and https://kubernetes.io/docs/tasks/run-application/horizontal-pod-autoscale/

@tekton-robot tekton-robot requested review from afrittoli and a team July 13, 2022 09:24
@vdemeester
Copy link
Member Author

/hold

@tekton-robot tekton-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 13, 2022
@vdemeester
Copy link
Member Author

/retest

1 similar comment
@abayer
Copy link
Contributor

abayer commented Aug 23, 2022

/retest

@dibyom
Copy link
Member

dibyom commented Oct 4, 2022

/retest

@vdemeester
Copy link
Member Author

Alright, so I was wrong, it's not 1.22 that is required, but 1.23... 😅

@vdemeester
Copy link
Member Author

In a gist, we need to bump our minimal k8s version to 1.23 to be able to support 1.25 🙃

@tekton-robot tekton-robot removed the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Oct 5, 2022
@tekton-robot tekton-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Oct 5, 2022
@vdemeester
Copy link
Member Author

It does require tektoncd/plumbing#1238 🙃

@dibyom
Copy link
Member

dibyom commented Oct 5, 2022

In a gist, we need to bump our minimal k8s version to 1.23 to be able to support 1.25 🙃

1.22 EOL is coming up on 2022-10-28 so bumping min to 1.23 makes sense 👍

https://kubernetes.io/releases/#release-v1-22

@vdemeester
Copy link
Member Author

/retest
I think kind should use 1.23 now 🙃

Comment on lines 121 to 122
- name: KUBERNETES_MIN_VERSION
value: v1.23.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that main is on min version 1.23 already, this is not needed anymore.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed 👼🏼

Comment on lines 110 to 111
- name: KUBERNETES_MIN_VERSION
value: v1.23.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed 👼🏼

@abayer
Copy link
Contributor

abayer commented Oct 30, 2022

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 30, 2022
Before this, we get a warning when applying the HPA:

    Warning: autoscaling/v2beta1 HorizontalPodAutoscaler is deprecated in v1.22+, unavailable in v1.25+; use autoscaling/v2 HorizontalPodAutoscaler

This also bumps the min version to 1.23.

Signed-off-by: Vincent Demeester <vdemeest@redhat.com>
@tekton-robot tekton-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 31, 2022
@vdemeester
Copy link
Member Author

/hold cancel

@tekton-robot tekton-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 31, 2022
Copy link
Member

@afrittoli afrittoli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 31, 2022
@tekton-robot tekton-robot merged commit e38d112 into tektoncd:main Oct 31, 2022
@vdemeester vdemeester deleted the 5128-hpa branch October 31, 2022 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/misc Categorizes issue or PR as a miscellaneuous one. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HorizontalPodAutoscaler v2beta1 is deprecated in v1.22+
5 participants