From f629f8d93460d6c274220b9dc49cd34d4823b0f9 Mon Sep 17 00:00:00 2001 From: Wen Zhou Date: Fri, 16 Aug 2024 02:15:22 -0400 Subject: [PATCH] update: remove unnecessary param from ApplyParams() (#1180) - extraParamsMaps is generic to use Signed-off-by: Wen Zhou (cherry picked from commit d84cd337ff1de31bf6570d977901c7619a6c5bd6) --- components/codeflare/codeflare.go | 3 +-- components/dashboard/dashboard.go | 2 +- .../datasciencepipelines/datasciencepipelines.go | 2 +- components/kserve/kserve.go | 2 +- components/kueue/kueue.go | 2 +- components/modelmeshserving/modelmeshserving.go | 4 ++-- components/ray/ray.go | 3 +-- components/trainingoperator/trainingoperator.go | 3 +-- components/trustyai/trustyai.go | 2 +- components/workbenches/workbenches.go | 4 ++-- pkg/deploy/envParams.go | 10 ++-------- 11 files changed, 14 insertions(+), 23 deletions(-) diff --git a/components/codeflare/codeflare.go b/components/codeflare/codeflare.go index f40d3f648ef..d4d608d7003 100644 --- 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 index fbe6c9f2979..4eb3049b91d 100644 --- 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 index 578c15614de..196923aa0e4 100644 --- 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 index 39ca5a6b923..2a62d93719a 100644 --- a/components/kserve/kserve.go +++ b/components/kserve/kserve.go @@ -143,7 +143,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 index 339a5c33d4f..8b0143ac51d 100644 --- 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 index 482a0cad513..d07aed61953 100644 --- 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/ray/ray.go b/components/ray/ray.go index fb721bba576..a854dbaaf4e 100644 --- a/components/ray/ray.go +++ b/components/ray/ray.go @@ -60,7 +60,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 @@ -74,7 +73,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 index 62092e881d7..06469b980c0 100644 --- a/components/trainingoperator/trainingoperator.go +++ b/components/trainingoperator/trainingoperator.go @@ -59,7 +59,6 @@ func (r *TrainingOperator) ReconcileComponent(ctx context.Context, cli client.Cl l := r.ConfigComponentLogger(logger, ComponentName, dscispec) var imageParamMap = map[string]string{ "odh-training-operator-controller-image": "RELATED_IMAGE_ODH_TRAINING_OPERATOR_IMAGE", - "namespace": dscispec.ApplicationsNamespace, } enabled := r.GetManagementState() == operatorv1.Managed @@ -73,7 +72,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 fmt.Errorf("failed to update image from %s : %w", TrainingOperatorPath, err) } } diff --git a/components/trustyai/trustyai.go b/components/trustyai/trustyai.go index 0bcf312eddc..f5603ab17a8 100644 --- 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 index e88a4305476..e5a522d711c 100644 --- 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 index b3d748ee689..e8250848ee7 100644 --- 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