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

feat: Implement Issue #1779: add rollout.Spec.Strategy.Canary.MinPodsPerReplicaSet #2448

Merged
merged 11 commits into from
Dec 19, 2022
5 changes: 5 additions & 0 deletions docs/features/specification.md
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,11 @@ spec:
# traffic routing.
scaleDownDelaySeconds: 30

# The minimum number of pods that will be requested for each ReplicaSet
# when using traffic routed canary. This is to ensure high availability
# of each ReplicaSet. Defaults to 1. +optional
minPodsPerReplicaSet: 2

# Limits the number of old RS that can run at one time before getting
# scaled down. Defaults to nil
scaleDownDelayRevisionLimit: 2
Expand Down
3 changes: 3 additions & 0 deletions manifests/crds/rollout-crd.yaml
100644 → 100755
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,9 @@ spec:
- type: integer
- type: string
x-kubernetes-int-or-string: true
minPodsPerReplicaSet:
format: int32
type: integer
pingPong:
properties:
pingService:
Expand Down
3 changes: 3 additions & 0 deletions manifests/install.yaml
100644 → 100755
Original file line number Diff line number Diff line change
Expand Up @@ -11403,6 +11403,9 @@ spec:
- type: integer
- type: string
x-kubernetes-int-or-string: true
minPodsPerReplicaSet:
format: int32
type: integer
pingPong:
properties:
pingService:
Expand Down
3 changes: 3 additions & 0 deletions manifests/namespace-install.yaml
100644 → 100755
Original file line number Diff line number Diff line change
Expand Up @@ -11403,6 +11403,9 @@ spec:
- type: integer
- type: string
x-kubernetes-int-or-string: true
minPodsPerReplicaSet:
format: int32
type: integer
pingPong:
properties:
pingService:
Expand Down
5 changes: 5 additions & 0 deletions pkg/apiclient/rollout/rollout.swagger.json
100644 → 100755
Original file line number Diff line number Diff line change
Expand Up @@ -927,6 +927,11 @@
"pingPong": {
"$ref": "#/definitions/github.com.argoproj.argo_rollouts.pkg.apis.rollouts.v1alpha1.PingPongSpec",
"title": "PingPongSpec holds the ping and pong services"
},
"minPodsPerReplicaSet": {
"type": "integer",
"format": "int32",
"title": "Assuming the desired number of pods in a stable or canary ReplicaSet is not zero, then make sure it is at least\nMinPodsPerReplicaSet for High Availability. Only applicable for TrafficRoutedCanary"
}
},
"title": "CanaryStrategy defines parameters for a Replica Based Canary"
Expand Down
1,004 changes: 518 additions & 486 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
100644 → 100755
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,9 @@ type CanaryStrategy struct {
DynamicStableScale bool `json:"dynamicStableScale,omitempty" protobuf:"varint,14,opt,name=dynamicStableScale"`
// PingPongSpec holds the ping and pong services
PingPong *PingPongSpec `json:"pingPong,omitempty" protobuf:"varint,15,opt,name=pingPong"`
// Assuming the desired number of pods in a stable or canary ReplicaSet is not zero, then make sure it is at least
// MinPodsPerReplicaSet for High Availability. Only applicable for TrafficRoutedCanary
MinPodsPerReplicaSet *int32 `json:"minPodsPerReplicaSet,omitempty" protobuf:"varint,16,opt,name=minPodsPerReplicaSet"`
}

// PingPongSpec holds the ping and pong service name.
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.

