fix(appset): Applicationset stuck when revision changes and app pending

  # 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

applicationset/controllers/applicationset_controller.go

@@ -1043,12 +1043,11 @@ func (r *ApplicationSetReconciler) updateApplicationSetApplicationStatus(ctx con
 		} else {
 			// we have an existing AppStatus
 			currentAppStatus = applicationSet.Status.ApplicationStatus[idx]
-
-			// upgrade any existing AppStatus that might have been set by an older argo-cd version
-			// note: currentAppStatus.TargetRevisions may be set to empty list earlier during migrations,
-			// to prevent other usage of r.Client.Status().Update to fail before reaching here.
-			if len(currentAppStatus.TargetRevisions) == 0 {
+			if !reflect.DeepEqual(currentAppStatus.TargetRevisions, app.Status.GetRevisions()) {
 				currentAppStatus.TargetRevisions = app.Status.GetRevisions()
+				currentAppStatus.Status = "Waiting"
+				currentAppStatus.Message = "Application has pending changes, setting status to Waiting."
+				currentAppStatus.LastTransitionTime = &now
 			}
 		}
 
@@ -1058,30 +1057,28 @@ func (r *ApplicationSetReconciler) updateApplicationSetApplicationStatus(ctx con
 		}
 
 		if appOutdated && currentAppStatus.Status != "Waiting" && currentAppStatus.Status != "Pending" {
+			// problema cuando esta pending y se queda en pending
+			// reconcile appset -> app1 = pending, appset.app1.targetRevision = oldSha
+			// push new commit to branch
+			// appset.targetRevision = newBranchSha
+			// app1.spec.targetRevision = newBranchSha
+			// appcontroller: app1.status.TargetRevision = newBranchSha?
+			// app1.status.targetRevision = newBranchSha, appset.app1.status = pending // deadlock
 			logCtx.Infof("Application %v is outdated, updating its ApplicationSet status to Waiting", app.Name)
 			currentAppStatus.LastTransitionTime = &now
 			currentAppStatus.Status = "Waiting"
 			currentAppStatus.Message = "Application has pending changes, setting status to Waiting."
 			currentAppStatus.Step = fmt.Sprint(appStepMap[currentAppStatus.Application] + 1)
-			currentAppStatus.TargetRevisions = app.Status.GetRevisions()
 		}
 
 		if currentAppStatus.Status == "Pending" {
 			if operationPhaseString == "Succeeded" {
-				revisions := []string{}
-				if len(app.Status.OperationState.SyncResult.Revisions) > 0 {
-					revisions = app.Status.OperationState.SyncResult.Revisions
-				} else if app.Status.OperationState.SyncResult.Revision != "" {
-					revisions = append(revisions, app.Status.OperationState.SyncResult.Revision)
-				}
+				logCtx.Infof("Application %v has completed a sync successfully, updating its ApplicationSet status to Progressing", app.Name)
+				currentAppStatus.LastTransitionTime = &now
+				currentAppStatus.Status = "Progressing"
+				currentAppStatus.Message = "Application resource completed a sync successfully, updating status from Pending to Progressing."
+				currentAppStatus.Step = fmt.Sprint(appStepMap[currentAppStatus.Application] + 1)
 
-				if reflect.DeepEqual(currentAppStatus.TargetRevisions, revisions) {
-					logCtx.Infof("Application %v has completed a sync successfully, updating its ApplicationSet status to Progressing", app.Name)
-					currentAppStatus.LastTransitionTime = &now
-					currentAppStatus.Status = "Progressing"
-					currentAppStatus.Message = "Application resource completed a sync successfully, updating status from Pending to Progressing."
-					currentAppStatus.Step = fmt.Sprint(appStepMap[currentAppStatus.Application] + 1)
-				}
 			} else if operationPhaseString == "Running" || healthStatusString == "Progressing" {
 				logCtx.Infof("Application %v has entered Progressing status, updating its ApplicationSet status to Progressing", app.Name)
 				currentAppStatus.LastTransitionTime = &now

applicationset/controllers/applicationset_controller_test.go

@@ -4585,6 +4585,9 @@ func TestUpdateApplicationSetApplicationStatus(t *testing.T) {
 						Health: v1alpha1.HealthStatus{
 							Status: health.HealthStatusProgressing,
 						},
+						Sync: v1alpha1.SyncStatus{
+							Revision: "Next",
+						},
 					},
 				},
 			},
@@ -4636,7 +4639,8 @@ func TestUpdateApplicationSetApplicationStatus(t *testing.T) {
 							Phase: common.OperationRunning,
 						},
 						Sync: v1alpha1.SyncStatus{
-							Status: v1alpha1.SyncStatusCodeSynced,
+							Status:   v1alpha1.SyncStatusCodeSynced,
+							Revision: "Current",
 						},
 					},
 				},
@@ -4689,7 +4693,8 @@ func TestUpdateApplicationSetApplicationStatus(t *testing.T) {
 							Phase: common.OperationSucceeded,
 						},
 						Sync: v1alpha1.SyncStatus{
-							Status: v1alpha1.SyncStatusCodeSynced,
+							Status:   v1alpha1.SyncStatusCodeSynced,
+							Revision: "Next",
 						},
 					},
 				},
