From eace7ed3fee354106385057cbc783d75671f034b Mon Sep 17 00:00:00 2001 From: Jason Rasmussen Date: Tue, 1 Oct 2024 10:16:30 -0400 Subject: [PATCH] feat(server): better mount checks --- docs/docs/administration/system-integrity.md | 47 ++++++++++ docs/docs/install/environment-variables.md | 1 + server/src/interfaces/config.interface.ts | 3 + server/src/repositories/config.repository.ts | 3 + server/src/services/database.service.spec.ts | 34 +++++--- server/src/services/storage.service.spec.ts | 24 +++++- server/src/services/storage.service.ts | 85 +++++++++++-------- .../repositories/config.repository.mock.ts | 21 +++-- 8 files changed, 157 insertions(+), 61 deletions(-) create mode 100644 docs/docs/administration/system-integrity.md diff --git a/docs/docs/administration/system-integrity.md b/docs/docs/administration/system-integrity.md new file mode 100644 index 0000000000000..5440e42489d83 --- /dev/null +++ b/docs/docs/administration/system-integrity.md @@ -0,0 +1,47 @@ +# System Integrity + +## Folder checks + +:::info +The folders considered for these checks include: `upload/`, `library/`, `thumbs/`, `encoded-video/`, `profile/` +::: + +When Immich starts, it performs a series of checks in order to validate that it can read and write files to the volume mounts used by the storage system. If it cannot perform all the required operations, it will fail to start. The checks include: + +- Creating an initial hidden file (`.immich`) in each folder +- Reading a hidden file (`.immich`) in each folder +- Overwriting a hidden file (`.immich`) in each folder + +The checks are designed to catch the following situations: + +- Incorrect permissions (cannot read/write files) +- Missing volume mount (`.immich` files should exist, but are missing) + +### Common issues + +:::note +`.immich` files serve as markers and help keep track of volume mounts being used by Immich. Except for the situations listed below, they should never be manually created or deleted. +::: + +#### Missing `.immich` files + +``` +Verifying system mount folder checks (enabled=true) +... +ENOENT: no such file or directory, open 'upload/encoded-video/.immich' +``` + +The above error messages show that the server has previously (successfully) written `.immich` files to each folder, but now does not detect them. This could be because any of the following: + +- Permission error - unable to read the file, but it exists +- File does not exist - volume mount has changed and should be corrected +- File does not exist - user manually deleted it and should be manually re-created (`touch .immich`) +- File does not exist - user restored from a backup, but did not restore each folder (user should restore all folders or manually create `.immich` in any missing folders) + +### Ignoring the checks + +The checks are designed to catch common problems that we have seen users have in the past, but if you want to disable them you can set the following environment variable: + +``` +IMMICH_IGNORE_MOUNT_CHECK_ERRORS=true +``` diff --git a/docs/docs/install/environment-variables.md b/docs/docs/install/environment-variables.md index 3944f6755b65a..29549586d359e 100644 --- a/docs/docs/install/environment-variables.md +++ b/docs/docs/install/environment-variables.md @@ -42,6 +42,7 @@ These environment variables are used by the `docker-compose.yml` file and do **N | `IMMICH_MICROSERVICES_METRICS_PORT` | Port for the OTEL metrics | `8082` | server | microservices | | `IMMICH_PROCESS_INVALID_IMAGES` | When `true`, generate thumbnails for invalid images | | server | microservices | | `IMMICH_TRUSTED_PROXIES` | List of comma separated IPs set as trusted proxies | | server | api | +| `IMMICH_IGNORE_MOUNT_CHECK_ERRORS` | See [System Integrity](/docs/administration/system-integrity) | | server | api, microservices | \*1: `TZ` should be set to a `TZ identifier` from [this list][tz-list]. For example, `TZ="Etc/UTC"`. `TZ` is used by `exiftool` as a fallback in case the timezone cannot be determined from the image metadata. It is also used for logfile timestamps and cron job execution. diff --git a/server/src/interfaces/config.interface.ts b/server/src/interfaces/config.interface.ts index 11bccbe348b1a..df999db18b570 100644 --- a/server/src/interfaces/config.interface.ts +++ b/server/src/interfaces/config.interface.ts @@ -7,6 +7,9 @@ export interface EnvData { skipMigrations: boolean; vectorExtension: VectorExtension; }; + storage: { + ignoreMountCheckErrors: boolean; + }; } export interface IConfigRepository { diff --git a/server/src/repositories/config.repository.ts b/server/src/repositories/config.repository.ts index f16fa3bbd4ef9..04d790b8123e8 100644 --- a/server/src/repositories/config.repository.ts +++ b/server/src/repositories/config.repository.ts @@ -10,6 +10,9 @@ export class ConfigRepository implements IConfigRepository { skipMigrations: process.env.DB_SKIP_MIGRATIONS === 'true', vectorExtension: getVectorExtension(), }, + storage: { + ignoreMountCheckErrors: process.env.IMMICH_IGNORE_MOUNT_CHECK_ERRORS === 'true', + }, }; } } diff --git a/server/src/services/database.service.spec.ts b/server/src/services/database.service.spec.ts index fc8130cadc273..5bce6d819ccb8 100644 --- a/server/src/services/database.service.spec.ts +++ b/server/src/services/database.service.spec.ts @@ -7,7 +7,7 @@ import { } from 'src/interfaces/database.interface'; import { ILoggerRepository } from 'src/interfaces/logger.interface'; import { DatabaseService } from 'src/services/database.service'; -import { newConfigRepositoryMock } from 'test/repositories/config.repository.mock'; +import { mockEnvData, newConfigRepositoryMock } from 'test/repositories/config.repository.mock'; import { newDatabaseRepositoryMock } from 'test/repositories/database.repository.mock'; import { newLoggerRepositoryMock } from 'test/repositories/logger.repository.mock'; import { Mocked } from 'vitest'; @@ -60,7 +60,9 @@ describe(DatabaseService.name, () => { { extension: DatabaseExtension.VECTORS, extensionName: EXTENSION_NAMES[DatabaseExtension.VECTORS] }, ])('should work with $extensionName', ({ extension, extensionName }) => { beforeEach(() => { - configMock.getEnv.mockReturnValue({ database: { skipMigrations: false, vectorExtension: extension } }); + configMock.getEnv.mockReturnValue( + mockEnvData({ database: { skipMigrations: false, vectorExtension: extension } }), + ); }); it(`should start up successfully with ${extension}`, async () => { @@ -244,12 +246,14 @@ describe(DatabaseService.name, () => { }); it('should skip migrations if DB_SKIP_MIGRATIONS=true', async () => { - configMock.getEnv.mockReturnValue({ - database: { - skipMigrations: true, - vectorExtension: DatabaseExtension.VECTORS, - }, - }); + configMock.getEnv.mockReturnValue( + mockEnvData({ + database: { + skipMigrations: true, + vectorExtension: DatabaseExtension.VECTORS, + }, + }), + ); await expect(sut.onBootstrap()).resolves.toBeUndefined(); @@ -257,12 +261,14 @@ describe(DatabaseService.name, () => { }); it(`should throw error if pgvector extension could not be created`, async () => { - configMock.getEnv.mockReturnValue({ - database: { - skipMigrations: true, - vectorExtension: DatabaseExtension.VECTOR, - }, - }); + configMock.getEnv.mockReturnValue( + mockEnvData({ + database: { + skipMigrations: true, + vectorExtension: DatabaseExtension.VECTOR, + }, + }), + ); databaseMock.getExtensionVersion.mockResolvedValue({ installedVersion: null, availableVersion: minVersionInRange, diff --git a/server/src/services/storage.service.spec.ts b/server/src/services/storage.service.spec.ts index 930fb3c726e2f..e0717df66860e 100644 --- a/server/src/services/storage.service.spec.ts +++ b/server/src/services/storage.service.spec.ts @@ -1,9 +1,11 @@ import { SystemMetadataKey } from 'src/enum'; +import { IConfigRepository } from 'src/interfaces/config.interface'; import { IDatabaseRepository } from 'src/interfaces/database.interface'; import { ILoggerRepository } from 'src/interfaces/logger.interface'; import { IStorageRepository } from 'src/interfaces/storage.interface'; import { ISystemMetadataRepository } from 'src/interfaces/system-metadata.interface'; import { StorageService } from 'src/services/storage.service'; +import { mockEnvData, newConfigRepositoryMock } from 'test/repositories/config.repository.mock'; import { newDatabaseRepositoryMock } from 'test/repositories/database.repository.mock'; import { newLoggerRepositoryMock } from 'test/repositories/logger.repository.mock'; import { newStorageRepositoryMock } from 'test/repositories/storage.repository.mock'; @@ -12,18 +14,20 @@ import { Mocked } from 'vitest'; describe(StorageService.name, () => { let sut: StorageService; + let configMock: Mocked; let databaseMock: Mocked; let storageMock: Mocked; let loggerMock: Mocked; let systemMock: Mocked; beforeEach(() => { + configMock = newConfigRepositoryMock(); databaseMock = newDatabaseRepositoryMock(); storageMock = newStorageRepositoryMock(); loggerMock = newLoggerRepositoryMock(); systemMock = newSystemMetadataRepositoryMock(); - sut = new StorageService(databaseMock, storageMock, loggerMock, systemMock); + sut = new StorageService(configMock, databaseMock, storageMock, loggerMock, systemMock); }); it('should work', () => { @@ -52,7 +56,7 @@ describe(StorageService.name, () => { systemMock.get.mockResolvedValue({ mountFiles: true }); storageMock.readFile.mockRejectedValue(new Error("ENOENT: no such file or directory, open '/app/.immich'")); - await expect(sut.onBootstrap()).rejects.toThrow('Failed to validate folder mount'); + await expect(sut.onBootstrap()).rejects.toThrow('Failed to read'); expect(storageMock.createOrOverwriteFile).not.toHaveBeenCalled(); expect(systemMock.set).not.toHaveBeenCalled(); @@ -62,7 +66,21 @@ describe(StorageService.name, () => { systemMock.get.mockResolvedValue({ mountFiles: true }); storageMock.overwriteFile.mockRejectedValue(new Error("ENOENT: no such file or directory, open '/app/.immich'")); - await expect(sut.onBootstrap()).rejects.toThrow('Failed to validate folder mount'); + await expect(sut.onBootstrap()).rejects.toThrow('Failed to write'); + + expect(systemMock.set).not.toHaveBeenCalled(); + }); + + it('should startup if checks are disabled', async () => { + systemMock.get.mockResolvedValue({ mountFiles: true }); + configMock.getEnv.mockReturnValue( + mockEnvData({ + storage: { ignoreMountCheckErrors: true }, + }), + ); + storageMock.overwriteFile.mockRejectedValue(new Error("ENOENT: no such file or directory, open '/app/.immich'")); + + await expect(sut.onBootstrap()).resolves.toBeUndefined(); expect(systemMock.set).not.toHaveBeenCalled(); }); diff --git a/server/src/services/storage.service.ts b/server/src/services/storage.service.ts index b32e48ea498ad..e2c1ef28e20ed 100644 --- a/server/src/services/storage.service.ts +++ b/server/src/services/storage.service.ts @@ -3,6 +3,7 @@ import { join } from 'node:path'; import { StorageCore } from 'src/cores/storage.core'; import { OnEvent } from 'src/decorators'; import { StorageFolder, SystemMetadataKey } from 'src/enum'; +import { IConfigRepository } from 'src/interfaces/config.interface'; import { DatabaseLock, IDatabaseRepository } from 'src/interfaces/database.interface'; import { IDeleteFilesJob, JobStatus } from 'src/interfaces/job.interface'; import { ILoggerRepository } from 'src/interfaces/logger.interface'; @@ -10,9 +11,12 @@ import { IStorageRepository } from 'src/interfaces/storage.interface'; import { ISystemMetadataRepository } from 'src/interfaces/system-metadata.interface'; import { ImmichStartupError } from 'src/utils/events'; +const docsMessage = `Please see https://immich.app/docs/administration/system-integrity#folder-checks for more information.`; + @Injectable() export class StorageService { constructor( + @Inject(IConfigRepository) private configRepository: IConfigRepository, @Inject(IDatabaseRepository) private databaseRepository: IDatabaseRepository, @Inject(IStorageRepository) private storageRepository: IStorageRepository, @Inject(ILoggerRepository) private logger: ILoggerRepository, @@ -23,30 +27,41 @@ export class StorageService { @OnEvent({ name: 'app.bootstrap' }) async onBootstrap() { + const envData = this.configRepository.getEnv(); + await this.databaseRepository.withLock(DatabaseLock.SystemFileMounts, async () => { const flags = (await this.systemMetadata.get(SystemMetadataKey.SYSTEM_FLAGS)) || { mountFiles: false }; const enabled = flags.mountFiles ?? false; this.logger.log(`Verifying system mount folder checks (enabled=${enabled})`); - // check each folder exists and is writable - for (const folder of Object.values(StorageFolder)) { - if (!enabled) { - this.logger.log(`Writing initial mount file for the ${folder} folder`); - await this.createMountFile(folder); + try { + // check each folder exists and is writable + for (const folder of Object.values(StorageFolder)) { + if (!enabled) { + this.logger.log(`Writing initial mount file for the ${folder} folder`); + await this.createMountFile(folder); + } + + await this.verifyReadAccess(folder); + await this.verifyWriteAccess(folder); } - await this.verifyReadAccess(folder); - await this.verifyWriteAccess(folder); - } + if (!flags.mountFiles) { + flags.mountFiles = true; + await this.systemMetadata.set(SystemMetadataKey.SYSTEM_FLAGS, flags); + this.logger.log('Successfully enabled system mount folders checks'); + } - if (!flags.mountFiles) { - flags.mountFiles = true; - await this.systemMetadata.set(SystemMetadataKey.SYSTEM_FLAGS, flags); - this.logger.log('Successfully enabled system mount folders checks'); + this.logger.log('Successfully verified system mount folder checks'); + } catch (error) { + if (envData.storage.ignoreMountCheckErrors) { + this.logger.error(error); + this.logger.warn('Ignoring mount folder errors'); + } else { + throw error; + } } - - this.logger.log('Successfully verified system mount folder checks'); }); } @@ -70,49 +85,45 @@ export class StorageService { } private async verifyReadAccess(folder: StorageFolder) { - const { filePath } = this.getMountFilePaths(folder); + const { internalPath, externalPath } = this.getMountFilePaths(folder); try { - await this.storageRepository.readFile(filePath); + await this.storageRepository.readFile(internalPath); } catch (error) { - this.logger.error(`Failed to read ${filePath}: ${error}`); - this.logger.error( - `The "${folder}" folder appears to be offline/missing, please make sure the volume is mounted with the correct permissions`, - ); - throw new ImmichStartupError(`Failed to validate folder mount (read from "/${folder}")`); + this.logger.error(`Failed to read ${internalPath}: ${error}`); + throw new ImmichStartupError(`Failed to read "${externalPath} - ${docsMessage}"`); } } private async createMountFile(folder: StorageFolder) { - const { folderPath, filePath } = this.getMountFilePaths(folder); + const { folderPath, internalPath, externalPath } = this.getMountFilePaths(folder); try { this.storageRepository.mkdirSync(folderPath); - await this.storageRepository.createFile(filePath, Buffer.from(`${Date.now()}`)); + await this.storageRepository.createFile(internalPath, Buffer.from(`${Date.now()}`)); } catch (error) { - this.logger.error(`Failed to create ${filePath}: ${error}`); - this.logger.error( - `The "${folder}" folder cannot be written to, please make sure the volume is mounted with the correct permissions`, - ); - throw new ImmichStartupError(`Failed to validate folder mount (write to "/${folder}")`); + if ((error as NodeJS.ErrnoException).code === 'EEXIST') { + this.logger.warn('Found existing mount file, skipping creation'); + return; + } + this.logger.error(`Failed to create ${internalPath}: ${error}`); + throw new ImmichStartupError(`Failed to create "${externalPath} - ${docsMessage}"`); } } private async verifyWriteAccess(folder: StorageFolder) { - const { filePath } = this.getMountFilePaths(folder); + const { internalPath, externalPath } = this.getMountFilePaths(folder); try { - await this.storageRepository.overwriteFile(filePath, Buffer.from(`${Date.now()}`)); + await this.storageRepository.overwriteFile(internalPath, Buffer.from(`${Date.now()}`)); } catch (error) { - this.logger.error(`Failed to write ${filePath}: ${error}`); - this.logger.error( - `The "${folder}" folder cannot be written to, please make sure the volume is mounted with the correct permissions`, - ); - throw new ImmichStartupError(`Failed to validate folder mount (write to "/${folder}")`); + this.logger.error(`Failed to write ${internalPath}: ${error}`); + throw new ImmichStartupError(`Failed to write "${externalPath} - ${docsMessage}"`); } } private getMountFilePaths(folder: StorageFolder) { const folderPath = StorageCore.getBaseFolder(folder); - const filePath = join(folderPath, '.immich'); + const internalPath = join(folderPath, '.immich'); + const externalPath = `/${folder}/.immich`; - return { folderPath, filePath }; + return { folderPath, internalPath, externalPath }; } } diff --git a/server/test/repositories/config.repository.mock.ts b/server/test/repositories/config.repository.mock.ts index 40110186f43f3..d18aa1b981e10 100644 --- a/server/test/repositories/config.repository.mock.ts +++ b/server/test/repositories/config.repository.mock.ts @@ -1,14 +1,21 @@ -import { IConfigRepository } from 'src/interfaces/config.interface'; +import { EnvData, IConfigRepository } from 'src/interfaces/config.interface'; import { DatabaseExtension } from 'src/interfaces/database.interface'; import { Mocked, vitest } from 'vitest'; +const envData: EnvData = { + database: { + skipMigrations: false, + vectorExtension: DatabaseExtension.VECTORS, + }, + storage: { + ignoreMountCheckErrors: false, + }, +}; + export const newConfigRepositoryMock = (): Mocked => { return { - getEnv: vitest.fn().mockReturnValue({ - database: { - skipMigration: false, - vectorExtension: DatabaseExtension.VECTORS, - }, - }), + getEnv: vitest.fn().mockReturnValue(envData), }; }; + +export const mockEnvData = (config: Partial) => ({ ...envData, ...config });