馃悰 CABPK: Remove redundant SetupWebhookWithManager funcs on list types #5725
Conversation
k8s-ci-robot
added
cncf-cla: yes
sbueringer
force-pushed
the
pr-deprecate-list-setup-funcs
branch
from
4303dd5
to
ff248dd
Compare
| @@ -20,6 +20,8 @@ import ( | |||
| ctrl "sigs.k8s.io/controller-runtime" | |||
| ) | |||
|
|
|||
| // SetupWebhookWithManager sets up KubeadmConfigList webhooks. | |||
| // Deprecated: This method is going to be removed in a next release. | |||
There was a problem hiding this comment.
Q: if I remember well we recently implemented a check to ensure all the webhooks exist; Have checked if this deprecation conflates with this check?
Also given that this is golang code and not API types (CRD), can we queue up this for 1.1?
There was a problem hiding this comment.
Q: if I remember well we recently implemented a check to ensure all the webhooks exist; Have checked if this deprecation conflates with this check?
The check verifies that the types implement the conversion interface correct (aka they have ConvertTo/ConvertFrom/Hub funcs). So they will still pass after this func is dropped, in fact for all other *List types these functions don't exist right now.
Also given that this is golang code and not API types (CRD), can we queue up this for 1.1?
I think based on our policies it should be okay to deprecate those funcs with v1.1.0 and drop them with v1.2.0. If I got it correctly we have to deprecate with a .0 release (because deprecations are not cherry-pickable) so we can drop them with the next following minor release.
Are you suggesting to deprecating later or removing earlier?
I'm not sure I understood the question correctly.
There was a problem hiding this comment.
I'd remove these now given that these are mostly for internal controller management and they don't do much
There was a problem hiding this comment.
Fine for me. Everyone who might use them can essentially just stop doing it with v1.1 without any downsides.
There was a problem hiding this comment.
Updated the PR accordingly
Signed-off-by: Stefan B眉ringer buringerst@vmware.com
sbueringer
changed the title
sbueringer
force-pushed
the
pr-deprecate-list-setup-funcs
branch
from
ff248dd
to
df4cfd8
Compare
k8s-ci-robot
added
size/M
|
@sbueringer: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fabriziopandini 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 |
k8s-ci-robot
added
the
approved
Signed-off-by: Stefan B眉ringer buringerst@vmware.com
What this PR does / why we need it:
This PR deprecates the redundant
SetupWebhookWithManagerfuncs on the CABPK list types. We don't have them for any other of our*Listtypes. We don't need them because: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 #