compose icon indicating copy to clipboard operation
compose copied to clipboard

profile-deactivated dependencies do not cause a failure

Open KonradHoeffner opened this issue 3 years ago • 14 comments

Description

According to the documentation, docker compose will fail if a dependency is deactivated through an inactive profile:

# this will fail because profile "dev" is disabled
$ docker compose up phpmyadmin

However in practice, docker compose will successfully start and not even print an error or warning (it "fails to fail").

Steps to reproduce the issue:

  1. Create the following docker-compose.yml
services:
  foo:
    image: hello-world
    profiles: ["foo"]
  bar:
    image: hello-world
    depends_on:
      foo:
        condition: service_completed_successfully
  1. docker compose up

Describe the results you received:

docker compose will just silently execute service "bar":

$ docker compose up               
[+] Running 1/0
 ⠿ Container test-bar-1  Created                                                                                                                         0.1s
Attaching to test-bar-1
test-bar-1  | 
test-bar-1  | Hello from Docker!
test-bar-1  | This message shows that your installation appears to be working correctly.
test-bar-1  | 
test-bar-1  | To generate this message, Docker took the following steps:
test-bar-1  |  1. The Docker client contacted the Docker daemon.
test-bar-1  |  2. The Docker daemon pulled the "hello-world" image from the Docker Hub.
test-bar-1  |     (amd64)
test-bar-1  |  3. The Docker daemon created a new container from that image which runs the
test-bar-1  |     executable that produces the output you are currently reading.
test-bar-1  |  4. The Docker daemon streamed that output to the Docker client, which sent it
test-bar-1  |     to your terminal.
test-bar-1  | 
test-bar-1  | To try something more ambitious, you can run an Ubuntu container with:
test-bar-1  |  $ docker run -it ubuntu bash
test-bar-1  | 
test-bar-1  | Share images, automate workflows, and more with a free Docker ID:
test-bar-1  |  https://hub.docker.com/
test-bar-1  | 
test-bar-1  | For more examples and ideas, visit:
test-bar-1  |  https://docs.docker.com/get-started/
test-bar-1  | 
test-bar-1 exited with code 0

Describe the results you expected:

According to the docs, I expected a failure, which I interpret as preferably docker compose not starting at all but instead exiting with an error message, or at least if it does start it should print an error message. Alternatively it could automatically start foo but that seems to be not intended according to the docs.

Output of docker compose version:

Docker Compose version 2.10.2

Output of docker info:

Client:
 Context:    default
 Debug Mode: false
 Plugins:
  buildx: Docker Buildx (Docker Inc., v0.8.2-docker)
  compose: Docker Compose (Docker Inc., 2.10.2)

Server:
 Containers: 175
  Running: 0
  Paused: 0
  Stopped: 175
 Images: 249
 Server Version: 20.10.17
 Storage Driver: overlay2
  Backing Filesystem: extfs
  Supports d_type: true
  Native Overlay Diff: false
  userxattr: false
 Logging Driver: json-file
 Cgroup Driver: systemd
 Cgroup Version: 2
 Plugins:
  Volume: local
  Network: bridge host ipvlan macvlan null overlay
  Log: awslogs fluentd gcplogs gelf journald json-file local logentries splunk syslog
 Swarm: inactive
 Runtimes: io.containerd.runc.v2 io.containerd.runtime.v1.linux runc
 Default Runtime: runc
 Init Binary: docker-init
 containerd version: 9cd3357b7fd7218e4aec3eae239db1f68a5a6ec6.m
 runc version: 
 init version: de40ad0
 Security Options:
  seccomp
   Profile: default
  cgroupns
 Kernel Version: 5.19.2-arch1-1
 Operating System: Arch Linux
 OSType: linux
 Architecture: x86_64
 CPUs: 20
 Total Memory: 46.97GiB
 Name: konradsdesktop
 ID: 3NEP:3UOO:RVV3:RCWK:44GY:N4UB:ZIRE:2BN7:KSBI:CMZG:5KFU:YRPN
 Docker Root Dir: /home/konrad/docker
 Debug Mode: false
 Registry: https://index.docker.io/v1/
 Labels:
 Experimental: false
 Insecure Registries:
  127.0.0.0/8
 Live Restore Enabled: false

