diff --git a/api/server/router/container/container_routes.go b/api/server/router/container/container_routes.go index 9c9bc0f8c3..3c389245ce 100644 --- a/api/server/router/container/container_routes.go +++ b/api/server/router/container/container_routes.go @@ -369,6 +369,11 @@ func (s *containerRouter) postContainersCreate(ctx context.Context, w http.Respo version := httputils.VersionFromContext(ctx) adjustCPUShares := versions.LessThan(version, "1.19") + // When using API 1.24 and under, the client is responsible for removing the container + if hostConfig != nil && versions.LessThan(version, "1.25") { + hostConfig.AutoRemove = false + } + ccr, err := s.backend.ContainerCreate(types.ContainerCreateConfig{ Name: name, Config: config, diff --git a/cli/command/container/opts.go b/cli/command/container/opts.go index c5fc152168..55cc3c3b29 100644 --- a/cli/command/container/opts.go +++ b/cli/command/container/opts.go @@ -622,6 +622,10 @@ func parse(flags *pflag.FlagSet, copts *containerOptions) (*container.Config, *c Runtime: copts.runtime, } + if copts.autoRemove && !hostConfig.RestartPolicy.IsNone() { + return nil, nil, nil, fmt.Errorf("Conflicting options: --restart and --rm") + } + // only set this value if the user provided the flag, else it should default to nil if flags.Changed("init") { hostConfig.Init = &copts.init diff --git a/cli/command/container/opts_test.go b/cli/command/container/opts_test.go index d02a0f7bfc..ce3bb21b41 100644 --- a/cli/command/container/opts_test.go +++ b/cli/command/container/opts_test.go @@ -456,6 +456,14 @@ func TestParseRestartPolicy(t *testing.T) { } } +func TestParseRestartPolicyAutoRemove(t *testing.T) { + expected := "Conflicting options: --restart and --rm" + _, _, _, err := parseRun([]string{"--rm", "--restart=always", "img", "cmd"}) + if err == nil || err.Error() != expected { + t.Fatalf("Expected error %v, but got none", expected) + } +} + func TestParseHealth(t *testing.T) { checkOk := func(args ...string) *container.HealthConfig { config, _, _, err := parseRun(args) diff --git a/cli/command/container/run.go b/cli/command/container/run.go index 0f8da3fa4e..4d85ee77ac 100644 --- a/cli/command/container/run.go +++ b/cli/command/container/run.go @@ -73,9 +73,8 @@ func runRun(dockerCli *command.DockerCli, flags *pflag.FlagSet, opts *runOptions cmdPath := "run" var ( - flAttach *opttypes.ListOpts - ErrConflictAttachDetach = errors.New("Conflicting options: -a and -d") - ErrConflictRestartPolicyAndAutoRemove = errors.New("Conflicting options: --restart and --rm") + flAttach *opttypes.ListOpts + ErrConflictAttachDetach = errors.New("Conflicting options: -a and -d") ) config, hostConfig, networkingConfig, err := parse(flags, copts) @@ -86,9 +85,6 @@ func runRun(dockerCli *command.DockerCli, flags *pflag.FlagSet, opts *runOptions return cli.StatusError{StatusCode: 125} } - if hostConfig.AutoRemove && !hostConfig.RestartPolicy.IsNone() { - return ErrConflictRestartPolicyAndAutoRemove - } if hostConfig.OomKillDisable != nil && *hostConfig.OomKillDisable && hostConfig.Memory == 0 { fmt.Fprintln(stderr, "WARNING: Disabling the OOM killer on containers without setting a '-m/--memory' limit may be dangerous.") } @@ -209,7 +205,7 @@ func runRun(dockerCli *command.DockerCli, flags *pflag.FlagSet, opts *runOptions }) } - statusChan := waitExitOrRemoved(ctx, dockerCli, createResponse.ID, hostConfig.AutoRemove) + statusChan := waitExitOrRemoved(ctx, dockerCli, createResponse.ID, copts.autoRemove) //start the container if err := client.ContainerStart(ctx, createResponse.ID, types.ContainerStartOptions{}); err != nil { @@ -222,7 +218,7 @@ func runRun(dockerCli *command.DockerCli, flags *pflag.FlagSet, opts *runOptions } reportError(stderr, cmdPath, err.Error(), false) - if hostConfig.AutoRemove { + if copts.autoRemove { // wait container to be removed <-statusChan } diff --git a/client/container_create.go b/client/container_create.go index 9f627aafa6..6841b0b282 100644 --- a/client/container_create.go +++ b/client/container_create.go @@ -7,6 +7,7 @@ import ( "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/network" + "github.com/docker/docker/api/types/versions" "golang.org/x/net/context" ) @@ -25,6 +26,11 @@ func (cli *Client) ContainerCreate(ctx context.Context, config *container.Config return response, err } + // When using API 1.24 and under, the client is responsible for removing the container + if hostConfig != nil && versions.LessThan(cli.ClientVersion(), "1.25") { + hostConfig.AutoRemove = false + } + query := url.Values{} if containerName != "" { query.Set("name", containerName) diff --git a/client/container_create_test.go b/client/container_create_test.go index 15dbd5ea01..73474cf56f 100644 --- a/client/container_create_test.go +++ b/client/container_create_test.go @@ -74,3 +74,45 @@ func TestContainerCreateWithName(t *testing.T) { t.Fatalf("expected `container_id`, got %s", r.ID) } } + +// TestContainerCreateAutoRemove validates that a client using API 1.24 always disables AutoRemove. When using API 1.25 +// or up, AutoRemove should not be disabled. +func TestContainerCreateAutoRemove(t *testing.T) { + autoRemoveValidator := func(expectedValue bool) func(req *http.Request) (*http.Response, error) { + return func(req *http.Request) (*http.Response, error) { + var config configWrapper + + if err := json.NewDecoder(req.Body).Decode(&config); err != nil { + return nil, err + } + if config.HostConfig.AutoRemove != expectedValue { + return nil, fmt.Errorf("expected AutoRemove to be %v, got %v", expectedValue, config.HostConfig.AutoRemove) + } + b, err := json.Marshal(container.ContainerCreateCreatedBody{ + ID: "container_id", + }) + if err != nil { + return nil, err + } + return &http.Response{ + StatusCode: http.StatusOK, + Body: ioutil.NopCloser(bytes.NewReader(b)), + }, nil + } + } + + client := &Client{ + client: newMockClient(autoRemoveValidator(false)), + version: "1.24", + } + if _, err := client.ContainerCreate(context.Background(), nil, &container.HostConfig{AutoRemove: true}, nil, ""); err != nil { + t.Fatal(err) + } + client = &Client{ + client: newMockClient(autoRemoveValidator(true)), + version: "1.25", + } + if _, err := client.ContainerCreate(context.Background(), nil, &container.HostConfig{AutoRemove: true}, nil, ""); err != nil { + t.Fatal(err) + } +}