diff --git a/controller/appcontroller.go b/controller/appcontroller.go index 5a48f1d41cb097..3659e2f598e2fc 100644 --- a/controller/appcontroller.go +++ b/controller/appcontroller.go @@ -1927,6 +1927,27 @@ func (ctrl *ApplicationController) persistAppStatus(orig *appv1.Application, new return patchMs } +// resetAppStatusOperationState updates the app status operation state to be nil. If no changes were made, it is a no-op +func (ctrl *ApplicationController) resetAppStatusOperationState(orig *appv1.Application) { + logCtx := getAppLog(orig).WithField("method", "resetAppStatusOperationState") + if orig.Status.OperationState == nil { + return + } + newStatus := orig.Status.DeepCopy() + newStatus.OperationState = nil + patch, _, err := createMergePatch( + &appv1.Application{Status: orig.Status}, + &appv1.Application{Status: *newStatus}) + if err != nil { + logCtx.Errorf("Error constructing app status patch: %v", err) + return + } + _, err = ctrl.PatchAppWithWriteBack(context.Background(), orig.Name, orig.Namespace, types.MergePatchType, patch, metav1.PatchOptions{}) + if err != nil { + logCtx.Warnf("Error updating application: %v", err) + } +} + // autoSync will initiate a sync operation for an application configured with automated sync func (ctrl *ApplicationController) autoSync(app *appv1.Application, syncStatus *appv1.SyncStatus, resources []appv1.ResourceStatus, revisionUpdated bool) (*appv1.ApplicationCondition, time.Duration) { logCtx := getAppLog(app) @@ -2318,6 +2339,11 @@ func (ctrl *ApplicationController) newApplicationInformerAndLister() (cache.Shar jitter := time.Duration(float64(ctrl.statusRefreshJitter) * rand.Float64()) delay = &jitter } + // Something updated application operation state bypassing the controller and + // operation state in status still reflects the old sync. Reset it. + if oldApp.Operation == nil && newApp.Operation != nil && newApp.Status.OperationState != nil { + ctrl.resetAppStatusOperationState(newApp) + } } ctrl.requestAppRefresh(newApp.QualifiedName(), compareWith, delay) diff --git a/server/application/application_test.go b/server/application/application_test.go index 0c409357995fe0..431ae9e6caf2cf 100644 --- a/server/application/application_test.go +++ b/server/application/application_test.go @@ -1972,6 +1972,28 @@ func TestAppJsonPatch(t *testing.T) { app, err = appServer.Patch(ctx, &application.ApplicationPatchRequest{Name: &testApp.Name, Patch: ptr.To(`[{"op": "remove", "path": "/metadata/annotations/test.annotation"}]`)}) require.NoError(t, err) assert.NotContains(t, app.Annotations, "test.annotation") + + app, err = appServer.Patch( + ctx, + &application.ApplicationPatchRequest{ + Name: &testApp.Name, + Patch: ptr.To(`[{ + "op": "add", + "path": "/operation", + "value": { + "sync": { + "revisions": ["test"] + }, + "initiatedBy": { + "username": "test" + } + } + }]`), + }, + ) + require.NoError(t, err) + // Can't patch the operation field directly + assert.Nil(t, app.Operation) } func TestAppMergePatch(t *testing.T) { diff --git a/test/e2e/app_management_test.go b/test/e2e/app_management_test.go index 113d18b5969b79..b3db6652b57b2e 100644 --- a/test/e2e/app_management_test.go +++ b/test/e2e/app_management_test.go @@ -3001,3 +3001,27 @@ func TestDeletionConfirmation(t *testing.T) { When().ConfirmDeletion(). Then().Expect(DoesNotExist()) } + +func TestNewOperationTriggeredByUpdatingAppObject(t *testing.T) { + Given(t). + Path(guestbookPath). + When(). + CreateApp(). + Sync(). + Then(). + Expect(OperationPhaseIs(OperationSucceeded)). + Expect(SyncStatusIs(SyncStatusCodeSynced)). + Expect(HealthIs(health.HealthStatusHealthy)). + When(). + // Can't patch using PatchApp* since they ignore Operation changes + UpdateApp(func(app *Application) { + app.Operation = &Operation{ + Sync: &SyncOperation{Revisions: []string{"test"}}, + InitiatedBy: OperationInitiator{Username: "test"}, + } + }). + Then(). + And(func(app *Application) { + assert.Nil(t, app.Status.OperationState) + }) +} diff --git a/test/e2e/fixture/app/actions.go b/test/e2e/fixture/app/actions.go index ca33ae151eb85b..55a68653eb803a 100644 --- a/test/e2e/fixture/app/actions.go +++ b/test/e2e/fixture/app/actions.go @@ -1,6 +1,7 @@ package app import ( + "context" "encoding/json" "fmt" "os" @@ -296,6 +297,16 @@ func (a *Actions) DeclarativeWithCustomRepo(filename string, repoURL string) *Ac return a } +func (a *Actions) UpdateApp(update func(app *Application)) *Actions { + a.context.t.Helper() + app, err := fixture.AppClientset.ArgoprojV1alpha1().Applications(a.context.AppNamespace()).Get(context.Background(), a.context.AppName(), v1.GetOptions{}) + errors.CheckError(err) + update(app) + _, err = fixture.AppClientset.ArgoprojV1alpha1().Applications(a.context.AppNamespace()).Update(context.Background(), app, v1.UpdateOptions{}) + errors.CheckError(err) + return a +} + func (a *Actions) PatchApp(patch string) *Actions { a.context.t.Helper() a.runCli("app", "patch", a.context.AppQualifiedName(), "--patch", patch)