update: remove unnecessary param from ApplyParams() (opendatahub-io#1180

) - extraParamsMaps is generic to use Signed-off-by: Wen Zhou <[email protected]>
ykaliuta · Aug 16, 2024 · d84cd33 · d84cd33
1 parent db144ed
commit d84cd33
Show file tree

Hide file tree

Showing 12 changed files with 15 additions and 24 deletions.
diff --git a/components/codeflare/codeflare.go b/components/codeflare/codeflare.go
@@ -67,7 +67,6 @@ func (c *CodeFlare) ReconcileComponent(ctx context.Context,
 	l := c.ConfigComponentLogger(logger, ComponentName, dscispec)
 	var imageParamMap = map[string]string{
 		"codeflare-operator-controller-image": "RELATED_IMAGE_ODH_CODEFLARE_OPERATOR_IMAGE", // no need mcad, embedded in cfo
-		"namespace":                           dscispec.ApplicationsNamespace,
 	}
 
 	enabled := c.GetManagementState() == operatorv1.Managed
@@ -93,7 +92,7 @@ func (c *CodeFlare) ReconcileComponent(ctx context.Context,
 
 		// Update image parameters only when we do not have customized manifests set
 		if (dscispec.DevFlags == nil || dscispec.DevFlags.ManifestsUri == "") && (c.DevFlags == nil || len(c.DevFlags.Manifests) == 0) {
-			if err := deploy.ApplyParams(ParamsPath, imageParamMap, true); err != nil {
+			if err := deploy.ApplyParams(ParamsPath, imageParamMap, map[string]string{"namespace": dscispec.ApplicationsNamespace}); err != nil {
 				return fmt.Errorf("failed update image from %s : %w", CodeflarePath+"/bases", err)
 			}
 		}

diff --git a/components/dashboard/dashboard.go b/components/dashboard/dashboard.go
@@ -122,7 +122,7 @@ func (d *Dashboard) ReconcileComponent(ctx context.Context,
 		}
 
 		// 4. update params.env regardless devFlags is provided of not
-		if err := deploy.ApplyParams(entryPath, imageParamMap, false, extraParamsMap); err != nil {
+		if err := deploy.ApplyParams(entryPath, imageParamMap, extraParamsMap); err != nil {
 			return fmt.Errorf("failed to update params.env  from %s : %w", entryPath, err)
 		}
 	}

diff --git a/components/datasciencepipelines/datasciencepipelines.go b/components/datasciencepipelines/datasciencepipelines.go
@@ -104,7 +104,7 @@ func (d *DataSciencePipelines) ReconcileComponent(ctx context.Context,
 		// skip check if the dependent operator has beeninstalled, this is done in dashboard
 		// Update image parameters only when we do not have customized manifests set
 		if (dscispec.DevFlags == nil || dscispec.DevFlags.ManifestsUri == "") && (d.DevFlags == nil || len(d.DevFlags.Manifests) == 0) {
-			if err := deploy.ApplyParams(Path, imageParamMap, false); err != nil {
+			if err := deploy.ApplyParams(Path, imageParamMap); err != nil {
 				return fmt.Errorf("failed to update image from %s : %w", Path, err)
 			}
 		}

diff --git a/components/kserve/kserve.go b/components/kserve/kserve.go
@@ -144,7 +144,7 @@ func (k *Kserve) ReconcileComponent(ctx context.Context, cli client.Client,
 		}
 		// Update image parameters for odh-model-controller
 		if (dscispec.DevFlags == nil || dscispec.DevFlags.ManifestsUri == "") && (k.DevFlags == nil || len(k.DevFlags.Manifests) == 0) {
-			if err := deploy.ApplyParams(DependentPath, dependentParamMap, false); err != nil {
+			if err := deploy.ApplyParams(DependentPath, dependentParamMap); err != nil {
 				return fmt.Errorf("failed to update image %s: %w", DependentPath, err)
 			}
 		}

diff --git a/components/kueue/kueue.go b/components/kueue/kueue.go
@@ -70,7 +70,7 @@ func (k *Kueue) ReconcileComponent(ctx context.Context, cli client.Client, logge
 			}
 		}
 		if (dscispec.DevFlags == nil || dscispec.DevFlags.ManifestsUri == "") && (k.DevFlags == nil || len(k.DevFlags.Manifests) == 0) {
-			if err := deploy.ApplyParams(Path, imageParamMap, true); err != nil {
+			if err := deploy.ApplyParams(Path, imageParamMap); err != nil {
 				return fmt.Errorf("failed to update image from %s : %w", Path, err)
 			}
 		}

diff --git a/components/modelmeshserving/modelmeshserving.go b/components/modelmeshserving/modelmeshserving.go
@@ -114,7 +114,7 @@ func (m *ModelMeshServing) ReconcileComponent(ctx context.Context,
 		}
 		// Update image parameters
 		if (dscispec.DevFlags == nil || dscispec.DevFlags.ManifestsUri == "") && (m.DevFlags == nil || len(m.DevFlags.Manifests) == 0) {
-			if err := deploy.ApplyParams(Path, imageParamMap, false); err != nil {
+			if err := deploy.ApplyParams(Path, imageParamMap); err != nil {
 				return fmt.Errorf("failed update image from %s : %w", Path, err)
 			}
 		}
@@ -132,7 +132,7 @@ func (m *ModelMeshServing) ReconcileComponent(ctx context.Context,
 		}
 		// Update image parameters for odh-model-controller
 		if dscispec.DevFlags == nil || dscispec.DevFlags.ManifestsUri == "" {
-			if err := deploy.ApplyParams(DependentPath, dependentImageParamMap, false); err != nil {
+			if err := deploy.ApplyParams(DependentPath, dependentImageParamMap); err != nil {
 				return err
 			}
 		}

diff --git a/components/modelregistry/modelregistry.go b/components/modelregistry/modelregistry.go
@@ -79,7 +79,7 @@ func (m *ModelRegistry) ReconcileComponent(ctx context.Context, cli client.Clien
 
 		// 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, imageParamMap, false); err != nil {
+			if err := deploy.ApplyParams(Path, imageParamMap); err != nil {
 				return fmt.Errorf("failed to update image from %s : %w", Path, err)
 			}
 		}

diff --git a/components/ray/ray.go b/components/ray/ray.go
@@ -61,7 +61,6 @@ func (r *Ray) ReconcileComponent(ctx context.Context, cli client.Client, logger
 
 	var imageParamMap = map[string]string{
 		"odh-kuberay-operator-controller-image": "RELATED_IMAGE_ODH_KUBERAY_OPERATOR_CONTROLLER_IMAGE",
-		"namespace":                             dscispec.ApplicationsNamespace,
 	}
 
 	enabled := r.GetManagementState() == operatorv1.Managed
@@ -75,7 +74,7 @@ func (r *Ray) ReconcileComponent(ctx context.Context, cli client.Client, logger
 			}
 		}
 		if (dscispec.DevFlags == nil || dscispec.DevFlags.ManifestsUri == "") && (r.DevFlags == nil || len(r.DevFlags.Manifests) == 0) {
-			if err := deploy.ApplyParams(RayPath, imageParamMap, true); err != nil {
+			if err := deploy.ApplyParams(RayPath, imageParamMap, map[string]string{"namespace": dscispec.ApplicationsNamespace}); err != nil {
 				return fmt.Errorf("failed to update image from %s : %w", RayPath, err)
 			}
 		}

diff --git a/components/trainingoperator/trainingoperator.go b/components/trainingoperator/trainingoperator.go
@@ -61,7 +61,6 @@ func (r *TrainingOperator) ReconcileComponent(ctx context.Context, cli client.Cl
 
 	var imageParamMap = map[string]string{
 		"odh-training-operator-controller-image": "RELATED_IMAGE_ODH_TRAINING_OPERATOR_IMAGE",
-		"namespace":                              dscispec.ApplicationsNamespace,
 	}
 
 	enabled := r.GetManagementState() == operatorv1.Managed
@@ -75,7 +74,7 @@ func (r *TrainingOperator) ReconcileComponent(ctx context.Context, cli client.Cl
 			}
 		}
 		if (dscispec.DevFlags == nil || dscispec.DevFlags.ManifestsUri == "") && (r.DevFlags == nil || len(r.DevFlags.Manifests) == 0) {
-			if err := deploy.ApplyParams(TrainingOperatorPath, imageParamMap, true); err != nil {
+			if err := deploy.ApplyParams(TrainingOperatorPath, imageParamMap); err != nil {
 				return err
 			}
 		}

diff --git a/components/trustyai/trustyai.go b/components/trustyai/trustyai.go
@@ -73,7 +73,7 @@ func (t *TrustyAI) ReconcileComponent(ctx context.Context, cli client.Client, lo
 			}
 		}
 		if (dscispec.DevFlags == nil || dscispec.DevFlags.ManifestsUri == "") && (t.DevFlags == nil || len(t.DevFlags.Manifests) == 0) {
-			if err := deploy.ApplyParams(Path, imageParamMap, false); err != nil {
+			if err := deploy.ApplyParams(Path, imageParamMap); err != nil {
 				return fmt.Errorf("failed to update image %s: %w", Path, err)
 			}
 		}

diff --git a/components/workbenches/workbenches.go b/components/workbenches/workbenches.go
@@ -141,11 +141,11 @@ func (w *Workbenches) ReconcileComponent(ctx context.Context, cli client.Client,
 		if (dscispec.DevFlags == nil || dscispec.DevFlags.ManifestsUri == "") && (w.DevFlags == nil || len(w.DevFlags.Manifests) == 0) {
 			if platform == cluster.ManagedRhods || platform == cluster.SelfManagedRhods {
 				// for kf-notebook-controller image
-				if err := deploy.ApplyParams(notebookControllerPath, imageParamMap, false); err != nil {
+				if err := deploy.ApplyParams(notebookControllerPath, imageParamMap); err != nil {
 					return fmt.Errorf("failed to update image %s: %w", notebookControllerPath, err)
 				}
 				// for odh-notebook-controller image
-				if err := deploy.ApplyParams(kfnotebookControllerPath, imageParamMap, false); err != nil {
+				if err := deploy.ApplyParams(kfnotebookControllerPath, imageParamMap); err != nil {
 					return fmt.Errorf("failed to update image %s: %w", kfnotebookControllerPath, err)
 				}
 			}

diff --git a/pkg/deploy/envParams.go b/pkg/deploy/envParams.go
@@ -15,10 +15,9 @@ priority of image values (from high to low):
 - image values set in manifests params.env if manifestsURI is set
 - RELATED_IMAGE_* values from CSV (if it is set)
 - image values set in manifests params.env if manifestsURI is not set.
-parameter isUpdateNamespace is used to set if should update namespace  with DSCI CR's applicationnamespace.
 extraParamsMaps is used to set extra parameters which are not carried from ENV variable. this can be passed per component.
 */
-func ApplyParams(componentPath string, imageParamsMap map[string]string, isUpdateNamespace bool, extraParamsMaps ...map[string]string) error {
+func ApplyParams(componentPath string, imageParamsMap map[string]string, extraParamsMaps ...map[string]string) error {
 	paramsFile := filepath.Join(componentPath, "params.env")
 	// Require params.env at the root folder
 	paramsEnv, err := os.Open(paramsFile)
@@ -54,12 +53,7 @@ func ApplyParams(componentPath string, imageParamsMap map[string]string, isUpdat
 		}
 	}
 
-	// 2. Update namespace variable with applicationNamepsace
-	if isUpdateNamespace {
-		paramsEnvMap["namespace"] = imageParamsMap["namespace"]
-	}
-
-	// 3. Update other fileds with extraParamsMap which are not carried from component
+	// 2. Update other fileds with extraParamsMap which are not carried from component
 	for _, extraParamsMap := range extraParamsMaps {
 		for eKey, eValue := range extraParamsMap {
 			paramsEnvMap[eKey] = eValue