KonradHoeffner avatar Aug 29 '22 06:08 KonradHoeffner

Hi,

I would like to make a case for changing the specification to keep this behavior of it silently ignoring depends_on because this allows you to have a docker-compose.yml file that looks like this:

services:
  postgres:
    profiles: ["postgres"]
  redis:
    profiles: ["redis"]
  web:
    profiles: ["web"]
    depends_on: ["postgres", "redis"]
  worker:
    profiles: ["worker"]
    depends_on: ["postgres", "redis"]
  assets:
    profiles: ["assets"]

In development you can set this environment variable in an .env file to start everything:

export COMPOSE_PROFILES=postgres,redis,assets,web,worker

In production you can set this instead:

export COMPOSE_PROFILES=web,worker

This would let you not run a dev-only set of assets containers in production for various watchers and also take advantage of using your cloud provider's managed Postgres and Redis services because the local containers for those services don't need to run since the cloud provider's managed service will fulfill that dependency.

If this bug were fixed then we're back to profiles being in my opinion not very useful because the above wouldn't be possible anymore. I actually made a blog post and video about this other day at https://nickjanetakis.com/blog/docker-tip-94-docker-compose-v2-and-profiles-are-the-best-thing-ever which goes into more details.

Personally if this were fixed and brought back to its previous form I would stop using Docker Compose profiles and go back to using an override file plus hacky sed replacements in prod which is way worse than the current unfixed but fully working approach.

Please consider not fixing this and officially change the spec to use the current behavior by default.

nickjj avatar Sep 13 '22 16:09 nickjj

As this behaviour is very specific to the development workflow, what do you think to manage that in a dedicated development section? @ndeloof started to draft a PR about this feature.

glours avatar Sep 13 '22 16:09 glours

I think this affects both development and production. It's extremely common to use managed databases in production but not in development and you'd still want depends_on for development while depends_on is inactive / ignored in prod (as needed).

The current unfixed behavior handles this use case perfectly. Having this fixed without another type of workaround as an alternative solution is a big hit to the developer experience or for anyone who wants to use Docker Compose in both development and production.

IMO the unfixed behavior is the user friendly / intuitive behavior. I want depends_on when I activate profiles that use it, however if I have depends_on defined but didn't activate the dependent profile then it's expected the dependency is coming from somewhere else. If it doesn't that's out of Docker's control to enforce a working system.

nickjj avatar Sep 13 '22 16:09 nickjj

@nickjj: Sorry, I don't understand the difference between your use case and not having any dependencies at all. What does depends_on actually do for you if it doesn't do the single thing it was meant for? Wouldn't the following docker-compose.yml have exactly the same behavior if this were fixed?

services:
  postgres:
    profiles: ["postgres"]
  redis:
    profiles: ["redis"]
  web:
    profiles: ["web"]
  worker:
    profiles: ["worker"]
  assets:
    profiles: ["assets"]

And then docker compose up starts everything.

KonradHoeffner avatar Sep 13 '22 16:09 KonradHoeffner

@KonradHoeffner In development having depends_on defined will ensure if I ever run a docker compose run [...] command then Postgres and Redis will be started automatically. Certain application commands such as running a database migration would require Postgres to be up. Without depends_on I'd have to manually start Postgres beforehand, which is a pain.

In the case where you're not using a managed database and you want to use everything having depends_on also enforces that your web and worker will be stopped before postgres and redis. In practice I don't know what the implication will be here but having the guarantee your app will be stopped before your db feels like a good guarantee to have.

Conceptually it's also a form of documentation that sets the specification of the application's dependencies. Someone can glance at it and know that both the web and worker depend on having Postgres and Redis. This helps the developer experience around exploring a new code base. It's minor but shouldn't be disregarded IMO.

nickjj avatar Sep 13 '22 16:09 nickjj

I wonder if a good in-between would be to have a warning when running something with profile-deactivated dependencies, but not cause a failure. This would be noticeable for the development workflow (if you have profile deactivated dependencies you get a Warning: X depends on Y which was not started because profile Z is deactivated), but would not be a blocker otherwise.

The current behavior as-is is misleading at best and can lead to confusion, and should be changed.

