Fix issue where pod template hash could be computed inconsistently

controller/controller.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"reflect"
 	"time"
+	"encoding/json"
 
 	"github.com/golang/glog"
 	"github.com/pkg/errors"
@@ -282,8 +283,10 @@ func (c *Controller) syncHandler(key string) error {
 		return err
 	}
 
-	// Deep-copy otherwise we are mutating our cache.
-	r := rollout.DeepCopy()
+	// Remarshal the rollout to normalize all fields so that when we calculate hashes against the
+	// rollout spec and pod template spec, the hash will be consistent. See issue #70
+	// This also returns a copy of the rollout to prevent mutation of the informer cache.
+	r := remarshalRollout(rollout)
 	defer func() {
 		duration := time.Since(startTime)
 		c.metricsServer.IncReconcile(r, duration)
@@ -341,6 +344,19 @@ func (c *Controller) syncHandler(key string) error {
 	return fmt.Errorf("no rollout strategy selected")
 }
 
+func remarshalRollout(r *v1alpha1.Rollout) *v1alpha1.Rollout {
+	rolloutBytes, err := json.Marshal(r)
+	if err != nil {
+		panic(err)
+	}
+	var remarshalled v1alpha1.Rollout
+	err = json.Unmarshal(rolloutBytes, &remarshalled)
+	if err != nil {
+		panic(err)
+	}
+	return &remarshalled
+}
+
 func (c *Controller) enqueue(obj interface{}) {
 	var key string
 	var err error

controller/controller_test.go

@@ -24,6 +24,7 @@ import (
 	"k8s.io/client-go/tools/record"
 	"k8s.io/kubernetes/pkg/controller"
 	"k8s.io/utils/pointer"
+	"github.com/ghodss/yaml"
 
 	"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
 	"github.com/argoproj/argo-rollouts/pkg/client/clientset/versioned/fake"
@@ -717,4 +718,52 @@ func TestSwitchInvalidSpecMessage(t *testing.T) {
 
 	patch := f.getPatchedRollout(patchIndex)
 	assert.Equal(t, calculatePatch(r, expectedPatch), patch)
-}
+}
+
+// TestPodTemplateHashEquivalence verifies the hash is computed consistently when there are slight
+// variations made to the pod template in equivalent ways.
+func TestPodTemplateHashEquivalence(t *testing.T) {
+	var err error
+	expectedReplicaSetName := "guestbook-796789b97b"
+
+	r1 := newBlueGreenRollout("guestbook", 1, nil, "active", "")
+	r1Resources := `
+limits:
+  cpu: 150m
+  memory: 8192M
+requests:
+  cpu: 2000m
+  memory: 8192M
+`
+	err = yaml.Unmarshal([]byte(r1Resources), &r1.Spec.Template.Spec.Containers[0].Resources)
+	assert.NoError(t, err)
+
+	r2 := newBlueGreenRollout("guestbook", 1, nil, "active", "")
+	r2Resources := `
+  limits:
+    cpu: 0.15
+    memory: 8192M
+  requests:
+    cpu: '2'
+    memory: 8192M
+`
+	err = yaml.Unmarshal([]byte(r2Resources), &r2.Spec.Template.Spec.Containers[0].Resources)
+	assert.NoError(t, err)
+
+	for _, r := range []*v1alpha1.Rollout{r1, r2} {
+		f := newFixture(t)
+		activeSvc := newService("active", 80, nil)
+		f.kubeobjects = append(f.kubeobjects, activeSvc)
+		f.rolloutLister = append(f.rolloutLister, r)
+		f.objects = append(f.objects, r)
+
+		_ = f.expectUpdateRolloutAction(r)
+		f.expectPatchRolloutAction(r)
+		rs := newReplicaSet(r, expectedReplicaSetName, 1)
+		f.expectGetServiceAction(activeSvc)
+		rsIdx := f.expectCreateReplicaSetAction(rs)
+		f.run(getKey(r, t))
+		rs = f.getCreatedReplicaSet(rsIdx)
+		assert.Equal(t, expectedReplicaSetName, rs.Name)
+	}
+}