feat(application-controller): Add support for rollback multi-source applications #14124

JorTurFer · 2023-06-19T18:57:04Z

Signed-off-by: Jorge Turrado jorge.turrado@scrm.lidl

Currently, ArgoCD support multi-source applications and that's a really nice feature, but using multi source, end users have to sacrify the option of executing fast rollbacks because the rollbacks are not supported for multi-source applications.
This PR adds the support for rollback multi-source applications (as single source applications do)

Note on DCO:

If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.

Checklist:

Please see Contribution FAQs if you have questions about your pull-request.

Fixes #12580

codecov · 2023-06-22T21:46:22Z

Codecov Report

Attention: Patch coverage is 62.16216% with 42 lines in your changes missing coverage. Please review.

Project coverage is 45.01%. Comparing base (2547abc) to head (26dd5ff).
Report is 5 commits behind head on master.

Files	Patch %	Lines
server/application/application.go	54.54%	13 Missing and 7 partials ⚠️
controller/sync.go	60.00%	3 Missing and 3 partials ⚠️
util/webhook/webhook.go	0.00%	6 Missing ⚠️
pkg/apis/application/v1alpha1/types.go	0.00%	4 Missing ⚠️
cmd/argocd/commands/app.go	0.00%	0 Missing and 2 partials ⚠️
server/repository/repository.go	90.47%	1 Missing and 1 partial ⚠️
util/argo/argo.go	80.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #14124      +/-   ##
==========================================
+ Coverage   45.00%   45.01%   +0.01%     
==========================================
  Files         354      354              
  Lines       47959    48018      +59     
==========================================
+ Hits        21582    21617      +35     
- Misses      23574    23594      +20     
- Partials     2803     2807       +4

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

JorTurFer · 2023-06-22T21:52:17Z

Please, put special attention reviewing the frontend because I'm not frontend developer and maybe I have done something terriblely wrong

JorTurFer · 2023-06-23T10:04:21Z

This is how it looks in the frontend:

ownyrd · 2023-07-03T11:33:20Z

Thanks for implementing this. Hope it gets merged soon :)

crenshaw-dev · 2023-07-18T18:34:33Z

Thanks for the PR! We're past feature freeze for 2.8, so I'll queue this up for the 2.9 roadmap.

JorTurFer · 2023-07-18T18:59:03Z

Thanks for the PR! We're past feature freeze for 2.8, so I'll queue this up for the 2.9 roadmap.

Sure 😄
Do you have any ETA for reviewing the PR? No rush at all, it's just for fixing merge conflicts when you are going to review it to not be rebasing it all the time xD

crenshaw-dev · 2023-07-18T19:03:09Z

Makes sense! At least 3 weeks out, I'm focusing on getting 2.8 ready for GA for now.

JorTurFer · 2023-07-18T19:09:07Z

Makes sense! At least 3 weeks out, I'm focusing on getting 2.8 ready for GA for now.

After vacations (I'll take them the first half of August so during second half) looks as a nice moment to rebase the PR with latest changes 😄
Thanks for the info 🙇

kewei5zhang · 2023-08-10T00:34:37Z

Hoping to have this merged soon. 🙏

JorTurFer · 2023-08-15T14:04:18Z

Hi @crenshaw-dev
Is it now a good moment to rebase my PR? 😄

crenshaw-dev

Only had time for a quick glance, opened one can of worms. :-)

reposerver/repository/repository.proto

JorTurFer · 2023-08-17T14:18:03Z

Sorry, I needed more time that I expected for rebasing the PR 🤦 (the summer break has erased my memory 100%)

JorTurFer · 2023-08-17T16:38:25Z

I have removed the logic from here and I have moved it into the state

Pehesi97 · 2023-08-22T19:16:22Z

Any news on this? Do you guys need an extra pair of hands?

JorTurFer · 2023-08-22T19:18:34Z

Any news on this? Do you guys need an extra pair of hands?

It's waiting a review 😄

crenshaw-dev · 2023-08-24T15:42:45Z

@Pehesi97 we need lots of extra pairs of hands. 😆

GuerreiroLeonardo · 2023-09-05T15:33:47Z

