fix: unsolicited rollout after upgrade from v0.10->v1.0 when pod was using service account (argoproj#1367)

Signed-off-by: Jesse Suen <[email protected]>

Author: jessesuen

utils/replicaset/replicaset.go

@@ -493,6 +493,15 @@ func PodTemplateEqualIgnoreHash(live, desired *corev1.PodTemplateSpec) bool {
 	}
 	corev1defaults.SetObjectDefaults_PodTemplate(&podTemplate)
 	desired = &podTemplate.Template
+
+	// Do not allow the deprecated spec.serviceAccount to factor into the equality check. In live
+	// ReplicaSet pod template, this field will be populated, but in the desired pod template
+	// it will be missing (even after defaulting), causing us to believe there is a diff
+	// (when there really wasn't), and hence causing an unsolicited update to be triggered.
+	// See: https://github.com/argoproj/argo-rollouts/issues/1356
+	desired.Spec.DeprecatedServiceAccount = ""
+	live.Spec.DeprecatedServiceAccount = ""
+
 	return apiequality.Semantic.DeepEqual(live, desired)
 }
 

utils/replicaset/replicaset_test.go

@@ -8,6 +8,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/ghodss/yaml"
 	"github.com/stretchr/testify/assert"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -1167,3 +1168,54 @@ func TestGetPodsOwnedByReplicaSet(t *testing.T) {
 	assert.Len(t, pods, 1)
 	assert.Equal(t, "guestbook-abc123", pods[0].Name)
 }
+
+// TestPodTemplateEqualIgnoreHashWithServiceAccount catches a corner case where the K8s ComputeHash
+// function changed from underneath us, and we fell back to deep equality checking, which then
+// incorrectly detected a diff because of a deprecated field being present in the live but not desired.
+func TestPodTemplateEqualIgnoreHashWithServiceAccount(t *testing.T) {
+	var desired corev1.PodTemplateSpec
+	desiredTemplate := `
+metadata:
+  labels:
+    app: serviceaccount-ro
+spec:
+  containers:
+  - image: nginx:1.19-alpine
+    name: app
+  serviceAccountName: default
+`
+	err := yaml.Unmarshal([]byte(desiredTemplate), &desired)
+	assert.NoError(t, err)
+
+	// liveTemplate was captured from a ReplicaSet generated from the above desired template using
+	// Argo Rollouts v0.10. The rollouts-pod-template-hash value will not match newer hashing
+	// versions, causing PodTemplateEqualIgnoreHash to fall back to a deep equality check and
+	// pod template defaulting.
+	liveTemplate := `
+metadata:
+  creationTimestamp: null
+  labels:
+    app: serviceaccount-ro
+    rollouts-pod-template-hash: 8684587d99
+spec:
+  containers:
+  - image: nginx:1.19-alpine
+    imagePullPolicy: IfNotPresent
+    name: app
+    resources: {}
+    terminationMessagePath: /dev/termination-log
+    terminationMessagePolicy: File
+  dnsPolicy: ClusterFirst
+  restartPolicy: Always
+  schedulerName: default-scheduler
+  securityContext: {}
+  serviceAccount: default
+  serviceAccountName: default
+  terminationGracePeriodSeconds: 30
+`
+	var live corev1.PodTemplateSpec
+	err = yaml.Unmarshal([]byte(liveTemplate), &live)
+	assert.NoError(t, err)
+
+	assert.True(t, PodTemplateEqualIgnoreHash(&live, &desired))
+}