-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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 rolling sync stuck in ArgoCD 2.12 #19535
Comments
# Context: When updating the status of the applicationset object it can happen that it fails due to a conflict since the resourceVersion has changed due to a different update. This makes the reconcile fails and we need to wait until the following reconcile loop until it updates the relevant status fields and hope that the update calls don't fail again due a conflict. It can even happen that it gets stuck constantly due to this erriors. A better approach I would say is retrying when there is a conflict error with the newest version of the object, so we make sure we update the object with the latest version always. This has been raised in issue argoproj#19535 that failing due to conflicts can make the reconcile not able to proceed. # What does this PR? - Wraps all the `Update().Status` calls inside a retry function that will retry when the update fails due a conflict. - Adds appset to fake client subresources, if not the client can not correctly determine the status subresource. Refer to: kubernetes-sigs/controller-runtime#2386, and kubernetes-sigs/controller-runtime#2362. Signed-off-by: Carlos Rejano <[email protected]>
# Context: When updating the status of the applicationset object it can happen that it fails due to a conflict since the resourceVersion has changed due to a different update. This makes the reconcile fails and we need to wait until the following reconcile loop until it updates the relevant status fields and hope that the update calls don't fail again due a conflict. It can even happen that it gets stuck constantly due to this erriors. A better approach I would say is retrying when there is a conflict error with the newest version of the object, so we make sure we update the object with the latest version always. This has been raised in issue argoproj#19535 that failing due to conflicts can make the reconcile not able to proceed. # What does this PR? - Wraps all the `Update().Status` calls inside a retry function that will retry when the update fails due a conflict. - Adds appset to fake client subresources, if not the client can not correctly determine the status subresource. Refer to: kubernetes-sigs/controller-runtime#2386, and kubernetes-sigs/controller-runtime#2362. Signed-off-by: Carlos Rejano <[email protected]>
* fix(appset): Retry on conflict when updating status # Context: When updating the status of the applicationset object it can happen that it fails due to a conflict since the resourceVersion has changed due to a different update. This makes the reconcile fails and we need to wait until the following reconcile loop until it updates the relevant status fields and hope that the update calls don't fail again due a conflict. It can even happen that it gets stuck constantly due to this erriors. A better approach I would say is retrying when there is a conflict error with the newest version of the object, so we make sure we update the object with the latest version always. This has been raised in issue #19535 that failing due to conflicts can make the reconcile not able to proceed. # What does this PR? - Wraps all the `Update().Status` calls inside a retry function that will retry when the update fails due a conflict. - Adds appset to fake client subresources, if not the client can not correctly determine the status subresource. Refer to: kubernetes-sigs/controller-runtime#2386, and kubernetes-sigs/controller-runtime#2362. Signed-off-by: Carlos Rejano <[email protected]> * fixup! fix(appset): Retry on conflict when updating status --------- Signed-off-by: Carlos Rejano <[email protected]> Signed-off-by: carlosrejano <[email protected]> Co-authored-by: Carlos Rejano <[email protected]>
I can confirm this seems to happen also in Today, we had such an occurence and there are a bunch of applications in "waves" preceding the one for the stuck application that have their status to I forced the status to |
I believe you might be right on other potential problems, with multiple commits. Between the time we store the revisions here and the time we compare it here there could have been other commits/revisions changes when the sync is applied (particularly in the case of a mono repo as source for multi apps for the appset), so it will stay stuck in pending again. I was wondering if @wparr-circle or @crenshaw-dev can confirm and if they have a take on that? |
I might be completely off, but what if we overthinked all this? So if an app is synced and healthy whatever the status of the app in the applicationset, its job is done isn't it? Here is an over simpified graph, we already have some of those transitions (waiting and progressing) |
Indeed, it would be interesting to understand the strategy(ies) that should be followed for the progressive sync, in particular when the ApplicationSet spec is updated or that the Personally, I mostly foresee 2 strategies
With those strategies in mind, what I don't get clear is the signification of
My 2 cents is that to achieve the first strategy, we need to ensure that all applications are effectively healthy with the latest revision. If they are not healthy, we need to go, in order, through all the applications and sync them. This means a high level algorithm like this: latestTargetRevisions = getCurrentTargetRevision(appSet)
appSet.status = getAppSetApplicationStatuses(appSet)
for appStatus in appSet.status :
if appStatus.targetRevisions != latestTargetRevisions:
syncApp(appStatus.Name)
break // we only update one app at a time
if not appStatus.Ready || not appStatus.Healthy:
break // We hold until the app is ready and healthy This makes me wonder why the appStatus targetRevision is not updated when the app status is Waiting or Pending. Strategy 2 is quite more complex as it requires to store a queue of updates to be performed (and wonder how to handle possible failing deployments (update worked for the first applications, but failed in the middle) |
Well my point is that the revisions themselves unless I'm mistaken are not important at the applicationset level. I believe we start using those in 2.12 because we were stuck in pending states in 2.11 sometimes, before we were using sync date/time comparaison, but the transition logic suffer the same problem in both, if someone do a manual sync during a rolling sync on one of the app or if other commit comes in (mono repo setup is highly subject to that), then the condition to get out of the pending state is never met. To my understanding, the application controller is already handling all the follow-up there and will flag an application as outOfSync if it needs a sync. So the applicationset controller shouldn't care which revision or date, it should only care to trigger a sync on outofsync applications in a certain order. i.e. I think our only problem is the state machine transitions and conditions. And we already have a "queue" system, the applicationset application statuses is basically a backup of the queue between reconciliation. It goes like that: One block is basically one reconcile of the applicationset. I believe it's really only the transition conditions that should simply not take into account revision or time but just application outofsync, health and operation statuses. I started a branch to test those simplifications but couldn't advance much in my calendar 😥 And also...it's highly possible that I completely misunderstood the logic and needs 😁 |
In my experience, progressive syncs goal is to reduce the risk of synchronising an application (i.e. propagating an update). In this sense, I see that revisions matters (the immutable ones like commit sha or helm versions), to understand and control what is being applied/deployed and where. |
Hum I don't think that's the role of argoCD, in your example:
That sounds like you need something above argoCD like kargo. The progressive sync in the applicationset only add logic on how to deploy applications relative to other applications, not between 2 commits for the same application. |
In case of using git generators to deploy several applications, I would agree. This said, when using cluster generators to deploy the same application into multiple clusters, I believe ApplicationSet converts themselves de-facto into an application deployment orchestrator. |
# Context: In issue argoproj#19535 we have discovered a bug where ApplicationSet Progressive Sync feature gets stuck and is not able to recover until you manually delete the existing `applicationsStatus` of the ApplicationSet affected. ## When is the bug triggered? When the ApplicationSet is preforming a progressive sync, the apps in the step being synced get the status `Pending` in the ApplicationSet `applicationStatus `. This means that the apps are gonna be synced and is waiting for the sync to start progressing. The problem is when the app needs to continue to the `Progressing` state, which means that it is syncing or has already synced the app but is waiting for it to become healthy. In other to proceed to change the state of the app in the ApplicationSet `applicationStatus` from `Pending` to `Progressing` the app needs to be syncing or if it has already synced it also needs to check that the revision that the ApplicationSet `applicationStatus` for that app matches the one in the application itself, to be sure we are checking that we synced the latest change and is not an old one. Here is the logic that performs this check: https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1078 This new check introduced in ArgoCD 2.12 causes a bug when a progressive sync is already being performed, we have some apps inside the ApplicationSet `applicationStatus` in "Pending" state and a new change is detected by the ApplicationSet, this new change makes the applicationset controller generate the new apps with the latest revision, but the apps in "Pending" inside the ApplicationSet `applicationStatus` are not updated with the new application revision, why? - Because first it will get the `applicationStatus` from the existing applicationStatus which is the one that has the old revision: https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1045 - And since the app inside the ApplicationSet `applicationStatus` is in "Pending" the revision is never updated when it enters the if statement: https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1069-L1092 (see how currentAppStatus.TargetRevision never will be updated) This means that the ApplicationSet will always think that the app is not being synced to the latest version and never progress, but in reality the app is actually in a later version but tha ApplicationSet never updated it in the apps that are in "Pending". # What does this PR? TBD
# Context: In issue argoproj#19535 we have discovered a bug where ApplicationSet Progressive Sync feature gets stuck and is not able to recover until you manually delete the existing `applicationsStatus` of the ApplicationSet affected. ## When is the bug triggered? When the ApplicationSet is preforming a progressive sync, the apps in the step being synced get the status `Pending` in the ApplicationSet `applicationStatus `. This means that the apps are gonna be synced and is waiting for the sync to start progressing. The problem is when the app needs to continue to the `Progressing` state, which means that it is syncing or has already synced the app but is waiting for it to become healthy. In other to proceed to change the state of the app in the ApplicationSet `applicationStatus` from `Pending` to `Progressing` the app needs to be syncing or if it has already synced it also needs to check that the revision that the ApplicationSet `applicationStatus` for that app matches the one in the application itself, to be sure we are checking that we synced the latest change and is not an old one. Here is the logic that performs this check: https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1078 This new check introduced in ArgoCD 2.12 causes a bug when a progressive sync is already being performed, we have some apps inside the ApplicationSet `applicationStatus` in "Pending" state and a new change is detected by the ApplicationSet, this new change makes the applicationset controller generate the new apps with the latest revision, but the apps in "Pending" inside the ApplicationSet `applicationStatus` are not updated with the new application revision, why? - Because first it will get the `applicationStatus` from the existing applicationStatus which is the one that has the old revision: https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1045 - And since the app inside the ApplicationSet `applicationStatus` is in "Pending" the revision is never updated when it enters the if statement: https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1069-L1092 (see how currentAppStatus.TargetRevision never will be updated) This means that the ApplicationSet will always think that the app is not being synced to the latest version and never progress, but in reality the app is actually in a later version but tha ApplicationSet never updated it in the apps that are in "Pending". # What does this PR? TBD
# Context: In issue argoproj#19535 we have discovered a bug where ApplicationSet Progressive Sync feature gets stuck and is not able to recover until you manually delete the existing `applicationsStatus` of the ApplicationSet affected. ## When is the bug triggered? When the ApplicationSet is preforming a progressive sync, the apps in the step being synced get the status `Pending` in the ApplicationSet `applicationStatus `. This means that the apps are gonna be synced and is waiting for the sync to start progressing. The problem is when the app needs to continue to the `Progressing` state, which means that it is syncing or has already synced the app but is waiting for it to become healthy. In other to proceed to change the state of the app in the ApplicationSet `applicationStatus` from `Pending` to `Progressing` the app needs to be syncing or if it has already synced it also needs to check that the revision that the ApplicationSet `applicationStatus` for that app matches the one in the application itself, to be sure we are checking that we synced the latest change and is not an old one. Here is the logic that performs this check: https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1078 This new check introduced in ArgoCD 2.12 causes a bug when a progressive sync is already being performed, we have some apps inside the ApplicationSet `applicationStatus` in "Pending" state and a new change is detected by the ApplicationSet, this new change makes the applicationset controller generate the new apps with the latest revision, but the apps in "Pending" inside the ApplicationSet `applicationStatus` are not updated with the new application revision, why? - Because first it will get the `applicationStatus` from the existing applicationStatus which is the one that has the old revision: https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1045 - And since the app inside the ApplicationSet `applicationStatus` is in "Pending" the revision is never updated when it enters the if statement: https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1069-L1092 (see how currentAppStatus.TargetRevision never will be updated) This means that the ApplicationSet will always think that the app is not being synced to the latest version and never progress, but in reality the app is actually in a later version but tha ApplicationSet never updated it in the apps that are in "Pending". # What does this PR? TBD
In issue argoproj#19535 we have discovered a bug where ApplicationSet Progressive Sync feature gets stuck and it never recovers and get stuck forever unless you manually delete the existing `applicationsStatus` of the ApplicationSet affected or trigger a manual sync. When the ApplicationSet is performing a progressive sync, the apps in the step being synced get the status `Pending` in the ApplicationSet `applicationStatus `. This means that the apps are gonna be synced and is waiting for the sync to start progressing. Let's set an example, applicationset generates 3 applications. In the initial moment applicationset points to commit A applicationset will generate those 3 applications and start the progressive sync, then application 2 is in Pending status, the applicationset status for application 2 is marked to targetrevision for app2 to A At this moment in time applicationset gets updated to point to commit B, since app2 is in pending state the progressive sync allows it to the app to be synced and hence the app2 is synced to commit B since applicationset targetrevision for app2 expects to be A but it's B will never move app2 from 'Pending' to 'Progressing' state. [Here](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1078) is the logic that performs this check. This new check was introduced in ArgoCD 2.12 causing this bug when a progressive sync is already being performed. - Because first it will get the `applicationStatus` from the existing applicationStatus which is the one that has the [old revision](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1045) - And since the app inside the ApplicationSet `applicationStatus` is in "Pending" the revision is never updated when it enters the [if statement](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1069-L1092) (see how currentAppStatus.TargetRevision never will be updated) This means that the ApplicationSet will always think that the app is not being synced to the latest version and never progress, but in reality the app is actually in a later version but tha ApplicationSet never updated it in the apps that are in "Pending". - This PR fixes this bug changing the logic that checks when an applications needs to be moved from Pending to Progressing, instead of rely on the targetrevision we actually rely just in the application being synced to move it. This also don't introduce a prior bug where it was cheched that the application was synced in a certain moment in time to ensure it was triggered by the applicationset controller. - Note that if someone manually sync one application of the applicationset while it's being progresively synced after merging this PR the applciationset controller will continue the rollout - Ensure that a certain revision is applied orderly in all applications generated from the applicationset it's certainly possible that a given application can be synced in a different revision than the one explicitly set in the appset Co-authored-by: Thibault Jamet <[email protected]> Co-authored-by: Carlos Rejano <[email protected]>
In issue argoproj#19535 we have discovered a bug where ApplicationSet Progressive Sync feature gets stuck and it never recovers and get stuck forever unless you manually delete the existing `applicationsStatus` of the ApplicationSet affected or trigger a manual sync. When the ApplicationSet is performing a progressive sync, the apps in the step being synced get the status `Pending` in the ApplicationSet `applicationStatus `. This means that the apps are gonna be synced and is waiting for the sync to start progressing. Let's set an example, applicationset generates 3 applications. In the initial moment applicationset points to commit A applicationset will generate those 3 applications and start the progressive sync, then application 2 is in Pending status, the applicationset status for application 2 is marked to targetrevision for app2 to A At this moment in time applicationset gets updated to point to commit B, since app2 is in pending state the progressive sync allows it to the app to be synced and hence the app2 is synced to commit B since applicationset targetrevision for app2 expects to be A but it's B will never move app2 from 'Pending' to 'Progressing' state. [Here](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1078) is the logic that performs this check. This new check was introduced in ArgoCD 2.12 causing this bug when a progressive sync is already being performed. - Because first it will get the `applicationStatus` from the existing applicationStatus which is the one that has the [old revision](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1045) - And since the app inside the ApplicationSet `applicationStatus` is in "Pending" the revision is never updated when it enters the [if statement](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1069-L1092) (see how currentAppStatus.TargetRevision never will be updated) This means that the ApplicationSet will always think that the app is not being synced to the latest version and never progress, but in reality the app is actually in a later version but tha ApplicationSet never updated it in the apps that are in "Pending". - This PR fixes this bug changing the logic that checks when an applications needs to be moved from Pending to Progressing, instead of rely on the targetrevision we actually rely just in the application being synced to move it. This also don't introduce a prior bug where it was cheched that the application was synced in a certain moment in time to ensure it was triggered by the applicationset controller. - Note that if someone manually sync one application of the applicationset while it's being progresively synced after merging this PR the applciationset controller will continue the rollout - Ensure that a certain revision is applied orderly in all applications generated from the applicationset it's certainly possible that a given application can be synced in a different revision than the one explicitly set in the appset Fixes: argoproj#19535 <!-- Note on DCO: If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the *Details* link next to the DCO action for instructions on how to resolve this. --> Checklist: * [X] Either (a) I've created an [enhancement proposal](https://github.com/argoproj/argo-cd/issues/new/choose) and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes. * [X] The title of the PR states what changed and the related issues number (used for the release note). * [X] The title of the PR conforms to the [Toolchain Guide](https://argo-cd.readthedocs.io/en/latest/developer-guide/toolchain-guide/#title-of-the-pr) * [X] I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue. * [ ] I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them. * [ ] Does this PR require documentation updates? * [ ] I've updated documentation as required by this PR. * [X] I have signed off all my commits as required by [DCO](https://github.com/argoproj/argoproj/blob/master/community/CONTRIBUTING.md#legal) * [X] I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged. * [X] My build is green ([troubleshooting builds](https://argo-cd.readthedocs.io/en/latest/developer-guide/ci/)). * [ ] My new feature complies with the [feature status](https://github.com/argoproj/argoproj/blob/master/community/feature-status.md) guidelines. * [ ] I have added a brief description of why this PR is necessary and/or what this PR solves. * [ ] Optional. My organization is added to USERS.md. * [ ] Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity). <!-- Please see [Contribution FAQs](https://argo-cd.readthedocs.io/en/latest/developer-guide/faq/) if you have questions about your pull-request. --> Co-authored-by: Thibault Jamet <[email protected]> Co-authored-by: Carlos Rejano <[email protected]> Signed-off-by: Fabián Sellés <[email protected]>
In issue argoproj#19535 we have discovered a bug where ApplicationSet Progressive Sync feature gets stuck and it never recovers and get stuck forever unless you manually delete the existing `applicationsStatus` of the ApplicationSet affected or trigger a manual sync. When the ApplicationSet is performing a progressive sync, the apps in the step being synced get the status `Pending` in the ApplicationSet `applicationStatus `. This means that the apps are gonna be synced and is waiting for the sync to start progressing. Let's set an example, applicationset generates 3 applications. In the initial moment applicationset points to commit A applicationset will generate those 3 applications and start the progressive sync, then application 2 is in Pending status, the applicationset status for application 2 is marked to targetrevision for app2 to A At this moment in time applicationset gets updated to point to commit B, since app2 is in pending state the progressive sync allows it to the app to be synced and hence the app2 is synced to commit B since applicationset targetrevision for app2 expects to be A but it's B will never move app2 from 'Pending' to 'Progressing' state. [Here](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1078) is the logic that performs this check. This new check was introduced in ArgoCD 2.12 causing this bug when a progressive sync is already being performed. - Because first it will get the `applicationStatus` from the existing applicationStatus which is the one that has the [old revision](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1045) - And since the app inside the ApplicationSet `applicationStatus` is in "Pending" the revision is never updated when it enters the [if statement](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1069-L1092) (see how currentAppStatus.TargetRevision never will be updated) This means that the ApplicationSet will always think that the app is not being synced to the latest version and never progress, but in reality the app is actually in a later version but tha ApplicationSet never updated it in the apps that are in "Pending". - This PR fixes this bug changing the logic that checks when an applications needs to be moved from Pending to Progressing, instead of rely on the targetrevision we actually rely just in the application being synced to move it. This also don't introduce a prior bug where it was cheched that the application was synced in a certain moment in time to ensure it was triggered by the applicationset controller. - Note that if someone manually sync one application of the applicationset while it's being progresively synced after merging this PR the applciationset controller will continue the rollout - Ensure that a certain revision is applied orderly in all applications generated from the applicationset it's certainly possible that a given application can be synced in a different revision than the one explicitly set in the appset Fixes: argoproj#19535 <!-- Note on DCO: If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the *Details* link next to the DCO action for instructions on how to resolve this. --> Checklist: * [X] Either (a) I've created an [enhancement proposal](https://github.com/argoproj/argo-cd/issues/new/choose) and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes. * [X] The title of the PR states what changed and the related issues number (used for the release note). * [X] The title of the PR conforms to the [Toolchain Guide](https://argo-cd.readthedocs.io/en/latest/developer-guide/toolchain-guide/#title-of-the-pr) * [X] I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue. * [ ] I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them. * [ ] Does this PR require documentation updates? * [ ] I've updated documentation as required by this PR. * [X] I have signed off all my commits as required by [DCO](https://github.com/argoproj/argoproj/blob/master/community/CONTRIBUTING.md#legal) * [X] I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged. * [X] My build is green ([troubleshooting builds](https://argo-cd.readthedocs.io/en/latest/developer-guide/ci/)). * [ ] My new feature complies with the [feature status](https://github.com/argoproj/argoproj/blob/master/community/feature-status.md) guidelines. * [ ] I have added a brief description of why this PR is necessary and/or what this PR solves. * [ ] Optional. My organization is added to USERS.md. * [ ] Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity). <!-- Please see [Contribution FAQs](https://argo-cd.readthedocs.io/en/latest/developer-guide/faq/) if you have questions about your pull-request. --> Co-authored-by: Thibault Jamet <[email protected]> Co-authored-by: Carlos Rejano <[email protected]> Signed-off-by: Fabián Sellés <[email protected]>
In issue argoproj#19535 we have discovered a bug where ApplicationSet Progressive Sync feature gets stuck and it never recovers and get stuck forever unless you manually delete the existing `applicationsStatus` of the ApplicationSet affected or trigger a manual sync. When the ApplicationSet is performing a progressive sync, the apps in the step being synced get the status `Pending` in the ApplicationSet `applicationStatus `. This means that the apps are gonna be synced and is waiting for the sync to start progressing. Let's set an example, applicationset generates 3 applications. In the initial moment applicationset points to commit A applicationset will generate those 3 applications and start the progressive sync, then application 2 is in Pending status, the applicationset status for application 2 is marked to targetrevision for app2 to A At this moment in time applicationset gets updated to point to commit B, since app2 is in pending state the progressive sync allows it to the app to be synced and hence the app2 is synced to commit B since applicationset targetrevision for app2 expects to be A but it's B will never move app2 from 'Pending' to 'Progressing' state. [Here](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1078) is the logic that performs this check. This new check was introduced in ArgoCD 2.12 causing this bug when a progressive sync is already being performed. - Because first it will get the `applicationStatus` from the existing applicationStatus which is the one that has the [old revision](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1045) - And since the app inside the ApplicationSet `applicationStatus` is in "Pending" the revision is never updated when it enters the [if statement](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1069-L1092) (see how currentAppStatus.TargetRevision never will be updated) This means that the ApplicationSet will always think that the app is not being synced to the latest version and never progress, but in reality the app is actually in a later version but tha ApplicationSet never updated it in the apps that are in "Pending". - This PR fixes this bug changing the logic that checks when an applications needs to be moved from Pending to Progressing, instead of rely on the targetrevision we actually rely just in the application being synced to move it. This also don't introduce a prior bug where it was cheched that the application was synced in a certain moment in time to ensure it was triggered by the applicationset controller. - Note that if someone manually sync one application of the applicationset while it's being progresively synced after merging this PR the applciationset controller will continue the rollout - Ensure that a certain revision is applied orderly in all applications generated from the applicationset it's certainly possible that a given application can be synced in a different revision than the one explicitly set in the appset Fixes: argoproj#19535 <!-- Note on DCO: If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the *Details* link next to the DCO action for instructions on how to resolve this. --> Checklist: * [X] Either (a) I've created an [enhancement proposal](https://github.com/argoproj/argo-cd/issues/new/choose) and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes. * [X] The title of the PR states what changed and the related issues number (used for the release note). * [X] The title of the PR conforms to the [Toolchain Guide](https://argo-cd.readthedocs.io/en/latest/developer-guide/toolchain-guide/#title-of-the-pr) * [X] I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue. * [ ] I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them. * [ ] Does this PR require documentation updates? * [ ] I've updated documentation as required by this PR. * [X] I have signed off all my commits as required by [DCO](https://github.com/argoproj/argoproj/blob/master/community/CONTRIBUTING.md#legal) * [X] I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged. * [X] My build is green ([troubleshooting builds](https://argo-cd.readthedocs.io/en/latest/developer-guide/ci/)). * [ ] My new feature complies with the [feature status](https://github.com/argoproj/argoproj/blob/master/community/feature-status.md) guidelines. * [ ] I have added a brief description of why this PR is necessary and/or what this PR solves. * [ ] Optional. My organization is added to USERS.md. * [ ] Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity). <!-- Please see [Contribution FAQs](https://argo-cd.readthedocs.io/en/latest/developer-guide/faq/) if you have questions about your pull-request. --> Co-authored-by: Thibault Jamet <[email protected]> Co-authored-by: Carlos Rejano <[email protected]> Co-authored-by: Fabián Sellés <[email protected]> Signed-off-by: Fabián Sellés <[email protected]> Signed-off-by: Thibault Jamet <[email protected]>
In issue argoproj#19535 we have discovered a bug where ApplicationSet Progressive Sync feature gets stuck and it never recovers and get stuck forever unless you manually delete the existing `applicationsStatus` of the ApplicationSet affected or trigger a manual sync. When the ApplicationSet is performing a progressive sync, the apps in the step being synced get the status `Pending` in the ApplicationSet `applicationStatus `. This means that the apps are gonna be synced and is waiting for the sync to start progressing. Let's set an example, applicationset generates 3 applications. In the initial moment applicationset points to commit A applicationset will generate those 3 applications and start the progressive sync, then application 2 is in Pending status, the applicationset status for application 2 is marked to targetrevision for app2 to A At this moment in time applicationset gets updated to point to commit B, since app2 is in pending state the progressive sync allows it to the app to be synced and hence the app2 is synced to commit B since applicationset targetrevision for app2 expects to be A but it's B will never move app2 from 'Pending' to 'Progressing' state. [Here](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1078) is the logic that performs this check. This new check was introduced in ArgoCD 2.12 causing this bug when a progressive sync is already being performed. - Because first it will get the `applicationStatus` from the existing applicationStatus which is the one that has the [old revision](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1045) - And since the app inside the ApplicationSet `applicationStatus` is in "Pending" the revision is never updated when it enters the [if statement](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1069-L1092) (see how currentAppStatus.TargetRevision never will be updated) This means that the ApplicationSet will always think that the app is not being synced to the latest version and never progress, but in reality the app is actually in a later version but tha ApplicationSet never updated it in the apps that are in "Pending". - This PR fixes this bug changing the logic that checks when an applications needs to be moved from Pending to Progressing, instead of rely on the targetrevision we actually rely just in the application being synced to move it. This also don't introduce a prior bug where it was cheched that the application was synced in a certain moment in time to ensure it was triggered by the applicationset controller. - Note that if someone manually sync one application of the applicationset while it's being progresively synced after merging this PR the applciationset controller will continue the rollout - Ensure that a certain revision is applied orderly in all applications generated from the applicationset it's certainly possible that a given application can be synced in a different revision than the one explicitly set in the appset Fixes: argoproj#19535 <!-- Note on DCO: If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the *Details* link next to the DCO action for instructions on how to resolve this. --> Checklist: * [X] Either (a) I've created an [enhancement proposal](https://github.com/argoproj/argo-cd/issues/new/choose) and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes. * [X] The title of the PR states what changed and the related issues number (used for the release note). * [X] The title of the PR conforms to the [Toolchain Guide](https://argo-cd.readthedocs.io/en/latest/developer-guide/toolchain-guide/#title-of-the-pr) * [X] I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue. * [ ] I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them. * [ ] Does this PR require documentation updates? * [ ] I've updated documentation as required by this PR. * [X] I have signed off all my commits as required by [DCO](https://github.com/argoproj/argoproj/blob/master/community/CONTRIBUTING.md#legal) * [X] I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged. * [X] My build is green ([troubleshooting builds](https://argo-cd.readthedocs.io/en/latest/developer-guide/ci/)). * [ ] My new feature complies with the [feature status](https://github.com/argoproj/argoproj/blob/master/community/feature-status.md) guidelines. * [ ] I have added a brief description of why this PR is necessary and/or what this PR solves. * [ ] Optional. My organization is added to USERS.md. * [ ] Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity). <!-- Please see [Contribution FAQs](https://argo-cd.readthedocs.io/en/latest/developer-guide/faq/) if you have questions about your pull-request. --> Co-authored-by: Thibault Jamet <[email protected]> Co-authored-by: Carlos Rejano <[email protected]> Signed-off-by: Fabián Sellés <[email protected]>
In issue argoproj#19535 we have discovered a bug where ApplicationSet Progressive Sync feature gets stuck and it never recovers and get stuck forever unless you manually delete the existing `applicationsStatus` of the ApplicationSet affected or trigger a manual sync. When the ApplicationSet is performing a progressive sync, the apps in the step being synced get the status `Pending` in the ApplicationSet `applicationStatus `. This means that the apps are gonna be synced and is waiting for the sync to start progressing. Let's set an example, applicationset generates 3 applications. In the initial moment applicationset points to commit A applicationset will generate those 3 applications and start the progressive sync, then application 2 is in Pending status, the applicationset status for application 2 is marked to targetrevision for app2 to A At this moment in time applicationset gets updated to point to commit B, since app2 is in pending state the progressive sync allows it to the app to be synced and hence the app2 is synced to commit B since applicationset targetrevision for app2 expects to be A but it's B will never move app2 from 'Pending' to 'Progressing' state. [Here](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1078) is the logic that performs this check. This new check was introduced in ArgoCD 2.12 causing this bug when a progressive sync is already being performed. - Because first it will get the `applicationStatus` from the existing applicationStatus which is the one that has the [old revision](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1045) - And since the app inside the ApplicationSet `applicationStatus` is in "Pending" the revision is never updated when it enters the [if statement](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1069-L1092) (see how currentAppStatus.TargetRevision never will be updated) This means that the ApplicationSet will always think that the app is not being synced to the latest version and never progress, but in reality the app is actually in a later version but tha ApplicationSet never updated it in the apps that are in "Pending". - This PR fixes this bug changing the logic that checks when an applications needs to be moved from Pending to Progressing, instead of rely on the targetrevision we actually rely just in the application being synced to move it. This also don't introduce a prior bug where it was cheched that the application was synced in a certain moment in time to ensure it was triggered by the applicationset controller. - Note that if someone manually sync one application of the applicationset while it's being progresively synced after merging this PR the applciationset controller will continue the rollout - Ensure that a certain revision is applied orderly in all applications generated from the applicationset it's certainly possible that a given application can be synced in a different revision than the one explicitly set in the appset Fixes: argoproj#19535 <!-- Note on DCO: If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the *Details* link next to the DCO action for instructions on how to resolve this. --> Checklist: * [X] Either (a) I've created an [enhancement proposal](https://github.com/argoproj/argo-cd/issues/new/choose) and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes. * [X] The title of the PR states what changed and the related issues number (used for the release note). * [X] The title of the PR conforms to the [Toolchain Guide](https://argo-cd.readthedocs.io/en/latest/developer-guide/toolchain-guide/#title-of-the-pr) * [X] I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue. * [ ] I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them. * [ ] Does this PR require documentation updates? * [ ] I've updated documentation as required by this PR. * [X] I have signed off all my commits as required by [DCO](https://github.com/argoproj/argoproj/blob/master/community/CONTRIBUTING.md#legal) * [X] I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged. * [X] My build is green ([troubleshooting builds](https://argo-cd.readthedocs.io/en/latest/developer-guide/ci/)). * [ ] My new feature complies with the [feature status](https://github.com/argoproj/argoproj/blob/master/community/feature-status.md) guidelines. * [ ] I have added a brief description of why this PR is necessary and/or what this PR solves. * [ ] Optional. My organization is added to USERS.md. * [ ] Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity). <!-- Please see [Contribution FAQs](https://argo-cd.readthedocs.io/en/latest/developer-guide/faq/) if you have questions about your pull-request. --> Co-authored-by: Thibault Jamet <[email protected]> Co-authored-by: Carlos Rejano <[email protected]> Signed-off-by: Fabián Sellés <[email protected]>
In issue argoproj#19535 we have discovered a bug where ApplicationSet Progressive Sync feature gets stuck and it never recovers and get stuck forever unless you manually delete the existing `applicationsStatus` of the ApplicationSet affected or trigger a manual sync. When the ApplicationSet is performing a progressive sync, the apps in the step being synced get the status `Pending` in the ApplicationSet `applicationStatus `. This means that the apps are gonna be synced and is waiting for the sync to start progressing. Let's set an example, applicationset generates 3 applications. In the initial moment applicationset points to commit A applicationset will generate those 3 applications and start the progressive sync, then application 2 is in Pending status, the applicationset status for application 2 is marked to targetrevision for app2 to A At this moment in time applicationset gets updated to point to commit B, since app2 is in pending state the progressive sync allows it to the app to be synced and hence the app2 is synced to commit B since applicationset targetrevision for app2 expects to be A but it's B will never move app2 from 'Pending' to 'Progressing' state. [Here](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1078) is the logic that performs this check. This new check was introduced in ArgoCD 2.12 causing this bug when a progressive sync is already being performed. - Because first it will get the `applicationStatus` from the existing applicationStatus which is the one that has the [old revision](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1045) - And since the app inside the ApplicationSet `applicationStatus` is in "Pending" the revision is never updated when it enters the [if statement](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1069-L1092) (see how currentAppStatus.TargetRevision never will be updated) This means that the ApplicationSet will always think that the app is not being synced to the latest version and never progress, but in reality the app is actually in a later version but tha ApplicationSet never updated it in the apps that are in "Pending". - This PR fixes this bug changing the logic that checks when an applications needs to be moved from Pending to Progressing, instead of rely on the targetrevision we actually rely just in the application being synced to move it. This also don't introduce a prior bug where it was cheched that the application was synced in a certain moment in time to ensure it was triggered by the applicationset controller. - Note that if someone manually sync one application of the applicationset while it's being progresively synced after merging this PR the applciationset controller will continue the rollout - Ensure that a certain revision is applied orderly in all applications generated from the applicationset it's certainly possible that a given application can be synced in a different revision than the one explicitly set in the appset Fixes: argoproj#19535 <!-- Note on DCO: If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the *Details* link next to the DCO action for instructions on how to resolve this. --> Checklist: * [X] Either (a) I've created an [enhancement proposal](https://github.com/argoproj/argo-cd/issues/new/choose) and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes. * [X] The title of the PR states what changed and the related issues number (used for the release note). * [X] The title of the PR conforms to the [Toolchain Guide](https://argo-cd.readthedocs.io/en/latest/developer-guide/toolchain-guide/#title-of-the-pr) * [X] I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue. * [ ] I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them. * [ ] Does this PR require documentation updates? * [ ] I've updated documentation as required by this PR. * [X] I have signed off all my commits as required by [DCO](https://github.com/argoproj/argoproj/blob/master/community/CONTRIBUTING.md#legal) * [X] I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged. * [X] My build is green ([troubleshooting builds](https://argo-cd.readthedocs.io/en/latest/developer-guide/ci/)). * [ ] My new feature complies with the [feature status](https://github.com/argoproj/argoproj/blob/master/community/feature-status.md) guidelines. * [ ] I have added a brief description of why this PR is necessary and/or what this PR solves. * [ ] Optional. My organization is added to USERS.md. * [ ] Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity). <!-- Please see [Contribution FAQs](https://argo-cd.readthedocs.io/en/latest/developer-guide/faq/) if you have questions about your pull-request. --> Co-authored-by: Thibault Jamet <[email protected]> Co-authored-by: Carlos Rejano <[email protected]> Signed-off-by: Fabián Sellés <[email protected]>
In issue argoproj#19535 we have discovered a bug where ApplicationSet Progressive Sync feature gets stuck and it never recovers and get stuck forever unless you manually delete the existing `applicationsStatus` of the ApplicationSet affected or trigger a manual sync. When the ApplicationSet is performing a progressive sync, the apps in the step being synced get the status `Pending` in the ApplicationSet `applicationStatus `. This means that the apps are gonna be synced and is waiting for the sync to start progressing. Let's set an example, applicationset generates 3 applications. In the initial moment applicationset points to commit A applicationset will generate those 3 applications and start the progressive sync, then application 2 is in Pending status, the applicationset status for application 2 is marked to targetrevision for app2 to A At this moment in time applicationset gets updated to point to commit B, since app2 is in pending state the progressive sync allows it to the app to be synced and hence the app2 is synced to commit B since applicationset targetrevision for app2 expects to be A but it's B will never move app2 from 'Pending' to 'Progressing' state. [Here](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1078) is the logic that performs this check. This new check was introduced in ArgoCD 2.12 causing this bug when a progressive sync is already being performed. - Because first it will get the `applicationStatus` from the existing applicationStatus which is the one that has the [old revision](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1045) - And since the app inside the ApplicationSet `applicationStatus` is in "Pending" the revision is never updated when it enters the [if statement](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1069-L1092) (see how currentAppStatus.TargetRevision never will be updated) This means that the ApplicationSet will always think that the app is not being synced to the latest version and never progress, but in reality the app is actually in a later version but tha ApplicationSet never updated it in the apps that are in "Pending". - This PR fixes this bug changing the logic that checks when an applications needs to be moved from Pending to Progressing, instead of rely on the targetrevision we actually rely just in the application being synced to move it. This also don't introduce a prior bug where it was cheched that the application was synced in a certain moment in time to ensure it was triggered by the applicationset controller. - Note that if someone manually sync one application of the applicationset while it's being progresively synced after merging this PR the applciationset controller will continue the rollout - Ensure that a certain revision is applied orderly in all applications generated from the applicationset it's certainly possible that a given application can be synced in a different revision than the one explicitly set in the appset Fixes: argoproj#19535 <!-- Note on DCO: If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the *Details* link next to the DCO action for instructions on how to resolve this. --> Checklist: * [X] Either (a) I've created an [enhancement proposal](https://github.com/argoproj/argo-cd/issues/new/choose) and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes. * [X] The title of the PR states what changed and the related issues number (used for the release note). * [X] The title of the PR conforms to the [Toolchain Guide](https://argo-cd.readthedocs.io/en/latest/developer-guide/toolchain-guide/#title-of-the-pr) * [X] I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue. * [ ] I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them. * [ ] Does this PR require documentation updates? * [ ] I've updated documentation as required by this PR. * [X] I have signed off all my commits as required by [DCO](https://github.com/argoproj/argoproj/blob/master/community/CONTRIBUTING.md#legal) * [X] I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged. * [X] My build is green ([troubleshooting builds](https://argo-cd.readthedocs.io/en/latest/developer-guide/ci/)). * [ ] My new feature complies with the [feature status](https://github.com/argoproj/argoproj/blob/master/community/feature-status.md) guidelines. * [ ] I have added a brief description of why this PR is necessary and/or what this PR solves. * [ ] Optional. My organization is added to USERS.md. * [ ] Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity). <!-- Please see [Contribution FAQs](https://argo-cd.readthedocs.io/en/latest/developer-guide/faq/) if you have questions about your pull-request. --> Co-authored-by: Thibault Jamet <[email protected]> Co-authored-by: Carlos Rejano <[email protected]> Signed-off-by: Fabián Sellés <[email protected]>
In issue argoproj#19535 we have discovered a bug where ApplicationSet Progressive Sync feature gets stuck and it never recovers and get stuck forever unless you manually delete the existing `applicationsStatus` of the ApplicationSet affected or trigger a manual sync. When the ApplicationSet is performing a progressive sync, the apps in the step being synced get the status `Pending` in the ApplicationSet `applicationStatus `. This means that the apps are gonna be synced and is waiting for the sync to start progressing. Let's set an example, applicationset generates 3 applications. In the initial moment applicationset points to commit A applicationset will generate those 3 applications and start the progressive sync, then application 2 is in Pending status, the applicationset status for application 2 is marked to targetrevision for app2 to A At this moment in time applicationset gets updated to point to commit B, since app2 is in pending state the progressive sync allows it to the app to be synced and hence the app2 is synced to commit B since applicationset targetrevision for app2 expects to be A but it's B will never move app2 from 'Pending' to 'Progressing' state. [Here](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1078) is the logic that performs this check. This new check was introduced in ArgoCD 2.12 causing this bug when a progressive sync is already being performed. - Because first it will get the `applicationStatus` from the existing applicationStatus which is the one that has the [old revision](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1045) - And since the app inside the ApplicationSet `applicationStatus` is in "Pending" the revision is never updated when it enters the [if statement](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1069-L1092) (see how currentAppStatus.TargetRevision never will be updated) This means that the ApplicationSet will always think that the app is not being synced to the latest version and never progress, but in reality the app is actually in a later version but tha ApplicationSet never updated it in the apps that are in "Pending". - This PR fixes this bug changing the logic that checks when an applications needs to be moved from Pending to Progressing, instead of rely on the targetrevision we actually rely just in the application being synced to move it. This also don't introduce a prior bug where it was cheched that the application was synced in a certain moment in time to ensure it was triggered by the applicationset controller. - Note that if someone manually sync one application of the applicationset while it's being progresively synced after merging this PR the applciationset controller will continue the rollout - Ensure that a certain revision is applied orderly in all applications generated from the applicationset it's certainly possible that a given application can be synced in a different revision than the one explicitly set in the appset Fixes: argoproj#19535 <!-- Note on DCO: If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the *Details* link next to the DCO action for instructions on how to resolve this. --> Checklist: * [X] Either (a) I've created an [enhancement proposal](https://github.com/argoproj/argo-cd/issues/new/choose) and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes. * [X] The title of the PR states what changed and the related issues number (used for the release note). * [X] The title of the PR conforms to the [Toolchain Guide](https://argo-cd.readthedocs.io/en/latest/developer-guide/toolchain-guide/#title-of-the-pr) * [X] I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue. * [ ] I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them. * [ ] Does this PR require documentation updates? * [ ] I've updated documentation as required by this PR. * [X] I have signed off all my commits as required by [DCO](https://github.com/argoproj/argoproj/blob/master/community/CONTRIBUTING.md#legal) * [X] I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged. * [X] My build is green ([troubleshooting builds](https://argo-cd.readthedocs.io/en/latest/developer-guide/ci/)). * [ ] My new feature complies with the [feature status](https://github.com/argoproj/argoproj/blob/master/community/feature-status.md) guidelines. * [ ] I have added a brief description of why this PR is necessary and/or what this PR solves. * [ ] Optional. My organization is added to USERS.md. * [ ] Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity). <!-- Please see [Contribution FAQs](https://argo-cd.readthedocs.io/en/latest/developer-guide/faq/) if you have questions about your pull-request. --> Co-authored-by: Thibault Jamet <[email protected]> Co-authored-by: Carlos Rejano <[email protected]> Signed-off-by: Fabián Sellés <[email protected]>
In issue argoproj#19535 we have discovered a bug where ApplicationSet Progressive Sync feature gets stuck and it never recovers and get stuck forever unless you manually delete the existing `applicationsStatus` of the ApplicationSet affected or trigger a manual sync. When the ApplicationSet is performing a progressive sync, the apps in the step being synced get the status `Pending` in the ApplicationSet `applicationStatus `. This means that the apps are gonna be synced and is waiting for the sync to start progressing. Let's set an example, applicationset generates 3 applications. In the initial moment applicationset points to commit A applicationset will generate those 3 applications and start the progressive sync, then application 2 is in Pending status, the applicationset status for application 2 is marked to targetrevision for app2 to A At this moment in time applicationset gets updated to point to commit B, since app2 is in pending state the progressive sync allows it to the app to be synced and hence the app2 is synced to commit B since applicationset targetrevision for app2 expects to be A but it's B will never move app2 from 'Pending' to 'Progressing' state. [Here](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1078) is the logic that performs this check. This new check was introduced in ArgoCD 2.12 causing this bug when a progressive sync is already being performed. - Because first it will get the `applicationStatus` from the existing applicationStatus which is the one that has the [old revision](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1045) - And since the app inside the ApplicationSet `applicationStatus` is in "Pending" the revision is never updated when it enters the [if statement](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1069-L1092) (see how currentAppStatus.TargetRevision never will be updated) This means that the ApplicationSet will always think that the app is not being synced to the latest version and never progress, but in reality the app is actually in a later version but tha ApplicationSet never updated it in the apps that are in "Pending". - This PR fixes this bug changing the logic that checks when an applications needs to be moved from Pending to Progressing, instead of rely on the targetrevision we actually rely just in the application being synced to move it. This also don't introduce a prior bug where it was cheched that the application was synced in a certain moment in time to ensure it was triggered by the applicationset controller. - Note that if someone manually sync one application of the applicationset while it's being progresively synced after merging this PR the applciationset controller will continue the rollout - Ensure that a certain revision is applied orderly in all applications generated from the applicationset it's certainly possible that a given application can be synced in a different revision than the one explicitly set in the appset Fixes: argoproj#19535 <!-- Note on DCO: If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the *Details* link next to the DCO action for instructions on how to resolve this. --> Checklist: * [X] Either (a) I've created an [enhancement proposal](https://github.com/argoproj/argo-cd/issues/new/choose) and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes. * [X] The title of the PR states what changed and the related issues number (used for the release note). * [X] The title of the PR conforms to the [Toolchain Guide](https://argo-cd.readthedocs.io/en/latest/developer-guide/toolchain-guide/#title-of-the-pr) * [X] I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue. * [ ] I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them. * [ ] Does this PR require documentation updates? * [ ] I've updated documentation as required by this PR. * [X] I have signed off all my commits as required by [DCO](https://github.com/argoproj/argoproj/blob/master/community/CONTRIBUTING.md#legal) * [X] I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged. * [X] My build is green ([troubleshooting builds](https://argo-cd.readthedocs.io/en/latest/developer-guide/ci/)). * [ ] My new feature complies with the [feature status](https://github.com/argoproj/argoproj/blob/master/community/feature-status.md) guidelines. * [ ] I have added a brief description of why this PR is necessary and/or what this PR solves. * [ ] Optional. My organization is added to USERS.md. * [ ] Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity). <!-- Please see [Contribution FAQs](https://argo-cd.readthedocs.io/en/latest/developer-guide/faq/) if you have questions about your pull-request. --> Co-authored-by: Thibault Jamet <[email protected]> Co-authored-by: Carlos Rejano <[email protected]> Signed-off-by: Fabián Sellés <[email protected]>
In issue argoproj#19535 we have discovered a bug where ApplicationSet Progressive Sync feature gets stuck and it never recovers and get stuck forever unless you manually delete the existing `applicationsStatus` of the ApplicationSet affected or trigger a manual sync. When the ApplicationSet is performing a progressive sync, the apps in the step being synced get the status `Pending` in the ApplicationSet `applicationStatus `. This means that the apps are gonna be synced and is waiting for the sync to start progressing. Let's set an example, applicationset generates 3 applications. In the initial moment applicationset points to commit A applicationset will generate those 3 applications and start the progressive sync, then application 2 is in Pending status, the applicationset status for application 2 is marked to targetrevision for app2 to A At this moment in time applicationset gets updated to point to commit B, since app2 is in pending state the progressive sync allows it to the app to be synced and hence the app2 is synced to commit B since applicationset targetrevision for app2 expects to be A but it's B will never move app2 from 'Pending' to 'Progressing' state. [Here](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1078) is the logic that performs this check. This new check was introduced in ArgoCD 2.12 causing this bug when a progressive sync is already being performed. - Because first it will get the `applicationStatus` from the existing applicationStatus which is the one that has the [old revision](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1045) - And since the app inside the ApplicationSet `applicationStatus` is in "Pending" the revision is never updated when it enters the [if statement](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1069-L1092) (see how currentAppStatus.TargetRevision never will be updated) This means that the ApplicationSet will always think that the app is not being synced to the latest version and never progress, but in reality the app is actually in a later version but tha ApplicationSet never updated it in the apps that are in "Pending". - This PR fixes this bug changing the logic that checks when an applications needs to be moved from Pending to Progressing, instead of rely on the targetrevision we actually rely just in the application being synced to move it. This also don't introduce a prior bug where it was cheched that the application was synced in a certain moment in time to ensure it was triggered by the applicationset controller. - Note that if someone manually sync one application of the applicationset while it's being progresively synced after merging this PR the applciationset controller will continue the rollout - Ensure that a certain revision is applied orderly in all applications generated from the applicationset it's certainly possible that a given application can be synced in a different revision than the one explicitly set in the appset Fixes: argoproj#19535 <!-- Note on DCO: If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the *Details* link next to the DCO action for instructions on how to resolve this. --> Checklist: * [X] Either (a) I've created an [enhancement proposal](https://github.com/argoproj/argo-cd/issues/new/choose) and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes. * [X] The title of the PR states what changed and the related issues number (used for the release note). * [X] The title of the PR conforms to the [Toolchain Guide](https://argo-cd.readthedocs.io/en/latest/developer-guide/toolchain-guide/#title-of-the-pr) * [X] I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue. * [ ] I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them. * [ ] Does this PR require documentation updates? * [ ] I've updated documentation as required by this PR. * [X] I have signed off all my commits as required by [DCO](https://github.com/argoproj/argoproj/blob/master/community/CONTRIBUTING.md#legal) * [X] I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged. * [X] My build is green ([troubleshooting builds](https://argo-cd.readthedocs.io/en/latest/developer-guide/ci/)). * [ ] My new feature complies with the [feature status](https://github.com/argoproj/argoproj/blob/master/community/feature-status.md) guidelines. * [ ] I have added a brief description of why this PR is necessary and/or what this PR solves. * [ ] Optional. My organization is added to USERS.md. * [ ] Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity). <!-- Please see [Contribution FAQs](https://argo-cd.readthedocs.io/en/latest/developer-guide/faq/) if you have questions about your pull-request. --> Co-authored-by: Thibault Jamet <[email protected]> Co-authored-by: Carlos Rejano <[email protected]> Signed-off-by: Fabián Sellés <[email protected]>
In issue argoproj#19535 we have discovered a bug where ApplicationSet Progressive Sync feature gets stuck and it never recovers and get stuck forever unless you manually delete the existing `applicationsStatus` of the ApplicationSet affected or trigger a manual sync. When the ApplicationSet is performing a progressive sync, the apps in the step being synced get the status `Pending` in the ApplicationSet `applicationStatus `. This means that the apps are gonna be synced and is waiting for the sync to start progressing. Let's set an example, applicationset generates 3 applications. In the initial moment applicationset points to commit A applicationset will generate those 3 applications and start the progressive sync, then application 2 is in Pending status, the applicationset status for application 2 is marked to targetrevision for app2 to A At this moment in time applicationset gets updated to point to commit B, since app2 is in pending state the progressive sync allows it to the app to be synced and hence the app2 is synced to commit B since applicationset targetrevision for app2 expects to be A but it's B will never move app2 from 'Pending' to 'Progressing' state. [Here](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1078) is the logic that performs this check. This new check was introduced in ArgoCD 2.12 causing this bug when a progressive sync is already being performed. - Because first it will get the `applicationStatus` from the existing applicationStatus which is the one that has the [old revision](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1045) - And since the app inside the ApplicationSet `applicationStatus` is in "Pending" the revision is never updated when it enters the [if statement](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1069-L1092) (see how currentAppStatus.TargetRevision never will be updated) This means that the ApplicationSet will always think that the app is not being synced to the latest version and never progress, but in reality the app is actually in a later version but tha ApplicationSet never updated it in the apps that are in "Pending". - This PR fixes this bug changing the logic that checks when an applications needs to be moved from Pending to Progressing, instead of rely on the targetrevision we actually rely just in the application being synced to move it. This also don't introduce a prior bug where it was cheched that the application was synced in a certain moment in time to ensure it was triggered by the applicationset controller. - Note that if someone manually sync one application of the applicationset while it's being progresively synced after merging this PR the applciationset controller will continue the rollout - Ensure that a certain revision is applied orderly in all applications generated from the applicationset it's certainly possible that a given application can be synced in a different revision than the one explicitly set in the appset Fixes: argoproj#19535 <!-- Note on DCO: If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the *Details* link next to the DCO action for instructions on how to resolve this. --> Checklist: * [X] Either (a) I've created an [enhancement proposal](https://github.com/argoproj/argo-cd/issues/new/choose) and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes. * [X] The title of the PR states what changed and the related issues number (used for the release note). * [X] The title of the PR conforms to the [Toolchain Guide](https://argo-cd.readthedocs.io/en/latest/developer-guide/toolchain-guide/#title-of-the-pr) * [X] I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue. * [ ] I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them. * [ ] Does this PR require documentation updates? * [ ] I've updated documentation as required by this PR. * [X] I have signed off all my commits as required by [DCO](https://github.com/argoproj/argoproj/blob/master/community/CONTRIBUTING.md#legal) * [X] I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged. * [X] My build is green ([troubleshooting builds](https://argo-cd.readthedocs.io/en/latest/developer-guide/ci/)). * [ ] My new feature complies with the [feature status](https://github.com/argoproj/argoproj/blob/master/community/feature-status.md) guidelines. * [ ] I have added a brief description of why this PR is necessary and/or what this PR solves. * [ ] Optional. My organization is added to USERS.md. * [ ] Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity). <!-- Please see [Contribution FAQs](https://argo-cd.readthedocs.io/en/latest/developer-guide/faq/) if you have questions about your pull-request. --> Co-authored-by: Thibault Jamet <[email protected]> Co-authored-by: Carlos Rejano <[email protected]> Signed-off-by: Fabián Sellés <[email protected]>
In issue argoproj#19535 we have discovered a bug where ApplicationSet Progressive Sync feature gets stuck and it never recovers and get stuck forever unless you manually delete the existing `applicationsStatus` of the ApplicationSet affected or trigger a manual sync. When the ApplicationSet is performing a progressive sync, the apps in the step being synced get the status `Pending` in the ApplicationSet `applicationStatus `. This means that the apps are gonna be synced and is waiting for the sync to start progressing. Let's set an example, applicationset generates 3 applications. In the initial moment applicationset points to commit A applicationset will generate those 3 applications and start the progressive sync, then application 2 is in Pending status, the applicationset status for application 2 is marked to targetrevision for app2 to A At this moment in time applicationset gets updated to point to commit B, since app2 is in pending state the progressive sync allows it to the app to be synced and hence the app2 is synced to commit B since applicationset targetrevision for app2 expects to be A but it's B will never move app2 from 'Pending' to 'Progressing' state. [Here](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1078) is the logic that performs this check. This new check was introduced in ArgoCD 2.12 causing this bug when a progressive sync is already being performed. - Because first it will get the `applicationStatus` from the existing applicationStatus which is the one that has the [old revision](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1045) - And since the app inside the ApplicationSet `applicationStatus` is in "Pending" the revision is never updated when it enters the [if statement](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1069-L1092) (see how currentAppStatus.TargetRevision never will be updated) This means that the ApplicationSet will always think that the app is not being synced to the latest version and never progress, but in reality the app is actually in a later version but tha ApplicationSet never updated it in the apps that are in "Pending". - This PR fixes this bug changing the logic that checks when an applications needs to be moved from Pending to Progressing, instead of rely on the targetrevision we actually rely just in the application being synced to move it. This also don't introduce a prior bug where it was cheched that the application was synced in a certain moment in time to ensure it was triggered by the applicationset controller. - Note that if someone manually sync one application of the applicationset while it's being progresively synced after merging this PR the applciationset controller will continue the rollout - Ensure that a certain revision is applied orderly in all applications generated from the applicationset it's certainly possible that a given application can be synced in a different revision than the one explicitly set in the appset Fixes: argoproj#19535 <!-- Note on DCO: If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the *Details* link next to the DCO action for instructions on how to resolve this. --> Checklist: * [X] Either (a) I've created an [enhancement proposal](https://github.com/argoproj/argo-cd/issues/new/choose) and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes. * [X] The title of the PR states what changed and the related issues number (used for the release note). * [X] The title of the PR conforms to the [Toolchain Guide](https://argo-cd.readthedocs.io/en/latest/developer-guide/toolchain-guide/#title-of-the-pr) * [X] I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue. * [ ] I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them. * [ ] Does this PR require documentation updates? * [ ] I've updated documentation as required by this PR. * [X] I have signed off all my commits as required by [DCO](https://github.com/argoproj/argoproj/blob/master/community/CONTRIBUTING.md#legal) * [X] I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged. * [X] My build is green ([troubleshooting builds](https://argo-cd.readthedocs.io/en/latest/developer-guide/ci/)). * [ ] My new feature complies with the [feature status](https://github.com/argoproj/argoproj/blob/master/community/feature-status.md) guidelines. * [ ] I have added a brief description of why this PR is necessary and/or what this PR solves. * [ ] Optional. My organization is added to USERS.md. * [ ] Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity). <!-- Please see [Contribution FAQs](https://argo-cd.readthedocs.io/en/latest/developer-guide/faq/) if you have questions about your pull-request. --> Co-authored-by: Thibault Jamet <[email protected]> Co-authored-by: Carlos Rejano <[email protected]> Signed-off-by: Fabián Sellés <[email protected]>
In issue argoproj#19535 we have discovered a bug where ApplicationSet Progressive Sync feature gets stuck and it never recovers and get stuck forever unless you manually delete the existing `applicationsStatus` of the ApplicationSet affected or trigger a manual sync. When the ApplicationSet is performing a progressive sync, the apps in the step being synced get the status `Pending` in the ApplicationSet `applicationStatus `. This means that the apps are gonna be synced and is waiting for the sync to start progressing. Let's set an example, applicationset generates 3 applications. In the initial moment applicationset points to commit A applicationset will generate those 3 applications and start the progressive sync, then application 2 is in Pending status, the applicationset status for application 2 is marked to targetrevision for app2 to A At this moment in time applicationset gets updated to point to commit B, since app2 is in pending state the progressive sync allows it to the app to be synced and hence the app2 is synced to commit B since applicationset targetrevision for app2 expects to be A but it's B will never move app2 from 'Pending' to 'Progressing' state. [Here](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1078) is the logic that performs this check. This new check was introduced in ArgoCD 2.12 causing this bug when a progressive sync is already being performed. - Because first it will get the `applicationStatus` from the existing applicationStatus which is the one that has the [old revision](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1045) - And since the app inside the ApplicationSet `applicationStatus` is in "Pending" the revision is never updated when it enters the [if statement](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1069-L1092) (see how currentAppStatus.TargetRevision never will be updated) This means that the ApplicationSet will always think that the app is not being synced to the latest version and never progress, but in reality the app is actually in a later version but tha ApplicationSet never updated it in the apps that are in "Pending". - This PR fixes this bug changing the logic that checks when an applications needs to be moved from Pending to Progressing, instead of rely on the targetrevision we actually rely just in the application being synced to move it. This also don't introduce a prior bug where it was cheched that the application was synced in a certain moment in time to ensure it was triggered by the applicationset controller. - Note that if someone manually sync one application of the applicationset while it's being progresively synced after merging this PR the applciationset controller will continue the rollout - Ensure that a certain revision is applied orderly in all applications generated from the applicationset it's certainly possible that a given application can be synced in a different revision than the one explicitly set in the appset Fixes: argoproj#19535 <!-- Note on DCO: If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the *Details* link next to the DCO action for instructions on how to resolve this. --> Checklist: * [X] Either (a) I've created an [enhancement proposal](https://github.com/argoproj/argo-cd/issues/new/choose) and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes. * [X] The title of the PR states what changed and the related issues number (used for the release note). * [X] The title of the PR conforms to the [Toolchain Guide](https://argo-cd.readthedocs.io/en/latest/developer-guide/toolchain-guide/#title-of-the-pr) * [X] I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue. * [ ] I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them. * [ ] Does this PR require documentation updates? * [ ] I've updated documentation as required by this PR. * [X] I have signed off all my commits as required by [DCO](https://github.com/argoproj/argoproj/blob/master/community/CONTRIBUTING.md#legal) * [X] I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged. * [X] My build is green ([troubleshooting builds](https://argo-cd.readthedocs.io/en/latest/developer-guide/ci/)). * [ ] My new feature complies with the [feature status](https://github.com/argoproj/argoproj/blob/master/community/feature-status.md) guidelines. * [ ] I have added a brief description of why this PR is necessary and/or what this PR solves. * [ ] Optional. My organization is added to USERS.md. * [ ] Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity). <!-- Please see [Contribution FAQs](https://argo-cd.readthedocs.io/en/latest/developer-guide/faq/) if you have questions about your pull-request. --> Co-authored-by: Thibault Jamet <[email protected]> Co-authored-by: Carlos Rejano <[email protected]> Signed-off-by: Fabián Sellés <[email protected]>
In issue argoproj#19535 we have discovered a bug where ApplicationSet Progressive Sync feature gets stuck and it never recovers and get stuck forever unless you manually delete the existing `applicationsStatus` of the ApplicationSet affected or trigger a manual sync. When the ApplicationSet is performing a progressive sync, the apps in the step being synced get the status `Pending` in the ApplicationSet `applicationStatus `. This means that the apps are gonna be synced and is waiting for the sync to start progressing. Let's set an example, applicationset generates 3 applications. In the initial moment applicationset points to commit A applicationset will generate those 3 applications and start the progressive sync, then application 2 is in Pending status, the applicationset status for application 2 is marked to targetrevision for app2 to A At this moment in time applicationset gets updated to point to commit B, since app2 is in pending state the progressive sync allows it to the app to be synced and hence the app2 is synced to commit B since applicationset targetrevision for app2 expects to be A but it's B will never move app2 from 'Pending' to 'Progressing' state. [Here](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1078) is the logic that performs this check. This new check was introduced in ArgoCD 2.12 causing this bug when a progressive sync is already being performed. - Because first it will get the `applicationStatus` from the existing applicationStatus which is the one that has the [old revision](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1045) - And since the app inside the ApplicationSet `applicationStatus` is in "Pending" the revision is never updated when it enters the [if statement](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1069-L1092) (see how currentAppStatus.TargetRevision never will be updated) This means that the ApplicationSet will always think that the app is not being synced to the latest version and never progress, but in reality the app is actually in a later version but tha ApplicationSet never updated it in the apps that are in "Pending". - This PR fixes this bug changing the logic that checks when an applications needs to be moved from Pending to Progressing, instead of rely on the targetrevision we actually rely just in the application being synced to move it. This also don't introduce a prior bug where it was cheched that the application was synced in a certain moment in time to ensure it was triggered by the applicationset controller. - Note that if someone manually sync one application of the applicationset while it's being progresively synced after merging this PR the applciationset controller will continue the rollout - Ensure that a certain revision is applied orderly in all applications generated from the applicationset it's certainly possible that a given application can be synced in a different revision than the one explicitly set in the appset Fixes: argoproj#19535 <!-- Note on DCO: If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the *Details* link next to the DCO action for instructions on how to resolve this. --> Checklist: * [X] Either (a) I've created an [enhancement proposal](https://github.com/argoproj/argo-cd/issues/new/choose) and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes. * [X] The title of the PR states what changed and the related issues number (used for the release note). * [X] The title of the PR conforms to the [Toolchain Guide](https://argo-cd.readthedocs.io/en/latest/developer-guide/toolchain-guide/#title-of-the-pr) * [X] I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue. * [ ] I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them. * [ ] Does this PR require documentation updates? * [ ] I've updated documentation as required by this PR. * [X] I have signed off all my commits as required by [DCO](https://github.com/argoproj/argoproj/blob/master/community/CONTRIBUTING.md#legal) * [X] I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged. * [X] My build is green ([troubleshooting builds](https://argo-cd.readthedocs.io/en/latest/developer-guide/ci/)). * [ ] My new feature complies with the [feature status](https://github.com/argoproj/argoproj/blob/master/community/feature-status.md) guidelines. * [ ] I have added a brief description of why this PR is necessary and/or what this PR solves. * [ ] Optional. My organization is added to USERS.md. * [ ] Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity). <!-- Please see [Contribution FAQs](https://argo-cd.readthedocs.io/en/latest/developer-guide/faq/) if you have questions about your pull-request. --> Co-authored-by: Thibault Jamet <[email protected]> Co-authored-by: Carlos Rejano <[email protected]> Signed-off-by: Fabián Sellés <[email protected]>
In issue argoproj#19535 we have discovered a bug where ApplicationSet Progressive Sync feature gets stuck and it never recovers and get stuck forever unless you manually delete the existing `applicationsStatus` of the ApplicationSet affected or trigger a manual sync. When the ApplicationSet is performing a progressive sync, the apps in the step being synced get the status `Pending` in the ApplicationSet `applicationStatus `. This means that the apps are gonna be synced and is waiting for the sync to start progressing. Let's set an example, applicationset generates 3 applications. In the initial moment applicationset points to commit A applicationset will generate those 3 applications and start the progressive sync, then application 2 is in Pending status, the applicationset status for application 2 is marked to targetrevision for app2 to A At this moment in time applicationset gets updated to point to commit B, since app2 is in pending state the progressive sync allows it to the app to be synced and hence the app2 is synced to commit B since applicationset targetrevision for app2 expects to be A but it's B will never move app2 from 'Pending' to 'Progressing' state. [Here](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1078) is the logic that performs this check. This new check was introduced in ArgoCD 2.12 causing this bug when a progressive sync is already being performed. - Because first it will get the `applicationStatus` from the existing applicationStatus which is the one that has the [old revision](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1045) - And since the app inside the ApplicationSet `applicationStatus` is in "Pending" the revision is never updated when it enters the [if statement](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1069-L1092) (see how currentAppStatus.TargetRevision never will be updated) This means that the ApplicationSet will always think that the app is not being synced to the latest version and never progress, but in reality the app is actually in a later version but tha ApplicationSet never updated it in the apps that are in "Pending". - This PR fixes this bug changing the logic that checks when an applications needs to be moved from Pending to Progressing, instead of rely on the targetrevision we actually rely just in the application being synced to move it. This also don't introduce a prior bug where it was cheched that the application was synced in a certain moment in time to ensure it was triggered by the applicationset controller. - Note that if someone manually sync one application of the applicationset while it's being progresively synced after merging this PR the applciationset controller will continue the rollout - Ensure that a certain revision is applied orderly in all applications generated from the applicationset it's certainly possible that a given application can be synced in a different revision than the one explicitly set in the appset Fixes: argoproj#19535 <!-- Note on DCO: If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the *Details* link next to the DCO action for instructions on how to resolve this. --> Checklist: * [X] Either (a) I've created an [enhancement proposal](https://github.com/argoproj/argo-cd/issues/new/choose) and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes. * [X] The title of the PR states what changed and the related issues number (used for the release note). * [X] The title of the PR conforms to the [Toolchain Guide](https://argo-cd.readthedocs.io/en/latest/developer-guide/toolchain-guide/#title-of-the-pr) * [X] I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue. * [ ] I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them. * [ ] Does this PR require documentation updates? * [ ] I've updated documentation as required by this PR. * [X] I have signed off all my commits as required by [DCO](https://github.com/argoproj/argoproj/blob/master/community/CONTRIBUTING.md#legal) * [X] I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged. * [X] My build is green ([troubleshooting builds](https://argo-cd.readthedocs.io/en/latest/developer-guide/ci/)). * [ ] My new feature complies with the [feature status](https://github.com/argoproj/argoproj/blob/master/community/feature-status.md) guidelines. * [ ] I have added a brief description of why this PR is necessary and/or what this PR solves. * [ ] Optional. My organization is added to USERS.md. * [ ] Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity). <!-- Please see [Contribution FAQs](https://argo-cd.readthedocs.io/en/latest/developer-guide/faq/) if you have questions about your pull-request. --> Co-authored-by: Thibault Jamet <[email protected]> Co-authored-by: Carlos Rejano <[email protected]> Signed-off-by: Fabián Sellés <[email protected]>
In issue argoproj#19535 we have discovered a bug where ApplicationSet Progressive Sync feature gets stuck and it never recovers and get stuck forever unless you manually delete the existing `applicationsStatus` of the ApplicationSet affected or trigger a manual sync. When the ApplicationSet is performing a progressive sync, the apps in the step being synced get the status `Pending` in the ApplicationSet `applicationStatus `. This means that the apps are gonna be synced and is waiting for the sync to start progressing. Let's set an example, applicationset generates 3 applications. In the initial moment applicationset points to commit A applicationset will generate those 3 applications and start the progressive sync, then application 2 is in Pending status, the applicationset status for application 2 is marked to targetrevision for app2 to A At this moment in time applicationset gets updated to point to commit B, since app2 is in pending state the progressive sync allows it to the app to be synced and hence the app2 is synced to commit B since applicationset targetrevision for app2 expects to be A but it's B will never move app2 from 'Pending' to 'Progressing' state. [Here](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1078) is the logic that performs this check. This new check was introduced in ArgoCD 2.12 causing this bug when a progressive sync is already being performed. - Because first it will get the `applicationStatus` from the existing applicationStatus which is the one that has the [old revision](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1045) - And since the app inside the ApplicationSet `applicationStatus` is in "Pending" the revision is never updated when it enters the [if statement](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1069-L1092) (see how currentAppStatus.TargetRevision never will be updated) This means that the ApplicationSet will always think that the app is not being synced to the latest version and never progress, but in reality the app is actually in a later version but tha ApplicationSet never updated it in the apps that are in "Pending". - This PR fixes this bug changing the logic that checks when an applications needs to be moved from Pending to Progressing, instead of rely on the targetrevision we actually rely just in the application being synced to move it. This also don't introduce a prior bug where it was cheched that the application was synced in a certain moment in time to ensure it was triggered by the applicationset controller. - Note that if someone manually sync one application of the applicationset while it's being progresively synced after merging this PR the applciationset controller will continue the rollout - Ensure that a certain revision is applied orderly in all applications generated from the applicationset it's certainly possible that a given application can be synced in a different revision than the one explicitly set in the appset Fixes: argoproj#19535 <!-- Note on DCO: If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the *Details* link next to the DCO action for instructions on how to resolve this. --> Checklist: * [X] Either (a) I've created an [enhancement proposal](https://github.com/argoproj/argo-cd/issues/new/choose) and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes. * [X] The title of the PR states what changed and the related issues number (used for the release note). * [X] The title of the PR conforms to the [Toolchain Guide](https://argo-cd.readthedocs.io/en/latest/developer-guide/toolchain-guide/#title-of-the-pr) * [X] I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue. * [ ] I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them. * [ ] Does this PR require documentation updates? * [ ] I've updated documentation as required by this PR. * [X] I have signed off all my commits as required by [DCO](https://github.com/argoproj/argoproj/blob/master/community/CONTRIBUTING.md#legal) * [X] I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged. * [X] My build is green ([troubleshooting builds](https://argo-cd.readthedocs.io/en/latest/developer-guide/ci/)). * [ ] My new feature complies with the [feature status](https://github.com/argoproj/argoproj/blob/master/community/feature-status.md) guidelines. * [ ] I have added a brief description of why this PR is necessary and/or what this PR solves. * [ ] Optional. My organization is added to USERS.md. * [ ] Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity). <!-- Please see [Contribution FAQs](https://argo-cd.readthedocs.io/en/latest/developer-guide/faq/) if you have questions about your pull-request. --> Co-authored-by: Thibault Jamet <[email protected]> Co-authored-by: Carlos Rejano <[email protected]> Signed-off-by: Fabián Sellés <[email protected]>
In issue argoproj#19535 we have discovered a bug where ApplicationSet Progressive Sync feature gets stuck and it never recovers and get stuck forever unless you manually delete the existing `applicationsStatus` of the ApplicationSet affected or trigger a manual sync. When the ApplicationSet is performing a progressive sync, the apps in the step being synced get the status `Pending` in the ApplicationSet `applicationStatus `. This means that the apps are gonna be synced and is waiting for the sync to start progressing. Let's set an example, applicationset generates 3 applications. In the initial moment applicationset points to commit A applicationset will generate those 3 applications and start the progressive sync, then application 2 is in Pending status, the applicationset status for application 2 is marked to targetrevision for app2 to A At this moment in time applicationset gets updated to point to commit B, since app2 is in pending state the progressive sync allows it to the app to be synced and hence the app2 is synced to commit B since applicationset targetrevision for app2 expects to be A but it's B will never move app2 from 'Pending' to 'Progressing' state. [Here](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1078) is the logic that performs this check. This new check was introduced in ArgoCD 2.12 causing this bug when a progressive sync is already being performed. - Because first it will get the `applicationStatus` from the existing applicationStatus which is the one that has the [old revision](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1045) - And since the app inside the ApplicationSet `applicationStatus` is in "Pending" the revision is never updated when it enters the [if statement](https://github.com/argoproj/argo-cd/blob/master/applicationset/controllers/applicationset_controller.go#L1069-L1092) (see how currentAppStatus.TargetRevision never will be updated) This means that the ApplicationSet will always think that the app is not being synced to the latest version and never progress, but in reality the app is actually in a later version but tha ApplicationSet never updated it in the apps that are in "Pending". - This PR fixes this bug changing the logic that checks when an applications needs to be moved from Pending to Progressing, instead of rely on the targetrevision we actually rely just in the application being synced to move it. This also don't introduce a prior bug where it was cheched that the application was synced in a certain moment in time to ensure it was triggered by the applicationset controller. - Note that if someone manually sync one application of the applicationset while it's being progresively synced after merging this PR the applciationset controller will continue the rollout - Ensure that a certain revision is applied orderly in all applications generated from the applicationset it's certainly possible that a given application can be synced in a different revision than the one explicitly set in the appset Fixes: argoproj#19535 <!-- Note on DCO: If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the *Details* link next to the DCO action for instructions on how to resolve this. --> Checklist: * [X] Either (a) I've created an [enhancement proposal](https://github.com/argoproj/argo-cd/issues/new/choose) and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes. * [X] The title of the PR states what changed and the related issues number (used for the release note). * [X] The title of the PR conforms to the [Toolchain Guide](https://argo-cd.readthedocs.io/en/latest/developer-guide/toolchain-guide/#title-of-the-pr) * [X] I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue. * [ ] I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them. * [ ] Does this PR require documentation updates? * [ ] I've updated documentation as required by this PR. * [X] I have signed off all my commits as required by [DCO](https://github.com/argoproj/argoproj/blob/master/community/CONTRIBUTING.md#legal) * [X] I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged. * [X] My build is green ([troubleshooting builds](https://argo-cd.readthedocs.io/en/latest/developer-guide/ci/)). * [ ] My new feature complies with the [feature status](https://github.com/argoproj/argoproj/blob/master/community/feature-status.md) guidelines. * [ ] I have added a brief description of why this PR is necessary and/or what this PR solves. * [ ] Optional. My organization is added to USERS.md. * [ ] Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity). <!-- Please see [Contribution FAQs](https://argo-cd.readthedocs.io/en/latest/developer-guide/faq/) if you have questions about your pull-request. --> Co-authored-by: Thibault Jamet <[email protected]> Co-authored-by: Carlos Rejano <[email protected]> Signed-off-by: Fabián Sellés <[email protected]>
Describe the bug
We have recently upgraded to ArgoCD
2.12
. We are using ApplicationSets withRollingSync
strategy to deploy our applications. Since the upgrade we have found that the rolling sync gets stuck after performing the first step. This seems to be because the targetVersions in the applicationset status do not get updated with the latest one.We've found the following issue.
The applicationset app status stays in
Pending
state even when the app has been synced and is healthy:This is the application status operation state:
The latest commit is the one in the application status. The one in the applicationset status is older. I think that's the reason why it never continues because the revisions do not match.
One of the errors in the logs I see is the following, it's happening from time to time:
I believe there is a problem there when trying to update the applicationset status.
Sharing a bit more of information of our use case, the applicationset generates the apps from a github repository which has multiple merges a day, this makes that normally there is a merge and a new generation of apps in the middle of a rolling sync. I think there could be an issue there too.
Expected behavior
The rolling sync should continue properly after the first step.
The text was updated successfully, but these errors were encountered: