Skip to content

Commit a2925aa

Browse files
alkakumari016alkakumari016
committed
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 <[email protected]> * GITOPS-6285: added reconcilialtion logic for labels Signed-off-by: Alka Kumari <[email protected]> * Removed a typo that resulted in test case failures Signed-off-by: Alka Kumari <[email protected]> * GITOPS-6285: error handling for r.Client.Update Signed-off-by: Alka Kumari <[email protected]> * GITOPS-6285: use r.Update Signed-off-by: Alka Kumari <[email protected]> * GITOPS-6285: changed func name to UpdateMapValues and added test cases Signed-off-by: Alka Kumari <[email protected]> * GITOPS-6285: removed addKubernetesData and replaced the references with UpdateMapValues Signed-off-by: Alka Kumari <[email protected]> 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 <[email protected]> * GITOPS-6285: changed comments Signed-off-by: Alka Kumari <[email protected]> --------- Signed-off-by: Alka Kumari <[email protected]>
1 parent 5135028 commit a2925aa

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
}
@@ -1757,9 +1756,9 @@ func (r *ReconcileArgoCD) reconcileServerDeployment(cr *argoproj.ArgoCD, useTLSF
17571756
}
17581757
}
17591758

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

17641763
if !reflect.DeepEqual(deploy.Spec.Template.Annotations, existing.Spec.Template.Annotations) {
17651764
existing.Spec.Template.Annotations = deploy.Spec.Template.Annotations
@@ -1769,8 +1768,8 @@ func (r *ReconcileArgoCD) reconcileServerDeployment(cr *argoproj.ArgoCD, useTLSF
17691768
explanation += "annotations"
17701769
changed = true
17711770
}
1772-
if !reflect.DeepEqual(deploy.Spec.Template.Labels, existing.Spec.Template.Labels) {
1773-
existing.Spec.Template.Labels = deploy.Spec.Template.Labels
1771+
// Preserve non-operator labels in the existing deployment.
1772+
if UpdateMapValues(&existing.Spec.Template.Labels, deploy.Spec.Template.Labels) {
17741773
if changed {
17751774
explanation += ", "
17761775
}

‎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
@@ -514,8 +514,7 @@ func matchAggregatedClusterRoleFields(expectedClusterRole *v1.ClusterRole, exist
514514

515515
// if ClusterRole is for View permissions then compare Labels
516516
if name == common.ArgoCDApplicationControllerComponentView {
517-
if !reflect.DeepEqual(existingClusterRole.Labels, expectedClusterRole.Labels) {
518-
existingClusterRole.Labels = expectedClusterRole.Labels
517+
if UpdateMapValues(&existingClusterRole.Labels, expectedClusterRole.Labels) {
519518
if changed {
520519
explanation += ", "
521520
}
@@ -535,8 +534,7 @@ func matchAggregatedClusterRoleFields(expectedClusterRole *v1.ClusterRole, exist
535534
changed = true
536535
}
537536

538-
if !reflect.DeepEqual(existingClusterRole.Labels, expectedClusterRole.Labels) {
539-
existingClusterRole.Labels = expectedClusterRole.Labels
537+
if UpdateMapValues(&existingClusterRole.Labels, expectedClusterRole.Labels) {
540538
if changed {
541539
explanation += ", "
542540
}

‎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
@@ -1729,29 +1729,20 @@ func UseServer(name string, cr *argoproj.ArgoCD) bool {
17291729
return true
17301730
}
17311731

1732-
// addKubernetesData checks for any Kubernetes-specific labels or annotations
1733-
// in the live object and updates the source object to ensure critical metadata
1734-
// (like scheduling, topology, or lifecycle information) is retained.
1735-
// This helps avoid loss of important Kubernetes-managed metadata during updates.
1736-
func addKubernetesData(source map[string]string, live map[string]string) {
1737-
1738-
// List of Kubernetes-specific substrings (wildcard match)
1739-
patterns := []string{
1740-
"*kubernetes.io*",
1741-
"*k8s.io*",
1742-
"*openshift.io*",
1743-
}
1744-
1745-
for key, value := range live {
1746-
found := glob.MatchStringInList(patterns, key, glob.GLOB)
1747-
if found {
1748-
// Don't override values already present in the source object.
1749-
// This allows the operator to update Kubernetes specific data when needed.
1750-
if _, ok := source[key]; !ok {
1751-
source[key] = value
1752-
}
1732+
// UpdateMapValues updates the values of an existing map with the values from a source map if they differ.
1733+
// It returns true if any values were changed.
1734+
func UpdateMapValues(existing *map[string]string, source map[string]string) bool {
1735+
changed := false
1736+
if *existing == nil {
1737+
*existing = make(map[string]string)
1738+
}
1739+
for key, value := range source {
1740+
if (*existing)[key] != value {
1741+
(*existing)[key] = value
1742+
changed = true
17531743
}
17541744
}
1745+
return changed
17551746
}
17561747

17571748
// 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
@@ -1169,69 +1169,76 @@ func TestReconcileArgoCD_reconcileDexOAuthClientSecret(t *testing.T) {
11691169
assert.True(t, tokenExists, "Dex is enabled but unable to create oauth client secret")
11701170
}
11711171

1172-
func TestRetainKubernetesData(t *testing.T) {
1172+
func TestUpdateMapValue(t *testing.T) {
11731173
tests := []struct {
11741174
name string
11751175
source map[string]string
1176-
live map[string]string
1176+
existing map[string]string
11771177
expected map[string]string
11781178
}{
11791179
{
1180-
name: "Add Kubernetes-specific keys not in source",
1180+
name: "Retain non-operator-specific keys not in source",
11811181
source: map[string]string{
1182-
"custom-label": "custom-value",
1182+
"node.kubernetes.io/pod": "true",
1183+
"kubectl.kubernetes.io/restartedAt": "2024-12-05T09:46:46+05:30",
1184+
"openshift.openshift.io/restartedAt": "2024-12-05T09:46:46+05:30",
11831185
},
1184-
live: map[string]string{
1186+
existing: map[string]string{
11851187
"node.kubernetes.io/pod": "true",
11861188
"kubectl.kubernetes.io/restartedAt": "2024-12-05T09:46:46+05:30",
11871189
"openshift.openshift.io/restartedAt": "2024-12-05T09:46:46+05:30",
1190+
"custom-label": "custom-value",
11881191
},
11891192
expected: map[string]string{
1190-
"custom-label": "custom-value", // unchanged
1191-
"node.kubernetes.io/pod": "true", // added
1192-
"kubectl.kubernetes.io/restartedAt": "2024-12-05T09:46:46+05:30", // added
1193-
"openshift.openshift.io/restartedAt": "2024-12-05T09:46:46+05:30", // added
1193+
"custom-label": "custom-value", // retained from existing
1194+
"node.kubernetes.io/pod": "true", // unchanged
1195+
"kubectl.kubernetes.io/restartedAt": "2024-12-05T09:46:46+05:30", // unchanged
1196+
"openshift.openshift.io/restartedAt": "2024-12-05T09:46:46+05:30", // unchanged
11941197
},
11951198
},
11961199
{
1197-
name: "Ignores non-Kubernetes-specific keys",
1200+
name: "Override operator-specific keys in live with source",
11981201
source: map[string]string{
1199-
"custom-label": "custom-value",
1202+
"node.kubernetes.io/pod": "source-true",
1203+
"kubectl.kubernetes.io/restartedAt": "2024-12-05T09:46:46+05:30",
12001204
},
1201-
live: map[string]string{
1202-
"non-k8s-key": "non-k8s-value",
1203-
"custom-label": "live-value",
1205+
existing: map[string]string{
1206+
"node.kubernetes.io/pod": "live-true", // should override
1207+
"kubectl.kubernetes.io/restartedAt": "2024-12-05T09:46:46+05:30",
12041208
},
12051209
expected: map[string]string{
1206-
"custom-label": "custom-value", // unchanged
1210+
"node.kubernetes.io/pod": "source-true",
1211+
"kubectl.kubernetes.io/restartedAt": "2024-12-05T09:46:46+05:30",
12071212
},
12081213
},
12091214
{
1210-
name: "Do not override existing Kubernetes-specific keys in source",
1215+
name: "Retain existing operator-specific keys from source",
12111216
source: map[string]string{
1212-
"node.kubernetes.io/pod": "source-true",
1217+
"node.kubernetes.io/pod": "source-true",
1218+
"kubectl.kubernetes.io/restartedAt": "2024-12-05T09:46:46+05:30",
12131219
},
1214-
live: map[string]string{
1215-
"node.kubernetes.io/pod": "live-true", // should not override
1220+
existing: map[string]string{
1221+
"node.kubernetes.io/pod": "source-true",
12161222
},
12171223
expected: map[string]string{
1218-
"node.kubernetes.io/pod": "source-true", // source takes precedence
1224+
"node.kubernetes.io/pod": "source-true",
1225+
"kubectl.kubernetes.io/restartedAt": "2024-12-05T09:46:46+05:30",
12191226
},
12201227
},
12211228
{
12221229
name: "Handles empty live map",
12231230
source: map[string]string{
12241231
"custom-label": "custom-value",
12251232
},
1226-
live: map[string]string{},
1233+
existing: map[string]string{},
12271234
expected: map[string]string{
12281235
"custom-label": "custom-value", // unchanged
12291236
},
12301237
},
12311238
{
12321239
name: "Handles empty source map",
12331240
source: map[string]string{},
1234-
live: map[string]string{
1241+
existing: map[string]string{
12351242
"openshift.io/resource": "value",
12361243
},
12371244
expected: map[string]string{
@@ -1242,8 +1249,8 @@ func TestRetainKubernetesData(t *testing.T) {
12421249

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

0 commit comments

Comments
 (0)