From dd54b1b15ffe0afefe0f8c2a9acb2b69b0d43c11 Mon Sep 17 00:00:00 2001 From: Dhiraj Bokde Date: Wed, 4 Sep 2024 14:23:18 -0700 Subject: [PATCH 01/18] feat: added shared registries namespace property in model registry dsc component, fixes RHOAIENG-12335 Signed-off-by: Dhiraj Bokde --- components/modelregistry/modelregistry.go | 12 ++++++++---- ...cecluster.opendatahub.io_datascienceclusters.yaml | 10 ++++++++++ tests/e2e/creation_test.go | 2 +- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/components/modelregistry/modelregistry.go b/components/modelregistry/modelregistry.go index 5d0610019e5..7808147f8f5 100644 --- a/components/modelregistry/modelregistry.go +++ b/components/modelregistry/modelregistry.go @@ -35,7 +35,6 @@ var ( // modelRegistryLabels = cluster.WithLabels( // labels.ODH.OwnedNamespace, "true", // ). - ModelRegistriesNamespace = "odh-model-registries" ) // Verifies that ModelRegistry implements ComponentInterface. @@ -45,6 +44,11 @@ var _ components.ComponentInterface = (*ModelRegistry)(nil) // +kubebuilder:object:generate=true type ModelRegistry struct { components.Component `json:""` + + // Namespace for model registries to be installed, configurable once, defaults to "odh-model-registries" + // +kubebuilder:default=odh-model-registries + // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="RegistriesNamespace is immutable" + RegistriesNamespace string `json:"registriesNamespace"` } func (m *ModelRegistry) OverrideManifests(ctx context.Context, _ cluster.Platform) error { @@ -109,17 +113,17 @@ func (m *ModelRegistry) ReconcileComponent(ctx context.Context, cli client.Clien // Create model registries namespace // We do not delete this namespace even when ModelRegistry is Removed or when operator is uninstalled. - ns, err := cluster.CreateNamespace(ctx, cli, ModelRegistriesNamespace) + ns, err := cluster.CreateNamespace(ctx, cli, m.RegistriesNamespace) if err != nil { return err } - l.Info("created model registry namespace", "namespace", ModelRegistriesNamespace) + l.Info("created model registry namespace", "namespace", m.RegistriesNamespace) // create servicemeshmember here, for now until post MVP solution err = enrollToServiceMesh(ctx, cli, dscispec, ns) if err != nil { return err } - l.Info("created model registry servicemesh member", "namespace", ModelRegistriesNamespace) + l.Info("created model registry servicemesh member", "namespace", m.RegistriesNamespace) } else { err := m.removeDependencies(ctx, cli, dscispec) if err != nil { diff --git a/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml b/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml index 327c95af25a..72cb57ed8d1 100644 --- a/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml +++ b/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml @@ -443,6 +443,16 @@ spec: - Removed pattern: ^(Managed|Unmanaged|Force|Removed)$ type: string + registriesNamespace: + default: odh-model-registries + description: Namespace for model registries to be installed, + configurable once, defaults to "odh-model-registries" + type: string + x-kubernetes-validations: + - message: RegistriesNamespace is immutable + rule: self == oldSelf + required: + - registriesNamespace type: object ray: description: Ray component configuration. diff --git a/tests/e2e/creation_test.go b/tests/e2e/creation_test.go index b4406b8ee27..df7d85041f5 100644 --- a/tests/e2e/creation_test.go +++ b/tests/e2e/creation_test.go @@ -491,7 +491,7 @@ func (tc *testContext) testMRServiceMeshMember() error { smm.SetAPIVersion("maistra.io/v1") smm.SetKind("ServiceMeshMember") err := tc.customClient.Get(tc.ctx, - client.ObjectKey{Namespace: modelregistry.ModelRegistriesNamespace, Name: "default"}, &smm) + client.ObjectKey{Namespace: tc.testDsc.Spec.Components.ModelRegistry.RegistriesNamespace, Name: "default"}, &smm) if err != nil { return fmt.Errorf("failed to get servicemesh member: %w", err) } From 81a1fb1baa47f5196583b77603ac4368fcf937de Mon Sep 17 00:00:00 2001 From: Dhiraj Bokde Date: Wed, 4 Sep 2024 14:47:11 -0700 Subject: [PATCH 02/18] fix: added registriesNamespace property in references to model registry component in docs, samples, etc. Signed-off-by: Dhiraj Bokde --- README.md | 1 + .../manifests/opendatahub-operator.clusterserviceversion.yaml | 3 ++- components/modelregistry/modelregistry.go | 2 +- config/samples/datasciencecluster_v1_datasciencecluster.yaml | 1 + docs/DESIGN.md | 1 + docs/upgrade-testing.md | 1 + 6 files changed, 7 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 294b406a009..4496f1bee72 100644 --- a/README.md +++ b/README.md @@ -327,6 +327,7 @@ spec: managementState: Managed modelregistry: managementState: Managed + registriesNamespace: "odh-model-registries" ray: managementState: Managed trainingoperator: diff --git a/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml b/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml index e7c9b7ec869..dab7f84994b 100644 --- a/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml +++ b/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml @@ -47,7 +47,8 @@ metadata: "managementState": "Managed" }, "modelregistry": { - "managementState": "Managed" + "managementState": "Managed", + "registriesNamespace": "odh-model-registries" }, "ray": { "managementState": "Managed" diff --git a/components/modelregistry/modelregistry.go b/components/modelregistry/modelregistry.go index 7808147f8f5..e13ee380e44 100644 --- a/components/modelregistry/modelregistry.go +++ b/components/modelregistry/modelregistry.go @@ -46,7 +46,7 @@ type ModelRegistry struct { components.Component `json:""` // Namespace for model registries to be installed, configurable once, defaults to "odh-model-registries" - // +kubebuilder:default=odh-model-registries + // +kubebuilder:default="odh-model-registries" // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="RegistriesNamespace is immutable" RegistriesNamespace string `json:"registriesNamespace"` } diff --git a/config/samples/datasciencecluster_v1_datasciencecluster.yaml b/config/samples/datasciencecluster_v1_datasciencecluster.yaml index ed5971d37e6..626fc5442d1 100644 --- a/config/samples/datasciencecluster_v1_datasciencecluster.yaml +++ b/config/samples/datasciencecluster_v1_datasciencecluster.yaml @@ -42,3 +42,4 @@ spec: managementState: "Managed" modelregistry: managementState: "Managed" + registriesNamespace: "odh-model-registries" diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 0e82ac0acb4..148517713b8 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -64,6 +64,7 @@ To deploy ODH components seamlessly, ODH operator will watch two CRDs: managementState: Managed modelregistry: managementState: Managed + registriesNamespace: "odh-model-registries" ray: managementState: Managed kueue: diff --git a/docs/upgrade-testing.md b/docs/upgrade-testing.md index ce9bd1ef0e4..d40d38f9418 100644 --- a/docs/upgrade-testing.md +++ b/docs/upgrade-testing.md @@ -138,6 +138,7 @@ spec: managementState: Managed modelregistry: managementState: Managed + registriesNamespace: "odh-model-registries" ray: managementState: Managed kueue: From 1c034dc2fcdf2e5b737fac55a14f70a8d1b51f31 Mon Sep 17 00:00:00 2001 From: Dhiraj Bokde Date: Wed, 4 Sep 2024 15:00:22 -0700 Subject: [PATCH 03/18] fix: updated api-overview.md Signed-off-by: Dhiraj Bokde --- docs/api-overview.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/api-overview.md b/docs/api-overview.md index c909cee2b11..064eb81cb15 100644 --- a/docs/api-overview.md +++ b/docs/api-overview.md @@ -251,6 +251,7 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | | `Component` _[Component](#component)_ | | | | +| `registriesNamespace` _string_ | Namespace for model registries to be installed, configurable once, defaults to "odh-model-registries" | odh-model-registries | | From 3c6cc938da9e17dba4c2809c7602e856d64d30f4 Mon Sep 17 00:00:00 2001 From: Dhiraj Bokde Date: Thu, 5 Sep 2024 08:50:41 -0700 Subject: [PATCH 04/18] fix: add missing field in setupDSCInstance() Signed-off-by: Dhiraj Bokde --- tests/e2e/creation_test.go | 2 +- tests/e2e/helper_test.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/e2e/creation_test.go b/tests/e2e/creation_test.go index df7d85041f5..d21ed308e1a 100644 --- a/tests/e2e/creation_test.go +++ b/tests/e2e/creation_test.go @@ -291,7 +291,7 @@ func (tc *testContext) testAllComponentCreation(t *testing.T) error { //nolint:f t.Run("Validate "+name, func(t *testing.T) { t.Parallel() err = tc.testComponentCreation(c) - require.NoError(t, err, "error validating component %v when "+c.GetManagementState()) + require.NoError(t, err, "error validating component %s when %v", name, c.GetManagementState()) }) } return nil diff --git a/tests/e2e/helper_test.go b/tests/e2e/helper_test.go index 7ea367207b5..39e5d770a15 100644 --- a/tests/e2e/helper_test.go +++ b/tests/e2e/helper_test.go @@ -171,6 +171,7 @@ func setupDSCInstance(name string) *dscv1.DataScienceCluster { Component: components.Component{ ManagementState: operatorv1.Managed, }, + RegistriesNamespace: "odh-model-registries", }, TrainingOperator: trainingoperator.TrainingOperator{ Component: components.Component{ From 5f1f1362b781166ff03d67266358fd24b3dc2f56 Mon Sep 17 00:00:00 2001 From: Dhiraj Bokde Date: Fri, 6 Sep 2024 18:30:41 -0700 Subject: [PATCH 05/18] fix: added debug log for failing e2e test --- tests/e2e/creation_test.go | 1 + tests/e2e/helper_test.go | 1 + 2 files changed, 2 insertions(+) diff --git a/tests/e2e/creation_test.go b/tests/e2e/creation_test.go index d21ed308e1a..3a7e8a0653b 100644 --- a/tests/e2e/creation_test.go +++ b/tests/e2e/creation_test.go @@ -490,6 +490,7 @@ func (tc *testContext) testMRServiceMeshMember() error { smm := unstructured.Unstructured{} smm.SetAPIVersion("maistra.io/v1") smm.SetKind("ServiceMeshMember") + log.Printf("***debug testMRServiceMeshMember RegistriesNamespace: %s", tc.testDsc.Spec.Components.ModelRegistry.RegistriesNamespace) err := tc.customClient.Get(tc.ctx, client.ObjectKey{Namespace: tc.testDsc.Spec.Components.ModelRegistry.RegistriesNamespace, Name: "default"}, &smm) if err != nil { diff --git a/tests/e2e/helper_test.go b/tests/e2e/helper_test.go index 39e5d770a15..6679d1f5ff9 100644 --- a/tests/e2e/helper_test.go +++ b/tests/e2e/helper_test.go @@ -181,6 +181,7 @@ func setupDSCInstance(name string) *dscv1.DataScienceCluster { }, }, } + log.Printf("***debug setupDSCInstance RegistriesNamespace: %s", dscTest.Spec.Components.ModelRegistry.RegistriesNamespace) return dscTest } From 8d0aad6d2b850183ab8cc98e016c94e4587d239a Mon Sep 17 00:00:00 2001 From: Dhiraj Bokde Date: Sat, 7 Sep 2024 19:30:20 -0700 Subject: [PATCH 06/18] debug: added log messages in test dsc creation Signed-off-by: Dhiraj Bokde --- tests/e2e/creation_test.go | 18 +++++++++++++++--- tests/e2e/helper_test.go | 4 +++- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/tests/e2e/creation_test.go b/tests/e2e/creation_test.go index 3a7e8a0653b..6c97f1b616c 100644 --- a/tests/e2e/creation_test.go +++ b/tests/e2e/creation_test.go @@ -21,6 +21,7 @@ import ( "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/util/retry" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/yaml" dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1" dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1" @@ -59,7 +60,7 @@ func creationTestSuite(t *testing.T) { // DSC t.Run("Creation of DataScienceCluster instance", func(t *testing.T) { - err = testCtx.testDSCCreation() + err = testCtx.testDSCCreation(t) require.NoError(t, err, "error creating DataScienceCluster instance") }) t.Run("Creation of more than one of DataScienceCluster instance", func(t *testing.T) { @@ -150,7 +151,8 @@ func (tc *testContext) testDSCICreation() error { return nil } -func (tc *testContext) testDSCCreation() error { +func (tc *testContext) testDSCCreation(t *testing.T) error { + t.Helper() // Create DataScienceCluster resource if not already created dscLookupKey := types.NamespacedName{Name: tc.testDsc.Name} @@ -162,6 +164,7 @@ func (tc *testContext) testDSCCreation() error { if len(existingDSCList.Items) > 0 { // Use DSC instance if it already exists tc.testDsc = &existingDSCList.Items[0] + t.Logf("Using existing e2e-test-dsc DSC %s", tc.testDsc.Name) return nil } @@ -182,10 +185,18 @@ func (tc *testContext) testDSCCreation() error { if dsciErr != nil { return fmt.Errorf("error creating e2e-test-dsc DSC %s: %w", tc.testDsc.Name, dsciErr) } + t.Logf("created e2e-test-dsc DSC %s", tc.testDsc.Name) } else { return fmt.Errorf("error getting e2e-test-dsc DSC %s: %w", tc.testDsc.Name, err) } } + + // print test DSC yaml to help debug using test log + bytes, err := yaml.Marshal(tc.testDsc) + if err != nil { + return fmt.Errorf("error marshaling e2e-test-dsc DSC %s: %w", tc.testDsc.Name, err) + } + t.Logf("e2e-test-dsc DSC %s:\n%s", tc.testDsc.Name, string(bytes)) return nil } @@ -274,10 +285,12 @@ func (tc *testContext) testAllComponentCreation(t *testing.T) error { //nolint:f // Wait for components to get deployed time.Sleep(1 * time.Minute) + t.Logf("*** debug input RegistriesNamespace %s", tc.testDsc.Spec.Components.ModelRegistry.RegistriesNamespace) err := tc.customClient.Get(tc.ctx, dscLookupKey, createdDSC) if err != nil { return fmt.Errorf("error getting DataScienceCluster instance :%v", tc.testDsc.Name) } + t.Logf("*** debug output RegistriesNamespace %s", createdDSC.Spec.Components.ModelRegistry.RegistriesNamespace) tc.testDsc = createdDSC components, err := tc.testDsc.GetComponents() @@ -490,7 +503,6 @@ func (tc *testContext) testMRServiceMeshMember() error { smm := unstructured.Unstructured{} smm.SetAPIVersion("maistra.io/v1") smm.SetKind("ServiceMeshMember") - log.Printf("***debug testMRServiceMeshMember RegistriesNamespace: %s", tc.testDsc.Spec.Components.ModelRegistry.RegistriesNamespace) err := tc.customClient.Get(tc.ctx, client.ObjectKey{Namespace: tc.testDsc.Spec.Components.ModelRegistry.RegistriesNamespace, Name: "default"}, &smm) if err != nil { diff --git a/tests/e2e/helper_test.go b/tests/e2e/helper_test.go index 6679d1f5ff9..4fa142d5941 100644 --- a/tests/e2e/helper_test.go +++ b/tests/e2e/helper_test.go @@ -171,7 +171,6 @@ func setupDSCInstance(name string) *dscv1.DataScienceCluster { Component: components.Component{ ManagementState: operatorv1.Managed, }, - RegistriesNamespace: "odh-model-registries", }, TrainingOperator: trainingoperator.TrainingOperator{ Component: components.Component{ @@ -181,6 +180,9 @@ func setupDSCInstance(name string) *dscv1.DataScienceCluster { }, }, } + + // set default values like RegistriesNamespace + scheme.Default(dscTest) log.Printf("***debug setupDSCInstance RegistriesNamespace: %s", dscTest.Spec.Components.ModelRegistry.RegistriesNamespace) return dscTest From 16bcbca9e2c598d8c05c9c24ec9b3b365920de67 Mon Sep 17 00:00:00 2001 From: Dhiraj Bokde Date: Sat, 7 Sep 2024 23:20:25 -0700 Subject: [PATCH 07/18] debug: make registriesNamespace omitempty so apiserver will set the default value, clean testDSCCreation with more log messages Signed-off-by: Dhiraj Bokde --- components/modelregistry/modelregistry.go | 2 +- ...tasciencecluster.opendatahub.io_datascienceclusters.yaml | 2 -- tests/e2e/creation_test.go | 6 +++--- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/components/modelregistry/modelregistry.go b/components/modelregistry/modelregistry.go index e13ee380e44..ebaf814fc58 100644 --- a/components/modelregistry/modelregistry.go +++ b/components/modelregistry/modelregistry.go @@ -48,7 +48,7 @@ type ModelRegistry struct { // Namespace for model registries to be installed, configurable once, defaults to "odh-model-registries" // +kubebuilder:default="odh-model-registries" // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="RegistriesNamespace is immutable" - RegistriesNamespace string `json:"registriesNamespace"` + RegistriesNamespace string `json:"registriesNamespace,omitempty"` } func (m *ModelRegistry) OverrideManifests(ctx context.Context, _ cluster.Platform) error { diff --git a/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml b/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml index 72cb57ed8d1..b21de834b90 100644 --- a/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml +++ b/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml @@ -451,8 +451,6 @@ spec: x-kubernetes-validations: - message: RegistriesNamespace is immutable rule: self == oldSelf - required: - - registriesNamespace type: object ray: description: Ray component configuration. diff --git a/tests/e2e/creation_test.go b/tests/e2e/creation_test.go index 6c97f1b616c..96edbb5419f 100644 --- a/tests/e2e/creation_test.go +++ b/tests/e2e/creation_test.go @@ -155,10 +155,7 @@ func (tc *testContext) testDSCCreation(t *testing.T) error { t.Helper() // Create DataScienceCluster resource if not already created - dscLookupKey := types.NamespacedName{Name: tc.testDsc.Name} - createdDSC := &dscv1.DataScienceCluster{} existingDSCList := &dscv1.DataScienceClusterList{} - err := tc.customClient.List(tc.ctx, existingDSCList) if err == nil { if len(existingDSCList.Items) > 0 { @@ -169,6 +166,9 @@ func (tc *testContext) testDSCCreation(t *testing.T) error { return nil } } + + dscLookupKey := types.NamespacedName{Name: tc.testDsc.Name} + createdDSC := &dscv1.DataScienceCluster{} err = tc.customClient.Get(tc.ctx, dscLookupKey, createdDSC) if err != nil { if k8serr.IsNotFound(err) { From dc305cd8c941d6c824bfefbff16e2224b7e8bd9a Mon Sep 17 00:00:00 2001 From: Wen Zhou Date: Mon, 9 Sep 2024 10:58:24 +0200 Subject: [PATCH 08/18] update: modelregistry with namespace - add namespace for rhoai case - remove omitemptry on namespace: if user does not set it, it will get default value to use. since its type is string Signed-off-by: Wen Zhou --- ...encecluster.opendatahub.io_datascienceclusters.yaml | 10 ++++++++++ components/modelregistry/modelregistry.go | 2 +- ...encecluster.opendatahub.io_datascienceclusters.yaml | 2 ++ main.go | 4 ++-- 4 files changed, 15 insertions(+), 3 deletions(-) diff --git a/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml b/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml index 761d550a5dc..f61012c70bd 100644 --- a/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml +++ b/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml @@ -443,6 +443,16 @@ spec: - Removed pattern: ^(Managed|Unmanaged|Force|Removed)$ type: string + registriesNamespace: + default: odh-model-registries + description: Namespace for model registries to be installed, + configurable once, defaults to "odh-model-registries" + type: string + x-kubernetes-validations: + - message: RegistriesNamespace is immutable + rule: self == oldSelf + required: + - registriesNamespace type: object ray: description: Ray component configuration. diff --git a/components/modelregistry/modelregistry.go b/components/modelregistry/modelregistry.go index ebaf814fc58..e13ee380e44 100644 --- a/components/modelregistry/modelregistry.go +++ b/components/modelregistry/modelregistry.go @@ -48,7 +48,7 @@ type ModelRegistry struct { // Namespace for model registries to be installed, configurable once, defaults to "odh-model-registries" // +kubebuilder:default="odh-model-registries" // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="RegistriesNamespace is immutable" - RegistriesNamespace string `json:"registriesNamespace,omitempty"` + RegistriesNamespace string `json:"registriesNamespace"` } func (m *ModelRegistry) OverrideManifests(ctx context.Context, _ cluster.Platform) error { diff --git a/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml b/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml index b21de834b90..72cb57ed8d1 100644 --- a/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml +++ b/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml @@ -451,6 +451,8 @@ spec: x-kubernetes-validations: - message: RegistriesNamespace is immutable rule: self == oldSelf + required: + - registriesNamespace type: object ray: description: Ray component configuration. diff --git a/main.go b/main.go index 8bf8cba1910..52fd181ef94 100644 --- a/main.go +++ b/main.go @@ -342,10 +342,10 @@ func createDeploymentCacheConfig(platform cluster.Platform) map[string]cache.Con case cluster.ManagedRhods: // no need workbench NS, only SFS no Deployment namespaceConfigs["redhat-ods-monitoring"] = cache.Config{} namespaceConfigs["redhat-ods-applications"] = cache.Config{} - //TODO: if ModelReg has a RHOAI NS + namespaceConfigs["rhoai-model-registries"] = cache.Config{} case cluster.SelfManagedRhods: namespaceConfigs["redhat-ods-applications"] = cache.Config{} - //TODO: if ModelReg has a RHOAI NS + namespaceConfigs["rhoai-model-registries"] = cache.Config{} default: namespaceConfigs["opendatahub"] = cache.Config{} namespaceConfigs["odh-model-registries"] = cache.Config{} From 7a6709f9875229cf7ad07824eb27fa679e71eaa8 Mon Sep 17 00:00:00 2001 From: Wen Zhou Date: Mon, 9 Sep 2024 14:41:29 +0200 Subject: [PATCH 09/18] fix: test need to update for modelreg namespace Signed-off-by: Wen Zhou --- tests/e2e/helper_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/e2e/helper_test.go b/tests/e2e/helper_test.go index 4fa142d5941..9ab425bc389 100644 --- a/tests/e2e/helper_test.go +++ b/tests/e2e/helper_test.go @@ -171,6 +171,7 @@ func setupDSCInstance(name string) *dscv1.DataScienceCluster { Component: components.Component{ ManagementState: operatorv1.Managed, }, + RegistriesNamespace: "odh-model-registries", }, TrainingOperator: trainingoperator.TrainingOperator{ Component: components.Component{ From 6e7bfe7cef7a8097e1470c7eded5884747be6734 Mon Sep 17 00:00:00 2001 From: Wen Zhou Date: Mon, 9 Sep 2024 17:24:10 +0200 Subject: [PATCH 10/18] update: cleanup debug info in test cases Signed-off-by: Wen Zhou --- tests/e2e/creation_test.go | 13 ------------- tests/e2e/helper_test.go | 3 --- 2 files changed, 16 deletions(-) diff --git a/tests/e2e/creation_test.go b/tests/e2e/creation_test.go index 96edbb5419f..fe4cee20c52 100644 --- a/tests/e2e/creation_test.go +++ b/tests/e2e/creation_test.go @@ -21,7 +21,6 @@ import ( "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/util/retry" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/yaml" dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1" dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1" @@ -161,8 +160,6 @@ func (tc *testContext) testDSCCreation(t *testing.T) error { if len(existingDSCList.Items) > 0 { // Use DSC instance if it already exists tc.testDsc = &existingDSCList.Items[0] - t.Logf("Using existing e2e-test-dsc DSC %s", tc.testDsc.Name) - return nil } } @@ -185,18 +182,10 @@ func (tc *testContext) testDSCCreation(t *testing.T) error { if dsciErr != nil { return fmt.Errorf("error creating e2e-test-dsc DSC %s: %w", tc.testDsc.Name, dsciErr) } - t.Logf("created e2e-test-dsc DSC %s", tc.testDsc.Name) } else { return fmt.Errorf("error getting e2e-test-dsc DSC %s: %w", tc.testDsc.Name, err) } } - - // print test DSC yaml to help debug using test log - bytes, err := yaml.Marshal(tc.testDsc) - if err != nil { - return fmt.Errorf("error marshaling e2e-test-dsc DSC %s: %w", tc.testDsc.Name, err) - } - t.Logf("e2e-test-dsc DSC %s:\n%s", tc.testDsc.Name, string(bytes)) return nil } @@ -285,12 +274,10 @@ func (tc *testContext) testAllComponentCreation(t *testing.T) error { //nolint:f // Wait for components to get deployed time.Sleep(1 * time.Minute) - t.Logf("*** debug input RegistriesNamespace %s", tc.testDsc.Spec.Components.ModelRegistry.RegistriesNamespace) err := tc.customClient.Get(tc.ctx, dscLookupKey, createdDSC) if err != nil { return fmt.Errorf("error getting DataScienceCluster instance :%v", tc.testDsc.Name) } - t.Logf("*** debug output RegistriesNamespace %s", createdDSC.Spec.Components.ModelRegistry.RegistriesNamespace) tc.testDsc = createdDSC components, err := tc.testDsc.GetComponents() diff --git a/tests/e2e/helper_test.go b/tests/e2e/helper_test.go index 9ab425bc389..fdae18bcfc8 100644 --- a/tests/e2e/helper_test.go +++ b/tests/e2e/helper_test.go @@ -74,9 +74,7 @@ func (tc *testContext) waitForOperatorDeployment(name string, replicas int32) er } } } - log.Printf("Error in %s deployment", name) - return false, nil }) @@ -184,7 +182,6 @@ func setupDSCInstance(name string) *dscv1.DataScienceCluster { // set default values like RegistriesNamespace scheme.Default(dscTest) - log.Printf("***debug setupDSCInstance RegistriesNamespace: %s", dscTest.Spec.Components.ModelRegistry.RegistriesNamespace) return dscTest } From c0619d7768bde6c57339fabc39f3d4e289a914a4 Mon Sep 17 00:00:00 2001 From: Dhiraj Bokde Date: Mon, 9 Sep 2024 14:12:10 -0700 Subject: [PATCH 11/18] fix: make registriesNamespace immutability check dynamic in cel validation rule --- components/modelregistry/modelregistry.go | 10 +++++++--- ...cecluster.opendatahub.io_datascienceclusters.yaml | 12 ++++++++---- docs/api-overview.md | 4 ++-- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/components/modelregistry/modelregistry.go b/components/modelregistry/modelregistry.go index e13ee380e44..086c4d37b39 100644 --- a/components/modelregistry/modelregistry.go +++ b/components/modelregistry/modelregistry.go @@ -41,14 +41,18 @@ var ( var _ components.ComponentInterface = (*ModelRegistry)(nil) // ModelRegistry struct holds the configuration for the ModelRegistry component. +// The property `registriesNamespace` is immutable when management state was not equal to `Removed` // +kubebuilder:object:generate=true +//+kubebuilder:validation:XValidation:rule="(self.managementState == 'Removed' || oldSelf.managementState == 'Removed') || (self.managementState != 'Removed' && self.registriesNamespace == oldSelf.registriesNamespace)",message="RegistriesNamespace is immutable when ManagementState was not equal to Removed" +//nolint:lll + type ModelRegistry struct { components.Component `json:""` - // Namespace for model registries to be installed, configurable once, defaults to "odh-model-registries" + // Namespace for model registries to be installed, configurable once when model registry is enabled, defaults to "odh-model-registries" + // +kubebuilder:validation:Required // +kubebuilder:default="odh-model-registries" - // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="RegistriesNamespace is immutable" - RegistriesNamespace string `json:"registriesNamespace"` + RegistriesNamespace string `json:"registriesNamespace,omitempty"` } func (m *ModelRegistry) OverrideManifests(ctx context.Context, _ cluster.Platform) error { diff --git a/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml b/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml index 72cb57ed8d1..47c382ef59f 100644 --- a/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml +++ b/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml @@ -446,14 +446,18 @@ spec: registriesNamespace: default: odh-model-registries description: Namespace for model registries to be installed, - configurable once, defaults to "odh-model-registries" + configurable once when model registry is enabled, defaults + to "odh-model-registries" type: string - x-kubernetes-validations: - - message: RegistriesNamespace is immutable - rule: self == oldSelf required: - registriesNamespace type: object + x-kubernetes-validations: + - message: RegistriesNamespace is immutable when ManagementState + was not equal to Removed + rule: (self.managementState == 'Removed' || oldSelf.managementState + == 'Removed') || (self.managementState != 'Removed' && self.registriesNamespace + == oldSelf.registriesNamespace) ray: description: Ray component configuration. properties: diff --git a/docs/api-overview.md b/docs/api-overview.md index 064eb81cb15..9f5183f27c6 100644 --- a/docs/api-overview.md +++ b/docs/api-overview.md @@ -241,7 +241,7 @@ Package modelregistry provides utility functions to config ModelRegistry, an ML -ModelRegistry struct holds the configuration for the ModelRegistry component. + @@ -251,7 +251,7 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | | `Component` _[Component](#component)_ | | | | -| `registriesNamespace` _string_ | Namespace for model registries to be installed, configurable once, defaults to "odh-model-registries" | odh-model-registries | | +| `registriesNamespace` _string_ | Namespace for model registries to be installed, configurable once when model registry is enabled, defaults to "odh-model-registries" | odh-model-registries | Required: \{\}
| From 2d4fc5ba31febca4233ff5bec901e8cff6d1e41c Mon Sep 17 00:00:00 2001 From: Dhiraj Bokde Date: Mon, 9 Sep 2024 23:31:57 -0700 Subject: [PATCH 12/18] fix: refactor validation rule, set registriesNamespace to optional, add patch func in modelregistry to set default value --- components/modelregistry/modelregistry.go | 42 ++++++++++++--- ...er.opendatahub.io_datascienceclusters.yaml | 12 ++--- docs/api-overview.md | 2 +- tests/e2e/creation_test.go | 53 ++++++++++++++++++- tests/e2e/helper_test.go | 4 -- 5 files changed, 94 insertions(+), 19 deletions(-) diff --git a/components/modelregistry/modelregistry.go b/components/modelregistry/modelregistry.go index 086c4d37b39..8031ca5543c 100644 --- a/components/modelregistry/modelregistry.go +++ b/components/modelregistry/modelregistry.go @@ -14,6 +14,9 @@ import ( operatorv1 "github.com/openshift/api/operator/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1" @@ -29,8 +32,9 @@ import ( const DefaultModelRegistryCert = "default-modelregistry-cert" var ( - ComponentName = "model-registry-operator" - Path = deploy.DefaultManifestPath + "/" + ComponentName + "/overlays/odh" + ComponentName = "model-registry-operator" + DefaultModelRegistriesNamespace = "odh-model-registries" + Path = deploy.DefaultManifestPath + "/" + ComponentName + "/overlays/odh" // we should not apply this label to the namespace, as it triggered namspace deletion during operator uninstall // modelRegistryLabels = cluster.WithLabels( // labels.ODH.OwnedNamespace, "true", @@ -41,16 +45,15 @@ var ( var _ components.ComponentInterface = (*ModelRegistry)(nil) // ModelRegistry struct holds the configuration for the ModelRegistry component. -// The property `registriesNamespace` is immutable when management state was not equal to `Removed` +// The property `registriesNamespace` is immutable when `managementState` is `Managed` // +kubebuilder:object:generate=true -//+kubebuilder:validation:XValidation:rule="(self.managementState == 'Removed' || oldSelf.managementState == 'Removed') || (self.managementState != 'Removed' && self.registriesNamespace == oldSelf.registriesNamespace)",message="RegistriesNamespace is immutable when ManagementState was not equal to Removed" +// +kubebuilder:validation:XValidation:rule="self.managementState != 'Managed' || (oldSelf.registriesNamespace == '' && self.registriesNamespace != '') || (self.registriesNamespace == oldSelf.registriesNamespace)",message="RegistriesNamespace is immutable when model registry is Managed" //nolint:lll type ModelRegistry struct { components.Component `json:""` - // Namespace for model registries to be installed, configurable once when model registry is enabled, defaults to "odh-model-registries" - // +kubebuilder:validation:Required + // Namespace for model registries to be installed, configurable only once when model registry is enabled, defaults to "odh-model-registries" // +kubebuilder:default="odh-model-registries" RegistriesNamespace string `json:"registriesNamespace,omitempty"` } @@ -94,6 +97,14 @@ func (m *ModelRegistry) ReconcileComponent(ctx context.Context, cli client.Clien return errors.New("ServiceMesh needs to be set to 'Managed' in DSCI CR, it is required by Model Registry") } + // if registriesNamespace is not set, patch it to default value to allow omitempty registriesNamespace + if len(m.RegistriesNamespace) == 0 { + err := m.setDefaultRegistriesNamespace(ctx, cli, l, owner) + if err != nil { + return fmt.Errorf("failed to patch registriesNamespace for DSC %s : %w", owner.GetName(), err) + } + } + if err := m.createDependencies(ctx, cli, dscispec); err != nil { return err } @@ -169,6 +180,25 @@ func (m *ModelRegistry) ReconcileComponent(ctx context.Context, cli client.Clien return nil } +func (m *ModelRegistry) setDefaultRegistriesNamespace(ctx context.Context, cli client.Client, l logr.Logger, owner metav1.Object) error { + m.RegistriesNamespace = DefaultModelRegistriesNamespace + + patch := GetRegistriesNamespacePatch(m.RegistriesNamespace) + l.Info("Patching empty registriesNamespace", "DSC", owner.GetName(), "registriesNamespace", m.RegistriesNamespace, "patch", patch) + // have to use unstructured to avoid circular references with DSC type + obj := unstructured.Unstructured{} + obj.SetGroupVersionKind(schema.GroupVersionKind{Group: "datasciencecluster.opendatahub.io", Version: "v1", Kind: "DataScienceCluster"}) + obj.SetName(owner.GetName()) + obj.SetNamespace(owner.GetNamespace()) + return cli.Patch(ctx, &obj, patch) +} + +//nolint:ireturn +func GetRegistriesNamespacePatch(namespace string) client.Patch { + patchStr := fmt.Sprintf("{\"spec\":{\"components\":{\"modelregistry\":{\"registriesNamespace\":\"%s\"}}}}", namespace) + return client.RawPatch(types.MergePatchType, []byte(patchStr)) +} + func (m *ModelRegistry) createDependencies(ctx context.Context, cli client.Client, dscispec *dsciv1.DSCInitializationSpec) error { // create DefaultModelRegistryCert if err := cluster.PropagateDefaultIngressCertificate(ctx, cli, DefaultModelRegistryCert, dscispec.ServiceMesh.ControlPlane.Namespace); err != nil { diff --git a/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml b/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml index 47c382ef59f..3b9e4cc7eca 100644 --- a/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml +++ b/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml @@ -446,17 +446,15 @@ spec: registriesNamespace: default: odh-model-registries description: Namespace for model registries to be installed, - configurable once when model registry is enabled, defaults + configurable only once when model registry is enabled, defaults to "odh-model-registries" type: string - required: - - registriesNamespace type: object x-kubernetes-validations: - - message: RegistriesNamespace is immutable when ManagementState - was not equal to Removed - rule: (self.managementState == 'Removed' || oldSelf.managementState - == 'Removed') || (self.managementState != 'Removed' && self.registriesNamespace + - message: RegistriesNamespace is immutable when model registry + is Managed + rule: self.managementState != 'Managed' || (oldSelf.registriesNamespace + == '' && self.registriesNamespace != '') || (self.registriesNamespace == oldSelf.registriesNamespace) ray: description: Ray component configuration. diff --git a/docs/api-overview.md b/docs/api-overview.md index 9f5183f27c6..44174f56b45 100644 --- a/docs/api-overview.md +++ b/docs/api-overview.md @@ -251,7 +251,7 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | | `Component` _[Component](#component)_ | | | | -| `registriesNamespace` _string_ | Namespace for model registries to be installed, configurable once when model registry is enabled, defaults to "odh-model-registries" | odh-model-registries | Required: \{\}
| +| `registriesNamespace` _string_ | Namespace for model registries to be installed, configurable only once when model registry is enabled, defaults to "odh-model-registries" | odh-model-registries | | diff --git a/tests/e2e/creation_test.go b/tests/e2e/creation_test.go index fe4cee20c52..f9674116230 100644 --- a/tests/e2e/creation_test.go +++ b/tests/e2e/creation_test.go @@ -91,6 +91,10 @@ func creationTestSuite(t *testing.T) { }) // ModelReg + t.Run("Validate model registry cert config", func(t *testing.T) { + err = testCtx.validateModelRegistryConfig() + require.NoError(t, err, "error validating ModelRegistry config") + }) t.Run("Validate default model registry cert available", func(t *testing.T) { err = testCtx.testDefaultModelRegistryCertAvailable() require.NoError(t, err, "error getting default cert secret for ModelRegistry") @@ -491,7 +495,7 @@ func (tc *testContext) testMRServiceMeshMember() error { smm.SetAPIVersion("maistra.io/v1") smm.SetKind("ServiceMeshMember") err := tc.customClient.Get(tc.ctx, - client.ObjectKey{Namespace: tc.testDsc.Spec.Components.ModelRegistry.RegistriesNamespace, Name: "default"}, &smm) + client.ObjectKey{Namespace: modelregistry.DefaultModelRegistriesNamespace, Name: "default"}, &smm) if err != nil { return fmt.Errorf("failed to get servicemesh member: %w", err) } @@ -607,3 +611,50 @@ func (tc *testContext) testUpdateDSCComponentEnabled() error { dashboardDeploymentName, tc.applicationsNamespace) } + +const testNs = "test-model-registries" + +func (tc *testContext) validateModelRegistryConfig() error { + // check immutable property registriesNamespace + if tc.testDsc.Spec.Components.ModelRegistry.ManagementState != operatorv1.Managed { + // allowed to set registriesNamespace to non-default + err := patchRegistriesNamespace(tc, testNs, testNs, false) + if err != nil { + return err + } + // allowed to set registriesNamespace back to default value + err = patchRegistriesNamespace(tc, modelregistry.DefaultModelRegistriesNamespace, + modelregistry.DefaultModelRegistriesNamespace, false) + if err != nil { + return err + } + } else { + // not allowed to change registriesNamespace + err := patchRegistriesNamespace(tc, testNs, modelregistry.DefaultModelRegistriesNamespace, true) + if err != nil { + return err + } + } + return nil +} + +func patchRegistriesNamespace(tc *testContext, namespace string, expected string, expectErr bool) error { + err := tc.customClient.Patch(tc.ctx, tc.testDsc, modelregistry.GetRegistriesNamespacePatch(namespace)) + if err != nil { + if !expectErr { + return fmt.Errorf("unexpected error when setting registriesNamespace in DSC %s to %s: %w", + tc.testDsc.Name, namespace, err) + } + } else { + if expectErr { + return fmt.Errorf("unexpected success when setting registriesNamespace in DSC %s to %s", + tc.testDsc.Name, namespace) + } + } + // compare expected against returned registriesNamespace + if tc.testDsc.Spec.Components.ModelRegistry.RegistriesNamespace != expected { + return fmt.Errorf("expected registriesNamespace %s, got %s", + expected, tc.testDsc.Spec.Components.ModelRegistry.RegistriesNamespace) + } + return nil +} diff --git a/tests/e2e/helper_test.go b/tests/e2e/helper_test.go index fdae18bcfc8..83c222cde19 100644 --- a/tests/e2e/helper_test.go +++ b/tests/e2e/helper_test.go @@ -169,7 +169,6 @@ func setupDSCInstance(name string) *dscv1.DataScienceCluster { Component: components.Component{ ManagementState: operatorv1.Managed, }, - RegistriesNamespace: "odh-model-registries", }, TrainingOperator: trainingoperator.TrainingOperator{ Component: components.Component{ @@ -180,9 +179,6 @@ func setupDSCInstance(name string) *dscv1.DataScienceCluster { }, } - // set default values like RegistriesNamespace - scheme.Default(dscTest) - return dscTest } From 11b11c6130fad0ee41f821749464287dbe1eac3b Mon Sep 17 00:00:00 2001 From: Dhiraj Bokde Date: Tue, 10 Sep 2024 19:42:20 -0700 Subject: [PATCH 13/18] fix: add defaulting webhook including envtest, remove patch from modelregistry component --- PROJECT | 1 + ...er.opendatahub.io_datascienceclusters.yaml | 14 +++-- ...atahub-operator.clusterserviceversion.yaml | 22 +++++++- components/modelregistry/modelregistry.go | 31 +---------- config/webhook/kustomizeconfig.yaml | 7 +++ config/webhook/manifests.yaml | 26 +++++++++ controllers/webhook/webhook.go | 55 +++++++++++++++++-- controllers/webhook/webhook_suite_test.go | 16 +++++- main.go | 7 ++- tests/e2e/creation_test.go | 3 +- 10 files changed, 137 insertions(+), 45 deletions(-) diff --git a/PROJECT b/PROJECT index 8619f2be939..e321d617330 100644 --- a/PROJECT +++ b/PROJECT @@ -34,6 +34,7 @@ resources: path: github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1 version: v1 webhooks: + defaulting: true validation: true webhookVersion: v1 version: "3" diff --git a/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml b/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml index f61012c70bd..0f7d2f2fc43 100644 --- a/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml +++ b/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml @@ -446,14 +446,16 @@ spec: registriesNamespace: default: odh-model-registries description: Namespace for model registries to be installed, - configurable once, defaults to "odh-model-registries" + configurable only once when model registry is enabled, defaults + to "odh-model-registries" type: string - x-kubernetes-validations: - - message: RegistriesNamespace is immutable - rule: self == oldSelf - required: - - registriesNamespace type: object + x-kubernetes-validations: + - message: RegistriesNamespace is immutable when model registry + is Managed + rule: self.managementState != 'Managed' || (oldSelf.registriesNamespace + == '' && self.registriesNamespace != '') || (self.registriesNamespace + == oldSelf.registriesNamespace) ray: description: Ray component configuration. properties: diff --git a/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml b/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml index dab7f84994b..96f5397b6d9 100644 --- a/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml +++ b/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml @@ -103,7 +103,7 @@ metadata: categories: AI/Machine Learning, Big Data certified: "False" containerImage: quay.io/opendatahub/opendatahub-operator:v2.17.0 - createdAt: "2024-08-28T19:30:24Z" + createdAt: "2024-09-11T01:43:34Z" olm.skipRange: '>=1.0.0 <2.17.0' operators.operatorframework.io/builder: operator-sdk-v1.31.0 operators.operatorframework.io/project_layout: go.kubebuilder.io/v3 @@ -1238,3 +1238,23 @@ spec: targetPort: 9443 type: ValidatingAdmissionWebhook webhookPath: /validate-opendatahub-io-v1 + - admissionReviewVersions: + - v1 + containerPort: 443 + deploymentName: opendatahub-operator-controller-manager + failurePolicy: Fail + generateName: operator.opendatahub.io + rules: + - apiGroups: + - datasciencecluster.opendatahub.io + apiVersions: + - v1 + operations: + - CREATE + - UPDATE + resources: + - datascienceclusters + sideEffects: None + targetPort: 9443 + type: MutatingAdmissionWebhook + webhookPath: /mutate-opendatahub-io-v1 diff --git a/components/modelregistry/modelregistry.go b/components/modelregistry/modelregistry.go index 8031ca5543c..8ae5940df9e 100644 --- a/components/modelregistry/modelregistry.go +++ b/components/modelregistry/modelregistry.go @@ -14,9 +14,6 @@ import ( operatorv1 "github.com/openshift/api/operator/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1" @@ -46,6 +43,7 @@ var _ components.ComponentInterface = (*ModelRegistry)(nil) // ModelRegistry struct holds the configuration for the ModelRegistry component. // The property `registriesNamespace` is immutable when `managementState` is `Managed` + // +kubebuilder:object:generate=true // +kubebuilder:validation:XValidation:rule="self.managementState != 'Managed' || (oldSelf.registriesNamespace == '' && self.registriesNamespace != '') || (self.registriesNamespace == oldSelf.registriesNamespace)",message="RegistriesNamespace is immutable when model registry is Managed" //nolint:lll @@ -97,14 +95,6 @@ func (m *ModelRegistry) ReconcileComponent(ctx context.Context, cli client.Clien return errors.New("ServiceMesh needs to be set to 'Managed' in DSCI CR, it is required by Model Registry") } - // if registriesNamespace is not set, patch it to default value to allow omitempty registriesNamespace - if len(m.RegistriesNamespace) == 0 { - err := m.setDefaultRegistriesNamespace(ctx, cli, l, owner) - if err != nil { - return fmt.Errorf("failed to patch registriesNamespace for DSC %s : %w", owner.GetName(), err) - } - } - if err := m.createDependencies(ctx, cli, dscispec); err != nil { return err } @@ -180,25 +170,6 @@ func (m *ModelRegistry) ReconcileComponent(ctx context.Context, cli client.Clien return nil } -func (m *ModelRegistry) setDefaultRegistriesNamespace(ctx context.Context, cli client.Client, l logr.Logger, owner metav1.Object) error { - m.RegistriesNamespace = DefaultModelRegistriesNamespace - - patch := GetRegistriesNamespacePatch(m.RegistriesNamespace) - l.Info("Patching empty registriesNamespace", "DSC", owner.GetName(), "registriesNamespace", m.RegistriesNamespace, "patch", patch) - // have to use unstructured to avoid circular references with DSC type - obj := unstructured.Unstructured{} - obj.SetGroupVersionKind(schema.GroupVersionKind{Group: "datasciencecluster.opendatahub.io", Version: "v1", Kind: "DataScienceCluster"}) - obj.SetName(owner.GetName()) - obj.SetNamespace(owner.GetNamespace()) - return cli.Patch(ctx, &obj, patch) -} - -//nolint:ireturn -func GetRegistriesNamespacePatch(namespace string) client.Patch { - patchStr := fmt.Sprintf("{\"spec\":{\"components\":{\"modelregistry\":{\"registriesNamespace\":\"%s\"}}}}", namespace) - return client.RawPatch(types.MergePatchType, []byte(patchStr)) -} - func (m *ModelRegistry) createDependencies(ctx context.Context, cli client.Client, dscispec *dsciv1.DSCInitializationSpec) error { // create DefaultModelRegistryCert if err := cluster.PropagateDefaultIngressCertificate(ctx, cli, DefaultModelRegistryCert, dscispec.ServiceMesh.ControlPlane.Namespace); err != nil { diff --git a/config/webhook/kustomizeconfig.yaml b/config/webhook/kustomizeconfig.yaml index e809f78208e..25e21e3c963 100644 --- a/config/webhook/kustomizeconfig.yaml +++ b/config/webhook/kustomizeconfig.yaml @@ -4,11 +4,18 @@ 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 diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index 3dcfb31f3e3..7aae016e8e5 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -1,5 +1,31 @@ --- apiVersion: admissionregistration.k8s.io/v1 +kind: MutatingWebhookConfiguration +metadata: + name: mutating-webhook-configuration +webhooks: +- admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /mutate-opendatahub-io-v1 + failurePolicy: Fail + name: operator.opendatahub.io + rules: + - apiGroups: + - datasciencecluster.opendatahub.io + apiVersions: + - v1 + operations: + - CREATE + - UPDATE + resources: + - datascienceclusters + sideEffects: None +--- +apiVersion: admissionregistration.k8s.io/v1 kind: ValidatingWebhookConfiguration metadata: name: validating-webhook-configuration diff --git a/controllers/webhook/webhook.go b/controllers/webhook/webhook.go index 26034d8b305..ce9f548a344 100644 --- a/controllers/webhook/webhook.go +++ b/controllers/webhook/webhook.go @@ -29,6 +29,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1" + "github.com/opendatahub-io/opendatahub-operator/v2/components/modelregistry" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk" ) @@ -37,12 +39,12 @@ var log = ctrl.Log.WithName("odh-controller-webhook") //+kubebuilder:webhook:path=/validate-opendatahub-io-v1,mutating=false,failurePolicy=fail,sideEffects=None,groups=datasciencecluster.opendatahub.io;dscinitialization.opendatahub.io,resources=datascienceclusters;dscinitializations,verbs=create;delete,versions=v1,name=operator.opendatahub.io,admissionReviewVersions=v1 //nolint:lll -type OpenDataHubWebhook struct { +type OpenDataHubValidatingWebhook struct { Client client.Client Decoder *admission.Decoder } -func (w *OpenDataHubWebhook) SetupWithManager(mgr ctrl.Manager) { +func (w *OpenDataHubValidatingWebhook) SetupWithManager(mgr ctrl.Manager) { hookServer := mgr.GetWebhookServer() odhWebhook := &webhook.Admission{ Handler: w, @@ -74,7 +76,7 @@ func denyCountGtZero(ctx context.Context, cli client.Client, gvk schema.GroupVer return admission.Allowed("") } -func (w *OpenDataHubWebhook) 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,7 +95,7 @@ func (w *OpenDataHubWebhook) checkDupCreation(ctx context.Context, req admission fmt.Sprintf("Only one instance of %s object is allowed", req.Kind.Kind)) } -func (w *OpenDataHubWebhook) 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("") } @@ -103,7 +105,7 @@ func (w *OpenDataHubWebhook) checkDeletion(ctx context.Context, req admission.Re fmt.Sprintln("Cannot delete DSCI object when DSC object still exists")) } -func (w *OpenDataHubWebhook) 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 @@ -122,3 +124,46 @@ func (w *OpenDataHubWebhook) Handle(ctx context.Context, req admission.Request) return admission.Allowed(fmt.Sprintf("Operation %s on %s allowed", req.Operation, req.Kind.Kind)) } + +//+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=operator.opendatahub.io,admissionReviewVersions=v1 +//nolint:lll + +// OpenDataHubMutatingWebhook is a mutating webhook to set defaults +// It currently only sets defaults for datascienceclusters. +type OpenDataHubMutatingWebhook struct { + Client client.Client + Decoder *admission.Decoder +} + +func (w *OpenDataHubMutatingWebhook) SetupWithManager(mgr ctrl.Manager) { + hookServer := mgr.GetWebhookServer() + odhWebhook := &webhook.Admission{ + Handler: w, + } + 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 + + dsc := &dscv1.DataScienceCluster{} + err := w.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 +} + +func (w *OpenDataHubMutatingWebhook) setDSCDefaults(_ context.Context, dsc *dscv1.DataScienceCluster) admission.Response { + // set default registriesNamespace if empty + if len(dsc.Spec.Components.ModelRegistry.RegistriesNamespace) == 0 { + return admission.Patched("Property registriesNamespace set to default value", webhook.JSONPatchOp{ + Operation: "replace", + Path: "spec.components.modelregistry.registriesNamespace", + Value: modelregistry.DefaultModelRegistriesNamespace, + }) + } + return admission.Allowed("No change") +} diff --git a/controllers/webhook/webhook_suite_test.go b/controllers/webhook/webhook_suite_test.go index 9558c7e8cf0..529ee3f5313 100644 --- a/controllers/webhook/webhook_suite_test.go +++ b/controllers/webhook/webhook_suite_test.go @@ -48,6 +48,7 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/components/datasciencepipelines" "github.com/opendatahub-io/opendatahub-operator/v2/components/kserve" "github.com/opendatahub-io/opendatahub-operator/v2/components/modelmeshserving" + "github.com/opendatahub-io/opendatahub-operator/v2/components/modelregistry" "github.com/opendatahub-io/opendatahub-operator/v2/components/ray" "github.com/opendatahub-io/opendatahub-operator/v2/components/trustyai" "github.com/opendatahub-io/opendatahub-operator/v2/components/workbenches" @@ -132,7 +133,12 @@ var _ = BeforeSuite(func() { }) Expect(err).NotTo(HaveOccurred()) - (&webhook.OpenDataHubWebhook{ + (&webhook.OpenDataHubValidatingWebhook{ + Client: mgr.GetClient(), + Decoder: admission.NewDecoder(mgr.GetScheme()), + }).SetupWithManager(mgr) + + (&webhook.OpenDataHubMutatingWebhook{ Client: mgr.GetClient(), Decoder: admission.NewDecoder(mgr.GetScheme()), }).SetupWithManager(mgr) @@ -202,6 +208,14 @@ var _ = Describe("DSC/DSCI webhook", func() { Expect(k8sClient.Delete(ctx, dscInstance)).Should(Succeed()) Expect(k8sClient.Delete(ctx, dsciInstance)).Should(Succeed()) }) + + It("Should set defaults in DSC instance", func(ctx context.Context) { + dscInstance := newDSC(nameBase+"-dsc-1", namespace) + Expect(k8sClient.Create(ctx, dscInstance)).Should(Succeed()) + Expect(dscInstance.Spec.Components.ModelRegistry.RegistriesNamespace). + Should(Equal(modelregistry.DefaultModelRegistriesNamespace)) + Expect(clearInstance(ctx, dscInstance)).Should(Succeed()) + }) }) func clearInstance(ctx context.Context, instance client.Object) error { diff --git a/main.go b/main.go index 52fd181ef94..d330fc82e4e 100644 --- a/main.go +++ b/main.go @@ -207,7 +207,12 @@ func main() { //nolint:funlen,maintidx os.Exit(1) } - (&webhook.OpenDataHubWebhook{ + (&webhook.OpenDataHubValidatingWebhook{ + Client: mgr.GetClient(), + Decoder: admission.NewDecoder(mgr.GetScheme()), + }).SetupWithManager(mgr) + + (&webhook.OpenDataHubMutatingWebhook{ Client: mgr.GetClient(), Decoder: admission.NewDecoder(mgr.GetScheme()), }).SetupWithManager(mgr) diff --git a/tests/e2e/creation_test.go b/tests/e2e/creation_test.go index f9674116230..2a2abc3cd2b 100644 --- a/tests/e2e/creation_test.go +++ b/tests/e2e/creation_test.go @@ -639,7 +639,8 @@ func (tc *testContext) validateModelRegistryConfig() error { } func patchRegistriesNamespace(tc *testContext, namespace string, expected string, expectErr bool) error { - err := tc.customClient.Patch(tc.ctx, tc.testDsc, modelregistry.GetRegistriesNamespacePatch(namespace)) + patchStr := fmt.Sprintf("{\"spec\":{\"components\":{\"modelregistry\":{\"registriesNamespace\":\"%s\"}}}}", namespace) + err := tc.customClient.Patch(tc.ctx, tc.testDsc, client.RawPatch(types.MergePatchType, []byte(patchStr))) if err != nil { if !expectErr { return fmt.Errorf("unexpected error when setting registriesNamespace in DSC %s to %s: %w", From 4d059a08fc24e60def16ab8b63898c7d060b4d8c Mon Sep 17 00:00:00 2001 From: Dhiraj Bokde Date: Tue, 10 Sep 2024 19:53:50 -0700 Subject: [PATCH 14/18] fix: add namespace name pattern for registriesNamespace, allow empty string to replace with default value --- components/modelregistry/modelregistry.go | 2 ++ .../datasciencecluster.opendatahub.io_datascienceclusters.yaml | 2 ++ docs/api-overview.md | 2 +- 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/components/modelregistry/modelregistry.go b/components/modelregistry/modelregistry.go index 8ae5940df9e..e30679e99c8 100644 --- a/components/modelregistry/modelregistry.go +++ b/components/modelregistry/modelregistry.go @@ -53,6 +53,8 @@ type ModelRegistry struct { // Namespace for model registries to be installed, configurable only once when model registry is enabled, defaults to "odh-model-registries" // +kubebuilder:default="odh-model-registries" + // +kubebuilder:validation:Pattern="^([a-z0-9]([-a-z0-9]*[a-z0-9])?)?$" + // +kubebuilder:validation:MaxLength=63 RegistriesNamespace string `json:"registriesNamespace,omitempty"` } diff --git a/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml b/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml index 3b9e4cc7eca..99905aeb196 100644 --- a/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml +++ b/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml @@ -448,6 +448,8 @@ spec: description: Namespace for model registries to be installed, configurable only once when model registry is enabled, defaults to "odh-model-registries" + maxLength: 63 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?)?$ type: string type: object x-kubernetes-validations: diff --git a/docs/api-overview.md b/docs/api-overview.md index 44174f56b45..026a784b283 100644 --- a/docs/api-overview.md +++ b/docs/api-overview.md @@ -251,7 +251,7 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | | `Component` _[Component](#component)_ | | | | -| `registriesNamespace` _string_ | Namespace for model registries to be installed, configurable only once when model registry is enabled, defaults to "odh-model-registries" | odh-model-registries | | +| `registriesNamespace` _string_ | Namespace for model registries to be installed, configurable only once when model registry is enabled, defaults to "odh-model-registries" | odh-model-registries | MaxLength: 63
Pattern: `^([a-z0-9]([-a-z0-9]*[a-z0-9])?)?$`
| From f52e06a9593b161fcd86e00596634e2abde6c2b7 Mon Sep 17 00:00:00 2001 From: Dhiraj Bokde Date: Wed, 11 Sep 2024 08:32:30 -0700 Subject: [PATCH 15/18] fix: use unique webhook names --- ...er.opendatahub.io_datascienceclusters.yaml | 2 ++ ...atahub-operator.clusterserviceversion.yaml | 22 +++++++++---------- config/webhook/manifests.yaml | 4 ++-- controllers/webhook/webhook.go | 4 ++-- 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml b/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml index 0f7d2f2fc43..79a65db7f69 100644 --- a/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml +++ b/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml @@ -448,6 +448,8 @@ spec: description: Namespace for model registries to be installed, configurable only once when model registry is enabled, defaults to "odh-model-registries" + maxLength: 63 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?)?$ type: string type: object x-kubernetes-validations: diff --git a/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml b/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml index 96f5397b6d9..49e55a0c921 100644 --- a/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml +++ b/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml @@ -103,7 +103,7 @@ metadata: categories: AI/Machine Learning, Big Data certified: "False" containerImage: quay.io/opendatahub/opendatahub-operator:v2.17.0 - createdAt: "2024-09-11T01:43:34Z" + createdAt: "2024-09-11T15:26:04Z" olm.skipRange: '>=1.0.0 <2.17.0' operators.operatorframework.io/builder: operator-sdk-v1.31.0 operators.operatorframework.io/project_layout: go.kubebuilder.io/v3 @@ -1221,40 +1221,40 @@ spec: containerPort: 443 deploymentName: opendatahub-operator-controller-manager failurePolicy: Fail - generateName: operator.opendatahub.io + generateName: mutate.operator.opendatahub.io rules: - apiGroups: - datasciencecluster.opendatahub.io - - dscinitialization.opendatahub.io apiVersions: - v1 operations: - CREATE - - DELETE + - UPDATE resources: - datascienceclusters - - dscinitializations sideEffects: None targetPort: 9443 - type: ValidatingAdmissionWebhook - webhookPath: /validate-opendatahub-io-v1 + type: MutatingAdmissionWebhook + webhookPath: /mutate-opendatahub-io-v1 - admissionReviewVersions: - v1 containerPort: 443 deploymentName: opendatahub-operator-controller-manager failurePolicy: Fail - generateName: operator.opendatahub.io + generateName: validate.operator.opendatahub.io rules: - apiGroups: - datasciencecluster.opendatahub.io + - dscinitialization.opendatahub.io apiVersions: - v1 operations: - CREATE - - UPDATE + - DELETE resources: - datascienceclusters + - dscinitializations sideEffects: None targetPort: 9443 - type: MutatingAdmissionWebhook - webhookPath: /mutate-opendatahub-io-v1 + type: ValidatingAdmissionWebhook + webhookPath: /validate-opendatahub-io-v1 diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index 7aae016e8e5..5270728a35a 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -12,7 +12,7 @@ webhooks: namespace: system path: /mutate-opendatahub-io-v1 failurePolicy: Fail - name: operator.opendatahub.io + name: mutate.operator.opendatahub.io rules: - apiGroups: - datasciencecluster.opendatahub.io @@ -38,7 +38,7 @@ webhooks: namespace: system path: /validate-opendatahub-io-v1 failurePolicy: Fail - name: operator.opendatahub.io + name: validate.operator.opendatahub.io rules: - apiGroups: - datasciencecluster.opendatahub.io diff --git a/controllers/webhook/webhook.go b/controllers/webhook/webhook.go index ce9f548a344..34824ea173c 100644 --- a/controllers/webhook/webhook.go +++ b/controllers/webhook/webhook.go @@ -36,7 +36,7 @@ import ( var log = ctrl.Log.WithName("odh-controller-webhook") -//+kubebuilder:webhook:path=/validate-opendatahub-io-v1,mutating=false,failurePolicy=fail,sideEffects=None,groups=datasciencecluster.opendatahub.io;dscinitialization.opendatahub.io,resources=datascienceclusters;dscinitializations,verbs=create;delete,versions=v1,name=operator.opendatahub.io,admissionReviewVersions=v1 +//+kubebuilder:webhook:path=/validate-opendatahub-io-v1,mutating=false,failurePolicy=fail,sideEffects=None,groups=datasciencecluster.opendatahub.io;dscinitialization.opendatahub.io,resources=datascienceclusters;dscinitializations,verbs=create;delete,versions=v1,name=validate.operator.opendatahub.io,admissionReviewVersions=v1 //nolint:lll type OpenDataHubValidatingWebhook struct { @@ -125,7 +125,7 @@ func (w *OpenDataHubValidatingWebhook) Handle(ctx context.Context, req admission return admission.Allowed(fmt.Sprintf("Operation %s on %s allowed", req.Operation, req.Kind.Kind)) } -//+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=operator.opendatahub.io,admissionReviewVersions=v1 +//+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 to set defaults From a68ca529589a861c1ff3582b5a6a46931ce8b136 Mon Sep 17 00:00:00 2001 From: Dhiraj Bokde Date: Wed, 11 Sep 2024 08:47:10 -0700 Subject: [PATCH 16/18] fix: don't change existing validating webhook name to avoid upgrade issues --- .../manifests/opendatahub-operator.clusterserviceversion.yaml | 4 ++-- config/webhook/manifests.yaml | 2 +- controllers/webhook/webhook.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml b/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml index 49e55a0c921..93e21b12f10 100644 --- a/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml +++ b/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml @@ -103,7 +103,7 @@ metadata: categories: AI/Machine Learning, Big Data certified: "False" containerImage: quay.io/opendatahub/opendatahub-operator:v2.17.0 - createdAt: "2024-09-11T15:26:04Z" + createdAt: "2024-09-11T15:45:50Z" olm.skipRange: '>=1.0.0 <2.17.0' operators.operatorframework.io/builder: operator-sdk-v1.31.0 operators.operatorframework.io/project_layout: go.kubebuilder.io/v3 @@ -1241,7 +1241,7 @@ spec: containerPort: 443 deploymentName: opendatahub-operator-controller-manager failurePolicy: Fail - generateName: validate.operator.opendatahub.io + generateName: operator.opendatahub.io rules: - apiGroups: - datasciencecluster.opendatahub.io diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index 5270728a35a..9c1174d5173 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -38,7 +38,7 @@ webhooks: namespace: system path: /validate-opendatahub-io-v1 failurePolicy: Fail - name: validate.operator.opendatahub.io + name: operator.opendatahub.io rules: - apiGroups: - datasciencecluster.opendatahub.io diff --git a/controllers/webhook/webhook.go b/controllers/webhook/webhook.go index 34824ea173c..6e1eaa1a493 100644 --- a/controllers/webhook/webhook.go +++ b/controllers/webhook/webhook.go @@ -36,7 +36,7 @@ import ( var log = ctrl.Log.WithName("odh-controller-webhook") -//+kubebuilder:webhook:path=/validate-opendatahub-io-v1,mutating=false,failurePolicy=fail,sideEffects=None,groups=datasciencecluster.opendatahub.io;dscinitialization.opendatahub.io,resources=datascienceclusters;dscinitializations,verbs=create;delete,versions=v1,name=validate.operator.opendatahub.io,admissionReviewVersions=v1 +//+kubebuilder:webhook:path=/validate-opendatahub-io-v1,mutating=false,failurePolicy=fail,sideEffects=None,groups=datasciencecluster.opendatahub.io;dscinitialization.opendatahub.io,resources=datascienceclusters;dscinitializations,verbs=create;delete,versions=v1,name=operator.opendatahub.io,admissionReviewVersions=v1 //nolint:lll type OpenDataHubValidatingWebhook struct { From b725dee0db8ed686d5a8cff0215531591a38f293 Mon Sep 17 00:00:00 2001 From: Dhiraj Bokde Date: Wed, 11 Sep 2024 12:35:27 -0700 Subject: [PATCH 17/18] feat: add model registry registriesNamespace in DSC status --- .../v1/datasciencecluster_types.go | 9 +++++ .../v1/zz_generated.deepcopy.go | 22 +++++++++++++ components/modelregistry/modelregistry.go | 8 +++++ .../modelregistry/zz_generated.deepcopy.go | 15 +++++++++ ...er.opendatahub.io_datascienceclusters.yaml | 13 ++++++++ .../datasciencecluster_controller.go | 12 +++++++ docs/api-overview.md | 33 +++++++++++++++++++ 7 files changed, 112 insertions(+) diff --git a/apis/datasciencecluster/v1/datasciencecluster_types.go b/apis/datasciencecluster/v1/datasciencecluster_types.go index 58d680e70c9..bc4135eb4e9 100644 --- a/apis/datasciencecluster/v1/datasciencecluster_types.go +++ b/apis/datasciencecluster/v1/datasciencecluster_types.go @@ -86,6 +86,12 @@ type Components struct { TrainingOperator trainingoperator.TrainingOperator `json:"trainingoperator,omitempty"` } +// ComponentsStatus defines the custom status of DataScienceCluster components. +type ComponentsStatus struct { + // ModelRegistry component status + ModelRegistry *modelregistry.ModelRegistryStatus `json:"modelregistry,omitempty"` +} + // DataScienceClusterStatus defines the observed state of DataScienceCluster. type DataScienceClusterStatus struct { // Phase describes the Phase of DataScienceCluster reconciliation state @@ -105,6 +111,9 @@ type DataScienceClusterStatus struct { // List of components with status if installed or not InstalledComponents map[string]bool `json:"installedComponents,omitempty"` + // Component specific status + Components ComponentsStatus `json:"components,omitempty"` + // Version and release type Release cluster.Release `json:"release,omitempty"` } diff --git a/apis/datasciencecluster/v1/zz_generated.deepcopy.go b/apis/datasciencecluster/v1/zz_generated.deepcopy.go index cf315a2ee0a..8c4a72dd5fd 100644 --- a/apis/datasciencecluster/v1/zz_generated.deepcopy.go +++ b/apis/datasciencecluster/v1/zz_generated.deepcopy.go @@ -21,6 +21,7 @@ limitations under the License. package v1 import ( + "github.com/opendatahub-io/opendatahub-operator/v2/components/modelregistry" conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" corev1 "k8s.io/api/core/v1" runtime "k8s.io/apimachinery/pkg/runtime" @@ -52,6 +53,26 @@ func (in *Components) DeepCopy() *Components { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ComponentsStatus) DeepCopyInto(out *ComponentsStatus) { + *out = *in + if in.ModelRegistry != nil { + in, out := &in.ModelRegistry, &out.ModelRegistry + *out = new(modelregistry.ModelRegistryStatus) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ComponentsStatus. +func (in *ComponentsStatus) DeepCopy() *ComponentsStatus { + if in == nil { + return nil + } + out := new(ComponentsStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *DataScienceCluster) DeepCopyInto(out *DataScienceCluster) { *out = *in @@ -149,6 +170,7 @@ func (in *DataScienceClusterStatus) DeepCopyInto(out *DataScienceClusterStatus) (*out)[key] = val } } + in.Components.DeepCopyInto(&out.Components) in.Release.DeepCopyInto(&out.Release) } diff --git a/components/modelregistry/modelregistry.go b/components/modelregistry/modelregistry.go index e30679e99c8..1d0e13c8f1e 100644 --- a/components/modelregistry/modelregistry.go +++ b/components/modelregistry/modelregistry.go @@ -58,6 +58,14 @@ type ModelRegistry struct { RegistriesNamespace string `json:"registriesNamespace,omitempty"` } +// ModelRegistryStatus struct holds the status for the ModelRegistry component. + +// +kubebuilder:object:generate=true +type ModelRegistryStatus struct { + // Namespace for model registries to be installed + RegistriesNamespace string `json:"registriesNamespace"` +} + func (m *ModelRegistry) OverrideManifests(ctx context.Context, _ cluster.Platform) error { // If devflags are set, update default manifests path if len(m.DevFlags.Manifests) != 0 { diff --git a/components/modelregistry/zz_generated.deepcopy.go b/components/modelregistry/zz_generated.deepcopy.go index 86c4a17e14c..349bde5c722 100644 --- a/components/modelregistry/zz_generated.deepcopy.go +++ b/components/modelregistry/zz_generated.deepcopy.go @@ -37,3 +37,18 @@ func (in *ModelRegistry) DeepCopy() *ModelRegistry { in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ModelRegistryStatus) DeepCopyInto(out *ModelRegistryStatus) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ModelRegistryStatus. +func (in *ModelRegistryStatus) DeepCopy() *ModelRegistryStatus { + if in == nil { + return nil + } + out := new(ModelRegistryStatus) + in.DeepCopyInto(out) + return out +} diff --git a/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml b/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml index 99905aeb196..78288ccbabe 100644 --- a/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml +++ b/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml @@ -647,6 +647,19 @@ spec: status: description: DataScienceClusterStatus defines the observed state of DataScienceCluster. properties: + components: + description: Component specific status + properties: + modelregistry: + description: ModelRegistry component status + properties: + registriesNamespace: + description: Namespace for model registries to be installed + type: string + required: + - registriesNamespace + type: object + type: object conditions: description: Conditions describes the state of the DataScienceCluster resource. diff --git a/controllers/datasciencecluster/datasciencecluster_controller.go b/controllers/datasciencecluster/datasciencecluster_controller.go index 088f3e308d9..07c58ee0a6c 100644 --- a/controllers/datasciencecluster/datasciencecluster_controller.go +++ b/controllers/datasciencecluster/datasciencecluster_controller.go @@ -53,6 +53,7 @@ import ( dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1" "github.com/opendatahub-io/opendatahub-operator/v2/components" "github.com/opendatahub-io/opendatahub-operator/v2/components/datasciencepipelines" + "github.com/opendatahub-io/opendatahub-operator/v2/components/modelregistry" "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" annotations "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations" @@ -316,6 +317,17 @@ func (r *DataScienceClusterReconciler) reconcileSubComponent(ctx context.Context } err = component.ReconcileComponent(ctx, r.Client, r.Log, instance, r.DataScienceCluster.DSCISpec, platform, installedComponentValue) + // TODO: replace this hack with a full refactor of component status in the future + if mr, isMR := component.(*modelregistry.ModelRegistry); isMR { + instance, err = status.UpdateWithRetry(ctx, r.Client, instance, func(saved *dscv1.DataScienceCluster) { + if enabled { + saved.Status.Components.ModelRegistry = &modelregistry.ModelRegistryStatus{RegistriesNamespace: mr.RegistriesNamespace} + } else { + saved.Status.Components.ModelRegistry = nil + } + }) + } + if err != nil { // reconciliation failed: log errors, raise event and update status accordingly instance = r.reportError(err, instance, "failed to reconcile "+componentName+" on DataScienceCluster") diff --git a/docs/api-overview.md b/docs/api-overview.md index 026a784b283..107762d651f 100644 --- a/docs/api-overview.md +++ b/docs/api-overview.md @@ -254,6 +254,22 @@ _Appears in:_ | `registriesNamespace` _string_ | Namespace for model registries to be installed, configurable only once when model registry is enabled, defaults to "odh-model-registries" | odh-model-registries | MaxLength: 63
Pattern: `^([a-z0-9]([-a-z0-9]*[a-z0-9])?)?$`
| +#### ModelRegistryStatus + + + + + + + +_Appears in:_ +- [ComponentsStatus](#componentsstatus) + +| Field | Description | Default | Validation | +| --- | --- | --- | --- | +| `registriesNamespace` _string_ | Namespace for model registries to be installed | | | + + ## datasciencecluster.opendatahub.io/ray @@ -413,6 +429,22 @@ _Appears in:_ | `trainingoperator` _[TrainingOperator](#trainingoperator)_ | Training Operator component configuration. | | | +#### ComponentsStatus + + + +ComponentsStatus defines the custom status of DataScienceCluster components. + + + +_Appears in:_ +- [DataScienceClusterStatus](#datascienceclusterstatus) + +| Field | Description | Default | Validation | +| --- | --- | --- | --- | +| `modelregistry` _[ModelRegistryStatus](#modelregistrystatus)_ | ModelRegistry component status | | | + + #### ControlPlaneSpec @@ -486,6 +518,7 @@ _Appears in:_ | `relatedObjects` _[ObjectReference](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.25/#objectreference-v1-core) array_ | RelatedObjects is a list of objects created and maintained by this operator.
Object references will be added to this list after they have been created AND found in the cluster. | | | | `errorMessage` _string_ | | | | | `installedComponents` _object (keys:string, values:boolean)_ | List of components with status if installed or not | | | +| `components` _[ComponentsStatus](#componentsstatus)_ | Component specific status | | | | `release` _[Release](#release)_ | Version and release type | | | From 8b1b12de01f25c38c2ec5f7a52a820ddb72eb20d Mon Sep 17 00:00:00 2001 From: Wen Zhou Date: Thu, 12 Sep 2024 11:37:30 +0200 Subject: [PATCH 18/18] update: - move ModelRegistryStatus from ModelReg into status - update validation rule for namespace: - when update is to removed/unmanaged modelreg: regardless previous state, can change name and set to removed - upgrade 2.13 to 2.14 when no NS set in 2.13 - when update is from remove/unmanaged: e.g change from removed to managed and change name at the same tie - keep same namespace, regardless state - reduce print on passed allowed webhook request - set to omitemptry so required is not add in the CSV - fix review comments: - move update status after reconcile is successful - set to use const for caching on ModeReg namespace Signed-off-by: Wen Zhou --- .../v1/datasciencecluster_types.go | 6 ++++-- .../v1/zz_generated.deepcopy.go | 4 ++-- ...ter.opendatahub.io_datascienceclusters.yaml | 14 ++++++++++++-- ...datahub-operator.clusterserviceversion.yaml | 2 +- components/modelregistry/modelregistry.go | 10 +--------- .../modelregistry/zz_generated.deepcopy.go | 15 --------------- ...ter.opendatahub.io_datascienceclusters.yaml | 9 +++------ .../datasciencecluster_controller.go | 18 +++++++++--------- controllers/status/status.go | 5 +++++ controllers/webhook/webhook.go | 6 +++--- docs/api-overview.md | 18 +----------------- main.go | 6 +++--- 12 files changed, 44 insertions(+), 69 deletions(-) diff --git a/apis/datasciencecluster/v1/datasciencecluster_types.go b/apis/datasciencecluster/v1/datasciencecluster_types.go index bc4135eb4e9..39166651ac1 100644 --- a/apis/datasciencecluster/v1/datasciencecluster_types.go +++ b/apis/datasciencecluster/v1/datasciencecluster_types.go @@ -36,6 +36,7 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/components/trainingoperator" "github.com/opendatahub-io/opendatahub-operator/v2/components/trustyai" "github.com/opendatahub-io/opendatahub-operator/v2/components/workbenches" + "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" ) @@ -89,7 +90,7 @@ type Components struct { // ComponentsStatus defines the custom status of DataScienceCluster components. type ComponentsStatus struct { // ModelRegistry component status - ModelRegistry *modelregistry.ModelRegistryStatus `json:"modelregistry,omitempty"` + ModelRegistry *status.ModelRegistryStatus `json:"modelregistry,omitempty"` } // DataScienceClusterStatus defines the observed state of DataScienceCluster. @@ -111,7 +112,8 @@ type DataScienceClusterStatus struct { // List of components with status if installed or not InstalledComponents map[string]bool `json:"installedComponents,omitempty"` - // Component specific status + // Expose component's specific status + // +optional Components ComponentsStatus `json:"components,omitempty"` // Version and release type diff --git a/apis/datasciencecluster/v1/zz_generated.deepcopy.go b/apis/datasciencecluster/v1/zz_generated.deepcopy.go index 8c4a72dd5fd..f089d70e9ee 100644 --- a/apis/datasciencecluster/v1/zz_generated.deepcopy.go +++ b/apis/datasciencecluster/v1/zz_generated.deepcopy.go @@ -21,7 +21,7 @@ limitations under the License. package v1 import ( - "github.com/opendatahub-io/opendatahub-operator/v2/components/modelregistry" + "github.com/opendatahub-io/opendatahub-operator/v2/controllers/status" conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" corev1 "k8s.io/api/core/v1" runtime "k8s.io/apimachinery/pkg/runtime" @@ -58,7 +58,7 @@ func (in *ComponentsStatus) DeepCopyInto(out *ComponentsStatus) { *out = *in if in.ModelRegistry != nil { in, out := &in.ModelRegistry, &out.ModelRegistry - *out = new(modelregistry.ModelRegistryStatus) + *out = new(status.ModelRegistryStatus) **out = **in } } diff --git a/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml b/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml index 79a65db7f69..da6d20509c6 100644 --- a/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml +++ b/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml @@ -455,8 +455,8 @@ spec: x-kubernetes-validations: - message: RegistriesNamespace is immutable when model registry is Managed - rule: self.managementState != 'Managed' || (oldSelf.registriesNamespace - == '' && self.registriesNamespace != '') || (self.registriesNamespace + rule: (self.managementState != 'Managed') || (oldSelf.registriesNamespace + == '') || (oldSelf.managementState != 'Managed')|| (self.registriesNamespace == oldSelf.registriesNamespace) ray: description: Ray component configuration. @@ -647,6 +647,16 @@ spec: status: description: DataScienceClusterStatus defines the observed state of DataScienceCluster. properties: + components: + description: Expose component's specific status + properties: + modelregistry: + description: ModelRegistry component status + properties: + registriesNamespace: + type: string + type: object + type: object conditions: description: Conditions describes the state of the DataScienceCluster resource. diff --git a/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml b/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml index 93e21b12f10..6989c2e8d7e 100644 --- a/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml +++ b/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml @@ -103,7 +103,7 @@ metadata: categories: AI/Machine Learning, Big Data certified: "False" containerImage: quay.io/opendatahub/opendatahub-operator:v2.17.0 - createdAt: "2024-09-11T15:45:50Z" + createdAt: "2024-08-28T19:30:24Z" olm.skipRange: '>=1.0.0 <2.17.0' operators.operatorframework.io/builder: operator-sdk-v1.31.0 operators.operatorframework.io/project_layout: go.kubebuilder.io/v3 diff --git a/components/modelregistry/modelregistry.go b/components/modelregistry/modelregistry.go index 1d0e13c8f1e..f05cbdd734a 100644 --- a/components/modelregistry/modelregistry.go +++ b/components/modelregistry/modelregistry.go @@ -45,7 +45,7 @@ var _ components.ComponentInterface = (*ModelRegistry)(nil) // The property `registriesNamespace` is immutable when `managementState` is `Managed` // +kubebuilder:object:generate=true -// +kubebuilder:validation:XValidation:rule="self.managementState != 'Managed' || (oldSelf.registriesNamespace == '' && self.registriesNamespace != '') || (self.registriesNamespace == oldSelf.registriesNamespace)",message="RegistriesNamespace is immutable when model registry is Managed" +// +kubebuilder:validation:XValidation:rule="(self.managementState != 'Managed') || (oldSelf.registriesNamespace == '') || (oldSelf.managementState != 'Managed')|| (self.registriesNamespace == oldSelf.registriesNamespace)",message="RegistriesNamespace is immutable when model registry is Managed" //nolint:lll type ModelRegistry struct { @@ -58,14 +58,6 @@ type ModelRegistry struct { RegistriesNamespace string `json:"registriesNamespace,omitempty"` } -// ModelRegistryStatus struct holds the status for the ModelRegistry component. - -// +kubebuilder:object:generate=true -type ModelRegistryStatus struct { - // Namespace for model registries to be installed - RegistriesNamespace string `json:"registriesNamespace"` -} - func (m *ModelRegistry) OverrideManifests(ctx context.Context, _ cluster.Platform) error { // If devflags are set, update default manifests path if len(m.DevFlags.Manifests) != 0 { diff --git a/components/modelregistry/zz_generated.deepcopy.go b/components/modelregistry/zz_generated.deepcopy.go index 349bde5c722..86c4a17e14c 100644 --- a/components/modelregistry/zz_generated.deepcopy.go +++ b/components/modelregistry/zz_generated.deepcopy.go @@ -37,18 +37,3 @@ func (in *ModelRegistry) DeepCopy() *ModelRegistry { in.DeepCopyInto(out) return out } - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *ModelRegistryStatus) DeepCopyInto(out *ModelRegistryStatus) { - *out = *in -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ModelRegistryStatus. -func (in *ModelRegistryStatus) DeepCopy() *ModelRegistryStatus { - if in == nil { - return nil - } - out := new(ModelRegistryStatus) - in.DeepCopyInto(out) - return out -} diff --git a/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml b/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml index 78288ccbabe..41d1c76d658 100644 --- a/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml +++ b/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml @@ -455,8 +455,8 @@ spec: x-kubernetes-validations: - message: RegistriesNamespace is immutable when model registry is Managed - rule: self.managementState != 'Managed' || (oldSelf.registriesNamespace - == '' && self.registriesNamespace != '') || (self.registriesNamespace + rule: (self.managementState != 'Managed') || (oldSelf.registriesNamespace + == '') || (oldSelf.managementState != 'Managed')|| (self.registriesNamespace == oldSelf.registriesNamespace) ray: description: Ray component configuration. @@ -648,16 +648,13 @@ spec: description: DataScienceClusterStatus defines the observed state of DataScienceCluster. properties: components: - description: Component specific status + description: Expose component's specific status properties: modelregistry: description: ModelRegistry component status properties: registriesNamespace: - description: Namespace for model registries to be installed type: string - required: - - registriesNamespace type: object type: object conditions: diff --git a/controllers/datasciencecluster/datasciencecluster_controller.go b/controllers/datasciencecluster/datasciencecluster_controller.go index 07c58ee0a6c..1be0f830877 100644 --- a/controllers/datasciencecluster/datasciencecluster_controller.go +++ b/controllers/datasciencecluster/datasciencecluster_controller.go @@ -318,15 +318,6 @@ func (r *DataScienceClusterReconciler) reconcileSubComponent(ctx context.Context err = component.ReconcileComponent(ctx, r.Client, r.Log, instance, r.DataScienceCluster.DSCISpec, platform, installedComponentValue) // TODO: replace this hack with a full refactor of component status in the future - if mr, isMR := component.(*modelregistry.ModelRegistry); isMR { - instance, err = status.UpdateWithRetry(ctx, r.Client, instance, func(saved *dscv1.DataScienceCluster) { - if enabled { - saved.Status.Components.ModelRegistry = &modelregistry.ModelRegistryStatus{RegistriesNamespace: mr.RegistriesNamespace} - } else { - saved.Status.Components.ModelRegistry = nil - } - }) - } if err != nil { // reconciliation failed: log errors, raise event and update status accordingly @@ -355,6 +346,15 @@ func (r *DataScienceClusterReconciler) reconcileSubComponent(ctx context.Context } else { status.RemoveComponentCondition(&saved.Status.Conditions, componentName) } + + // TODO: replace this hack with a full refactor of component status in the future + if mr, isMR := component.(*modelregistry.ModelRegistry); isMR { + if enabled { + saved.Status.Components.ModelRegistry = &status.ModelRegistryStatus{RegistriesNamespace: mr.RegistriesNamespace} + } else { + saved.Status.Components.ModelRegistry = nil + } + } }) if err != nil { instance = r.reportError(err, instance, "failed to update DataScienceCluster status after reconciling "+componentName) diff --git a/controllers/status/status.go b/controllers/status/status.go index 52e3d79145f..808cfee2f7b 100644 --- a/controllers/status/status.go +++ b/controllers/status/status.go @@ -209,3 +209,8 @@ func SetComponentCondition(conditions *[]conditionsv1.Condition, component strin func RemoveComponentCondition(conditions *[]conditionsv1.Condition, component string) { conditionsv1.RemoveStatusCondition(conditions, conditionsv1.ConditionType(component+ReadySuffix)) } + +// ModelRegistryStatus struct holds the status for the ModelRegistry component. +type ModelRegistryStatus struct { + RegistriesNamespace string `json:"registriesNamespace,omitempty"` +} diff --git a/controllers/webhook/webhook.go b/controllers/webhook/webhook.go index 6e1eaa1a493..dfda6d0fa32 100644 --- a/controllers/webhook/webhook.go +++ b/controllers/webhook/webhook.go @@ -128,8 +128,8 @@ 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 -// OpenDataHubMutatingWebhook is a mutating webhook to set defaults -// It currently only sets defaults for datascienceclusters. +// OpenDataHubMutatingWebhook is a mutating webhook +// It currently only sets defaults for modelregiestry in datascienceclusters. type OpenDataHubMutatingWebhook struct { Client client.Client Decoder *admission.Decoder @@ -165,5 +165,5 @@ func (w *OpenDataHubMutatingWebhook) setDSCDefaults(_ context.Context, dsc *dscv Value: modelregistry.DefaultModelRegistriesNamespace, }) } - return admission.Allowed("No change") + return admission.Allowed("") } diff --git a/docs/api-overview.md b/docs/api-overview.md index 107762d651f..0f0f7030595 100644 --- a/docs/api-overview.md +++ b/docs/api-overview.md @@ -254,22 +254,6 @@ _Appears in:_ | `registriesNamespace` _string_ | Namespace for model registries to be installed, configurable only once when model registry is enabled, defaults to "odh-model-registries" | odh-model-registries | MaxLength: 63
Pattern: `^([a-z0-9]([-a-z0-9]*[a-z0-9])?)?$`
| -#### ModelRegistryStatus - - - - - - - -_Appears in:_ -- [ComponentsStatus](#componentsstatus) - -| Field | Description | Default | Validation | -| --- | --- | --- | --- | -| `registriesNamespace` _string_ | Namespace for model registries to be installed | | | - - ## datasciencecluster.opendatahub.io/ray @@ -518,7 +502,7 @@ _Appears in:_ | `relatedObjects` _[ObjectReference](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.25/#objectreference-v1-core) array_ | RelatedObjects is a list of objects created and maintained by this operator.
Object references will be added to this list after they have been created AND found in the cluster. | | | | `errorMessage` _string_ | | | | | `installedComponents` _object (keys:string, values:boolean)_ | List of components with status if installed or not | | | -| `components` _[ComponentsStatus](#componentsstatus)_ | Component specific status | | | +| `components` _[ComponentsStatus](#componentsstatus)_ | Expose component's specific status | | | | `release` _[Release](#release)_ | Version and release type | | | diff --git a/main.go b/main.go index d330fc82e4e..e0b84ff654b 100644 --- a/main.go +++ b/main.go @@ -57,6 +57,7 @@ import ( dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1" dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1" featurev1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/features/v1" + "github.com/opendatahub-io/opendatahub-operator/v2/components/modelregistry" "github.com/opendatahub-io/opendatahub-operator/v2/controllers/certconfigmapgenerator" dscctrl "github.com/opendatahub-io/opendatahub-operator/v2/controllers/datasciencecluster" dscictrl "github.com/opendatahub-io/opendatahub-operator/v2/controllers/dscinitialization" @@ -347,13 +348,12 @@ func createDeploymentCacheConfig(platform cluster.Platform) map[string]cache.Con case cluster.ManagedRhods: // no need workbench NS, only SFS no Deployment namespaceConfigs["redhat-ods-monitoring"] = cache.Config{} namespaceConfigs["redhat-ods-applications"] = cache.Config{} - namespaceConfigs["rhoai-model-registries"] = cache.Config{} case cluster.SelfManagedRhods: namespaceConfigs["redhat-ods-applications"] = cache.Config{} - namespaceConfigs["rhoai-model-registries"] = cache.Config{} default: namespaceConfigs["opendatahub"] = cache.Config{} - namespaceConfigs["odh-model-registries"] = cache.Config{} } + // for modelregistry namespace + namespaceConfigs[modelregistry.DefaultModelRegistriesNamespace] = cache.Config{} return namespaceConfigs }