diff --git a/Makefile b/Makefile index c972f9cea..cf1f04ecd 100644 --- a/Makefile +++ b/Makefile @@ -270,7 +270,7 @@ docker-tests: load-basic $(RUNTIME_BIN) @$(call install_runtime,$(RUNTIME)-fdlimit,--fdlimit=2000) # Used by TestRlimitNoFile. @$(call install_runtime,$(RUNTIME)-dcache,--fdlimit=2000 --dcache=100) # Used by TestDentryCacheLimit. @$(call install_runtime,$(RUNTIME)-host-uds,--host-uds=all) # Used by TestHostSocketConnect. - @$(call install_runtime,$(RUNTIME)-overlay,--overlay2=root:/tmp) # Used by TestOverlay*. + @$(call install_runtime,$(RUNTIME)-overlay,--overlay2=root:dir=/tmp) # Used by TestOverlay*. @$(call test_runtime,$(RUNTIME),$(INTEGRATION_TARGETS) //test/e2e:integration_runtime_test) .PHONY: docker-tests diff --git a/g3doc/user_guide/filesystem.md b/g3doc/user_guide/filesystem.md index 1774e809e..a43e6681b 100644 --- a/g3doc/user_guide/filesystem.md +++ b/g3doc/user_guide/filesystem.md @@ -47,9 +47,9 @@ up container memory usage. To circumvent this, you can have root mount's upper layer (tmpfs) be backed by a host file, so all file data is stored on disk. The newer `--overlay2` flag allows you to achieve these. You can specify -`--overlay2=root:/dir/path` in `runtimeArgs`. `/dir/path` can be any existing -directory inside which the tmpfs filestore file will be created. When the -container exits, this filestore file will be destroyed. +`--overlay2=root:dir=/dir/path` in `runtimeArgs`. `/dir/path` should be an +existing directory inside which the tmpfs filestore file will be created. When +the container exits, this filestore file will be destroyed. ## Shared root filesystem diff --git a/runsc/cli/main.go b/runsc/cli/main.go index 1ac26ea07..3321e64b8 100644 --- a/runsc/cli/main.go +++ b/runsc/cli/main.go @@ -227,7 +227,7 @@ func Main(version string) { log.Infof("\t\tPlatform: %v", conf.Platform) log.Infof("\t\tFileAccess: %v", conf.FileAccess) overlay2 := conf.GetOverlay2() - log.Infof("\t\tOverlay: Root=%t, SubMounts=%t, FilestoreDir=%q", overlay2.RootMount, overlay2.SubMounts, overlay2.FilestoreDir) + log.Infof("\t\tOverlay: Root=%t, SubMounts=%t, Medium=%q", overlay2.RootMount, overlay2.SubMounts, overlay2.Medium) log.Infof("\t\tNetwork: %v, logging: %t", conf.Network, conf.LogPackets) log.Infof("\t\tStrace: %t, max size: %d, syscalls: %s", conf.Strace, conf.StraceLogSize, conf.StraceSyscalls) log.Infof("\t\tIOURING: %t", conf.IOUring) diff --git a/runsc/config/config.go b/runsc/config/config.go index 693191d0b..a57ab38d1 100644 --- a/runsc/config/config.go +++ b/runsc/config/config.go @@ -19,6 +19,7 @@ package config import ( "fmt" + "path/filepath" "strconv" "strings" "time" @@ -346,8 +347,8 @@ func (c *Config) GetOverlay2() Overlay2 { if c.Overlay2.Enabled() { panic(fmt.Sprintf("Overlay2 cannot be set when --overlay=true")) } - // Using deprecated flag, honor it to avoid breaking users. - return Overlay2{RootMount: true, SubMounts: true, FilestoreDir: ""} + // Using a deprecated flag, honor it to avoid breaking users. + return Overlay2{RootMount: true, SubMounts: true, Medium: "memory"} } return c.Overlay2 } @@ -643,9 +644,9 @@ func (g HostFifo) AllowOpen() bool { // Overlay2 holds the configuration for setting up overlay filesystems for the // container. type Overlay2 struct { - RootMount bool - SubMounts bool - FilestoreDir string + RootMount bool + SubMounts bool + Medium string } func defaultOverlay2() *Overlay2 { @@ -673,11 +674,16 @@ func (o *Overlay2) Set(v string) error { return fmt.Errorf("unexpected mount specifier for --overlay2: %q", mount) } - switch medium := vs[1]; medium { - case "memory": - o.FilestoreDir = "" + o.Medium = vs[1] + switch o.Medium { + case "memory": // OK default: - o.FilestoreDir = medium + if !strings.HasPrefix(o.Medium, "dir=") { + return fmt.Errorf("unexpected medium specifier for --overlay2: %q", o.Medium) + } + if hostFileDir := strings.TrimPrefix(o.Medium, "dir="); !filepath.IsAbs(hostFileDir) { + return fmt.Errorf("overlay host file directory should be an absolute path, got %q", hostFileDir) + } } return nil } @@ -702,17 +708,30 @@ func (o Overlay2) String() string { panic("invalid state of subMounts = true and rootMount = false") } - res += ":" - switch o.FilestoreDir { - case "": - res += "memory" - default: - res += o.FilestoreDir - } - return res + return res + ":" + o.Medium } -// Enabled returns true if overlay option is enabled for any mounts. +// Enabled returns true if the overlay option is enabled for any mounts. func (o *Overlay2) Enabled() bool { return o.RootMount || o.SubMounts } + +// IsBackedByHostFile indicates whether the overlay is backed by a host file. +func (o *Overlay2) IsBackedByHostFile() bool { + return o.Enabled() && o.Medium != "memory" +} + +// HostFileDir indicates the directory in which the overlay-backing host file +// should be created. +// +// Precondition: o.IsBackedByHostFile() == true. +func (o *Overlay2) HostFileDir() string { + if !strings.HasPrefix(o.Medium, "dir=") { + panic(fmt.Sprintf("Overlay2.Medium = %q does not have dir= prefix when overlay is backed by a host file", o.Medium)) + } + hostFileDir := strings.TrimPrefix(o.Medium, "dir=") + if !filepath.IsAbs(hostFileDir) { + panic(fmt.Sprintf("overlay host file directory should be an absolute path, got %q", hostFileDir)) + } + return hostFileDir +} diff --git a/runsc/config/config_test.go b/runsc/config/config_test.go index b4f512dcb..ed64b079d 100644 --- a/runsc/config/config_test.go +++ b/runsc/config/config_test.go @@ -115,41 +115,59 @@ func TestToFlags(t *testing.T) { func TestInvalidFlags(t *testing.T) { for _, tc := range []struct { name string + value string error string }{ { name: "file-access", + value: "invalid", error: "invalid file access type", }, { name: "network", + value: "invalid", error: "invalid network type", }, { name: "qdisc", + value: "invalid", error: "invalid qdisc", }, { name: "watchdog-action", + value: "invalid", error: "invalid watchdog action", }, { name: "ref-leak-mode", + value: "invalid", error: "invalid ref leak mode", }, { name: "host-uds", + value: "invalid", error: "invalid host UDS", }, { name: "host-fifo", + value: "invalid", error: "invalid host fifo", }, + { + name: "overlay2", + value: "root:/tmp", + error: "unexpected medium specifier for --overlay2: \"/tmp\"", + }, + { + name: "overlay2", + value: "root:dir=tmp", + error: "overlay host file directory should be an absolute path, got \"tmp\"", + }, } { t.Run(tc.name, func(t *testing.T) { testFlags := flag.NewFlagSet("test", flag.ContinueOnError) RegisterFlags(testFlags) - if err := testFlags.Lookup(tc.name).Value.Set("invalid"); err == nil || !strings.Contains(err.Error(), tc.error) { + if err := testFlags.Lookup(tc.name).Value.Set(tc.value); err == nil || !strings.Contains(err.Error(), tc.error) { t.Errorf("flag.Value.Set(invalid) wrong error reported: %v", err) } }) diff --git a/runsc/config/flags.go b/runsc/config/flags.go index 1f3cbf6df..1717d8508 100644 --- a/runsc/config/flags.go +++ b/runsc/config/flags.go @@ -83,7 +83,7 @@ func RegisterFlags(flagSet *flag.FlagSet) { flagSet.Var(fileAccessTypePtr(FileAccessExclusive), "file-access", "specifies which filesystem validation to use for the root mount: exclusive (default), shared.") flagSet.Var(fileAccessTypePtr(FileAccessShared), "file-access-mounts", "specifies which filesystem validation to use for volumes other than the root mount: shared (default), exclusive.") flagSet.Bool("overlay", false, "DEPRECATED: use --overlay2=all:memory to achieve the same effect") - flagSet.Var(defaultOverlay2(), "overlay2", "wrap mounts with overlayfs. Format is {mount}:{medium}, where 'mount' can be 'root' or 'all' and medium can be 'memory' or existing directory path in which filestore will be created. 'none' will turn overlay mode off.") + flagSet.Var(defaultOverlay2(), "overlay2", "wrap mounts with overlayfs. Format is {mount}:{medium}, where 'mount' can be 'root' or 'all' and medium can be 'memory' or 'dir=/abs/dir/path' in which filestore will be created. 'none' will turn overlay mode off.") flagSet.Bool("fsgofer-host-uds", false, "DEPRECATED: use host-uds=all") flagSet.Var(hostUDSPtr(HostUDSNone), "host-uds", "controls permission to access host Unix-domain sockets. Values: none|open|create|all, default: none") flagSet.Var(hostFifoPtr(HostFifoNone), "host-fifo", "controls permission to access host FIFOs (or named pipes). Values: none|open, default: none") diff --git a/runsc/container/container.go b/runsc/container/container.go index 757ba7d5a..c07b0fce7 100644 --- a/runsc/container/container.go +++ b/runsc/container/container.go @@ -776,12 +776,13 @@ func (c *Container) Destroy() error { } func createOverlayFilestore(overlay2 config.Overlay2) (*os.File, error) { - if overlay2.FilestoreDir == "" { + if !overlay2.IsBackedByHostFile() { return nil, nil } - fileInfo, err := os.Stat(overlay2.FilestoreDir) + filestoreDir := overlay2.HostFileDir() + fileInfo, err := os.Stat(filestoreDir) if err != nil { - return nil, fmt.Errorf("failed to stat overlay filestore directory %q: %v", overlay2.FilestoreDir, err) + return nil, fmt.Errorf("failed to stat overlay filestore directory %q: %v", filestoreDir, err) } if !fileInfo.IsDir() { return nil, fmt.Errorf("overlay2 flag should specify an existing directory") @@ -791,9 +792,9 @@ func createOverlayFilestore(overlay2 config.Overlay2) (*os.File, error) { // it is not supported on all filesystems. So we simulate it by creating a // named file and then immediately unlinking it while keeping an FD on it. // This file will be deleted when the container exits. - filestoreFile, err := os.CreateTemp(overlay2.FilestoreDir, "runsc-overlay-filestore-*") + filestoreFile, err := os.CreateTemp(filestoreDir, "runsc-overlay-filestore-*") if err != nil { - return nil, fmt.Errorf("failed to create a temporary file inside %q: %v", overlay2.FilestoreDir, err) + return nil, fmt.Errorf("failed to create a temporary file inside %q: %v", filestoreDir, err) } if err := unix.Unlink(filestoreFile.Name()); err != nil { return nil, fmt.Errorf("failed to unlink temporary file %q: %v", filestoreFile.Name(), err) diff --git a/runsc/container/multi_container_test.go b/runsc/container/multi_container_test.go index 6626962f6..23f799953 100644 --- a/runsc/container/multi_container_test.go +++ b/runsc/container/multi_container_test.go @@ -2232,8 +2232,8 @@ func TestMultiContainerOverlayLeaks(t *testing.T) { // Configure root overlay backed by a file from /tmp. conf.Overlay2 = config.Overlay2{ - RootMount: true, - FilestoreDir: "/tmp", + RootMount: true, + Medium: "dir=/tmp", } // Root container will just sleep. @@ -2313,6 +2313,7 @@ func TestMultiContainerMemoryLeakStress(t *testing.T) { // files in the root directory. conf.Overlay2 = config.Overlay2{ RootMount: true, + Medium: "memory", } // Root container will just sleep. diff --git a/runsc/container/shared_volume_test.go b/runsc/container/shared_volume_test.go index ff84485d6..8d8402886 100644 --- a/runsc/container/shared_volume_test.go +++ b/runsc/container/shared_volume_test.go @@ -270,9 +270,9 @@ func TestSharedVolumeFile(t *testing.T) { func TestSharedVolumeOverlay(t *testing.T) { conf := testutil.TestConfig(t) conf.Overlay2 = config.Overlay2{ - RootMount: true, - SubMounts: true, - FilestoreDir: "/tmp", + RootMount: true, + SubMounts: true, + Medium: "dir=/tmp", } // File that will be used to check consistency inside/outside sandbox. diff --git a/test/runner/main.go b/test/runner/main.go index 685e19026..0974eb394 100644 --- a/test/runner/main.go +++ b/test/runner/main.go @@ -206,7 +206,7 @@ func runRunsc(tc *gtest.TestCase, spec *specs.Spec) error { "-file-access", *fileAccess, } if *overlay { - args = append(args, "-overlay2=all:/tmp") + args = append(args, "-overlay2=all:dir=/tmp") } if *debug { args = append(args, "-debug", "-log-packets=true")