Skip to content

fix(appset): setApplicationSetStatusCondition updates appset in memory#19622

Closed
carlosrejano wants to merge 2 commits intoargoproj:masterfrom
carlosrejano:fix-appset-setApplicationSetStatusCondition-conflict
Closed

fix(appset): setApplicationSetStatusCondition updates appset in memory#19622
carlosrejano wants to merge 2 commits intoargoproj:masterfrom
carlosrejano:fix-appset-setApplicationSetStatusCondition-conflict

Conversation

@carlosrejano
Copy link
Contributor

@carlosrejano carlosrejano commented Aug 21, 2024

Context:

setApplicationSetStatusCondition function does not update the appset
in memory after updating it in the cluster, this means that new
updates using the appset object in memory may fail due to conflict.
Before it was not an issue since setApplicationSetStatusCondition
was almost always used before finishing the reconcile so no new
updates were done.

Now this function is also used when performing progressive syncs as
introduced in this PR: #19473
So it's very likely that a new update after this function so new
updates may fail due to conflict.

What does this PR?

  • After updating the appset object in setApplicationSetStatusCondition it gets the object again to reference it in the appset pointer of the reconciler.

Probably fixes: #19535

  # Context:
  `setApplicationSetStatusCondition` function does not update the appset
  in memory after updating it in the cluster, this means that new
  updates using the appset object in memory may fail due to conflict.
  Before it was not an issue since `setApplicationSetStatusCondition`
  was almost always used before finishing the reconcile so no new
  updates were done.

  Now this function is also used when performing progressive syncs as
  introduced in this PR: argoproj#19473
  So it's very likely that a new update after this function so new
  updates may fail due to conflict.

  # What does this PR?
  - After updating the appset object in `setApplicationSetStatusCondition` it gets the
    object again to reference it in the appset pointer of the reconciler.

Signed-off-by: Carlos Rejano <carlos.rejano@adevinta.com>
@carlosrejano carlosrejano requested a review from a team as a code owner August 21, 2024 16:58
@bunnyshell
Copy link

bunnyshell bot commented Aug 21, 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 21, 2024

❌ Preview Environment deleted from Bunnyshell

Available commands (reply to this comment):

  • 🚀 /bns:deploy to deploy the environment

@carlosrejano
Copy link
Contributor Author

I believe that this does not do any change, I checked and the Status().Update call should already update the object.

I'm closing It in favor of #19663

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ApplicationSet rolling sync stuck in ArgoCD 2.12

1 participant