initContainer: Use unix.Mount syscall instead of mount external command #1277
Conversation
|
Build failed. ✔️ unit-test SUCCESS in 8m 50s |
angiglesias
force-pushed
the
fixes/use-native-mount-syscall
branch
from
759775f to
12389c1
Compare
|
Build failed. ✔️ unit-test SUCCESS in 8m 38s |
angiglesias
force-pushed
the
fixes/use-native-mount-syscall
branch
from
12389c1 to
d5b3b30
Compare
|
Build succeeded. ✔️ unit-test SUCCESS in 9m 24s |
There was a problem hiding this comment.
Thanks for working on this, @angiglesias ! Was there an immediate problem that inspired you to make this change?
In principle, it will be good to use the mount(2) system call through golang.org/x/sys/unix, instead of going through the mount(8) command, but it's not that simple a replacement. :)
For example, I think Podman makes mistakes using the mount(2) system call, which leads to #1073 There's this pitfall that mount(2) doesn't propagate the mount flags to existing sub-mounts when performing a recursive bind mount. Notice how the bug can't be reproduced when the bind mounts are handled by mount(8) instead of Podman.
A good example of using mount(2) to do bind mounts is the bind_mount() function in bwrap(1). Especially the part where it works around the mount flags not getting applying to existing sub-mounts for recursive bind mounts.
So, before we change how bind mounts are done in Toolbx itself, we first need to solve #1073 so that we know what we are doing. Then, if we implement a replacement for mount --rbind in Toolbx itself, we would need good test coverage for it.
|
Also, please squash the two commits into one, because we don't want to break the tests between commits. |
While I was investigating #1001 I thought it would be easier to debug the program reducing shell invocations to utilities such as mount using programmatic alternatives when possible. For the mounts, as the syscall is available through the already imported |
angiglesias
force-pushed
the
fixes/use-native-mount-syscall
branch
from
d5b3b30 to
078a5cc
Compare
Use golang unix syscalls package to perform mount operations instead of invoking in a shell the mount command. Performing this operation programmatically should provide more observability debugging the code. It also reduces dependencies on external commands that may change or break functionality between versions. Signed-off-by: Angel Iglesias <[email protected]>
angiglesias
force-pushed
the
fixes/use-native-mount-syscall
branch
from
078a5cc to
7d63981
Compare
|
Build succeeded. ✔️ unit-test SUCCESS in 9m 09s |
There was a problem hiding this comment.
Thanks for squashing the commits, @angiglesias ! Did you manage to find some time for a more complete replacement of the mount(8) command using the mount(2) system call and/or a solution for #1073 and/or increasing our test coverage so that we can be sure that we are not regressing anything when we make changes?
|
I am going to close this because this still doesn't have a complete replacement of the Please feel free to submit a new merge request when you have some updates. |
|
Thanks for playing with Toolbox @angiglesias ! |
|
@debarshiray hey sorry for the unresponsiviness! I will try to find a moment to redo this PR properly to address those issues |
Use golang unix syscalls package to perform mount operations instead of invoking in a shell the mount command.
Performing this operation programmatically should provide more observability debugging the code. It also reduces dependencies on external commands that may change or break functionality between versions.