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

Changes from 2 commits
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
40 changes: 37 additions & 3 deletions internal/compose/compose.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"strings"
"time"

"github.com/Masterminds/semver"
"github.com/pkg/errors"
"gopkg.in/yaml.v3"

Expand All @@ -25,6 +26,7 @@ import (
// Project represents a Docker Compose project.
type Project struct {
name string
dcMajorVersion string
composeFilePaths []string
}

Expand Down Expand Up @@ -141,8 +143,9 @@ func NewProject(name string, paths ...string) (*Project, error) {
}

c := Project{
name,
paths,
name: name,
dcMajorVersion: "",
Copy link
Contributor

Choose a reason for hiding this comment

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

As suggested in the other comment, dcMajorVersion can be replaced with dockerComposeV1 (bool).

Then in the NewProject, you can evaluate its value.

BTW I hope that the Project isn't created somewhere just with {}.

composeFilePaths: paths,
}

return &c, nil
Expand Down Expand Up @@ -340,7 +343,38 @@ func (p *Project) runDockerComposeCmd(opts dockerComposeOptions) error {
return cmd.Run()
}

func (p *Project) dockerComposeVersion() (*semver.Version, error) {

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)

var b bytes.Buffer

args := []string{
"version",
"--short",
}
if err := p.runDockerComposeCmd(dockerComposeOptions{args: args, stdout: &b}); err != nil {
return nil, errors.Wrap(err, "running Docker Compose version command failed")
}
dcVersion := b.String()
ver, err := semver.NewVersion(dcVersion)
if err != nil {
return nil, errors.Wrapf(err, "docker compose version not a valid semver %s", dcVersion)
mtojek marked this conversation as resolved.
Show resolved Hide resolved
}
return ver, 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)
if p.dcMajorVersion == "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be cleaner to keep the ContainerName function as simple as possible:

if p.dockerComposeV1 {
   return fmt.Sprintf("%s_%s_1", p.name, serviceName)
}
return fmt.Sprintf("%s-%s-1", p.name, serviceName)

Then, the logic determining p.dockerComposeV1 can be moved to the NewProject.

dockerComposeV1 can be bool, as the Compose V2 seems to deprecate V1 in the future.

ver, err := p.dockerComposeVersion()
if err != nil {
logger.Errorf("Unable to determine docker-compose version: %v. Defaulting to 1.x", err)
p.dcMajorVersion = "1"
} else {
p.dcMajorVersion = strconv.FormatInt(ver.Major(), 10)
}
}
if p.dcMajorVersion == "1" {
return fmt.Sprintf("%s_%s_1", p.name, serviceName)
}
return fmt.Sprintf("%s-%s-1", p.name, serviceName)
}