diff --git a/assets/swagger.json b/assets/swagger.json index 35978eb307c56..8740c1f157735 100644 --- a/assets/swagger.json +++ b/assets/swagger.json @@ -9963,7 +9963,7 @@ }, "refresh": { "type": "boolean", - "title": "Refresh indicates if a new revision should trigger a new sync (default: false)" + "title": "Refresh indicates if the latest revision should be used on retry instead of the initial one (default: false)" } } }, diff --git a/cmd/argocd/commands/app.go b/cmd/argocd/commands/app.go index 7c916067faf3b..e1dad479685b7 100644 --- a/cmd/argocd/commands/app.go +++ b/cmd/argocd/commands/app.go @@ -353,7 +353,7 @@ func NewApplicationGetCommand(clientOpts *argocdclient.ClientOptions) *cobra.Com command := &cobra.Command{ Use: "get APPNAME", Short: "Get application details", - Example: templates.Examples(` + Example: templates.Examples(` # Get basic details about the application "my-app" in wide format argocd app get my-app -o wide @@ -383,7 +383,7 @@ func NewApplicationGetCommand(clientOpts *argocdclient.ClientOptions) *cobra.Com # Get application details and display them in a tree format argocd app get my-app --output tree - + # Get application details and display them in a detailed tree format argocd app get my-app --output tree=detailed `), @@ -541,7 +541,7 @@ func NewApplicationLogsCommand(clientOpts *argocdclient.ClientOptions) *cobra.Co command := &cobra.Command{ Use: "logs APPNAME", Short: "Get logs of application pods", - Example: templates.Examples(` + Example: templates.Examples(` # Get logs of pods associated with the application "my-app" argocd app logs my-app @@ -855,7 +855,7 @@ func NewApplicationSetCommand(clientOpts *argocdclient.ClientOptions) *cobra.Com command := &cobra.Command{ Use: "set APPNAME", Short: "Set application parameters", - Example: templates.Examples(` + Example: templates.Examples(` # Set application parameters for the application "my-app" argocd app set my-app --parameter key1=value1 --parameter key2=value2 @@ -2085,6 +2085,7 @@ func NewApplicationSyncCommand(clientOpts *argocdclient.ClientOptions) *cobra.Co applyOutOfSyncOnly bool async bool retryLimit int64 + retryRefresh bool retryBackoffDuration time.Duration retryBackoffMaxDuration time.Duration retryBackoffFactor int64 @@ -2356,9 +2357,10 @@ func NewApplicationSyncCommand(clientOpts *argocdclient.ClientOptions) *cobra.Co default: log.Fatalf("Unknown sync strategy: '%s'", strategy) } - if retryLimit > 0 { + if retryLimit != 0 { syncReq.RetryStrategy = &argoappv1.RetryStrategy{ - Limit: retryLimit, + Limit: retryLimit, + Refresh: retryRefresh, Backoff: &argoappv1.Backoff{ Duration: retryBackoffDuration.String(), MaxDuration: retryBackoffMaxDuration.String(), @@ -2427,6 +2429,7 @@ func NewApplicationSyncCommand(clientOpts *argocdclient.ClientOptions) *cobra.Co command.Flags().StringArrayVar(&labels, "label", []string{}, "Sync only specific resources with a label. This option may be specified repeatedly.") command.Flags().UintVar(&timeout, "timeout", defaultCheckTimeoutSeconds, "Time out after this many seconds") command.Flags().Int64Var(&retryLimit, "retry-limit", 0, "Max number of allowed sync retries") + command.Flags().BoolVar(&retryRefresh, "retry-refresh", false, "Indicates if the latest revision should be used on retry instead of the initial one") command.Flags().DurationVar(&retryBackoffDuration, "retry-backoff-duration", argoappv1.DefaultSyncRetryDuration, "Retry backoff base duration. Input needs to be a duration (e.g. 2m, 1h)") command.Flags().DurationVar(&retryBackoffMaxDuration, "retry-backoff-max-duration", argoappv1.DefaultSyncRetryMaxDuration, "Max retry backoff duration. Input needs to be a duration (e.g. 2m, 1h)") command.Flags().Int64Var(&retryBackoffFactor, "retry-backoff-factor", argoappv1.DefaultSyncRetryFactor, "Factor multiplies the base duration after each failed retry") @@ -3484,7 +3487,7 @@ func NewApplicationRemoveSourceCommand(clientOpts *argocdclient.ClientOptions) * Short: "Remove a source from multiple sources application.", Example: ` # Remove the source at position 1 from application's sources. Counting starts at 1. argocd app remove-source myapplication --source-position 1 - + # Remove the source named "test" from application's sources. argocd app remove-source myapplication --source-name test`, Run: func(c *cobra.Command, args []string) { diff --git a/cmd/util/app.go b/cmd/util/app.go index 4e0c37219769b..102a3256abe97 100644 --- a/cmd/util/app.go +++ b/cmd/util/app.go @@ -169,7 +169,7 @@ func AddAppFlags(command *cobra.Command, opts *AppOptions) { command.Flags().DurationVar(&opts.retryBackoffDuration, "sync-retry-backoff-duration", argoappv1.DefaultSyncRetryDuration, "Sync retry backoff base duration. Input needs to be a duration (e.g. 2m, 1h)") command.Flags().DurationVar(&opts.retryBackoffMaxDuration, "sync-retry-backoff-max-duration", argoappv1.DefaultSyncRetryMaxDuration, "Max sync retry backoff duration. Input needs to be a duration (e.g. 2m, 1h)") command.Flags().Int64Var(&opts.retryBackoffFactor, "sync-retry-backoff-factor", argoappv1.DefaultSyncRetryFactor, "Factor multiplies the base duration after each failed sync retry") - command.Flags().BoolVar(&opts.retryRefresh, "sync-retry-refresh", false, "Set if a new revision should trigger a new sync") + command.Flags().BoolVar(&opts.retryRefresh, "sync-retry-refresh", false, "Indicates if the latest revision should be used on retry instead of the initial one") command.Flags().StringVar(&opts.ref, "ref", "", "Ref is reference to another source within sources field") command.Flags().StringVar(&opts.SourceName, "source-name", "", "Name of the source from the list of sources of the app.") } diff --git a/controller/appcontroller.go b/controller/appcontroller.go index 89299e3ccde07..881ecce282dc9 100644 --- a/controller/appcontroller.go +++ b/controller/appcontroller.go @@ -1424,23 +1424,21 @@ func (ctrl *ApplicationController) processRequestedAppOperation(app *appv1.Appli return } + // Remove the desired revisions if the sync failed and we are retrying. The latest revision from the source will be used. + extraMsg := "" if state.Operation.Retry.Refresh { - if app.Operation != nil && app.Operation.InitiatedBy.Automated { - logCtx.Infof("Refreshing the retry") - state.Operation.Sync.Revision = "" - state.Operation.Sync.Revisions = nil - } else { - logCtx.Infof("Not refreshing the retry during manual sync") - } + extraMsg += " with latest revisions" + state.Operation.Sync.Revision = "" + state.Operation.Sync.Revisions = nil } // Get rid of sync results and null out previous operation completion time // This will start the retry attempt - state.Message = fmt.Sprintf("Retrying operation. Attempt #%d", state.RetryCount) + state.Message = fmt.Sprintf("Retrying operation%s. Attempt #%d", extraMsg, state.RetryCount) state.FinishedAt = nil state.SyncResult = nil ctrl.setOperationState(app, state) - logCtx.Infof("Retrying operation. Attempt #%d", state.RetryCount) + logCtx.Infof("Retrying operation%s. Attempt #%d", extraMsg, state.RetryCount) default: logCtx.Infof("Resuming in-progress operation. phase: %s, message: %s", state.Phase, state.Message) } @@ -2139,16 +2137,20 @@ func (ctrl *ApplicationController) autoSync(app *appv1.Application, syncStatus * } } + source := ptr.To(app.Spec.GetSource()) desiredRevisions := []string{syncStatus.Revision} if app.Spec.HasMultipleSources() { + source = nil desiredRevisions = syncStatus.Revisions } op := appv1.Operation{ Sync: &appv1.SyncOperation{ + Source: source, Revision: syncStatus.Revision, Prune: app.Spec.SyncPolicy.Automated.Prune, SyncOptions: app.Spec.SyncPolicy.SyncOptions, + Sources: app.Spec.Sources, Revisions: syncStatus.Revisions, }, InitiatedBy: appv1.OperationInitiator{Automated: true}, diff --git a/docs/user-guide/commands/argocd_admin_app_generate-spec.md b/docs/user-guide/commands/argocd_admin_app_generate-spec.md index 16783b9fdaa1c..569e5d16e27b8 100644 --- a/docs/user-guide/commands/argocd_admin_app_generate-spec.md +++ b/docs/user-guide/commands/argocd_admin_app_generate-spec.md @@ -107,7 +107,7 @@ argocd admin app generate-spec APPNAME [flags] --sync-retry-backoff-factor int Factor multiplies the base duration after each failed sync retry (default 2) --sync-retry-backoff-max-duration duration Max sync retry backoff duration. Input needs to be a duration (e.g. 2m, 1h) (default 3m0s) --sync-retry-limit int Max number of allowed sync retries - --sync-retry-refresh Set if a new revision should trigger a new sync + --sync-retry-refresh Indicates if the latest revision should be used on retry instead of the initial one --sync-source-branch string The branch from which the app will sync --sync-source-path string The path in the repository from which the app will sync --validate Validation of repo and cluster (default true) diff --git a/docs/user-guide/commands/argocd_app_add-source.md b/docs/user-guide/commands/argocd_app_add-source.md index 09d1cec3c39ca..962039c994d4a 100644 --- a/docs/user-guide/commands/argocd_app_add-source.md +++ b/docs/user-guide/commands/argocd_app_add-source.md @@ -84,7 +84,7 @@ argocd app add-source APPNAME [flags] --sync-retry-backoff-factor int Factor multiplies the base duration after each failed sync retry (default 2) --sync-retry-backoff-max-duration duration Max sync retry backoff duration. Input needs to be a duration (e.g. 2m, 1h) (default 3m0s) --sync-retry-limit int Max number of allowed sync retries - --sync-retry-refresh Set if a new revision should trigger a new sync + --sync-retry-refresh Indicates if the latest revision should be used on retry instead of the initial one --sync-source-branch string The branch from which the app will sync --sync-source-path string The path in the repository from which the app will sync --validate Validation of repo and cluster (default true) diff --git a/docs/user-guide/commands/argocd_app_create.md b/docs/user-guide/commands/argocd_app_create.md index 73de6a512d9de..cb5c1d0c6309f 100644 --- a/docs/user-guide/commands/argocd_app_create.md +++ b/docs/user-guide/commands/argocd_app_create.md @@ -107,7 +107,7 @@ argocd app create APPNAME [flags] --sync-retry-backoff-factor int Factor multiplies the base duration after each failed sync retry (default 2) --sync-retry-backoff-max-duration duration Max sync retry backoff duration. Input needs to be a duration (e.g. 2m, 1h) (default 3m0s) --sync-retry-limit int Max number of allowed sync retries - --sync-retry-refresh Set if a new revision should trigger a new sync + --sync-retry-refresh Indicates if the latest revision should be used on retry instead of the initial one --sync-source-branch string The branch from which the app will sync --sync-source-path string The path in the repository from which the app will sync --upsert Allows to override application with the same name even if supplied application spec is different from existing spec diff --git a/docs/user-guide/commands/argocd_app_remove-source.md b/docs/user-guide/commands/argocd_app_remove-source.md index c04950b550c69..8353ddb70fc94 100644 --- a/docs/user-guide/commands/argocd_app_remove-source.md +++ b/docs/user-guide/commands/argocd_app_remove-source.md @@ -13,7 +13,7 @@ argocd app remove-source APPNAME [flags] ``` # Remove the source at position 1 from application's sources. Counting starts at 1. argocd app remove-source myapplication --source-position 1 - + # Remove the source named "test" from application's sources. argocd app remove-source myapplication --source-name test ``` diff --git a/docs/user-guide/commands/argocd_app_set.md b/docs/user-guide/commands/argocd_app_set.md index a49cad82369d4..3390d4ac3d5c2 100644 --- a/docs/user-guide/commands/argocd_app_set.md +++ b/docs/user-guide/commands/argocd_app_set.md @@ -97,7 +97,7 @@ argocd app set APPNAME [flags] --sync-retry-backoff-factor int Factor multiplies the base duration after each failed sync retry (default 2) --sync-retry-backoff-max-duration duration Max sync retry backoff duration. Input needs to be a duration (e.g. 2m, 1h) (default 3m0s) --sync-retry-limit int Max number of allowed sync retries - --sync-retry-refresh Set if a new revision should trigger a new sync + --sync-retry-refresh Indicates if the latest revision should be used on retry instead of the initial one --sync-source-branch string The branch from which the app will sync --sync-source-path string The path in the repository from which the app will sync --validate Validation of repo and cluster (default true) diff --git a/docs/user-guide/commands/argocd_app_sync.md b/docs/user-guide/commands/argocd_app_sync.md index e9f09603e0f83..1cc09d399ed5e 100644 --- a/docs/user-guide/commands/argocd_app_sync.md +++ b/docs/user-guide/commands/argocd_app_sync.md @@ -64,6 +64,7 @@ argocd app sync [APPNAME... | -l selector | --project project-name] [flags] --retry-backoff-factor int Factor multiplies the base duration after each failed retry (default 2) --retry-backoff-max-duration duration Max retry backoff duration. Input needs to be a duration (e.g. 2m, 1h) (default 3m0s) --retry-limit int Max number of allowed sync retries + --retry-refresh Indicates if the latest revision should be used on retry instead of the initial one --revision string Sync to a specific revision. Preserves parameter overrides --revisions stringArray Show manifests at specific revisions for source position in source-positions -l, --selector string Sync apps that match this label. Supports '=', '==', '!=', in, notin, exists & not exists. Matching apps must satisfy all of the specified label constraints. diff --git a/manifests/core-install-with-hydrator.yaml b/manifests/core-install-with-hydrator.yaml index 179f99347bb94..a04995f511af3 100644 --- a/manifests/core-install-with-hydrator.yaml +++ b/manifests/core-install-with-hydrator.yaml @@ -112,8 +112,8 @@ spec: format: int64 type: integer refresh: - description: 'Refresh indicates if a new revision should trigger - a new sync (default: false)' + description: 'Refresh indicates if the latest revision should + be used on retry instead of the initial one (default: false)' type: boolean type: object sync: @@ -1943,8 +1943,8 @@ spec: format: int64 type: integer refresh: - description: 'Refresh indicates if a new revision should trigger - a new sync (default: false)' + description: 'Refresh indicates if the latest revision should + be used on retry instead of the initial one (default: false)' type: boolean type: object syncOptions: @@ -2915,8 +2915,9 @@ spec: format: int64 type: integer refresh: - description: 'Refresh indicates if a new revision should - trigger a new sync (default: false)' + description: 'Refresh indicates if the latest revision + should be used on retry instead of the initial one (default: + false)' type: boolean type: object sync: diff --git a/manifests/core-install.yaml b/manifests/core-install.yaml index 703b7de55773c..331c732e756b7 100644 --- a/manifests/core-install.yaml +++ b/manifests/core-install.yaml @@ -112,8 +112,8 @@ spec: format: int64 type: integer refresh: - description: 'Refresh indicates if a new revision should trigger - a new sync (default: false)' + description: 'Refresh indicates if the latest revision should + be used on retry instead of the initial one (default: false)' type: boolean type: object sync: @@ -1943,8 +1943,8 @@ spec: format: int64 type: integer refresh: - description: 'Refresh indicates if a new revision should trigger - a new sync (default: false)' + description: 'Refresh indicates if the latest revision should + be used on retry instead of the initial one (default: false)' type: boolean type: object syncOptions: @@ -2915,8 +2915,9 @@ spec: format: int64 type: integer refresh: - description: 'Refresh indicates if a new revision should - trigger a new sync (default: false)' + description: 'Refresh indicates if the latest revision + should be used on retry instead of the initial one (default: + false)' type: boolean type: object sync: diff --git a/manifests/crds/application-crd.yaml b/manifests/crds/application-crd.yaml index d37c09add845e..2161027ee42d0 100644 --- a/manifests/crds/application-crd.yaml +++ b/manifests/crds/application-crd.yaml @@ -111,8 +111,8 @@ spec: format: int64 type: integer refresh: - description: 'Refresh indicates if a new revision should trigger - a new sync (default: false)' + description: 'Refresh indicates if the latest revision should + be used on retry instead of the initial one (default: false)' type: boolean type: object sync: @@ -1942,8 +1942,8 @@ spec: format: int64 type: integer refresh: - description: 'Refresh indicates if a new revision should trigger - a new sync (default: false)' + description: 'Refresh indicates if the latest revision should + be used on retry instead of the initial one (default: false)' type: boolean type: object syncOptions: @@ -2914,8 +2914,9 @@ spec: format: int64 type: integer refresh: - description: 'Refresh indicates if a new revision should - trigger a new sync (default: false)' + description: 'Refresh indicates if the latest revision + should be used on retry instead of the initial one (default: + false)' type: boolean type: object sync: diff --git a/manifests/ha/install-with-hydrator.yaml b/manifests/ha/install-with-hydrator.yaml index eb20da1d1f434..43632af5c9ce4 100644 --- a/manifests/ha/install-with-hydrator.yaml +++ b/manifests/ha/install-with-hydrator.yaml @@ -112,8 +112,8 @@ spec: format: int64 type: integer refresh: - description: 'Refresh indicates if a new revision should trigger - a new sync (default: false)' + description: 'Refresh indicates if the latest revision should + be used on retry instead of the initial one (default: false)' type: boolean type: object sync: @@ -1943,8 +1943,8 @@ spec: format: int64 type: integer refresh: - description: 'Refresh indicates if a new revision should trigger - a new sync (default: false)' + description: 'Refresh indicates if the latest revision should + be used on retry instead of the initial one (default: false)' type: boolean type: object syncOptions: @@ -2915,8 +2915,9 @@ spec: format: int64 type: integer refresh: - description: 'Refresh indicates if a new revision should - trigger a new sync (default: false)' + description: 'Refresh indicates if the latest revision + should be used on retry instead of the initial one (default: + false)' type: boolean type: object sync: diff --git a/manifests/ha/install.yaml b/manifests/ha/install.yaml index 5e603476e7bd2..b8d231d2699e6 100644 --- a/manifests/ha/install.yaml +++ b/manifests/ha/install.yaml @@ -112,8 +112,8 @@ spec: format: int64 type: integer refresh: - description: 'Refresh indicates if a new revision should trigger - a new sync (default: false)' + description: 'Refresh indicates if the latest revision should + be used on retry instead of the initial one (default: false)' type: boolean type: object sync: @@ -1943,8 +1943,8 @@ spec: format: int64 type: integer refresh: - description: 'Refresh indicates if a new revision should trigger - a new sync (default: false)' + description: 'Refresh indicates if the latest revision should + be used on retry instead of the initial one (default: false)' type: boolean type: object syncOptions: @@ -2915,8 +2915,9 @@ spec: format: int64 type: integer refresh: - description: 'Refresh indicates if a new revision should - trigger a new sync (default: false)' + description: 'Refresh indicates if the latest revision + should be used on retry instead of the initial one (default: + false)' type: boolean type: object sync: diff --git a/manifests/install-with-hydrator.yaml b/manifests/install-with-hydrator.yaml index fb60fa977bb98..5523cfb7761ba 100644 --- a/manifests/install-with-hydrator.yaml +++ b/manifests/install-with-hydrator.yaml @@ -112,8 +112,8 @@ spec: format: int64 type: integer refresh: - description: 'Refresh indicates if a new revision should trigger - a new sync (default: false)' + description: 'Refresh indicates if the latest revision should + be used on retry instead of the initial one (default: false)' type: boolean type: object sync: @@ -1943,8 +1943,8 @@ spec: format: int64 type: integer refresh: - description: 'Refresh indicates if a new revision should trigger - a new sync (default: false)' + description: 'Refresh indicates if the latest revision should + be used on retry instead of the initial one (default: false)' type: boolean type: object syncOptions: @@ -2915,8 +2915,9 @@ spec: format: int64 type: integer refresh: - description: 'Refresh indicates if a new revision should - trigger a new sync (default: false)' + description: 'Refresh indicates if the latest revision + should be used on retry instead of the initial one (default: + false)' type: boolean type: object sync: diff --git a/manifests/install.yaml b/manifests/install.yaml index 61d5802f41da4..14914e16c9c8b 100644 --- a/manifests/install.yaml +++ b/manifests/install.yaml @@ -112,8 +112,8 @@ spec: format: int64 type: integer refresh: - description: 'Refresh indicates if a new revision should trigger - a new sync (default: false)' + description: 'Refresh indicates if the latest revision should + be used on retry instead of the initial one (default: false)' type: boolean type: object sync: @@ -1943,8 +1943,8 @@ spec: format: int64 type: integer refresh: - description: 'Refresh indicates if a new revision should trigger - a new sync (default: false)' + description: 'Refresh indicates if the latest revision should + be used on retry instead of the initial one (default: false)' type: boolean type: object syncOptions: @@ -2915,8 +2915,9 @@ spec: format: int64 type: integer refresh: - description: 'Refresh indicates if a new revision should - trigger a new sync (default: false)' + description: 'Refresh indicates if the latest revision + should be used on retry instead of the initial one (default: + false)' type: boolean type: object sync: diff --git a/pkg/apis/application/v1alpha1/generated.proto b/pkg/apis/application/v1alpha1/generated.proto index f15e48ac12627..b6db18fd8c7d5 100644 --- a/pkg/apis/application/v1alpha1/generated.proto +++ b/pkg/apis/application/v1alpha1/generated.proto @@ -2226,7 +2226,7 @@ message RetryStrategy { // Backoff controls how to backoff on subsequent retries of failed syncs optional Backoff backoff = 2; - // Refresh indicates if a new revision should trigger a new sync (default: false) + // Refresh indicates if the latest revision should be used on retry instead of the initial one (default: false) optional bool refresh = 3; } diff --git a/pkg/apis/application/v1alpha1/types.go b/pkg/apis/application/v1alpha1/types.go index 9b9860d8d5c32..3806e51f6f7ff 100644 --- a/pkg/apis/application/v1alpha1/types.go +++ b/pkg/apis/application/v1alpha1/types.go @@ -1467,7 +1467,7 @@ type RetryStrategy struct { Limit int64 `json:"limit,omitempty" protobuf:"bytes,1,opt,name=limit"` // Backoff controls how to backoff on subsequent retries of failed syncs Backoff *Backoff `json:"backoff,omitempty" protobuf:"bytes,2,opt,name=backoff,casttype=Backoff"` - // Refresh indicates if a new revision should trigger a new sync (default: false) + // Refresh indicates if the latest revision should be used on retry instead of the initial one (default: false) Refresh bool `json:"refresh,omitempty" protobuf:"bytes,3,opt,name=refresh"` } diff --git a/server/application/application.go b/server/application/application.go index 7e97d174d9bd2..7ba5dd29bda0c 100644 --- a/server/application/application.go +++ b/server/application/application.go @@ -2060,8 +2060,15 @@ func (s *Server) Sync(ctx context.Context, syncReq *application.ApplicationSyncR } } } + + var source *v1alpha1.ApplicationSource + if !a.Spec.HasMultipleSources() { + source = ptr.To(a.Spec.GetSource()) + } + op := v1alpha1.Operation{ Sync: &v1alpha1.SyncOperation{ + Source: source, Revision: revision, Prune: syncReq.GetPrune(), DryRun: syncReq.GetDryRun(), diff --git a/server/application/application_test.go b/server/application/application_test.go index 8108c28fd0167..71b70b3fa74f7 100644 --- a/server/application/application_test.go +++ b/server/application/application_test.go @@ -2053,6 +2053,7 @@ func TestSyncAndTerminate(t *testing.T) { require.NoError(t, err) assert.NotNil(t, app) assert.NotNil(t, app.Operation) + assert.Equal(t, testApp.Spec.GetSource(), *app.Operation.Sync.Source) events, err := appServer.kubeclientset.CoreV1().Events(appServer.ns).List(t.Context(), metav1.ListOptions{}) require.NoError(t, err) @@ -2131,8 +2132,63 @@ func TestSyncGit(t *testing.T) { assert.Equal(t, "Unknown user initiated sync locally", events.Items[1].Message) } +func TestSync_WithRefresh(t *testing.T) { + ctx := t.Context() + appServer := newTestAppServer(t) + testApp := newTestApp() + testApp.Spec.SyncPolicy = &v1alpha1.SyncPolicy{ + Retry: &v1alpha1.RetryStrategy{ + Refresh: true, + }, + } + testApp.Spec.Source.RepoURL = "https://github.com/argoproj/argo-cd.git" + createReq := application.ApplicationCreateRequest{ + Application: testApp, + } + app, err := appServer.Create(ctx, &createReq) + require.NoError(t, err) + app, err = appServer.Sync(ctx, &application.ApplicationSyncRequest{Name: &app.Name}) + require.NoError(t, err) + assert.NotNil(t, app) + assert.NotNil(t, app.Operation) + assert.True(t, app.Operation.Retry.Refresh) +} + func TestRollbackApp(t *testing.T) { testApp := newTestApp() + testApp.Status.History = []v1alpha1.RevisionHistory{{ + ID: 1, + Revision: "abc", + Revisions: []string{"abc"}, + Source: *testApp.Spec.Source.DeepCopy(), + Sources: []v1alpha1.ApplicationSource{*testApp.Spec.Source.DeepCopy()}, + }} + appServer := newTestAppServer(t, testApp) + + updatedApp, err := appServer.Rollback(t.Context(), &application.ApplicationRollbackRequest{ + Name: &testApp.Name, + Id: ptr.To(int64(1)), + }) + + require.NoError(t, err) + + assert.NotNil(t, updatedApp.Operation) + assert.NotNil(t, updatedApp.Operation.Sync) + assert.NotNil(t, updatedApp.Operation.Sync.Source) + assert.Equal(t, testApp.Status.History[0].Source, *updatedApp.Operation.Sync.Source) + assert.Equal(t, testApp.Status.History[0].Sources, updatedApp.Operation.Sync.Sources) + assert.Equal(t, testApp.Status.History[0].Revision, updatedApp.Operation.Sync.Revision) + assert.Equal(t, testApp.Status.History[0].Revisions, updatedApp.Operation.Sync.Revisions) +} + +func TestRollbackApp_WithRefresh(t *testing.T) { + testApp := newTestApp() + testApp.Spec.SyncPolicy = &v1alpha1.SyncPolicy{ + Retry: &v1alpha1.RetryStrategy{ + Refresh: true, + }, + } + testApp.Status.History = []v1alpha1.RevisionHistory{{ ID: 1, Revision: "abc", @@ -2148,9 +2204,8 @@ func TestRollbackApp(t *testing.T) { require.NoError(t, err) assert.NotNil(t, updatedApp.Operation) - assert.NotNil(t, updatedApp.Operation.Sync) - assert.NotNil(t, updatedApp.Operation.Sync.Source) - assert.Equal(t, "abc", updatedApp.Operation.Sync.Revision) + assert.NotNil(t, updatedApp.Operation.Retry) + assert.False(t, updatedApp.Operation.Retry.Refresh, "refresh should never be set on rollback") } func TestUpdateAppProject(t *testing.T) { diff --git a/test/e2e/app_autosync_test.go b/test/e2e/app_autosync_test.go index e80742912458a..b2006334f5cd1 100644 --- a/test/e2e/app_autosync_test.go +++ b/test/e2e/app_autosync_test.go @@ -1,6 +1,7 @@ package e2e import ( + "fmt" "testing" "time" @@ -8,6 +9,7 @@ import ( "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/ptr" . "github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1" "github.com/argoproj/argo-cd/v3/test/e2e/fixture" @@ -94,60 +96,8 @@ func TestAutoSyncSelfHealEnabled(t *testing.T) { }) } -// TestAutoSyncRetryAndRefreshEnabled verifies that auto-sync+refresh picks up fixed commits automatically +// TestAutoSyncRetryAndRefreshEnabled verifies that auto-sync+refresh picks up new commits automatically func TestAutoSyncRetryAndRefreshEnabled(t *testing.T) { - limits := []int64{ - 100, // Repeat enough times to see we move on to the 3rd commit without reaching the limit - -1, // Repeat forever - } - - for _, limit := range limits { - Given(t). - Path(guestbookPath). - When(). // I create an app with auto-sync and Refresh - CreateFromFile(func(app *Application) { - app.Spec.SyncPolicy = &SyncPolicy{ - Automated: &SyncPolicyAutomated{}, - Retry: &RetryStrategy{ - Limit: limit, - Refresh: true, - }, - } - }). - Then(). // It should auto-sync correctly - Expect(OperationPhaseIs(OperationSucceeded)). - Expect(SyncStatusIs(SyncStatusCodeSynced)). - Expect(NoConditions()). - When(). // Auto-sync encounters broken commit - PatchFile("guestbook-ui-deployment.yaml", `[{"op": "replace", "path": "/spec/revisionHistoryLimit", "value": "badValue"}]`). - Refresh(RefreshTypeNormal). - Then(). // It should keep on trying to sync it - Expect(OperationPhaseIs(OperationRunning)). - Expect(SyncStatusIs(SyncStatusCodeOutOfSync)). - Expect(OperationRetriedTimes(1)). - // Wait to make sure the condition is consistent - And(func(_ *Application) { - time.Sleep(10 * time.Second) - }). - Expect(OperationPhaseIs(OperationRunning)). - Expect(SyncStatusIs(SyncStatusCodeOutOfSync)). - Expect(OperationRetriedTimes(2)). - When(). // I push a fixed commit (while auto-sync in progress) - PatchFile("guestbook-ui-deployment.yaml", `[{"op": "replace", "path": "/spec/revisionHistoryLimit", "value": 42}]`). - Refresh(RefreshTypeNormal). - Then(). // Argo CD should pick it up and sync it successfully - // Wait for the sync retry to pick up a new commit - And(func(_ *Application) { - time.Sleep(10 * time.Second) - }). - Expect(NoConditions()). - Expect(SyncStatusIs(SyncStatusCodeSynced)). - Expect(OperationPhaseIs(OperationSucceeded)) - } -} - -// TestAutoSyncRetryAndRefreshManualSync verifies that auto-sync+refresh do not pick new commits on manual sync -func TestAutoSyncRetryAndRefreshManualSync(t *testing.T) { Given(t). Path(guestbookPath). When(). // I create an app with auto-sync and Refresh @@ -157,6 +107,11 @@ func TestAutoSyncRetryAndRefreshManualSync(t *testing.T) { Retry: &RetryStrategy{ Limit: -1, Refresh: true, + Backoff: &Backoff{ + Duration: time.Second.String(), + Factor: ptr.To(int64(1)), + MaxDuration: time.Second.String(), + }, }, } }). @@ -164,43 +119,25 @@ func TestAutoSyncRetryAndRefreshManualSync(t *testing.T) { Expect(OperationPhaseIs(OperationSucceeded)). Expect(SyncStatusIs(SyncStatusCodeSynced)). Expect(NoConditions()). - When(). // I manually sync the app on a broken commit + When(). // Auto-sync encounters broken commit PatchFile("guestbook-ui-deployment.yaml", `[{"op": "replace", "path": "/spec/revisionHistoryLimit", "value": "badValue"}]`). - Sync("--async"). - Then(). // Argo should keep on retrying - Expect(OperationPhaseIs(OperationRunning)). - Expect(OperationRetriedTimes(1)). - And(func(_ *Application) { - // Wait to make sure the condition is consistent - time.Sleep(10 * time.Second) - }). + Refresh(RefreshTypeNormal). + Then(). // It should keep on trying to sync it Expect(OperationPhaseIs(OperationRunning)). - Expect(OperationRetriedTimes(2)). - When(). // I push a fixed commit (during manual sync) + Expect(SyncStatusIs(SyncStatusCodeOutOfSync)). + Expect(OperationRetriedMinimumTimes(1)). + When(). // I push a fixed commit (while auto-sync in progress) PatchFile("guestbook-ui-deployment.yaml", `[{"op": "replace", "path": "/spec/revisionHistoryLimit", "value": 42}]`). - Then(). // Argo CD should keep on retrying the one from the time of the sync start - And(func(_ *Application) { - // Wait to make sure the condition is consistent - time.Sleep(10 * time.Second) - }). - Expect(OperationRetriedTimes(3)). - When(). // I terminate the stuck sync and start a new manual one (when ref points to fixed commit) - TerminateOp(). - And(func() { - // Wait for the operation to terminate before starting new sync - time.Sleep(1 * time.Second) - }). - Sync("--async"). - Then(). // Argo CD syncs successfully - Expect(NoConditions()). - Expect(SyncStatusIs(SyncStatusCodeSynced)). - Expect(OperationPhaseIs(OperationSucceeded)) + Refresh(RefreshTypeNormal). + Then(). + // Argo CD should pick it up and sync it successfully + Expect(OperationPhaseIs(OperationSucceeded)). + Expect(SyncStatusIs(SyncStatusCodeSynced)) } -func TestAutoSyncRetryAndRefreshAppRefChange(t *testing.T) { - var workingRevision string - var brokenRevision string - +// TestAutoSyncRetryAndRefreshEnabled verifies that auto-sync+refresh picks up new commits automatically on the original source +// at the time the sync was triggered +func TestAutoSyncRetryAndRefreshEnabledChangedSource(t *testing.T) { Given(t). Path(guestbookPath). When(). // I create an app with auto-sync and Refresh @@ -208,8 +145,13 @@ func TestAutoSyncRetryAndRefreshAppRefChange(t *testing.T) { app.Spec.SyncPolicy = &SyncPolicy{ Automated: &SyncPolicyAutomated{}, Retry: &RetryStrategy{ - Limit: -1, + Limit: -1, // Repeat forever Refresh: true, + Backoff: &Backoff{ + Duration: time.Second.String(), + Factor: ptr.To(int64(1)), + MaxDuration: time.Second.String(), + }, }, } }). @@ -217,43 +159,28 @@ func TestAutoSyncRetryAndRefreshAppRefChange(t *testing.T) { Expect(OperationPhaseIs(OperationSucceeded)). Expect(SyncStatusIs(SyncStatusCodeSynced)). Expect(NoConditions()). - And(func(app *Application) { - workingRevision = app.Status.Sync.Revision - println("WR: " + workingRevision) - }). - When(). // I push a broken commit + When(). // Auto-sync encounters broken commit PatchFile("guestbook-ui-deployment.yaml", `[{"op": "replace", "path": "/spec/revisionHistoryLimit", "value": "badValue"}]`). Refresh(RefreshTypeNormal). - Then(). // Argo should keep on retrying - Expect(OperationPhaseIs(OperationRunning)). - Expect(OperationRetriedTimes(1)). - And(func(app *Application) { - // Wait to make sure the condition is consistent - time.Sleep(10 * time.Second) - brokenRevision = app.Operation.Sync.Revision - println("BR: " + workingRevision) - }). - Expect(OperationPhaseIs(OperationRunning)). - Expect(OperationRetriedTimes(2)). - When(). // I change app spec to use working revision - PatchApp(`[{"op": "replace", "path": "/spec/source/targetRevision", "value": "` + workingRevision + `"}]`). - Then(). // Argo CD should keep on retrying the one from the time of the sync start - Expect(OperationPhaseIs(OperationRunning)). - And(func(app *Application) { - // Argo CD have not moved to the declared (broken) revision - assert.Equal(t, brokenRevision, app.Operation.Sync.Revision) - assert.NotEqual(t, workingRevision, app.Operation.Sync.Revision) - }). - Expect(OperationRetriedTimes(2)). - And(func(_ *Application) { - // Wait to make sure the condition is consistent - time.Sleep(30 * time.Second) - }). + Then(). // It should keep on trying to sync it Expect(OperationPhaseIs(OperationRunning)). - And(func(app *Application) { - // Argo CD have not moved to the declared (broken) revision - assert.Equal(t, brokenRevision, app.Operation.Sync.Revision) - assert.NotEqual(t, workingRevision, app.Operation.Sync.Revision) - }). - Expect(OperationRetriedTimes(3)) + Expect(SyncStatusIs(SyncStatusCodeOutOfSync)). + Expect(OperationRetriedMinimumTimes(1)). + When(). + PatchApp(`[{"op": "add", "path": "/spec/source/path", "value": "failure-during-sync"}]`). + // push a fixed commit on HEAD branch + PatchFile("guestbook-ui-deployment.yaml", `[{"op": "replace", "path": "/spec/revisionHistoryLimit", "value": 42}]`). + Refresh(RefreshTypeNormal). + Then(). + Expect(Status(func(status ApplicationStatus) (bool, string) { + // Validate that the history contains the sync to the previous sources + // The history will only contain successful sync + if len(status.History) != 2 { + return false, "expected len to be 2" + } + if status.History[1].Source.Path != guestbookPath { + return false, fmt.Sprintf("expected source path to be '%s'", guestbookPath) + } + return true, "" + })) } diff --git a/test/e2e/app_management_test.go b/test/e2e/app_management_test.go index df302e8641911..b06c0341b09ae 100644 --- a/test/e2e/app_management_test.go +++ b/test/e2e/app_management_test.go @@ -2112,6 +2112,60 @@ func TestSyncWithInfos(t *testing.T) { }) } +// TestSyncWithRetryAndRefreshEnabled verifies that sync+refresh picks up new commits automatically on the original source +// at the time the sync was triggered +func TestSyncWithRetryAndRefreshEnabled(t *testing.T) { + Given(t). + Timeout(2). // Quick timeout since Sync operation is expected to retry forever + Path(guestbookPath). + When(). + CreateFromFile(func(app *Application) { + app.Spec.SyncPolicy = &SyncPolicy{ + Retry: &RetryStrategy{ + Limit: -1, // Repeat forever + Refresh: true, + Backoff: &Backoff{ + Duration: time.Second.String(), + Factor: ptr.To(int64(1)), + MaxDuration: time.Second.String(), + }, + }, + } + }). + Sync(). + Then(). + Expect(OperationPhaseIs(OperationSucceeded)). + Expect(SyncStatusIs(SyncStatusCodeSynced)). + When(). + PatchFile("guestbook-ui-deployment.yaml", `[{"op": "replace", "path": "/spec/revisionHistoryLimit", "value": "badValue"}]`). + IgnoreErrors(). + Sync(). + DoNotIgnoreErrors(). + Then(). + Expect(OperationPhaseIs(OperationRunning)). + Expect(SyncStatusIs(SyncStatusCodeOutOfSync)). + Expect(OperationRetriedMinimumTimes(1)). + When(). + PatchApp(`[{"op": "add", "path": "/spec/source/path", "value": "failure-during-sync"}]`). + // push a fixed commit on HEAD branch + PatchFile("guestbook-ui-deployment.yaml", `[{"op": "replace", "path": "/spec/revisionHistoryLimit", "value": 42}]`). + IgnoreErrors(). + Sync(). + DoNotIgnoreErrors(). + Then(). + Expect(Status(func(status ApplicationStatus) (bool, string) { + // Validate that the history contains the sync to the previous sources + // The history will only contain successful sync + if len(status.History) != 2 { + return false, "expected len to be 2" + } + if status.History[1].Source.Path != guestbookPath { + return false, fmt.Sprintf("expected source path to be '%s'", guestbookPath) + } + return true, "" + })) +} + // Given: argocd app create does not provide --dest-namespace // // Manifest contains resource console which does not require namespace diff --git a/test/e2e/fixture/app/expectation.go b/test/e2e/fixture/app/expectation.go index cf8987853e4c0..92f5bf44bbe51 100644 --- a/test/e2e/fixture/app/expectation.go +++ b/test/e2e/fixture/app/expectation.go @@ -53,12 +53,12 @@ func OperationMessageContains(text string) Expectation { } } -func OperationRetriedTimes(expected int64) Expectation { +func OperationRetriedMinimumTimes(minRetries int64) Expectation { return func(c *Consequences) (state, string) { operationState := c.app().Status.OperationState actual := operationState.RetryCount - message := fmt.Sprintf("operation state retry cound should be %d, is %d, message: '%s'", expected, actual, operationState.Message) - return simple(actual == expected, message) + message := fmt.Sprintf("operation state retry cound should be at least %d, is %d, message: '%s'", minRetries, actual, operationState.Message) + return simple(actual >= minRetries, message) } } @@ -126,6 +126,16 @@ func StatusExists() Expectation { } } +func Status(f func(v1alpha1.ApplicationStatus) (bool, string)) Expectation { + return func(c *Consequences) (state, string) { + ok, msg := f(c.app().Status) + if !ok { + return pending, msg + } + return succeeded, msg + } +} + func Namespace(name string, block func(app *v1alpha1.Application, ns *corev1.Namespace)) Expectation { return func(c *Consequences) (state, string) { ns, err := namespace(name) @@ -359,7 +369,7 @@ func Success(message string, matchers ...func(string, string) bool) Expectation } return func(c *Consequences) (state, string) { if c.actions.lastError != nil { - return failed, "error" + return failed, fmt.Errorf("error: %w", c.actions.lastError).Error() } if !match(c.actions.lastOutput, message) { return failed, fmt.Sprintf("output did not contain '%s'", message)