BastilleBSD / bastille

Bastille is an open-source system for automating deployment and management of containerized applications on FreeBSD.
https://bastillebsd.org
BSD 3-Clause "New" or "Revised" License
860 stars 140 forks source link

Refuse to destroy a jail with mounted filesystems #667

Closed gahr closed 1 day ago

gahr commented 10 months ago
# bastille create temp 14.0-RELEASE 192.168.1.10 lo1
...
# mkdir test
# cp /usr/bin/less test/
# mkdir /usr/local/bastille/jails/temp/root/test
# bastille mount temp $(realpath test) test
[temp]:
Added: /root/admin/bastille/test /usr/local/bastille/jails/temp/root/test nullfs ro 0 0
# /usr/local/bastille/jails/temp/root/test/less -f /dev/stdin &
# bastille destroy force temp
rdr-anchor not found in pf.conf
[temp]:
temp: removed
umount: unmount of /usr/local/bastille/jails/temp/root/test failed: Device busy
jail: temp: /sbin/umount -t nullfs /usr/local/bastille/jails/temp/root/test: failed

Deleting Jail: temp.
Jail has mounted filesystems:
/usr/local/bastille/jails/temp/root/test

Fixes #662

web-sst commented 4 months ago

I think this should be considered a bug fix rather than an enhancement. A likely consequence of operating without this protection is the loss of files within the mounted file system. That's a very bad outcome and not something a user would expect or think to guard against externally.

yaazkal commented 4 months ago

@gahr in your PR, the message comes after the dataset has been destroyed if it's on ZFS. If we are going to have that warning, it should come earlier so the user can check mounts and try to destroy again.

gahr commented 4 months ago

Good point. I don't use ZFS myself so I can't easily test any changes in that area. Would you please improve my PR in that direction?

tschettervictor commented 1 month ago

Will this destroy the files that are mounted inside the jail? It looks like it does empty the directory inside the jail, so what happens to the directory outside?

tschettervictor commented 2 days ago

@yaazkal I have gone over the changes and moved the mount check to it's proper place. It is functioning as expected. How do I update the PR here?

yaazkal commented 2 days ago

@yaazkal I have gone over the changes and moved the mount check to it's proper place. It is functioning as expected. How do I update the PR here?

thanks, just commit the changes to the branch you created whe did the PR

tschettervictor commented 2 days ago

749

This PR can now be closed in favor of the above.

gahr commented 1 day ago

Thank you!