Skip to content

Commit

Permalink
feat: Implement Issue #1779: Add MinReplicas option to SetCanaryScale
Browse files Browse the repository at this point in the history
Signed-off-by: Shlomo Sanders <[email protected]>
  • Loading branch information
Shlomo Sanders committed Mar 28, 2022
1 parent 55a041a commit 93c7595
Show file tree
Hide file tree
Showing 12 changed files with 591 additions and 449 deletions.
3 changes: 3 additions & 0 deletions manifests/crds/rollout-crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,9 @@ spec:
properties:
matchTrafficWeight:
type: boolean
minReplicas:
format: int32
type: integer
replicas:
format: int32
type: integer
Expand Down
3 changes: 3 additions & 0 deletions manifests/install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11554,6 +11554,9 @@ spec:
properties:
matchTrafficWeight:
type: boolean
minReplicas:
format: int32
type: integer
replicas:
format: int32
type: integer
Expand Down
3 changes: 3 additions & 0 deletions manifests/namespace-install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11554,6 +11554,9 @@ spec:
properties:
matchTrafficWeight:
type: boolean
minReplicas:
format: int32
type: integer
replicas:
format: int32
type: integer
Expand Down
5 changes: 5 additions & 0 deletions pkg/apiclient/rollout/rollout.swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -1558,6 +1558,11 @@
"matchTrafficWeight": {
"type": "boolean",
"title": "MatchTrafficWeight cancels out previously set Replicas or Weight, effectively activating SetWeight\n+optional"
},
"minReplicas": {
"type": "integer",
"format": "int32",
"title": "MinReplicas is used together with Weight to define a minimum number of pods for all ReplicaSets\n+optional"
}
},
"title": "SetCanaryScale defines how to scale the newRS without changing traffic weight"
Expand Down
906 changes: 468 additions & 438 deletions pkg/apis/rollouts/v1alpha1/generated.pb.go

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions pkg/apis/rollouts/v1alpha1/generated.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions pkg/apis/rollouts/v1alpha1/openapi_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions pkg/apis/rollouts/v1alpha1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,9 @@ type SetCanaryScale struct {
// MatchTrafficWeight cancels out previously set Replicas or Weight, effectively activating SetWeight
// +optional
MatchTrafficWeight bool `json:"matchTrafficWeight,omitempty" protobuf:"varint,3,opt,name=matchTrafficWeight"`
// MinReplicas is used together with Weight to define a minimum number of pods for all ReplicaSets
// +optional
MinReplicas *int32 `json:"minReplicas,omitempty" protobuf:"varint,4,opt,name=minReplicas"`
}

// RolloutAnalysisBackground defines a template that is used to create a background analysisRun
Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/rollouts/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

