Skip to content

Commit f23f173

Browse files
committed
fix: auto-sync skipped when newer commit arrives during sync with manifest-generate-paths (#27875)
When an app uses the manifest-generate-paths annotation, the revisionsMayHaveChanges optimization was also used to decide whether to run auto-sync. If a newer commit arrived while the app was still syncing, UpdateRevisionForPaths could report no changes, so auto-sync was skipped and the app stayed OutOfSync until a manual sync or controller restart. Only use that optimization to skip manifest regeneration, not to gate the sync decision. When the app is OutOfSync, always compare the desired revision against the last synced one. Drift at the same revision with selfHeal disabled is still left untouched. Signed-off-by: pepe9012 <sotkovsky@gmail.com>
1 parent 0dd8874 commit f23f173

2 files changed

Lines changed: 52 additions & 1 deletion

File tree

controller/appcontroller.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1895,7 +1895,13 @@ func (ctrl *ApplicationController) processAppRefreshQueueItem() (processNext boo
18951895

18961896
canSync, _ := project.Spec.SyncWindows.Matches(app).CanSync(false, nil)
18971897
if canSync {
1898-
syncErrCond, opDuration := ctrl.autoSync(app, compareResult.syncStatus, compareResult.resources, compareResult.revisionsMayHaveChanges)
1898+
// The manifest-generate-paths optimization can report no changes for a newer commit that
1899+
// arrives while the app is still syncing, which would skip auto-sync and leave the app
1900+
// stuck OutOfSync. Only use it to avoid regenerating manifests, never to gate the sync
1901+
// decision: when the app is OutOfSync, always let autoSync compare the desired revision
1902+
// against the last synced one (#27875).
1903+
shouldCompareRevisions := compareResult.revisionsMayHaveChanges || compareResult.syncStatus.Status == appv1.SyncStatusCodeOutOfSync
1904+
syncErrCond, opDuration := ctrl.autoSync(app, compareResult.syncStatus, compareResult.resources, shouldCompareRevisions)
18991905
setOpDuration = opDuration
19001906
if syncErrCond != nil {
19011907
app.Status.SetConditions(

controller/appcontroller_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -735,6 +735,51 @@ func TestAutoSyncMultiSourceWithoutSelfHeal(t *testing.T) {
735735
})
736736
}
737737

738+
func TestAutoSyncManifestGeneratePathsNewCommit(t *testing.T) {
739+
// Regression for #27875: with the manifest-generate-paths annotation, an app must still
740+
// auto-sync to a newer commit while it is OutOfSync, and must not sync when it is already
741+
// synced to that revision.
742+
revA := "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
743+
revB := "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"
744+
745+
t.Run("NewCommitTriggersAutoSync", func(t *testing.T) {
746+
app := newFakeApp()
747+
app.Annotations = map[string]string{v1alpha1.AnnotationKeyManifestGeneratePaths: "."}
748+
app.Spec.SyncPolicy.Automated.SelfHeal = new(false)
749+
app.Status.OperationState.SyncResult.Revision = revA
750+
ctrl := newFakeController(t.Context(), &fakeData{apps: []runtime.Object{app}}, nil)
751+
syncStatus := v1alpha1.SyncStatus{
752+
Status: v1alpha1.SyncStatusCodeOutOfSync,
753+
Revision: revB,
754+
}
755+
cond, _ := ctrl.autoSync(app, &syncStatus, []v1alpha1.ResourceStatus{{Name: "guestbook", Kind: kube.DeploymentKind, Status: v1alpha1.SyncStatusCodeOutOfSync}}, false || syncStatus.Status == v1alpha1.SyncStatusCodeOutOfSync)
756+
assert.Nil(t, cond)
757+
app, err := ctrl.applicationClientset.ArgoprojV1alpha1().Applications(test.FakeArgoCDNamespace).Get(t.Context(), "my-app", metav1.GetOptions{})
758+
require.NoError(t, err)
759+
require.NotNil(t, app.Operation)
760+
require.NotNil(t, app.Operation.Sync)
761+
assert.Equal(t, revB, app.Operation.Sync.Revision)
762+
assert.Empty(t, app.Operation.Sync.Resources)
763+
})
764+
765+
t.Run("SameRevisionDriftDoesNotTriggerAutoSync", func(t *testing.T) {
766+
app := newFakeApp()
767+
app.Annotations = map[string]string{v1alpha1.AnnotationKeyManifestGeneratePaths: "."}
768+
app.Spec.SyncPolicy.Automated.SelfHeal = new(false)
769+
app.Status.OperationState.SyncResult.Revision = revB
770+
ctrl := newFakeController(t.Context(), &fakeData{apps: []runtime.Object{app}}, nil)
771+
syncStatus := v1alpha1.SyncStatus{
772+
Status: v1alpha1.SyncStatusCodeOutOfSync,
773+
Revision: revB,
774+
}
775+
cond, _ := ctrl.autoSync(app, &syncStatus, []v1alpha1.ResourceStatus{{Name: "guestbook", Kind: kube.DeploymentKind, Status: v1alpha1.SyncStatusCodeOutOfSync}}, false || syncStatus.Status == v1alpha1.SyncStatusCodeOutOfSync)
776+
assert.Nil(t, cond)
777+
app, err := ctrl.applicationClientset.ArgoprojV1alpha1().Applications(test.FakeArgoCDNamespace).Get(t.Context(), "my-app", metav1.GetOptions{})
778+
require.NoError(t, err)
779+
assert.Nil(t, app.Operation)
780+
})
781+
}
782+
738783
func TestAutoSyncNotAllowEmpty(t *testing.T) {
739784
app := newFakeApp()
740785
app.Spec.SyncPolicy.Automated.Prune = new(true)

0 commit comments

Comments
 (0)