Move Supervisor-specific from lockfile.ts to update-lock.ts to

make lockfile module more generic

BASE_LOCK_DIR, LOCKFILE_UID moved to update-lock.ts

Signed-off-by: Christina Wang <christina@balena.io>
This commit is contained in:
Christina Wang 2022-04-11 14:07:36 -07:00
parent cfd3f03e4a
commit babe10e2a7
4 changed files with 46 additions and 34 deletions

View File

@ -1,17 +1,11 @@
import * as fs from 'fs'; import * as fs from 'fs';
import * as os from 'os';
import { dirname } from 'path'; import { dirname } from 'path';
import { isRight } from 'fp-ts/lib/Either';
import { exec, unlinkAll } from './fs-utils'; import { exec, unlinkAll } from './fs-utils';
import { NumericIdentifier } from '../types';
// Equivalent to `drwxrwxrwt` // Equivalent to `drwxrwxrwt`
const STICKY_WRITE_PERMISSIONS = 0o1777; const STICKY_WRITE_PERMISSIONS = 0o1777;
export const BASE_LOCK_DIR =
process.env.BASE_LOCK_DIR || '/tmp/balena-supervisor/services';
const decodedUid = NumericIdentifier.decode(process.env.LOCKFILE_UID);
export const LOCKFILE_UID = isRight(decodedUid) ? decodedUid.right : 65534;
/** /**
* Internal lockfile manager to track files in memory * Internal lockfile manager to track files in memory
@ -61,7 +55,7 @@ export class LockfileExistsError implements ChildProcessError {
} }
} }
export async function lock(path: string, uid = LOCKFILE_UID) { export async function lock(path: string, uid: number = os.userInfo().uid) {
/** /**
* Set parent directory permissions to `drwxrwxrwt` (octal 1777), which are needed * Set parent directory permissions to `drwxrwxrwt` (octal 1777), which are needed
* for lockfile binary to run successfully as the any non-root uid, if executing * for lockfile binary to run successfully as the any non-root uid, if executing
@ -71,7 +65,9 @@ export async function lock(path: string, uid = LOCKFILE_UID) {
* *
* `chmod` does not fail or throw if the directory already has the proper permissions. * `chmod` does not fail or throw if the directory already has the proper permissions.
*/ */
await fs.promises.chmod(dirname(path), STICKY_WRITE_PERMISSIONS); if (uid !== 0) {
await fs.promises.chmod(dirname(path), STICKY_WRITE_PERMISSIONS);
}
/** /**
* Run the lockfile binary as the provided UID. See https://linux.die.net/man/1/lockfile * Run the lockfile binary as the provided UID. See https://linux.die.net/man/1/lockfile
@ -102,16 +98,16 @@ export async function lock(path: string, uid = LOCKFILE_UID) {
* - other systems-based errors * - other systems-based errors
*/ */
throw new Error( throw new Error(
`Got code ${(error as ChildProcessError).code} while locking updates: ${ `Got code ${
(error as ChildProcessError).stderr (error as ChildProcessError).code
}`, } while trying to take lock: ${(error as ChildProcessError).stderr}`,
); );
} }
} }
} }
export async function unlock(path: string): Promise<void> { export async function unlock(path: string): Promise<void> {
// Removing the updates.lock file releases the lock // Removing the lockfile releases the lock
await unlinkAll(path); await unlinkAll(path);
// Remove lockfile's in-memory tracking of a file // Remove lockfile's in-memory tracking of a file
delete locksTaken[path]; delete locksTaken[path];

View File

@ -3,6 +3,7 @@ import * as _ from 'lodash';
import { promises as fs } from 'fs'; import { promises as fs } from 'fs';
import * as path from 'path'; import * as path from 'path';
import * as Lock from 'rwlock'; import * as Lock from 'rwlock';
import { isRight } from 'fp-ts/lib/Either';
import * as constants from './constants'; import * as constants from './constants';
import { import {
@ -13,9 +14,16 @@ import {
import { getPathOnHost, pathExistsOnHost } from './fs-utils'; import { getPathOnHost, pathExistsOnHost } from './fs-utils';
import * as config from '../config'; import * as config from '../config';
import * as lockfile from './lockfile'; import * as lockfile from './lockfile';
import { NumericIdentifier } from '../types';
const decodedUid = NumericIdentifier.decode(process.env.LOCKFILE_UID);
export const LOCKFILE_UID = isRight(decodedUid) ? decodedUid.right : 65534;
export const BASE_LOCK_DIR =
process.env.BASE_LOCK_DIR || '/tmp/balena-supervisor/services';
export function lockPath(appId: number, serviceName?: string): string { export function lockPath(appId: number, serviceName?: string): string {
return path.join(lockfile.BASE_LOCK_DIR, appId.toString(), serviceName ?? ''); return path.join(BASE_LOCK_DIR, appId.toString(), serviceName ?? '');
} }
function lockFilesOnHost(appId: number, serviceName: string): string[] { function lockFilesOnHost(appId: number, serviceName: string): string[] {
@ -69,7 +77,7 @@ function dispose(
): Bluebird<void> { ): Bluebird<void> {
return Bluebird.map( return Bluebird.map(
lockfile.getLocksTaken((p: string) => lockfile.getLocksTaken((p: string) =>
p.includes(`${lockfile.BASE_LOCK_DIR}/${appIdentifier}`), p.includes(`${BASE_LOCK_DIR}/${appIdentifier}`),
), ),
(lockName) => { (lockName) => {
return lockfile.unlock(lockName); return lockfile.unlock(lockName);
@ -117,7 +125,7 @@ export function lock<T extends unknown>(
} }
}) })
.then(() => { .then(() => {
return lockfile.lock(tmpLockName); return lockfile.lock(tmpLockName, LOCKFILE_UID);
}) })
// If lockfile exists, throw a user-friendly error. // If lockfile exists, throw a user-friendly error.
// Otherwise throw the error as-is. // Otherwise throw the error as-is.

View File

@ -6,22 +6,23 @@ import mock = require('mock-fs');
import * as lockfile from '../../../src/lib/lockfile'; import * as lockfile from '../../../src/lib/lockfile';
import * as fsUtils from '../../../src/lib/fs-utils'; import * as fsUtils from '../../../src/lib/fs-utils';
import { BASE_LOCK_DIR, LOCKFILE_UID } from '../../../src/lib/update-lock';
describe('lib/lockfile', () => { describe('lib/lockfile', () => {
const lockPath = `${lockfile.BASE_LOCK_DIR}/1234567/one/updates.lock`; const lockPath = `${BASE_LOCK_DIR}/1234567/one/updates.lock`;
const lockPath2 = `${lockfile.BASE_LOCK_DIR}/7654321/two/updates.lock`; const lockPath2 = `${BASE_LOCK_DIR}/7654321/two/updates.lock`;
const mockDir = (opts: { createLock: boolean } = { createLock: false }) => { const mockDir = (opts: { createLock: boolean } = { createLock: false }) => {
mock({ mock({
[lockfile.BASE_LOCK_DIR]: { [BASE_LOCK_DIR]: {
'1234567': { '1234567': {
one: opts.createLock one: opts.createLock
? { 'updates.lock': mock.file({ uid: lockfile.LOCKFILE_UID }) } ? { 'updates.lock': mock.file({ uid: LOCKFILE_UID }) }
: {}, : {},
}, },
'7654321': { '7654321': {
two: opts.createLock two: opts.createLock
? { 'updates.lock': mock.file({ uid: lockfile.LOCKFILE_UID }) } ? { 'updates.lock': mock.file({ uid: LOCKFILE_UID }) }
: {}, : {},
}, },
}, },
@ -58,7 +59,7 @@ describe('lib/lockfile', () => {
await fs.chown(targetPath, opts!.uid!, 0); await fs.chown(targetPath, opts!.uid!, 0);
}); });
mock({ [lockfile.BASE_LOCK_DIR]: {} }); mock({ [BASE_LOCK_DIR]: {} });
}); });
afterEach(async () => { afterEach(async () => {
@ -75,13 +76,13 @@ describe('lib/lockfile', () => {
it('should create a lockfile as the `nobody` user at target path', async () => { it('should create a lockfile as the `nobody` user at target path', async () => {
mockDir(); mockDir();
await lockfile.lock(lockPath); await lockfile.lock(lockPath, LOCKFILE_UID);
// Verify lockfile exists // Verify lockfile exists
await checkLockDirFiles(lockPath, { shouldExist: true }); await checkLockDirFiles(lockPath, { shouldExist: true });
// Verify lockfile UID // Verify lockfile UID
expect((await fs.stat(lockPath)).uid).to.equal(lockfile.LOCKFILE_UID); expect((await fs.stat(lockPath)).uid).to.equal(LOCKFILE_UID);
}); });
it('should create a lockfile with the provided `uid` if specified', async () => { it('should create a lockfile with the provided `uid` if specified', async () => {
@ -109,7 +110,7 @@ describe('lib/lockfile', () => {
execStub = stub(fsUtils, 'exec').throws(childProcessError); execStub = stub(fsUtils, 'exec').throws(childProcessError);
try { try {
await lockfile.lock(lockPath); await lockfile.lock(lockPath, LOCKFILE_UID);
expect.fail('lockfile.lock should throw an error'); expect.fail('lockfile.lock should throw an error');
} catch (err) { } catch (err) {
expect(err).to.exist; expect(err).to.exist;
@ -159,8 +160,8 @@ describe('lib/lockfile', () => {
mockDir({ createLock: false }); mockDir({ createLock: false });
// Create lockfiles for multiple appId / uuids // Create lockfiles for multiple appId / uuids
await lockfile.lock(lockPath); await lockfile.lock(lockPath, LOCKFILE_UID);
await lockfile.lock(lockPath2); await lockfile.lock(lockPath2, LOCKFILE_UID);
// @ts-ignore // @ts-ignore
process.emit('exit'); process.emit('exit');
@ -174,8 +175,8 @@ describe('lib/lockfile', () => {
mockDir({ createLock: false }); mockDir({ createLock: false });
// Create lockfiles for multiple appId / uuids // Create lockfiles for multiple appId / uuids
await lockfile.lock(lockPath); await lockfile.lock(lockPath, LOCKFILE_UID);
await lockfile.lock(lockPath2); await lockfile.lock(lockPath2, LOCKFILE_UID);
expect( expect(
lockfile.getLocksTaken((path) => path.includes('1234567')), lockfile.getLocksTaken((path) => path.includes('1234567')),

View File

@ -23,10 +23,10 @@ describe('lib/update-lock', () => {
const lockDirFiles: any = {}; const lockDirFiles: any = {};
if (createLockfile) { if (createLockfile) {
lockDirFiles['updates.lock'] = mockFs.file({ lockDirFiles['updates.lock'] = mockFs.file({
uid: lockfile.LOCKFILE_UID, uid: updateLock.LOCKFILE_UID,
}); });
lockDirFiles['resin-updates.lock'] = mockFs.file({ lockDirFiles['resin-updates.lock'] = mockFs.file({
uid: lockfile.LOCKFILE_UID, uid: updateLock.LOCKFILE_UID,
}); });
} }
mockFs({ mockFs({
@ -177,7 +177,9 @@ describe('lib/update-lock', () => {
expect(lockSpy.args).to.have.length(2); expect(lockSpy.args).to.have.length(2);
// Everything that was locked should have been unlocked // Everything that was locked should have been unlocked
expect(lockSpy.args).to.deep.equal(unlockSpy.args); expect(lockSpy.args.map(([lock]) => [lock])).to.deep.equal(
unlockSpy.args,
);
}); });
it('should throw UpdatesLockedError if lockfile exists', async () => { it('should throw UpdatesLockedError if lockfile exists', async () => {
@ -199,7 +201,10 @@ describe('lib/update-lock', () => {
} }
// Should only have attempted to take `updates.lock` // Should only have attempted to take `updates.lock`
expect(lockSpy.args.flat()).to.deep.equal([lockPath]); expect(lockSpy.args.flat()).to.deep.equal([
lockPath,
updateLock.LOCKFILE_UID,
]);
// Since the lock-taking failed, there should be no locks to dispose of // Since the lock-taking failed, there should be no locks to dispose of
expect(lockfile.getLocksTaken()).to.have.length(0); expect(lockfile.getLocksTaken()).to.have.length(0);
@ -232,7 +237,9 @@ describe('lib/update-lock', () => {
expect(lockSpy.args).to.have.length(2); expect(lockSpy.args).to.have.length(2);
// Everything that was locked should have been unlocked // Everything that was locked should have been unlocked
expect(lockSpy.args).to.deep.equal(unlockSpy.args); expect(lockSpy.args.map(([lock]) => [lock])).to.deep.equal(
unlockSpy.args,
);
}); });
it('resolves input function without locking when appId is null', async () => { it('resolves input function without locking when appId is null', async () => {