Move Argo CD sync & wait logic to promotion

[jessesuen]

Proposed Feature

Currently, the Argo CD sync happens in the promotion, but waiting for the sync operation to complete is currently happening in the Stage reconciler. I believe that the wait should instead be part of the promotion. Instead of the Stage reconciler doing the sync and wait, I propose this logic be moved to the Promotion reconciler.

Motivation

Earlier, we did not do this because Promotions were not long-lived. But now that they are, I feel it would make more sense for sync + wait to be part of the promotion. This will align better with our future desire for promotions to integrate with other tools aside from Argo CD, which might have asynchronous, long-lived deployment processes.

Moving it to Promotion will make it more visible when the sync failed, because the promotion would fail too.

Suggested Implementation

• When a promotion job issues a sync, it does not complete the Promotion immediately like it does now. It leaves it in Running.
• We have a predicate for Argo CD such that when an operation completes, we enqueue the promotion job instead of the stage. When the Promotion job sees that it is completed successfully, the promotion is Successful. If the operation fails, the promotion is considered failed.
• We will need a way to terminate Promotions, which in the case of Argo CD, will terminate the operation. This can be a separate issue.

Open Issues

There is also the question of whether or not waiting for an Application to become Healthy should be part of promotion. I suggest we leave this in Stage reconciler, because this is more like a verification than it is a promotion.

[krancour]

I agree with this, but tentatively think that checking sync state should happen in both places. While an Application's sync state might be distinct from its health, this feels less true for a Kargo Stage. Detecting that an Application isn't synced to the current Freight indicates the Stage is unhealthy (even if the App is). I feel that's an important check to continue doing periodically.

wdyt?

[jessesuen]

think that checking sync state should happen in both places

I didn't consider moving the sync state check into promotion. I think that would be fine to have that in promotion as well, because we could clearly indicate why the promotion job failed in the status message, and have a record of it.

[hiddeco]

Based on a lot of reading today, I believe this is an opportunity to also address #1451.