Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bugfix: make container exit with real exit code #1099

Merged
merged 1 commit into from
Apr 13, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions cli/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,3 +140,14 @@ func (c *Cli) Print(obj interface{}) {

display.Flush()
}

// ExitError defines exit error produce by cli commands.
type ExitError struct {
Code int
Status string
}

// Error inplements error interface.
func (e ExitError) Error() string {
return fmt.Sprintf("Exit Code: %d, Status: %s", e.Code, e.Status)
}
14 changes: 14 additions & 0 deletions cli/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,20 @@ func main() {
cli.AddCommand(base, &GenDocCommand{})

if err := cli.Run(); err != nil {
// deal with ExitError, which should be recognize as error, and should
// not be exit with status 0.
if exitErr, ok := err.(ExitError); ok {
if exitErr.Status != "" {
fmt.Fprintln(os.Stderr, exitErr.Status)
}
if exitErr.Code == 0 {
// when get error with ExitError, code should not be 0.
exitErr.Code = 1
}
os.Exit(exitErr.Code)
}

// not ExitError, print error to os.Stderr, exit code 1.
fmt.Fprintf(os.Stderr, "Error: %v\n", err)
os.Exit(1)
}
Expand Down
13 changes: 12 additions & 1 deletion cli/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,14 +108,14 @@ func (rc *RunCommand) runRun(args []string) error {
if err != nil {
return fmt.Errorf("failed to attach container: %v", err)
}
defer conn.Close()

go func() {
io.Copy(os.Stdout, br)
wait <- struct{}{}
}()
go func() {
io.Copy(conn, os.Stdin)
wait <- struct{}{}
}()
}

Expand All @@ -130,6 +130,17 @@ func (rc *RunCommand) runRun(args []string) error {
} else {
fmt.Fprintf(os.Stdout, "%s\n", result.ID)
}

info, err := apiClient.ContainerGet(ctx, containerName)
if err != nil {

}

code := info.State.ExitCode
if code != 0 {
return ExitError{Code: int(code)}
}

return nil
}

