Skip to content

Commit

Permalink
Address PR
Browse files Browse the repository at this point in the history
  • Loading branch information
dthomson25 committed Jul 31, 2020
1 parent 557a16e commit 531bc0c
Show file tree
Hide file tree
Showing 9 changed files with 47 additions and 27 deletions.
2 changes: 2 additions & 0 deletions manifests/crds/rollout-crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2909,6 +2909,8 @@ spec:
type: integer
abort:
type: boolean
abortedAt:
type: boolean
availableReplicas:
format: int32
type: integer
Expand Down
9 changes: 8 additions & 1 deletion pkg/apis/rollouts/v1alpha1/openapi_generated.go

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

14 changes: 7 additions & 7 deletions pkg/apis/rollouts/v1alpha1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -476,11 +476,11 @@ type RolloutStatus struct {
PauseConditions []PauseCondition `json:"pauseConditions,omitempty"`
//ControllerPause indicates the controller has paused the rollout
ControllerPause bool `json:"controllerPause,omitempty"`
// ReconciledAbort indicates the controller reconciled an aborted rollout. The controller uses this to understand if
// AbortedAt indicates the controller reconciled an aborted rollout. The controller uses this to understand if
// the controller needs to do some specific work when a Rollout is aborted. For example, the reconcileAbort is used
// to indicate if the Rollout should enter an aborted state when the latest AnalysisRun is a failure, or the controller
// has already put the Rollout into an aborted and should create a new AnalysisRun.
ReconciledAbort bool `json:"reconciledAbort,omitempty"`
AbortedAt *metav1.Time `json:"abortedAt,omitempty"`
// CurrentPodHash the hash of the current pod template
// +optional
CurrentPodHash string `json:"currentPodHash,omitempty"`
Expand Down Expand Up @@ -555,12 +555,12 @@ type BlueGreenStatus struct {
// +optional
ScaleUpPreviewCheckPoint bool `json:"scaleUpPreviewCheckPoint,omitempty"`
// PrePromotionAnalysisRun is the current analysis run running before the active service promotion
// TODO(Depreciated): Remove in v0.10
// TODO(Deprecated): Remove in v0.10
PrePromotionAnalysisRun string `json:"prePromotionAnalysisRun,omitempty"`
// PrePromotionAnalysisRunStatus indicates the status of the current prepromotion analysis run
PrePromotionAnalysisRunStatus *RolloutAnalysisRunStatus `json:"prePromotionAnalysisRunStatus,omitempty"`
// PostPromotionAnalysisRun is the current analysis run running after the active service promotion
// TODO(Depreciated): Remove in v0.10
// TODO(Deprecated): Remove in v0.10
PostPromotionAnalysisRun string `json:"postPromotionAnalysisRun,omitempty"`
// PostPromotionAnalysisRunStatus indicates the status of the current post promotion analysis run
PostPromotionAnalysisRunStatus *RolloutAnalysisRunStatus `json:"postPromotionAnalysisRunStatus,omitempty"`
Expand All @@ -572,14 +572,14 @@ type CanaryStatus struct {
// +optional
StableRS string `json:"stableRS,omitempty"`
// CurrentStepAnalysisRun indicates the analysisRun for the current step index
// TODO(Depreciated): Remove in v0.10
// TODO(Deprecated): Remove in v0.10
CurrentStepAnalysisRun string `json:"currentStepAnalysisRun,omitempty"`
// CurrentStepAnalysisRunStatus indicates the status of the current step analysis run
CurrentStepAnalysisRunStatus *RolloutAnalysisRunStatus `json:"currentStepAnalysisRunStatus,omitempty"`
// CurrentBackgroundAnalysisRun indicates the analysisRun for the Background step
// TODO(Depreciated): Remove in v0.10
// TODO(Deprecated): Remove in v0.10
CurrentBackgroundAnalysisRun string `json:"currentBackgroundAnalysisRun,omitempty"`
// CurrentStepAnalysisRunStatus indicates the status of the current background analysis run
// CurrentBackgroundAnalysisRunStatus indicates the status of the current background analysis run
CurrentBackgroundAnalysisRunStatus *RolloutAnalysisRunStatus `json:"currentBackgroundAnalysisRunStatus,omitempty"`
// CurrentExperiment indicates the running experiment
CurrentExperiment string `json:"currentExperiment,omitempty"`
Expand Down
2 changes: 1 addition & 1 deletion rollout/analysis.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func needsNewAnalysisRun(currentAr *v1alpha1.AnalysisRun, rollout *v1alpha1.Roll
if rollout.Status.ControllerPause && getPauseCondition(rollout, v1alpha1.PauseReasonBlueGreenPause) == nil {
return currentAr.Status.Phase == v1alpha1.AnalysisPhaseInconclusive
}
return rollout.Status.ReconciledAbort
return rollout.Status.AbortedAt != nil
}

// emitAnalysisRunStatusChanges emits a Kubernetes event if the analysis run of that type has changed status
Expand Down
25 changes: 15 additions & 10 deletions rollout/analysis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1194,12 +1194,13 @@ func TestErrorConditionAfterErrorAnalysisRunStep(t *testing.T) {
},
"conditions": %s,
"abort": true,
"reconciledAbort": true
"abortedAt": "%s"
}
}`
now := metav1.Now().UTC().Format(time.RFC3339)
condition := generateConditionsPatch(true, conditions.RolloutAbortedReason, r2, false, ar.Status.Message)

assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, ar.Name, condition)), patch)
assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, ar.Name, condition, now)), patch)
}

func TestErrorConditionAfterErrorAnalysisRunBackground(t *testing.T) {
Expand Down Expand Up @@ -1259,13 +1260,14 @@ func TestErrorConditionAfterErrorAnalysisRunBackground(t *testing.T) {
}
},
"conditions": %s,
"reconciledAbort": true,
"abortedAt": "%s",
"abort": true
}
}`
condition := generateConditionsPatch(true, conditions.RolloutAbortedReason, r2, false, "")

assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, condition)), patch)
now := metav1.Now().UTC().Format(time.RFC3339)
assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, condition, now)), patch)
}

