Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Honor manifest-generate-paths annotation during CompareWithLatest #14242

Closed
agaudreault opened this issue Jun 27, 2023 · 1 comment · Fixed by #15636
Closed

Honor manifest-generate-paths annotation during CompareWithLatest #14242

agaudreault opened this issue Jun 27, 2023 · 1 comment · Fixed by #15636
Labels
enhancement New feature or request

Comments

@agaudreault
Copy link
Member

agaudreault commented Jun 27, 2023

Summary

When the monorepo pattern is used and multiple Applications synchronize sources from different folders in the same repo, the timeout.reconciliation will cause all Applications in that repo to be refreshed with CompareWithLatest CompareWith = 2. If the applications revision points to a branch that was updated, their manifest will all be regenerated by the repo-server because the last revision changed.

The argocd.argoproj.io/manifest-generate-paths exist to support the monerepo use case, but is only honored during the GitHub webhooks and not during the CompareWithLatest sync.

Slack thread: https://cloud-native.slack.com/archives/C04SURUPDL2/p1686849819036059

Motivation

Avoiding regenerating the Application manifest when the sources did not change would reduce the load on the repo-server. Since the CompareWithLatest is performed by a timeout, this is currently causing spikes in the repo-server. The repo-server operation queue will fill up based on the limit defined in reposerver.parallelism.limit and the number of replicas.

We can see that the repo-server has to do a lot more work every timeout.reconciliation period happening during business hours - when the monorepos have new commits. During the night - when developers are not working as much - the repo-server is more stable.
image

This can also be mitigated further in combination with #14241

Proposal

The suggestion is to add a new method on the repo-server. This would prevent the application-controller to perform git operation during the already complex GenerateManifest method.

  • Create a new CompareAppState method on the repo-server that would be called if the annotation is set
  • The method would look at the list of files touched between the desired revision (HEAD) and the app lastSyncRevision in the repository and see if it applies for a given path defined in the annotation.
  • If it does not, it returns the manifest in the cache and it updates the cache so that comparision can be avoided the next time.
  • If it does, it proceeds with the current behavior.
@zswanson
Copy link

zswanson commented Oct 30, 2023

Since work appears to be in-progress here, I'll add that there's an older related issue from 2022 #10679 where it was pointed out that one of the other major side effects here is that this effectively equates to 'self heal' being enabled for every app in the monorepo, as any time a commit occurs it will re-sync the state of all apps.

We are eagerly watching for this to get resolved as it blocks our use of autosync from a large monorepo of app configs.

crenshaw-dev added a commit that referenced this issue Apr 4, 2024
… (#15636)

* squash commits

Signed-off-by: Alexy Mantha <[email protected]>

* Update util/git/client.go

Co-authored-by: Michael Crenshaw <[email protected]>
Signed-off-by: Alexy Mantha <[email protected]>

* fix error message

Signed-off-by: Alexy Mantha <[email protected]>

* add git client options

Signed-off-by: Alexy Mantha <[email protected]>

* Update generated code

Signed-off-by: Alexy Mantha <[email protected]>

* run fmt

Signed-off-by: Alexy Mantha <[email protected]>

* fix tests

Signed-off-by: Alexy Mantha <[email protected]>

* failed gen

Signed-off-by: Alexy Mantha <[email protected]>

* tweak logs and rename cache

Signed-off-by: Alexy Mantha <[email protected]>

* validate revisions

Signed-off-by: Alexy Mantha <[email protected]>

* fix tests

Signed-off-by: Alexy Mantha <[email protected]>

* fix tests

Signed-off-by: Alexy Mantha <[email protected]>

* fmt

Signed-off-by: Alexy Mantha <[email protected]>

* fix linting

Signed-off-by: Alexy Mantha <[email protected]>

* fixes from review

Signed-off-by: Alexy Mantha <[email protected]>

* generate

Signed-off-by: Alexy Mantha <[email protected]>

* fix

Signed-off-by: Alexy Mantha <[email protected]>

* use log context

Signed-off-by: Alexy Mantha <[email protected]>

---------

Signed-off-by: Alexy Mantha <[email protected]>
Co-authored-by: Michael Crenshaw <[email protected]>
mkieweg pushed a commit to mkieweg/argo-cd that referenced this issue Jun 11, 2024
…roj#14242) (argoproj#15636)

* squash commits

Signed-off-by: Alexy Mantha <[email protected]>

* Update util/git/client.go

Co-authored-by: Michael Crenshaw <[email protected]>
Signed-off-by: Alexy Mantha <[email protected]>

* fix error message

Signed-off-by: Alexy Mantha <[email protected]>

* add git client options

Signed-off-by: Alexy Mantha <[email protected]>

* Update generated code

Signed-off-by: Alexy Mantha <[email protected]>

* run fmt

Signed-off-by: Alexy Mantha <[email protected]>

* fix tests

Signed-off-by: Alexy Mantha <[email protected]>

* failed gen

Signed-off-by: Alexy Mantha <[email protected]>

* tweak logs and rename cache

Signed-off-by: Alexy Mantha <[email protected]>

* validate revisions

Signed-off-by: Alexy Mantha <[email protected]>

* fix tests

Signed-off-by: Alexy Mantha <[email protected]>

* fix tests

Signed-off-by: Alexy Mantha <[email protected]>

* fmt

Signed-off-by: Alexy Mantha <[email protected]>

* fix linting

Signed-off-by: Alexy Mantha <[email protected]>

* fixes from review

Signed-off-by: Alexy Mantha <[email protected]>

* generate

Signed-off-by: Alexy Mantha <[email protected]>

* fix

Signed-off-by: Alexy Mantha <[email protected]>

* use log context

Signed-off-by: Alexy Mantha <[email protected]>

---------

Signed-off-by: Alexy Mantha <[email protected]>
Co-authored-by: Michael Crenshaw <[email protected]>
Hariharasuthan99 pushed a commit to AmadeusITGroup/argo-cd that referenced this issue Jun 16, 2024
…roj#14242) (argoproj#15636)

* squash commits

Signed-off-by: Alexy Mantha <[email protected]>

* Update util/git/client.go

Co-authored-by: Michael Crenshaw <[email protected]>
Signed-off-by: Alexy Mantha <[email protected]>

* fix error message

Signed-off-by: Alexy Mantha <[email protected]>

* add git client options

Signed-off-by: Alexy Mantha <[email protected]>

* Update generated code

Signed-off-by: Alexy Mantha <[email protected]>

* run fmt

Signed-off-by: Alexy Mantha <[email protected]>

* fix tests

Signed-off-by: Alexy Mantha <[email protected]>

* failed gen

Signed-off-by: Alexy Mantha <[email protected]>

* tweak logs and rename cache

Signed-off-by: Alexy Mantha <[email protected]>

* validate revisions

Signed-off-by: Alexy Mantha <[email protected]>

* fix tests

Signed-off-by: Alexy Mantha <[email protected]>

* fix tests

Signed-off-by: Alexy Mantha <[email protected]>

* fmt

Signed-off-by: Alexy Mantha <[email protected]>

* fix linting

Signed-off-by: Alexy Mantha <[email protected]>

* fixes from review

Signed-off-by: Alexy Mantha <[email protected]>

* generate

Signed-off-by: Alexy Mantha <[email protected]>

* fix

Signed-off-by: Alexy Mantha <[email protected]>

* use log context

Signed-off-by: Alexy Mantha <[email protected]>

---------

Signed-off-by: Alexy Mantha <[email protected]>
Co-authored-by: Michael Crenshaw <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants