Skip to content
This repository was archived by the owner on Jul 18, 2025. It is now read-only.

Multiple -f#116

Closed
joshwget wants to merge 4 commits into
docker-archive-public:masterfrom
joshwget:multiple-alternate-files
Closed

Multiple -f#116
joshwget wants to merge 4 commits into
docker-archive-public:masterfrom
joshwget:multiple-alternate-files

Conversation

@joshwget
Copy link
Copy Markdown
Contributor

@joshwget joshwget commented Dec 3, 2015

Fixes #98. This changes some properties of Context (string ComposeFile to []string ComposeFiles and []byte ComposeBytes to [][]byte ComposeBytes) as well as Project (string File to []string Files).

Signed-off-by: Josh Curl hello@joshcurl.com

Signed-off-by: Josh Curl <hello@joshcurl.com>
@dnephin
Copy link
Copy Markdown
Contributor

dnephin commented Dec 3, 2015

I don't think this handles merge the same way as compose. For one, it needs to merge extends, links and volumes_from, which is not done by `extends.

@joshwget
Copy link
Copy Markdown
Contributor Author

joshwget commented Dec 4, 2015

@dnephin If links and volume_from are merged with multiple files the same way as when using extends, I think this will support those. It definitely doesn't merge extends though. I'll address this.

@ibuildthecloud
Copy link
Copy Markdown
Contributor

@joshwget I like the approach, took me a bit to notice you are storing [][]byte.

@dnephin Is the merging behavior of extends documented? From what I understand of this PR is if you do -f a.yml -f b.yml it will evaluate the extends in a.yml and b.yml before merging. That seems like the correct behavior to me, but I'm not sure what docker-compose is actually doing.

@dnephin
Copy link
Copy Markdown
Contributor

dnephin commented Dec 18, 2015

That sounds right

@joshwget
Copy link
Copy Markdown
Contributor Author

@dnephin I think that addresses all of your original comments then, but please let me know if I missed something!

Signed-off-by: Josh Curl <hello@joshcurl.com>
@vdemeester
Copy link
Copy Markdown
Contributor

@joshwget According to the documentaiton compose looks by default for docker-compose.yml and an optionnal docker-compose.override.yml.

Signed-off-by: Josh Curl <hello@joshcurl.com>
@joshwget
Copy link
Copy Markdown
Contributor Author

Thanks @vdemeester, should be addressed now!

@vdemeester
Copy link
Copy Markdown
Contributor

Hum… doesn't seem to work as expected, I'm using the test/fixtures/multiple-composefiles files from the docker/compose project :

# With docker-compose
$ docker-compose -f docker-compose.yml -f compose2.yml up -d
Creating multiplecomposefiles_simple_1
Creating multiplecomposefiles_yetanother_1
Creating multiplecomposefiles_another_1
$ docker-compose -f docker-compose.yml -f compose2.yml ps
Name                  Command   State   Ports 
-----------------------------------------------------------
multiplecomposefiles_another_1      top       Up            
multiplecomposefiles_simple_1       top       Up            
multiplecomposefiles_yetanother_1   top       Up  

# With libcompose-cli on this PR
$ libcompose-cli -f docker-compose.yml -f compose2.yml up -d
WARN[0000] Note: This is an experimental alternate implementation of the Compose CLI (https://github.com/docker/compose) 
INFO[0000] Project [multiple-composefiles]: Starting project  
INFO[0000] [0/1] [yetanother]: Starting                 
INFO[0000] [1/1] [yetanother]: Started  
$ libcompose-cli -f docker-compose.yml -f compose2.yml ps 
WARN[0000] Note: This is an experimental alternate implementation of the Compose CLI (https://github.com/docker/compose) 
Name                                Command  State          Ports
multiple-composefiles_yetanother_1  top      Up 29 seconds  

It only picked that latest one and did no merge 😅.

Signed-off-by: Josh Curl <hello@joshcurl.com>
@joshwget
Copy link
Copy Markdown
Contributor Author

@vdemeester When I try reproducing your example I get results similar to Compose. Would you mind checking this again? 😄

@vdemeester
Copy link
Copy Markdown
Contributor

@joshwget you are right, I might have messed up, it's working as expected 😝 ; with docker-compose.override.conf and such.

Comment thread cli/command/command.go
Copy link
Copy Markdown
Contributor

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...

    context.ComposeFiles = c.GlobalStringSlice("file")

    if len(context.ComposeFiles) == 0 {
        context.ComposeFiles = []string{"docker-compose.yml"}
        if _, err := os.Stat("docker-compose.override.yml"); err == nil {
            context.ComposeFiles = append(context.ComposeFiles, "docker-compose.override.yml")
        }
    }

Comment thread project/context.go
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not sure we should do this check, docker-compose will fail if we manage to specify an empty compose file :

$ 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'./'

@vdemeester
Copy link
Copy Markdown
Contributor

@joshwget So far, so good 😉. Few commands on code but it almost work as expected, that's cool 👍 😉.

I would add more integration test though :

  • Using the default docker-compose.yml and docker-compose.override.yml
  • Pointing to one or several files without specifying the project name and verifying that the project name is named based on the first folder.

@vdemeester
Copy link
Copy Markdown
Contributor

/ping @joshwget

@vdemeester
Copy link
Copy Markdown
Contributor

Carrying into #140, thanks for the hard work @joshwget

@vdemeester vdemeester closed this Jan 16, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants