Преглед на файлове

User compose.service.domainname rather than custom ACI extension for ACI DNSLabelName

Signed-off-by: Guillaume Tardif <[email protected]>
Guillaume Tardif преди 5 години
родител
ревизия
cf3bb18c0e
променени са 6 файла, в които са добавени 123 реда и са изтрити 28 реда
  1. 0 3
      aci/containers.go
  2. 1 0
      aci/convert/container.go
  3. 12 0
      aci/convert/container_test.go
  4. 23 21
      aci/convert/convert.go
  5. 83 1
      aci/convert/convert_test.go
  6. 4 3
      cli/cmd/compose/up.go

+ 0 - 3
aci/containers.go

@@ -86,9 +86,6 @@ func (cs *aciContainerService) Run(ctx context.Context, r containers.ContainerCo
 		return err
 	}
 	addTag(&groupDefinition, singleContainerTag)
-	if r.DomainName != "" {
-		groupDefinition.ContainerGroupProperties.IPAddress.DNSNameLabel = &r.DomainName
-	}
 
 	return createACIContainers(ctx, cs.ctx, groupDefinition)
 }

+ 1 - 0
aci/convert/container.go

@@ -49,6 +49,7 @@ func ContainerToComposeProject(r containers.ContainerConfig) (types.Project, err
 				Ports:       ports,
 				Labels:      r.Labels,
 				Volumes:     serviceConfigVolumes,
+				DomainName:  r.DomainName,
 				Environment: toComposeEnvs(r.Environment),
 				Deploy: &types.DeployConfig{
 					Resources: types.Resources{

+ 12 - 0
aci/convert/container_test.go

@@ -54,6 +54,18 @@ func TestConvertRestartPolicy(t *testing.T) {
 	assert.Equal(t, service1.Deploy.RestartPolicy.Condition, "none")
 }
 
+func TestConvertDomainName(t *testing.T) {
+	container := containers.ContainerConfig{
+		ID:         "container1",
+		DomainName: "myapp",
+	}
+	project, err := ContainerToComposeProject(container)
+	assert.NilError(t, err)
+	service1 := project.Services[0]
+	assert.Equal(t, service1.Name, container.ID)
+	assert.Equal(t, service1.DomainName, "myapp")
+}
+
 func TestConvertEnvVariables(t *testing.T) {
 	container := containers.ContainerConfig{
 		ID: "container1",

+ 23 - 21
aci/convert/convert.go

@@ -43,8 +43,6 @@ const (
 	StatusRunning = "Running"
 	// ComposeDNSSidecarName name of the dns sidecar container
 	ComposeDNSSidecarName = "aci--dns--sidecar"
-	// ExtensionDomainName compose extension to set ACI DNS label name
-	ExtensionDomainName = "x-aci-domain-name"
 
 	dnsSidecarImage                = "busybox:1.31.1"
 	azureFileDriverName            = "azure_file"
@@ -95,6 +93,7 @@ func ToContainerGroup(ctx context.Context, aciContext store.AciContext, p types.
 	}
 
 	var groupPorts []containerinstance.Port
+	var dnsLabelName *string
 	for _, s := range project.Services {
 		service := serviceConfigAciHelper(s)
 		containerDefinition, err := service.getAciContainer(volumesCache)
@@ -104,22 +103,29 @@ func ToContainerGroup(ctx context.Context, aciContext store.AciContext, p types.
 		if service.Labels != nil && len(service.Labels) > 0 {
 			return containerinstance.ContainerGroup{}, errors.New("ACI integration does not support labels in compose applications")
 		}
-		if service.Ports != nil {
-			containerPorts, serviceGroupPorts, dnsLabelName, err := convertPortsToAci(service, p)
-			if err != nil {
-				return groupDefinition, err
-			}
-			containerDefinition.ContainerProperties.Ports = &containerPorts
-			groupPorts = append(groupPorts, serviceGroupPorts...)
-			groupDefinition.ContainerGroupProperties.IPAddress = &containerinstance.IPAddress{
-				Type:         containerinstance.Public,
-				Ports:        &groupPorts,
-				DNSNameLabel: dnsLabelName,
+
+		containerPorts, serviceGroupPorts, serviceDomainName, err := convertPortsToAci(service)
+		if err != nil {
+			return groupDefinition, err
+		}
+		containerDefinition.ContainerProperties.Ports = &containerPorts
+		groupPorts = append(groupPorts, serviceGroupPorts...)
+		if serviceDomainName != nil {
+			if dnsLabelName != nil && *serviceDomainName != *dnsLabelName {
+				return containerinstance.ContainerGroup{}, fmt.Errorf("ACI integration does not support specifying different domain names on services in the same compose application")
 			}
+			dnsLabelName = serviceDomainName
 		}
 
 		containers = append(containers, containerDefinition)
 	}
+	if len(groupPorts) > 0 {
+		groupDefinition.ContainerGroupProperties.IPAddress = &containerinstance.IPAddress{
+			Type:         containerinstance.Public,
+			Ports:        &groupPorts,
+			DNSNameLabel: dnsLabelName,
+		}
+	}
 	if len(containers) > 1 {
 		dnsSideCar := getDNSSidecar(containers)
 		containers = append(containers, dnsSideCar)
@@ -129,7 +135,7 @@ func ToContainerGroup(ctx context.Context, aciContext store.AciContext, p types.
 	return groupDefinition, nil
 }
 
-func convertPortsToAci(service serviceConfigAciHelper, p types.Project) ([]containerinstance.ContainerPort, []containerinstance.Port, *string, error) {
+func convertPortsToAci(service serviceConfigAciHelper) ([]containerinstance.ContainerPort, []containerinstance.Port, *string, error) {
 	var groupPorts []containerinstance.Port
 	var containerPorts []containerinstance.ContainerPort
 	for _, portConfig := range service.Ports {
@@ -148,12 +154,8 @@ func convertPortsToAci(service serviceConfigAciHelper, p types.Project) ([]conta
 		})
 	}
 	var dnsLabelName *string = nil
-	if extension, ok := p.Extensions[ExtensionDomainName]; ok {
-		domain, ok := extension.(string)
-		if !ok {
-			return nil, nil, nil, fmt.Errorf("could not read %s compose extension as string", ExtensionDomainName)
-		}
-		dnsLabelName = &domain
+	if service.DomainName != "" {
+		dnsLabelName = &service.DomainName
 	}
 	return containerPorts, groupPorts, dnsLabelName, nil
 }
@@ -270,7 +272,7 @@ func (p projectAciHelper) getRestartPolicy() (containerinstance.ContainerGroupRe
 					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 containers in the same compose application")
+					return "", errors.New("ACI integration does not support specifying different restart policies on services in the same compose application")
 				}
 
 			}

+ 83 - 1
aci/convert/convert_test.go

@@ -370,7 +370,7 @@ func TestComposeInconsistentMultiContainerRestartPolicy(t *testing.T) {
 	}
 
 	_, err := ToContainerGroup(context.TODO(), convertCtx, project, mockStorageHelper)
-	assert.Error(t, err, "ACI integration does not support specifying different restart policies on containers in the same compose application")
+	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) {
@@ -452,6 +452,88 @@ func TestComposeContainerGroupToContainerMultiplePorts(t *testing.T) {
 	assert.Assert(t, is.Len(groupPorts, 2))
 	assert.Equal(t, *groupPorts[0].Port, int32(80))
 	assert.Equal(t, *groupPorts[1].Port, int32(8080))
+	assert.Assert(t, group.IPAddress.DNSNameLabel == nil)
+}
+
+func TestComposeContainerGroupToContainerWithDomainName(t *testing.T) {
+	project := types.Project{
+		Services: []types.ServiceConfig{
+			{
+				Name:  "service1",
+				Image: "image1",
+				Ports: []types.ServicePortConfig{
+					{
+						Published: 80,
+						Target:    80,
+					},
+				},
+				DomainName: "myApp",
+			},
+			{
+				Name:  "service2",
+				Image: "image2",
+				Ports: []types.ServicePortConfig{
+					{
+						Published: 8080,
+						Target:    8080,
+					},
+				},
+			},
+		},
+	}
+
+	group, err := ToContainerGroup(context.TODO(), convertCtx, project, mockStorageHelper)
+	assert.NilError(t, err)
+	assert.Assert(t, is.Len(*group.Containers, 3))
+
+	groupPorts := *group.IPAddress.Ports
+	assert.Assert(t, is.Len(groupPorts, 2))
+	assert.Equal(t, *groupPorts[0].Port, int32(80))
+	assert.Equal(t, *groupPorts[1].Port, int32(8080))
+	assert.Equal(t, *group.IPAddress.DNSNameLabel, "myApp")
+}
+
+func TestComposeContainerGroupToContainerErrorWhenSeveralDomainNames(t *testing.T) {
+	project := types.Project{
+		Services: []types.ServiceConfig{
+			{
+				Name:       "service1",
+				Image:      "image1",
+				DomainName: "myApp",
+			},
+			{
+				Name:       "service2",
+				Image:      "image2",
+				DomainName: "myApp2",
+			},
+		},
+	}
+
+	_, err := ToContainerGroup(context.TODO(), convertCtx, project, mockStorageHelper)
+	assert.Error(t, err, "ACI integration does not support specifying different domain names on services in the same compose application")
+}
+
+// ACI fails if group definition IPAddress has no ports
+func TestComposeContainerGroupToContainerIgnoreDomainNameWithoutPorts(t *testing.T) {
+	project := types.Project{
+		Services: []types.ServiceConfig{
+			{
+				Name:       "service1",
+				Image:      "image1",
+				DomainName: "myApp",
+			},
+			{
+				Name:       "service2",
+				Image:      "image2",
+				DomainName: "myApp",
+			},
+		},
+	}
+
+	group, err := ToContainerGroup(context.TODO(), convertCtx, project, mockStorageHelper)
+	assert.NilError(t, err)
+	assert.Assert(t, is.Len(*group.Containers, 3))
+	assert.Assert(t, group.IPAddress == nil)
 }
 
 func TestComposeContainerGroupToContainerResourceLimits(t *testing.T) {

+ 4 - 3
cli/cmd/compose/up.go

@@ -19,10 +19,10 @@ package compose
 import (
 	"context"
 
-	"github.com/compose-spec/compose-go/cli"
 	"github.com/spf13/cobra"
 
-	aciconvert "github.com/docker/compose-cli/aci/convert"
+	"github.com/compose-spec/compose-go/cli"
+
 	"github.com/docker/compose-cli/api/client"
 	"github.com/docker/compose-cli/context/store"
 	"github.com/docker/compose-cli/progress"
@@ -62,7 +62,8 @@ func runUp(ctx context.Context, opts composeOptions) error {
 		}
 		project, err := cli.ProjectFromOptions(options)
 		if opts.DomainName != "" {
-			project.Extensions = map[string]interface{}{aciconvert.ExtensionDomainName: opts.DomainName}
+			//arbitrarily set the domain name on the first service ; ACI backend will expose the entire project
+			project.Services[0].DomainName = opts.DomainName
 		}
 		if err != nil {
 			return "", err