From c4fe64c5ef18c99854abe262fbfb2a7100fd408d Mon Sep 17 00:00:00 2001 From: Ayush Ranjan Date: Thu, 26 Jan 2023 22:54:18 -0800 Subject: [PATCH] Add dir= prefix in overlay2 flag's medium. This is to make it clear that the host file will be created inside this directory. This also makes it look cleaner when other medium options are added later. PiperOrigin-RevId: 505033408 --- Makefile | 2 +- g3doc/user_guide/filesystem.md | 6 +-- runsc/cli/main.go | 2 +- runsc/config/config.go | 55 +++++++++++++++++-------- runsc/config/config_test.go | 20 ++++++++- runsc/config/flags.go | 2 +- runsc/container/container.go | 11 ++--- runsc/container/multi_container_test.go | 5 ++- runsc/container/shared_volume_test.go | 6 +-- test/runner/main.go | 2 +- 10 files changed, 75 insertions(+), 36 deletions(-) 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")