Skip to content

fix: ArgoCD 2.11 - Loop of PATCH calls to Application objects#19340

Merged
crenshaw-dev merged 1 commit intoargoproj:masterfrom
alexmt:18151-loop-patch
Aug 1, 2024
Merged

fix: ArgoCD 2.11 - Loop of PATCH calls to Application objects#19340
crenshaw-dev merged 1 commit intoargoproj:masterfrom
alexmt:18151-loop-patch

Conversation

@alexmt
Copy link
Collaborator

@alexmt alexmt commented Aug 1, 2024

Closes #18151

PR fixes a bug introduced by #18061 . We were trying to fix #15126 - error which happened while generating status patch using strategic merge strategy.
We've added replace merge strategy to status.comparedTo field which apparently does not work as I expected. Instead of generating a patch that replaces comparedTo field it just uses new comparedTo field value as a patch without trying to merge. As a result, fields under status. compared never gets removed, which causes all kinds of issues. For example, if the user switches the app destination from cluster name to cluster server URL, then status.comparedTo.destination ends up having both name and server fields. Once it happens, the controller will keep continuously reconciling the application because it thinks the destination has changed.

Fix: just don't use a strategic merge patch because controller is the only manager of application status. PR changes controller to generate simple merge patch to update secret. It solves both #18151 and #15126 .

@alexmt alexmt requested a review from a team as a code owner August 1, 2024 16:49
@bunnyshell
Copy link

bunnyshell bot commented Aug 1, 2024

❌ Preview Environment deleted from Bunnyshell

Available commands (reply to this comment):

  • 🚀 /bns:deploy to deploy the environment

@bunnyshell
Copy link

bunnyshell bot commented Aug 1, 2024

❌ Preview Environment deleted from Bunnyshell

Available commands (reply to this comment):

  • 🚀 /bns:deploy to deploy the environment

@alexmt alexmt force-pushed the 18151-loop-patch branch 2 times, most recently from f537bbf to 002091d Compare August 1, 2024 17:40
Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
@alexmt alexmt force-pushed the 18151-loop-patch branch from 002091d to 60b1e8c Compare August 1, 2024 17:43
@crenshaw-dev
Copy link
Member

/cherry-pick release-2.11

@crenshaw-dev
Copy link
Member

/cherry-pick release-2.12

@rumstead
Copy link
Member

rumstead commented Aug 1, 2024

nice!

@codecov
Copy link

codecov bot commented Aug 1, 2024

Codecov Report

Attention: Patch coverage is 53.84615% with 6 lines in your changes missing coverage. Please review.

Project coverage is 52.79%. Comparing base (267f243) to head (60b1e8c).
Report is 499 commits behind head on master.

Files with missing lines Patch % Lines
controller/appcontroller.go 53.84% 3 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #19340      +/-   ##
==========================================
+ Coverage   52.78%   52.79%   +0.01%     
==========================================
  Files         316      316              
  Lines       43581    43602      +21     
==========================================
+ Hits        23004    23021      +17     
+ Misses      18026    18024       -2     
- Partials     2551     2557       +6     

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

@crenshaw-dev crenshaw-dev merged commit b5a3712 into argoproj:master Aug 1, 2024
@gcp-cherry-pick-bot
Copy link

Cherry-pick failed with Merge error b5a3712303d85e7466c2ecf6f889eddc2776c20a into temp-cherry-pick-e84661-release-2.11

gcp-cherry-pick-bot bot pushed a commit that referenced this pull request Aug 1, 2024
Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
@crenshaw-dev
Copy link
Member

@alexmt mind manually cherry-picking 2.11?

@alexmt
Copy link
Collaborator Author

alexmt commented Aug 1, 2024

Sure, doing it!

@daftping
Copy link
Contributor

daftping commented Aug 1, 2024

since the issue has been introduced in 2.10.10, would it make sense to cherry-pick to 2.10 as well?

crenshaw-dev pushed a commit that referenced this pull request Aug 1, 2024
#19343)

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
Co-authored-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
alexmt added a commit to alexmt/argo-cd that referenced this pull request Aug 1, 2024
…oj#19340)

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
@alexmt alexmt deleted the 18151-loop-patch branch August 1, 2024 20:26
alexmt added a commit to alexmt/argo-cd that referenced this pull request Aug 1, 2024
…oj#19340)

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
@alexmt
Copy link
Collaborator Author

alexmt commented Aug 1, 2024

@kencochrane
Copy link

@alexmt thank you for finding and fixing that issue.

alexmt added a commit that referenced this pull request Aug 4, 2024
#19348)

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
alexmt added a commit that referenced this pull request Aug 4, 2024
#19347)

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
rhyswilliamsza pushed a commit to rhyswilliamsza/argo-cd that referenced this pull request Aug 12, 2024
…oj#19340)

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
Signed-off-by: Rhys Williams <rhys.williams@electrum.co.za>
alexmt added a commit to alexmt/argo-cd that referenced this pull request Aug 16, 2024
…oj#19340)

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
alexmt added a commit to alexmt/argo-cd that referenced this pull request Aug 16, 2024
…oj#19340)

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
alexmt added a commit to alexmt/argo-cd that referenced this pull request Aug 18, 2024
…oj#19340)

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
alexmt added a commit to alexmt/argo-cd that referenced this pull request Aug 18, 2024
…oj#19340)

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
todaywasawesome pushed a commit that referenced this pull request Aug 21, 2024
#19569)

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
todaywasawesome pushed a commit that referenced this pull request Aug 21, 2024
#19570)

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

5 participants