Skip to content

Commit 0b1d123

Browse files
authored
Merge pull request kubernetes#127899 from jsafrane/revert-tmp
Revert 126599: fix: preserve options after remount for bind mounting
2 parents 8155327 + 7306cc1 commit 0b1d123

File tree

6 files changed

+71
-28
lines changed

6 files changed

+71
-28
lines changed

staging/src/k8s.io/mount-utils/go.mod

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ godebug default=go1.23
88

99
require (
1010
github.com/moby/sys/mountinfo v0.7.1
11+
github.com/opencontainers/runc v1.1.14
1112
github.com/stretchr/testify v1.9.0
1213
golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56
1314
golang.org/x/sys v0.23.0
@@ -16,11 +17,15 @@ require (
1617
)
1718

1819
require (
20+
github.com/coreos/go-systemd/v22 v22.5.0 // indirect
1921
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
2022
github.com/go-logr/logr v1.4.2 // indirect
23+
github.com/godbus/dbus/v5 v5.1.0 // indirect
2124
github.com/kr/pretty v0.3.1 // indirect
25+
github.com/opencontainers/runtime-spec v1.2.0 // indirect
2226
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
2327
github.com/rogpeppe/go-internal v1.12.0 // indirect
28+
github.com/sirupsen/logrus v1.9.3 // indirect
2429
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
2530
gopkg.in/yaml.v3 v3.0.1 // indirect
2631
)

staging/src/k8s.io/mount-utils/go.sum

Lines changed: 35 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

staging/src/k8s.io/mount-utils/mount_linux.go

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import (
3535
"github.com/moby/sys/mountinfo"
3636
"golang.org/x/sys/unix"
3737

38+
libcontaineruserns "github.com/opencontainers/runc/libcontainer/userns"
3839
"k8s.io/klog/v2"
3940
utilexec "k8s.io/utils/exec"
4041
)
@@ -113,7 +114,7 @@ func (mounter *Mounter) hasSystemd() bool {
113114

114115
// Map unix.Statfs mount flags ro, nodev, noexec, nosuid, noatime, relatime,
115116
// nodiratime to mount option flag strings.
116-
func getBindMountOptions(path string, statfs func(path string, buf *unix.Statfs_t) (err error)) ([]string, error) {
117+
func getUserNSBindMountOptions(path string, statfs func(path string, buf *unix.Statfs_t) (err error)) ([]string, error) {
117118
var s unix.Statfs_t
118119
var mountOpts []string
119120
if err := statfs(path, &s); err != nil {
@@ -136,23 +137,32 @@ func getBindMountOptions(path string, statfs func(path string, buf *unix.Statfs_
136137
return mountOpts, nil
137138
}
138139

139-
// Performs a bind mount with the specified options, and then remounts
140-
// the mount point with the same `nodev`, `nosuid`, `noexec`, `nosuid`, `noatime`,
141-
// `relatime`, `nodiratime` options as the original mount point.
140+
// Do a bind mount including the needed remount for applying the bind opts.
141+
// If the remount fails and we are running in a user namespace
142+
// figure out if the source filesystem has the ro, nodev, noexec, nosuid,
143+
// noatime, relatime or nodiratime flag set and try another remount with the found flags.
142144
func (mounter *Mounter) bindMountSensitive(mounterPath string, mountCmd string, source string, target string, fstype string, bindOpts []string, bindRemountOpts []string, bindRemountOptsSensitive []string, mountFlags []string, systemdMountRequired bool) error {
143-
err := mounter.doMount(mounterPath, mountCmd, source, target, fstype, bindOpts, bindRemountOptsSensitive, mountFlags, systemdMountRequired)
145+
err := mounter.doMount(mounterPath, defaultMountCommand, source, target, fstype, bindOpts, bindRemountOptsSensitive, mountFlags, systemdMountRequired)
144146
if err != nil {
145147
return err
146148
}
147-
// Check if the source has ro, nodev, noexec, nosuid, noatime, relatime,
148-
// nodiratime flag...
149-
fixMountOpts, err := getBindMountOptions(source, unix.Statfs)
150-
if err != nil {
151-
return &os.PathError{Op: "statfs", Path: source, Err: err}
149+
err = mounter.doMount(mounterPath, defaultMountCommand, source, target, fstype, bindRemountOpts, bindRemountOptsSensitive, mountFlags, systemdMountRequired)
150+
if libcontaineruserns.RunningInUserNS() {
151+
if err == nil {
152+
return nil
153+
}
154+
// Check if the source has ro, nodev, noexec, nosuid, noatime, relatime,
155+
// nodiratime flag...
156+
fixMountOpts, err := getUserNSBindMountOptions(source, unix.Statfs)
157+
if err != nil {
158+
return &os.PathError{Op: "statfs", Path: source, Err: err}
159+
}
160+
// ... and retry the mount with flags found above.
161+
bindRemountOpts = append(bindRemountOpts, fixMountOpts...)
162+
return mounter.doMount(mounterPath, defaultMountCommand, source, target, fstype, bindRemountOpts, bindRemountOptsSensitive, mountFlags, systemdMountRequired)
163+
} else {
164+
return err
152165
}
153-
// ... and retry the mount with flags found above.
154-
bindRemountOpts = append(bindRemountOpts, fixMountOpts...)
155-
return mounter.doMount(mounterPath, mountCmd, source, target, fstype, bindRemountOpts, bindRemountOptsSensitive, mountFlags, systemdMountRequired)
156166
}
157167

158168
// Mount mounts source to target as fstype with given options. 'source' and 'fstype' must

staging/src/k8s.io/mount-utils/mount_linux_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -821,7 +821,7 @@ func mkStatfsFlags[T1 constraints.Integer, T2 constraints.Integer](orig T1, add
821821
return orig | T1(add)
822822
}
823823

824-
func TestGetBindMountOptions(t *testing.T) {
824+
func TestGetUserNSBindMountOptions(t *testing.T) {
825825
var testCases = map[string]struct {
826826
flags int32 // smallest size used by any platform we care about
827827
mountoptions string
@@ -843,9 +843,9 @@ func TestGetBindMountOptions(t *testing.T) {
843843
return nil
844844
}
845845

846-
testGetBindMountOptionsSingleCase := func(t *testing.T) {
846+
testGetUserNSBindMountOptionsSingleCase := func(t *testing.T) {
847847
path := strings.Split(t.Name(), "/")[1]
848-
options, _ := getBindMountOptions(path, statfsMock)
848+
options, _ := getUserNSBindMountOptions(path, statfsMock)
849849
sort.Strings(options)
850850
optionString := strings.Join(options, ",")
851851
mountOptions := testCases[path].mountoptions
@@ -855,7 +855,7 @@ func TestGetBindMountOptions(t *testing.T) {
855855
}
856856

857857
for k := range testCases {
858-
t.Run(k, testGetBindMountOptionsSingleCase)
858+
t.Run(k, testGetUserNSBindMountOptionsSingleCase)
859859
}
860860
}
861861

test/e2e/storage/drivers/in_tree.go

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ import (
4343
"time"
4444

4545
"github.com/onsi/ginkgo/v2"
46-
4746
v1 "k8s.io/api/core/v1"
4847
rbacv1 "k8s.io/api/rbac/v1"
4948
storagev1 "k8s.io/api/storage/v1"
@@ -1224,12 +1223,7 @@ func (l *localDriver) PrepareTest(ctx context.Context, f *framework.Framework) *
12241223
framework.ExpectNoError(err)
12251224

12261225
l.hostExec = utils.NewHostExec(f)
1227-
// It is recommended to mount /tmp with options noexec, nodev, nosuid.
1228-
// tmpfs on /tmp type tmpfs (rw,nosuid,nodev,noexec,relatime,seclabel,inode64)
1229-
// This prevents scripts and binaries from being executed from the /tmp directory.
1230-
// This can cause errors like "Permission denied" when executing files from `/tmp`.
1231-
// To pass the test that verifies the execution of files on a volume, we use `/var/tmp` instead of `/tmp`.
1232-
l.ltrMgr = utils.NewLocalResourceManager("local-driver", l.hostExec, "/var/tmp")
1226+
l.ltrMgr = utils.NewLocalResourceManager("local-driver", l.hostExec, "/tmp")
12331227

12341228
// This can't be done in SkipUnsupportedTest because the test framework is not initialized yet
12351229
if l.volumeType == utils.LocalVolumeGCELocalSSD {

test/e2e/storage/utils/local.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ import (
2727
"strings"
2828

2929
"github.com/onsi/ginkgo/v2"
30-
3130
v1 "k8s.io/api/core/v1"
3231
"k8s.io/apimachinery/pkg/util/uuid"
3332
"k8s.io/kubernetes/test/e2e/framework"
@@ -216,7 +215,7 @@ func (l *ltrMgr) cleanupLocalVolumeDirectory(ctx context.Context, ltr *LocalTest
216215
func (l *ltrMgr) setupLocalVolumeDirectoryLink(ctx context.Context, node *v1.Node, parameters map[string]string) *LocalTestResource {
217216
hostDir := l.getTestDir()
218217
hostDirBackend := hostDir + "-backend"
219-
cmd := fmt.Sprintf("mkdir -p %s && ln -s %s %s", hostDirBackend, hostDirBackend, hostDir)
218+
cmd := fmt.Sprintf("mkdir %s && ln -s %s %s", hostDirBackend, hostDirBackend, hostDir)
220219
err := l.hostExec.IssueCommand(ctx, cmd, node)
221220
framework.ExpectNoError(err)
222221
return &LocalTestResource{
@@ -236,7 +235,7 @@ func (l *ltrMgr) cleanupLocalVolumeDirectoryLink(ctx context.Context, ltr *Local
236235

237236
func (l *ltrMgr) setupLocalVolumeDirectoryBindMounted(ctx context.Context, node *v1.Node, parameters map[string]string) *LocalTestResource {
238237
hostDir := l.getTestDir()
239-
cmd := fmt.Sprintf("mkdir -p %s && mount --bind %s %s", hostDir, hostDir, hostDir)
238+
cmd := fmt.Sprintf("mkdir %s && mount --bind %s %s", hostDir, hostDir, hostDir)
240239
err := l.hostExec.IssueCommand(ctx, cmd, node)
241240
framework.ExpectNoError(err)
242241
return &LocalTestResource{
@@ -256,7 +255,7 @@ func (l *ltrMgr) cleanupLocalVolumeDirectoryBindMounted(ctx context.Context, ltr
256255
func (l *ltrMgr) setupLocalVolumeDirectoryLinkBindMounted(ctx context.Context, node *v1.Node, parameters map[string]string) *LocalTestResource {
257256
hostDir := l.getTestDir()
258257
hostDirBackend := hostDir + "-backend"
259-
cmd := fmt.Sprintf("mkdir -p %s && mount --bind %s %s && ln -s %s %s", hostDirBackend, hostDirBackend, hostDirBackend, hostDirBackend, hostDir)
258+
cmd := fmt.Sprintf("mkdir %s && mount --bind %s %s && ln -s %s %s", hostDirBackend, hostDirBackend, hostDirBackend, hostDirBackend, hostDir)
260259
err := l.hostExec.IssueCommand(ctx, cmd, node)
261260
framework.ExpectNoError(err)
262261
return &LocalTestResource{

0 commit comments

Comments
 (0)