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

Overriding the set of values when extending the "ports" option. #2260

Closed
ghost opened this issue Oct 27, 2015 · 60 comments
Closed

Overriding the set of values when extending the "ports" option. #2260

ghost opened this issue Oct 27, 2015 · 60 comments

Comments

@ghost
Copy link

ghost commented Oct 27, 2015

The default behavior when extending a service or overriding a whole docker-compose.yml file is to concatenate the sets of values on the multi-value options: ports, expose, external_links, dns and dns_search.

The problem appears when for example you have two production hosts both serving on the port 80 and a single machine for development, there is no way of extending or overriding the services for a development environment that avoids the port collision on the host.

An example: lets assume there are two servers one is a REST API and the other is a webpage that uses the API, both listen on port 80 on the production host

  • compose.yml on host A:
restserver:
  image: node
  ports:
    - "80:80"
  • compose.yml on host B:
webserver:
  image: django
  ports:
    - "80:80"

On a development environment where you want to test this setup you would define:

  • compose.override.yml on dev for A:
restserver:
  ports:
    - "127.0.0.1:3030:80"
  • compose.override.yml on dev for B:
webserver:
  external_links:
    - "service_restserver_1:api.restserver.domain.com"
  ports:
    - "127.0.0.1:8080:80"

Of course the "ports" option is concatenated with the production and there is no way of running both containers on the same host because both try to bind to port 80.

Is there any workaround or yml specific syntax for this use case?

@dnephin
Copy link

dnephin commented Oct 27, 2015

I would do something like this:

docker-compose.yml (no ports)

restserver:
  image: node

webserver:
  image: django

For dev you can setup some host ports

docker-compose.override.yml

restserver:
  ports: ['3030:80']

webserver:
  ports: ['8080:80']

and for prod you can set them to a different port:

docker-compose.prod.yml

restserver:
  ports: ['80:80']

webserver:
  ports: ['80:80']

In prod you would run docker-compose -f docker-compose.yml -f docker-compose.prod.yml up

@ghost
Copy link
Author

ghost commented Oct 27, 2015

It seems like a good, clean approach. Only cons would be the verbose command on production but that one is automatic.

Thanks @dnephin

@gregmartyn
Copy link

Another downside is that it no longer "just works". The user has to either create an override file or specify ports manually.

It would be nice if the override file could actually override array values like it does for scalar values. Instead, the override file appends array values and only actually overrides scalar values.

@mkurzeja
Copy link

mkurzeja commented May 5, 2016

I agree with @gregmartyn . A feature that would allow to override ports array instead of merging them would be awesome.

In my case we use it to override a port when one of the developers is not able to work on the default one.

Maybe docker-compose could just override the ports that are already forwarded: If port 80 is specified in both files, it uses just the one from docker-compose.override.yml?

@udhayakumaran
Copy link

udhayakumaran commented May 9, 2016

@gregmartyn @mkurzeja Docker is already having that feature. I tried that to override with simple docker-compose, it is working.

For example,

docker-compose.yml with content,

my-httpd:
  image: httpd:latest
  ports:
    - "1110:80"  

And docker-compose.override.yml with content,

my-httpd:
  image: httpd:2.4

This is the docker ps info,

12286c55b76b httpd:2.4 "httpd-foreground" 5 seconds ago Up 4 seconds 0.0.0.0:1110->80/tcp dockercomposetest_my-httpd_1

It is taking override's image and port from the compose file.

@gregmartyn
Copy link

@udhayakumaran see above. "the override file appends array values and only actually overrides scalar values."

The image in your example is a scalar value and did get overridden. The port you specified is part of an array and was not overridden. You didn't specify any ports in your override file, so they were not overridden. Had you specified ports in the override file, they would have been combined with the port specified in the main config file.

@udhayakumaran
Copy link

udhayakumaran commented May 9, 2016

@gregmartyn Okay, I understand now. Thanks for correcting me..

@datacarl
Copy link

datacarl commented May 23, 2016

@gregmartyn If you are using the .env file introduced in 1.7 you can define the compose files there. You'll have to ensure you have the correct .env file (a prod version for prod, dev version for dev) ofc but that's the only manual change required.

For dev environments you can set
COMPOSE_FILE=docker-compose.yml:docker-compose.dev.yml
and for production
COMPOSE_FILE=docker-compose.yml:docker-compose.prod.yml

Together with what @dnephin suggested it works fine.

@kevin-cantwell
Copy link

@datacarl Do you have a link to documentation on the DOCKER_FILE env var? It's not clear to me how it's used by Docker Compose.

@datacarl
Copy link

datacarl commented Jun 7, 2016

@kevin-cantwell https://docs.docker.com/compose/env-file/

(should be COMPOSE_FILE, my bad. Edited my reply above)

