-
Notifications
You must be signed in to change notification settings - Fork 5.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add sync delete option #12448
feat: add sync delete option #12448
Conversation
Signed-off-by: Soumya Ghosh Dastidar <gdsoumya@gmail.com>
Signed-off-by: Soumya Ghosh Dastidar <gdsoumya@gmail.com>
go.mod
Outdated
@@ -256,6 +256,7 @@ require ( | |||
) | |||
|
|||
replace ( | |||
github.com/argoproj/gitops-engine => github.com/gdsoumya/gitops-engine v0.0.0-20230213130857-26f153a15602 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will need to merge argoproj/gitops-engine#507 and update the go.mod before merging this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
argoproj/gitops-engine#507 is merged. Please update go.mod
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! updated.
Codecov ReportBase: 47.62% // Head: 47.62% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## master #12448 +/- ##
=======================================
Coverage 47.62% 47.62%
=======================================
Files 246 246
Lines 41806 41806
=======================================
Hits 19911 19911
Misses 19902 19902
Partials 1993 1993
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Signed-off-by: Soumya Ghosh Dastidar <gdsoumya@gmail.com>
…nto feat/syncDeleteOption
@@ -941,7 +942,7 @@ func (ctrl *ApplicationController) removeProjectFinalizer(proj *appv1.AppProject | |||
|
|||
// shouldBeDeleted returns whether a given resource obj should be deleted on cascade delete of application app | |||
func (ctrl *ApplicationController) shouldBeDeleted(app *appv1.Application, obj *unstructured.Unstructured) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be an easy to test. Can you please add couple unit tests ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added tests
Signed-off-by: Soumya Ghosh Dastidar <gdsoumya@gmail.com>
feat: add sync delete option (argoproj#12448) Signed-off-by: Dylan Decrulle <dylan.decrulle@outlook.fr>
Signed-off-by: Soumya Ghosh Dastidar gdsoumya@gmail.com
Fixes #7124
Depends on argoproj/gitops-engine#507
Thanks to @mvazquezc, the solution proposed works according to my testing. This is the original PR #7125, but because the PRs were pretty old and had merge conflicts I created a fresh one to close it out.
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: