From 99dc56c7a193cbbb7ba42f5d95374f0490639347 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 30 Jul 2024 10:20:21 +0200 Subject: [PATCH] refactor(core): Make instance role clearer (no-changelog) (#10188) --- packages/cli/src/License.ts | 2 +- packages/cli/src/commands/start.ts | 7 ++----- packages/cli/src/config/schema.ts | 11 ++++++----- .../__tests__/execution-recovery.service.test.ts | 4 ++-- .../cli/src/executions/execution-recovery.service.ts | 2 +- packages/cli/src/services/orchestration.service.ts | 9 +++------ .../services/orchestration/main/MultiMainSetup.ee.ts | 12 ++++++------ packages/cli/src/services/pruning.service.ts | 2 +- .../test/unit/services/orchestration.service.test.ts | 4 ++-- 9 files changed, 24 insertions(+), 29 deletions(-) diff --git a/packages/cli/src/License.ts b/packages/cli/src/License.ts index 25abf0ecd6829..9a2740ce7cbb7 100644 --- a/packages/cli/src/License.ts +++ b/packages/cli/src/License.ts @@ -55,7 +55,7 @@ export class License { * This ensures the mains do not cause a 429 (too many requests) on license init. */ if (config.getEnv('multiMainSetup.enabled')) { - return autoRenewEnabled && config.getEnv('multiMainSetup.instanceType') === 'leader'; + return autoRenewEnabled && config.getEnv('instanceRole') === 'leader'; } return autoRenewEnabled; diff --git a/packages/cli/src/commands/start.ts b/packages/cli/src/commands/start.ts index 2e88c62e06194..c35344326e8e7 100644 --- a/packages/cli/src/commands/start.ts +++ b/packages/cli/src/commands/start.ts @@ -184,10 +184,7 @@ export class Start extends BaseCommand { await this.initOrchestration(); this.logger.debug('Orchestration init complete'); - if ( - !config.getEnv('license.autoRenewEnabled') && - config.getEnv('multiMainSetup.instanceType') === 'leader' - ) { + if (!config.getEnv('license.autoRenewEnabled') && config.getEnv('instanceRole') === 'leader') { this.logger.warn( 'Automatic license renewal is disabled. The license will not renew automatically, and access to licensed features may be lost!', ); @@ -211,7 +208,7 @@ export class Start extends BaseCommand { async initOrchestration() { if (config.getEnv('executions.mode') === 'regular') { - config.set('multiMainSetup.instanceType', 'leader'); + config.set('instanceRole', 'leader'); return; } diff --git a/packages/cli/src/config/schema.ts b/packages/cli/src/config/schema.ts index 1e6d00aed7699..0b689b8f4cfbe 100644 --- a/packages/cli/src/config/schema.ts +++ b/packages/cli/src/config/schema.ts @@ -883,12 +883,13 @@ export const schema = { }, }, + instanceRole: { + doc: 'Always `leader` in single-main setup. `leader` or `follower` in multi-main setup.', + format: ['unset', 'leader', 'follower'] as const, + default: 'unset', // only until Start.initOrchestration + }, + multiMainSetup: { - instanceType: { - doc: 'Type of instance in multi-main setup', - format: ['unset', 'leader', 'follower'] as const, - default: 'unset', // only until first leader key check - }, enabled: { doc: 'Whether to enable multi-main setup for queue mode (license required)', format: Boolean, diff --git a/packages/cli/src/executions/__tests__/execution-recovery.service.test.ts b/packages/cli/src/executions/__tests__/execution-recovery.service.test.ts index c1ad38538e337..7e33c6ac74a8a 100644 --- a/packages/cli/src/executions/__tests__/execution-recovery.service.test.ts +++ b/packages/cli/src/executions/__tests__/execution-recovery.service.test.ts @@ -48,7 +48,7 @@ describe('ExecutionRecoveryService', () => { }); beforeEach(() => { - config.set('multiMainSetup.instanceType', 'leader'); + config.set('instanceRole', 'leader'); }); afterEach(async () => { @@ -130,7 +130,7 @@ describe('ExecutionRecoveryService', () => { /** * Arrange */ - config.set('multiMainSetup.instanceType', 'follower'); + config.set('instanceRole', 'follower'); // @ts-expect-error Private method const amendSpy = jest.spyOn(executionRecoveryService, 'amend'); const messages = setupMessages('123', 'Some workflow'); diff --git a/packages/cli/src/executions/execution-recovery.service.ts b/packages/cli/src/executions/execution-recovery.service.ts index 4b4d90de54ad4..05431a6d9c9d5 100644 --- a/packages/cli/src/executions/execution-recovery.service.ts +++ b/packages/cli/src/executions/execution-recovery.service.ts @@ -332,7 +332,7 @@ export class ExecutionRecoveryService { private shouldScheduleQueueRecovery() { return ( config.getEnv('executions.mode') === 'queue' && - config.getEnv('multiMainSetup.instanceType') === 'leader' && + config.getEnv('instanceRole') === 'leader' && !this.isShuttingDown ); } diff --git a/packages/cli/src/services/orchestration.service.ts b/packages/cli/src/services/orchestration.service.ts index 3d74f187acced..bfc1d140fc6fa 100644 --- a/packages/cli/src/services/orchestration.service.ts +++ b/packages/cli/src/services/orchestration.service.ts @@ -43,15 +43,12 @@ export class OrchestrationService { return config.getEnv('redis.queueModeId'); } - /** - * Whether this instance is the leader in a multi-main setup. Always `false` in single-main setup. - */ get isLeader() { - return config.getEnv('multiMainSetup.instanceType') === 'leader'; + return config.getEnv('instanceRole') === 'leader'; } get isFollower() { - return config.getEnv('multiMainSetup.instanceType') !== 'leader'; + return config.getEnv('instanceRole') !== 'leader'; } sanityCheck() { @@ -66,7 +63,7 @@ export class OrchestrationService { if (this.isMultiMainSetupEnabled) { await this.multiMainSetup.init(); } else { - config.set('multiMainSetup.instanceType', 'leader'); + config.set('instanceRole', 'leader'); } this.isInitialized = true; diff --git a/packages/cli/src/services/orchestration/main/MultiMainSetup.ee.ts b/packages/cli/src/services/orchestration/main/MultiMainSetup.ee.ts index 19cd14c4a87b9..3c8c052da70ae 100644 --- a/packages/cli/src/services/orchestration/main/MultiMainSetup.ee.ts +++ b/packages/cli/src/services/orchestration/main/MultiMainSetup.ee.ts @@ -45,7 +45,7 @@ export class MultiMainSetup extends EventEmitter { async shutdown() { clearInterval(this.leaderCheckInterval); - const isLeader = config.getEnv('multiMainSetup.instanceType') === 'leader'; + const isLeader = config.getEnv('instanceRole') === 'leader'; if (isLeader) await this.redisPublisher.clear(this.leaderKey); } @@ -64,8 +64,8 @@ export class MultiMainSetup extends EventEmitter { if (leaderId && leaderId !== this.instanceId) { this.logger.debug(`[Instance ID ${this.instanceId}] Leader is other instance "${leaderId}"`); - if (config.getEnv('multiMainSetup.instanceType') === 'leader') { - config.set('multiMainSetup.instanceType', 'follower'); + if (config.getEnv('instanceRole') === 'leader') { + config.set('instanceRole', 'follower'); this.emit('leader-stepdown'); // lost leadership - stop triggers, pollers, pruning, wait-tracking, queue recovery @@ -80,7 +80,7 @@ export class MultiMainSetup extends EventEmitter { `[Instance ID ${this.instanceId}] Leadership vacant, attempting to become leader...`, ); - config.set('multiMainSetup.instanceType', 'follower'); + config.set('instanceRole', 'follower'); /** * Lost leadership - stop triggers, pollers, pruning, wait tracking, license renewal, queue recovery @@ -101,7 +101,7 @@ export class MultiMainSetup extends EventEmitter { if (keySetSuccessfully) { this.logger.debug(`[Instance ID ${this.instanceId}] Leader is now this instance`); - config.set('multiMainSetup.instanceType', 'leader'); + config.set('instanceRole', 'leader'); await this.redisPublisher.setExpiration(this.leaderKey, this.leaderKeyTtl); @@ -110,7 +110,7 @@ export class MultiMainSetup extends EventEmitter { */ this.emit('leader-takeover'); } else { - config.set('multiMainSetup.instanceType', 'follower'); + config.set('instanceRole', 'follower'); } } diff --git a/packages/cli/src/services/pruning.service.ts b/packages/cli/src/services/pruning.service.ts index daa0883e1f861..78c0aad03750c 100644 --- a/packages/cli/src/services/pruning.service.ts +++ b/packages/cli/src/services/pruning.service.ts @@ -56,7 +56,7 @@ export class PruningService { if ( config.getEnv('multiMainSetup.enabled') && config.getEnv('generic.instanceType') === 'main' && - config.getEnv('multiMainSetup.instanceType') === 'follower' + config.getEnv('instanceRole') === 'follower' ) { return false; } diff --git a/packages/cli/test/unit/services/orchestration.service.test.ts b/packages/cli/test/unit/services/orchestration.service.test.ts index 43aba8d4843c7..0cdd73f605963 100644 --- a/packages/cli/test/unit/services/orchestration.service.test.ts +++ b/packages/cli/test/unit/services/orchestration.service.test.ts @@ -146,7 +146,7 @@ describe('Orchestration Service', () => { describe('shouldAddWebhooks', () => { beforeEach(() => { - config.set('multiMainSetup.instanceType', 'leader'); + config.set('instanceRole', 'leader'); }); test('should return true for init', () => { // We want to ensure that webhooks are populated on init @@ -169,7 +169,7 @@ describe('Orchestration Service', () => { }); test('should return false for update or activate when not leader', () => { - config.set('multiMainSetup.instanceType', 'follower'); + config.set('instanceRole', 'follower'); const modes = ['update', 'activate'] as WorkflowActivateMode[]; for (const mode of modes) { const result = os.shouldAddWebhooks(mode);