Browse code

Merge pull request #30113 from thaJeztah/fix-autoremove-on-older-api

Don't use AutoRemove on older daemons
(cherry picked from commit e74623d283111dc6492b1a97a8299a9377794b99)

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Brian Goff authored on 2017/01/16 22:40:00
Showing 6 changed files
... ...
@@ -369,6 +369,11 @@ func (s *containerRouter) postContainersCreate(ctx context.Context, w http.Respo
369 369
 	version := httputils.VersionFromContext(ctx)
370 370
 	adjustCPUShares := versions.LessThan(version, "1.19")
371 371
 
372
+	// When using API 1.24 and under, the client is responsible for removing the container
373
+	if hostConfig != nil && versions.LessThan(version, "1.25") {
374
+		hostConfig.AutoRemove = false
375
+	}
376
+
372 377
 	ccr, err := s.backend.ContainerCreate(types.ContainerCreateConfig{
373 378
 		Name:             name,
374 379
 		Config:           config,
... ...
@@ -74,9 +74,8 @@ func runRun(dockerCli *command.DockerCli, flags *pflag.FlagSet, opts *runOptions
74 74
 	cmdPath := "run"
75 75
 
76 76
 	var (
77
-		flAttach                              *opttypes.ListOpts
78
-		ErrConflictAttachDetach               = fmt.Errorf("Conflicting options: -a and -d")
79
-		ErrConflictRestartPolicyAndAutoRemove = fmt.Errorf("Conflicting options: --restart and --rm")
77
+		flAttach                *opttypes.ListOpts
78
+		ErrConflictAttachDetach = fmt.Errorf("Conflicting options: -a and -d")
80 79
 	)
81 80
 
82 81
 	config, hostConfig, networkingConfig, err := runconfigopts.Parse(flags, copts)
... ...
@@ -87,9 +86,6 @@ func runRun(dockerCli *command.DockerCli, flags *pflag.FlagSet, opts *runOptions
87 87
 		return cli.StatusError{StatusCode: 125}
88 88
 	}
89 89
 
90
-	if hostConfig.AutoRemove && !hostConfig.RestartPolicy.IsNone() {
91
-		return ErrConflictRestartPolicyAndAutoRemove
92
-	}
93 90
 	if hostConfig.OomKillDisable != nil && *hostConfig.OomKillDisable && hostConfig.Memory == 0 {
94 91
 		fmt.Fprintf(stderr, "WARNING: Disabling the OOM killer on containers without setting a '-m/--memory' limit may be dangerous.\n")
95 92
 	}
... ...
@@ -7,6 +7,7 @@ import (
7 7
 
8 8
 	"github.com/docker/docker/api/types/container"
9 9
 	"github.com/docker/docker/api/types/network"
10
+	"github.com/docker/docker/api/types/versions"
10 11
 	"golang.org/x/net/context"
11 12
 )
12 13
 
... ...
@@ -25,6 +26,11 @@ func (cli *Client) ContainerCreate(ctx context.Context, config *container.Config
25 25
 		return response, err
26 26
 	}
27 27
 
28
+	// When using API 1.24 and under, the client is responsible for removing the container
29
+	if hostConfig != nil && versions.LessThan(cli.ClientVersion(), "1.25") {
30
+		hostConfig.AutoRemove = false
31
+	}
32
+
28 33
 	query := url.Values{}
29 34
 	if containerName != "" {
30 35
 		query.Set("name", containerName)
... ...
@@ -74,3 +74,45 @@ func TestContainerCreateWithName(t *testing.T) {
74 74
 		t.Fatalf("expected `container_id`, got %s", r.ID)
75 75
 	}
76 76
 }
77
+
78
+// TestContainerCreateAutoRemove validates that a client using API 1.24 always disables AutoRemove. When using API 1.25
79
+// or up, AutoRemove should not be disabled.
80
+func TestContainerCreateAutoRemove(t *testing.T) {
81
+	autoRemoveValidator := func(expectedValue bool) func(req *http.Request) (*http.Response, error) {
82
+		return func(req *http.Request) (*http.Response, error) {
83
+			var config configWrapper
84
+
85
+			if err := json.NewDecoder(req.Body).Decode(&config); err != nil {
86
+				return nil, err
87
+			}
88
+			if config.HostConfig.AutoRemove != expectedValue {
89
+				return nil, fmt.Errorf("expected AutoRemove to be %v, got %v", expectedValue, config.HostConfig.AutoRemove)
90
+			}
91
+			b, err := json.Marshal(container.ContainerCreateCreatedBody{
92
+				ID: "container_id",
93
+			})
94
+			if err != nil {
95
+				return nil, err
96
+			}
97
+			return &http.Response{
98
+				StatusCode: http.StatusOK,
99
+				Body:       ioutil.NopCloser(bytes.NewReader(b)),
100
+			}, nil
101
+		}
102
+	}
103
+
104
+	client := &Client{
105
+		client:  newMockClient(autoRemoveValidator(false)),
106
+		version: "1.24",
107
+	}
108
+	if _, err := client.ContainerCreate(context.Background(), nil, &container.HostConfig{AutoRemove: true}, nil, ""); err != nil {
109
+		t.Fatal(err)
110
+	}
111
+	client = &Client{
112
+		client:  newMockClient(autoRemoveValidator(true)),
113
+		version: "1.25",
114
+	}
115
+	if _, err := client.ContainerCreate(context.Background(), nil, &container.HostConfig{AutoRemove: true}, nil, ""); err != nil {
116
+		t.Fatal(err)
117
+	}
118
+}
... ...
@@ -621,6 +621,10 @@ func Parse(flags *pflag.FlagSet, copts *ContainerOptions) (*container.Config, *c
621 621
 		Runtime:        copts.runtime,
622 622
 	}
623 623
 
624
+	if copts.autoRemove && !hostConfig.RestartPolicy.IsNone() {
625
+		return nil, nil, nil, fmt.Errorf("Conflicting options: --restart and --rm")
626
+	}
627
+
624 628
 	// only set this value if the user provided the flag, else it should default to nil
625 629
 	if flags.Changed("init") {
626 630
 		hostConfig.Init = &copts.init
... ...
@@ -586,6 +586,14 @@ func TestParseRestartPolicy(t *testing.T) {
586 586
 	}
587 587
 }
588 588
 
589
+func TestParseRestartPolicyAutoRemove(t *testing.T) {
590
+	expected := "Conflicting options: --restart and --rm"
591
+	_, _, _, err := parseRun([]string{"--rm", "--restart=always", "img", "cmd"})
592
+	if err == nil || err.Error() != expected {
593
+		t.Fatalf("Expected error %v, but got none", expected)
594
+	}
595
+}
596
+
589 597
 func TestParseHealth(t *testing.T) {
590 598
 	checkOk := func(args ...string) *container.HealthConfig {
591 599
 		config, _, _, err := parseRun(args)