Add volume removal when removing accessories#1751
Open
cyppe wants to merge 1 commit intobasecamp:mainfrom
Open
Add volume removal when removing accessories#1751cyppe wants to merge 1 commit intobasecamp:mainfrom
cyppe wants to merge 1 commit intobasecamp:mainfrom
Conversation
When running `kamal accessory remove`, directories created via the `directories:` configuration are deleted, but Docker volumes created via the `volumes:` configuration are not. This seems inconsistent, especially since deleting host-mounted directories is arguably more drastic than deleting Docker volumes. This change adds volume deletion to match the directory deletion behavior: - Add `volume_names` method to Configuration::Accessory that extracts Docker volume names from the `volumes:` config (excluding bind mounts which start with `/`) - Add `remove_volumes` command to Commands::Accessory that runs `docker volume rm` for the accessory's named volumes - Add `remove_volumes` CLI method and call it in `remove_accessory` - Update the remove confirmation message to mention volumes - Add tests for the new functionality Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When running
kamal accessory remove, directories created via thedirectories:configuration are deleted from the host, but Docker volumes specified via thevolumes:configuration are not removed. This creates an inconsistency in behavior.Arguably, deleting host-mounted directories is more drastic than deleting Docker-managed volumes, yet only the former happens automatically. This PR adds volume deletion to match the directory deletion behavior, making
kamal accessory removefully clean up all accessory-related storage.Example Output
Before this change,
kamal accessory remove postgres-backup -ywould output:After this change, an additional step removes Docker volumes:
Behavior
Given an accessory configured with:
After this change,
kamal accessory remove mysqlwill:docker volume rm mysql-data) - NEWOnly named Docker volumes are removed. Bind mounts (paths starting with
/) are correctly excluded since they're not Docker volumes.Changes
volume_namesmethod toConfiguration::Accessorythat extracts Docker volume names from thevolumes:config (excluding bind mounts which use absolute paths starting with/)remove_volumescommand toCommands::Accessorythat runsdocker volume rmfor the accessory's named volumesremove_volumesCLI method and call it inremove_accessoryremovecommand description and confirmation message to mention volumes