@@ -4742,7 +4747,8 @@ func TestUpdateApplicationSetApplicationStatus(t *testing.T) {
 							Phase: common.OperationSucceeded,
 						},
 						Sync: v1alpha1.SyncStatus{
-							Status: v1alpha1.SyncStatusCodeSynced,
+							Revision: "Current",
+							Status:   v1alpha1.SyncStatusCodeSynced,
 						},
 					},
 				},
@@ -4946,7 +4952,7 @@ func TestUpdateApplicationSetApplicationStatus(t *testing.T) {
 			},
 		},
 		{
-			name: "does not progresses a pending application with a successful sync triggered by controller with invalid revision to progressing",
+			name: "removes the appStatus for applications that no longer exist",
 			appSet: v1alpha1.ApplicationSet{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "name",
@@ -4961,14 +4967,18 @@ func TestUpdateApplicationSetApplicationStatus(t *testing.T) {
 				Status: v1alpha1.ApplicationSetStatus{
 					ApplicationStatus: []v1alpha1.ApplicationSetApplicationStatus{
 						{
-							Application: "app1",
-							LastTransitionTime: &metav1.Time{
-								Time: time.Now().Add(time.Duration(-1) * time.Minute),
-							},
-							Message:         "",
-							Status:          "Pending",
+							Application:     "app1",
+							Message:         "Application has pending changes, setting status to Waiting.",
+							Status:          "Waiting",
 							Step:            "1",
-							TargetRevisions: []string{"Next"},
+							TargetRevisions: []string{"Current"},
+						},
+						{
+							Application:     "app2",
+							Message:         "Application has pending changes, setting status to Waiting.",
+							Status:          "Waiting",
+							Step:            "1",
+							TargetRevisions: []string{"Current"},
 						},
 					},
 				},
@@ -4980,41 +4990,30 @@ func TestUpdateApplicationSetApplicationStatus(t *testing.T) {
 					},
 					Status: v1alpha1.ApplicationStatus{
 						Health: v1alpha1.HealthStatus{
-							Status: health.HealthStatusDegraded,
+							Status: health.HealthStatusHealthy,
 						},
 						OperationState: &v1alpha1.OperationState{
 							Phase: common.OperationSucceeded,
-							StartedAt: metav1.Time{
-								Time: time.Now(),
-							},
-							Operation: v1alpha1.Operation{
-								InitiatedBy: v1alpha1.OperationInitiator{
-									Username:  "applicationset-controller",
-									Automated: true,
-								},
-							},
-							SyncResult: &v1alpha1.SyncOperationResult{
-								Revision: "Previous",
-							},
 						},
 						Sync: v1alpha1.SyncStatus{
-							Status: v1alpha1.SyncStatusCodeSynced,
+							Status:   v1alpha1.SyncStatusCodeSynced,
+							Revision: "Current",
 						},
 					},
 				},
 			},
 			expectedAppStatus: []v1alpha1.ApplicationSetApplicationStatus{
 				{
 					Application:     "app1",
-					Message:         "",
-					Status:          "Pending",
+					Message:         "Application resource is already Healthy, updating status from Waiting to Healthy.",
+					Status:          "Healthy",
 					Step:            "1",
-					TargetRevisions: []string{"Next"},
+					TargetRevisions: []string{"Current"},
 				},
 			},
 		},
 		{
-			name: "removes the appStatus for applications that no longer exist",
+			name: "progresses a pending synced application with an old revision to progressing with the Current one",
 			appSet: v1alpha1.ApplicationSet{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "name",
@@ -5030,17 +5029,10 @@ func TestUpdateApplicationSetApplicationStatus(t *testing.T) {
 					ApplicationStatus: []v1alpha1.ApplicationSetApplicationStatus{
 						{
 							Application:     "app1",
-							Message:         "Application has pending changes, setting status to Waiting.",
-							Status:          "Waiting",
-							Step:            "1",
-							TargetRevisions: []string{"Current"},
-						},
-						{
-							Application:     "app2",
-							Message:         "Application has pending changes, setting status to Waiting.",
-							Status:          "Waiting",
+							Message:         "",
+							Status:          "Pending",
 							Step:            "1",
-							TargetRevisions: []string{"Current"},
+							TargetRevisions: []string{"Old"},
 						},
 					},
 				},
@@ -5056,9 +5048,13 @@ func TestUpdateApplicationSetApplicationStatus(t *testing.T) {
 						},
 						OperationState: &v1alpha1.OperationState{
 							Phase: common.OperationSucceeded,
+							SyncResult: &v1alpha1.SyncOperationResult{
+								Revision: "Current",
+							},
 						},
 						Sync: v1alpha1.SyncStatus{
-							Status: v1alpha1.SyncStatusCodeSynced,
+							Status:    v1alpha1.SyncStatusCodeSynced,
+							Revisions: []string{"Current"},
 						},
 					},
 				},