@nickjj I wonder if your usecase would not be addressed better by having another compose.yaml for development which extends the primary one and sets depends_on and other development attributes. This would also more explicitly show the difference between development pre-conditions/dependencies and essential ones for prod.

laurazard avatar Sep 13 '22 16:09 laurazard

@laurazard having the same docker-compose.yml used in development, production and any other environment is a very nice thing to have. It avoids duplication and ensures the same code paths are used between dev -> CI -> staging -> prod with no chance of messing something up or encountering unexpected events. It's very nice to be able to just know you can run docker compose up and you're golden across the board. I think going back to separate docker-compose.yml files for different environments (something I used to do in 2016) would be a serious downgrade for usability, safety and confidence.

Keeping it "unfixed" but emitting a warning seems like a reasonable compromise to inform users, would you also consider including an environment variable to disable this warning too for the folks who know what it does? I know certain web frameworks have done this idea of emitting warnings and over time they always end up introducing a feature to disable them because seeing the warning for the 837th time adds log clutter and it makes you think you're doing something wrong when you're not.

But, if it begins with just a warning while the app continues to up and work as it does today in its current state that would be a really great start. Perhaps you can poll users on if they want the ignore feature afterwards.

nickjj avatar Sep 13 '22 16:09 nickjj

Philosophically, having a dependency that I can't depend on doesn't sound right to me as it would be a more of a "hopefully" than a "dependency", but practically, having a warning would be a definite improvement to the current state in my opinion and an option to suppress that warning would be a net positive to users, as those who don't know about it would not set it and those who know what they are doing can use it if they like. I don't know how much work it would be for the developers to add and maintain it but it doesn't sound like a big feature.

I like the approach of the Rust programming language more however, which has the concept of an "optional dependency" which is explicitly specified like this:

[dependencies]
foo = { version = "1.0", optional = true }
bar = { version = "1.0", optional = true }

[features]
fancy-feature = ["foo", "bar"]

And they also have development dependencies:

[dev-dependencies]
tempdir = "0.3"

KonradHoeffner avatar Sep 13 '22 17:09 KonradHoeffner

@nickjj I agree it's nice to use the same compose.yaml in all enviroments! What I meant with my suggestion would be to have a main compose.yaml such as:

services:
  a:
    image: alpine
    command: top

and a compose.dev.yaml such as:

services:
  a:
    depends_on:
      - b
  b:
    image: alpine
    command: top

In this case, running compose up only runs a, and running compose -f compose.yaml -f compose.dev.yaml up runs both a and b, with the correct dependency condition.

I don't believe this incurs any regression regarding duplication/safety, as you're still using the same code paths/compose files for dev/staging/prod, just customizing/extending with dependencies as needed for the different environments.

Conceptually, regarding the "development dependencies" approach like @KonradHoeffner mentioned, this would make sense to have a discussion in the spec side regarding the development section, to add explicitly development specific workflows like @glours mentioned.

laurazard avatar Sep 13 '22 17:09 laurazard

Yeah I'm just trying to make a case that this isn't specific to development, it affects all environments. Unless the purpose of that workflow PR draft isn't about development specifically but more about using Docker Compose in general?

Your proposed solution is still not as portable I think? Ideally I'd like to be able to fulfill these 3 use cases:

  1. In development I almost always run everything
  2. In production I never run assets
  3. In production I sometimes run postgres + redis as depends_on dependencies but not always (it depends on the project)

With the current "unfixed" approach this is easy since all you do is change a single COMPOSE_PROFILES environment variable and use the same default docker-compose.yml everywhere. I can then use the same Flask, Rails, Django, Node and Phoenix example apps I have in the same way for all 3 use cases.

Use case 1 is also easy to modify if you want to temporarily use a managed DB in development which is something that comes up kind of frequently to debug issues in production. Sometimes you want to connect to a remote staging or pre-prod DB in development to see something. That becomes a simple COMPOSE_PROFILES env change.

With your proposed solution you'd need to include assets in the compose.dev.yaml but what about postgres and redis? They might be dev only dependencies for some projects but not all. Now the user would need a third compose.postgres-redis.yml file because it can't go in the same compose.dev.yaml file with the assets since those never run in production. It's a lot more complicated than a single COMPOSE_PROFILES env change, especially since my example is a baseline web app, you would need to keep introducing combos of files for other services too. It also becomes cumbersome to to do the temporary use case 1 scenario of using a remote DB in development.

