api: ValidateRestartPolicy: improve errors for invalid policies

Make the error message slightly clearer on "what" part is not valid,
and provide suggestions on what are acceptable values.

Before this change:

    docker create --restart=always:3 busybox
    Error response from daemon: invalid restart policy: maximum retry count cannot be used with restart policy 'always'

    docker create --restart=always:-1 busybox
    Error response from daemon: invalid restart policy: maximum retry count cannot be used with restart policy 'always'

    docker create --restart=unknown busybox
    Error response from daemon: invalid restart policy 'unknown'

After this change:

    docker create --restart=always:3 busybox
    Error response from daemon: invalid restart policy: maximum retry count can only be used with 'on-failure'

    docker create --restart=always:-1 busybox
    Error response from daemon: invalid restart policy: maximum retry count can only be used with 'on-failure' and cannot be negative

    docker create --restart=unknown busybox
    Error response from daemon: invalid restart policy: unknown policy 'unknown'; use one of 'no', 'always', 'on-failure', or 'unless-stopped'

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

api/types/container/hostconfig.go

@@ -320,7 +320,11 @@ func ValidateRestartPolicy(policy RestartPolicy) error {
 	switch policy.Name {
 	case RestartPolicyAlways, RestartPolicyUnlessStopped, RestartPolicyDisabled:
 		if policy.MaximumRetryCount != 0 {
-			return &errInvalidParameter{fmt.Errorf("invalid restart policy: maximum retry count cannot be used with restart policy '%s'", policy.Name)}
+			msg := "invalid restart policy: maximum retry count can only be used with 'on-failure'"
+			if policy.MaximumRetryCount < 0 {
+				msg += " and cannot be negative"
+			}
+			return &errInvalidParameter{fmt.Errorf(msg)}
 		}
 		return nil
 	case RestartPolicyOnFailure:
@@ -334,7 +338,7 @@ func ValidateRestartPolicy(policy RestartPolicy) error {
 		// backward-compatibility.
 		return nil
 	default:
-		return &errInvalidParameter{fmt.Errorf("invalid restart policy: '%s'", policy.Name)}
+		return &errInvalidParameter{fmt.Errorf("invalid restart policy: unknown policy '%s'; use one of '%s', '%s', '%s', or '%s'", policy.Name, RestartPolicyDisabled, RestartPolicyAlways, RestartPolicyOnFailure, RestartPolicyUnlessStopped)}
 	}
 }
 

api/types/container/hostconfig_test.go

@@ -35,12 +35,12 @@ func TestValidateRestartPolicy(t *testing.T) {
 		{
 			name:        "always with MaxRestartCount",
 			input:       RestartPolicy{Name: RestartPolicyAlways, MaximumRetryCount: 123},
-			expectedErr: "invalid restart policy: maximum retry count cannot be used with restart policy 'always'",
+			expectedErr: "invalid restart policy: maximum retry count can only be used with 'on-failure'",
 		},
 		{
 			name:        "always with negative MaxRestartCount",
 			input:       RestartPolicy{Name: RestartPolicyAlways, MaximumRetryCount: -123},
-			expectedErr: "invalid restart policy: maximum retry count cannot be used with restart policy 'always'",
+			expectedErr: "invalid restart policy: maximum retry count can only be used with 'on-failure' and cannot be negative",
 		},
 		{
 			name:  "unless-stopped",
@@ -49,12 +49,12 @@ func TestValidateRestartPolicy(t *testing.T) {
 		{
 			name:        "unless-stopped with MaxRestartCount",
 			input:       RestartPolicy{Name: RestartPolicyUnlessStopped, MaximumRetryCount: 123},
-			expectedErr: "invalid restart policy: maximum retry count cannot be used with restart policy 'unless-stopped'",
+			expectedErr: "invalid restart policy: maximum retry count can only be used with 'on-failure'",
 		},
 		{
 			name:        "unless-stopped with negative MaxRestartCount",
 			input:       RestartPolicy{Name: RestartPolicyUnlessStopped, MaximumRetryCount: -123},
-			expectedErr: "invalid restart policy: maximum retry count cannot be used with restart policy 'unless-stopped'",
+			expectedErr: "invalid restart policy: maximum retry count can only be used with 'on-failure' and cannot be negative",
 		},
 		{
 			name:  "disabled",
@@ -63,12 +63,12 @@ func TestValidateRestartPolicy(t *testing.T) {
 		{
 			name:        "disabled with MaxRestartCount",
 			input:       RestartPolicy{Name: RestartPolicyDisabled, MaximumRetryCount: 123},
-			expectedErr: "invalid restart policy: maximum retry count cannot be used with restart policy 'no'",
+			expectedErr: "invalid restart policy: maximum retry count can only be used with 'on-failure'",
 		},
 		{
 			name:        "disabled with negative MaxRestartCount",
 			input:       RestartPolicy{Name: RestartPolicyDisabled, MaximumRetryCount: -123},
-			expectedErr: "invalid restart policy: maximum retry count cannot be used with restart policy 'no'",
+			expectedErr: "invalid restart policy: maximum retry count can only be used with 'on-failure' and cannot be negative",
 		},
 		{
 			name:  "on-failure",
@@ -85,8 +85,8 @@ func TestValidateRestartPolicy(t *testing.T) {
 		},
 		{
 			name:        "unknown policy",
-			input:       RestartPolicy{Name: "I do not exist"},
-			expectedErr: "invalid restart policy: 'I do not exist'",
+			input:       RestartPolicy{Name: "unknown"},
+			expectedErr: "invalid restart policy: unknown policy 'unknown'; use one of 'no', 'always', 'on-failure', or 'unless-stopped'",
 		},
 	}
 

integration-cli/docker_api_containers_test.go

@@ -758,7 +758,7 @@ func (s *DockerAPISuite) TestContainerAPIRestartPolicyRetryMismatch(c *testing.T
 
 	b, err := request.ReadBody(body)
 	assert.NilError(c, err)
-	assert.Assert(c, strings.Contains(string(b[:]), "maximum retry count cannot be used with restart policy"))
+	assert.Assert(c, strings.Contains(string(b[:]), "invalid restart policy: maximum retry count can only be used with 'on-failure'"))
 }
 
 func (s *DockerAPISuite) TestContainerAPIRestartPolicyNegativeRetryCount(c *testing.T) {