feat: auto-refresh on new revisions during sync retries (Alpha) (#11494) by aslafy-z · Pull Request #15603 · argoproj/argo-cd

aslafy-z · 2023-09-20T16:48:00Z

This feature allows users to configure their apps to refresh on new revisions, when the current sync is retrying.
This is controlled by a new boolean Application CRD field syncPolicy.retry.refresh or via the --sync-retry-refresh flag.

Closes #11494
Related to #6055
Discussed at https://www.youtube.com/watch?v=baIX9Bk6f5w&t=1173s

Initially based of the work Sayrus did at Sayrus@817bc34

Checklist:

codecov · 2023-09-23T00:46:38Z

Codecov Report

❌ Patch coverage is 53.33333% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 44.92%. Comparing base (21b1514) to head (20b6f91).
⚠️ Report is 2174 commits behind head on master.

Files with missing lines	Patch %	Lines
controller/appcontroller.go	0.00%	6 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #15603      +/-   ##
==========================================
- Coverage   44.93%   44.92%   -0.02%     
==========================================
  Files         354      354              
  Lines       47742    47720      -22     
==========================================
- Hits        21454    21437      -17     
  Misses      23485    23485              
+ Partials     2803     2798       -5

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

aslafy-z · 2023-09-28T21:42:34Z

controller/appcontroller.go

 				ctrl.requestAppRefresh(app.QualifiedName(), CompareWithLatest.Pointer(), &retryAfter)
 				return
 			} else {
+				if state.Phase == synccommon.OperationRunning && app.Spec.SyncPolicy.Retry.Refresh && app.Status.Sync.Revision != state.Operation.Sync.Revision {


This triggers when processing resources, not hooks. Are hooks processed somewhere else?

controller/appcontroller.go

aslafy-z · 2023-10-02T11:53:38Z

controller/appcontroller.go

+					logCtx.Infof("A new revision is available, refreshing and terminating app, was phase: %s, message: %s", state.Phase, state.Message)
+					ctrl.requestAppRefresh(app.QualifiedName(), CompareWithLatest.Pointer(), nil)
+					state.Phase = synccommon.OperationTerminating
+					state.Message = "Operation forced to terminate (new revision available)"


I could not find this message on the UI, is there a way to make it appear?

I think we have to move setOperationState above the requestAppRefresh, since otherwise there can be a race condition by the refresh processor kicking in and message not being ready yet.

aslafy-z · 2023-11-01T06:21:29Z

@alexec any inputs?

assets/swagger.json

aslafy-z · 2024-05-25T18:40:37Z

@todaywasawesome I applied the required change.
@pasha-codefresh I added a sparse documentation for the feature, let me know if it's OK to you.

I also added the argocd app set --sync-retry-refresh flag.

aslafy-z · 2024-05-25T20:15:36Z

I will fix the tests asap

Signed-off-by: Zadkiel AHARONIAN <hello@zadkiel.fr>

pasha-codefresh · 2024-06-06T10:39:42Z

@aslafy-z any news?

pasha-codefresh · 2024-06-20T07:20:56Z

hey @aslafy-z , are you going to work on it?

aslafy-z · 2024-07-08T10:16:31Z

Hi @pasha-codefresh,

I couldn't reproduce the issue correctly in the end-to-end tests. I'm quite short on time these days. Please feel free to take over if needed, and any suggestions are welcome.

oleghalin · 2024-07-30T23:56:35Z

Hi @pasha-codefresh,

I couldn't reproduce the issue correctly in the end-to-end tests. I'm quite short on time these days. Please feel free to take over if needed, and any suggestions are welcome.

I think your test case shouldnt wait for Failed state because patched selector will not be applied and you will stuck at Running state.

error when patching "/dev/shm/919124178": Deployment.apps "guestbook-ui" is invalid: spec.selector: Invalid value: v1.LabelSelector{MatchLabels:map[string]string{"app":"guestbook-ui", "foo":"bar"}, MatchExpressions:[]v1.LabelSelectorRequirement(nil)}: field is immutable

oleghalin · 2024-07-31T00:05:27Z

test/e2e/app_autosync_test.go

+		PatchFile("guestbook-ui-deployment.yaml", `[{"op": "add", "path": "/spec/selector/matchLabels/foo", "value": "bar"}, {"op": "add", "path": "/spec/template/metadata/labels/foo", "value": "bar"}]`).
+		Refresh(RefreshTypeNormal).
+		Then().
+		Expect(OperationPhaseIs(OperationFailed)).
+		When().
+		// Trigger refresh again to make sure controller notices previously failed sync attempt before expectation timeout expires
+		Refresh(RefreshTypeNormal).
+		Then().
+		Expect(SyncStatusIs(SyncStatusCodeOutOfSync)).
+		Expect(Condition(ApplicationConditionSyncError, "Failed sync attempt")).


Suggested change

PatchFile("guestbook-ui-deployment.yaml", `[{"op": "add", "path": "/spec/selector/matchLabels/foo", "value": "bar"}, {"op": "add", "path": "/spec/template/metadata/labels/foo", "value": "bar"}]`).

Refresh(RefreshTypeNormal).

Then().

Expect(OperationPhaseIs(OperationFailed)).

When().

// Trigger refresh again to make sure controller notices previously failed sync attempt before expectation timeout expires

Refresh(RefreshTypeNormal).

Then().

Expect(SyncStatusIs(SyncStatusCodeOutOfSync)).

Expect(Condition(ApplicationConditionSyncError, "Failed sync attempt")).

PatchFile("guestbook-ui-deployment.yaml", `[{"op": "add", "path": "/spec/selector/matchLabels/foo", "value": "bar"}, {"op": "add", "path": "/spec/template/metadata/labels/foo", "value": "bar"}]`).

Refresh(RefreshTypeNormal).

Then().

Expect(OperationPhaseIs(OperationRunning)).

When().

I suppose that you copied this part from previous test and your case doesnt require it.

Applying healthy manifest

Waiting for Healthy state

Patching with corrupted manifest (this case application will stuck in Syncing state. (operationState.phase == 'Running')

Expecting phase running

Patching with healthy manifest

Expecting that introduced feature works as expected (previous sync stopped and new started & application goes to healthy)

Let me know if it makes sense

@aslafy-z I will see if I can give this a try

andrii-korotkov-verkada · 2024-11-17T11:34:26Z

controller/appcontroller.go

+					state.Message = "Operation forced to terminate (new revision available)"
+					ctrl.setOperationState(app, state)
+					return
+				}


There's a setOperationState below as well, let's combine the two.

olivergondza · 2025-04-16T12:24:05Z

@aslafy-z, @giepa, Thanks a lot for pushing the RFE this far.

Do you have plans to follow up with this? Any work in progress you can share on top of this PR? Thanks!

chaehni · 2025-04-20T13:51:04Z

I don't think tying this to the retry logic is a good thng. The problem is orthogonal to retries. For more context, see: #11494 (comment)

olivergondza · 2025-05-19T12:06:28Z

Can be closed in favor of #23038

aslafy-z · 2025-05-19T17:36:31Z

Thank you for taking over @olivergondza!

aslafy-z force-pushed the patch-3 branch 2 times, most recently from 3872209 to 0a95fd5 Compare September 27, 2023 10:54

aslafy-z changed the title ~~feat: add sync timeout~~ feat: add application sync timeout Sep 27, 2023

aslafy-z force-pushed the patch-3 branch 5 times, most recently from 539d026 to a363b85 Compare September 28, 2023 18:00

aslafy-z changed the title ~~feat: add application sync timeout~~ feat: abort ongoing operation when a new revision is available Sep 28, 2023

aslafy-z force-pushed the patch-3 branch 5 times, most recently from 0713719 to 3850169 Compare September 28, 2023 18:44

aslafy-z commented Sep 28, 2023

View reviewed changes

aslafy-z mentioned this pull request Oct 2, 2023

Retrying failed sync's block newer commits; how to achieve declarative, level based gitops semantics? #11494

Closed

3 tasks

aslafy-z commented Oct 2, 2023

View reviewed changes

controller/appcontroller.go Show resolved Hide resolved

aslafy-z commented Oct 2, 2023

View reviewed changes

aslafy-z force-pushed the patch-3 branch 4 times, most recently from 0d2a152 to ca363b9 Compare October 2, 2023 12:10

aslafy-z changed the title ~~feat: abort ongoing operation when a new revision is available~~ feat: abort ongoing operation when a new revision is available (#11494) Oct 2, 2023

aslafy-z mentioned this pull request Oct 9, 2023

Timeout for sync operations #6055

Closed

aslafy-z force-pushed the patch-3 branch from ca363b9 to f423f27 Compare November 1, 2023 06:20

aslafy-z force-pushed the patch-3 branch from f423f27 to b9a6076 Compare November 6, 2023 14:23

erchn reviewed Nov 13, 2023

View reviewed changes

assets/swagger.json Outdated Show resolved Hide resolved

aslafy-z force-pushed the patch-3 branch from e52237d to 861728a Compare November 20, 2023 12:28

aslafy-z force-pushed the patch-3 branch from dca0970 to 9c51755 Compare May 25, 2024 18:42

aslafy-z force-pushed the patch-3 branch from 9c51755 to 9d27bde Compare May 25, 2024 23:18

aslafy-z changed the title ~~feat: abort ongoing operation when a new revision is available (#11494)~~ feat: auto-refresh on new revisions during sync retries (Alpha) (#11494) May 25, 2024

aslafy-z force-pushed the patch-3 branch 2 times, most recently from 864ce27 to 7129857 Compare May 25, 2024 23:52

aslafy-z added 3 commits May 26, 2024 02:42

feat: abort ongoing operation when a new revision is available

a3d62d8

Signed-off-by: Zadkiel AHARONIAN <hello@zadkiel.fr>

feat(cli): add --sync-retry-refresh flag

c5473dd

Signed-off-by: Zadkiel AHARONIAN <hello@zadkiel.fr>

docs: add sync retry refresh feature to auto-sync docs

c186de2

Signed-off-by: Zadkiel AHARONIAN <hello@zadkiel.fr>

aslafy-z force-pushed the patch-3 branch 2 times, most recently from e1ce49c to a65cbd3 Compare May 26, 2024 00:44

chore: generate files

56b16d6

Signed-off-by: Zadkiel AHARONIAN <hello@zadkiel.fr>

aslafy-z force-pushed the patch-3 branch from a65cbd3 to 56b16d6 Compare May 26, 2024 00:44

Merge branch 'master' into patch-3

20b6f91

oleghalin reviewed Jul 31, 2024

View reviewed changes

andrii-korotkov-verkada reviewed Nov 17, 2024

View reviewed changes

olivergondza mentioned this pull request May 19, 2025

feat(controller): Permit using newer revision when retrying failed sync (#11494) #23038

Merged

13 tasks

aslafy-z closed this May 19, 2025

aslafy-z deleted the patch-3 branch May 19, 2025 17:36

This was referenced Feb 27, 2026

🌱 CNCF mission generation 2026-02-27 kubestellar/console-kb#6

Closed

🌱 CNCF mission generation 2026-02-27 kubestellar/console-kb#11

Merged

Conversation

aslafy-z commented Sep 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Sep 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

aslafy-z Sep 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aslafy-z Oct 2, 2023

Choose a reason for hiding this comment

Uh oh!

andrii-korotkov-verkada Nov 17, 2024

Choose a reason for hiding this comment

Uh oh!

aslafy-z commented Nov 1, 2023

Uh oh!

Uh oh!

aslafy-z commented May 25, 2024

Uh oh!

aslafy-z commented May 25, 2024

Uh oh!

pasha-codefresh commented Jun 6, 2024

Uh oh!

pasha-codefresh commented Jun 20, 2024

Uh oh!

aslafy-z commented Jul 8, 2024

Uh oh!

oleghalin commented Jul 30, 2024

Uh oh!

oleghalin Jul 31, 2024

Choose a reason for hiding this comment

Uh oh!

giepa Oct 11, 2024

Choose a reason for hiding this comment

Uh oh!

andrii-korotkov-verkada Nov 17, 2024

Choose a reason for hiding this comment

Uh oh!

olivergondza commented Apr 16, 2025

Uh oh!

chaehni commented Apr 20, 2025

Uh oh!

olivergondza commented May 19, 2025

Uh oh!

aslafy-z commented May 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

aslafy-z commented Sep 20, 2023 •

edited

Loading

codecov bot commented Sep 23, 2023 •

edited

Loading

aslafy-z Sep 28, 2023 •

edited

Loading