@kevin-cantwell
Copy link

@datacarl Ah, COMPOSE_FILE. Found it now, thanks: https://docs.docker.com/compose/reference/envvars/#compose-file

CarstenHoyer pushed a commit to CarstenHoyer/compose that referenced this issue Sep 12, 2016
CarstenHoyer pushed a commit to CarstenHoyer/compose that referenced this issue Sep 12, 2016
CarstenHoyer pushed a commit to CarstenHoyer/compose that referenced this issue Sep 12, 2016
  - Relates to docker#2260

Signed-off-by: CarstenHoyer <[email protected]>
@CarstenHoyer
Copy link

I have added a preliminary pull request, that introduces a way to add an overwrite strategy other than the default append/union. The overwrite config currently only supports "ports" but it would be a smallish thing to support the other array options too.

Your compose file would then look like either:

version: "2"
services:
  web:
    override_strategy: overwrite

or

version: "2"
services:
  web:
    overwrite:
      - ports

And the result could be that if docker-compose.yml has ports: 80, 443, and your override file has 8080 and 8443 fx, then only the 8080 and 8443 ports would be in the final config.

CarstenHoyer pushed a commit to CarstenHoyer/compose that referenced this issue Sep 13, 2016
CarstenHoyer pushed a commit to CarstenHoyer/compose that referenced this issue Sep 13, 2016
CarstenHoyer pushed a commit to CarstenHoyer/compose that referenced this issue Sep 13, 2016
CarstenHoyer pushed a commit to CarstenHoyer/compose that referenced this issue Sep 16, 2016
CarstenHoyer added a commit to CarstenHoyer/compose that referenced this issue Sep 17, 2016
dliappis added a commit to elastic/elasticsearch-docker that referenced this issue Oct 25, 2016
Running the test targets (single-node-test, cluster-unicast-test) can
sometimes lead to CI failures:
https://devops-ci.elastic.co/view/Docker/job/elastic+elasticsearch-docker+5.0/21/console. This
can happen when both targets are run simultaneously due to a conflict on
the locally exposed port :9200.

Remove local port references for ES in docker-compose.yml. This change
doesn't affect the tests as the test container accesses ES through the
docker network driver.

Retain backwards compatibility for run-es(single|cluster) targets, by
overriding the localhost port definitions with
docker-compose.hostports.yml. This strategy is described in:
docker/compose#2260 (comment)
dliappis added a commit to elastic/elasticsearch-docker that referenced this issue Oct 25, 2016
Running the test targets (single-node-test, cluster-unicast-test) can
sometimes lead to CI failures:
https://devops-ci.elastic.co/view/Docker/job/elastic+elasticsearch-docker+5.0/21/console. This
can happen when both targets are run simultaneously due to a conflict on
the locally exposed port :9200.

Remove local port references for ES in docker-compose.yml. This change
doesn't affect the tests as the test container accesses ES through the
docker network driver.

Retain backwards compatibility for run-es(single|cluster) targets, by
overriding the localhost port definitions with
docker-compose.hostports.yml. This strategy is described in:
docker/compose#2260 (comment)
dliappis added a commit to elastic/elasticsearch-docker that referenced this issue Oct 25, 2016
Running the test targets (single-node-test, cluster-unicast-test) can
sometimes lead to CI failures:
https://devops-ci.elastic.co/view/Docker/job/elastic+elasticsearch-docker+5.0/21/console. This
can happen when both targets are run simultaneously due to a conflict on
the locally exposed port :9200.

Remove local port references for ES in docker-compose.yml. This change
doesn't affect the tests as the test container accesses ES through the
docker network driver.

Retain backwards compatibility for run-es(single|cluster) targets, by
overriding the localhost port definitions with
docker-compose.hostports.yml. This strategy is described in:
docker/compose#2260 (comment)
@dimaqq
Copy link

dimaqq commented Apr 13, 2017

@CarstenHoyer did your overwrite: change make it to any release?

@CarstenHoyer
Copy link

@dimaqq - I am afraid not. I have not kept the PR up to date with the upstream, and it also doesnt contain any automated tests.

Since I never got any official feedback, I figured it was not in scope.

@keithy
Copy link

keithy commented May 3, 2019 via email

@nyetwurk
Copy link

nyetwurk commented May 3, 2019

Does anybody else see why I am suggesting a dictionary rather than an array?

@mike-code
Copy link

I do but the compose project looks semi abandoned. I wouldn't recommend it using in production.

@im-n1
Copy link

im-n1 commented May 6, 2019

@mike-code thats nonsense this project is quite active.

@vectorjohn
Copy link

I don't think the dictionary suggestion is a good one, docker-compose shouldn't have some new half baked scheme for merging files when YAML already has a standard that does all that is needed (other than @keithy 's use case, which sounds so complex it's out of scope).

