diff --git a/cli/command/container/opts_test.go b/cli/command/container/opts_test.go index 4b8b8f07cc..d8bb7043d4 100644 --- a/cli/command/container/opts_test.go +++ b/cli/command/container/opts_test.go @@ -700,34 +700,75 @@ func TestRunFlagsParseShmSize(t *testing.T) { } func TestParseRestartPolicy(t *testing.T) { - invalids := map[string]string{ - "always:2:3": "invalid restart policy format: maximum retry count must be an integer", - "on-failure:invalid": "invalid restart policy format: maximum retry count must be an integer", - } - valids := map[string]container.RestartPolicy{ - "": {}, - "always": { - Name: "always", - MaximumRetryCount: 0, + tests := []struct { + input string + expected container.RestartPolicy + expectedErr string + }{ + { + input: "", }, - "on-failure:1": { - Name: "on-failure", - MaximumRetryCount: 1, + { + input: "no", + expected: container.RestartPolicy{ + Name: "no", + }, + }, + { + input: ":1", + expectedErr: "invalid restart policy format: no policy provided before colon", + }, + { + input: "always", + expected: container.RestartPolicy{ + Name: "always", + }, + }, + { + input: "always:1", + expected: container.RestartPolicy{ + Name: "always", + MaximumRetryCount: 1, + }, + }, + { + input: "always:2:3", + expectedErr: "invalid restart policy format: maximum retry count must be an integer", + }, + { + input: "on-failure:1", + expected: container.RestartPolicy{ + Name: "on-failure", + MaximumRetryCount: 1, + }, + }, + { + input: "on-failure:invalid", + expectedErr: "invalid restart policy format: maximum retry count must be an integer", + }, + { + input: "unless-stopped", + expected: container.RestartPolicy{ + Name: "unless-stopped", + }, + }, + { + input: "unless-stopped:invalid", + expectedErr: "invalid restart policy format: maximum retry count must be an integer", }, } - for restart, expectedError := range invalids { - if _, _, _, err := parseRun([]string{fmt.Sprintf("--restart=%s", restart), "img", "cmd"}); err == nil || err.Error() != expectedError { - t.Fatalf("Expected an error with message '%v' for %v, got %v", expectedError, restart, err) - } - } - for restart, expected := range valids { - _, hostconfig, _, err := parseRun([]string{fmt.Sprintf("--restart=%v", restart), "img", "cmd"}) - if err != nil { - t.Fatal(err) - } - if hostconfig.RestartPolicy != expected { - t.Fatalf("Expected %v, got %v", expected, hostconfig.RestartPolicy) - } + for _, tc := range tests { + tc := tc + t.Run(tc.input, func(t *testing.T) { + _, hostConfig, _, err := parseRun([]string{"--restart=" + tc.input, "img", "cmd"}) + if tc.expectedErr != "" { + assert.Check(t, is.Error(err, tc.expectedErr)) + assert.Check(t, is.Nil(hostConfig)) + } else { + assert.NilError(t, err) + assert.Check(t, is.DeepEqual(hostConfig.RestartPolicy, tc.expected)) + } + }) } } diff --git a/opts/parse.go b/opts/parse.go index 017577e4bf..8b8012d300 100644 --- a/opts/parse.go +++ b/opts/parse.go @@ -71,17 +71,22 @@ func ConvertKVStringsToMapWithNil(values []string) map[string]*string { // ParseRestartPolicy returns the parsed policy or an error indicating what is incorrect func ParseRestartPolicy(policy string) (container.RestartPolicy, error) { - p := container.RestartPolicy{} - if policy == "" { - return p, nil + // for backward-compatibility, we don't set the default ("no") + // policy here, because older versions of the engine may not + // support it. + return container.RestartPolicy{}, nil } - k, v, _ := strings.Cut(policy, ":") + p := container.RestartPolicy{} + k, v, ok := strings.Cut(policy, ":") + if ok && k == "" { + return container.RestartPolicy{}, fmt.Errorf("invalid restart policy format: no policy provided before colon") + } if v != "" { count, err := strconv.Atoi(v) if err != nil { - return p, fmt.Errorf("invalid restart policy format: maximum retry count must be an integer") + return container.RestartPolicy{}, fmt.Errorf("invalid restart policy format: maximum retry count must be an integer") } p.MaximumRetryCount = count }