-
Notifications
You must be signed in to change notification settings - Fork 43
feat: temporarily remount read-only mounts before cleaning container fs #183
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
Merged
Merged
Changes from 21 commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
fafa949
fix: allow envbuilder on sysbox container runtime
janLo 39b4222
formatting
coadler 3ee8a9e
fixup! formatting
coadler c9042fb
Merge remote-tracking branch 'origin/main' into cj/janl/sysbox-issues
johnstcn f008666
fixup! Merge remote-tracking branch 'origin/main' into cj/janl/sysbox…
johnstcn 9c5b005
fixup! Merge remote-tracking branch 'origin/main' into cj/janl/sysbox…
johnstcn 4c1060c
reduce diff size
johnstcn 7f223a0
refactor temp remount logic into util package, add unit tests
johnstcn 3f0e022
fmt
johnstcn 6caad1d
rename internal/util -> internal/ebutil
johnstcn adc6c9c
rename remount func to restore
johnstcn 752e25d
add clarifying comment
johnstcn 8cc4261
fix mockgen invocation
johnstcn 5af94b7
DRY remount func
johnstcn a07c597
restore multierror
johnstcn a3b89aa
add sync.Once to remount func()
johnstcn d3f41df
defer restoreMounts()
johnstcn e129889
move to /MagicDir/mnt subdir
johnstcn c225ef9
TODO
johnstcn 797a4d9
fixup! defer restoreMounts()
johnstcn a06ce23
Merge branch 'main' into cj/janl/sysbox-issues
johnstcn 34d809f
address comments
johnstcn File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,134 @@ | ||
| package ebutil | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "os" | ||
| "path/filepath" | ||
| "strings" | ||
| "sync" | ||
| "syscall" | ||
|
|
||
| "github.com/coder/coder/v2/codersdk" | ||
| "github.com/hashicorp/go-multierror" | ||
| "github.com/prometheus/procfs" | ||
| ) | ||
|
|
||
| // TempRemount iterates through all read-only mounted filesystems, bind-mounts them at dest, | ||
| // and unmounts them from their original source. All mount points underneath ignorePrefixes | ||
| // will not be touched. | ||
| // | ||
| // Some container runtimes such as sysbox-runc will mount in `/lib/modules` read-only. | ||
| // See https://github.com/nestybox/sysbox/issues/564 | ||
| // This trips us up because: | ||
| // 1. We call a Kaniko library function `util.DeleteFilesystem` that does exactly what it says | ||
| // on the tin. If this hits a read-only volume mounted in, unhappiness is the result. | ||
| // 2. After deleting the filesystem and building the image, we extract it to the filesystem. | ||
| // If some paths mounted in via volume are present at that time, unhappiness is also likely | ||
| // to result -- especially in case of read-only mounts. | ||
| // | ||
| // To work around this we move the mounts out of the way temporarily by bind-mounting them | ||
| // while we do our thing, and move them back when we're done. | ||
| // | ||
| // It is the responsibility of the caller to call the returned function | ||
| // to restore the original mount points. If an error is encountered while attempting to perform | ||
| // the operation, calling the returned function will make a best-effort attempt to restore | ||
| // the original state. | ||
| func TempRemount(logf func(codersdk.LogLevel, string, ...any), dest string, ignorePrefixes ...string) (restore func() error, err error, | ||
| ) { | ||
| return tempRemount(&realMounter{}, logf, dest, ignorePrefixes...) | ||
| } | ||
|
|
||
| func tempRemount(m mounter, logf func(codersdk.LogLevel, string, ...any), base string, ignorePrefixes ...string) (restore func() error, err error) { | ||
| mountInfos, err := m.GetMounts() | ||
| if err != nil { | ||
| return func() error { return nil }, fmt.Errorf("get mounts: %w", err) | ||
| } | ||
|
|
||
| // temp move of all ro mounts | ||
| mounts := map[string]string{} | ||
| // closer to attempt to restore original mount points | ||
| var restoreOnce sync.Once | ||
| restore = func() error { | ||
| var merr error | ||
johnstcn marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| restoreOnce.Do(func() { | ||
| for orig, moved := range mounts { | ||
| if err := remount(m, moved, orig); err != nil { | ||
| merr = multierror.Append(merr, fmt.Errorf("restore mount: %w", err)) | ||
| } | ||
| } | ||
| }) | ||
| return merr | ||
| } | ||
|
|
||
| outer: | ||
| for _, mountInfo := range mountInfos { | ||
| // TODO: do this for all mounts | ||
| if _, ok := mountInfo.Options["ro"]; !ok { | ||
| logf(codersdk.LogLevelTrace, "skip rw mount %s", mountInfo.MountPoint) | ||
| continue | ||
| } | ||
|
|
||
| for _, prefix := range ignorePrefixes { | ||
| if strings.HasPrefix(mountInfo.MountPoint, prefix) { | ||
| logf(codersdk.LogLevelTrace, "skip mount %s under ignored prefix %s", mountInfo.MountPoint, prefix) | ||
| continue outer | ||
| } | ||
| } | ||
|
|
||
| src := mountInfo.MountPoint | ||
| dest := filepath.Join("/", base, src) | ||
johnstcn marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if err := remount(m, src, dest); err != nil { | ||
| return restore, fmt.Errorf("temp remount: %w", err) | ||
| } | ||
|
|
||
| mounts[src] = dest | ||
| } | ||
|
|
||
| return restore, nil | ||
| } | ||
|
|
||
| func remount(m mounter, src, dest string) error { | ||
| if err := m.MkdirAll(dest, 0o750); err != nil { | ||
| return fmt.Errorf("ensure path: %w", err) | ||
| } | ||
| if err := m.Mount(src, dest, "bind", syscall.MS_BIND, ""); err != nil { | ||
| return fmt.Errorf("bind mount %s => %s: %w", src, dest, err) | ||
| } | ||
| if err := m.Unmount(src, 0); err != nil { | ||
| return fmt.Errorf("unmount orig src %s: %w", src, err) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // mounter is an interface to system-level calls used by TempRemount. | ||
| type mounter interface { | ||
| // GetMounts wraps procfs.GetMounts | ||
| GetMounts() ([]*procfs.MountInfo, error) | ||
| // MkdirAll wraps os.MkdirAll | ||
| MkdirAll(string, os.FileMode) error | ||
| // Mount wraps syscall.Mount | ||
| Mount(string, string, string, uintptr, string) error | ||
| // Unmount wraps syscall.Unmount | ||
| Unmount(string, int) error | ||
| } | ||
|
|
||
| // realMounter implements mounter and actually does the thing. | ||
| type realMounter struct{} | ||
|
|
||
| var _ mounter = &realMounter{} | ||
|
|
||
| func (m *realMounter) Mount(src string, dest string, fstype string, flags uintptr, data string) error { | ||
| return syscall.Mount(src, dest, fstype, flags, data) | ||
| } | ||
|
|
||
| func (m *realMounter) Unmount(tgt string, flags int) error { | ||
| return syscall.Unmount(tgt, flags) | ||
| } | ||
|
|
||
| func (m *realMounter) GetMounts() ([]*procfs.MountInfo, error) { | ||
| return procfs.GetMounts() | ||
| } | ||
|
|
||
| func (m *realMounter) MkdirAll(path string, perm os.FileMode) error { | ||
| return os.MkdirAll(path, perm) | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Should these be configurable?
Will we only ever support UNIX-based builds?
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.
Kaniko is currently Linux-only to the best of my knowledge.