From 4c0773aee4fb6ba57e2a4ce751034b0fc8c17441 Mon Sep 17 00:00:00 2001 From: Chris Sherwood Date: Tue, 6 Oct 2026 16:34:16 -0700 Subject: [PATCH] fix(docker): cap container logs so they can't fill the root disk Docker's default json-file log never rotates, and NOMAD set no log options on any container. On a small root disk an unbounded nomad_admin log filled / and took MySQL down with it (#1412). - management compose: a shared x-logging anchor caps every service at 3 x 10 MB. - app containers: the admin now adds the same cap when it creates, updates, or migrates a container, but only when the container would otherwise get an unbounded json-file log. Another driver, or an existing max-size from the user or the daemon's log-opts, is left alone (max-size is not valid for most other drivers and would fail the create). - the update path previously dropped LogConfig entirely; it now carries it over. Closes #1412 Co-Authored-By: Claude Opus 5.5 --- admin/app/services/docker_service.ts | 32 ++++++++++++++++- admin/app/utils/log_rotation.ts | 42 +++++++++++++++++++++++ admin/tests/unit/log_rotation.spec.ts | 49 +++++++++++++++++++++++++++ install/management_compose.yaml | 15 ++++++++ 4 files changed, 137 insertions(+), 1 deletion(-) create mode 100644 admin/app/utils/log_rotation.ts create mode 100644 admin/tests/unit/log_rotation.spec.ts diff --git a/admin/app/services/docker_service.ts b/admin/app/services/docker_service.ts index ca8fc0d2..3c91965c 100644 --- a/admin/app/services/docker_service.ts +++ b/admin/app/services/docker_service.ts @@ -27,6 +27,7 @@ import KVStore from '#models/kv_store' import { BROADCAST_CHANNELS } from '../../constants/broadcast.js' import { KIWIX_LIBRARY_CMD } from '../../constants/kiwix.js' import { DEFAULT_OLLAMA_CONTEXT_LENGTH } from '../../constants/ollama.js' +import { withLogRotation } from '../utils/log_rotation.js' // Written by install_nomad.sh with the host AMD GPU's gfx target. const AMD_GFX_MARKER_PATH = '/app/storage/.nomad-amd-gfx' @@ -53,6 +54,10 @@ export class DockerService { private _servicesStatusCache: { data: { service_name: string; status: string }[]; expiresAt: number } | null = null private _servicesStatusInflight: Promise<{ service_name: string; status: string }[]> | null = null + // The daemon's default log driver, resolved once. Log rotation is only applied + // when containers would otherwise land on an unbounded json-file log (#1412). + private _daemonLogDriver: Promise | null = null + constructor() { // Support both Linux (production) and Windows (development with Docker Desktop) const isWindows = process.platform === 'win32' @@ -864,6 +869,8 @@ export class DockerService { 'creating', `Creating Docker container for service ${service.service_name}...` ) + gpuHostConfig = await this._withLogRotation(gpuHostConfig) + // Built once and reused, so the AMD fallback below cannot drift from the // real payload as this config grows. const buildCreateOptions = (image: string, hostConfig: any, env: string[]) => ({ @@ -1433,7 +1440,7 @@ export class DockerService { const newContainer = await this.docker.createContainer({ Image: service.container_image, name: service.service_name, - HostConfig: containerConfig?.HostConfig ?? {}, + HostConfig: await this._withLogRotation(containerConfig?.HostConfig ?? {}), ...(containerConfig?.ExposedPorts && { ExposedPorts: containerConfig.ExposedPorts }), Cmd: KIWIX_LIBRARY_CMD.split(' '), ...(process.env.NODE_ENV === 'production' && { @@ -1923,6 +1930,7 @@ export class DockerService { RestartPolicy: hostConfig.RestartPolicy || undefined, DeviceRequests: serviceName === SERVICE_NAMES.OLLAMA ? updatedDeviceRequests : (hostConfig.DeviceRequests || undefined), Devices: serviceName === SERVICE_NAMES.OLLAMA && updatedAmdDevices ? updatedAmdDevices : (hostConfig.Devices || undefined), + LogConfig: hostConfig.LogConfig || undefined, }, NetworkingConfig: inspectData.NetworkSettings?.Networks ? { @@ -1939,6 +1947,7 @@ export class DockerService { delete newContainerConfig.HostConfig[key] } }) + newContainerConfig.HostConfig = await this._withLogRotation(newContainerConfig.HostConfig) let newContainer: any try { @@ -2065,6 +2074,27 @@ export class DockerService { logger.info(`[DockerService] [${service}] ${status}: ${message}`) } + + /** + * Add a size cap to `hostConfig`'s log config when the container would + * otherwise get Docker's unbounded json-file log (#1412). Anything the user + * chose, a different driver or their own `max-size`, is left alone. + */ + private async _withLogRotation>(hostConfig: T): Promise { + if (!this._daemonLogDriver) { + this._daemonLogDriver = this.docker + .info() + .then((info: any) => info?.LoggingDriver as string | undefined) + .catch(() => { + // Don't memoize a failure; try again on the next container create. + this._daemonLogDriver = null + return undefined + }) + } + const logConfig = withLogRotation(hostConfig?.LogConfig, await this._daemonLogDriver) + return logConfig ? { ...hostConfig, LogConfig: logConfig } : hostConfig + } + private _parseContainerConfig(containerConfig: any): any { if (!containerConfig) { return {} diff --git a/admin/app/utils/log_rotation.ts b/admin/app/utils/log_rotation.ts new file mode 100644 index 00000000..b26e7723 --- /dev/null +++ b/admin/app/utils/log_rotation.ts @@ -0,0 +1,42 @@ +/** + * Log rotation for the containers NOMAD creates. + * + * Docker's default `json-file` driver keeps every line a container ever writes, + * in one file, forever. On a small root disk that is an outage waiting to happen: + * the log fills `/`, and MySQL is the first thing to fall over (#1412). Capping + * each container at a few rotated files bounds the cost at ~30 MB. + * + * Kept pure so it runs under bare `node --test`; DockerService supplies the + * daemon's default driver. + */ + +export const LOG_MAX_SIZE = '10m' +export const LOG_MAX_FILE = '3' + +export type LogConfig = { Type?: string; Config?: Record } + +/** + * Return the LogConfig a new container should be created with, or undefined to + * leave Docker's default untouched. + * + * Only `json-file` is ever touched. Any other driver (journald, local, syslog...) + * was chosen by someone on purpose, and `max-size` is not even a valid option for + * most of them, so passing it would make container creation fail. Likewise a + * json-file config that already sets `max-size` (from the container's own config + * or the daemon's `log-opts`) is the user's call and is kept as-is. + */ +export function withLogRotation( + existing: LogConfig | undefined, + daemonDefaultDriver: string | undefined +): LogConfig | undefined { + const driver = existing?.Type || daemonDefaultDriver + if (driver !== 'json-file') return existing + + const config = existing?.Config ?? {} + if (config['max-size']) return existing + + return { + Type: 'json-file', + Config: { ...config, 'max-size': LOG_MAX_SIZE, 'max-file': LOG_MAX_FILE }, + } +} diff --git a/admin/tests/unit/log_rotation.spec.ts b/admin/tests/unit/log_rotation.spec.ts new file mode 100644 index 00000000..fad11e48 --- /dev/null +++ b/admin/tests/unit/log_rotation.spec.ts @@ -0,0 +1,49 @@ +import * as assert from 'node:assert/strict' +import { test } from 'node:test' + +import { LOG_MAX_FILE, LOG_MAX_SIZE, withLogRotation } from '../../app/utils/log_rotation.js' + +const ROTATED = { + Type: 'json-file', + Config: { 'max-size': LOG_MAX_SIZE, 'max-file': LOG_MAX_FILE }, +} + +/** Regression: #1412. An unbounded nomad_admin log filled / and took MySQL down. */ +test('a new container on a json-file daemon gets a size cap', () => { + assert.deepEqual(withLogRotation(undefined, 'json-file'), ROTATED) +}) + +test('an inspected container with an empty json-file config gets a size cap', () => { + // What `docker inspect` reports for a container created with no log options. + assert.deepEqual(withLogRotation({ Type: 'json-file', Config: {} }, 'json-file'), ROTATED) +}) + +test('an existing max-size is kept, whether from the daemon log-opts or the user', () => { + const existing = { Type: 'json-file', Config: { 'max-size': '50m', 'max-file': '5' } } + assert.deepEqual(withLogRotation(existing, 'json-file'), existing) +}) + +test('unrelated json-file options are preserved alongside the cap', () => { + const result = withLogRotation({ Type: 'json-file', Config: { compress: 'true' } }, 'json-file') + assert.deepEqual(result?.Config, { + 'compress': 'true', + 'max-size': LOG_MAX_SIZE, + 'max-file': LOG_MAX_FILE, + }) +}) + +test('a non-json-file daemon default is left alone', () => { + // max-size is not a valid journald option; passing it would fail container creation. + for (const driver of ['journald', 'local', 'syslog', 'none']) { + assert.equal(withLogRotation(undefined, driver), undefined, driver) + } +}) + +test('an explicit non-json-file driver on the container is kept', () => { + const existing = { Type: 'local', Config: {} } + assert.deepEqual(withLogRotation(existing, 'json-file'), existing) +}) + +test('an unknown daemon driver leaves the config untouched rather than guessing', () => { + assert.equal(withLogRotation(undefined, undefined), undefined) +}) diff --git a/install/management_compose.yaml b/install/management_compose.yaml index 7dd6a51d..0c727cad 100644 --- a/install/management_compose.yaml +++ b/install/management_compose.yaml @@ -7,11 +7,21 @@ # which must be updated for the admin server to start successfully. The default values are optimized for ease of installation and use, # but you can customize them as needed (e.g. changing ports, database connection details, log level, etc.) name: project-nomad + +# Cap every container's log at 3 x 10 MB. Docker's default json-file log never rotates, and on a +# small root disk an unbounded log can fill / and take MySQL down with it (#1412). +x-logging: &default-logging + driver: json-file + options: + max-size: "10m" + max-file: "3" + services: admin: image: ghcr.io/crosstalk-solutions/project-nomad:latest pull_policy: always container_name: nomad_admin + logging: *default-logging restart: unless-stopped extra_hosts: - "host.docker.internal:host-gateway" # Enables host.docker.internal on Linux @@ -74,6 +84,7 @@ services: # Dozzle is an optional container that allows for easily viewing container logs. We recommend including it unless you have a specific reason not to. Note that if you don't install it, the "Service Logs & Metrics" link in Settings that launches Dozzle will not work. image: amir20/dozzle:v10.0 container_name: nomad_dozzle + logging: *default-logging restart: unless-stopped ports: - "9999:8080" @@ -85,6 +96,7 @@ services: mysql: image: mysql:8.0 container_name: nomad_mysql + logging: *default-logging restart: unless-stopped environment: - MYSQL_ROOT_PASSWORD=replaceme @@ -102,6 +114,7 @@ services: redis: image: redis:7-alpine container_name: nomad_redis + logging: *default-logging restart: unless-stopped volumes: - /opt/project-nomad/redis:/data # Persist Redis data on the host. This path can be changed if needed, just make sure it's writable by the container. Host persistence is important for Redis to ensure your data isn't lost when the container is removed or updated. @@ -115,6 +128,7 @@ services: image: ghcr.io/crosstalk-solutions/project-nomad-sidecar-updater:latest pull_policy: always container_name: nomad_updater + logging: *default-logging restart: unless-stopped volumes: - /var/run/docker.sock:/var/run/docker.sock # Allows communication with the Host's Docker daemon @@ -126,6 +140,7 @@ services: image: ghcr.io/crosstalk-solutions/project-nomad-disk-collector:latest pull_policy: always container_name: nomad_disk_collector + logging: *default-logging restart: unless-stopped volumes: - /:/host:ro,rslave # Read-only view of host FS with rslave propagation so /sys and /proc submounts are visible