fix: manifest-generate-paths with autosync causes an undesirable refresh sync

[pasha-codefresh]

Closes: #18326

alreadyAttemptedSync is deciding if self heal should happens or no, and today we just have check app.Status.OperationState.SyncResult.Revision != commitSHA which is not enough, because in monorepo these two revisions can be different.

So i have added additional check, which verify if change actually happened in git for current application or no, and if revisions are different but no changes in git for app, self heal will not be executed

Also i am passing resolved revision from UpdateRevisionForPaths response, because if GenerateManifest will accept revision[i] that in most cases HEAD, resolved revision in UpdateRevisionForPaths and GenerateManifest can be different if commit happens between function execution.

[bunnyshell]

❌ Preview Environment deleted from Bunnyshell

Available commands (reply to this comment):

• 🚀 /bns:deploy to deploy the environment

[bunnyshell]

❌ Preview Environment deleted from Bunnyshell

Available commands (reply to this comment):

• 🚀 /bns:deploy to deploy the environment

[pasha-codefresh]

@agaudreault Could you please take a look?

[todaywasawesome]

This is a big improvement for monorepos with a lot of activity. Well done!

[ishitasequeira]

Thanks @pasha-codefresh for tackling this. The changes looks good to me 🚀

[taemery]

Great win for monorepos!

[agaudreault]

Waiting for answer because I have trouble understanding the logic if the boolean is always true.

[agaudreault]

After this point, nothing is setting the boolean back to false 🤔 so it always returns true, except during error. It looks like this return variable is not necessary.

[pasha-codefresh]

Yeah, i changed default to true, because it always returned false before for apps that has no AnnotationKeyManifestGeneratePaths annotation. I am reverting it back

[agaudreault]

LGTM. Default behavior (when there is no annotation set) is to always return true.
When annotations is set, returns true if at least one revision changed relevant files.

Result: Will not perform an initial "selfHeal" on non-relevant commits the first time drift are detected.

Important note: When annotation is not set, even with selfHeal=false, it will always trigger a sync once, the first time drift are detected (if AutoSync=true). If the commit that was made did not cause an outOfSync (e.g. updating a README file), this will be delayed until the first time the app goes OutOfSync (can be caused by cluster changes which will "selfHeal" even if selfHeal=false). So this is fixing the behavior only when annotation is set.

[codecov]

Codecov Report

Attention: Patch coverage is 64.61538% with 23 lines in your changes missing coverage. Please review.

Project coverage is 55.82%. Comparing base (3d66b05) to head (842a62e).
Report is 459 commits behind head on master.

Files with missing lines	Patch %	Lines
controller/state.go	48.57%	16 Missing and 2 partials ⚠️
reposerver/repository/repository.go	78.57%	3 Missing ⚠️
controller/appcontroller.go	86.66%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #19799      +/-   ##
==========================================
+ Coverage   55.79%   55.82%   +0.02%     
==========================================
  Files         320      320              
  Lines       44223    44250      +27     
==========================================
+ Hits        24675    24703      +28     
- Misses      16995    16996       +1     
+ Partials     2553     2551       -2     

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

[pasha-codefresh]

LGTM. Default behavior (when there is no annotation set) is to always return true. When annotations is set, returns true if at least one revision changed relevant files.

Result: Will not perform an initial "selfHeal" on non-relevant commits the first time drift are detected.

Important note: When annotation is not set, even with selfHeal=false, it will always trigger a sync once, the first time drift are detected (if AutoSync=true). If the commit that was made did not cause an outOfSync (e.g. updating a README file), this will be delayed until the first time the app goes OutOfSync (can be caused by cluster changes which will "selfHeal" even if selfHeal=false). So this is fixing the behavior only when annotation is set.

Yes! thank you! we will cover it with documentation on Monday

[pasha-codefresh]

Thank you for everyone to review. I also going to open PR that will cover argocd.argoproj.io/manifest-generate-paths with e2e