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.

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

extends – Inherit services from other services #1088

Merged
merged 2 commits into from
Mar 20, 2015
Merged

Conversation

aanand
Copy link

@aanand aanand commented Mar 10, 2015

Here's a first pass at the feature also known as "import", "include" and "copy", most recently proposed in #988 and spiked out in #1080.

I feel like the example in the docs should be simpler. Perhaps there should be a new documentation page with a fuller explanation of the various intricacies (overriding individual env vars, 3-file example, not inheriting links or volumes_from), and a more concise explanation here.

Also, maybe ports should be included in the "not inherited" blacklist.

TODO:

@bfirsh
Copy link

bfirsh commented Mar 10, 2015

Nice! Looks great.

@bfirsh
Copy link

bfirsh commented Mar 10, 2015

If we want a simpler example, we could have a development.yml which is extended by production.yml.

I think there should be separate guides for setting up different environments with extends, and how to split up your configuration into multiple files. Perhaps the reference could just link off to those pages.

image: postgres
```

Here, the `web` service in **development.yml** inherits the configuration of

Choose a reason for hiding this comment

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

I'm not sure if the Here refers to the example above, or below - need a stronger way to delineate the 2 examples.

@aanand aanand changed the title First pass at docs for extends WIP: extends Mar 12, 2015
@aanand
Copy link
Author

aanand commented Mar 12, 2015

Update: done a basic implementation. TODO in the description.

@aanand
Copy link
Author

aanand commented Mar 12, 2015

Also, this now sits atop #1099.

@funkyfuture
Copy link

the config-layout looks reasonable to me.

still, i think it's important to clarify the possibilities what can be used in the file-key. as this is much about reusing patterns, imo absolute paths or a fallback-path will be requested rather sooner than later.

@dnephin
Copy link

dnephin commented Mar 13, 2015

It seems kind of arbitrary that links and volumes_from are not copied over. What about net, isn't that exactly like links (or could be if the net is container:...) ?

I think this would be a lot more obvious to developers if it raised an error if the service you are trying to extend has any of these options, instead of copying over half the definition.

A developer would still be able to have a similar setup, they'd just be forced to create some extra "base" services that exclude the links/net/volumes.

@aanand
Copy link
Author

aanand commented Mar 13, 2015

@dnephin net:container should be blacklisted too, yes.

I can see two sensible approaches with links/volumes_from/net:container:

  1. Refuse to extend a service that has any of them defined.
  2. Refuse to extend a service that has any of them defined, unless the user provides overrides for those specific entries.

Looking at it now, (1) seems better for multiple reasons:

  • It's easier to implement.
  • It's easier to explain.
  • It's low-commitment: we can change it based on feedback later, and no-one's configuration will break.

@aanand
Copy link
Author

aanand commented Mar 13, 2015

OK, I've blacklisted links and volumes_from. net:container is waiting on #1076.

@bfirsh bfirsh changed the title WIP: extends WIP: extends – Inherit services from other services Mar 13, 2015
@aanand
Copy link
Author

aanand commented Mar 13, 2015

OK, #1076 is merged and net:container is now blacklisted. This PR is now ready for review, though #1099 should be merged first.

@aanand aanand changed the title WIP: extends – Inherit services from other services extends – Inherit services from other services Mar 13, 2015
@aanand aanand added this to the 1.2.0 milestone Mar 13, 2015
@bfirsh
Copy link

bfirsh commented Mar 13, 2015

Looks great. Two things:

  • Docs say that links and volumes_from aren't inherited, but it fails hard. The docs should probably explain the recommended way to get round this (create some "base" services).
  • Referencing the current file or a circular reference blows up with a stack overflow. Should be pretty easy to detect circular imports.

An aside: I'm not 100% convinced by having "base" services. I quite like the idea of having the default config inside the GitHub repo, then some downstream deployment system could overlay a production configuration on top without imposing that a base config exists. These are kinks we can smooth out over time though...

@dnephin
Copy link

dnephin commented Mar 13, 2015

I quite like the idea of having the default config inside the GitHub repo, then some downstream deployment system could overlay...

@bfirsh that's fair. I think any time you try to "extend" a service definition that isn't in the current repo, it's more likely you actually want to pull in links and volumes_from, so it would be a different type (this would allow support for the feature described in #318).

My understanding (based on #988) is that there will be support for doing this eventually, but we're addressing the easier case of "local includes" first (which is a bit of a different use case).

It would be great to have a name to refer to these "external project includes".

@aanand
Copy link
Author

aanand commented Mar 18, 2015

Docs say that links and volumes_from aren't inherited, but it fails hard. The docs should probably explain the recommended way to get round this (create some "base" services).

Agreed - created #1127.

Referencing the current file or a circular reference blows up with a stack overflow. Should be pretty easy to detect circular imports.

Circular import detection now pushed.

@aanand
Copy link
Author

aanand commented Mar 19, 2015

It was easier to implement #1109 on top of these changes, rather than independently, so I've done that.

@aanand
Copy link
Author

aanand commented Mar 19, 2015

This is ready to go, in my opinion. ping @dnephin

Signed-off-by: Aanand Prasad <aanand.prasad@gmail.com>
Signed-off-by: Aanand Prasad <aanand.prasad@gmail.com>
@aanand
Copy link
Author

aanand commented Mar 20, 2015

extend-a-step-5

aanand added a commit that referenced this pull request Mar 20, 2015
`extends` – Inherit services from other services
@aanand aanand merged commit 49c2080 into docker:master Mar 20, 2015
@aanand aanand deleted the extends branch March 20, 2015 23:00
yuval-k pushed a commit to yuval-k/compose that referenced this pull request Apr 10, 2015
`extends` – Inherit services from other services
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
dnephin added a commit that referenced this pull request Apr 27, 2015
modified the release notes section the first[PR #972]to[PR #1088]
@iafilatov
Copy link

It's definitely good to be able to extend services defined elsewhere. One thing I don't understand is, why is it prohibited to extend services that have links and volumes_from defined? If only to ensure "dependencies between services are clearly visible when reading the current file", then I don't think it's a good enough reason. And I don't think these options have anything to do with local configuration. "Local" is something that reflects basically the host where Docker is installed. It could be ports or directories which I want the containers to bind to, but not links within the project. What's more if, if I want to extend a service within a project, I don't want to care how the containers are linked, and being obliged to cut bits of yml out of base service definitions seems really weird. Consider an example:

# base.yml
app:
  image: myimages/app
  links:
  - db
  ports:
  - "8000:80"
  volumes:
  - /var/www/app   # or even in Dockerfile 

db:
  image: myimages/mydb

A simple project with an app and a db container. It should work anywhere there's Docker. If myimages is accessible, then base.yml is all you really need. Which is great. Now imagine you starts working on this project. You fetch the yml but you've got something other on port 8000, so the natural thing would be to extend the app service like this to change the port:

# local.yml
app:
  extends: base.yml
  service: app
ports:
- "8080:80"

Which obviously fails. Because app is linked to something (or gets volumes from somewhere...). The "solution" would be to pull links and volumes_from out of base.yml and put them into local.yml. But... why is it necessary to fiddle with the base project definition to change the port that Docker binds to on your personal machine?

The recommended approach seems to be to create base definitions for services and the write top-level ymls with links/volumes_from for every environment you have. But these options alone can lead to lots of duplication in complex projects. Besides, once you define any of them, the service becomes inextendable, and every time you need to do it, you have to pull them out to the top. Whoever does this will have to know more about services and containers and stuff that they really want or need to.

I believe there should be no limitations as to which containers can be extended. Scalars should be overridden, lists should be merged. But besides that, it should be up to users how and what to extend.

@aanm
Copy link

aanm commented May 2, 2015

@iafilatov See the #1380 it might fit what you want

@iafilatov
Copy link

Mostly yes, thanks.

@vdemeester vdemeester mentioned this pull request May 11, 2015
@hoIIer
Copy link

hoIIer commented May 30, 2015

hello, how can I approach a case where I have multiple services which are each essentially isolated (microservice architecture w/REST communication between services), but I want a master compose repo/file that controls orchestrating/building the entire platform? Right now using the current extends syntax doesn't work bc the individual services all have their own links (databases/cache/etc)

Any info appreciated, thanks!

@pikeas
Copy link

pikeas commented Jun 4, 2015

+1, I have a similar use case as @erichonkanen.

Specifically, I have server and client directories, each with their own Dockerfile and docker-compose.yml. The client compose file has build (gulp/grunt) and run (nginx) containers, which are independent. The server compose file has app (django/rails/node) and database containers, which are linked.

I would like to have a project-level (the directory above server and client) compose file which brings up everything. This is currently impossible because extending services with links is disallowed.

It seems reasonable to support links, in the following way: if you extend a service with links (or volumes_from), this should be allowed if and only you define a service of the same name (which can either also be an extension, or be created from scratch).

@vnwhlr
Copy link

vnwhlr commented Jun 25, 2015

+1’ing this, also, too.
We have a “production” docker-compose.yml that copies the source code into our containers for deployment to ECS. However, we extend that with a “development” docker-compose.yml definition that mounts the source code from our local development environment as a volume onto the container, thus overwriting it.
This restriction means that if we want to use links, we’d have to shuffle the links back and forth between those docker-compose.ymls, and that’s just icky.
Also +1’ing @pikeas' suggestion - correct me if I’m wrong, but if you needed to link to a service in the parent, all you'd need to do is to define that same service and use extends. Also, you’re referencing the “parent” docker-compose.yml anyways, so there isn’t really anything implicit about it.

@hoIIer
Copy link

hoIIer commented Jun 25, 2015

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants