⚠️ Run webhooks with managers #3985

fabriziopandini · 2020-12-03T18:39:13Z

What this PR does / why we need it:
Given that #3042 for v1alpha4 requires single controllers managing all namespaces, this PR moves the webhooks back in the main manager and run them behind RBAC in case we'll need a client in the future.

Which issue(s) this PR fixes:
Fixes #3822

fabriziopandini · 2020-12-03T18:40:41Z

/milestone v0.4.0

fabriziopandini · 2020-12-03T18:42:37Z

/hold
while doing some more testing

detiber · 2020-12-03T20:17:16Z

bootstrap/kubeadm/config/default/kustomizeconfig.yaml

+# This configuration is for teaching kustomize how to update name ref and var substitution
+varReference:
+- kind: Deployment
+  path: spec/template/spec/volumes/secret/secretName


Is this actually needed? I feel like this was something that we previously used to hack together the separate manager and webhook deployments. I'm not seeing this in the upstream kubuebuilder examples: https://github.com/kubernetes-sigs/kubebuilder/tree/63b35350fe8b4d133c0bdb8fdd9a9a61fe30ac61/testdata/project-v3/config/default

This is not in the kubuilder example, but I felt that we are on the safe path if we ensure all the webhook services get a different name for each provider, so during the reshuffle I preserved:

secretName: $(SERVICE_NAME)-cert # this secret will not be prefixed since it's not managed by kustomize

in bootstrap/kubeadm/config/certmanager/certificate.yaml and made the necessary changes for it being resolved in config/default/kustomize.yaml which basically consist of creating this file and referencing it

bootstrap/kubeadm/config/default/manager_auth_proxy_patch.yaml

bootstrap/kubeadm/main.go

CecileRobertMichon

would be good to also describe these changes in https://github.com/kubernetes-sigs/cluster-api/blob/master/docs/book/src/developer/providers/v1alpha3-to-v1alpha4.md

fabriziopandini · 2020-12-03T21:06:23Z

@CecileRobertMichon

would be good to also describe these changes in https://github.com/kubernetes-sigs/cluster-api/blob/master/docs/book/src/developer/providers/v1alpha3-to-v1alpha4.md

Done!

fabriziopandini · 2020-12-03T21:34:48Z

/test pull-cluster-api-e2e-full-main

fabriziopandini · 2020-12-04T10:02:57Z

/test pull-cluster-api-e2e-full-main

Makefile

docs/book/src/developer/providers/v1alpha3-to-v1alpha4.md

main.go

test/infrastructure/docker/config/default/manager_auth_proxy_patch.yaml

bootstrap/kubeadm/config/default/manager_auth_proxy_patch.yaml

vincepri · 2020-12-04T19:01:01Z

LGTM pending squash

fabriziopandini · 2020-12-04T19:58:11Z

@vincepri squashed!

vincepri

/lgtm
/assign @CecileRobertMichon @detiber
for approval

fabriziopandini · 2021-01-15T20:32:04Z

/test pull-cluster-api-e2e-full-main

fabriziopandini · 2021-01-18T18:17:05Z

/test pull-cluster-api-e2e-full-main

fabriziopandini · 2021-01-18T19:04:51Z

rebase & fixed error on machine pools test
@vincepri this PR should be ready to go now

vincepri

/lgtm
/assign @detiber @CecileRobertMichon

wfernandes · 2021-01-19T21:46:11Z

/lgtm

CecileRobertMichon

lgtm overall

sorry for not getting to this earlier @fabriziopandini

docs/book/src/developer/providers/v1alpha3-to-v1alpha4.md

bootstrap/kubeadm/main.go

fabriziopandini · 2021-02-03T15:56:51Z

Rebased + adressed @CecileRobertMichon comments

fabriziopandini · 2021-02-03T16:06:12Z

/test pull-cluster-api-e2e-full-main

vincepri · 2021-02-03T16:14:54Z

/test pull-cluster-api-e2e-full-main

The panic should be resolved in 1.21

vincepri · 2021-02-03T16:16:10Z

/test pull-cluster-api-test-main

fabriziopandini · 2021-02-03T20:42:41Z

/retest

vincepri

/lgtm

vincepri · 2021-02-03T21:47:03Z

/retest

k8s-ci-robot · 2021-02-03T22:22:30Z

@fabriziopandini: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-cluster-api-e2e-full-main	`280db9a`	link	`/test pull-cluster-api-e2e-full-main`

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.

CecileRobertMichon · 2021-02-03T22:50:25Z

/retest
/approve

k8s-ci-robot · 2021-02-03T22:50:33Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CecileRobertMichon

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~~ [CecileRobertMichon]

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

fabriziopandini requested a review from vincepri December 3, 2020 18:39

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 3, 2020

k8s-ci-robot requested review from CecileRobertMichon and detiber December 3, 2020 18:39

k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Dec 3, 2020

fabriziopandini marked this pull request as draft December 3, 2020 18:39

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 3, 2020

k8s-ci-robot added this to the v0.4.0 milestone Dec 3, 2020

fabriziopandini marked this pull request as ready for review December 3, 2020 18:42

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 3, 2020

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 3, 2020

detiber reviewed Dec 3, 2020

View reviewed changes

CecileRobertMichon reviewed Dec 3, 2020

View reviewed changes

fabriziopandini force-pushed the run-webhooks-with-managers branch from 51427d3 to c2c839a Compare December 3, 2020 21:03

