diff options
| -rw-r--r-- | runsc/container/container.go | 58 | ||||
| -rw-r--r-- | runsc/container/multi_container_test.go | 72 | ||||
| -rw-r--r-- | runsc/test/testutil/testutil.go | 11 |
3 files changed, 116 insertions, 25 deletions
diff --git a/runsc/container/container.go b/runsc/container/container.go index 27e9c2e0f..2a8453931 100644 --- a/runsc/container/container.go +++ b/runsc/container/container.go @@ -138,6 +138,34 @@ type Container struct { RootContainerDir string } +// loadSandbox loads all containers that belong to the sandbox with the given +// ID. +func loadSandbox(rootDir, id string) ([]*Container, error) { + cids, err := List(rootDir) + if err != nil { + return nil, err + } + + // Load the container metadata. + var containers []*Container + for _, cid := range cids { + container, err := Load(rootDir, cid) + if err != nil { + // Container file may not exist if it raced with creation/deletion or + // directory was left behind. Load provides a snapshot in time, so it's + // fine to skip it. + if os.IsNotExist(err) { + continue + } + return nil, fmt.Errorf("loading container %q: %v", id, err) + } + if container.Sandbox.ID == id { + containers = append(containers, container) + } + } + return containers, nil +} + // Load loads a container with the given id from a metadata file. id may be an // abbreviation of the full container id, in which case Load loads the // container to which id unambiguously refers to. @@ -180,7 +208,7 @@ func Load(rootDir, id string) (*Container, error) { // If the status is "Running" or "Created", check that the sandbox // process still exists, and set it to Stopped if it does not. // - // This is inherently racey. + // This is inherently racy. if c.Status == Running || c.Status == Created { // Check if the sandbox process is still running. if !c.isSandboxRunning() { @@ -237,7 +265,13 @@ func List(rootDir string) ([]string, error) { } var out []string for _, f := range fs { - out = append(out, f.Name()) + // Filter out directories that do no belong to a container. + cid := f.Name() + if validateID(cid) == nil { + if _, err := os.Stat(filepath.Join(rootDir, cid, metadataFilename)); err == nil { + out = append(out, f.Name()) + } + } } return out, nil } @@ -1108,6 +1142,10 @@ func runInCgroup(cg *cgroup.Cgroup, fn func() error) error { // adjustOOMScoreAdj sets the oom_score_adj for the sandbox and all gofers. // oom_score_adj is set to the lowest oom_score_adj among the containers // running in the sandbox. +// +// TODO(gvisor.dev/issue/512): This call could race with other containers being +// created at the same time and end up setting the wrong oom_score_adj to the +// sandbox. func (c *Container) adjustOOMScoreAdj(conf *boot.Config) error { // If this container's OOMScoreAdj is nil then we can exit early as no // change should be made to oom_score_adj for the sandbox. @@ -1115,21 +1153,9 @@ func (c *Container) adjustOOMScoreAdj(conf *boot.Config) error { return nil } - ids, err := List(conf.RootDir) + containers, err := loadSandbox(conf.RootDir, c.Sandbox.ID) if err != nil { - return err - } - - // Load the container metadata. - var containers []*Container - for _, id := range ids { - container, err := Load(conf.RootDir, id) - if err != nil { - return fmt.Errorf("loading container %q: %v", id, err) - } - if container.Sandbox.ID == c.Sandbox.ID { - containers = append(containers, container) - } + return fmt.Errorf("loading sandbox containers: %v", err) } // Get the lowest score for all containers. diff --git a/runsc/container/multi_container_test.go b/runsc/container/multi_container_test.go index 2ef065a15..2d51fecc6 100644 --- a/runsc/container/multi_container_test.go +++ b/runsc/container/multi_container_test.go @@ -60,11 +60,14 @@ func createSpecs(cmds ...[]string) ([]*specs.Spec, []string) { } func startContainers(conf *boot.Config, specs []*specs.Spec, ids []string) ([]*Container, func(), error) { - rootDir, err := testutil.SetupRootDir() - if err != nil { - return nil, nil, fmt.Errorf("error creating root dir: %v", err) + // Setup root dir if one hasn't been provided. + if len(conf.RootDir) == 0 { + rootDir, err := testutil.SetupRootDir() + if err != nil { + return nil, nil, fmt.Errorf("error creating root dir: %v", err) + } + conf.RootDir = rootDir } - conf.RootDir = rootDir var containers []*Container var bundles []string @@ -75,7 +78,7 @@ func startContainers(conf *boot.Config, specs []*specs.Spec, ids []string) ([]*C for _, b := range bundles { os.RemoveAll(b) } - os.RemoveAll(rootDir) + os.RemoveAll(conf.RootDir) } for i, spec := range specs { bundleDir, err := testutil.SetupBundleDir(spec) @@ -1423,3 +1426,62 @@ func TestMultiContainerGoferKilled(t *testing.T) { } } } + +func TestMultiContainerLoadSandbox(t *testing.T) { + sleep := []string{"sleep", "100"} + specs, ids := createSpecs(sleep, sleep, sleep) + conf := testutil.TestConfig() + + // Create containers for the sandbox. + wants, cleanup, err := startContainers(conf, specs, ids) + if err != nil { + t.Fatalf("error starting containers: %v", err) + } + defer cleanup() + + // Then create unrelated containers. + for i := 0; i < 3; i++ { + specs, ids = createSpecs(sleep, sleep, sleep) + _, cleanup, err = startContainers(conf, specs, ids) + if err != nil { + t.Fatalf("error starting containers: %v", err) + } + defer cleanup() + } + + // Create an unrelated directory under root. + dir := filepath.Join(conf.RootDir, "not-a-container") + if err := os.MkdirAll(dir, 0755); err != nil { + t.Fatalf("os.MkdirAll(%q)=%v", dir, err) + } + + // Create a valid but empty container directory. + randomCID := testutil.UniqueContainerID() + dir = filepath.Join(conf.RootDir, randomCID) + if err := os.MkdirAll(dir, 0755); err != nil { + t.Fatalf("os.MkdirAll(%q)=%v", dir, err) + } + + // Load the sandbox and check that the correct containers were returned. + id := wants[0].Sandbox.ID + gots, err := loadSandbox(conf.RootDir, id) + if err != nil { + t.Fatalf("loadSandbox()=%v", err) + } + wantIDs := make(map[string]struct{}) + for _, want := range wants { + wantIDs[want.ID] = struct{}{} + } + for _, got := range gots { + if got.Sandbox.ID != id { + t.Errorf("wrong sandbox ID, got: %v, want: %v", got.Sandbox.ID, id) + } + if _, ok := wantIDs[got.ID]; !ok { + t.Errorf("wrong container ID, got: %v, wants: %v", got.ID, wantIDs) + } + delete(wantIDs, got.ID) + } + if len(wantIDs) != 0 { + t.Errorf("containers not found: %v", wantIDs) + } +} diff --git a/runsc/test/testutil/testutil.go b/runsc/test/testutil/testutil.go index 406f6d08a..e288c7758 100644 --- a/runsc/test/testutil/testutil.go +++ b/runsc/test/testutil/testutil.go @@ -189,11 +189,14 @@ func SetupRootDir() (string, error) { // SetupContainer creates a bundle and root dir for the container, generates a // test config, and writes the spec to config.json in the bundle dir. func SetupContainer(spec *specs.Spec, conf *boot.Config) (rootDir, bundleDir string, err error) { - rootDir, err = SetupRootDir() - if err != nil { - return "", "", err + // Setup root dir if one hasn't been provided. + if len(conf.RootDir) == 0 { + rootDir, err = SetupRootDir() + if err != nil { + return "", "", err + } + conf.RootDir = rootDir } - conf.RootDir = rootDir bundleDir, err = SetupBundleDir(spec) return rootDir, bundleDir, err } |