Basically what you're describing is in my opinion a form of profiles except through multiple files and extensions. In my mind profiles are the perfect abstraction for this, it solves the exact use case of wanting to run different containers in different scenarios. Isn't that why this feature was created?

If profiles aren't made to be used like this then I think the feature and documentation should be updated and rethought of from ground zero because right now if this PR is fixed with no workaround users cannot use profiles with optional depends_on which means not using profiles and going back to manual pre-deploy hooks to use sed and other command line tools to modify the docker-compose.yml file which is extremely cryptic and potentially error prone.


@KonradHoeffner Philosophically having a dependency that I can't depend on doesn't sound right to me as it would be a more of a "hopefully" than a "dependency"

In practice applications can be more than Docker Compose. I think depends_on can be optional from Docker Compose's perspective because the dependency can be met externally through a managed database service. It's not technically optional, it's only optional from Docker Compose's view of the world.

"Use it when we have it, else ignore it" feels like a reasonable way to live in such a world which is what the unfixed implementation does. It made me so happy to see that it began existing this way that I stopped everything and upgraded everyone I know of to use Docker Compose v2 and profiles. Having to pin Docker Compose 2.10.2 indefinitely or go back to the hacky sed approach is a huge reduction to developer quality of life and user experience.

nickjj avatar Sep 13 '22 18:09 nickjj

@nickjj I understand your specific need, but they way you're using profiles is not in line with the way profiles are described in the spec and proposed to be used.

Since you'd like to have all your services in a single compose.yaml, I wonder if you could achieve your goal by running compose up --no-deps [services you want to start]. This would ensure that dependencies are not started unless you specify them, and still allow you to only start the services you like.

laurazard avatar Sep 13 '22 18:09 laurazard

@laurazard To be fair, v2 was GA'd with this behavior as is.

You now have folks out there who have changed their whole world and the worlds of others based on that behavior you defined. "Fixing" this is making a backwards incompatible change that will very much break everything for anyone using this pattern since v2 was initially released as a beta tool until now. It's also been almost 6 months of it being GA'd.

On the other hand allowing this behavior doesn't break anything for anyone. All it requires is changing a sentence in the specification to officially make depends_on optional in a "Use it when you start this service, else ignore it" fashion. We know this solution works in both cases because folks are currently using v2 as is.

Your --no-deps suggestion is close to what KonradHoeffner wrote a few replies ago except it involves dynamically changing your up command to pass in whatever services you want to start instead of continuing to use COMPOSE_PROFILES and removing all traces of depends_on. I've written why that's not ideal in https://github.com/docker/compose/issues/9795#issuecomment-1245662182.

I'm not sure what else to say.

I'm coming at this from the perspective of using Docker since 2014 and have deployed and deployed well over 50+ different apps for lots of companies since then with Docker Compose. Since profiles were first introduced in 2020 or maybe early 2021 I remember eagerly waiting for optional depends_on support. I'm not trying to be condescending but I'm not sure you realize how many doors this opens for simplifying things with it working in its unfixed state.

I really do think it should be kept as is. I believe in this so strongly that I'd consider running and maintaining a fork of Docker Compose v2 if pinning v2.10.2 stopped working due to potential future Docker API incompatibilities.

nickjj avatar Sep 13 '22 19:09 nickjj

@nickjj Allowing this behavior doesn't break anything for anyone, but as this issue existing shows, can be confusing and is not the expected behavior.

From my understanding, you would like to be able to use an env var to set which services to run when running compose up -- this could be achieved in other, more explicit ways, such as a COMPOSE_SERVICE_FILTER var for compose up, compose start, or similar improvement.

I would prefer an approach like that over using profiles this way since that seems more explicit: "I am purposefully selecting these services as the services which I would like to run" vs "I am using only these profiles and as a side effect of that this dependency is being ignored", and this also creates less opportunities for people to be confused by accidentally disabling a dependency by forgetting to add a profile, and not realizing it.

