Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ApplicationSet rolling sync stuck in ArgoCD 2.12 #19535

Open
carlosrejano opened this issue Aug 14, 2024 · 9 comments · May be fixed by #20230
Open

ApplicationSet rolling sync stuck in ArgoCD 2.12 #19535

carlosrejano opened this issue Aug 14, 2024 · 9 comments · May be fixed by #20230
Labels
bug Something isn't working component:application-sets Bulk application management related type:bug

Comments

@carlosrejano
Copy link
Contributor

Describe the bug

We have recently upgraded to ArgoCD 2.12. We are using ApplicationSets with RollingSync 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:

- application: step-1-application
  lastTransitionTime: "2024-08-13T15:46:32Z"
  message: Application moved to Pending status, watching for the Application resource to start Progressing.
  status: Pending
  step: "1"
  targetRevisions:
    - d6c3c26af50671c9ec1c0fde89c0100ef7398c09

This is the application status operation state:

finishedAt: "2024-08-14T06:31:12Z"
message: successfully synced (all tasks run)
operation:
  info:
    - name: Reason
      value: ApplicationSet RollingSync triggered a sync of this Application resource.
  initiatedBy:
    automated: true
    username: applicationset-controller
  retry: {}
  sync:
    prune: true
    revision: f3929224827b8c9f9be72d70fbd3e06ba9c8baa8
    syncStrategy:
      hook: {}
phase: Succeeded
startedAt: "2024-08-14T06:31:09Z"
syncResult:
  resources:
    - group: argoproj.io
      hookPhase: Succeeded
      kind: AppProject
      message: appproject.argoproj.io/step-applications unchanged
      name: step-applications
      namespace: ppp-system
      status: Synced
      syncPhase: Sync
      version: v1alpha1
    - group: argoproj.io
      hookPhase: Running
      kind: Application
      message: application.argoproj.io/step-application configured
      name: step-application
      namespace: ppp-system
      status: Synced
      syncPhase: Sync
      version: v1alpha1
  revision: 2fadb0cf3bd1ff6848a096f08ab7d1b3d87c694b
  source:
    helm:
      valuesObject:
        targetRevision: 2fadb0cf3bd1ff6848a096f08ab7d1b3d87c694b
    path: ------
    repoURL: -------
    targetRevision: 2fadb0cf3bd1ff6848a096f08ab7d1b3d87c694b

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:

unable to set application set status: Operation cannot be fulfilled on applicationsets.argoproj.io \"step-applications\": the object has been modified; please apply your changes to the latest version and try again

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.

@carlosrejano carlosrejano added the bug Something isn't working label Aug 14, 2024
@alexmt alexmt added component:application-sets Bulk application management related type:bug labels Aug 14, 2024
carlosrejano added a commit to carlosrejano/argo-cd that referenced this issue Aug 23, 2024
  # 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]>
carlosrejano added a commit to carlosrejano/argo-cd that referenced this issue Aug 23, 2024
  # 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]>
gdsoumya pushed a commit that referenced this issue Sep 7, 2024
* 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]>
@gmauleon
Copy link

I can confirm this seems to happen also in 1.11.7 and probably older versions, we sometimes have applications that stay OutOfSync forever and would not sync with the RollingSync option activated in our applicationset.

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 Pending although they have been healthy/synced for a while.

I forced the status to Progressing for those and everything went back to normal and my stuck OutOfSync application synced automatically.

@gmauleon
Copy link

#12202

@gmauleon
Copy link

gmauleon commented Sep 19, 2024

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?
Like would there be way to just detect it's a later sync and progress the status anyway? Maybe this needs a similar feature as the metadata.generation on the application sync status itself to detect if the "sync generation" is higher than the one we stored?

@gmauleon
Copy link

gmauleon commented Sep 20, 2024

I might be completely off, but what if we overthinked all this?
Applicationset RollingSync is ultimately meant to trigger sync on OutofSync application in a certain order right?
And this behavior can still be bypassed by manual operations at any point (I can sync any app I want manually).

So if an app is synced and healthy whatever the status of the app in the applicationset, its job is done isn't it?
So should we care at all to check revisions or times, couldn't we just check the application syncstatus and healthstatus?

