-
Notifications
You must be signed in to change notification settings - Fork 28
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
Fix squashfuse issues #1445
base: master
Are you sure you want to change the base?
Fix squashfuse issues #1445
Conversation
286cabf
to
ae404ea
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add a test for this so that we don't hit this issue in the future.
A simple wakebox unit test that just runs cats a file out of a squash fuse mount would suffice
I agree we should have a test case for this. I didn't realize we did not have one |
Me neither but the fact that this bug slipped passed CI indicates to me that it must be untested |
Made a test case and update description:
|
515af0a
to
7f0e081
Compare
I'll take over this PR so it can land soon |
c83a60d
to
9cec112
Compare
I'm going to wait to debug the rest of this. Issues like this are best debugged using a local podman/docker container but I've had serious issues getting docker to work correctly with namespacing since the kernel they use on the VM to run the containers on doesn't have things configured correctly. There is a way to use your own kernel on Mac docker but I'm lazy and would rather wait until I'm back home with my linux dev laptop that I can run podman on. |
Fix issues caught by @V-FEXrt after #1439 was merged
Also built a test case for wakebox squashfuse. In doing so I discovered that I was not invoking squashfuse correctly so that is corrected now.
As a side note the test case sleeps for 3 seconds before it exits because it is possible that fuse is still unmounting when the
rm -rf
cleanup happens. Maybewakebox
needs a-o notify_pipe
😆List of changes:
bash
to Alpine Dockerfile because I think that is a small requirement.