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

Docker Compose depends_on #2210

Closed
fgallese opened this issue Sep 18, 2019 · 9 comments
Closed

Docker Compose depends_on #2210

fgallese opened this issue Sep 18, 2019 · 9 comments
Labels
enhancement New feature or request

Comments

@fgallese
Copy link

Hello,

I am trying to use tilt with our existing docker-compose files, and I am having issues with dependencies.

For example, I have some apps that declare a "depends_on" in order to not be started before a required database. See: https://docs.docker.com/compose/compose-file/compose-file-v2/#depends_on

I see that this is not being respected by tilt. Is there a way for me to make tilt build/start a docker-compose image/service before another ?

I am new with tilt so maybe I am missing something ? If so please let me know.. I didn't find anything while searching previous issues and in the doc.

Thank you.

@nicks
Copy link
Member

nicks commented Sep 18, 2019

Hmmm... I'm surprised that Tilt isn't respecting the depends_on.

I have a suspicion that this should be easy to fix by just changing the builder to sort by depends_on here: https://github.com/windmilleng/tilt/blob/master/internal/engine/buildcontroller.go#L36

Lemme poke a bit at it

@nicks nicks added the bug Something isn't working label Sep 18, 2019
@nicks
Copy link
Member

nicks commented Sep 18, 2019

Hmmm...I was not able to reproduce this bug

As far as I can tell, Tilt uses docker-compose config --services to sort the services and start them in that order. In every sample project I tried, this properly handled depends_on (e.g., in https://github.com/dockersamples/example-voting-app, Tilt will start 'redis' and 'db' before 'worker')

@fgallese Do you have a sample project I could try that demonstrates what you're seeing?

@nicks nicks added help wanted Extra attention is needed needs repro case and removed help wanted Extra attention is needed labels Sep 18, 2019
@fgallese
Copy link
Author

fgallese commented Sep 18, 2019

Thank you for your time reviewing this.

Well, after some more testing, I think I've realized what is going on.. and it is probably caused by my own misunderstanding of the way Tilt works.

In short: I am setting up a Tiltfile where I define some docker-compose files using docker_compose(). Also, in the same Tiltfile I define a docker_build() for a service myServiceB.
In one of the docker-compose files, I have a service myServiceA with a depends_on myServiceB (both of this services are included in one of the docker-compose files loaded with docker_compose())

What I was expecting to see, is that myServiceA is not run until myServiceB is built and up. But this is not happening. I guess that when Tilt has a docker_build() definition it always runs it after every other service from docker_compose(). Is this correct ?

This is a simplified version of my setup:

docker-compose.myServiceA.yml:

services:
  myServiceA:
    extends:
      file: docker-compose.base.yml
      service: myServiceA_base # here I define some stuff like the image (myRegistry/myServiceA)
    restart: unless-stopped
    networks:
      - a_network
    ports:
      - 3000:7000
    depends_on:
      mariadb:
        condition: service_healthy
      myServiceB:
        condition: service_started
    command: ["npm", "run", "dev"]

docker-compose.myServiceB.yml:

services:
  myServiceB:
    networks:
      - a_network
    extends:
      file: docker-compose.base.yml
      service: myServiceB # here I define some stuff like the image (myRegistry/myServiceB)
    restart: unless-stopped

Tilltfile:

compose_files = [
    './myApp/docker-compose.base.yml',
    './myApp/docker-compose.myServiceA.yml',
    './myApp/docker-compose.myServiceB.yml'
]

docker_build('myRegistry/myServiceB', './myServiceB', dockerfile='./myServiceB/Dockerfile.dev',
    live_update=[
        sync('./myServiceB/src', '/usr/src/app/src'),
        run('npm i', trigger='./myServiceB/package.json'),
        restart_container()
    ])

docker_compose(compose_files)

Is this the correct behaviour ? In case it is, how can I make Tilt respect the depends_on even when I define a docker_build() ?

Again, I am new with tilt, so I apologize in advance if this is some kind of silly question.

@nicks
Copy link
Member

nicks commented Sep 19, 2019

Ah, OK, the issue is specifically with depends_on with healthchecks ("wait until the server is ready").

This is not a silly question at all! But the answer is complicated, because depends_on with healthchecks has a complicated history. It was added in Docker Compose 2.1 but removed in Docker Compose 3. There's a long discussion on it here with a lot more technical detail into why it was removed: docker/compose#4305

To support this, we would have to support the docker-compose health checks in Tilt. There's been some discussion of this (#1846) but it's not something we're actively working on.

The current workaround (in both Tilt, and in Docker Compose 3) is to make the serviceA restart if it's dependencies aren't ready yet, and just let the system restart the container until the deps are healthy.

@nicks nicks added enhancement New feature or request and removed bug Something isn't working needs repro case labels Sep 19, 2019
@fgallese
Copy link
Author

I see. Unfortunately, we cannot change to docker compose 3 cause it dropped extends functionality which we heavily use.

Anyway @nicks I don't want to keep insisting on this, but even when removed the health check conditions, tilt is still building myServiceB after every single service without a docker_build() in docker_compose().
At this point I am just guessing that this is the expected behaviour and I should just change my strategy.

So, my question for you is the following: Is there a way to make Tilt execute the docker_build() according to the depends_on defined in the docker_compose ?

In other words, I'd expect Tilt to integrate what it picked up at docker_compose() with what was defined in docker_build(), but it is just executing them separately, like "first do everything in the compose and then build/start whatever you got in docker_build()".

I've made a very simple example to demonstrate this, here: https://github.com/fgallese/tilt-example there you'll see how even tho myServiceA depends on myServiceB, this it is ignored and myServiceA started before myServiceB is even done building/starting.

@nicks
Copy link
Member

nicks commented Sep 19, 2019

ooh, thanks for the example! I'm able to reproduce the issue now. Ya, you are right. The healthcheck thing is a separate issue (that we do want to fix). But there's also something else going on. Poking at it now.

@nicks
Copy link
Member

nicks commented Sep 20, 2019

OK, we fixed the startup order bug in #2223. It should be in the latest Tilt release. Would you like us to leave this bug open for the condition: service_healthy part of the issue?

@fgallese
Copy link
Author

fgallese commented Sep 22, 2019

This is great news. I've just tried it and it works exactly as one would expect, respecting the docker-compose order even when a service has a specific docker_build() definition.

Thank you for looking into this, I appreciate your great work.

Regarding the issue, lemme create a new one addressing the healtcheck issue, that way you can close this one and we don't mix different issues.

@nicks
Copy link
Member

nicks commented Sep 23, 2019

Great, thanks for filing that! OK, going to close this issue then.

@nicks nicks closed this as completed Sep 23, 2019
richpjames pushed a commit to ministryofjustice/hmpps-approved-premises-tools that referenced this issue Oct 11, 2022
Tilt doesn’t support dependencies so we have to remove them and
manually wait until the community-api is up tilt-dev/tilt#2210 (comment)
richpjames pushed a commit to ministryofjustice/hmpps-approved-premises-tools that referenced this issue Oct 11, 2022
Tilt doesn’t support dependencies so we have to remove them and
manually wait until the community-api is up tilt-dev/tilt#2210 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants