Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix operator: remove unused mutating and conversion webhook configs #1705

Merged
merged 1 commit into from
Dec 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 0 additions & 6 deletions ray-operator/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,6 @@ install: manifests kustomize ## Install CRDs into the K8s cluster specified in ~
uninstall: manifests kustomize ## Uninstall CRDs from the K8s cluster specified in ~/.kube/config.
$(KUSTOMIZE) build config/crd | kubectl delete -f -

install-with-webhooks: manifests kustomize ## Install CRDs with webhooks into the K8s cluster specified in ~/.kube/config.
($(KUSTOMIZE) build --load-restrictor LoadRestrictionsNone config/crd-with-webhooks | kubectl create -f -) || ($(KUSTOMIZE) build --load-restrictor LoadRestrictionsNone config/crd-with-webhooks | kubectl replace -f -)

uninstall-with-webhooks: manifests kustomize ## Uninstall CRDs with webhooks from the K8s cluster specified in ~/.kube/config.
$(KUSTOMIZE) build --load-restrictor LoadRestrictionsNone config/crd-with-webhooks | kubectl delete -f -
Comment on lines -114 to -118
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need because CRDs don't need any changes. config/crd-with-webhooks is only needed for conversion webhooks which we don't use


deploy: manifests kustomize ## Deploy controller to the K8s cluster specified in ~/.kube/config.
cd config/default && $(KUSTOMIZE) edit set image kuberay/operator=${IMG}
$(KUSTOMIZE) build config/default | kubectl apply --server-side=true -f -
Expand Down
11 changes: 0 additions & 11 deletions ray-operator/apis/ray/v1/raycluster_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,6 @@ func (r *RayCluster) SetupWebhookWithManager(mgr ctrl.Manager) error {
Complete()
}

//+kubebuilder:webhook:path=/mutate-ray-io-v1-raycluster,mutating=true,failurePolicy=fail,sideEffects=None,groups=ray.io,resources=rayclusters,verbs=create;update,versions=v1,name=mraycluster.kb.io,admissionReviewVersions=v1

var _ webhook.Defaulter = &RayCluster{}

// Default implements webhook.Defaulter so a webhook will be registered for the type
func (r *RayCluster) Default() {
rayclusterlog.Info("default", "name", r.Name)

// TODO(user): fill in your defaulting logic.
}
Comment on lines -23 to -32
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for mutating webhook which we don't use


// TODO(user): change verbs to "verbs=create;update;delete" if you want to enable deletion validation.
//+kubebuilder:webhook:path=/validate-ray-io-v1-raycluster,mutating=false,failurePolicy=fail,sideEffects=None,groups=ray.io,resources=rayclusters,verbs=create;update,versions=v1,name=vraycluster.kb.io,admissionReviewVersions=v1

Expand Down
21 changes: 0 additions & 21 deletions ray-operator/config/crd-with-webhooks/kustomization.yaml

This file was deleted.

19 changes: 0 additions & 19 deletions ray-operator/config/crd-with-webhooks/kustomizeconfig.yaml

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,18 +1,5 @@
# This patch add annotation to admission webhook config and
# the variables $(CERTIFICATE_NAMESPACE) and $(CERTIFICATE_NAME) will be substituted by kustomize.
apiVersion: admissionregistration.k8s.io/v1
kind: MutatingWebhookConfiguration
metadata:
labels:
app.kubernetes.io/name: mutatingwebhookconfiguration
app.kubernetes.io/instance: mutating-webhook-configuration
app.kubernetes.io/component: webhook
app.kubernetes.io/created-by: ray-operator
app.kubernetes.io/part-of: ray-operator
app.kubernetes.io/managed-by: kustomize
name: mutating-webhook-configuration
annotations:
cert-manager.io/inject-ca-from: $(CERTIFICATE_NAMESPACE)/$(CERTIFICATE_NAME)
---
apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingWebhookConfiguration
Expand Down
10 changes: 10 additions & 0 deletions ray-operator/config/webhook/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,13 @@ resources:

configurations:
- kustomizeconfig.yaml

patches:
- patch: |-
- op: replace
path: /webhooks/0/clientConfig/service/namespace
value: ray-system
target:
kind: ValidatingWebhookConfiguration
name: validating-webhook-configuration
version: v1
Comment on lines +8 to +16
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

webhook generator hardcodes the namespace of the service to system (issue). We change it to ray-system

7 changes: 0 additions & 7 deletions ray-operator/config/webhook/kustomizeconfig.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,11 @@ nameReference:
- kind: Service
version: v1
fieldSpecs:
- kind: MutatingWebhookConfiguration
group: admissionregistration.k8s.io
path: webhooks/clientConfig/service/name
- kind: ValidatingWebhookConfiguration
group: admissionregistration.k8s.io
path: webhooks/clientConfig/service/name

namespace:
- kind: MutatingWebhookConfiguration
group: admissionregistration.k8s.io
path: webhooks/clientConfig/service/namespace
create: true
- kind: ValidatingWebhookConfiguration
group: admissionregistration.k8s.io
path: webhooks/clientConfig/service/namespace
Expand Down
26 changes: 0 additions & 26 deletions ray-operator/config/webhook/manifests.yaml
Original file line number Diff line number Diff line change
@@ -1,31 +1,5 @@
---
apiVersion: admissionregistration.k8s.io/v1
kind: MutatingWebhookConfiguration
metadata:
name: mutating-webhook-configuration
webhooks:
- admissionReviewVersions:
- v1
clientConfig:
service:
name: webhook-service
namespace: system
path: /mutate-ray-io-v1-raycluster
failurePolicy: Fail
name: mraycluster.kb.io
rules:
- apiGroups:
- ray.io
apiVersions:
- v1
operations:
- CREATE
- UPDATE
resources:
- rayclusters
sideEffects: None
---
apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingWebhookConfiguration
metadata:
name: validating-webhook-configuration
Expand Down
2 changes: 1 addition & 1 deletion ray-operator/config/webhook/service.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ metadata:
app.kubernetes.io/part-of: ray-operator
app.kubernetes.io/managed-by: kustomize
name: webhook-service
namespace: system
namespace: ray-system
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this matches the namespace referenced by the ValidatingWebhookConfiguration

spec:
ports:
- port: 443
Expand Down