@nyetwurk
Copy link

nyetwurk commented May 6, 2019

Portmapping is fundamentally a dictionary, not an array, and merging comes for free.

@im-n1
Copy link

im-n1 commented May 6, 2019

@vectorjohn is there any official yaml lib that can do the merging?

@keithy
Copy link

keithy commented May 6, 2019

My use case and solution is very simple actually:

docker-compose allows you to supply .env as an environment file. The environment variable COMPOSE_FILE provides a list of compose_files to be assembled.

In those individual files I place environment variables where there are ports mentioned, so that the .env file can supply those values also.

The missing piece of the puzzle, as with so much of the docker stuff, is to make the .env file readable. I do this by creating the .env file from a readable version using an all-to-simple pre-processor docker-compose-use <my.env>

@vectorjohn
Copy link

@vectorjohn is there any official yaml lib that can do the merging?

Oddly enough, docker-compose already supports this it looks like:
https://medium.com/@kinghuang/docker-compose-anchors-aliases-extensions-a1e4105d70bd

nl-brett-stime added a commit to nl-brett-stime/compose that referenced this issue Oct 1, 2019
This is a proposed fix for docker#2260 which has remained an issue for many users even though it was closed almost immediately after being opened because a partial work-around exists.

My intent for this change is to allow the author of a docker-compose.yml file to specify a default port mapping that can have the external/host-side mappings be overridden.

E.g., the author of a particular file may specify that the container/guest port `tcp/8000` be mapped to the sam external/host port: `tcp/8000`. This works well for most users except some users may already have port 8000 in use on their workstations. In those cases, the user should be able to specify something like `ports: ['8001:8000/tcp']` in docker-compose.override.yml to replace the predefined default on their system and avoid a 'port is already allocated' binding error.

From what I can tell, the existing merge_field only allows the port's `mode` attribute to be overridden. This change allows the `published` and `external_ip` attributes to be overridden. Since the container/guest's `protocol` and `target` are the two attributes that are given/fixed/predefined/immutable based on the image being loaded, they seem like a more natural merge key. After this change, if overriders want only the `mode` attribute to change, they can simply reiterate/restate the `published` port and `external_ip` defined in the base file.

This change is technically backwards-breaking. Although I don't expect it will negatively impact the vast majority of users, a more comprehensive change could be to add another property to the `ports` specification/schema in compose files called e.g., `merge_strategy`. Then ServicePort.merge_field could be redefined as follows:

```
if self.merge_strategy == 'internal_only':
    return (self.protocol, self.target)
elif self.merge_strategy == 'include_published_and_external_ip':
    return (self.target, self.published, self.external_ip, self.protocol)
else:
    return (self.target, self.published, self.external_ip, self.protocol)
```
 
Assuming that's the route we take, I'd add a deprecation warning to the release notes saying that the `include_published_and_external_ip` will stop being the default and that future versions will default to `merge_strategy == 'internal_only'`.
nl-brett-stime added a commit to nl-brett-stime/compose that referenced this issue Oct 1, 2019
This is a proposed fix for docker#2260 which has remained an issue for many users even though it was closed almost immediately after being opened because a partial work-around exists.

My intent for this change is to allow the author of a docker-compose.yml file to specify a default port mapping that can have the external/host-side mappings be overridden.

E.g., the author of a particular file may specify that the container/guest port `tcp/8000` be mapped to the sam external/host port: `tcp/8000`. This works well for most users except some users may already have port 8000 in use on their workstations. In those cases, the user should be able to specify something like `ports: ['8001:8000/tcp']` in docker-compose.override.yml to replace the predefined default on their system and avoid a 'port is already allocated' binding error.

From what I can tell, the existing merge_field only allows the port's `mode` attribute to be overridden. This change allows the `published` and `external_ip` attributes to be overridden. Since the container/guest's `protocol` and `target` are the two attributes that are given/fixed/predefined/immutable based on the image being loaded, they seem like a more natural merge key. After this change, if overriders want only the `mode` attribute to change, they can simply reiterate/restate the `published` port and `external_ip` defined in the base file.

This change is technically backwards-breaking. Although I don't expect it will negatively impact the vast majority of users, a more comprehensive change could be to add another property to the `ports` specification/schema in compose files called e.g., `merge_strategy`. Then ServicePort.merge_field could be redefined as follows:

```
if self.merge_strategy == 'internal_only':
    return (self.protocol, self.target)
elif self.merge_strategy == 'include_published_and_external_ip':
    return (self.target, self.published, self.external_ip, self.protocol)
else:
    return (self.target, self.published, self.external_ip, self.protocol)
```

