deploy webhook by yaml #2346

hwdef · 2022-07-11T08:33:28Z

steps:

Remove the code that creates the webhook by go
Create yaml files for deploying the webhook, set the certificate field blank.
Supplementary certificate fields via volcano-admission

a little problem:
There will be a delay in supplementing the certificate fields through volcano-admission. During this period, volcano-scheduler will restart because it cannot connect to the webhook.

I don't know if this is a serious problem, please give me some advice.
@Thor-wl @william-wang @shinytang6 @qiankunli

Thor-wl · 2022-07-13T09:13:59Z

cmd/webhook-manager/app/util.go

 	v1 "k8s.io/api/admissionregistration/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"time"


Please sort the packages regularlly.

shinytang6 · 2022-07-14T03:57:24Z

cmd/webhook-manager/app/util.go

+func addCaCertForWebhook(kubeClient *kubernetes.Clientset, caBundle []byte) error {
+	for _, mutatingWebhookName := range mutatingWebhooksName {
+		var mutatingWebhook *v1.MutatingWebhookConfiguration
+		if err := wait.PollInfinite(time.Second, func() (done bool, err error) {


it's better to add timeout and exit

How long should the timeout be set?

@shinytang6
I added a 5 minutes timeout.

shinytang6 · 2022-07-21T11:04:44Z

cmd/webhook-manager/app/util.go

-			klog.V(3).Infof("Registered mutating webhook for path <%s>.", service.Path)
+		for index := 0; index < len(mutatingWebhook.Webhooks); index++ {
+			if mutatingWebhook.Webhooks[index].ClientConfig.CABundle == nil ||
+				bytes.Equal(mutatingWebhook.Webhooks[index].ClientConfig.CABundle, caBundle) {


!bytes.Equal ?

Yes, you are right. I fixed it.

shinytang6 · 2022-07-21T11:06:20Z

cmd/webhook-manager/app/util.go

-			service.ValidatingConfig.Webhooks[i].AdmissionReviewVersions = reviewVersions
-			service.ValidatingConfig.Webhooks[i].ClientConfig = clientConfig
-			service.ValidatingConfig.Webhooks[i].NamespaceSelector = webhookLabelSelector
+		if _, err := kubeClient.AdmissionregistrationV1().MutatingWebhookConfigurations().Update(context.TODO(), mutatingWebhook, metav1.UpdateOptions{}); err != nil {


it's better to compare the updated webhook with original mutatingWebhook here, if no change, we don't need to call Update func

Ok, I added a flag to record if the webhook has been changed, and if so. then call the update function. reflection is not used, which may affect performance

shinytang6 · 2022-07-21T11:44:27Z

During this period, volcano-scheduler will restart because it cannot connect to the webhook.

Will the scheduler connect to the webhook? l am a little confused here

hwdef · 2022-07-22T06:53:30Z

During this period, volcano-scheduler will restart because it cannot connect to the webhook.

Will the scheduler connect to the webhook? l am a little confused here

Yes, I got some error log

$ kubectl logs -n volcano-system       volcano-scheduler-5499bd684f-v6c8g  -p 
2022/07/22 06:50:51 maxprocs: Leaving GOMAXPROCS=16: CPU quota undefined
W0722 06:50:51.150374       1 client_config.go:617] Neither --kubeconfig nor --master was specified.  Using the inClusterConfig.  This might not work.
panic: failed init default queue, with err: Internal error occurred: failed calling webhook "mutatequeue.volcano.sh": failed to call webhook: Post "https://volcano-admission-service.volcano-system.svc:443/queues/mutate?timeout=10s": dial tcp 10.96.139.202:443: connect: connection refused

goroutine 1 [running]:
volcano.sh/volcano/pkg/scheduler/cache.newSchedulerCache(0x7ffcc03f4b15?, {0x1c1c30d, 0x7}, {0x1c1b6db, 0x7}, {0x0, 0x0, 0x0?})
        /home/edward/code/go/volcano/pkg/scheduler/cache/cache.go:402 +0x20fc
volcano.sh/volcano/pkg/scheduler/cache.New(...)
        /home/edward/code/go/volcano/pkg/scheduler/cache/cache.go:87
volcano.sh/volcano/pkg/scheduler.NewScheduler(0x0?, {0x1c1c30d?, 0x7?}, {0x7ffcc03f4b15, 0x29}, 0x3b9aca00, {0x1c1b6db?, 0x7?}, {0x0, 0x0, ...})
        /home/edward/code/go/volcano/pkg/scheduler/scheduler.go:75 +0xf0
volcano.sh/volcano/cmd/scheduler/app.Run(0xc0001fc2d0)
        /home/edward/code/go/volcano/cmd/scheduler/app/server.go:76 +0x1bf
main.main()
        /home/edward/code/go/volcano/cmd/scheduler/main.go:65 +0x190

cmd/webhook-manager/app/server.go

william-wang · 2022-07-28T09:14:52Z

a little problem: There will be a delay in supplementing the certificate fields through volcano-admission. During this period, volcano-scheduler will restart because it cannot connect to the webhook.

I don't know if this is a serious problem, please give me some advice. @Thor-wl @william-wang @shinytang6 @qiankunli

@hwdef Yes, it is a serious problem. Woud you elaborate more on "volcano-scheduler restart because it can not connect to the webhook". scheduler has no direct connection with webhook.

hwdef · 2022-07-28T09:28:49Z

a little problem: There will be a delay in supplementing the certificate fields through volcano-admission. During this period, volcano-scheduler will restart because it cannot connect to the webhook.
I don't know if this is a serious problem, please give me some advice. @Thor-wl @william-wang @shinytang6 @qiankunli

@hwdef Yes, it is a serious problem. Woud you elaborate more on "volcano-scheduler restart because it can not connect to the webhook". scheduler has no direct connection with webhook.

After discussing it with @shinytang6 , we have roughly located the problem. I will try to solve it later.

hwdef · 2022-08-02T03:45:33Z

The error mentioned above is caused by the failure to call the webhook when creating the initial queue. ~~For this I use the wait package to retry the creation of the initial queue at a frequency of once per second. Until it is created successfully.~~

~~In addition, I also set a timeout of 5 minutes, and when a fatal problem is encountered, the program can be exited by panic.~~
~~Not sure if such a solution would work.~~
@william-wang @shinytang6 What do you think?

hwdef · 2022-08-04T10:49:29Z

@shinytang6
I use retry package to create init queue, please review it again.
Thanks!

cmd/webhook-manager/app/util.go

shinytang6

Another questions: due to we apply all the webhook configurations now, will enabled-admission arguments still work as expected?

shinytang6 · 2022-08-05T07:50:38Z

cmd/webhook-manager/app/util.go


 	v1 "k8s.io/api/admissionregistration/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+


remove the line

Thanks for your suggestion. I deleted this.

hwdef · 2022-08-05T08:35:20Z

Another questions: due to we apply all the webhook configurations now, will enabled-admission arguments still work as expected?

After checking the source of this parameter and related code, I think that deploying webhook and this parameter are two separate functions that have no effect on each other

shinytang6 · 2022-08-09T11:33:59Z

cmd/webhook-manager/app/options/options.go

@@ -79,7 +79,7 @@ func (c *Config) AddFlags(fs *pflag.FlagSet) {
 	fs.StringVar(&c.WebhookNamespace, "webhook-namespace", "", "The namespace of this webhook")
 	fs.StringVar(&c.WebhookName, "webhook-service-name", "", "The name of this webhook")
 	fs.StringVar(&c.WebhookURL, "webhook-url", "", "The url of this webhook")
-	fs.StringVar(&c.EnabledAdmission, "enabled-admission", defaultEnabledAdmission, "enabled admission webhooks")
+	fs.StringVar(&c.EnabledAdmission, "enabled-admission", defaultEnabledAdmission, "enabled admission webhooks,if this parameter is modified, please modify the deployed webhook as well. Make sure the same webhook is deployed and enabled.")


enabled admission webhooks, if this parameter is modified, make sure corresponding webhook configurations are the same.

Thanks. I changed this.

Signed-off-by: hwdef <hwdefcom@outlook.com>

hwdef · 2022-08-12T02:48:32Z

@shinytang6 @william-wang @Thor-wl
I think this PR is ready to review. Please take a look.

shinytang6 · 2022-08-12T03:07:43Z

Generally LGTM.

volcano-sh-bot · 2022-08-12T03:18:24Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Thor-wl

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~~ [Thor-wl]

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

volcano-sh-bot requested review from hzxuzhonghu and merryzhou July 11, 2022 08:33

volcano-sh-bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jul 11, 2022

hwdef force-pushed the delete-webhook-gen-by-go branch from 3a03db2 to 5614179 Compare July 11, 2022 08:35

Thor-wl requested review from william-wang, Thor-wl, qiankunli and shinytang6 and removed request for merryzhou and hzxuzhonghu July 11, 2022 08:43

hwdef force-pushed the delete-webhook-gen-by-go branch from 5614179 to ae62368 Compare July 11, 2022 09:31

Thor-wl reviewed Jul 13, 2022

View reviewed changes

hwdef force-pushed the delete-webhook-gen-by-go branch from ae62368 to 0f14fa4 Compare July 13, 2022 09:29

shinytang6 suggested changes Jul 14, 2022

View reviewed changes

volcano-sh-bot assigned shinytang6 Jul 14, 2022

shinytang6 mentioned this pull request Jul 14, 2022

fix(deploy): add helm pre delete hook to delete orphan resources #2203

Closed

hwdef force-pushed the delete-webhook-gen-by-go branch 2 times, most recently from dea17da to 72aa2f9 Compare July 20, 2022 06:09

hwdef mentioned this pull request Jul 21, 2022

Uninstall Volcano Doesn't Clean ValidateWebhook Configuration And MutatingWebhook Configuration #2102

Closed

shinytang6 suggested changes Jul 21, 2022

View reviewed changes

hwdef force-pushed the delete-webhook-gen-by-go branch from 72aa2f9 to 05dc1fa Compare July 22, 2022 06:48

shinytang6 suggested changes Jul 27, 2022

View reviewed changes

cmd/webhook-manager/app/server.go Show resolved Hide resolved

hwdef force-pushed the delete-webhook-gen-by-go branch 2 times, most recently from 6c578f3 to 47e1ca6 Compare August 2, 2022 03:43

hwdef force-pushed the delete-webhook-gen-by-go branch from 47e1ca6 to 94a16dc Compare August 4, 2022 10:46

shinytang6 suggested changes Aug 5, 2022

View reviewed changes

cmd/webhook-manager/app/util.go Show resolved Hide resolved

hwdef force-pushed the delete-webhook-gen-by-go branch 2 times, most recently from fc88bca to 68af3dc Compare August 5, 2022 07:23

shinytang6 reviewed Aug 5, 2022

View reviewed changes

hwdef force-pushed the delete-webhook-gen-by-go branch 4 times, most recently from dd804fb to 1a86782 Compare August 8, 2022 05:55

shinytang6 suggested changes Aug 9, 2022

View reviewed changes

deploy webhook by yaml

35b0ac1

Signed-off-by: hwdef <hwdefcom@outlook.com>

hwdef force-pushed the delete-webhook-gen-by-go branch from 1a86782 to 35b0ac1 Compare August 10, 2022 03:16

Thor-wl approved these changes Aug 12, 2022

View reviewed changes

volcano-sh-bot assigned Thor-wl Aug 12, 2022

volcano-sh-bot added the lgtm Indicates that a PR is ready to be merged. label Aug 12, 2022

volcano-sh-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 12, 2022

volcano-sh-bot merged commit 3c3d4fc into volcano-sh:master Aug 12, 2022

This was referenced Aug 12, 2022

Add doc to note how to cleanup Volcano completely #2079

Closed

k8s cluster totally not work after helm uninstall volcano #2191

Closed

Mutatingwebhook respect namespacing #2247

Closed

cmd: add --managed-namespaces flag to vc-webhook-manager #2290

Closed

nature1995 mentioned this pull request Dec 21, 2022

volcano-admission restart error, failed calling webhook "mutatepod.volcano.sh" #2610

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

deploy webhook by yaml #2346

deploy webhook by yaml #2346

hwdef commented Jul 11, 2022

Thor-wl Jul 13, 2022

hwdef Jul 13, 2022

shinytang6 Jul 14, 2022

hwdef Jul 14, 2022

hwdef Jul 20, 2022

shinytang6 Jul 21, 2022

hwdef Jul 22, 2022

shinytang6 Jul 21, 2022

hwdef Jul 22, 2022

shinytang6 commented Jul 21, 2022 •

edited

hwdef commented Jul 22, 2022

william-wang commented Jul 28, 2022

hwdef commented Jul 28, 2022

hwdef commented Aug 2, 2022 •

edited

hwdef commented Aug 4, 2022

shinytang6 left a comment

shinytang6 Aug 5, 2022

hwdef Aug 5, 2022

hwdef commented Aug 5, 2022

shinytang6 Aug 9, 2022

hwdef Aug 10, 2022

hwdef commented Aug 12, 2022

shinytang6 commented Aug 12, 2022

volcano-sh-bot commented Aug 12, 2022


		v1 "k8s.io/api/admissionregistration/v1"
		apierrors "k8s.io/apimachinery/pkg/api/errors"

deploy webhook by yaml #2346

deploy webhook by yaml #2346

Conversation

hwdef commented Jul 11, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shinytang6 commented Jul 21, 2022 • edited

hwdef commented Jul 22, 2022

william-wang commented Jul 28, 2022

hwdef commented Jul 28, 2022

hwdef commented Aug 2, 2022 • edited

hwdef commented Aug 4, 2022

shinytang6 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hwdef commented Aug 5, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hwdef commented Aug 12, 2022

shinytang6 commented Aug 12, 2022

volcano-sh-bot commented Aug 12, 2022

shinytang6 commented Jul 21, 2022 •

edited

hwdef commented Aug 2, 2022 •

edited