update: add check before calling ApplyParams

- only when any of value(image, env) not present in params.env then call ApplyParams
- avoild frequently disk written to params.env by every reconcile on the components
- remove RELATED_IMAGE_ODH_MODEL_CONTROLLER_IMAGE from modelserving imageParams list
  because it is in the dependent one and never exists in params.env
- fix a wrong path in log for codeflare

Signed-off-by: Wen Zhou <[email protected]>

components/codeflare/codeflare.go

@@ -6,6 +6,7 @@ package codeflare
 import (
 	"context"
 	"fmt"
+	"os"
 	"path/filepath"
 
 	"github.com/go-logr/logr"
@@ -68,7 +69,6 @@ func (c *CodeFlare) ReconcileComponent(ctx context.Context,
 	var imageParamMap = map[string]string{
 		"codeflare-operator-controller-image": "RELATED_IMAGE_ODH_CODEFLARE_OPERATOR_IMAGE", // no need mcad, embedded in cfo
 	}
-
 	enabled := c.GetManagementState() == operatorv1.Managed
 	monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed
 
@@ -92,8 +92,14 @@ 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, map[string]string{"namespace": dscispec.ApplicationsNamespace}); err != nil {
-				return fmt.Errorf("failed update image from %s : %w", CodeflarePath+"/bases", err)
+			// only update if passing image does not exist in params.env, to avoid unnecessary disk written
+			if err := deploy.CheckParams(ParamsPath, []string{
+				os.Getenv("RELATED_IMAGE_ODH_CODEFLARE_OPERATOR_IMAGE"),
+				dscispec.ApplicationsNamespace,
+			}); err != nil {
+				if err := deploy.ApplyParams(ParamsPath, imageParamMap, map[string]string{"namespace": dscispec.ApplicationsNamespace}); err != nil {
+					return fmt.Errorf("failed to update image from %s: %w", ParamsPath, err)
+				}
 			}
 		}
 	}

components/dashboard/dashboard.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"os"
 	"path/filepath"
 
 	"github.com/go-logr/logr"
@@ -121,9 +122,17 @@ func (d *Dashboard) ReconcileComponent(ctx context.Context,
 			return errors.New("failed to set variable for extraParamsMap")
 		}
 
-		// 4. update params.env regardless devFlags is provided of not
-		if err := deploy.ApplyParams(entryPath, imageParamMap, extraParamsMap); err != nil {
-			return fmt.Errorf("failed to update params.env  from %s : %w", entryPath, err)
+		// 4. update params.env regardless devFlags is provided or not
+		// check is to avoid unnecessary disk written
+		var paramsMapValues []string
+		for _, envVariable := range extraParamsMap {
+			paramsMapValues = append(paramsMapValues, envVariable)
+		}
+		paramsMapValues = append(paramsMapValues, os.Getenv("RELATED_IMAGE_ODH_DASHBOARD_IMAGE"))
+		if err := deploy.CheckParams(entryPath, paramsMapValues); err != nil {
+			if err := deploy.ApplyParams(entryPath, imageParamMap, extraParamsMap); err != nil {
+				return fmt.Errorf("failed to update image and env variabls from %s: %w", entryPath, err)
+			}
 		}
 	}
 

components/datasciencepipelines/datasciencepipelines.go

@@ -6,6 +6,7 @@ package datasciencepipelines
 import (
 	"context"
 	"fmt"
+	"os"
 	"path/filepath"
 
 	"github.com/go-logr/logr"
@@ -104,8 +105,15 @@ 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); err != nil {
-				return fmt.Errorf("failed to update image from %s : %w", Path, err)
+			// only update if passing image does not exist in params.env, to avoid unnecessary disk written
+			var paramsMapValues []string
+			for _, image := range imageParamMap {
+				paramsMapValues = append(paramsMapValues, os.Getenv(image))
+			}
+			if err := deploy.CheckParams(Path, paramsMapValues); err != nil {
+				if err := deploy.ApplyParams(Path, imageParamMap); err != nil {
+					return fmt.Errorf("failed to update images from %s: %w", Path, err)
+				}
 			}
 		}
 		// Check for existing Argo Workflows

components/kserve/kserve.go

@@ -5,6 +5,7 @@ package kserve
 import (
 	"context"
 	"fmt"
+	"os"
 	"path/filepath"
 	"strings"
 
@@ -144,8 +145,11 @@ 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); err != nil {
-				return fmt.Errorf("failed to update image %s: %w", DependentPath, err)
+			// only update if passing image does not exist in params.env, to avoid unnecessary disk written
+			if err := deploy.CheckParams(Path, []string{os.Getenv("RELATED_IMAGE_ODH_MODEL_CONTROLLER_IMAGE")}); err != nil {
+				if err := deploy.ApplyParams(DependentPath, dependentParamMap); err != nil {
+					return fmt.Errorf("failed to update image from %s: %w", DependentPath, err)
+				}
 			}
 		}
 	}

