diff --git a/package-lock.json b/package-lock.json index d05d6f2f8..be1cf513e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -102,7 +102,7 @@ "ts-node": "^10.0.0", "tsconfig-paths": "^4.1.0", "typed-error": "^3.2.1", - "typescript": "^5.3.3", + "typescript": "^5.5.4", "webpack": "^5.74.0", "webpack-cli": "^5.0.0", "winston": "^3.3.3", @@ -13520,9 +13520,9 @@ "dev": true }, "node_modules/typescript": { - "version": "5.4.5", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.4.5.tgz", - "integrity": "sha512-vcI4UpRgg81oIRUFwR0WSIHKt11nJ7SAVlYNIu+QpqeyXP+gpQJy/Z4+F0aGxSE4MqwjyXvW/TzgkLAx2AGHwQ==", + "version": "5.5.4", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.5.4.tgz", + "integrity": "sha512-Mtq29sKDAEYP7aljRgtPOpTvOfbwRWlS6dPRzwjdE+C0R4brX/GUyhHSecbHMFLNBLcJIPt9nl9yG5TZ1weH+Q==", "dev": true, "bin": { "tsc": "bin/tsc", diff --git a/package.json b/package.json index dd1a79804..5bed58077 100644 --- a/package.json +++ b/package.json @@ -128,7 +128,7 @@ "ts-node": "^10.0.0", "tsconfig-paths": "^4.1.0", "typed-error": "^3.2.1", - "typescript": "^5.3.3", + "typescript": "^5.5.4", "webpack": "^5.74.0", "webpack-cli": "^5.0.0", "winston": "^3.3.3", diff --git a/src/api-binder/index.ts b/src/api-binder/index.ts index fe67b27ac..887b882a3 100644 --- a/src/api-binder/index.ts +++ b/src/api-binder/index.ts @@ -9,12 +9,11 @@ import * as deviceConfig from '../device-config'; import * as eventTracker from '../event-tracker'; import { loadBackupFromMigration } from '../lib/migration'; +import { InternalInconsistencyError, TargetStateError } from '../lib/errors'; import { ContractValidationError, ContractViolationError, - InternalInconsistencyError, - TargetStateError, -} from '../lib/errors'; +} from '../lib/contracts'; import log from '../lib/supervisor-console'; diff --git a/src/compose/app.ts b/src/compose/app.ts index bcf406f14..03bde90e4 100644 --- a/src/compose/app.ts +++ b/src/compose/app.ts @@ -38,6 +38,7 @@ export interface AppConstructOpts { commit?: string; source?: string; isHost?: boolean; + isRejected?: boolean; services: Service[]; volumes: Volume[]; @@ -57,6 +58,7 @@ class AppImpl implements App { public commit?: string; public source?: string; public isHost?: boolean; + public isRejected?: boolean; // Services are stored as an array, as at any one time we could have more than one // service for a single service ID running (for example handover) public services: Service[]; @@ -77,6 +79,10 @@ class AppImpl implements App { this.networks = opts.networks; this.isHost = !!opts.isHost; + if (isTargetState) { + this.isRejected = !!opts.isRejected; + } + if ( this.networks.find((n) => n.name === 'default') == null && isTargetState @@ -1054,6 +1060,7 @@ class AppImpl implements App { appName: app.name, source: app.source, isHost: app.isHost, + isRejected: app.rejected, services, volumes, networks, diff --git a/src/compose/application-manager.ts b/src/compose/application-manager.ts index 937d382d8..cbf013e56 100644 --- a/src/compose/application-manager.ts +++ b/src/compose/application-manager.ts @@ -4,18 +4,14 @@ import type StrictEventEmitter from 'strict-event-emitter-types'; import * as config from '../config'; import type { Transaction } from '../db'; -import { transaction } from '../db'; import * as logger from '../logger'; import LocalModeManager from '../local-mode'; import * as dbFormat from '../device-state/db-format'; -import { validateTargetContracts } from '../lib/contracts'; +import * as contracts from '../lib/contracts'; import * as constants from '../lib/constants'; import log from '../lib/supervisor-console'; -import { - ContractViolationError, - InternalInconsistencyError, -} from '../lib/errors'; +import { InternalInconsistencyError } from '../lib/errors'; import { getServicesLockedByAppId, LocksTakenMap } from '../lib/update-lock'; import { checkTruthy } from '../lib/validation'; @@ -195,8 +191,19 @@ export async function inferNextSteps( // We want to remove images before moving on to anything else if (steps.length === 0) { - const targetAndCurrent = _.intersection(currentAppIds, targetAppIds); - const onlyTarget = _.difference(targetAppIds, currentAppIds); + // We only want to modify existing apps for accepted targets + const acceptedTargetAppIds = targetAppIds.filter( + (id) => !targetApps[id].isRejected, + ); + + const targetAndCurrent = _.intersection( + currentAppIds, + acceptedTargetAppIds, + ); + const onlyTarget = _.difference(acceptedTargetAppIds, currentAppIds); + + // We do not want to remove rejected apps, so we compare with the + // original target id list const onlyCurrent = _.difference(currentAppIds, targetAppIds); // For apps that exist in both current and target state, calculate what we need to @@ -503,87 +510,78 @@ export async function executeStep( export async function setTarget( apps: TargetApps, source: string, - maybeTrx?: Transaction, + trx: Transaction, ) { const setInTransaction = async ( - $filteredApps: TargetApps, - trx: Transaction, + $apps: TargetApps, + $rejectedApps: string[], + $trx: Transaction, ) => { - await dbFormat.setApps($filteredApps, source, trx); - await trx('app') + await dbFormat.setApps($apps, source, $rejectedApps, $trx); + await $trx('app') .where({ source }) .whereNotIn( 'appId', - // Use apps here, rather than filteredApps, to - // avoid removing a release from the database - // without an application to replace it. - // Currently this will only happen if the release - // which would replace it fails a contract - // validation check - Object.values(apps).map(({ id: appId }) => appId), + // Delete every appId not in the target list + Object.values($apps).map(({ id: appId }) => appId), ) .del(); }; - // We look at the container contracts here, as if we - // cannot run the release, we don't want it to be added - // 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 - The exception to this rule is when the only - // failing services are marked as optional, then we - // filter those out and add the target state to the database - const contractViolators: { [appName: string]: string[] } = {}; - const fulfilledContracts = validateTargetContracts(apps); + // We look at the container contracts here, apps with failing contract requirements + // are stored in the database with a `rejected: true property`, which tells + // the inferNextSteps function to ignore them when making changes. + // + // Apps with optional services with unmet requirements are stored as + // `rejected: false`, but services with unmet requirements are removed + const contractViolators: contracts.ContractViolators = {}; + const fulfilledContracts = contracts.validateTargetContracts(apps); const filteredApps = structuredClone(apps); - _.each( - fulfilledContracts, - ( - { valid, unmetServices, fulfilledServices, unmetAndOptional }, - appUuid, - ) => { - if (!valid) { - contractViolators[apps[appUuid].name] = unmetServices; - return delete filteredApps[appUuid]; - } else { - // valid is true, but we could still be missing - // some optional containers, and need to filter - // these out of the target state - const [releaseUuid] = Object.keys(filteredApps[appUuid].releases); - if (releaseUuid) { - const services = - filteredApps[appUuid].releases[releaseUuid].services ?? {}; - filteredApps[appUuid].releases[releaseUuid].services = _.pick( - services, - Object.keys(services).filter((serviceName) => - fulfilledServices.includes(serviceName), - ), - ); - } + for (const [ + appUuid, + { valid, unmetServices, unmetAndOptional }, + ] of Object.entries(fulfilledContracts)) { + if (!valid) { + // Add the app to the list of contract violators to generate a system + // error + contractViolators[appUuid] = { + appId: apps[appUuid].id, + appName: apps[appUuid].name, + services: unmetServices.map(({ serviceName }) => serviceName), + }; + } else { + // App is valid, but we could still be missing + // some optional containers, and need to filter + // these out of the target state + const app = filteredApps[appUuid]; + for (const { commit, serviceName } of unmetAndOptional) { + delete app.releases[commit].services[serviceName]; + } - if (unmetAndOptional.length !== 0) { - return reportOptionalContainers(unmetAndOptional); - } + if (unmetAndOptional.length !== 0) { + reportOptionalContainers( + unmetAndOptional.map(({ serviceName }) => serviceName), + ); } - }, - ); - let promise; - if (maybeTrx != null) { - promise = setInTransaction(filteredApps, maybeTrx); - } else { - promise = transaction((trx) => setInTransaction(filteredApps, trx)); + } } - await promise; + + let rejectedApps: string[] = []; if (!_.isEmpty(contractViolators)) { - throw new ContractViolationError(contractViolators); + rejectedApps = Object.keys(contractViolators); + reportRejectedReleases(contractViolators); } + await setInTransaction(filteredApps, rejectedApps, trx); } export async function getTargetApps(): Promise { return await dbFormat.getTargetJson(); } +export async function getTargetAppsWithRejections() { + return await dbFormat.getTargetWithRejections(); +} + /** * This is only used by the API. Do not use as the use of serviceIds is getting * deprecated @@ -788,12 +786,19 @@ function reportOptionalContainers(serviceNames: string[]) { '. ', )}`; log.info(message); - return logger.logSystemMessage( - message, - {}, - 'optionalContainerViolation', - true, + logger.logSystemMessage(message, {}); +} + +function reportRejectedReleases(violators: contracts.ContractViolators) { + const appStrings = Object.values(violators).map( + ({ appName, services }) => + `${appName}: Services with unmet requirements: ${services.join(', ')}`, ); + const message = `Some releases were rejected due to having unmet requirements:\n ${appStrings.join( + '\n ', + )}`; + log.error(message); + logger.logSystemMessage(message, { error: true }); } /** @@ -875,9 +880,9 @@ export async function getLegacyState() { return { local: apps }; } -// TODO: this function is probably more inefficient than it needs to be, since -// it tried to optimize for readability, look for a way to make it simpler -export async function getState() { +type AppsReport = { [uuid: string]: AppState }; + +export async function getState(): Promise { const [services, images] = await Promise.all([ serviceManager.getState(), imageManager.getState(), @@ -990,7 +995,7 @@ export async function getState() { ); // Assemble the state of apps - const state: { [appUuid: string]: AppState } = {}; + const state: AppsReport = {}; for (const { appId, appUuid, @@ -999,21 +1004,54 @@ export async function getState() { createdAt, ...svc } of servicesToReport) { - state[appUuid] = { - ...state[appUuid], + const app = state[appUuid] ?? { // Add the release_uuid if the commit has been stored in the database ...(commitsForApp[appId] && { release_uuid: commitsForApp[appId] }), - releases: { - ...state[appUuid]?.releases, - [commit]: { - ...state[appUuid]?.releases[commit], - services: { - ...state[appUuid]?.releases[commit]?.services, - [serviceName]: svc, - }, - }, - }, + releases: {}, }; + + const releases = app.releases; + releases[commit] = releases[commit] ?? { + update_status: 'done', + services: {}, + }; + + releases[commit].services[serviceName] = svc; + + // The update_status precedence order is as follows + // - aborted + // - downloading + // - downloaded + // - applying changes + // - done + if (svc.status === 'Aborted') { + releases[commit].update_status = 'aborted'; + } else if ( + releases[commit].update_status !== 'aborted' && + svc.download_progress != null && + svc.download_progress !== 100 + ) { + releases[commit].update_status = 'downloading'; + } else if ( + !['aborted', 'downloading'].includes(releases[commit].update_status!) && + (svc.download_progress === 100 || svc.status === 'Downloaded') + ) { + releases[commit].update_status = 'downloaded'; + } else if ( + // The `applying changes` state has lower precedence over the aborted/downloading/downloaded + // state + !['aborted', 'downloading', 'downloaded'].includes( + releases[commit].update_status!, + ) && + ['installing', 'installed', 'awaiting handover'].includes( + svc.status.toLowerCase(), + ) + ) { + releases[commit].update_status = 'applying changes'; + } + + // Update the state object + state[appUuid] = app; } return state; } diff --git a/src/compose/types/app.ts b/src/compose/types/app.ts index 3b700e9b8..3363d276c 100644 --- a/src/compose/types/app.ts +++ b/src/compose/types/app.ts @@ -21,6 +21,7 @@ export interface App { commit?: string; source?: string; isHost?: boolean; + isRejected?: boolean; // Services are stored as an array, as at any one time we could have more than one // service for a single service ID running (for example handover) services: Service[]; diff --git a/src/compose/types/application-manager.ts b/src/compose/types/application-manager.ts index 5afdd7afd..e5b44074d 100644 --- a/src/compose/types/application-manager.ts +++ b/src/compose/types/application-manager.ts @@ -1,3 +1,5 @@ import type { App } from './app'; export type InstancedAppState = { [appId: number]: App }; + +export type AppRelease = { appUuid: string; releaseUuid: string }; diff --git a/src/device-state/db-format.ts b/src/device-state/db-format.ts index 902d07c9b..2af88fe62 100644 --- a/src/device-state/db-format.ts +++ b/src/device-state/db-format.ts @@ -1,5 +1,3 @@ -import _ from 'lodash'; - import type * as db from '../db'; import * as targetStateCache from './target-state-cache'; import type { DatabaseApp, DatabaseService } from './target-state-cache'; @@ -7,13 +5,8 @@ import type { DatabaseApp, DatabaseService } from './target-state-cache'; import { App } from '../compose/app'; import * as images from '../compose/images'; -import type { - TargetApp, - TargetApps, - TargetRelease, - TargetService, -} from '../types/state'; -import type { InstancedAppState } from '../compose/types'; +import type { UUID, TargetApps, TargetRelease, TargetService } from '../types'; +import type { InstancedAppState, AppRelease } from '../compose/types'; type InstancedApp = InstancedAppState[0]; @@ -40,16 +33,17 @@ export async function getApps(): Promise { export async function setApps( apps: TargetApps, source: string, + rejectedApps: UUID[] = [], trx?: db.Transaction, ) { const dbApps = Object.keys(apps).map((uuid) => { const { id: appId, ...app } = apps[uuid]; + const rejected = rejectedApps.includes(uuid); // Get the first uuid - const [releaseUuid] = Object.keys(app.releases); - const release = releaseUuid - ? app.releases[releaseUuid] - : ({} as TargetRelease); + const releaseUuid = Object.keys(app.releases).shift(); + const release = + releaseUuid != null ? app.releases[releaseUuid] : ({} as TargetRelease); const services = Object.keys(release.services ?? {}).map((serviceName) => { const { id: releaseId } = release; @@ -77,9 +71,13 @@ export async function setApps( uuid, source, isHost: !!app.is_host, + rejected, class: app.class, name: app.name, - ...(releaseUuid && { releaseId: release.id, commit: releaseUuid }), + ...(releaseUuid && { + releaseId: release.id, + commit: releaseUuid, + }), services: JSON.stringify(services), networks: JSON.stringify(release.networks ?? {}), volumes: JSON.stringify(release.volumes ?? {}), @@ -92,67 +90,78 @@ export async function setApps( /** * Create target state from database state */ -export async function getTargetJson(): Promise { +export async function getTargetWithRejections(): Promise<{ + apps: TargetApps; + rejections: AppRelease[]; +}> { const dbApps = await getDBEntry(); - return dbApps - .map( - ({ - source, - uuid, - releaseId, - commit: releaseUuid, - ...app - }): [string, TargetApp] => { - const services = (JSON.parse(app.services) as DatabaseService[]) - .map( - ({ - serviceName, - serviceId, - imageId, - ...service - }): [string, TargetService] => [ - serviceName, - { - id: serviceId, - image_id: imageId, - ..._.omit(service, ['appId', 'appUuid', 'commit', 'releaseId']), - } as TargetService, - ], - ) - // Map by serviceName - .reduce( - (svcs, [serviceName, s]) => ({ - ...svcs, - [serviceName]: s, - }), - {}, - ); - - const releases = releaseUuid - ? { - [releaseUuid]: { - id: releaseId, - services, - networks: JSON.parse(app.networks), - volumes: JSON.parse(app.volumes), - } as TargetRelease, - } - : {}; - - return [ - uuid, + const apps: TargetApps = {}; + const rejections: AppRelease[] = []; + + for (const { + source, + rejected, + uuid, + releaseId, + commit: releaseUuid, + ...app + } of dbApps) { + const services = Object.fromEntries( + (JSON.parse(app.services) as DatabaseService[]).map( + ({ + serviceName, + serviceId, + imageId, + // Ignore these fields + appId: _appId, + appUuid: _appUuid, + commit: _commit, + releaseId: _releaseId, + // Use the remainder of the fields for the service description + ...service + }) => [ + serviceName, { - id: app.appId, - name: app.name, - class: app.class, - is_host: !!app.isHost, - releases, - }, - ]; - }, - ) - .reduce((apps, [uuid, app]) => ({ ...apps, [uuid]: app }), {}); + id: serviceId, + image_id: imageId, + ...service, + } satisfies TargetService, + ], + ), + ); + + const releases = + releaseUuid && releaseId + ? { + [releaseUuid]: { + id: releaseId, + services, + networks: JSON.parse(app.networks), + volumes: JSON.parse(app.volumes), + } satisfies TargetRelease, + } + : {}; + + if (rejected && releaseUuid) { + rejections.push({ appUuid: uuid, releaseUuid }); + } + + apps[uuid] = { + id: app.appId, + name: app.name, + class: app.class, + is_host: !!app.isHost, + releases, + }; + } + + return { apps, rejections }; +} + +export async function getTargetJson(): Promise { + const { apps } = await getTargetWithRejections(); + return apps; } function getDBEntry(): Promise; diff --git a/src/device-state/index.ts b/src/device-state/index.ts index e8fdfc1e2..de38ed0d9 100644 --- a/src/device-state/index.ts +++ b/src/device-state/index.ts @@ -26,12 +26,7 @@ import type { InstancedDeviceState } from './target-state'; import * as TargetState from './target-state'; export { getTarget, setTarget } from './target-state'; -import type { - DeviceLegacyState, - DeviceState, - DeviceReport, - AppState, -} from '../types'; +import type { DeviceLegacyState, DeviceState, DeviceReport } from '../types'; import type { CompositionStepT, CompositionStepAction, @@ -366,18 +361,34 @@ export async function getCurrentForReport( ): Promise { const apps = await applicationManager.getState(); + const { apps: targetApps, rejections } = + await applicationManager.getTargetAppsWithRejections(); + const targetAppUuids = Object.keys(targetApps); + // Fiter current apps by the target state as the supervisor cannot // report on apps for which it doesn't have API permissions - const targetAppUuids = Object.keys(await applicationManager.getTargetApps()); - const appsForReport = Object.keys(apps) - .filter((appUuid) => targetAppUuids.includes(appUuid)) - .reduce( - (filteredApps, appUuid) => ({ - ...filteredApps, - [appUuid]: apps[appUuid], - }), - {} as { [appUuid: string]: AppState }, - ); + // this step also adds rejected commits for the report + const appsForReport = Object.fromEntries( + Object.entries(apps).flatMap(([appUuid, app]) => { + if (!targetAppUuids.includes(appUuid)) { + return []; + } + + for (const r of rejections) { + if (r.appUuid !== appUuid) { + continue; + } + + // Add the rejected release to apps for report + app.releases[r.releaseUuid] = { + update_status: 'rejected', + services: {}, + }; + } + + return [[appUuid, app]]; + }), + ); const { uuid, localMode } = await config.getMany(['uuid', 'localMode']); diff --git a/src/device-state/target-state-cache.ts b/src/device-state/target-state-cache.ts index 50f6b8ca4..6d5d9595e 100644 --- a/src/device-state/target-state-cache.ts +++ b/src/device-state/target-state-cache.ts @@ -8,6 +8,8 @@ import type { TargetAppClass } from '../types'; // at all, and we can use the below type for both insertion and retrieval. export interface DatabaseApp { name: string; + // releaseId and commit may be empty as the device could + // have been moved to an app without any releases /** * @deprecated to be removed in target state v4 */ @@ -24,6 +26,7 @@ export interface DatabaseApp { source: string; class: TargetAppClass; isHost: boolean; + rejected: boolean; } export type DatabaseService = { diff --git a/src/lib/contracts.ts b/src/lib/contracts.ts index a63d32c74..bd0da48b1 100644 --- a/src/lib/contracts.ts +++ b/src/lib/contracts.ts @@ -2,23 +2,66 @@ import { isLeft } from 'fp-ts/lib/Either'; import * as t from 'io-ts'; import Reporter from 'io-ts-reporters'; import _ from 'lodash'; +import { TypedError } from 'typed-error'; import type { ContractObject } from '@balena/contrato'; import { Blueprint, Contract } from '@balena/contrato'; -import { ContractValidationError, InternalInconsistencyError } from './errors'; +import { InternalInconsistencyError } from './errors'; import { checkTruthy } from './validation'; import type { TargetApps } from '../types'; -export interface ApplicationContractResult { +/** + * This error is thrown when a container contract does not + * match the minimum we expect from it + */ +export class ContractValidationError extends TypedError { + constructor(serviceName: string, error: string) { + super( + `The contract for service ${serviceName} failed validation, with error: ${error}`, + ); + } +} + +export interface ContractViolators { + [appUuid: string]: { appName: string; appId: number; services: string[] }; +} + +/** + * This error is thrown when one or releases cannot be ran + * as one or more of their container have unmet requirements. + * It accepts a map of app names to arrays of service names + * which have unmet requirements. + */ +export class ContractViolationError extends TypedError { + constructor(violators: ContractViolators) { + const appStrings = Object.values(violators).map( + ({ appName, services }) => + `${appName}: Services with unmet requirements: ${services.join(', ')}`, + ); + super( + `Some releases were rejected due to having unmet requirements:\n ${appStrings.join( + '\n ', + )}`, + ); + } +} + +export interface ServiceCtx { + serviceName: string; + commit: string; +} + +export interface AppContractResult { valid: boolean; - unmetServices: string[]; - fulfilledServices: string[]; - unmetAndOptional: string[]; + unmetServices: ServiceCtx[]; + fulfilledServices: ServiceCtx[]; + unmetAndOptional: ServiceCtx[]; } -export interface ServiceContracts { - [serviceName: string]: { contract?: ContractObject; optional: boolean }; +interface ServiceWithContract extends ServiceCtx { + contract?: ContractObject; + optional: boolean; } type PotentialContractRequirements = @@ -52,12 +95,15 @@ function isValidRequirementType( } export function containerContractsFulfilled( - serviceContracts: ServiceContracts, -): ApplicationContractResult { - const containers = _(serviceContracts).map('contract').compact().value(); + servicesWithContract: ServiceWithContract[], +): AppContractResult { + const containers = servicesWithContract + .map(({ contract }) => contract) + .filter((c) => c != null) satisfies ContractObject[]; + const contractTypes = Object.keys(contractRequirementVersions); const blueprintMembership: Dictionary = {}; - for (const component of _.keys(contractRequirementVersions)) { + for (const component of contractTypes) { blueprintMembership[component] = 1; } const blueprint = new Blueprint( @@ -89,10 +135,11 @@ export function containerContractsFulfilled( 'More than one solution available for container contracts when only one is expected!', ); } + if (solution.length === 0) { return { valid: false, - unmetServices: _.keys(serviceContracts), + unmetServices: servicesWithContract, fulfilledServices: [], unmetAndOptional: [], }; @@ -108,7 +155,7 @@ export function containerContractsFulfilled( return { valid: true, unmetServices: [], - fulfilledServices: _.keys(serviceContracts), + fulfilledServices: servicesWithContract, unmetAndOptional: [], }; } else { @@ -117,16 +164,14 @@ export function containerContractsFulfilled( // 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]; + servicesWithContract, + ({ contract }) => { if (!contract) { return true; } // Did we find the contract in the generated state? - return _.some(children, (child) => + return children.some((child) => _.isEqual((child as any).raw, contract), ); }, @@ -134,9 +179,7 @@ export function containerContractsFulfilled( const [unmetAndRequired, unmetAndOptional] = _.partition( unfulfilledServices, - (serviceName) => { - return !serviceContracts[serviceName].optional; - }, + ({ optional }) => !optional, ); return { @@ -198,67 +241,43 @@ export function validateContract(contract: unknown): boolean { return true; } + export function validateTargetContracts( apps: TargetApps, -): Dictionary { - return Object.keys(apps) - .map((appUuid): [string, ApplicationContractResult] => { - const app = apps[appUuid]; - const [release] = Object.values(app.releases); - const serviceContracts = Object.keys(release?.services ?? []) - .map((serviceName) => { - const service = release.services[serviceName]; - const { contract } = service; - if (contract) { - try { - // Validate the contract syntax - validateContract(contract); - - return { - serviceName, - contract, - optional: checkTruthy( - service.labels?.['io.balena.features.optional'], - ), - }; - } catch (e: any) { - throw new ContractValidationError(serviceName, e.message); - } - } +): Dictionary { + const result: Dictionary = {}; - // Return a default contract for the service if no contract is defined - return { serviceName, contract: undefined, optional: false }; - }) - // map by serviceName - .reduce( - (contracts, { serviceName, ...serviceContract }) => ({ - ...contracts, - [serviceName]: serviceContract, - }), - {} as ServiceContracts, - ); + for (const [appUuid, app] of Object.entries(apps)) { + const releases = Object.entries(app.releases); + if (releases.length === 0) { + continue; + } - if (Object.keys(serviceContracts).length > 0) { - // Validate service contracts if any - return [appUuid, containerContractsFulfilled(serviceContracts)]; - } - - // Return success if no services are found - return [ - appUuid, - { - valid: true, - fulfilledServices: Object.keys(release?.services ?? []), - unmetAndOptional: [], - unmetServices: [], - }, - ]; - }) - .reduce( - (result, [appUuid, contractFulfilled]) => ({ - ...result, - [appUuid]: contractFulfilled, - }), - {} as Dictionary, + // While app.releases is an object, we expect a target to only + // contain a single release per app so we use just the first element + const [commit, release] = releases[0]; + + const servicesWithContract = Object.entries(release.services ?? {}).map( + ([serviceName, { contract, labels = {} }]) => { + if (contract) { + try { + validateContract(contract); + } catch (e: any) { + throw new ContractValidationError(serviceName, e.message); + } + } + + return { + serviceName, + commit, + contract, + optional: checkTruthy(labels['io.balena.features.optional']), + }; + }, ); + + result[appUuid] = containerContractsFulfilled(servicesWithContract); + } + + return result; } diff --git a/src/lib/errors.ts b/src/lib/errors.ts index f7fb9b86b..eb4a1a08d 100644 --- a/src/lib/errors.ts +++ b/src/lib/errors.ts @@ -1,4 +1,4 @@ -import { endsWith, map } from 'lodash'; +import { endsWith } from 'lodash'; import { TypedError } from 'typed-error'; import { checkInt } from './validation'; @@ -104,39 +104,6 @@ export class TargetStateError extends TypedError {} */ export class SupervisorContainerNotFoundError extends TypedError {} -/** - * This error is thrown when a container contract does not - * match the minimum we expect from it - */ -export class ContractValidationError extends TypedError { - constructor(serviceName: string, error: string) { - super( - `The contract for service ${serviceName} failed validation, with error: ${error}`, - ); - } -} - -/** - * This error is thrown when one or releases cannot be ran - * as one or more of their container have unmet requirements. - * It accepts a map of app names to arrays of service names - * which have unmet requirements. - */ -export class ContractViolationError extends TypedError { - constructor(violators: { [appName: string]: string[] }) { - const appStrings = map( - violators, - (svcs, name) => - `${name}: Services with unmet requirements: ${svcs.join(', ')}`, - ); - super( - `Some releases were rejected due to having unmet requirements:\n ${appStrings.join( - '\n ', - )}`, - ); - } -} - export class AppsJsonParseError extends TypedError {} export class DatabaseParseError extends TypedError {} export class BackupError extends TypedError {} diff --git a/src/migrations/M00012.js b/src/migrations/M00012.js new file mode 100644 index 000000000..be4558e27 --- /dev/null +++ b/src/migrations/M00012.js @@ -0,0 +1,10 @@ +export async function up(knex) { + // Add a `rejected` field to the target app + await knex.schema.table('app', (table) => { + table.boolean('rejected').defaultTo(false); + }); +} + +export function down() { + throw new Error('Not implemented'); +} diff --git a/src/types/state.ts b/src/types/state.ts index cf3239dd2..2de45afad 100644 --- a/src/types/state.ts +++ b/src/types/state.ts @@ -127,10 +127,22 @@ const fromType = (name: string) => // Alias short string to UUID so code reads more clearly export const UUID = ShortString; +export type UUID = t.TypeOf; /** *************** * Current state * *****************/ + +const UpdateStatus = t.union([ + t.literal('rejected'), + t.literal('downloading'), + t.literal('downloaded'), + t.literal('applying changes'), + t.literal('aborted'), + t.literal('done'), +]); +export type UpdateStatus = t.TypeOf; + const ServiceState = t.intersection([ t.type({ image: t.string, @@ -143,6 +155,7 @@ const ServiceState = t.intersection([ export type ServiceState = t.TypeOf; const ReleaseState = t.type({ + update_status: UpdateStatus, services: t.record(DockerName, ServiceState), }); export type ReleaseState = t.TypeOf; @@ -182,6 +195,8 @@ const DeviceReport = t.partial({ cpu_usage: t.number, cpu_id: t.string, is_undervolted: t.boolean, + // These are for internal reporting only, they are not sent + // to the API update_failed: t.boolean, update_pending: t.boolean, update_downloaded: t.boolean, diff --git a/test/integration/compose/application-manager.spec.ts b/test/integration/compose/application-manager.spec.ts index b018ee5bd..c7288b455 100644 --- a/test/integration/compose/application-manager.spec.ts +++ b/test/integration/compose/application-manager.spec.ts @@ -1595,6 +1595,97 @@ describe('compose/application-manager', () => { .that.deep.includes({ name: 'main-image' }); }); + it('should not calculate steps for a rejected app', async () => { + const targetApps = createApps( + { + services: [ + await createService({ + running: true, + image: 'main-image-1', + appId: 1, + appUuid: 'app-one', + commit: 'commit-for-app-1', + }), + await createService({ + running: true, + image: 'main-image-2', + appId: 2, + appUuid: 'app-two', + commit: 'commit-for-app-2', + }), + ], + networks: [ + // Default networks for two apps + Network.fromComposeObject('default', 1, 'app-one', {}), + Network.fromComposeObject('default', 2, 'app-two', {}), + ], + rejectedAppIds: [1], + }, + true, + ); + const { currentApps, availableImages, downloading, containerIdsByAppId } = + createCurrentState({ + services: [ + await createService({ + running: true, + image: 'old-image-1', + appId: 1, + appUuid: 'app-one', + commit: 'commit-for-app-0', + }), + ], + networks: [ + // Default networks for two apps + Network.fromComposeObject('default', 1, 'app-one', {}), + Network.fromComposeObject('default', 2, 'app-two', {}), + ], + images: [ + createImage({ + name: 'main-image-1', + appId: 1, + appUuid: 'app-one', + serviceName: 'main', + commit: 'commit-for-app-1', + }), + createImage({ + name: 'main-image-2', + appId: 2, + appUuid: 'app-two', + serviceName: 'main', + commit: 'commit-for-app-2', + }), + ], + }); + + const steps = await applicationManager.inferNextSteps( + currentApps, + targetApps, + { + downloading, + availableImages, + containerIdsByAppId, + // Mock locks taken to avoid takeLock step + locksTaken: new LocksTakenMap([{ appId: 2, services: ['main'] }]), + }, + ); + + // Expect a start step for both apps + expect( + steps.filter((s: any) => s.target && s.target.appId === 1), + ).to.have.lengthOf(0); + expect( + steps.filter((s: any) => s.image && s.image.appId === 1), + ).to.have.lengthOf(0); + expect( + steps.filter( + (s: any) => + s.action === 'start' && + s.target.appId === 2 && + s.target.serviceName === 'main', + ), + ).to.have.lengthOf(1); + }); + it('should correctly generate steps for multiple apps', async () => { const targetApps = createApps( { @@ -2444,6 +2535,7 @@ describe('compose/application-manager', () => { download_progress: 50, }, }, + update_status: 'downloading', }, }, }, @@ -2456,6 +2548,7 @@ describe('compose/application-manager', () => { status: 'Downloaded', }, }, + update_status: 'downloaded', }, newrelease: { services: { @@ -2465,6 +2558,7 @@ describe('compose/application-manager', () => { download_progress: 75, }, }, + update_status: 'downloading', }, }, }, @@ -2548,6 +2642,7 @@ describe('compose/application-manager', () => { download_progress: 0, }, }, + update_status: 'downloading', }, }, }, @@ -2560,6 +2655,81 @@ describe('compose/application-manager', () => { status: 'exited', }, }, + update_status: 'done', + }, + }, + }, + }); + }); + + it('reports aborted state if one of the services/images status is aborted', async () => { + getImagesState.resolves([ + { + name: 'ubuntu:latest', + commit: 'latestrelease', + appUuid: 'myapp', + serviceName: 'ubuntu', + status: 'Downloaded', + }, + { + name: 'node:latest', + commit: 'latestrelease', + appUuid: 'myapp', + serviceName: 'node', + status: 'Downloaded', + downloadProgress: 100, + }, + { + name: 'alpine:latest', + commit: 'latestrelease', + appUuid: 'myapp', + serviceName: 'alpine', + status: 'Aborted', + downloadProgress: 0, + }, + ]); + getServicesState.resolves([ + { + commit: 'latestrelease', + appUuid: 'myapp', + serviceName: 'ubuntu', + status: 'Running', + createdAt: new Date('2021-09-01T13:00:00'), + }, + { + appUuid: 'myapp', + commit: 'latestrelease', + serviceName: 'node', + // we don't have a way to abort a failing service install yet, but + // once we do it will need to use the status field + status: 'Aborted', + createdAt: new Date('2021-09-01T12:00:00'), + }, + ]); + + expect(await applicationManager.getState()).to.deep.equal({ + myapp: { + releases: { + latestrelease: { + services: { + ubuntu: { + image: 'ubuntu:latest', + status: 'Running', + }, + alpine: { + image: 'alpine:latest', + // we don't have a way to abort a failing download yet, but + // once we do it will need to use the status field + status: 'Aborted', + download_progress: 0, + }, + node: { + image: 'node:latest', + status: 'Aborted', + download_progress: 100, + }, + }, + update_status: 'aborted', }, }, }, @@ -2610,6 +2780,7 @@ describe('compose/application-manager', () => { status: 'Awaiting handover', }, }, + update_status: 'applying changes', }, oldrelease: { services: { @@ -2618,6 +2789,7 @@ describe('compose/application-manager', () => { status: 'Handing over', }, }, + update_status: 'done', }, }, }, diff --git a/test/integration/device-state.spec.ts b/test/integration/device-state.spec.ts index 388532481..fec2f52fa 100644 --- a/test/integration/device-state.spec.ts +++ b/test/integration/device-state.spec.ts @@ -368,6 +368,7 @@ describe('device-state', () => { const app = targetState.local.apps[1234]; expect(app).to.have.property('appName').that.equals('superapp'); expect(app).to.have.property('commit').that.equals('one'); + expect(app).to.have.property('isRejected').that.is.false; // Only a single service should be on the target state expect(app).to.have.property('services').that.is.an('array').with.length(1); expect(app.services[0]) @@ -375,7 +376,8 @@ describe('device-state', () => { .that.equals('valid'); }); - it('rejects a target state with invalid contract and non optional service', async () => { + it('accepts target state with invalid contract setting isRejected to true and resets state when a valid target is received', async () => { + // Set the rejected target await expect( deviceState.setTarget({ local: { @@ -424,7 +426,66 @@ describe('device-state', () => { }, }, } as TargetState), - ).to.be.rejected; + ).to.not.be.rejected; + const targetState = await deviceState.getTarget(); + const app = targetState.local.apps[1234]; + expect(app).to.have.property('appName').that.equals('superapp'); + expect(app).to.have.property('commit').that.equals('one'); + expect(app).to.have.property('isRejected').that.is.true; + + // Now set a good target for the same app + await deviceState.setTarget({ + local: { + name: 'aDeviceWithDifferentName', + config: {}, + apps: { + myapp: { + id: 1234, + name: 'superapp', + class: 'fleet', + releases: { + two: { + id: 2, + services: { + valid: { + id: 23, + image_id: 12345, + image: 'registry2.resin.io/superapp/valid', + environment: {}, + labels: {}, + }, + alsoValid: { + id: 24, + image_id: 12346, + image: 'registry2.resin.io/superapp/afaff', + contract: { + type: 'sw.container', + slug: 'supervisor-version', + name: 'Enforce supervisor version', + requires: [ + { + type: 'sw.supervisor', + version: '>=11.0.0', + }, + ], + }, + environment: {}, + labels: {}, + }, + }, + volumes: {}, + networks: {}, + }, + }, + }, + }, + }, + } as TargetState); + + const targetState2 = await deviceState.getTarget(); + const app2 = targetState2.local.apps[1234]; + expect(app2).to.have.property('commit').that.equals('two'); + expect(app2).to.have.property('isRejected').that.is.false; }); // TODO: There is no easy way to test this behaviour with the current diff --git a/test/lib/db-helper.ts b/test/lib/db-helper.ts deleted file mode 100644 index a27d58e15..000000000 --- a/test/lib/db-helper.ts +++ /dev/null @@ -1,84 +0,0 @@ -import * as constants from '~/lib/constants'; -import * as db from '~/src/db'; -import * as sinon from 'sinon'; - -// Creates a test database and returns a query builder -export async function createDB() { - const oldDatabasePath = process.env.DATABASE_PATH; - - // for testing we use an in memory database - process.env.DATABASE_PATH = ':memory:'; - - // @ts-expect-error need to rewrite the value of databasePath as that - // is used directly by the db module - constants.databasePath = process.env.DATABASE_PATH; - - // Cleanup the module cache in order to have it reloaded in the local context - delete require.cache[require.resolve('~/src/db')]; - - // Initialize the database module - await db.initialized(); - - // Get the knex instance to allow queries to the db - const { models, upsertModel } = db; - - // This is hacky but haven't found another way to do it, - // stubbing the db methods here ensures the module under test - // is using the database we want - sinon.stub(db, 'models').callsFake(models); - sinon.stub(db, 'upsertModel').callsFake(upsertModel); - - return { - // Returns a query builder instance for the given - // table in order perform data operations - models, - - // Resets the database to initial value post - // migrations - async reset() { - // Reset the contents of the db - await db.transaction(async (trx: any) => { - const result = await trx.raw(` - SELECT name, sql - FROM sqlite_master - WHERE type='table'`); - for (const r of result) { - // We don't run the migrations again - if (r.name !== 'knex_migrations') { - await trx.raw(`DELETE FROM ${r.name}`); - } - } - - // The supervisor expects this value to already have - // been pre-populated - await trx('deviceConfig').insert({ targetValues: '{}' }); - }); - - // Reset stub call history - (db.models as sinon.SinonStub).resetHistory(); - (db.upsertModel as sinon.SinonStub).resetHistory(); - }, - - // Destroys the in-memory database and resets environment - async destroy() { - // Remove data from the in memory database just in case - await this.reset(); - - // Restore the old datbase path - process.env.DATABASE_PATH = oldDatabasePath; - - // Restore stubs - (db.models as sinon.SinonStub).restore(); - (db.upsertModel as sinon.SinonStub).restore(); - - // @ts-expect-error restore the constant default - constants.databasePath = process.env.DATABASE_PATH; - - // Cleanup the module cache in order to have it reloaded - // correctly next time it's used - delete require.cache[require.resolve('~/src/db')]; - }, - }; -} - -export type TestDatabase = UnwrappedPromise>; diff --git a/test/lib/state-helper.ts b/test/lib/state-helper.ts index e33bd11e3..0b6cee8d8 100644 --- a/test/lib/state-helper.ts +++ b/test/lib/state-helper.ts @@ -85,6 +85,7 @@ export function createApp({ isTarget = false, appId = 1, appUuid = 'appuuid', + isRejected = false, } = {}) { return new App( { @@ -93,6 +94,7 @@ export function createApp({ services, networks, volumes, + isRejected, }, isTarget, ); @@ -103,6 +105,7 @@ export function createApps( services = [] as Service[], networks = [] as Network[], volumes = [] as Volume[], + rejectedAppIds = [] as number[], }, target = false, ) { @@ -129,6 +132,7 @@ export function createApps( const apps: InstancedAppState = {}; for (const appId of allAppIds) { + const isRejected = rejectedAppIds.includes(appId); apps[appId] = createApp({ services: servicesByAppId[appId] ?? [], networks: networksByAppId[appId] ?? [], @@ -136,6 +140,7 @@ export function createApps( appId, appUuid: servicesByAppId[appId]?.[0]?.appUuid ?? 'deadbeef', isTarget: target, + isRejected, }); } diff --git a/test/unit/lib/contracts.spec.ts b/test/unit/lib/contracts.spec.ts index fd655a5af..cd204534b 100644 --- a/test/unit/lib/contracts.spec.ts +++ b/test/unit/lib/contracts.spec.ts @@ -101,35 +101,41 @@ describe('lib/contracts', () => { it('Should correctly run containers with no requirements', async () => { expect( - contracts.containerContractsFulfilled({ - service: { + contracts.containerContractsFulfilled([ + { + commit: 'd0', + serviceName: 'service', contract: { type: 'sw.container', slug: 'user-container', }, optional: false, }, - }), + ]), ) .to.have.property('valid') .that.equals(true); expect( - contracts.containerContractsFulfilled({ - service: { + contracts.containerContractsFulfilled([ + { + commit: 'd0', + serviceName: 'service', contract: { type: 'sw.container', slug: 'user-container1', }, optional: false, }, - service2: { + { + commit: 'd0', + serviceName: 'service2', contract: { type: 'sw.container', slug: 'user-container2', }, optional: false, }, - }), + ]), ) .to.have.property('valid') .that.equals(true); @@ -137,8 +143,10 @@ describe('lib/contracts', () => { it('should correctly run containers whose requirements are satisfied', async () => { expect( - contracts.containerContractsFulfilled({ - service: { + contracts.containerContractsFulfilled([ + { + commit: 'd0', + serviceName: 'service', contract: { type: 'sw.container', name: 'user-container', @@ -153,14 +161,16 @@ describe('lib/contracts', () => { }, optional: false, }, - }), + ]), ) .to.have.property('valid') .that.equals(true); expect( - contracts.containerContractsFulfilled({ - service: { + contracts.containerContractsFulfilled([ + { + commit: 'd0', + serviceName: 'service', contract: { type: 'sw.container', name: 'user-container', @@ -176,14 +186,16 @@ describe('lib/contracts', () => { }, optional: false, }, - }), + ]), ) .to.have.property('valid') .that.equals(true); expect( - contracts.containerContractsFulfilled({ - service: { + contracts.containerContractsFulfilled([ + { + commit: 'd0', + serviceName: 'service', contract: { type: 'sw.container', name: 'user-container', @@ -198,14 +210,16 @@ describe('lib/contracts', () => { }, optional: false, }, - }), + ]), ) .to.have.property('valid') .that.equals(true); expect( - contracts.containerContractsFulfilled({ - service: { + contracts.containerContractsFulfilled([ + { + commit: 'd0', + serviceName: 'service', contract: { type: 'sw.container', name: 'user-container', @@ -224,14 +238,16 @@ describe('lib/contracts', () => { }, optional: false, }, - }), + ]), ) .to.have.property('valid') .that.equals(true); expect( - contracts.containerContractsFulfilled({ - service: { + contracts.containerContractsFulfilled([ + { + commit: 'd0', + serviceName: 'service', contract: { type: 'sw.container', name: 'user-container1', @@ -245,7 +261,9 @@ describe('lib/contracts', () => { }, optional: false, }, - service2: { + { + commit: 'd0', + serviceName: 'service2', contract: { type: 'sw.container', name: 'user-container1', @@ -261,15 +279,17 @@ describe('lib/contracts', () => { }, optional: false, }, - }), + ]), ) .to.have.property('valid') .that.equals(true); }); - it('Should refuse to run containers whose requirements are not satisfied', async () => { - let fulfilled = contracts.containerContractsFulfilled({ - service: { + it('should refuse to run containers whose requirements are not satisfied', async () => { + let fulfilled = contracts.containerContractsFulfilled([ + { + commit: 'd0', + serviceName: 'service', contract: { type: 'sw.container', name: 'user-container', @@ -283,14 +303,18 @@ describe('lib/contracts', () => { }, optional: false, }, - }); + ]); expect(fulfilled).to.have.property('valid').that.equals(false); - expect(fulfilled) - .to.have.property('unmetServices') - .that.deep.equals(['service']); + expect(fulfilled).to.have.property('unmetServices').with.lengthOf(1); + expect(fulfilled.unmetServices[0]).to.deep.include({ + serviceName: 'service', + commit: 'd0', + }); - fulfilled = contracts.containerContractsFulfilled({ - service: { + fulfilled = contracts.containerContractsFulfilled([ + { + commit: 'd0', + serviceName: 'service', contract: { type: 'sw.container', name: 'user-container', @@ -304,14 +328,18 @@ describe('lib/contracts', () => { }, optional: false, }, - }); + ]); expect(fulfilled).to.have.property('valid').that.equals(false); - expect(fulfilled) - .to.have.property('unmetServices') - .that.deep.equals(['service']); + expect(fulfilled).to.have.property('unmetServices').with.lengthOf(1); + expect(fulfilled.unmetServices[0]).to.deep.include({ + serviceName: 'service', + commit: 'd0', + }); - fulfilled = contracts.containerContractsFulfilled({ - service: { + fulfilled = contracts.containerContractsFulfilled([ + { + commit: 'd0', + serviceName: 'service', contract: { type: 'sw.container', name: 'user-container', @@ -325,14 +353,18 @@ describe('lib/contracts', () => { }, optional: false, }, - }); + ]); expect(fulfilled).to.have.property('valid').that.equals(false); - expect(fulfilled) - .to.have.property('unmetServices') - .that.deep.equals(['service']); + expect(fulfilled).to.have.property('unmetServices').with.lengthOf(1); + expect(fulfilled.unmetServices[0]).to.deep.include({ + serviceName: 'service', + commit: 'd0', + }); - fulfilled = contracts.containerContractsFulfilled({ - service: { + fulfilled = contracts.containerContractsFulfilled([ + { + commit: 'd0', + serviceName: 'service', contract: { type: 'sw.container', name: 'user-container', @@ -346,14 +378,18 @@ describe('lib/contracts', () => { }, optional: false, }, - }); + ]); expect(fulfilled).to.have.property('valid').that.equals(false); - expect(fulfilled) - .to.have.property('unmetServices') - .that.deep.equals(['service']); + expect(fulfilled).to.have.property('unmetServices').with.lengthOf(1); + expect(fulfilled.unmetServices[0]).to.deep.include({ + serviceName: 'service', + commit: 'd0', + }); - fulfilled = contracts.containerContractsFulfilled({ - service2: { + fulfilled = contracts.containerContractsFulfilled([ + { + commit: 'd0', + serviceName: 'service2', contract: { type: 'sw.container', name: 'user-container2', @@ -367,14 +403,18 @@ describe('lib/contracts', () => { }, optional: false, }, - }); + ]); expect(fulfilled).to.have.property('valid').that.equals(false); - expect(fulfilled) - .to.have.property('unmetServices') - .that.deep.equals(['service2']); + expect(fulfilled).to.have.property('unmetServices').with.lengthOf(1); + expect(fulfilled.unmetServices[0]).to.deep.include({ + serviceName: 'service2', + commit: 'd0', + }); - fulfilled = contracts.containerContractsFulfilled({ - service: { + fulfilled = contracts.containerContractsFulfilled([ + { + commit: 'd0', + serviceName: 'service', contract: { type: 'sw.container', name: 'user-container1', @@ -388,7 +428,9 @@ describe('lib/contracts', () => { }, optional: false, }, - service2: { + { + commit: 'd0', + serviceName: 'service2', contract: { type: 'sw.container', name: 'user-container2', @@ -402,18 +444,22 @@ describe('lib/contracts', () => { }, optional: false, }, - }); + ]); expect(fulfilled).to.have.property('valid').that.equals(false); - expect(fulfilled) - .to.have.property('unmetServices') - .that.deep.equals(['service2']); + expect(fulfilled).to.have.property('unmetServices').with.lengthOf(1); + expect(fulfilled.unmetServices[0]).to.deep.include({ + serviceName: 'service2', + commit: 'd0', + }); }); describe('Optional containers', () => { it('should correctly run passing optional containers', async () => { const { valid, unmetServices, fulfilledServices } = - contracts.containerContractsFulfilled({ - service1: { + contracts.containerContractsFulfilled([ + { + commit: 'd0', + serviceName: 'service1', contract: { type: 'sw.container', slug: 'service1', @@ -426,16 +472,22 @@ describe('lib/contracts', () => { }, optional: true, }, - }); + ]); + expect(valid).to.equal(true); expect(unmetServices).to.deep.equal([]); - expect(fulfilledServices).to.deep.equal(['service1']); + expect(fulfilledServices[0]).to.deep.include({ + serviceName: 'service1', + commit: 'd0', + }); }); it('should corrrectly omit failing optional containers', async () => { const { valid, unmetServices, fulfilledServices } = - contracts.containerContractsFulfilled({ - service1: { + contracts.containerContractsFulfilled([ + { + commit: 'd0', + serviceName: 'service1', contract: { type: 'sw.container', slug: 'service1', @@ -448,14 +500,18 @@ describe('lib/contracts', () => { }, optional: true, }, - service2: { + { + commit: 'd0', + serviceName: 'service2', contract: { type: 'sw.container', slug: 'service2', }, optional: false, }, - service3: { + { + commit: 'd0', + serviceName: 'service3', contract: { type: 'sw.container', slug: 'service3', @@ -468,10 +524,12 @@ describe('lib/contracts', () => { }, optional: true, }, - service4: { + { + commit: 'd0', + serviceName: 'service4', contract: { type: 'sw.container', - slug: 'service3', + slug: 'service4', requires: [ { type: 'arch.sw', @@ -481,14 +539,18 @@ describe('lib/contracts', () => { }, optional: true, }, - }); + ]); expect(valid).to.equal(true); - expect(unmetServices).to.deep.equal([ + expect(unmetServices.map((s) => s.serviceName)).to.deep.equal([ 'service1', 'service3', 'service4', ]); - expect(fulfilledServices).to.deep.equal(['service2']); + expect(fulfilledServices).to.have.lengthOf(1); + expect(fulfilledServices[0]).to.deep.include({ + serviceName: 'service2', + commit: 'd0', + }); }); }); }); @@ -548,8 +610,10 @@ describe('lib/contracts', () => { const engine = await seedEngine('4.4.38-l4t-r31.0'); expect( - engine.containerContractsFulfilled({ - service: { + engine.containerContractsFulfilled([ + { + commit: 'd0', + serviceName: 'service', contract: { type: 'sw.container', slug: 'user-container', @@ -562,14 +626,16 @@ describe('lib/contracts', () => { }, optional: false, }, - }), + ]), ) .to.have.property('valid') .that.equals(true); expect( - engine.containerContractsFulfilled({ - service: { + engine.containerContractsFulfilled([ + { + commit: 'd0', + serviceName: 'service', contract: { type: 'sw.container', slug: 'user-container', @@ -582,7 +648,7 @@ describe('lib/contracts', () => { }, optional: false, }, - }), + ]), ) .to.have.property('valid') .that.equals(false); @@ -592,8 +658,10 @@ describe('lib/contracts', () => { const engine = await seedEngine('4.4.38-l4t-r31.0.1'); expect( - engine.containerContractsFulfilled({ - service: { + engine.containerContractsFulfilled([ + { + commit: 'd0', + serviceName: 'service', contract: { type: 'sw.container', slug: 'user-container', @@ -606,14 +674,16 @@ describe('lib/contracts', () => { }, optional: false, }, - }), + ]), ) .to.have.property('valid') .that.equals(true); expect( - engine.containerContractsFulfilled({ - service: { + engine.containerContractsFulfilled([ + { + commit: 'd0', + serviceName: 'service', contract: { type: 'sw.container', slug: 'user-container', @@ -626,7 +696,7 @@ describe('lib/contracts', () => { }, optional: false, }, - }), + ]), ) .to.have.property('valid') .that.equals(false);