From 43aa389ea7d66169c2768f7787eb68c6b999b747 Mon Sep 17 00:00:00 2001 From: Tomi Turtiainen <10324676+tomi@users.noreply.github.com> Date: Tue, 19 Nov 2024 23:16:57 +0200 Subject: [PATCH] refactor(core): Bundle the go based launcher to the n8n docker image (no-changelog) (#11792) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Iván Ovejero <ivov.src@gmail.com> --- docker/images/n8n-custom/Dockerfile | 27 ++++------ docker/images/n8n/Dockerfile | 27 ++++------ docker/images/n8n/n8n-task-runners.json | 6 +-- .../@n8n/config/src/configs/runners.config.ts | 15 ++---- packages/@n8n/config/test/config.test.ts | 4 +- .../__tests__/task-runner-process.test.ts | 4 +- .../cli/src/runners/task-runner-module.ts | 2 +- .../cli/src/runners/task-runner-process.ts | 48 ++--------------- .../task-runner-module.internal.test.ts | 4 +- .../runners/task-runner-process.test.ts | 54 +------------------ 10 files changed, 38 insertions(+), 153 deletions(-) diff --git a/docker/images/n8n-custom/Dockerfile b/docker/images/n8n-custom/Dockerfile index 78eedaa2c3..797e78b3c6 100644 --- a/docker/images/n8n-custom/Dockerfile +++ b/docker/images/n8n-custom/Dockerfile @@ -33,27 +33,22 @@ COPY docker/images/n8n/docker-entrypoint.sh / # Setup the Task Runner Launcher ARG TARGETPLATFORM -ARG LAUNCHER_VERSION=0.1.1 -ENV N8N_RUNNERS_MODE=internal_launcher \ - N8N_RUNNERS_LAUNCHER_PATH=/usr/local/bin/task-runner-launcher +ARG LAUNCHER_VERSION=0.3.0-rc COPY docker/images/n8n/n8n-task-runners.json /etc/n8n-task-runners.json -# First, download, verify, then extract the launcher binary -# Second, chmod with 4555 to allow the use of setuid -# Third, create a new user and group to execute the Task Runners under +# Download, verify, then extract the launcher binary RUN \ - if [[ "$TARGETPLATFORM" = "linux/amd64" ]]; then export ARCH_NAME="x86_64"; \ - elif [[ "$TARGETPLATFORM" = "linux/arm64" ]]; then export ARCH_NAME="aarch64"; fi; \ + if [[ "$TARGETPLATFORM" = "linux/amd64" ]]; then export ARCH_NAME="amd64"; \ + elif [[ "$TARGETPLATFORM" = "linux/arm64" ]]; then export ARCH_NAME="arm64"; fi; \ mkdir /launcher-temp && \ cd /launcher-temp && \ - wget https://github.com/n8n-io/task-runner-launcher/releases/download/${LAUNCHER_VERSION}/task-runner-launcher-$ARCH_NAME-unknown-linux-musl.zip && \ - wget https://github.com/n8n-io/task-runner-launcher/releases/download/${LAUNCHER_VERSION}/task-runner-launcher-$ARCH_NAME-unknown-linux-musl.sha256 && \ - sha256sum -c task-runner-launcher-$ARCH_NAME-unknown-linux-musl.sha256 && \ - unzip -d $(dirname ${N8N_RUNNERS_LAUNCHER_PATH}) task-runner-launcher-$ARCH_NAME-unknown-linux-musl.zip task-runner-launcher && \ + wget https://github.com/n8n-io/task-runner-launcher/releases/download/${LAUNCHER_VERSION}/task-runner-launcher-${LAUNCHER_VERSION}-linux-${ARCH_NAME}.tar.gz && \ + wget https://github.com/n8n-io/task-runner-launcher/releases/download/${LAUNCHER_VERSION}/task-runner-launcher-${LAUNCHER_VERSION}-linux-${ARCH_NAME}.tar.gz.sha256 && \ + # The .sha256 does not contain the filename --> Form the correct checksum file + echo "$(cat task-runner-launcher-${LAUNCHER_VERSION}-linux-${ARCH_NAME}.tar.gz.sha256) task-runner-launcher-${LAUNCHER_VERSION}-linux-${ARCH_NAME}.tar.gz" > checksum.sha256 && \ + sha256sum -c checksum.sha256 && \ + tar xvf task-runner-launcher-${LAUNCHER_VERSION}-linux-${ARCH_NAME}.tar.gz --directory=/usr/local/bin && \ cd - && \ - rm -r /launcher-temp && \ - chmod 4555 ${N8N_RUNNERS_LAUNCHER_PATH} && \ - addgroup -g 2000 task-runner && \ - adduser -D -u 2000 -g "Task Runner User" -G task-runner task-runner + rm -r /launcher-temp RUN \ cd /usr/local/lib/node_modules/n8n && \ diff --git a/docker/images/n8n/Dockerfile b/docker/images/n8n/Dockerfile index 8a94d0c9ec..8acfc411cf 100644 --- a/docker/images/n8n/Dockerfile +++ b/docker/images/n8n/Dockerfile @@ -24,27 +24,22 @@ RUN set -eux; \ # Setup the Task Runner Launcher ARG TARGETPLATFORM -ARG LAUNCHER_VERSION=0.1.1 -ENV N8N_RUNNERS_MODE=internal_launcher \ - N8N_RUNNERS_LAUNCHER_PATH=/usr/local/bin/task-runner-launcher +ARG LAUNCHER_VERSION=0.3.0-rc COPY n8n-task-runners.json /etc/n8n-task-runners.json -# First, download, verify, then extract the launcher binary -# Second, chmod with 4555 to allow the use of setuid -# Third, create a new user and group to execute the Task Runners under +# Download, verify, then extract the launcher binary RUN \ - if [[ "$TARGETPLATFORM" = "linux/amd64" ]]; then export ARCH_NAME="x86_64"; \ - elif [[ "$TARGETPLATFORM" = "linux/arm64" ]]; then export ARCH_NAME="aarch64"; fi; \ + if [[ "$TARGETPLATFORM" = "linux/amd64" ]]; then export ARCH_NAME="amd64"; \ + elif [[ "$TARGETPLATFORM" = "linux/arm64" ]]; then export ARCH_NAME="arm64"; fi; \ mkdir /launcher-temp && \ cd /launcher-temp && \ - wget https://github.com/n8n-io/task-runner-launcher/releases/download/${LAUNCHER_VERSION}/task-runner-launcher-$ARCH_NAME-unknown-linux-musl.zip && \ - wget https://github.com/n8n-io/task-runner-launcher/releases/download/${LAUNCHER_VERSION}/task-runner-launcher-$ARCH_NAME-unknown-linux-musl.sha256 && \ - sha256sum -c task-runner-launcher-$ARCH_NAME-unknown-linux-musl.sha256 && \ - unzip -d $(dirname ${N8N_RUNNERS_LAUNCHER_PATH}) task-runner-launcher-$ARCH_NAME-unknown-linux-musl.zip task-runner-launcher && \ + wget https://github.com/n8n-io/task-runner-launcher/releases/download/${LAUNCHER_VERSION}/task-runner-launcher-${LAUNCHER_VERSION}-linux-${ARCH_NAME}.tar.gz && \ + wget https://github.com/n8n-io/task-runner-launcher/releases/download/${LAUNCHER_VERSION}/task-runner-launcher-${LAUNCHER_VERSION}-linux-${ARCH_NAME}.tar.gz.sha256 && \ + # The .sha256 does not contain the filename --> Form the correct checksum file + echo "$(cat task-runner-launcher-${LAUNCHER_VERSION}-linux-${ARCH_NAME}.tar.gz.sha256) task-runner-launcher-${LAUNCHER_VERSION}-linux-${ARCH_NAME}.tar.gz" > checksum.sha256 && \ + sha256sum -c checksum.sha256 && \ + tar xvf task-runner-launcher-${LAUNCHER_VERSION}-linux-${ARCH_NAME}.tar.gz --directory=/usr/local/bin && \ cd - && \ - rm -r /launcher-temp && \ - chmod 4555 ${N8N_RUNNERS_LAUNCHER_PATH} && \ - addgroup -g 2000 task-runner && \ - adduser -D -u 2000 -g "Task Runner User" -G task-runner task-runner + rm -r /launcher-temp COPY docker-entrypoint.sh / diff --git a/docker/images/n8n/n8n-task-runners.json b/docker/images/n8n/n8n-task-runners.json index 9eab58d91b..1d4e34b1a9 100644 --- a/docker/images/n8n/n8n-task-runners.json +++ b/docker/images/n8n/n8n-task-runners.json @@ -2,7 +2,7 @@ "task-runners": [ { "runner-type": "javascript", - "workdir": "/home/task-runner", + "workdir": "/home/node", "command": "/usr/local/bin/node", "args": ["/usr/local/lib/node_modules/n8n/node_modules/@n8n/task-runner/dist/start.js"], "allowed-env": [ @@ -18,9 +18,7 @@ "N8N_VERSION", "ENVIRONMENT", "DEPLOYMENT_NAME" - ], - "uid": 2000, - "gid": 2000 + ] } ] } diff --git a/packages/@n8n/config/src/configs/runners.config.ts b/packages/@n8n/config/src/configs/runners.config.ts index b7d125cf53..406512f832 100644 --- a/packages/@n8n/config/src/configs/runners.config.ts +++ b/packages/@n8n/config/src/configs/runners.config.ts @@ -2,11 +2,10 @@ import { Config, Env } from '../decorators'; /** * Whether to enable task runners and how to run them - * - internal_childprocess: Task runners are run as a child process and launched by n8n - * - internal_launcher: Task runners are run as a child process and launched by n8n using a separate launch program + * - internal: Task runners are run as a child process and launched by n8n * - external: Task runners are run as a separate program not launched by n8n */ -export type TaskRunnerMode = 'internal_childprocess' | 'internal_launcher' | 'external'; +export type TaskRunnerMode = 'internal' | 'external'; @Config export class TaskRunnersConfig { @@ -15,8 +14,9 @@ export class TaskRunnersConfig { // Defaults to true for now @Env('N8N_RUNNERS_MODE') - mode: TaskRunnerMode = 'internal_childprocess'; + mode: TaskRunnerMode = 'internal'; + /** Endpoint which task runners connect to */ @Env('N8N_RUNNERS_PATH') path: string = '/runners'; @@ -35,13 +35,6 @@ export class TaskRunnersConfig { @Env('N8N_RUNNERS_MAX_PAYLOAD') maxPayload: number = 1024 * 1024 * 1024; - @Env('N8N_RUNNERS_LAUNCHER_PATH') - launcherPath: string = ''; - - /** Which task runner to launch from the config */ - @Env('N8N_RUNNERS_LAUNCHER_RUNNER') - launcherRunner: string = 'javascript'; - /** The --max-old-space-size option to use for the runner (in MB). Default means node.js will determine it based on the available memory. */ @Env('N8N_RUNNERS_MAX_OLD_SPACE_SIZE') maxOldSpaceSize: string = ''; diff --git a/packages/@n8n/config/test/config.test.ts b/packages/@n8n/config/test/config.test.ts index c60431e97a..d67085d4dd 100644 --- a/packages/@n8n/config/test/config.test.ts +++ b/packages/@n8n/config/test/config.test.ts @@ -223,14 +223,12 @@ describe('GlobalConfig', () => { }, taskRunners: { enabled: false, - mode: 'internal_childprocess', + mode: 'internal', path: '/runners', authToken: '', listenAddress: '127.0.0.1', maxPayload: 1024 * 1024 * 1024, port: 5679, - launcherPath: '', - launcherRunner: 'javascript', maxOldSpaceSize: '', maxConcurrency: 5, assertDeduplicationOutput: false, diff --git a/packages/cli/src/runners/__tests__/task-runner-process.test.ts b/packages/cli/src/runners/__tests__/task-runner-process.test.ts index 9eeb8d69fc..447a57d3c7 100644 --- a/packages/cli/src/runners/__tests__/task-runner-process.test.ts +++ b/packages/cli/src/runners/__tests__/task-runner-process.test.ts @@ -25,7 +25,7 @@ describe('TaskRunnerProcess', () => { const logger = mockInstance(Logger); const runnerConfig = mockInstance(TaskRunnersConfig); runnerConfig.enabled = true; - runnerConfig.mode = 'internal_childprocess'; + runnerConfig.mode = 'internal'; const authService = mock<TaskRunnerAuthService>(); let taskRunnerProcess = new TaskRunnerProcess(logger, runnerConfig, authService, mock()); @@ -39,7 +39,7 @@ describe('TaskRunnerProcess', () => { expect(() => new TaskRunnerProcess(logger, runnerConfig, authService, mock())).toThrow(); - runnerConfig.mode = 'internal_childprocess'; + runnerConfig.mode = 'internal'; }); it('should register listener for `runner:failed-heartbeat-check` event', () => { diff --git a/packages/cli/src/runners/task-runner-module.ts b/packages/cli/src/runners/task-runner-module.ts index 612f3d4fc1..123dc723e6 100644 --- a/packages/cli/src/runners/task-runner-module.ts +++ b/packages/cli/src/runners/task-runner-module.ts @@ -36,7 +36,7 @@ export class TaskRunnerModule { await this.loadTaskManager(); await this.loadTaskRunnerServer(); - if (mode === 'internal_childprocess' || mode === 'internal_launcher') { + if (mode === 'internal') { await this.startInternalTaskRunner(); } } diff --git a/packages/cli/src/runners/task-runner-process.ts b/packages/cli/src/runners/task-runner-process.ts index 3129fcb524..01e351c0e4 100644 --- a/packages/cli/src/runners/task-runner-process.ts +++ b/packages/cli/src/runners/task-runner-process.ts @@ -42,10 +42,6 @@ export class TaskRunnerProcess extends TypedEmitter<TaskRunnerProcessEventMap> { return this._runPromise; } - private get useLauncher() { - return this.runnerConfig.mode === 'internal_launcher'; - } - private process: ChildProcess | null = null; private _runPromise: Promise<void> | null = null; @@ -99,9 +95,7 @@ export class TaskRunnerProcess extends TypedEmitter<TaskRunnerProcessEventMap> { const grantToken = await this.authService.createGrantToken(); const n8nUri = `127.0.0.1:${this.runnerConfig.port}`; - this.process = this.useLauncher - ? this.startLauncher(grantToken, n8nUri) - : this.startNode(grantToken, n8nUri); + this.process = this.startNode(grantToken, n8nUri); forwardToLogger(this.logger, this.process, '[Task Runner]: '); @@ -116,16 +110,6 @@ export class TaskRunnerProcess extends TypedEmitter<TaskRunnerProcessEventMap> { }); } - startLauncher(grantToken: string, n8nUri: string) { - return spawn(this.runnerConfig.launcherPath, ['launch', this.runnerConfig.launcherRunner], { - env: { - ...this.getProcessEnvVars(grantToken, n8nUri), - // For debug logging if enabled - RUST_LOG: process.env.RUST_LOG, - }, - }); - } - @OnShutdown() async stop() { if (!this.process) return; @@ -133,11 +117,7 @@ export class TaskRunnerProcess extends TypedEmitter<TaskRunnerProcessEventMap> { this.isShuttingDown = true; // TODO: Timeout & force kill - if (this.useLauncher) { - await this.killLauncher(); - } else { - this.killNode(); - } + this.killNode(); await this._runPromise; this.isShuttingDown = false; @@ -147,11 +127,7 @@ export class TaskRunnerProcess extends TypedEmitter<TaskRunnerProcessEventMap> { async forceRestart() { if (!this.process) return; - if (this.useLauncher) { - await this.killLauncher(); // @TODO: Implement SIGKILL in launcher - } else { - this.process.kill('SIGKILL'); - } + this.process.kill('SIGKILL'); await this._runPromise; } @@ -162,24 +138,6 @@ export class TaskRunnerProcess extends TypedEmitter<TaskRunnerProcessEventMap> { this.process.kill(); } - async killLauncher() { - if (!this.process?.pid) { - return; - } - - const killProcess = spawn(this.runnerConfig.launcherPath, [ - 'kill', - this.runnerConfig.launcherRunner, - this.process.pid.toString(), - ]); - - await new Promise<void>((resolve) => { - killProcess.on('exit', () => { - resolve(); - }); - }); - } - private monitorProcess(taskRunnerProcess: ChildProcess) { this._runPromise = new Promise((resolve) => { this.oomDetector = new NodeProcessOomDetector(taskRunnerProcess); diff --git a/packages/cli/test/integration/runners/task-runner-module.internal.test.ts b/packages/cli/test/integration/runners/task-runner-module.internal.test.ts index 444d576e87..cb993f0a87 100644 --- a/packages/cli/test/integration/runners/task-runner-module.internal.test.ts +++ b/packages/cli/test/integration/runners/task-runner-module.internal.test.ts @@ -6,10 +6,10 @@ import { TaskRunnerModule } from '@/runners/task-runner-module'; import { InternalTaskRunnerDisconnectAnalyzer } from '../../../src/runners/internal-task-runner-disconnect-analyzer'; import { TaskRunnerWsServer } from '../../../src/runners/runner-ws-server'; -describe('TaskRunnerModule in internal_childprocess mode', () => { +describe('TaskRunnerModule in internal mode', () => { const runnerConfig = Container.get(TaskRunnersConfig); runnerConfig.port = 0; // Random port - runnerConfig.mode = 'internal_childprocess'; + runnerConfig.mode = 'internal'; const module = Container.get(TaskRunnerModule); afterEach(async () => { diff --git a/packages/cli/test/integration/runners/task-runner-process.test.ts b/packages/cli/test/integration/runners/task-runner-process.test.ts index 8c5289a5c7..df2fe8fc75 100644 --- a/packages/cli/test/integration/runners/task-runner-process.test.ts +++ b/packages/cli/test/integration/runners/task-runner-process.test.ts @@ -11,7 +11,7 @@ describe('TaskRunnerProcess', () => { const authToken = 'token'; const runnerConfig = Container.get(TaskRunnersConfig); runnerConfig.enabled = true; - runnerConfig.mode = 'internal_childprocess'; + runnerConfig.mode = 'internal'; runnerConfig.authToken = authToken; runnerConfig.port = 0; // Use any port const taskRunnerServer = Container.get(TaskRunnerServer); @@ -20,11 +20,6 @@ describe('TaskRunnerProcess', () => { const taskBroker = Container.get(TaskBroker); const taskRunnerService = Container.get(TaskRunnerWsServer); - const startLauncherSpy = jest.spyOn(runnerProcess, 'startLauncher'); - const startNodeSpy = jest.spyOn(runnerProcess, 'startNode'); - const killLauncherSpy = jest.spyOn(runnerProcess, 'killLauncher'); - const killNodeSpy = jest.spyOn(runnerProcess, 'killNode'); - beforeAll(async () => { await taskRunnerServer.start(); // Set the port to the actually used port @@ -37,11 +32,6 @@ describe('TaskRunnerProcess', () => { afterEach(async () => { await runnerProcess.stop(); - - startLauncherSpy.mockClear(); - startNodeSpy.mockClear(); - killLauncherSpy.mockClear(); - killNodeSpy.mockClear(); }); const getNumConnectedRunners = () => taskRunnerService.runnerConnections.size; @@ -100,46 +90,4 @@ describe('TaskRunnerProcess', () => { expect(getNumRegisteredRunners()).toBe(1); expect(runnerProcess.pid).not.toBe(processId); }); - - it('should launch runner directly if not using a launcher', async () => { - runnerConfig.mode = 'internal_childprocess'; - - await runnerProcess.start(); - - expect(startLauncherSpy).toBeCalledTimes(0); - expect(startNodeSpy).toBeCalledTimes(1); - }); - - it('should use a launcher if configured', async () => { - runnerConfig.mode = 'internal_launcher'; - runnerConfig.launcherPath = 'node'; - - await runnerProcess.start(); - - expect(startLauncherSpy).toBeCalledTimes(1); - expect(startNodeSpy).toBeCalledTimes(0); - runnerConfig.mode = 'internal_childprocess'; - }); - - it('should kill the process directly if not using a launcher', async () => { - runnerConfig.mode = 'internal_childprocess'; - - await runnerProcess.start(); - await runnerProcess.stop(); - - expect(killLauncherSpy).toBeCalledTimes(0); - expect(killNodeSpy).toBeCalledTimes(1); - }); - - it('should kill the process using a launcher if configured', async () => { - runnerConfig.mode = 'internal_launcher'; - runnerConfig.launcherPath = 'node'; - - await runnerProcess.start(); - await runnerProcess.stop(); - - expect(killLauncherSpy).toBeCalledTimes(1); - expect(killNodeSpy).toBeCalledTimes(0); - runnerConfig.mode = 'internal_childprocess'; - }); });