Wait for service dependencies to be running #2393
Conversation
| @@ -881,7 +885,7 @@ class AppImpl implements App { | |||
| // different to a dependency which is in the servicePairs below, as these | |||
| // are services which are changing). We could have a dependency which is | |||
| // starting up, but is not yet running. | |||
| const depInstallingButNotRunning = _.some(targetApp.services, (svc) => { | |||
| const depInstallingButNotRunning = _.some(this.services, (svc) => { | |||
There was a problem hiding this comment.
Bug fix. Services in the target app are generally configured a running: true so dependencies would get started without waiting for the status of the actual container
| } | ||
| return [generateStep('start', { target })]; | ||
| } else { | ||
| // Wait for dependencies to be started |
There was a problem hiding this comment.
We need this noop here to avoid settling the state too early
If the dependency requires a reboot, this will prevent the release of locks :(
| @@ -1294,29 +1293,31 @@ describe('compose/app', () => { | |||
| .to.deep.include({ serviceName: 'main' }); | |||
| }); | |||
|
|
|||
| it('should not try to start a container which has exited and has restart policy of no', async () => { | |||
| it('should not try to start a container which has exited', async () => { | |||
There was a problem hiding this comment.
Maybe I'm misunderstanding this test change, but wouldn't we want to restart a container that has exited with an error code if the restart policy is always or on-failure? Doesn't this test case go against that?
There was a problem hiding this comment.
The supervisor never restarts a container that has already been started before. The restart policy is handled by the engine. The only exception is for the engine/hostOS race bug
balena-supervisor/src/compose/app.ts
Line 747 in 81b3075
This fixes a regression where dependencies would only be started in order and would start the dependent service if its dependency had been started at some point in the past, regardless of the running condition. This makes the behavior more consistent with docker compose where the [dependency needs to be running or healthy](https://github.com/docker/compose/blob/69a83d1303a103d82b05d512baf273244b4dbd94/pkg/compose/convergence.go#L441) for the service to be started. Change-type: patch
pipex
force-pushed
the
depends-on-fix
branch
from
79a1402 to
8e6c0fc
Compare
This fixes a regression where dependencies would only be started in order and would start the dependent service if its dependency had been started at some point in the past, regardless of the running condition.
This makes the behavior more consistent with docker compose where the dependency needs to be
running or healthy for the service to be started.
Change-type: patch