28 changes: 18 additions & 10 deletions utils/replicaset/canary.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,14 @@ func AtDesiredReplicaCountsForCanary(ro *v1alpha1.Rollout, newRS, stableRS *apps
// For more examples, check the CalculateReplicaCountsForBasicCanary test in canary/canary_test.go
func CalculateReplicaCountsForBasicCanary(rollout *v1alpha1.Rollout, newRS *appsv1.ReplicaSet, stableRS *appsv1.ReplicaSet, oldRSs []*appsv1.ReplicaSet) (int32, int32) {
rolloutSpecReplica := defaults.GetReplicasOrDefault(rollout.Spec.Replicas)
_, desiredWeight := GetCanaryReplicasOrWeight(rollout)
_, desiredWeight, minReplicas := GetCanaryReplicasOrWeight(rollout)
maxSurge := MaxSurge(rollout)

desiredNewRSReplicaCount, desiredStableRSReplicaCount := approximateWeightedCanaryStableReplicaCounts(rolloutSpecReplica, desiredWeight, maxSurge)

desiredNewRSReplicaCount = max(desiredNewRSReplicaCount, minReplicas)
desiredStableRSReplicaCount = max(desiredStableRSReplicaCount, minReplicas)

stableRSReplicaCount := int32(0)
newRSReplicaCount := int32(0)
if newRS != nil {
Expand Down Expand Up @@ -148,6 +151,7 @@ func CalculateReplicaCountsForBasicCanary(rollout *v1alpha1.Rollout, newRS *apps
// weight (e.g. we are aborting), then we can ignore pod availability of the canaryRS.
isIncreasing := newRS == nil || desiredNewRSReplicaCount >= *newRS.Spec.Replicas
replicasToScaleDown := GetReplicasForScaleDown(newRS, !isIncreasing) + GetReplicasForScaleDown(stableRS, isIncreasing)

if replicasToScaleDown <= minAvailableReplicaCount {
// Cannot scale down stableRS or newRS without going below min available replica count
return newRSReplicaCount, stableRSReplicaCount
Expand Down Expand Up @@ -314,12 +318,12 @@ func maxValue(countA int32, countB int32) int32 {
func CalculateReplicaCountsForTrafficRoutedCanary(rollout *v1alpha1.Rollout, weights *v1alpha1.TrafficWeights) (int32, int32) {
var canaryCount, stableCount int32
rolloutSpecReplica := defaults.GetReplicasOrDefault(rollout.Spec.Replicas)
setCanaryScaleReplicas, desiredWeight := GetCanaryReplicasOrWeight(rollout)
setCanaryScaleReplicas, desiredWeight, minReplicas := GetCanaryReplicasOrWeight(rollout)
if setCanaryScaleReplicas != nil {
// a canary count was explicitly set
canaryCount = *setCanaryScaleReplicas
} else {
canaryCount = trafficWeightToReplicas(rolloutSpecReplica, desiredWeight)
canaryCount = max(trafficWeightToReplicas(rolloutSpecReplica, desiredWeight), minReplicas)
}

if !rollout.Spec.Strategy.Canary.DynamicStableScale {
Expand Down Expand Up @@ -350,7 +354,7 @@ func CalculateReplicaCountsForTrafficRoutedCanary(rollout *v1alpha1.Rollout, wei
canaryCount = max(trafficWeightReplicaCount, canaryCount)
}
}
return canaryCount, stableCount
return canaryCount, max(stableCount, minReplicas)
}

// trafficWeightToReplicas returns the appropriate replicas given the full spec.replicas and a weight
Expand Down Expand Up @@ -434,19 +438,23 @@ func GetCurrentCanaryStep(rollout *v1alpha1.Rollout) (*v1alpha1.CanaryStep, *int
return &rollout.Spec.Strategy.Canary.Steps[currentStepIndex], &currentStepIndex
}

// GetCanaryReplicasOrWeight either returns a static set of replicas or a weight percentage
func GetCanaryReplicasOrWeight(rollout *v1alpha1.Rollout) (*int32, int32) {
// GetCanaryReplicasOrWeight either returns a static set of replicas or a weight percentage plus minReplicas (minReplicas defaults to 0)
func GetCanaryReplicasOrWeight(rollout *v1alpha1.Rollout) (*int32, int32, int32) {
if rollout.Status.PromoteFull || rollout.Status.StableRS == "" || rollout.Status.CurrentPodHash == rollout.Status.StableRS {
return nil, 100
return nil, 100, 0
}
if scs := UseSetCanaryScale(rollout); scs != nil {
if scs.Replicas != nil {
return scs.Replicas, 0
return scs.Replicas, 0, 0
} else if scs.Weight != nil {
return nil, *scs.Weight
if scs.MinReplicas != nil {
return nil, *scs.Weight, *scs.MinReplicas
} else {
return nil, *scs.Weight, 0
}
}
}
return nil, GetCurrentSetWeight(rollout)
return nil, GetCurrentSetWeight(rollout), 0
}

// GetCurrentSetWeight grabs the current setWeight used by the rollout by iterating backwards from the current step
Expand Down
69 changes: 69 additions & 0 deletions utils/replicaset/canary_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ func TestCalculateReplicaCountsForCanary(t *testing.T) {

abortScaleDownDelaySeconds *int32
statusAbort bool
minReplicas *int32
}{
{
name: "Do not add extra RSs in scaleDownCount when .Spec.Replica < AvailableReplicas",
Expand Down Expand Up @@ -448,6 +449,33 @@ func TestCalculateReplicaCountsForCanary(t *testing.T) {
setCanaryScale: newSetCanaryScale(nil, intPnt(200), false),
trafficRouting: &v1alpha1.RolloutTrafficRouting{},
},
{
name: "Use setCanaryScale.minReplicas for canary replicas",

rolloutSpecReplicas: 4,
stableSpecReplica: 4,
stableAvailableReplica: 4,
minReplicas: intPnt(2),

expectedStableReplicaCount: 4,
expectedCanaryReplicaCount: 2,
setWeight: 20,
setCanaryScale: newSetCanaryScale(nil, intPnt(10), false),
trafficRouting: &v1alpha1.RolloutTrafficRouting{},
},
{
name: "test without setCanaryScale.minReplicas for canary replicas",

rolloutSpecReplicas: 4,
stableSpecReplica: 4,
stableAvailableReplica: 4,

expectedStableReplicaCount: 4,
expectedCanaryReplicaCount: 1,
setWeight: 20,
setCanaryScale: newSetCanaryScale(nil, intPnt(10), false),
trafficRouting: &v1alpha1.RolloutTrafficRouting{},
},
{
name: "Ignore setCanaryScale replicas/weight when matchTrafficWeight is true",
rolloutSpecReplicas: 10,
Expand Down Expand Up @@ -678,6 +706,9 @@ func TestCalculateReplicaCountsForCanary(t *testing.T) {
for i := range tests {
test := tests[i]
t.Run(test.name, func(t *testing.T) {
if test.setCanaryScale != nil {
test.setCanaryScale.MinReplicas = test.minReplicas
}
rollout := newRollout(test.rolloutSpecReplicas, test.setWeight, test.maxSurge, test.maxUnavailable, "canary", "stable", test.setCanaryScale, test.trafficRouting)
rollout.Status.PromoteFull = test.promoteFull
rollout.Status.Abort = test.statusAbort
Expand Down Expand Up @@ -869,6 +900,44 @@ func TestCalculateReplicaCountsForCanaryTrafficRoutingDynamicScale(t *testing.T)
assert.Equal(t, int32(1), newRSReplicaCount)
assert.Equal(t, int32(10), stableRSReplicaCount)
}
{
// verify scaledown of stable without minReplicas
rollout := newRollout(10, 90, intstr.FromInt(0), intstr.FromInt(1), "canary", "stable", nil, nil)
rollout.Spec.Strategy.Canary.TrafficRouting = &v1alpha1.RolloutTrafficRouting{}
rollout.Spec.Strategy.Canary.DynamicStableScale = true
weights := v1alpha1.TrafficWeights{
Canary: v1alpha1.WeightDestination{
Weight: 90,
},
Stable: v1alpha1.WeightDestination{
Weight: 10,
},
}
newRSReplicaCount, stableRSReplicaCount := CalculateReplicaCountsForTrafficRoutedCanary(rollout, &weights)
assert.Equal(t, int32(9), newRSReplicaCount)
assert.Equal(t, int32(1), stableRSReplicaCount)
}
{
// verify scaledown of stable with` minReplicas
weight := int32(90)
minReplicas := int32(2)
scs := newSetCanaryScale(nil, &weight, false)
scs.MinReplicas = &minReplicas
rollout := newRollout(10, 90, intstr.FromInt(0), intstr.FromInt(1), "canary", "stable", scs, nil)
rollout.Spec.Strategy.Canary.TrafficRouting = &v1alpha1.RolloutTrafficRouting{}
rollout.Spec.Strategy.Canary.DynamicStableScale = true
weights := v1alpha1.TrafficWeights{
Canary: v1alpha1.WeightDestination{
Weight: 90,
},
Stable: v1alpha1.WeightDestination{
Weight: 10,
},
}
newRSReplicaCount, stableRSReplicaCount := CalculateReplicaCountsForTrafficRoutedCanary(rollout, &weights)
assert.Equal(t, int32(9), newRSReplicaCount)
assert.Equal(t, int32(2), stableRSReplicaCount)
}
}

func TestCalculateReplicaCountsForCanaryStableRSdEdgeCases(t *testing.T) {
Expand Down
4 changes: 3 additions & 1 deletion utils/rollout/rolloututil.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,9 @@ func CanaryStepString(c v1alpha1.CanaryStep) string {
return "analysis"
}
if c.SetCanaryScale != nil {
if c.SetCanaryScale.Weight != nil {
if c.SetCanaryScale.Weight != nil && c.SetCanaryScale.MinReplicas != nil {
return fmt.Sprintf("setCanaryScale{weight: %d, min %d}", *c.SetCanaryScale.Weight, *c.SetCanaryScale.MinReplicas)
} else if c.SetCanaryScale.Weight != nil {
return fmt.Sprintf("setCanaryScale{weight: %d}", *c.SetCanaryScale.Weight)
} else if c.SetCanaryScale.MatchTrafficWeight {
return "setCanaryScale{matchTrafficWeight: true}"
Expand Down

0 comments on commit 93c7595

Please sign in to comment.