From 3187120d948f11e78ca49ccbfd240fc78b0ca8ad Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Thu, 18 Jun 2020 15:50:49 +0200 Subject: [PATCH 1/7] Initial sidecar container for Aci DNS --- azure/aci.go | 28 ---------------------------- azure/convert/convert.go | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 28 deletions(-) diff --git a/azure/aci.go b/azure/aci.go index 9d566e2d5..90c9c3cca 100644 --- a/azure/aci.go +++ b/azure/aci.go @@ -87,10 +87,6 @@ func createOrUpdateACIContainers(ctx context.Context, aciContext store.AciContex return err } - containerGroup, err := future.Result(containerGroupsClient) - if err != nil { - return err - } for _, c := range *groupDefinition.Containers { w.Event(progress.Event{ ID: *c.Name, @@ -99,30 +95,6 @@ func createOrUpdateACIContainers(ctx context.Context, aciContext store.AciContex }) } - if len(*containerGroup.Containers) > 1 { - var commands []string - for _, container := range *containerGroup.Containers { - commands = append(commands, fmt.Sprintf("echo 127.0.0.1 %s >> /etc/hosts", *container.Name)) - } - commands = append(commands, "exit") - - containers := *containerGroup.Containers - container := containers[0] - response, err := execACIContainer(ctx, aciContext, "/bin/sh", *containerGroup.Name, *container.Name) - if err != nil { - return err - } - - if err = execCommands( - ctx, - *response.WebSocketURI, - *response.Password, - commands, - ); err != nil { - return err - } - } - return err } diff --git a/azure/convert/convert.go b/azure/convert/convert.go index 8bdbf2d0f..0802fa83a 100644 --- a/azure/convert/convert.go +++ b/azure/convert/convert.go @@ -17,6 +17,10 @@ import ( ) const ( + // ComposeDnsSidecarName name of the dns sidecar container + ComposeDnsSidecarName = "aci--dns--sidecar" + + azureFileDriverName = "azure_file" volumeDriveroptsShareNameKey = "share_name" volumeDriveroptsAccountNameKey = "storage_account_name" @@ -94,10 +98,44 @@ func ToContainerGroup(aciContext store.AciContext, p compose.Project) (container containers = append(containers, containerDefinition) } + if len(containers) > 1 { + dnsSideCar := getDnsSidecar(containers) + containers = append(containers, dnsSideCar) + } groupDefinition.ContainerGroupProperties.Containers = &containers + return groupDefinition, nil } +func getDnsSidecar(containers []containerinstance.Container) containerinstance.Container { + var commands []string + for _, container := range containers { + commands = append(commands, fmt.Sprintf("echo 127.0.0.1 %s >> /etc/hosts", *container.Name)) + } + // ACI restart policy is currently at container group level, cannot let the sidecar terminate quietly once /etc/hosts has been edited + // Pricing is done at the container group level so letting the sidecar container "sleep" does not impact the proce for the whole group + commands = append(commands, "sleep infinity") + alpineCmd := []string{"sh", "-c", strings.Join(commands, ";")} + dnsSideCar := containerinstance.Container{ + Name: to.StringPtr(ComposeDnsSidecarName), + ContainerProperties: &containerinstance.ContainerProperties{ + Image: to.StringPtr("alpine:3.12.0"), + Command: &alpineCmd, + Resources: &containerinstance.ResourceRequirements{ + Limits: &containerinstance.ResourceLimits{ + MemoryInGB: to.Float64Ptr(1), + CPU: to.Float64Ptr(1), + }, + Requests: &containerinstance.ResourceRequests{ + MemoryInGB: to.Float64Ptr(1), + CPU: to.Float64Ptr(1), + }, + }, + }, + } + return dnsSideCar +} + type projectAciHelper compose.Project func (p projectAciHelper) getAciSecretVolumes() ([]containerinstance.Volume, error) { From 3f544f0e01cb75010a653c276e26d32d066c4734 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Thu, 18 Jun 2020 15:51:13 +0200 Subject: [PATCH 2/7] =?UTF-8?q?Don=E2=80=99t=20show=20ACI=20sidecar=20in?= =?UTF-8?q?=20`docker=20ps`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- azure/backend.go | 4 ++++ azure/convert/convert.go | 11 +++++------ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/azure/backend.go b/azure/backend.go index ac15d41cd..1cece518a 100644 --- a/azure/backend.go +++ b/azure/backend.go @@ -113,6 +113,10 @@ func (cs *aciContainerService) List(ctx context.Context, _ bool) ([]containers.C for _, container := range *group.Containers { var containerID string + // don't list sidecar container + if *container.Name == convert.ComposeDNSSidecarName { + continue + } if *container.Name == singleContainerName { containerID = *containerGroup.Name } else { diff --git a/azure/convert/convert.go b/azure/convert/convert.go index 0802fa83a..629734c4f 100644 --- a/azure/convert/convert.go +++ b/azure/convert/convert.go @@ -17,9 +17,8 @@ import ( ) const ( - // ComposeDnsSidecarName name of the dns sidecar container - ComposeDnsSidecarName = "aci--dns--sidecar" - + // ComposeDNSSidecarName name of the dns sidecar container + ComposeDNSSidecarName = "aci--dns--sidecar" azureFileDriverName = "azure_file" volumeDriveroptsShareNameKey = "share_name" @@ -99,7 +98,7 @@ func ToContainerGroup(aciContext store.AciContext, p compose.Project) (container containers = append(containers, containerDefinition) } if len(containers) > 1 { - dnsSideCar := getDnsSidecar(containers) + dnsSideCar := getDNSSidecar(containers) containers = append(containers, dnsSideCar) } groupDefinition.ContainerGroupProperties.Containers = &containers @@ -107,7 +106,7 @@ func ToContainerGroup(aciContext store.AciContext, p compose.Project) (container return groupDefinition, nil } -func getDnsSidecar(containers []containerinstance.Container) containerinstance.Container { +func getDNSSidecar(containers []containerinstance.Container) containerinstance.Container { var commands []string for _, container := range containers { commands = append(commands, fmt.Sprintf("echo 127.0.0.1 %s >> /etc/hosts", *container.Name)) @@ -117,7 +116,7 @@ func getDnsSidecar(containers []containerinstance.Container) containerinstance.C commands = append(commands, "sleep infinity") alpineCmd := []string{"sh", "-c", strings.Join(commands, ";")} dnsSideCar := containerinstance.Container{ - Name: to.StringPtr(ComposeDnsSidecarName), + Name: to.StringPtr(ComposeDNSSidecarName), ContainerProperties: &containerinstance.ContainerProperties{ Image: to.StringPtr("alpine:3.12.0"), Command: &alpineCmd, From 4398560a4047d53ec8959b24837fb3d30633bbc0 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Thu, 18 Jun 2020 16:13:14 +0200 Subject: [PATCH 3/7] Remove unused code --- azure/aci.go | 33 --------------------------------- 1 file changed, 33 deletions(-) diff --git a/azure/aci.go b/azure/aci.go index 90c9c3cca..101ce66d0 100644 --- a/azure/aci.go +++ b/azure/aci.go @@ -4,9 +4,7 @@ import ( "context" "fmt" "io" - "io/ioutil" "net/http" - "strings" "time" "github.com/Azure/azure-sdk-for-go/services/containerinstance/mgmt/2018-10-01/containerinstance" @@ -144,37 +142,6 @@ func getTermSize() (*int32, *int32) { return to.Int32Ptr(int32(rows)), to.Int32Ptr(int32(cols)) } -type commandSender struct { - commands string -} - -func (cs *commandSender) Read(p []byte) (int, error) { - if len(cs.commands) == 0 { - return 0, io.EOF - } - - var command string - if len(p) >= len(cs.commands) { - command = cs.commands - cs.commands = "" - } else { - command = cs.commands[:len(p)] - cs.commands = cs.commands[len(p):] - } - - copy(p, command) - - return len(command), nil -} - -func execCommands(ctx context.Context, address string, password string, commands []string) error { - writer := ioutil.Discard - reader := &commandSender{ - commands: strings.Join(commands, "\n"), - } - return exec(ctx, address, password, reader, writer) -} - func exec(ctx context.Context, address string, password string, reader io.Reader, writer io.Writer) error { conn, _, _, err := ws.DefaultDialer.Dial(ctx, address) if err != nil { From 25d912c1edc7ad4c41b8fc1bf4c6e29e125efa38 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Thu, 18 Jun 2020 16:44:56 +0200 Subject: [PATCH 4/7] Tests on compose conversion --- azure/backend_test.go | 58 --------------- azure/convert/convert.go | 3 +- azure/convert/convert_test.go | 136 ++++++++++++++++++++++++++++++---- 3 files changed, 122 insertions(+), 75 deletions(-) diff --git a/azure/backend_test.go b/azure/backend_test.go index 6f014aa18..ef2ab4d10 100644 --- a/azure/backend_test.go +++ b/azure/backend_test.go @@ -3,14 +3,9 @@ package azure import ( "testing" - "github.com/Azure/azure-sdk-for-go/profiles/latest/containerinstance/mgmt/containerinstance" - "github.com/Azure/go-autorest/autorest/to" "github.com/stretchr/testify/suite" . "github.com/onsi/gomega" - - "github.com/docker/api/azure/convert" - "github.com/docker/api/containers" ) type BackendSuiteTest struct { @@ -35,56 +30,3 @@ func TestBackendSuite(t *testing.T) { RegisterTestingT(t) suite.Run(t, new(BackendSuiteTest)) } - -func TestContainerGroupToContainer(t *testing.T) { - myContainerGroup := containerinstance.ContainerGroup{ - ContainerGroupProperties: &containerinstance.ContainerGroupProperties{ - IPAddress: &containerinstance.IPAddress{ - Ports: &[]containerinstance.Port{{ - Port: to.Int32Ptr(80), - }}, - IP: to.StringPtr("42.42.42.42"), - }, - }, - } - myContainer := containerinstance.Container{ - Name: to.StringPtr("myContainerID"), - ContainerProperties: &containerinstance.ContainerProperties{ - Image: to.StringPtr("sha256:666"), - Command: to.StringSlicePtr([]string{"mycommand"}), - Ports: &[]containerinstance.ContainerPort{{ - Port: to.Int32Ptr(80), - }}, - EnvironmentVariables: nil, - InstanceView: &containerinstance.ContainerPropertiesInstanceView{ - RestartCount: nil, - CurrentState: &containerinstance.ContainerState{ - State: to.StringPtr("Running"), - }, - }, - Resources: &containerinstance.ResourceRequirements{ - Limits: &containerinstance.ResourceLimits{ - MemoryInGB: to.Float64Ptr(9), - }, - }, - }, - } - - var expectedContainer = containers.Container{ - ID: "myContainerID", - Status: "Running", - Image: "sha256:666", - Command: "mycommand", - MemoryLimit: 9, - Ports: []containers.Port{{ - HostPort: uint32(80), - ContainerPort: uint32(80), - Protocol: "tcp", - HostIP: "42.42.42.42", - }}, - } - - container, err := convert.ContainerGroupToContainer("myContainerID", myContainerGroup, myContainer) - Expect(err).To(BeNil()) - Expect(container).To(Equal(expectedContainer)) -} diff --git a/azure/convert/convert.go b/azure/convert/convert.go index 629734c4f..87ebfe606 100644 --- a/azure/convert/convert.go +++ b/azure/convert/convert.go @@ -19,6 +19,7 @@ import ( const ( // ComposeDNSSidecarName name of the dns sidecar container ComposeDNSSidecarName = "aci--dns--sidecar" + dnsSidecarImage = "alpine:3.12.0" azureFileDriverName = "azure_file" volumeDriveroptsShareNameKey = "share_name" @@ -118,7 +119,7 @@ func getDNSSidecar(containers []containerinstance.Container) containerinstance.C dnsSideCar := containerinstance.Container{ Name: to.StringPtr(ComposeDNSSidecarName), ContainerProperties: &containerinstance.ContainerProperties{ - Image: to.StringPtr("alpine:3.12.0"), + Image: to.StringPtr(dnsSidecarImage), Command: &alpineCmd, Resources: &containerinstance.ResourceRequirements{ Limits: &containerinstance.ResourceLimits{ diff --git a/azure/convert/convert_test.go b/azure/convert/convert_test.go index 660a35418..e27477963 100644 --- a/azure/convert/convert_test.go +++ b/azure/convert/convert_test.go @@ -3,44 +3,148 @@ package convert import ( "testing" + "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" + "github.com/docker/api/compose" + "github.com/docker/api/containers" "github.com/docker/api/context/store" + . "github.com/onsi/gomega" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" ) -const ( - projectName = "TEST" - expectedProjectName = "test" -) - type ConvertTestSuite struct { suite.Suite - ctx store.AciContext - project compose.Project + ctx store.AciContext } func (suite *ConvertTestSuite) BeforeTest(suiteName, testName string) { - ctx := store.AciContext{ + suite.ctx = store.AciContext{ SubscriptionID: "subID", ResourceGroup: "rg", Location: "eu", } - project := compose.Project{ - Name: projectName, - } - - suite.ctx = ctx - suite.project = project } func (suite *ConvertTestSuite) TestProjectName() { - containerGroup, err := ToContainerGroup(suite.ctx, suite.project) + project := compose.Project{ + Name: "TEST", + } + containerGroup, err := ToContainerGroup(suite.ctx, project) require.NoError(suite.T(), err) - require.Equal(suite.T(), *containerGroup.Name, expectedProjectName) + require.Equal(suite.T(), *containerGroup.Name, "test") +} + +func (suite *ConvertTestSuite) TestContainerGroupToContainer() { + myContainerGroup := containerinstance.ContainerGroup{ + ContainerGroupProperties: &containerinstance.ContainerGroupProperties{ + IPAddress: &containerinstance.IPAddress{ + Ports: &[]containerinstance.Port{{ + Port: to.Int32Ptr(80), + }}, + IP: to.StringPtr("42.42.42.42"), + }, + }, + } + myContainer := containerinstance.Container{ + Name: to.StringPtr("myContainerID"), + ContainerProperties: &containerinstance.ContainerProperties{ + Image: to.StringPtr("sha256:666"), + Command: to.StringSlicePtr([]string{"mycommand"}), + Ports: &[]containerinstance.ContainerPort{{ + Port: to.Int32Ptr(80), + }}, + EnvironmentVariables: nil, + InstanceView: &containerinstance.ContainerPropertiesInstanceView{ + RestartCount: nil, + CurrentState: &containerinstance.ContainerState{ + State: to.StringPtr("Running"), + }, + }, + Resources: &containerinstance.ResourceRequirements{ + Limits: &containerinstance.ResourceLimits{ + MemoryInGB: to.Float64Ptr(9), + }, + }, + }, + } + + var expectedContainer = containers.Container{ + ID: "myContainerID", + Status: "Running", + Image: "sha256:666", + Command: "mycommand", + MemoryLimit: 9, + Ports: []containers.Port{{ + HostPort: uint32(80), + ContainerPort: uint32(80), + Protocol: "tcp", + HostIP: "42.42.42.42", + }}, + } + + container, err := ContainerGroupToContainer("myContainerID", myContainerGroup, myContainer) + Expect(err).To(BeNil()) + Expect(container).To(Equal(expectedContainer)) +} + +func (suite *ConvertTestSuite) TestComposeContainerGroupToContainerWithDnsSideCarSide() { + project := compose.Project{ + Name: "", + Config: types.Config{ + Services: []types.ServiceConfig{ + { + Name: "service1", + Image: "image1", + }, + { + Name: "service2", + Image: "image2", + }, + }, + }, + } + + group, err := ToContainerGroup(suite.ctx, project) + Expect(err).To(BeNil()) + Expect(len(*group.Containers)).To(Equal(3)) + + Expect(*(*group.Containers)[0].Name).To(Equal("service1")) + Expect(*(*group.Containers)[1].Name).To(Equal("service2")) + Expect(*(*group.Containers)[2].Name).To(Equal(ComposeDNSSidecarName)) + + Expect(*(*group.Containers)[2].Command).To(Equal([]string{"sh", "-c", "echo 127.0.0.1 service1 >> /etc/hosts;echo 127.0.0.1 service2 >> /etc/hosts;sleep infinity"})) + + Expect(*(*group.Containers)[0].Image).To(Equal("image1")) + Expect(*(*group.Containers)[1].Image).To(Equal("image2")) + Expect(*(*group.Containers)[2].Image).To(Equal(dnsSidecarImage)) +} + +func (suite *ConvertTestSuite) TestComposeSingleContainerGroupToContainerNoDnsSideCarSide() { + project := compose.Project{ + Name: "", + Config: types.Config{ + 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.Containers)[0].Image).To(Equal("image1")) } func TestConvertTestSuite(t *testing.T) { + RegisterTestingT(t) suite.Run(t, new(ConvertTestSuite)) } From 76a1753396bbf35e324efc3744c2ff6d09755c40 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Thu, 18 Jun 2020 17:45:01 +0200 Subject: [PATCH 5/7] ACI e2e required change, because the output includes the progress info. --- tests/aci-e2e/e2e-aci_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/aci-e2e/e2e-aci_test.go b/tests/aci-e2e/e2e-aci_test.go index f4daa6280..603f9f2f3 100644 --- a/tests/aci-e2e/e2e-aci_test.go +++ b/tests/aci-e2e/e2e-aci_test.go @@ -99,7 +99,7 @@ func (s *E2eACISuite) TestACIBackend() { testStorageAccountName, firstKey, testShareName, mountTarget), "-p", "80:80", "--name", testContainerName).ExecOrDie() - Expect(output).To(Equal(testContainerName + "\n")) + Expect(output).To(ContainSubstring(testContainerName)) output = s.NewDockerCommand("ps").ExecOrDie() lines := Lines(output) Expect(len(lines)).To(Equal(2)) From e4db0d2b9e622ce2ae6320954ccbf9338f5e2b41 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Thu, 18 Jun 2020 17:55:56 +0200 Subject: [PATCH 6/7] Lower CPU/ memory of sidecar container to minimum (reached limits and commented with error message) --- azure/convert/convert.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/azure/convert/convert.go b/azure/convert/convert.go index 87ebfe606..a6f715d4e 100644 --- a/azure/convert/convert.go +++ b/azure/convert/convert.go @@ -113,7 +113,7 @@ func getDNSSidecar(containers []containerinstance.Container) containerinstance.C commands = append(commands, fmt.Sprintf("echo 127.0.0.1 %s >> /etc/hosts", *container.Name)) } // ACI restart policy is currently at container group level, cannot let the sidecar terminate quietly once /etc/hosts has been edited - // Pricing is done at the container group level so letting the sidecar container "sleep" does not impact the proce for the whole group + // Pricing is done at the container group level so letting the sidecar container "sleep" should not impact the price for the whole group commands = append(commands, "sleep infinity") alpineCmd := []string{"sh", "-c", strings.Join(commands, ";")} dnsSideCar := containerinstance.Container{ @@ -123,12 +123,12 @@ func getDNSSidecar(containers []containerinstance.Container) containerinstance.C Command: &alpineCmd, Resources: &containerinstance.ResourceRequirements{ Limits: &containerinstance.ResourceLimits{ - MemoryInGB: to.Float64Ptr(1), - CPU: to.Float64Ptr(1), + MemoryInGB: to.Float64Ptr(0.1), // "The memory requirement should be in incrememts of 0.1 GB." + CPU: to.Float64Ptr(0.01), // "The CPU requirement should be in incrememts of 0.01." }, Requests: &containerinstance.ResourceRequests{ - MemoryInGB: to.Float64Ptr(1), - CPU: to.Float64Ptr(1), + MemoryInGB: to.Float64Ptr(0.1), + CPU: to.Float64Ptr(0.01), }, }, }, From 8bfe0c5ebc098cfc1785c77f8dae7d2c76bf1632 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Thu, 18 Jun 2020 18:02:46 +0200 Subject: [PATCH 7/7] use busy box instead of alpine (much smaller) --- azure/convert/convert.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azure/convert/convert.go b/azure/convert/convert.go index a6f715d4e..064b59acb 100644 --- a/azure/convert/convert.go +++ b/azure/convert/convert.go @@ -19,7 +19,7 @@ import ( const ( // ComposeDNSSidecarName name of the dns sidecar container ComposeDNSSidecarName = "aci--dns--sidecar" - dnsSidecarImage = "alpine:3.12.0" + dnsSidecarImage = "busybox:1.31.1" azureFileDriverName = "azure_file" volumeDriveroptsShareNameKey = "share_name"