Skip to content

Commit b8e2947

Browse files
johnstcnjanLocoadler
authored
feat: temporarily remount read-only mounts before cleaning container fs (#183)
Temporarily moves read-only mounts to $MAGIC_DIR/mnt to allow envbuilder to run in sysbox container runtime. Signed-off-by: Jan Losinski <[email protected]> Co-authored-by: Jan Losinski <[email protected]> Co-authored-by: Colin Adler <[email protected]>
1 parent c196133 commit b8e2947

File tree

6 files changed

+459
-2
lines changed

6 files changed

+459
-2
lines changed

envbuilder.go

+18
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import (
3636
"github.com/GoogleContainerTools/kaniko/pkg/util"
3737
"github.com/coder/coder/v2/codersdk"
3838
"github.com/coder/envbuilder/devcontainer"
39+
"github.com/coder/envbuilder/internal/ebutil"
3940
"github.com/containerd/containerd/platforms"
4041
"github.com/distribution/distribution/v3/configuration"
4142
"github.com/distribution/distribution/v3/registry/handlers"
@@ -401,6 +402,19 @@ func Run(ctx context.Context, options Options) error {
401402
})
402403
}
403404

405+
// temp move of all ro mounts
406+
tempRemountDest := filepath.Join("/", MagicDir, "mnt")
407+
ignorePrefixes := []string{tempRemountDest, "/proc", "/sys"}
408+
restoreMounts, err := ebutil.TempRemount(options.Logger, tempRemountDest, ignorePrefixes...)
409+
defer func() { // restoreMounts should never be nil
410+
if err := restoreMounts(); err != nil {
411+
options.Logger(codersdk.LogLevelError, "restore mounts: %s", err.Error())
412+
}
413+
}()
414+
if err != nil {
415+
return fmt.Errorf("temp remount: %w", err)
416+
}
417+
404418
skippedRebuild := false
405419
build := func() (v1.Image, error) {
406420
_, err := options.Filesystem.Stat(MagicFile)
@@ -547,6 +561,10 @@ func Run(ctx context.Context, options Options) error {
547561
closeAfterBuild()
548562
}
549563

564+
if err := restoreMounts(); err != nil {
565+
return fmt.Errorf("restore mounts: %w", err)
566+
}
567+
550568
// Create the magic file to indicate that this build
551569
// has already been ran before!
552570
file, err := options.Filesystem.Create(MagicFile)

go.mod

+3-2
Original file line numberDiff line numberDiff line change
@@ -30,14 +30,17 @@ require (
3030
github.com/go-git/go-billy/v5 v5.5.0
3131
github.com/go-git/go-git/v5 v5.12.0
3232
github.com/google/go-containerregistry v0.15.2
33+
github.com/hashicorp/go-multierror v1.1.1
3334
github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51
3435
github.com/mattn/go-isatty v0.0.20
3536
github.com/moby/buildkit v0.11.6
3637
github.com/otiai10/copy v1.14.0
38+
github.com/prometheus/procfs v0.12.0
3739
github.com/sirupsen/logrus v1.9.3
3840
github.com/skeema/knownhosts v1.2.2
3941
github.com/stretchr/testify v1.9.0
4042
github.com/tailscale/hujson v0.0.0-20221223112325-20486734a56a
43+
go.uber.org/mock v0.4.0
4144
golang.org/x/crypto v0.22.0
4245
golang.org/x/sync v0.7.0
4346
golang.org/x/xerrors v0.0.0-20231012003039-104605ab7028
@@ -155,7 +158,6 @@ require (
155158
github.com/hashicorp/go-hclog v1.5.0 // indirect
156159
github.com/hashicorp/go-immutable-radix v1.3.1 // indirect
157160
github.com/hashicorp/go-memdb v1.3.2 // indirect
158-
github.com/hashicorp/go-multierror v1.1.1 // indirect
159161
github.com/hashicorp/go-uuid v1.0.3 // indirect
160162
github.com/hashicorp/go-version v1.6.0 // indirect
161163
github.com/hashicorp/golang-lru v1.0.2 // indirect
@@ -221,7 +223,6 @@ require (
221223
github.com/prometheus/client_golang v1.18.0 // indirect
222224
github.com/prometheus/client_model v0.5.0 // indirect
223225
github.com/prometheus/common v0.46.0 // indirect
224-
github.com/prometheus/procfs v0.12.0 // indirect
225226
github.com/richardartoul/molecule v1.0.1-0.20221107223329-32cfee06a052 // indirect
226227
github.com/rivo/uniseg v0.4.4 // indirect
227228
github.com/robfig/cron/v3 v3.0.1 // indirect

go.sum

+2
Original file line numberDiff line numberDiff line change
@@ -910,6 +910,8 @@ go.uber.org/atomic v1.11.0 h1:ZvwS0R+56ePWxUNi+Atn9dWONBPp/AUETXlHW0DxSjE=
910910
go.uber.org/atomic v1.11.0/go.mod h1:LUxbIzbOniOlMKjJjyPfpl4v+PKK2cNJn91OQbhoJI0=
911911
go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto=
912912
go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE=
913+
go.uber.org/mock v0.4.0 h1:VcM4ZOtdbR4f6VXfiOpwpVJDL6lCReaZ6mw31wqh7KU=
914+
go.uber.org/mock v0.4.0/go.mod h1:a6FSlNadKUHUa9IP5Vyt1zh4fC7uAwxMutEAscFbkZc=
913915
go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU=
914916
go.uber.org/zap v1.17.0/go.mod h1:MXVU+bhUf/A7Xi2HNOnopQOrmycQ5Ih87HtOu4q5SSo=
915917
go4.org/intern v0.0.0-20211027215823-ae77deb06f29/go.mod h1:cS2ma+47FKrLPdXFpr7CuxiTW3eyJbWew4qx0qtQWDA=

internal/ebutil/mock_mounter_test.go

+100
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

internal/ebutil/remount.go

+134
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
package ebutil
2+
3+
import (
4+
"fmt"
5+
"os"
6+
"path/filepath"
7+
"strings"
8+
"sync"
9+
"syscall"
10+
11+
"github.com/coder/coder/v2/codersdk"
12+
"github.com/hashicorp/go-multierror"
13+
"github.com/prometheus/procfs"
14+
)
15+
16+
// TempRemount iterates through all read-only mounted filesystems, bind-mounts them at dest,
17+
// and unmounts them from their original source. All mount points underneath ignorePrefixes
18+
// will not be touched.
19+
//
20+
// Some container runtimes such as sysbox-runc will mount in `/lib/modules` read-only.
21+
// See https://github.com/nestybox/sysbox/issues/564
22+
// This trips us up because:
23+
// 1. We call a Kaniko library function `util.DeleteFilesystem` that does exactly what it says
24+
// on the tin. If this hits a read-only volume mounted in, unhappiness is the result.
25+
// 2. After deleting the filesystem and building the image, we extract it to the filesystem.
26+
// If some paths mounted in via volume are present at that time, unhappiness is also likely
27+
// to result -- especially in case of read-only mounts.
28+
//
29+
// To work around this we move the mounts out of the way temporarily by bind-mounting them
30+
// while we do our thing, and move them back when we're done.
31+
//
32+
// It is the responsibility of the caller to call the returned function
33+
// to restore the original mount points. If an error is encountered while attempting to perform
34+
// the operation, calling the returned function will make a best-effort attempt to restore
35+
// the original state.
36+
func TempRemount(logf func(codersdk.LogLevel, string, ...any), dest string, ignorePrefixes ...string) (restore func() error, err error,
37+
) {
38+
return tempRemount(&realMounter{}, logf, dest, ignorePrefixes...)
39+
}
40+
41+
func tempRemount(m mounter, logf func(codersdk.LogLevel, string, ...any), base string, ignorePrefixes ...string) (restore func() error, err error) {
42+
mountInfos, err := m.GetMounts()
43+
if err != nil {
44+
return func() error { return nil }, fmt.Errorf("get mounts: %w", err)
45+
}
46+
47+
// temp move of all ro mounts
48+
mounts := map[string]string{}
49+
var restoreOnce sync.Once
50+
var merr error
51+
// closer to attempt to restore original mount points
52+
restore = func() error {
53+
restoreOnce.Do(func() {
54+
for orig, moved := range mounts {
55+
if err := remount(m, moved, orig); err != nil {
56+
merr = multierror.Append(merr, fmt.Errorf("restore mount: %w", err))
57+
}
58+
}
59+
})
60+
return merr
61+
}
62+
63+
outer:
64+
for _, mountInfo := range mountInfos {
65+
// TODO: do this for all mounts
66+
if _, ok := mountInfo.Options["ro"]; !ok {
67+
logf(codersdk.LogLevelTrace, "skip rw mount %s", mountInfo.MountPoint)
68+
continue
69+
}
70+
71+
for _, prefix := range ignorePrefixes {
72+
if strings.HasPrefix(mountInfo.MountPoint, prefix) {
73+
logf(codersdk.LogLevelTrace, "skip mount %s under ignored prefix %s", mountInfo.MountPoint, prefix)
74+
continue outer
75+
}
76+
}
77+
78+
src := mountInfo.MountPoint
79+
dest := filepath.Join(base, src)
80+
if err := remount(m, src, dest); err != nil {
81+
return restore, fmt.Errorf("temp remount: %w", err)
82+
}
83+
84+
mounts[src] = dest
85+
}
86+
87+
return restore, nil
88+
}
89+
90+
func remount(m mounter, src, dest string) error {
91+
if err := m.MkdirAll(dest, 0o750); err != nil {
92+
return fmt.Errorf("ensure path: %w", err)
93+
}
94+
if err := m.Mount(src, dest, "bind", syscall.MS_BIND, ""); err != nil {
95+
return fmt.Errorf("bind mount %s => %s: %w", src, dest, err)
96+
}
97+
if err := m.Unmount(src, 0); err != nil {
98+
return fmt.Errorf("unmount orig src %s: %w", src, err)
99+
}
100+
return nil
101+
}
102+
103+
// mounter is an interface to system-level calls used by TempRemount.
104+
type mounter interface {
105+
// GetMounts wraps procfs.GetMounts
106+
GetMounts() ([]*procfs.MountInfo, error)
107+
// MkdirAll wraps os.MkdirAll
108+
MkdirAll(string, os.FileMode) error
109+
// Mount wraps syscall.Mount
110+
Mount(string, string, string, uintptr, string) error
111+
// Unmount wraps syscall.Unmount
112+
Unmount(string, int) error
113+
}
114+
115+
// realMounter implements mounter and actually does the thing.
116+
type realMounter struct{}
117+
118+
var _ mounter = &realMounter{}
119+
120+
func (m *realMounter) Mount(src string, dest string, fstype string, flags uintptr, data string) error {
121+
return syscall.Mount(src, dest, fstype, flags, data)
122+
}
123+
124+
func (m *realMounter) Unmount(tgt string, flags int) error {
125+
return syscall.Unmount(tgt, flags)
126+
}
127+
128+
func (m *realMounter) GetMounts() ([]*procfs.MountInfo, error) {
129+
return procfs.GetMounts()
130+
}
131+
132+
func (m *realMounter) MkdirAll(path string, perm os.FileMode) error {
133+
return os.MkdirAll(path, perm)
134+
}

0 commit comments

Comments
 (0)