Skip to content

Commit

Permalink
Only allow single container operation
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
hqhq committed Mar 8, 2017
1 parent 66781a7 commit e0c7b6c
Show file tree
Hide file tree
Showing 10 changed files with 87 additions and 280 deletions.
76 changes: 27 additions & 49 deletions delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -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> [container-id...]
Usage: "delete any resources held by the container often used with detached container",
ArgsUsage: `<container-id>
Where "<container-id>" is the name for the instance of the container.
Expand All @@ -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
},
}
4 changes: 2 additions & 2 deletions man/runc-delete.8.md
Original file line number Diff line number Diff line change
@@ -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> [container-id...]
runc delete [command options] <container-id>

Where "<container-id>" is the name for the instance of the container.

Expand Down
2 changes: 1 addition & 1 deletion man/runc-pause.8.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
runc pause - pause suspends all processes inside the container

# SYNOPSIS
runc pause <container-id> [container-id...]
runc pause <container-id>

Where "<container-id>" is the name for the instance of the container to be
paused.
Expand Down
2 changes: 1 addition & 1 deletion man/runc-resume.8.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
runc resume - resumes all processes that have been previously paused

# SYNOPSIS
runc resume <container-id> [container-id...]
runc resume <container-id>

Where "<container-id>" is the name for the instance of the container to be
resumed.
Expand Down
6 changes: 3 additions & 3 deletions man/runc-start.8.md
Original file line number Diff line number Diff line change
@@ -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> [container-id...]
runc start <container-id>

Where "<container-id>" 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.
55 changes: 11 additions & 44 deletions pause.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,89 +2,56 @@

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> [container-id...]
ArgsUsage: `<container-id>
Where "<container-id>" is the name for the instance of the container to be
paused. `,
Description: `The pause command suspends all processes in the instance of the container.
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
},
}

var resumeCommand = cli.Command{
Name: "resume",
Usage: "resumes all processes that have been previously paused",
ArgsUsage: `<container-id> [container-id...]
ArgsUsage: `<container-id>
Where "<container-id>" is the name for the instance of the container to be
resumed.`,
Description: `The resume command resumes all processes in the instance of the container.
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
},
}
57 changes: 17 additions & 40 deletions start.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package main

import (
"errors"
"fmt"
"os"

"github.com/opencontainers/runc/libcontainer"
"github.com/urfave/cli"
Expand All @@ -11,56 +11,33 @@ import (
var startCommand = cli.Command{
Name: "start",
Usage: "executes the user defined process in a created container",
ArgsUsage: `<container-id> [container-id...]
ArgsUsage: `<container-id>
Where "<container-id>" 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
},
}
59 changes: 0 additions & 59 deletions tests/integration/delete.bats
Original file line number Diff line number Diff line change
Expand Up @@ -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 ]
}
Loading

0 comments on commit e0c7b6c

Please sign in to comment.