Move more specific code out of convert.go: restart policy

Signed-off-by: Guillaume Tardif <guillaume.tardif@docker.com>

aci/convert/convert.go

@@ -155,54 +155,6 @@ func getDNSSidecar(containers []containerinstance.Container) containerinstance.C
 
 type projectAciHelper types.Project
 
-func (p projectAciHelper) getRestartPolicy() (containerinstance.ContainerGroupRestartPolicy, error) {
-	var restartPolicyCondition containerinstance.ContainerGroupRestartPolicy
-	if len(p.Services) >= 1 {
-		alreadySpecified := false
-		restartPolicyCondition = containerinstance.Always
-		for _, service := range p.Services {
-			if service.Deploy != nil &&
-				service.Deploy.RestartPolicy != nil {
-				if !alreadySpecified {
-					alreadySpecified = true
-					restartPolicyCondition = toAciRestartPolicy(service.Deploy.RestartPolicy.Condition)
-				}
-				if alreadySpecified && restartPolicyCondition != toAciRestartPolicy(service.Deploy.RestartPolicy.Condition) {
-					return "", errors.New("ACI integration does not support specifying different restart policies on services in the same compose application")
-				}
-
-			}
-		}
-	}
-	return restartPolicyCondition, 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
-	}
-}
-
 type serviceConfigAciHelper types.ServiceConfig
 
 func (s serviceConfigAciHelper) getAciContainer(volumesCache map[string]bool) (containerinstance.Container, error) {

aci/convert/convert_test.go

@@ -204,91 +204,6 @@ func TestComposeSingleContainerGroupToContainerNoDnsSideCarSide(t *testing.T) {
 	assert.Equal(t, *(*group.Containers)[0].Image, "image1")
 }
 
-func TestComposeSingleContainerRestartPolicy(t *testing.T) {
-	project := types.Project{
-		Services: []types.ServiceConfig{
-			{
-				Name:  "service1",
-				Image: "image1",
-				Deploy: &types.DeployConfig{
-					RestartPolicy: &types.RestartPolicy{
-						Condition: "on-failure",
-					},
-				},
-			},
-		},
-	}
-
-	group, err := ToContainerGroup(context.TODO(), convertCtx, project, mockStorageHelper)
-	assert.NilError(t, err)
-
-	assert.Assert(t, is.Len(*group.Containers, 1))
-	assert.Equal(t, *(*group.Containers)[0].Name, "service1")
-	assert.Equal(t, group.RestartPolicy, containerinstance.OnFailure)
-}
-
-func TestComposeMultiContainerRestartPolicy(t *testing.T) {
-	project := types.Project{
-		Services: []types.ServiceConfig{
-			{
-				Name:  "service1",
-				Image: "image1",
-				Deploy: &types.DeployConfig{
-					RestartPolicy: &types.RestartPolicy{
-						Condition: "on-failure",
-					},
-				},
-			},
-			{
-				Name:  "service2",
-				Image: "image2",
-				Deploy: &types.DeployConfig{
-					RestartPolicy: &types.RestartPolicy{
-						Condition: "on-failure",
-					},
-				},
-			},
-		},
-	}
-
-	group, err := ToContainerGroup(context.TODO(), convertCtx, project, mockStorageHelper)
-	assert.NilError(t, err)
-
-	assert.Assert(t, is.Len(*group.Containers, 3))
-	assert.Equal(t, *(*group.Containers)[0].Name, "service1")
-	assert.Equal(t, group.RestartPolicy, containerinstance.OnFailure)
-	assert.Equal(t, *(*group.Containers)[1].Name, "service2")
-	assert.Equal(t, group.RestartPolicy, containerinstance.OnFailure)
-}
-
-func TestComposeInconsistentMultiContainerRestartPolicy(t *testing.T) {
-	project := types.Project{
-		Services: []types.ServiceConfig{
-			{
-				Name:  "service1",
-				Image: "image1",
-				Deploy: &types.DeployConfig{
-					RestartPolicy: &types.RestartPolicy{
-						Condition: "any",
-					},
-				},
-			},
-			{
-				Name:  "service2",
-				Image: "image2",
-				Deploy: &types.DeployConfig{
-					RestartPolicy: &types.RestartPolicy{
-						Condition: "on-failure",
-					},
-				},
-			},
-		},
-	}
-
-	_, err := ToContainerGroup(context.TODO(), convertCtx, project, mockStorageHelper)
-	assert.Error(t, err, "ACI integration does not support specifying different restart policies on services in the same compose application")
-}
-
 func TestLabelsErrorMessage(t *testing.T) {
 	project := types.Project{
 		Services: []types.ServiceConfig{
@@ -306,24 +221,6 @@ func TestLabelsErrorMessage(t *testing.T) {
 	assert.Error(t, err, "ACI integration does not support labels in compose applications")
 }
 
-func TestComposeSingleContainerGroupToContainerDefaultRestartPolicy(t *testing.T) {
-	project := types.Project{
-		Services: []types.ServiceConfig{
-			{
-				Name:  "service1",
-				Image: "image1",
-			},
-		},
-	}
-
-	group, err := ToContainerGroup(context.TODO(), convertCtx, project, mockStorageHelper)
-	assert.NilError(t, err)
-
-	assert.Assert(t, is.Len(*group.Containers, 1))
-	assert.Equal(t, *(*group.Containers)[0].Name, "service1")
-	assert.Equal(t, group.RestartPolicy, containerinstance.Always)
-}
-
 func TestComposeContainerGroupToContainerWithDomainName(t *testing.T) {
 	project := types.Project{
 		Services: []types.ServiceConfig{
@@ -549,20 +446,6 @@ func TestComposeContainerGroupToContainerenvVar(t *testing.T) {
 	assert.Assert(t, is.Contains(envVars, containerinstance.EnvironmentVariable{Name: to.StringPtr("key2"), Value: to.StringPtr("value2")}))
 }
 
-func TestConvertToAciRestartPolicyCondition(t *testing.T) {
-	assert.Equal(t, toAciRestartPolicy("none"), containerinstance.Never)
-	assert.Equal(t, toAciRestartPolicy("always"), containerinstance.Always)
-	assert.Equal(t, toAciRestartPolicy("on-failure"), containerinstance.OnFailure)
-	assert.Equal(t, toAciRestartPolicy("on-failure:5"), containerinstance.Always)
-}
-
-func TestConvertToDockerRestartPolicyCondition(t *testing.T) {
-	assert.Equal(t, toContainerRestartPolicy(containerinstance.Never), "none")
-	assert.Equal(t, toContainerRestartPolicy(containerinstance.Always), "any")
-	assert.Equal(t, toContainerRestartPolicy(containerinstance.OnFailure), "on-failure")
-	assert.Equal(t, toContainerRestartPolicy(""), "any")
-}
-
 func TestConvertContainerGroupStatus(t *testing.T) {
 	assert.Equal(t, "Running", GetStatus(container(to.StringPtr("Running")), group(to.StringPtr("Started"))))
 	assert.Equal(t, "Terminated", GetStatus(container(to.StringPtr("Terminated")), group(to.StringPtr("Stopped"))))

aci/convert/restartpolicy.go

@@ -0,0 +1,72 @@
+/*
+   Copyright 2020 Docker Compose CLI authors
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package convert
+
+import (
+	"github.com/Azure/azure-sdk-for-go/services/containerinstance/mgmt/2018-10-01/containerinstance"
+	"github.com/pkg/errors"
+
+	"github.com/docker/compose-cli/api/containers"
+)
+
+func (p projectAciHelper) getRestartPolicy() (containerinstance.ContainerGroupRestartPolicy, error) {
+	var restartPolicyCondition containerinstance.ContainerGroupRestartPolicy
+	if len(p.Services) >= 1 {
+		alreadySpecified := false
+		restartPolicyCondition = containerinstance.Always
+		for _, service := range p.Services {
+			if service.Deploy != nil &&
+				service.Deploy.RestartPolicy != nil {
+				if !alreadySpecified {
+					alreadySpecified = true
+					restartPolicyCondition = toAciRestartPolicy(service.Deploy.RestartPolicy.Condition)
+				}
+				if alreadySpecified && restartPolicyCondition != toAciRestartPolicy(service.Deploy.RestartPolicy.Condition) {
+					return "", errors.New("ACI integration does not support specifying different restart policies on services in the same compose application")
+				}
+
+			}
+		}
+	}
+	return restartPolicyCondition, 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
+	}
+}

aci/convert/restartpolicy_test.go

@@ -0,0 +1,144 @@
+/*
+   Copyright 2020 Docker Compose CLI authors
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package convert
+
+import (
+	"context"
+	"testing"
+
+	"github.com/Azure/azure-sdk-for-go/services/containerinstance/mgmt/2018-10-01/containerinstance"
+	"github.com/compose-spec/compose-go/types"
+	"gotest.tools/v3/assert"
+	is "gotest.tools/v3/assert/cmp"
+)
+
+func TestComposeSingleContainerRestartPolicy(t *testing.T) {
+	project := types.Project{
+		Services: []types.ServiceConfig{
+			{
+				Name:  "service1",
+				Image: "image1",
+				Deploy: &types.DeployConfig{
+					RestartPolicy: &types.RestartPolicy{
+						Condition: "on-failure",
+					},
+				},
+			},
+		},
+	}
+
+	group, err := ToContainerGroup(context.TODO(), convertCtx, project, mockStorageHelper)
+	assert.NilError(t, err)
+
+	assert.Assert(t, is.Len(*group.Containers, 1))
+	assert.Equal(t, *(*group.Containers)[0].Name, "service1")
+	assert.Equal(t, group.RestartPolicy, containerinstance.OnFailure)
+}
+
+func TestComposeMultiContainerRestartPolicy(t *testing.T) {
+	project := types.Project{
+		Services: []types.ServiceConfig{
+			{
+				Name:  "service1",
+				Image: "image1",
+				Deploy: &types.DeployConfig{
+					RestartPolicy: &types.RestartPolicy{
+						Condition: "on-failure",
+					},
+				},
+			},
+			{
+				Name:  "service2",
+				Image: "image2",
+				Deploy: &types.DeployConfig{
+					RestartPolicy: &types.RestartPolicy{
+						Condition: "on-failure",
+					},
+				},
+			},
+		},
+	}
+
+	group, err := ToContainerGroup(context.TODO(), convertCtx, project, mockStorageHelper)
+	assert.NilError(t, err)
+
+	assert.Assert(t, is.Len(*group.Containers, 3))
+	assert.Equal(t, *(*group.Containers)[0].Name, "service1")
+	assert.Equal(t, group.RestartPolicy, containerinstance.OnFailure)
+	assert.Equal(t, *(*group.Containers)[1].Name, "service2")
+	assert.Equal(t, group.RestartPolicy, containerinstance.OnFailure)
+}
+
+func TestComposeInconsistentMultiContainerRestartPolicy(t *testing.T) {
+	project := types.Project{
+		Services: []types.ServiceConfig{
+			{
+				Name:  "service1",
+				Image: "image1",
+				Deploy: &types.DeployConfig{
+					RestartPolicy: &types.RestartPolicy{
+						Condition: "any",
+					},
+				},
+			},
+			{
+				Name:  "service2",
+				Image: "image2",
+				Deploy: &types.DeployConfig{
+					RestartPolicy: &types.RestartPolicy{
+						Condition: "on-failure",
+					},
+				},
+			},
+		},
+	}
+
+	_, err := ToContainerGroup(context.TODO(), convertCtx, project, mockStorageHelper)
+	assert.Error(t, err, "ACI integration does not support specifying different restart policies on services in the same compose application")
+}
+
+func TestComposeSingleContainerGroupToContainerDefaultRestartPolicy(t *testing.T) {
+	project := types.Project{
+		Services: []types.ServiceConfig{
+			{
+				Name:  "service1",
+				Image: "image1",
+			},
+		},
+	}
+
+	group, err := ToContainerGroup(context.TODO(), convertCtx, project, mockStorageHelper)
+	assert.NilError(t, err)
+
+	assert.Assert(t, is.Len(*group.Containers, 1))
+	assert.Equal(t, *(*group.Containers)[0].Name, "service1")
+	assert.Equal(t, group.RestartPolicy, containerinstance.Always)
+}
+
+func TestConvertToAciRestartPolicyCondition(t *testing.T) {
+	assert.Equal(t, toAciRestartPolicy("none"), containerinstance.Never)
+	assert.Equal(t, toAciRestartPolicy("always"), containerinstance.Always)
+	assert.Equal(t, toAciRestartPolicy("on-failure"), containerinstance.OnFailure)
+	assert.Equal(t, toAciRestartPolicy("on-failure:5"), containerinstance.Always)
+}
+
+func TestConvertToDockerRestartPolicyCondition(t *testing.T) {
+	assert.Equal(t, toContainerRestartPolicy(containerinstance.Never), "none")
+	assert.Equal(t, toContainerRestartPolicy(containerinstance.Always), "any")
+	assert.Equal(t, toContainerRestartPolicy(containerinstance.OnFailure), "on-failure")
+	assert.Equal(t, toContainerRestartPolicy(""), "any")
+}