Merge pull request #434 from docker/aci_error_compose_restart_policy

Allow consistent multi container restart policies but provide clear error message for inconsistent restart policies

aci/convert/convert.go

@@ -72,6 +72,10 @@ func ToContainerGroup(aciContext store.AciContext, p types.Project) (containerin
 	}
 
 	var containers []containerinstance.Container
+	restartPolicy, err := project.getRestartPolicy()
+	if err != nil {
+		return containerinstance.ContainerGroup{}, err
+	}
 	groupDefinition := containerinstance.ContainerGroup{
 		Name:     &containerGroupName,
 		Location: &aciContext.Location,
@@ -80,7 +84,7 @@ func ToContainerGroup(aciContext store.AciContext, p types.Project) (containerin
 			Containers:               &containers,
 			Volumes:                  volumes,
 			ImageRegistryCredentials: &registryCreds,
-			RestartPolicy:            project.getRestartPolicy(),
+			RestartPolicy:            restartPolicy,
 		},
 	}
 
@@ -216,16 +220,26 @@ func (p projectAciHelper) getAciFileVolumes() (map[string]bool, []containerinsta
 	return azureFileVolumesMap, azureFileVolumesSlice, nil
 }
 
-func (p projectAciHelper) getRestartPolicy() containerinstance.ContainerGroupRestartPolicy {
+func (p projectAciHelper) getRestartPolicy() (containerinstance.ContainerGroupRestartPolicy, error) {
 	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 {
+	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)
 				}
-	return restartPolicyCondition
+				if alreadySpecified && restartPolicyCondition != toAciRestartPolicy(service.Deploy.RestartPolicy.Condition) {
+					return "", errors.New("ACI integration does not support specifying different restart policies on containers in the same compose application")
+				}
+
+			}
+		}
+	}
+	return restartPolicyCondition, nil
 }
 
 func toAciRestartPolicy(restartPolicy string) containerinstance.ContainerGroupRestartPolicy {

aci/convert/convert_test.go

@@ -23,11 +23,10 @@ import (
 	"github.com/Azure/azure-sdk-for-go/profiles/latest/containerinstance/mgmt/containerinstance"
 	"github.com/Azure/go-autorest/autorest/to"
 	"github.com/compose-spec/compose-go/types"
-	"gotest.tools/v3/assert"
-	is "gotest.tools/v3/assert/cmp"
-
 	"github.com/docker/api/containers"
 	"github.com/docker/api/context/store"
+	"gotest.tools/v3/assert"
+	is "gotest.tools/v3/assert/cmp"
 )
 
 var convertCtx = store.AciContext{
@@ -150,7 +149,7 @@ func TestComposeSingleContainerGroupToContainerNoDnsSideCarSide(t *testing.T) {
 	assert.Equal(t, *(*group.Containers)[0].Image, "image1")
 }
 
-func TestComposeSingleContainerGroupToContainerSpecificRestartPolicy(t *testing.T) {
+func TestComposeSingleContainerRestartPolicy(t *testing.T) {
 	project := types.Project{
 		Services: []types.ServiceConfig{
 			{
@@ -173,6 +172,68 @@ func TestComposeSingleContainerGroupToContainerSpecificRestartPolicy(t *testing.
 	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(convertCtx, project)
+	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(convertCtx, project)
+	assert.Error(t, err, "ACI integration does not support specifying different restart policies on containers in the same compose application")
+}
+
 func TestComposeSingleContainerGroupToContainerDefaultRestartPolicy(t *testing.T) {
 	project := types.Project{
 		Services: []types.ServiceConfig{