diff --git a/pkg/apis/rollouts/v1alpha1/types.go b/pkg/apis/rollouts/v1alpha1/types.go index 9ab7809624..f24e522fc6 100644 --- a/pkg/apis/rollouts/v1alpha1/types.go +++ b/pkg/apis/rollouts/v1alpha1/types.go @@ -652,6 +652,8 @@ const ( RolloutReplicaFailure RolloutConditionType = "ReplicaFailure" // RolloutPaused means that rollout is in a paused state. It is still progressing at this point. RolloutPaused RolloutConditionType = "Paused" + // RolloutCompleted means that rollout is in a completed state. It is still progressing at this point. + RolloutCompleted RolloutConditionType = "Completed" ) // RolloutCondition describes the state of a rollout at a certain point. diff --git a/rollout/analysis_test.go b/rollout/analysis_test.go index a09c89df1e..ca5d49ff54 100644 --- a/rollout/analysis_test.go +++ b/rollout/analysis_test.go @@ -1628,7 +1628,7 @@ func TestDoNotCreatePrePromotionAnalysisAfterPromotionRollout(t *testing.T) { f.run(getKey(r2, t)) - newConditions := generateConditionsPatch(true, conditions.NewRSAvailableReason, rs2, true, "") + newConditions := generateConditionsPatchWithComplete(true, conditions.NewRSAvailableReason, rs2, true, "", true) expectedPatch := fmt.Sprintf(`{ "status":{ "conditions":%s diff --git a/rollout/bluegreen_test.go b/rollout/bluegreen_test.go index 894a951556..7357506e0f 100644 --- a/rollout/bluegreen_test.go +++ b/rollout/bluegreen_test.go @@ -1,6 +1,7 @@ package rollout import ( + "encoding/json" "fmt" "strconv" "testing" @@ -1012,7 +1013,7 @@ func TestBlueGreenRolloutCompleted(t *testing.T) { f.run(getKey(r2, t)) - newConditions := generateConditionsPatch(true, conditions.NewRSAvailableReason, rs2, true, "") + newConditions := generateConditionsPatchWithComplete(true, conditions.NewRSAvailableReason, rs2, true, "", true) expectedPatch := fmt.Sprintf(`{ "status":{ "conditions":%s @@ -1022,6 +1023,49 @@ func TestBlueGreenRolloutCompleted(t *testing.T) { assert.Equal(t, cleanPatch(expectedPatch), patch) } +func TestBlueGreenRolloutCompletedFalse(t *testing.T) { + f := newFixture(t) + defer f.Close() + + r1 := newBlueGreenRollout("foo", 1, nil, "bar", "") + completedCondition, _ := newCompletedCondition(true) + conditions.SetRolloutCondition(&r1.Status, completedCondition) + + r2 := bumpVersion(r1) + rs2 := newReplicaSetWithStatus(r2, 1, 1) + progressingCondition, _ := newProgressingCondition(conditions.PausedRolloutReason, rs2, "") + conditions.SetRolloutCondition(&r2.Status, progressingCondition) + pausedCondition, _ := newPausedCondition(true) + conditions.SetRolloutCondition(&r2.Status, pausedCondition) + + f.kubeobjects = append(f.kubeobjects, rs2) + f.replicaSetLister = append(f.replicaSetLister, rs2) + rs2PodHash := rs2.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] + + serviceSelector := map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs2PodHash} + s := newService("bar", 80, serviceSelector, r2) + f.kubeobjects = append(f.kubeobjects, s) + + r2 = updateBlueGreenRolloutStatus(r2, "", rs2PodHash, rs2PodHash, 1, 1, 1, 1, true, false) + r2.Status.ObservedGeneration = strconv.Itoa(int(r2.Generation)) + + f.rolloutLister = append(f.rolloutLister, r2) + f.objects = append(f.objects, r2) + f.serviceLister = append(f.serviceLister, s) + + patchIndex := f.expectPatchRolloutAction(r1) + f.run(getKey(r2, t)) + + patch := f.getPatchedRollout(patchIndex) + rolloutPatch := v1alpha1.Rollout{} + err := json.Unmarshal([]byte(patch), &rolloutPatch) + assert.NoError(t, err) + + index := len(rolloutPatch.Status.Conditions) - 1 + assert.Equal(t, v1alpha1.RolloutCompleted, rolloutPatch.Status.Conditions[index].Type) + assert.Equal(t, corev1.ConditionFalse, rolloutPatch.Status.Conditions[index].Status) +} + func TestBlueGreenUnableToReadScaleDownAt(t *testing.T) { f := newFixture(t) defer f.Close() diff --git a/rollout/controller_test.go b/rollout/controller_test.go index c3c7157db0..315e6b3919 100644 --- a/rollout/controller_test.go +++ b/rollout/controller_test.go @@ -185,6 +185,26 @@ func newPausedCondition(isPaused bool) (v1alpha1.RolloutCondition, string) { return condition, string(conditionBytes) } +func newCompletedCondition(isCompleted bool) (v1alpha1.RolloutCondition, string) { + status := corev1.ConditionTrue + if !isCompleted { + status = corev1.ConditionFalse + } + condition := v1alpha1.RolloutCondition{ + LastTransitionTime: metav1.Now(), + LastUpdateTime: metav1.Now(), + Message: conditions.RolloutCompletedReason, + Reason: conditions.RolloutCompletedReason, + Status: status, + Type: v1alpha1.RolloutCompleted, + } + conditionBytes, err := json.Marshal(condition) + if err != nil { + panic(err) + } + return condition, string(conditionBytes) +} + func newProgressingCondition(reason string, resourceObj runtime.Object, optionalMessage string) (v1alpha1.RolloutCondition, string) { status := corev1.ConditionTrue msg := "" @@ -307,6 +327,16 @@ func generateConditionsPatchWithPause(available bool, progressingReason string, return fmt.Sprintf("[%s, %s, %s]", progressingCondition, pauseCondition, availableCondition) } +func generateConditionsPatchWithComplete(available bool, progressingReason string, progressingResource runtime.Object, availableConditionFirst bool, progressingMessage string, isCompleted bool) string { + _, availableCondition := newAvailableCondition(available) + _, progressingCondition := newProgressingCondition(progressingReason, progressingResource, progressingMessage) + _, completeCondition := newCompletedCondition(isCompleted) + if availableConditionFirst { + return fmt.Sprintf("[%s, %s, %s]", availableCondition, completeCondition, progressingCondition) + } + return fmt.Sprintf("[%s, %s, %s]", completeCondition, progressingCondition, availableCondition) +} + // func updateBlueGreenRolloutStatus(r *v1alpha1.Rollout, preview, active string, availableReplicas, updatedReplicas, hpaReplicas int32, pause bool, available bool, progressingStatus string) *v1alpha1.Rollout { func updateBlueGreenRolloutStatus(r *v1alpha1.Rollout, preview, active, stable string, availableReplicas, updatedReplicas, totalReplicas, hpaReplicas int32, pause bool, available bool) *v1alpha1.Rollout { newRollout := updateBaseRolloutStatus(r, availableReplicas, updatedReplicas, totalReplicas, hpaReplicas) diff --git a/rollout/sync.go b/rollout/sync.go index 0d6768e306..13fea3149e 100644 --- a/rollout/sync.go +++ b/rollout/sync.go @@ -563,7 +563,20 @@ func isIndefiniteStep(r *v1alpha1.Rollout) bool { } func (c *rolloutContext) calculateRolloutConditions(newStatus v1alpha1.RolloutStatus) v1alpha1.RolloutStatus { - if len(c.rollout.Status.PauseConditions) > 0 || c.rollout.Spec.Paused { + isPaused := len(c.rollout.Status.PauseConditions) > 0 || c.rollout.Spec.Paused + + completeCond := conditions.GetRolloutCondition(c.rollout.Status, v1alpha1.RolloutCompleted) + if !isPaused && conditions.RolloutComplete(c.rollout, &newStatus) { + updateCompletedCond := conditions.NewRolloutCondition(v1alpha1.RolloutCompleted, corev1.ConditionTrue, conditions.RolloutCompletedReason, conditions.RolloutCompletedReason) + conditions.SetRolloutCondition(&newStatus, *updateCompletedCond) + } else { + if completeCond != nil { + updateCompletedCond := conditions.NewRolloutCondition(v1alpha1.RolloutCompleted, corev1.ConditionFalse, conditions.RolloutCompletedReason, conditions.RolloutCompletedReason) + conditions.SetRolloutCondition(&newStatus, *updateCompletedCond) + } + } + + if isPaused { return newStatus } @@ -571,6 +584,7 @@ func (c *rolloutContext) calculateRolloutConditions(newStatus v1alpha1.RolloutSt // a new rollout and this is a resync where we don't need to estimate any progress. // In such a case, we should simply not estimate any progress for this rollout. currentCond := conditions.GetRolloutCondition(c.rollout.Status, v1alpha1.RolloutProgressing) + isCompleteRollout := newStatus.Replicas == newStatus.AvailableReplicas && currentCond != nil && currentCond.Reason == conditions.NewRSAvailableReason // Check for progress only if the latest rollout hasn't completed yet. if !isCompleteRollout { @@ -590,8 +604,8 @@ func (c *rolloutContext) calculateRolloutConditions(newStatus v1alpha1.RolloutSt if c.newRS != nil { msg = fmt.Sprintf(conditions.ReplicaSetCompletedMessage, c.newRS.Name) } - condition := conditions.NewRolloutCondition(v1alpha1.RolloutProgressing, corev1.ConditionTrue, conditions.NewRSAvailableReason, msg) - conditions.SetRolloutCondition(&newStatus, *condition) + progressingCondition := conditions.NewRolloutCondition(v1alpha1.RolloutProgressing, corev1.ConditionTrue, conditions.NewRSAvailableReason, msg) + conditions.SetRolloutCondition(&newStatus, *progressingCondition) case conditions.RolloutProgressing(c.rollout, &newStatus): // If there is any progress made, continue by not checking if the rollout failed. This // behavior emulates the rolling updater progressDeadline check. diff --git a/test/e2e/functional_test.go b/test/e2e/functional_test.go index 4bab71b3d6..81ded0cbbf 100644 --- a/test/e2e/functional_test.go +++ b/test/e2e/functional_test.go @@ -904,3 +904,64 @@ spec: }). ExpectActiveRevision("2") } + +func (s *FunctionalSuite) TestKubectlWaitForCompleted() { + s.Given(). + RolloutObjects(` +kind: Service +apiVersion: v1 +metadata: + name: rollout-bluegreen-active +spec: + selector: + app: rollout-bluegreen + ports: + - protocol: TCP + port: 80 + targetPort: 8080 +--- +apiVersion: argoproj.io/v1alpha1 +kind: Rollout +metadata: + name: rollout-bluegreen +spec: + replicas: 1 + revisionHistoryLimit: 2 + selector: + matchLabels: + app: rollout-bluegreen + template: + metadata: + labels: + app: rollout-bluegreen + spec: + containers: + - name: rollouts-demo + image: argoproj/rollouts-demo:blue + imagePullPolicy: Always + ports: + - containerPort: 8080 + strategy: + blueGreen: + activeService: rollout-bluegreen-active + autoPromotionEnabled: true +`). + When(). + ApplyManifests(). + WaitForRolloutReplicas(1). + WaitForRolloutStatus("Healthy"). + UpdateSpec(). + Then(). + ExpectRollout("Completed", func(r *v1alpha1.Rollout) bool { + cmd := exec.Command("kubectl", "wait", "--for=condition=Completed=False", fmt.Sprintf("rollout/%s", r.Name)) + return cmd.Run() == nil + }). + When(). + PromoteRollout(). + Then(). + ExpectRollout("Completed", func(r *v1alpha1.Rollout) bool { + cmd := exec.Command("kubectl", "wait", "--for=condition=Completed=True", fmt.Sprintf("rollout/%s", r.Name)) + return cmd.Run() == nil + }). + ExpectActiveRevision("2") +} diff --git a/utils/conditions/conditions.go b/utils/conditions/conditions.go index 871ab9f866..4ac20c9e52 100644 --- a/utils/conditions/conditions.go +++ b/utils/conditions/conditions.go @@ -87,6 +87,8 @@ const ( // within the given deadline (progressDeadlineSeconds). ReplicaSetTimeOutMessage = "ReplicaSet %q has timed out progressing." + // RolloutCompletedReason is added in a rollout when it is completed. + RolloutCompletedReason = "RolloutCompleted" // RolloutCompletedMessage is added when the rollout is completed RolloutCompletedMessage = "Rollout %q has successfully progressed." // ReplicaSetCompletedMessage is added when the rollout is completed