fix: add SyncError condition to Argo CD application health checks

[bmbferreira]

• Introduced ApplicationConditionSyncError to represent synchronization errors.
• Updated health checker tests to account for the new SyncError condition, increasing the issue count in assertions.
• Modified health assessment logic to include SyncError in the evaluation of application health.

This should fix #5166. Also noticed this comment saying that we were not sure about all the error states that should be considered here and I think it's important to have SyncError because that's the status we will get if we are using sync waves on argo and one of the "waves" fail (for example running database migrations in the first step, as I reference here).

[netlify]

✅ Deploy Preview for docs-kargo-io canceled.

Name	Link
🔨 Latest commit	61d8829
🔍 Latest deploy log	https://app.netlify.com/projects/docs-kargo-io/deploys/691eff26e9e33c00087ea81b

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.12%. Comparing base (d4facc0) to head (61d8829).
⚠️ Report is 282 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #5414   +/-   ##
=======================================
  Coverage   56.12%   56.12%           
=======================================
  Files         411      411           
  Lines       30055    30055           
=======================================
  Hits        16868    16868           
  Misses      12213    12213           
  Partials      974      974           

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[antoinedeschenes]

Perhaps looking at Status.OperationState.Phase rather than individual conditions would be more reliable? That's what was suggested to me when forwarding the issue to the ArgoCD Slack channel

[bmbferreira]

Perhaps looking at Status.OperationState.Phase rather than individual conditions would be more reliable? That's what was suggested to me when forwarding the issue to the ArgoCD Slack channel

Probably, but that might require more changes. This seems to be a reasonable change to fix this problem. I would say that SyncError should be considered as a "health error" condition.

[hiddeco]

Thanks for looking into this, and proposing a fix.

I believe adding this condition could indeed improve things, but I would need a second sign-off on this from either @jessesuen or @krancour.

[jessesuen]

This should fix #5166.

Shouldn't #5490 have fixed #5166? It seems the root cause of #5166 was because of the use of errorThreshold, not the fact that the application had a SyncError condition.

I'm not sure adding the check for SyncError is appropriate, since it will broaden the conditions that affect Stage health, and this might be a transient condition that might self-correct.

@jessesuen expressed some skepticism about this, so I'm dismissing this approval until I've dug into it more.

[krancour]

Shouldn't #5490 have fixed #5166?

That appears to be the case.