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

Update API versions of HorizontalPodAutoscaler and PodDisruptionBudget #1287

Merged
merged 2 commits into from
Jan 26, 2023

Conversation

creydr
Copy link
Contributor

@creydr creydr commented Jan 20, 2023

The nightly release job has some issues with deprecated APIs on upgrade jobs:

Jan 20 10:02:44.846 install_released_consolidated_channel [OUT] Applying: /logs/artifacts/channel-consolidated-knative-v1.8.1.yaml
...
Jan 20 10:02:50.925 install_released_consolidated_channel [ERR] Warning: autoscaling/v2beta2 HorizontalPodAutoscaler is deprecated in v1.23+, unavailable in v1.26+; use autoscaling/v2 HorizontalPodAutoscaler
Jan 20 10:02:51.081 install_released_consolidated_channel [OUT] horizontalpodautoscaler.autoscaling/kafka-webhook created
...
Jan 20 10:02:51.298 install_released_consolidated_channel [ERR] error: resource mapping not found for name: "kafka-webhook" namespace: "knative-eventing" from "/logs/artifacts/channel-consolidated-knative-v1.8.1.yaml": no matches for kind "PodDisruptionBudget" in version "policy/v1beta1"
Jan 20 10:02:51.298 install_released_consolidated_channel [ERR] ensure CRDs are installed first
    shell.go:28: exit status 1

This PR addresses it and updates the API versions of HorizontalPodAutoscaler to autoscaling/v2 (d704427) and PodDisruptionBudget to policy/v1 (c10f2ff).
This will require to backport this PR for a few releases to make the upgrade tests work.

Deprecation guide for 1.25: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#horizontalpodautoscaler-v125

Release Note

Update HorizontalPodAutoscaler API version to autoscaling/v2
Update PodDisruptionBudget API version to policy/v1

Signed-off-by: Christoph Stäbler <cstabler@redhat.com>
Signed-off-by: Christoph Stäbler <cstabler@redhat.com>
@knative-prow knative-prow bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jan 20, 2023
@codecov
Copy link

codecov bot commented Jan 20, 2023

Codecov Report

Merging #1287 (a52514b) into main (20e1b33) will not change coverage.
The diff coverage is n/a.

❗ Current head a52514b differs from pull request most recent head d704427. Consider uploading reports for the commit d704427 to get more accurate results

@@           Coverage Diff           @@
##             main    #1287   +/-   ##
=======================================
  Coverage   74.93%   74.93%           
=======================================
  Files         128      128           
  Lines        9017     9017           
=======================================
  Hits         6757     6757           
  Misses       2029     2029           
  Partials      231      231           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@aliok aliok self-assigned this Jan 23, 2023
@aliok
Copy link
Member

aliok commented Jan 23, 2023

@creydr I was working on the same issue... I eventually gave up :)

See #1278

In summary, in previous release, we use PodDisruptionBudget v1beta1 and upgrade tests will fail during the installation of the previous version of eventing-kafka.

[ERR] error: resource mapping not found for name: "kafka-webhook" namespace: "knative-eventing" from "/logs/artifacts/channel-consolidated-knative-v1.8.1.yaml": no matches for kind "PodDisruptionBudget" in version "policy/v1beta1"

Proper fix would be, fixing PodDisruptionBudget version all the way back to 1.5/1.6.

What we talked in Knative Slack is, that we will remove the KafkaChannel in this repository and problem will be solved as that's the only component using PodDisruptionBudgets.

@creydr creydr mentioned this pull request Jan 23, 2023
@matzew
Copy link
Contributor

matzew commented Jan 25, 2023

Proper fix would be, fixing PodDisruptionBudget version all the way back to 1.5/1.6.

yeah, all the way back, or to at least (main -1) 😄

@pierDipi
Copy link
Member

pierDipi commented Jan 25, 2023

We can disable the upgrade test in this repository

creydr added a commit to creydr/knative-eventing-kafka that referenced this pull request Jan 25, 2023
As we currently have some issues with upgrade test, we disable them. See knative-extensions#1287 (comment)
for details.

Signed-off-by: Christoph Stäbler <cstabler@redhat.com>
@knative-prow knative-prow bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 25, 2023
@creydr
Copy link
Contributor Author

creydr commented Jan 25, 2023

/assign @pierDipi @matzew @aliok

@knative-prow knative-prow bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 25, 2023
@knative-prow
Copy link

knative-prow bot commented Jan 25, 2023

@creydr: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
upgrade-tests_eventing-kafka_main d704427 link true /test upgrade-tests

Your PR dashboard.

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. I understand the commands that are listed here.

@pierDipi
Copy link
Member

/lgtm

@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jan 25, 2023
@pierDipi
Copy link
Member

/approve

@knative-prow
Copy link

knative-prow bot commented Jan 26, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: creydr, pierDipi

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

@knative-prow knative-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 26, 2023
@knative-prow knative-prow bot merged commit 53e9c07 into knative-extensions:main Jan 26, 2023
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. lgtm Indicates that a PR is ready to be merged. 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.

None yet

4 participants