From 21e18aced4a1392053407dfab0fca997bdb50bf3 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Wed, 20 Jan 2021 17:18:07 +0100 Subject: [PATCH 01/37] Create inital class structure --- .../src/scaffolder/tasks/Database.ts | 35 +++++++++++++++++++ .../src/scaffolder/tasks/TaskObserver.ts | 15 ++++++++ .../src/scaffolder/tasks/taskWorker.ts | 34 ++++++++++++++++++ .../src/scaffolder/tasks/tasksDispatcher.ts | 29 +++++++++++++++ .../src/scaffolder/tasks/types.ts | 30 ++++++++++++++++ 5 files changed, 143 insertions(+) create mode 100644 plugins/scaffolder-backend/src/scaffolder/tasks/Database.ts create mode 100644 plugins/scaffolder-backend/src/scaffolder/tasks/TaskObserver.ts create mode 100644 plugins/scaffolder-backend/src/scaffolder/tasks/taskWorker.ts create mode 100644 plugins/scaffolder-backend/src/scaffolder/tasks/tasksDispatcher.ts create mode 100644 plugins/scaffolder-backend/src/scaffolder/tasks/types.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/Database.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/Database.ts new file mode 100644 index 0000000000..f446f1e90a --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/Database.ts @@ -0,0 +1,35 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { Task, Status } from './types'; + +export class Database { + private readonly store = new Map(); + + get(taskId: string) { + return this.store.get(taskId); + } + + write(task: Task) { + return task; + } + + writeStatus(taskId: string, status: Status) { + const task = this.store.get(taskId); + if (task) { + this.store.set(taskId, { ...task, status }); + } + } +} diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskObserver.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskObserver.ts new file mode 100644 index 0000000000..863d6e76e1 --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskObserver.ts @@ -0,0 +1,15 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/taskWorker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/taskWorker.ts new file mode 100644 index 0000000000..4b1754523c --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/taskWorker.ts @@ -0,0 +1,34 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Status, ClaimResponse } from './types'; +import { Database } from './Database'; + +export class TaskWorker { + static async fromConfig() {} + + constructor(private readonly database: Database) {} + + claim(): Promise { + return Promise.resolve(undefined); + } + + setStatus(taskId: string, status: Status) { + this.database.writeStatus(taskId, status); + } + + heartbeat(runId: number) {} +} diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/tasksDispatcher.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/tasksDispatcher.ts new file mode 100644 index 0000000000..c3dfaa3c88 --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/tasksDispatcher.ts @@ -0,0 +1,29 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Task } from './types'; +import { Database } from './Database'; + +export class taskDispatcher { + static async fromConfig(config: { database: Database }) {} + + constructor(private readonly db: Database) {} + + dispatch(task: Task): Promise { + this.db.write(task); + return Promise.resolve('uuid'); + } +} diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts new file mode 100644 index 0000000000..000e292e16 --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts @@ -0,0 +1,30 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export type Status = 'OPEN' | 'PROCESSING' | 'FAILED' | 'CANCELLED'; +export type Task = { + taskId: string; + metadata: string; + status: Status; + lastHeartbeat: string; + retryCount: number; + createdAt: string; +}; + +export type ClaimResponse = { + runId: number; + task: Task; +}; From 5e06a9f45b65f26b971ae7101dc2c2f709a4bba5 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Thu, 21 Jan 2021 15:53:01 +0100 Subject: [PATCH 02/37] Refactor into broker --- plugins/scaffolder-backend/package.json | 2 + .../src/scaffolder/tasks/Database.ts | 74 ++++++++++-- .../src/scaffolder/tasks/TaskBroker.test.ts | 63 ++++++++++ .../src/scaffolder/tasks/TaskBroker.ts | 109 ++++++++++++++++++ .../src/scaffolder/tasks/TaskObserver.ts | 15 --- .../src/scaffolder/tasks/taskWorker.ts | 34 ------ .../src/scaffolder/tasks/tasksDispatcher.ts | 29 ----- .../src/scaffolder/tasks/types.ts | 40 +++++-- 8 files changed, 272 insertions(+), 94 deletions(-) create mode 100644 plugins/scaffolder-backend/src/scaffolder/tasks/TaskBroker.test.ts create mode 100644 plugins/scaffolder-backend/src/scaffolder/tasks/TaskBroker.ts delete mode 100644 plugins/scaffolder-backend/src/scaffolder/tasks/TaskObserver.ts delete mode 100644 plugins/scaffolder-backend/src/scaffolder/tasks/taskWorker.ts delete mode 100644 plugins/scaffolder-backend/src/scaffolder/tasks/tasksDispatcher.ts diff --git a/plugins/scaffolder-backend/package.json b/plugins/scaffolder-backend/package.json index 7af9f567e3..97ab723e81 100644 --- a/plugins/scaffolder-backend/package.json +++ b/plugins/scaffolder-backend/package.json @@ -53,7 +53,9 @@ "helmet": "^4.0.0", "isomorphic-git": "^1.8.0", "jsonschema": "^1.2.6", + "luxon": "^1.25.0", "morgan": "^1.10.0", + "p-queue": "^6.3.0", "uuid": "^8.2.0", "winston": "^3.2.1", "yaml": "^1.10.0" diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/Database.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/Database.ts index f446f1e90a..7c3f343b0f 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/Database.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/Database.ts @@ -13,23 +13,79 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { Task, Status } from './types'; +import { DbTaskRow, TaskSpec } from './types'; +import { v4 as uuid } from 'uuid'; +import { DateTime } from 'luxon'; -export class Database { - private readonly store = new Map(); +export interface Database { + get(taskId: string): Promise; + // updateTask(task: Task): Promise; + createTask(task: TaskSpec): Promise; + claimTask(): Promise; + heartBeat(runId: string): Promise; +} - get(taskId: string) { - return this.store.get(taskId); +export class InMemoryDatabase implements Database { + private readonly store = new Map(); + + async heartBeat(runId: string): Promise { + let task: DbTaskRow | undefined; + + for (const t of this.store.values()) { + if (t.runId === runId) { + task = t; + } + } + + if (!task) { + throw new Error('No task with matching runId found'); + } + + this.store.set(task.taskId, { + ...task, + lastHeartbeat: DateTime.local().toString(), + }); } - write(task: Task) { - return task; + async claimTask(): Promise { + for (const t of this.store.values()) { + if (t.status === 'OPEN') { + const task: DbTaskRow = { + ...t, + status: 'PROCESSING', + runId: uuid(), + }; + this.store.set(t.taskId, task); + return task; + } + } + throw new Error('No task found'); } - writeStatus(taskId: string, status: Status) { + async createTask(spec: TaskSpec): Promise { + return { + taskId: uuid(), + spec, + status: 'OPEN', + retryCount: 0, + createdAt: new Date().toISOString(), + }; + } + + async get(taskId: string): Promise { const task = this.store.get(taskId); if (task) { - this.store.set(taskId, { ...task, status }); + return task; } + throw new Error(`could not found task ${taskId}`); } + + // async updateTask(task: Task): Promise { + // if (!task.taskId) { + // throw new Error('Task must contain id'); + // } + + // this.store.set(task.taskId, task); + // return task; + // } } diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskBroker.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskBroker.test.ts new file mode 100644 index 0000000000..4fb7439bf8 --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskBroker.test.ts @@ -0,0 +1,63 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { MemoryTaskBroker, TaskAgent } from './TaskBroker'; + +describe('MemoryTaskBroker', () => { + it('should claim a dispatched work item', async () => { + const broker = new MemoryTaskBroker(); + + await broker.dispatch({}); + await expect(broker.claim()).resolves.toEqual(expect.any(TaskAgent)); + }); + + it('should wait for a dispatched work item', async () => { + const broker = new MemoryTaskBroker(); + + const promise = broker.claim(); + + await expect(Promise.race([promise, 'waiting'])).resolves.toBe('waiting'); + + await broker.dispatch({}); + await expect(promise).resolves.toEqual(expect.any(TaskAgent)); + }); + + it('should dispatch multiple items and claim them in order', async () => { + const broker = new MemoryTaskBroker(); + + await broker.dispatch({ name: 'a' }); + await broker.dispatch({ name: 'b' }); + await broker.dispatch({ name: 'c' }); + + const taskA = await broker.claim(); + const taskB = await broker.claim(); + const taskC = await broker.claim(); + await expect(taskA).toEqual(expect.any(TaskAgent)); + await expect(taskB).toEqual(expect.any(TaskAgent)); + await expect(taskC).toEqual(expect.any(TaskAgent)); + await expect(taskA.spec.name).toBe('a'); + await expect(taskB.spec.name).toBe('b'); + await expect(taskC.spec.name).toBe('c'); + }); + + it('should complete a task', async () => { + const broker = new MemoryTaskBroker(); + + await broker.dispatch({}); + const task = await broker.claim(); + await task.complete('COMPLETED'); + }); +}); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskBroker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskBroker.ts new file mode 100644 index 0000000000..322e669373 --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskBroker.ts @@ -0,0 +1,109 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + CompletedTaskState, + Task, + TaskSpec, + TaskBroker, + Status, +} from './types'; +import { v4 as uuid } from 'uuid'; +import { InMemoryDatabase } from './database'; + +export class TaskAgent implements Task { + private heartbeartInterval?: ReturnType; + + static create(state: TaskState, db: InMemoryDatabase) { + const agent = new TaskAgent(state, db); + agent.start(); + return agent; + } + + // Runs heartbeat internally + private constructor( + private readonly state: TaskState, + private readonly db: InMemoryDatabase, + ) {} + + get spec() { + return this.state.spec; + } + + async emitLog(message: string): Promise { + throw new Error('Method not implemented.'); + } + + async complete(result: CompletedTaskState): Promise { + this.state.status = result === 'FAILED' ? 'COMPLETED' : 'FAILED'; + } + + private start() { + this.heartbeartInterval = setInterval(() => { + const runId = 'iiiid'; + this.db.heartBeat(runId); + }, 4269); + } +} + +interface TaskState { + spec: TaskSpec; + status: Status; + runId: string | undefined; +} + +function defer() { + let resolve = () => {}; + const promise = new Promise(_resolve => { + resolve = _resolve; + }); + return { promise, resolve }; +} + +export class MemoryTaskBroker implements TaskBroker { + private readonly db = new InMemoryDatabase(); + private readonly tasks = new Array(); + private deferredDispatch = defer(); + + async claim(): Promise { + for (;;) { + const pendingTask = await this.db.claimTask(); + if (pendingTask) { + return TaskAgent.create(pendingTask, this.db); + } + + await this.waitForDispatch(); + } + } + + async dispatch(spec: TaskSpec): Promise { + this.tasks.push({ + spec, + status: 'OPEN', + runId: undefined, + }); + this.signalDispatch(); + } + + private waitForDispatch() { + return this.deferredDispatch.promise; + } + + private signalDispatch() { + this.deferredDispatch.resolve(); + this.deferredDispatch = defer(); + } +} diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskObserver.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskObserver.ts deleted file mode 100644 index 863d6e76e1..0000000000 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskObserver.ts +++ /dev/null @@ -1,15 +0,0 @@ -/* - * Copyright 2021 Spotify AB - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/taskWorker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/taskWorker.ts deleted file mode 100644 index 4b1754523c..0000000000 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/taskWorker.ts +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright 2021 Spotify AB - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { Status, ClaimResponse } from './types'; -import { Database } from './Database'; - -export class TaskWorker { - static async fromConfig() {} - - constructor(private readonly database: Database) {} - - claim(): Promise { - return Promise.resolve(undefined); - } - - setStatus(taskId: string, status: Status) { - this.database.writeStatus(taskId, status); - } - - heartbeat(runId: number) {} -} diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/tasksDispatcher.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/tasksDispatcher.ts deleted file mode 100644 index c3dfaa3c88..0000000000 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/tasksDispatcher.ts +++ /dev/null @@ -1,29 +0,0 @@ -/* - * Copyright 2021 Spotify AB - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { Task } from './types'; -import { Database } from './Database'; - -export class taskDispatcher { - static async fromConfig(config: { database: Database }) {} - - constructor(private readonly db: Database) {} - - dispatch(task: Task): Promise { - this.db.write(task); - return Promise.resolve('uuid'); - } -} diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts index 000e292e16..bd8bb0ade5 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts @@ -14,17 +14,43 @@ * limitations under the License. */ -export type Status = 'OPEN' | 'PROCESSING' | 'FAILED' | 'CANCELLED'; -export type Task = { +export type Status = + | 'OPEN' + | 'PROCESSING' + | 'FAILED' + | 'CANCELLED' + | 'COMPLETED'; + +export type CompletedTaskState = 'FAILED' | 'COMPLETED'; + +export type DbTaskRow = { taskId: string; - metadata: string; + spec: TaskSpec; status: Status; - lastHeartbeat: string; + lastHeartbeat?: string; retryCount: number; createdAt: string; + runId?: string; +}; + +export type DbTaskEventRow = { + id: number; + runId?: number; + stageName: string; + createdAt: string; }; -export type ClaimResponse = { - runId: number; - task: Task; +export type TaskSpec = { + metadata: string; }; + +export interface Task { + spec: TaskSpec; + emitLog(message: string): Promise; + complete(result: CompletedTaskState): Promise; +} + +export interface TaskBroker { + claim(): Promise; + dispatch(spec: TaskSpec): Promise; +} From 5958318b322cba9c95464eadd5d617cde838d62b Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Fri, 22 Jan 2021 09:33:30 +0100 Subject: [PATCH 03/37] Implement createTask and setStatus --- .../src/scaffolder/tasks/Database.ts | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/Database.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/Database.ts index 7c3f343b0f..a3ce732586 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/Database.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/Database.ts @@ -13,16 +13,16 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { DbTaskRow, TaskSpec } from './types'; +import { DbTaskRow, Status, TaskSpec } from './types'; import { v4 as uuid } from 'uuid'; import { DateTime } from 'luxon'; export interface Database { get(taskId: string): Promise; - // updateTask(task: Task): Promise; createTask(task: TaskSpec): Promise; claimTask(): Promise; heartBeat(runId: string): Promise; + setStatus(taskId: string, status: Status): Promise; } export class InMemoryDatabase implements Database { @@ -63,13 +63,15 @@ export class InMemoryDatabase implements Database { } async createTask(spec: TaskSpec): Promise { - return { + const taskRow = { taskId: uuid(), spec, - status: 'OPEN', + status: 'OPEN' as Status, retryCount: 0, createdAt: new Date().toISOString(), }; + this.store.set(taskRow.taskId, taskRow); + return taskRow; } async get(taskId: string): Promise { @@ -80,12 +82,11 @@ export class InMemoryDatabase implements Database { throw new Error(`could not found task ${taskId}`); } - // async updateTask(task: Task): Promise { - // if (!task.taskId) { - // throw new Error('Task must contain id'); - // } - - // this.store.set(task.taskId, task); - // return task; - // } + async setStatus(taskId: string, status: Status): Promise { + const task = this.store.get(taskId); + if (!task) { + throw new Error(`no task found`); + } + this.store.set(task.taskId, { ...task, status }); + } } From a003fffbbb52a898166a91054a95f4ed51146ec6 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Fri, 22 Jan 2021 09:34:05 +0100 Subject: [PATCH 04/37] Use InMemoryDatabase --- .../src/scaffolder/tasks/TaskBroker.test.ts | 18 +++++---- .../src/scaffolder/tasks/TaskBroker.ts | 40 +++++++++---------- 2 files changed, 30 insertions(+), 28 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskBroker.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskBroker.test.ts index 4fb7439bf8..ef4b60cfd6 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskBroker.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskBroker.test.ts @@ -20,7 +20,9 @@ describe('MemoryTaskBroker', () => { it('should claim a dispatched work item', async () => { const broker = new MemoryTaskBroker(); - await broker.dispatch({}); + await broker.dispatch({ + metadata: '', + }); await expect(broker.claim()).resolves.toEqual(expect.any(TaskAgent)); }); @@ -31,16 +33,16 @@ describe('MemoryTaskBroker', () => { await expect(Promise.race([promise, 'waiting'])).resolves.toBe('waiting'); - await broker.dispatch({}); + await broker.dispatch({ metadata: 'foo' }); await expect(promise).resolves.toEqual(expect.any(TaskAgent)); }); it('should dispatch multiple items and claim them in order', async () => { const broker = new MemoryTaskBroker(); - await broker.dispatch({ name: 'a' }); - await broker.dispatch({ name: 'b' }); - await broker.dispatch({ name: 'c' }); + await broker.dispatch({ metadata: 'a' }); + await broker.dispatch({ metadata: 'b' }); + await broker.dispatch({ metadata: 'c' }); const taskA = await broker.claim(); const taskB = await broker.claim(); @@ -48,9 +50,9 @@ describe('MemoryTaskBroker', () => { await expect(taskA).toEqual(expect.any(TaskAgent)); await expect(taskB).toEqual(expect.any(TaskAgent)); await expect(taskC).toEqual(expect.any(TaskAgent)); - await expect(taskA.spec.name).toBe('a'); - await expect(taskB.spec.name).toBe('b'); - await expect(taskC.spec.name).toBe('c'); + await expect(taskA.spec.metadata).toBe('a'); + await expect(taskB.spec.metadata).toBe('b'); + await expect(taskC.spec.metadata).toBe('c'); }); it('should complete a task', async () => { diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskBroker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskBroker.ts index 322e669373..c6fb56b42a 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskBroker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskBroker.ts @@ -14,14 +14,7 @@ * limitations under the License. */ -import { - CompletedTaskState, - Task, - TaskSpec, - TaskBroker, - Status, -} from './types'; -import { v4 as uuid } from 'uuid'; +import { CompletedTaskState, Task, TaskSpec, TaskBroker } from './types'; import { InMemoryDatabase } from './database'; export class TaskAgent implements Task { @@ -48,20 +41,25 @@ export class TaskAgent implements Task { } async complete(result: CompletedTaskState): Promise { - this.state.status = result === 'FAILED' ? 'COMPLETED' : 'FAILED'; + this.db.setStatus( + this.state.taskId, + result === 'FAILED' ? 'COMPLETED' : 'FAILED', + ); } private start() { this.heartbeartInterval = setInterval(() => { - const runId = 'iiiid'; - this.db.heartBeat(runId); - }, 4269); + if (!this.state.runId) { + throw new Error('no run id provided'); + } + this.db.heartBeat(this.state.runId); + }, 1000); } } interface TaskState { spec: TaskSpec; - status: Status; + taskId: string; runId: string | undefined; } @@ -75,14 +73,20 @@ function defer() { export class MemoryTaskBroker implements TaskBroker { private readonly db = new InMemoryDatabase(); - private readonly tasks = new Array(); private deferredDispatch = defer(); async claim(): Promise { for (;;) { const pendingTask = await this.db.claimTask(); if (pendingTask) { - return TaskAgent.create(pendingTask, this.db); + return TaskAgent.create( + { + runId: pendingTask.runId, + taskId: pendingTask.taskId, + spec: pendingTask.spec, + }, + this.db, + ); } await this.waitForDispatch(); @@ -90,11 +94,7 @@ export class MemoryTaskBroker implements TaskBroker { } async dispatch(spec: TaskSpec): Promise { - this.tasks.push({ - spec, - status: 'OPEN', - runId: undefined, - }); + await this.db.createTask(spec); this.signalDispatch(); } From 971742b6cbdcf5e020cc9f76ebc4f8c66e76ccab Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Fri, 22 Jan 2021 11:21:51 +0100 Subject: [PATCH 05/37] return undefined if no task is found --- plugins/scaffolder-backend/src/scaffolder/tasks/Database.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/Database.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/Database.ts index a3ce732586..6f49a1f577 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/Database.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/Database.ts @@ -20,7 +20,7 @@ import { DateTime } from 'luxon'; export interface Database { get(taskId: string): Promise; createTask(task: TaskSpec): Promise; - claimTask(): Promise; + claimTask(): Promise; heartBeat(runId: string): Promise; setStatus(taskId: string, status: Status): Promise; } @@ -47,7 +47,7 @@ export class InMemoryDatabase implements Database { }); } - async claimTask(): Promise { + async claimTask(): Promise { for (const t of this.store.values()) { if (t.status === 'OPEN') { const task: DbTaskRow = { @@ -59,7 +59,7 @@ export class InMemoryDatabase implements Database { return task; } } - throw new Error('No task found'); + return undefined; } async createTask(spec: TaskSpec): Promise { From fcbfc56be8fcc5417219960a5a64e854151093d4 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Fri, 22 Jan 2021 11:50:09 +0100 Subject: [PATCH 06/37] Add dispatchResult MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Co-authored-by: blam Co-authored-by: Patrik Oldsberg --- .../src/scaffolder/tasks/TaskBroker.test.ts | 24 ++++++++----- .../src/scaffolder/tasks/TaskBroker.ts | 35 ++++++++++++------- .../src/scaffolder/tasks/types.ts | 6 +++- 3 files changed, 42 insertions(+), 23 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskBroker.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskBroker.test.ts index ef4b60cfd6..388e2fdec8 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskBroker.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskBroker.test.ts @@ -14,12 +14,14 @@ * limitations under the License. */ +import { InMemoryDatabase } from './Database'; import { MemoryTaskBroker, TaskAgent } from './TaskBroker'; describe('MemoryTaskBroker', () => { - it('should claim a dispatched work item', async () => { - const broker = new MemoryTaskBroker(); + const storage = new InMemoryDatabase(); + const broker = new MemoryTaskBroker(storage); + it('should claim a dispatched work item', async () => { await broker.dispatch({ metadata: '', }); @@ -27,8 +29,6 @@ describe('MemoryTaskBroker', () => { }); it('should wait for a dispatched work item', async () => { - const broker = new MemoryTaskBroker(); - const promise = broker.claim(); await expect(Promise.race([promise, 'waiting'])).resolves.toBe('waiting'); @@ -38,8 +38,6 @@ describe('MemoryTaskBroker', () => { }); it('should dispatch multiple items and claim them in order', async () => { - const broker = new MemoryTaskBroker(); - await broker.dispatch({ metadata: 'a' }); await broker.dispatch({ metadata: 'b' }); await broker.dispatch({ metadata: 'c' }); @@ -56,10 +54,18 @@ describe('MemoryTaskBroker', () => { }); it('should complete a task', async () => { - const broker = new MemoryTaskBroker(); - - await broker.dispatch({}); + const dispatchResult = await broker.dispatch({ metadata: 'foo' }); const task = await broker.claim(); await task.complete('COMPLETED'); + const taskRow = await storage.get(dispatchResult.taskId); + expect(taskRow.status).toBe('COMPLETED'); + }); + + it('should fail a task', async () => { + const dispatchResult = await broker.dispatch({ metadata: 'foo' }); + const task = await broker.claim(); + await task.complete('FAILED'); + const taskRow = await storage.get(dispatchResult.taskId); + expect(taskRow.status).toBe('FAILED'); }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskBroker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskBroker.ts index c6fb56b42a..6d6fb0a72f 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskBroker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskBroker.ts @@ -14,14 +14,20 @@ * limitations under the License. */ -import { CompletedTaskState, Task, TaskSpec, TaskBroker } from './types'; -import { InMemoryDatabase } from './database'; +import { + CompletedTaskState, + Task, + TaskSpec, + TaskBroker, + DispatchResult, +} from './types'; +import { InMemoryDatabase } from './Database'; export class TaskAgent implements Task { private heartbeartInterval?: ReturnType; - static create(state: TaskState, db: InMemoryDatabase) { - const agent = new TaskAgent(state, db); + static create(state: TaskState, storage: InMemoryDatabase) { + const agent = new TaskAgent(state, storage); agent.start(); return agent; } @@ -29,7 +35,7 @@ export class TaskAgent implements Task { // Runs heartbeat internally private constructor( private readonly state: TaskState, - private readonly db: InMemoryDatabase, + private readonly storage: InMemoryDatabase, ) {} get spec() { @@ -41,9 +47,9 @@ export class TaskAgent implements Task { } async complete(result: CompletedTaskState): Promise { - this.db.setStatus( + this.storage.setStatus( this.state.taskId, - result === 'FAILED' ? 'COMPLETED' : 'FAILED', + result === 'FAILED' ? 'FAILED' : 'COMPLETED', ); } @@ -52,7 +58,7 @@ export class TaskAgent implements Task { if (!this.state.runId) { throw new Error('no run id provided'); } - this.db.heartBeat(this.state.runId); + this.storage.heartBeat(this.state.runId); }, 1000); } } @@ -72,12 +78,12 @@ function defer() { } export class MemoryTaskBroker implements TaskBroker { - private readonly db = new InMemoryDatabase(); + constructor(private readonly storage: InMemoryDatabase) {} private deferredDispatch = defer(); async claim(): Promise { for (;;) { - const pendingTask = await this.db.claimTask(); + const pendingTask = await this.storage.claimTask(); if (pendingTask) { return TaskAgent.create( { @@ -85,7 +91,7 @@ export class MemoryTaskBroker implements TaskBroker { taskId: pendingTask.taskId, spec: pendingTask.spec, }, - this.db, + this.storage, ); } @@ -93,9 +99,12 @@ export class MemoryTaskBroker implements TaskBroker { } } - async dispatch(spec: TaskSpec): Promise { - await this.db.createTask(spec); + async dispatch(spec: TaskSpec): Promise { + const taskRow = await this.storage.createTask(spec); this.signalDispatch(); + return { + taskId: taskRow.taskId, + }; } private waitForDispatch() { diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts index bd8bb0ade5..e8cea06005 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts @@ -44,6 +44,10 @@ export type TaskSpec = { metadata: string; }; +export type DispatchResult = { + taskId: string; +}; + export interface Task { spec: TaskSpec; emitLog(message: string): Promise; @@ -52,5 +56,5 @@ export interface Task { export interface TaskBroker { claim(): Promise; - dispatch(spec: TaskSpec): Promise; + dispatch(spec: TaskSpec): Promise; } From 06da4425e0ecb46920ad95c700864c15f4eda4ba Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Fri, 22 Jan 2021 11:58:59 +0100 Subject: [PATCH 07/37] Prepend broker files with Memory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Co-authored-by: blam Co-authored-by: Patrik Oldsberg --- .../src/scaffolder/tasks/{Database.ts => MemoryDatabase.ts} | 2 +- .../tasks/{TaskBroker.test.ts => MemoryTaskBroker.test.ts} | 6 +++--- .../scaffolder/tasks/{TaskBroker.ts => MemoryTaskBroker.ts} | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) rename plugins/scaffolder-backend/src/scaffolder/tasks/{Database.ts => MemoryDatabase.ts} (97%) rename plugins/scaffolder-backend/src/scaffolder/tasks/{TaskBroker.test.ts => MemoryTaskBroker.test.ts} (93%) rename plugins/scaffolder-backend/src/scaffolder/tasks/{TaskBroker.ts => MemoryTaskBroker.ts} (94%) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/Database.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryDatabase.ts similarity index 97% rename from plugins/scaffolder-backend/src/scaffolder/tasks/Database.ts rename to plugins/scaffolder-backend/src/scaffolder/tasks/MemoryDatabase.ts index 6f49a1f577..65e64bf2a8 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/Database.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryDatabase.ts @@ -25,7 +25,7 @@ export interface Database { setStatus(taskId: string, status: Status): Promise; } -export class InMemoryDatabase implements Database { +export class MemoryDatabase implements Database { private readonly store = new Map(); async heartBeat(runId: string): Promise { diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskBroker.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.test.ts similarity index 93% rename from plugins/scaffolder-backend/src/scaffolder/tasks/TaskBroker.test.ts rename to plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.test.ts index 388e2fdec8..f1493ab3d0 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskBroker.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.test.ts @@ -14,11 +14,11 @@ * limitations under the License. */ -import { InMemoryDatabase } from './Database'; -import { MemoryTaskBroker, TaskAgent } from './TaskBroker'; +import { MemoryDatabase } from './MemoryDatabase'; +import { MemoryTaskBroker, TaskAgent } from './MemoryTaskBroker'; describe('MemoryTaskBroker', () => { - const storage = new InMemoryDatabase(); + const storage = new MemoryDatabase(); const broker = new MemoryTaskBroker(storage); it('should claim a dispatched work item', async () => { diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskBroker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.ts similarity index 94% rename from plugins/scaffolder-backend/src/scaffolder/tasks/TaskBroker.ts rename to plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.ts index 6d6fb0a72f..d29aeb9be6 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskBroker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.ts @@ -21,12 +21,12 @@ import { TaskBroker, DispatchResult, } from './types'; -import { InMemoryDatabase } from './Database'; +import { MemoryDatabase } from './MemoryDatabase'; export class TaskAgent implements Task { private heartbeartInterval?: ReturnType; - static create(state: TaskState, storage: InMemoryDatabase) { + static create(state: TaskState, storage: MemoryDatabase) { const agent = new TaskAgent(state, storage); agent.start(); return agent; @@ -35,7 +35,7 @@ export class TaskAgent implements Task { // Runs heartbeat internally private constructor( private readonly state: TaskState, - private readonly storage: InMemoryDatabase, + private readonly storage: MemoryDatabase, ) {} get spec() { From 4f67e8aa21c9d1455137990a493614731d7d7333 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Fri, 22 Jan 2021 12:51:05 +0100 Subject: [PATCH 08/37] Fix typo --- .../scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.ts index d29aeb9be6..a5d90898f5 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.ts @@ -78,7 +78,7 @@ function defer() { } export class MemoryTaskBroker implements TaskBroker { - constructor(private readonly storage: InMemoryDatabase) {} + constructor(private readonly storage: MemoryDatabase) {} private deferredDispatch = defer(); async claim(): Promise { From 1c1ff59ebac9a8b150da6d47a1e07ac1a8f860d8 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Fri, 22 Jan 2021 16:58:41 +0100 Subject: [PATCH 09/37] Add new apis using task broker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Co-authored-by: blam Co-authored-by: Patrik Oldsberg --- .../scaffolder/tasks/MemoryDatabase.test.ts | 21 ++++ .../src/scaffolder/tasks/MemoryDatabase.ts | 50 +++++++- .../src/scaffolder/tasks/MemoryTaskBroker.ts | 54 ++++++++- .../src/scaffolder/tasks/TaskWorker.ts | 110 ++++++++++++++++++ .../src/scaffolder/tasks/index.ts | 19 +++ .../src/scaffolder/tasks/types.ts | 11 +- .../scaffolder-backend/src/service/router.ts | 73 ++++++++++++ 7 files changed, 325 insertions(+), 13 deletions(-) create mode 100644 plugins/scaffolder-backend/src/scaffolder/tasks/MemoryDatabase.test.ts create mode 100644 plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts create mode 100644 plugins/scaffolder-backend/src/scaffolder/tasks/index.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryDatabase.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryDatabase.test.ts new file mode 100644 index 0000000000..c5522cb38a --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryDatabase.test.ts @@ -0,0 +1,21 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +describe('MemoryDatabase', () => { + it('should be tested', async () => { + expect(1).toBe(2); + }); +}); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryDatabase.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryDatabase.ts index 65e64bf2a8..69fb6a1567 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryDatabase.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryDatabase.ts @@ -13,22 +13,62 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { DbTaskRow, Status, TaskSpec } from './types'; + +import { DbTaskRow, DbTaskEventRow, Status, TaskSpec } from './types'; import { v4 as uuid } from 'uuid'; -import { DateTime } from 'luxon'; export interface Database { get(taskId: string): Promise; createTask(task: TaskSpec): Promise; claimTask(): Promise; - heartBeat(runId: string): Promise; + heartbeat(runId: string): Promise; setStatus(taskId: string, status: Status): Promise; } +type EmitOptions = { + taskId: string; + runId: string; + event: string; +}; + +type ReadOptions = { + taskId: string; + after?: number | undefined; +}; + export class MemoryDatabase implements Database { private readonly store = new Map(); + private readonly events = new Array(); - async heartBeat(runId: string): Promise { + async emit({ taskId, runId, event }: EmitOptions) { + this.events.push({ + id: this.events.length, + taskId, + runId, + event, + createdAt: new Date().toISOString(), + }); + } + + async getEvents({ + taskId, + after, + }: ReadOptions): Promise<{ events: DbTaskEventRow[] }> { + const events = this.events.filter(event => { + if (event.taskId !== taskId) { + return false; + } + if (after !== undefined) { + if (event.id <= after) { + return false; + } + } + return true; + }); + return { events }; + } + + async heartbeat(runId: string): Promise { let task: DbTaskRow | undefined; for (const t of this.store.values()) { @@ -43,7 +83,7 @@ export class MemoryDatabase implements Database { this.store.set(task.taskId, { ...task, - lastHeartbeat: DateTime.local().toString(), + lastHeartbeat: new Date().toISOString(), }); } diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.ts index a5d90898f5..2491c9a091 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.ts @@ -20,6 +20,7 @@ import { TaskSpec, TaskBroker, DispatchResult, + DbTaskEventRow, } from './types'; import { MemoryDatabase } from './MemoryDatabase'; @@ -43,14 +44,22 @@ export class TaskAgent implements Task { } async emitLog(message: string): Promise { - throw new Error('Method not implemented.'); + await this.storage.emit({ + taskId: this.state.taskId, + runId: this.state.runId, + event: message, + }); } async complete(result: CompletedTaskState): Promise { - this.storage.setStatus( + await this.storage.setStatus( this.state.taskId, result === 'FAILED' ? 'FAILED' : 'COMPLETED', ); + + if (this.heartbeartInterval) { + clearInterval(this.heartbeartInterval); + } } private start() { @@ -58,7 +67,7 @@ export class TaskAgent implements Task { if (!this.state.runId) { throw new Error('no run id provided'); } - this.storage.heartBeat(this.state.runId); + this.storage.heartbeat(this.state.runId); }, 1000); } } @@ -66,7 +75,7 @@ export class TaskAgent implements Task { interface TaskState { spec: TaskSpec; taskId: string; - runId: string | undefined; + runId: string; } function defer() { @@ -87,7 +96,7 @@ export class MemoryTaskBroker implements TaskBroker { if (pendingTask) { return TaskAgent.create( { - runId: pendingTask.runId, + runId: pendingTask.runId!, taskId: pendingTask.taskId, spec: pendingTask.spec, }, @@ -107,6 +116,41 @@ export class MemoryTaskBroker implements TaskBroker { }; } + observe( + options: { + taskId: string; + after: number | undefined; + }, + callback: (result: { events: DbTaskEventRow[] }) => void, + ): () => void { + const { taskId } = options; + + let cancelled = false; + const unsubscribe = () => { + cancelled = true; + }; + + (async () => { + let after = options.after; + while (!cancelled) { + const result = await this.storage.getEvents({ taskId, after: after }); + const { events } = result; + if (events.length) { + after = events[events.length - 1].id; + try { + callback(result); + } catch (error) { + console.log('DEBUG: error =', error); + } + } + + await new Promise(resolve => setTimeout(resolve, 1000)); + } + })(); + + return unsubscribe; + } + private waitForDispatch() { return this.deferredDispatch.promise; } diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts new file mode 100644 index 0000000000..dc15b3db65 --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts @@ -0,0 +1,110 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { TaskBroker, Task } from './types'; +import { Logger } from 'winston'; +import Docker from 'dockerode'; +import { CatalogEntityClient } from '../../lib/catalog'; +import { + FilePreparer, + parseLocationAnnotation, + PreparerBuilder, + TemplaterBuilder, + PublisherBuilder, +} from '../stages'; + +type Options = { + logger: Logger; + taskBroker: TaskBroker; + workingDirectory: string; + dockerClient: Docker; + entityClient: CatalogEntityClient; + preparers: PreparerBuilder; + templaters: TemplaterBuilder; + publishers: PublisherBuilder; +}; + +export class TaskWorker { + constructor(private readonly options: Options) {} + + start() { + (async () => { + for (;;) { + const task = await this.options.taskBroker.claim(); + await this.runOneTask(task); + } + })(); + } + + async runOneTask(task: Task) { + const { + dockerClient, + preparers, + templaters, + publishers, + workingDirectory, + logger, + } = this.options; + + try { + task.emitLog('Task claimed, waiting ...'); + // Give us some time to curl observe + await new Promise(resolve => setTimeout(resolve, 5000)); + + const { values, template } = task.spec; + task.emitLog('Prepare the skeleton'); + const { protocol, location: pullPath } = parseLocationAnnotation( + task.spec.template, + ); + + const preparer = + protocol === 'file' ? new FilePreparer() : preparers.get(pullPath); + const templater = templaters.get(template); + const publisher = publishers.get(values.storePath); + + const skeletonDir = await preparer.prepare(task.spec.template, { + logger, + workingDirectory: workingDirectory, + }); + + task.emitLog('Run the templater'); + const { resultDir } = await templater.run({ + directory: skeletonDir, + dockerClient, + logStream: process.stdout, // yay + values: values, + }); + + task.emitLog('Publish template'); + logger.info('Will now store the template'); + + logger.info('Totally storing the template now'); + await new Promise(resolve => setTimeout(resolve, 5000)); + // const result = await publisher.publish({ + // values: values, + // directory: resultDir, + // logger, + // }); + // task.emitLog(`Result: ${JSON.stringify(result)}`); + + task.emitLog(`Completely done now!`); + + await task.complete('COMPLETED'); + } catch (error) { + await task.complete('FAILED'); + } + } +} diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/index.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/index.ts new file mode 100644 index 0000000000..1cc9842fed --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/index.ts @@ -0,0 +1,19 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export { MemoryDatabase } from './MemoryDatabase'; +export { MemoryTaskBroker } from './MemoryTaskBroker'; +export { TaskWorker } from './TaskWorker'; diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts index e8cea06005..12cb9fee1f 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts @@ -14,6 +14,9 @@ * limitations under the License. */ +import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; +import { TemplaterValues } from '..'; + export type Status = | 'OPEN' | 'PROCESSING' @@ -35,13 +38,15 @@ export type DbTaskRow = { export type DbTaskEventRow = { id: number; - runId?: number; - stageName: string; + runId: string; + taskId: string; + event: string; createdAt: string; }; export type TaskSpec = { - metadata: string; + template: TemplateEntityV1alpha1; + values: TemplaterValues; }; export type DispatchResult = { diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 9098e5d05e..706027f62e 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -33,6 +33,11 @@ import { import { CatalogEntityClient } from '../lib/catalog'; import { validate, ValidatorResult } from 'jsonschema'; import parseGitUrl from 'git-url-parse'; +import { + MemoryTaskBroker, + MemoryDatabase, + TaskWorker, +} from '../scaffolder/tasks'; export interface RouterOptions { preparers: PreparerBuilder; @@ -63,6 +68,18 @@ export async function createRouter( const logger = parentLogger.child({ plugin: 'scaffolder' }); const jobProcessor = await JobProcessor.fromConfig({ config, logger }); + const taskBroker = new MemoryTaskBroker(new MemoryDatabase()); + const worker = new TaskWorker({ + logger, + taskBroker, + workingDirectory: 'todo', + dockerClient, + entityClient, + preparers, + publishers, + templaters, + }); + worker.start(); router .get('/v1/job/:jobId', ({ params }, res) => { @@ -88,6 +105,62 @@ export async function createRouter( error: job.error, }); }) + // curl -X POST -d '{"templateName":"springboot-template","values": {"storePath":"https://github.com/jhaals/foo", "component_id":"woop", "description": "apa", "owner": "me" }}' -H 'Content-Type: application/json' localhost:7000/api/scaffolder/v2/tasks + .post('/v2/tasks', async (req, res) => { + const templateName: string = req.body.templateName; + const values: TemplaterValues = { + ...req.body.values, + destination: { + git: parseGitUrl(req.body.values.storePath), + }, + }; + const template = await entityClient.findTemplate(templateName); + + const validationResult: ValidatorResult = validate( + values, + template.spec.schema, + ); + + if (!validationResult.valid) { + res.status(400).json({ errors: validationResult.errors }); + return; + } + const result = await taskBroker.dispatch({ + template, + values, + }); + + res.status(201).json({ id: result.taskId }); + }) + + .get('/v2/tasks/:taskId/eventstream', async (req, res) => { + const { taskId } = req.params; + const after = Number(req.query.after) || undefined; + logger.info('event stream opened'); + + // Mandatory headers and http status to keep connection open + res.writeHead(200, { + Connection: 'keep-alive', + 'Cache-Control': 'no-cache', + 'Content-Type': 'text/event-stream', + }); + // After client opens connection send all nests as string + const unsubscribe = taskBroker.observe( + { taskId, after }, + ({ events }) => { + for (const event of events) { + res.write(`event:${JSON.stringify(event)}\n\n`); + } + }, + ); + // When client closes connection we update the clients list + // avoiding the disconnected one + req.on('close', () => { + unsubscribe(); + logger.info('event stream closed'); + }); + }) + .post('/v1/jobs', async (req, res) => { const templateName: string = req.body.templateName; const values: TemplaterValues = { From dfea89fa98562ef30713eba9db5942d8351f3e70 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Mon, 25 Jan 2021 10:12:15 +0100 Subject: [PATCH 10/37] Pass taskSpec in tests --- .../scaffolder/tasks/MemoryTaskBroker.test.ts | 38 +++++++++++++------ 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.test.ts index f1493ab3d0..aa23044749 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.test.ts @@ -14,6 +14,8 @@ * limitations under the License. */ +import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; +import { TemplaterValues } from '../stages/templater/types'; import { MemoryDatabase } from './MemoryDatabase'; import { MemoryTaskBroker, TaskAgent } from './MemoryTaskBroker'; @@ -21,10 +23,13 @@ describe('MemoryTaskBroker', () => { const storage = new MemoryDatabase(); const broker = new MemoryTaskBroker(storage); + const taskSpec = { + values: {} as TemplaterValues, + template: {} as TemplateEntityV1alpha1, + }; + it('should claim a dispatched work item', async () => { - await broker.dispatch({ - metadata: '', - }); + await broker.dispatch(taskSpec); await expect(broker.claim()).resolves.toEqual(expect.any(TaskAgent)); }); @@ -33,14 +38,23 @@ describe('MemoryTaskBroker', () => { await expect(Promise.race([promise, 'waiting'])).resolves.toBe('waiting'); - await broker.dispatch({ metadata: 'foo' }); + await broker.dispatch(taskSpec); await expect(promise).resolves.toEqual(expect.any(TaskAgent)); }); it('should dispatch multiple items and claim them in order', async () => { - await broker.dispatch({ metadata: 'a' }); - await broker.dispatch({ metadata: 'b' }); - await broker.dispatch({ metadata: 'c' }); + await broker.dispatch({ + values: { owner: 'a' } as TemplaterValues, + template: {} as TemplateEntityV1alpha1, + }); + await broker.dispatch({ + values: { owner: 'b' } as TemplaterValues, + template: {} as TemplateEntityV1alpha1, + }); + await broker.dispatch({ + values: { owner: 'c' } as TemplaterValues, + template: {} as TemplateEntityV1alpha1, + }); const taskA = await broker.claim(); const taskB = await broker.claim(); @@ -48,13 +62,13 @@ describe('MemoryTaskBroker', () => { await expect(taskA).toEqual(expect.any(TaskAgent)); await expect(taskB).toEqual(expect.any(TaskAgent)); await expect(taskC).toEqual(expect.any(TaskAgent)); - await expect(taskA.spec.metadata).toBe('a'); - await expect(taskB.spec.metadata).toBe('b'); - await expect(taskC.spec.metadata).toBe('c'); + await expect(taskA.spec.values.owner).toBe('a'); + await expect(taskB.spec.values.owner).toBe('b'); + await expect(taskC.spec.values.owner).toBe('c'); }); it('should complete a task', async () => { - const dispatchResult = await broker.dispatch({ metadata: 'foo' }); + const dispatchResult = await broker.dispatch(taskSpec); const task = await broker.claim(); await task.complete('COMPLETED'); const taskRow = await storage.get(dispatchResult.taskId); @@ -62,7 +76,7 @@ describe('MemoryTaskBroker', () => { }); it('should fail a task', async () => { - const dispatchResult = await broker.dispatch({ metadata: 'foo' }); + const dispatchResult = await broker.dispatch(taskSpec); const task = await broker.claim(); await task.complete('FAILED'); const taskRow = await storage.get(dispatchResult.taskId); From 7d13f22c3db614d9be40b6652e42fdb07bb5185f Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Mon, 25 Jan 2021 13:16:28 +0100 Subject: [PATCH 11/37] Add eventType for closing observer stream Co-authored-by: Patrik Oldsberg --- .../src/scaffolder/tasks/MemoryDatabase.ts | 22 +++++++++++++------ .../scaffolder/tasks/MemoryTaskBroker.test.ts | 8 +++---- .../src/scaffolder/tasks/MemoryTaskBroker.ts | 12 +++++++--- .../src/scaffolder/tasks/TaskWorker.ts | 4 ++-- .../src/scaffolder/tasks/types.ts | 16 ++++++++------ .../scaffolder-backend/src/service/router.ts | 4 ++++ 6 files changed, 43 insertions(+), 23 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryDatabase.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryDatabase.ts index 69fb6a1567..b2d1cf667b 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryDatabase.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryDatabase.ts @@ -14,7 +14,13 @@ * limitations under the License. */ -import { DbTaskRow, DbTaskEventRow, Status, TaskSpec } from './types'; +import { + DbTaskRow, + DbTaskEventRow, + Status, + TaskSpec, + TaskEventType, +} from './types'; import { v4 as uuid } from 'uuid'; export interface Database { @@ -28,7 +34,8 @@ export interface Database { type EmitOptions = { taskId: string; runId: string; - event: string; + body: string; + type: TaskEventType; }; type ReadOptions = { @@ -40,12 +47,13 @@ export class MemoryDatabase implements Database { private readonly store = new Map(); private readonly events = new Array(); - async emit({ taskId, runId, event }: EmitOptions) { + async emit({ taskId, runId, body, type }: EmitOptions) { this.events.push({ id: this.events.length, taskId, runId, - event, + body, + type, createdAt: new Date().toISOString(), }); } @@ -89,10 +97,10 @@ export class MemoryDatabase implements Database { async claimTask(): Promise { for (const t of this.store.values()) { - if (t.status === 'OPEN') { + if (t.status === 'open') { const task: DbTaskRow = { ...t, - status: 'PROCESSING', + status: 'processing', runId: uuid(), }; this.store.set(t.taskId, task); @@ -106,7 +114,7 @@ export class MemoryDatabase implements Database { const taskRow = { taskId: uuid(), spec, - status: 'OPEN' as Status, + status: 'open' as Status, retryCount: 0, createdAt: new Date().toISOString(), }; diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.test.ts index aa23044749..1ac416ad4f 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.test.ts @@ -70,16 +70,16 @@ describe('MemoryTaskBroker', () => { it('should complete a task', async () => { const dispatchResult = await broker.dispatch(taskSpec); const task = await broker.claim(); - await task.complete('COMPLETED'); + await task.complete('completed'); const taskRow = await storage.get(dispatchResult.taskId); - expect(taskRow.status).toBe('COMPLETED'); + expect(taskRow.status).toBe('completed'); }); it('should fail a task', async () => { const dispatchResult = await broker.dispatch(taskSpec); const task = await broker.claim(); - await task.complete('FAILED'); + await task.complete('failed'); const taskRow = await storage.get(dispatchResult.taskId); - expect(taskRow.status).toBe('FAILED'); + expect(taskRow.status).toBe('failed'); }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.ts index 2491c9a091..c22bb0bddb 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.ts @@ -47,16 +47,22 @@ export class TaskAgent implements Task { await this.storage.emit({ taskId: this.state.taskId, runId: this.state.runId, - event: message, + body: message, + type: 'log', }); } async complete(result: CompletedTaskState): Promise { await this.storage.setStatus( this.state.taskId, - result === 'FAILED' ? 'FAILED' : 'COMPLETED', + result === 'failed' ? 'failed' : 'completed', ); - + this.storage.emit({ + taskId: this.state.taskId, + runId: this.state.runId, + body: `Run completed with status: ${result}`, + type: 'completion', + }); if (this.heartbeartInterval) { clearInterval(this.heartbeartInterval); } diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts index dc15b3db65..641a8cd9b1 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts @@ -102,9 +102,9 @@ export class TaskWorker { task.emitLog(`Completely done now!`); - await task.complete('COMPLETED'); + await task.complete('completed'); } catch (error) { - await task.complete('FAILED'); + await task.complete('failed'); } } } diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts index 12cb9fee1f..47226dc6e9 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts @@ -18,13 +18,13 @@ import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; import { TemplaterValues } from '..'; export type Status = - | 'OPEN' - | 'PROCESSING' - | 'FAILED' - | 'CANCELLED' - | 'COMPLETED'; + | 'open' + | 'processing' + | 'failed' + | 'cancelled' + | 'completed'; -export type CompletedTaskState = 'FAILED' | 'COMPLETED'; +export type CompletedTaskState = 'failed' | 'completed'; export type DbTaskRow = { taskId: string; @@ -36,11 +36,13 @@ export type DbTaskRow = { runId?: string; }; +export type TaskEventType = 'completion' | 'log'; export type DbTaskEventRow = { id: number; runId: string; taskId: string; - event: string; + body: string; + type: TaskEventType; createdAt: string; }; diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 706027f62e..9514d215d0 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -150,6 +150,10 @@ export async function createRouter( ({ events }) => { for (const event of events) { res.write(`event:${JSON.stringify(event)}\n\n`); + if (event.type === 'completion') { + unsubscribe(); + res.end(); + } } }, ); From fef53431df1c3745bfb210eb6b8cf8a637b75213 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Mon, 25 Jan 2021 14:02:51 +0100 Subject: [PATCH 12/37] create logger and stream that emit events --- .../src/scaffolder/tasks/TaskWorker.ts | 28 +++++++++++++++---- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts index 641a8cd9b1..d500fc8e5c 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts @@ -14,9 +14,11 @@ * limitations under the License. */ -import { TaskBroker, Task } from './types'; +import { PassThrough } from 'stream'; import { Logger } from 'winston'; +import * as winston from 'winston'; import Docker from 'dockerode'; +import { TaskBroker, Task } from './types'; import { CatalogEntityClient } from '../../lib/catalog'; import { FilePreparer, @@ -59,6 +61,24 @@ export class TaskWorker { logger, } = this.options; + const taskLogger = winston.createLogger({ + level: process.env.LOG_LEVEL || 'info', + format: winston.format.combine( + winston.format.colorize(), + winston.format.timestamp(), + winston.format.simple(), + ), + defaultMeta: {}, + }); + + const stream = new PassThrough(); + stream.on('data', data => { + const message = data.toString().trim(); + if (message?.length > 1) task.emitLog(message); + }); + + taskLogger.add(new winston.transports.Stream({ stream })); + try { task.emitLog('Task claimed, waiting ...'); // Give us some time to curl observe @@ -76,7 +96,7 @@ export class TaskWorker { const publisher = publishers.get(values.storePath); const skeletonDir = await preparer.prepare(task.spec.template, { - logger, + logger: taskLogger, workingDirectory: workingDirectory, }); @@ -84,7 +104,7 @@ export class TaskWorker { const { resultDir } = await templater.run({ directory: skeletonDir, dockerClient, - logStream: process.stdout, // yay + logStream: stream, values: values, }); @@ -100,8 +120,6 @@ export class TaskWorker { // }); // task.emitLog(`Result: ${JSON.stringify(result)}`); - task.emitLog(`Completely done now!`); - await task.complete('completed'); } catch (error) { await task.complete('failed'); From 8623025c9b789b85bdf87b9cdf9cc94995f55859 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Tue, 26 Jan 2021 12:00:40 +0100 Subject: [PATCH 13/37] Wip wip --- .../src/scaffolder/stages/legacy.ts | 106 +++++++++++++++++ .../scaffolder/tasks/MemoryTaskBroker.test.ts | 2 +- .../src/scaffolder/tasks/TemplateConverter.ts | 110 ++++++++++++++++++ .../src/scaffolder/tasks/types.ts | 11 +- .../scaffolder-backend/src/service/router.ts | 2 + 5 files changed, 226 insertions(+), 5 deletions(-) create mode 100644 plugins/scaffolder-backend/src/scaffolder/stages/legacy.ts create mode 100644 plugins/scaffolder-backend/src/scaffolder/tasks/TemplateConverter.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/legacy.ts b/plugins/scaffolder-backend/src/scaffolder/stages/legacy.ts new file mode 100644 index 0000000000..c9bd89c3dd --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/stages/legacy.ts @@ -0,0 +1,106 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Config } from '@backstage/config'; +import { TemplateActionRegistry } from '../TemplateConverter'; +import { FilePreparer } from './prepare'; +import Docker from 'dockerode'; + +type Options = { + logger: Logger; + config: Config; + dockerClient: Docker; +}; + +export function registerLegacyActions( + registry: TemplateActionRegistry, + options: Options, +) { + registry.register({ + id: 'legacy:prepare', + async handler(ctx) { + const { logger } = ctx; + console.log(ctx); + logger.info('Task claimed, waiting ...'); + // Give us some time to curl observe + await new Promise(resolve => setTimeout(resolve, 5000)); + + logger.info('Prepare the skeleton'); + + const { protocol, pullPath } = ctx.parameters; + + const preparer = + protocol === 'file' + ? new FilePreparer() + : preparers.get(pullPath as string); + + await preparer.prepare(task.spec.template, { + logger, + ctx.workspaceDir, + }); + ctx.output('catalogInfoUrl', 'httpderp://asdasd'); + }, + }); + + // try { + // const { values, template } = task.spec; + // task.emitLog('Prepare the skeleton'); + // const { protocol, location: pullPath } = parseLocationAnnotation( + // task.spec.template, + // ); + // const preparer = + // protocol === 'file' ? new FilePreparer() : preparers.get(pullPath); + // const templater = templaters.get(template); + // const publisher = publishers.get(values.storePath); + + // const skeletonDir = await preparer.prepare(task.spec.template, { + // logger: taskLogger, + // workingDirectory: workingDirectory, + // }); + + registry.register({ + id: 'legacy:template', + async handler(ctx) { + const { logger } = ctx; + + const templater = templaters.get(ctx.parameters.templater as string); + + logger.info('Run the templater'); + const { resultDir } = await templater.run({ + directory: ctx.workspaceDir, + dockerClient, + logStream: ctx.logStream, + values: ctx.parameters.values as TemplaterValues, + }); + }, + }); + // task.emitLog('Publish template'); + // logger.info('Will now store the template'); + + // logger.info('Totally storing the template now'); + // await new Promise(resolve => setTimeout(resolve, 5000)); + // // const result = await publisher.publish({ + // // values: values, + // // directory: resultDir, + // // logger, + // // }); + // // task.emitLog(`Result: ${JSON.stringify(result)}`); + + // await task.complete('completed'); + // } catch (error) { + // await task.complete('failed'); + // } +} diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.test.ts index 1ac416ad4f..686a6d9132 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.test.ts @@ -15,7 +15,7 @@ */ import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; -import { TemplaterValues } from '../stages/templater/types'; +import { TemplaterValues } from './actions/templater/types'; import { MemoryDatabase } from './MemoryDatabase'; import { MemoryTaskBroker, TaskAgent } from './MemoryTaskBroker'; diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TemplateConverter.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TemplateConverter.ts new file mode 100644 index 0000000000..e435812fe5 --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TemplateConverter.ts @@ -0,0 +1,110 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { JsonValue } from '@backstage/config'; +import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; +import { Logger } from 'winston'; +import type { Writable } from 'stream'; +import { + getTemplaterKey, + parseLocationAnnotation, + TemplaterValues, +} from '../jobs/actions'; +import { TaskSpec } from './types'; +import { ConflictError, NotFoundError } from '@backstage/backend-common'; + +function templateEntityToSpec( + template: TemplateEntityV1alpha1, + values: TemplaterValues, +): TaskSpec { + const steps: TaskSpec['steps'] = []; + + const { protocol, location: pullPath } = parseLocationAnnotation(template); + const templater = getTemplaterKey(template); + + steps.push({ + id: 'prepare', + name: 'Prepare', + action: 'legacy:prepare', + parameters: { + protocol, + pullPath, + }, + }); + + steps.push({ + id: 'template', + name: 'Template', + action: 'legacy:template', + parameters: { + templater, + values, + }, + }); + + steps.push({ + id: 'publish', + name: 'Publishing', + action: 'publish', + parameters: { + values, + directory, + }, + }); + + return { steps }; +} + +type ActionContext = { + logger: Logger; + logStream: Writable; + + workspaceDir: string; + parameters: { [name: string]: JsonValue }; + output(name: string, value: JsonValue): void; +}; + +type TemplateAction = { + id: string; + handler: (ctx: ActionContext) => Promise; +}; + +export class TemplateActionRegistry { + private readonly actions = new Map(); + + register(action: TemplateAction) { + if (this.actions.has(action.id)) { + throw new ConflictError( + `Template action with id ${action.id} as already been registered`, + ); + } + this.actions.set(action.id, action); + } + + // validate + // ensure that action exist. + // template variables exist. + + get(actionId: string): TemplateAction { + const action = this.actions.get(actionId); + if (!action) { + throw new NotFoundError( + `Template action with id ${actionId} is not registered.`, + ); + } + return action; + } +} diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts index 47226dc6e9..620b6e7243 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts @@ -14,8 +14,7 @@ * limitations under the License. */ -import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; -import { TemplaterValues } from '..'; +import { JsonObject } from '@backstage/config'; export type Status = | 'open' @@ -47,8 +46,12 @@ export type DbTaskEventRow = { }; export type TaskSpec = { - template: TemplateEntityV1alpha1; - values: TemplaterValues; + steps: Array<{ + id: string; + name: string; + action: string; + parameters?: JsonObject; + }>; }; export type DispatchResult = { diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 9514d215d0..fc40d45ce2 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -38,6 +38,8 @@ import { MemoryDatabase, TaskWorker, } from '../scaffolder/tasks'; +import { TemplateActionRegistry } from '../scaffolder/tasks/TemplateConverter'; +import { LOCATION_ANNOTATION } from '@backstage/catalog-model'; export interface RouterOptions { preparers: PreparerBuilder; From cf896972ef873b4a94f669d10b9a8f5a3e186e00 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Fri, 29 Jan 2021 13:16:50 +0100 Subject: [PATCH 14/37] Wire up task registry with legacy tasks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Co-authored-by: blam Co-authored-by: Patrik Oldsberg --- .../src/scaffolder/stages/legacy.ts | 105 ++++++------ .../src/scaffolder/tasks/MemoryTaskBroker.ts | 8 + .../src/scaffolder/tasks/TaskWorker.ts | 155 ++++++++++-------- .../src/scaffolder/tasks/TemplateConverter.ts | 31 ++-- .../src/scaffolder/tasks/types.ts | 6 +- .../scaffolder-backend/src/service/helpers.ts | 44 +++++ .../scaffolder-backend/src/service/router.ts | 23 ++- 7 files changed, 242 insertions(+), 130 deletions(-) create mode 100644 plugins/scaffolder-backend/src/service/helpers.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/legacy.ts b/plugins/scaffolder-backend/src/scaffolder/stages/legacy.ts index c9bd89c3dd..6244e40d68 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/legacy.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/legacy.ts @@ -14,21 +14,25 @@ * limitations under the License. */ -import { Config } from '@backstage/config'; -import { TemplateActionRegistry } from '../TemplateConverter'; -import { FilePreparer } from './prepare'; +import { TemplateActionRegistry } from '../tasks/TemplateConverter'; +import { FilePreparer, PreparerBuilder } from './prepare'; import Docker from 'dockerode'; +import { TemplaterBuilder, TemplaterValues } from './templater'; +import { PublisherBuilder } from './publish'; type Options = { - logger: Logger; - config: Config; dockerClient: Docker; + preparers: PreparerBuilder; + templaters: TemplaterBuilder; + publishers: PublisherBuilder; }; export function registerLegacyActions( registry: TemplateActionRegistry, options: Options, ) { + const { dockerClient, preparers, templaters, publishers } = options; + registry.register({ id: 'legacy:prepare', async handler(ctx) { @@ -41,36 +45,18 @@ export function registerLegacyActions( logger.info('Prepare the skeleton'); const { protocol, pullPath } = ctx.parameters; - + const url = pullPath as string; const preparer = - protocol === 'file' - ? new FilePreparer() - : preparers.get(pullPath as string); + protocol === 'file' ? new FilePreparer() : preparers.get(url); - await preparer.prepare(task.spec.template, { - logger, - ctx.workspaceDir, + await preparer.prepare({ + url, + logger: ctx.logger, + workspacePath: ctx.workspacePath, }); - ctx.output('catalogInfoUrl', 'httpderp://asdasd'); }, }); - // try { - // const { values, template } = task.spec; - // task.emitLog('Prepare the skeleton'); - // const { protocol, location: pullPath } = parseLocationAnnotation( - // task.spec.template, - // ); - // const preparer = - // protocol === 'file' ? new FilePreparer() : preparers.get(pullPath); - // const templater = templaters.get(template); - // const publisher = publishers.get(values.storePath); - - // const skeletonDir = await preparer.prepare(task.spec.template, { - // logger: taskLogger, - // workingDirectory: workingDirectory, - // }); - registry.register({ id: 'legacy:template', async handler(ctx) { @@ -79,28 +65,55 @@ export function registerLegacyActions( const templater = templaters.get(ctx.parameters.templater as string); logger.info('Run the templater'); - const { resultDir } = await templater.run({ - directory: ctx.workspaceDir, + await templater.run({ + workspacePath: ctx.workspacePath, dockerClient, logStream: ctx.logStream, values: ctx.parameters.values as TemplaterValues, }); }, }); - // task.emitLog('Publish template'); - // logger.info('Will now store the template'); - // logger.info('Totally storing the template now'); - // await new Promise(resolve => setTimeout(resolve, 5000)); - // // const result = await publisher.publish({ - // // values: values, - // // directory: resultDir, - // // logger, - // // }); - // // task.emitLog(`Result: ${JSON.stringify(result)}`); - - // await task.complete('completed'); - // } catch (error) { - // await task.complete('failed'); - // } + registry.register({ + id: 'legacy:publish', + async handler(ctx) { + const { values } = ctx.parameters; + if ( + typeof values !== 'object' || + values === null || + Array.isArray(values) + ) { + throw new Error( + `Invalid values passed to publish, got ${typeof values}`, + ); + } + const storePath = values.storePath as unknown; + if (typeof storePath !== 'string') { + throw new Error( + `Invalid store path passed to publish, got ${typeof storePath}`, + ); + } + const owner = values.owner as unknown; + if (typeof owner !== 'string') { + throw new Error( + `Invalid store path passed to publish, got ${typeof owner}`, + ); + } + const publisher = publishers.get(storePath); + ctx.logger.info('Will now store the template'); + const { remoteUrl, catalogInfoUrl } = await publisher.publish({ + values: { + ...values, + owner, + storePath, + }, + workspacePath: ctx.workspacePath, + logger: ctx.logger, + }); + ctx.output('remoteUrl', remoteUrl); + if (catalogInfoUrl) { + ctx.output('catalogInfoUrl', catalogInfoUrl); + } + }, + }); } diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.ts index c22bb0bddb..c5920fe836 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.ts @@ -43,6 +43,14 @@ export class TaskAgent implements Task { return this.state.spec; } + get runId() { + return this.state.runId; + } + + get taskId() { + return this.state.taskId; + } + async emitLog(message: string): Promise { await this.storage.emit({ taskId: this.state.taskId, diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts index d500fc8e5c..6eb1c4763c 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts @@ -17,26 +17,17 @@ import { PassThrough } from 'stream'; import { Logger } from 'winston'; import * as winston from 'winston'; -import Docker from 'dockerode'; +import { JsonValue } from '@backstage/config'; import { TaskBroker, Task } from './types'; -import { CatalogEntityClient } from '../../lib/catalog'; -import { - FilePreparer, - parseLocationAnnotation, - PreparerBuilder, - TemplaterBuilder, - PublisherBuilder, -} from '../stages'; +import { TemplateActionRegistry } from './TemplateConverter'; +import fs from 'fs-extra'; +import path from 'path'; type Options = { logger: Logger; taskBroker: TaskBroker; workingDirectory: string; - dockerClient: Docker; - entityClient: CatalogEntityClient; - preparers: PreparerBuilder; - templaters: TemplaterBuilder; - publishers: PublisherBuilder; + actionRegistry: TemplateActionRegistry; }; export class TaskWorker { @@ -52,66 +43,90 @@ export class TaskWorker { } async runOneTask(task: Task) { - const { - dockerClient, - preparers, - templaters, - publishers, - workingDirectory, - logger, - } = this.options; - - const taskLogger = winston.createLogger({ - level: process.env.LOG_LEVEL || 'info', - format: winston.format.combine( - winston.format.colorize(), - winston.format.timestamp(), - winston.format.simple(), - ), - defaultMeta: {}, - }); - - const stream = new PassThrough(); - stream.on('data', data => { - const message = data.toString().trim(); - if (message?.length > 1) task.emitLog(message); - }); - - taskLogger.add(new winston.transports.Stream({ stream })); - try { - task.emitLog('Task claimed, waiting ...'); + const { actionRegistry, logger } = this.options; + + // bbl LUUUNCH + // taskID and runId not part of task? O_o + const workspacePath = await this.createWorkPath(task.taskId, task.runId); + + const taskLogger = winston.createLogger({ + level: process.env.LOG_LEVEL || 'info', + format: winston.format.combine( + winston.format.colorize(), + winston.format.timestamp(), + winston.format.simple(), + ), + defaultMeta: {}, + }); + + const stream = new PassThrough(); + stream.on('data', data => { + const message = data.toString().trim(); + if (message?.length > 1) task.emitLog(message); + }); + + taskLogger.add(new winston.transports.Stream({ stream })); + // Give us some time to curl observe + task.emitLog('Task claimed, waiting ...'); await new Promise(resolve => setTimeout(resolve, 5000)); + task.emitLog(`Starting up work with ${task.spec.steps.length} steps`); - const { values, template } = task.spec; - task.emitLog('Prepare the skeleton'); - const { protocol, location: pullPath } = parseLocationAnnotation( - task.spec.template, - ); + const outputs: { [name: string]: JsonValue } = {}; - const preparer = - protocol === 'file' ? new FilePreparer() : preparers.get(pullPath); - const templater = templaters.get(template); - const publisher = publishers.get(values.storePath); + for (const step of task.spec.steps) { + task.emitLog(`Beginning step ${step.name}`); - const skeletonDir = await preparer.prepare(task.spec.template, { - logger: taskLogger, - workingDirectory: workingDirectory, - }); + const action = actionRegistry.get(step.action); + if (!action) { + throw new Error(`Action '${step.action}' does not exist`); + } - task.emitLog('Run the templater'); - const { resultDir } = await templater.run({ - directory: skeletonDir, - dockerClient, - logStream: stream, - values: values, - }); + // TODO: substitute any placeholders with output from previous steps + const parameters = step.parameters!; - task.emitLog('Publish template'); - logger.info('Will now store the template'); + await action.handler({ + logger, + logStream: stream, + parameters, + workspacePath, + output(name: string, value: JsonValue) { + outputs[name] = value; + }, + }); - logger.info('Totally storing the template now'); + task.emitLog(`Finished step ${step.name}`); + } + + // const { values, template } = task.spec; + // task.emitLog('Prepare the skeleton'); + // const { protocol, location: pullPath } = parseLocationAnnotation( + // task.spec.template, + // ); + + // const preparer = + // protocol === 'file' ? new FilePreparer() : preparers.get(pullPath); + // const templater = templaters.get(template); + // const publisher = publishers.get(values.storePath); + + // const skeletonDir = await preparer.prepare(task.spec.template, { + // logger: taskLogger, + // workingDirectory: workingDirectory, + // }); + + // task.emitLog('Run the templater'); + // const { resultDir } = await templater.run({ + // directory: skeletonDir, + // dockerClient, + // logStream: stream, + // values: values, + // }); + + // task.emitLog('Publish template'); + // logger.info('Will now store the template'); + + logger.info('So done right now'); await new Promise(resolve => setTimeout(resolve, 5000)); // const result = await publisher.publish({ // values: values, @@ -125,4 +140,14 @@ export class TaskWorker { await task.complete('failed'); } } + + async createWorkPath(taskId: string, runId: string): Promise { + const workspacePath = path.join( + this.options.workingDirectory, + taskId, + runId, + ); + fs.ensureDir(workspacePath); + return workspacePath; + } } diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TemplateConverter.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TemplateConverter.ts index e435812fe5..bd06dbaa91 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TemplateConverter.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TemplateConverter.ts @@ -14,25 +14,37 @@ * limitations under the License. */ +import { resolve as resolvePath } from 'path'; import { JsonValue } from '@backstage/config'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; import { Logger } from 'winston'; import type { Writable } from 'stream'; -import { - getTemplaterKey, - parseLocationAnnotation, - TemplaterValues, -} from '../jobs/actions'; + import { TaskSpec } from './types'; import { ConflictError, NotFoundError } from '@backstage/backend-common'; +import { + getTemplaterKey, + joinGitUrlPath, + parseLocationAnnotation, + TemplaterValues, +} from '../stages'; -function templateEntityToSpec( +export function templateEntityToSpec( template: TemplateEntityV1alpha1, values: TemplaterValues, ): TaskSpec { const steps: TaskSpec['steps'] = []; - const { protocol, location: pullPath } = parseLocationAnnotation(template); + const { protocol, location } = parseLocationAnnotation(template); + + let url: string; + if (protocol === 'file') { + const path = resolvePath(location, template.spec.path || '.'); + + url = `file://${path}`; + } else { + url = joinGitUrlPath(location, template.spec.path); + } const templater = getTemplaterKey(template); steps.push({ @@ -41,7 +53,7 @@ function templateEntityToSpec( action: 'legacy:prepare', parameters: { protocol, - pullPath, + url, }, }); @@ -61,7 +73,6 @@ function templateEntityToSpec( action: 'publish', parameters: { values, - directory, }, }); @@ -72,7 +83,7 @@ type ActionContext = { logger: Logger; logStream: Writable; - workspaceDir: string; + workspacePath: string; parameters: { [name: string]: JsonValue }; output(name: string, value: JsonValue): void; }; diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts index 620b6e7243..496385d4ea 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { JsonObject } from '@backstage/config'; +import { JsonValue } from '@backstage/config'; export type Status = | 'open' @@ -50,7 +50,7 @@ export type TaskSpec = { id: string; name: string; action: string; - parameters?: JsonObject; + parameters?: { [name: string]: JsonValue }; }>; }; @@ -60,6 +60,8 @@ export type DispatchResult = { export interface Task { spec: TaskSpec; + taskId: string; + runId: string; emitLog(message: string): Promise; complete(result: CompletedTaskState): Promise; } diff --git a/plugins/scaffolder-backend/src/service/helpers.ts b/plugins/scaffolder-backend/src/service/helpers.ts new file mode 100644 index 0000000000..dd3d43c6d1 --- /dev/null +++ b/plugins/scaffolder-backend/src/service/helpers.ts @@ -0,0 +1,44 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import os from 'os'; +import fs from 'fs-extra'; +import { Logger } from 'winston'; +import { Config } from '@backstage/config'; + +export async function getWorkingDirectory( + config: Config, + logger: Logger, +): Promise { + if (!config.has('backend.workingDirectory')) { + return os.tmpdir(); + } + + const workingDirectory = config.getString('backend.workingDirectory'); + try { + // Check if working directory exists and is writable + await fs.access(workingDirectory, fs.constants.F_OK | fs.constants.W_OK); + logger.info(`using working directory: ${workingDirectory}`); + } catch (err) { + logger.error( + `working directory ${workingDirectory} ${ + err.code === 'ENOENT' ? 'does not exist' : 'is not writable' + }`, + ); + throw err; + } + return workingDirectory; +} diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index fc40d45ce2..a72a5c5d48 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -38,8 +38,13 @@ import { MemoryDatabase, TaskWorker, } from '../scaffolder/tasks'; -import { TemplateActionRegistry } from '../scaffolder/tasks/TemplateConverter'; +import { + TemplateActionRegistry, + templateEntityToSpec, +} from '../scaffolder/tasks/TemplateConverter'; import { LOCATION_ANNOTATION } from '@backstage/catalog-model'; +import { registerLegacyActions } from '../scaffolder/stages/legacy'; +import { getWorkingDirectory } from './helpers'; export interface RouterOptions { preparers: PreparerBuilder; @@ -69,18 +74,24 @@ export async function createRouter( } = options; const logger = parentLogger.child({ plugin: 'scaffolder' }); + const workingDirectory = await getWorkingDirectory(config, logger); const jobProcessor = await JobProcessor.fromConfig({ config, logger }); const taskBroker = new MemoryTaskBroker(new MemoryDatabase()); + const actionRegistry = new TemplateActionRegistry(); const worker = new TaskWorker({ logger, taskBroker, - workingDirectory: 'todo', + actionRegistry, + workingDirectory, + }); + + registerLegacyActions(actionRegistry, { dockerClient, - entityClient, preparers, publishers, templaters, }); + worker.start(); router @@ -127,10 +138,8 @@ export async function createRouter( res.status(400).json({ errors: validationResult.errors }); return; } - const result = await taskBroker.dispatch({ - template, - values, - }); + const taskSpec = templateEntityToSpec(template, values); + const result = await taskBroker.dispatch(taskSpec); res.status(201).json({ id: result.taskId }); }) From abff060e67dbacacdf3343a20df927390fd8b802 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Fri, 29 Jan 2021 15:56:29 +0100 Subject: [PATCH 15/37] Use url from parameters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Co-authored-by: blam Co-authored-by: Patrik Oldsberg --- .../scaffolder-backend/src/scaffolder/stages/legacy.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/legacy.ts b/plugins/scaffolder-backend/src/scaffolder/stages/legacy.ts index 6244e40d68..a3fe01c96d 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/legacy.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/legacy.ts @@ -40,17 +40,15 @@ export function registerLegacyActions( console.log(ctx); logger.info('Task claimed, waiting ...'); // Give us some time to curl observe - await new Promise(resolve => setTimeout(resolve, 5000)); + await new Promise(resolve => setTimeout(resolve, 1000)); logger.info('Prepare the skeleton'); - - const { protocol, pullPath } = ctx.parameters; - const url = pullPath as string; + const { protocol, url } = ctx.parameters; const preparer = - protocol === 'file' ? new FilePreparer() : preparers.get(url); + protocol === 'file' ? new FilePreparer() : preparers.get(url as string); await preparer.prepare({ - url, + url: url as string, logger: ctx.logger, workspacePath: ctx.workspacePath, }); From 7b7230f75bdd1d7a4c7513f11e9451b58f1f41f1 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Fri, 29 Jan 2021 15:57:37 +0100 Subject: [PATCH 16/37] Add getWorkspaceName --- .../src/scaffolder/tasks/MemoryTaskBroker.ts | 8 +-- .../src/scaffolder/tasks/TaskWorker.ts | 54 +++---------------- .../src/scaffolder/tasks/types.ts | 3 +- 3 files changed, 10 insertions(+), 55 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.ts index c5920fe836..6937c535dc 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.ts @@ -43,12 +43,8 @@ export class TaskAgent implements Task { return this.state.spec; } - get runId() { - return this.state.runId; - } - - get taskId() { - return this.state.taskId; + async getWorkspaceName() { + return `${this.state.taskId}_${this.state.runId}`; } async emitLog(message: string): Promise { diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts index 6eb1c4763c..c80a1b81bf 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts @@ -19,9 +19,9 @@ import { Logger } from 'winston'; import * as winston from 'winston'; import { JsonValue } from '@backstage/config'; import { TaskBroker, Task } from './types'; -import { TemplateActionRegistry } from './TemplateConverter'; import fs from 'fs-extra'; import path from 'path'; +import { TemplateActionRegistry } from './TemplateConverter'; type Options = { logger: Logger; @@ -46,9 +46,11 @@ export class TaskWorker { try { const { actionRegistry, logger } = this.options; - // bbl LUUUNCH - // taskID and runId not part of task? O_o - const workspacePath = await this.createWorkPath(task.taskId, task.runId); + const workspacePath = path.join( + this.options.workingDirectory, + await task.getWorkspaceName(), + ); + await fs.ensureDir(workspacePath); const taskLogger = winston.createLogger({ level: process.env.LOG_LEVEL || 'info', @@ -99,55 +101,13 @@ export class TaskWorker { task.emitLog(`Finished step ${step.name}`); } - // const { values, template } = task.spec; - // task.emitLog('Prepare the skeleton'); - // const { protocol, location: pullPath } = parseLocationAnnotation( - // task.spec.template, - // ); - - // const preparer = - // protocol === 'file' ? new FilePreparer() : preparers.get(pullPath); - // const templater = templaters.get(template); - // const publisher = publishers.get(values.storePath); - - // const skeletonDir = await preparer.prepare(task.spec.template, { - // logger: taskLogger, - // workingDirectory: workingDirectory, - // }); - - // task.emitLog('Run the templater'); - // const { resultDir } = await templater.run({ - // directory: skeletonDir, - // dockerClient, - // logStream: stream, - // values: values, - // }); - - // task.emitLog('Publish template'); - // logger.info('Will now store the template'); - logger.info('So done right now'); await new Promise(resolve => setTimeout(resolve, 5000)); - // const result = await publisher.publish({ - // values: values, - // directory: resultDir, - // logger, - // }); - // task.emitLog(`Result: ${JSON.stringify(result)}`); await task.complete('completed'); } catch (error) { + task.emitLog(error); await task.complete('failed'); } } - - async createWorkPath(taskId: string, runId: string): Promise { - const workspacePath = path.join( - this.options.workingDirectory, - taskId, - runId, - ); - fs.ensureDir(workspacePath); - return workspacePath; - } } diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts index 496385d4ea..d3c08085fd 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts @@ -60,10 +60,9 @@ export type DispatchResult = { export interface Task { spec: TaskSpec; - taskId: string; - runId: string; emitLog(message: string): Promise; complete(result: CompletedTaskState): Promise; + getWorkspaceName(): Promise; } export interface TaskBroker { From 95c2719809adec52fb1979757559e1172e1e3073 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Fri, 29 Jan 2021 15:58:20 +0100 Subject: [PATCH 17/37] Rename publish to legacy:publish --- .../src/scaffolder/tasks/TemplateConverter.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TemplateConverter.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TemplateConverter.ts index bd06dbaa91..86f722e475 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TemplateConverter.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TemplateConverter.ts @@ -70,7 +70,7 @@ export function templateEntityToSpec( steps.push({ id: 'publish', name: 'Publishing', - action: 'publish', + action: 'legacy:publish', parameters: { values, }, @@ -105,10 +105,6 @@ export class TemplateActionRegistry { this.actions.set(action.id, action); } - // validate - // ensure that action exist. - // template variables exist. - get(actionId: string): TemplateAction { const action = this.actions.get(actionId); if (!action) { From 605eb7f59401429fa5ab48ec83f4acf0a313dbfa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 20 Jan 2021 15:36:36 +0100 Subject: [PATCH 18/37] scaffolder: add the bare minimum of a db abstraction --- plugins/scaffolder-backend/knexfile.js | 25 +++++ .../migrations/20210120143715_init.js | 100 ++++++++++++++++++ plugins/scaffolder-backend/package.json | 3 +- .../scaffolder-backend/src/tasks/Database.ts | 70 ++++++++++++ plugins/scaffolder-backend/src/tasks/types.ts | 37 +++++++ 5 files changed, 234 insertions(+), 1 deletion(-) create mode 100644 plugins/scaffolder-backend/knexfile.js create mode 100644 plugins/scaffolder-backend/migrations/20210120143715_init.js create mode 100644 plugins/scaffolder-backend/src/tasks/Database.ts create mode 100644 plugins/scaffolder-backend/src/tasks/types.ts diff --git a/plugins/scaffolder-backend/knexfile.js b/plugins/scaffolder-backend/knexfile.js new file mode 100644 index 0000000000..f469df4c08 --- /dev/null +++ b/plugins/scaffolder-backend/knexfile.js @@ -0,0 +1,25 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +module.exports = { + development: { + client: 'sqlite3', + connection: ':memory:', + migrations: { + directory: 'migrations', + }, + }, +}; diff --git a/plugins/scaffolder-backend/migrations/20210120143715_init.js b/plugins/scaffolder-backend/migrations/20210120143715_init.js new file mode 100644 index 0000000000..d2535bfa86 --- /dev/null +++ b/plugins/scaffolder-backend/migrations/20210120143715_init.js @@ -0,0 +1,100 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// @ts-check + +/** + * @param {import('knex')} knex + */ +exports.up = async function up(knex) { + await knex.schema.createTable('tasks', table => { + table.comment('The table of scaffolder tasks'); + table.uuid('id').primary().notNullable().comment('The ID of the task'); + table + .text('context') + .notNullable() + .comment('A JSON object with task specific context information'); + table + .text('spec') + .notNullable() + .comment('A JSON encoded task specification'); + table + .text('status') + .notNullable() + .comment('The current status of the task'); + table + .integer('run_id') + .nullable() + .comment('The current run ID of the task'); + table + .dateTime('created_at') + .notNullable() + .comment('The timestamp when this task was created'); + table + .dateTime('last_heartbeat_at') + .nullable() + .comment('The last timestamp when a heartbeat was received'); + table + .integer('retry_count') + .notNullable() + .defaultTo(0) + .comment('The number of times that this task has been attempted'); + }); + await knex.schema.createTable('task_events', table => { + table.comment('The event stream a given task'); + table + .bigIncrements('id') + .primary() + .notNullable() + .comment('The ID of the event'); + table + .uuid('task_id') + .references('id') + .inTable('tasks') + .notNullable() + .onDelete('CASCADE') + .comment('The task that generated the event'); + table + .integer('run_id') + .nullable() + .comment('The run ID of the task that this event applies to'); + table + .text('stage_name') + .nullable() + .comment('The stage of the task that this event applies to'); + table.text('event_type').notNullable().comment('The type of event'); + table + .dateTime('created_at') + .defaultTo(knex.fn.now()) + .notNullable() + .comment('The timestamp when this event was generated'); + table.text('text').notNullable().comment('The text of the event'); + table.index(['task_id'], 'task_events_task_id_idx'); + }); +}; + +/** + * @param {import('knex')} knex + */ +exports.down = async function down(knex) { + if (knex.client.config.client !== 'sqlite3') { + await knex.schema.alterTable('task_events', table => { + table.dropIndex([], 'task_events_task_id_idx'); + }); + } + await knex.schema.dropTable('task_events'); + await knex.schema.dropTable('tasks'); +}; diff --git a/plugins/scaffolder-backend/package.json b/plugins/scaffolder-backend/package.json index 97ab723e81..5d024c2798 100644 --- a/plugins/scaffolder-backend/package.json +++ b/plugins/scaffolder-backend/package.json @@ -53,7 +53,7 @@ "helmet": "^4.0.0", "isomorphic-git": "^1.8.0", "jsonschema": "^1.2.6", - "luxon": "^1.25.0", + "knex": "^0.21.6", "morgan": "^1.10.0", "p-queue": "^6.3.0", "uuid": "^8.2.0", @@ -73,6 +73,7 @@ }, "files": [ "dist", + "migrations", "config.d.ts" ], "configSchema": "config.d.ts" diff --git a/plugins/scaffolder-backend/src/tasks/Database.ts b/plugins/scaffolder-backend/src/tasks/Database.ts new file mode 100644 index 0000000000..c135380f87 --- /dev/null +++ b/plugins/scaffolder-backend/src/tasks/Database.ts @@ -0,0 +1,70 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { ConflictError, resolvePackagePath } from '@backstage/backend-common'; +import Knex from 'knex'; +import { Logger } from 'winston'; +import { Database, Transaction } from './types'; + +const migrationsDir = resolvePackagePath( + '@backstage/plugin-scaffolder-backend', + 'migrations', +); + +export class CommonDatabase implements Database { + static async create(knex: Knex, logger: Logger): Promise { + await knex.migrate.latest({ + directory: migrationsDir, + }); + return new CommonDatabase(knex, logger); + } + + constructor( + private readonly database: Knex, + private readonly logger: Logger, + ) {} + + async transaction(fn: (tx: Transaction) => Promise): Promise { + try { + let result: T | undefined = undefined; + + await this.database.transaction( + async tx => { + // We can't return here, as knex swallows the return type in case the transaction is rolled back: + // https://github.com/knex/knex/blob/e37aeaa31c8ef9c1b07d2e4d3ec6607e557d800d/lib/transaction.js#L136 + result = await fn(tx); + }, + { + // If we explicitly trigger a rollback, don't fail. + doNotRejectOnRollback: true, + }, + ); + + return result!; + } catch (e) { + this.logger.debug(`Error during transaction, ${e}`); + + if ( + /SQLITE_CONSTRAINT: UNIQUE/.test(e.message) || + /unique constraint/.test(e.message) + ) { + throw new ConflictError(`Rejected due to a conflicting entity`, e); + } + + throw e; + } + } +} diff --git a/plugins/scaffolder-backend/src/tasks/types.ts b/plugins/scaffolder-backend/src/tasks/types.ts new file mode 100644 index 0000000000..27cbe7f7f1 --- /dev/null +++ b/plugins/scaffolder-backend/src/tasks/types.ts @@ -0,0 +1,37 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * The core database implementation. + */ +export interface Database { + /** + * Runs a transaction. + * + * The callback is expected to make calls back into this class. When it + * completes, the transaction is closed. + * + * @param fn The callback that implements the transaction + */ + transaction(fn: (tx: Transaction) => Promise): Promise; +} + +/** + * An abstraction for transactions of the underlying database technology. + */ +export type Transaction = { + rollback(): Promise; +}; From 71dc78b01ff6d64c644da18986be7b5d450b8042 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Mon, 1 Feb 2021 14:06:09 +0100 Subject: [PATCH 19/37] Wire up database and rename database classes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Co-authored-by: blam Co-authored-by: Patrik Oldsberg --- packages/backend/src/plugins/scaffolder.ts | 2 + .../backend/src/plugins/scaffolder.ts | 2 + .../migrations/20210120143715_init.js | 14 +- .../src/scaffolder/tasks/DatabaseTaskStore.ts | 248 ++++++++++++++++++ ...tabase.test.ts => MemoryTaskStore.test.ts} | 0 .../{MemoryDatabase.ts => MemoryTaskStore.ts} | 44 +--- ...oker.test.ts => StorageTaskBroker.test.ts} | 12 +- ...moryTaskBroker.ts => StorageTaskBroker.ts} | 18 +- .../src/scaffolder/tasks/TaskWorker.ts | 3 +- .../src/scaffolder/tasks/index.ts | 5 +- .../src/scaffolder/tasks/types.ts | 30 ++- .../scaffolder-backend/src/service/router.ts | 14 +- 12 files changed, 328 insertions(+), 64 deletions(-) create mode 100644 plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts rename plugins/scaffolder-backend/src/scaffolder/tasks/{MemoryDatabase.test.ts => MemoryTaskStore.test.ts} (100%) rename plugins/scaffolder-backend/src/scaffolder/tasks/{MemoryDatabase.ts => MemoryTaskStore.ts} (73%) rename plugins/scaffolder-backend/src/scaffolder/tasks/{MemoryTaskBroker.test.ts => StorageTaskBroker.test.ts} (90%) rename plugins/scaffolder-backend/src/scaffolder/tasks/{MemoryTaskBroker.ts => StorageTaskBroker.ts} (90%) diff --git a/packages/backend/src/plugins/scaffolder.ts b/packages/backend/src/plugins/scaffolder.ts index 4e2257a46c..723165ff82 100644 --- a/packages/backend/src/plugins/scaffolder.ts +++ b/packages/backend/src/plugins/scaffolder.ts @@ -30,6 +30,7 @@ import Docker from 'dockerode'; export default async function createPlugin({ logger, config, + database, }: PluginEnvironment) { const cookiecutterTemplater = new CookieCutter(); const craTemplater = new CreateReactAppTemplater(); @@ -54,5 +55,6 @@ export default async function createPlugin({ config, dockerClient, entityClient, + database, }); } diff --git a/packages/create-app/templates/default-app/packages/backend/src/plugins/scaffolder.ts b/packages/create-app/templates/default-app/packages/backend/src/plugins/scaffolder.ts index c8bd3e5012..d68f90ce08 100644 --- a/packages/create-app/templates/default-app/packages/backend/src/plugins/scaffolder.ts +++ b/packages/create-app/templates/default-app/packages/backend/src/plugins/scaffolder.ts @@ -14,6 +14,7 @@ import Docker from 'dockerode'; export default async function createPlugin({ logger, config, + database, }: PluginEnvironment) { const cookiecutterTemplater = new CookieCutter(); const craTemplater = new CreateReactAppTemplater(); @@ -38,5 +39,6 @@ export default async function createPlugin({ config, dockerClient, entityClient, + database, }); } diff --git a/plugins/scaffolder-backend/migrations/20210120143715_init.js b/plugins/scaffolder-backend/migrations/20210120143715_init.js index d2535bfa86..bb65e1e2fb 100644 --- a/plugins/scaffolder-backend/migrations/20210120143715_init.js +++ b/plugins/scaffolder-backend/migrations/20210120143715_init.js @@ -23,10 +23,6 @@ exports.up = async function up(knex) { await knex.schema.createTable('tasks', table => { table.comment('The table of scaffolder tasks'); table.uuid('id').primary().notNullable().comment('The ID of the task'); - table - .text('context') - .notNullable() - .comment('A JSON object with task specific context information'); table .text('spec') .notNullable() @@ -41,6 +37,7 @@ exports.up = async function up(knex) { .comment('The current run ID of the task'); table .dateTime('created_at') + .defaultTo(knex.fn.now()) .notNullable() .comment('The timestamp when this task was created'); table @@ -53,6 +50,7 @@ exports.up = async function up(knex) { .defaultTo(0) .comment('The number of times that this task has been attempted'); }); + await knex.schema.createTable('task_events', table => { table.comment('The event stream a given task'); table @@ -72,16 +70,16 @@ exports.up = async function up(knex) { .nullable() .comment('The run ID of the task that this event applies to'); table - .text('stage_name') - .nullable() - .comment('The stage of the task that this event applies to'); + .text('body') + .notNullable() + .comment('The JSON encoded body of the event'); table.text('event_type').notNullable().comment('The type of event'); table .dateTime('created_at') .defaultTo(knex.fn.now()) .notNullable() .comment('The timestamp when this event was generated'); - table.text('text').notNullable().comment('The text of the event'); + table.index(['task_id'], 'task_events_task_id_idx'); }); }; diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts new file mode 100644 index 0000000000..e6de1735d8 --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts @@ -0,0 +1,248 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { JsonObject } from '@backstage/config'; +import { + ConflictError, + NotFoundError, + resolvePackagePath, +} from '@backstage/backend-common'; +import Knex, { Transaction } from 'knex'; +import { Logger } from 'winston'; +import { v4 as uuid } from 'uuid'; +import { + DbTaskEventRow, + DbTaskRow, + Status, + TaskEventType, + TaskSpec, + TaskStore, + TaskStoreEmitOptions, + TaskStoreGetEventsOptions, +} from './types'; + +const migrationsDir = resolvePackagePath( + '@backstage/plugin-scaffolder-backend', + 'migrations', +); + +export type RawDbTaskRow = { + id: string; + spec: string; + status: Status; + last_heartbeat_at?: string; + retry_count: number; + created_at: string; + run_id?: string; +}; + +export type RawDbTaskEventRow = { + id: number; + run_id: string; + task_id: string; + body: string; + event_type: TaskEventType; + created_at: string; +}; + +export class DatabaseTaskStore implements TaskStore { + static async create(knex: Knex): Promise { + await knex.migrate.latest({ + directory: migrationsDir, + }); + return new DatabaseTaskStore(knex); + } + + constructor(private readonly db: Knex) {} + + async get(taskId: string): Promise { + const [result] = await this.db('tasks') + .where({ id: taskId }) + .select(); + if (!result) { + throw new NotFoundError(`No task with id '${taskId}' found`); + } + try { + const spec = JSON.parse(result.spec); + return { + id: result.id, + spec, + status: result.status, + lastHeartbeat: result.last_heartbeat_at, + retryCount: result.retry_count, + createdAt: result.created_at, + runId: result.run_id, + }; + } catch (error) { + throw new Error(`Failed to parse spec of task '${taskId}', ${error}`); + } + } + + async createTask(spec: TaskSpec): Promise<{ taskId: string }> { + const taskId = uuid(); + await this.db('tasks').insert({ + id: taskId, + spec: JSON.stringify(spec), + status: 'open', + retry_count: 0, + }); + return { taskId }; + } + + async claimTask(): Promise { + return this.db.transaction(async tx => { + const [task] = await tx('tasks') + .where({ + status: 'open', + }) + .limit(1) + .select(); + + if (!task) { + return undefined; + } + + const runId = uuid(); + const updateCount = await tx('tasks') + .where({ id: task.id, status: 'open' }) + .update({ + status: 'processing', + run_id: runId, + }); + + if (updateCount < 1) { + return undefined; + } + + try { + const spec = JSON.parse(task.spec); + return { + id: task.id, + spec, + status: 'processing', + lastHeartbeat: task.last_heartbeat_at, + retryCount: task.retry_count, + createdAt: task.created_at, + runId: runId, + }; + } catch (error) { + throw new Error(`Failed to parse spec of task '${task.id}', ${error}`); + } + }); + } + + async heartbeat(runId: string): Promise { + const updateCount = await this.db('tasks') + .where({ run_id: runId, status: 'processing' }) + .update({ + last_heartbeat_at: this.db.fn.now(), + }); + if (updateCount === 0) { + throw new Error(`No running task with runId ${runId} found`); + } + } + + async setStatus(runId: string, status: Status): Promise { + let oldStatus: string; + if (status === 'failed' || status === 'completed') { + oldStatus = 'processing'; + } else { + throw new Error( + `Invalid status update of run '${runId}' to status '${status}'`, + ); + } + await this.db.transaction(async tx => { + const [task] = await tx('tasks') + .where({ + run_id: runId, + }) + .limit(1) + .select(); + + if (!task) { + throw new Error(`No task with runId ${runId} found`); + } + if (task.status !== oldStatus) { + throw new ConflictError( + `Refusing to update status of run '${runId}' to status '${status}' ` + + `as it is currently '${task.status}', expected '${oldStatus}'`, + ); + } + const updateCount = await tx('tasks') + .where({ + run_id: runId, + status: oldStatus, + }) + .update({ + status, + }); + if (updateCount !== 1) { + throw new Error( + `Failed to update status to '${status}' for runId ${runId}`, + ); + } + }); + } + + async emit({ + taskId, + runId, + body, + type, + }: TaskStoreEmitOptions): Promise { + const serliazedBody = JSON.stringify(body); + await this.db('task_events').insert({ + task_id: taskId, + run_id: runId, + event_type: type, + body: serliazedBody, + }); + } + + async getEvents({ + taskId, + after, + }: TaskStoreGetEventsOptions): Promise<{ events: DbTaskEventRow[] }> { + let query = this.db('task_events').where({ + task_id: taskId, + }); + if (typeof after === 'number') { + query = query + .where('task_events.id', '>', after) + .orWhere({ event_type: 'completion' }); + } + + const rawEvents = await query.select(); + const events = rawEvents.map(event => { + try { + const body = JSON.parse(event.body) as JsonObject; + return { + id: event.id, + runId: event.run_id, + taskId: event.task_id, + body, + type: event.event_type, + createdAt: event.created_at, + }; + } catch (error) { + throw new Error( + `Failed to parse event body from event taskId=${taskId} id=${event.id}, ${error}`, + ); + } + }); + return { events }; + } +} diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryDatabase.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskStore.test.ts similarity index 100% rename from plugins/scaffolder-backend/src/scaffolder/tasks/MemoryDatabase.test.ts rename to plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskStore.test.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryDatabase.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskStore.ts similarity index 73% rename from plugins/scaffolder-backend/src/scaffolder/tasks/MemoryDatabase.ts rename to plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskStore.ts index b2d1cf667b..e181edf524 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryDatabase.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskStore.ts @@ -19,35 +19,17 @@ import { DbTaskEventRow, Status, TaskSpec, - TaskEventType, + TaskStore, + TaskStoreGetEventsOptions, + TaskStoreEmitOptions, } from './types'; import { v4 as uuid } from 'uuid'; -export interface Database { - get(taskId: string): Promise; - createTask(task: TaskSpec): Promise; - claimTask(): Promise; - heartbeat(runId: string): Promise; - setStatus(taskId: string, status: Status): Promise; -} - -type EmitOptions = { - taskId: string; - runId: string; - body: string; - type: TaskEventType; -}; - -type ReadOptions = { - taskId: string; - after?: number | undefined; -}; - -export class MemoryDatabase implements Database { +export class MemoryTaskStore implements TaskStore { private readonly store = new Map(); private readonly events = new Array(); - async emit({ taskId, runId, body, type }: EmitOptions) { + async emit({ taskId, runId, body, type }: TaskStoreEmitOptions) { this.events.push({ id: this.events.length, taskId, @@ -61,7 +43,7 @@ export class MemoryDatabase implements Database { async getEvents({ taskId, after, - }: ReadOptions): Promise<{ events: DbTaskEventRow[] }> { + }: TaskStoreGetEventsOptions): Promise<{ events: DbTaskEventRow[] }> { const events = this.events.filter(event => { if (event.taskId !== taskId) { return false; @@ -89,7 +71,7 @@ export class MemoryDatabase implements Database { throw new Error('No task with matching runId found'); } - this.store.set(task.taskId, { + this.store.set(task.id, { ...task, lastHeartbeat: new Date().toISOString(), }); @@ -103,23 +85,23 @@ export class MemoryDatabase implements Database { status: 'processing', runId: uuid(), }; - this.store.set(t.taskId, task); + this.store.set(t.id, task); return task; } } return undefined; } - async createTask(spec: TaskSpec): Promise { + async createTask(spec: TaskSpec): Promise<{ taskId: string }> { const taskRow = { - taskId: uuid(), + id: uuid(), spec, status: 'open' as Status, retryCount: 0, createdAt: new Date().toISOString(), }; - this.store.set(taskRow.taskId, taskRow); - return taskRow; + this.store.set(taskRow.id, taskRow); + return { taskId: taskRow.id }; } async get(taskId: string): Promise { @@ -135,6 +117,6 @@ export class MemoryDatabase implements Database { if (!task) { throw new Error(`no task found`); } - this.store.set(task.taskId, { ...task, status }); + this.store.set(task.id, { ...task, status }); } } diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.test.ts similarity index 90% rename from plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.test.ts rename to plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.test.ts index 686a6d9132..5bbe91ea87 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.test.ts @@ -15,13 +15,13 @@ */ import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; -import { TemplaterValues } from './actions/templater/types'; -import { MemoryDatabase } from './MemoryDatabase'; -import { MemoryTaskBroker, TaskAgent } from './MemoryTaskBroker'; +import { TemplaterValues } from '../stages'; +import { MemoryTaskStore } from './MemoryTaskStore'; +import { StorageTaskBroker, TaskAgent } from './StorageTaskBroker'; -describe('MemoryTaskBroker', () => { - const storage = new MemoryDatabase(); - const broker = new MemoryTaskBroker(storage); +describe('StorageTaskBroker', () => { + const storage = new MemoryTaskStore(); + const broker = new StorageTaskBroker(storage); const taskSpec = { values: {} as TemplaterValues, diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts similarity index 90% rename from plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.ts rename to plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts index 6937c535dc..6eedc023f8 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskBroker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts @@ -18,16 +18,16 @@ import { CompletedTaskState, Task, TaskSpec, + TaskStore, TaskBroker, DispatchResult, DbTaskEventRow, } from './types'; -import { MemoryDatabase } from './MemoryDatabase'; export class TaskAgent implements Task { private heartbeartInterval?: ReturnType; - static create(state: TaskState, storage: MemoryDatabase) { + static create(state: TaskState, storage: TaskStore) { const agent = new TaskAgent(state, storage); agent.start(); return agent; @@ -36,7 +36,7 @@ export class TaskAgent implements Task { // Runs heartbeat internally private constructor( private readonly state: TaskState, - private readonly storage: MemoryDatabase, + private readonly storage: TaskStore, ) {} get spec() { @@ -51,20 +51,20 @@ export class TaskAgent implements Task { await this.storage.emit({ taskId: this.state.taskId, runId: this.state.runId, - body: message, + body: { message }, type: 'log', }); } async complete(result: CompletedTaskState): Promise { await this.storage.setStatus( - this.state.taskId, + this.state.runId, result === 'failed' ? 'failed' : 'completed', ); this.storage.emit({ taskId: this.state.taskId, runId: this.state.runId, - body: `Run completed with status: ${result}`, + body: { message: `Run completed with status: ${result}` }, type: 'completion', }); if (this.heartbeartInterval) { @@ -96,8 +96,8 @@ function defer() { return { promise, resolve }; } -export class MemoryTaskBroker implements TaskBroker { - constructor(private readonly storage: MemoryDatabase) {} +export class StorageTaskBroker implements TaskBroker { + constructor(private readonly storage: TaskStore) {} private deferredDispatch = defer(); async claim(): Promise { @@ -107,7 +107,7 @@ export class MemoryTaskBroker implements TaskBroker { return TaskAgent.create( { runId: pendingTask.runId!, - taskId: pendingTask.taskId, + taskId: pendingTask.id, spec: pendingTask.spec, }, this.storage, diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts index c80a1b81bf..64cfdc6f96 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts @@ -73,6 +73,7 @@ export class TaskWorker { // Give us some time to curl observe task.emitLog('Task claimed, waiting ...'); await new Promise(resolve => setTimeout(resolve, 5000)); + console.log('DEBUG: task.spec =', JSON.stringify(task.spec, null, 2)); task.emitLog(`Starting up work with ${task.spec.steps.length} steps`); const outputs: { [name: string]: JsonValue } = {}; @@ -106,7 +107,7 @@ export class TaskWorker { await task.complete('completed'); } catch (error) { - task.emitLog(error); + task.emitLog(String(error.stack)); await task.complete('failed'); } } diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/index.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/index.ts index 1cc9842fed..7b6552a3e0 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/index.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/index.ts @@ -14,6 +14,7 @@ * limitations under the License. */ -export { MemoryDatabase } from './MemoryDatabase'; -export { MemoryTaskBroker } from './MemoryTaskBroker'; +export { MemoryTaskStore } from './MemoryTaskStore'; +export { DatabaseTaskStore } from './DatabaseTaskStore'; +export { StorageTaskBroker } from './StorageTaskBroker'; export { TaskWorker } from './TaskWorker'; diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts index d3c08085fd..f9081af6b6 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { JsonValue } from '@backstage/config'; +import { JsonValue, JsonObject } from '@backstage/config'; export type Status = | 'open' @@ -26,7 +26,7 @@ export type Status = export type CompletedTaskState = 'failed' | 'completed'; export type DbTaskRow = { - taskId: string; + id: string; spec: TaskSpec; status: Status; lastHeartbeat?: string; @@ -40,7 +40,7 @@ export type DbTaskEventRow = { id: number; runId: string; taskId: string; - body: string; + body: JsonObject; type: TaskEventType; createdAt: string; }; @@ -69,3 +69,27 @@ export interface TaskBroker { claim(): Promise; dispatch(spec: TaskSpec): Promise; } + +export type TaskStoreEmitOptions = { + taskId: string; + runId: string; + body: JsonObject; + type: TaskEventType; +}; + +export type TaskStoreGetEventsOptions = { + taskId: string; + after?: number | undefined; +}; +export interface TaskStore { + get(taskId: string): Promise; + createTask(task: TaskSpec): Promise<{ taskId: string }>; + claimTask(): Promise; + heartbeat(runId: string): Promise; + setStatus(runId: string, status: Status): Promise; + emit({ taskId, runId, body, type }: TaskStoreEmitOptions): Promise; + getEvents({ + taskId, + after, + }: TaskStoreGetEventsOptions): Promise<{ events: DbTaskEventRow[] }>; +} diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index a72a5c5d48..23af538df8 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -34,17 +34,17 @@ import { CatalogEntityClient } from '../lib/catalog'; import { validate, ValidatorResult } from 'jsonschema'; import parseGitUrl from 'git-url-parse'; import { - MemoryTaskBroker, - MemoryDatabase, + DatabaseTaskStore, + StorageTaskBroker, TaskWorker, } from '../scaffolder/tasks'; import { TemplateActionRegistry, templateEntityToSpec, } from '../scaffolder/tasks/TemplateConverter'; -import { LOCATION_ANNOTATION } from '@backstage/catalog-model'; import { registerLegacyActions } from '../scaffolder/stages/legacy'; import { getWorkingDirectory } from './helpers'; +import { PluginDatabaseManager } from '@backstage/backend-common'; export interface RouterOptions { preparers: PreparerBuilder; @@ -55,6 +55,7 @@ export interface RouterOptions { config: Config; dockerClient: Docker; entityClient: CatalogEntityClient; + database: PluginDatabaseManager; } export async function createRouter( @@ -71,12 +72,17 @@ export async function createRouter( config, dockerClient, entityClient, + database, } = options; const logger = parentLogger.child({ plugin: 'scaffolder' }); const workingDirectory = await getWorkingDirectory(config, logger); const jobProcessor = await JobProcessor.fromConfig({ config, logger }); - const taskBroker = new MemoryTaskBroker(new MemoryDatabase()); + + const databaseTaskStore = await DatabaseTaskStore.create( + await database.getClient(), + ); + const taskBroker = new StorageTaskBroker(databaseTaskStore); const actionRegistry = new TemplateActionRegistry(); const worker = new TaskWorker({ logger, From 5da5e6b2243516a448fbbe88fd806b550ba2f0ed Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Mon, 1 Feb 2021 14:07:19 +0100 Subject: [PATCH 20/37] Delete MemoryTaskStore This is replaced by running sqlite in memory. --- .../scaffolder/tasks/MemoryTaskStore.test.ts | 21 --- .../src/scaffolder/tasks/MemoryTaskStore.ts | 122 ------------------ .../src/scaffolder/tasks/index.ts | 1 - 3 files changed, 144 deletions(-) delete mode 100644 plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskStore.test.ts delete mode 100644 plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskStore.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskStore.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskStore.test.ts deleted file mode 100644 index c5522cb38a..0000000000 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskStore.test.ts +++ /dev/null @@ -1,21 +0,0 @@ -/* - * Copyright 2021 Spotify AB - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -describe('MemoryDatabase', () => { - it('should be tested', async () => { - expect(1).toBe(2); - }); -}); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskStore.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskStore.ts deleted file mode 100644 index e181edf524..0000000000 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/MemoryTaskStore.ts +++ /dev/null @@ -1,122 +0,0 @@ -/* - * Copyright 2021 Spotify AB - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { - DbTaskRow, - DbTaskEventRow, - Status, - TaskSpec, - TaskStore, - TaskStoreGetEventsOptions, - TaskStoreEmitOptions, -} from './types'; -import { v4 as uuid } from 'uuid'; - -export class MemoryTaskStore implements TaskStore { - private readonly store = new Map(); - private readonly events = new Array(); - - async emit({ taskId, runId, body, type }: TaskStoreEmitOptions) { - this.events.push({ - id: this.events.length, - taskId, - runId, - body, - type, - createdAt: new Date().toISOString(), - }); - } - - async getEvents({ - taskId, - after, - }: TaskStoreGetEventsOptions): Promise<{ events: DbTaskEventRow[] }> { - const events = this.events.filter(event => { - if (event.taskId !== taskId) { - return false; - } - if (after !== undefined) { - if (event.id <= after) { - return false; - } - } - return true; - }); - return { events }; - } - - async heartbeat(runId: string): Promise { - let task: DbTaskRow | undefined; - - for (const t of this.store.values()) { - if (t.runId === runId) { - task = t; - } - } - - if (!task) { - throw new Error('No task with matching runId found'); - } - - this.store.set(task.id, { - ...task, - lastHeartbeat: new Date().toISOString(), - }); - } - - async claimTask(): Promise { - for (const t of this.store.values()) { - if (t.status === 'open') { - const task: DbTaskRow = { - ...t, - status: 'processing', - runId: uuid(), - }; - this.store.set(t.id, task); - return task; - } - } - return undefined; - } - - async createTask(spec: TaskSpec): Promise<{ taskId: string }> { - const taskRow = { - id: uuid(), - spec, - status: 'open' as Status, - retryCount: 0, - createdAt: new Date().toISOString(), - }; - this.store.set(taskRow.id, taskRow); - return { taskId: taskRow.id }; - } - - async get(taskId: string): Promise { - const task = this.store.get(taskId); - if (task) { - return task; - } - throw new Error(`could not found task ${taskId}`); - } - - async setStatus(taskId: string, status: Status): Promise { - const task = this.store.get(taskId); - if (!task) { - throw new Error(`no task found`); - } - this.store.set(task.id, { ...task, status }); - } -} diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/index.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/index.ts index 7b6552a3e0..c85135426e 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/index.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/index.ts @@ -14,7 +14,6 @@ * limitations under the License. */ -export { MemoryTaskStore } from './MemoryTaskStore'; export { DatabaseTaskStore } from './DatabaseTaskStore'; export { StorageTaskBroker } from './StorageTaskBroker'; export { TaskWorker } from './TaskWorker'; From 88c31ab16ba778b43c5c0e6fb6aad4a3f68cf8c0 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Tue, 2 Feb 2021 09:27:18 +0100 Subject: [PATCH 21/37] Add tests. Extend storage with fetch staletasks method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Co-authored-by: blam Co-authored-by: Patrik Oldsberg --- plugins/scaffolder-backend/package.json | 3 +- .../src/scaffolder/tasks/DatabaseTaskStore.ts | 35 +++-- .../tasks/StorageTaskBroker.test.ts | 145 ++++++++++++++---- .../src/scaffolder/tasks/StorageTaskBroker.ts | 17 +- .../src/scaffolder/tasks/types.ts | 1 + .../src/service/router.test.ts | 25 ++- .../scaffolder-backend/src/service/router.ts | 6 +- 7 files changed, 182 insertions(+), 50 deletions(-) diff --git a/plugins/scaffolder-backend/package.json b/plugins/scaffolder-backend/package.json index 5d024c2798..bdecf6196e 100644 --- a/plugins/scaffolder-backend/package.json +++ b/plugins/scaffolder-backend/package.json @@ -58,7 +58,8 @@ "p-queue": "^6.3.0", "uuid": "^8.2.0", "winston": "^3.2.1", - "yaml": "^1.10.0" + "yaml": "^1.10.0", + "luxon": "^1.25.0" }, "devDependencies": { "@backstage/cli": "^0.5.0", diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts index e6de1735d8..1d99cd2958 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts @@ -20,8 +20,7 @@ import { NotFoundError, resolvePackagePath, } from '@backstage/backend-common'; -import Knex, { Transaction } from 'knex'; -import { Logger } from 'winston'; +import Knex from 'knex'; import { v4 as uuid } from 'uuid'; import { DbTaskEventRow, @@ -121,6 +120,7 @@ export class DatabaseTaskStore implements TaskStore { .update({ status: 'processing', run_id: runId, + last_heartbeat_at: this.db.fn.now(), }); if (updateCount < 1) { @@ -155,6 +155,18 @@ export class DatabaseTaskStore implements TaskStore { } } + async listStaleTasks(): Promise<{ tasks: DbTaskRow }> { + const rows = await this.db('tasks') + .where('status', 'processing') + .andWhere( + 'last_heartbeat_at', + '<', + this.db.type === 'sqlite' + ? this.db.raw("datetime('now', '-2 seconds')") + : this.db.raw("dateadd('second', -2, now())"), + ); + } + async setStatus(runId: string, status: Status): Promise { let oldStatus: string; if (status === 'failed' || status === 'completed') { @@ -216,16 +228,17 @@ export class DatabaseTaskStore implements TaskStore { taskId, after, }: TaskStoreGetEventsOptions): Promise<{ events: DbTaskEventRow[] }> { - let query = this.db('task_events').where({ - task_id: taskId, - }); - if (typeof after === 'number') { - query = query - .where('task_events.id', '>', after) - .orWhere({ event_type: 'completion' }); - } + const rawEvents = await this.db('task_events') + .where({ + task_id: taskId, + }) + .andWhere(builder => { + if (typeof after === 'number') { + builder.where('id', '>', after).orWhere('event_type', 'completion'); + } + }) + .select(); - const rawEvents = await query.select(); const events = rawEvents.map(event => { try { const body = JSON.parse(event.body) as JsonObject; diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.test.ts index 5bbe91ea87..ebbac1db5f 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.test.ts @@ -14,47 +14,54 @@ * limitations under the License. */ -import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; -import { TemplaterValues } from '../stages'; -import { MemoryTaskStore } from './MemoryTaskStore'; +import { SingleConnectionDatabaseManager } from '@backstage/backend-common'; +import { ConfigReader } from '@backstage/config'; +import { DatabaseTaskStore } from './DatabaseTaskStore'; import { StorageTaskBroker, TaskAgent } from './StorageTaskBroker'; +import { TaskStore, TaskSpec, DbTaskEventRow } from './types'; + +async function createStore(): Promise { + const manager = SingleConnectionDatabaseManager.fromConfig( + new ConfigReader({ + backend: { + database: { + client: 'sqlite3', + connection: ':memory:', + }, + }, + }), + ).forPlugin('scaffolder'); + return await DatabaseTaskStore.create(await manager.getClient()); +} describe('StorageTaskBroker', () => { - const storage = new MemoryTaskStore(); - const broker = new StorageTaskBroker(storage); + let storage: TaskStore; - const taskSpec = { - values: {} as TemplaterValues, - template: {} as TemplateEntityV1alpha1, - }; + beforeAll(async () => { + storage = await createStore(); + }); it('should claim a dispatched work item', async () => { - await broker.dispatch(taskSpec); + const broker = new StorageTaskBroker(storage); + await broker.dispatch({ steps: [] }); await expect(broker.claim()).resolves.toEqual(expect.any(TaskAgent)); }); it('should wait for a dispatched work item', async () => { + const broker = new StorageTaskBroker(storage); const promise = broker.claim(); await expect(Promise.race([promise, 'waiting'])).resolves.toBe('waiting'); - await broker.dispatch(taskSpec); + await broker.dispatch({ steps: [] }); await expect(promise).resolves.toEqual(expect.any(TaskAgent)); }); it('should dispatch multiple items and claim them in order', async () => { - await broker.dispatch({ - values: { owner: 'a' } as TemplaterValues, - template: {} as TemplateEntityV1alpha1, - }); - await broker.dispatch({ - values: { owner: 'b' } as TemplaterValues, - template: {} as TemplateEntityV1alpha1, - }); - await broker.dispatch({ - values: { owner: 'c' } as TemplaterValues, - template: {} as TemplateEntityV1alpha1, - }); + const broker = new StorageTaskBroker(storage); + await broker.dispatch({ steps: [{ id: 'a' }] } as TaskSpec); + await broker.dispatch({ steps: [{ id: 'b' }] } as TaskSpec); + await broker.dispatch({ steps: [{ id: 'c' }] } as TaskSpec); const taskA = await broker.claim(); const taskB = await broker.claim(); @@ -62,13 +69,14 @@ describe('StorageTaskBroker', () => { await expect(taskA).toEqual(expect.any(TaskAgent)); await expect(taskB).toEqual(expect.any(TaskAgent)); await expect(taskC).toEqual(expect.any(TaskAgent)); - await expect(taskA.spec.values.owner).toBe('a'); - await expect(taskB.spec.values.owner).toBe('b'); - await expect(taskC.spec.values.owner).toBe('c'); + await expect(taskA.spec.steps[0].id).toBe('a'); + await expect(taskB.spec.steps[0].id).toBe('b'); + await expect(taskC.spec.steps[0].id).toBe('c'); }); it('should complete a task', async () => { - const dispatchResult = await broker.dispatch(taskSpec); + const broker = new StorageTaskBroker(storage); + const dispatchResult = await broker.dispatch({ steps: [] }); const task = await broker.claim(); await task.complete('completed'); const taskRow = await storage.get(dispatchResult.taskId); @@ -76,10 +84,91 @@ describe('StorageTaskBroker', () => { }); it('should fail a task', async () => { - const dispatchResult = await broker.dispatch(taskSpec); + const broker = new StorageTaskBroker(storage); + const dispatchResult = await broker.dispatch({ steps: [] }); const task = await broker.claim(); await task.complete('failed'); const taskRow = await storage.get(dispatchResult.taskId); expect(taskRow.status).toBe('failed'); }); + + it('multiple brokers should be able to observe a single task', async () => { + const broker1 = new StorageTaskBroker(storage); + const broker2 = new StorageTaskBroker(storage); + + const { taskId } = await broker1.dispatch({ steps: [] }); + + const logPromise = new Promise(resolve => { + const observedEvents = new Array(); + + broker2.observe({ taskId, after: undefined }, (_err, { events }) => { + observedEvents.push(...events); + if (events.some(e => e.type === 'completion')) { + resolve(observedEvents); + } + }); + }); + const task = await broker1.claim(); + await task.emitLog('log 1'); + await task.emitLog('log 2'); + await task.emitLog('log 3'); + await task.complete('completed'); + + const logs = await logPromise; + expect(logs.map(l => l.body.message)).toEqual([ + 'log 1', + 'log 2', + 'log 3', + 'Run completed with status: completed', + ]); + + const afterLogs = await new Promise(resolve => { + broker2.observe({ taskId, after: logs[1].id }, (_err, { events }) => + resolve(events.map(e => e.body.message as string)), + ); + }); + expect(afterLogs).toEqual([ + 'log 3', + 'Run completed with status: completed', + ]); + }); + + it('should heartbeat', async () => { + const broker = new StorageTaskBroker(storage); + const { taskId } = await broker.dispatch({ steps: [] }); + const task = await broker.claim(); + + const initialTask = await storage.get(taskId); + + for (;;) { + const maybeTask = await storage.get(taskId); + if (maybeTask.lastHeartbeat !== initialTask.lastHeartbeat) { + break; + } + await new Promise(resolve => setTimeout(resolve, 50)); + } + await task.complete('completed'); + expect.assertions(0); + }); + + it('should be cancelled if heartbeat stops', async () => { + const broker = new StorageTaskBroker(storage); + const { taskId } = await broker.dispatch({ steps: [] }); + console.log('DEBUG: taskId =', taskId); + const task = await broker.claim(); + clearInterval((task as any).heartbeatInterval); + + setTimeout(() => { + storage.listStaleTasks(); + }, 4000); + + for (;;) { + const maybeTask = await storage.get(taskId); + if (maybeTask.status === 'cancelled') { + break; + } + await new Promise(resolve => setTimeout(resolve, 50)); + } + expect.assertions(0); + }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts index 6eedc023f8..8c0d63ee71 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts @@ -25,7 +25,7 @@ import { } from './types'; export class TaskAgent implements Task { - private heartbeartInterval?: ReturnType; + private heartbeatInterval?: ReturnType; static create(state: TaskState, storage: TaskStore) { const agent = new TaskAgent(state, storage); @@ -67,13 +67,13 @@ export class TaskAgent implements Task { body: { message: `Run completed with status: ${result}` }, type: 'completion', }); - if (this.heartbeartInterval) { - clearInterval(this.heartbeartInterval); + if (this.heartbeatInterval) { + clearInterval(this.heartbeatInterval); } } private start() { - this.heartbeartInterval = setInterval(() => { + this.heartbeatInterval = setInterval(() => { if (!this.state.runId) { throw new Error('no run id provided'); } @@ -131,7 +131,10 @@ export class StorageTaskBroker implements TaskBroker { taskId: string; after: number | undefined; }, - callback: (result: { events: DbTaskEventRow[] }) => void, + callback: ( + error: Error | undefined, + result: { events: DbTaskEventRow[] }, + ) => void, ): () => void { const { taskId } = options; @@ -148,9 +151,9 @@ export class StorageTaskBroker implements TaskBroker { if (events.length) { after = events[events.length - 1].id; try { - callback(result); + callback(undefined, result); } catch (error) { - console.log('DEBUG: error =', error); + callback(error, { events: [] }); } } diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts index f9081af6b6..dd9a1ad9ad 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts @@ -86,6 +86,7 @@ export interface TaskStore { createTask(task: TaskSpec): Promise<{ taskId: string }>; claimTask(): Promise; heartbeat(runId: string): Promise; + listStaleTasks(): Promise<{ tasks: DbTaskRow }>; setStatus(runId: string, status: Status): Promise; emit({ taskId, runId, body, type }: TaskStoreEmitOptions): Promise; getEvents({ diff --git a/plugins/scaffolder-backend/src/service/router.test.ts b/plugins/scaffolder-backend/src/service/router.test.ts index 31f62febd2..8a0fa6cba6 100644 --- a/plugins/scaffolder-backend/src/service/router.test.ts +++ b/plugins/scaffolder-backend/src/service/router.test.ts @@ -16,6 +16,7 @@ const mockAccess = jest.fn(); jest.doMock('fs-extra', () => ({ + access: mockAccess, promises: { access: mockAccess, }, @@ -27,7 +28,11 @@ jest.doMock('fs-extra', () => ({ remove: jest.fn(), })); -import { getVoidLogger } from '@backstage/backend-common'; +import { + SingleConnectionDatabaseManager, + PluginDatabaseManager, + getVoidLogger, +} from '@backstage/backend-common'; import { ConfigReader } from '@backstage/config'; import express from 'express'; import request from 'supertest'; @@ -41,6 +46,19 @@ const generateEntityClient: any = (template: any) => ({ findTemplate: () => Promise.resolve(template), }); +function createDatabase(): PluginDatabaseManager { + return SingleConnectionDatabaseManager.fromConfig( + new ConfigReader({ + backend: { + database: { + client: 'sqlite3', + connection: ':memory:', + }, + }, + }), + ).forPlugin('scaffolder'); +} + describe('createRouter - working directory', () => { const mockPrepare = jest.fn(); const mockPreparers = new Preparers(); @@ -78,7 +96,6 @@ describe('createRouter - working directory', () => { }; const mockedEntityClient = generateEntityClient(template); - it('should throw an error when working directory does not exist or is not writable', async () => { mockAccess.mockImplementation(() => { throw new Error('access error'); @@ -93,6 +110,7 @@ describe('createRouter - working directory', () => { config: new ConfigReader(workDirConfig('/path')), dockerClient: new Docker(), entityClient: mockedEntityClient, + database: createDatabase(), }), ).rejects.toThrow('access error'); }); @@ -106,6 +124,7 @@ describe('createRouter - working directory', () => { config: new ConfigReader(workDirConfig('/path')), dockerClient: new Docker(), entityClient: mockedEntityClient, + database: createDatabase(), }); const app = express().use(router); @@ -134,6 +153,7 @@ describe('createRouter - working directory', () => { config: new ConfigReader({}), dockerClient: new Docker(), entityClient: mockedEntityClient, + database: createDatabase(), }); const app = express().use(router); @@ -203,6 +223,7 @@ describe('createRouter', () => { config: new ConfigReader({}), dockerClient: new Docker(), entityClient: generateEntityClient(template), + database: createDatabase(), }); app = express().use(router); }); diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 23af538df8..6eb112fa3c 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -164,7 +164,11 @@ export async function createRouter( // After client opens connection send all nests as string const unsubscribe = taskBroker.observe( { taskId, after }, - ({ events }) => { + (error, { events }) => { + logger.error( + `Received error from event stream when observing task ${taskId}`, + error, + ); for (const event of events) { res.write(`event:${JSON.stringify(event)}\n\n`); if (event.type === 'completion') { From 6608f9f5ee4cab89e5f658924db7b3ed975ff712 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Tue, 2 Feb 2021 16:10:13 +0100 Subject: [PATCH 22/37] Drop runId, update TaskStore interface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Co-authored-by: blam Co-authored-by: Patrik Oldsberg --- .../migrations/20210120143715_init.js | 17 +--- .../src/scaffolder/tasks/DatabaseTaskStore.ts | 85 +++++++++--------- .../tasks/StorageTaskBroker.test.ts | 57 ++++++------ .../src/scaffolder/tasks/StorageTaskBroker.ts | 86 +++++++++++++------ .../src/scaffolder/tasks/TaskWorker.ts | 1 - .../src/scaffolder/tasks/types.ts | 39 ++++++--- .../scaffolder-backend/src/service/router.ts | 4 +- 7 files changed, 168 insertions(+), 121 deletions(-) diff --git a/plugins/scaffolder-backend/migrations/20210120143715_init.js b/plugins/scaffolder-backend/migrations/20210120143715_init.js index bb65e1e2fb..248b4f932f 100644 --- a/plugins/scaffolder-backend/migrations/20210120143715_init.js +++ b/plugins/scaffolder-backend/migrations/20210120143715_init.js @@ -31,10 +31,6 @@ exports.up = async function up(knex) { .text('status') .notNullable() .comment('The current status of the task'); - table - .integer('run_id') - .nullable() - .comment('The current run ID of the task'); table .dateTime('created_at') .defaultTo(knex.fn.now()) @@ -44,11 +40,6 @@ exports.up = async function up(knex) { .dateTime('last_heartbeat_at') .nullable() .comment('The last timestamp when a heartbeat was received'); - table - .integer('retry_count') - .notNullable() - .defaultTo(0) - .comment('The number of times that this task has been attempted'); }); await knex.schema.createTable('task_events', table => { @@ -65,17 +56,13 @@ exports.up = async function up(knex) { .notNullable() .onDelete('CASCADE') .comment('The task that generated the event'); - table - .integer('run_id') - .nullable() - .comment('The run ID of the task that this event applies to'); table .text('body') .notNullable() .comment('The JSON encoded body of the event'); table.text('event_type').notNullable().comment('The type of event'); table - .dateTime('created_at') + .timestamp('created_at', { precision: 9 }) .defaultTo(knex.fn.now()) .notNullable() .comment('The timestamp when this event was generated'); @@ -90,7 +77,7 @@ exports.up = async function up(knex) { exports.down = async function down(knex) { if (knex.client.config.client !== 'sqlite3') { await knex.schema.alterTable('task_events', table => { - table.dropIndex([], 'task_events_task_id_idx'); + table.dropIndex([], 'ctask_events_task_id_idx'); }); } await knex.schema.dropTable('task_events'); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts index 1d99cd2958..cf1bc18621 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts @@ -43,14 +43,11 @@ export type RawDbTaskRow = { spec: string; status: Status; last_heartbeat_at?: string; - retry_count: number; created_at: string; - run_id?: string; }; export type RawDbTaskEventRow = { id: number; - run_id: string; task_id: string; body: string; event_type: TaskEventType; @@ -80,10 +77,8 @@ export class DatabaseTaskStore implements TaskStore { id: result.id, spec, status: result.status, - lastHeartbeat: result.last_heartbeat_at, - retryCount: result.retry_count, + lastHeartbeatAt: result.last_heartbeat_at, createdAt: result.created_at, - runId: result.run_id, }; } catch (error) { throw new Error(`Failed to parse spec of task '${taskId}', ${error}`); @@ -96,7 +91,6 @@ export class DatabaseTaskStore implements TaskStore { id: taskId, spec: JSON.stringify(spec), status: 'open', - retry_count: 0, }); return { taskId }; } @@ -114,12 +108,10 @@ export class DatabaseTaskStore implements TaskStore { return undefined; } - const runId = uuid(); const updateCount = await tx('tasks') .where({ id: task.id, status: 'open' }) .update({ status: 'processing', - run_id: runId, last_heartbeat_at: this.db.fn.now(), }); @@ -133,10 +125,8 @@ export class DatabaseTaskStore implements TaskStore { id: task.id, spec, status: 'processing', - lastHeartbeat: task.last_heartbeat_at, - retryCount: task.retry_count, + lastHeartbeatAt: task.last_heartbeat_at, createdAt: task.created_at, - runId: runId, }; } catch (error) { throw new Error(`Failed to parse spec of task '${task.id}', ${error}`); @@ -144,58 +134,76 @@ export class DatabaseTaskStore implements TaskStore { }); } - async heartbeat(runId: string): Promise { + async heartbeatTask(taskId: string): Promise { const updateCount = await this.db('tasks') - .where({ run_id: runId, status: 'processing' }) + .where({ id: taskId, status: 'processing' }) .update({ last_heartbeat_at: this.db.fn.now(), }); if (updateCount === 0) { - throw new Error(`No running task with runId ${runId} found`); + throw new Error(`No running task with taskId ${taskId} found`); } } - async listStaleTasks(): Promise<{ tasks: DbTaskRow }> { - const rows = await this.db('tasks') + async listStaleTasks({ + timeoutS, + }: { + timeoutS: number; + }): Promise<{ + tasks: { taskId: string }[]; + }> { + const rawRows = await this.db('tasks') .where('status', 'processing') .andWhere( 'last_heartbeat_at', - '<', - this.db.type === 'sqlite' - ? this.db.raw("datetime('now', '-2 seconds')") - : this.db.raw("dateadd('second', -2, now())"), + '<=', + this.db.client.config.client === 'sqlite3' + ? this.db.raw(`datetime('now', '-${Number(timeoutS)} seconds')`) + : this.db.raw(`dateadd('second', -${Number(timeoutS)}, now())`), ); + const tasks = rawRows.map(row => ({ + taskId: row.id, + })); + return { tasks }; } - async setStatus(runId: string, status: Status): Promise { + async completeTask({ + taskId, + status, + eventBody, + }: { + taskId: string; + status: Status; + eventBody: JsonObject; + }): Promise { let oldStatus: string; if (status === 'failed' || status === 'completed') { oldStatus = 'processing'; } else { throw new Error( - `Invalid status update of run '${runId}' to status '${status}'`, + `Invalid status update of run '${taskId}' to status '${status}'`, ); } await this.db.transaction(async tx => { const [task] = await tx('tasks') .where({ - run_id: runId, + id: taskId, }) .limit(1) .select(); if (!task) { - throw new Error(`No task with runId ${runId} found`); + throw new Error(`No task with taskId ${taskId} found`); } if (task.status !== oldStatus) { throw new ConflictError( - `Refusing to update status of run '${runId}' to status '${status}' ` + + `Refusing to update status of run '${taskId}' to status '${status}' ` + `as it is currently '${task.status}', expected '${oldStatus}'`, ); } const updateCount = await tx('tasks') .where({ - run_id: runId, + id: taskId, status: oldStatus, }) .update({ @@ -203,28 +211,28 @@ export class DatabaseTaskStore implements TaskStore { }); if (updateCount !== 1) { throw new Error( - `Failed to update status to '${status}' for runId ${runId}`, + `Failed to update status to '${status}' for taskId ${taskId}`, ); } + + await tx('task_events').insert({ + task_id: taskId, + event_type: 'completion', + body: JSON.stringify(eventBody), + }); }); } - async emit({ - taskId, - runId, - body, - type, - }: TaskStoreEmitOptions): Promise { + async emitLogEvent({ taskId, body }: TaskStoreEmitOptions): Promise { const serliazedBody = JSON.stringify(body); await this.db('task_events').insert({ task_id: taskId, - run_id: runId, - event_type: type, + event_type: 'log', body: serliazedBody, }); } - async getEvents({ + async listEvents({ taskId, after, }: TaskStoreGetEventsOptions): Promise<{ events: DbTaskEventRow[] }> { @@ -244,8 +252,7 @@ export class DatabaseTaskStore implements TaskStore { const body = JSON.parse(event.body) as JsonObject; return { id: event.id, - runId: event.run_id, - taskId: event.task_id, + taskId, body, type: event.event_type, createdAt: event.created_at, diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.test.ts index ebbac1db5f..c45fa46335 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.test.ts @@ -14,13 +14,16 @@ * limitations under the License. */ -import { SingleConnectionDatabaseManager } from '@backstage/backend-common'; +import { + getVoidLogger, + SingleConnectionDatabaseManager, +} from '@backstage/backend-common'; import { ConfigReader } from '@backstage/config'; import { DatabaseTaskStore } from './DatabaseTaskStore'; import { StorageTaskBroker, TaskAgent } from './StorageTaskBroker'; -import { TaskStore, TaskSpec, DbTaskEventRow } from './types'; +import { TaskSpec, DbTaskEventRow } from './types'; -async function createStore(): Promise { +async function createStore(): Promise { const manager = SingleConnectionDatabaseManager.fromConfig( new ConfigReader({ backend: { @@ -35,20 +38,21 @@ async function createStore(): Promise { } describe('StorageTaskBroker', () => { - let storage: TaskStore; + let storage: DatabaseTaskStore; beforeAll(async () => { storage = await createStore(); }); + const logger = getVoidLogger(); it('should claim a dispatched work item', async () => { - const broker = new StorageTaskBroker(storage); + const broker = new StorageTaskBroker(storage, logger); await broker.dispatch({ steps: [] }); await expect(broker.claim()).resolves.toEqual(expect.any(TaskAgent)); }); it('should wait for a dispatched work item', async () => { - const broker = new StorageTaskBroker(storage); + const broker = new StorageTaskBroker(storage, logger); const promise = broker.claim(); await expect(Promise.race([promise, 'waiting'])).resolves.toBe('waiting'); @@ -58,7 +62,7 @@ describe('StorageTaskBroker', () => { }); it('should dispatch multiple items and claim them in order', async () => { - const broker = new StorageTaskBroker(storage); + const broker = new StorageTaskBroker(storage, logger); await broker.dispatch({ steps: [{ id: 'a' }] } as TaskSpec); await broker.dispatch({ steps: [{ id: 'b' }] } as TaskSpec); await broker.dispatch({ steps: [{ id: 'c' }] } as TaskSpec); @@ -75,16 +79,16 @@ describe('StorageTaskBroker', () => { }); it('should complete a task', async () => { - const broker = new StorageTaskBroker(storage); + const broker = new StorageTaskBroker(storage, logger); const dispatchResult = await broker.dispatch({ steps: [] }); const task = await broker.claim(); await task.complete('completed'); const taskRow = await storage.get(dispatchResult.taskId); expect(taskRow.status).toBe('completed'); - }); + }, 10000); it('should fail a task', async () => { - const broker = new StorageTaskBroker(storage); + const broker = new StorageTaskBroker(storage, logger); const dispatchResult = await broker.dispatch({ steps: [] }); const task = await broker.claim(); await task.complete('failed'); @@ -93,8 +97,8 @@ describe('StorageTaskBroker', () => { }); it('multiple brokers should be able to observe a single task', async () => { - const broker1 = new StorageTaskBroker(storage); - const broker2 = new StorageTaskBroker(storage); + const broker1 = new StorageTaskBroker(storage, logger); + const broker2 = new StorageTaskBroker(storage, logger); const { taskId } = await broker1.dispatch({ steps: [] }); @@ -115,7 +119,7 @@ describe('StorageTaskBroker', () => { await task.complete('completed'); const logs = await logPromise; - expect(logs.map(l => l.body.message)).toEqual([ + expect(logs.map(l => l.body.message, logger)).toEqual([ 'log 1', 'log 2', 'log 3', @@ -134,7 +138,7 @@ describe('StorageTaskBroker', () => { }); it('should heartbeat', async () => { - const broker = new StorageTaskBroker(storage); + const broker = new StorageTaskBroker(storage, logger); const { taskId } = await broker.dispatch({ steps: [] }); const task = await broker.claim(); @@ -142,7 +146,7 @@ describe('StorageTaskBroker', () => { for (;;) { const maybeTask = await storage.get(taskId); - if (maybeTask.lastHeartbeat !== initialTask.lastHeartbeat) { + if (maybeTask.lastHeartbeatAt !== initialTask.lastHeartbeatAt) { break; } await new Promise(resolve => setTimeout(resolve, 50)); @@ -151,24 +155,29 @@ describe('StorageTaskBroker', () => { expect.assertions(0); }); - it('should be cancelled if heartbeat stops', async () => { - const broker = new StorageTaskBroker(storage); + it('should be update the status to failed if heartbeat fails', async () => { + const broker = new StorageTaskBroker(storage, logger); const { taskId } = await broker.dispatch({ steps: [] }); - console.log('DEBUG: taskId =', taskId); const task = await broker.claim(); - clearInterval((task as any).heartbeatInterval); - setTimeout(() => { - storage.listStaleTasks(); - }, 4000); + jest + .spyOn((task as any).storage, 'heartbeatTask') + .mockRejectedValue(new Error('nah m8')); + + const intervalId = setInterval(() => { + broker.vacuumTasks({ timeoutS: 2 }).catch(fail); + }, 500); for (;;) { const maybeTask = await storage.get(taskId); - if (maybeTask.status === 'cancelled') { + if (maybeTask.status === 'failed') { break; } await new Promise(resolve => setTimeout(resolve, 50)); } - expect.assertions(0); + + clearInterval(intervalId); + + expect(task.done).toBe(true); }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts index 8c0d63ee71..48c0d5e5ba 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - +import { Logger } from 'winston'; import { CompletedTaskState, Task, @@ -25,11 +25,13 @@ import { } from './types'; export class TaskAgent implements Task { - private heartbeatInterval?: ReturnType; + private isDone = false; - static create(state: TaskState, storage: TaskStore) { - const agent = new TaskAgent(state, storage); - agent.start(); + private heartbeatTimeoutId?: ReturnType; + + static create(state: TaskState, storage: TaskStore, logger: Logger) { + const agent = new TaskAgent(state, storage, logger); + agent.startTimeout(); return agent; } @@ -37,6 +39,7 @@ export class TaskAgent implements Task { private constructor( private readonly state: TaskState, private readonly storage: TaskStore, + private readonly logger: Logger, ) {} get spec() { @@ -44,40 +47,45 @@ export class TaskAgent implements Task { } async getWorkspaceName() { - return `${this.state.taskId}_${this.state.runId}`; + return this.state.taskId; + } + + get done() { + return this.isDone; } async emitLog(message: string): Promise { - await this.storage.emit({ + await this.storage.emitLogEvent({ taskId: this.state.taskId, - runId: this.state.runId, body: { message }, - type: 'log', }); } async complete(result: CompletedTaskState): Promise { - await this.storage.setStatus( - this.state.runId, - result === 'failed' ? 'failed' : 'completed', - ); - this.storage.emit({ + await this.storage.completeTask({ taskId: this.state.taskId, - runId: this.state.runId, - body: { message: `Run completed with status: ${result}` }, - type: 'completion', + status: result === 'failed' ? 'failed' : 'completed', + eventBody: { message: `Run completed with status: ${result}` }, }); - if (this.heartbeatInterval) { - clearInterval(this.heartbeatInterval); + this.isDone = true; + if (this.heartbeatTimeoutId) { + clearTimeout(this.heartbeatTimeoutId); } } - private start() { - this.heartbeatInterval = setInterval(() => { - if (!this.state.runId) { - throw new Error('no run id provided'); + private startTimeout() { + this.heartbeatTimeoutId = setTimeout(async () => { + try { + await this.storage.heartbeatTask(this.state.taskId); + this.startTimeout(); + } catch (error) { + this.isDone = true; + + this.logger.error( + `Heartbeat for task ${this.state.taskId} failed`, + error, + ); } - this.storage.heartbeat(this.state.runId); }, 1000); } } @@ -85,7 +93,6 @@ export class TaskAgent implements Task { interface TaskState { spec: TaskSpec; taskId: string; - runId: string; } function defer() { @@ -97,7 +104,10 @@ function defer() { } export class StorageTaskBroker implements TaskBroker { - constructor(private readonly storage: TaskStore) {} + constructor( + private readonly storage: TaskStore, + private readonly logger: Logger, + ) {} private deferredDispatch = defer(); async claim(): Promise { @@ -106,11 +116,11 @@ export class StorageTaskBroker implements TaskBroker { if (pendingTask) { return TaskAgent.create( { - runId: pendingTask.runId!, taskId: pendingTask.id, spec: pendingTask.spec, }, this.storage, + this.logger, ); } @@ -146,7 +156,7 @@ export class StorageTaskBroker implements TaskBroker { (async () => { let after = options.after; while (!cancelled) { - const result = await this.storage.getEvents({ taskId, after: after }); + const result = await this.storage.listEvents({ taskId, after: after }); const { events } = result; if (events.length) { after = events[events.length - 1].id; @@ -164,6 +174,26 @@ export class StorageTaskBroker implements TaskBroker { return unsubscribe; } + async vacuumTasks(timeoutS: { timeoutS: number }): Promise { + const { tasks } = await this.storage.listStaleTasks(timeoutS); + await Promise.all( + tasks.map(async task => { + try { + await this.storage.completeTask({ + taskId: task.taskId, + status: 'failed', + eventBody: { + message: + 'The task was cancelled because the task worker lost connection to the task broker', + }, + }); + } catch (error) { + this.logger.warn(`Failed to cancel task '${task.taskId}', ${error}`); + } + }), + ); + } + private waitForDispatch() { return this.deferredDispatch.promise; } diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts index 64cfdc6f96..f87d2df934 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts @@ -73,7 +73,6 @@ export class TaskWorker { // Give us some time to curl observe task.emitLog('Task claimed, waiting ...'); await new Promise(resolve => setTimeout(resolve, 5000)); - console.log('DEBUG: task.spec =', JSON.stringify(task.spec, null, 2)); task.emitLog(`Starting up work with ${task.spec.steps.length} steps`); const outputs: { [name: string]: JsonValue } = {}; diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts index dd9a1ad9ad..0c2592109b 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts @@ -29,16 +29,13 @@ export type DbTaskRow = { id: string; spec: TaskSpec; status: Status; - lastHeartbeat?: string; - retryCount: number; createdAt: string; - runId?: string; + lastHeartbeatAt?: string; }; export type TaskEventType = 'completion' | 'log'; export type DbTaskEventRow = { id: number; - runId: string; taskId: string; body: JsonObject; type: TaskEventType; @@ -60,6 +57,7 @@ export type DispatchResult = { export interface Task { spec: TaskSpec; + done: boolean; emitLog(message: string): Promise; complete(result: CompletedTaskState): Promise; getWorkspaceName(): Promise; @@ -68,13 +66,22 @@ export interface Task { export interface TaskBroker { claim(): Promise; dispatch(spec: TaskSpec): Promise; + vacuumTasks(timeoutS: { timeoutS: number }): Promise; + observe( + options: { + taskId: string; + after: number | undefined; + }, + callback: ( + error: Error | undefined, + result: { events: DbTaskEventRow[] }, + ) => void, + ): () => void; } export type TaskStoreEmitOptions = { taskId: string; - runId: string; body: JsonObject; - type: TaskEventType; }; export type TaskStoreGetEventsOptions = { @@ -82,14 +89,22 @@ export type TaskStoreGetEventsOptions = { after?: number | undefined; }; export interface TaskStore { - get(taskId: string): Promise; createTask(task: TaskSpec): Promise<{ taskId: string }>; claimTask(): Promise; - heartbeat(runId: string): Promise; - listStaleTasks(): Promise<{ tasks: DbTaskRow }>; - setStatus(runId: string, status: Status): Promise; - emit({ taskId, runId, body, type }: TaskStoreEmitOptions): Promise; - getEvents({ + completeTask(options: { + taskId: string; + status: Status; + eventBody: JsonObject; + }): Promise; + heartbeatTask(taskId: string): Promise; + listStaleTasks(options: { + timeoutS: number; + }): Promise<{ + tasks: { taskId: string }[]; + }>; + + emitLogEvent({ taskId, body }: TaskStoreEmitOptions): Promise; + listEvents({ taskId, after, }: TaskStoreGetEventsOptions): Promise<{ events: DbTaskEventRow[] }>; diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 6eb112fa3c..8da6f02507 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -82,7 +82,7 @@ export async function createRouter( const databaseTaskStore = await DatabaseTaskStore.create( await database.getClient(), ); - const taskBroker = new StorageTaskBroker(databaseTaskStore); + const taskBroker = new StorageTaskBroker(databaseTaskStore, logger); const actionRegistry = new TemplateActionRegistry(); const worker = new TaskWorker({ logger, @@ -124,7 +124,7 @@ export async function createRouter( error: job.error, }); }) - // curl -X POST -d '{"templateName":"springboot-template","values": {"storePath":"https://github.com/jhaals/foo", "component_id":"woop", "description": "apa", "owner": "me" }}' -H 'Content-Type: application/json' localhost:7000/api/scaffolder/v2/tasks + .post('/v2/tasks', async (req, res) => { const templateName: string = req.body.templateName; const values: TemplaterValues = { From 9b0bba5439c7fe7383f897756b6394d807a62393 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Tue, 2 Feb 2021 16:25:45 +0100 Subject: [PATCH 23/37] Reshuffle api endpoints --- .../scaffolder-backend/src/service/router.ts | 124 +++++++++--------- 1 file changed, 62 insertions(+), 62 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 8da6f02507..16cefa5297 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -124,68 +124,6 @@ export async function createRouter( error: job.error, }); }) - - .post('/v2/tasks', async (req, res) => { - const templateName: string = req.body.templateName; - const values: TemplaterValues = { - ...req.body.values, - destination: { - git: parseGitUrl(req.body.values.storePath), - }, - }; - const template = await entityClient.findTemplate(templateName); - - const validationResult: ValidatorResult = validate( - values, - template.spec.schema, - ); - - if (!validationResult.valid) { - res.status(400).json({ errors: validationResult.errors }); - return; - } - const taskSpec = templateEntityToSpec(template, values); - const result = await taskBroker.dispatch(taskSpec); - - res.status(201).json({ id: result.taskId }); - }) - - .get('/v2/tasks/:taskId/eventstream', async (req, res) => { - const { taskId } = req.params; - const after = Number(req.query.after) || undefined; - logger.info('event stream opened'); - - // Mandatory headers and http status to keep connection open - res.writeHead(200, { - Connection: 'keep-alive', - 'Cache-Control': 'no-cache', - 'Content-Type': 'text/event-stream', - }); - // After client opens connection send all nests as string - const unsubscribe = taskBroker.observe( - { taskId, after }, - (error, { events }) => { - logger.error( - `Received error from event stream when observing task ${taskId}`, - error, - ); - for (const event of events) { - res.write(`event:${JSON.stringify(event)}\n\n`); - if (event.type === 'completion') { - unsubscribe(); - res.end(); - } - } - }, - ); - // When client closes connection we update the clients list - // avoiding the disconnected one - req.on('close', () => { - unsubscribe(); - logger.info('event stream closed'); - }); - }) - .post('/v1/jobs', async (req, res) => { const templateName: string = req.body.templateName; const values: TemplaterValues = { @@ -282,6 +220,68 @@ export async function createRouter( res.status(201).json({ id: job.id }); }); + // NOTE: The v2 API is unstable + router + .post('/v2/tasks', async (req, res) => { + const templateName: string = req.body.templateName; + const values: TemplaterValues = { + ...req.body.values, + destination: { + git: parseGitUrl(req.body.values.storePath), + }, + }; + const template = await entityClient.findTemplate(templateName); + + const validationResult: ValidatorResult = validate( + values, + template.spec.schema, + ); + + if (!validationResult.valid) { + res.status(400).json({ errors: validationResult.errors }); + return; + } + const taskSpec = templateEntityToSpec(template, values); + const result = await taskBroker.dispatch(taskSpec); + + res.status(201).json({ id: result.taskId }); + }) + .get('/v2/tasks/:taskId/eventstream', async (req, res) => { + const { taskId } = req.params; + const after = Number(req.query.after) || undefined; + logger.info('event stream opened'); + + // Mandatory headers and http status to keep connection open + res.writeHead(200, { + Connection: 'keep-alive', + 'Cache-Control': 'no-cache', + 'Content-Type': 'text/event-stream', + }); + // After client opens connection send all nests as string + const unsubscribe = taskBroker.observe( + { taskId, after }, + (error, { events }) => { + logger.error( + `Received error from event stream when observing task ${taskId}`, + error, + ); + for (const event of events) { + res.write(`event:${JSON.stringify(event)}\n\n`); + if (event.type === 'completion') { + unsubscribe(); + res.end(); + } + } + }, + ); + // When client closes connection we update the clients list + // avoiding the disconnected one + req.on('close', () => { + unsubscribe(); + logger.info('event stream closed'); + }); + }); + const app = express(); app.set('logger', logger); app.use('/', router); From 615103a631993ec21745e6b3d93652f74c48cd4d Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Tue, 2 Feb 2021 16:33:00 +0100 Subject: [PATCH 24/37] Add changesets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Co-authored-by: blam Co-authored-by: Patrik Oldsberg --- .changeset/afraid-teachers-cross.md | 29 +++++++++++++++++++++++++++++ .changeset/large-terms-invite.md | 7 +++++++ 2 files changed, 36 insertions(+) create mode 100644 .changeset/afraid-teachers-cross.md create mode 100644 .changeset/large-terms-invite.md diff --git a/.changeset/afraid-teachers-cross.md b/.changeset/afraid-teachers-cross.md new file mode 100644 index 0000000000..8fc376441f --- /dev/null +++ b/.changeset/afraid-teachers-cross.md @@ -0,0 +1,29 @@ +--- +'@backstage/create-app': patch +--- + +Pass on plugin database management instance that is now required by the scaffolder plugin. + +To apply this change to an existing application, add the following to `src/plugins/scaffolder.ts`: + +```diff +export default async function createPlugin({ + logger, + config, ++ database, +}: PluginEnvironment) { + +// ...omitted... + + return await createRouter({ + preparers, + templaters, + publishers, + logger, + config, + dockerClient, + entityClient, ++ database, + }); +} +``` diff --git a/.changeset/large-terms-invite.md b/.changeset/large-terms-invite.md new file mode 100644 index 0000000000..6653ac8e1e --- /dev/null +++ b/.changeset/large-terms-invite.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-scaffolder-backend': minor +--- + +Introduced `v2` Scaffolder REST API, which uses an implementation that is database backed, making the scaffolder instances stateless. The `createRouter` function now requires a `PluginDatabaseManager` instance to be passed in, commonly available as `database` in the plugin environment in the backend. + +This API should be considered unstable until used by the scaffolder frontend. From a173d47527b0ceb960c3a2a815b59edf2ebb6e2e Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Tue, 2 Feb 2021 16:45:28 +0100 Subject: [PATCH 25/37] Delete knexfile --- plugins/scaffolder-backend/knexfile.js | 25 ------------------------- 1 file changed, 25 deletions(-) delete mode 100644 plugins/scaffolder-backend/knexfile.js diff --git a/plugins/scaffolder-backend/knexfile.js b/plugins/scaffolder-backend/knexfile.js deleted file mode 100644 index f469df4c08..0000000000 --- a/plugins/scaffolder-backend/knexfile.js +++ /dev/null @@ -1,25 +0,0 @@ -/* - * Copyright 2020 Spotify AB - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -module.exports = { - development: { - client: 'sqlite3', - connection: ':memory:', - migrations: { - directory: 'migrations', - }, - }, -}; From c386de896d65dbe05db2a1c23d0bc2a68a4aa864 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Tue, 2 Feb 2021 16:46:11 +0100 Subject: [PATCH 26/37] Remove unused dependencies --- plugins/scaffolder-backend/package.json | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/plugins/scaffolder-backend/package.json b/plugins/scaffolder-backend/package.json index bdecf6196e..9f45c218dc 100644 --- a/plugins/scaffolder-backend/package.json +++ b/plugins/scaffolder-backend/package.json @@ -55,11 +55,9 @@ "jsonschema": "^1.2.6", "knex": "^0.21.6", "morgan": "^1.10.0", - "p-queue": "^6.3.0", "uuid": "^8.2.0", "winston": "^3.2.1", - "yaml": "^1.10.0", - "luxon": "^1.25.0" + "yaml": "^1.10.0" }, "devDependencies": { "@backstage/cli": "^0.5.0", From 693ae8c6258d5cd923e89a981e8f2fae2a07eaff Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Wed, 3 Feb 2021 10:41:06 +0100 Subject: [PATCH 27/37] Remove date precision --- plugins/scaffolder-backend/migrations/20210120143715_init.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/scaffolder-backend/migrations/20210120143715_init.js b/plugins/scaffolder-backend/migrations/20210120143715_init.js index 248b4f932f..256ec7d423 100644 --- a/plugins/scaffolder-backend/migrations/20210120143715_init.js +++ b/plugins/scaffolder-backend/migrations/20210120143715_init.js @@ -62,7 +62,7 @@ exports.up = async function up(knex) { .comment('The JSON encoded body of the event'); table.text('event_type').notNullable().comment('The type of event'); table - .timestamp('created_at', { precision: 9 }) + .timestamp('created_at') .defaultTo(knex.fn.now()) .notNullable() .comment('The timestamp when this event was generated'); From 1955c62398793a3c385de98e8667fb9e2b0d3ffe Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Wed, 3 Feb 2021 10:47:44 +0100 Subject: [PATCH 28/37] Delete unused database code --- .../scaffolder-backend/src/tasks/Database.ts | 70 ------------------- plugins/scaffolder-backend/src/tasks/types.ts | 37 ---------- 2 files changed, 107 deletions(-) delete mode 100644 plugins/scaffolder-backend/src/tasks/Database.ts delete mode 100644 plugins/scaffolder-backend/src/tasks/types.ts diff --git a/plugins/scaffolder-backend/src/tasks/Database.ts b/plugins/scaffolder-backend/src/tasks/Database.ts deleted file mode 100644 index c135380f87..0000000000 --- a/plugins/scaffolder-backend/src/tasks/Database.ts +++ /dev/null @@ -1,70 +0,0 @@ -/* - * Copyright 2021 Spotify AB - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { ConflictError, resolvePackagePath } from '@backstage/backend-common'; -import Knex from 'knex'; -import { Logger } from 'winston'; -import { Database, Transaction } from './types'; - -const migrationsDir = resolvePackagePath( - '@backstage/plugin-scaffolder-backend', - 'migrations', -); - -export class CommonDatabase implements Database { - static async create(knex: Knex, logger: Logger): Promise { - await knex.migrate.latest({ - directory: migrationsDir, - }); - return new CommonDatabase(knex, logger); - } - - constructor( - private readonly database: Knex, - private readonly logger: Logger, - ) {} - - async transaction(fn: (tx: Transaction) => Promise): Promise { - try { - let result: T | undefined = undefined; - - await this.database.transaction( - async tx => { - // We can't return here, as knex swallows the return type in case the transaction is rolled back: - // https://github.com/knex/knex/blob/e37aeaa31c8ef9c1b07d2e4d3ec6607e557d800d/lib/transaction.js#L136 - result = await fn(tx); - }, - { - // If we explicitly trigger a rollback, don't fail. - doNotRejectOnRollback: true, - }, - ); - - return result!; - } catch (e) { - this.logger.debug(`Error during transaction, ${e}`); - - if ( - /SQLITE_CONSTRAINT: UNIQUE/.test(e.message) || - /unique constraint/.test(e.message) - ) { - throw new ConflictError(`Rejected due to a conflicting entity`, e); - } - - throw e; - } - } -} diff --git a/plugins/scaffolder-backend/src/tasks/types.ts b/plugins/scaffolder-backend/src/tasks/types.ts deleted file mode 100644 index 27cbe7f7f1..0000000000 --- a/plugins/scaffolder-backend/src/tasks/types.ts +++ /dev/null @@ -1,37 +0,0 @@ -/* - * Copyright 2021 Spotify AB - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -/** - * The core database implementation. - */ -export interface Database { - /** - * Runs a transaction. - * - * The callback is expected to make calls back into this class. When it - * completes, the transaction is closed. - * - * @param fn The callback that implements the transaction - */ - transaction(fn: (tx: Transaction) => Promise): Promise; -} - -/** - * An abstraction for transactions of the underlying database technology. - */ -export type Transaction = { - rollback(): Promise; -}; From e5520064eabd725c038c878220522bd1aa2b3db9 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Wed, 3 Feb 2021 10:48:34 +0100 Subject: [PATCH 29/37] Drop console log --- plugins/scaffolder-backend/src/scaffolder/stages/legacy.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/legacy.ts b/plugins/scaffolder-backend/src/scaffolder/stages/legacy.ts index a3fe01c96d..cb06f5a1e7 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/legacy.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/legacy.ts @@ -37,7 +37,6 @@ export function registerLegacyActions( id: 'legacy:prepare', async handler(ctx) { const { logger } = ctx; - console.log(ctx); logger.info('Task claimed, waiting ...'); // Give us some time to curl observe await new Promise(resolve => setTimeout(resolve, 1000)); From ff0eed8200e0a5886f8201bf0358a1e0ded3a9bf Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Wed, 3 Feb 2021 10:51:48 +0100 Subject: [PATCH 30/37] Remove sleep from prepare step --- plugins/scaffolder-backend/src/scaffolder/stages/legacy.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/legacy.ts b/plugins/scaffolder-backend/src/scaffolder/stages/legacy.ts index cb06f5a1e7..6daf1ba4f6 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/legacy.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/legacy.ts @@ -36,16 +36,11 @@ export function registerLegacyActions( registry.register({ id: 'legacy:prepare', async handler(ctx) { - const { logger } = ctx; - logger.info('Task claimed, waiting ...'); - // Give us some time to curl observe - await new Promise(resolve => setTimeout(resolve, 1000)); - - logger.info('Prepare the skeleton'); const { protocol, url } = ctx.parameters; const preparer = protocol === 'file' ? new FilePreparer() : preparers.get(url as string); + ctx.logger.info('Prepare the skeleton'); await preparer.prepare({ url: url as string, logger: ctx.logger, From 586607e417877aaa34d92845731c969aab0ac4c3 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Thu, 4 Feb 2021 09:39:05 +0100 Subject: [PATCH 31/37] Order events by id Co-authored-by: blam --- .../scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts index cf1bc18621..22697f5fdb 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts @@ -245,6 +245,7 @@ export class DatabaseTaskStore implements TaskStore { builder.where('id', '>', after).orWhere('event_type', 'completion'); } }) + .orderBy('id') .select(); const events = rawEvents.map(event => { From 6ca31fb834ea620e2c4de23d26efd076ffe6646c Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Thu, 4 Feb 2021 09:42:04 +0100 Subject: [PATCH 32/37] scaffolder: Flush events to eventstream Co-authored-by: blam --- .../scaffolder-backend/src/service/router.ts | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 16cefa5297..a8a61972af 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -249,7 +249,7 @@ export async function createRouter( .get('/v2/tasks/:taskId/eventstream', async (req, res) => { const { taskId } = req.params; const after = Number(req.query.after) || undefined; - logger.info('event stream opened'); + logger.debug(`Event stream observing taskId '${taskId}' opened`); // Mandatory headers and http status to keep connection open res.writeHead(200, { @@ -257,28 +257,35 @@ export async function createRouter( 'Cache-Control': 'no-cache', 'Content-Type': 'text/event-stream', }); + // After client opens connection send all nests as string const unsubscribe = taskBroker.observe( { taskId, after }, (error, { events }) => { - logger.error( - `Received error from event stream when observing task ${taskId}`, - error, - ); + if (error) { + logger.error( + `Received error from event stream when observing taskId '${taskId}', ${error}`, + ); + } + for (const event of events) { - res.write(`event:${JSON.stringify(event)}\n\n`); + res.write( + `event: ${event.type}\ndata: ${JSON.stringify(event)}\n\n`, + ); if (event.type === 'completion') { unsubscribe(); - res.end(); + // Closing the event stream here would cause the frontend + // to automatically reconnect because it lost connection. } } + res.flush(); }, ); // When client closes connection we update the clients list // avoiding the disconnected one req.on('close', () => { unsubscribe(); - logger.info('event stream closed'); + logger.debug(`Event stream observing taskId '${taskId}' closed`); }); }); From ba3a6a9be0128d1b952263187b0d5d3a59e0e12a Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Fri, 5 Feb 2021 13:52:20 +0100 Subject: [PATCH 33/37] Scaffolder: fix typos and log output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw --- .../src/scaffolder/tasks/TemplateConverter.ts | 4 ++-- plugins/scaffolder-backend/src/service/router.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TemplateConverter.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TemplateConverter.ts index 86f722e475..69788238cc 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TemplateConverter.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TemplateConverter.ts @@ -99,7 +99,7 @@ export class TemplateActionRegistry { register(action: TemplateAction) { if (this.actions.has(action.id)) { throw new ConflictError( - `Template action with id ${action.id} as already been registered`, + `Template action with ID '${action.id}' has already been registered`, ); } this.actions.set(action.id, action); @@ -109,7 +109,7 @@ export class TemplateActionRegistry { const action = this.actions.get(actionId); if (!action) { throw new NotFoundError( - `Template action with id ${actionId} is not registered.`, + `Template action with ID '${actionId}' is not registered.`, ); } return action; diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index a8a61972af..5821edfe8c 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -258,7 +258,7 @@ export async function createRouter( 'Content-Type': 'text/event-stream', }); - // After client opens connection send all nests as string + // After client opens connection send all events as string const unsubscribe = taskBroker.observe( { taskId, after }, (error, { events }) => { From 95cd312d55659c34b3b4f5ea39903b8db001fafb Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Fri, 5 Feb 2021 14:10:41 +0100 Subject: [PATCH 34/37] scaffolder: Make updateCount errors ConflictError --- .../src/scaffolder/tasks/DatabaseTaskStore.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts index 22697f5fdb..373a4486f1 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts @@ -141,7 +141,7 @@ export class DatabaseTaskStore implements TaskStore { last_heartbeat_at: this.db.fn.now(), }); if (updateCount === 0) { - throw new Error(`No running task with taskId ${taskId} found`); + throw new ConflictError(`No running task with taskId ${taskId} found`); } } @@ -210,7 +210,7 @@ export class DatabaseTaskStore implements TaskStore { status, }); if (updateCount !== 1) { - throw new Error( + throw new ConflictError( `Failed to update status to '${status}' for taskId ${taskId}`, ); } From b3b766b79b2efd7f9e0201627c5e7fe10ffb1daa Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Fri, 5 Feb 2021 14:34:00 +0100 Subject: [PATCH 35/37] Scaffolder: Remove sleep --- plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts index f87d2df934..c62495ca4c 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts @@ -101,9 +101,6 @@ export class TaskWorker { task.emitLog(`Finished step ${step.name}`); } - logger.info('So done right now'); - await new Promise(resolve => setTimeout(resolve, 5000)); - await task.complete('completed'); } catch (error) { task.emitLog(String(error.stack)); From 9afa2d24c63bad5e9b1f789c06bb5a6ae3f5bcfc Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Fri, 5 Feb 2021 15:25:34 +0100 Subject: [PATCH 36/37] scaffolder: Fix legacy publish error message --- plugins/scaffolder-backend/src/scaffolder/stages/legacy.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/legacy.ts b/plugins/scaffolder-backend/src/scaffolder/stages/legacy.ts index 6daf1ba4f6..f151780ba9 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/legacy.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/legacy.ts @@ -87,10 +87,9 @@ export function registerLegacyActions( } const owner = values.owner as unknown; if (typeof owner !== 'string') { - throw new Error( - `Invalid store path passed to publish, got ${typeof owner}`, - ); + throw new Error(`Invalid owner passed to publish, got ${typeof owner}`); } + const publisher = publishers.get(storePath); ctx.logger.info('Will now store the template'); const { remoteUrl, catalogInfoUrl } = await publisher.publish({ From 44da5d09c581e54f94ad989b9a411e3c35219de5 Mon Sep 17 00:00:00 2001 From: Johan Haals Date: Mon, 8 Feb 2021 09:24:48 +0100 Subject: [PATCH 37/37] scaffolder: Convert to prepared statements --- .../src/scaffolder/tasks/DatabaseTaskStore.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts index 373a4486f1..5717f3c8c1 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts @@ -158,8 +158,11 @@ export class DatabaseTaskStore implements TaskStore { 'last_heartbeat_at', '<=', this.db.client.config.client === 'sqlite3' - ? this.db.raw(`datetime('now', '-${Number(timeoutS)} seconds')`) - : this.db.raw(`dateadd('second', -${Number(timeoutS)}, now())`), + ? this.db.raw(`datetime('now', ?)`, [`-${timeoutS} seconds`]) + : this.db.raw(`dateadd('second', ?, ?)`, [ + `-${timeoutS}`, + this.db.fn.now(), + ]), ); const tasks = rawRows.map(row => ({ taskId: row.id,