Set dsp-version label on managed resources

This change makes it so that new reconciles on dspa owned resources are labelled with a dsp-version=<dspa-version> label. This is to allow for the dspa selectively watch (for manual WatchesRawSources) on resources that are descendents of DSPAs with .spec.dspVersion set to supported versions. Signed-off-by: Humair Khan <[email protected]>
opendatahub-io · Oct 18, 2024 · c5e2af7 · c5e2af7
1 parent b541718
commit c5e2af7
Show file tree

Hide file tree

Showing 33 changed files with 168 additions and 19 deletions.
diff --git a/...ses/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml b/...ses/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml
@@ -859,7 +859,7 @@ spec:
       status: {}
   - deprecated: true
     deprecationWarning: datasciencepipelinesapplications.opendatahub.io/v1alpha1 is
-      deprecated
+      deprecated.
     name: v1alpha1
     schema:
       openAPIV3Schema:

diff --git a/controllers/config/defaults.go b/controllers/config/defaults.go
@@ -27,10 +27,13 @@ import (
 	"k8s.io/apimachinery/pkg/api/resource"
 )
 
+const DSPV2VersionString = "v2"
+const DSPVersionk8sLabel = "dsp-version"
+
+var SupportedDSPVersions = []string{DSPV2VersionString}
+
 const (
-	DSPDefaultVersion  = "v2"
-	DSPV2VersionString = DSPDefaultVersion
-	DefaultImageValue  = "MustSetInConfig"
+	DefaultImageValue = "MustSetInConfig"
 
 	CustomCABundleRootMountPath = "/dsp-custom-certs"
 
@@ -222,3 +225,7 @@ func GetDefaultDBExtraParams(params DBExtraParams, log logr.Logger) (string, err
 	}
 	return string(extraParamsJson), nil
 }