Regardless, @nickjj the proper place for discussions regarding the spec is in https://github.com/compose-spec/compose-spec/ . Feel free to create an issue there with your proposal to amend the spec if you desire to.

laurazard avatar Sep 13 '22 23:09 laurazard

Changing the documentation to adhere to this unfixed ruleset or adding a warning would make it not confusing without breaking functionality that v2 has allowed for all of the months v2 was available.

You are choosing to make a backwards incompatible change if you fix it, this is going to upset a lot of people. Just so we're in full agreement, it sounds like you'd rather break everyone's applications that folks have been using since you introduced this functionality since v2 came out earlier this year than change 1 sentence of documentation which doesn't break anything for others who are not using it?

As soon as I told everyone I know about v2 profiles working with optional depends_on they were thrilled because we all know how hacky alternative solutions are. Just a ton of custom shell scripting on every single project to programmatically modify a docker compose file before running it. It's the most unpleasant experience of using Docker Compose.

Your COMPOSE_SERVER_FILETER solution sounds exactly like what profiles are. It's a mechanism to control which services to run. Isn't that what profiles do? The official documentation states that in the first sentence with "selectively enable services". Your new proposed env variable is a literal definition of enabling selected services. Is the only difference that your new suggested env var will allow optional depends_on through needing to always provide --no-deps? In my opinion this will be more confusing for users because now they have 2 options to enable services in different ways, 1 of which is a lot more inconvenient to use.

What about adding a new necessity: "optional" property which defaults to necessity: "required" to depends_on so it can all work the same with profiles being explicit, or maybe required: false which defaults to required: true? That's in my opinion the easiest solution and it's backwards compatible for the "fixed" behavior while giving people who have already depended on the "unfixed" behavior a way to use Docker Compose in the way v2 has allowed us to use since the beginning of the year.

I just added that suggestion to https://github.com/compose-spec/compose-spec/issues/274 but I think it's important to have in this ticket too because if this gets fixed without the above or something similar as a workaround then you're knowingly making a backwards incompatible change that will force folks into not upgrading Docker Compose v2 and Docker Desktop to continue using the latest version without the "fix".

nickjj avatar Sep 14 '22 00:09 nickjj

did this get changed? My compose file looks like this:

api:
    build: .
    restart: unless-stopped
    networks:
      - nginx-proxy
    profiles:
      - deploy
api_dev:
    build: .
    depends_on:
      - db
    ports:
      - "8000:80"
    networks:
      - internal
    profiles:
      - dev
worker:
    build: "./worker"
    user: nobody
    depends_on:
      - api_dev
      - api

but now this happens:

> docker-compose --profile dev up
no such service: api

RottenSchnitzel avatar Jul 12 '23 03:07 RottenSchnitzel

yes, this seems to be fixed in v2.19.0 by #10602

A workaround to keep the previous behavior is possible with the new depends_on.required (#10792) since v2.20.2, see compose-spec/compose-spec#382 / compose-spec/compose-spec#274

So I guess this issue can be closed!?

acran avatar Sep 02 '23 10:09 acran

Yes, this is fixed now, thanks a lot to everyone involved! Verified with this docker-compose.yml:

 services: 
  a:
      image: hello-world
      profiles:
        - deploy
  b:
      image: hello-world
      depends_on:
        - a
$ docker compose up     
no such service: a
$ docker compose --profile deploy up
[+] Running 2/0
[...]

Versions

$ docker compose version
Docker Compose version 2.20.3
$ docker version
Client:
 Version:           24.0.5
 API version:       1.43
 Go version:        go1.20.6
 Git commit:        ced0996600
 Built:             Wed Jul 26 21:44:58 2023
 OS/Arch:           linux/amd64
 Context:           default

Server:
 Engine:
  Version:          24.0.5
  API version:      1.43 (minimum version 1.12)
  Go version:       go1.20.6
  Git commit:       a61e2b4c9c
  Built:            Wed Jul 26 21:44:58 2023
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          v1.7.2
  GitCommit:        0cae528dd6cb557f7201036e9f43420650207b58.m
 runc:
  Version:          1.1.9
  GitCommit:        
 docker-init:
  Version:          0.19.0
  GitCommit:        de40ad0

KonradHoeffner avatar Sep 02 '23 12:09 KonradHoeffner