From 112d3f14cbe54361d936582dcb44fbd71a69f862 Mon Sep 17 00:00:00 2001 From: Dhiraj Bokde Date: Thu, 1 Feb 2024 01:21:59 -0800 Subject: [PATCH] feat: Add ModelRegistry component (#775) (#776) * feat: Add ModelRegistry component (#775) * fix: Fix modelregistry odh overlays path * fix: fix dsc_create_test tests err nil check * fix: refactor ModelRegistry.ReconcileComponent for new parameters * chore: added modelregistry to README.md * fix: add missing rbac rules for deploymentconfigs and daemonsets * chore: code lint cleanup * fix: added check for nil DevFlags in model-registry component * fix: add nil check for dscispec.DevFlags in model-registry ReconcileComponent * fix: remove RBAC rules for daemonsets and deploymentconfigs * fix(chore): fix lint errors in dsc_deletion_test.go --- README.md | 2 + .../v1/datasciencecluster_types.go | 4 + .../v1/zz_generated.deepcopy.go | 1 + ...er.opendatahub.io_datascienceclusters.yaml | 43 +++++++++ ...atahub-operator.clusterserviceversion.yaml | 29 +++++++ components/README.md | 1 + components/modelregistry/modelregistry.go | 87 +++++++++++++++++++ .../modelregistry/zz_generated.deepcopy.go | 40 +++++++++ ...er.opendatahub.io_datascienceclusters.yaml | 43 +++++++++ config/rbac/role.yaml | 26 ++++++ ...asciencecluster_v1_datasciencecluster.yaml | 2 + .../datasciencecluster/kubebuilder_rbac.go | 4 + get_all_manifests.sh | 3 +- pkg/upgrade/upgrade.go | 4 + tests/e2e/dsc_creation_test.go | 11 +++ tests/e2e/dsc_deletion_test.go | 23 +++-- tests/e2e/helper_test.go | 6 ++ 17 files changed, 319 insertions(+), 10 deletions(-) create mode 100644 components/modelregistry/modelregistry.go create mode 100644 components/modelregistry/zz_generated.deepcopy.go diff --git a/README.md b/README.md index f289534fed3..7657d8cdc36 100644 --- a/README.md +++ b/README.md @@ -254,6 +254,8 @@ spec: managementState: Managed trustyai: managementState: Managed + modelregistry: + managementState: Managed ``` 2. Enable only Dashboard and Workbenches diff --git a/apis/datasciencecluster/v1/datasciencecluster_types.go b/apis/datasciencecluster/v1/datasciencecluster_types.go index f181554ab35..9e79beaebfb 100644 --- a/apis/datasciencecluster/v1/datasciencecluster_types.go +++ b/apis/datasciencecluster/v1/datasciencecluster_types.go @@ -26,6 +26,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" @@ -67,6 +68,9 @@ type Components struct { // TrustyAI component configuration. TrustyAI trustyai.TrustyAI `json:"trustyai,omitempty"` + + // ModelRegistry component configuration. + ModelRegistry modelregistry.ModelRegistry `json:"modelregistry,omitempty"` } // DataScienceClusterStatus defines the observed state of DataScienceCluster. diff --git a/apis/datasciencecluster/v1/zz_generated.deepcopy.go b/apis/datasciencecluster/v1/zz_generated.deepcopy.go index b8e57ccaca7..723e45d0c3c 100644 --- a/apis/datasciencecluster/v1/zz_generated.deepcopy.go +++ b/apis/datasciencecluster/v1/zz_generated.deepcopy.go @@ -38,6 +38,7 @@ func (in *Components) DeepCopyInto(out *Components) { in.CodeFlare.DeepCopyInto(&out.CodeFlare) in.Ray.DeepCopyInto(&out.Ray) in.TrustyAI.DeepCopyInto(&out.TrustyAI) + in.ModelRegistry.DeepCopyInto(&out.ModelRegistry) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Components. diff --git a/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml b/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml index 53406a108f1..6825ee479b7 100644 --- a/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml +++ b/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml @@ -322,6 +322,49 @@ spec: pattern: ^(Managed|Unmanaged|Force|Removed)$ type: string type: object + modelregistry: + description: ModelRegistry component configuration. + properties: + devFlags: + description: Add developer fields + properties: + manifests: + description: List of custom manifests for the given component + items: + properties: + contextDir: + default: "" + description: contextDir is the relative path to + the folder containing manifests in a repository + type: string + sourcePath: + default: "" + description: 'sourcePath is the subpath within contextDir + where kustomize builds start. Examples include + any sub-folder or path: `base`, `overlays/dev`, + `default`, `odh` etc' + type: string + uri: + default: "" + description: uri is the URI point to a git repo + with tag/branch. e.g https://github.com/org/repo/tarball/ + type: string + type: object + type: array + type: object + managementState: + description: "Set to one of the following values: \n - \"Managed\" + : the operator is actively managing the component and trying + to keep it active. It will only upgrade the component if + it is safe to do so \n - \"Removed\" : the operator is actively + managing the component and will not install it, or if it + is installed, the operator will try to remove it" + enum: + - Managed + - Removed + pattern: ^(Managed|Unmanaged|Force|Removed)$ + type: string + type: object ray: description: Ray component configuration. properties: diff --git a/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml b/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml index 861b7189dab..cea6354dbec 100644 --- a/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml +++ b/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml @@ -43,6 +43,9 @@ metadata: "modelmeshserving": { "managementState": "Managed" }, + "modelregistry": { + "managementState": "Removed" + }, "ray": { "managementState": "Removed" }, @@ -980,6 +983,32 @@ spec: - get - list - patch + - apiGroups: + - modelregistry.opendatahub.io + resources: + - modelregistries + verbs: + - create + - delete + - get + - list + - patch + - update + - watch + - apiGroups: + - modelregistry.opendatahub.io + resources: + - modelregistries/finalizers + verbs: + - update + - apiGroups: + - modelregistry.opendatahub.io + resources: + - modelregistries/status + verbs: + - get + - patch + - update - apiGroups: - monitoring.coreos.com resources: diff --git a/components/README.md b/components/README.md index a4f6ccf1c47..ff650d986b5 100644 --- a/components/README.md +++ b/components/README.md @@ -61,3 +61,4 @@ can be found [here](https://github.com/opendatahub-io/opendatahub-operator/tree/ - [ModelMesh Serving](https://github.com/opendatahub-io/opendatahub-operator/tree/main/components/modelmeshserving) - [Workbenches](https://github.com/opendatahub-io/opendatahub-operator/tree/main/components/workbenches) - [TrustyAI](https://github.com/opendatahub-io/opendatahub-operator/tree/main/components/trustyai) +- [ModelRegistry](https://github.com/opendatahub-io/opendatahub-operator/tree/main/components/modelregistry) diff --git a/components/modelregistry/modelregistry.go b/components/modelregistry/modelregistry.go new file mode 100644 index 00000000000..aaf4c5297be --- /dev/null +++ b/components/modelregistry/modelregistry.go @@ -0,0 +1,87 @@ +// Package modelregistry provides utility functions to config ModelRegistry, an ML Model metadata repository service +package modelregistry + +import ( + "context" + "path/filepath" + + operatorv1 "github.com/openshift/api/operator/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/rest" + "sigs.k8s.io/controller-runtime/pkg/client" + + 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/pkg/deploy" +) + +var ( + ComponentName = "model-registry-operator" + Path = deploy.DefaultManifestPath + "/" + ComponentName + "/overlays/odh" +) + +// Verifies that ModelRegistry implements ComponentInterface. +var _ components.ComponentInterface = (*ModelRegistry)(nil) + +// ModelRegistry struct holds the configuration for the ModelRegistry component. +// +kubebuilder:object:generate=true +type ModelRegistry struct { + components.Component `json:""` +} + +func (m *ModelRegistry) OverrideManifests(_ string) error { + // If devflags are set, update default manifests path + if len(m.DevFlags.Manifests) != 0 { + manifestConfig := m.DevFlags.Manifests[0] + if err := deploy.DownloadManifests(ComponentName, manifestConfig); err != nil { + return err + } + // If overlay is defined, update paths + defaultKustomizePath := "overlays/odh" + if manifestConfig.SourcePath != "" { + defaultKustomizePath = manifestConfig.SourcePath + } + Path = filepath.Join(deploy.DefaultManifestPath, ComponentName, defaultKustomizePath) + } + + return nil +} + +func (m *ModelRegistry) GetComponentName() string { + return ComponentName +} + +func (m *ModelRegistry) ReconcileComponent(_ context.Context, cli client.Client, _ *rest.Config, + owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, _ bool) error { + var imageParamMap = map[string]string{ + "IMAGES_MODELREGISTRY_OPERATOR": "RELATED_IMAGE_ODH_MODELREGISTRY_OPERATOR_IMAGE", + "IMAGES_GRPC_SERVICE": "RELATED_IMAGE_ODH_MODELREGISTRY_GRPC_SERVICE_IMAGE", + "IMAGES_REST_SERVICE": "RELATED_IMAGE_ODH_MODELREGISTRY_REST_SERVICE_IMAGE", + } + enabled := m.GetManagementState() == operatorv1.Managed + + platform, err := deploy.GetPlatform(cli) + if err != nil { + return err + } + + if enabled { + if m.DevFlags != nil { + // Download manifests and update paths + if err = m.OverrideManifests(string(platform)); err != nil { + return err + } + } + + // Update image parameters only when we do not have customized manifests set + if (dscispec.DevFlags == nil || dscispec.DevFlags.ManifestsUri == "") && (m.DevFlags == nil || len(m.DevFlags.Manifests) == 0) { + if err := deploy.ApplyParams(Path, m.SetImageParamsMap(imageParamMap), false); err != nil { + return err + } + } + } + // Deploy ModelRegistry Operator + err = deploy.DeployManifestsFromPath(cli, owner, Path, dscispec.ApplicationsNamespace, m.GetComponentName(), enabled) + + return err +} diff --git a/components/modelregistry/zz_generated.deepcopy.go b/components/modelregistry/zz_generated.deepcopy.go new file mode 100644 index 00000000000..e0e2c2f1618 --- /dev/null +++ b/components/modelregistry/zz_generated.deepcopy.go @@ -0,0 +1,40 @@ +//go:build !ignore_autogenerated +// +build !ignore_autogenerated + +/* +Copyright 2023. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Code generated by controller-gen. DO NOT EDIT. + +package modelregistry + +import () + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (m *ModelRegistry) DeepCopyInto(out *ModelRegistry) { + *out = *m + m.Component.DeepCopyInto(&out.Component) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ModelRegistry. +func (m *ModelRegistry) DeepCopy() *ModelRegistry { + if m == nil { + return nil + } + out := new(ModelRegistry) + m.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 580edd90538..240a93e6c34 100644 --- a/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml +++ b/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml @@ -323,6 +323,49 @@ spec: pattern: ^(Managed|Unmanaged|Force|Removed)$ type: string type: object + modelregistry: + description: ModelRegistry component configuration. + properties: + devFlags: + description: Add developer fields + properties: + manifests: + description: List of custom manifests for the given component + items: + properties: + contextDir: + default: "" + description: contextDir is the relative path to + the folder containing manifests in a repository + type: string + sourcePath: + default: "" + description: 'sourcePath is the subpath within contextDir + where kustomize builds start. Examples include + any sub-folder or path: `base`, `overlays/dev`, + `default`, `odh` etc' + type: string + uri: + default: "" + description: uri is the URI point to a git repo + with tag/branch. e.g https://github.com/org/repo/tarball/ + type: string + type: object + type: array + type: object + managementState: + description: "Set to one of the following values: \n - \"Managed\" + : the operator is actively managing the component and trying + to keep it active. It will only upgrade the component if + it is safe to do so \n - \"Removed\" : the operator is actively + managing the component and will not install it, or if it + is installed, the operator will try to remove it" + enum: + - Managed + - Removed + pattern: ^(Managed|Unmanaged|Force|Removed)$ + type: string + type: object ray: description: Ray component configuration. properties: diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 3c9f83c2384..a351329d58e 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -800,6 +800,32 @@ rules: - get - list - patch +- apiGroups: + - modelregistry.opendatahub.io + resources: + - modelregistries + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - modelregistry.opendatahub.io + resources: + - modelregistries/finalizers + verbs: + - update +- apiGroups: + - modelregistry.opendatahub.io + resources: + - modelregistries/status + verbs: + - get + - patch + - update - apiGroups: - monitoring.coreos.com resources: diff --git a/config/samples/datasciencecluster_v1_datasciencecluster.yaml b/config/samples/datasciencecluster_v1_datasciencecluster.yaml index 08a5404eacf..49a934c4b31 100644 --- a/config/samples/datasciencecluster_v1_datasciencecluster.yaml +++ b/config/samples/datasciencecluster_v1_datasciencecluster.yaml @@ -36,3 +36,5 @@ spec: managementState: "Managed" trustyai: managementState: "Managed" + modelregistry: + managementState: "Removed" diff --git a/controllers/datasciencecluster/kubebuilder_rbac.go b/controllers/datasciencecluster/kubebuilder_rbac.go index b95a54a3f48..00bfcd02cae 100644 --- a/controllers/datasciencecluster/kubebuilder_rbac.go +++ b/controllers/datasciencecluster/kubebuilder_rbac.go @@ -118,6 +118,10 @@ package datasciencecluster //+kubebuilder:rbac:groups=trustyai.opendatahub.io,resources=trustyaiservices/status,verbs=get;update;patch //+kubebuilder:rbac:groups=trustyai.opendatahub.io,resources=trustyaiservices/finalizers,verbs=update +//+kubebuilder:rbac:groups=modelregistry.opendatahub.io,resources=modelregistries,verbs=get;list;watch;create;update;patch;delete +//+kubebuilder:rbac:groups=modelregistry.opendatahub.io,resources=modelregistries/status,verbs=get;update;patch +//+kubebuilder:rbac:groups=modelregistry.opendatahub.io,resources=modelregistries/finalizers,verbs=update + // +kubebuilder:rbac:groups="monitoring.coreos.com",resources=prometheuses/finalizers,verbs=get;create;patch;delete;deletecollection // +kubebuilder:rbac:groups="monitoring.coreos.com",resources=prometheuses/status,verbs=get;create;patch;delete;deletecollection diff --git a/get_all_manifests.sh b/get_all_manifests.sh index e0561e34c77..df15ac04dfb 100755 --- a/get_all_manifests.sh +++ b/get_all_manifests.sh @@ -3,7 +3,7 @@ set -e GITHUB_URL="https://github.com/" -# component: notebook, dsp, kserve, dashbaord, cf/ray, trustyai, modelmesh. +# component: notebook, dsp, kserve, dashbaord, cf/ray, trustyai, modelmesh, modelregistry. # in the format of "repo-org:repo-name:branch-name:source-folder:target-folder". declare -A COMPONENT_MANIFESTS=( ["codeflare"]="opendatahub-io:codeflare-operator:main:config:codeflare" @@ -17,6 +17,7 @@ declare -A COMPONENT_MANIFESTS=( ["model-mesh"]="opendatahub-io:modelmesh-serving:release-0.11.0:config:model-mesh" ["odh-model-controller"]="opendatahub-io:odh-model-controller:release-0.11.0:config:odh-model-controller" ["kserve"]="opendatahub-io:kserve:release-v0.11.0:config:kserve" + ["modelregistry"]="opendatahub-io:model-registry-operator:main:config:model-registry-operator" ) # Allow overwriting repo using flags component=repo diff --git a/pkg/upgrade/upgrade.go b/pkg/upgrade/upgrade.go index 3cb57376d43..61eb2ff42d2 100644 --- a/pkg/upgrade/upgrade.go +++ b/pkg/upgrade/upgrade.go @@ -29,6 +29,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" @@ -173,6 +174,9 @@ func CreateDefaultDSC(cli client.Client, _ deploy.Platform) error { TrustyAI: trustyai.TrustyAI{ Component: components.Component{ManagementState: operatorv1.Managed}, }, + ModelRegistry: modelregistry.ModelRegistry{ + Component: components.Component{ManagementState: operatorv1.Removed}, + }, }, }, } diff --git a/tests/e2e/dsc_creation_test.go b/tests/e2e/dsc_creation_test.go index 7e37fb0674d..d2f9b664c24 100644 --- a/tests/e2e/dsc_creation_test.go +++ b/tests/e2e/dsc_creation_test.go @@ -252,6 +252,17 @@ func (tc *testContext) testAllApplicationCreation(t *testing.T) error { //nolint } }) + t.Run("Validate ModelRegistry", func(t *testing.T) { + // speed testing in parallel + t.Parallel() + err = tc.testApplicationCreation(&(tc.testDsc.Spec.Components.ModelRegistry)) + if tc.testDsc.Spec.Components.ModelRegistry.ManagementState == operatorv1.Managed { + require.NoError(t, err, "error validating application %v when enabled", tc.testDsc.Spec.Components.ModelRegistry.GetComponentName()) + } else { + require.Error(t, err, "error validating application %v when disabled", tc.testDsc.Spec.Components.ModelRegistry.GetComponentName()) + } + }) + return nil } diff --git a/tests/e2e/dsc_deletion_test.go b/tests/e2e/dsc_deletion_test.go index 440678f244c..643af7b4627 100644 --- a/tests/e2e/dsc_deletion_test.go +++ b/tests/e2e/dsc_deletion_test.go @@ -78,37 +78,42 @@ func (tc *testContext) testApplicationDeletion(component components.ComponentInt func (tc *testContext) testAllApplicationDeletion() error { // Deletion all listed components' deployments - if err := tc.testApplicationDeletion(&(tc.testDsc.Spec.Components.Dashboard)); err != nil { + var err error + if err = tc.testApplicationDeletion(&(tc.testDsc.Spec.Components.Dashboard)); err != nil { return err } - if err := tc.testApplicationDeletion(&(tc.testDsc.Spec.Components.ModelMeshServing)); err != nil { + if err = tc.testApplicationDeletion(&(tc.testDsc.Spec.Components.ModelMeshServing)); err != nil { return err } - if err := tc.testApplicationDeletion(&(tc.testDsc.Spec.Components.Kserve)); err != nil { + if err = tc.testApplicationDeletion(&(tc.testDsc.Spec.Components.Kserve)); err != nil { return err } - if err := tc.testApplicationDeletion(&(tc.testDsc.Spec.Components.Workbenches)); err != nil { + if err = tc.testApplicationDeletion(&(tc.testDsc.Spec.Components.Workbenches)); err != nil { return err } - if err := tc.testApplicationDeletion(&(tc.testDsc.Spec.Components.DataSciencePipelines)); err != nil { + if err = tc.testApplicationDeletion(&(tc.testDsc.Spec.Components.DataSciencePipelines)); err != nil { return err } - if err := tc.testApplicationDeletion(&(tc.testDsc.Spec.Components.CodeFlare)); err != nil { + if err = tc.testApplicationDeletion(&(tc.testDsc.Spec.Components.CodeFlare)); err != nil { return err } - if err := tc.testApplicationDeletion(&(tc.testDsc.Spec.Components.Ray)); err != nil { + if err = tc.testApplicationDeletion(&(tc.testDsc.Spec.Components.Ray)); err != nil { return err } - if err := tc.testApplicationDeletion(&(tc.testDsc.Spec.Components.TrustyAI)); err != nil { //nolint:revive,nolintlint + if err = tc.testApplicationDeletion(&(tc.testDsc.Spec.Components.TrustyAI)); err != nil { //nolint:revive,nolintlint return err } - return nil + if err = tc.testApplicationDeletion(&(tc.testDsc.Spec.Components.ModelRegistry)); err != nil { + return err + } + + return err } diff --git a/tests/e2e/helper_test.go b/tests/e2e/helper_test.go index 3a451cfd557..e3cbd8e10e6 100644 --- a/tests/e2e/helper_test.go +++ b/tests/e2e/helper_test.go @@ -21,6 +21,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" @@ -118,6 +119,11 @@ func setupDSCInstance() *dsc.DataScienceCluster { ManagementState: operatorv1.Managed, }, }, + ModelRegistry: modelregistry.ModelRegistry{ + Component: components.Component{ + ManagementState: operatorv1.Managed, + }, + }, }, }, }