fabriziopandini mentioned this pull request Dec 3, 2020

⚠️ Adapt clusterctl to the new manifests format #3988

Closed

vincepri reviewed Dec 4, 2020

View reviewed changes

fabriziopandini mentioned this pull request Dec 4, 2020

Remove the example provider #3991

Closed

2 tasks

fabriziopandini force-pushed the run-webhooks-with-managers branch from 2b9da96 to df4014d Compare December 4, 2020 19:57

k8s-ci-robot assigned CecileRobertMichon Dec 4, 2020

vincepri approved these changes Dec 4, 2020

View reviewed changes

k8s-ci-robot assigned detiber and vincepri Dec 4, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 4, 2020

fabriziopandini force-pushed the run-webhooks-with-managers branch from a196908 to 3533800 Compare January 18, 2021 18:16

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 18, 2021

vincepri approved these changes Jan 19, 2021

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 19, 2021

fabriziopandini mentioned this pull request Jan 27, 2021

✨ Initial scaffolding for operator #4117

Merged

CecileRobertMichon reviewed Jan 28, 2021

View reviewed changes

docs/book/src/developer/providers/v1alpha3-to-v1alpha4.md Outdated Show resolved Hide resolved

docs/book/src/developer/providers/v1alpha3-to-v1alpha4.md Outdated Show resolved Hide resolved

bootstrap/kubeadm/main.go Show resolved Hide resolved

Run webhooks with managers

280db9a

fabriziopandini force-pushed the run-webhooks-with-managers branch from 3533800 to fa9edea Compare February 3, 2021 15:55

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 3, 2021

fabriziopandini force-pushed the run-webhooks-with-managers branch from fa9edea to 280db9a Compare February 3, 2021 15:58

fabriziopandini mentioned this pull request Feb 3, 2021

Refactor main to be in line with kubebuilder generated ones #4136

Closed

vincepri approved these changes Feb 3, 2021

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 3, 2021

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 3, 2021

k8s-ci-robot merged commit 53d4091 into kubernetes-sigs:master Feb 3, 2021

fabriziopandini deleted the run-webhooks-with-managers branch February 10, 2021 13:38

fabriziopandini mentioned this pull request Mar 11, 2021

⚠️ Adapt clusterctl to webhook deployed with managers #4297

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

⚠️ Run webhooks with managers #3985

⚠️ Run webhooks with managers #3985

fabriziopandini commented Dec 3, 2020 •

edited

Loading

fabriziopandini commented Dec 3, 2020

fabriziopandini commented Dec 3, 2020

detiber Dec 3, 2020

fabriziopandini Dec 3, 2020

CecileRobertMichon left a comment

fabriziopandini commented Dec 3, 2020

fabriziopandini commented Dec 3, 2020

fabriziopandini commented Dec 4, 2020

vincepri commented Dec 4, 2020

fabriziopandini commented Dec 4, 2020

vincepri left a comment

fabriziopandini commented Jan 15, 2021

fabriziopandini commented Jan 18, 2021

fabriziopandini commented Jan 18, 2021

vincepri left a comment

wfernandes commented Jan 19, 2021

CecileRobertMichon left a comment

fabriziopandini commented Feb 3, 2021 •

edited

Loading

fabriziopandini commented Feb 3, 2021

vincepri commented Feb 3, 2021

vincepri commented Feb 3, 2021

fabriziopandini commented Feb 3, 2021

vincepri left a comment

vincepri commented Feb 3, 2021

k8s-ci-robot commented Feb 3, 2021

CecileRobertMichon commented Feb 3, 2021

k8s-ci-robot commented Feb 3, 2021

⚠️ Run webhooks with managers #3985

⚠️ Run webhooks with managers #3985

Conversation

fabriziopandini commented Dec 3, 2020 • edited Loading

fabriziopandini commented Dec 3, 2020

fabriziopandini commented Dec 3, 2020

detiber Dec 3, 2020

Choose a reason for hiding this comment

fabriziopandini Dec 3, 2020

Choose a reason for hiding this comment

CecileRobertMichon left a comment

Choose a reason for hiding this comment

fabriziopandini commented Dec 3, 2020

fabriziopandini commented Dec 3, 2020

fabriziopandini commented Dec 4, 2020

vincepri commented Dec 4, 2020

fabriziopandini commented Dec 4, 2020

vincepri left a comment

Choose a reason for hiding this comment

fabriziopandini commented Jan 15, 2021

fabriziopandini commented Jan 18, 2021

fabriziopandini commented Jan 18, 2021

vincepri left a comment

Choose a reason for hiding this comment

wfernandes commented Jan 19, 2021

CecileRobertMichon left a comment

Choose a reason for hiding this comment

fabriziopandini commented Feb 3, 2021 • edited Loading

fabriziopandini commented Feb 3, 2021

vincepri commented Feb 3, 2021

vincepri commented Feb 3, 2021

fabriziopandini commented Feb 3, 2021

vincepri left a comment

Choose a reason for hiding this comment

vincepri commented Feb 3, 2021

k8s-ci-robot commented Feb 3, 2021

CecileRobertMichon commented Feb 3, 2021

k8s-ci-robot commented Feb 3, 2021

fabriziopandini commented Dec 3, 2020 •

edited

Loading

fabriziopandini commented Feb 3, 2021 •

edited

Loading