-
Notifications
You must be signed in to change notification settings - Fork 16
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
Purge Newer Applications Deployed on Rollback #352
Conversation
jonathan-innis
commented
Jun 26, 2021
- We still need to add purging of dependent applications as it's possible that there are dependencies between the newer applications that are rolled out.
- Let's discuss how we can handle this scenario
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.
Check for out-of-bounds if when removing last application in the Remove()
function
Looks good overall - maybe a few test cases for the |
I'm slightly concerned about this in the context of HelmReleases with dependencies. If we were to purge HelmReleases due to a rollback for things that have dependencies between each other, is it possible that we may end up in a state where something can't be deleted? Would we have to build a reverse dependency graph for this as well? @nitishm any thoughts on this? |
Codecov Report
@@ Coverage Diff @@
## main #352 +/- ##
===========================================
+ Coverage 32.55% 68.00% +35.44%
===========================================
Files 13 8 -5
Lines 596 350 -246
===========================================
+ Hits 194 238 +44
+ Misses 396 104 -292
- Partials 6 8 +2
Continue to review full report at Codecov.
|
61f1624
to
3252d73
Compare
Can you walk me through a scenario? Do you mean deleting a DAG node with child node(s) and parent node(s)? |
Yes, as in I deploy applications through AppGroup with the following:
Then we deploy the following set of applications in generation 2 with the following dependency structure
This rollout fails on D so we purge everything. However, would it not be possible that C needs B to be present for removal so this purging would fail? |
Yep it seems like removal should use the reversal DAG as well. |
Let's try to enhance the existing delete-all using reversal to support this case (i.e delete-all means mark all nodes) |
a6725c3
to
af277ff
Compare
2362c6f
to
b0fb13d
Compare
b0fb13d
to
e15b3f8
Compare
8a5bc01
to
9af803b
Compare
5cf5cd2
to
efdcfe1
Compare
efdcfe1
to
52cb2ec
Compare
c0add7d
to
a9e0890
Compare
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.
🚀 Brilliant stuff