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

feat: adds option to remove persistent user data #214

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sam-berning
Copy link
Contributor

Provides a native way for Finch to delete a persistent user data disk. Calling finch vm remove --user-data will now cleanup both the VM and any persistent user data.

Signed-off-by: Sam Berning [email protected]

Issue #, if available:

Description of changes:

Provides a native way for Finch to delete a persistent user data disk. Calling finch vm remove --user-data will now cleanup both the VM and any persistent user data.

Testing done:

E2E, Unit, and manual testing

$ ./_output/bin/finch vm init
$ ./_output/bin/finch vm stop
$ ./_output/bin/finch vm remove --user-data
$ ls ./_output/lima/data/_disks/finch
ls: ./_output/lima/data/_disks/finch: No such file or directory
  • I've reviewed the guidance in CONTRIBUTING.md

License Acceptance

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Provides a native way for Finch to delete a persistent user data disk.
Calling `finch vm remove --user-data` will now cleanup both the VM and
any persistent user data.

Signed-off-by: Sam Berning <[email protected]>
@pendo324
Copy link
Member

I like the idea behind this PR, but it got me wondering if we need an entire disk lifecycle management command instead of just tacking it on to the vm commands. Like, do we need finch disk remove or finch disk recreate? Or I guess a better question: is there a use-case for deleting a disk without removing the entire VM?

@sam-berning
Copy link
Contributor Author

Or I guess a better question: is there a use-case for deleting a disk without removing the entire VM?

Theoretically, users could also remove the disk on finch vm stop safely, and they might want to do that. Another case would be if a user runs finch vm remove without --user-data, they might later want to delete user data.

The reason I chose to attach this to finch vm remove was because persistent disk is invisible to users by design, and having a separate management command for the disk felt kind of counter to that. I think you're right that adding a separate management command would make this more flexible, perhaps as a vm subcommand though, such as finch vm clear-user-data?

The other option would be to also add a similar flag to finch vm stop and changing the logic such that the user data cleanup will still happen even if the VM is already in the state it's supposed to transition to.

Not sure which one I like better, WDYT?

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.

3 participants