Skip to content
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

Ensure cleanup of Stack in foreground deletion #760

Merged
merged 7 commits into from
Nov 27, 2024
Merged

Conversation

EronWright
Copy link
Contributor

@EronWright EronWright commented Nov 26, 2024

Problem

There's two edge cases where a Stack could become stuck in a deleting state, and this PR addresses both.

  1. A missing Update object. The stack controller processes the 'current update' (if any) before doing anything else. The stack keeps the name of the current update in its status block, and performs a lookup of the named Update object to check its progress. If the update doesn't exist, it assumes a cache miss and simply requeues. Let's imagine that the update was actually deleted, e.g. via cascading foreground delete. In that case, the stack freezes up.
  2. A missing Workspace object. The stack controller strictly handles workspace setup before kicking off an update, and doesn't re-create the workspace if it were to disappear while the update is in progress. If the workspace goes missing, e.g. due to foreground delete, then the Update blocks indefinitely and likewise does the Stack.

An additional constraint is that completed Update objects serve as a useful historical record, and would ideally not be garbage-collected when the associated Workspace is deleted. It is desirable that a user be able to delete an idle workspace, and one would not want to discard the history in that case.

Proposed changes

  1. Teach the stack controller to place a "stack" finalizer to the "current" Update object, to ensure that it doesn't disappear while a Stack is waiting for its completion. Once complete, the finalizer is removed and the completed Update object is then targetable by a cleanup routine.
  2. Be sure to save the status (which removes the finalizer as mentioned above) in the "fast path" deletion case.
  3. Automatically abort an Update when it is deleted.
  4. Automatically abort an Update when its managing workspace is deleted.

Related issues (optional)

Closes #753

Copy link

codecov bot commented Nov 26, 2024

Codecov Report

Attention: Patch coverage is 28.98551% with 49 lines in your changes missing coverage. Please review.

Project coverage is 50.19%. Comparing base (09f8484) to head (d1085c8).
Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
...ator/internal/controller/auto/update_controller.go 14.28% 24 Missing ⚠️
...tor/internal/controller/pulumi/stack_controller.go 53.33% 8 Missing and 6 partials ⚠️
operator/internal/controller/auto/utils.go 0.00% 11 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #760      +/-   ##
==========================================
- Coverage   50.71%   50.19%   -0.53%     
==========================================
  Files          30       30              
  Lines        4180     4210      +30     
==========================================
- Hits         2120     2113       -7     
- Misses       1876     1910      +34     
- Partials      184      187       +3     

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

@EronWright EronWright requested a review from blampe November 26, 2024 19:10
Copy link
Contributor

@blampe blampe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense to me. Very appropriate use of finalizers to make sure stuff happens in the correct order. I think I ran into the first scenario (no update) when I had things misconfigured, so this should be a good QOL improvement overall.

@EronWright EronWright marked this pull request as ready for review November 27, 2024 20:02
@EronWright EronWright merged commit b648f73 into master Nov 27, 2024
7 checks passed
@EronWright EronWright deleted the issue-753-b branch November 27, 2024 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stack stalls when deleted using foreground cascading delete
2 participants