From f75b8aad2bc7661de803b80dc7008de9a2c6a6af Mon Sep 17 00:00:00 2001 From: Cameron Diver Date: Tue, 29 Oct 2019 11:56:40 +0000 Subject: [PATCH] Support optional containers based on their contract Change-type: minor Signed-off-by: Cameron Diver --- src/application-manager.coffee | 19 +- src/lib/contracts.ts | 78 ++++++--- test/24-contracts.ts | 306 ++++++++++++++++++++++----------- 3 files changed, 271 insertions(+), 132 deletions(-) diff --git a/src/application-manager.coffee b/src/application-manager.coffee index db194c27..25f93b37 100644 --- a/src/application-manager.coffee +++ b/src/application-manager.coffee @@ -689,7 +689,8 @@ module.exports = class ApplicationManager extends EventEmitter # to the database, overwriting the current release. This # is because if we just reject the release, but leave it # in the db, if for any reason the current state stops - # running, we won't restart it, leaving the device useless + # running, we won't restart it, leaving the device + # useless contractsFulfilled = _.mapValues apps, (app) -> serviceContracts = {} _.each app.services, (s) -> @@ -698,12 +699,16 @@ module.exports = class ApplicationManager extends EventEmitter validateContract(s.contract) catch e throw new ContractValidationError(s.serviceName, e.message) - serviceContracts[s.serviceName] = s.contract + serviceContracts[s.serviceName] = + contract: s.contract, + optional: checkTruthy(s.labels['io.balena.features.optional']) ? false + else + serviceContracts[s.serviceName] = { contract: null, optional: false } if !_.isEmpty(serviceContracts) containerContractsFulfilled(serviceContracts) else - { valid: true } + { valid: true, fulfilledServices: _.map(app.services, 'serviceName') } setInTransaction = (filteredApps, trx) => @@ -732,10 +737,16 @@ module.exports = class ApplicationManager extends EventEmitter contractViolators = {} Promise.props(contractsFulfilled).then (fulfilledContracts) -> filteredApps = _.cloneDeep(apps) - _.each fulfilledContracts, ({ valid, unmetServices }, appId) -> + _.each fulfilledContracts, ({ valid, unmetServices, fulfilledServices }, appId) -> if not valid contractViolators[apps[appId].name] = unmetServices delete filteredApps[appId] + else + # valid is true, but we could still be missing + # some optional containers, and need to filter + # these out of the target state + filteredApps[appId].services = _.pickBy filteredApps[appId].services, ({ serviceName }) -> + fulfilledServices.includes(serviceName) if trx? setInTransaction(filteredApps, trx) else diff --git a/src/lib/contracts.ts b/src/lib/contracts.ts index 382670f1..78220b28 100644 --- a/src/lib/contracts.ts +++ b/src/lib/contracts.ts @@ -13,13 +13,20 @@ import supervisorVersion = require('./supervisor-version'); export { ContractObject }; export interface ServiceContracts { - [serviceName: string]: ContractObject; + [serviceName: string]: { contract?: ContractObject; optional: boolean }; } export async function containerContractsFulfilled( serviceContracts: ServiceContracts, -): Promise<{ valid: boolean; unmetServices: string[] }> { - const containers = _.values(serviceContracts); +): Promise<{ + valid: boolean; + unmetServices: string[]; + fulfilledServices: string[]; +}> { + const containers = _(serviceContracts) + .map('contract') + .compact() + .value(); const osContract = new Contract({ slug: 'balenaOS', @@ -51,11 +58,11 @@ export async function containerContractsFulfilled( type: 'meta.universe', }); - universe.addChildren( - [osContract, supervisorContract].concat( - containers.map(c => new Contract(c)), - ), - ); + universe.addChildren([ + osContract, + supervisorContract, + ...containers.map(c => new Contract(c)), + ]); const solution = blueprint.reproduce(universe); @@ -65,7 +72,11 @@ export async function containerContractsFulfilled( ); } if (solution.length === 0) { - return { valid: false, unmetServices: _.keys(serviceContracts) }; + return { + valid: false, + unmetServices: _.keys(serviceContracts), + fulfilledServices: [], + }; } // Detect how many containers are present in the resulting @@ -75,23 +86,42 @@ export async function containerContractsFulfilled( }); if (children.length === containers.length) { - return { valid: true, unmetServices: [] }; + return { + valid: true, + unmetServices: [], + fulfilledServices: _.keys(serviceContracts), + }; } else { - // Work out which service violated the contracts they - // provided - const unmetServices = _(serviceContracts) - .map((contract, serviceName) => { - const found = _.find(children, child => { - return _.isEqual((child as any).raw, contract); - }); - if (found == null) { - return serviceName; + // If we got here, it means that at least one of the + // container contracts was not fulfilled. If *all* of + // those containers whose contract was not met are + // marked as optional, the target state is still valid, + // but we ignore the optional containers + + const [fulfilledServices, unfulfilledServices] = _.partition( + _.keys(serviceContracts), + serviceName => { + const { contract } = serviceContracts[serviceName]; + if (!contract) { + return true; } - return; - }) - .filter(n => n != null) - .value() as string[]; - return { valid: false, unmetServices }; + // Did we find the contract in the generated state? + return _.some(children, child => + _.isEqual((child as any).raw, contract), + ); + }, + ); + + const valid = !_.some( + unfulfilledServices, + svcName => !serviceContracts[svcName].optional, + ); + + return { + valid, + unmetServices: unfulfilledServices, + fulfilledServices, + }; } } diff --git a/test/24-contracts.ts b/test/24-contracts.ts index 2c128e6c..23350a4a 100644 --- a/test/24-contracts.ts +++ b/test/24-contracts.ts @@ -98,8 +98,11 @@ describe('Container contracts', () => { expect( await containerContractsFulfilled({ service: { - type: 'sw.container', - slug: 'user-container', + contract: { + type: 'sw.container', + slug: 'user-container', + }, + optional: false, }, }), ) @@ -108,12 +111,18 @@ describe('Container contracts', () => { expect( await containerContractsFulfilled({ service: { - type: 'sw.container', - slug: 'user-container1', + contract: { + type: 'sw.container', + slug: 'user-container1', + }, + optional: false, }, service2: { - type: 'sw.container', - slug: 'user-container2', + contract: { + type: 'sw.container', + slug: 'user-container2', + }, + optional: false, }, }), ) @@ -125,15 +134,18 @@ describe('Container contracts', () => { expect( await containerContractsFulfilled({ service: { - type: 'sw.container', - name: 'user-container', - slug: 'user-container', - requires: [ - { - type: 'sw.os', - version: '>2.0.0', - }, - ], + contract: { + type: 'sw.container', + name: 'user-container', + slug: 'user-container', + requires: [ + { + type: 'sw.os', + version: '>2.0.0', + }, + ], + }, + optional: false, }, }), ) @@ -143,15 +155,18 @@ describe('Container contracts', () => { expect( await containerContractsFulfilled({ service: { - type: 'sw.container', - name: 'user-container', - slug: 'user-container', - requires: [ - { - type: 'sw.supervisor', - version: `<${supervisorVersionGreater}`, - }, - ], + contract: { + type: 'sw.container', + name: 'user-container', + slug: 'user-container', + requires: [ + { + type: 'sw.supervisor', + version: `<${supervisorVersionGreater}`, + }, + ], + }, + optional: false, }, }), ) @@ -161,15 +176,18 @@ describe('Container contracts', () => { expect( await containerContractsFulfilled({ service: { - type: 'sw.container', - name: 'user-container', - slug: 'user-container', - requires: [ - { - type: 'sw.supervisor', - version: `>${supervisorVersionLesser}`, - }, - ], + contract: { + type: 'sw.container', + name: 'user-container', + slug: 'user-container', + requires: [ + { + type: 'sw.supervisor', + version: `>${supervisorVersionLesser}`, + }, + ], + }, + optional: false, }, }), ) @@ -179,19 +197,22 @@ describe('Container contracts', () => { expect( await containerContractsFulfilled({ service: { - type: 'sw.container', - name: 'user-container', - slug: 'user-container', - requires: [ - { - type: 'sw.supervisor', - version: `>${supervisorVersionLesser}`, - }, - { - type: 'sw.os', - version: '<3.0.0', - }, - ], + contract: { + type: 'sw.container', + name: 'user-container', + slug: 'user-container', + requires: [ + { + type: 'sw.supervisor', + version: `>${supervisorVersionLesser}`, + }, + { + type: 'sw.os', + version: '<3.0.0', + }, + ], + }, + optional: false, }, }), ) @@ -200,26 +221,32 @@ describe('Container contracts', () => { expect( await containerContractsFulfilled({ service: { - type: 'sw.container', - name: 'user-container1', - slug: 'user-container1', - requires: [ - { - type: 'sw.supervisor', - version: `>${supervisorVersionLesser}`, - }, - ], + contract: { + type: 'sw.container', + name: 'user-container1', + slug: 'user-container1', + requires: [ + { + type: 'sw.supervisor', + version: `>${supervisorVersionLesser}`, + }, + ], + }, + optional: false, }, service2: { - type: 'sw.container', - name: 'user-container1', - slug: 'user-container1', - requires: [ - { - type: 'sw.os', - version: '<3.0.0', - }, - ], + contract: { + type: 'sw.container', + name: 'user-container1', + slug: 'user-container1', + requires: [ + { + type: 'sw.os', + version: '<3.0.0', + }, + ], + }, + optional: false, }, }), ) @@ -230,15 +257,18 @@ describe('Container contracts', () => { it('Should refuse to run containers whose requirements are not satisfied', async () => { let fulfilled = await containerContractsFulfilled({ service: { - type: 'sw.container', - name: 'user-container', - slug: 'user-container', - requires: [ - { - type: 'sw.os', - version: '>=3.0.0', - }, - ], + contract: { + type: 'sw.container', + name: 'user-container', + slug: 'user-container', + requires: [ + { + type: 'sw.os', + version: '>=3.0.0', + }, + ], + }, + optional: false, }, }); expect(fulfilled) @@ -250,19 +280,22 @@ describe('Container contracts', () => { fulfilled = await containerContractsFulfilled({ service2: { - type: 'sw.container', - name: 'user-container2', - slug: 'user-container2', - requires: [ - { - type: 'sw.supervisor', - version: `>=${supervisorVersionLesser}`, - }, - { - type: 'sw.os', - version: '>3.0.0', - }, - ], + contract: { + type: 'sw.container', + name: 'user-container2', + slug: 'user-container2', + requires: [ + { + type: 'sw.supervisor', + version: `>=${supervisorVersionLesser}`, + }, + { + type: 'sw.os', + version: '>3.0.0', + }, + ], + }, + optional: false, }, }); expect(fulfilled) @@ -274,26 +307,32 @@ describe('Container contracts', () => { fulfilled = await containerContractsFulfilled({ service: { - type: 'sw.container', - name: 'user-container1', - slug: 'user-container1', - requires: [ - { - type: 'sw.supervisor', - version: `>=${supervisorVersionLesser}`, - }, - ], + contract: { + type: 'sw.container', + name: 'user-container1', + slug: 'user-container1', + requires: [ + { + type: 'sw.supervisor', + version: `>=${supervisorVersionLesser}`, + }, + ], + }, + optional: false, }, service2: { - type: 'sw.container', - name: 'user-container2', - slug: 'user-container2', - requires: [ - { - type: 'sw.supervisor', - version: `<=${supervisorVersionLesser}`, - }, - ], + contract: { + type: 'sw.container', + name: 'user-container2', + slug: 'user-container2', + requires: [ + { + type: 'sw.supervisor', + version: `<=${supervisorVersionLesser}`, + }, + ], + }, + optional: false, }, }); expect(fulfilled) @@ -303,5 +342,64 @@ describe('Container contracts', () => { .to.have.property('unmetServices') .that.deep.equals(['service2']); }); + + describe('Optional containers', () => { + it('should correctly run passing optional containers', async () => { + const { + valid, + unmetServices, + fulfilledServices, + } = await containerContractsFulfilled({ + service1: { + contract: { + type: 'sw.container', + slug: 'service1', + requires: [ + { + type: 'sw.os', + version: `<${supervisorVersionGreater}`, + }, + ], + }, + optional: true, + }, + }); + expect(valid).to.equal(true); + expect(unmetServices).to.deep.equal([]); + expect(fulfilledServices).to.deep.equal(['service1']); + }); + + it('should corrrectly omit failing optional containers', async () => { + const { + valid, + unmetServices, + fulfilledServices, + } = await containerContractsFulfilled({ + service1: { + contract: { + type: 'sw.container', + slug: 'service1', + requires: [ + { + type: 'sw.os', + version: `>${supervisorVersionGreater}`, + }, + ], + }, + optional: true, + }, + service2: { + contract: { + type: 'sw.container', + slug: 'service2', + }, + optional: false, + }, + }); + expect(valid).to.equal(true); + expect(unmetServices).to.deep.equal(['service1']); + expect(fulfilledServices).to.deep.equal(['service2']); + }); + }); }); });