diff --git a/aci/convert/container.go b/aci/convert/container.go index 29bfc966c..6929e97fd 100644 --- a/aci/convert/container.go +++ b/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, }, }, }, diff --git a/aci/convert/container_test.go b/aci/convert/container_test.go index b2f7a0472..3512f14c1 100644 --- a/aci/convert/container_test.go +++ b/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()) diff --git a/aci/convert/convert.go b/aci/convert/convert.go index 0711170b8..8b7d4e006 100644 --- a/aci/convert/convert.go +++ b/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: ®istryCreds, - 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) { diff --git a/cli/cmd/run/run.go b/cli/cmd/run/run.go index b05750489..044459639 100644 --- a/cli/cmd/run/run.go +++ b/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 } diff --git a/cli/cmd/run/testdata/run-help.golden b/cli/cmd/run/testdata/run-help.golden index b1e7f0536..645257263 100644 --- a/cli/cmd/run/testdata/run-help.golden +++ b/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 diff --git a/cli/cmd/testdata/inspect-out-id.golden b/cli/cmd/testdata/inspect-out-id.golden index 98cf5c2fe..db549f913 100644 --- a/cli/cmd/testdata/inspect-out-id.golden +++ b/cli/cmd/testdata/inspect-out-id.golden @@ -12,5 +12,5 @@ "Labels": null, "Ports": null, "Platform": "Linux", - "RestartPolicyCondition": "" + "RestartPolicyCondition": "none" } diff --git a/cli/options/run/opts.go b/cli/options/run/opts.go index 460df71f5..8e1792ce1 100644 --- a/cli/options/run/opts.go +++ b/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 { diff --git a/cli/options/run/opts_test.go b/cli/options/run/opts_test.go index 55e5d0402..f668b77fa 100644 --- a/cli/options/run/opts_test.go +++ b/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) + } +} diff --git a/containers/api.go b/containers/api.go index 2246b9b58..b2a77bf74 100644 --- a/containers/api.go +++ b/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 diff --git a/containers/types.go b/containers/types.go deleted file mode 100644 index 8a6553776..000000000 --- a/containers/types.go +++ /dev/null @@ -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" -) diff --git a/docs/cli/run.md b/docs/cli/run.md index 3be03df63..1e7b42e52 100644 --- a/docs/cli/run.md +++ b/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 diff --git a/example/backend.go b/example/backend.go index befce4f28..0db8f1e3b 100644 --- a/example/backend.go +++ b/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 } diff --git a/metrics/metrics.go b/metrics/metrics.go index e201f92f7..eae08576e 100644 --- a/metrics/metrics.go +++ b/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{} diff --git a/progress/tty.go b/progress/tty.go index caac150d3..b71a36d66 100644 --- a/progress/tty.go +++ b/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 -} diff --git a/tests/e2e/testdata/inspect-id.golden b/tests/e2e/testdata/inspect-id.golden index 09020fb04..db549f913 100644 --- a/tests/e2e/testdata/inspect-id.golden +++ b/tests/e2e/testdata/inspect-id.golden @@ -11,5 +11,6 @@ "PidsLimit": 0, "Labels": null, "Ports": null, - "Platform": "Linux" + "Platform": "Linux", + "RestartPolicyCondition": "none" } diff --git a/utils/stringutils.go b/utils/stringutils.go new file mode 100644 index 000000000..6febf8e9a --- /dev/null +++ b/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 +}