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

applicationset controller should not clear argocd.argoproj.io/refresh annotation #10500

Closed
jessesuen opened this issue Sep 1, 2022 · 1 comment · Fixed by #10510
Closed
Labels
bug Something isn't working component:applications-set Bulk application management related good first issue Good for newcomers

Comments

@jessesuen
Copy link
Member

Describe the bug

When refreshing an application, the appset-controller will blindly replace the Applications .metadata.annotations with the annotations specified in the template. This means it will also clear the argocd.argoproj.io/refresh annotation that gets set on the application during a refresh

This could lead to situations where the app-controller could miss the refresh request because the appset-controller had already deleted the annotation.

To Reproduce

  1. create an appset that generates an application foo
  2. scale down application controller
  3. refresh application foo
  4. the foo application will be considered refreshed even though app-controller was down

Expected behavior

ApplicationSet controller should not touch argocd.argoproj.io/refresh as this field should only be cleared by app-controller.

Version

Argo CD v2.4.*

@jessesuen jessesuen added bug Something isn't working component:applications-set Bulk application management related labels Sep 1, 2022
@jessesuen
Copy link
Member Author

jessesuen commented Sep 1, 2022

The fix can be similar to what we do for notified.notifications.argoproj.io annotation, in that it is preserved when updating an app

https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L530-L537

			// Preserve argo cd notifications state (https://github.com/argoproj/applicationset/issues/180)
			if state, exists := found.ObjectMeta.Annotations[NotifiedAnnotationKey]; exists {
				if generatedApp.Annotations == nil {
					generatedApp.Annotations = map[string]string{}
				}
				generatedApp.Annotations[NotifiedAnnotationKey] = state
			}
			found.ObjectMeta.Annotations = generatedApp.Annotations

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working component:applications-set Bulk application management related good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants