From 44e0246e29998097dd0099a91f60202a5dd2eee5 Mon Sep 17 00:00:00 2001 From: Ayush Ranjan Date: Tue, 10 Jan 2023 14:02:14 -0800 Subject: [PATCH] Add multi-container test to check for memory leaks when using overlay filestore. When a container exits, it releases all its filesystems. If an overlay was configured, the tmpfs upper mount is released. Any files created in the overlay should be deleted and the memory released. Add a test to verify that no such memory is leaked when containers exit. I had to add a new OverlayFileUsage method to containerManager to test this accurately. I tested the test by intentionally introducing a bug in tmpfs to not release data from its memory file and this test failed with the following errors: ``` overlay filestore usage changed: old = 0, new = 4096 overlay filestore usage changed: old = 0, new = 8192 overlay filestore usage changed: old = 0, new = 12288 ``` This shows that the file created by each sub-containers occupied a different page in the filestore, which was not cleaned up and hence the test failed. PiperOrigin-RevId: 501088426 --- runsc/boot/controller.go | 19 ++++++ runsc/container/multi_container_test.go | 79 +++++++++++++++++++++++++ runsc/sandbox/sandbox.go | 12 ++++ test/cmd/test_app/main.go | 56 ++++++++++++++++++ 4 files changed, 166 insertions(+) diff --git a/runsc/boot/controller.go b/runsc/boot/controller.go index 9ca112540..a989cb858 100644 --- a/runsc/boot/controller.go +++ b/runsc/boot/controller.go @@ -92,6 +92,10 @@ const ( // ContMgrProcfsDump dumps sandbox procfs state. ContMgrProcfsDump = "containerManager.ProcfsDump" + + // CongMgrOverlayFileUsage returns the current usage (bytes) of the overlay + // filestore. + CongMgrOverlayFileUsage = "containerManager.OverlayFileUsage" ) const ( @@ -630,3 +634,18 @@ func (cm *containerManager) ProcfsDump(_ *struct{}, out *[]procfs.ProcessProcfsD } return nil } + +// OverlayFileUsage returns the current usage (bytes) of the overlay filestore. +func (cm *containerManager) OverlayFileUsage(_ *struct{}, out *uint64) error { + *out = 0 + if cm.l.root.overlayFilestore == nil { + return nil + } + + usage, err := cm.l.root.overlayFilestore.TotalUsage() + if err != nil { + return err + } + *out = usage + return nil +} diff --git a/runsc/container/multi_container_test.go b/runsc/container/multi_container_test.go index ac840b3e0..61553d5f2 100644 --- a/runsc/container/multi_container_test.go +++ b/runsc/container/multi_container_test.go @@ -2212,3 +2212,82 @@ func TestMultiContainerShm(t *testing.T) { t.Fatalf("wrong output, want: %q, got: %v", want, out) } } + +// Test that using file-backed overlay does not lead to memory leak or leaks +// in the host-file backing the overlay. +func TestMultiContainerOverlayLeaks(t *testing.T) { + conf := testutil.TestConfig(t) + app, err := testutil.FindFile("test/cmd/test_app/test_app") + if err != nil { + t.Fatal("error finding test_app:", err) + } + + rootDir, cleanup, err := testutil.SetupRootDir() + if err != nil { + t.Fatalf("error creating root dir: %v", err) + } + defer cleanup() + conf.RootDir = rootDir + + // Configure root overlay backed by a file from /tmp. + conf.Overlay2 = config.Overlay2{ + RootMount: true, + FilestoreDir: "/tmp", + } + + // Root container will just sleep. + sleep := []string{"sleep", "100"} + // Since all containers share the same conf.RootDir, and root filesystems + // have overlay enabled, the root directory should never be modified. Hence, + // creating files at the same locations should not lead to EEXIST error. + createFsTree := []string{app, "fsTreeCreate", "--depth=10", "--file-per-level=10", "--file-size=4096"} + testSpecs, ids := createSpecs(sleep, createFsTree, createFsTree, createFsTree) + // Make sure none of the root filesystems are read-only, otherwise we won't + // be able to create the file. + for _, s := range testSpecs { + s.Root.Readonly = false + } + + // Start the root container. + rootCont, cleanup, err := startContainers(conf, testSpecs[:1], ids[:1]) + if err != nil { + t.Fatalf("error starting containers: %v", err) + } + defer cleanup() + + // Remember the overlay filestore usage right now. + oldOverlayUsage, err := rootCont[0].Sandbox.OverlayFileUsage() + if err != nil { + t.Fatalf("sandbox.OverlayFileUsage failed: %v", err) + } + + subConts, cleanup, err := startContainers(conf, testSpecs[1:], ids[1:]) + if err != nil { + t.Fatalf("error starting containers: %v", err) + } + defer cleanup() + + for i, c := range subConts { + // Wait for the sub-container to stop. + if ws, err := c.Wait(); err != nil { + t.Errorf("failed to wait for subcontainer number %d: %v", i, err) + } else if es := ws.ExitStatus(); es != 0 { + t.Errorf("subcontainer number %d exited with non-zero status %d", i, es) + } + } + + // Give the reclaimer goroutine some time to reclaim. + time.Sleep(3 * time.Second) + + // Make sure the overlay filestore usage is back to what it was. The + // sub-containers create files in overlay. But it should have been cleaned + // up once the container exited and the reclaimer ran. + newOverlayUsage, err := rootCont[0].Sandbox.OverlayFileUsage() + if err != nil { + t.Fatalf("sandbox.OverlayFileUsage failed: %v", err) + } + + if oldOverlayUsage != newOverlayUsage { + t.Errorf("overlay filestore usage changed: old = %d, new = %d", oldOverlayUsage, newOverlayUsage) + } +} diff --git a/runsc/sandbox/sandbox.go b/runsc/sandbox/sandbox.go index 5cf222b7d..6d2d2d335 100644 --- a/runsc/sandbox/sandbox.go +++ b/runsc/sandbox/sandbox.go @@ -1317,6 +1317,18 @@ func (s *Sandbox) ChangeLogging(args control.LoggingArgs) error { return nil } +// OverlayFileUsage returns the current usage (bytes) of the overlay filestore. +func (s *Sandbox) OverlayFileUsage() (uint64, error) { + conn, err := s.sandboxConnect() + if err != nil { + return 0, err + } + defer conn.Close() + + var usage uint64 + return usage, conn.Call(boot.CongMgrOverlayFileUsage, nil, &usage) +} + // DestroyContainer destroys the given container. If it is the root container, // then the entire sandbox is destroyed. func (s *Sandbox) DestroyContainer(cid string) error { diff --git a/test/cmd/test_app/main.go b/test/cmd/test_app/main.go index 9fe792cf2..092221fff 100644 --- a/test/cmd/test_app/main.go +++ b/test/cmd/test_app/main.go @@ -22,9 +22,11 @@ import ( "io" "io/ioutil" "log" + "math/rand" "net" "os" "os/exec" + "path/filepath" "regexp" "strconv" sys "syscall" @@ -48,6 +50,7 @@ func main() { subcommands.Register(new(syscall), "") subcommands.Register(new(taskTree), "") subcommands.Register(new(uds), "") + subcommands.Register(new(fsTreeCreator), "") flag.Parse() @@ -55,6 +58,59 @@ func main() { os.Exit(int(exitCode)) } +type fsTreeCreator struct { + depth uint + numFilesPerLevel uint + fileSize uint +} + +// Name implements subcommands.Command.Name. +func (*fsTreeCreator) Name() string { + return "fsTreeCreate" +} + +// Synopsis implements subcommands.Command.Synopsys. +func (*fsTreeCreator) Synopsis() string { + return "creates a filesystem tree of a certain depth, with a certain number of files on each level and each file with a certain size. Some randomization is added on top of this" +} + +// Usage implements subcommands.Command.Usage. +func (*fsTreeCreator) Usage() string { + return "fsTreeCreate " +} + +// SetFlags implements subcommands.Command.SetFlags. +func (c *fsTreeCreator) SetFlags(f *flag.FlagSet) { + f.UintVar(&c.depth, "depth", 10, "number of levels to create") + f.UintVar(&c.numFilesPerLevel, "file-per-level", 10, "number of files to create per level") + f.UintVar(&c.fileSize, "file-size", 4096, "size of each file") +} + +// Execute implements subcommands.Command.Execute. +func (c *fsTreeCreator) Execute(ctx context.Context, f *flag.FlagSet, args ...any) subcommands.ExitStatus { + depth := c.depth + uint(rand.Uint32())%c.depth + numFilesPerLevel := c.numFilesPerLevel + uint(rand.Uint32())%c.numFilesPerLevel + fileSize := c.fileSize + uint(rand.Uint32())%c.fileSize + + curDir := "/" + data := make([]byte, fileSize) + rand.Read(data) + for i := uint(0); i < depth; i++ { + for j := uint(0); j < numFilesPerLevel; j++ { + filePath := filepath.Join(curDir, fmt.Sprintf("file%d", j)) + if err := os.WriteFile(filePath, data, 0666); err != nil { + log.Fatalf("error writing file %q: %v", filePath, err) + } + } + nextDir := filepath.Join(curDir, "dir") + if err := os.Mkdir(nextDir, 0777); err != nil { + log.Fatalf("error creating directory %q: %v", nextDir, err) + } + curDir = nextDir + } + return subcommands.ExitSuccess +} + type uds struct { fileName string socketPath string