Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: use container name consistent with Docker Compose API #633

Merged
merged 9 commits into from
Dec 24, 2021
Merged

Fix: use container name consistent with Docker Compose API #633

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
906267f
Set service naming per docker compoose version
r00tu53r Dec 22, 2021
3904cfb
refactor code, fix linter failure
r00tu53r Dec 23, 2021
dc8dd27
refactor
r00tu53r Dec 23, 2021
35ba5cf
Update internal/compose/compose.go
mtojek Dec 23, 2021
8a0f337
remove extraneous newline
r00tu53r Dec 24, 2021
a9710fe
Adjust Project{}
Dec 24, 2021
201fd2c
Fix
Dec 24, 2021
27c58be
nit
Dec 24, 2021
42416ef
Fix: make format
Dec 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 29 additions & 2 deletions internal/compose/compose.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ import (
"github.com/elastic/elastic-package/internal/signal"
)

const (
DockerComposeV1 = "v1"
DockerComposeV2 = "v2"
r00tu53r marked this conversation as resolved.
Show resolved Hide resolved
)

// Project represents a Docker Compose project.
type Project struct {
name string
Expand Down Expand Up @@ -340,7 +345,29 @@ func (p *Project) runDockerComposeCmd(opts dockerComposeOptions) error {
return cmd.Run()
}

func (p *Project) DockerComposeVersion() (string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this method is used only by ContainerName, do you think we could make it package-visible and access it only in ContainerName?

This way you would prevent adjusting all calling code. If we need this condition in other places, we can refactor it then.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. I have refactored this to make it package-visible. To avoid calling docker-compose --version each time ContainerName is invoked I've stored the value in a project variable.


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: empty line (cane be removed)

const (
v1marker = "docker-compose version 1"
r00tu53r marked this conversation as resolved.
Show resolved Hide resolved
)
var b bytes.Buffer

args := []string{"version"}
r00tu53r marked this conversation as resolved.
Show resolved Hide resolved
if err := p.runDockerComposeCmd(dockerComposeOptions{args: args, stdout: &b}); err != nil {
return "", errors.Wrap(err, "running Docker Compose version command failed")
}
versionStr := string(b.Bytes())
if strings.Contains(versionStr, v1marker) {
return DockerComposeV1, nil
}
// v2marker: Docker Compose version v2.x.y
return DockerComposeV2, nil
}

// ContainerName method the container name for the service.
func (p *Project) ContainerName(serviceName string) string {
return fmt.Sprintf("%s_%s_1", p.name, serviceName)
func (p *Project) ContainerName(dcver, serviceName string) string {
if dcver == DockerComposeV1 {
return fmt.Sprintf("%s_%s_1", p.name, serviceName)
}
return fmt.Sprintf("%s-%s-1", p.name, serviceName)
}
6 changes: 5 additions & 1 deletion internal/stack/logs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,11 @@ func copyDockerInternalLogs(serviceName, outputPath string) error {
}

outputPath = filepath.Join(outputPath, serviceName+"-internal")
serviceContainer := p.ContainerName(serviceName)
dcver, err := p.DockerComposeVersion()
if err != nil {
return errors.Wrap(err, "failed to copy docker internal logs")
}
serviceContainer := p.ContainerName(dcver, serviceName)
err = docker.Copy(serviceContainer, "/usr/share/elastic-agent/state/data/logs/default", outputPath)
if err != nil {
return errors.Wrap(err, "docker copy failed")
Expand Down
10 changes: 7 additions & 3 deletions internal/testrunner/runners/system/servicedeployer/compose.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,16 +93,20 @@ func (d *DockerComposeServiceDeployer) SetUp(inCtxt ServiceContext) (DeployedSer
return nil, errors.Wrap(err, "service is unhealthy")
}

dcver, err := p.DockerComposeVersion()
r00tu53r marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, errors.Wrap(err, "unable to determine version")
}
// Build service container name
outCtxt.Hostname = p.ContainerName(serviceName)
outCtxt.Hostname = p.ContainerName(dcver, serviceName)

// Connect service network with stack network (for the purpose of metrics collection)
err = docker.ConnectToNetwork(p.ContainerName(serviceName), stack.Network())
err = docker.ConnectToNetwork(p.ContainerName(dcver, serviceName), stack.Network())
if err != nil {
return nil, errors.Wrapf(err, "can't attach service container to the stack network")
}

logger.Debugf("adding service container %s internal ports to context", p.ContainerName(serviceName))
logger.Debugf("adding service container %s internal ports to context", p.ContainerName(dcver, serviceName))
serviceComposeConfig, err := p.Config(compose.CommandOptions{
Env: []string{fmt.Sprintf("%s=%s", serviceLogsDirEnv, outCtxt.Logs.Folder.Local)},
})
Expand Down