-
Notifications
You must be signed in to change notification settings - Fork 184
Multiple -f #116
Multiple -f #116
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| multiple: | ||
| image: tianon/true | ||
| environment: | ||
| - KEY1=VAL1 | ||
| simple: | ||
| image: busybox:latest | ||
| command: top | ||
| another: | ||
| image: busybox:latest | ||
| command: top |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| multiple: | ||
| image: busybox | ||
| command: echo two | ||
| environment: | ||
| - KEY2=VAL2 | ||
| yetanother: | ||
| image: busybox:latest | ||
| command: top |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,8 +24,8 @@ type Context struct { | |
| ForceRecreate bool | ||
| NoRecreate bool | ||
| Signal int | ||
| ComposeFile string | ||
| ComposeBytes []byte | ||
| ComposeFiles []string | ||
| ComposeBytes [][]byte | ||
| ProjectName string | ||
| isOpen bool | ||
| ServiceFactory ServiceFactory | ||
|
|
@@ -36,32 +36,35 @@ type Context struct { | |
| Project *Project | ||
| } | ||
|
|
||
| func (c *Context) readComposeFile() error { | ||
| func (c *Context) readComposeFiles() error { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This method feels a little bit complicated to me (too much complexity / indentation). func (c *Context) readComposeFiles() error {
if c.ComposeBytes != nil {
return nil
}
logrus.Debugf("Opening compose files: %s", strings.Join(c.ComposeFiles, ","))
// Handle STDIN (`-f -`)
if len(c.ComposeFiles) == 1 && c.ComposeFiles[0] == "-" {
composeBytes, err := ioutil.ReadAll(os.Stdin)
if err != nil {
logrus.Errorf("Failed to read compose file from stdin: %v", err)
return err
}
c.ComposeBytes = [][]byte{composeBytes}
return nil
}
for _, composeFile := range c.ComposeFiles {
composeBytes, err := ioutil.ReadFile(composeFile)
if err != nil {
if os.IsNotExist(err) && !c.IgnoreMissingConfig {
logrus.Errorf("Failed to find %s", composeFile)
return err
}
logrus.Errorf("Failed to open %s", composeFile)
return err
}
c.ComposeBytes = append(c.ComposeBytes, composeBytes)
}
return nil
}or even : func (c *Context) readComposeFiles() error {
if c.ComposeBytes != nil {
return nil
}
logrus.Debugf("Opening compose files: %s", strings.Join(c.ComposeFiles, ","))
// Handle STDIN (`-f -`)
if len(c.ComposeFiles) == 1 && c.ComposeFiles[0] == "-" {
composeBytes, err := readComposeFile(os.Stdin)
if err != nil {
return err
}
c.ComposeBytes = [][]byte{composeBytes}
return nil
}
for _, composeFile := range c.ComposeFiles {
composeFileF, err := os.Open(composeFile)
if os.IsNotExist(err) && !c.IgnoreMissingConfig {
logrus.Errorf("Failed to find %s", composeFile)
return err
}
composeBytes, err := readComposeFile(composeFileF)
if err != nil {
return err
}
c.ComposeBytes = append(c.ComposeBytes, composeBytes)
}
return nil
}
func readComposeFile(reader io.Reader) ([]byte, error) {
composeBytes, err := ioutil.ReadAll(reader)
if err != nil {
logrus.Errorf("Failed to read compose file : %v", err)
return nil, err
}
return composeBytes, nil
}After looking at both, I prefer the first one 😺. |
||
| if c.ComposeBytes != nil { | ||
| return nil | ||
| } | ||
|
|
||
| logrus.Debugf("Opening compose file: %s", c.ComposeFile) | ||
| logrus.Debugf("Opening compose files: %s", strings.Join(c.ComposeFiles, ",")) | ||
|
|
||
| if c.ComposeFile == "-" { | ||
| if len(c.ComposeFiles) == 1 && c.ComposeFiles[0] == "-" { | ||
| composeBytes, err := ioutil.ReadAll(os.Stdin) | ||
| if err != nil { | ||
| logrus.Errorf("Failed to read compose file from stdin: %v", err) | ||
| return err | ||
| } | ||
| c.ComposeBytes = composeBytes | ||
| } else if c.ComposeFile != "" { | ||
| if composeBytes, err := ioutil.ReadFile(c.ComposeFile); os.IsNotExist(err) { | ||
| if c.IgnoreMissingConfig { | ||
| return nil | ||
| c.ComposeBytes = [][]byte{composeBytes} | ||
| } else { | ||
| for _, composeFile := range c.ComposeFiles { | ||
| if composeFile != "" { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure we should do this check, $ libcompose-cli -f ../ports-composefile/docker-compose.yml -f "" up -d
WARN[0000] Note: This is an experimental alternate implementation of the Compose CLI (https://github.com/docker/compose)
INFO[0000] Project [ports-composefile]: Starting project
INFO[0000] [0/1] [simple]: Starting
INFO[0000] [1/1] [simple]: Started
$ docker-compose -f ../ports-composefile/docker-compose.yml -f "" up -d
ERROR: .IOError: [Errno 21] Is a directory: u'./' |
||
| if composeBytes, err := ioutil.ReadFile(composeFile); os.IsNotExist(err) { | ||
| if !c.IgnoreMissingConfig { | ||
| logrus.Errorf("Failed to find %s", composeFile) | ||
| return err | ||
| } | ||
| } else if err != nil { | ||
| logrus.Errorf("Failed to open %s", composeFile) | ||
| return err | ||
| } else { | ||
| c.ComposeBytes = append(c.ComposeBytes, composeBytes) | ||
| } | ||
| } | ||
| logrus.Errorf("Failed to find %s", c.ComposeFile) | ||
| return err | ||
| } else if err != nil { | ||
| logrus.Errorf("Failed to open %s", c.ComposeFile) | ||
| return err | ||
| } else { | ||
| c.ComposeBytes = composeBytes | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -96,9 +99,14 @@ func (c *Context) lookupProjectName() (string, error) { | |
| return envProject, nil | ||
| } | ||
|
|
||
| f, err := filepath.Abs(c.ComposeFile) | ||
| file := "" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would use |
||
| if len(c.ComposeFiles) > 0 { | ||
| file = c.ComposeFiles[0] | ||
| } | ||
|
|
||
| f, err := filepath.Abs(file) | ||
| if err != nil { | ||
| logrus.Errorf("Failed to get absolute directory for: %s", c.ComposeFile) | ||
| logrus.Errorf("Failed to get absolute directory for: %s", file) | ||
| return "", err | ||
| } | ||
|
|
||
|
|
@@ -123,7 +131,7 @@ func (c *Context) open() error { | |
| return nil | ||
| } | ||
|
|
||
| if err := c.readComposeFile(); err != nil { | ||
| if err := c.readComposeFiles(); err != nil { | ||
| return err | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,7 +31,7 @@ var ( | |
| type rawService map[string]interface{} | ||
| type rawServiceMap map[string]rawService | ||
|
|
||
| func mergeProject(p *Project, bytes []byte) (map[string]*ServiceConfig, error) { | ||
| func mergeProject(p *Project, file string, bytes []byte) (map[string]*ServiceConfig, error) { | ||
| configs := make(map[string]*ServiceConfig) | ||
|
|
||
| datas := make(rawServiceMap) | ||
|
|
@@ -44,20 +44,31 @@ func mergeProject(p *Project, bytes []byte) (map[string]*ServiceConfig, error) { | |
| } | ||
|
|
||
| for name, data := range datas { | ||
| data, err := parse(p.context.ConfigLookup, p.context.EnvironmentLookup, p.File, data, datas) | ||
| data, err := parse(p.context.ConfigLookup, p.context.EnvironmentLookup, file, data, datas) | ||
|
|
||
| if err != nil { | ||
| logrus.Errorf("Failed to parse service %s: %v", name, err) | ||
| return nil, err | ||
| } | ||
|
|
||
| datas[name] = data | ||
| if _, ok := p.Configs[name]; ok { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if _, ok := p.Configs[name]; ok {
var rawExistingService rawService
if err := utils.Convert(p.Configs[name], &rawExistingService); err != nil {
return nil, err
}
data = mergeConfig(rawExistingService, data)
}
datas[name] = data |
||
| var rawExistingService rawService | ||
| if err := utils.Convert(p.Configs[name], &rawExistingService); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| datas[name] = mergeConfig(rawExistingService, data) | ||
| } else { | ||
| datas[name] = data | ||
| } | ||
| } | ||
|
|
||
| if err := utils.Convert(datas, &configs); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| adjustValues(configs) | ||
|
|
||
| return configs, nil | ||
| } | ||
|
|
||
|
|
@@ -237,6 +248,14 @@ func parse(configLookup ConfigLookup, environmentLookup EnvironmentLookup, inFil | |
| } | ||
| } | ||
|
|
||
| baseService = mergeConfig(baseService, serviceData) | ||
|
|
||
| logrus.Debugf("Merged result %#v", baseService) | ||
|
|
||
| return baseService, nil | ||
| } | ||
|
|
||
| func mergeConfig(baseService, serviceData rawService) rawService { | ||
| for k, v := range serviceData { | ||
| // Image and build are mutually exclusive in merge | ||
| if k == "image" { | ||
|
|
@@ -252,9 +271,7 @@ func parse(configLookup ConfigLookup, environmentLookup EnvironmentLookup, inFil | |
| } | ||
| } | ||
|
|
||
| logrus.Debugf("Merged result %#v", baseService) | ||
|
|
||
| return baseService, nil | ||
| return baseService | ||
| } | ||
|
|
||
| func merge(existing, value interface{}) interface{} { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -63,14 +63,24 @@ func (p *Project) Parse() error { | |
|
|
||
| p.Name = p.context.ProjectName | ||
|
|
||
| if p.context.ComposeFile == "-" { | ||
| p.File = "." | ||
| if len(p.context.ComposeFiles) == 1 && p.context.ComposeFiles[0] == "-" { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. p.Files = p.context.ComposeFiles
if len(p.Files) == 1 && p.Files[0] == "-" {
p.Files = []string{"."}
} |
||
| p.Files = []string{"."} | ||
| } else { | ||
| p.File = p.context.ComposeFile | ||
| p.Files = p.context.ComposeFiles | ||
| } | ||
|
|
||
| if p.context.ComposeBytes != nil { | ||
| return p.Load(p.context.ComposeBytes) | ||
| for i, composeBytes := range p.context.ComposeBytes { | ||
| if i < len(p.context.ComposeFiles) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about : file := ""
if i < len(p.context.ComposeFiles) {
file = p.Files[i]
}
if err := p.load(file, composeBytes); err != nil {
return err
}But then, I would refactor the |
||
| if err := p.load(p.Files[i], composeBytes); err != nil { | ||
| return err | ||
| } | ||
| } else { | ||
| if err := p.Load(composeBytes); err != nil { | ||
| return err | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
|
|
@@ -123,8 +133,12 @@ func (p *Project) AddConfig(name string, config *ServiceConfig) error { | |
| // Load loads the specified byte array (the composefile content) and adds the | ||
| // service configuration to the project. | ||
| func (p *Project) Load(bytes []byte) error { | ||
| return p.load("", bytes) | ||
| } | ||
|
|
||
| func (p *Project) load(file string, bytes []byte) error { | ||
| configs := make(map[string]*ServiceConfig) | ||
| configs, err := mergeProject(p, bytes) | ||
| configs, err := mergeProject(p, file, bytes) | ||
| if err != nil { | ||
| log.Errorf("Could not parse config for project %s : %v", p.Name, err) | ||
| return err | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would have written like this (because I don't like
else's 😝), but not sure if it's better or not...