chore: clear app status op state

[krancour]

cc @jessesuen

[netlify]

✅ Deploy Preview for docs-kargo-io ready!

Name	Link
🔨 Latest commit	68108d7
🔍 Latest deploy log	https://app.netlify.com/sites/docs-kargo-io/deploys/67520edc6553df00080154f3
😎 Deploy Preview	https://deploy-preview-3069.docs.kargo.io
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[jessesuen]

In principle, Kargo (and even Argo CD API server) should not update Application status when creating the operation. In fact, if status subresources were enabled for the CRD, this would not even be possible.

I take the blame for this, having wrote the argo-cd code that does exactly this.

We'll have to live with ugly hack until Argo CD improves its behavior.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 51.22%. Comparing base (cda7fee) to head (68108d7).
Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3069   +/-   ##
=======================================
  Coverage   51.22%   51.22%           
=======================================
  Files         283      283           
  Lines       25456    25459    +3     
=======================================
+ Hits        13039    13042    +3     
  Misses      11720    11720           
  Partials      697      697           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[akuitybot]

Successfully created backport PR for release-1.1:

• chore(backport release-1.1): chore: clear app status op state #3073

[stephaneetje]

Hello, i just tried the patch in v1.1.0, it is still not working for me, it syncs before it sees the outofsync ,so i need another manual sync...
FYI, i first tried a patch from argocd that was working : argoproj/argo-cd#20915
Then i took it off to try kargo 1.1 and see if it was fixing the problem, which is not.

Edit:

In argocd's PR they only set operation state to nil when new operation is getting started : argoproj/argo-cd@bb5db09#diff-f952d05ea83b61f771541425e28fa3931af2f2e7950261dd59cab93bdcfe2e9eR2344

[krancour]

@stephaneetje we are clearing it only when initiating an operation.

I wonder if you aren't actually running into #3082 now.

[stephaneetje]

@krancour i don't think so as i do see the sync. The problem is exactly as before i used argocd's patch. It starts the sync Before we see the app OutOfSync.

we are clearing it only when initiating an operation

I was thinking more of the time ordering than just the condition. I think it is cleared too early, it should be cleared "when sync is getting started"

[krancour]

That is what we are doing. When we update app.Operation, we clear app.Status.OperationState.

[andrii-korotkov-verkada]

@krancour, shall we also do

dst.Object["status"] = src.Object["status"]

below?

[krancour]

Ugh. Missed that, although it probably shouldn't be status, but just status.operationState. As previously discussed, we really don't want to be messing with status at all, so we need to be very surgical about what we patch here.

[andrii-korotkov-verkada]

Was there a fix done for this?

[krancour]

Look down 👇