diff --git a/src/compose/app.ts b/src/compose/app.ts index f140f162..3590ee2a 100644 --- a/src/compose/app.ts +++ b/src/compose/app.ts @@ -29,6 +29,7 @@ import { pathExistsOnHost } from '../lib/fs-utils'; export interface AppConstructOpts { appId: number; + appUuid?: string; appName?: string; commit?: string; source?: string; @@ -52,6 +53,7 @@ interface ChangingPair { export class App { public appId: number; + public appUuid?: string; // When setting up an application from current state, these values are not available public appName?: string; public commit?: string; @@ -65,6 +67,7 @@ export class App { public constructor(opts: AppConstructOpts, public isTargetState: boolean) { this.appId = opts.appId; + this.appUuid = opts.appUuid; this.appName = opts.appName; this.commit = opts.commit; this.source = opts.source; @@ -77,6 +80,7 @@ export class App { this.networks.default = Network.fromComposeObject( 'default', opts.appId, + opts.appUuid!, // app uuid always exists on the target state {}, ); } @@ -160,7 +164,6 @@ export class App { target.commit != null && this.commit !== target.commit ) { - // TODO: The next PR should change this to support multiapp commit values steps.push( generateStep('updateCommit', { target: target.commit, @@ -732,7 +735,7 @@ export class App { const networks = _.mapValues( JSON.parse(app.networks) ?? {}, (conf, name) => { - return Network.fromComposeObject(name, app.appId, conf ?? {}); + return Network.fromComposeObject(name, app.appId, app.uuid, conf ?? {}); }, ); @@ -799,6 +802,7 @@ export class App { return new App( { appId: app.appId, + appUuid: app.uuid, commit: app.commit, appName: app.name, source: app.source, diff --git a/src/compose/network-manager.ts b/src/compose/network-manager.ts index e60c6be3..a880c3a1 100644 --- a/src/compose/network-manager.ts +++ b/src/compose/network-manager.ts @@ -23,16 +23,12 @@ export function getAll(): Bluebird { }); } -export function getAllByAppId(appId: number): Bluebird { - return getAll().filter((network: Network) => network.appId === appId); -} - -export async function get(network: { +async function get(network: { name: string; - appId: number; + appUuid: string; }): Promise { const dockerNet = await docker - .getNetwork(Network.generateDockerName(network.appId, network.name)) + .getNetwork(Network.generateDockerName(network.appUuid, network.name)) .inspect(); return Network.fromDockerNetwork(dockerNet); } @@ -41,7 +37,7 @@ export async function create(network: Network) { try { const existing = await get({ name: network.name, - appId: network.appId, + appUuid: network.appUuid!, // new networks will always have uuid }); if (!network.isEqualConfig(existing)) { throw new ResourceRecreationAttemptError('network', network.name); @@ -52,7 +48,7 @@ export async function create(network: Network) { } catch (e) { if (!NotFoundError(e)) { logger.logSystemEvent(logTypes.createNetworkError, { - network: { name: network.name, appId: network.appId }, + network: { name: network.name, appUuid: network.appUuid }, error: e, }); throw e; diff --git a/src/compose/network.ts b/src/compose/network.ts index e1e9fd4e..d19f7c56 100644 --- a/src/compose/network.ts +++ b/src/compose/network.ts @@ -1,4 +1,3 @@ -import * as Bluebird from 'bluebird'; import * as _ from 'lodash'; import * as dockerode from 'dockerode'; @@ -11,29 +10,64 @@ import * as ComposeUtils from './utils'; import { ComposeNetworkConfig, NetworkConfig } from './types/network'; import { InvalidNetworkNameError } from './errors'; +import { InternalInconsistencyError } from '../lib/errors'; export class Network { public appId: number; + public appUuid?: string; public name: string; public config: NetworkConfig; private constructor() {} + private static deconstructDockerName( + name: string, + ): { name: string; appId?: number; appUuid?: string } { + const matchWithAppId = name.match(/^(\d+)_(\S+)/); + if (matchWithAppId == null) { + const matchWithAppUuid = name.match(/^([0-9a-f-A-F]{32,})_(\S+)/); + + if (!matchWithAppUuid) { + throw new InvalidNetworkNameError(name); + } + + const appUuid = matchWithAppUuid[1]; + return { name: matchWithAppUuid[2], appUuid }; + } + + const appId = parseInt(matchWithAppId[1], 10); + if (isNaN(appId)) { + throw new InvalidNetworkNameError(name); + } + + return { + appId, + name: matchWithAppId[2], + }; + } + public static fromDockerNetwork( network: dockerode.NetworkInspectInfo, ): Network { const ret = new Network(); - const match = network.Name.match(/^([0-9]+)_(.+)$/); - if (match == null) { - throw new InvalidNetworkNameError(network.Name); + // Detect the name and appId from the inspect data + const { name, appId, appUuid } = Network.deconstructDockerName( + network.Name, + ); + + const labels = network.Labels ?? {}; + if (!appId && isNaN(parseInt(labels['io.balena.app-id'], 10))) { + // This should never happen as supervised networks will always have either + // the id or the label + throw new InternalInconsistencyError( + `Could not read app id from network: ${network.Name}`, + ); } - // If the regex match succeeds `match[1]` should be a number - const appId = parseInt(match[1], 10); - - ret.appId = appId; - ret.name = match[2]; + ret.appId = appId ?? parseInt(labels['io.balena.app-id'], 10); + ret.name = name; + ret.appUuid = appUuid; const config = network.IPAM?.Config || []; @@ -51,7 +85,7 @@ export class Network { }, enableIPv6: network.EnableIPv6, internal: network.Internal, - labels: _.omit(ComposeUtils.normalizeLabels(network.Labels ?? {}), [ + labels: _.omit(ComposeUtils.normalizeLabels(labels), [ 'io.balena.supervised', ]), options: network.Options ?? {}, @@ -63,6 +97,7 @@ export class Network { public static fromComposeObject( name: string, appId: number, + appUuid: string, network: Partial> & { ipam?: Partial; }, @@ -70,6 +105,7 @@ export class Network { const net = new Network(); net.name = name; net.appId = appId; + net.appUuid = appUuid; Network.validateComposeConfig(network); @@ -95,12 +131,13 @@ export class Network { }, enableIPv6: network.enable_ipv6 || false, internal: network.internal || false, - labels: network.labels || {}, + labels: { + 'io.balena.app-id': String(appId), + ...ComposeUtils.normalizeLabels(network.labels || {}), + }, options: network.driver_opts || {}, }; - net.config.labels = ComposeUtils.normalizeLabels(net.config.labels); - return net; } @@ -117,7 +154,7 @@ export class Network { public async create(): Promise { logger.logSystemEvent(logTypes.createNetwork, { - network: { name: this.name }, + network: { name: this.name, appUuid: this.appUuid }, }); await docker.createNetwork(this.toDockerConfig()); @@ -125,7 +162,7 @@ export class Network { public toDockerConfig(): dockerode.NetworkCreateOptions { return { - Name: Network.generateDockerName(this.appId, this.name), + Name: Network.generateDockerName(this.appUuid!, this.name), Driver: this.config.driver, CheckDuplicate: true, Options: this.config.options, @@ -153,28 +190,41 @@ export class Network { }; } - public remove(): Bluebird { + public async remove() { logger.logSystemEvent(logTypes.removeNetwork, { - network: { name: this.name, appId: this.appId }, + network: { name: this.name, appUuid: this.appUuid }, }); - const networkName = Network.generateDockerName(this.appId, this.name); - - return Bluebird.resolve(docker.listNetworks()) - .then((networks) => networks.filter((n) => n.Name === networkName)) - .then(([network]) => { - if (!network) { - return Bluebird.resolve(); + // Find the network + const [networkName] = (await docker.listNetworks()) + .filter((network) => { + try { + const { appId, appUuid, name } = Network.deconstructDockerName( + network.Name, + ); + return ( + name === this.name && + (appId === this.appId || appUuid === this.appUuid) + ); + } catch { + return false; } - return Bluebird.resolve( - docker.getNetwork(networkName).remove(), - ).tapCatch((error) => { - logger.logSystemEvent(logTypes.removeNetworkError, { - network: { name: this.name, appId: this.appId }, - error, - }); - }); + }) + .map((network) => network.Name); + + if (!networkName) { + return; + } + + try { + await docker.getNetwork(networkName).remove(); + } catch (error) { + logger.logSystemEvent(logTypes.removeNetworkError, { + network: { name: this.name, appUuid: this.appUuid }, + error, }); + throw error; + } } public isEqualConfig(network: Network): boolean { @@ -210,8 +260,8 @@ export class Network { } } - public static generateDockerName(appId: number, name: string) { - return `${appId}_${name}`; + public static generateDockerName(appIdOrUuid: number | string, name: string) { + return `${appIdOrUuid}_${name}`; } } diff --git a/src/compose/service.ts b/src/compose/service.ts index 5c09352e..aee37b97 100644 --- a/src/compose/service.ts +++ b/src/compose/service.ts @@ -171,7 +171,7 @@ export class Service { networks = config.networks || {}; } // Prefix the network entries with the app id - networks = _.mapKeys(networks, (_v, k) => `${service.appId}_${k}`); + networks = _.mapKeys(networks, (_v, k) => `${service.appUuid}_${k}`); // Ensure that we add an alias of the service name networks = _.mapValues(networks, (v) => { if (v.aliases == null) { @@ -257,7 +257,7 @@ export class Service { ) { if (networks[config.networkMode!] == null && !serviceNetworkMode) { // The network mode has not been set explicitly - config.networkMode = `${service.appId}_${config.networkMode}`; + config.networkMode = `${service.appUuid}_${config.networkMode}`; // If we don't have any networks, we need to // create the default with some default options networks[config.networkMode] = { @@ -1008,11 +1008,23 @@ export class Service { public hasNetwork(networkName: string) { // TODO; we could probably export network naming methods to another // module to avoid duplicate code - return `${this.appId}_${networkName}` in this.config.networks; + // We don't know if this service is current or target state so we need + // to check both appId and appUuid since the current service may still + // have appId + return ( + `${this.appUuid}_${networkName}` in this.config.networks || + `${this.appId}_${networkName}` in this.config.networks + ); } public hasNetworkMode(networkName: string) { - return `${this.appId}_${networkName}` === this.config.networkMode; + // We don't know if this service is current or target state so we need + // to check both appId and appUuid since the current service may still + // have appId + return ( + `${this.appUuid}_${networkName}` === this.config.networkMode || + `${this.appId}_${networkName}` === this.config.networkMode + ); } public hasVolume(volumeName: string) { diff --git a/test/28-db-format.spec.ts b/test/28-db-format.spec.ts index 1a7b8223..16cb9260 100644 --- a/test/28-db-format.spec.ts +++ b/test/28-db-format.spec.ts @@ -12,7 +12,7 @@ import { withMockerode } from './lib/mockerode'; function getDefaultNetwork(appId: number) { return { - default: Network.fromComposeObject('default', appId, {}), + default: Network.fromComposeObject('default', appId, 'deadbeef', {}), }; } diff --git a/test/data/docker-states/entrypoint/inspect.json b/test/data/docker-states/entrypoint/inspect.json index 51301aff..9699d7f3 100644 --- a/test/data/docker-states/entrypoint/inspect.json +++ b/test/data/docker-states/entrypoint/inspect.json @@ -42,7 +42,7 @@ "Type": "journald", "Config": {} }, - "NetworkMode": "1011165_default", + "NetworkMode": "aaaaaaaa_default", "PortBindings": {}, "RestartPolicy": { "Name": "always", @@ -209,7 +209,7 @@ "IPv6Gateway": "", "MacAddress": "", "Networks": { - "1011165_default": { + "aaaaaaaa_default": { "IPAMConfig": {}, "Links": null, "Aliases": [ diff --git a/test/data/docker-states/network-mode-service/inspect.json b/test/data/docker-states/network-mode-service/inspect.json index d8a52a4b..34c3f9b8 100644 --- a/test/data/docker-states/network-mode-service/inspect.json +++ b/test/data/docker-states/network-mode-service/inspect.json @@ -209,7 +209,7 @@ "IPv6Gateway": "", "MacAddress": "", "Networks": { - "1011165_default": { + "aaaaaaaa_default": { "IPAMConfig": null, "Links": null, "Aliases": [ diff --git a/test/data/docker-states/simple/inspect.json b/test/data/docker-states/simple/inspect.json index 1da69639..33793785 100644 --- a/test/data/docker-states/simple/inspect.json +++ b/test/data/docker-states/simple/inspect.json @@ -43,7 +43,7 @@ "Type": "journald", "Config": {} }, - "NetworkMode": "1011165_default", + "NetworkMode": "aaaaaaaa_default", "PortBindings": {}, "RestartPolicy": { "Name": "always", @@ -209,7 +209,7 @@ "IPv6Gateway": "", "MacAddress": "", "Networks": { - "1011165_default": { + "aaaaaaaa_default": { "IPAMConfig": null, "Links": null, "Aliases": [ diff --git a/test/lib/mocked-device-api.ts b/test/lib/mocked-device-api.ts index b594c7c4..a111c711 100644 --- a/test/lib/mocked-device-api.ts +++ b/test/lib/mocked-device-api.ts @@ -4,7 +4,6 @@ import rewire = require('rewire'); import { unlinkAll } from '../../src/lib/fs-utils'; import * as applicationManager from '../../src/compose/application-manager'; -import * as networkManager from '../../src/compose/network-manager'; import * as serviceManager from '../../src/compose/service-manager'; import * as volumeManager from '../../src/compose/volume-manager'; import * as commitStore from '../../src/compose/commit'; @@ -185,7 +184,6 @@ function buildRoutes(): Router { } // TO-DO: Create a cleaner way to restore previous values. -const originalNetGetAll = networkManager.getAllByAppId; const originalVolGetAll = volumeManager.getAllByAppId; const originalSvcGetAppId = serviceManager.getAllByAppId; const originalSvcGetStatus = serviceManager.getState; @@ -194,8 +192,6 @@ const originalReadyForUpdates = apiBinder.__get__('readyForUpdates'); function setupStubs() { apiBinder.__set__('readyForUpdates', true); // @ts-expect-error Assigning to a RO property - networkManager.getAllByAppId = async () => STUBBED_VALUES.networks; - // @ts-expect-error Assigning to a RO property volumeManager.getAllByAppId = async () => STUBBED_VALUES.volumes; // @ts-expect-error Assigning to a RO property serviceManager.getState = async () => STUBBED_VALUES.services; @@ -207,8 +203,6 @@ function setupStubs() { function restoreStubs() { apiBinder.__set__('readyForUpdates', originalReadyForUpdates); // @ts-expect-error Assigning to a RO property - networkManager.getAllByAppId = originalNetGetAll; - // @ts-expect-error Assigning to a RO property volumeManager.getAllByAppId = originalVolGetAll; // @ts-expect-error Assigning to a RO property serviceManager.getState = originalSvcGetStatus; diff --git a/test/src/compose/app.spec.ts b/test/src/compose/app.spec.ts index a6fd123f..b5ade6a9 100644 --- a/test/src/compose/app.spec.ts +++ b/test/src/compose/app.spec.ts @@ -26,10 +26,12 @@ function createApp({ volumes = [] as Volume[], isTarget = false, appId = 1, + appUuid = 'appuuid', } = {}) { return new App( { appId, + appUuid, services, networks: networks.reduce( (res, net) => ({ ...res, [net.name]: net }), @@ -44,6 +46,7 @@ function createApp({ async function createService( { appId = 1, + appUuid = 'appuuid', serviceName = 'test', commit = 'test-commit', ...conf @@ -53,6 +56,7 @@ async function createService( const svc = await Service.fromComposeObject( { appId, + appUuid, serviceName, commit, running: true, @@ -71,14 +75,18 @@ async function createService( function createImage( { appId = 1, + appUuid = 'appuuid', dependent = 0, name = 'test-image', serviceName = 'test', + commit = 'test-commit', ...extra } = {} as Partial, ) { return { appId, + appUuid, + commit, dependent, name, serviceName, @@ -107,7 +115,7 @@ function expectNoStep(action: CompositionStepAction, steps: CompositionStep[]) { expectSteps(action, steps, 0, 0); } -const defaultNetwork = Network.fromComposeObject('default', 1, {}); +const defaultNetwork = Network.fromComposeObject('default', 1, 'appuuid', {}); describe('compose/app', () => { before(() => { @@ -323,14 +331,12 @@ describe('compose/app', () => { it('should generate the correct step sequence for a volume purge request', async () => { const service = await createService({ + appId: 1, + appUuid: 'deadbeef', image: 'test-image', composition: { volumes: ['db-volume:/data'] }, }); - const volume = Volume.fromComposeObject( - 'db-volume', - service.appId, - 'deadbeef', - ); + const volume = Volume.fromComposeObject('db-volume', 1, 'deadbeef'); const contextWithImages = { ...defaultContext, ...{ @@ -428,7 +434,7 @@ describe('compose/app', () => { it('should correctly infer a network create step', () => { const current = createApp({ networks: [] }); const target = createApp({ - networks: [Network.fromComposeObject('default', 1, {})], + networks: [Network.fromComposeObject('default', 1, 'deadbeef', {})], isTarget: true, }); @@ -442,7 +448,9 @@ describe('compose/app', () => { it('should correctly infer a network remove step', () => { const current = createApp({ - networks: [Network.fromComposeObject('test-network', 1, {})], + networks: [ + Network.fromComposeObject('test-network', 1, 'deadbeef', {}), + ], isTarget: true, }); const target = createApp({ networks: [], isTarget: true }); @@ -459,8 +467,8 @@ describe('compose/app', () => { it('should correctly infer more than one network removal step', () => { const current = createApp({ networks: [ - Network.fromComposeObject('test-network', 1, {}), - Network.fromComposeObject('test-network-2', 1, {}), + Network.fromComposeObject('test-network', 1, 'deadbeef', {}), + Network.fromComposeObject('test-network-2', 1, 'deadbeef', {}), ], isTarget: true, }); @@ -480,11 +488,13 @@ describe('compose/app', () => { it('should correctly infer a network recreation step', () => { const current = createApp({ - networks: [Network.fromComposeObject('test-network', 1, {})], + networks: [ + Network.fromComposeObject('test-network', 1, 'deadbeef', {}), + ], }); const target = createApp({ networks: [ - Network.fromComposeObject('test-network', 1, { + Network.fromComposeObject('test-network', 1, 'deadbeef', { labels: { TEST: 'TEST' }, }), ], @@ -524,20 +534,28 @@ describe('compose/app', () => { expect(createNetworkStep) .to.have.property('target') .that.has.property('config') - .that.deep.includes({ labels: { TEST: 'TEST' } }); + .that.deep.includes({ + labels: { TEST: 'TEST', 'io.balena.app-id': '1' }, + }); }); it('should kill dependencies of networks before removing', async () => { const current = createApp({ + appUuid: 'deadbeef', services: [ await createService({ - composition: { networks: { 'test-network': {} } }, + appId: 1, + appUuid: 'deadbeef', + composition: { networks: ['test-network'] }, }), ], - networks: [Network.fromComposeObject('test-network', 1, {})], + networks: [ + Network.fromComposeObject('test-network', 1, 'deadbeef', {}), + ], }); const target = createApp({ - services: [await createService()], + appUuid: 'deadbeef', + services: [await createService({ appUuid: 'deadbeef' })], networks: [], isTarget: true, }); @@ -554,10 +572,10 @@ describe('compose/app', () => { const current = createApp({ services: [ await createService({ - composition: { networks: { 'test-network': {} } }, + composition: { networks: ['test-network'] }, }), ], - networks: [Network.fromComposeObject('test-network', 1, {})], + networks: [Network.fromComposeObject('test-network', 1, 'appuuid', {})], }); const target = createApp({ services: [ @@ -566,7 +584,7 @@ describe('compose/app', () => { }), ], networks: [ - Network.fromComposeObject('test-network', 1, { + Network.fromComposeObject('test-network', 1, 'appuuid', { labels: { test: 'test' }, }), ], @@ -599,7 +617,7 @@ describe('compose/app', () => { it('should not create the default network if it already exists', () => { const current = createApp({ - networks: [Network.fromComposeObject('default', 1, {})], + networks: [Network.fromComposeObject('default', 1, 'deadbeef', {})], }); const target = createApp({ networks: [], isTarget: true }); @@ -611,17 +629,17 @@ describe('compose/app', () => { }); describe('service state behavior', () => { - it('should create a kill step for service which is no longer referenced', async () => { + it('should create a kill step for a service which is no longer referenced', async () => { const current = createApp({ services: [ await createService({ appId: 1, serviceName: 'main' }), await createService({ appId: 1, serviceName: 'aux' }), ], - networks: [Network.fromComposeObject('test-network', 1, {})], + networks: [defaultNetwork], }); const target = createApp({ services: [await createService({ appId: 1, serviceName: 'main' })], - networks: [Network.fromComposeObject('test-network', 1, {})], + networks: [defaultNetwork], isTarget: true, }); @@ -827,7 +845,7 @@ describe('compose/app', () => { const intermediate = createApp({ services: [], // Default network was already created - networks: [Network.fromComposeObject('default', 1, {})], + networks: [defaultNetwork], }); // now should see a 'start' @@ -1189,7 +1207,10 @@ describe('compose/app', () => { appId: 1, }), ], - networks: [defaultNetwork, Network.fromComposeObject('test', 1, {})], + networks: [ + defaultNetwork, + Network.fromComposeObject('test', 1, 'appuuid', {}), + ], isTarget: true, }); diff --git a/test/src/compose/application-manager.spec.ts b/test/src/compose/application-manager.spec.ts index aed2f35f..a06020c9 100644 --- a/test/src/compose/application-manager.spec.ts +++ b/test/src/compose/application-manager.spec.ts @@ -15,12 +15,12 @@ import { InstancedAppState } from '../../../src/types/state'; import * as dbHelper from '../../lib/db-helper'; -const DEFAULT_NETWORK = Network.fromComposeObject('default', 1, {}); +const DEFAULT_NETWORK = Network.fromComposeObject('default', 1, 'appuuid', {}); async function createService( { appId = 1, - appUuid = 'app-uuid', + appUuid = 'appuuid', serviceName = 'main', commit = 'main-commit', ...conf @@ -54,7 +54,7 @@ async function createService( function createImage( { appId = 1, - appUuid = 'app-uuid', + appUuid = 'appuuid', name = 'test-image', serviceName = 'main', commit = 'main-commit', @@ -582,7 +582,7 @@ describe('compose/application-manager', () => { await createService({ image: 'main-image', appId: 1, - appUuid: 'app-uuid', + appUuid: 'appuuid', commit: 'new-release', serviceName: 'main', composition: { @@ -592,7 +592,7 @@ describe('compose/application-manager', () => { await createService({ image: 'dep-image', appId: 1, - appUuid: 'app-uuid', + appUuid: 'appuuid', commit: 'new-release', serviceName: 'dep', }), @@ -611,7 +611,7 @@ describe('compose/application-manager', () => { services: [ await createService({ appId: 1, - appUuid: 'app-uuid', + appUuid: 'appuuid', commit: 'old-release', serviceName: 'main', composition: { @@ -620,7 +620,7 @@ describe('compose/application-manager', () => { }), await createService({ appId: 1, - appUuid: 'app-uuid', + appUuid: 'appuuid', commit: 'old-release', serviceName: 'dep', }), @@ -630,14 +630,14 @@ describe('compose/application-manager', () => { // Both images have been downloaded createImage({ appId: 1, - appUuid: 'app-uuid', + appUuid: 'appuuid', name: 'main-image', serviceName: 'main', commit: 'new-release', }), createImage({ appId: 1, - appUuid: 'app-uuid', + appUuid: 'appuuid', name: 'dep-image', serviceName: 'dep', commit: 'new-release', @@ -1202,8 +1202,8 @@ describe('compose/application-manager', () => { ], networks: [ // Default networks for two apps - Network.fromComposeObject('default', 1, {}), - Network.fromComposeObject('default', 2, {}), + Network.fromComposeObject('default', 1, 'app-one', {}), + Network.fromComposeObject('default', 2, 'app-two', {}), ], }, true, @@ -1217,8 +1217,8 @@ describe('compose/application-manager', () => { services: [], networks: [ // Default networks for two apps - Network.fromComposeObject('default', 1, {}), - Network.fromComposeObject('default', 2, {}), + Network.fromComposeObject('default', 1, 'app-one', {}), + Network.fromComposeObject('default', 2, 'app-two', {}), ], images: [ createImage({ diff --git a/test/src/compose/network.spec.ts b/test/src/compose/network.spec.ts index 59815f43..6944c9d9 100644 --- a/test/src/compose/network.spec.ts +++ b/test/src/compose/network.spec.ts @@ -10,10 +10,16 @@ import { log } from '../../../src/lib/supervisor-console'; describe('compose/network', () => { describe('creating a network from a compose object', () => { it('creates a default network configuration if no config is given', () => { - const network = Network.fromComposeObject('default', 12345, {}); + const network = Network.fromComposeObject( + 'default', + 12345, + 'deadbeef', + {}, + ); expect(network.name).to.equal('default'); expect(network.appId).to.equal(12345); + expect(network.appUuid).to.equal('deadbeef'); // Default configuration options expect(network.config.driver).to.equal('bridge'); @@ -23,12 +29,14 @@ describe('compose/network', () => { options: {}, }); expect(network.config.enableIPv6).to.equal(false); - expect(network.config.labels).to.deep.equal({}); + expect(network.config.labels).to.deep.equal({ + 'io.balena.app-id': '12345', + }); expect(network.config.options).to.deep.equal({}); }); it('normalizes legacy labels', () => { - const network = Network.fromComposeObject('default', 12345, { + const network = Network.fromComposeObject('default', 12345, 'deadbeef', { labels: { 'io.resin.features.something': '1234', }, @@ -36,11 +44,12 @@ describe('compose/network', () => { expect(network.config.labels).to.deep.equal({ 'io.balena.features.something': '1234', + 'io.balena.app-id': '12345', }); }); it('accepts valid IPAM configurations', () => { - const network0 = Network.fromComposeObject('default', 12345, { + const network0 = Network.fromComposeObject('default', 12345, 'deadbeef', { ipam: { driver: 'dummy', config: [], options: {} }, }); @@ -51,7 +60,7 @@ describe('compose/network', () => { options: {}, }); - const network1 = Network.fromComposeObject('default', 12345, { + const network1 = Network.fromComposeObject('default', 12345, 'deadbeef', { ipam: { driver: 'default', config: [ @@ -84,7 +93,7 @@ describe('compose/network', () => { it('warns about IPAM configuration without both gateway and subnet', () => { const logSpy = sinon.spy(log, 'warn'); - Network.fromComposeObject('default', 12345, { + Network.fromComposeObject('default', 12345, 'deadbeef', { ipam: { driver: 'default', config: [ @@ -103,7 +112,7 @@ describe('compose/network', () => { logSpy.resetHistory(); - Network.fromComposeObject('default', 12345, { + Network.fromComposeObject('default', 12345, 'deadbeef', { ipam: { driver: 'default', config: [ @@ -124,7 +133,7 @@ describe('compose/network', () => { }); it('parses values from a compose object', () => { - const network1 = Network.fromComposeObject('default', 12345, { + const network1 = Network.fromComposeObject('default', 12345, 'deadbeef', { driver: 'bridge', enable_ipv6: true, internal: false, @@ -171,6 +180,7 @@ describe('compose/network', () => { expect(dockerConfig.Labels).to.deep.equal({ 'io.balena.supervised': 'true', + 'io.balena.app-id': '12345', 'com.docker.some-label': 'yes', }); @@ -209,9 +219,17 @@ describe('compose/network', () => { Name: '1234', } as NetworkInspectInfo), ).to.throw(); + + expect(() => + Network.fromDockerNetwork({ + Id: 'deadbeef', + Name: 'a173bdb734884b778f5cc3dffd18733e_default', + Labels: {}, // no app-id + } as NetworkInspectInfo), + ).to.throw(); }); - it('creates a network object from a docker network configuration', () => { + it('creates a network object from a legacy docker network configuration', () => { const network = Network.fromDockerNetwork({ Id: 'deadbeef', Name: '1234_default', @@ -233,6 +251,7 @@ describe('compose/network', () => { 'com.docker.some-option': 'abcd', } as NetworkInspectInfo['Options'], Labels: { + 'io.balena.supervised': 'true', 'io.balena.features.something': '123', } as NetworkInspectInfo['Labels'], } as NetworkInspectInfo); @@ -257,6 +276,56 @@ describe('compose/network', () => { }); }); + it('creates a network object from a docker network configuration', () => { + const network = Network.fromDockerNetwork({ + Id: 'deadbeef', + Name: 'a173bdb734884b778f5cc3dffd18733e_default', + Driver: 'bridge', + EnableIPv6: true, + IPAM: { + Driver: 'default', + Options: {}, + Config: [ + { + Subnet: '172.18.0.0/16', + Gateway: '172.18.0.1', + }, + ], + } as NetworkInspectInfo['IPAM'], + Internal: true, + Containers: {}, + Options: { + 'com.docker.some-option': 'abcd', + } as NetworkInspectInfo['Options'], + Labels: { + 'io.balena.supervised': 'true', + 'io.balena.features.something': '123', + 'io.balena.app-id': '1234', + } as NetworkInspectInfo['Labels'], + } as NetworkInspectInfo); + + expect(network.appId).to.equal(1234); + expect(network.appUuid).to.equal('a173bdb734884b778f5cc3dffd18733e'); + expect(network.name).to.equal('default'); + expect(network.config.enableIPv6).to.equal(true); + expect(network.config.ipam.driver).to.equal('default'); + expect(network.config.ipam.options).to.deep.equal({}); + expect(network.config.ipam.config).to.deep.equal([ + { + subnet: '172.18.0.0/16', + gateway: '172.18.0.1', + }, + ]); + expect(network.config.internal).to.equal(true); + expect(network.config.options).to.deep.equal({ + 'com.docker.some-option': 'abcd', + }); + expect(network.config.labels).to.deep.equal({ + 'io.balena.features.something': '123', + 'io.balena.app-id': '1234', + }); + }); + it('normalizes legacy label names and excludes supervised label', () => { const network = Network.fromDockerNetwork({ Id: 'deadbeef', @@ -284,7 +353,7 @@ describe('compose/network', () => { it('creates a docker compose network object from the internal network config', () => { const network = Network.fromDockerNetwork({ Id: 'deadbeef', - Name: '1234_default', + Name: 'a173bdb734884b778f5cc3dffd18733e_default', Driver: 'bridge', EnableIPv6: true, IPAM: { @@ -304,9 +373,13 @@ describe('compose/network', () => { } as NetworkInspectInfo['Options'], Labels: { 'io.balena.features.something': '123', + 'io.balena.app-id': '12345', } as NetworkInspectInfo['Labels'], } as NetworkInspectInfo); + expect(network.appId).to.equal(12345); + expect(network.appUuid).to.equal('a173bdb734884b778f5cc3dffd18733e'); + // Convert to compose object const compose = network.toComposeObject(); expect(compose.driver).to.equal('bridge'); @@ -327,23 +400,26 @@ describe('compose/network', () => { }); expect(compose.labels).to.deep.equal({ 'io.balena.features.something': '123', + 'io.balena.app-id': '12345', }); }); }); describe('generateDockerName', () => { - it('creates a proper network name from the user given name and the app id', () => { - expect(Network.generateDockerName(12345, 'default')).to.equal( - '12345_default', + it('creates a proper network name from the user given name and the app uuid', () => { + expect(Network.generateDockerName('deadbeef', 'default')).to.equal( + 'deadbeef_default', + ); + expect(Network.generateDockerName('deadbeef', 'bleh')).to.equal( + 'deadbeef_bleh', ); - expect(Network.generateDockerName(12345, 'bleh')).to.equal('12345_bleh'); expect(Network.generateDockerName(1, 'default')).to.equal('1_default'); }); }); describe('comparing network configurations', () => { it('ignores IPAM configuration', () => { - const network = Network.fromComposeObject('default', 12345, { + const network = Network.fromComposeObject('default', 12345, 'deadbeef', { ipam: { driver: 'default', config: [ @@ -357,13 +433,15 @@ describe('compose/network', () => { }, }); expect( - network.isEqualConfig(Network.fromComposeObject('default', 12345, {})), + network.isEqualConfig( + Network.fromComposeObject('default', 12345, 'deadbeef', {}), + ), ).to.be.true; // Only ignores ipam.config, not other ipam elements expect( network.isEqualConfig( - Network.fromComposeObject('default', 12345, { + Network.fromComposeObject('default', 12345, 'deadbeef', { ipam: { driver: 'aaa' }, }), ), @@ -372,26 +450,61 @@ describe('compose/network', () => { it('compares configurations recursively', () => { expect( - Network.fromComposeObject('default', 12345, {}).isEqualConfig( - Network.fromComposeObject('default', 12345, {}), + Network.fromComposeObject( + 'default', + 12345, + 'deadbeef', + {}, + ).isEqualConfig( + Network.fromComposeObject('default', 12345, 'deadbeef', {}), ), ).to.be.true; expect( - Network.fromComposeObject('default', 12345, { + Network.fromComposeObject('default', 12345, 'deadbeef', { driver: 'default', - }).isEqualConfig(Network.fromComposeObject('default', 12345, {})), + }).isEqualConfig( + Network.fromComposeObject('default', 12345, 'deadbeef', {}), + ), ).to.be.false; expect( - Network.fromComposeObject('default', 12345, { + Network.fromComposeObject('default', 12345, 'deadbeef', { enable_ipv6: true, - }).isEqualConfig(Network.fromComposeObject('default', 12345, {})), + }).isEqualConfig( + Network.fromComposeObject('default', 12345, 'deadbeef', {}), + ), ).to.be.false; expect( - Network.fromComposeObject('default', 12345, { + Network.fromComposeObject('default', 12345, 'deadbeef', { enable_ipv6: false, internal: false, }).isEqualConfig( - Network.fromComposeObject('default', 12345, { internal: true }), + Network.fromComposeObject('default', 12345, 'deadbeef', { + internal: true, + }), + ), + ).to.be.false; + + // Comparison of a network without the app-uuid and a network + // with uuid has to return false + expect( + Network.fromComposeObject( + 'default', + 12345, + 'deadbeef', + {}, + ).isEqualConfig( + Network.fromDockerNetwork({ + Id: 'deadbeef', + Name: '12345_default', + IPAM: { + Driver: 'default', + Options: {}, + Config: [], + } as NetworkInspectInfo['IPAM'], + Labels: { + 'io.balena.supervised': 'true', + } as NetworkInspectInfo['Labels'], + } as NetworkInspectInfo), ), ).to.be.false; }); @@ -400,26 +513,31 @@ describe('compose/network', () => { describe('creating networks', () => { it('creates a new network on the engine with the given data', async () => { await withMockerode(async (mockerode) => { - const network = Network.fromComposeObject('default', 12345, { - ipam: { - driver: 'default', - config: [ - { - subnet: '172.20.0.0/16', - ip_range: '172.20.10.0/24', - gateway: '172.20.0.1', - }, - ], - options: {}, + const network = Network.fromComposeObject( + 'default', + 12345, + 'deadbeef', + { + ipam: { + driver: 'default', + config: [ + { + subnet: '172.20.0.0/16', + ip_range: '172.20.10.0/24', + gateway: '172.20.0.1', + }, + ], + options: {}, + }, }, - }); + ); // Create the network await network.create(); // Check that the create function was called with proper arguments expect(mockerode.createNetwork).to.have.been.calledOnceWith({ - Name: '12345_default', + Name: 'deadbeef_default', Driver: 'bridge', CheckDuplicate: true, IPAM: { @@ -437,6 +555,7 @@ describe('compose/network', () => { Internal: false, Labels: { 'io.balena.supervised': 'true', + 'io.balena.app-id': '12345', }, Options: {}, }); @@ -445,19 +564,24 @@ describe('compose/network', () => { it('throws the error if there is a problem while creating the network', async () => { await withMockerode(async (mockerode) => { - const network = Network.fromComposeObject('default', 12345, { - ipam: { - driver: 'default', - config: [ - { - subnet: '172.20.0.0/16', - ip_range: '172.20.10.0/24', - gateway: '172.20.0.1', - }, - ], - options: {}, + const network = Network.fromComposeObject( + 'default', + 12345, + 'deadbeef', + { + ipam: { + driver: 'default', + config: [ + { + subnet: '172.20.0.0/16', + ip_range: '172.20.10.0/24', + gateway: '172.20.0.1', + }, + ], + options: {}, + }, }, - }); + ); // Re-define the dockerode.createNetwork to throw mockerode.createNetwork.rejects('Unknown engine error'); @@ -471,10 +595,10 @@ describe('compose/network', () => { }); describe('removing a network', () => { - it('removes the network from the engine if it exists', async () => { + it('removes the legacy network from the engine if it exists', async () => { // Create a mock network to add to the mock engine const dockerNetwork = createNetwork({ - Id: 'deadbeef', + Id: 'aaaaaaa', Name: '12345_default', }); @@ -484,7 +608,48 @@ describe('compose/network', () => { expect(await mockerode.listNetworks()).to.have.lengthOf(1); // Create a dummy network object - const network = Network.fromComposeObject('default', 12345, {}); + const network = Network.fromComposeObject( + 'default', + 12345, + 'deadbeef', + {}, + ); + + // Perform the operation + await network.remove(); + + // The removal step should delete the object from the engine data + expect(mockerode.removeNetwork).to.have.been.calledOnceWith( + 'aaaaaaa', + ); + }, + { networks: [dockerNetwork] }, + ); + }); + + it('removes the network from the engine if it exists', async () => { + // Create a mock network to add to the mock engine + const dockerNetwork = createNetwork({ + Id: 'deadbeef', + Name: 'a173bdb734884b778f5cc3dffd18733e_default', + Labels: { + 'io.balena.supervised': 'true', + 'io.balena.app-id': '12345', + }, + }); + + await withMockerode( + async (mockerode) => { + // Check that the engine has the network + expect(await mockerode.listNetworks()).to.have.lengthOf(1); + + // Create a dummy network object + const network = Network.fromComposeObject( + 'default', + 12345, + 'a173bdb734884b778f5cc3dffd18733e', + {}, + ); // Perform the operation await network.remove(); @@ -501,7 +666,7 @@ describe('compose/network', () => { it('ignores the request if the given network does not exist on the engine', async () => { // Create a mock network to add to the mock engine const mockNetwork = createNetwork({ - Id: 'deadbeef', + Id: 'aaaaaaaa', Name: 'some_network', }); @@ -511,7 +676,12 @@ describe('compose/network', () => { expect(await mockerode.listNetworks()).to.have.lengthOf(1); // Create a dummy network object - const network = Network.fromComposeObject('default', 12345, {}); + const network = Network.fromComposeObject( + 'default', + 12345, + 'deadbeef', + {}, + ); // This should not fail await expect(network.remove()).to.not.be.rejected; @@ -526,18 +696,29 @@ describe('compose/network', () => { it('throws the error if there is a problem while removing the network', async () => { // Create a mock network to add to the mock engine const mockNetwork = createNetwork({ - Id: 'deadbeef', - Name: '12345_default', + Id: 'aaaaaaaa', + Name: 'a173bdb734884b778f5cc3dffd18733e_default', + Labels: { + 'io.balena.app-id': '12345', + }, }); await withMockerode( async (mockerode) => { // We can change the return value of the mockerode removeNetwork // to have the remove operation fail - mockerode.removeNetwork.throws('Failed to remove the network'); + mockerode.removeNetwork.throws({ + statusCode: 500, + message: 'Failed to remove the network', + }); // Create a dummy network object - const network = Network.fromComposeObject('default', 12345, {}); + const network = Network.fromComposeObject( + 'default', + 12345, + 'a173bdb734884b778f5cc3dffd18733e', + {}, + ); await expect(network.remove()).to.be.rejected; }, diff --git a/test/src/compose/service.spec.ts b/test/src/compose/service.spec.ts index c491bec9..ebf6ac1b 100644 --- a/test/src/compose/service.spec.ts +++ b/test/src/compose/service.spec.ts @@ -429,6 +429,7 @@ describe('compose/service', () => { await Service.fromComposeObject( { appId: 123456, + appUuid: 'deadbeef', serviceId: 123456, serviceName: 'test', composition: { @@ -448,7 +449,7 @@ describe('compose/service', () => { ).toDockerContainer({ deviceName: 'foo' } as any).NetworkingConfig, ).to.deep.equal({ EndpointsConfig: { - '123456_balena': { + deadbeef_balena: { IPAMConfig: { IPv4Address: '1.2.3.4', }, @@ -470,7 +471,7 @@ describe('compose/service', () => { ).toDockerContainer({ deviceName: 'foo' } as any).NetworkingConfig, ).to.deep.equal({ EndpointsConfig: { - '123456_balena': { + deadbeef_balena: { IPAMConfig: { IPv4Address: '1.2.3.4', IPv6Address: '5.6.7.8',