Any predictions on when it will be released?

JorTurFer · 2023-09-05T18:35:38Z

@GuerreiroLeonardo , if you could help them reviewing the code, I think this will be faster for sure 😄
I guess that we are a bit overloaded atm with the summer break and the last release

GuerreiroLeonardo · 2023-09-06T20:38:34Z

@GuerreiroLeonardo , if you could help them reviewing the code, I think this will be faster for sure 😄 I guess that we are a bit overloaded atm with the summer break and the last release

@JorTurFer JorTurFer I wish I could, I have no knowledge on the argoCD code. I am simply a user right now hahah. I gues it would take a lot of hours until I am proficient enough to give any relevant contribution. Sorry =/
I guess maybe crenshaw-dev could help

gioppoluca · 2023-09-08T12:00:36Z

We also consider this PR important, do not have to capabilities to look at the code, but could test it once it gets released.

jsoref

I'd want to see:

design doc
user facing documentation
a ui demo (so I can kick the tires) -- the picture in feat(application-controller): Add support for rollback multi-source applications #14124 (comment) should be in the description.

Note that I'm not wearing a project owner hat, this what I'd want as a drive-by reviewer / UI person / end user.

jsoref · 2023-09-26T17:19:43Z

util/argo/ref_sources.go

+}
+
+// GetRefSources creates a map of ref keys (from the sources' 'ref' fields) to information about the referenced source.
+// This function also validates the references use allowed characters and does not define the same ref key more than


Suggested change

// This function also validates the references use allowed characters and does not define the same ref key more than

// This function also validates the references use allowed characters and do not define the same ref key more than

Is a spelling error on a comment a good reason to ask for a review on such a feature? Seems a good way to make contributors stop contributing.

I took the code from argo.go and I moved it as it was and i didn't notice it. I'll update it 😄

jsoref · 2023-09-26T17:20:23Z

util/argo/ref_sources.go

