Skip to content
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

Handle chroot mounts with symlink as mountpoint #162

Merged
merged 5 commits into from
Oct 18, 2022

Conversation

diijkstra
Copy link
Contributor

During cleanup of the step_setup_chroot, we will try to unmount
all paths mounted by step's Run. This is done be reading mtab
and looking if each of our mountpoints is present there.

Unfortunately, when mountpoint is a symlink to other directory,
mtab will contain the real canonical path. In such cases we would
skip unmounting directory and later (usually) fail to unmount
device partitions. Moreover, we will endup with still attached
loop device because losetup -d will only schedule detachment
once device is no longer needed.

To avoid this fiasco, we try to resolve paths to canonical path
when unmounting. If this succeed, we will use that path to unmount,
if not normal logic applies. Whenever a mountpoint is considered
as already unmounted (for example by the user provisioning script),
that fact is now logged.

Additionally, some logging messages were added (visible with
PACKER_LOG) to help debug the errors of unmounting and busy
file systems. Unfortunately regular umount error does not contain
the path and might be mistaken by 'optional' fuser failure.

@diijkstra
Copy link
Contributor Author

diijkstra commented Jan 6, 2022

I was also considering converting mountpoints when mounting (obviously we can not do that when reading config), but decided not to, so that a user is not confused with ui messages with different paths.

One more insight to this PR: In my case the failure to unmount tmpfs mount from image, resulted in strange issue with checksum post processor. For some ugly reasons the checksum computed by it, did not match the checksum computed on resulting image file after packer finished running (both on docker and plain host). Offending entry was /var/run which under ArchlinuxARM image is a symlink to /run. Obviously, this also resulted with many stale loopback attachments when not running in docker.

I'm puzzled why checksum was wrongly computed, my only 'guess' is that file behind loop device was not fully flushed when post processor was running. Unfortunately this was very consistent behavior and tracking this issue took me some time. The worst part was, that the resulting image appeared to be working fine.

As my newly added warning says, I'm not sure if we can detect the failure to detach the loop device. One thing to consider would be to fail the build if we can not properly unmount any partition but I'm not sure if that would be acceptable. Looking at the fuser attempts, at least some degree of 'ignoring failure' is here so maybe it is fine as it is now. But, let me ask @mkaczanowski would you accept the PR with modified builder which fails (does not produce artifacts for post processors) when unmounting of image partition did not succeed? I'm not fluent at golang but could give it a try :-)

@mkaczanowski
Copy link
Owner

@diijkstra would you mind rebasing it (I merge the other diff of yours and now it complains :) )

During cleanup of the `step_setup_chroot`, we will try to unmount
all paths mounted by step's Run. This is done be reading mtab
and looking if each of our mountpoints is present there.

Unfortunately, when mountpoint is a symlink to other directory,
mtab will contain the real canonical path. In such cases we would
skip unmounting directory and later (usually) fail to unmount
device partitions. Moreover, we will endup with still attached
loop device because `losetup -d` will only schedule detachment
once device is no longer needed.

To avoid this fiasco, we try to resolve paths to canonical path
when unmounting. If this succeed, we will use that path to unmount,
if not normal logic applies. Whenever a mountpoint is considered
as already unmounted (for example by the user provisioning script),
that fact is now logged.

Additionally, some logging messages were added (visible with
`PACKER_LOG`) to help debug the errors of unmounting and busy
file systems. Unfortunately regular umount error does not contain
the path and might be mistaken by 'optional' fuser failure.
@diijkstra
Copy link
Contributor Author

Thanks, should be fine now :-)

@dbast
Copy link
Collaborator

dbast commented Oct 16, 2022

@diijkstra Updated the PR to use ui.Message instead of log.Printf as suggested by mkaczanowski.

What about a small example of the use case described here being added to one of the boards for testing this new functionality?

@dbast dbast closed this Oct 17, 2022
@dbast dbast reopened this Oct 17, 2022
@diijkstra
Copy link
Contributor Author

@dbast thank you. But I'm no longer using this project and as such I don't remember details enough to add examples, sorry for that. Albeit I was running this 'version' for some time without issues, so long term tests passed ;-)

@dbast dbast merged commit 3d10c41 into mkaczanowski:master Oct 18, 2022
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.

3 participants