[release-0.9] 🌱 Move webhooks into pkg/webhooks #2612
Conversation
|
Skipping CI for Draft Pull Request. |
k8s-ci-robot
added
do-not-merge/work-in-progress
✅ Deploy Preview for kubernetes-sigs-cluster-api-openstack ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/XXL
|
fwiw, this backport should allow us to address an issue that is currently blocking work on the migration path from MAPO to CAPO in OpenShift in OpenShift. While I appreciate this is primarily for downstream, I'm proposing it upstream as I hope it should introduce minimal risk while allowing us/me to stay as close to upstream as possible (always my preferred approach, personally). I'm happy to discuss why we need this (tl;dr: MAPO is stuck with CAPO v0.9.x for $reasons) and build a case for doing this here, as needed, though if we really don't want to do this here I'd understand. |
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
|
(marking as ready-for-review with a hold to get CI results) |
|
anything that helps an active downstream get closer to upstream is something I am in favour of :) |
stephenfin
force-pushed
the
webhooks-0.9
branch
3 times, most recently
from
6e5f0d9 to
d645938
Compare
|
I looks like the problems with the CI are due to a mismatch of versions between go and golangci-lint. By the look of https://storage.googleapis.com/kubernetes-ci-logs/pr-logs/pull/kubernetes-sigs_cluster-a[…]-provider-openstack-test/1938281313530286080/prowjob.json it seems the job uses gcr.io/k8s-staging-test-infra/kubekins-e2e:v20250613-876fb90a97-master with go 1.24. But golangci-lint is at version v1.54.2 which is not compatible with go 1.24. I wonder if bumping it to 1.64 (first version to support go 1.24) helps. |
I tried bumping golangci-lint with #2619 and it's now effectively showing different errors, with |
|
/test pull-cluster-api-provider-openstack-e2e-test |
mandre
force-pushed
the
webhooks-0.9
branch
2 times, most recently
from
ffd5751 to
b41a83a
Compare
Moves webhooks from api to pkg/webhooks making only mechanical code changes except for the removal of the defaulting webhooks, because they weren't used. This results in there now being no mutating webhook configured. NOTE(stephenfin): There were a lot of conflicts here. These were mostly mitigated by faking the addition of v1alpha8, which moved the webhooks to the 'api/v1alpha8' package (commit 750b84d), followed by the subsequent rename of this package to v1beta1 (commit e9fb53c), for the webhook files and tests. This still resulted in some merge conflicts due the v1alpha8 changes such as 564b6bd and 4368c4f (which we obviously don't want to include here) but it made the backport much simpler. NOTE(stephenfin): We also include commit 30ba121 which was a follow-up that fixed the CRD generation broken in this commit. Signed-off-by: Stephen Finucane <stephenfin@redhat.com> (cherry picked from commit 750b84d)
stephenfin
force-pushed
the
webhooks-0.9
branch
from
b41a83a to
cb3d212
Compare
The conversion webhook fails to register without this.
Conflicts:
api/v1alpha7/conversion.go
NOTE(stephenfin): Conflicts are due to absence of v1beta1 API on this
branch. We also need to `s/v1beta1/v1alpha7/` on the other files to
handle this.
(cherry picked from commit cb09d5f)
stephenfin
force-pushed
the
webhooks-0.9
branch
from
257df7f to
614bc67
Compare
|
/retest |
|
It failed to properly bring up the worker nodes so the multiple AZs test failed. Trying again. |
|
Hurrah! |
|
/unhold This is ready for review now. |
k8s-ci-robot
removed
the
do-not-merge/hold
|
/cc |
lentzi90
left a comment
lentzi90
left a comment
There was a problem hiding this comment.
I didn't spend too much time on this, considering that it is an older branch.
It looks good to me, no objections
/approve
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lentzi90 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
k8s-ci-robot
added
the
lgtm
6 participants
This is a manual backport of #1920.
/hold