diff --git a/src/compose/app.ts b/src/compose/app.ts index e3e1dac5..b6e926e0 100644 --- a/src/compose/app.ts +++ b/src/compose/app.ts @@ -25,6 +25,7 @@ import { checkTruthy } from '../lib/validation'; import type { ServiceComposeConfig, DeviceMetadata } from './types/service'; import { pathExistsOnRoot } from '../lib/host-utils'; import { isSupervisor } from '../lib/supervisor-metadata'; +import { isLockInProgress } from '../lib/update-lock'; import type { LocksTakenMap } from '../lib/update-lock'; export interface AppConstructOpts { @@ -169,14 +170,21 @@ export class App { // Generate lock steps from appsToLock for (const [appId, services] of Object.entries(appsToLock)) { + const numAppId = parseInt(appId, 10); if (services.size > 0) { - steps.push( - generateStep('takeLock', { - appId: parseInt(appId, 10), - services: Array.from(services), - force: state.force, - }), - ); + // Keep state funnel alive while locks are in process of + // being taken, as it's not an instant operation + if (isLockInProgress(numAppId)) { + steps.push(generateStep('noop', {})); + } else { + steps.push( + generateStep('takeLock', { + appId: numAppId, + services: Array.from(services), + force: state.force, + }), + ); + } } } @@ -219,6 +227,7 @@ export class App { ); } } + console.log({ steps }); return steps; } @@ -232,13 +241,19 @@ export class App { (svc) => !state.locksTaken.isLocked(svc.appId, svc.serviceName), ) ) { - return [ - generateStep('takeLock', { - appId: this.appId, - services: this.services.map((svc) => svc.serviceName), - force: state.force, - }), - ]; + // Keep state funnel alive while locks are in process of + // being taken, as it's not an instant operation + if (isLockInProgress(this.appId)) { + return [generateStep('noop', {})]; + } else { + return [ + generateStep('takeLock', { + appId: this.appId, + services: this.services.map((svc) => svc.serviceName), + force: state.force, + }), + ]; + } } return Object.values(this.services).map((service) => diff --git a/src/compose/composition-steps.ts b/src/compose/composition-steps.ts index ded3b966..3c64236b 100644 --- a/src/compose/composition-steps.ts +++ b/src/compose/composition-steps.ts @@ -10,6 +10,7 @@ import type Volume from './volume'; import * as commitStore from './commit'; import * as updateLock from '../lib/update-lock'; import type { DeviceLegacyReport } from '../types/state'; +import { promises as fs } from 'fs'; interface CompositionStepArgs { stop: { @@ -217,6 +218,20 @@ export function getExecutors(app: { callbacks: CompositionCallbacks }) { }, takeLock: async (step) => { await updateLock.takeLock(step.appId, step.services, step.force); + console.log( + 'services locked after takeLock action according to getServicesLockedByAppId', + JSON.stringify(await updateLock.getServicesLockedByAppId(), null, 2), + ); + const services = await fs.readdir( + `/mnt/root/tmp/balena-supervisor/services/${step.appId}`, + ); + for (const service of services) { + const servicePath = `/mnt/root/tmp/balena-supervisor/services/${step.appId}/${service}`; + const contents = await fs.readdir(servicePath); + if (contents.length) { + console.log(`Service ${service} has ${contents.join(', ')}`); + } + } }, releaseLock: async (step) => { await updateLock.releaseLock(step.appId); diff --git a/src/lib/update-lock.ts b/src/lib/update-lock.ts index 9e72dbf8..2a60f1fc 100644 --- a/src/lib/update-lock.ts +++ b/src/lib/update-lock.ts @@ -80,6 +80,11 @@ async function dispose( } } +const locksInProgress: { [appId: number]: boolean } = {}; + +export const isLockInProgress = (appId: number): boolean => + !!locksInProgress[appId]; + /** * Composition step used by Supervisor compose module. * Take all locks for an appId | appUuid, creating directories if they don't exist. @@ -96,7 +101,9 @@ export async function takeLock( }); const release = await takeGlobalLockRW(appId); + locksInProgress[appId] = true; try { + const lockOverride = await config.get('lockOverride'); const actuallyLocked: string[] = []; const locksTaken = await getServicesLockedByAppId(); // Filter out services that already have Supervisor-taken locks. @@ -107,7 +114,7 @@ export async function takeLock( ); for (const service of servicesWithoutLock) { await mkdirp(pathOnRoot(lockPath(appId, service))); - await lockService(appId, service, force); + await lockService(appId, service, force || lockOverride); actuallyLocked.push(service); } return actuallyLocked; @@ -116,12 +123,14 @@ export async function takeLock( // lockfiles that may have been created so that all services return // to unlocked status. await dispose(appId, release); + locksInProgress[appId] = false; // Re-throw error to be handled in caller throw err; } finally { // If not already released from catch, released the RW process lock. // If already released, this will not error. release(); + locksInProgress[appId] = false; } } @@ -215,7 +224,10 @@ export async function getLocksTaken( * [appId, serviceName] pair for a service to be considered locked. */ export async function getServicesLockedByAppId(): Promise { + console.log('In getServicesLockedByAppId'); const locksTaken = await getLocksTaken(); + console.log({ locksTaken }); + // Group locksTaken paths by appId & serviceName. // filesTakenByAppId is of type Map>> // and represents files taken under every [appId, serviceName] pair. @@ -223,11 +235,13 @@ export async function getServicesLockedByAppId(): Promise { for (const lockTakenPath of locksTaken) { const [appId, serviceName, filename] = getIdentifiersFromPath(lockTakenPath); + console.log({ appId, serviceName, filename }); if ( !StringIdentifier.is(appId) || !DockerName.is(serviceName) || !filename?.match(/updates\.lock/) ) { + console.log(`Skipping ${lockTakenPath}`); continue; } const numAppId = +appId; @@ -245,7 +259,9 @@ export async function getServicesLockedByAppId(): Promise { // services locked by the Supervisor. const servicesByAppId = new LocksTakenMap(); for (const [appId, servicesTaken] of filesTakenByAppId) { + console.log({ appId, servicesTaken }); for (const [serviceName, filenames] of servicesTaken) { + console.log({ serviceName, filenames }); if ( filenames.has('resin-updates.lock') && filenames.has('updates.lock') diff --git a/src/types/basic.ts b/src/types/basic.ts index 6a22345a..1c921527 100644 --- a/src/types/basic.ts +++ b/src/types/basic.ts @@ -96,7 +96,7 @@ const CONFIG_VAR_NAME_REGEX = /^[a-zA-Z_][a-zA-Z0-9_:]*$/; const shortStringWithRegex = (name: string, regex: RegExp, message: string) => new t.Type( name, - (s: unknown): s is string => ShortString.is(s) && VAR_NAME_REGEX.test(s), + (s: unknown): s is string => ShortString.is(s) && regex.test(s), (i, c) => pipe( ShortString.validate(i, c), diff --git a/test/integration/lib/update-lock.spec.ts b/test/integration/lib/update-lock.spec.ts index 67a92584..067a09dc 100644 --- a/test/integration/lib/update-lock.spec.ts +++ b/test/integration/lib/update-lock.spec.ts @@ -15,6 +15,11 @@ import { mkdirp } from '~/lib/fs-utils'; import { takeGlobalLockRW } from '~/lib/process-lock'; describe('lib/update-lock', () => { + beforeEach(async () => { + await config.initialized(); + await config.set({ lockOverride: false }); + }); + describe('abortIfHUPInProgress', () => { const breadcrumbFiles = [ 'rollback-health-breadcrumb', @@ -106,9 +111,6 @@ describe('lib/update-lock', () => { ).to.eventually.deep.equal(exists ? supportedLockfiles : []); before(async () => { - await config.initialized(); - await config.set({ lockOverride: false }); - // Ensure the directory is available for all tests await fs.mkdir(lockdir(testAppId, testServiceName), { recursive: true, diff --git a/test/unit/types.spec.ts b/test/unit/types.spec.ts index 744b6496..21d8475d 100644 --- a/test/unit/types.spec.ts +++ b/test/unit/types.spec.ts @@ -1,7 +1,7 @@ import { expect } from 'chai'; import * as t from 'io-ts'; -import { withDefault } from '~/src/types'; +import { withDefault, DockerName } from '~/src/types'; describe('types', () => { describe('withDefault', () => { @@ -39,4 +39,32 @@ describe('types', () => { .that.equals(false); }); }); + + describe('DockerName', () => { + // Should match /^[a-zA-Z0-9][a-zA-Z0-9_.-]*$/ + it('decodes valid Docker names', () => { + const validDockerNames = [ + 'a', + 'A', + '0', + 'a0', + 'A0', + 'a.', + 'A.', + 'a-', + 'A-', + 'a_', + 'A_', + 'a.a', + 'A.A', + 'a-a', + 'A-A', + 'a_a', + 'A_A', + ]; + for (const name of validDockerNames) { + expect(DockerName.is(name)).to.be.true; + } + }); + }); });