[RHOAIENG-48289] Don't reconcile labels we don't own

components/common/reconcilehelper/util.go

@@ -106,19 +106,32 @@ func VirtualService(ctx context.Context, r client.Client, virtualServiceName, na
 // Returns true if the fields copied from don't match to.
 func CopyStatefulSetFields(from, to *appsv1.StatefulSet) bool {
 	requireUpdate := false
-	for k, v := range to.Labels {
-		if from.Labels[k] != v {
+	// Reconcile only the labels provided by `from` (controller-owned).
+	// Preserve any extra labels already present on `to`.
+	for k, v := range from.Labels {
+		if to.Labels == nil || to.Labels[k] != v {
 			requireUpdate = true
 		}
 	}
-	to.Labels = from.Labels
+	if to.Labels == nil {
+		to.Labels = map[string]string{}
+	}
+	for k, v := range from.Labels {
+		to.Labels[k] = v
+	}
 
-	for k, v := range to.Annotations {
-		if from.Annotations[k] != v {
+	// Same approach for annotations: reconcile only controller-owned keys.
+	for k, v := range from.Annotations {
+		if to.Annotations == nil || to.Annotations[k] != v {
 			requireUpdate = true
 		}
 	}
-	to.Annotations = from.Annotations
+	if to.Annotations == nil {
+		to.Annotations = map[string]string{}
+	}
+	for k, v := range from.Annotations {
+		to.Annotations[k] = v
+	}
 
 	if *from.Spec.Replicas != *to.Spec.Replicas {
 		*to.Spec.Replicas = *from.Spec.Replicas
@@ -135,19 +148,29 @@ func CopyStatefulSetFields(from, to *appsv1.StatefulSet) bool {
 
 func CopyDeploymentSetFields(from, to *appsv1.Deployment) bool {
 	requireUpdate := false
-	for k, v := range to.Labels {
-		if from.Labels[k] != v {
+	for k, v := range from.Labels {
+		if to.Labels == nil || to.Labels[k] != v {
 			requireUpdate = true
 		}
 	}
-	to.Labels = from.Labels
+	if to.Labels == nil {
+		to.Labels = map[string]string{}
+	}
+	for k, v := range from.Labels {
+		to.Labels[k] = v
+	}
 
-	for k, v := range to.Annotations {
-		if from.Annotations[k] != v {
+	for k, v := range from.Annotations {
+		if to.Annotations == nil || to.Annotations[k] != v {
 			requireUpdate = true
 		}
 	}
-	to.Annotations = from.Annotations
+	if to.Annotations == nil {
+		to.Annotations = map[string]string{}
+	}
+	for k, v := range from.Annotations {
+		to.Annotations[k] = v
+	}
 
 	if from.Spec.Replicas != to.Spec.Replicas {
 		to.Spec.Replicas = from.Spec.Replicas
@@ -165,19 +188,29 @@ func CopyDeploymentSetFields(from, to *appsv1.Deployment) bool {
 // CopyServiceFields copies the owned fields from one Service to another
 func CopyServiceFields(from, to *corev1.Service) bool {
 	requireUpdate := false
-	for k, v := range to.Labels {
-		if from.Labels[k] != v {
+	for k, v := range from.Labels {
+		if to.Labels == nil || to.Labels[k] != v {
 			requireUpdate = true
 		}
 	}
-	to.Labels = from.Labels
+	if to.Labels == nil {
+		to.Labels = map[string]string{}
+	}
+	for k, v := range from.Labels {
+		to.Labels[k] = v
+	}
 
-	for k, v := range to.Annotations {
-		if from.Annotations[k] != v {
+	for k, v := range from.Annotations {
+		if to.Annotations == nil || to.Annotations[k] != v {
 			requireUpdate = true
 		}
 	}
-	to.Annotations = from.Annotations
+	if to.Annotations == nil {
+		to.Annotations = map[string]string{}
+	}
+	for k, v := range from.Annotations {
+		to.Annotations[k] = v
+	}
 
 	// Don't copy the entire Spec, because we can't overwrite the clusterIp field
 

components/notebook-controller/controllers/notebook_controller.go

@@ -188,8 +188,22 @@ func (r *NotebookReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 	// Copy the pod template labels, but reconcilation is not required
 	// exclusively based on ths pod template labels
 	if ss.Spec.Replicas != nil && foundStateful.Spec.Replicas != nil && *ss.Spec.Replicas != *foundStateful.Spec.Replicas {
-		if !reflect.DeepEqual(foundStateful.Spec.Template.ObjectMeta.Labels, ss.Spec.Template.ObjectMeta.Labels) {
-			foundStateful.Spec.Template.ObjectMeta.Labels = ss.Spec.Template.ObjectMeta.Labels
+		// Reconcile only controller-owned labels (present on `ss`), preserve any
+		// extra labels on the existing StatefulSet pod template.
+		needsLabelUpdate := false
+		for k, v := range ss.Spec.Template.Labels {
+			if foundStateful.Spec.Template.Labels == nil || foundStateful.Spec.Template.Labels[k] != v {
+				needsLabelUpdate = true
+				break
+			}
+		}
+		if needsLabelUpdate {
+			if foundStateful.Spec.Template.Labels == nil {
+				foundStateful.Spec.Template.Labels = map[string]string{}
+			}
+			for k, v := range ss.Spec.Template.Labels {
+				foundStateful.Spec.Template.Labels[k] = v
+			}
 		}
 	}
 

components/odh-notebook-controller/controllers/notebook_controller.go

@@ -112,8 +112,10 @@ type OpenshiftNotebookReconciler struct {
 
 // CompareNotebooks checks if two notebooks are equal, if not return false.
 func CompareNotebooks(nb1 nbv1.Notebook, nb2 nbv1.Notebook) bool {
-	return reflect.DeepEqual(nb1.Labels, nb2.Labels) &&
-		reflect.DeepEqual(nb1.Annotations, nb2.Annotations) &&
+	// Do not treat all labels as controller-managed. Labels on Notebooks are often
+	// used by external automation (e.g., sharding / policy / ops tooling) and
+	// should not cause perpetual reconciliation.
+	return reflect.DeepEqual(nb1.Annotations, nb2.Annotations) &&
 		reflect.DeepEqual(nb1.Spec, nb2.Spec)
 }
 

components/odh-notebook-controller/controllers/notebook_controller_test.go

@@ -149,6 +149,104 @@ var _ = Describe("The Openshift Notebook controller", func() {
 			Expect(*httpRoute).To(BeMatchingK8sResource(expectedHTTPRoute, CompareNotebookHTTPRoutes))
 		})
 
+		It("Should preserve extra labels (e.g., sharding) when reconciling HTTPRoute", func() {
+			const shardLabelKey = "route-shard"
+			const shardLabelValue = "blue"
+
+			By("By creating a new Notebook dedicated to this test")
+			shardedNotebookName := "test-notebook-shard-label"
+			shardedNotebook := createNotebook(shardedNotebookName, Namespace)
+			Expect(cli.Create(ctx, shardedNotebook)).Should(Succeed())
+
+			shardedHTTPRoute := &gatewayv1.HTTPRoute{}
+
+			By("By checking that the controller has created the HTTPRoute")
+			Eventually(func() error {
+				key := types.NamespacedName{Name: "nb-" + Namespace + "-" + shardedNotebookName, Namespace: odhNotebookControllerTestNamespace}
+				return cli.Get(ctx, key, shardedHTTPRoute)
+			}, duration, interval).Should(Succeed())
+
+			shardedHTTPRouteName := "nb-" + Namespace + "-" + shardedNotebookName
+			expectedShardedHTTPRoute := gatewayv1.HTTPRoute{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      shardedHTTPRouteName,
+					Namespace: odhNotebookControllerTestNamespace, // Central namespace
+					Labels: map[string]string{
+						"notebook-name":      shardedNotebookName,
+						"notebook-namespace": Namespace,
+					},
+				},
+				Spec: gatewayv1.HTTPRouteSpec{
+					CommonRouteSpec: gatewayv1.CommonRouteSpec{
+						ParentRefs: []gatewayv1.ParentReference{
+							{
+								Group:     func() *gatewayv1.Group { g := gatewayv1.Group("gateway.networking.k8s.io"); return &g }(),
+								Kind:      func() *gatewayv1.Kind { k := gatewayv1.Kind("Gateway"); return &k }(),
+								Name:      gatewayv1.ObjectName("data-science-gateway"),
+								Namespace: func() *gatewayv1.Namespace { ns := gatewayv1.Namespace("openshift-ingress"); return &ns }(),
+							},
+						},
+					},
+					Rules: []gatewayv1.HTTPRouteRule{
+						{
+							Matches: []gatewayv1.HTTPRouteMatch{
+								{
+									Path: &gatewayv1.HTTPPathMatch{
+										Type:  &pathPrefix,
+										Value: (*string)(&[]string{"/notebook/" + Namespace + "/" + shardedNotebookName}[0]),
+									},
+								},
+							},
+							BackendRefs: []gatewayv1.HTTPBackendRef{
+								{
+									BackendRef: gatewayv1.BackendRef{
+										BackendObjectReference: gatewayv1.BackendObjectReference{
+											Group:     func() *gatewayv1.Group { g := gatewayv1.Group(""); return &g }(),
+											Kind:      func() *gatewayv1.Kind { k := gatewayv1.Kind("Service"); return &k }(),
+											Name:      gatewayv1.ObjectName(shardedNotebookName),
+											Namespace: func() *gatewayv1.Namespace { ns := gatewayv1.Namespace(Namespace); return &ns }(),
+											Port:      (*gatewayv1.PortNumber)(&[]gatewayv1.PortNumber{8888}[0]),
+										},
+										Weight: func() *int32 { w := int32(1); return &w }(),
+									},
+								},
+							},
+						},
+					},
+				},
+			}
+
+			By("By simulating a manual HTTPRoute spec modification and adding a shard label")
+			patch := client.RawPatch(types.MergePatchType, []byte(`{"metadata":{"labels":{"`+shardLabelKey+`":"`+shardLabelValue+`"}},"spec":{"rules":[{"backendRefs":[{"name":"foo","port":8888}]}]}}`))
+			Expect(cli.Patch(ctx, shardedHTTPRoute, patch)).Should(Succeed())
+
+			By("By checking that the controller restored the spec while preserving the shard label")
+			Eventually(func() error {
+				key := types.NamespacedName{Name: "nb-" + Namespace + "-" + shardedNotebookName, Namespace: odhNotebookControllerTestNamespace}
+				err := cli.Get(ctx, key, shardedHTTPRoute)
+				if err != nil {
+					return err
+				}
+				backendName := ""
+				if len(shardedHTTPRoute.Spec.Rules) > 0 && len(shardedHTTPRoute.Spec.Rules[0].BackendRefs) > 0 {
+					backendName = string(shardedHTTPRoute.Spec.Rules[0].BackendRefs[0].BackendRef.BackendObjectReference.Name)
+				}
+				if backendName != shardedNotebookName {
+					return fmt.Errorf("expected backend name %q, got %q", shardedNotebookName, backendName)
+				}
+				if shardedHTTPRoute.Labels[shardLabelKey] != shardLabelValue {
+					return fmt.Errorf("expected %s=%q, got %q", shardLabelKey, shardLabelValue, shardedHTTPRoute.Labels[shardLabelKey])
+				}
+				return nil
+			}, duration, interval).Should(Succeed())
+
+			Expect(shardedHTTPRoute.GetLabels()).To(HaveKeyWithValue(shardLabelKey, shardLabelValue))
+			Expect(*shardedHTTPRoute).To(BeMatchingK8sResource(expectedShardedHTTPRoute, CompareNotebookHTTPRoutes))
+
+			By("By deleting the Notebook created for this test")
+			Expect(cli.Delete(ctx, shardedNotebook)).Should(Succeed())
+		})
+
 		It("Should recreate the HTTPRoute when deleted", func() {
 			By("By deleting the notebook HTTPRoute")
 			Expect(cli.Delete(ctx, httpRoute)).Should(Succeed())
@@ -258,7 +356,7 @@ var _ = Describe("The Openshift Notebook controller", func() {
 				return cli.Update(ctx, referenceGrant)
 			}, duration, interval).Should(Succeed())
 
-			By("By checking that the controller has restored the ReferenceGrant labels")
+			By("By checking that the controller has restored the managed ReferenceGrant labels (without removing extra labels)")
 			Eventually(func() map[string]string {
 				if err := cli.Get(ctx, referenceGrantKey, referenceGrant); err != nil {
 					return nil
@@ -267,6 +365,7 @@ var _ = Describe("The Openshift Notebook controller", func() {
 			}, duration, interval).Should(And(
 				HaveKeyWithValue("app.kubernetes.io/managed-by", "odh-notebook-controller"),
 				HaveKeyWithValue("opendatahub.io/component", "notebook-controller"),
+				HaveKeyWithValue("wrong-label", "wrong-value"),
 			))
 		})
 

components/odh-notebook-controller/controllers/notebook_dspa_secret.go

@@ -394,8 +394,12 @@ func SyncElyraRuntimeConfigSecret(ctx context.Context, cli client.Client, config
 
 	if requiresUpdate {
 		log.Info("Updating existing Elyra runtime config secret", "name", elyraRuntimeSecretName)
-		// Set correct label and data
-		existingSecret.Labels = desiredSecret.Labels
+		// Set controller-managed label and data. Preserve any other labels that
+		// may have been added by external tooling.
+		if existingSecret.Labels == nil {
+			existingSecret.Labels = map[string]string{}
+		}
+		existingSecret.Labels[managedByKey] = managedByValue
 		existingSecret.Data = desiredSecret.Data
 
 		if err := cli.Update(ctx, existingSecret); err != nil {

components/odh-notebook-controller/controllers/notebook_kube_rbac_auth.go

@@ -251,16 +251,12 @@ func (r *OpenshiftNotebookReconciler) ReconcileKubeRbacProxyConfigMap(notebook *
 			}
 		}
 
-		// Check if labels differ
+		// Check if controller-managed label(s) differ. Preserve any other labels.
 		if !needsUpdate {
-			if len(foundConfigMap.Labels) != len(desiredConfigMap.Labels) {
-				needsUpdate = true
-			} else {
-				for key, value := range desiredConfigMap.Labels {
-					if foundConfigMap.Labels[key] != value {
-						needsUpdate = true
-						break
-					}
+			for key, value := range desiredConfigMap.Labels {
+				if foundConfigMap.Labels == nil || foundConfigMap.Labels[key] != value {
+					needsUpdate = true
+					break
 				}
 			}
 		}
@@ -269,7 +265,13 @@ func (r *OpenshiftNotebookReconciler) ReconcileKubeRbacProxyConfigMap(notebook *
 			log.Info("Reconciling kube-rbac-proxy ConfigMap")
 			// Update the existing ConfigMap with desired values
 			foundConfigMap.Data = desiredConfigMap.Data
-			foundConfigMap.Labels = desiredConfigMap.Labels
+			if foundConfigMap.Labels == nil {
+				foundConfigMap.Labels = map[string]string{}
+			}
+			// Only reconcile controller-managed labels; preserve any others.
+			for key, value := range desiredConfigMap.Labels {
+				foundConfigMap.Labels[key] = value
+			}
 			err = r.Update(ctx, foundConfigMap)
 			if err != nil {
 				log.Error(err, "Unable to reconcile the kube-rbac-proxy ConfigMap")

components/odh-notebook-controller/controllers/notebook_network.go

@@ -107,9 +107,9 @@ func (r *OpenshiftNotebookReconciler) reconcileNetworkPolicy(desiredNetworkPolic
 			}, foundNetworkPolicy); err != nil {
 				return err
 			}
-			// Reconcile labels and spec field
+			// Reconcile spec only. Preserve any existing labels that may be used by
+			// other automation (e.g., network sharding / policy tooling).
 			foundNetworkPolicy.Spec = desiredNetworkPolicy.Spec
-			foundNetworkPolicy.Labels = desiredNetworkPolicy.Labels
 			return r.Update(ctx, foundNetworkPolicy)
 		})
 		if err != nil {
@@ -123,9 +123,9 @@ func (r *OpenshiftNotebookReconciler) reconcileNetworkPolicy(desiredNetworkPolic
 
 // CompareNotebookNetworkPolicies checks if two services are equal, if not return false
 func CompareNotebookNetworkPolicies(np1 netv1.NetworkPolicy, np2 netv1.NetworkPolicy) bool {
-	// Two network policies will be equal if the labels and specs are identical
-	return reflect.DeepEqual(np1.Labels, np2.Labels) &&
-		reflect.DeepEqual(np1.Spec, np2.Spec)
+	// Only compare spec. Labels are not treated as fully controller-managed to
+	// avoid clobbering external labels used for sharding/policy/ops tooling.
+	return reflect.DeepEqual(np1.Spec, np2.Spec)
 }
 
 // NewNotebookNetworkPolicy defines the desired network policy for Notebook port

components/odh-notebook-controller/controllers/notebook_referencegrant.go

@@ -70,9 +70,21 @@ func NewNotebookReferenceGrant(namespace string, centralNamespace string) *gatew
 
 // CompareNotebookReferenceGrants checks if two ReferenceGrants are equal, if not return false
 func CompareNotebookReferenceGrants(rg1 gatewayv1beta1.ReferenceGrant, rg2 gatewayv1beta1.ReferenceGrant) bool {
-	// Two ReferenceGrants will be equal if the labels and specs are identical
-	return reflect.DeepEqual(rg1.Labels, rg2.Labels) &&
-		reflect.DeepEqual(rg1.Spec, rg2.Spec)
+	// Only compare controller-managed labels. Extra labels (e.g., added for
+	// sharding / policy / ops tooling) should not cause perpetual reconciliation.
+	managedLabelKeys := []string{
+		"app.kubernetes.io/managed-by",
+		"opendatahub.io/component",
+	}
+	for _, k := range managedLabelKeys {
+		v1, ok1 := rg1.Labels[k]
+		v2, ok2 := rg2.Labels[k]
+		if ok1 != ok2 || v1 != v2 {
+			return false
+		}
+	}
+
+	return reflect.DeepEqual(rg1.Spec, rg2.Spec)
 }
 
 // ReconcileReferenceGrant ensures a ReferenceGrant exists in the Notebook's namespace
@@ -112,7 +124,19 @@ func (r *OpenshiftNotebookReconciler) ReconcileReferenceGrant(notebook *nbv1.Not
 		if !CompareNotebookReferenceGrants(*desiredRefGrant, *foundRefGrant) {
 			log.Info("Updating ReferenceGrant to match desired spec and labels")
 			foundRefGrant.Spec = desiredRefGrant.Spec
-			foundRefGrant.Labels = desiredRefGrant.Labels
+
+			if foundRefGrant.Labels == nil {
+				foundRefGrant.Labels = map[string]string{}
+			}
+			// Only reconcile controller-managed labels; preserve any others.
+			for _, k := range []string{"app.kubernetes.io/managed-by", "opendatahub.io/component"} {
+				if desiredRefGrant.Labels != nil {
+					if v, ok := desiredRefGrant.Labels[k]; ok {
+						foundRefGrant.Labels[k] = v
+					}
+				}
+			}
+
 			err = r.Update(ctx, foundRefGrant)
 			if err != nil {
 				log.Error(err, "Unable to update ReferenceGrant")