Skip to content

Commit 85cdf40

Browse files
authored
fix: client can detect if rollout is in the process of unpausing (argoproj#1798)
Signed-off-by: Jesse Suen <[email protected]>
1 parent edfe42e commit 85cdf40

File tree

8 files changed

+48
-10
lines changed

8 files changed

+48
-10
lines changed

pkg/apiclient/rollout/rollout.swagger.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1300,7 +1300,7 @@
13001300
"items": {
13011301
"$ref": "#/definitions/github.com.argoproj.argo_rollouts.pkg.apis.rollouts.v1alpha1.PauseCondition"
13021302
},
1303-
"title": "PauseConditions indicates why the rollout is currently paused"
1303+
"title": "PauseConditions is a list of reasons why rollout became automatically paused (e.g.\nCanaryPauseStep, BlueGreenPause, InconclusiveAnalysis). The items in this list are populated\nby the controller but are cleared by the user (e.g. plugin, argo-cd resume action) when they\nwish to unpause. If pause conditions is empty, but controllerPause is true, it indicates\nthe user manually unpaused the Rollout"
13041304
},
13051305
"controllerPause": {
13061306
"type": "boolean",

pkg/apis/rollouts/v1alpha1/generated.proto

Lines changed: 5 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/apis/rollouts/v1alpha1/openapi_generated.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/apis/rollouts/v1alpha1/types.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -684,7 +684,11 @@ const (
684684
type RolloutStatus struct {
685685
// Abort cancel the current rollout progression
686686
Abort bool `json:"abort,omitempty" protobuf:"varint,1,opt,name=abort"`
687-
// PauseConditions indicates why the rollout is currently paused
687+
// PauseConditions is a list of reasons why rollout became automatically paused (e.g.
688+
// CanaryPauseStep, BlueGreenPause, InconclusiveAnalysis). The items in this list are populated
689+
// by the controller but are cleared by the user (e.g. plugin, argo-cd resume action) when they
690+
// wish to unpause. If pause conditions is empty, but controllerPause is true, it indicates
691+
// the user manually unpaused the Rollout
688692
PauseConditions []PauseCondition `json:"pauseConditions,omitempty" protobuf:"bytes,2,rep,name=pauseConditions"`
689693
// ControllerPause indicates the controller has paused the rollout. It is set to true when
690694
// the controller adds a pause condition. This field helps to discern the scenario where a

test/e2e/istio_test.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
//go:build e2e
12
// +build e2e
23

34
package e2e
@@ -292,10 +293,6 @@ func (s *IstioSuite) TestIstioAbortUpdateDeleteAllCanaryPods() {
292293
PromoteRollout().
293294
WaitForRolloutStatus("Paused").
294295
Then().
295-
When().
296-
PromoteRollout().
297-
WaitForRolloutStatus("Paused").
298-
Then().
299296
ExpectRevisionPodCount("2", 4).
300297
When().
301298
AbortRollout().

utils/replicaset/canary.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,7 @@ func GetCurrentCanaryStep(rollout *v1alpha1.Rollout) (*v1alpha1.CanaryStep, *int
436436

437437
// GetCanaryReplicasOrWeight either returns a static set of replicas or a weight percentage
438438
func GetCanaryReplicasOrWeight(rollout *v1alpha1.Rollout) (*int32, int32) {
439-
if rollout.Status.PromoteFull || rollout.Status.CurrentPodHash == rollout.Status.StableRS {
439+
if rollout.Status.PromoteFull || rollout.Status.StableRS == "" || rollout.Status.CurrentPodHash == rollout.Status.StableRS {
440440
return nil, 100
441441
}
442442
if scs := UseSetCanaryScale(rollout); scs != nil {

utils/rollout/rolloututil.go

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@ func GetRolloutPhase(ro *v1alpha1.Rollout) (v1alpha1.RolloutPhase, string) {
2323
if !isGenerationObserved(ro) {
2424
return v1alpha1.RolloutPhaseProgressing, "waiting for rollout spec update to be observed"
2525
}
26-
26+
if IsUnpausing(ro) {
27+
return v1alpha1.RolloutPhaseProgressing, "waiting for rollout to unpause"
28+
}
2729
if ro.Spec.TemplateResolvedFromRef && !isWorkloadGenerationObserved(ro) {
2830
return v1alpha1.RolloutPhaseProgressing, "waiting for rollout spec update to be observed for the reference workload"
2931
}
@@ -51,6 +53,17 @@ func isGenerationObserved(ro *v1alpha1.Rollout) bool {
5153
return int64(observedGen) == ro.Generation
5254
}
5355

56+
// IsUnpausing detects if we are in the process of unpausing a rollout. This is determined by seeing
57+
// if status.controllerPause is true, but the list of pause conditions (status.pauseConditions)
58+
// is empty. This implies that a user cleared the pause conditions but controller has not yet
59+
// observed or reacted to it.
60+
// NOTE: this function is necessary because unlike metadata.generation & status.observedGeneration
61+
// status.controllerPause & status.pauseConditions are both status fields and does not benefit from
62+
// the auto-incrementing behavior of metadata.generation.
63+
func IsUnpausing(ro *v1alpha1.Rollout) bool {
64+
return ro.Status.ControllerPause && len(ro.Status.PauseConditions) == 0
65+
}
66+
5467
func isWorkloadGenerationObserved(ro *v1alpha1.Rollout) bool {
5568
if _, ok := annotations.GetWorkloadGenerationAnnotation(ro); !ok {
5669
return true

utils/rollout/rolloututil_test.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,3 +394,23 @@ func TestCanaryStepString(t *testing.T) {
394394
assert.Equal(t, test.expectedString, CanaryStepString(test.step))
395395
}
396396
}
397+
398+
func TestIsUnpausing(t *testing.T) {
399+
ro := newCanaryRollout()
400+
ro.Status.Phase = v1alpha1.RolloutPhasePaused
401+
ro.Status.Message = "canary pause"
402+
ro.Status.PauseConditions = []v1alpha1.PauseCondition{
403+
{
404+
Reason: v1alpha1.PauseReasonCanaryPauseStep,
405+
},
406+
}
407+
ro.Status.ControllerPause = true
408+
status, message := GetRolloutPhase(ro)
409+
assert.Equal(t, v1alpha1.RolloutPhasePaused, status)
410+
assert.Equal(t, "canary pause", message)
411+
412+
ro.Status.PauseConditions = nil
413+
status, message = GetRolloutPhase(ro)
414+
assert.Equal(t, v1alpha1.RolloutPhaseProgressing, status)
415+
assert.Equal(t, "waiting for rollout to unpause", message)
416+
}

0 commit comments

Comments
 (0)