diff --git a/pkg/apis/rollouts/v1alpha1/types.go b/pkg/apis/rollouts/v1alpha1/types.go index 826ded984b..3eacc4f830 100644 --- a/pkg/apis/rollouts/v1alpha1/types.go +++ b/pkg/apis/rollouts/v1alpha1/types.go @@ -987,10 +987,12 @@ 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 indicates that the rollout completed its update to the desired revision and is not in the middle + // of any update. Note that a Completed rollout could also be considered Progressing or Degraded, if its Pods become + // unavailable sometime after the update completes. RolloutCompleted RolloutConditionType = "Completed" - // HealthyAndCompleted means that rollout is in a completed state and is healthy. - HealthyAndCompleted RolloutConditionType = "HealthyAndCompleted" + // RolloutHealthy means that rollout is in a completed state and is healthy. + RolloutHealthy RolloutConditionType = "Healthy" ) // RolloutCondition describes the state of a rollout at a certain point. diff --git a/rollout/bluegreen_test.go b/rollout/bluegreen_test.go index d12c11c966..8ad0b1f838 100644 --- a/rollout/bluegreen_test.go +++ b/rollout/bluegreen_test.go @@ -45,7 +45,7 @@ func TestBlueGreenCompletedRolloutRestart(t *testing.T) { r := newBlueGreenRollout("foo", 1, nil, "active", "preview") r.Status.Conditions = []v1alpha1.RolloutCondition{} - completedHealthyCond := conditions.NewRolloutCondition(v1alpha1.HealthyAndCompleted, corev1.ConditionFalse, conditions.RolloutHealthyAndCompletedReason, conditions.RolloutNotHealthyAndCompletedMessage) + completedHealthyCond := conditions.NewRolloutCondition(v1alpha1.RolloutHealthy, corev1.ConditionFalse, conditions.RolloutHealthyReason, conditions.RolloutNotHealthyMessage) conditions.SetRolloutCondition(&r.Status, *completedHealthyCond) completedCond, _ := newCompletedCondition(true) conditions.SetRolloutCondition(&r.Status, completedCond) @@ -1218,7 +1218,7 @@ func TestBlueGreenRolloutCompletedFalse(t *testing.T) { assert.NoError(t, err) index := len(rolloutPatch.Status.Conditions) - 3 - assert.Equal(t, v1alpha1.HealthyAndCompleted, rolloutPatch.Status.Conditions[index].Type) + assert.Equal(t, v1alpha1.RolloutHealthy, rolloutPatch.Status.Conditions[index].Type) assert.Equal(t, corev1.ConditionFalse, rolloutPatch.Status.Conditions[index].Status) } diff --git a/rollout/controller_test.go b/rollout/controller_test.go index f4489dc212..0e97a405e9 100644 --- a/rollout/controller_test.go +++ b/rollout/controller_test.go @@ -195,18 +195,18 @@ func newPausedCondition(isPaused bool) (v1alpha1.RolloutCondition, string) { func newHealthyCondition(isHealthy bool) (v1alpha1.RolloutCondition, string) { status := corev1.ConditionTrue - msg := conditions.RolloutHealthyAndCompletedMessage + msg := conditions.RolloutHealthyMessage if !isHealthy { status = corev1.ConditionFalse - msg = conditions.RolloutNotHealthyAndCompletedMessage + msg = conditions.RolloutNotHealthyMessage } condition := v1alpha1.RolloutCondition{ LastTransitionTime: timeutil.MetaNow(), LastUpdateTime: timeutil.MetaNow(), Message: msg, - Reason: conditions.RolloutHealthyAndCompletedReason, + Reason: conditions.RolloutHealthyReason, Status: status, - Type: v1alpha1.HealthyAndCompleted, + Type: v1alpha1.RolloutHealthy, } conditionBytes, err := json.Marshal(condition) if err != nil { diff --git a/rollout/sync.go b/rollout/sync.go index 4759f490a4..21ca6ea2b6 100644 --- a/rollout/sync.go +++ b/rollout/sync.go @@ -547,19 +547,19 @@ func (c *rolloutContext) calculateRolloutConditions(newStatus v1alpha1.RolloutSt isPaused := len(c.rollout.Status.PauseConditions) > 0 || c.rollout.Spec.Paused isAborted := c.pauseContext.IsAborted() - var becameIncompleteUnhealthy bool // remember if we transitioned from completed - completeCond := conditions.GetRolloutCondition(c.rollout.Status, v1alpha1.HealthyAndCompleted) - if !isPaused && conditions.RolloutHealthyAndCompleted(c.rollout, &newStatus) { - updateHealthyCond := conditions.NewRolloutCondition(v1alpha1.HealthyAndCompleted, corev1.ConditionTrue, conditions.RolloutHealthyAndCompletedReason, conditions.RolloutHealthyAndCompletedMessage) + var becameUnhealthy bool // remember if we transitioned from completed + completeCond := conditions.GetRolloutCondition(c.rollout.Status, v1alpha1.RolloutHealthy) + if !isPaused && conditions.RolloutHealthy(c.rollout, &newStatus) { + updateHealthyCond := conditions.NewRolloutCondition(v1alpha1.RolloutHealthy, corev1.ConditionTrue, conditions.RolloutHealthyReason, conditions.RolloutHealthyMessage) conditions.SetRolloutCondition(&newStatus, *updateHealthyCond) // If we ever wanted to emit a healthy event here it would be noisy and somewhat unpredictable for tests and so should probably be skipped // when checking in e2e and unit tests. - //c.recorder.Warnf(c.rollout, record.EventOptions{EventReason: conditions.RolloutHealthyAndCompletedReason}, conditions.RolloutHealthyAndCompletedMessage) + //c.recorder.Warnf(c.rollout, record.EventOptions{EventReason: conditions.RolloutHealthyReason}, conditions.RolloutHealthyMessage) } else { if completeCond != nil { - updateHealthyCond := conditions.NewRolloutCondition(v1alpha1.HealthyAndCompleted, corev1.ConditionFalse, conditions.RolloutHealthyAndCompletedReason, conditions.RolloutNotHealthyAndCompletedMessage) - becameIncompleteUnhealthy = conditions.SetRolloutCondition(&newStatus, *updateHealthyCond) - //c.recorder.Warnf(c.rollout, record.EventOptions{EventReason: conditions.RolloutHealthyAndCompletedReason}, conditions.RolloutNotHealthyAndCompletedMessage) + updateHealthyCond := conditions.NewRolloutCondition(v1alpha1.RolloutHealthy, corev1.ConditionFalse, conditions.RolloutHealthyReason, conditions.RolloutNotHealthyMessage) + becameUnhealthy = conditions.SetRolloutCondition(&newStatus, *updateHealthyCond) + //c.recorder.Warnf(c.rollout, record.EventOptions{EventReason: conditions.RolloutHealthyReason}, conditions.RolloutNotHealthyMessage) } } @@ -580,11 +580,11 @@ func (c *rolloutContext) calculateRolloutConditions(newStatus v1alpha1.RolloutSt // In such a case, we should simply not estimate any progress for this rollout. currentCond := conditions.GetRolloutCondition(c.rollout.Status, v1alpha1.RolloutProgressing) - isHealthyAndCompletedRollout := newStatus.Replicas == newStatus.AvailableReplicas && currentCond != nil && currentCond.Reason == conditions.NewRSAvailableReason && currentCond.Type != v1alpha1.RolloutProgressing + isHealthyRollout := newStatus.Replicas == newStatus.AvailableReplicas && currentCond != nil && currentCond.Reason == conditions.NewRSAvailableReason && currentCond.Type != v1alpha1.RolloutProgressing // Check for progress. Only do this if the latest rollout hasn't completed yet and it is not aborted - if !isHealthyAndCompletedRollout && !isAborted { + if !isHealthyRollout && !isAborted { switch { - case conditions.RolloutHealthyAndCompleted(c.rollout, &newStatus): + case conditions.RolloutHealthy(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. rsName := "" @@ -594,7 +594,7 @@ func (c *rolloutContext) calculateRolloutConditions(newStatus v1alpha1.RolloutSt msg := fmt.Sprintf(conditions.ReplicaSetCompletedMessage, rsName) progressingCondition := conditions.NewRolloutCondition(v1alpha1.RolloutProgressing, corev1.ConditionTrue, conditions.NewRSAvailableReason, msg) conditions.SetRolloutCondition(&newStatus, *progressingCondition) - case conditions.RolloutProgressing(c.rollout, &newStatus) || becameIncompleteUnhealthy: + case conditions.RolloutProgressing(c.rollout, &newStatus) || becameUnhealthy: // If there is any progress made, continue by not checking if the rollout failed. This // behavior emulates the rolling updater progressDeadline check. msg := fmt.Sprintf(conditions.RolloutProgressingMessage, c.rollout.Name) @@ -603,7 +603,7 @@ func (c *rolloutContext) calculateRolloutConditions(newStatus v1alpha1.RolloutSt } var reason string - if newStatus.StableRS == newStatus.CurrentPodHash && becameIncompleteUnhealthy { + if newStatus.StableRS == newStatus.CurrentPodHash && becameUnhealthy { // When a fully promoted rollout becomes Incomplete, e.g., due to the ReplicaSet status changes like // pod restarts, evicted -> recreated, we'll need to reset the rollout's condition to `PROGRESSING` to // avoid any timeouts. @@ -774,7 +774,7 @@ func (c *rolloutContext) requeueStuckRollout(newStatus v1alpha1.RolloutStatus) t } // No need to estimate progress if the rollout is complete or already timed out. isPaused := len(c.rollout.Status.PauseConditions) > 0 || c.rollout.Spec.Paused - if conditions.RolloutHealthyAndCompleted(c.rollout, &newStatus) || currentCond.Reason == conditions.TimedOutReason || isPaused || c.rollout.Status.Abort || isIndefiniteStep(c.rollout) { + if conditions.RolloutHealthy(c.rollout, &newStatus) || currentCond.Reason == conditions.TimedOutReason || isPaused || c.rollout.Status.Abort || isIndefiniteStep(c.rollout) { return time.Duration(-1) } // If there is no sign of progress at this point then there is a high chance that the diff --git a/test/e2e/functional_test.go b/test/e2e/functional_test.go index 7d24d9655a..8ca5a026ac 100644 --- a/test/e2e/functional_test.go +++ b/test/e2e/functional_test.go @@ -1123,10 +1123,10 @@ func (s *FunctionalSuite) TestKubectlWaitForCompleted() { kind: Service apiVersion: v1 metadata: - name: kubectl-wait-completed + name: kubectl-wait-healthy spec: selector: - app: kubectl-wait-completed + app: kubectl-wait-healthy ports: - protocol: TCP port: 80 @@ -1135,19 +1135,19 @@ spec: apiVersion: argoproj.io/v1alpha1 kind: Rollout metadata: - name: kubectl-wait-completed + name: kubectl-wait-healthy spec: replicas: 1 selector: matchLabels: - app: kubectl-wait-completed + app: kubectl-wait-healthy template: metadata: labels: - app: kubectl-wait-completed + app: kubectl-wait-healthy spec: containers: - - name: kubectl-wait-completed + - name: kubectl-wait-healthy image: nginx:1.19-alpine imagePullPolicy: Always ports: @@ -1161,21 +1161,21 @@ spec: strategy: blueGreen: - activeService: kubectl-wait-completed + activeService: kubectl-wait-healthy autoPromotionEnabled: true `). When(). UpdateSpec(). Then(). - ExpectRollout("HealthyAndCompleted=False", func(r *v1alpha1.Rollout) bool { - cmd := exec.Command("kubectl", "wait", "--for=condition=HealthyAndCompleted=False", fmt.Sprintf("rollout/%s", r.Name)) + ExpectRollout("Healthy=False", func(r *v1alpha1.Rollout) bool { + cmd := exec.Command("kubectl", "wait", "--for=condition=Healthy=False", fmt.Sprintf("rollout/%s", r.Name)) out, err := cmd.CombinedOutput() return err == nil && strings.Contains(string(out), fmt.Sprintf("rollout.argoproj.io/%s condition met", r.Name)) }). ExpectRolloutStatus("Progressing"). ExpectActiveRevision("1"). - ExpectRollout("HealthyAndCompleted=True", func(r *v1alpha1.Rollout) bool { - cmd := exec.Command("kubectl", "wait", "--for=condition=HealthyAndCompleted=True", fmt.Sprintf("rollout/%s", r.Name)) + ExpectRollout("Healthy=True", func(r *v1alpha1.Rollout) bool { + cmd := exec.Command("kubectl", "wait", "--for=condition=Healthy=True", fmt.Sprintf("rollout/%s", r.Name)) out, err := cmd.CombinedOutput() return err == nil && strings.Contains(string(out), fmt.Sprintf("rollout.argoproj.io/%s condition met", r.Name)) }). diff --git a/utils/conditions/conditions.go b/utils/conditions/conditions.go index 66275ff48b..bdb7a562f9 100644 --- a/utils/conditions/conditions.go +++ b/utils/conditions/conditions.go @@ -73,12 +73,12 @@ const ( // RolloutNotCompletedMessage is added when the rollout is completed RolloutNotCompletedMessage = "Rollout not completed, started update to revision %d (%s)" - // RolloutHealthyAndCompletedReason is added in a rollout when it is healthy. - RolloutHealthyAndCompletedReason = "HealthyAndCompleted" - // RolloutHealthyAndCompletedMessage is added when the rollout is completed and is healthy or not. - RolloutHealthyAndCompletedMessage = "Rollout is healthy and completed" - // RolloutNotHealthyAndCompletedMessage is added when the rollout is completed and is healthy or not. - RolloutNotHealthyAndCompletedMessage = "Rollout is not healthy and completed" + // RolloutHealthyReason is added in a rollout when it is healthy. + RolloutHealthyReason = "RolloutHealthy" + // RolloutHealthyMessage is added when the rollout is completed and is healthy or not. + RolloutHealthyMessage = "Rollout is healthy and completed" + // RolloutNotHealthyMessage is added when the rollout is completed and is healthy or not. + RolloutNotHealthyMessage = "Rollout is not healthy and completed" // RolloutAbortedReason indicates that the rollout was aborted RolloutAbortedReason = "RolloutAborted" @@ -268,9 +268,9 @@ func RolloutProgressing(rollout *v1alpha1.Rollout, newStatus *v1alpha1.RolloutSt strategySpecificProgress } -// RolloutHealthyAndCompleted considers a rollout to be complete once all of its desired replicas +// RolloutHealthy considers a rollout to be healthy once all of its desired replicas // are updated, available, and receiving traffic from the active service, and no old pods are running. -func RolloutHealthyAndCompleted(rollout *v1alpha1.Rollout, newStatus *v1alpha1.RolloutStatus) bool { +func RolloutHealthy(rollout *v1alpha1.Rollout, newStatus *v1alpha1.RolloutStatus) bool { completedStrategy := true replicas := defaults.GetReplicasOrDefault(rollout.Spec.Replicas) diff --git a/utils/conditions/rollouts_test.go b/utils/conditions/rollouts_test.go index 9a4d49d09e..842f01f594 100644 --- a/utils/conditions/rollouts_test.go +++ b/utils/conditions/rollouts_test.go @@ -350,7 +350,7 @@ func TestRolloutProgressing(t *testing.T) { } -func TestRolloutHealthyAndCompleted(t *testing.T) { +func TestRolloutHealthy(t *testing.T) { rollout := func(desired, current, updated, available int32, correctObservedGeneration bool) *v1alpha1.Rollout { r := &v1alpha1.Rollout{ Spec: v1alpha1.RolloutSpec{ @@ -475,7 +475,7 @@ func TestRolloutHealthyAndCompleted(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - assert.Equal(t, test.expected, RolloutHealthyAndCompleted(test.r, &test.r.Status)) + assert.Equal(t, test.expected, RolloutHealthy(test.r, &test.r.Status)) }) }