+				}
+				refKey := "$" + source.Ref
+				if _, ok := refKeys[refKey]; ok {
+					return nil, fmt.Errorf("invalid sources: multiple sources had the same `ref` key")


it'd be really nice if errors like this reported at least a pair of sources so that users have something to chase

I took the code from argo.go and I moved it as it is but I could add the key to the message if you think is useful

My goal is to be able to read a message and figure out how to address it. If this error will be shown as is, I'd want to see the id of the thing (is it an application/<x>?) and two sources that had the same ref (preferably with the ref). If it's going to be pasted w/ other things, then as long as once it's pasted it has those bits, that'd make it usable.

I'd try to improve the message 😄
I'll review the logic trying to figure out the best way to print the message

jsoref · 2023-09-26T17:26:09Z

In:

Why do I see the same revision twice?

If I can't kick the tires, could you at please kick the tires for me?

User Story:

User has a "multi-source application" (some sample content for me to look at)
User deploys it (show picture of screens)
User makes a change (?) (show change, picture of how things change)
User needs to trigger a rollback (show path through UI that user takes to trigger it)
User sees rollback complete (show how the UI looks)
User wants to review to see what rollback(s) have happened (show path through UI for this).

JorTurFer · 2023-09-26T19:07:02Z

design doc
user facing documentation
a ui demo (so I can kick the tires) -- the picture in #14124 (comment) should be in the description.

Hi @jsoref
I appreciate your feedback but you couldn't see those things just because the PR template doesn't require them and I'm new contributing to this project.
I followed it and I have tried to do my best, but based on the documentation I had, those things weren't required. If you point me to any doc about how can I provide the UI demo, I'd be more than happy providing it. I'd really appreciate any guide on that part because I'm totally lost in front side.

About the user documentation, the UX is the same because I haven't added any new interaction, I just enabled an already existing button.

Why do I see the same revision twice?

Because probably I used the same repository for both sources, sorry. I didn't think that it was important.

If I can't kick the tires, could you at please kick the tires for me?

Except for how the multiple sources are presented, all the other points are exactly the same as the current scenario. It's true that I added the extra sources information to the UI (using the parenthesis), but I can remove it if you think that it's wrong.

Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>

Co-authored-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com> Signed-off-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>

Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>

Signed-off-by: Jorge Turrado <jorge.turrado@scrm.lidl>

Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>

Signed-off-by: Jorge Turrado <jorge.turrado@scrm.lidl>

ishitasequeira

LGTM!! Thanks @JorTurFer !! Approving with fact that @keithchong will be raising a follow on PR to fix the UI issues he identified.

JorTurFer · 2024-06-11T06:16:49Z

Thanks @crenshaw-dev @ishitasequeira and @keithchong for your support! ❤️

richardpj · 2024-06-11T06:31:40Z

This is wonderful news! Thank you all for your efforts.

…pplications (argoproj#14124) * feat(application-controller): Add support for rollback multi-source applications Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es> * regenerate codegen after rebase Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es> * fix tests Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es> * fix front linting Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es> * update test Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es> * update codegen Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es> * Update server/application/application.go Co-authored-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com> Signed-off-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es> * apply feedback Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es> * fix errors Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es> * add support for switching between single and multi Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es> * fix dereference issue Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es> * remove unnecesary code Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es> * Rebase master Signed-off-by: Jorge Turrado <jorge.turrado@scrm.lidl> * fix style Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es> * fix reference Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es> * add a comment Signed-off-by: Jorge Turrado <jorge.turrado@scrm.lidl> --------- Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es> Signed-off-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es> Signed-off-by: Jorge Turrado <jorge.turrado@scrm.lidl> Co-authored-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com>

…pplications (argoproj#14124) * feat(application-controller): Add support for rollback multi-source applications Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es> * regenerate codegen after rebase Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es> * fix tests Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es> * fix front linting Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es> * update test Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es> * update codegen Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es> * Update server/application/application.go Co-authored-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com> Signed-off-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es> * apply feedback Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es> * fix errors Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es> * add support for switching between single and multi Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es> * fix dereference issue Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es> * remove unnecesary code Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es> * Rebase master Signed-off-by: Jorge Turrado <jorge.turrado@scrm.lidl> * fix style Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es> * fix reference Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es> * add a comment Signed-off-by: Jorge Turrado <jorge.turrado@scrm.lidl> --------- Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es> Signed-off-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es> Signed-off-by: Jorge Turrado <jorge.turrado@scrm.lidl> Co-authored-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com> Signed-off-by: Javier Solana <javier.solana@cabify.com> Signed-off-by: Javier Solana <javier.solana@cabify.com>

afreyermuth98 · 2024-07-30T07:45:34Z

Hello !
Any idea when this feature would be available on a release ? :)

blakepettersson · 2024-07-30T09:50:38Z

@afreyermuth98 this should be available in 2.12.

JorTurFer changed the title ~~enable buttons and support multiversion in details~~ feat(application-controller): Add support for rollback multi-source applications using the UI Jun 19, 2023

JorTurFer force-pushed the multi-source-history-rollback branch from fd30f30 to b2d284b Compare June 20, 2023 16:34

JorTurFer marked this pull request as ready for review June 22, 2023 16:29

JorTurFer marked this pull request as draft June 22, 2023 16:46

JorTurFer marked this pull request as ready for review June 22, 2023 21:45

JorTurFer changed the title ~~feat(application-controller): Add support for rollback multi-source applications using the UI~~ feat(application-controller): Add support for rollback multi-source applications Jun 30, 2023

crenshaw-dev previously requested changes Aug 15, 2023

View reviewed changes

reposerver/repository/repository.proto Outdated Show resolved Hide resolved

JorTurFer force-pushed the multi-source-history-rollback branch from fadc943 to 99e0b80 Compare August 17, 2023 14:16

jsoref reviewed Sep 26, 2023

View reviewed changes

JorTurFer and others added 14 commits June 10, 2024 19:30

fix tests

5a566e8

Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>

fix front linting

91d4f5d

Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>

update test

5aa1a9c

Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>

update codegen

cdf945a

Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>

Update server/application/application.go

5547b99

Co-authored-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com> Signed-off-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>

apply feedback

9ba82f4

Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>

fix errors

56c2bd2

Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>

add support for switching between single and multi

fe5e4ba

Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>

fix dereference issue

5d6561f

Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>

remove unnecesary code

c5038eb

Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>

Rebase master

1a7fe08

Signed-off-by: Jorge Turrado <jorge.turrado@scrm.lidl>

fix style

bd97327

Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>

fix reference

b112884

Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>

add a comment

49a8b31

Signed-off-by: Jorge Turrado <jorge.turrado@scrm.lidl>

JorTurFer force-pushed the multi-source-history-rollback branch from 9813bdd to 49a8b31 Compare June 10, 2024 17:30

ishitasequeira approved these changes Jun 10, 2024

View reviewed changes

Merge branch 'master' into multi-source-history-rollback

26dd5ff

keithchong approved these changes Jun 10, 2024

View reviewed changes

ishitasequeira merged commit 0e531c9 into argoproj:master Jun 10, 2024
28 checks passed

JorTurFer deleted the multi-source-history-rollback branch June 11, 2024 06:33

keithchong mentioned this pull request Jun 12, 2024

feat: Support Rollback multi-source apps; Follow-up to PR 14124 #18615

Merged

14 tasks

agaudreault mentioned this pull request Jun 20, 2024

[2.12-RC1] "revision not found" error when calling Application API RevisionMetadata #18744

Closed

	// This function also validates the references use allowed characters and does not define the same ref key more than
	// This function also validates the references use allowed characters and do not define the same ref key more than

feat(application-controller): Add support for rollback multi-source applications #14124

feat(application-controller): Add support for rollback multi-source applications #14124

Conversation

JorTurFer commented Jun 19, 2023 • edited Loading

codecov bot commented Jun 22, 2023 • edited Loading

Codecov Report

JorTurFer commented Jun 22, 2023 • edited Loading

JorTurFer commented Jun 23, 2023

ownyrd commented Jul 3, 2023 • edited Loading

crenshaw-dev commented Jul 18, 2023

JorTurFer commented Jul 18, 2023

crenshaw-dev commented Jul 18, 2023

JorTurFer commented Jul 18, 2023 • edited Loading

kewei5zhang commented Aug 10, 2023

JorTurFer commented Aug 15, 2023

crenshaw-dev left a comment

Choose a reason for hiding this comment

JorTurFer commented Aug 17, 2023

JorTurFer commented Aug 17, 2023

Pehesi97 commented Aug 22, 2023

JorTurFer commented Aug 22, 2023

crenshaw-dev commented Aug 24, 2023

GuerreiroLeonardo commented Sep 5, 2023

JorTurFer commented Sep 5, 2023

GuerreiroLeonardo commented Sep 6, 2023 • edited Loading

gioppoluca commented Sep 8, 2023

jsoref left a comment • edited Loading

Choose a reason for hiding this comment

jsoref Sep 26, 2023

Choose a reason for hiding this comment

gioppoluca Sep 26, 2023

Choose a reason for hiding this comment

JorTurFer Sep 26, 2023

Choose a reason for hiding this comment

jsoref Sep 26, 2023

Choose a reason for hiding this comment

JorTurFer Sep 26, 2023

Choose a reason for hiding this comment

jsoref Sep 26, 2023

Choose a reason for hiding this comment

JorTurFer Sep 26, 2023

Choose a reason for hiding this comment

jsoref commented Sep 26, 2023

JorTurFer commented Sep 26, 2023 • edited Loading

ishitasequeira left a comment • edited Loading

Choose a reason for hiding this comment

JorTurFer commented Jun 11, 2024

richardpj commented Jun 11, 2024

afreyermuth98 commented Jul 30, 2024

blakepettersson commented Jul 30, 2024

JorTurFer commented Jun 19, 2023 •

edited

Loading

codecov bot commented Jun 22, 2023 •

edited

Loading

JorTurFer commented Jun 22, 2023 •

edited

Loading

ownyrd commented Jul 3, 2023 •

edited

Loading

JorTurFer commented Jul 18, 2023 •

edited

Loading

GuerreiroLeonardo commented Sep 6, 2023 •

edited

Loading

jsoref left a comment •

edited

Loading

JorTurFer commented Sep 26, 2023 •

edited

Loading

ishitasequeira left a comment •

edited

Loading