Uninstall Volcano Doesn't Clean ValidateWebhook Configuration And MutatingWebhook Configuration

[whybeyoung]

How to reproduce it (as minimally and precisely as possible):

step1: kubectlcreate -f https://raw.githubusercontent.com/volcano-sh/volcano/v1.5.1/installer/volcano-development.yaml

step2: kubectl delete -f https://raw.githubusercontent.com/volcano-sh/volcano/v1.5.1/installer/volcano-development.yaml

The Webhook Configuration is not Cleaned because they are not in the yaml.

And the remained configuration will cause the cluster podCreateFailed beacuse of no pod validate endpoint , this is a little problem but not easy to find.

Suggesstion:

Clean them by using ownerrefernce or labels mechanism

[hwdef]

I think it is useful

[hwdef]

It is not feasible to use the ownerrefernce,
because webhook should be associated with deployment(volcano-admission), but deployment is namespace scoped, webhook is cluster scoped. kubernetes does not allow deletion across namespaces.

https://kubernetes.io/zh/docs/concepts/overview/working-with-objects/owners-dependents/

$ kubectl get events -A --field-selector=reason=OwnerRefInvalidNamespace
NAMESPACE   LAST SEEN   TYPE      REASON                     OBJECT                                                                    MESSAGE
default     5m22s       Warning   OwnerRefInvalidNamespace   mutatingwebhookconfiguration/volcano-admission-service-jobs-mutate        ownerRef [apps/v1/Deployment, namespace: , name: volcano-admission, uid: 0d14f232-61dc-40d2-a5f4-a24150e76f25] does not exist in namespace ""
default     5m22s       Warning   OwnerRefInvalidNamespace   mutatingwebhookconfiguration/volcano-admission-service-podgroups-mutate   ownerRef [apps/v1/Deployment, namespace: , name: volcano-admission, uid: 0d14f232-61dc-40d2-a5f4-a24150e76f25] does not exist in namespace ""
default     5m22s       Warning   OwnerRefInvalidNamespace   mutatingwebhookconfiguration/volcano-admission-service-pods-mutate        ownerRef [apps/v1/Deployment, namespace: , name: volcano-admission, uid: 0d14f232-61dc-40d2-a5f4-a24150e76f25] does not exist in namespace ""
default     5m22s       Warning   OwnerRefInvalidNamespace   mutatingwebhookconfiguration/volcano-admission-service-queues-mutate      ownerRef [apps/v1/Deployment, namespace: , name: volcano-admission, uid: 0d14f232-61dc-40d2-a5f4-a24150e76f25] does not exist in namespace ""

@Thor-wl Do you have any suggestions?

[shinytang6]

Supplyment: volcano-admission-init will create a secret which cannot be cleaned thoroughly.

[shinytang6]

similar issue: #2079

[Yikun]

Is it possible adding some placeholder in volcano-development.yaml to help delete related resources? Then users can cleanup them using kubectl delete -f volcano-development.yaml.

Otherwise we probably want to add a volcano-development-delete.yaml separately.

[whybeyoung]

i think the we should fix the webhook code here first.

volcano/cmd/webhook-manager/app/util.go

Line 65 in 183a178

service.MutatingConfig.ObjectMeta.Name = webhookConfigName(config.WebhookName, service.Path)

and

volcano/cmd/webhook-manager/app/util.go

Line 81 in 183a178

service.ValidatingConfig.ObjectMeta.Name = webhookConfigName(config.WebhookName, service.Path)

;
And

volcano/cmd/webhook-manager/app/util.go

Line 51 in 183a178

Name: config.WebhookName,

This two place generate different config name prefix when user uses url webhook or uses serivice webhook..And i think it's no need. we can use the default webhook name like "webhook" if user not specify, add an variable WebhookServiceName to distinguish the WebhookName,

then we can put those 8 webhooks in the yaml then we can deal with this problem.

BTW, I try to use finalizers to deal with problem ,but it looks more complex .....

[hwdef]

I think webhook should not be generated by code, webhook should be defined by yaml.
If I want to add some rules for webhook, for example, ignore some pods. I can't do this when generating with code,unless modify the source code.

[Yikun]

Could you give some suggestion or inputs in here? @Thor-wl

[hwdef]

fix in #2346

/assign

[hwdef]

#2346 is merged.
/close

[volcano-sh-bot]

@hwdef: Closing this issue.

In response to this:

#2346 is merged.
/close

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.