+
+func GetSupportedDSPAVersions() []string {
+	return SupportedDSPVersions
+}
diff --git a/controllers/dspipeline_controller.go b/controllers/dspipeline_controller.go
@@ -19,7 +19,6 @@ package controllers
 import (
 	"context"
 	"fmt"
-
 	"github.com/opendatahub-io/data-science-pipelines-operator/controllers/dspastatus"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
@@ -82,18 +81,25 @@ func (r *DSPAReconciler) Apply(owner mf.Owner, params *DSPAParams, template stri
 	if err != nil {
 		return fmt.Errorf("error loading template (%s) yaml: %w", template, err)
 	}
+
+	// Apply the owner injection transformation
 	tmplManifest, err = tmplManifest.Transform(
 		mf.InjectOwner(owner),
+		// Apply dsp-version=<ver> label on all resources managed by this dspo
+		util.AddLabelTransformer(config.DSPVersionk8sLabel, params.DSPVersion),
+		util.AddDeploymentPodLabelTransformer(config.DSPVersionk8sLabel, params.DSPVersion),
 	)
 	if err != nil {
 		return err
 	}
 
+	// Apply dsp-version labels to all manifests
 	tmplManifest, err = tmplManifest.Transform(fns...)
 	if err != nil {
 		return err
 	}
 
+	// Apply the manifest
 	return tmplManifest.Apply()
 }
 
@@ -177,7 +183,6 @@ func (r *DSPAReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
 	dspa := &dspav1.DataSciencePipelinesApplication{}
 	err := r.Get(ctx, req.NamespacedName, dspa)
 	if err != nil && apierrs.IsNotFound(err) {
-		// TODO change this to a Debug message when doing https://issues.redhat.com/browse/RHOAIENG-1650
 		log.Info("DSPA resource was not found, assuming it was recently deleted, nothing to do here")
 		return ctrl.Result{}, nil
 	} else if err != nil {
@@ -189,7 +194,7 @@ func (r *DSPAReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
 
 	defer r.updateStatus(ctx, dspa, dspaStatus, log, req)
 
-	if dspa.Spec.DSPVersion != config.DSPV2VersionString {
+	if !util.DSPAWithSupportedDSPVersion(dspa) {
 		err1 := fmt.Errorf("unsupported DSP version %s detected. Please manually remove "+
 			"this DSP resource and re-apply with a supported version field set", dspa.Spec.DSPVersion)
 		dspaStatus.SetDatabaseNotReady(err1, config.UnsupportedVersion)
@@ -595,7 +600,6 @@ func (r *DSPAReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		// Watch for global ca bundle, if one is added to this namespace
 		// we need to reconcile on all the dspa's in this namespace
 		// so they may mount this cert in the appropriate containers
-
 		WatchesRawSource(source.Kind(mgr.GetCache(), &corev1.ConfigMap{}),
 			handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, o client.Object) []reconcile.Request {
 				cm := o.(*corev1.ConfigMap)
@@ -606,8 +610,6 @@ func (r *DSPAReconciler) SetupWithManager(mgr ctrl.Manager) error {
 					return nil
 				}
 
-				log.V(1).Info(fmt.Sprintf("Reconcile event triggered by change in event on Global CA Bundle: %s", cm.Name))
-
 				var dspaList dspav1.DataSciencePipelinesApplicationList
 				if err := r.List(ctx, &dspaList, client.InNamespace(thisNamespace)); err != nil {
 					log.Error(err, "unable to list DSPA's when attempting to handle Global CA Bundle event.")
@@ -616,11 +618,18 @@ func (r *DSPAReconciler) SetupWithManager(mgr ctrl.Manager) error {
 
 				var reconcileRequests []reconcile.Request
 				for _, dspa := range dspaList.Items {
-					namespacedName := types.NamespacedName{
-						Name:      dspa.Name,
-						Namespace: thisNamespace,
+					// Only update supported DSP versions
+					if util.DSPAWithSupportedDSPVersion(&dspa) {
+						namespacedName := types.NamespacedName{
+							Name:      dspa.Name,
+							Namespace: thisNamespace,
+						}
+						reconcileRequests = append(reconcileRequests, reconcile.Request{NamespacedName: namespacedName})
 					}
-					reconcileRequests = append(reconcileRequests, reconcile.Request{NamespacedName: namespacedName})
+				}
+
+				if len(reconcileRequests) > 0 {
+					log.V(1).Info(fmt.Sprintf("Reconcile event triggered by change in event on Global CA Bundle: %s", cm.Name))
 				}
 
 				return reconcileRequests
@@ -636,6 +645,12 @@ func (r *DSPAReconciler) SetupWithManager(mgr ctrl.Manager) error {
 					return nil
 				}
 
+				// Silently skip reconcile on this pod if the resource was owned
+				// by an unsupported dspa
+				if !util.HasSupportedDSPVersionLabel(pod.Labels) {
+					return nil
+				}
+
 				dspaName, hasDSPALabel := pod.Labels["dspa"]
 				if !hasDSPALabel {
 					msg := fmt.Sprintf("Pod with data-science-pipelines label encountered, but is missing dspa "+
@@ -660,9 +675,6 @@ func (r *DSPAReconciler) SetupWithManager(mgr ctrl.Manager) error {
 				if secret.Annotations["openshift.io/owning-component"] != "service-ca" {
 					return nil
 				}
-
-				log.V(1).Info(fmt.Sprintf("Reconcile event triggered by change on Secret owned by service-ca: %s", secret.Name))
-
 				serviceName := secret.Annotations["service.beta.openshift.io/originating-service-name"]
 
 				namespacedServiceName := types.NamespacedName{
@@ -676,17 +688,22 @@ func (r *DSPAReconciler) SetupWithManager(mgr ctrl.Manager) error {
 				if err != nil {
 					return nil
 				}
-
 				dspaName, hasDSPALabel := service.Labels["dspa"]
 				if !hasDSPALabel {
 					return nil
 				}
 
-				log.V(1).Info(fmt.Sprintf("Reconcile event triggered by [Service: %s] ", serviceName))
+				// Silently skip reconcile on this ervice if the resource was owned
+				// by an unsupported DSPA
+				if !util.HasSupportedDSPVersionLabel(service.Labels) {
+					return nil
+				}
+
 				namespacedDspaName := types.NamespacedName{
 					Name:      dspaName,
 					Namespace: secret.Namespace,
 				}
+				log.V(1).Info(fmt.Sprintf("Reconcile event triggered by change on Secret: %s owned by service-ca: %s", secret.Name, serviceName))
 				return []reconcile.Request{{NamespacedName: namespacedDspaName}}
 			}),
 		).

diff --git a/controllers/testdata/declarative/case_0/expected/created/apiserver_deployment.yaml b/controllers/testdata/declarative/case_0/expected/created/apiserver_deployment.yaml
@@ -4,6 +4,7 @@ metadata:
   name: ds-pipeline-testdsp0
   namespace: default
   labels:
+    dsp-version: v2
     app: ds-pipeline-testdsp0
     component: data-science-pipelines
     dspa: testdsp0
@@ -16,6 +17,7 @@ spec:
   template:
     metadata:
       labels:
+        dsp-version: v2
         app: ds-pipeline-testdsp0
         component: data-science-pipelines
         dspa: testdsp0

diff --git a/controllers/testdata/declarative/case_0/expected/created/configmap_server_config.yaml b/controllers/testdata/declarative/case_0/expected/created/configmap_server_config.yaml
@@ -4,6 +4,7 @@ metadata:
   name: ds-pipeline-server-config-testdsp0
   namespace: default
   labels:
+    dsp-version: v2
     app: ds-pipeline-testdsp0
     component: data-science-pipelines
 data:

diff --git a/controllers/testdata/declarative/case_0/expected/created/mariadb_deployment.yaml b/controllers/testdata/declarative/case_0/expected/created/mariadb_deployment.yaml
@@ -5,6 +5,7 @@ metadata:
   name: mariadb-testdsp0
   namespace: default
   labels:
+    dsp-version: v2
     app: mariadb-testdsp0
     component: data-science-pipelines
     dspa: testdsp0
@@ -19,6 +20,7 @@ spec:
   template:
     metadata:
       labels:
+        dsp-version: v2
         app: mariadb-testdsp0
         component: data-science-pipelines
         dspa: testdsp0

diff --git a/controllers/testdata/declarative/case_0/expected/created/persistence-agent_deployment.yaml b/controllers/testdata/declarative/case_0/expected/created/persistence-agent_deployment.yaml
@@ -4,6 +4,7 @@ metadata:
   name: ds-pipeline-persistenceagent-testdsp0
   namespace: default
   labels:
+    dsp-version: v2
     app: ds-pipeline-persistenceagent-testdsp0
     component: data-science-pipelines
     dspa: testdsp0
@@ -18,6 +19,7 @@ spec:
       annotations:
         cluster-autoscaler.kubernetes.io/safe-to-evict: "true"
       labels:
+        dsp-version: v2
         app: ds-pipeline-persistenceagent-testdsp0
         component: data-science-pipelines
         dspa: testdsp0

diff --git a/controllers/testdata/declarative/case_0/expected/created/scheduled-workflow_deployment.yaml b/controllers/testdata/declarative/case_0/expected/created/scheduled-workflow_deployment.yaml
@@ -4,6 +4,7 @@ metadata:
   name: ds-pipeline-scheduledworkflow-testdsp0
   namespace: default
   labels:
+    dsp-version: v2
     app: ds-pipeline-scheduledworkflow-testdsp0
     component: data-science-pipelines
     dspa: testdsp0
@@ -18,6 +19,7 @@ spec:
       annotations:
         cluster-autoscaler.kubernetes.io/safe-to-evict: "true"
       labels:
+        dsp-version: v2
         app: ds-pipeline-scheduledworkflow-testdsp0
         component: data-science-pipelines
         dspa: testdsp0

diff --git a/controllers/testdata/declarative/case_2/expected/created/apiserver_deployment.yaml b/controllers/testdata/declarative/case_2/expected/created/apiserver_deployment.yaml
@@ -4,6 +4,7 @@ metadata:
   name: ds-pipeline-testdsp2
   namespace: default
   labels:
+    dsp-version: v2
     app: ds-pipeline-testdsp2
     component: data-science-pipelines
     dspa: testdsp2
@@ -16,6 +17,7 @@ spec:
   template:
     metadata:
       labels:
+        dsp-version: v2
         app: ds-pipeline-testdsp2
         component: data-science-pipelines
         dspa: testdsp2

diff --git a/controllers/testdata/declarative/case_2/expected/created/mariadb_deployment.yaml b/controllers/testdata/declarative/case_2/expected/created/mariadb_deployment.yaml
@@ -5,6 +5,7 @@ metadata:
   name: mariadb-testdsp2
   namespace: default
   labels:
+    dsp-version: v2
     app: mariadb-testdsp2
     component: data-science-pipelines
     dspa: testdsp2
@@ -19,6 +20,7 @@ spec:
   template:
     metadata:
       labels:
+        dsp-version: v2
         app: mariadb-testdsp2
         component: data-science-pipelines
         dspa: testdsp2

diff --git a/controllers/testdata/declarative/case_2/expected/created/minio_deployment.yaml b/controllers/testdata/declarative/case_2/expected/created/minio_deployment.yaml
@@ -4,6 +4,7 @@ metadata:
   name: minio-testdsp2
   namespace: default
   labels:
+    dsp-version: v2
     app: minio-testdsp2
     component: data-science-pipelines
     dspa: testdsp2
@@ -18,6 +19,7 @@ spec:
   template:
     metadata:
       labels:
+        dsp-version: v2
         app: minio-testdsp2
         component: data-science-pipelines
         dspa: testdsp2

diff --git a/controllers/testdata/declarative/case_2/expected/created/mlmd_envoy_deployment.yaml b/controllers/testdata/declarative/case_2/expected/created/mlmd_envoy_deployment.yaml
@@ -4,6 +4,7 @@ metadata:
   name: ds-pipeline-metadata-envoy-testdsp2
   namespace: default
   labels:
+    dsp-version: v2
     app: ds-pipeline-metadata-envoy-testdsp2
     component: data-science-pipelines
     dspa: testdsp2
@@ -19,6 +20,7 @@ spec:
       annotations:
         sidecar.istio.io/inject: "false"
       labels:
+        dsp-version: v2
         app: ds-pipeline-metadata-envoy-testdsp2
         component: data-science-pipelines
         dspa: testdsp2

diff --git a/controllers/testdata/declarative/case_2/expected/created/mlmd_grpc_deployment.yaml b/controllers/testdata/declarative/case_2/expected/created/mlmd_grpc_deployment.yaml
@@ -4,6 +4,7 @@ metadata:
   name: ds-pipeline-metadata-grpc-testdsp2
   namespace: default
   labels:
+    dsp-version: v2
     app: ds-pipeline-metadata-grpc-testdsp2
     component: data-science-pipelines
     dspa: testdsp2
@@ -17,6 +18,7 @@ spec:
   template:
     metadata:
       labels:
+        dsp-version: v2
         app: ds-pipeline-metadata-grpc-testdsp2
         component: data-science-pipelines
         dspa: testdsp2

diff --git a/controllers/testdata/declarative/case_2/expected/created/mlpipelines-ui_deployment.yaml b/controllers/testdata/declarative/case_2/expected/created/mlpipelines-ui_deployment.yaml
@@ -4,6 +4,7 @@ metadata:
   name: ds-pipeline-ui-testdsp2
   namespace: default
   labels:
+    dsp-version: v2
     app: ds-pipeline-ui-testdsp2
     component: data-science-pipelines
     dspa: testdsp2
@@ -18,6 +19,7 @@ spec:
       annotations:
         cluster-autoscaler.kubernetes.io/safe-to-evict: "true"
       labels:
+        dsp-version: v2
         app: ds-pipeline-ui-testdsp2
         component: data-science-pipelines
         dspa: testdsp2

diff --git a/controllers/testdata/declarative/case_2/expected/created/persistence-agent_deployment.yaml b/controllers/testdata/declarative/case_2/expected/created/persistence-agent_deployment.yaml
@@ -4,6 +4,7 @@ metadata:
   name: ds-pipeline-persistenceagent-testdsp2
   namespace: default
   labels:
+    dsp-version: v2
     app: ds-pipeline-persistenceagent-testdsp2
     component: data-science-pipelines
     dspa: testdsp2
@@ -18,6 +19,7 @@ spec:
       annotations:
         cluster-autoscaler.kubernetes.io/safe-to-evict: "true"
       labels:
+        dsp-version: v2
         app: ds-pipeline-persistenceagent-testdsp2
         component: data-science-pipelines
         dspa: testdsp2

diff --git a/controllers/testdata/declarative/case_2/expected/created/sample-config.yaml.tmpl b/controllers/testdata/declarative/case_2/expected/created/sample-config.yaml.tmpl
@@ -4,6 +4,7 @@ metadata:
     name: sample-config-testdsp2
     namespace: default
     labels:
+        dsp-version: v2
         app: ds-pipeline-testdsp2
         component: data-science-pipelines
 data:

diff --git a/controllers/testdata/declarative/case_2/expected/created/scheduled-workflow_deployment.yaml b/controllers/testdata/declarative/case_2/expected/created/scheduled-workflow_deployment.yaml
@@ -4,6 +4,7 @@ metadata:
   name: ds-pipeline-scheduledworkflow-testdsp2
   namespace: default
   labels:
+    dsp-version: v2
     app: ds-pipeline-scheduledworkflow-testdsp2
     component: data-science-pipelines
     dspa: testdsp2
@@ -18,6 +19,7 @@ spec:
       annotations:
         cluster-autoscaler.kubernetes.io/safe-to-evict: "true"
       labels:
+        dsp-version: v2
         app: ds-pipeline-scheduledworkflow-testdsp2
         component: data-science-pipelines
         dspa: testdsp2

diff --git a/controllers/testdata/declarative/case_3/expected/created/apiserver_deployment.yaml b/controllers/testdata/declarative/case_3/expected/created/apiserver_deployment.yaml
@@ -4,6 +4,7 @@ metadata:
   name: ds-pipeline-testdsp3
   namespace: default
   labels:
+    dsp-version: v2
     app: ds-pipeline-testdsp3
     component: data-science-pipelines
     dspa: testdsp3
@@ -16,6 +17,7 @@ spec:
   template:
     metadata:
       labels:
+        dsp-version: v2
         app: ds-pipeline-testdsp3
         component: data-science-pipelines
         dspa: testdsp3