Skip to content

Commit

Permalink
feat: scaledown rs when update exceeds progressDeadlineSeconds
Browse files Browse the repository at this point in the history
- Add AbortExceedProgressDeadline to rollout spec
- when rollout doesn't use analysis, set AbortExceedProgressDeadline
  to true to scale down the new RS.
- e2e test added

Signed-off-by: Hui Kang <hui.kang@salesforce.com>
  • Loading branch information
Hui Kang committed Aug 5, 2021
1 parent ffe70da commit a17b4e4
Show file tree
Hide file tree
Showing 11 changed files with 115 additions and 0 deletions.
4 changes: 4 additions & 0 deletions docs/features/specification.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ spec:
# Defaults to 600s
progressDeadlineSeconds: 600

# Whether to abort the update when ProgressDeadlineSeconds
# is exceeded if analysis is not used. Optional and default is false.
AbortExceedProgressDeadline: false

# UTC timestamp in which a Rollout should sequentially restart all of
# its pods. Used by the `kubectl argo rollouts restart ROLLOUT` command.
# The controller will ensure all pods have a creationTimestamp greater
Expand Down
2 changes: 2 additions & 0 deletions manifests/crds/rollout-crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ spec:
type: object
spec:
properties:
abortExceedProgressDeadline:
type: boolean
analysis:
properties:
successfulRunHistoryLimit:
Expand Down
2 changes: 2 additions & 0 deletions manifests/install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9745,6 +9745,8 @@ spec:
type: object
spec:
properties:
abortExceedProgressDeadline:
type: boolean
analysis:
properties:
successfulRunHistoryLimit:
Expand Down
2 changes: 2 additions & 0 deletions manifests/namespace-install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9745,6 +9745,8 @@ spec:
type: object
spec:
properties:
abortExceedProgressDeadline:
type: boolean
analysis:
properties:
successfulRunHistoryLimit:
Expand Down
4 changes: 4 additions & 0 deletions pkg/apiclient/rollout/rollout.swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -1178,6 +1178,10 @@
"format": "int32",
"description": "ProgressDeadlineSeconds The maximum time in seconds for a rollout to\nmake progress before it is considered to be failed. Argo Rollouts will\ncontinue to process failed rollouts and a condition with a\nProgressDeadlineExceeded reason will be surfaced in the rollout status.\nNote that progress will not be estimated during the time a rollout is paused.\nDefaults to 600s."
},
"abortExceedProgressDeadline": {
"type": "boolean",
"title": "AbortExceedProgressDeadline to whether abort the update when ProgressDeadlineSeconds\nis exceeded if analysis is not used. Default is false.\n+optional"
},
"restartAt": {
"$ref": "#/definitions/k8s.io.apimachinery.pkg.apis.meta.v1.Time",
"title": "RestartAt indicates when all the pods of a Rollout should be restarted"
Expand Down
4 changes: 4 additions & 0 deletions pkg/apis/rollouts/v1alpha1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,10 @@ type RolloutSpec struct {
// Note that progress will not be estimated during the time a rollout is paused.
// Defaults to 600s.
ProgressDeadlineSeconds *int32 `json:"progressDeadlineSeconds,omitempty" protobuf:"varint,8,opt,name=progressDeadlineSeconds"`
// AbortExceedProgressDeadline to whether abort the update when ProgressDeadlineSeconds
// is exceeded if analysis is not used. Default is false.
// +optional
AbortExceedProgressDeadline *bool `json:"abortExceedProgressDeadline,omitempty" protobuf:"varint,12,opt,name=abortExceedProgressDeadline"`
// RestartAt indicates when all the pods of a Rollout should be restarted
RestartAt *metav1.Time `json:"restartAt,omitempty" protobuf:"bytes,9,opt,name=restartAt"`
// Analysis configuration for the analysis runs to retain
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.

5 changes: 5 additions & 0 deletions rollout/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,11 @@ func (c *rolloutContext) calculateRolloutConditions(newStatus v1alpha1.RolloutSt
// Check for progress. Only do this if the latest rollout hasn't completed yet and it is not aborted
if !isCompleteRollout && !isAborted {
switch {
case conditions.RolloutTimedOut(c.rollout, &newStatus) && defaults.GetAbortExceedProgressDeadlineOrDefault(c.rollout):
msg := fmt.Sprintf(conditions.RolloutTimeOutMessage, c.rollout.Name)
c.pauseContext.AddAbort(msg)
abortCondition := conditions.NewRolloutCondition(v1alpha1.RolloutPaused, corev1.ConditionTrue, conditions.TimedOutReason, msg)
conditions.SetRolloutCondition(&newStatus, *abortCondition)
case conditions.RolloutComplete(c.rollout, &newStatus):
// Update the rollout conditions with a message for the new replica set that
// was successfully deployed. If the condition already exists, we ignore this update.
Expand Down
65 changes: 65 additions & 0 deletions test/e2e/functional_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -885,6 +885,71 @@ spec:
ExpectReplicaCounts(1, 2, 1, 1, 1)
}

// TestBlueGreenAbortExceedProgressDeadline verifies the AbortExceedProgressDeadline feature
func (s *FunctionalSuite) TestBlueGreenAbortExceedProgressDeadline() {
s.Given().
RolloutObjects(newService("bluegreen-scaledowndelay-active")).
RolloutObjects(`
apiVersion: argoproj.io/v1alpha1
kind: Rollout
metadata:
name: bluegreen-scaledowndelay
spec:
replicas: 1
strategy:
blueGreen:
activeService: bluegreen-scaledowndelay-active
abortScaleDownDelaySeconds: 2
selector:
matchLabels:
app: bluegreen-scaledowndelay
template:
metadata:
labels:
app: bluegreen-scaledowndelay
spec:
containers:
- name: bluegreen-scaledowndelay
image: nginx:1.19-alpine
resources:
requests:
memory: 16Mi
cpu: 1m
`).
When().
ApplyManifests().
WaitForRolloutStatus("Healthy").
PatchSpec(`
spec:
abortExceedProgressDeadline: false
progressDeadlineSeconds: 1
template:
spec:
containers:
- name: bad2good
image: nginx:1.19-alpine-argo-error
command: null`).
WaitForRolloutStatus("Degraded").
Sleep(3*time.Second).
Then().
ExpectRevisionPodCount("2", 1).
When().
PatchSpec(`
spec:
abortExceedProgressDeadline: true
progressDeadlineSeconds: 1
template:
spec:
containers:
- name: bad2good
image: nginx:1.19-alpine-argo-error
command: null`).
WaitForRolloutStatus("Degraded").
Sleep(3*time.Second).
Then().
ExpectRevisionPodCount("2", 0)
}

// TestBlueGreenScaleDownOnAbort verifies the scaleDownOnAbort feature
func (s *FunctionalSuite) TestBlueGreenScaleDownOnAbort() {
s.Given().
Expand Down
9 changes: 9 additions & 0 deletions utils/defaults/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ const (
DefaultMaxUnavailable = "25"
// DefaultProgressDeadlineSeconds default number of seconds for the rollout to be making progress
DefaultProgressDeadlineSeconds = int32(600)
// DefaultAbortExceedProgressDeadline
DefaultAbortExceedProgressDeadline = false
// DefaultScaleDownDelaySeconds default seconds before scaling down old replicaset after switching services
DefaultScaleDownDelaySeconds = int32(30)
// DefaultAbortScaleDownDelaySeconds default seconds before scaling down old replicaset after switching services
Expand Down Expand Up @@ -116,6 +118,13 @@ func GetExperimentProgressDeadlineSecondsOrDefault(e *v1alpha1.Experiment) int32
return DefaultProgressDeadlineSeconds
}

func GetAbortExceedProgressDeadlineOrDefault(rollout *v1alpha1.Rollout) bool {
if rollout.Spec.AbortExceedProgressDeadline != nil {
return *rollout.Spec.AbortExceedProgressDeadline
}
return DefaultAbortExceedProgressDeadline
}

func GetExperimentScaleDownDelaySecondsOrDefault(e *v1alpha1.Experiment) int32 {
if e.Spec.ScaleDownDelaySeconds != nil {
return *e.Spec.ScaleDownDelaySeconds
Expand Down
13 changes: 13 additions & 0 deletions utils/defaults/defaults_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,19 @@ func TestGetProgressDeadlineSecondsOrDefault(t *testing.T) {
assert.Equal(t, DefaultProgressDeadlineSeconds, GetProgressDeadlineSecondsOrDefault(rolloutDefaultValue))
}

func TestGetAbortExceedProgressDeadlineOrDefault(t *testing.T) {
abort := true
rolloutNonDefaultValue := &v1alpha1.Rollout{
Spec: v1alpha1.RolloutSpec{
AbortExceedProgressDeadline: &abort,
},
}

assert.Equal(t, abort, GetAbortExceedProgressDeadlineOrDefault(rolloutNonDefaultValue))
rolloutDefaultValue := &v1alpha1.Rollout{}
assert.Equal(t, DefaultAbortExceedProgressDeadline, GetAbortExceedProgressDeadlineOrDefault(rolloutDefaultValue))
}

func TestGetScaleDownDelaySecondsOrDefault(t *testing.T) {
{
scaleDownDelaySeconds := int32(60)
Expand Down

0 comments on commit a17b4e4

Please sign in to comment.