From 46446d7191661a4e3c41b5380f894796de5db928 Mon Sep 17 00:00:00 2001 From: Wen Zhou Date: Tue, 17 Sep 2024 11:50:18 +0200 Subject: [PATCH 1/6] refactor: change for mutating webhook to use Default to set ModelReg namespace - rename SetupValidatingWebhookWithManager and SetupMutateWebhookWithManager per each - use Default() instead of setDSCDefaults() - return call PatchResponseFromRaw than admission.Allowed() Signed-off-by: Wen Zhou --- controllers/webhook/webhook.go | 64 +++++++++++++---------- controllers/webhook/webhook_suite_test.go | 4 +- main.go | 4 +- 3 files changed, 41 insertions(+), 31 deletions(-) diff --git a/controllers/webhook/webhook.go b/controllers/webhook/webhook.go index e404e673b88..613891f70db 100644 --- a/controllers/webhook/webhook.go +++ b/controllers/webhook/webhook.go @@ -18,12 +18,14 @@ package webhook import ( "context" + "encoding/json" + "errors" "fmt" "net/http" - operatorv1 "github.com/openshift/api/operator/v1" admissionv1 "k8s.io/api/admission/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -45,10 +47,10 @@ type OpenDataHubValidatingWebhook struct { Decoder *admission.Decoder } -func (w *OpenDataHubValidatingWebhook) SetupWithManager(mgr ctrl.Manager) { +func (v *OpenDataHubValidatingWebhook) SetupValidatingWebhookWithManager(mgr ctrl.Manager) { hookServer := mgr.GetWebhookServer() odhWebhook := &webhook.Admission{ - Handler: w, + Handler: v, } hookServer.Register("/validate-opendatahub-io-v1", odhWebhook) } @@ -77,7 +79,7 @@ func denyCountGtZero(ctx context.Context, cli client.Client, gvk schema.GroupVer return admission.Allowed("") } -func (w *OpenDataHubValidatingWebhook) checkDupCreation(ctx context.Context, req admission.Request) admission.Response { +func (v *OpenDataHubValidatingWebhook) checkDupCreation(ctx context.Context, req admission.Request) admission.Response { switch req.Kind.Kind { case "DataScienceCluster", "DSCInitialization": default: @@ -92,29 +94,29 @@ func (w *OpenDataHubValidatingWebhook) checkDupCreation(ctx context.Context, req } // if count == 1 now creation of #2 is being handled - return denyCountGtZero(ctx, w.Client, gvk, + return denyCountGtZero(ctx, v.Client, gvk, fmt.Sprintf("Only one instance of %s object is allowed", req.Kind.Kind)) } -func (w *OpenDataHubValidatingWebhook) checkDeletion(ctx context.Context, req admission.Request) admission.Response { +func (v *OpenDataHubValidatingWebhook) checkDeletion(ctx context.Context, req admission.Request) admission.Response { if req.Kind.Kind == "DataScienceCluster" { return admission.Allowed("") } // Restrict deletion of DSCI if DSC exists - return denyCountGtZero(ctx, w.Client, gvk.DataScienceCluster, + return denyCountGtZero(ctx, v.Client, gvk.DataScienceCluster, fmt.Sprintln("Cannot delete DSCI object when DSC object still exists")) } -func (w *OpenDataHubValidatingWebhook) Handle(ctx context.Context, req admission.Request) admission.Response { +func (v *OpenDataHubValidatingWebhook) Handle(ctx context.Context, req admission.Request) admission.Response { var resp admission.Response resp.Allowed = true // initialize Allowed to be true in case Operation falls into "default" case switch req.Operation { case admissionv1.Create: - resp = w.checkDupCreation(ctx, req) + resp = v.checkDupCreation(ctx, req) case admissionv1.Delete: - resp = w.checkDeletion(ctx, req) + resp = v.checkDeletion(ctx, req) default: // for other operations by default it is admission.Allowed("") // no-op } @@ -136,35 +138,43 @@ type OpenDataHubMutatingWebhook struct { Decoder *admission.Decoder } -func (w *OpenDataHubMutatingWebhook) SetupWithManager(mgr ctrl.Manager) { +func (m *OpenDataHubMutatingWebhook) SetupMutateWebhookWithManager(mgr ctrl.Manager) { hookServer := mgr.GetWebhookServer() odhWebhook := &webhook.Admission{ - Handler: w, + Handler: m, } hookServer.Register("/mutate-opendatahub-io-v1", odhWebhook) } -func (w *OpenDataHubMutatingWebhook) Handle(ctx context.Context, req admission.Request) admission.Response { - var resp admission.Response - resp.Allowed = true // initialize Allowed to be true in case Operation falls into "default" case - +func (m *OpenDataHubMutatingWebhook) Handle(ctx context.Context, req admission.Request) admission.Response { dsc := &dscv1.DataScienceCluster{} - err := w.Decoder.Decode(req, dsc) + err := m.Decoder.Decode(req, dsc) if err != nil { return admission.Denied(fmt.Sprintf("failed to decode request body: %s", err)) } - resp = w.setDSCDefaults(ctx, dsc) - return resp + + if err := m.Default(ctx, dsc); err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + + // convert to []byte + dscBytes, err := json.Marshal(dsc) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + // use PatchResponseFromRaw to ensure dsc CR is updated with defaults + return admission.PatchResponseFromRaw(req.Object.Raw, dscBytes) } -func (w *OpenDataHubMutatingWebhook) setDSCDefaults(_ context.Context, dsc *dscv1.DataScienceCluster) admission.Response { +// implement admission.CustomDefaulter interface. +func (m *OpenDataHubMutatingWebhook) Default(_ context.Context, obj runtime.Object) error { + dsc, isDSC := obj.(*dscv1.DataScienceCluster) + if !isDSC { + return errors.New("expected DataScienceCluster but got a different type") + } // set default registriesNamespace if empty - if len(dsc.Spec.Components.ModelRegistry.RegistriesNamespace) == 0 && dsc.Spec.Components.ModelRegistry.ManagementState == operatorv1.Managed { - return admission.Patched("Property registriesNamespace set to default value", webhook.JSONPatchOp{ - Operation: "replace", - Path: "spec.components.modelregistry.registriesNamespace", - Value: modelregistry.DefaultModelRegistriesNamespace, - }) + if len(dsc.Spec.Components.ModelRegistry.RegistriesNamespace) == 0 { + dsc.Spec.Components.ModelRegistry.RegistriesNamespace = modelregistry.DefaultModelRegistriesNamespace } - return admission.Allowed("") + return nil } diff --git a/controllers/webhook/webhook_suite_test.go b/controllers/webhook/webhook_suite_test.go index 529ee3f5313..cb4b5bd4930 100644 --- a/controllers/webhook/webhook_suite_test.go +++ b/controllers/webhook/webhook_suite_test.go @@ -136,12 +136,12 @@ var _ = BeforeSuite(func() { (&webhook.OpenDataHubValidatingWebhook{ Client: mgr.GetClient(), Decoder: admission.NewDecoder(mgr.GetScheme()), - }).SetupWithManager(mgr) + }).SetupValidatingWebhookWithManager(mgr) (&webhook.OpenDataHubMutatingWebhook{ Client: mgr.GetClient(), Decoder: admission.NewDecoder(mgr.GetScheme()), - }).SetupWithManager(mgr) + }).SetupMutateWebhookWithManager(mgr) // +kubebuilder:scaffold:webhook diff --git a/main.go b/main.go index 44966275e1b..707e9378e92 100644 --- a/main.go +++ b/main.go @@ -211,12 +211,12 @@ func main() { //nolint:funlen,maintidx (&webhook.OpenDataHubValidatingWebhook{ Client: mgr.GetClient(), Decoder: admission.NewDecoder(mgr.GetScheme()), - }).SetupWithManager(mgr) + }).SetupValidatingWebhookWithManager(mgr) (&webhook.OpenDataHubMutatingWebhook{ Client: mgr.GetClient(), Decoder: admission.NewDecoder(mgr.GetScheme()), - }).SetupWithManager(mgr) + }).SetupMutateWebhookWithManager(mgr) if err = (&dscictrl.DSCInitializationReconciler{ Client: mgr.GetClient(), From 4fec71b19d425877b41733b0d866b09d4c50df74 Mon Sep 17 00:00:00 2001 From: Wen Zhou Date: Tue, 17 Sep 2024 12:58:22 +0200 Subject: [PATCH 2/6] update: rework implmentation Signed-off-by: Wen Zhou --- controllers/webhook/webhook.go | 39 ++++------------------- controllers/webhook/webhook_suite_test.go | 5 +-- main.go | 5 +-- 3 files changed, 9 insertions(+), 40 deletions(-) diff --git a/controllers/webhook/webhook.go b/controllers/webhook/webhook.go index 613891f70db..4277a6518b5 100644 --- a/controllers/webhook/webhook.go +++ b/controllers/webhook/webhook.go @@ -18,7 +18,6 @@ package webhook import ( "context" - "encoding/json" "errors" "fmt" "net/http" @@ -131,43 +130,19 @@ func (v *OpenDataHubValidatingWebhook) Handle(ctx context.Context, req admission //+kubebuilder:webhook:path=/mutate-opendatahub-io-v1,mutating=true,failurePolicy=fail,sideEffects=None,groups=datasciencecluster.opendatahub.io,resources=datascienceclusters,verbs=create;update,versions=v1,name=mutate.operator.opendatahub.io,admissionReviewVersions=v1 //nolint:lll -// OpenDataHubMutatingWebhook is a mutating webhook // It currently only sets defaults for modelregiestry in datascienceclusters. -type OpenDataHubMutatingWebhook struct { - Client client.Client - Decoder *admission.Decoder -} - -func (m *OpenDataHubMutatingWebhook) SetupMutateWebhookWithManager(mgr ctrl.Manager) { - hookServer := mgr.GetWebhookServer() - odhWebhook := &webhook.Admission{ - Handler: m, - } - hookServer.Register("/mutate-opendatahub-io-v1", odhWebhook) -} +type DSCDefaulter struct{} -func (m *OpenDataHubMutatingWebhook) Handle(ctx context.Context, req admission.Request) admission.Response { - dsc := &dscv1.DataScienceCluster{} - err := m.Decoder.Decode(req, dsc) - if err != nil { - return admission.Denied(fmt.Sprintf("failed to decode request body: %s", err)) - } +// just assert that DSCDefaulter implements webhook.CustomDefaulter. +var _ webhook.CustomDefaulter = &DSCDefaulter{} - if err := m.Default(ctx, dsc); err != nil { - return admission.Errored(http.StatusInternalServerError, err) - } - - // convert to []byte - dscBytes, err := json.Marshal(dsc) - if err != nil { - return admission.Errored(http.StatusInternalServerError, err) - } - // use PatchResponseFromRaw to ensure dsc CR is updated with defaults - return admission.PatchResponseFromRaw(req.Object.Raw, dscBytes) +func (m *DSCDefaulter) SetupMutateWebhookWithManager(mgr ctrl.Manager) { + mutateWebhook := admission.WithCustomDefaulter(mgr.GetScheme(), &dscv1.DataScienceCluster{}, m) + mgr.GetWebhookServer().Register("/mutate-opendatahub-io-v1", mutateWebhook) } // implement admission.CustomDefaulter interface. -func (m *OpenDataHubMutatingWebhook) Default(_ context.Context, obj runtime.Object) error { +func (m *DSCDefaulter) Default(_ context.Context, obj runtime.Object) error { dsc, isDSC := obj.(*dscv1.DataScienceCluster) if !isDSC { return errors.New("expected DataScienceCluster but got a different type") diff --git a/controllers/webhook/webhook_suite_test.go b/controllers/webhook/webhook_suite_test.go index cb4b5bd4930..c4c26524723 100644 --- a/controllers/webhook/webhook_suite_test.go +++ b/controllers/webhook/webhook_suite_test.go @@ -138,10 +138,7 @@ var _ = BeforeSuite(func() { Decoder: admission.NewDecoder(mgr.GetScheme()), }).SetupValidatingWebhookWithManager(mgr) - (&webhook.OpenDataHubMutatingWebhook{ - Client: mgr.GetClient(), - Decoder: admission.NewDecoder(mgr.GetScheme()), - }).SetupMutateWebhookWithManager(mgr) + (&webhook.DSCDefaulter{}).SetupMutateWebhookWithManager(mgr) // +kubebuilder:scaffold:webhook diff --git a/main.go b/main.go index 707e9378e92..523d9c830ba 100644 --- a/main.go +++ b/main.go @@ -213,10 +213,7 @@ func main() { //nolint:funlen,maintidx Decoder: admission.NewDecoder(mgr.GetScheme()), }).SetupValidatingWebhookWithManager(mgr) - (&webhook.OpenDataHubMutatingWebhook{ - Client: mgr.GetClient(), - Decoder: admission.NewDecoder(mgr.GetScheme()), - }).SetupMutateWebhookWithManager(mgr) + (&webhook.DSCDefaulter{}).SetupMutateWebhookWithManager(mgr) if err = (&dscictrl.DSCInitializationReconciler{ Client: mgr.GetClient(), From ad31f0518cd9f20c79233c4625b3aa0551f8ecb5 Mon Sep 17 00:00:00 2001 From: Wen Zhou Date: Tue, 17 Sep 2024 14:46:26 +0200 Subject: [PATCH 3/6] update: revert some unnecessary changes and add type in log Signed-off-by: Wen Zhou --- controllers/webhook/webhook.go | 21 ++++++++++----------- controllers/webhook/webhook_suite_test.go | 2 +- main.go | 2 +- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/controllers/webhook/webhook.go b/controllers/webhook/webhook.go index 4277a6518b5..200cc19e5a0 100644 --- a/controllers/webhook/webhook.go +++ b/controllers/webhook/webhook.go @@ -18,7 +18,6 @@ package webhook import ( "context" - "errors" "fmt" "net/http" @@ -46,10 +45,10 @@ type OpenDataHubValidatingWebhook struct { Decoder *admission.Decoder } -func (v *OpenDataHubValidatingWebhook) SetupValidatingWebhookWithManager(mgr ctrl.Manager) { +func (w *OpenDataHubValidatingWebhook) SetupWithManager(mgr ctrl.Manager) { hookServer := mgr.GetWebhookServer() odhWebhook := &webhook.Admission{ - Handler: v, + Handler: w, } hookServer.Register("/validate-opendatahub-io-v1", odhWebhook) } @@ -78,7 +77,7 @@ func denyCountGtZero(ctx context.Context, cli client.Client, gvk schema.GroupVer return admission.Allowed("") } -func (v *OpenDataHubValidatingWebhook) checkDupCreation(ctx context.Context, req admission.Request) admission.Response { +func (w *OpenDataHubValidatingWebhook) checkDupCreation(ctx context.Context, req admission.Request) admission.Response { switch req.Kind.Kind { case "DataScienceCluster", "DSCInitialization": default: @@ -93,29 +92,29 @@ func (v *OpenDataHubValidatingWebhook) checkDupCreation(ctx context.Context, req } // if count == 1 now creation of #2 is being handled - return denyCountGtZero(ctx, v.Client, gvk, + return denyCountGtZero(ctx, w.Client, gvk, fmt.Sprintf("Only one instance of %s object is allowed", req.Kind.Kind)) } -func (v *OpenDataHubValidatingWebhook) checkDeletion(ctx context.Context, req admission.Request) admission.Response { +func (w *OpenDataHubValidatingWebhook) checkDeletion(ctx context.Context, req admission.Request) admission.Response { if req.Kind.Kind == "DataScienceCluster" { return admission.Allowed("") } // Restrict deletion of DSCI if DSC exists - return denyCountGtZero(ctx, v.Client, gvk.DataScienceCluster, + return denyCountGtZero(ctx, w.Client, gvk.DataScienceCluster, fmt.Sprintln("Cannot delete DSCI object when DSC object still exists")) } -func (v *OpenDataHubValidatingWebhook) Handle(ctx context.Context, req admission.Request) admission.Response { +func (w *OpenDataHubValidatingWebhook) Handle(ctx context.Context, req admission.Request) admission.Response { var resp admission.Response resp.Allowed = true // initialize Allowed to be true in case Operation falls into "default" case switch req.Operation { case admissionv1.Create: - resp = v.checkDupCreation(ctx, req) + resp = w.checkDupCreation(ctx, req) case admissionv1.Delete: - resp = v.checkDeletion(ctx, req) + resp = w.checkDeletion(ctx, req) default: // for other operations by default it is admission.Allowed("") // no-op } @@ -145,7 +144,7 @@ func (m *DSCDefaulter) SetupMutateWebhookWithManager(mgr ctrl.Manager) { func (m *DSCDefaulter) Default(_ context.Context, obj runtime.Object) error { dsc, isDSC := obj.(*dscv1.DataScienceCluster) if !isDSC { - return errors.New("expected DataScienceCluster but got a different type") + return fmt.Errorf("expected DataScienceCluster but got a different type: %T", obj) } // set default registriesNamespace if empty if len(dsc.Spec.Components.ModelRegistry.RegistriesNamespace) == 0 { diff --git a/controllers/webhook/webhook_suite_test.go b/controllers/webhook/webhook_suite_test.go index c4c26524723..57d6f61359b 100644 --- a/controllers/webhook/webhook_suite_test.go +++ b/controllers/webhook/webhook_suite_test.go @@ -136,7 +136,7 @@ var _ = BeforeSuite(func() { (&webhook.OpenDataHubValidatingWebhook{ Client: mgr.GetClient(), Decoder: admission.NewDecoder(mgr.GetScheme()), - }).SetupValidatingWebhookWithManager(mgr) + }).SetupWithManager(mgr) (&webhook.DSCDefaulter{}).SetupMutateWebhookWithManager(mgr) diff --git a/main.go b/main.go index 523d9c830ba..ebbea81c7cc 100644 --- a/main.go +++ b/main.go @@ -211,7 +211,7 @@ func main() { //nolint:funlen,maintidx (&webhook.OpenDataHubValidatingWebhook{ Client: mgr.GetClient(), Decoder: admission.NewDecoder(mgr.GetScheme()), - }).SetupValidatingWebhookWithManager(mgr) + }).SetupWithManager(mgr) (&webhook.DSCDefaulter{}).SetupMutateWebhookWithManager(mgr) From 469750b21aa2cef36130e654cd88b9fc5aaf1604 Mon Sep 17 00:00:00 2001 From: Wen Zhou Date: Wed, 18 Sep 2024 16:20:46 +0200 Subject: [PATCH 4/6] update: make it more clear for the "defaulter" case is when namespace is "" - update unit-test Signed-off-by: Wen Zhou --- controllers/webhook/webhook.go | 10 +++++-- controllers/webhook/webhook_suite_test.go | 35 +++++++++++++++++++++-- 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/controllers/webhook/webhook.go b/controllers/webhook/webhook.go index 200cc19e5a0..18a7b202e4d 100644 --- a/controllers/webhook/webhook.go +++ b/controllers/webhook/webhook.go @@ -21,6 +21,7 @@ import ( "fmt" "net/http" + operatorv1 "github.com/openshift/api/operator/v1" admissionv1 "k8s.io/api/admission/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" @@ -146,9 +147,12 @@ func (m *DSCDefaulter) Default(_ context.Context, obj runtime.Object) error { if !isDSC { return fmt.Errorf("expected DataScienceCluster but got a different type: %T", obj) } - // set default registriesNamespace if empty - if len(dsc.Spec.Components.ModelRegistry.RegistriesNamespace) == 0 { - dsc.Spec.Components.ModelRegistry.RegistriesNamespace = modelregistry.DefaultModelRegistriesNamespace + + // set default registriesNamespace if empty "" but ModelRegistry is enabled + if dsc.Spec.Components.ModelRegistry.ManagementState == operatorv1.Managed { + if dsc.Spec.Components.ModelRegistry.RegistriesNamespace == "" { + dsc.Spec.Components.ModelRegistry.RegistriesNamespace = modelregistry.DefaultModelRegistriesNamespace + } } return nil } diff --git a/controllers/webhook/webhook_suite_test.go b/controllers/webhook/webhook_suite_test.go index 57d6f61359b..bcd98d547eb 100644 --- a/controllers/webhook/webhook_suite_test.go +++ b/controllers/webhook/webhook_suite_test.go @@ -61,6 +61,7 @@ import ( const ( namespace = "webhook-test-ns" nameBase = "webhook-test" + mrNS = "model-registry-namespace" ) // These tests use Ginkgo (BDD-style Go testing framework). Refer to @@ -170,7 +171,7 @@ var _ = AfterSuite(func() { Expect(err).NotTo(HaveOccurred()) }) -var _ = Describe("DSC/DSCI webhook", func() { +var _ = Describe("DSC/DSCI validating webhook", func() { It("Should not have more than one DSCI instance in the cluster", func(ctx context.Context) { desiredDsci := newDSCI(nameBase + "-dsci-1") Expect(k8sClient.Create(ctx, desiredDsci)).Should(Succeed()) @@ -206,8 +207,12 @@ var _ = Describe("DSC/DSCI webhook", func() { Expect(k8sClient.Delete(ctx, dsciInstance)).Should(Succeed()) }) - It("Should set defaults in DSC instance", func(ctx context.Context) { - dscInstance := newDSC(nameBase+"-dsc-1", namespace) +}) + +// mutating webhook tests for model registry. +var _ = Describe("DSC mutating webhook", func() { + It("Should use defaults for DSC if empty string for MR namespace when MR is enabled", func(ctx context.Context) { + dscInstance := newMRDSC(nameBase+"-dsc-mr", "", operatorv1.Managed) Expect(k8sClient.Create(ctx, dscInstance)).Should(Succeed()) Expect(dscInstance.Spec.Components.ModelRegistry.RegistriesNamespace). Should(Equal(modelregistry.DefaultModelRegistriesNamespace)) @@ -290,6 +295,30 @@ func newDSC(name string, namespace string) *dscv1.DataScienceCluster { ManagementState: operatorv1.Removed, }, }, + ModelRegistry: modelregistry.ModelRegistry{ + Component: components.Component{ + ManagementState: operatorv1.Removed, + }, + }, + }, + }, + } +} + +func newMRDSC(name string, mrNamespace string, state operatorv1.ManagementState) *dscv1.DataScienceCluster { + return &dscv1.DataScienceCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: "appNS", + }, + Spec: dscv1.DataScienceClusterSpec{ + Components: dscv1.Components{ + ModelRegistry: modelregistry.ModelRegistry{ + Component: components.Component{ + ManagementState: state, + }, + RegistriesNamespace: mrNamespace, + }, }, }, } From 3716db905a31653d16e0c7165dc18b12ea4c9654 Mon Sep 17 00:00:00 2001 From: Wen Zhou Date: Thu, 19 Sep 2024 12:38:42 +0200 Subject: [PATCH 5/6] test: add more testcase Signed-off-by: Wen Zhou --- controllers/webhook/webhook_suite_test.go | 28 +++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/controllers/webhook/webhook_suite_test.go b/controllers/webhook/webhook_suite_test.go index bcd98d547eb..d088f98c9e6 100644 --- a/controllers/webhook/webhook_suite_test.go +++ b/controllers/webhook/webhook_suite_test.go @@ -212,12 +212,18 @@ var _ = Describe("DSC/DSCI validating webhook", func() { // mutating webhook tests for model registry. var _ = Describe("DSC mutating webhook", func() { It("Should use defaults for DSC if empty string for MR namespace when MR is enabled", func(ctx context.Context) { - dscInstance := newMRDSC(nameBase+"-dsc-mr", "", operatorv1.Managed) + dscInstance := newMRDSC1(nameBase+"-dsc-mr1", "", operatorv1.Managed) Expect(k8sClient.Create(ctx, dscInstance)).Should(Succeed()) Expect(dscInstance.Spec.Components.ModelRegistry.RegistriesNamespace). Should(Equal(modelregistry.DefaultModelRegistriesNamespace)) Expect(clearInstance(ctx, dscInstance)).Should(Succeed()) }) + + It("Should create DSC if no MR is set (for upgrade case)", func(ctx context.Context) { + dscInstance := newMRDSC2(nameBase + "-dsc-mr2") + Expect(k8sClient.Create(ctx, dscInstance)).Should(Succeed()) + Expect(clearInstance(ctx, dscInstance)).Should(Succeed()) + }) }) func clearInstance(ctx context.Context, instance client.Object) error { @@ -305,7 +311,7 @@ func newDSC(name string, namespace string) *dscv1.DataScienceCluster { } } -func newMRDSC(name string, mrNamespace string, state operatorv1.ManagementState) *dscv1.DataScienceCluster { +func newMRDSC1(name string, mrNamespace string, state operatorv1.ManagementState) *dscv1.DataScienceCluster { return &dscv1.DataScienceCluster{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -323,3 +329,21 @@ func newMRDSC(name string, mrNamespace string, state operatorv1.ManagementState) }, } } + +func newMRDSC2(name string) *dscv1.DataScienceCluster { + return &dscv1.DataScienceCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: "appNS", + }, + Spec: dscv1.DataScienceClusterSpec{ + Components: dscv1.Components{ + Workbenches: workbenches.Workbenches{ + Component: components.Component{ + ManagementState: operatorv1.Removed, + }, + }, + }, + }, + } +} From 9507a1df474d338e50389e14e76ba71a37505863 Mon Sep 17 00:00:00 2001 From: Wen Zhou Date: Thu, 19 Sep 2024 16:08:35 +0200 Subject: [PATCH 6/6] update: rename function and move comments Signed-off-by: Wen Zhou --- controllers/webhook/webhook.go | 6 +++--- controllers/webhook/webhook_suite_test.go | 2 +- main.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/controllers/webhook/webhook.go b/controllers/webhook/webhook.go index 18a7b202e4d..9455cfd08ba 100644 --- a/controllers/webhook/webhook.go +++ b/controllers/webhook/webhook.go @@ -130,18 +130,18 @@ func (w *OpenDataHubValidatingWebhook) Handle(ctx context.Context, req admission //+kubebuilder:webhook:path=/mutate-opendatahub-io-v1,mutating=true,failurePolicy=fail,sideEffects=None,groups=datasciencecluster.opendatahub.io,resources=datascienceclusters,verbs=create;update,versions=v1,name=mutate.operator.opendatahub.io,admissionReviewVersions=v1 //nolint:lll -// It currently only sets defaults for modelregiestry in datascienceclusters. type DSCDefaulter struct{} // just assert that DSCDefaulter implements webhook.CustomDefaulter. var _ webhook.CustomDefaulter = &DSCDefaulter{} -func (m *DSCDefaulter) SetupMutateWebhookWithManager(mgr ctrl.Manager) { +func (m *DSCDefaulter) SetupWithManager(mgr ctrl.Manager) { mutateWebhook := admission.WithCustomDefaulter(mgr.GetScheme(), &dscv1.DataScienceCluster{}, m) mgr.GetWebhookServer().Register("/mutate-opendatahub-io-v1", mutateWebhook) } -// implement admission.CustomDefaulter interface. +// Implement admission.CustomDefaulter interface. +// It currently only sets defaults for modelregiestry in datascienceclusters. func (m *DSCDefaulter) Default(_ context.Context, obj runtime.Object) error { dsc, isDSC := obj.(*dscv1.DataScienceCluster) if !isDSC { diff --git a/controllers/webhook/webhook_suite_test.go b/controllers/webhook/webhook_suite_test.go index d088f98c9e6..f697c5fecdf 100644 --- a/controllers/webhook/webhook_suite_test.go +++ b/controllers/webhook/webhook_suite_test.go @@ -139,7 +139,7 @@ var _ = BeforeSuite(func() { Decoder: admission.NewDecoder(mgr.GetScheme()), }).SetupWithManager(mgr) - (&webhook.DSCDefaulter{}).SetupMutateWebhookWithManager(mgr) + (&webhook.DSCDefaulter{}).SetupWithManager(mgr) // +kubebuilder:scaffold:webhook diff --git a/main.go b/main.go index ebbea81c7cc..0c086435573 100644 --- a/main.go +++ b/main.go @@ -213,7 +213,7 @@ func main() { //nolint:funlen,maintidx Decoder: admission.NewDecoder(mgr.GetScheme()), }).SetupWithManager(mgr) - (&webhook.DSCDefaulter{}).SetupMutateWebhookWithManager(mgr) + (&webhook.DSCDefaulter{}).SetupWithManager(mgr) if err = (&dscictrl.DSCInitializationReconciler{ Client: mgr.GetClient(),