6 changes: 6 additions & 0 deletions ui/src/models/rollout/generated/api.ts
100644 → 100755
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,12 @@ export interface GithubComArgoprojArgoRolloutsPkgApisRolloutsV1alpha1CanaryStrat
* @memberof GithubComArgoprojArgoRolloutsPkgApisRolloutsV1alpha1CanaryStrategy
*/
pingPong?: GithubComArgoprojArgoRolloutsPkgApisRolloutsV1alpha1PingPongSpec;
/**
*
* @type {number}
* @memberof GithubComArgoprojArgoRolloutsPkgApisRolloutsV1alpha1CanaryStrategy
*/
minPodsPerReplicaSet?: number;
}
/**
* DryRun defines the settings for running the analysis in Dry-Run mode.
Expand Down
17 changes: 15 additions & 2 deletions utils/replicaset/canary.go
100644 → 100755
Original file line number Diff line number Diff line change
Expand Up @@ -312,9 +312,22 @@ func maxValue(countA int32, countB int32) int32 {
return countA
}

// CheckMinPodsPerReplicaSet ensures that if the desired number of pods in a stable or canary ReplicaSet is not zero,
// then it is at least MinPodsPerReplicaSet for High Availability. Only applicable if using TrafficRouting
func CheckMinPodsPerReplicaSet(rollout *v1alpha1.Rollout, count int32) int32 {
if count == 0 {
return count
}
if rollout.Spec.Strategy.Canary == nil || rollout.Spec.Strategy.Canary.MinPodsPerReplicaSet == nil || rollout.Spec.Strategy.Canary.TrafficRouting == nil {
return count
}
return max(count, *rollout.Spec.Strategy.Canary.MinPodsPerReplicaSet)
}

// CalculateReplicaCountsForTrafficRoutedCanary calculates the canary and stable replica counts
// when using canary with traffic routing. If current traffic weights are supplied, we factor the
// those weights into the and return the higher of current traffic scale vs. desired traffic scale
// If MinPodsPerReplicaSet is defined and the number of replicas in either RS is not 0, then return at least MinPodsPerReplicaSet
func CalculateReplicaCountsForTrafficRoutedCanary(rollout *v1alpha1.Rollout, weights *v1alpha1.TrafficWeights) (int32, int32) {
var canaryCount, stableCount int32
rolloutSpecReplica := defaults.GetReplicasOrDefault(rollout.Spec.Replicas)
Expand All @@ -323,7 +336,7 @@ func CalculateReplicaCountsForTrafficRoutedCanary(rollout *v1alpha1.Rollout, wei
// a canary count was explicitly set
canaryCount = *setCanaryScaleReplicas
} else {
canaryCount = trafficWeightToReplicas(rolloutSpecReplica, desiredWeight)
canaryCount = CheckMinPodsPerReplicaSet(rollout, trafficWeightToReplicas(rolloutSpecReplica, desiredWeight))
}

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

// trafficWeightToReplicas returns the appropriate replicas given the full spec.replicas and a weight
Expand Down
21 changes: 21 additions & 0 deletions utils/replicaset/canary_test.go
100644 → 100755
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
minPodsPerReplicaSet *int32
}{
{
name: "Do not add extra RSs in scaleDownCount when .Spec.Replica < AvailableReplicas",
Expand Down Expand Up @@ -674,6 +675,23 @@ func TestCalculateReplicaCountsForCanary(t *testing.T) {
expectedStableReplicaCount: 1,
expectedCanaryReplicaCount: 0,
},
{
name: "Honor MinPodsPerReplicaSet when using trafficRouting and starting canary",
rolloutSpecReplicas: 10,
setWeight: 5,

stableSpecReplica: 10,
stableAvailableReplica: 10,

canarySpecReplica: 0,
canaryAvailableReplica: 0,

trafficRouting: &v1alpha1.RolloutTrafficRouting{},
minPodsPerReplicaSet: intPnt(2),

expectedStableReplicaCount: 10,
expectedCanaryReplicaCount: 2,
},
}
for i := range tests {
test := tests[i]
Expand All @@ -684,6 +702,9 @@ func TestCalculateReplicaCountsForCanary(t *testing.T) {
stableRS := newRS("stable", test.stableSpecReplica, test.stableAvailableReplica)
canaryRS := newRS("canary", test.canarySpecReplica, test.canaryAvailableReplica)
rollout.Spec.Strategy.Canary.AbortScaleDownDelaySeconds = test.abortScaleDownDelaySeconds
if test.minPodsPerReplicaSet != nil {
rollout.Spec.Strategy.Canary.MinPodsPerReplicaSet = test.minPodsPerReplicaSet
}
var newRSReplicaCount, stableRSReplicaCount int32
if test.trafficRouting != nil {
newRSReplicaCount, stableRSReplicaCount = CalculateReplicaCountsForTrafficRoutedCanary(rollout, nil)
Expand Down