Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: using our own pod template hashing #1809

Merged
Merged
4 changes: 2 additions & 2 deletions experiments/replicaset.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
patchtypes "k8s.io/apimachinery/pkg/types"
"k8s.io/kubernetes/pkg/controller"
labelsutil "k8s.io/kubernetes/pkg/util/labels"

"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
"github.com/argoproj/argo-rollouts/utils/conditions"
"github.com/argoproj/argo-rollouts/utils/defaults"
experimentutil "github.com/argoproj/argo-rollouts/utils/experiment"
"github.com/argoproj/argo-rollouts/utils/hash"
logutil "github.com/argoproj/argo-rollouts/utils/log"
"github.com/argoproj/argo-rollouts/utils/record"
replicasetutil "github.com/argoproj/argo-rollouts/utils/replicaset"
Expand Down Expand Up @@ -161,7 +161,7 @@ func newReplicaSetFromTemplate(experiment *v1alpha1.Experiment, template v1alpha
delete(newRSTemplate.Labels, v1alpha1.DefaultRolloutUniqueLabelKey)
}
}
podHash := controller.ComputeHash(&newRSTemplate, collisionCount)
podHash := hash.ComputePodTemplateHash(&newRSTemplate, collisionCount)

newRSTemplate.Labels = labelsutil.CloneAndAddLabel(newRSTemplate.Labels, v1alpha1.DefaultRolloutUniqueLabelKey, podHash)
// Add podTemplateHash label to selector.
Expand Down
6 changes: 3 additions & 3 deletions rollout/analysis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ import (
"testing"
"time"

"github.com/argoproj/argo-rollouts/utils/hash"
timeutil "github.com/argoproj/argo-rollouts/utils/time"

"github.com/stretchr/testify/assert"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/kubernetes/pkg/controller"
"k8s.io/utils/pointer"

"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
Expand Down Expand Up @@ -58,7 +58,7 @@ func clusterAnalysisTemplate(name string) *v1alpha1.ClusterAnalysisTemplate {

func clusterAnalysisRun(cat *v1alpha1.ClusterAnalysisTemplate, analysisRunType string, r *v1alpha1.Rollout) *v1alpha1.AnalysisRun {
labels := map[string]string{}
podHash := controller.ComputeHash(&r.Spec.Template, r.Status.CollisionCount)
podHash := hash.ComputePodTemplateHash(&r.Spec.Template, r.Status.CollisionCount)
var name string
if analysisRunType == v1alpha1.RolloutTypeStepLabel {
labels = analysisutil.StepLabels(*r.Status.CurrentStepIndex, podHash, "")
Expand Down Expand Up @@ -89,7 +89,7 @@ func clusterAnalysisRun(cat *v1alpha1.ClusterAnalysisTemplate, analysisRunType s

func analysisRun(at *v1alpha1.AnalysisTemplate, analysisRunType string, r *v1alpha1.Rollout) *v1alpha1.AnalysisRun {
labels := map[string]string{}
podHash := controller.ComputeHash(&r.Spec.Template, r.Status.CollisionCount)
podHash := hash.ComputePodTemplateHash(&r.Spec.Template, r.Status.CollisionCount)
var name string
if analysisRunType == v1alpha1.RolloutTypeStepLabel {
labels = analysisutil.StepLabels(*r.Status.CurrentStepIndex, podHash, "")
Expand Down
4 changes: 2 additions & 2 deletions rollout/bluegreen_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
core "k8s.io/client-go/testing"
"k8s.io/kubernetes/pkg/controller"
"k8s.io/utils/pointer"

"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
"github.com/argoproj/argo-rollouts/utils/annotations"
"github.com/argoproj/argo-rollouts/utils/conditions"
"github.com/argoproj/argo-rollouts/utils/hash"
rolloututil "github.com/argoproj/argo-rollouts/utils/rollout"
timeutil "github.com/argoproj/argo-rollouts/utils/time"
)
Expand All @@ -34,7 +34,7 @@ func newBlueGreenRollout(name string, replicas int, revisionHistoryLimit *int32,
AbortScaleDownDelaySeconds: &abortScaleDownDelaySeconds,
}
rollout.Status.CurrentStepHash = conditions.ComputeStepHash(rollout)
rollout.Status.CurrentPodHash = controller.ComputeHash(&rollout.Spec.Template, rollout.Status.CollisionCount)
rollout.Status.CurrentPodHash = hash.ComputePodTemplateHash(&rollout.Spec.Template, rollout.Status.CollisionCount)
return rollout
}

Expand Down
10 changes: 5 additions & 5 deletions rollout/canary_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@ import (
"k8s.io/apimachinery/pkg/util/intstr"
k8sinformers "k8s.io/client-go/informers"
k8sfake "k8s.io/client-go/kubernetes/fake"
"k8s.io/kubernetes/pkg/controller"
"k8s.io/utils/pointer"

"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
"github.com/argoproj/argo-rollouts/pkg/client/clientset/versioned/fake"
"github.com/argoproj/argo-rollouts/utils/annotations"
"github.com/argoproj/argo-rollouts/utils/conditions"
"github.com/argoproj/argo-rollouts/utils/hash"
logutil "github.com/argoproj/argo-rollouts/utils/log"
"github.com/argoproj/argo-rollouts/utils/record"
replicasetutil "github.com/argoproj/argo-rollouts/utils/replicaset"
Expand All @@ -39,7 +39,7 @@ func newCanaryRollout(name string, replicas int, revisionHistoryLimit *int32, st
}
rollout.Status.CurrentStepIndex = stepIndex
rollout.Status.CurrentStepHash = conditions.ComputeStepHash(rollout)
rollout.Status.CurrentPodHash = controller.ComputeHash(&rollout.Spec.Template, rollout.Status.CollisionCount)
rollout.Status.CurrentPodHash = hash.ComputePodTemplateHash(&rollout.Spec.Template, rollout.Status.CollisionCount)
rollout.Status.Selector = metav1.FormatLabelSelector(rollout.Spec.Selector)
rollout.Status.Phase, rollout.Status.Message = rolloututil.CalculateRolloutPhase(rollout.Spec, rollout.Status)
return rollout
Expand All @@ -54,7 +54,7 @@ func bumpVersion(rollout *v1alpha1.Rollout) *v1alpha1.Rollout {
newRevisionStr := strconv.FormatInt(int64(newRevision), 10)
annotations.SetRolloutRevision(newRollout, newRevisionStr)
newRollout.Spec.Template.Spec.Containers[0].Image = "foo/bar" + newRevisionStr
newRollout.Status.CurrentPodHash = controller.ComputeHash(&newRollout.Spec.Template, newRollout.Status.CollisionCount)
newRollout.Status.CurrentPodHash = hash.ComputePodTemplateHash(&newRollout.Spec.Template, newRollout.Status.CollisionCount)
newRollout.Status.CurrentStepHash = conditions.ComputeStepHash(newRollout)
newRollout.Status.Phase, newRollout.Status.Message = rolloututil.CalculateRolloutPhase(newRollout.Spec, newRollout.Status)
return newRollout
Expand Down Expand Up @@ -841,7 +841,7 @@ func TestRollBackToStable(t *testing.T) {
}
}`
newConditions := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, rs1, false, "")
expectedPatch := fmt.Sprintf(expectedPatchWithoutSub, controller.ComputeHash(&r2.Spec.Template, r2.Status.CollisionCount), newConditions)
expectedPatch := fmt.Sprintf(expectedPatchWithoutSub, hash.ComputePodTemplateHash(&r2.Spec.Template, r2.Status.CollisionCount), newConditions)
patch := f.getPatchedRollout(patchIndex)
assert.Equal(t, calculatePatch(r2, expectedPatch), patch)
}
Expand Down Expand Up @@ -929,7 +929,7 @@ func TestRollBackToStableAndStepChange(t *testing.T) {
"conditions": %s
}
}`
newPodHash := controller.ComputeHash(&r2.Spec.Template, r2.Status.CollisionCount)
newPodHash := hash.ComputePodTemplateHash(&r2.Spec.Template, r2.Status.CollisionCount)
newStepHash := conditions.ComputeStepHash(r2)
newConditions := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, rs1, false, "")
expectedPatch := fmt.Sprintf(expectedPatchWithoutSub, newPodHash, newStepHash, newConditions)
Expand Down
2 changes: 1 addition & 1 deletion rollout/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ func (c *Controller) newRolloutContext(rollout *v1alpha1.Rollout) (*rolloutConte
}

newRS := replicasetutil.FindNewReplicaSet(rollout, rsList)
olderRSs := replicasetutil.FindOldReplicaSets(rollout, rsList)
olderRSs := replicasetutil.FindOldReplicaSets(rollout, rsList, newRS)
stableRS := replicasetutil.GetStableRS(rollout, newRS, olderRSs)
otherRSs := replicasetutil.GetOtherRSs(rollout, newRS, stableRS, rsList)

Expand Down
6 changes: 3 additions & 3 deletions rollout/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import (
"k8s.io/client-go/tools/cache"
"k8s.io/client-go/util/workqueue"
corev1defaults "k8s.io/kubernetes/pkg/apis/core/v1"
"k8s.io/kubernetes/pkg/controller"
"k8s.io/utils/pointer"

"github.com/argoproj/argo-rollouts/controller/metrics"
Expand All @@ -47,6 +46,7 @@ import (
"github.com/argoproj/argo-rollouts/utils/annotations"
"github.com/argoproj/argo-rollouts/utils/conditions"
"github.com/argoproj/argo-rollouts/utils/defaults"
"github.com/argoproj/argo-rollouts/utils/hash"
ingressutil "github.com/argoproj/argo-rollouts/utils/ingress"
istioutil "github.com/argoproj/argo-rollouts/utils/istio"
"github.com/argoproj/argo-rollouts/utils/queue"
Expand Down Expand Up @@ -401,7 +401,7 @@ func updateBaseRolloutStatus(r *v1alpha1.Rollout, availableReplicas, updatedRepl
}

func newReplicaSet(r *v1alpha1.Rollout, replicas int) *appsv1.ReplicaSet {
podHash := controller.ComputeHash(&r.Spec.Template, r.Status.CollisionCount)
podHash := hash.ComputePodTemplateHash(&r.Spec.Template, r.Status.CollisionCount)
rsLabels := map[string]string{
v1alpha1.DefaultRolloutUniqueLabelKey: podHash,
}
Expand Down Expand Up @@ -1300,7 +1300,7 @@ func TestPodTemplateHashEquivalence(t *testing.T) {
var err error
// NOTE: This test will fail on every k8s library upgrade.
// To fix it, update expectedReplicaSetName to match the new hash.
expectedReplicaSetName := "guestbook-859fc5d686"
expectedReplicaSetName := "guestbook-6c5667f666"

r1 := newBlueGreenRollout("guestbook", 1, nil, "active", "")
r1Resources := `
Expand Down
15 changes: 8 additions & 7 deletions rollout/experiment.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,20 @@ import (
"context"
"fmt"

appsv1 "k8s.io/api/apps/v1"
"k8s.io/apimachinery/pkg/api/errors"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"

"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
analysisutil "github.com/argoproj/argo-rollouts/utils/analysis"
"github.com/argoproj/argo-rollouts/utils/annotations"
"github.com/argoproj/argo-rollouts/utils/defaults"
experimentutil "github.com/argoproj/argo-rollouts/utils/experiment"
"github.com/argoproj/argo-rollouts/utils/hash"
"github.com/argoproj/argo-rollouts/utils/record"
replicasetutil "github.com/argoproj/argo-rollouts/utils/replicaset"
appsv1 "k8s.io/api/apps/v1"
"k8s.io/apimachinery/pkg/api/errors"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/kubernetes/pkg/controller"
)

// GetExperimentFromTemplate takes the canary experiment step and converts it to an experiment
Expand All @@ -25,7 +26,7 @@ func GetExperimentFromTemplate(r *v1alpha1.Rollout, stableRS, newRS *appsv1.Repl
if step == nil {
return nil, nil
}
podHash := controller.ComputeHash(&r.Spec.Template, r.Status.CollisionCount)
podHash := hash.ComputePodTemplateHash(&r.Spec.Template, r.Status.CollisionCount)
currentStep := int32(0)
if r.Status.CurrentStepIndex != nil {
currentStep = *r.Status.CurrentStepIndex
Expand Down
5 changes: 3 additions & 2 deletions rollout/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/argoproj/argo-rollouts/utils/defaults"
"github.com/argoproj/argo-rollouts/utils/diff"
experimentutil "github.com/argoproj/argo-rollouts/utils/experiment"
"github.com/argoproj/argo-rollouts/utils/hash"
logutil "github.com/argoproj/argo-rollouts/utils/log"
"github.com/argoproj/argo-rollouts/utils/record"
replicasetutil "github.com/argoproj/argo-rollouts/utils/replicaset"
Expand Down Expand Up @@ -139,7 +140,7 @@ func (c *rolloutContext) createDesiredReplicaSet() (*appsv1.ReplicaSet, error) {
newRSTemplate := *c.rollout.Spec.Template.DeepCopy()
// Add default anti-affinity rule if antiAffinity bool set and RSTemplate meets requirements
newRSTemplate.Spec.Affinity = replicasetutil.GenerateReplicaSetAffinity(*c.rollout)
podTemplateSpecHash := controller.ComputeHash(&c.rollout.Spec.Template, c.rollout.Status.CollisionCount)
podTemplateSpecHash := hash.ComputePodTemplateHash(&c.rollout.Spec.Template, c.rollout.Status.CollisionCount)
newRSTemplate.Labels = labelsutil.CloneAndAddLabel(c.rollout.Spec.Template.Labels, v1alpha1.DefaultRolloutUniqueLabelKey, podTemplateSpecHash)
// Add podTemplateHash label to selector.
newRSSelector := labelsutil.CloneSelectorAndAddLabel(c.rollout.Spec.Selector, v1alpha1.DefaultRolloutUniqueLabelKey, podTemplateSpecHash)
Expand Down Expand Up @@ -378,7 +379,7 @@ func (c *rolloutContext) calculateBaseStatus() v1alpha1.RolloutStatus {
// newRS potentially might be nil when called by syncReplicasOnly(). For this
// to happen, the user would have had to simultaneously change the number of replicas, and
// the pod template spec at the same time.
currentPodHash = controller.ComputeHash(&c.rollout.Spec.Template, c.rollout.Status.CollisionCount)
currentPodHash = hash.ComputePodTemplateHash(&c.rollout.Spec.Template, c.rollout.Status.CollisionCount)
c.log.Infof("Assuming %s for new replicaset pod hash", currentPodHash)
} else {
currentPodHash = c.newRS.Labels[v1alpha1.DefaultRolloutUniqueLabelKey]
Expand Down
8 changes: 4 additions & 4 deletions utils/conditions/rollouts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ import (
"github.com/stretchr/testify/assert"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/kubernetes/pkg/controller"
"k8s.io/utils/pointer"

"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
"github.com/argoproj/argo-rollouts/utils/hash"
)

var (
Expand Down Expand Up @@ -363,7 +363,7 @@ func TestRolloutComplete(t *testing.T) {
},
}
r.Generation = 123
podHash := controller.ComputeHash(&r.Spec.Template, r.Status.CollisionCount)
podHash := hash.ComputePodTemplateHash(&r.Spec.Template, r.Status.CollisionCount)
r.Status.CurrentPodHash = podHash
return r
}
Expand Down Expand Up @@ -411,13 +411,13 @@ func TestRolloutComplete(t *testing.T) {
{
name: "BlueGreen complete",
// update hash to status.CurrentPodHash after k8s library update
r: blueGreenRollout(5, 5, 5, 5, true, "78957574d7", "78957574d7"),
r: blueGreenRollout(5, 5, 5, 5, true, "76bbb58f74", "76bbb58f74"),
expected: true,
},
{
name: "BlueGreen complete with extra old replicas",
// update hash to status.CurrentPodHash after k8s library update
r: blueGreenRollout(5, 6, 5, 5, true, "78957574d7", "78957574d7"),
r: blueGreenRollout(5, 6, 5, 5, true, "76bbb58f74", "76bbb58f74"),
expected: true,
},
{
Expand Down
5 changes: 3 additions & 2 deletions utils/experiment/experiment.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ import (

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
patchtypes "k8s.io/apimachinery/pkg/types"
"k8s.io/kubernetes/pkg/controller"

"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
rolloutsclient "github.com/argoproj/argo-rollouts/pkg/client/clientset/versioned/typed/rollouts/v1alpha1"
"github.com/argoproj/argo-rollouts/utils/defaults"
"github.com/argoproj/argo-rollouts/utils/hash"
timeutil "github.com/argoproj/argo-rollouts/utils/time"
)

Expand Down Expand Up @@ -131,8 +131,9 @@ func GetCollisionCountForTemplate(experiment *v1alpha1.Experiment, template v1al

// ReplicasetNameFromExperiment gets the replicaset name based off of the experiment and the template
func ReplicasetNameFromExperiment(experiment *v1alpha1.Experiment, template v1alpha1.TemplateSpec) string {
// todo: review this method for deletion as it's not using
collisionCount := GetCollisionCountForTemplate(experiment, template)
podTemplateSpecHash := controller.ComputeHash(&template.Template, collisionCount)
podTemplateSpecHash := hash.ComputePodTemplateHash(&template.Template, collisionCount)
return fmt.Sprintf("%s-%s-%s", experiment.Name, template.Name, podTemplateSpecHash)
}

Expand Down
10 changes: 6 additions & 4 deletions utils/experiment/experiment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@ import (
"testing"
"time"

"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
"github.com/argoproj/argo-rollouts/pkg/client/clientset/versioned/fake"
"github.com/stretchr/testify/assert"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
kubetesting "k8s.io/client-go/testing"
"k8s.io/utils/pointer"

"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
"github.com/argoproj/argo-rollouts/pkg/client/clientset/versioned/fake"
)

func TestHasFinished(t *testing.T) {
Expand Down Expand Up @@ -90,6 +91,7 @@ func TestGetTemplateStatusMapping(t *testing.T) {
assert.Equal(t, int32(1), mapping["test"].Replicas)
assert.Equal(t, int32(2), mapping["test2"].Replicas)
}

func TestReplicaSetNameFromExperiment(t *testing.T) {
templateName := "template"
template := v1alpha1.TemplateSpec{
Expand All @@ -100,14 +102,14 @@ func TestReplicaSetNameFromExperiment(t *testing.T) {
Name: "foo",
},
}
assert.Equal(t, "foo-template-78957574d7", ReplicasetNameFromExperiment(e, template))
assert.Equal(t, "foo-template-76bbb58f74", ReplicasetNameFromExperiment(e, template))

newTemplateStatus := v1alpha1.TemplateStatus{
Name: templateName,
CollisionCount: pointer.Int32Ptr(1),
}
e.Status.TemplateStatuses = append(e.Status.TemplateStatuses, newTemplateStatus)
assert.Equal(t, "foo-template-db98f55f8", ReplicasetNameFromExperiment(e, template))
assert.Equal(t, "foo-template-688c48b575", ReplicasetNameFromExperiment(e, template))
}

func TestExperimentByCreationTimestamp(t *testing.T) {
Expand Down
34 changes: 34 additions & 0 deletions utils/hash/hash.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package hash

import (
"encoding/binary"
"encoding/json"
"fmt"
"hash/fnv"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/util/rand"
)

// ComputePodTemplateHash returns a hash value calculated from pod template.
// The hash will be safe encoded to avoid bad words.
func ComputePodTemplateHash(template *corev1.PodTemplateSpec, collisionCount *int32) string {
podTemplateSpecHasher := fnv.New32a()
stepsBytes, err := json.Marshal(template)
if err != nil {
panic(err)
}
_, err = podTemplateSpecHasher.Write(stepsBytes)
if err != nil {
panic(err)
}
if collisionCount != nil {
collisionCountBytes := make([]byte, 8)
binary.LittleEndian.PutUint32(collisionCountBytes, uint32(*collisionCount))
_, err = podTemplateSpecHasher.Write(collisionCountBytes)
if err != nil {
panic(err)
}
}
return rand.SafeEncodeString(fmt.Sprint(podTemplateSpecHasher.Sum32()))
}
Loading