Skip to content

Harden command failure - #18

Open
marcepartment wants to merge 1 commit into
mainfrom
bugfix/restore-force-in-use-volume
Open

marcepartment wants to merge 1 commit into
mainfrom
bugfix/restore-force-in-use-volume

Conversation

@marcepartment

Copy link
Copy Markdown
Collaborator

No description provided.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The shutdown path mutates stopped environments during dry runs, and the documented removal command fails for running containers.

3 open findings
What changed in this PR

Hardens forced volume restores by removing stopped/orphaned containers and reporting removal failures clearly.

Changes:

  • Tear down all environment containers before restoration.
  • Abort safely when volume removal fails and report holders.
  • Document behavior and troubleshooting.
File Description
utils/​backup.sh Strengthens container teardown and volume-removal handling.
docs/​backup-restore.md Documents failure recovery.
CHANGELOG.md Records restore fixes.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread utils/backup.sh
Comment on lines +791 to +793
if [[ -n "${env_containers}" ]]; then
logMessage INFO "Stopping environment..."
"${ROLL_DIR}/bin/roll" env down || true
"${ROLL_DIR}/bin/roll" env down --remove-orphans || true
Comment thread utils/backup.sh
if [[ -n "$holders" ]]; then
logMessage ERROR "Containers still holding it (running or stopped): $holders"
fi
logMessage ERROR "Remove them first - 'roll env down' for this environment's containers, 'docker rm <name>' for any other - then rerun the restore."
Comment thread docs/backup-restore.md
Comment on lines +330 to +332
# Remove the container by the name the error printed, then retry
docker rm <name>
roll restore --force

This branch has not been deployed

No deployments
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.

2 participants