func TestCancelAnalysisRunsWhenAborted(t *testing.T) {
Expand Down Expand Up @@ -1316,10 +1318,11 @@ func TestCancelAnalysisRunsWhenAborted(t *testing.T) {
expectedPatch := `{
"status": {
"conditions": %s,
"reconciledAbort": true
"abortedAt": "%s"
}
}`
assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, newConditions)), patch)
now := metav1.Now().UTC().Format(time.RFC3339)
assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, newConditions, now)), patch)
}

func TestCancelBackgroundAnalysisRunWhenRolloutIsCompleted(t *testing.T) {
Expand Down Expand Up @@ -1879,7 +1882,7 @@ func TestAbortRolloutOnErrorPrePromotionAnalysis(t *testing.T) {
expectedPatch := `{
"status": {
"abort": true,
"reconciledAbort": true,
"abortedAt": "%s",
"pauseConditions": null,
"controllerPause":null,
"blueGreen": {
Expand All @@ -1889,7 +1892,8 @@ func TestAbortRolloutOnErrorPrePromotionAnalysis(t *testing.T) {
}
}
}`
assert.Equal(t, calculatePatch(r2, expectedPatch), patch)
now := metav1.Now().UTC().Format(time.RFC3339)
assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, now)), patch)
}

func TestCreatePostPromotionAnalysisRun(t *testing.T) {
Expand Down Expand Up @@ -2096,7 +2100,7 @@ func TestAbortRolloutOnErrorPostPromotionAnalysis(t *testing.T) {
expectedPatch := `{
"status": {
"abort": true,
"reconciledAbort": true,
"abortedAt": "%s",
"pauseConditions": null,
"controllerPause":null,
"blueGreen": {
Expand All @@ -2106,5 +2110,6 @@ func TestAbortRolloutOnErrorPostPromotionAnalysis(t *testing.T) {
}
}
}`
assert.Equal(t, calculatePatch(r2, expectedPatch), patch)
now := metav1.Now().UTC().Format(time.RFC3339)
assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, now)), patch)
}
3 changes: 2 additions & 1 deletion rollout/bluegreen_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1196,7 +1196,8 @@ func TestBlueGreenAbort(t *testing.T) {
r1 := newBlueGreenRollout("foo", 1, nil, "bar", "")
r2 := bumpVersion(r1)
r2.Status.Abort = true
r2.Status.ReconciledAbort = true
now := metav1.Now()
r2.Status.AbortedAt = &now

rs1 := newReplicaSetWithStatus(r1, 1, 1)
rs2 := newReplicaSetWithStatus(r2, 1, 1)
Expand Down
6 changes: 4 additions & 2 deletions rollout/canary_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1258,7 +1258,8 @@ func TestHandleCanaryAbort(t *testing.T) {

r2 = updateCanaryRolloutStatus(r2, rs1PodHash, 10, 1, 10, false)
r2.Status.Abort = true
r2.Status.ReconciledAbort = true
now := metav1.Now()
r2.Status.AbortedAt = &now
f.rolloutLister = append(f.rolloutLister, r2)
f.objects = append(f.objects, r2)

Expand Down Expand Up @@ -1291,7 +1292,8 @@ func TestHandleCanaryAbort(t *testing.T) {
}
r1 := newCanaryRollout("foo", 2, nil, steps, int32Ptr(3), intstr.FromInt(1), intstr.FromInt(0))
r1.Status.Abort = true
r1.Status.ReconciledAbort = true
now := metav1.Now()
r1.Status.AbortedAt = &now
rs1 := newReplicaSetWithStatus(r1, 2, 2)
rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey]
r1 = updateCanaryRolloutStatus(r1, rs1PodHash, 2, 2, 2, false)
Expand Down
5 changes: 3 additions & 2 deletions rollout/experiment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -299,15 +299,16 @@ func TestAbortRolloutAfterFailedExperiment(t *testing.T) {
expectedPatch := `{
"status": {
"abort": true,
"reconciledAbort": true,
"abortedAt": "%s",
"conditions": %s,
"canary": {
"currentExperiment": null
}
}
}`
now := metav1.Now().UTC().Format(time.RFC3339)
generatedConditons := generateConditionsPatch(true, conditions.RolloutAbortedReason, r2, false, "")
assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, generatedConditons)), patch)
assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, now, generatedConditons)), patch)
}

func TestPauseRolloutAfterInconclusiveExperiment(t *testing.T) {
Expand Down
8 changes: 5 additions & 3 deletions rollout/pause.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,16 +59,18 @@ func (pCtx *pauseContext) ClearPauseConditions() {
func (pCtx *pauseContext) CalculatePauseStatus(newStatus *v1alpha1.RolloutStatus) {
if pCtx.addAbort {
newStatus.Abort = true
newStatus.ReconciledAbort = true
now := metav1.Now()
newStatus.AbortedAt = &now
return
}
if !pCtx.removeAbort && pCtx.rollout.Status.Abort {
newStatus.Abort = true
newStatus.ReconciledAbort = true
now := metav1.Now()
newStatus.AbortedAt = &now
return
}
newStatus.Abort = false
newStatus.ReconciledAbort = false
newStatus.AbortedAt = nil

if pCtx.clearPauseConditions {
return
Expand Down

0 comments on commit 531bc0c

Please sign in to comment.