Skip to content

Commit 66a5032

Browse files
svghadisvghadi
authored
[release-0.13] fix: update logic to preserve k8s specific labels & annotations (#1616) (#1617)
* fix: update logic to preserve k8s specific labels & annotations (#1616) * Fix update logic to preserve k8s specific labels & annotations Operator overwrites k8s added labels or annotations. This sometimes results into unexpected behaviour. Eg. A kubectl rollout restart on deployment adds an annotation "kubectl.kubernetes.io/restartedAt" with timestamp. However operator detects this as a config drift and removes the annotations resulting into k8s terminating the rollout due to change in config. This commit fixes the issue by preserving such annotations & labels added onto live object during comparison. Signed-off-by: Siddhesh Ghadi <[email protected]> * Fix lint Signed-off-by: Siddhesh Ghadi <[email protected]> --------- Signed-off-by: Siddhesh Ghadi <[email protected]>
1 parent 8892f04 commit 66a5032

File tree

5 files changed

+133
-16
lines changed

5 files changed

+133
-16
lines changed

‎controllers/argocd/applicationset.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,9 @@ func (r *ReconcileArgoCD) reconcileApplicationSetDeployment(cr *argoproj.ArgoCD,
252252
}
253253

254254
if cr.Spec.ApplicationSet.Annotations != nil {
255-
deploy.Spec.Template.Annotations = cr.Spec.ApplicationSet.Annotations
255+
for key, value := range cr.Spec.ApplicationSet.Annotations {
256+
deploy.Spec.Template.Annotations[key] = value
257+
}
256258
}
257259

258260
if cr.Spec.ApplicationSet.Labels != nil {
@@ -270,6 +272,10 @@ func (r *ReconcileArgoCD) reconcileApplicationSetDeployment(cr *argoproj.ArgoCD,
270272

271273
existingSpec := existing.Spec.Template.Spec
272274

275+
// Add Kubernetes-specific labels/annotations from the live object in the source to preserve metadata.
276+
addKubernetesData(deploy.Spec.Template.Labels, existing.Spec.Template.Labels)
277+
addKubernetesData(deploy.Spec.Template.Annotations, existing.Spec.Template.Annotations)
278+
273279
deploymentsDifferent := !reflect.DeepEqual(existingSpec.Containers[0], podSpec.Containers) ||
274280
!reflect.DeepEqual(existingSpec.Volumes, podSpec.Volumes) ||
275281
existingSpec.ServiceAccountName != podSpec.ServiceAccountName ||

‎controllers/argocd/deployment.go‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,7 @@ func newDeploymentWithName(name string, component string, cr *argoproj.ArgoCD) *
395395
Labels: map[string]string{
396396
common.ArgoCDKeyName: name,
397397
},
398+
Annotations: make(map[string]string),
398399
},
399400
Spec: corev1.PodSpec{
400401
NodeSelector: common.DefaultNodeSelector(),
@@ -1110,7 +1111,9 @@ func (r *ReconcileArgoCD) reconcileRepoDeployment(cr *argoproj.ArgoCD, useTLSFor
11101111
}
11111112

11121113
if cr.Spec.Repo.Annotations != nil {
1113-
deploy.Spec.Template.Annotations = cr.Spec.Repo.Annotations
1114+
for key, value := range cr.Spec.Repo.Annotations {
1115+
deploy.Spec.Template.Annotations[key] = value
1116+
}
11141117
}
11151118

11161119
if cr.Spec.Repo.Labels != nil {
@@ -1197,15 +1200,15 @@ func (r *ReconcileArgoCD) reconcileRepoDeployment(cr *argoproj.ArgoCD, useTLSFor
11971200
changed = true
11981201
}
11991202

1200-
deploy.Spec.Template.Annotations = cr.Spec.Repo.Annotations
1203+
// Add Kubernetes-specific labels/annotations from the live object in the source to preserve metadata.
1204+
addKubernetesData(deploy.Spec.Template.Labels, existing.Spec.Template.Labels)
1205+
addKubernetesData(deploy.Spec.Template.Annotations, existing.Spec.Template.Annotations)
1206+
12011207
if !reflect.DeepEqual(deploy.Spec.Template.Annotations, existing.Spec.Template.Annotations) {
12021208
existing.Spec.Template.Annotations = deploy.Spec.Template.Annotations
12031209
changed = true
12041210
}
12051211

1206-
for key, value := range cr.Spec.Repo.Labels {
1207-
deploy.Spec.Template.Labels[key] = value
1208-
}
12091212
if !reflect.DeepEqual(deploy.Spec.Template.Labels, existing.Spec.Template.Labels) {
12101213
existing.Spec.Template.Labels = deploy.Spec.Template.Labels
12111214
changed = true
@@ -1405,7 +1408,9 @@ func (r *ReconcileArgoCD) reconcileServerDeployment(cr *argoproj.ArgoCD, useTLSF
14051408
}
14061409

14071410
if cr.Spec.Server.Annotations != nil {
1408-
deploy.Spec.Template.Annotations = cr.Spec.Server.Annotations
1411+
for key, value := range cr.Spec.Server.Annotations {
1412+
deploy.Spec.Template.Annotations[key] = value
1413+
}
14091414
}
14101415

14111416
if cr.Spec.Server.Labels != nil {
@@ -1479,11 +1484,10 @@ func (r *ReconcileArgoCD) reconcileServerDeployment(cr *argoproj.ArgoCD, useTLSF
14791484
}
14801485
}
14811486

1482-
deploy.Spec.Template.Annotations = cr.Spec.Server.Annotations
1487+
// Add Kubernetes-specific labels/annotations from the live object in the source to preserve metadata.
1488+
addKubernetesData(deploy.Spec.Template.Labels, existing.Spec.Template.Labels)
1489+
addKubernetesData(deploy.Spec.Template.Annotations, existing.Spec.Template.Annotations)
14831490

1484-
for key, value := range cr.Spec.Server.Labels {
1485-
deploy.Spec.Template.Labels[key] = value
1486-
}
14871491
if !reflect.DeepEqual(deploy.Spec.Template.Annotations, existing.Spec.Template.Annotations) {
14881492
existing.Spec.Template.Annotations = deploy.Spec.Template.Annotations
14891493
changed = true

‎controllers/argocd/statefulset.go‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ func newStatefulSetWithName(name string, component string, cr *argoproj.ArgoCD)
7171
Labels: map[string]string{
7272
common.ArgoCDKeyName: name,
7373
},
74+
Annotations: make(map[string]string),
7475
},
7576
Spec: corev1.PodSpec{
7677
NodeSelector: common.DefaultNodeSelector(),
@@ -743,7 +744,9 @@ func (r *ReconcileArgoCD) reconcileApplicationControllerStatefulSet(cr *argoproj
743744
}
744745

745746
if cr.Spec.Controller.Annotations != nil {
746-
ss.Spec.Template.Annotations = cr.Spec.Controller.Annotations
747+
for key, value := range cr.Spec.Controller.Annotations {
748+
ss.Spec.Template.Annotations[key] = value
749+
}
747750
}
748751

749752
if cr.Spec.Controller.Labels != nil {
@@ -814,15 +817,15 @@ func (r *ReconcileArgoCD) reconcileApplicationControllerStatefulSet(cr *argoproj
814817
changed = true
815818
}
816819

817-
ss.Spec.Template.Annotations = cr.Spec.Controller.Annotations
820+
// Add Kubernetes-specific labels/annotations from the live object in the source to preserve metadata.
821+
addKubernetesData(ss.Spec.Template.Labels, existing.Spec.Template.Labels)
822+
addKubernetesData(ss.Spec.Template.Annotations, existing.Spec.Template.Annotations)
823+
818824
if !reflect.DeepEqual(ss.Spec.Template.Annotations, existing.Spec.Template.Annotations) {
819825
existing.Spec.Template.Annotations = ss.Spec.Template.Annotations
820826
changed = true
821827
}
822828

823-
for key, value := range cr.Spec.Controller.Labels {
824-
ss.Spec.Template.Labels[key] = value
825-
}
826829
if !reflect.DeepEqual(ss.Spec.Template.Labels, existing.Spec.Template.Labels) {
827830
existing.Spec.Template.Labels = ss.Spec.Template.Labels
828831
changed = true

‎controllers/argocd/util.go‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1664,3 +1664,28 @@ func getApplicationSetHTTPServerHost(cr *argoproj.ArgoCD) (string, error) {
16641664
}
16651665
return host, nil
16661666
}
1667+
1668+
// addKubernetesData checks for any Kubernetes-specific labels or annotations
1669+
// in the live object and updates the source object to ensure critical metadata
1670+
// (like scheduling, topology, or lifecycle information) is retained.
1671+
// This helps avoid loss of important Kubernetes-managed metadata during updates.
1672+
func addKubernetesData(source map[string]string, live map[string]string) {
1673+
1674+
// List of Kubernetes-specific substrings (wildcard match)
1675+
patterns := []string{
1676+
"*kubernetes.io*",
1677+
"*k8s.io*",
1678+
"*openshift.io*",
1679+
}
1680+
1681+
for key, value := range live {
1682+
found := glob.MatchStringInList(patterns, key, glob.GLOB)
1683+
if found {
1684+
// Don't override values already present in the source object.
1685+
// This allows the operator to update Kubernetes specific data when needed.
1686+
if _, ok := source[key]; !ok {
1687+
source[key] = value
1688+
}
1689+
}
1690+
}
1691+
}

‎controllers/argocd/util_test.go‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1094,3 +1094,82 @@ func TestReconcileArgoCD_reconcileDexOAuthClientSecret(t *testing.T) {
10941094
}
10951095
assert.True(t, tokenExists, "Dex is enabled but unable to create oauth client secret")
10961096
}
1097+
1098+
func TestRetainKubernetesData(t *testing.T) {
1099+
tests := []struct {
1100+
name string
1101+
source map[string]string
1102+
live map[string]string
1103+
expected map[string]string
1104+
}{
1105+
{
1106+
name: "Add Kubernetes-specific keys not in source",
1107+
source: map[string]string{
1108+
"custom-label": "custom-value",
1109+
},
1110+
live: map[string]string{
1111+
"node.kubernetes.io/pod": "true",
1112+
"kubectl.kubernetes.io/restartedAt": "2024-12-05T09:46:46+05:30",
1113+
"openshift.openshift.io/restartedAt": "2024-12-05T09:46:46+05:30",
1114+
},
1115+
expected: map[string]string{
1116+
"custom-label": "custom-value", // unchanged
1117+
"node.kubernetes.io/pod": "true", // added
1118+
"kubectl.kubernetes.io/restartedAt": "2024-12-05T09:46:46+05:30", // added
1119+
"openshift.openshift.io/restartedAt": "2024-12-05T09:46:46+05:30", // added
1120+
},
1121+
},
1122+
{
1123+
name: "Ignores non-Kubernetes-specific keys",
1124+
source: map[string]string{
1125+
"custom-label": "custom-value",
1126+
},
1127+
live: map[string]string{
1128+
"non-k8s-key": "non-k8s-value",
1129+
"custom-label": "live-value",
1130+
},
1131+
expected: map[string]string{
1132+
"custom-label": "custom-value", // unchanged
1133+
},
1134+
},
1135+
{
1136+
name: "Do not override existing Kubernetes-specific keys in source",
1137+
source: map[string]string{
1138+
"node.kubernetes.io/pod": "source-true",
1139+
},
1140+
live: map[string]string{
1141+
"node.kubernetes.io/pod": "live-true", // should not override
1142+
},
1143+
expected: map[string]string{
1144+
"node.kubernetes.io/pod": "source-true", // source takes precedence
1145+
},
1146+
},
1147+
{
1148+
name: "Handles empty live map",
1149+
source: map[string]string{
1150+
"custom-label": "custom-value",
1151+
},
1152+
live: map[string]string{},
1153+
expected: map[string]string{
1154+
"custom-label": "custom-value", // unchanged
1155+
},
1156+
},
1157+
{
1158+
name: "Handles empty source map",
1159+
source: map[string]string{},
1160+
live: map[string]string{
1161+
"openshift.io/resource": "value",
1162+
},
1163+
expected: map[string]string{
1164+
"openshift.io/resource": "value", // added from live
1165+
},
1166+
},
1167+
}
1168+
1169+
for _, tt := range tests {
1170+
t.Run(tt.name, func(t *testing.T) {
1171+
addKubernetesData(tt.source, tt.live)
1172+
assert.Equal(t, tt.expected, tt.source)
1173+
})
1174+
}
1175+
}

0 commit comments

Comments
 (0)