diff options
author | Fabricio Voznika <fvoznika@google.com> | 2018-05-29 17:57:26 -0700 |
---|---|---|
committer | Shentubot <shentubot@google.com> | 2018-05-29 17:58:12 -0700 |
commit | 812e83d3bbb99d4fa1ece4712a1ac85e84fe6ec3 (patch) | |
tree | 559b9a5a5fc2b37899bf2ffb915e1ed4eadf6bfa /runsc | |
parent | c5dc873e441706e8aaff7389e26c862f1386c6a8 (diff) |
Supress error when deleting non-existing container with --force
This addresses the first issue reported in #59. CRI-O expects runsc to
return success to delete when --force is used with a non-existing container.
PiperOrigin-RevId: 198487418
Change-Id: If7660e8fdab1eb29549d0a7a45ea82e20a1d4f4a
Diffstat (limited to 'runsc')
-rw-r--r-- | runsc/cmd/BUILD | 6 | ||||
-rw-r--r-- | runsc/cmd/delete.go | 25 | ||||
-rw-r--r-- | runsc/cmd/delete_test.go | 41 | ||||
-rw-r--r-- | runsc/container/container.go | 35 |
4 files changed, 75 insertions, 32 deletions
diff --git a/runsc/cmd/BUILD b/runsc/cmd/BUILD index 08aaee996..4b4afa4a0 100644 --- a/runsc/cmd/BUILD +++ b/runsc/cmd/BUILD @@ -44,13 +44,17 @@ go_library( go_test( name = "cmd_test", size = "small", - srcs = ["exec_test.go"], + srcs = [ + "delete_test.go", + "exec_test.go", + ], embed = [":cmd"], deps = [ "//pkg/abi/linux", "//pkg/sentry/control", "//pkg/sentry/kernel/auth", "//pkg/urpc", + "//runsc/boot", "@com_github_google_go-cmp//cmp:go_default_library", "@com_github_google_go-cmp//cmp/cmpopts:go_default_library", "@com_github_opencontainers_runtime-spec//specs-go:go_default_library", diff --git a/runsc/cmd/delete.go b/runsc/cmd/delete.go index 769a11c45..46de5f348 100644 --- a/runsc/cmd/delete.go +++ b/runsc/cmd/delete.go @@ -15,9 +15,13 @@ package cmd import ( + "fmt" + "os" + "context" "flag" "github.com/google/subcommands" + "gvisor.googlesource.com/gvisor/pkg/log" "gvisor.googlesource.com/gvisor/runsc/boot" "gvisor.googlesource.com/gvisor/runsc/container" ) @@ -56,19 +60,28 @@ func (d *Delete) Execute(_ context.Context, f *flag.FlagSet, args ...interface{} } conf := args[0].(*boot.Config) + if err := d.execute(f.Args(), conf); err != nil { + Fatalf("%v", err) + } + return subcommands.ExitSuccess +} - for i := 0; i < f.NArg(); i++ { - id := f.Arg(i) +func (d *Delete) execute(ids []string, conf *boot.Config) error { + for _, id := range ids { c, err := container.Load(conf.RootDir, id) if err != nil { - Fatalf("error loading container %q: %v", id, err) + if os.IsNotExist(err) && d.force { + log.Warningf("couldn't find container %q: %v", id, err) + return nil + } + return fmt.Errorf("error loading container %q: %v", id, err) } if !d.force && (c.Status == container.Running) { - Fatalf("cannot stop running container without --force flag") + return fmt.Errorf("cannot stop running container without --force flag") } if err := c.Destroy(); err != nil { - Fatalf("error destroying container: %v", err) + return fmt.Errorf("error destroying container: %v", err) } } - return subcommands.ExitSuccess + return nil } diff --git a/runsc/cmd/delete_test.go b/runsc/cmd/delete_test.go new file mode 100644 index 000000000..928e9ee2c --- /dev/null +++ b/runsc/cmd/delete_test.go @@ -0,0 +1,41 @@ +// Copyright 2018 Google Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package cmd + +import ( + "io/ioutil" + "testing" + + "gvisor.googlesource.com/gvisor/runsc/boot" +) + +func TestNotFound(t *testing.T) { + ids := []string{"123"} + dir, err := ioutil.TempDir("", "metadata") + if err != nil { + t.Fatalf("error creating dir: %v", err) + } + conf := &boot.Config{RootDir: dir} + + d := Delete{} + if err := d.execute(ids, conf); err == nil { + t.Error("Deleting non-existend container should have failed") + } + + d = Delete{force: true} + if err := d.execute(ids, conf); err != nil { + t.Errorf("Deleting non-existend container with --force should NOT have failed: %v", err) + } +} diff --git a/runsc/container/container.go b/runsc/container/container.go index ae86e40c9..f20ec2453 100644 --- a/runsc/container/container.go +++ b/runsc/container/container.go @@ -93,21 +93,19 @@ type Container struct { } // Load loads a container with the given id from a metadata file. +// Returns ErrNotExist if container doesn't exits. func Load(rootDir, id string) (*Container, error) { log.Debugf("Load container %q %q", rootDir, id) if err := validateID(id); err != nil { return nil, err } - cRoot := filepath.Join(rootDir, id) - if !exists(cRoot) { - return nil, fmt.Errorf("container with id %q does not exist", id) - } - metaFile := filepath.Join(cRoot, metadataFilename) - if !exists(metaFile) { - return nil, fmt.Errorf("container with id %q does not have metadata file %q", id, metaFile) - } + metaFile := filepath.Join(rootDir, id, metadataFilename) metaBytes, err := ioutil.ReadFile(metaFile) if err != nil { + if os.IsNotExist(err) { + // Preserve error so that callers can distinguish 'not found' errors. + return nil, err + } return nil, fmt.Errorf("error reading container metadata file %q: %v", metaFile, err) } var c Container @@ -161,8 +159,10 @@ func Create(id string, spec *specs.Spec, conf *boot.Config, bundleDir, consoleSo } containerRoot := filepath.Join(conf.RootDir, id) - if exists(containerRoot) { - return nil, fmt.Errorf("container with id %q already exists: %q ", id, containerRoot) + if _, err := os.Stat(containerRoot); err == nil { + return nil, fmt.Errorf("container with id %q already exists: %q", id, containerRoot) + } else if !os.IsNotExist(err) { + return nil, fmt.Errorf("error looking for existing container in %q: %v", containerRoot, err) } c := &Container{ @@ -328,11 +328,6 @@ func (c *Container) Destroy() error { return err } - // Then destroy all the metadata. - if err := os.RemoveAll(c.Root); err != nil { - log.Warningf("Failed to delete container root directory %q, err: %v", c.Root, err) - } - // "If any poststop hook fails, the runtime MUST log a warning, but the // remaining hooks and lifecycle continue as if the hook had succeeded". if c.Spec.Hooks != nil && (c.Status == Created || c.Status == Running) { @@ -372,13 +367,3 @@ func (c *Container) save() error { } return nil } - -// exists returns true if the given file exists. -func exists(f string) bool { - if _, err := os.Stat(f); err == nil { - return true - } else if !os.IsNotExist(err) { - log.Warningf("error checking for file %q: %v", f, err) - } - return false -} |