Skip to content

Commit e79296a

Browse files
perenesenkoperenesenko
authored
fix: using our own pod template hashing (argoproj#1809)
* fix: implement pod template hash calculate function Signed-off-by: Andrii Perenesenko <[email protected]> * fix: using new hashing for FindNewReplicaSet and other places Signed-off-by: Andrii Perenesenko <[email protected]> * fix tests Signed-off-by: Andrii Perenesenko <[email protected]> * fix tests with deprecated hash Signed-off-by: Andrii Perenesenko <[email protected]> * test TestFindNewReplicaSet, TestHashUtils Signed-off-by: Andrii Perenesenko <[email protected]> * fix the doc for ComputePodTemplateHash Signed-off-by: Andrii Perenesenko <[email protected]> * test for ComputePodTemplateHash with collisionCount Signed-off-by: Andrii Perenesenko <[email protected]> * refactoring avoid double FindNewReplicaSet call: pass newRS as a parameter to the FindOldReplicaSets as it using only right after the FindNewReplicaSet Signed-off-by: Andrii Perenesenko <[email protected]> * fix time dependent flaky test Signed-off-by: Andrii Perenesenko <[email protected]>
1 parent f388937 commit e79296a

File tree

15 files changed

+175
-53
lines changed

15 files changed

+175
-53
lines changed

‎experiments/replicaset.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,13 @@ import (
1313
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1414
"k8s.io/apimachinery/pkg/labels"
1515
patchtypes "k8s.io/apimachinery/pkg/types"
16-
"k8s.io/kubernetes/pkg/controller"
1716
labelsutil "k8s.io/kubernetes/pkg/util/labels"
1817

1918
"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
2019
"github.com/argoproj/argo-rollouts/utils/conditions"
2120
"github.com/argoproj/argo-rollouts/utils/defaults"
2221
experimentutil "github.com/argoproj/argo-rollouts/utils/experiment"
22+
"github.com/argoproj/argo-rollouts/utils/hash"
2323
logutil "github.com/argoproj/argo-rollouts/utils/log"
2424
"github.com/argoproj/argo-rollouts/utils/record"
2525
replicasetutil "github.com/argoproj/argo-rollouts/utils/replicaset"
@@ -161,7 +161,7 @@ func newReplicaSetFromTemplate(experiment *v1alpha1.Experiment, template v1alpha
161161
delete(newRSTemplate.Labels, v1alpha1.DefaultRolloutUniqueLabelKey)
162162
}
163163
}
164-
podHash := controller.ComputeHash(&newRSTemplate, collisionCount)
164+
podHash := hash.ComputePodTemplateHash(&newRSTemplate, collisionCount)
165165

166166
newRSTemplate.Labels = labelsutil.CloneAndAddLabel(newRSTemplate.Labels, v1alpha1.DefaultRolloutUniqueLabelKey, podHash)
167167
// Add podTemplateHash label to selector.

‎rollout/analysis_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,13 @@ import (
88
"testing"
99
"time"
1010

11+
"github.com/argoproj/argo-rollouts/utils/hash"
1112
timeutil "github.com/argoproj/argo-rollouts/utils/time"
1213

1314
"github.com/stretchr/testify/assert"
1415
corev1 "k8s.io/api/core/v1"
1516
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1617
"k8s.io/apimachinery/pkg/util/intstr"
17-
"k8s.io/kubernetes/pkg/controller"
1818
"k8s.io/utils/pointer"
1919

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

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

9090
func analysisRun(at *v1alpha1.AnalysisTemplate, analysisRunType string, r *v1alpha1.Rollout) *v1alpha1.AnalysisRun {
9191
labels := map[string]string{}
92-
podHash := controller.ComputeHash(&r.Spec.Template, r.Status.CollisionCount)
92+
podHash := hash.ComputePodTemplateHash(&r.Spec.Template, r.Status.CollisionCount)
9393
var name string
9494
if analysisRunType == v1alpha1.RolloutTypeStepLabel {
9595
labels = analysisutil.StepLabels(*r.Status.CurrentStepIndex, podHash, "")

‎rollout/bluegreen_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,12 @@ import (
1111
corev1 "k8s.io/api/core/v1"
1212
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1313
core "k8s.io/client-go/testing"
14-
"k8s.io/kubernetes/pkg/controller"
1514
"k8s.io/utils/pointer"
1615

1716
"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
1817
"github.com/argoproj/argo-rollouts/utils/annotations"
1918
"github.com/argoproj/argo-rollouts/utils/conditions"
19+
"github.com/argoproj/argo-rollouts/utils/hash"
2020
rolloututil "github.com/argoproj/argo-rollouts/utils/rollout"
2121
timeutil "github.com/argoproj/argo-rollouts/utils/time"
2222
)
@@ -34,7 +34,7 @@ func newBlueGreenRollout(name string, replicas int, revisionHistoryLimit *int32,
3434
AbortScaleDownDelaySeconds: &abortScaleDownDelaySeconds,
3535
}
3636
rollout.Status.CurrentStepHash = conditions.ComputeStepHash(rollout)
37-
rollout.Status.CurrentPodHash = controller.ComputeHash(&rollout.Spec.Template, rollout.Status.CollisionCount)
37+
rollout.Status.CurrentPodHash = hash.ComputePodTemplateHash(&rollout.Spec.Template, rollout.Status.CollisionCount)
3838
return rollout
3939
}
4040

‎rollout/canary_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,13 @@ import (
1515
"k8s.io/apimachinery/pkg/util/intstr"
1616
k8sinformers "k8s.io/client-go/informers"
1717
k8sfake "k8s.io/client-go/kubernetes/fake"
18-
"k8s.io/kubernetes/pkg/controller"
1918
"k8s.io/utils/pointer"
2019

2120
"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
2221
"github.com/argoproj/argo-rollouts/pkg/client/clientset/versioned/fake"
2322
"github.com/argoproj/argo-rollouts/utils/annotations"
2423
"github.com/argoproj/argo-rollouts/utils/conditions"
24+
"github.com/argoproj/argo-rollouts/utils/hash"
2525
logutil "github.com/argoproj/argo-rollouts/utils/log"
2626
"github.com/argoproj/argo-rollouts/utils/record"
2727
replicasetutil "github.com/argoproj/argo-rollouts/utils/replicaset"
@@ -39,7 +39,7 @@ func newCanaryRollout(name string, replicas int, revisionHistoryLimit *int32, st
3939
}
4040
rollout.Status.CurrentStepIndex = stepIndex
4141
rollout.Status.CurrentStepHash = conditions.ComputeStepHash(rollout)
42-
rollout.Status.CurrentPodHash = controller.ComputeHash(&rollout.Spec.Template, rollout.Status.CollisionCount)
42+
rollout.Status.CurrentPodHash = hash.ComputePodTemplateHash(&rollout.Spec.Template, rollout.Status.CollisionCount)
4343
rollout.Status.Selector = metav1.FormatLabelSelector(rollout.Spec.Selector)
4444
rollout.Status.Phase, rollout.Status.Message = rolloututil.CalculateRolloutPhase(rollout.Spec, rollout.Status)
4545
return rollout
@@ -54,7 +54,7 @@ func bumpVersion(rollout *v1alpha1.Rollout) *v1alpha1.Rollout {
5454
newRevisionStr := strconv.FormatInt(int64(newRevision), 10)
5555
annotations.SetRolloutRevision(newRollout, newRevisionStr)
5656
newRollout.Spec.Template.Spec.Containers[0].Image = "foo/bar" + newRevisionStr
57-
newRollout.Status.CurrentPodHash = controller.ComputeHash(&newRollout.Spec.Template, newRollout.Status.CollisionCount)
57+
newRollout.Status.CurrentPodHash = hash.ComputePodTemplateHash(&newRollout.Spec.Template, newRollout.Status.CollisionCount)
5858
newRollout.Status.CurrentStepHash = conditions.ComputeStepHash(newRollout)
5959
newRollout.Status.Phase, newRollout.Status.Message = rolloututil.CalculateRolloutPhase(newRollout.Spec, newRollout.Status)
6060
return newRollout
@@ -841,7 +841,7 @@ func TestRollBackToStable(t *testing.T) {
841841
}
842842
}`
843843
newConditions := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, rs1, false, "")
844-
expectedPatch := fmt.Sprintf(expectedPatchWithoutSub, controller.ComputeHash(&r2.Spec.Template, r2.Status.CollisionCount), newConditions)
844+
expectedPatch := fmt.Sprintf(expectedPatchWithoutSub, hash.ComputePodTemplateHash(&r2.Spec.Template, r2.Status.CollisionCount), newConditions)
845845
patch := f.getPatchedRollout(patchIndex)
846846
assert.Equal(t, calculatePatch(r2, expectedPatch), patch)
847847
}
@@ -929,7 +929,7 @@ func TestRollBackToStableAndStepChange(t *testing.T) {
929929
"conditions": %s
930930
}
931931
}`
932-
newPodHash := controller.ComputeHash(&r2.Spec.Template, r2.Status.CollisionCount)
932+
newPodHash := hash.ComputePodTemplateHash(&r2.Spec.Template, r2.Status.CollisionCount)
933933
newStepHash := conditions.ComputeStepHash(r2)
934934
newConditions := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, rs1, false, "")
935935
expectedPatch := fmt.Sprintf(expectedPatchWithoutSub, newPodHash, newStepHash, newConditions)

‎rollout/controller.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -433,7 +433,7 @@ func (c *Controller) newRolloutContext(rollout *v1alpha1.Rollout) (*rolloutConte
433433
}
434434

435435
newRS := replicasetutil.FindNewReplicaSet(rollout, rsList)
436-
olderRSs := replicasetutil.FindOldReplicaSets(rollout, rsList)
436+
olderRSs := replicasetutil.FindOldReplicaSets(rollout, rsList, newRS)
437437
stableRS := replicasetutil.GetStableRS(rollout, newRS, olderRSs)
438438
otherRSs := replicasetutil.GetOtherRSs(rollout, newRS, stableRS, rsList)
439439

‎rollout/controller_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ import (
3434
"k8s.io/client-go/tools/cache"
3535
"k8s.io/client-go/util/workqueue"
3636
corev1defaults "k8s.io/kubernetes/pkg/apis/core/v1"
37-
"k8s.io/kubernetes/pkg/controller"
3837
"k8s.io/utils/pointer"
3938

4039
"github.com/argoproj/argo-rollouts/controller/metrics"
@@ -47,6 +46,7 @@ import (
4746
"github.com/argoproj/argo-rollouts/utils/annotations"
4847
"github.com/argoproj/argo-rollouts/utils/conditions"
4948
"github.com/argoproj/argo-rollouts/utils/defaults"
49+
"github.com/argoproj/argo-rollouts/utils/hash"
5050
ingressutil "github.com/argoproj/argo-rollouts/utils/ingress"
5151
istioutil "github.com/argoproj/argo-rollouts/utils/istio"
5252
"github.com/argoproj/argo-rollouts/utils/queue"
@@ -401,7 +401,7 @@ func updateBaseRolloutStatus(r *v1alpha1.Rollout, availableReplicas, updatedRepl
401401
}
402402

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

13051305
r1 := newBlueGreenRollout("guestbook", 1, nil, "active", "")
13061306
r1Resources := `

‎rollout/experiment.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,20 @@ import (
44
"context"
55
"fmt"
66

7+
appsv1 "k8s.io/api/apps/v1"
8+
"k8s.io/apimachinery/pkg/api/errors"
9+
k8serrors "k8s.io/apimachinery/pkg/api/errors"
10+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
11+
"k8s.io/apimachinery/pkg/labels"
12+
713
"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
814
analysisutil "github.com/argoproj/argo-rollouts/utils/analysis"
915
"github.com/argoproj/argo-rollouts/utils/annotations"
1016
"github.com/argoproj/argo-rollouts/utils/defaults"
1117
experimentutil "github.com/argoproj/argo-rollouts/utils/experiment"
18+
"github.com/argoproj/argo-rollouts/utils/hash"
1219
"github.com/argoproj/argo-rollouts/utils/record"
1320
replicasetutil "github.com/argoproj/argo-rollouts/utils/replicaset"
14-
appsv1 "k8s.io/api/apps/v1"
15-
"k8s.io/apimachinery/pkg/api/errors"
16-
k8serrors "k8s.io/apimachinery/pkg/api/errors"
17-
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
18-
"k8s.io/apimachinery/pkg/labels"
19-
"k8s.io/kubernetes/pkg/controller"
2021
)
2122

2223
// GetExperimentFromTemplate takes the canary experiment step and converts it to an experiment
@@ -25,7 +26,7 @@ func GetExperimentFromTemplate(r *v1alpha1.Rollout, stableRS, newRS *appsv1.Repl
2526
if step == nil {
2627
return nil, nil
2728
}
28-
podHash := controller.ComputeHash(&r.Spec.Template, r.Status.CollisionCount)
29+
podHash := hash.ComputePodTemplateHash(&r.Spec.Template, r.Status.CollisionCount)
2930
currentStep := int32(0)
3031
if r.Status.CurrentStepIndex != nil {
3132
currentStep = *r.Status.CurrentStepIndex

‎rollout/sync.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"github.com/argoproj/argo-rollouts/utils/defaults"
2424
"github.com/argoproj/argo-rollouts/utils/diff"
2525
experimentutil "github.com/argoproj/argo-rollouts/utils/experiment"
26+
"github.com/argoproj/argo-rollouts/utils/hash"
2627
logutil "github.com/argoproj/argo-rollouts/utils/log"
2728
"github.com/argoproj/argo-rollouts/utils/record"
2829
replicasetutil "github.com/argoproj/argo-rollouts/utils/replicaset"
@@ -139,7 +140,7 @@ func (c *rolloutContext) createDesiredReplicaSet() (*appsv1.ReplicaSet, error) {
139140
newRSTemplate := *c.rollout.Spec.Template.DeepCopy()
140141
// Add default anti-affinity rule if antiAffinity bool set and RSTemplate meets requirements
141142
newRSTemplate.Spec.Affinity = replicasetutil.GenerateReplicaSetAffinity(*c.rollout)
142-
podTemplateSpecHash := controller.ComputeHash(&c.rollout.Spec.Template, c.rollout.Status.CollisionCount)
143+
podTemplateSpecHash := hash.ComputePodTemplateHash(&c.rollout.Spec.Template, c.rollout.Status.CollisionCount)
143144
newRSTemplate.Labels = labelsutil.CloneAndAddLabel(c.rollout.Spec.Template.Labels, v1alpha1.DefaultRolloutUniqueLabelKey, podTemplateSpecHash)
144145
// Add podTemplateHash label to selector.
145146
newRSSelector := labelsutil.CloneSelectorAndAddLabel(c.rollout.Spec.Selector, v1alpha1.DefaultRolloutUniqueLabelKey, podTemplateSpecHash)
@@ -378,7 +379,7 @@ func (c *rolloutContext) calculateBaseStatus() v1alpha1.RolloutStatus {
378379
// newRS potentially might be nil when called by syncReplicasOnly(). For this
379380
// to happen, the user would have had to simultaneously change the number of replicas, and
380381
// the pod template spec at the same time.
381-
currentPodHash = controller.ComputeHash(&c.rollout.Spec.Template, c.rollout.Status.CollisionCount)
382+
currentPodHash = hash.ComputePodTemplateHash(&c.rollout.Spec.Template, c.rollout.Status.CollisionCount)
382383
c.log.Infof("Assuming %s for new replicaset pod hash", currentPodHash)
383384
} else {
384385
currentPodHash = c.newRS.Labels[v1alpha1.DefaultRolloutUniqueLabelKey]

‎utils/conditions/rollouts_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@ import (
88
"github.com/stretchr/testify/assert"
99
v1 "k8s.io/api/core/v1"
1010
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
11-
"k8s.io/kubernetes/pkg/controller"
1211
"k8s.io/utils/pointer"
1312

1413
"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
14+
"github.com/argoproj/argo-rollouts/utils/hash"
1515
)
1616

1717
var (
@@ -363,7 +363,7 @@ func TestRolloutComplete(t *testing.T) {
363363
},
364364
}
365365
r.Generation = 123
366-
podHash := controller.ComputeHash(&r.Spec.Template, r.Status.CollisionCount)
366+
podHash := hash.ComputePodTemplateHash(&r.Spec.Template, r.Status.CollisionCount)
367367
r.Status.CurrentPodHash = podHash
368368
return r
369369
}
@@ -411,13 +411,13 @@ func TestRolloutComplete(t *testing.T) {
411411
{
412412
name: "BlueGreen complete",
413413
// update hash to status.CurrentPodHash after k8s library update
414-
r: blueGreenRollout(5, 5, 5, 5, true, "78957574d7", "78957574d7"),
414+
r: blueGreenRollout(5, 5, 5, 5, true, "76bbb58f74", "76bbb58f74"),
415415
expected: true,
416416
},
417417
{
418418
name: "BlueGreen complete with extra old replicas",
419419
// update hash to status.CurrentPodHash after k8s library update
420-
r: blueGreenRollout(5, 6, 5, 5, true, "78957574d7", "78957574d7"),
420+
r: blueGreenRollout(5, 6, 5, 5, true, "76bbb58f74", "76bbb58f74"),
421421
expected: true,
422422
},
423423
{

‎utils/experiment/experiment.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@ import (
88

99
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1010
patchtypes "k8s.io/apimachinery/pkg/types"
11-
"k8s.io/kubernetes/pkg/controller"
1211

1312
"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
1413
rolloutsclient "github.com/argoproj/argo-rollouts/pkg/client/clientset/versioned/typed/rollouts/v1alpha1"
1514
"github.com/argoproj/argo-rollouts/utils/defaults"
15+
"github.com/argoproj/argo-rollouts/utils/hash"
1616
timeutil "github.com/argoproj/argo-rollouts/utils/time"
1717
)
1818

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

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

0 commit comments

Comments
 (0)