From ba9abf4fbac58b94508ce7360a318313832e1cfd Mon Sep 17 00:00:00 2001 From: Tim Hansen Date: Fri, 12 Jul 2024 15:24:05 -0600 Subject: [PATCH 1/7] Allow manual trigger frequency in backend-tasks Signed-off-by: Tim Hansen --- .changeset/hip-fishes-guess.md | 6 +++ .changeset/metal-planes-nail.md | 5 +++ .../20240712211735_nullable_next_run.js | 37 +++++++++++++++++++ .../lib/PluginTaskSchedulerImpl.test.ts | 3 ++ .../scheduler/lib/PluginTaskSchedulerImpl.ts | 3 ++ .../scheduler/lib/TaskWorker.test.ts | 28 ++++++++++++++ .../entrypoints/scheduler/lib/TaskWorker.ts | 17 +++++++-- .../src/entrypoints/scheduler/lib/types.ts | 9 +++++ .../services/definitions/SchedulerService.ts | 22 ++++++++--- .../src/tasks/PluginTaskSchedulerImpl.ts | 3 ++ .../src/tasks/TaskWorker.test.ts | 28 ++++++++++++++ .../backend-tasks/src/tasks/TaskWorker.ts | 17 +++++++-- .../readTaskScheduleDefinitionFromConfig.ts | 12 ++++-- packages/backend-tasks/src/tasks/types.ts | 7 +++- 14 files changed, 180 insertions(+), 17 deletions(-) create mode 100644 .changeset/hip-fishes-guess.md create mode 100644 .changeset/metal-planes-nail.md create mode 100644 packages/backend-defaults/migrations/scheduler/20240712211735_nullable_next_run.js diff --git a/.changeset/hip-fishes-guess.md b/.changeset/hip-fishes-guess.md new file mode 100644 index 0000000000..909448c673 --- /dev/null +++ b/.changeset/hip-fishes-guess.md @@ -0,0 +1,6 @@ +--- +'@backstage/backend-plugin-api': patch +'@backstage/backend-defaults': patch +--- + +The `SchedulerService` now allows tasks with `frequency: { trigger: 'manual' }`. This means that the task will not be scheduled, but rather run only when manually triggered with `SchedulerService.triggerTask`. diff --git a/.changeset/metal-planes-nail.md b/.changeset/metal-planes-nail.md new file mode 100644 index 0000000000..2cfa12a435 --- /dev/null +++ b/.changeset/metal-planes-nail.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-tasks': patch +--- + +The `PluginTaskScheduler` now allows tasks with `frequency: { trigger: 'manual' }`. This means that the task will not be scheduled, but rather run only when manually triggered with `PluginTaskScheduler.triggerTask`. diff --git a/packages/backend-defaults/migrations/scheduler/20240712211735_nullable_next_run.js b/packages/backend-defaults/migrations/scheduler/20240712211735_nullable_next_run.js new file mode 100644 index 0000000000..b636df35d7 --- /dev/null +++ b/packages/backend-defaults/migrations/scheduler/20240712211735_nullable_next_run.js @@ -0,0 +1,37 @@ +/* + * Copyright 2024 The Backstage Authors + * + * 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 } knex + * @returns { Promise } + */ +exports.up = async function up(knex) { + await knex.schema.alterTable('backstage_backend_tasks__tasks', table => { + table.setNullable('next_run_start_at'); + }); +}; + +/** + * @param { import("knex").Knex } knex + * @returns { Promise } + */ +exports.down = async function down(knex) { + await knex.schema.alterTable('backstage_backend_tasks__tasks', table => { + table.dropNullable('next_run_start_at'); + }); +}; diff --git a/packages/backend-defaults/src/entrypoints/scheduler/lib/PluginTaskSchedulerImpl.test.ts b/packages/backend-defaults/src/entrypoints/scheduler/lib/PluginTaskSchedulerImpl.test.ts index c5c4c8708a..0c9219e98e 100644 --- a/packages/backend-defaults/src/entrypoints/scheduler/lib/PluginTaskSchedulerImpl.test.ts +++ b/packages/backend-defaults/src/entrypoints/scheduler/lib/PluginTaskSchedulerImpl.test.ts @@ -347,6 +347,9 @@ describe('PluginTaskManagerImpl', () => { expect(parseDuration({ milliseconds: 5000 })).toEqual('PT5S'); expect(parseDuration(Duration.fromMillis(5000))).toEqual('PT5S'); expect(parseDuration({ cron: '1 * * * *' })).toEqual('1 * * * *'); + expect(parseDuration({ trigger: 'manual' })).toEqual({ + trigger: 'manual', + }); }); }); }); diff --git a/packages/backend-defaults/src/entrypoints/scheduler/lib/PluginTaskSchedulerImpl.ts b/packages/backend-defaults/src/entrypoints/scheduler/lib/PluginTaskSchedulerImpl.ts index 40aa9d521c..f8d06baf56 100644 --- a/packages/backend-defaults/src/entrypoints/scheduler/lib/PluginTaskSchedulerImpl.ts +++ b/packages/backend-defaults/src/entrypoints/scheduler/lib/PluginTaskSchedulerImpl.ts @@ -155,6 +155,9 @@ export function parseDuration( if (typeof frequency === 'object' && 'cron' in frequency) { return frequency.cron; } + if (typeof frequency === 'object' && 'trigger' in frequency) { + return frequency.trigger; + } const parsed = Duration.isDuration(frequency) ? frequency diff --git a/packages/backend-defaults/src/entrypoints/scheduler/lib/TaskWorker.test.ts b/packages/backend-defaults/src/entrypoints/scheduler/lib/TaskWorker.test.ts index 1cafe00670..5db82f3ece 100644 --- a/packages/backend-defaults/src/entrypoints/scheduler/lib/TaskWorker.test.ts +++ b/packages/backend-defaults/src/entrypoints/scheduler/lib/TaskWorker.test.ts @@ -503,4 +503,32 @@ describe('TaskWorker', () => { await knex.destroy(); }, ); + + it.each(databases.eachSupportedId())( + 'next_run_start_at is not set for manually-triggered tasks, %p', + async databaseId => { + const knex = await databases.init(databaseId); + await migrateBackendTasks(knex); + + const fn = jest.fn( + async () => new Promise(resolve => setTimeout(resolve, 50)), + ); + + const initialSettings: TaskSettingsV2 = { + version: 2, + cadence: 'manual', + timeoutAfterDuration: 'PT1M', + }; + + const worker = new TaskWorker('task99', fn, knex, logger); + await worker.persistTask(initialSettings); + await worker.tryClaimTask('ticket', initialSettings); + await worker.tryReleaseTask('ticket', initialSettings); + + const row = (await knex(DB_TASKS_TABLE))[0]; + expect(row.next_run_start_at).toBeNull(); + + await knex.destroy(); + }, + ); }); diff --git a/packages/backend-defaults/src/entrypoints/scheduler/lib/TaskWorker.ts b/packages/backend-defaults/src/entrypoints/scheduler/lib/TaskWorker.ts index a1c4ec44fd..c5570a8e30 100644 --- a/packages/backend-defaults/src/entrypoints/scheduler/lib/TaskWorker.ts +++ b/packages/backend-defaults/src/entrypoints/scheduler/lib/TaskWorker.ts @@ -53,8 +53,8 @@ export class TaskWorker { ); let workCheckFrequency = this.workCheckFrequency; - const isCron = !settings?.cadence.startsWith('P'); - if (!isCron) { + const isDuration = settings?.cadence.startsWith('P'); + if (isDuration) { const cadence = Duration.fromISO(settings.cadence); if (cadence < workCheckFrequency) { workCheckFrequency = cadence; @@ -175,7 +175,9 @@ export class TaskWorker { // read it back again. taskSettingsV2Schema.parse(settings); - const isCron = !settings?.cadence.startsWith('P'); + const isManual = settings?.cadence === 'manual'; + const isDuration = settings?.cadence.startsWith('P'); + const isCron = !isManual && !isDuration; let startAt: Knex.Raw | undefined; let nextStartAt: Knex.Raw | undefined; @@ -194,6 +196,9 @@ export class TaskWorker { nextStartAt = this.nextRunAtRaw(time); startAt ||= nextStartAt; + } else if (isManual) { + nextStartAt = this.knex.raw('null'); + startAt ||= nextStartAt; } else { startAt ||= this.knex.fn.now(); nextStartAt = nowPlus(Duration.fromISO(settings.cadence), this.knex); @@ -317,7 +322,9 @@ export class TaskWorker { ticket: string, settings: TaskSettingsV2, ): Promise { - const isCron = !settings?.cadence.startsWith('P'); + const isManual = settings?.cadence === 'manual'; + const isDuration = settings?.cadence.startsWith('P'); + const isCron = !isManual && !isDuration; let nextRun: Knex.Raw; if (isCron) { @@ -325,6 +332,8 @@ export class TaskWorker { this.logger.debug(`task: ${this.taskId} will next occur around ${time}`); nextRun = this.nextRunAtRaw(time); + } else if (isManual) { + nextRun = this.knex.raw('null'); } else { const dt = Duration.fromISO(settings.cadence).as('seconds'); this.logger.debug( diff --git a/packages/backend-defaults/src/entrypoints/scheduler/lib/types.ts b/packages/backend-defaults/src/entrypoints/scheduler/lib/types.ts index 61e6c1d9a8..173c989543 100644 --- a/packages/backend-defaults/src/entrypoints/scheduler/lib/types.ts +++ b/packages/backend-defaults/src/entrypoints/scheduler/lib/types.ts @@ -40,6 +40,10 @@ function isValidCronFormat(c: string | undefined): boolean { } } +function isValidTrigger(t: string): boolean { + return t === 'manual'; +} + export const taskSettingsV1Schema = z.object({ version: z.literal(1), initialDelayDuration: z @@ -68,6 +72,11 @@ export const taskSettingsV2Schema = z.object({ cadence: z .string() .refine(isValidCronFormat, { message: 'Invalid cron' }) + .or( + z.string().refine(isValidTrigger, { + message: "Invalid trigger, expecting 'manual'", + }), + ) .or( z.string().refine(isValidOptionalDurationString, { message: 'Invalid duration, expecting ISO Period', diff --git a/packages/backend-plugin-api/src/services/definitions/SchedulerService.ts b/packages/backend-plugin-api/src/services/definitions/SchedulerService.ts index a7ec79a336..a8b84b9084 100644 --- a/packages/backend-plugin-api/src/services/definitions/SchedulerService.ts +++ b/packages/backend-plugin-api/src/services/definitions/SchedulerService.ts @@ -96,7 +96,8 @@ export interface SchedulerServiceTaskScheduleDefinition { cron: string; } | Duration - | HumanDuration; + | HumanDuration + | { trigger: 'manual' }; /** * The maximum amount of time that a single task invocation can take, before @@ -193,7 +194,12 @@ export interface SchedulerServiceTaskScheduleDefinitionConfig { cron: string; } | string - | HumanDuration; + | HumanDuration + /** + * This task will only run when manually triggered with the `triggerTask` method; no automatic + * scheduling. This is useful for locking of global tasks that should not be run concurrently. + */ + | { trigger: 'manual' }; /** * The maximum amount of time that a single task invocation can take, before @@ -361,14 +367,20 @@ function readDuration(config: Config, key: string): HumanDuration { return readDurationFromConfig(config, { key }); } -function readCronOrDuration( +function readFrequency( config: Config, key: string, -): { cron: string } | HumanDuration { +): { cron: string } | HumanDuration | { trigger: 'manual' } { const value = config.get(key); if (typeof value === 'object' && (value as { cron?: string }).cron) { return value as { cron: string }; } + if ( + typeof value === 'object' && + (value as { trigger?: string }).trigger === 'manual' + ) { + return { trigger: 'manual' }; + } return readDuration(config, key); } @@ -383,7 +395,7 @@ function readCronOrDuration( export function readSchedulerServiceTaskScheduleDefinitionFromConfig( config: Config, ): SchedulerServiceTaskScheduleDefinition { - const frequency = readCronOrDuration(config, 'frequency'); + const frequency = readFrequency(config, 'frequency'); const timeout = readDuration(config, 'timeout'); const initialDelay = config.has('initialDelay') diff --git a/packages/backend-tasks/src/tasks/PluginTaskSchedulerImpl.ts b/packages/backend-tasks/src/tasks/PluginTaskSchedulerImpl.ts index b1647f4edf..0ccc77a6c1 100644 --- a/packages/backend-tasks/src/tasks/PluginTaskSchedulerImpl.ts +++ b/packages/backend-tasks/src/tasks/PluginTaskSchedulerImpl.ts @@ -152,6 +152,9 @@ export function parseDuration( if ('cron' in frequency) { return frequency.cron; } + if ('trigger' in frequency) { + return frequency.trigger; + } const parsed = Duration.isDuration(frequency) ? frequency diff --git a/packages/backend-tasks/src/tasks/TaskWorker.test.ts b/packages/backend-tasks/src/tasks/TaskWorker.test.ts index 10ebfde5b4..829d7754f8 100644 --- a/packages/backend-tasks/src/tasks/TaskWorker.test.ts +++ b/packages/backend-tasks/src/tasks/TaskWorker.test.ts @@ -503,4 +503,32 @@ describe('TaskWorker', () => { await knex.destroy(); }, ); + + it.each(databases.eachSupportedId())( + 'next_run_start_at is not set for manually-triggered tasks, %p', + async databaseId => { + const knex = await databases.init(databaseId); + await migrateBackendTasks(knex); + + const fn = jest.fn( + async () => new Promise(resolve => setTimeout(resolve, 50)), + ); + + const initialSettings: TaskSettingsV2 = { + version: 2, + cadence: 'manual', + timeoutAfterDuration: 'PT1M', + }; + + const worker = new TaskWorker('task99', fn, knex, logger); + await worker.persistTask(initialSettings); + await worker.tryClaimTask('ticket', initialSettings); + await worker.tryReleaseTask('ticket', initialSettings); + + const row = (await knex(DB_TASKS_TABLE))[0]; + expect(row.next_run_start_at).toBeNull(); + + await knex.destroy(); + }, + ); }); diff --git a/packages/backend-tasks/src/tasks/TaskWorker.ts b/packages/backend-tasks/src/tasks/TaskWorker.ts index 57cbf2203e..a50b988522 100644 --- a/packages/backend-tasks/src/tasks/TaskWorker.ts +++ b/packages/backend-tasks/src/tasks/TaskWorker.ts @@ -52,8 +52,8 @@ export class TaskWorker { ); let workCheckFrequency = this.workCheckFrequency; - const isCron = !settings?.cadence.startsWith('P'); - if (!isCron) { + const isDuration = settings?.cadence.startsWith('P'); + if (isDuration) { const cadence = Duration.fromISO(settings.cadence); if (cadence < workCheckFrequency) { workCheckFrequency = cadence; @@ -174,7 +174,9 @@ export class TaskWorker { // read it back again. taskSettingsV2Schema.parse(settings); - const isCron = !settings?.cadence.startsWith('P'); + const isManual = settings?.cadence === 'manual'; + const isDuration = settings?.cadence.startsWith('P'); + const isCron = !isManual && !isDuration; let startAt: Knex.Raw | undefined; let nextStartAt: Knex.Raw | undefined; @@ -193,6 +195,9 @@ export class TaskWorker { nextStartAt = this.nextRunAtRaw(time); startAt ||= nextStartAt; + } else if (isManual) { + nextStartAt = this.knex.raw('null'); + startAt ||= nextStartAt; } else { startAt ||= this.knex.fn.now(); nextStartAt = nowPlus(Duration.fromISO(settings.cadence), this.knex); @@ -316,7 +321,9 @@ export class TaskWorker { ticket: string, settings: TaskSettingsV2, ): Promise { - const isCron = !settings?.cadence.startsWith('P'); + const isManual = settings?.cadence === 'manual'; + const isDuration = settings?.cadence.startsWith('P'); + const isCron = !isManual && !isDuration; let nextRun: Knex.Raw; if (isCron) { @@ -324,6 +331,8 @@ export class TaskWorker { this.logger.debug(`task: ${this.taskId} will next occur around ${time}`); nextRun = this.nextRunAtRaw(time); + } else if (isManual) { + nextRun = this.knex.raw('null'); } else { const dt = Duration.fromISO(settings.cadence).as('seconds'); this.logger.debug( diff --git a/packages/backend-tasks/src/tasks/readTaskScheduleDefinitionFromConfig.ts b/packages/backend-tasks/src/tasks/readTaskScheduleDefinitionFromConfig.ts index e94d06d5c3..85099d82db 100644 --- a/packages/backend-tasks/src/tasks/readTaskScheduleDefinitionFromConfig.ts +++ b/packages/backend-tasks/src/tasks/readTaskScheduleDefinitionFromConfig.ts @@ -32,14 +32,20 @@ function readDuration(config: Config, key: string): HumanDuration { return readDurationFromConfig(config, { key }); } -function readCronOrDuration( +function readFrequency( config: Config, key: string, -): { cron: string } | Duration | HumanDuration { +): { cron: string } | Duration | HumanDuration | { trigger: 'manual' } { const value = config.get(key); if (typeof value === 'object' && (value as { cron?: string }).cron) { return value as { cron: string }; } + if ( + typeof value === 'object' && + (value as { trigger?: string }).trigger === 'manual' + ) { + return { trigger: 'manual' }; + } return readDuration(config, key); } @@ -56,7 +62,7 @@ function readCronOrDuration( export function readTaskScheduleDefinitionFromConfig( config: Config, ): TaskScheduleDefinition { - const frequency = readCronOrDuration(config, 'frequency'); + const frequency = readFrequency(config, 'frequency'); const timeout = readDuration(config, 'timeout'); const initialDelay = config.has('initialDelay') diff --git a/packages/backend-tasks/src/tasks/types.ts b/packages/backend-tasks/src/tasks/types.ts index e6a31873be..aaa6ca8bd2 100644 --- a/packages/backend-tasks/src/tasks/types.ts +++ b/packages/backend-tasks/src/tasks/types.ts @@ -100,7 +100,12 @@ export interface TaskScheduleDefinition { cron: string; } | Duration - | HumanDuration; + | HumanDuration + /** + * This task will only run when manually triggered with the `triggerTask` method; no automatic + * scheduling. This is useful for locking of global tasks that should not be run concurrently. + */ + | { trigger: 'manual' }; /** * The maximum amount of time that a single task invocation can take, before From 1e6c3ac445c32330cc25f52234ad007daa8842a7 Mon Sep 17 00:00:00 2001 From: Tim Hansen Date: Tue, 16 Jul 2024 08:30:02 -0400 Subject: [PATCH 2/7] Move migration tests Signed-off-by: Tim Hansen --- packages/backend-defaults/migrations.test.ts | 128 ++++++++++++++++++ .../20240712211735_nullable_next_run.js | 37 +++++ packages/backend-tasks/src/migrations.test.ts | 44 ++++++ 3 files changed, 209 insertions(+) create mode 100644 packages/backend-defaults/migrations.test.ts create mode 100644 packages/backend-tasks/migrations/20240712211735_nullable_next_run.js diff --git a/packages/backend-defaults/migrations.test.ts b/packages/backend-defaults/migrations.test.ts new file mode 100644 index 0000000000..98f4e41706 --- /dev/null +++ b/packages/backend-defaults/migrations.test.ts @@ -0,0 +1,128 @@ +/* + * Copyright 2024 The Backstage Authors + * + * 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 { Knex } from 'knex'; +import { TestDatabases } from '@backstage/backend-test-utils'; +import fs from 'fs'; + +const migrationsDir = `${__dirname}/../migrations`; +const migrationsFiles = fs.readdirSync(migrationsDir).sort(); + +async function migrateUpOnce(knex: Knex): Promise { + await knex.migrate.up({ directory: migrationsDir }); +} + +async function migrateDownOnce(knex: Knex): Promise { + await knex.migrate.down({ directory: migrationsDir }); +} + +async function migrateUntilBefore(knex: Knex, target: string): Promise { + const index = migrationsFiles.indexOf(target); + if (index === -1) { + throw new Error(`Migration ${target} not found`); + } + for (let i = 0; i < index; i++) { + await migrateUpOnce(knex); + } +} + +jest.setTimeout(60_000); + +describe('migrations', () => { + const databases = TestDatabases.create(); + + it.each(databases.eachSupportedId())( + '20210928160613_init.js, %p', + async databaseId => { + const knex = await databases.init(databaseId); + + await migrateUntilBefore(knex, '20210928160613_init.js'); + await migrateUpOnce(knex); + + await knex('backstage_backend_tasks__tasks').insert({ + id: 'test', + settings_json: '{}', + next_run_start_at: knex.fn.now(), + }); + + await expect(knex('backstage_backend_tasks__tasks')).resolves.toEqual([ + { + id: 'test', + settings_json: '{}', + next_run_start_at: expect.anything(), + current_run_ticket: null, + current_run_started_at: null, + current_run_expires_at: null, + }, + ]); + + await migrateDownOnce(knex); + + // This looks odd - you might expect a .toThrow at the end but that + // actually is flaky for some reason specifically on sqlite when + // performing multiple runs in sequence + await expect(knex('backstage_backend_tasks__tasks')).rejects.toEqual( + expect.anything(), + ); + + await knex.destroy(); + }, + ); + + it.each(databases.eachSupportedId())( + '20240712211735_nullable_next_run.js, %p', + async databaseId => { + const knex = await databases.init(databaseId); + + await migrateUntilBefore(knex, '20240712211735_nullable_next_run.js'); + await migrateUpOnce(knex); + + await knex('backstage_backend_tasks__tasks').insert({ + id: 'test', + settings_json: '{}', + next_run_start_at: knex.raw('null'), + }); + + await expect(knex('backstage_backend_tasks__tasks')).resolves.toEqual([ + { + id: 'test', + settings_json: '{}', + next_run_start_at: null, + current_run_ticket: null, + current_run_started_at: null, + current_run_expires_at: null, + }, + ]); + + await knex + .delete() + .from('backstage_backend_tasks__tasks') + .where({ next_run_start_at: null }); + + await migrateDownOnce(knex); + + await expect( + knex('backstage_backend_tasks__tasks').insert({ + id: 'test', + settings_json: '{}', + next_run_start_at: knex.raw('null'), + }), + ).rejects.toEqual(expect.anything()); + + await knex.destroy(); + }, + ); +}); diff --git a/packages/backend-tasks/migrations/20240712211735_nullable_next_run.js b/packages/backend-tasks/migrations/20240712211735_nullable_next_run.js new file mode 100644 index 0000000000..b636df35d7 --- /dev/null +++ b/packages/backend-tasks/migrations/20240712211735_nullable_next_run.js @@ -0,0 +1,37 @@ +/* + * Copyright 2024 The Backstage Authors + * + * 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 } knex + * @returns { Promise } + */ +exports.up = async function up(knex) { + await knex.schema.alterTable('backstage_backend_tasks__tasks', table => { + table.setNullable('next_run_start_at'); + }); +}; + +/** + * @param { import("knex").Knex } knex + * @returns { Promise } + */ +exports.down = async function down(knex) { + await knex.schema.alterTable('backstage_backend_tasks__tasks', table => { + table.dropNullable('next_run_start_at'); + }); +}; diff --git a/packages/backend-tasks/src/migrations.test.ts b/packages/backend-tasks/src/migrations.test.ts index 82208da975..49576d8a58 100644 --- a/packages/backend-tasks/src/migrations.test.ts +++ b/packages/backend-tasks/src/migrations.test.ts @@ -81,4 +81,48 @@ describe('migrations', () => { await knex.destroy(); }, ); + + it.each(databases.eachSupportedId())( + '20240712211735_nullable_next_run.js, %p', + async databaseId => { + const knex = await databases.init(databaseId); + + await migrateUntilBefore(knex, '20240712211735_nullable_next_run.js'); + await migrateUpOnce(knex); + + await knex('backstage_backend_tasks__tasks').insert({ + id: 'test', + settings_json: '{}', + next_run_start_at: knex.raw('null'), + }); + + await expect(knex('backstage_backend_tasks__tasks')).resolves.toEqual([ + { + id: 'test', + settings_json: '{}', + next_run_start_at: null, + current_run_ticket: null, + current_run_started_at: null, + current_run_expires_at: null, + }, + ]); + + await knex + .delete() + .from('backstage_backend_tasks__tasks') + .where({ next_run_start_at: null }); + + await migrateDownOnce(knex); + + await expect( + knex('backstage_backend_tasks__tasks').insert({ + id: 'test', + settings_json: '{}', + next_run_start_at: knex.raw('null'), + }), + ).rejects.toEqual(expect.anything()); + + await knex.destroy(); + }, + ); }); From 03692ff5396e5ca8c071320deca3bf446ad85880 Mon Sep 17 00:00:00 2001 From: Tim Hansen Date: Tue, 16 Jul 2024 09:34:27 -0400 Subject: [PATCH 3/7] api-reports Signed-off-by: Tim Hansen --- packages/backend-plugin-api/api-report.md | 14 ++++++++++++-- packages/backend-tasks/api-report.md | 9 ++++++++- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/packages/backend-plugin-api/api-report.md b/packages/backend-plugin-api/api-report.md index 7b1e6ef2d3..0938ae4e7e 100644 --- a/packages/backend-plugin-api/api-report.md +++ b/packages/backend-plugin-api/api-report.md @@ -608,7 +608,10 @@ export interface SchedulerServiceTaskScheduleDefinition { cron: string; } | Duration - | HumanDuration; + | HumanDuration + | { + trigger: 'manual'; + }; initialDelay?: Duration | HumanDuration; scope?: 'global' | 'local'; timeout: Duration | HumanDuration; @@ -621,7 +624,14 @@ export interface SchedulerServiceTaskScheduleDefinitionConfig { cron: string; } | string - | HumanDuration; + | HumanDuration + /** + * This task will only run when manually triggered with the `triggerTask` method; no automatic + * scheduling. This is useful for locking of global tasks that should not be run concurrently. + */ + | { + trigger: 'manual'; + }; initialDelay?: string | HumanDuration; scope?: 'global' | 'local'; timeout: string | HumanDuration; diff --git a/packages/backend-tasks/api-report.md b/packages/backend-tasks/api-report.md index 0d5ced044a..c0f9da3a01 100644 --- a/packages/backend-tasks/api-report.md +++ b/packages/backend-tasks/api-report.md @@ -62,7 +62,14 @@ export interface TaskScheduleDefinition { cron: string; } | Duration - | HumanDuration_2; + | HumanDuration_2 + /** + * This task will only run when manually triggered with the `triggerTask` method; no automatic + * scheduling. This is useful for locking of global tasks that should not be run concurrently. + */ + | { + trigger: 'manual'; + }; initialDelay?: Duration | HumanDuration_2; scope?: 'global' | 'local'; timeout: Duration | HumanDuration_2; From c8b0732b5cf798b703181adaf1ce70a0ea367d5c Mon Sep 17 00:00:00 2001 From: Tim Hansen Date: Tue, 16 Jul 2024 09:59:15 -0400 Subject: [PATCH 4/7] Fix parseDuration test Signed-off-by: Tim Hansen --- .../entrypoints/scheduler/lib/PluginTaskSchedulerImpl.test.ts | 4 +--- .../backend-tasks/src/tasks/PluginTaskSchedulerImpl.test.ts | 1 + 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/backend-defaults/src/entrypoints/scheduler/lib/PluginTaskSchedulerImpl.test.ts b/packages/backend-defaults/src/entrypoints/scheduler/lib/PluginTaskSchedulerImpl.test.ts index 0c9219e98e..3e0782d4a3 100644 --- a/packages/backend-defaults/src/entrypoints/scheduler/lib/PluginTaskSchedulerImpl.test.ts +++ b/packages/backend-defaults/src/entrypoints/scheduler/lib/PluginTaskSchedulerImpl.test.ts @@ -347,9 +347,7 @@ describe('PluginTaskManagerImpl', () => { expect(parseDuration({ milliseconds: 5000 })).toEqual('PT5S'); expect(parseDuration(Duration.fromMillis(5000))).toEqual('PT5S'); expect(parseDuration({ cron: '1 * * * *' })).toEqual('1 * * * *'); - expect(parseDuration({ trigger: 'manual' })).toEqual({ - trigger: 'manual', - }); + expect(parseDuration({ trigger: 'manual' })).toEqual('manual'); }); }); }); diff --git a/packages/backend-tasks/src/tasks/PluginTaskSchedulerImpl.test.ts b/packages/backend-tasks/src/tasks/PluginTaskSchedulerImpl.test.ts index 77965632e9..eca899098f 100644 --- a/packages/backend-tasks/src/tasks/PluginTaskSchedulerImpl.test.ts +++ b/packages/backend-tasks/src/tasks/PluginTaskSchedulerImpl.test.ts @@ -345,6 +345,7 @@ describe('PluginTaskManagerImpl', () => { expect(parseDuration({ milliseconds: 5000 })).toEqual('PT5S'); expect(parseDuration(Duration.fromMillis(5000))).toEqual('PT5S'); expect(parseDuration({ cron: '1 * * * *' })).toEqual('1 * * * *'); + expect(parseDuration({ trigger: 'manual' })).toEqual('manual'); }); }); }); From d160ce8925c3d67fa711fa7d2de707237eb3dbd9 Mon Sep 17 00:00:00 2001 From: Tim Hansen Date: Tue, 16 Jul 2024 14:49:20 -0400 Subject: [PATCH 5/7] Add trigger to backend-tasks schema Signed-off-by: Tim Hansen --- packages/backend-tasks/src/tasks/types.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/backend-tasks/src/tasks/types.ts b/packages/backend-tasks/src/tasks/types.ts index aaa6ca8bd2..a0bdbbaa64 100644 --- a/packages/backend-tasks/src/tasks/types.ts +++ b/packages/backend-tasks/src/tasks/types.ts @@ -378,6 +378,10 @@ function isValidCronFormat(c: string | undefined): boolean { } } +function isValidTrigger(t: string): boolean { + return t === 'manual'; +} + export const taskSettingsV1Schema = z.object({ version: z.literal(1), initialDelayDuration: z @@ -406,6 +410,11 @@ export const taskSettingsV2Schema = z.object({ cadence: z .string() .refine(isValidCronFormat, { message: 'Invalid cron' }) + .or( + z.string().refine(isValidTrigger, { + message: "Invalid trigger, expecting 'manual'", + }), + ) .or( z.string().refine(isValidOptionalDurationString, { message: 'Invalid duration, expecting ISO Period', From df17af74d61c4422a546ac63afe568cd70b5ffe4 Mon Sep 17 00:00:00 2001 From: Tim Hansen Date: Tue, 16 Jul 2024 17:41:09 -0400 Subject: [PATCH 6/7] Update migration, move test file Signed-off-by: Tim Hansen --- .../migrations/scheduler/20240712211735_nullable_next_run.js | 4 ++++ packages/backend-defaults/{ => src}/migrations.test.ts | 5 ----- .../migrations/20240712211735_nullable_next_run.js | 4 ++++ packages/backend-tasks/src/migrations.test.ts | 5 ----- 4 files changed, 8 insertions(+), 10 deletions(-) rename packages/backend-defaults/{ => src}/migrations.test.ts (96%) diff --git a/packages/backend-defaults/migrations/scheduler/20240712211735_nullable_next_run.js b/packages/backend-defaults/migrations/scheduler/20240712211735_nullable_next_run.js index b636df35d7..efbef1c76f 100644 --- a/packages/backend-defaults/migrations/scheduler/20240712211735_nullable_next_run.js +++ b/packages/backend-defaults/migrations/scheduler/20240712211735_nullable_next_run.js @@ -31,6 +31,10 @@ exports.up = async function up(knex) { * @returns { Promise } */ exports.down = async function down(knex) { + await knex + .delete() + .from('backstage_backend_tasks__tasks') + .where({ next_run_start_at: null }); await knex.schema.alterTable('backstage_backend_tasks__tasks', table => { table.dropNullable('next_run_start_at'); }); diff --git a/packages/backend-defaults/migrations.test.ts b/packages/backend-defaults/src/migrations.test.ts similarity index 96% rename from packages/backend-defaults/migrations.test.ts rename to packages/backend-defaults/src/migrations.test.ts index 98f4e41706..cae127442b 100644 --- a/packages/backend-defaults/migrations.test.ts +++ b/packages/backend-defaults/src/migrations.test.ts @@ -107,11 +107,6 @@ describe('migrations', () => { }, ]); - await knex - .delete() - .from('backstage_backend_tasks__tasks') - .where({ next_run_start_at: null }); - await migrateDownOnce(knex); await expect( diff --git a/packages/backend-tasks/migrations/20240712211735_nullable_next_run.js b/packages/backend-tasks/migrations/20240712211735_nullable_next_run.js index b636df35d7..efbef1c76f 100644 --- a/packages/backend-tasks/migrations/20240712211735_nullable_next_run.js +++ b/packages/backend-tasks/migrations/20240712211735_nullable_next_run.js @@ -31,6 +31,10 @@ exports.up = async function up(knex) { * @returns { Promise } */ exports.down = async function down(knex) { + await knex + .delete() + .from('backstage_backend_tasks__tasks') + .where({ next_run_start_at: null }); await knex.schema.alterTable('backstage_backend_tasks__tasks', table => { table.dropNullable('next_run_start_at'); }); diff --git a/packages/backend-tasks/src/migrations.test.ts b/packages/backend-tasks/src/migrations.test.ts index 49576d8a58..7583948f71 100644 --- a/packages/backend-tasks/src/migrations.test.ts +++ b/packages/backend-tasks/src/migrations.test.ts @@ -107,11 +107,6 @@ describe('migrations', () => { }, ]); - await knex - .delete() - .from('backstage_backend_tasks__tasks') - .where({ next_run_start_at: null }); - await migrateDownOnce(knex); await expect( From fff1f50a7fbceb1245fd9c519a893a34994ab6b5 Mon Sep 17 00:00:00 2001 From: Tim Hansen Date: Tue, 16 Jul 2024 19:43:03 -0400 Subject: [PATCH 7/7] Fix migration path Signed-off-by: Tim Hansen --- packages/backend-defaults/src/migrations.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend-defaults/src/migrations.test.ts b/packages/backend-defaults/src/migrations.test.ts index cae127442b..579c32e626 100644 --- a/packages/backend-defaults/src/migrations.test.ts +++ b/packages/backend-defaults/src/migrations.test.ts @@ -18,7 +18,7 @@ import { Knex } from 'knex'; import { TestDatabases } from '@backstage/backend-test-utils'; import fs from 'fs'; -const migrationsDir = `${__dirname}/../migrations`; +const migrationsDir = `${__dirname}/../migrations/scheduler`; const migrationsFiles = fs.readdirSync(migrationsDir).sort(); async function migrateUpOnce(knex: Knex): Promise {