Purge Newer Applications Deployed on Rollback

[jonathan-innis]

• 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

[nitishm]

Looks good overall - maybe a few test cases for the Remove() function or rather the getDiff() function would be good.

[jonathan-innis]

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-commenter]

Codecov Report

Merging #352 (485dcb7) into main (0088d40) will increase coverage by 35.44%. The diff coverage is 79.23%.

@@             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     

Impacted Files	Coverage Δ
pkg/templates/templates.go	65.65% <16.00%> (ø)
pkg/templates/utils.go	50.00% <50.00%> (ø)
pkg/graph/graph.go	95.91% <95.91%> (ø)
pkg/utils/helpers.go	47.61% <100.00%> (+4.02%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 256ba46...485dcb7. Read the comment docs.

[nitishm]

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?

Can you walk me through a scenario? Do you mean deleting a DAG node with child node(s) and parent node(s)?

[jonathan-innis]

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?

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:

Applications
-----
A

Then we deploy the following set of applications in generation 2 with the following dependency structure

Applications
-----
A -> B -> C -> D

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?

[nitishm]

Yep it seems like removal should use the reversal DAG as well.

[nitishm]

Removal Scenarios:

1. Root Node,
2. Leaf Node,
3. Middle Node
4. DAG (leaf)

Solution:

• Build reverse DAG
• Mark applications to be removed in reverse DAG
• Delete marked applications only following reverse DAG
• Reapply last successful spec

[jonathan-innis]

Removal Scenarios:

1. Root Node,
2. Leaf Node,
3. Middle Node
4. DAG (leaf)

Solution:

• Build reverse DAG
• Mark applications to be removed in reverse DAG
• Delete marked applications only following reverse DAG
• Reapply last successful spec

Yep, this is what I was thinking about 😃

[nitishm]

Removal Scenarios:

1. Root Node,
2. Leaf Node,
3. Middle Node
4. DAG (leaf)

Solution:

• Build reverse DAG
• Mark applications to be removed in reverse DAG
• Delete marked applications only following reverse DAG
• Reapply last successful spec

Yep, this is what I was thinking about 😃

Let's try to enhance the existing delete-all using reversal to support this case (i.e delete-all means mark all nodes)