components/kueue/kueue.go

@@ -4,6 +4,7 @@ package kueue
 import (
 	"context"
 	"fmt"
+	"os"
 	"path/filepath"
 
 	"github.com/go-logr/logr"
@@ -70,8 +71,11 @@ 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); err != nil {
-				return fmt.Errorf("failed to update image from %s : %w", Path, err)
+			// only update if passing image does not exist in params.env, to avoid unnecessary disk written
+			if err := deploy.CheckParams(Path, []string{os.Getenv("RELATED_IMAGE_ODH_KUEUE_CONTROLLER_IMAGE")}); err != nil {
+				if err := deploy.ApplyParams(Path, imageParamMap); err != nil {
+					return fmt.Errorf("failed to update image from %s : %w", Path, err)
+				}
 			}
 		}
 	}

components/modelmeshserving/modelmeshserving.go

@@ -5,6 +5,7 @@ package modelmeshserving
 import (
 	"context"
 	"fmt"
+	"os"
 	"path/filepath"
 	"strings"
 
@@ -85,7 +86,6 @@ func (m *ModelMeshServing) ReconcileComponent(ctx context.Context,
 		"odh-modelmesh-runtime-adapter": "RELATED_IMAGE_ODH_MODELMESH_RUNTIME_ADAPTER_IMAGE",
 		"odh-modelmesh":                 "RELATED_IMAGE_ODH_MODELMESH_IMAGE",
 		"odh-modelmesh-controller":      "RELATED_IMAGE_ODH_MODELMESH_CONTROLLER_IMAGE",
-		"odh-model-controller":          "RELATED_IMAGE_ODH_MODEL_CONTROLLER_IMAGE",
 	}
 
 	// odh-model-controller to use
@@ -114,8 +114,15 @@ 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); err != nil {
-				return fmt.Errorf("failed update image from %s : %w", Path, err)
+			// only update if passing image does not exist in params.env, to avoid unnecessary disk written
+			var paramsMapValues []string
+			for _, image := range imageParamMap {
+				paramsMapValues = append(paramsMapValues, os.Getenv(image))
+			}
+			if err := deploy.CheckParams(Path, paramsMapValues); err != nil {
+				if err := deploy.ApplyParams(Path, imageParamMap); err != nil {
+					return fmt.Errorf("failed to update image from %s: %w", Path, err)
+				}
 			}
 		}
 	}
@@ -132,8 +139,11 @@ 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); err != nil {
-				return err
+			// only update if passing image does not exist in params.env, to avoid unnecessary disk written
+			if err := deploy.CheckParams(Path, []string{os.Getenv("RELATED_IMAGE_ODH_MODEL_CONTROLLER_IMAGE")}); err != nil {
+				if err := deploy.ApplyParams(DependentPath, dependentImageParamMap); err != nil {
+					return fmt.Errorf("failed to update image from %s: %w", dependentImageParamMap, err)
+				}
 			}
 		}
 	}

components/modelregistry/modelregistry.go

@@ -5,6 +5,7 @@ package modelregistry
 import (
 	"context"
 	"fmt"
+	"os"
 	"path/filepath"
 
 	"github.com/go-logr/logr"
@@ -79,8 +80,15 @@ 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); err != nil {
-				return fmt.Errorf("failed to update image from %s : %w", Path, err)
+			// only update if passing image does not exist in params.env, to avoid unnecessary disk written
+			var paramsMapValues []string
+			for _, image := range imageParamMap {
+				paramsMapValues = append(paramsMapValues, os.Getenv(image))
+			}
+			if err := deploy.CheckParams(Path, paramsMapValues); err != nil {
+				if err := deploy.ApplyParams(Path, imageParamMap); err != nil {
+					return fmt.Errorf("failed to update image from %s: %w", Path, err)
+				}
 			}
 		}
 

components/ray/ray.go

@@ -6,6 +6,7 @@ package ray
 import (
 	"context"
 	"fmt"
+	"os"
 	"path/filepath"
 
 	"github.com/go-logr/logr"
@@ -74,8 +75,14 @@ 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, map[string]string{"namespace": dscispec.ApplicationsNamespace}); err != nil {
-				return fmt.Errorf("failed to update image from %s : %w", RayPath, err)
+			// only update if passing image does not exist in params.env, to avoid unnecessary disk written
+			if err := deploy.CheckParams(RayPath, []string{
+				os.Getenv("RELATED_IMAGE_ODH_KUBERAY_OPERATOR_CONTROLLER_IMAGE"),
+				dscispec.ApplicationsNamespace,
+			}); 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)
+				}
 			}
 		}
 	}

components/trainingoperator/trainingoperator.go

