[ApplicationSet Controller] Failed resource update cause refresh annotation to remain in place

[nromriell]

Checklist:

• I've searched in the docs and FAQ for my answer: https://bit.ly/argocd-faq.
• I've included steps to reproduce the bug.
• I've pasted the output of argocd version.

Describe the bug

While the applicationset refresh annotation is in place all calls to generate params in a git generator skip the cache. Since the call to

if applicationSetInfo.RefreshRequired() {
    delete(applicationSetInfo.Annotations, common.AnnotationApplicationSetRefresh)
    err := r.Client.Update(ctx, &applicationSetInfo)

is fairly low on the Reconcile stack any calls that fail due to conflicts or other network issues that result in an error including the r.Client.Update call cause the annotation to remain on the ApplicationSet and reconciles following will retain RefreshRequired() == true

To Reproduce

• Create an ApplicationSet
• Artificially cause an early exit on the reconcile before or during

if applicationSetInfo.RefreshRequired() {
    delete(applicationSetInfo.Annotations, common.AnnotationApplicationSetRefresh)
    err := r.Client.Update(ctx, &applicationSetInfo)

• Annotate the applicationset with argocd.argoproj.io/application-set-refresh=true
• Observe ApplicationSet annotation not getting removed

Expected behavior

The argocd.argoproj.io/application-set-refresh=true annotation is removed after generator GenerateParams calls are finished

Version

~$ argocd version
argocd: v2.12.3+6b9cd82
  BuildDate: 2024-08-27T11:57:48Z
  GitCommit: 6b9cd828c6e9807398869ad5ac44efd2c28422d6
  GitTreeState: clean
  GoVersion: go1.22.4
  Compiler: gc
  Platform: linux/arm64

Logs

Most of these exits are not logged, however the failures I saw the most adding custom logs were conflict errors on the resource having been updated. Especially during

err = r.updateResourcesStatus(ctx, logCtx, &applicationSetInfo, currentApplications)
if err != nil {
  return ctrl.Result{}, fmt.Errorf("failed to get update resources status for application set: %w", err)
}

example custom log:

 applicationset-controller time="2024-11-01T15:29:42Z" level=info msg="exiting after updateResourcesStatus: unable to set application set status: Operation cannot be fulfilled on applicationsets.argoproj.io \"example\": the object has been modified; please apply your changes to the latest version and try again"

I think the call to remove the annotation could likely be moved up the stack but wrapping the resource updates with something like the retry.RetryOnConflict used in other parts of the application would go a long way in mitigating this issue

[rumstead]

A similar issue was brought up on slack .

Do you have any idea what is changing to cause the conflict? Are you running multiple instances of the applicationset controller?

[llavaud]

upgrading to 2.13 seems to resolve this issue

[blakepettersson]

Indeed, it seems like @nromriell's proposed fix was done with #19663, which is in 2.13.