Here is an over simpified graph, we already have some of those transitions (waiting and progressing)

image

@tjamet
Copy link
Contributor

tjamet commented Sep 27, 2024

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 targetRevision resolves to a new sha during a rolling sync.

Personally, I mostly foresee 2 strategies

  1. Reset the rolling sync and restart from the first Application
    This means not all revisions will make it to all applications
  2. Enforce that all applications receives progressively all revisions
    This means there needs to be a mechanism to maintain a queue of revisions to be applied
    Main use-case: upgrades. Say commit A bumps a controller that adds a CRD and run a migration script, commit B drops support for the legacy behaviour. You want to ensure that commit A is successfully applied to all clusters before commit B is applied.

With those strategies in mind, what I don't get clear is the signification of ApplicationSet.status.ApplicationStatus[].TargetRevisions.
Does it aim at reflecting:

  • The desired target revisions to be applied on the next sync
  • The actual Application target revision (i.e. branch/tag/...) as defined in spec
  • The actual Application revisions (i.e. commit sha) observed and deployed as defined in the app status

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.
Shouldn't it always be up to date?

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)

@gmauleon
Copy link

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:

applicationset-queue

One block is basically one reconcile of the applicationset.
Pending is the state where a sync is triggered by adding an operation on the application, the application controller is responsible to execute the sync itself.
So if at any stage the applicationset is updated or an application health or sync state change, it is reevaluated in each reconciliation so that part should already works.

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 😁

@tjamet
Copy link
Contributor

tjamet commented Sep 27, 2024

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.

@gmauleon
Copy link

Hum I don't think that's the role of argoCD, in your example:

Say commit A bumps a controller that adds a CRD and run a migration script, commit B drops support for the legacy behaviour. You want to ensure that commit A is successfully applied to all clusters before commit B is applied.

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.

@tjamet
Copy link
Contributor

tjamet commented Sep 30, 2024

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.
Means it needs to have some mechanisms to control the progression of a release, similarly to what Deployments, StatefulSets or DaemonSet do.
This need is, IMO, increased with the use of indirect targetRevisions like HEAD, main or my-branch which may point to different commit sha depending on the time at which the sync action was requested

carlosrejano pushed a commit to adevinta/argo-cd that referenced this issue Oct 4, 2024
  # 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
carlosrejano pushed a commit to adevinta/argo-cd that referenced this issue Oct 4, 2024
  # 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
carlosrejano pushed a commit to adevinta/argo-cd that referenced this issue Oct 4, 2024
  # 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
Fsero added a commit to adevinta/argo-cd that referenced this issue Oct 4, 2024
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]>
Fsero added a commit to adevinta/argo-cd that referenced this issue Oct 4, 2024
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]>
Fsero added a commit to adevinta/argo-cd that referenced this issue Oct 4, 2024
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]>
tjamet pushed a commit to adevinta/argo-cd that referenced this issue Oct 4, 2024
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]>
Fsero added a commit to adevinta/argo-cd that referenced this issue Oct 4, 2024
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]>
Fsero added a commit to adevinta/argo-cd that referenced this issue Oct 4, 2024
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]>
Fsero added a commit to adevinta/argo-cd that referenced this issue Oct 7, 2024
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]>
Fsero added a commit to adevinta/argo-cd that referenced this issue Oct 7, 2024
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]>
Fsero added a commit to adevinta/argo-cd that referenced this issue Oct 7, 2024
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]>
Fsero added a commit to adevinta/argo-cd that referenced this issue Oct 7, 2024
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]>
Fsero added a commit to adevinta/argo-cd that referenced this issue Oct 7, 2024
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]>
Fsero added a commit to adevinta/argo-cd that referenced this issue Oct 8, 2024
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]>
Fsero added a commit to adevinta/argo-cd that referenced this issue Oct 8, 2024
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]>
Fsero added a commit to adevinta/argo-cd that referenced this issue Oct 8, 2024
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]>
Fsero added a commit to adevinta/argo-cd that referenced this issue Oct 10, 2024
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]>
Fsero added a commit to adevinta/argo-cd that referenced this issue Oct 17, 2024
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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working component:application-sets Bulk application management related type:bug
Projects
None yet
4 participants