diff --git a/src/compose/app.ts b/src/compose/app.ts index 23d3b461..42a32057 100644 --- a/src/compose/app.ts +++ b/src/compose/app.ts @@ -150,7 +150,7 @@ export class App { .map((pair) => this.generateStepsForService(pair, { ...state, - servicePairs: installPairs.concat(updatePairs), + servicePairs, targetApp: target, networkPairs: networkChanges, volumePairs: volumeChanges, @@ -528,10 +528,7 @@ export class App { return generateStep('noop', {}); } - if (target && current?.isEqualConfig(target, context.containerIds)) { - // we're only starting/stopping a service - return this.generateContainerStep(current, target); - } else if (current == null) { + if (current == null) { // Either this is a new service, or the current one has already been killed return this.generateFetchOrStartStep( target!, @@ -548,12 +545,21 @@ export class App { 'An empty changing pair passed to generateStepsForService', ); } + const needsSpecialKill = this.serviceHasNetworkOrVolume( current, context.networkPairs, context.volumePairs, ); + if ( + !needsSpecialKill && + current.isEqualConfig(target, context.containerIds) + ) { + // we're only starting/stopping a service + return this.generateContainerStep(current, target); + } + let strategy = checkString(target.config.labels['io.balena.update.strategy']) || ''; const validStrategies = [ @@ -601,7 +607,7 @@ export class App { service.status !== 'Stopping' && !_.some( changingServices, - ({ current }) => current?.serviceName !== service.serviceName, + ({ current }) => current?.serviceName === service.serviceName, ) ) { return [generateStep('kill', { current: service })]; diff --git a/src/lib/docker-utils.ts b/src/lib/docker-utils.ts index 66b0fc43..ac309014 100644 --- a/src/lib/docker-utils.ts +++ b/src/lib/docker-utils.ts @@ -247,13 +247,18 @@ export async function fetchImageWithProgress( ): Promise { const { registry } = await dockerToolbelt.getRegistryAndName(image); - const dockerOpts = { - authconfig: { - username: `d_${uuid}`, - password: currentApiKey, - serverAddress: registry, - }, - }; + const dockerOpts = + // If no registry is specified, we assume the image is a public + // image on the default engine registry, and we don't need to pass any auth + registry != null + ? { + authconfig: { + username: `d_${uuid}`, + password: currentApiKey, + serverAddress: registry, + }, + } + : {}; await dockerProgress.pull(image, onProgress, dockerOpts); return (await docker.getImage(image).inspect()).Id; diff --git a/src/lib/legacy.ts b/src/lib/legacy.ts index b51faa99..6a460dbd 100644 --- a/src/lib/legacy.ts +++ b/src/lib/legacy.ts @@ -234,7 +234,10 @@ export type TargetAppsV2 = { }; }; -type TargetStateV2 = { +/** + * @deprecated exported only for testing + */ +export type TargetStateV2 = { local: { name: string; config: { [name: string]: string }; diff --git a/test/integration/state-engine.spec.ts b/test/integration/state-engine.spec.ts new file mode 100644 index 00000000..c894dacf --- /dev/null +++ b/test/integration/state-engine.spec.ts @@ -0,0 +1,370 @@ +import { expect } from 'chai'; +import * as Docker from 'dockerode'; +import { TargetStateV2 } from '~/lib/legacy'; +import * as request from 'supertest'; +import { setTimeout as delay } from 'timers/promises'; + +const BALENA_SUPERVISOR_ADDRESS = + process.env.BALENA_SUPERVISOR_ADDRESS || 'http://balena-supervisor:48484'; + +const getCurrentState = async () => + await request(BALENA_SUPERVISOR_ADDRESS) + .get('/v2/local/target-state') + .expect(200) + .then(({ body }) => body.state.local); + +const setTargetState = async ( + target: Omit, + timeout = 0, +) => { + const { name, config } = await getCurrentState(); + const targetState = { + local: { + name, + config, + apps: target.apps, + }, + }; + + await request(BALENA_SUPERVISOR_ADDRESS) + .post('/v2/local/target-state') + .set('Content-Type', 'application/json') + .send(JSON.stringify(targetState)) + .expect(200); + + return new Promise(async (resolve, reject) => { + const timer = + timeout > 0 + ? setTimeout( + () => + reject( + new Error( + `Timeout while waiting for the target state to be applied`, + ), + ), + timeout, + ) + : undefined; + + while (true) { + const status = await getStatus(); + if (status.appState === 'applied') { + clearTimeout(timer); + resolve(true); + break; + } + await delay(1000); + } + }); +}; + +const getStatus = async () => + await request(BALENA_SUPERVISOR_ADDRESS) + .get('/v2/state/status') + .then(({ body }) => body); + +const docker = new Docker(); + +describe('state engine', () => { + beforeEach(async () => { + await setTargetState({ + config: {}, + apps: {}, + }); + }); + + after(async () => { + await setTargetState({ + config: {}, + apps: {}, + }); + }); + + it('installs an app with two services', async () => { + await setTargetState({ + config: {}, + apps: { + '123': { + name: 'test-app', + commit: 'deadbeef', + releaseId: 1, + services: { + '1': { + image: 'alpine', + imageId: 11, + serviceName: 'one', + restart: 'unless-stopped', + running: true, + command: 'sleep infinity', + stop_signal: 'SIGKILL', + networks: ['default'], + labels: {}, + environment: {}, + }, + '2': { + image: 'alpine', + imageId: 12, + serviceName: 'two', + restart: 'unless-stopped', + running: true, + command: 'sleep infinity', + stop_signal: 'SIGKILL', + networks: ['default'], + labels: {}, + environment: {}, + }, + }, + networks: {}, + volumes: {}, + }, + }, + }); + + const state = await getCurrentState(); + expect( + state.apps['123'].services.map((s: any) => s.serviceName), + ).to.deep.equal(['one', 'two']); + + const containers = await docker.listContainers(); + expect( + containers.map(({ Names, State }) => ({ Name: Names[0], State })), + ).to.have.deep.members([ + { Name: '/one_11_1_deadbeef', State: 'running' }, + { Name: '/two_12_1_deadbeef', State: 'running' }, + ]); + }); + + // This test recovery from issue #1576, where a device running a service from the target release + // would not stop the service even if there were still network and container changes to be applied + it('always stops running services depending on a network being changed', async () => { + // Install part of the target release + await setTargetState({ + config: {}, + apps: { + '123': { + name: 'test-app', + commit: 'deadca1f', + releaseId: 2, + services: { + '1': { + image: 'alpine:latest', + imageId: 21, + serviceName: 'one', + restart: 'unless-stopped', + running: true, + command: 'sleep infinity', + stop_signal: 'SIGKILL', + labels: {}, + environment: {}, + }, + }, + networks: {}, + volumes: {}, + }, + }, + }); + + const state = await getCurrentState(); + expect( + state.apps['123'].services.map((s: any) => s.serviceName), + ).to.deep.equal(['one']); + + const containers = await docker.listContainers(); + expect( + containers.map(({ Names, State }) => ({ Name: Names[0], State })), + ).to.have.deep.members([{ Name: '/one_21_2_deadca1f', State: 'running' }]); + const containerIds = containers.map(({ Id }) => Id); + + await setTargetState({ + config: {}, + apps: { + '123': { + name: 'test-app', + commit: 'deadca1f', + releaseId: 2, + services: { + '1': { + image: 'alpine:latest', + imageId: 21, + serviceName: 'one', + restart: 'unless-stopped', + running: true, + command: 'sleep infinity', + stop_signal: 'SIGKILL', + networks: ['default'], + labels: {}, + environment: {}, + }, + '2': { + image: 'alpine:latest', + imageId: 22, + serviceName: 'two', + restart: 'unless-stopped', + running: true, + command: 'sh -c "echo two && sleep infinity"', + stop_signal: 'SIGKILL', + networks: ['default'], + labels: {}, + environment: {}, + }, + }, + networks: { + default: { + driver: 'bridge', + ipam: { + config: [ + { gateway: '192.168.91.1', subnet: '192.168.91.0/24' }, + ], + driver: 'default', + }, + }, + }, + volumes: {}, + }, + }, + }); + + const updatedContainers = await docker.listContainers(); + expect( + updatedContainers.map(({ Names, State }) => ({ Name: Names[0], State })), + ).to.have.deep.members([ + { Name: '/one_21_2_deadca1f', State: 'running' }, + { Name: '/two_22_2_deadca1f', State: 'running' }, + ]); + + // Container ids must have changed + expect(updatedContainers.map(({ Id }) => Id)).to.not.have.members( + containerIds, + ); + + expect(await docker.getNetwork('123_default').inspect()) + .to.have.property('IPAM') + .to.deep.equal({ + Config: [{ Gateway: '192.168.91.1', Subnet: '192.168.91.0/24' }], + Driver: 'default', + Options: {}, + }); + }); + + it('updates an app with two services with a network change', async () => { + await setTargetState({ + config: {}, + apps: { + '123': { + name: 'test-app', + commit: 'deadbeef', + releaseId: 1, + services: { + '1': { + image: 'alpine:latest', + imageId: 11, + serviceName: 'one', + restart: 'unless-stopped', + running: true, + command: 'sleep infinity', + stop_signal: 'SIGKILL', + labels: {}, + environment: {}, + }, + '2': { + image: 'alpine:latest', + imageId: 12, + serviceName: 'two', + restart: 'unless-stopped', + running: true, + command: 'sleep infinity', + labels: {}, + environment: {}, + }, + }, + networks: {}, + volumes: {}, + }, + }, + }); + + const state = await getCurrentState(); + expect( + state.apps['123'].services.map((s: any) => s.serviceName), + ).to.deep.equal(['one', 'two']); + + const containers = await docker.listContainers(); + expect( + containers.map(({ Names, State }) => ({ Name: Names[0], State })), + ).to.have.deep.members([ + { Name: '/one_11_1_deadbeef', State: 'running' }, + { Name: '/two_12_1_deadbeef', State: 'running' }, + ]); + const containerIds = containers.map(({ Id }) => Id); + + await setTargetState({ + config: {}, + apps: { + '123': { + name: 'test-app', + commit: 'deadca1f', + releaseId: 2, + services: { + '1': { + image: 'alpine:latest', + imageId: 21, + serviceName: 'one', + restart: 'unless-stopped', + running: true, + command: 'sleep infinity', + stop_signal: 'SIGKILL', + networks: ['default'], + labels: {}, + environment: {}, + }, + '2': { + image: 'alpine:latest', + imageId: 22, + serviceName: 'two', + restart: 'unless-stopped', + running: true, + command: 'sh -c "echo two && sleep infinity"', + stop_signal: 'SIGKILL', + networks: ['default'], + labels: {}, + environment: {}, + }, + }, + networks: { + default: { + driver: 'bridge', + ipam: { + config: [ + { gateway: '192.168.91.1', subnet: '192.168.91.0/24' }, + ], + driver: 'default', + }, + }, + }, + volumes: {}, + }, + }, + }); + + const updatedContainers = await docker.listContainers(); + expect( + updatedContainers.map(({ Names, State }) => ({ Name: Names[0], State })), + ).to.have.deep.members([ + { Name: '/one_21_2_deadca1f', State: 'running' }, + { Name: '/two_22_2_deadca1f', State: 'running' }, + ]); + + // Container ids must have changed + expect(updatedContainers.map(({ Id }) => Id)).to.not.have.members( + containerIds, + ); + + expect(await docker.getNetwork('123_default').inspect()) + .to.have.property('IPAM') + .to.deep.equal({ + Config: [{ Gateway: '192.168.91.1', Subnet: '192.168.91.0/24' }], + Driver: 'default', + Options: {}, + }); + }); +}); diff --git a/test/unit/compose/app.spec.ts b/test/unit/compose/app.spec.ts index 6e9d35be..d9abc80a 100644 --- a/test/unit/compose/app.spec.ts +++ b/test/unit/compose/app.spec.ts @@ -639,8 +639,12 @@ describe('compose/app', () => { isTarget: true, }); - const steps = current.nextStepsForAppUpdate(defaultContext, target); + const availableImages = [createImage({ appUuid: 'deadbeef' })]; + const steps = current.nextStepsForAppUpdate( + { ...defaultContext, availableImages }, + target, + ); const [killStep] = expectSteps('kill', steps); expect(killStep) .to.have.property('current') @@ -681,6 +685,129 @@ describe('compose/app', () => { expectNoStep('removeNetwork', steps); }); + it('should always kill dependencies of networks before removing', async () => { + const current = createApp({ + services: [ + // The device for some reason is already running some services + // of the new release, but we need to kill it anyways + await createService({ + image: 'alpine', + serviceName: 'one', + commit: 'deadca1f', + composition: { command: 'sleep infinity', networks: ['default'] }, + }), + ], + networks: [Network.fromComposeObject('default', 1, 'appuuid', {})], + }); + const target = createApp({ + services: [ + await createService({ + image: 'alpine', + serviceName: 'one', + commit: 'deadca1f', + composition: { command: 'sleep infinity', networks: ['default'] }, + }), + await createService({ + image: 'alpine', + serviceName: 'two', + commit: 'deadca1f', + composition: { + command: 'sh -c "echo two && sleep infinity"', + networks: ['default'], + }, + }), + ], + networks: [ + Network.fromComposeObject('default', 1, 'appuuid', { + labels: { test: 'test' }, + }), + ], + isTarget: true, + }); + + const availableImages = [ + createImage({ appId: 1, serviceName: 'one', name: 'alpine' }), + createImage({ appId: 1, serviceName: 'two', name: 'alpine' }), + ]; + + const steps = current.nextStepsForAppUpdate( + { ...defaultContext, availableImages }, + target, + ); + const [killStep] = expectSteps('kill', steps); + + expect(killStep) + .to.have.property('current') + .that.deep.includes({ serviceName: 'one' }); + + // We shouldn't try to remove the network until we have gotten rid of the dependencies + expectNoStep('removeNetwork', steps); + }); + + it('should kill dependencies of networks before updating between releases', async () => { + const current = createApp({ + services: [ + await createService({ + image: 'alpine', + serviceName: 'one', + commit: 'deadbeef', + composition: { command: 'sleep infinity', networks: ['default'] }, + }), + await createService({ + image: 'alpine', + serviceName: 'two', + commit: 'deadbeef', + composition: { command: 'sleep infinity', networks: ['default'] }, + }), + ], + networks: [Network.fromComposeObject('default', 1, 'appuuid', {})], + }); + const target = createApp({ + services: [ + await createService({ + image: 'alpine', + serviceName: 'one', + commit: 'deadca1f', + composition: { command: 'sleep infinity', networks: ['default'] }, + }), + await createService({ + image: 'alpine', + serviceName: 'two', + commit: 'deadca1f', + composition: { + command: 'sh -c "echo two && sleep infinity"', + networks: ['default'], + }, + }), + ], + networks: [ + Network.fromComposeObject('default', 1, 'appuuid', { + labels: { test: 'test' }, + }), + ], + isTarget: true, + }); + + const availableImages = [ + createImage({ appId: 1, serviceName: 'one', name: 'alpine' }), + createImage({ appId: 1, serviceName: 'two', name: 'alpine' }), + ]; + + const steps = current.nextStepsForAppUpdate( + { ...defaultContext, availableImages }, + target, + ); + expectSteps('kill', steps, 2); + + expect(steps.map((s) => (s as any).current.serviceName)).to.have.members([ + 'one', + 'two', + ]); + + // We shouldn't try to remove the network until we have gotten rid of the dependencies + expectNoStep('removeNetwork', steps); + }); + it('should create the default network if it does not exist', () => { const current = createApp({ networks: [] }); const target = createApp({ networks: [], isTarget: true });