From a2c2d6aa5dfcb6603bf4652b41c5edbf4d5d9ced Mon Sep 17 00:00:00 2001 From: Ulysses Souza Date: Thu, 16 Jul 2020 11:05:31 +0200 Subject: [PATCH] Add Restart Policy support when running single container --- aci/convert/container.go | 8 ++++ aci/convert/container_test.go | 12 +++++ aci/convert/convert.go | 65 ++++++++++++++++++++------ aci/convert/convert_test.go | 56 ++++++++++++++++++++++ cli/cmd/run/run.go | 1 + cli/cmd/run/testdata/run-help.golden | 1 + cli/cmd/testdata/inspect-out-id.golden | 3 +- cli/options/run/opts.go | 34 +++++++------- containers/api.go | 29 ++++++------ containers/types.go | 11 +++++ ecs/backend.go | 3 +- tests/aci-e2e/e2e-aci_test.go | 14 ++++-- 12 files changed, 189 insertions(+), 48 deletions(-) create mode 100644 containers/types.go diff --git a/aci/convert/container.go b/aci/convert/container.go index e684deddf..29bfc966c 100644 --- a/aci/convert/container.go +++ b/aci/convert/container.go @@ -24,6 +24,11 @@ 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{ @@ -41,6 +46,9 @@ func ContainerToComposeProject(r containers.ContainerConfig) (types.Project, err MemoryBytes: types.UnitBytes(r.MemLimit.Value()), }, }, + RestartPolicy: &types.RestartPolicy{ + Condition: composeRestartPolicyCondition, + }, }, }, }, diff --git a/aci/convert/container_test.go b/aci/convert/container_test.go index 8b05cf9f8..b2f7a0472 100644 --- a/aci/convert/container_test.go +++ b/aci/convert/container_test.go @@ -48,6 +48,18 @@ func (suite *ContainerConvertTestSuite) TestConvertContainerEnvironment() { })) } +func (suite *ContainerConvertTestSuite) TestConvertRestartPolicy() { + container := containers.ContainerConfig{ + ID: "container1", + RestartPolicyCondition: "no", + } + project, err := ContainerToComposeProject(container) + Expect(err).To(BeNil()) + service1 := project.Services[0] + Expect(service1.Name).To(Equal(container.ID)) + Expect(service1.Deploy.RestartPolicy.Condition).To(Equal("none")) +} + func TestContainerConvertTestSuite(t *testing.T) { RegisterTestingT(t) suite.Run(t, new(ContainerConvertTestSuite)) diff --git a/aci/convert/convert.go b/aci/convert/convert.go index 60bface71..0711170b8 100644 --- a/aci/convert/convert.go +++ b/aci/convert/convert.go @@ -26,7 +26,7 @@ import ( "strconv" "strings" - "github.com/Azure/azure-sdk-for-go/profiles/latest/containerinstance/mgmt/containerinstance" + "github.com/Azure/azure-sdk-for-go/services/containerinstance/mgmt/2018-10-01/containerinstance" "github.com/Azure/go-autorest/autorest/to" "github.com/compose-spec/compose-go/types" @@ -71,6 +71,15 @@ 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, @@ -80,6 +89,7 @@ func ToContainerGroup(aciContext store.AciContext, p types.Project) (containerin Containers: &containers, Volumes: volumes, ImageRegistryCredentials: ®istryCreds, + RestartPolicy: restartPolicyCondition, }, } @@ -125,6 +135,32 @@ 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 { @@ -348,19 +384,20 @@ func ContainerGroupToContainer(containerID string, cg containerinstance.Containe platform := string(cg.OsType) c := containers.Container{ - ID: containerID, - Status: status, - Image: to.String(cc.Image), - Command: command, - CPUTime: 0, - CPULimit: cpuLimit, - MemoryUsage: 0, - MemoryLimit: uint64(memLimits), - PidsCurrent: 0, - PidsLimit: 0, - Labels: nil, - Ports: ToPorts(cg.IPAddress, *cc.Ports), - Platform: platform, + ID: containerID, + Status: status, + Image: to.String(cc.Image), + Command: command, + CPUTime: 0, + CPULimit: cpuLimit, + MemoryUsage: 0, + MemoryLimit: uint64(memLimits), + PidsCurrent: 0, + PidsLimit: 0, + Labels: nil, + Ports: ToPorts(cg.IPAddress, *cc.Ports), + Platform: platform, + RestartPolicyCondition: toContainerRestartPolicy(cg.RestartPolicy), } return c, nil diff --git a/aci/convert/convert_test.go b/aci/convert/convert_test.go index ea004b591..8b46fd847 100644 --- a/aci/convert/convert_test.go +++ b/aci/convert/convert_test.go @@ -104,6 +104,7 @@ func (suite *ConvertTestSuite) TestContainerGroupToContainer() { Protocol: "tcp", HostIP: "42.42.42.42", }}, + RestartPolicyCondition: "any", } container, err := ContainerGroupToContainer("myContainerID", myContainerGroup, myContainer) @@ -158,6 +159,47 @@ func (suite *ConvertTestSuite) TestComposeSingleContainerGroupToContainerNoDnsSi Expect(*(*group.Containers)[0].Image).To(Equal("image1")) } +func (suite *ConvertTestSuite) TestComposeSingleContainerGroupToContainerSpecificRestartPolicy() { + project := types.Project{ + Services: []types.ServiceConfig{ + { + Name: "service1", + Image: "image1", + Deploy: &types.DeployConfig{ + RestartPolicy: &types.RestartPolicy{ + Condition: "on-failure", + }, + }, + }, + }, + } + + group, err := ToContainerGroup(suite.ctx, project) + Expect(err).To(BeNil()) + + Expect(len(*group.Containers)).To(Equal(1)) + Expect(*(*group.Containers)[0].Name).To(Equal("service1")) + Expect(group.RestartPolicy).To(Equal(containerinstance.OnFailure)) +} + +func (suite *ConvertTestSuite) TestComposeSingleContainerGroupToContainerDefaultRestartPolicy() { + project := types.Project{ + Services: []types.ServiceConfig{ + { + Name: "service1", + Image: "image1", + }, + }, + } + + group, err := ToContainerGroup(suite.ctx, project) + Expect(err).To(BeNil()) + + Expect(len(*group.Containers)).To(Equal(1)) + Expect(*(*group.Containers)[0].Name).To(Equal("service1")) + Expect(group.RestartPolicy).To(Equal(containerinstance.Always)) +} + func (suite *ConvertTestSuite) TestComposeContainerGroupToContainerMultiplePorts() { project := types.Project{ Services: []types.ServiceConfig{ @@ -287,6 +329,20 @@ func (suite *ConvertTestSuite) TestComposeContainerGroupToContainerenvVar() { Expect(envVars).To(ContainElement(containerinstance.EnvironmentVariable{Name: to.StringPtr("key2"), Value: to.StringPtr("value2")})) } +func (suite *ConvertTestSuite) TestConvertToAciRestartPolicyCondition() { + Expect(toAciRestartPolicy("none")).To(Equal(containerinstance.Never)) + Expect(toAciRestartPolicy("always")).To(Equal(containerinstance.Always)) + Expect(toAciRestartPolicy("on-failure")).To(Equal(containerinstance.OnFailure)) + Expect(toAciRestartPolicy("on-failure:5")).To(Equal(containerinstance.Always)) +} + +func (suite *ConvertTestSuite) TestConvertToDockerRestartPolicyCondition() { + Expect(toContainerRestartPolicy(containerinstance.Never)).To(Equal("none")) + Expect(toContainerRestartPolicy(containerinstance.Always)).To(Equal("any")) + Expect(toContainerRestartPolicy(containerinstance.OnFailure)).To(Equal("on-failure")) + Expect(toContainerRestartPolicy("")).To(Equal("any")) +} + func TestConvertTestSuite(t *testing.T) { RegisterTestingT(t) suite.Run(t, new(ConvertTestSuite)) diff --git a/cli/cmd/run/run.go b/cli/cmd/run/run.go index faca28a5f..b05750489 100644 --- a/cli/cmd/run/run.go +++ b/cli/cmd/run/run.go @@ -52,6 +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") return cmd } diff --git a/cli/cmd/run/testdata/run-help.golden b/cli/cmd/run/testdata/run-help.golden index f759be025..b1e7f0536 100644 --- a/cli/cmd/run/testdata/run-help.golden +++ b/cli/cmd/run/testdata/run-help.golden @@ -11,4 +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") -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 09020fb04..98cf5c2fe 100644 --- a/cli/cmd/testdata/inspect-out-id.golden +++ b/cli/cmd/testdata/inspect-out-id.golden @@ -11,5 +11,6 @@ "PidsLimit": 0, "Labels": null, "Ports": null, - "Platform": "Linux" + "Platform": "Linux", + "RestartPolicyCondition": "" } diff --git a/cli/options/run/opts.go b/cli/options/run/opts.go index 6db435269..460df71f5 100644 --- a/cli/options/run/opts.go +++ b/cli/options/run/opts.go @@ -30,14 +30,15 @@ import ( // Opts contain run command options type Opts struct { - Name string - Publish []string - Labels []string - Volumes []string - Cpus float64 - Memory formatter.MemBytes - Detach bool - Environment []string + Name string + Publish []string + Labels []string + Volumes []string + Cpus float64 + Memory formatter.MemBytes + Detach bool + Environment []string + RestartPolicyCondition string } // ToContainerConfig convert run options to a container configuration @@ -57,14 +58,15 @@ func (r *Opts) ToContainerConfig(image string) (containers.ContainerConfig, erro } return containers.ContainerConfig{ - ID: r.Name, - Image: image, - Ports: publish, - Labels: labels, - Volumes: r.Volumes, - MemLimit: r.Memory, - CPULimit: r.Cpus, - Environment: r.Environment, + ID: r.Name, + Image: image, + Ports: publish, + Labels: labels, + Volumes: r.Volumes, + MemLimit: r.Memory, + CPULimit: r.Cpus, + Environment: r.Environment, + RestartPolicyCondition: r.RestartPolicyCondition, }, nil } diff --git a/containers/api.go b/containers/api.go index 41c1ce296..2246b9b58 100644 --- a/containers/api.go +++ b/containers/api.go @@ -25,19 +25,20 @@ import ( // Container represents a created container type Container struct { - ID string - Status string - Image string - Command string - CPUTime uint64 - CPULimit float64 - MemoryUsage uint64 - MemoryLimit uint64 - PidsCurrent uint64 - PidsLimit uint64 - Labels []string - Ports []Port - Platform string + ID string + Status string + Image string + Command string + CPUTime uint64 + CPULimit float64 + MemoryUsage uint64 + MemoryLimit uint64 + PidsCurrent uint64 + PidsLimit uint64 + Labels []string + Ports []Port + Platform string + RestartPolicyCondition string } // Port represents a published port of a container @@ -70,6 +71,8 @@ type ContainerConfig struct { CPULimit float64 // Environment variables Environment []string + // Restart policy condition + RestartPolicyCondition string } // ExecRequest contaiens configuration about an exec request diff --git a/containers/types.go b/containers/types.go new file mode 100644 index 000000000..8a6553776 --- /dev/null +++ b/containers/types.go @@ -0,0 +1,11 @@ +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/ecs/backend.go b/ecs/backend.go index 6fe5d187e..02cf121a6 100644 --- a/ecs/backend.go +++ b/ecs/backend.go @@ -18,6 +18,8 @@ package ecs import ( "context" + ecsplugin "github.com/docker/ecs-plugin/pkg/amazon/backend" + "github.com/docker/api/backend" "github.com/docker/api/compose" "github.com/docker/api/containers" @@ -25,7 +27,6 @@ import ( "github.com/docker/api/context/cloud" "github.com/docker/api/context/store" "github.com/docker/api/errdefs" - ecsplugin "github.com/docker/ecs-plugin/pkg/amazon/backend" ) const backendType = store.EcsContextType diff --git a/tests/aci-e2e/e2e-aci_test.go b/tests/aci-e2e/e2e-aci_test.go index 67e71badd..17f8cc759 100644 --- a/tests/aci-e2e/e2e-aci_test.go +++ b/tests/aci-e2e/e2e-aci_test.go @@ -86,6 +86,7 @@ func (s *E2eACISuite) TestACIRunSingleContainer() { defer deleteResourceGroup(resourceGroupName) var nginxExposedURL string + var containerID string s.Step("runs nginx on port 80", func() { aciContext := store.AciContext{ SubscriptionID: subscriptionID, @@ -118,7 +119,7 @@ func (s *E2eACISuite) TestACIRunSingleContainer() { Expect(containerFields[1]).To(Equal("nginx")) Expect(containerFields[2]).To(Equal("Running")) exposedIP := containerFields[3] - containerID := containerFields[0] + containerID = containerFields[0] Expect(exposedIP).To(ContainSubstring(":80->80/tcp")) nginxExposedURL = strings.ReplaceAll(exposedIP, "->80/tcp", "") @@ -129,6 +130,13 @@ func (s *E2eACISuite) TestACIRunSingleContainer() { Expect(output).To(ContainSubstring("GET")) }) + s.Step("inspect command", func() { + inspect := s.NewDockerCommand("inspect", containerID).ExecOrDie() + Expect(inspect).To(ContainSubstring("\"Platform\": \"Linux\"")) + Expect(inspect).To(ContainSubstring("\"CPULimit\": 1")) + Expect(inspect).To(ContainSubstring("\"RestartPolicyCondition\": \"none\"")) + }) + s.Step("exec command", func() { output := s.NewDockerCommand("exec", containerName, "pwd").ExecOrDie() Expect(output).To(ContainSubstring("/")) @@ -174,13 +182,12 @@ func (s *E2eACISuite) TestACIRunSingleContainer() { shutdown := make(chan time.Time) errs := make(chan error) outChan := make(chan string) - cmd := s.NewDockerCommand("run", "nginx", "--memory", "0.1G", "--cpus", "0.1", "-p", "80:80", "--name", testContainerName).WithTimeout(shutdown) + cmd := s.NewDockerCommand("run", "nginx", "--restart", "on-failure", "--memory", "0.1G", "--cpus", "0.1", "-p", "80:80", "--name", testContainerName).WithTimeout(shutdown) go func() { output, err := cmd.Exec() outChan <- output errs <- err }() - var containerID string err := WaitFor(time.Second, 100*time.Second, errs, func() bool { output := s.NewDockerCommand("ps").ExecOrDie() lines := Lines(output) @@ -201,6 +208,7 @@ func (s *E2eACISuite) TestACIRunSingleContainer() { inspect := s.NewDockerCommand("inspect", containerID).ExecOrDie() Expect(inspect).To(ContainSubstring("\"CPULimit\": 0.1")) Expect(inspect).To(ContainSubstring("\"MemoryLimit\": 107374182")) + Expect(inspect).To(ContainSubstring("\"RestartPolicyCondition\": \"on-failure\"")) // Give a little time to get logs of the curl call time.Sleep(5 * time.Second)