@@ -6,6 +6,7 @@ package trainingoperator
 import (
 	"context"
 	"fmt"
+	"os"
 	"path/filepath"
 
 	"github.com/go-logr/logr"
@@ -74,8 +75,11 @@ 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); err != nil {
-				return err
+			// only update if passing image does not exist in params.env, to avoid unnecessary disk written
+			if err := deploy.CheckParams(TrainingOperatorPath, []string{os.Getenv("RELATED_IMAGE_ODH_TRAINING_OPERATOR_IMAGE")}); err != nil {
+				if err := deploy.ApplyParams(TrainingOperatorPath, imageParamMap); err != nil {
+					return fmt.Errorf("failed to update image from %s: %w", TrainingOperatorPath, err)
+				}
 			}
 		}
 	}

components/trustyai/trustyai.go

@@ -5,6 +5,7 @@ package trustyai
 import (
 	"context"
 	"fmt"
+	"os"
 	"path/filepath"
 
 	"github.com/go-logr/logr"
@@ -73,8 +74,14 @@ 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); err != nil {
-				return fmt.Errorf("failed to update image %s: %w", Path, err)
+			// only update if passing image does not exist in params.env, to avoid unnecessary disk written
+			if err := deploy.CheckParams(Path, []string{
+				os.Getenv("RELATED_IMAGE_ODH_TRUSTYAI_SERVICE_IMAGE"),
+				os.Getenv("RELATED_IMAGE_ODH_TRUSTYAI_SERVICE_OPERATOR_IMAGE"),
+			}); err != nil {
+				if err := deploy.ApplyParams(Path, imageParamMap); err != nil {
+					return fmt.Errorf("failed to update image from %s: %w", Path, err)
+				}
 			}
 		}
 	}

components/workbenches/workbenches.go

@@ -5,6 +5,7 @@ package workbenches
 import (
 	"context"
 	"fmt"
+	"os"
 	"path/filepath"
 	"strings"
 
@@ -140,13 +141,18 @@ func (w *Workbenches) ReconcileComponent(ctx context.Context, cli client.Client,
 	if enabled {
 		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); err != nil {
-					return fmt.Errorf("failed to update image %s: %w", notebookControllerPath, err)
+				// for odh-notebook-controller image: adding post-check to ensure params.env is updated
+				if err := deploy.CheckParams(notebookControllerPath, []string{os.Getenv("RELATED_IMAGE_ODH_NOTEBOOK_CONTROLLER_IMAGE")}); 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); err != nil {
-					return fmt.Errorf("failed to update image %s: %w", kfnotebookControllerPath, err)
+
+				// for kf-notebook-controller image: only update if passing image does not exist, to avoid unnecessary disk written
+				if err := deploy.CheckParams(kfnotebookControllerPath, []string{os.Getenv("RELATED_IMAGE_ODH_KF_NOTEBOOK_CONTROLLER_IMAGE")}); err != nil {
+					if err := deploy.ApplyParams(kfnotebookControllerPath, imageParamMap); err != nil {
+						return fmt.Errorf("failed to update image %s: %w", kfnotebookControllerPath, err)
+					}
 				}
 			}
 		}

pkg/deploy/deploy_suite_test.go

@@ -0,0 +1,77 @@
+package deploy_test
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+func TestStringSearch(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Check Params Suite")
+}
+
+var _ = Describe("Throw error if image still use old image registry", func() {
+
+	var (
+		tmpFile   *os.File
+		filePath  = "/tmp/manifests/components/one"
+		err       error
+		paramsenv string
+	)
+
+	// create a temp params.env file
+	BeforeEach(func() {
+		err = os.MkdirAll(filePath, 0755)
+		tmpFile, err = os.Create(filepath.Join(filePath, "params.env"))
+		Expect(err).NotTo(HaveOccurred())
+
+		// fake some lines
+		paramsenv = `url=https://www.odh.rbac.com
+myimagee=quay.io/opendatahub/repo:abc
+namespace=opendatahub`
+		_, err = tmpFile.WriteString(paramsenv)
+		Expect(err).NotTo(HaveOccurred())
+
+		err = tmpFile.Close()
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+	AfterEach(func() {
+		err = os.Remove(tmpFile.Name())
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+	Context("when searching in params.env", func() {
+		It("Should not throw error when string found", func() {
+			err := deploy.CheckParams(filePath, []string{"abc"})
+			Expect(err).NotTo(HaveOccurred())
+		})
+
+		It("Should not throw error when all string are matched", func() {
+			err := deploy.CheckParams(filePath, []string{"rbac", "abc"})
+			Expect(err).NotTo(HaveOccurred())
+		})
+
+		It("Should not throw error when emptry string provided(as env not found)", func() {
+			err := deploy.CheckParams(filePath, []string{""})
+			Expect(err).NotTo(HaveOccurred())
+		})
+
+		It("Should throw error when string not found", func() {
+			err := deploy.CheckParams(filePath, []string{"vw"})
+			Expect(err).To(HaveOccurred())
+		})
+
+		It("Should throw error when multiple strings are not found", func() {
+			err := deploy.CheckParams(filePath, []string{"vw", "123"})
+			Expect(err).To(HaveOccurred())
+		})
+	})
+
+})