update: remove check in ModelReg code, add it to upgrade logic

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

components/modelregistry/modelregistry.go

@@ -11,10 +11,8 @@ import (
 	"text/template"
 
 	operatorv1 "github.com/openshift/api/operator/v1"
-	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 
@@ -24,7 +22,6 @@ import (
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/conversion"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy"
-	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/labels"
 
 	_ "embed"
 )
@@ -162,10 +159,6 @@ func (m *ModelRegistry) ReconcileComponent(ctx context.Context, cli client.Clien
 	l.Info("apply extra manifests done")
 
 	if enabled {
-		// remove leftover kube-rbac-proxy container from older ODH installs
-		if err := removeUnusedKubeRbacProxy(ctx, cli, m.GetComponentName(), dscispec.ApplicationsNamespace); err != nil {
-			return fmt.Errorf("failed to remove kube-rbac-proxy from deployment for %s in namespace %s: %w", ComponentName, dscispec.ApplicationsNamespace, err)
-		}
 		if err := cluster.WaitForDeploymentAvailable(ctx, cli, m.GetComponentName(), dscispec.ApplicationsNamespace, 10, 1); err != nil {
 			return fmt.Errorf("deployment for %s is not ready to server: %w", ComponentName, err)
 		}
@@ -187,38 +180,6 @@ func (m *ModelRegistry) ReconcileComponent(ctx context.Context, cli client.Clien
 	return nil
 }
 
-// removeUnusedKubeRbacProxy removes older kube-rbac-proxy container in mr operator deployment.
-// This is required because patching a deployment using apply-patch doesn't remove older containers.
-// See: https://kubernetes.io/docs/tasks/manage-kubernetes-objects/update-api-object-kubectl-patch/#notes-on-the-strategic-merge-patch
-func removeUnusedKubeRbacProxy(ctx context.Context, cli client.Client, name string, namespace string) error {
-	// get MR operator deployment
-	componentDeploymentList := &appsv1.DeploymentList{}
-	err := cli.List(ctx, componentDeploymentList, client.InNamespace(namespace), client.HasLabels{labels.ODH.Component(name)})
-	if err != nil {
-		return fmt.Errorf("error fetching list of deployments: %w", err)
-	}
-	nItems := len(componentDeploymentList.Items)
-	if nItems != 1 {
-		return fmt.Errorf("error fetching model registry operator deployment, found %d deployments", nItems)
-	}
-
-	// check if deployment has a kube-rbac-proxy container
-	deployment := componentDeploymentList.Items[0]
-	containers := deployment.Spec.Template.Spec.Containers
-	// there should be a single container in latest deployment without kube-rbac-proxy
-	if len(containers) == 1 {
-		return nil
-	}
-	for i, container := range containers {
-		if container.Name == "kube-rbac-proxy" {
-			// remove if found
-			return cli.Patch(ctx, &deployment, client.RawPatch(types.JSONPatchType,
-				[]byte(fmt.Sprintf("[{\"op\": \"remove\", \"path\": \"/spec/template/spec/containers/%d\"}]", i))))
-		}
-	}
-	return nil
-}
-
 func (m *ModelRegistry) createDependencies(ctx context.Context, cli client.Client, dscispec *dsciv1.DSCInitializationSpec) error {
 	// create DefaultModelRegistryCert
 	if err := cluster.PropagateDefaultIngressCertificate(ctx, cli, DefaultModelRegistryCert, dscispec.ServiceMesh.ControlPlane.Namespace); err != nil {

pkg/upgrade/upgrade.go

@@ -43,6 +43,7 @@ import (
 	"github.com/opendatahub-io/opendatahub-operator/v2/components/workbenches"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk"
+	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/labels"
 )
 
 type ResourceSpec struct {
@@ -275,6 +276,12 @@ func CleanupExistingResource(ctx context.Context,
 			return err
 		}
 	}
+	// remove modelreg proxy container from deployment in ODH
+	if platform == cluster.OpenDataHub {
+		if err := removeRBACProxyModelRegistry(ctx, cli, "model-registry-operator", "kube-rbac-proxy", dscApplicationsNamespace); err != nil {
+			return err
+		}
+	}
 
 	// to take a reference
 	toDelete := getDashboardWatsonResources(dscApplicationsNamespace)
@@ -487,6 +494,38 @@ func updateODCModelRegistry(ctx context.Context, cli client.Client, instanceName
 	return nil
 }
 
+// workaround for RHOAIENG-15328
+// TODO: this can be removed from ODH 2.22.
+func removeRBACProxyModelRegistry(ctx context.Context, cli client.Client, componentName string, containerName string, applicationNS string) error {
+	log := logf.FromContext(ctx)
+	deploymentList := &appsv1.DeploymentList{}
+	if err := cli.List(ctx, deploymentList, client.InNamespace(applicationNS), client.HasLabels{labels.ODH.Component(componentName)}); err != nil {
+		return fmt.Errorf("error fetching list of deployments: %w", err)
+	}
+
+	if len(deploymentList.Items) != 1 { // ModelRegistry operator is not deployed
+		return nil
+	}
+	mrDeployment := deploymentList.Items[0]
+	mrContainerList := mrDeployment.Spec.Template.Spec.Containers
+	// if only one container in deployment, we are already on newer deployment, no need more action
+	if len(mrContainerList) == 1 {
+		return nil
+	}
+
+	log.Info("Upgrade force ModelRegistry to remove container from deployment")
+	for i, container := range mrContainerList {
+		if container.Name == containerName {
+			removeUnusedKubeRbacProxy := []byte(fmt.Sprintf("[{\"op\": \"remove\", \"path\": \"/spec/template/spec/containers/%d\"}]", i))
+			if err := cli.Patch(ctx, &mrDeployment, client.RawPatch(types.JSONPatchType, removeUnusedKubeRbacProxy)); err != nil {
+				return fmt.Errorf("error removing ModelRegistry %s container from deployment: %w", containerName, err)
+			}
+			break
+		}
+	}
+	return nil
+}
+
 func RemoveLabel(ctx context.Context, cli client.Client, objectName string, labelKey string) error {
 	foundNamespace := &corev1.Namespace{}
 	if err := cli.Get(ctx, client.ObjectKey{Name: objectName}, foundNamespace); err != nil {