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
3 changes: 3 additions & 0 deletions manifests/crds/rollout-crd.yaml
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
minPodsPerRS:
format: int32
type: integer
pingPong:
properties:
pingService:
Expand Down
3 changes: 3 additions & 0 deletions manifests/install.yaml
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
minPodsPerRS:
format: int32
type: integer
pingPong:
properties:
pingService:
Expand Down
3 changes: 3 additions & 0 deletions manifests/namespace-install.yaml
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
minPodsPerRS:
format: int32
type: integer
pingPong:
properties:
pingService:
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 @@ -900,6 +900,11 @@
"pingPong": {
"$ref": "#/definitions/github.com.argoproj.argo_rollouts.pkg.apis.rollouts.v1alpha1.PingPongSpec",
"title": "PingPongSpec holds the ping and pong services"
},
"minPodsPerRS": {
"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\nMinPodsPerRS for High Availability. Only applicable for TrafficRoutedCanary"
}
},
"title": "CanaryStrategy defines parameters for a Replica Based Canary"
Expand Down
991 changes: 512 additions & 479 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 @@ -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
// MinPodsPerRS for High Availability. Only applicable for TrafficRoutedCanary
MinPodsPerRS *int32 `json:"minPodsPerRS,omitempty" protobuf:"varint,16,opt,name=minPodsPerRS"`
ssanders1449 marked this conversation as resolved.
Show resolved Hide resolved
}

// 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
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,12 @@ export interface GithubComArgoprojArgoRolloutsPkgApisRolloutsV1alpha1CanaryStrat
* @memberof GithubComArgoprojArgoRolloutsPkgApisRolloutsV1alpha1CanaryStrategy
*/
pingPong?: GithubComArgoprojArgoRolloutsPkgApisRolloutsV1alpha1PingPongSpec;
/**
*
* @type {number}
* @memberof GithubComArgoprojArgoRolloutsPkgApisRolloutsV1alpha1CanaryStrategy
*/
minPodsPerRS?: 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
}

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

// 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 MinPodsPerRS is defined and the number of replicas in either RS is not 0, then return at least MinPodsPerRS
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 = CheckMinPodsPerRS(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 CheckMinPodsPerRS(rollout, canaryCount), CheckMinPodsPerRS(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
minPodsPerRS *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 MinPodsPerRS when using trafficRouting and starting canary",
rolloutSpecReplicas: 10,
setWeight: 5,

stableSpecReplica: 10,
stableAvailableReplica: 10,

canarySpecReplica: 0,
canaryAvailableReplica: 0,

trafficRouting: &v1alpha1.RolloutTrafficRouting{},
minPodsPerRS: 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.minPodsPerRS != nil {
rollout.Spec.Strategy.Canary.MinPodsPerRS = test.minPodsPerRS
}
var newRSReplicaCount, stableRSReplicaCount int32
if test.trafficRouting != nil {
newRSReplicaCount, stableRSReplicaCount = CalculateReplicaCountsForTrafficRoutedCanary(rollout, nil)
Expand Down