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

Destroying a release does not destroy the corresponding cache directory (ZFS) #715

Open joh-ku opened 3 months ago

joh-ku commented 3 months ago

[MANDATORY] Describe the bug [MANDATORY] When running bastille destroy RELEASE (ZFS) the corresponding cache directory remains untouched.

[MANDATORY] Bastille and FreeBSD version (paste bastille -v && freebsd-version -kru output) 0.10.20231125 14.1-RELEASE-p3 14.1-RELEASE-p3 14.1-RELEASE-p3

[MANDATORY] How did you install bastille? (port/pkg/git) pkg

[optional] Steps to reproduce?

  1. Run e.g. bastille destroy 14.0-RELEASE (or any release version you may want to destroy)
  2. /usr/local/bastille/cache/14.0-RELEASE still exists

[optional] Expected behavior When a release is destroyed, its corresponding cache directory should be deleted as well.

foudfou commented 2 weeks ago

This is not well documented but the cache is deleted on force: bastille destroy force 14.0-RELEASE .

joh-ku commented 2 weeks ago

@foudfou correct, there is a force flag. Thanks for pointing this out!

The question for me is, if an additional flag is required at all and hence, if "destroy" should delete the release and cache directories all together. Pulling the image again is cheap anyway.

foudfou commented 2 weeks ago

Pulling the image is not so cheap to be honest (minutes) compared to creating a release from the cache (seconds). Plus upstream might not have the locally destroyed release anymore (although subsequent jails could be hard to use).

That said, I would also expect destroying a release to also clean up the cache and have amend the PR accordingly.

joh-ku commented 2 weeks ago

Well, not as cheap as serving it from the cache, agreed. Thank you for the PR! I think the behavior is clearer now.

bmac2 commented 2 days ago

@cedwards what do you think the desired action is here?? Isn't the way itworks not destroying cache the designed way, or should it be changed? Don't want to make changes without your input.