Skip to content

Commit

Permalink
fix: unsolicited rollout after upgrade from v0.10->v1.0 when pod was …
Browse files Browse the repository at this point in the history
…using service account

Signed-off-by: Jesse Suen <[email protected]>
  • Loading branch information
jessesuen committed Jul 22, 2021
1 parent 0c559a0 commit e4e39e9
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 0 deletions.
9 changes: 9 additions & 0 deletions utils/replicaset/replicaset.go
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,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)
}

Expand Down
51 changes: 51 additions & 0 deletions utils/replicaset/replicaset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -1186,5 +1187,55 @@ func TestGetTimeRemainingBeforeScaleDownDeadline(t *testing.T) {
assert.Nil(t, err)
assert.NotNil(t, remainingTime)
}
}

// 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))
}

0 comments on commit e4e39e9

Please sign in to comment.