From 384ceb07e971f7dcd07a74ad92dd286cefddaf4b Mon Sep 17 00:00:00 2001 From: Boaz Shuster Date: Wed, 30 Aug 2017 13:47:53 +0300 Subject: [PATCH 1/3] When nothing found in stack exit with exit code 1 To keep on a consistent behaviour such as in docker-service-ps if docker-stack-ps didn't find a given stack, the command line should exit with exit code 1. Signed-off-by: Boaz Shuster Upstream-commit: 79f9af24751f05666f1fb2611aac31fa8884b8cb Component: cli --- components/cli/cli/command/stack/ps.go | 3 +-- components/cli/cli/command/stack/ps_test.go | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/components/cli/cli/command/stack/ps.go b/components/cli/cli/command/stack/ps.go index 25bd1eee10..d586b1e1e4 100644 --- a/components/cli/cli/command/stack/ps.go +++ b/components/cli/cli/command/stack/ps.go @@ -57,8 +57,7 @@ func runPS(dockerCli command.Cli, options psOptions) error { } if len(tasks) == 0 { - fmt.Fprintf(dockerCli.Err(), "Nothing found in stack: %s\n", namespace) - return nil + return fmt.Errorf("nothing found in stack: %s", namespace) } format := options.format diff --git a/components/cli/cli/command/stack/ps_test.go b/components/cli/cli/command/stack/ps_test.go index 282e5e01da..3bb22609b5 100644 --- a/components/cli/cli/command/stack/ps_test.go +++ b/components/cli/cli/command/stack/ps_test.go @@ -60,9 +60,9 @@ func TestStackPsEmptyStack(t *testing.T) { cmd := newPsCommand(fakeCli) cmd.SetArgs([]string{"foo"}) - assert.NoError(t, cmd.Execute()) + assert.Error(t, cmd.Execute()) + assert.EqualError(t, cmd.Execute(), "nothing found in stack: foo") assert.Equal(t, "", fakeCli.OutBuffer().String()) - assert.Equal(t, "Nothing found in stack: foo\n", fakeCli.ErrBuffer().String()) } func TestStackPsWithQuietOption(t *testing.T) { From fdee4e99e99a78f183cd89ca5917650d0b766976 Mon Sep 17 00:00:00 2001 From: Eli Uriegas Date: Wed, 30 Aug 2017 15:06:22 -0700 Subject: [PATCH 2/3] Re-adds test target to the Makefile The test target existed before, this is to provide a legacy interface to allow easy testing for downstream Docker CE. Without this we would need separate Makefiles/Jenkinsfiles for releases past 17.07. Later on this target could also be used to test both unit tests and integration tests at the same time. Signed-off-by: Eli Uriegas Upstream-commit: d53c8de06b93ea6775cffc87087f47e8dd1552b9 Component: cli --- components/cli/Makefile | 3 +++ components/cli/docker.Makefile | 3 +++ 2 files changed, 6 insertions(+) diff --git a/components/cli/Makefile b/components/cli/Makefile index 17ba8e5b2d..f6e73e0671 100644 --- a/components/cli/Makefile +++ b/components/cli/Makefile @@ -14,6 +14,9 @@ clean: ## remove build artifacts test-unit: ## run unit test ./scripts/test/unit $(shell go list ./... | grep -vE '/vendor/|/e2e/') +.PHONY: test +test: test-unit ## run tests + .PHONY: test-coverage test-coverage: ## run test coverage ./scripts/test/unit-with-coverage $(shell go list ./... | grep -vE '/vendor/|/e2e/') diff --git a/components/cli/docker.Makefile b/components/cli/docker.Makefile index 097ad49399..79f7b3c42c 100644 --- a/components/cli/docker.Makefile +++ b/components/cli/docker.Makefile @@ -46,6 +46,9 @@ clean: build_docker_image test-unit: build_docker_image docker run --rm $(ENVVARS) $(MOUNTS) $(DEV_DOCKER_IMAGE_NAME) make test-unit +.PHONY: test +test: test-unit test-e2e + # build the CLI for multiple architectures using a container .PHONY: cross cross: build_cross_image From 264557313dbc6c21578782f8fb6bf0d4f7e358c7 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 31 Aug 2017 17:07:16 -0400 Subject: [PATCH 3/3] Fix crash in containe run after pulling an image. Signed-off-by: Daniel Nephin Upstream-commit: a0d8d80250af0fc6673f8428920faa45c2ce50cf Component: cli --- .../cli/cli/command/container/client_test.go | 40 ++++++++++++- .../cli/cli/command/container/create.go | 5 +- .../cli/cli/command/container/create_test.go | 60 ++++++++++++++++++- components/cli/cli/command/trust.go | 4 ++ 4 files changed, 102 insertions(+), 7 deletions(-) diff --git a/components/cli/cli/command/container/client_test.go b/components/cli/cli/command/container/client_test.go index 32f9a28fbf..875fee7049 100644 --- a/components/cli/cli/command/container/client_test.go +++ b/components/cli/cli/command/container/client_test.go @@ -1,16 +1,23 @@ package container import ( + "io" + "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/network" "github.com/docker/docker/client" "golang.org/x/net/context" ) type fakeClient struct { client.Client - inspectFunc func(string) (types.ContainerJSON, error) - execInspectFunc func(execID string) (types.ContainerExecInspect, error) - execCreateFunc func(container string, config types.ExecConfig) (types.IDResponse, error) + inspectFunc func(string) (types.ContainerJSON, error) + execInspectFunc func(execID string) (types.ContainerExecInspect, error) + execCreateFunc func(container string, config types.ExecConfig) (types.IDResponse, error) + createContainerFunc func(config *container.Config, hostConfig *container.HostConfig, networkingConfig *network.NetworkingConfig, containerName string) (container.ContainerCreateCreatedBody, error) + imageCreateFunc func(parentReference string, options types.ImageCreateOptions) (io.ReadCloser, error) + infoFunc func() (types.Info, error) } func (f *fakeClient) ContainerInspect(_ context.Context, containerID string) (types.ContainerJSON, error) { @@ -37,3 +44,30 @@ func (f *fakeClient) ContainerExecInspect(_ context.Context, execID string) (typ func (f *fakeClient) ContainerExecStart(ctx context.Context, execID string, config types.ExecStartCheck) error { return nil } + +func (f *fakeClient) ContainerCreate( + _ context.Context, + config *container.Config, + hostConfig *container.HostConfig, + networkingConfig *network.NetworkingConfig, + containerName string, +) (container.ContainerCreateCreatedBody, error) { + if f.createContainerFunc != nil { + return f.createContainerFunc(config, hostConfig, networkingConfig, containerName) + } + return container.ContainerCreateCreatedBody{}, nil +} + +func (f *fakeClient) ImageCreate(ctx context.Context, parentReference string, options types.ImageCreateOptions) (io.ReadCloser, error) { + if f.imageCreateFunc != nil { + return f.imageCreateFunc(parentReference, options) + } + return nil, nil +} + +func (f *fakeClient) Info(_ context.Context) (types.Info, error) { + if f.infoFunc != nil { + return f.infoFunc() + } + return types.Info{}, nil +} diff --git a/components/cli/cli/command/container/create.go b/components/cli/cli/command/container/create.go index 97bc1863f0..e87e3fb28a 100644 --- a/components/cli/cli/command/container/create.go +++ b/components/cli/cli/command/container/create.go @@ -198,7 +198,7 @@ func createContainer(ctx context.Context, dockerCli command.Cli, containerConfig fmt.Fprintf(stderr, "Unable to find image '%s' locally\n", reference.FamiliarString(namedRef)) // we don't want to write to stdout anything apart from container.ID - if err = pullImage(ctx, dockerCli, config.Image, stderr); err != nil { + if err := pullImage(ctx, dockerCli, config.Image, stderr); err != nil { return nil, err } if taggedRef, ok := namedRef.(reference.NamedTagged); ok && trustedRef != nil { @@ -212,8 +212,9 @@ func createContainer(ctx context.Context, dockerCli command.Cli, containerConfig if retryErr != nil { return nil, retryErr } + } else { + return nil, err } - return nil, err } for _, warning := range response.Warnings { diff --git a/components/cli/cli/command/container/create_test.go b/components/cli/cli/command/container/create_test.go index 467cdb3a6e..21354e093a 100644 --- a/components/cli/cli/command/container/create_test.go +++ b/components/cli/cli/command/container/create_test.go @@ -1,13 +1,20 @@ package container import ( + "context" + "io" + "io/ioutil" "os" + "strings" "testing" - "io/ioutil" - + "github.com/docker/cli/internal/test" "github.com/docker/cli/internal/test/testutil" + "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/network" "github.com/gotestyourself/gotestyourself/fs" + "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -62,3 +69,52 @@ func TestCIDFileCloseWithWrite(t *testing.T) { _, err = os.Stat(path) require.NoError(t, err) } + +func TestCreateContainerPullsImageIfMissing(t *testing.T) { + imageName := "does-not-exist-locally" + responseCounter := 0 + containerID := "abcdef" + + client := &fakeClient{ + createContainerFunc: func( + config *container.Config, + hostConfig *container.HostConfig, + networkingConfig *network.NetworkingConfig, + containerName string, + ) (container.ContainerCreateCreatedBody, error) { + defer func() { responseCounter++ }() + switch responseCounter { + case 0: + return container.ContainerCreateCreatedBody{}, fakeNotFound{} + case 1: + return container.ContainerCreateCreatedBody{ID: containerID}, nil + default: + return container.ContainerCreateCreatedBody{}, errors.New("unexpected") + } + }, + imageCreateFunc: func(parentReference string, options types.ImageCreateOptions) (io.ReadCloser, error) { + return ioutil.NopCloser(strings.NewReader("")), nil + }, + infoFunc: func() (types.Info, error) { + return types.Info{IndexServerAddress: "http://indexserver"}, nil + }, + } + cli := test.NewFakeCli(client) + config := &containerConfig{ + Config: &container.Config{ + Image: imageName, + }, + HostConfig: &container.HostConfig{}, + } + body, err := createContainer(context.Background(), cli, config, "name") + require.NoError(t, err) + expected := container.ContainerCreateCreatedBody{ID: containerID} + assert.Equal(t, expected, *body) + stderr := cli.ErrBuffer().String() + assert.Contains(t, stderr, "Unable to find image 'does-not-exist-locally:latest' locally") +} + +type fakeNotFound struct{} + +func (f fakeNotFound) NotFound() bool { return true } +func (f fakeNotFound) Error() string { return "error fake not found" } diff --git a/components/cli/cli/command/trust.go b/components/cli/cli/command/trust.go index c0742bc5b2..51f760ac5b 100644 --- a/components/cli/cli/command/trust.go +++ b/components/cli/cli/command/trust.go @@ -12,6 +12,10 @@ var ( untrusted bool ) +func init() { + untrusted = !getDefaultTrustState() +} + // AddTrustVerificationFlags adds content trust flags to the provided flagset func AddTrustVerificationFlags(fs *pflag.FlagSet) { trusted := getDefaultTrustState()