Assuming that's the route we take, I'd add a deprecation warning to the release notes saying that the `include_published_and_external_ip` will stop being the default and that future versions will default to `merge_strategy == 'internal_only'`.

Signed-off-by: Brett Stime <[email protected]>
@typoworx-de
Copy link

typoworx-de commented Nov 13, 2019

How does the override feature work for version: '3.6' in yaml head? Trying to override some settings in DDEV-Project (https://github.com/drud/ddev) also using docker-compose (version 1.24.1 which seems to be latest release already).

Adding override_strategy: overwrite won't work and breaks docker-compose:

Unsupported config option for services.web: 'override_strategy'

@mcabrams
Copy link

@typoworx-de That was a proposed solution with a corresponding pull request that did not get merged. It is not available in version 3.6, or any other version for that matter.

@noahtallen
Copy link

One reason this is very important (didn't see it mentioned yet) is that we don't always have access to change the base docker-compose.yml file. In fact, I'm using a dockerfile provided by a different service that I cannot control. I probably ought not to update that file since it is in the source control of that other service.

If I was able to create env files and remove ports from the base config, then sure, I'd accept that there's a workaround. But you aren't always in control of the base config file, and modifying it directly generally isn't the best idea.

@nyetwurk
Copy link

nyetwurk commented Jan 22, 2020

I'm also unclear how you could use yaml array merge to remove an entry from the list, whereas this is trivial if it is a dict

@ebuildy
Copy link

ebuildy commented Apr 26, 2020

What about adding a excludes option in extends clause?

service_A:
   extends:
     file: docker-compose.yaml
     service: service_A
     excludes: ["image", "ports"]
   build: .

@kkom
Copy link

kkom commented May 18, 2020

@ebuildy - looks quite promising to me, but note that extends is not available in Compose file versions 2.1 and up: moby/moby#31101

@azimuthdeveloper
Copy link

This should be reopened as the existing functionality is inconsistent with scalar values being overwritten by default. Or people should be able to make a choice - it makes no sense for them to be appended by default.

@adrianhelvik
Copy link

It makes no sense, but changing it would break a lot of configs. I've proposed another non-breaking solution. #8276

@gostega
Copy link

gostega commented Apr 29, 2021

I'd really like to have the override functionally suggested by CarstenHoyer on 13 Sep 2016. I have a situation where by default I want the application exposed on a port for easy testing. But also for testing, and in prod, staging etc, I want to deploy the app behind nginx, and therefore not expose the ports. So the default docker-compose.yml file would have the ports specified, then the docker-compose-withnginx.yml which just defines the nginx service, should override the ports.

@wivaku
Copy link

wivaku commented Dec 21, 2021

@adrianhelvik 's proposal #8276 is clean and does not require modifying the config / original docker-compose file.

@luke-hill
Copy link

Why don't people copy what rubocop do. They have this exact same problem.

For those who don't know rubocop is a ruby gem that provides stylistic configuration through yml files. Often people may have 2/3/4 on their repo at any time.

There is a top level config option that specifies how to combine (In rubocop's example you often want to merge them, but you can have both behaviours).

https://docs.rubocop.org/rubocop/configuration.html#merging-arrays-using-inherit_mode

@oizik
Copy link

oizik commented Mar 10, 2023

Perhaps this is an old discussion but I needed to do this quickly today, my solution was pretty simple:

In docker-compose.yml define the ports as follows:

web:
  ...
  ports:
      - ${APP_PORTS:-8000:8000}		
      - ${DEBUG_PORTS:-5678:5678}	

Default ports used are 8000:8000 for the app and 5678:5678 for debug attachment.

Now I needed to start another web container to run a command within that container, so simply prepended APP_PORTS=8001:8000 and DEBUG_PORTS=5679:5678 to the docker compose run. This overrides the host ports, hence no port conflicts.

The full command as follows:

(export APP_PORTS=8001:8000; export DEBUG_PORTS=5679:5678;  docker compose run --service-ports --name debug_django_command_container  django python -m debugpy --listen 0.0.0.0:5678 -m manage my_django_command)

Notice the use of brackets around the whole command. This is so the exports do not remain in the current running shell session when the command finishes.

This works on my Linux machine, should be the same on Mac, not 100% sure on Windows, but should have similar options to pass environment variables to a command, maybe: this or this.

Reference: Use an Environment File

Hope this helps someone.

@zba
Copy link

zba commented Jan 17, 2024

The problem is with vendor's source, nowdays ot os essy to distribute your systems using docker and not all vendors makes variables or overridable ports, so for example if I have

ports: 
  - 8080:8080

in vendor's compose file, I can't change it anyway without changing the source, but it means that on update by git pull I will have to deal with this and moving things in and out or had to remeber that in the project i have to use some startup script and need to watch my composer up2date or so on. would be nice to have a way to reset lists in overrides

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