浏览代码

Validate run restart option value. Default value is “none”, instead of “no”, this is more in line with compose values, and changes only the default so should not have too much impact on legacy usage.

Guillaume Tardif 5 年之前
父节点
当前提交
f442eafe0b

+ 1 - 6
aci/convert/container.go

@@ -24,11 +24,6 @@ func ContainerToComposeProject(r containers.ContainerConfig) (types.Project, err
 		return types.Project{}, err
 	}
 
-	composeRestartPolicyCondition := r.RestartPolicyCondition
-	if composeRestartPolicyCondition == "no" {
-		composeRestartPolicyCondition = "none"
-	}
-
 	project := types.Project{
 		Name: r.ID,
 		Services: []types.ServiceConfig{
@@ -47,7 +42,7 @@ func ContainerToComposeProject(r containers.ContainerConfig) (types.Project, err
 						},
 					},
 					RestartPolicy: &types.RestartPolicy{
-						Condition: composeRestartPolicyCondition,
+						Condition: r.RestartPolicyCondition,
 					},
 				},
 			},

+ 1 - 1
aci/convert/container_test.go

@@ -51,7 +51,7 @@ func (suite *ContainerConvertTestSuite) TestConvertContainerEnvironment() {
 func (suite *ContainerConvertTestSuite) TestConvertRestartPolicy() {
 	container := containers.ContainerConfig{
 		ID:                     "container1",
-		RestartPolicyCondition: "no",
+		RestartPolicyCondition: "none",
 	}
 	project, err := ContainerToComposeProject(container)
 	Expect(err).To(BeNil())

+ 39 - 36
aci/convert/convert.go

@@ -71,15 +71,6 @@ func ToContainerGroup(aciContext store.AciContext, p types.Project) (containerin
 		return containerinstance.ContainerGroup{}, err
 	}
 
-	var restartPolicyCondition containerinstance.ContainerGroupRestartPolicy
-	if len(p.Services) == 1 &&
-		p.Services[0].Deploy != nil &&
-		p.Services[0].Deploy.RestartPolicy != nil {
-		restartPolicyCondition = toAciRestartPolicy(p.Services[0].Deploy.RestartPolicy.Condition)
-	} else {
-		restartPolicyCondition = containerinstance.Always
-	}
-
 	var containers []containerinstance.Container
 	groupDefinition := containerinstance.ContainerGroup{
 		Name:     &containerGroupName,
@@ -89,7 +80,7 @@ func ToContainerGroup(aciContext store.AciContext, p types.Project) (containerin
 			Containers:               &containers,
 			Volumes:                  volumes,
 			ImageRegistryCredentials: &registryCreds,
-			RestartPolicy:            restartPolicyCondition,
+			RestartPolicy:            project.getRestartPolicy(),
 		},
 	}
 
@@ -135,32 +126,6 @@ func ToContainerGroup(aciContext store.AciContext, p types.Project) (containerin
 	return groupDefinition, nil
 }
 
-func toAciRestartPolicy(restartPolicy string) containerinstance.ContainerGroupRestartPolicy {
-	switch restartPolicy {
-	case containers.RestartPolicyNone:
-		return containerinstance.Never
-	case containers.RestartPolicyAny:
-		return containerinstance.Always
-	case containers.RestartPolicyOnFailure:
-		return containerinstance.OnFailure
-	default:
-		return containerinstance.Always
-	}
-}
-
-func toContainerRestartPolicy(aciRestartPolicy containerinstance.ContainerGroupRestartPolicy) string {
-	switch aciRestartPolicy {
-	case containerinstance.Never:
-		return containers.RestartPolicyNone
-	case containerinstance.Always:
-		return containers.RestartPolicyAny
-	case containerinstance.OnFailure:
-		return containers.RestartPolicyOnFailure
-	default:
-		return containers.RestartPolicyAny
-	}
-}
-
 func getDNSSidecar(containers []containerinstance.Container) containerinstance.Container {
 	var commands []string
 	for _, container := range containers {
@@ -251,6 +216,44 @@ func (p projectAciHelper) getAciFileVolumes() (map[string]bool, []containerinsta
 	return azureFileVolumesMap, azureFileVolumesSlice, nil
 }
 
+func (p projectAciHelper) getRestartPolicy() containerinstance.ContainerGroupRestartPolicy {
+	var restartPolicyCondition containerinstance.ContainerGroupRestartPolicy
+	if len(p.Services) == 1 &&
+		p.Services[0].Deploy != nil &&
+		p.Services[0].Deploy.RestartPolicy != nil {
+		restartPolicyCondition = toAciRestartPolicy(p.Services[0].Deploy.RestartPolicy.Condition)
+	} else {
+		restartPolicyCondition = containerinstance.Always
+	}
+	return restartPolicyCondition
+}
+
+func toAciRestartPolicy(restartPolicy string) containerinstance.ContainerGroupRestartPolicy {
+	switch restartPolicy {
+	case containers.RestartPolicyNone:
+		return containerinstance.Never
+	case containers.RestartPolicyAny:
+		return containerinstance.Always
+	case containers.RestartPolicyOnFailure:
+		return containerinstance.OnFailure
+	default:
+		return containerinstance.Always
+	}
+}
+
+func toContainerRestartPolicy(aciRestartPolicy containerinstance.ContainerGroupRestartPolicy) string {
+	switch aciRestartPolicy {
+	case containerinstance.Never:
+		return containers.RestartPolicyNone
+	case containerinstance.Always:
+		return containers.RestartPolicyAny
+	case containerinstance.OnFailure:
+		return containers.RestartPolicyOnFailure
+	default:
+		return containers.RestartPolicyAny
+	}
+}
+
 type serviceConfigAciHelper types.ServiceConfig
 
 func (s serviceConfigAciHelper) getAciFileVolumeMounts(volumesCache map[string]bool) ([]containerinstance.VolumeMount, error) {

+ 1 - 1
cli/cmd/run/run.go

@@ -52,7 +52,7 @@ func Command() *cobra.Command {
 	cmd.Flags().Float64Var(&opts.Cpus, "cpus", 1., "Number of CPUs")
 	cmd.Flags().VarP(&opts.Memory, "memory", "m", "Memory limit")
 	cmd.Flags().StringArrayVarP(&opts.Environment, "env", "e", []string{}, "Set environment variables")
-	cmd.Flags().StringVarP(&opts.RestartPolicyCondition, "restart", "", "no", "Restart policy to apply when a container exits")
+	cmd.Flags().StringVarP(&opts.RestartPolicyCondition, "restart", "", containers.RestartPolicyNone, "Restart policy to apply when a container exits")
 
 	return cmd
 }

+ 1 - 1
cli/cmd/run/testdata/run-help.golden

@@ -11,5 +11,5 @@ Flags:
   -m, --memory bytes          Memory limit
       --name string           Assign a name to the container
   -p, --publish stringArray   Publish a container's port(s). [HOST_PORT:]CONTAINER_PORT
-      --restart string        Restart policy to apply when a container exits (default "no")
+      --restart string        Restart policy to apply when a container exits (default "none")
   -v, --volume stringArray    Volume. Ex: user:key@my_share:/absolute/path/to/target

+ 1 - 1
cli/cmd/testdata/inspect-out-id.golden

@@ -12,5 +12,5 @@
     "Labels": null,
     "Ports": null,
     "Platform": "Linux",
-    "RestartPolicyCondition": ""
+    "RestartPolicyCondition": "none"
 }

+ 19 - 1
cli/options/run/opts.go

@@ -21,6 +21,8 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/docker/api/utils"
+
 	"github.com/docker/docker/pkg/namesgenerator"
 	"github.com/docker/go-connections/nat"
 
@@ -57,6 +59,11 @@ func (r *Opts) ToContainerConfig(image string) (containers.ContainerConfig, erro
 		return containers.ContainerConfig{}, err
 	}
 
+	restartPolicy, err := toRestartPolicy(r.RestartPolicyCondition)
+	if err != nil {
+		return containers.ContainerConfig{}, err
+	}
+
 	return containers.ContainerConfig{
 		ID:                     r.Name,
 		Image:                  image,
@@ -66,10 +73,21 @@ func (r *Opts) ToContainerConfig(image string) (containers.ContainerConfig, erro
 		MemLimit:               r.Memory,
 		CPULimit:               r.Cpus,
 		Environment:            r.Environment,
-		RestartPolicyCondition: r.RestartPolicyCondition,
+		RestartPolicyCondition: restartPolicy,
 	}, nil
 }
 
+func toRestartPolicy(value string) (string, error) {
+	if value == "" {
+		return containers.RestartPolicyNone, nil
+	}
+	if utils.StringContains(containers.RestartPolicyList, value) {
+		return value, nil
+	}
+
+	return "", fmt.Errorf("invalid restart value, must be one of %s", strings.Join(containers.RestartPolicyList, ", "))
+}
+
 func (r *Opts) toPorts() ([]containers.Port, error) {
 	_, bindings, err := nat.ParsePortSpecs(r.Publish)
 	if err != nil {

+ 43 - 0
cli/options/run/opts_test.go

@@ -173,3 +173,46 @@ func TestLabels(t *testing.T) {
 		assert.DeepEqual(t, result, testCase.expected)
 	}
 }
+
+func TestValidateRestartPolicy(t *testing.T) {
+	testCases := []struct {
+		in            string
+		expected      string
+		expectedError error
+	}{
+		{
+			in:            "none",
+			expected:      "none",
+			expectedError: nil,
+		},
+		{
+			in:            "any",
+			expected:      "any",
+			expectedError: nil,
+		},
+		{
+			in:            "on-failure",
+			expected:      "on-failure",
+			expectedError: nil,
+		},
+		{
+			in:            "",
+			expected:      "none",
+			expectedError: nil,
+		},
+		{
+			in:            "toto",
+			expected:      "",
+			expectedError: errors.New("invalid restart value, must be one of none, any, on-failure"),
+		},
+	}
+	for _, testCase := range testCases {
+		result, err := toRestartPolicy(testCase.in)
+		if testCase.expectedError == nil {
+			assert.NilError(t, err)
+		} else {
+			assert.Error(t, err, testCase.expectedError.Error())
+		}
+		assert.Equal(t, testCase.expected, result)
+	}
+}

+ 12 - 0
containers/api.go

@@ -23,6 +23,18 @@ import (
 	"github.com/docker/api/formatter"
 )
 
+const (
+	// RestartPolicyAny Always restarts
+	RestartPolicyAny = "any"
+	// RestartPolicyNone Never restarts
+	RestartPolicyNone = "none"
+	// RestartPolicyOnFailure Restarts only on failure
+	RestartPolicyOnFailure = "on-failure"
+)
+
+// RestartPolicyList all available restart policy values
+var RestartPolicyList = []string{RestartPolicyNone, RestartPolicyAny, RestartPolicyOnFailure}
+
 // Container represents a created container
 type Container struct {
 	ID                     string

+ 0 - 11
containers/types.go

@@ -1,11 +0,0 @@
-package containers
-
-const (
-	// RestartPolicyAny Always restarts
-	RestartPolicyAny = "any"
-	// RestartPolicyNone Never restarts
-	// "no" is the value for docker run, "none" is the value in compose file (and default differ
-	RestartPolicyNone = "none"
-	// RestartPolicyOnFailure Restarts only on failure
-	RestartPolicyOnFailure = "on-failure"
-)

+ 1 - 1
docs/cli/run.md

@@ -31,7 +31,7 @@ docker run [OPTIONS] _image_ [COMMAND] [ARG...]
       --cpus                           Number of CPUs to allocate, approximately
   -p, --publish list                   Publish a container's port(s) to the host
   -P, --publish-all                    Publish all exposed ports to random ports
-      --restart string                 Restart policy to apply when a container exits (default "no")
+      --restart string                 Restart policy to apply when a container exits (default "none")
       --entrypoint string              Overwrite the default ENTRYPOINT of the image
 
       --mount mount                    Attach a filesystem mount to the container

+ 4 - 3
example/backend.go

@@ -57,9 +57,10 @@ type containerService struct{}
 
 func (cs *containerService) Inspect(ctx context.Context, id string) (containers.Container, error) {
 	return containers.Container{
-		ID:       "id",
-		Image:    "nginx",
-		Platform: "Linux",
+		ID:                     "id",
+		Image:                  "nginx",
+		Platform:               "Linux",
+		RestartPolicyCondition: "none",
 	}, nil
 }
 

+ 6 - 13
metrics/metrics.go

@@ -20,6 +20,8 @@ import (
 	"strings"
 
 	flag "github.com/spf13/pflag"
+
+	"github.com/docker/api/utils"
 )
 
 var managementCommands = []string{
@@ -111,7 +113,7 @@ func getCommand(args []string, flags *flag.FlagSet) string {
 		}
 
 		for {
-			if contains(managementCommands, command) {
+			if utils.StringContains(managementCommands, command) {
 				if sub := getSubCommand(command, strippedArgs[1:]); sub != "" {
 					command += " " + sub
 					strippedArgs = strippedArgs[1:]
@@ -128,11 +130,11 @@ func getCommand(args []string, flags *flag.FlagSet) string {
 func getScanCommand(args []string) string {
 	command := args[0]
 
-	if contains(args, "--auth") {
+	if utils.StringContains(args, "--auth") {
 		return command + " auth"
 	}
 
-	if contains(args, "--version") {
+	if utils.StringContains(args, "--version") {
 		return command + " version"
 	}
 
@@ -145,7 +147,7 @@ func getSubCommand(command string, args []string) string {
 	}
 
 	if val, ok := managementSubCommands[command]; ok {
-		if contains(val, args[0]) {
+		if utils.StringContains(val, args[0]) {
 			return args[0]
 		}
 		return ""
@@ -158,15 +160,6 @@ func getSubCommand(command string, args []string) string {
 	return ""
 }
 
-func contains(array []string, needle string) bool {
-	for _, val := range array {
-		if val == needle {
-			return true
-		}
-	}
-	return false
-}
-
 func stripFlags(args []string, flags *flag.FlagSet) []string {
 	commands := []string{}
 

+ 3 - 10
progress/tty.go

@@ -25,6 +25,8 @@ import (
 	"sync"
 	"time"
 
+	"github.com/docker/api/utils"
+
 	"github.com/buger/goterm"
 	"github.com/morikuni/aec"
 )
@@ -63,7 +65,7 @@ func (w *ttyWriter) Stop() {
 func (w *ttyWriter) Event(e Event) {
 	w.mtx.Lock()
 	defer w.mtx.Unlock()
-	if !contains(w.eventIDs, e.ID) {
+	if !utils.StringContains(w.eventIDs, e.ID) {
 		w.eventIDs = append(w.eventIDs, e.ID)
 	}
 	if _, ok := w.events[e.ID]; ok {
@@ -181,12 +183,3 @@ func numDone(events map[string]Event) int {
 func align(l, r string, w int) string {
 	return fmt.Sprintf("%-[2]*[1]s %[3]s", l, w-len(r)-1, r)
 }
-
-func contains(ar []string, needle string) bool {
-	for _, v := range ar {
-		if needle == v {
-			return true
-		}
-	}
-	return false
-}

+ 2 - 1
tests/e2e/testdata/inspect-id.golden

@@ -11,5 +11,6 @@
     "PidsLimit": 0,
     "Labels": null,
     "Ports": null,
-    "Platform": "Linux"
+    "Platform": "Linux",
+    "RestartPolicyCondition": "none"
 }

+ 11 - 0
utils/stringutils.go

@@ -0,0 +1,11 @@
+package utils
+
+// StringContains check if an array contains a specific value
+func StringContains(array []string, needle string) bool {
+	for _, val := range array {
+		if val == needle {
+			return true
+		}
+	}
+	return false
+}