From e0c7b6ceb71c8844b144622d13ed73e37092d6e1 Mon Sep 17 00:00:00 2001 From: Qiang Huang Date: Wed, 8 Mar 2017 10:02:39 +0800 Subject: [PATCH] Only allow single container operation As per the discussions in #1156 , we think it's a bad idea to allow multi container operations in runc. So revert it. Signed-off-by: Qiang Huang --- delete.go | 76 ++++++++++++--------------------- man/runc-delete.8.md | 4 +- man/runc-pause.8.md | 2 +- man/runc-resume.8.md | 2 +- man/runc-start.8.md | 6 +-- pause.go | 55 +++++------------------- start.go | 57 ++++++++----------------- tests/integration/delete.bats | 59 -------------------------- tests/integration/pause.bats | 80 ++++++++--------------------------- tests/integration/start.bats | 26 ++++-------- 10 files changed, 87 insertions(+), 280 deletions(-) diff --git a/delete.go b/delete.go index 21d7656a20e..b43dcaa7cd9 100644 --- a/delete.go +++ b/delete.go @@ -27,8 +27,8 @@ func killContainer(container libcontainer.Container) error { var deleteCommand = cli.Command{ Name: "delete", - Usage: "delete any resources held by one or more containers often used with detached containers", - ArgsUsage: ` [container-id...] + Usage: "delete any resources held by the container often used with detached container", + ArgsUsage: ` Where "" is the name for the instance of the container. @@ -45,62 +45,40 @@ status of "ubuntu01" as "stopped" the following will delete resources held for }, }, Action: func(context *cli.Context) error { - if err := checkArgs(context, 1, minArgs); err != nil { + if err := checkArgs(context, 1, exactArgs); err != nil { return err } - hasError := false - factory, err := loadFactory(context) + id := context.Args().First() + container, err := getContainer(context) if err != nil { - return err - } - for _, id := range context.Args() { - container, err := factory.Load(id) - if err != nil { - if lerr, ok := err.(libcontainer.Error); ok && lerr.Code() == libcontainer.ContainerNotExists { - // if there was an aborted start or something of the sort then the container's directory could exist but - // libcontainer does not see it because the state.json file inside that directory was never created. - path := filepath.Join(context.GlobalString("root"), id) - if err = os.RemoveAll(path); err != nil { - fmt.Fprintf(os.Stderr, "remove %s: %v\n", path, err) - } - fmt.Fprintf(os.Stderr, "container %s does not exist\n", id) + if lerr, ok := err.(libcontainer.Error); ok && lerr.Code() == libcontainer.ContainerNotExists { + // if there was an aborted start or something of the sort then the container's directory could exist but + // libcontainer does not see it because the state.json file inside that directory was never created. + path := filepath.Join(context.GlobalString("root"), id) + if e := os.RemoveAll(path); e != nil { + fmt.Fprintf(os.Stderr, "remove %s: %v\n", path, e) } - hasError = true - continue - } - s, err := container.Status() - if err != nil { - fmt.Fprintf(os.Stderr, "status for %s: %v\n", id, err) - hasError = true - continue } - switch s { - case libcontainer.Stopped: - destroy(container) - case libcontainer.Created: - err := killContainer(container) - if err != nil { - fmt.Fprintf(os.Stderr, "kill container %s: %v\n", id, err) - hasError = true - } - default: - if context.Bool("force") { - err := killContainer(container) - if err != nil { - fmt.Fprintf(os.Stderr, "kill container %s: %v\n", id, err) - hasError = true - } - } else { - fmt.Fprintf(os.Stderr, "cannot delete container %s that is not stopped: %s\n", id, s) - hasError = true - } + return err + } + s, err := container.Status() + if err != nil { + return err + } + switch s { + case libcontainer.Stopped: + destroy(container) + case libcontainer.Created: + return killContainer(container) + default: + if context.Bool("force") { + return killContainer(container) + } else { + return fmt.Errorf("cannot delete container %s that is not stopped: %s\n", id, s) } } - if hasError { - return fmt.Errorf("one or more of the container deletions failed") - } return nil }, } diff --git a/man/runc-delete.8.md b/man/runc-delete.8.md index 12ea711b628..42235e39e0c 100644 --- a/man/runc-delete.8.md +++ b/man/runc-delete.8.md @@ -1,8 +1,8 @@ # NAME - runc delete - delete any resources held by one or more containers often used with detached containers + runc delete - delete any resources held by the container often used with detached container # SYNOPSIS - runc delete [command options] [container-id...] + runc delete [command options] Where "" is the name for the instance of the container. diff --git a/man/runc-pause.8.md b/man/runc-pause.8.md index 489bb907c70..4a4265e86e1 100644 --- a/man/runc-pause.8.md +++ b/man/runc-pause.8.md @@ -2,7 +2,7 @@ runc pause - pause suspends all processes inside the container # SYNOPSIS - runc pause [container-id...] + runc pause Where "" is the name for the instance of the container to be paused. diff --git a/man/runc-resume.8.md b/man/runc-resume.8.md index d864a5426b2..627885f1412 100644 --- a/man/runc-resume.8.md +++ b/man/runc-resume.8.md @@ -2,7 +2,7 @@ runc resume - resumes all processes that have been previously paused # SYNOPSIS - runc resume [container-id...] + runc resume Where "" is the name for the instance of the container to be resumed. diff --git a/man/runc-start.8.md b/man/runc-start.8.md index 1e737b20832..89bb3ccbdac 100644 --- a/man/runc-start.8.md +++ b/man/runc-start.8.md @@ -1,12 +1,12 @@ # NAME - runc start - start signals a created container to execute the user defined process + runc start - start executes the user defined process in a created container # SYNOPSIS - runc start [container-id...] + runc start Where "" is your name for the instance of the container that you are starting. The name you provide for the container instance must be unique on your host. # DESCRIPTION - The start command signals the container to start the user's defined process. + The start command executes the user defined process in a created container. diff --git a/pause.go b/pause.go index f08cf4f2613..3b98dbbbf3a 100644 --- a/pause.go +++ b/pause.go @@ -2,17 +2,12 @@ package main -import ( - "fmt" - "os" - - "github.com/urfave/cli" -) +import "github.com/urfave/cli" var pauseCommand = cli.Command{ Name: "pause", Usage: "pause suspends all processes inside the container", - ArgsUsage: ` [container-id...] + ArgsUsage: ` Where "" is the name for the instance of the container to be paused. `, @@ -20,31 +15,17 @@ paused. `, Use runc list to identiy instances of containers and their current status.`, Action: func(context *cli.Context) error { - if err := checkArgs(context, 1, minArgs); err != nil { + if err := checkArgs(context, 1, exactArgs); err != nil { return err } - hasError := false - factory, err := loadFactory(context) + container, err := getContainer(context) if err != nil { return err } - - for _, id := range context.Args() { - container, err := factory.Load(id) - if err != nil { - fmt.Fprintf(os.Stderr, "container %s does not exist\n", id) - hasError = true - continue - } - if err := container.Pause(); err != nil { - fmt.Fprintf(os.Stderr, "pause container %s : %s\n", id, err) - hasError = true - } + if err := container.Pause(); err != nil { + return err } - if hasError { - return fmt.Errorf("one or more of container pause failed") - } return nil }, } @@ -52,7 +33,7 @@ Use runc list to identiy instances of containers and their current status.`, var resumeCommand = cli.Command{ Name: "resume", Usage: "resumes all processes that have been previously paused", - ArgsUsage: ` [container-id...] + ArgsUsage: ` Where "" is the name for the instance of the container to be resumed.`, @@ -60,31 +41,17 @@ resumed.`, Use runc list to identiy instances of containers and their current status.`, Action: func(context *cli.Context) error { - if err := checkArgs(context, 1, minArgs); err != nil { + if err := checkArgs(context, 1, exactArgs); err != nil { return err } - hasError := false - factory, err := loadFactory(context) + container, err := getContainer(context) if err != nil { return err } - - for _, id := range context.Args() { - container, err := factory.Load(id) - if err != nil { - fmt.Fprintf(os.Stderr, "container %s does not exist\n", id) - hasError = true - continue - } - if err := container.Resume(); err != nil { - fmt.Fprintf(os.Stderr, "resume container %s : %s\n", id, err) - hasError = true - } + if err := container.Resume(); err != nil { + return err } - if hasError { - return fmt.Errorf("one or more of container resume failed") - } return nil }, } diff --git a/start.go b/start.go index 43566149157..2bb698b2095 100644 --- a/start.go +++ b/start.go @@ -1,8 +1,8 @@ package main import ( + "errors" "fmt" - "os" "github.com/opencontainers/runc/libcontainer" "github.com/urfave/cli" @@ -11,56 +11,33 @@ import ( var startCommand = cli.Command{ Name: "start", Usage: "executes the user defined process in a created container", - ArgsUsage: ` [container-id...] + ArgsUsage: ` Where "" is your name for the instance of the container that you are starting. The name you provide for the container instance must be unique on your host.`, - Description: `The start command executes the user defined process in a created container .`, + Description: `The start command executes the user defined process in a created container.`, Action: func(context *cli.Context) error { - if err := checkArgs(context, 1, minArgs); err != nil { + if err := checkArgs(context, 1, exactArgs); err != nil { return err } - hasError := false - factory, err := loadFactory(context) + container, err := getContainer(context) if err != nil { return err } - - for _, id := range context.Args() { - container, err := factory.Load(id) - if err != nil { - fmt.Fprintf(os.Stderr, "container %s does not exist\n", id) - hasError = true - continue - } - status, err := container.Status() - if err != nil { - fmt.Fprintf(os.Stderr, "status for %s: %v\n", id, err) - hasError = true - continue - } - switch status { - case libcontainer.Created: - if err := container.Exec(); err != nil { - fmt.Fprintf(os.Stderr, "start for %s failed: %v\n", id, err) - hasError = true - } - case libcontainer.Stopped: - fmt.Fprintln(os.Stderr, "cannot start a container that has stopped") - hasError = true - case libcontainer.Running: - fmt.Fprintln(os.Stderr, "cannot start an already running container") - hasError = true - default: - fmt.Fprintf(os.Stderr, "cannot start a container in the %s state\n", status) - hasError = true - } + status, err := container.Status() + if err != nil { + return err } - - if hasError { - return fmt.Errorf("one or more of container start failed") + switch status { + case libcontainer.Created: + return container.Exec() + case libcontainer.Stopped: + return errors.New("cannot start a container that has stopped") + case libcontainer.Running: + return errors.New("cannot start an already running container") + default: + return fmt.Errorf("cannot start a container in the %s state\n", status) } - return nil }, } diff --git a/tests/integration/delete.bats b/tests/integration/delete.bats index 61a9c4c9eb5..cdadd7dcce5 100644 --- a/tests/integration/delete.bats +++ b/tests/integration/delete.bats @@ -48,62 +48,3 @@ function teardown() { runc state test_busybox [ "$status" -ne 0 ] } - -@test "run delete with multi-containers" { - # create busybox1 detached - runc create --console-socket $CONSOLE_SOCKET test_busybox1 - [ "$status" -eq 0 ] - - testcontainer test_busybox1 created - - # run busybox2 detached - runc run -d --console-socket $CONSOLE_SOCKET test_busybox2 - [ "$status" -eq 0 ] - - wait_for_container 15 1 test_busybox2 - testcontainer test_busybox2 running - - # delete both test_busybox1 and test_busybox2 container - runc delete test_busybox1 test_busybox2 - - runc state test_busybox1 - [ "$status" -ne 0 ] - - runc state test_busybox2 - [ "$status" -eq 0 ] - - runc kill test_busybox2 KILL - # wait for busybox2 to be in the destroyed state - retry 10 1 eval "__runc state test_busybox2 | grep -q 'stopped'" - - # delete test_busybox2 - runc delete test_busybox2 - - runc state test_busybox2 - [ "$status" -ne 0 ] -} - - -@test "run delete --force with multi-containers" { - # create busybox1 detached - runc create --console-socket $CONSOLE_SOCKET test_busybox1 - [ "$status" -eq 0 ] - - testcontainer test_busybox1 created - - # run busybox2 detached - runc run -d --console-socket $CONSOLE_SOCKET test_busybox2 - [ "$status" -eq 0 ] - - wait_for_container 15 1 test_busybox2 - testcontainer test_busybox2 running - - # delete both test_busybox1 and test_busybox2 container - runc delete --force test_busybox1 test_busybox2 - - runc state test_busybox1 - [ "$status" -ne 0 ] - - runc state test_busybox2 - [ "$status" -ne 0 ] -} diff --git a/tests/integration/pause.bats b/tests/integration/pause.bats index e657d0a1edf..2f46a6cae07 100644 --- a/tests/integration/pause.bats +++ b/tests/integration/pause.bats @@ -33,80 +33,34 @@ function teardown() { testcontainer test_busybox running } -@test "runc pause and resume with multi-container" { - # run test_busybox1 detached - runc run -d --console-socket $CONSOLE_SOCKET test_busybox1 - [ "$status" -eq 0 ] - - wait_for_container 15 1 test_busybox1 - - # run test_busybox2 detached - runc run -d --console-socket $CONSOLE_SOCKET test_busybox2 - [ "$status" -eq 0 ] - - wait_for_container 15 1 test_busybox2 - - # pause test_busybox1 and test_busybox2 - runc pause test_busybox1 test_busybox2 - [ "$status" -eq 0 ] - - # test state of test_busybox1 and test_busybox2 is paused - testcontainer test_busybox1 paused - testcontainer test_busybox2 paused - - # resume test_busybox1 and test_busybox2 - runc resume test_busybox1 test_busybox2 - [ "$status" -eq 0 ] - - # test state of two containers is back to running - testcontainer test_busybox1 running - testcontainer test_busybox2 running - - # delete test_busybox1 and test_busybox2 - runc delete --force test_busybox1 test_busybox2 - - runc state test_busybox1 - [ "$status" -ne 0 ] - - runc state test_busybox2 - [ "$status" -ne 0 ] -} - @test "runc pause and resume with nonexist container" { - # run test_busybox1 detached - runc run -d --console-socket $CONSOLE_SOCKET test_busybox1 + # run test_busybox detached + runc run -d --console-socket $CONSOLE_SOCKET test_busybox [ "$status" -eq 0 ] - wait_for_container 15 1 test_busybox1 + wait_for_container 15 1 test_busybox - # run test_busybox2 detached - runc run -d --console-socket $CONSOLE_SOCKET test_busybox2 + # pause test_busybox and nonexistent container + runc pause test_busybox [ "$status" -eq 0 ] - - wait_for_container 15 1 test_busybox2 - - # pause test_busybox1, test_busybox2 and nonexistent container - runc pause test_busybox1 test_busybox2 nonexistent + runc pause nonexistent [ "$status" -ne 0 ] - # test state of test_busybox1 and test_busybox2 is paused - testcontainer test_busybox1 paused - testcontainer test_busybox2 paused + # test state of test_busybox is paused + testcontainer test_busybox paused - # resume test_busybox1, test_busybox2 and nonexistent container - runc resume test_busybox1 test_busybox2 nonexistent + # resume test_busybox and nonexistent container + runc resume test_busybox + [ "$status" -eq 0 ] + runc resume nonexistent [ "$status" -ne 0 ] - # test state of two containers is back to running - testcontainer test_busybox1 running - testcontainer test_busybox2 running - - # delete test_busybox1 and test_busybox2 - runc delete --force test_busybox1 test_busybox2 + # test state of test_busybox is back to running + testcontainer test_busybox running - runc state test_busybox1 - [ "$status" -ne 0 ] + # delete test_busybox + runc delete --force test_busybox - runc state test_busybox2 + runc state test_busybox [ "$status" -ne 0 ] } diff --git a/tests/integration/start.bats b/tests/integration/start.bats index cd33dee557e..1f0ea8e1c3b 100644 --- a/tests/integration/start.bats +++ b/tests/integration/start.bats @@ -12,30 +12,20 @@ function teardown() { } @test "runc start" { - runc create --console-socket $CONSOLE_SOCKET test_busybox1 + runc create --console-socket $CONSOLE_SOCKET test_busybox [ "$status" -eq 0 ] - testcontainer test_busybox1 created + testcontainer test_busybox created - runc create --console-socket $CONSOLE_SOCKET test_busybox2 + # start container test_busybox + runc start test_busybox [ "$status" -eq 0 ] - testcontainer test_busybox2 created + testcontainer test_busybox running + # delete test_busybox + runc delete --force test_busybox - # start container test_busybox1 and test_busybox2 - runc start test_busybox1 test_busybox2 - [ "$status" -eq 0 ] - - testcontainer test_busybox1 running - testcontainer test_busybox2 running - - # delete test_busybox1 and test_busybox2 - runc delete --force test_busybox1 test_busybox2 - - runc state test_busybox1 - [ "$status" -ne 0 ] - - runc state test_busybox2 + runc state test_busybox [ "$status" -ne 0 ] }