Skip to content

Commit 4ef7a80

Browse files
alkakumari016alkakumari016
authored
fix: reconcile operator owned labels for configMap and ignore non operator labels (#1737)
* GITOPS-6285: reconcile operator owned labels for configMap and ignore non operator labels Signed-off-by: Alka Kumari <alkumari@redhat.com> * GITOPS-6285: added reconcilialtion logic for labels Signed-off-by: Alka Kumari <alkumari@redhat.com> * Removed a typo that resulted in test case failures Signed-off-by: Alka Kumari <alkumari@redhat.com> * GITOPS-6285: error handling for r.Client.Update Signed-off-by: Alka Kumari <alkumari@redhat.com> * GITOPS-6285: use r.Update Signed-off-by: Alka Kumari <alkumari@redhat.com> * GITOPS-6285: changed func name to UpdateMapValues and added test cases Signed-off-by: Alka Kumari <alkumari@redhat.com> * GITOPS-6285: removed addKubernetesData and replaced the references with UpdateMapValues Signed-off-by: Alka Kumari <alkumari@redhat.com> GITOPS-6285: removed addKubernetesData and replaced the references with UpdateMapValues * GITOPS-6285: removed labels update logic from applicationset and statefulset Signed-off-by: Alka Kumari <alkumari@redhat.com> * GITOPS-6285: changed comments Signed-off-by: Alka Kumari <alkumari@redhat.com> --------- Signed-off-by: Alka Kumari <alkumari@redhat.com>
1 parent 03caafb commit 4ef7a80

File tree

8 files changed

+89
-72
lines changed

8 files changed

+89
-72
lines changed

‎controllers/argocd/applicationset.go‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -277,18 +277,17 @@ func (r *ReconcileArgoCD) reconcileApplicationSetDeployment(cr *argoproj.ArgoCD,
277277
AddSeccompProfileForOpenShift(r.Client, podSpec)
278278

279279
if deplExists {
280-
// Add Kubernetes-specific labels/annotations from the live object in the source to preserve metadata.
281-
addKubernetesData(deploy.Spec.Template.Labels, existing.Spec.Template.Labels)
282-
addKubernetesData(deploy.Spec.Template.Annotations, existing.Spec.Template.Annotations)
280+
//Check if annotations have changed
281+
UpdateMapValues(&existing.Spec.Template.Annotations, deploy.Spec.Template.Annotations)
283282

284283
// If the Deployment already exists, make sure the values we care about are up-to-date
285284
deploymentsDifferent := identifyDeploymentDifference(*existing, *deploy)
286285
if len(deploymentsDifferent) > 0 {
287286
existing.Spec.Template.Spec.Containers = podSpec.Containers
288287
existing.Spec.Template.Spec.Volumes = podSpec.Volumes
289288
existing.Spec.Template.Spec.ServiceAccountName = podSpec.ServiceAccountName
290-
existing.Labels = deploy.Labels
291-
existing.Spec.Template.Labels = deploy.Spec.Template.Labels
289+
UpdateMapValues(&existing.Labels, deploy.Labels)
290+
UpdateMapValues(&existing.Spec.Template.Labels, deploy.Spec.Template.Labels)
292291
existing.Spec.Selector = deploy.Spec.Selector
293292
existing.Spec.Template.Spec.NodeSelector = deploy.Spec.Template.Spec.NodeSelector
294293
existing.Spec.Template.Spec.Tolerations = deploy.Spec.Template.Spec.Tolerations

‎controllers/argocd/configmap.go‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,25 @@ func (r *ReconcileArgoCD) reconcileConfigMaps(cr *argoproj.ArgoCD, useTLSForRedi
334334
// This ConfigMap holds the CA Certificate data for client use.
335335
func (r *ReconcileArgoCD) reconcileCAConfigMap(cr *argoproj.ArgoCD) error {
336336
cm := newConfigMapWithName(getCAConfigMapName(cr), cr)
337+
existingCM := &corev1.ConfigMap{}
338+
exists, err := argoutil.IsObjectFound(r.Client, cr.Namespace, cm.Name, existingCM)
339+
if err != nil {
340+
return err
341+
}
342+
if exists {
343+
changed := false
344+
//Check if labels have changed
345+
if UpdateMapValues(&existingCM.Labels, cm.GetLabels()) {
346+
argoutil.LogResourceUpdate(log, existingCM, "updating", "CAConfigMap labels")
347+
changed = true
348+
if changed {
349+
if err := r.Update(context.TODO(), existingCM); err != nil {
350+
log.Error(err, "failed to update service object")
351+
}
352+
}
353+
}
354+
355+
}
337356

338357
configMapExists, err := argoutil.IsObjectFound(r.Client, cr.Namespace, cm.Name, cm)
339358
if err != nil {
@@ -499,6 +518,12 @@ func (r *ReconcileArgoCD) reconcileArgoConfigMap(cr *argoproj.ArgoCD) error {
499518
}
500519

501520
changed := false
521+
//Check if labels have changed
522+
if UpdateMapValues(&existingCM.Labels, cm.GetLabels()) {
523+
argoutil.LogResourceUpdate(log, existingCM, "updating", "ConfigMap labels")
524+
changed = true
525+
}
526+
502527
if !reflect.DeepEqual(cm.Data, existingCM.Data) {
503528
existingCM.Data = cm.Data
504529
changed = true

‎controllers/argocd/deployment.go‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1371,9 +1371,9 @@ func (r *ReconcileArgoCD) reconcileRepoDeployment(cr *argoproj.ArgoCD, useTLSFor
13711371
changed = true
13721372
}
13731373

1374-
// Add Kubernetes-specific labels/annotations from the live object in the source to preserve metadata.
1375-
addKubernetesData(deploy.Spec.Template.Labels, existing.Spec.Template.Labels)
1376-
addKubernetesData(deploy.Spec.Template.Annotations, existing.Spec.Template.Annotations)
1374+
//Check if labels/annotations have changed
1375+
UpdateMapValues(&existing.Spec.Template.Labels, deploy.Spec.Template.Labels)
1376+
UpdateMapValues(&existing.Spec.Template.Annotations, deploy.Spec.Template.Annotations)
13771377

13781378
if !reflect.DeepEqual(deploy.Spec.Template.Annotations, existing.Spec.Template.Annotations) {
13791379
existing.Spec.Template.Annotations = deploy.Spec.Template.Annotations
@@ -1383,9 +1383,8 @@ func (r *ReconcileArgoCD) reconcileRepoDeployment(cr *argoproj.ArgoCD, useTLSFor
13831383
explanation += "annotations"
13841384
changed = true
13851385
}
1386-
1387-
if !reflect.DeepEqual(deploy.Spec.Template.Labels, existing.Spec.Template.Labels) {
1388-
existing.Spec.Template.Labels = deploy.Spec.Template.Labels
1386+
// Preserve non-operator labels in the existing deployment.
1387+
if UpdateMapValues(&existing.Spec.Template.Labels, deploy.Spec.Template.Labels) {
13891388
if changed {
13901389
explanation += ", "
13911390
}
@@ -1760,9 +1759,9 @@ func (r *ReconcileArgoCD) reconcileServerDeployment(cr *argoproj.ArgoCD, useTLSF
17601759
}
17611760
}
17621761

1763-
// Add Kubernetes-specific labels/annotations from the live object in the source to preserve metadata.
1764-
addKubernetesData(deploy.Spec.Template.Labels, existing.Spec.Template.Labels)
1765-
addKubernetesData(deploy.Spec.Template.Annotations, existing.Spec.Template.Annotations)
1762+
//Check if labels/annotations have changed
1763+
UpdateMapValues(&existing.Spec.Template.Labels, deploy.Spec.Template.Labels)
1764+
UpdateMapValues(&existing.Spec.Template.Annotations, deploy.Spec.Template.Annotations)
17661765

17671766
if !reflect.DeepEqual(deploy.Spec.Template.Annotations, existing.Spec.Template.Annotations) {
17681767
existing.Spec.Template.Annotations = deploy.Spec.Template.Annotations
@@ -1772,8 +1771,8 @@ func (r *ReconcileArgoCD) reconcileServerDeployment(cr *argoproj.ArgoCD, useTLSF
17721771
explanation += "annotations"
17731772
changed = true
17741773
}
1775-
if !reflect.DeepEqual(deploy.Spec.Template.Labels, existing.Spec.Template.Labels) {
1776-
existing.Spec.Template.Labels = deploy.Spec.Template.Labels
1774+
// Preserve non-operator labels in the existing deployment.
1775+
if UpdateMapValues(&existing.Spec.Template.Labels, deploy.Spec.Template.Labels) {
17771776
if changed {
17781777
explanation += ", "
17791778
}

‎controllers/argocd/notifications.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -527,17 +527,15 @@ func (r *ReconcileArgoCD) reconcileNotificationsDeployment(cr *argoproj.ArgoCD,
527527
deploymentChanged = true
528528
}
529529

530-
if !reflect.DeepEqual(existingDeployment.Labels, desiredDeployment.Labels) {
531-
existingDeployment.Labels = desiredDeployment.Labels
530+
if UpdateMapValues(&existingDeployment.Labels, desiredDeployment.Labels) {
532531
if deploymentChanged {
533532
explanation = ", "
534533
}
535534
explanation += "labels"
536535
deploymentChanged = true
537536
}
538537

539-
if !reflect.DeepEqual(existingDeployment.Spec.Template.Labels, desiredDeployment.Spec.Template.Labels) {
540-
existingDeployment.Spec.Template.Labels = desiredDeployment.Spec.Template.Labels
538+
if UpdateMapValues(&existingDeployment.Spec.Template.Labels, desiredDeployment.Spec.Template.Labels) {
541539
if deploymentChanged {
542540
explanation = ", "
543541
}

‎controllers/argocd/role.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -518,8 +518,7 @@ func matchAggregatedClusterRoleFields(expectedClusterRole *v1.ClusterRole, exist
518518

519519
// if ClusterRole is for View permissions then compare Labels
520520
if name == common.ArgoCDApplicationControllerComponentView {
521-
if !reflect.DeepEqual(existingClusterRole.Labels, expectedClusterRole.Labels) {
522-
existingClusterRole.Labels = expectedClusterRole.Labels
521+
if UpdateMapValues(&existingClusterRole.Labels, expectedClusterRole.Labels) {
523522
if changed {
524523
explanation += ", "
525524
}
@@ -539,8 +538,7 @@ func matchAggregatedClusterRoleFields(expectedClusterRole *v1.ClusterRole, exist
539538
changed = true
540539
}
541540

542-
if !reflect.DeepEqual(existingClusterRole.Labels, expectedClusterRole.Labels) {
543-
existingClusterRole.Labels = expectedClusterRole.Labels
541+
if UpdateMapValues(&existingClusterRole.Labels, expectedClusterRole.Labels) {
544542
if changed {
545543
explanation += ", "
546544
}

‎controllers/argocd/statefulset.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1023,9 +1023,9 @@ func (r *ReconcileArgoCD) reconcileApplicationControllerStatefulSet(cr *argoproj
10231023
changed = true
10241024
}
10251025

1026-
// Add Kubernetes-specific labels/annotations from the live object in the source to preserve metadata.
1027-
addKubernetesData(ss.Spec.Template.Labels, existing.Spec.Template.Labels)
1028-
addKubernetesData(ss.Spec.Template.Annotations, existing.Spec.Template.Annotations)
1026+
//Check if labels/annotations have changed
1027+
UpdateMapValues(&existing.Spec.Template.Labels, ss.Spec.Template.Labels)
1028+
UpdateMapValues(&existing.Spec.Template.Annotations, ss.Spec.Template.Annotations)
10291029

10301030
if !reflect.DeepEqual(ss.Spec.Template.Annotations, existing.Spec.Template.Annotations) {
10311031
existing.Spec.Template.Annotations = ss.Spec.Template.Annotations

‎controllers/argocd/util.go‎

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1763,29 +1763,20 @@ func UseServer(name string, cr *argoproj.ArgoCD) bool {
17631763
return true
17641764
}
17651765

1766-
// addKubernetesData checks for any Kubernetes-specific labels or annotations
1767-
// in the live object and updates the source object to ensure critical metadata
1768-
// (like scheduling, topology, or lifecycle information) is retained.
1769-
// This helps avoid loss of important Kubernetes-managed metadata during updates.
1770-
func addKubernetesData(source map[string]string, live map[string]string) {
1771-
1772-
// List of Kubernetes-specific substrings (wildcard match)
1773-
patterns := []string{
1774-
"*kubernetes.io*",
1775-
"*k8s.io*",
1776-
"*openshift.io*",
1777-
}
1778-
1779-
for key, value := range live {
1780-
found := glob.MatchStringInList(patterns, key, glob.GLOB)
1781-
if found {
1782-
// Don't override values already present in the source object.
1783-
// This allows the operator to update Kubernetes specific data when needed.
1784-
if _, ok := source[key]; !ok {
1785-
source[key] = value
1786-
}
1766+
// UpdateMapValues updates the values of an existing map with the values from a source map if they differ.
1767+
// It returns true if any values were changed.
1768+
func UpdateMapValues(existing *map[string]string, source map[string]string) bool {
1769+
changed := false
1770+
if *existing == nil {
1771+
*existing = make(map[string]string)
1772+
}
1773+
for key, value := range source {
1774+
if (*existing)[key] != value {
1775+
(*existing)[key] = value
1776+
changed = true
17871777
}
17881778
}
1779+
return changed
17891780
}
17901781

17911782
// updateStatusConditionOfArgoCD calls Set Condition of ArgoCD status

‎controllers/argocd/util_test.go‎

Lines changed: 31 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1170,69 +1170,76 @@ func TestReconcileArgoCD_reconcileDexOAuthClientSecret(t *testing.T) {
11701170
assert.True(t, tokenExists, "Dex is enabled but unable to create oauth client secret")
11711171
}
11721172

1173-
func TestRetainKubernetesData(t *testing.T) {
1173+
func TestUpdateMapValue(t *testing.T) {
11741174
tests := []struct {
11751175
name string
11761176
source map[string]string
1177-
live map[string]string
1177+
existing map[string]string
11781178
expected map[string]string
11791179
}{
11801180
{
1181-
name: "Add Kubernetes-specific keys not in source",
1181+
name: "Retain non-operator-specific keys not in source",
11821182
source: map[string]string{
1183-
"custom-label": "custom-value",
1183+
"node.kubernetes.io/pod": "true",
1184+
"kubectl.kubernetes.io/restartedAt": "2024-12-05T09:46:46+05:30",
1185+
"openshift.openshift.io/restartedAt": "2024-12-05T09:46:46+05:30",
11841186
},
1185-
live: map[string]string{
1187+
existing: map[string]string{
11861188
"node.kubernetes.io/pod": "true",
11871189
"kubectl.kubernetes.io/restartedAt": "2024-12-05T09:46:46+05:30",
11881190
"openshift.openshift.io/restartedAt": "2024-12-05T09:46:46+05:30",
1191+
"custom-label": "custom-value",
11891192
},
11901193
expected: map[string]string{
1191-
"custom-label": "custom-value", // unchanged
1192-
"node.kubernetes.io/pod": "true", // added
1193-
"kubectl.kubernetes.io/restartedAt": "2024-12-05T09:46:46+05:30", // added
1194-
"openshift.openshift.io/restartedAt": "2024-12-05T09:46:46+05:30", // added
1194+
"custom-label": "custom-value", // retained from existing
1195+
"node.kubernetes.io/pod": "true", // unchanged
1196+
"kubectl.kubernetes.io/restartedAt": "2024-12-05T09:46:46+05:30", // unchanged
1197+
"openshift.openshift.io/restartedAt": "2024-12-05T09:46:46+05:30", // unchanged
11951198
},
11961199
},
11971200
{
1198-
name: "Ignores non-Kubernetes-specific keys",
1201+
name: "Override operator-specific keys in live with source",
11991202
source: map[string]string{
1200-
"custom-label": "custom-value",
1203+
"node.kubernetes.io/pod": "source-true",
1204+
"kubectl.kubernetes.io/restartedAt": "2024-12-05T09:46:46+05:30",
12011205
},
1202-
live: map[string]string{
1203-
"non-k8s-key": "non-k8s-value",
1204-
"custom-label": "live-value",
1206+
existing: map[string]string{
1207+
"node.kubernetes.io/pod": "live-true", // should override
1208+
"kubectl.kubernetes.io/restartedAt": "2024-12-05T09:46:46+05:30",
12051209
},
12061210
expected: map[string]string{
1207-
"custom-label": "custom-value", // unchanged
1211+
"node.kubernetes.io/pod": "source-true",
1212+
"kubectl.kubernetes.io/restartedAt": "2024-12-05T09:46:46+05:30",
12081213
},
12091214
},
12101215
{
1211-
name: "Do not override existing Kubernetes-specific keys in source",
1216+
name: "Retain existing operator-specific keys from source",
12121217
source: map[string]string{
1213-
"node.kubernetes.io/pod": "source-true",
1218+
"node.kubernetes.io/pod": "source-true",
1219+
"kubectl.kubernetes.io/restartedAt": "2024-12-05T09:46:46+05:30",
12141220
},
1215-
live: map[string]string{
1216-
"node.kubernetes.io/pod": "live-true", // should not override
1221+
existing: map[string]string{
1222+
"node.kubernetes.io/pod": "source-true",
12171223
},
12181224
expected: map[string]string{
1219-
"node.kubernetes.io/pod": "source-true", // source takes precedence
1225+
"node.kubernetes.io/pod": "source-true",
1226+
"kubectl.kubernetes.io/restartedAt": "2024-12-05T09:46:46+05:30",
12201227
},
12211228
},
12221229
{
12231230
name: "Handles empty live map",
12241231
source: map[string]string{
12251232
"custom-label": "custom-value",
12261233
},
1227-
live: map[string]string{},
1234+
existing: map[string]string{},
12281235
expected: map[string]string{
12291236
"custom-label": "custom-value", // unchanged
12301237
},
12311238
},
12321239
{
12331240
name: "Handles empty source map",
12341241
source: map[string]string{},
1235-
live: map[string]string{
1242+
existing: map[string]string{
12361243
"openshift.io/resource": "value",
12371244
},
12381245
expected: map[string]string{
@@ -1243,8 +1250,8 @@ func TestRetainKubernetesData(t *testing.T) {
12431250

12441251
for _, tt := range tests {
12451252
t.Run(tt.name, func(t *testing.T) {
1246-
addKubernetesData(tt.source, tt.live)
1247-
assert.Equal(t, tt.expected, tt.source)
1253+
UpdateMapValues(&tt.existing, tt.source)
1254+
assert.Equal(t, tt.expected, tt.existing)
12481255
})
12491256
}
12501257
}

0 commit comments

Comments
 (0)