Manifests cleanup (opendatahub-io#1450)

* Refactor manifests folder 1. reorganize the manifests folder and the manifest fetcher script to use the compoennt name as a folder for manifests 2. introduce a LegacyComponentName variable to hold the legacy name of a component; this is required because existing deployment's selectors are immutable once created, hence to avoid disruption we can't change the selectgor to use the new name 3. add missing CRD watcher where missing 4. add additional tests 5. fix status handling to use legacy component names for legacy status fields * Fix datasciencepipelines * Fix e2e tests * Remove unrelated changes from e2e tests
ugiordan · Dec 18, 2024 · 1b67705 · 1b67705
1 parent dc23f27
commit 1b67705
Show file tree

Hide file tree

Showing 69 changed files with 335 additions and 192 deletions.
diff --git a/apis/components/v1alpha1/codeflare_types.go b/apis/components/v1alpha1/codeflare_types.go
@@ -24,7 +24,7 @@ import (
 const (
 	CodeFlareComponentName = "codeflare"
 	// value should match whats set in the XValidation below
-	CodeFlareInstanceName = "default-codeflare"
+	CodeFlareInstanceName = "default-" + CodeFlareComponentName
 	CodeFlareKind         = "CodeFlare"
 )
 

diff --git a/apis/components/v1alpha1/dashboard_types.go b/apis/components/v1alpha1/dashboard_types.go
@@ -25,7 +25,7 @@ const (
 	DashboardComponentName = "dashboard"
 	// DashboardInstanceName the name of the Dashboard instance singleton.
 	// value should match whats set in the XValidation below
-	DashboardInstanceName = "default-dashboard"
+	DashboardInstanceName = "default-" + DashboardComponentName
 	DashboardKind         = "Dashboard"
 )
 

diff --git a/apis/components/v1alpha1/datasciencepipelines_types.go b/apis/components/v1alpha1/datasciencepipelines_types.go
@@ -22,16 +22,18 @@ import (
 )
 
 const (
-	DataSciencePipelinesComponentName = "data-science-pipelines-operator"
+	DataSciencePipelinesComponentName = "datasciencepipelines"
 	// value should match whats set in the XValidation below
-	DataSciencePipelinesInstanceName = "default-datasciencepipelines"
+	DataSciencePipelinesInstanceName = "default-" + DataSciencePipelinesComponentName
 	DataSciencePipelinesKind         = "DataSciencePipelines"
 )
 
 // +kubebuilder:object:root=true
 // +kubebuilder:subresource:status
 // +kubebuilder:resource:scope=Cluster
 // +kubebuilder:validation:XValidation:rule="self.metadata.name == 'default-datasciencepipelines'",message="DataSciencePipelines name must be default-datasciencepipelines"
+// +kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].status`,description="Ready"
+// +kubebuilder:printcolumn:name="Reason",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].reason`,description="Reason"
 
 // DataSciencePipelines is the Schema for the datasciencepipelines API
 type DataSciencePipelines struct {

diff --git a/apis/components/v1alpha1/kserve_types.go b/apis/components/v1alpha1/kserve_types.go
@@ -25,10 +25,9 @@ import (
 )
 
 const (
-	KserveComponentName          = "kserve"
-	ModelControllerComponentName = "odh-model-controller" // shared by kserve and mm
+	KserveComponentName = "kserve"
 	// value should match what's set in the XValidation below
-	KserveInstanceName = "default-kserve"
+	KserveInstanceName = "default-" + KserveComponentName
 	KserveKind         = "Kserve"
 )
 

diff --git a/apis/components/v1alpha1/kueue_types.go b/apis/components/v1alpha1/kueue_types.go
@@ -24,7 +24,7 @@ import (
 const (
 	KueueComponentName = "kueue"
 	// value should match whats set in the XValidation below
-	KueueInstanceName = "default-kueue"
+	KueueInstanceName = "default-" + KueueComponentName
 	KueueKind         = "Kueue"
 )
 

diff --git a/apis/components/v1alpha1/modelcontroller_types.go b/apis/components/v1alpha1/modelcontroller_types.go
@@ -23,9 +23,10 @@ import (
 )
 
 const (
+	ModelControllerComponentName = "modelcontroller"
 	// shared by kserve and modelmeshserving
 	// value should match whats set in the XValidation below
-	ModelControllerInstanceName = "default-modelcontroller"
+	ModelControllerInstanceName = "default-" + ModelControllerComponentName
 	ModelControllerKind         = "ModelController"
 )
 

diff --git a/apis/components/v1alpha1/modelmeshserving_types.go b/apis/components/v1alpha1/modelmeshserving_types.go
@@ -22,9 +22,9 @@ import (
 )
 
 const (
-	ModelMeshServingComponentName = "model-mesh"
+	ModelMeshServingComponentName = "modelmeshserving"
 	// value should match whats set in the XValidation below
-	ModelMeshServingInstanceName = "default-modelmesh"
+	ModelMeshServingInstanceName = "default-" + ModelMeshServingComponentName
 	ModelMeshServingKind         = "ModelMeshServing"
 )
 
@@ -33,7 +33,7 @@ const (
 // +kubebuilder:object:root=true
 // +kubebuilder:subresource:status
 // +kubebuilder:resource:scope=Cluster
-// +kubebuilder:validation:XValidation:rule="self.metadata.name == 'default-modelmesh'",message="ModelMeshServing name must be default-modelmesh"
+// +kubebuilder:validation:XValidation:rule="self.metadata.name == 'default-modelmeshserving'",message="ModelMeshServing name must be default-modelmeshserving"
 // +kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].status`,description="Ready"
 // +kubebuilder:printcolumn:name="Reason",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].reason`,description="Reason"
 

diff --git a/apis/components/v1alpha1/modelregistry_types.go b/apis/components/v1alpha1/modelregistry_types.go
@@ -22,10 +22,10 @@ import (
 )
 
 const (
-	ModelRegistryComponentName = "model-registry-operator"
+	ModelRegistryComponentName = "modelregistry"
 	// ModelRegistryInstanceName the name of the ModelRegistry instance singleton.
 	// value should match what's set in the XValidation below
-	ModelRegistryInstanceName = "default-model-registry"
+	ModelRegistryInstanceName = "default-" + ModelRegistryComponentName
 	ModelRegistryKind         = "ModelRegistry"
 )
 
@@ -62,7 +62,7 @@ type ModelRegistryStatus struct {
 // +kubebuilder:object:root=true
 // +kubebuilder:subresource:status
 // +kubebuilder:resource:scope=Cluster
-// +kubebuilder:validation:XValidation:rule="self.metadata.name == 'default-model-registry'",message="ModelRegistry name must be default-model-registry"
+// +kubebuilder:validation:XValidation:rule="self.metadata.name == 'default-modelregistry'",message="ModelRegistry name must be default-modelregistry"
 // +kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].status`,description="Ready"
 // +kubebuilder:printcolumn:name="Reason",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].reason`,description="Reason"
 

diff --git a/apis/components/v1alpha1/ray_types.go b/apis/components/v1alpha1/ray_types.go
@@ -24,7 +24,7 @@ import (
 const (
 	RayComponentName = "ray"
 	// value should match whats set in the XValidation below
-	RayInstanceName = "default-ray"
+	RayInstanceName = "default-" + RayComponentName
 	RayKind         = "Ray"
 )
 

diff --git a/apis/components/v1alpha1/trainingoperator_types.go b/apis/components/v1alpha1/trainingoperator_types.go
@@ -24,7 +24,7 @@ import (
 const (
 	TrainingOperatorComponentName = "trainingoperator"
 	// value should match whats set in the XValidation below
-	TrainingOperatorInstanceName = "default-trainingoperator"
+	TrainingOperatorInstanceName = "default-" + TrainingOperatorComponentName
 	TrainingOperatorKind         = "TrainingOperator"
 )
 

diff --git a/apis/components/v1alpha1/trustyai_types.go b/apis/components/v1alpha1/trustyai_types.go
@@ -24,7 +24,7 @@ import (
 const (
 	TrustyAIComponentName = "trustyai"
 	// value should match whats set in the XValidation below
-	TrustyAIInstanceName = "default-trustyai"
+	TrustyAIInstanceName = "default-" + TrustyAIComponentName
 	TrustyAIKind         = "TrustyAI"
 )
 

diff --git a/apis/components/v1alpha1/workbenches_types.go b/apis/components/v1alpha1/workbenches_types.go
@@ -25,7 +25,7 @@ const (
 	WorkbenchesComponentName = "workbenches"
 	// WorkbenchesInstanceName the name of the Workbenches instance singleton.
 	// value should match what is set in the XValidation below.
-	WorkbenchesInstanceName = "default-workbenches"
+	WorkbenchesInstanceName = "default-" + WorkbenchesComponentName
 	WorkbenchesKind         = "Workbenches"
 )
 

diff --git a/bundle/manifests/components.platform.opendatahub.io_datasciencepipelines.yaml b/bundle/manifests/components.platform.opendatahub.io_datasciencepipelines.yaml
@@ -14,7 +14,16 @@ spec:
     singular: datasciencepipelines
   scope: Cluster
   versions:
-  - name: v1alpha1
+  - additionalPrinterColumns:
+    - description: Ready
+      jsonPath: .status.conditions[?(@.type=="Ready")].status
+      name: Ready
+      type: string
+    - description: Reason
+      jsonPath: .status.conditions[?(@.type=="Ready")].reason
+      name: Reason
+      type: string
+    name: v1alpha1
     schema:
       openAPIV3Schema:
         description: DataSciencePipelines is the Schema for the datasciencepipelines

diff --git a/bundle/manifests/components.platform.opendatahub.io_modelmeshservings.yaml b/bundle/manifests/components.platform.opendatahub.io_modelmeshservings.yaml
@@ -145,8 +145,8 @@ spec:
             type: object
         type: object
         x-kubernetes-validations:
-        - message: ModelMeshServing name must be default-modelmesh
-          rule: self.metadata.name == 'default-modelmesh'
+        - message: ModelMeshServing name must be default-modelmeshserving
+          rule: self.metadata.name == 'default-modelmeshserving'
     served: true
     storage: true
     subresources:

diff --git a/bundle/manifests/components.platform.opendatahub.io_modelregistries.yaml b/bundle/manifests/components.platform.opendatahub.io_modelregistries.yaml
@@ -154,8 +154,8 @@ spec:
             type: object
         type: object
         x-kubernetes-validations:
-        - message: ModelRegistry name must be default-model-registry
-          rule: self.metadata.name == 'default-model-registry'
+        - message: ModelRegistry name must be default-modelregistry
+          rule: self.metadata.name == 'default-modelregistry'
     served: true
     storage: true
     subresources:

diff --git a/config/crd/bases/components.platform.opendatahub.io_datasciencepipelines.yaml b/config/crd/bases/components.platform.opendatahub.io_datasciencepipelines.yaml
@@ -14,7 +14,16 @@ spec:
     singular: datasciencepipelines
   scope: Cluster
   versions:
-  - name: v1alpha1
+  - additionalPrinterColumns:
+    - description: Ready
+      jsonPath: .status.conditions[?(@.type=="Ready")].status
+      name: Ready
+      type: string
+    - description: Reason
+      jsonPath: .status.conditions[?(@.type=="Ready")].reason
+      name: Reason
+      type: string
+    name: v1alpha1
     schema:
       openAPIV3Schema:
         description: DataSciencePipelines is the Schema for the datasciencepipelines

diff --git a/config/crd/bases/components.platform.opendatahub.io_modelmeshservings.yaml b/config/crd/bases/components.platform.opendatahub.io_modelmeshservings.yaml
@@ -145,8 +145,8 @@ spec:
             type: object
         type: object
         x-kubernetes-validations:
-        - message: ModelMeshServing name must be default-modelmesh
-          rule: self.metadata.name == 'default-modelmesh'
+        - message: ModelMeshServing name must be default-modelmeshserving
+          rule: self.metadata.name == 'default-modelmeshserving'
     served: true
     storage: true
     subresources:

diff --git a/config/crd/bases/components.platform.opendatahub.io_modelregistries.yaml b/config/crd/bases/components.platform.opendatahub.io_modelregistries.yaml
@@ -154,8 +154,8 @@ spec:
             type: object
         type: object
         x-kubernetes-validations:
-        - message: ModelRegistry name must be default-model-registry
-          rule: self.metadata.name == 'default-model-registry'
+        - message: ModelRegistry name must be default-modelregistry
+          rule: self.metadata.name == 'default-modelregistry'
     served: true
     storage: true
     subresources:

diff --git a/controllers/components/codeflare/codeflare.go b/controllers/components/codeflare/codeflare.go
@@ -70,7 +70,7 @@ func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj cl
 		return errors.New("failed to convert to CodeFlare")
 	}
 
-	dsc.Status.InstalledComponents[s.GetName()] = false
+	dsc.Status.InstalledComponents[LegacyComponentName] = false
 	dsc.Status.Components.CodeFlare.ManagementSpec.ManagementState = s.GetManagementState(dsc)
 	dsc.Status.Components.CodeFlare.CodeFlareCommonStatus = nil
 
@@ -83,7 +83,7 @@ func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj cl
 
 	switch s.GetManagementState(dsc) {
 	case operatorv1.Managed:
-		dsc.Status.InstalledComponents[s.GetName()] = true
+		dsc.Status.InstalledComponents[LegacyComponentName] = true
 		dsc.Status.Components.CodeFlare.CodeFlareCommonStatus = c.Status.CodeFlareCommonStatus.DeepCopy()
 
 		if rc := meta.FindStatusCondition(c.Status.Conditions, status.ConditionTypeReady); rc != nil {

diff --git a/controllers/components/codeflare/codeflare_controller.go b/controllers/components/codeflare/codeflare_controller.go
@@ -62,15 +62,15 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.
 			reconciler.WithEventHandler(
 				handlers.ToNamed(componentApi.CodeFlareInstanceName)),
 			reconciler.WithPredicates(
-				component.ForLabel(labels.ODH.Component(ComponentName), labels.True)),
+				component.ForLabel(labels.ODH.Component(LegacyComponentName), labels.True)),
 		).
 		// Add CodeFlare-specific actions
 		WithAction(initialize).
 		WithAction(devFlags).
 		WithAction(kustomize.NewAction(
 			kustomize.WithCache(),
-			kustomize.WithLabel(labels.ODH.Component(ComponentName), labels.True),
-			kustomize.WithLabel(labels.K8SCommon.PartOf, ComponentName),
+			kustomize.WithLabel(labels.ODH.Component(LegacyComponentName), labels.True),
+			kustomize.WithLabel(labels.K8SCommon.PartOf, LegacyComponentName),
 		)).
 		WithAction(deploy.NewAction(
 			deploy.WithCache(),

diff --git a/controllers/components/codeflare/codeflare_support.go b/controllers/components/codeflare/codeflare_support.go
@@ -10,6 +10,11 @@ import (
 
 const (
 	ComponentName = componentApi.CodeFlareComponentName
+
+	// LegacyComponentName is the name of the component that is assigned to deployments
+	// via Kustomize. Since a deployment selector is immutable, we can't upgrade existing
+	// deployment to the new component name, so keep it around till we figure out a solution.
+	LegacyComponentName = "codeflare"
 )
 
 var (

diff --git a/controllers/components/dashboard/dashboard.go b/controllers/components/dashboard/dashboard.go
@@ -42,7 +42,7 @@ func (s *componentHandler) Init(platform cluster.Platform) error {
 	mi := defaultManifestInfo(platform)
 
 	if err := odhdeploy.ApplyParams(mi.String(), imagesMap); err != nil {
-		return fmt.Errorf("failed to update images on path %s: %w", manifestPaths[platform], err)
+		return fmt.Errorf("failed to update images on path %s: %w", mi, err)
 	}
 
 	return nil
@@ -72,7 +72,7 @@ func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj cl
 		return errors.New("failed to convert to Dashboard")
 	}
 
-	dsc.Status.InstalledComponents[s.GetName()] = false
+	dsc.Status.InstalledComponents[LegacyComponentNameUpstream] = false
 	dsc.Status.Components.Dashboard.ManagementSpec.ManagementState = s.GetManagementState(dsc)
 	dsc.Status.Components.Dashboard.DashboardCommonStatus = nil
 
@@ -85,7 +85,7 @@ func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj cl
 
 	switch s.GetManagementState(dsc) {
 	case operatorv1.Managed:
-		dsc.Status.InstalledComponents[s.GetName()] = true
+		dsc.Status.InstalledComponents[LegacyComponentNameUpstream] = true
 		dsc.Status.Components.Dashboard.DashboardCommonStatus = c.Status.DashboardCommonStatus.DeepCopy()
 
 		if rc := meta.FindStatusCondition(c.Status.Conditions, status.ConditionTypeReady); rc != nil {

diff --git a/controllers/components/dashboard/dashboard_controller_actions.go b/controllers/components/dashboard/dashboard_controller_actions.go
@@ -49,12 +49,12 @@ func devFlags(ctx context.Context, rr *odhtypes.ReconciliationRequest) error {
 	// If dev flags are set, update default manifests path
 	if len(dashboard.Spec.DevFlags.Manifests) != 0 {
 		manifestConfig := dashboard.Spec.DevFlags.Manifests[0]
-		if err := odhdeploy.DownloadManifests(ctx, ComponentNameUpstream, manifestConfig); err != nil {
+		if err := odhdeploy.DownloadManifests(ctx, ComponentName, manifestConfig); err != nil {
 			return err
 		}
 		if manifestConfig.SourcePath != "" {
 			rr.Manifests[0].Path = odhdeploy.DefaultManifestPath
-			rr.Manifests[0].ContextDir = ComponentNameUpstream
+			rr.Manifests[0].ContextDir = ComponentName
 			rr.Manifests[0].SourcePath = manifestConfig.SourcePath
 		}
 	}

diff --git a/controllers/components/dashboard/dashboard_support.go b/controllers/components/dashboard/dashboard_support.go
@@ -6,24 +6,25 @@ import (
 
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
+	componentsApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1"
 	dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
 	odhtypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types"
 	odhdeploy "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy"
 )
 
 const (
-	ComponentName           = "dashboard"
-	ComponentNameUpstream   = ComponentName
-	ComponentNameDownstream = "rhods-dashboard"
+	ComponentName = componentsApi.DashboardComponentName
+
+	// Legacy component names are the name of the component that is assigned to deployments
+	// via Kustomize. Since a deployment selector is immutable, we can't upgrade existing
+	// deployment to the new component name, so keep it around till we figure out a solution.
+
+	LegacyComponentNameUpstream   = "dashboard"
+	LegacyComponentNameDownstream = "rhods-dashboard"
 )
 
 var (
-	PathUpstream          = odhdeploy.DefaultManifestPath + "/" + ComponentNameUpstream + "/odh"
-	PathDownstream        = odhdeploy.DefaultManifestPath + "/" + ComponentNameUpstream + "/rhoai"
-	PathSelfDownstream    = PathDownstream + "/onprem"
-	PathManagedDownstream = PathDownstream + "/addon"
-
 	adminGroups = map[cluster.Platform]string{
 		cluster.SelfManagedRhoai: "rhods-admins",
 		cluster.ManagedRhoai:     "dedicated-admins",
@@ -45,11 +46,11 @@ var (
 		cluster.Unknown:          "https://odh-dashboard-",
 	}
 
-	manifestPaths = map[cluster.Platform]string{
-		cluster.SelfManagedRhoai: PathSelfDownstream,
-		cluster.ManagedRhoai:     PathManagedDownstream,
-		cluster.OpenDataHub:      PathUpstream,
-		cluster.Unknown:          PathUpstream,
+	overlaysSourcePaths = map[cluster.Platform]string{
+		cluster.SelfManagedRhoai: "/rhoai/onprem",
+		cluster.ManagedRhoai:     "/rhoai/addon",
+		cluster.OpenDataHub:      "/odh",
+		cluster.Unknown:          "/odh",
 	}
 
 	serviceAccounts = map[cluster.Platform][]string{
@@ -66,9 +67,9 @@ var (
 
 func defaultManifestInfo(p cluster.Platform) odhtypes.ManifestInfo {
 	return odhtypes.ManifestInfo{
-		Path:       manifestPaths[p],
-		ContextDir: "",
-		SourcePath: "",
+		Path:       odhdeploy.DefaultManifestPath,
+		ContextDir: ComponentName,
+		SourcePath: overlaysSourcePaths[p],
 	}
 }
 
@@ -88,9 +89,9 @@ func computeKustomizeVariable(ctx context.Context, cli client.Client, platform c
 func computeComponentName() string {
 	release := cluster.GetRelease()
 
-	name := ComponentNameUpstream
+	name := LegacyComponentNameUpstream
 	if release.Name == cluster.SelfManagedRhoai || release.Name == cluster.ManagedRhoai {
-		name = ComponentNameDownstream
+		name = LegacyComponentNameDownstream
 	}
 
 	return name