🐛 remove webhooks for old APIversion v1alpha4 #1240

bavarianbidi · 2022-05-09T13:55:36Z

This PR removes webhooks for old v1alpha4 apiVersions as the new default version is already set to v1alpha5.

This is done the same way how other Infrastructureproviders have implemented it (e.g. CAPA)

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #1238

Special notes for your reviewer:

i just take a look how CAPA is doing it. They only have one webhooks for the current apiVersion. For older apiVersions they don't have any webhook anymore:

Mainbranch - v1beta1: https://github.com/kubernetes-sigs/cluster-api-provider-aws/tree/main/api/v1beta1
Mainbranch - v1alpha4: https://github.com/kubernetes-sigs/cluster-api-provider-aws/tree/main/api/v1alpha4

Commit: kubernetes-sigs/cluster-api-provider-aws@4274a5a?diff=split

TODOs:

squashed commits
if necessary:
- includes documentation
- adds unit tests

/hold

linux-foundation-easycla · 2022-05-09T13:55:39Z

The committers listed above are authorized under a signed CLA.

✅ login: bavarianbidi / name: Mario Constanti (5502f11, fd58e36)

netlify · 2022-05-09T13:55:41Z

✅ Deploy Preview for kubernetes-sigs-cluster-api-openstack ready!

Name	Link
🔨 Latest commit	`51fdcd9`
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-cluster-api-openstack/deploys/627a3ebc6a761000080aec30
😎 Deploy Preview	https://deploy-preview-1240--kubernetes-sigs-cluster-api-openstack.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

k8s-ci-robot · 2022-05-09T13:55:45Z

Welcome @bavarianbidi!

It looks like this is your first PR to kubernetes-sigs/cluster-api-provider-openstack 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/cluster-api-provider-openstack has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2022-05-09T13:55:45Z

Hi @bavarianbidi. Thanks for your PR.

I'm waiting for a kubernetes-sigs 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.

tobiasgiese · 2022-05-09T13:56:12Z

/ok-to-test

api/v1alpha4/openstackcluster_webhook.go

tobiasgiese · 2022-05-10T07:20:30Z

The test is failing because of

api/v1alpha4/identity_types.go:19:7: `defaultIdentityRefKind` is unused (deadcode)
const defaultIdentityRefKind = "Secret"

mdbooth · 2022-05-10T09:11:59Z

@bavarianbidi @tobiasgiese I wonder if we should pre-emptively change the v1alpha5 names to match. I say 'I wonder' because I genuinely don't know. Going to look at a couple of other projects to see what they do.

mdbooth · 2022-05-10T10:21:24Z

@bavarianbidi @tobiasgiese I wonder if we should pre-emptively change the v1alpha5 names to match. I say 'I wonder' because I genuinely don't know. Going to look at a couple of other projects to see what they do.

... and now I see the previous discussion and you're way ahead of me 🤦 Oh well, it was interesting reading!

mdbooth · 2022-05-10T10:22:35Z

/approve

k8s-ci-robot · 2022-05-10T10:22:46Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bavarianbidi, mdbooth

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:

~~OWNERS~~ [mdbooth]

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

tobiasgiese · 2022-05-10T10:24:50Z

Would you squash the commits before merging @bavarianbidi? I don't think it's worth having them separately

* as we already have webhooks for v1alpha5 we don't need webhooks for older versions anymore. * remove v1alpha4 from webhook mgr setup * remove defaultIdentityRefKind const in v1alpha4 Signed-off-by: Mario Constanti <[email protected]>

tobiasgiese · 2022-05-10T10:37:11Z

/lgtm
/hold cancel

Thanks for contributing @bavarianbidi 🙂 👍🏻

mdbooth · 2022-05-10T10:39:19Z

/cherry-pick release-0.6

k8s-infra-cherrypick-robot · 2022-05-10T10:39:20Z

@mdbooth: once the present PR merges, I will cherry-pick it on top of release-0.6 in a new PR and assign it to you.

In response to this:

/cherry-pick release-0.6

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-infra-cherrypick-robot · 2022-05-10T12:00:24Z

@mdbooth: new pull request created: #1241

In response to this:

/cherry-pick release-0.6

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 added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 9, 2022

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label May 9, 2022

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label May 9, 2022

k8s-ci-robot requested review from apricote and mdbooth May 9, 2022 13:55

k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label May 9, 2022

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 9, 2022

bavarianbidi force-pushed the webhook_name_renaming branch from 528ae1d to bb648d4 Compare May 9, 2022 14:01

jichenjc reviewed May 10, 2022

View reviewed changes

api/v1alpha4/openstackcluster_webhook.go Outdated Show resolved Hide resolved

bavarianbidi force-pushed the webhook_name_renaming branch from bb648d4 to 73ce44d Compare May 10, 2022 04:42

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 10, 2022

bavarianbidi force-pushed the webhook_name_renaming branch from 0108db3 to fd58e36 Compare May 10, 2022 04:57

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels May 10, 2022

bavarianbidi changed the title ~~🐛 support mulitple webhook config per capo versions~~ 🐛 remove webhooks for old APIversion v1alpha4 May 10, 2022

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 10, 2022

remove v1alpha4 webhooks

51fdcd9

* as we already have webhooks for v1alpha5 we don't need webhooks for older versions anymore. * remove v1alpha4 from webhook mgr setup * remove defaultIdentityRefKind const in v1alpha4 Signed-off-by: Mario Constanti <[email protected]>

bavarianbidi force-pushed the webhook_name_renaming branch from fd19b3a to 51fdcd9 Compare May 10, 2022 10:30

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 10, 2022

k8s-ci-robot assigned tobiasgiese May 10, 2022

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 10, 2022

k8s-ci-robot merged commit eb8ad43 into kubernetes-sigs:main May 10, 2022

k8s-infra-cherrypick-robot mentioned this pull request May 10, 2022

[release-0.6] 🐛 remove webhooks for old APIversion v1alpha4 #1241

Merged

🐛 remove webhooks for old APIversion v1alpha4 #1240

🐛 remove webhooks for old APIversion v1alpha4 #1240

Uh oh!

Conversation

bavarianbidi commented May 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

linux-foundation-easycla bot commented May 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netlify bot commented May 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for kubernetes-sigs-cluster-api-openstack ready!

Uh oh!

k8s-ci-robot commented May 9, 2022

Uh oh!

k8s-ci-robot commented May 9, 2022

Uh oh!

tobiasgiese commented May 9, 2022

Uh oh!

Uh oh!

tobiasgiese commented May 10, 2022

Uh oh!

mdbooth commented May 10, 2022

Uh oh!

mdbooth commented May 10, 2022

Uh oh!

mdbooth commented May 10, 2022

Uh oh!

k8s-ci-robot commented May 10, 2022

Uh oh!

tobiasgiese commented May 10, 2022

Uh oh!

tobiasgiese commented May 10, 2022

Uh oh!

mdbooth commented May 10, 2022

Uh oh!

k8s-infra-cherrypick-robot commented May 10, 2022

Uh oh!

k8s-infra-cherrypick-robot commented May 10, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

bavarianbidi commented May 9, 2022 •

edited

Loading

linux-foundation-easycla bot commented May 9, 2022 •

edited

Loading

netlify bot commented May 9, 2022 •

edited

Loading