argo-rollouts icon indicating copy to clipboard operation
argo-rollouts copied to clipboard

Completed Event/Condition should only trigger during updates

Open jessesuen opened this issue 2 years ago • 1 comments

Summary

Currently, we equate the Completed condition to a Healthy rollout. However, a Rollout becomes Progressing for other reasons than an update (e.g. pod restarts).

Looking at the intent of the condition, it is intended to only emit when updates complete. See:

	// RolloutCompletedReason is added in a rollout when it is completed.
	RolloutCompletedReason = "RolloutCompleted"
	// RolloutCompletedMessage is added when the rollout is completed
	RolloutCompletedMessage = "Rollout completed update to revision %d (%s): %s"

The Completed condition should only be set to False in the middle of an update and set to True at the end of the update. A good way to detect this is simply comparing current and stable pod hashes:

status:
  currentPodHash: 549676655f
  stableRS: 549676655f

Diagnostics

What version of Argo Rollouts are you running? v1.2.0


Message from the maintainers:

Impacted by this bug? Give it a 👍. We prioritize the issues with the most 👍.

jessesuen avatar Jun 16 '22 22:06 jessesuen

To fix this, I think the existing RolloutComplete() helper should be renamed to RolloutHealthy() and we should introduce a new RolloutComplete() helper that ignores pod availability and only considers stable/current pod hash.

https://github.com/argoproj/argo-rollouts/blob/03636ab13003bcf487e8db83b6cde394c8237c23/utils/conditions/conditions.go#L257-L286

jessesuen avatar Jun 16 '22 22:06 jessesuen