Expand Down
13 changes: 12 additions & 1 deletion cli/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ func (s *StartCommand) runStart(args []string) error {
if err != nil {
return fmt.Errorf("failed to attach container: %v", err)
}
defer conn.Close()

wait = make(chan struct{})
go func() {
Expand All @@ -80,7 +81,6 @@ func (s *StartCommand) runStart(args []string) error {
}()
go func() {
io.Copy(conn, os.Stdin)
close(wait)
}()
}

Expand All @@ -93,6 +93,17 @@ func (s *StartCommand) runStart(args []string) error {
if s.attach || s.stdin {
<-wait
}

info, err := apiClient.ContainerGet(ctx, container)
if err != nil {

}

code := info.State.ExitCode
if code != 0 {
return ExitError{Code: int(code)}
}

return nil
}

Expand Down
1 change: 1 addition & 0 deletions daemon/mgr/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -626,6 +626,7 @@ func (mgr *ContainerManager) createContainerdContainer(ctx context.Context, c *C
return errors.Wrapf(err, "failed to get PID of container: %s", c.ID())
}
c.meta.State.Pid = int64(pid)
c.meta.State.ExitCode = 0
} else {
c.meta.State.FinishedAt = time.Now().UTC().Format(utils.TimeLayout)
c.meta.State.Error = err.Error()
Expand Down
4 changes: 2 additions & 2 deletions test/cli_network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func (suite *PouchNetworkSuite) TestNetworkBridgeWorks(c *check.C) {
ExitCode: 1,
Err: "has active endpoints",
}
command.PouchRun("run", "--name", funcname, "--net", funcname, busyboxImage, "top").Assert(c, icmd.Success)
command.PouchRun("run", "-d", "--name", funcname, "--net", funcname, busyboxImage, "top").Assert(c, icmd.Success)

err := command.PouchRun("network", "remove", funcname).Compare(expct)
c.Assert(err, check.IsNil)
Expand Down Expand Up @@ -181,7 +181,7 @@ func (suite *PouchNetworkSuite) TestNetworkBridgeWorks(c *check.C) {
}
{
// running container is stopped, then the veth device should also been removed
command.PouchRun("run", "--name", funcname, "--net", funcname, busyboxImage, "top").Assert(c, icmd.Success)
command.PouchRun("run", "-d", "--name", funcname, "--net", funcname, busyboxImage, "top").Assert(c, icmd.Success)
command.PouchRun("stop", funcname).Assert(c, icmd.Success)

// get the ID of bridge to construct the bridge name.
Expand Down
36 changes: 26 additions & 10 deletions test/cli_run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ func (suite *PouchRunSuite) TestRunInWrongWay(c *check.C) {
func (suite *PouchRunSuite) TestRunEnableLxcfs(c *check.C) {
name := "test-run-lxcfs"

command.PouchRun("run", "--name", name, "-m", "512M", "--enableLxcfs=true",
command.PouchRun("run", "-d", "--name", name, "-m", "512M", "--enableLxcfs=true",
busyboxImage, "sleep", "10000").Assert(c, icmd.Success)

res := command.PouchRun("exec", name, "head", "-n", "5", "/proc/meminfo")
Expand Down Expand Up @@ -237,7 +237,7 @@ func (suite *PouchRunSuite) TestRunRestartPolicyNone(c *check.C) {
func (suite *PouchRunSuite) TestRunWithIPCMode(c *check.C) {
name := "test-run-with-ipc-mode"

res := command.PouchRun("run", "--name", name, "--ipc", "host", busyboxImage)
res := command.PouchRun("run", "-d", "--name", name, "--ipc", "host", busyboxImage)
res.Assert(c, icmd.Success)
DelContainerForceMultyTime(c, name)
}
Expand All @@ -247,7 +247,7 @@ func (suite *PouchRunSuite) TestRunWithIPCMode(c *check.C) {
func (suite *PouchRunSuite) TestRunWithPIDMode(c *check.C) {
name := "test-run-with-pid-mode"

res := command.PouchRun("run", "--name", name, "--pid", "host", busyboxImage)
res := command.PouchRun("run", "-d", "--name", name, "--pid", "host", busyboxImage)
res.Assert(c, icmd.Success)
DelContainerForceMultyTime(c, name)
}
Expand All @@ -256,7 +256,7 @@ func (suite *PouchRunSuite) TestRunWithPIDMode(c *check.C) {
func (suite *PouchRunSuite) TestRunWithUTSMode(c *check.C) {
name := "test-run-with-uts-mode"

res := command.PouchRun("run", "--name", name, "--uts", "host", busyboxImage)
res := command.PouchRun("run", "-d", "--name", name, "--uts", "host", busyboxImage)
res.Assert(c, icmd.Success)
DelContainerForceMultyTime(c, name)
}
Expand All @@ -266,7 +266,7 @@ func (suite *PouchRunSuite) TestRunWithSysctls(c *check.C) {
sysctl := "net.ipv4.ip_forward=1"
name := "run-sysctl"

res := command.PouchRun("run", "--name", name, "--sysctl", sysctl, busyboxImage)
res := command.PouchRun("run", "-d", "--name", name, "--sysctl", sysctl, busyboxImage)
res.Assert(c, icmd.Success)

output := command.PouchRun("exec", name, "cat", "/proc/sys/net/ipv4/ip_forward").Stdout()
Expand All @@ -281,7 +281,7 @@ func (suite *PouchRunSuite) TestRunWithUser(c *check.C) {
user := "1001"
name := "run-user"

res := command.PouchRun("run", "--name", name, "--user", user, busyboxImage)
res := command.PouchRun("run", "-d", "--name", name, "--user", user, busyboxImage)
res.Assert(c, icmd.Success)

output := command.PouchRun("exec", name, "id", "-u").Stdout()
Expand All @@ -304,7 +304,7 @@ func (suite *PouchRunSuite) TestRunWithAppArmor(c *check.C) {
appArmor := "apparmor=unconfined"
name := "run-apparmor"

res := command.PouchRun("run", "--name", name, "--security-opt", appArmor, busyboxImage)
res := command.PouchRun("run", "-d", "--name", name, "--security-opt", appArmor, busyboxImage)
res.Assert(c, icmd.Success)

// TODO: do the test more strictly with effective AppArmor profile.
Expand All @@ -317,7 +317,7 @@ func (suite *PouchRunSuite) TestRunWithSeccomp(c *check.C) {
seccomp := "seccomp=unconfined"
name := "run-seccomp"

res := command.PouchRun("run", "--name", name, "--security-opt", seccomp, busyboxImage)
res := command.PouchRun("run", "-d", "--name", name, "--security-opt", seccomp, busyboxImage)
res.Assert(c, icmd.Success)

// TODO: do the test more strictly with effective seccomp profile.
Expand Down Expand Up @@ -359,7 +359,7 @@ func (suite *PouchRunSuite) TestRunWithPrivilege(c *check.C) {
func (suite *PouchRunSuite) TestRunWithBlkioWeight(c *check.C) {
name := "test-run-with-blkio-weight"

res := command.PouchRun("run", "--name", name, "--blkio-weight", "500", busyboxImage)
res := command.PouchRun("run", "-d", "--name", name, "--blkio-weight", "500", busyboxImage)
res.Assert(c, icmd.Success)
DelContainerForceMultyTime(c, name)
}
Expand Down Expand Up @@ -817,7 +817,7 @@ func (suite *PouchRunSuite) TestRunWithDiskQuota(c *check.C) {
// TestRunWithAnnotation is to verify the valid running container with annotation, and verify SpecAnnotation filed has been in inspect output.
func (suite *PouchRunSuite) TestRunWithAnnotation(c *check.C) {
cname := "TestRunWithAnnotation"
command.PouchRun("run", "-d", "--annotation", "a=b", "--annotation", "foo=bar", "--name", cname, busyboxImage).Stdout()
command.PouchRun("run", "-d", "--annotation", "a=b", "--annotation", "foo=bar", "--name", cname, busyboxImage).Assert(c, icmd.Success)

output := command.PouchRun("inspect", cname).Stdout()
result := []types.ContainerJSON{}
Expand All @@ -835,3 +835,19 @@ func (suite *PouchRunSuite) TestRunWithAnnotation(c *check.C) {
c.Assert(util.PartialEqual(annotationStr, "a=b"), check.IsNil)
c.Assert(util.PartialEqual(annotationStr, "foo=bar"), check.IsNil)
}

// TestRunWithExitCode is to verify the valid running container with exit code != 0.
func (suite *PouchRunSuite) TestRunWithExitCode(c *check.C) {
cname := "TestRunWithExitCode"
ret := command.PouchRun("run", "--name", cname, busyboxImage, "sh", "-c", "exit 101")
// test process exit code $? == 101
ret.Assert(c, icmd.Expected{ExitCode: 101})

// test container ExitCode == 101
output := command.PouchRun("inspect", cname).Stdout()
result := []types.ContainerJSON{}
if err := json.Unmarshal([]byte(output), &result); err != nil {
c.Errorf("failed to decode inspect output: %v", err)
}
c.Assert(result[0].State.ExitCode, check.Equals, int64(101))
}
23 changes: 23 additions & 0 deletions test/cli_start_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ package main

import (
"bufio"
"encoding/json"
"os/exec"
"strings"

"github.com/alibaba/pouch/apis/types"
"github.com/alibaba/pouch/test/command"
"github.com/alibaba/pouch/test/environment"

Expand Down Expand Up @@ -244,3 +246,24 @@ func (suite *PouchStartSuite) TestStartWithAnnotation(c *check.C) {

command.PouchRun("start", name).Assert(c, icmd.Success)
}

// TestStartWithExitCode starts a container with annotation.
func (suite *PouchStartSuite) TestStartWithExitCode(c *check.C) {
name := "start-exitcode"

res := command.PouchRun("create", "--name", name, busyboxImage, "sh", "-c", "exit 101")
res.Assert(c, icmd.Success)
defer DelContainerForceMultyTime(c, name)

// test process exit code $? == 101
ret := command.PouchRun("start", "-a", name)
ret.Assert(c, icmd.Expected{ExitCode: 101})

// test container ExitCode == 101
output := command.PouchRun("inspect", name).Stdout()
result := []types.ContainerJSON{}
if err := json.Unmarshal([]byte(output), &result); err != nil {
c.Errorf("failed to decode inspect output: %v", err)
}
c.Assert(result[0].State.ExitCode, check.Equals, int64(101))
}
2 changes: 1 addition & 1 deletion test/z_cli_daemon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func (suite *PouchDaemonSuite) TestDaemonCgroupParent(c *check.C) {
}
}
{
result := command.PouchRun("--host", daemon.Listen, "run", "--name", cname, busyboxImage)
result := command.PouchRun("--host", daemon.Listen, "run", "-d", "--name", cname, busyboxImage)
if result.ExitCode != 0 {
dcfg.DumpLog()
c.Fatalf("run container failed, err:%v", result)
Expand Down