From 01143afe54f1be7308c5663a0cc110740626c62b Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 8 Nov 2017 11:04:42 -0800 Subject: [PATCH 1/3] integration: add container.Exec() Some test cases might need an ability to execute a command inside a container (in order to analyse its output and/or exit code). It is a bit complicated operation to do so using engine API. The function provided aims to hide this complexity, making exec almost as simple as 'docker exec'. NOTE that the exec is synchronous, and command's stdin is closed. Signed-off-by: Kir Kolyshkin --- integration/internal/container/exec.go | 86 ++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 integration/internal/container/exec.go diff --git a/integration/internal/container/exec.go b/integration/internal/container/exec.go new file mode 100644 index 0000000000..2af0b90738 --- /dev/null +++ b/integration/internal/container/exec.go @@ -0,0 +1,86 @@ +package container + +import ( + "bytes" + + "github.com/docker/docker/api/types" + "github.com/docker/docker/client" + "github.com/docker/docker/pkg/stdcopy" + "golang.org/x/net/context" +) + +// ExecResult represents a result returned from Exec() +type ExecResult struct { + ExitCode int + outBuffer *bytes.Buffer + errBuffer *bytes.Buffer +} + +// Stdout returns stdout output of a command run by Exec() +func (res *ExecResult) Stdout() string { + return res.outBuffer.String() +} + +// Stderr returns stderr output of a command run by Exec() +func (res *ExecResult) Stderr() string { + return res.errBuffer.String() +} + +// Combined returns combined stdout and stderr output of a command run by Exec() +func (res *ExecResult) Combined() string { + return res.outBuffer.String() + res.errBuffer.String() +} + +// Exec executes a command inside a container, returning the result +// containing stdout, stderr, and exit code. Note: +// - this is a synchronous operation; +// - cmd stdin is closed. +func Exec(ctx context.Context, cli client.APIClient, id string, cmd []string) (ExecResult, error) { + // prepare exec + execConfig := types.ExecConfig{ + AttachStdout: true, + AttachStderr: true, + Cmd: cmd, + } + cresp, err := cli.ContainerExecCreate(ctx, id, execConfig) + if err != nil { + return ExecResult{}, err + } + execID := cresp.ID + + // run it, with stdout/stderr attached + aresp, err := cli.ContainerExecAttach(ctx, execID, types.ExecStartCheck{}) + if err != nil { + return ExecResult{}, err + } + defer aresp.Close() + + // read the output + var outBuf, errBuf bytes.Buffer + outputDone := make(chan error) + + go func() { + // StdCopy demultiplexes the stream into two buffers + _, err = stdcopy.StdCopy(&outBuf, &errBuf, aresp.Reader) + outputDone <- err + }() + + select { + case err := <-outputDone: + if err != nil { + return ExecResult{}, err + } + break + + case <-ctx.Done(): + return ExecResult{}, ctx.Err() + } + + // get the exit code + iresp, err := cli.ContainerExecInspect(ctx, execID) + if err != nil { + return ExecResult{}, err + } + + return ExecResult{ExitCode: iresp.ExitCode, outBuffer: &outBuf, errBuffer: &errBuf}, nil +} From 8a7d6143fca69623e2f5d409328c97603843ccb6 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 15 Feb 2018 11:39:21 -0800 Subject: [PATCH 2/3] integration/TestUpdateCPUQUota: use exec An implementation of exec in TestUpdateCPUQUota had a few issues, including resource leaking and calling both ContainerExecAttach and ContainerExecRun. The last one makes the test flaky: update_linux_test.go:136: expected cgroup value 20000, got: Error: Exec command f923baf709525f6b38f6511126addc5d9bb88fb477eeca1c22440551090fa2bb is already running Fix by using the integration/internal/exec package. While at it, use require/assert to further improve code readability. Signed-off-by: Kir Kolyshkin --- integration/container/update_linux_test.go | 54 ++++------------------ 1 file changed, 8 insertions(+), 46 deletions(-) diff --git a/integration/container/update_linux_test.go b/integration/container/update_linux_test.go index ef9397c209..e13ac60c88 100644 --- a/integration/container/update_linux_test.go +++ b/integration/container/update_linux_test.go @@ -88,55 +88,17 @@ func TestUpdateCPUQUota(t *testing.T) { } inspect, err := client.ContainerInspect(ctx, cID) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) + assert.Equal(t, test.update, inspect.HostConfig.CPUQuota) - if inspect.HostConfig.CPUQuota != test.update { - t.Fatalf("quota not updated in the API, expected %d, got: %d", test.update, inspect.HostConfig.CPUQuota) - } + res, err := container.Exec(ctx, client, cID, + []string{"/bin/cat", "/sys/fs/cgroup/cpu/cpu.cfs_quota_us"}) + require.NoError(t, err) + require.Empty(t, res.Stderr()) + require.Equal(t, 0, res.ExitCode) - execCreate, err := client.ContainerExecCreate(ctx, cID, types.ExecConfig{ - Cmd: []string{"/bin/cat", "/sys/fs/cgroup/cpu/cpu.cfs_quota_us"}, - AttachStdout: true, - AttachStderr: true, - }) - if err != nil { - t.Fatal(err) - } - - attach, err := client.ContainerExecAttach(ctx, execCreate.ID, types.ExecStartCheck{}) - if err != nil { - t.Fatal(err) - } - - if err := client.ContainerExecStart(ctx, execCreate.ID, types.ExecStartCheck{}); err != nil { - t.Fatal(err) - } - - buf := bytes.NewBuffer(nil) - ready := make(chan error) - - go func() { - _, err := stdcopy.StdCopy(buf, buf, attach.Reader) - ready <- err - }() - - select { - case <-time.After(60 * time.Second): - t.Fatal("timeout waiting for exec to complete") - case err := <-ready: - if err != nil { - t.Fatal(err) - } - } - - actual := strings.TrimSpace(buf.String()) - if actual != strconv.Itoa(int(test.update)) { - t.Fatalf("expected cgroup value %d, got: %s", test.update, actual) - } + assert.Equal(t, strconv.FormatInt(test.update, 10), strings.TrimSpace(res.Stdout())) } - } func getContainerSysFSValue(ctx context.Context, client client.APIClient, cID string, path string) (string, error) { From 0f9da07b569f0d9cbe574db3af3951b4d5c968c0 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 15 Feb 2018 11:59:53 -0800 Subject: [PATCH 3/3] integration/TestUpdateMemory: simplify 1. Use integration/internal/exec, removing the getContainerSysFSValue(). 2. Avoid repeating magic numbers, use a variable for those. 3. Fix order of arguments to assert.Equal (first "expected", then "actual"). Signed-off-by: Kir Kolyshkin --- integration/container/update_linux_test.go | 63 +++++++--------------- 1 file changed, 19 insertions(+), 44 deletions(-) diff --git a/integration/container/update_linux_test.go b/integration/container/update_linux_test.go index e13ac60c88..bc8011c1d3 100644 --- a/integration/container/update_linux_test.go +++ b/integration/container/update_linux_test.go @@ -1,21 +1,15 @@ package container // import "github.com/docker/docker/integration/container" import ( - "bytes" "context" - "io/ioutil" "strconv" "strings" "testing" "time" - "github.com/docker/docker/api/types" containertypes "github.com/docker/docker/api/types/container" - "github.com/docker/docker/api/types/strslice" - "github.com/docker/docker/client" "github.com/docker/docker/integration/internal/container" "github.com/docker/docker/integration/internal/request" - "github.com/docker/docker/pkg/stdcopy" "github.com/gotestyourself/gotestyourself/poll" "github.com/gotestyourself/gotestyourself/skip" "github.com/stretchr/testify/assert" @@ -39,26 +33,37 @@ func TestUpdateMemory(t *testing.T) { poll.WaitOn(t, containerIsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) + const ( + setMemory int64 = 314572800 + setMemorySwap = 524288000 + ) + _, err := client.ContainerUpdate(ctx, cID, containertypes.UpdateConfig{ Resources: containertypes.Resources{ - Memory: 314572800, - MemorySwap: 524288000, + Memory: setMemory, + MemorySwap: setMemorySwap, }, }) require.NoError(t, err) inspect, err := client.ContainerInspect(ctx, cID) require.NoError(t, err) - assert.Equal(t, inspect.HostConfig.Memory, int64(314572800)) - assert.Equal(t, inspect.HostConfig.MemorySwap, int64(524288000)) + assert.Equal(t, setMemory, inspect.HostConfig.Memory) + assert.Equal(t, setMemorySwap, inspect.HostConfig.MemorySwap) - body, err := getContainerSysFSValue(ctx, client, cID, "/sys/fs/cgroup/memory/memory.limit_in_bytes") + res, err := container.Exec(ctx, client, cID, + []string{"cat", "/sys/fs/cgroup/memory/memory.limit_in_bytes"}) require.NoError(t, err) - assert.Equal(t, strings.TrimSpace(body), "314572800") + require.Empty(t, res.Stderr()) + require.Equal(t, 0, res.ExitCode) + assert.Equal(t, strconv.FormatInt(setMemory, 10), strings.TrimSpace(res.Stdout())) - body, err = getContainerSysFSValue(ctx, client, cID, "/sys/fs/cgroup/memory/memory.memsw.limit_in_bytes") + res, err = container.Exec(ctx, client, cID, + []string{"cat", "/sys/fs/cgroup/memory/memory.memsw.limit_in_bytes"}) require.NoError(t, err) - assert.Equal(t, strings.TrimSpace(body), "524288000") + require.Empty(t, res.Stderr()) + require.Equal(t, 0, res.ExitCode) + assert.Equal(t, strconv.FormatInt(setMemorySwap, 10), strings.TrimSpace(res.Stdout())) } func TestUpdateCPUQUota(t *testing.T) { @@ -100,33 +105,3 @@ func TestUpdateCPUQUota(t *testing.T) { assert.Equal(t, strconv.FormatInt(test.update, 10), strings.TrimSpace(res.Stdout())) } } - -func getContainerSysFSValue(ctx context.Context, client client.APIClient, cID string, path string) (string, error) { - var b bytes.Buffer - - ex, err := client.ContainerExecCreate(ctx, cID, - types.ExecConfig{ - AttachStdout: true, - Cmd: strslice.StrSlice([]string{"cat", path}), - }, - ) - if err != nil { - return "", err - } - - resp, err := client.ContainerExecAttach(ctx, ex.ID, - types.ExecStartCheck{ - Detach: false, - Tty: false, - }, - ) - if err != nil { - return "", err - } - - defer resp.Close() - - b.Reset() - _, err = stdcopy.StdCopy(&b, ioutil.Discard, resp.Reader) - return b.String(), err -}