Skip to content

Commit

Permalink
refactor(core): Move instanceType to InstanceSettings (no-changel…
Browse files Browse the repository at this point in the history
…og) (#10640)
  • Loading branch information
netroy committed Sep 16, 2024
1 parent 50beefb commit 25c8a32
Show file tree
Hide file tree
Showing 25 changed files with 85 additions and 89 deletions.
26 changes: 16 additions & 10 deletions packages/cli/src/__tests__/license.test.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
import { LicenseManager } from '@n8n_io/license-sdk';
import { mock } from 'jest-mock-extended';
import { InstanceSettings } from 'n8n-core';
import type { InstanceSettings } from 'n8n-core';

import config from '@/config';
import { N8N_VERSION } from '@/constants';
import { License } from '@/license';
import { Logger } from '@/logger';
import { OrchestrationService } from '@/services/orchestration.service';
import { mockInstance } from '@test/mocking';
import type { Logger } from '@/logger';

jest.mock('@n8n_io/license-sdk');

Expand All @@ -27,9 +25,11 @@ describe('License', () => {
});

let license: License;
const logger = mockInstance(Logger);
const instanceSettings = mockInstance(InstanceSettings, { instanceId: MOCK_INSTANCE_ID });
mockInstance(OrchestrationService);
const logger = mock<Logger>();
const instanceSettings = mock<InstanceSettings>({
instanceId: MOCK_INSTANCE_ID,
instanceType: 'main',
});

beforeEach(async () => {
license = new License(logger, instanceSettings, mock(), mock(), mock());
Expand All @@ -56,8 +56,14 @@ describe('License', () => {
});

test('initializes license manager for worker', async () => {
license = new License(logger, instanceSettings, mock(), mock(), mock());
await license.init('worker');
license = new License(
logger,
mock<InstanceSettings>({ instanceType: 'worker' }),
mock(),
mock(),
mock(),
);
await license.init();
expect(LicenseManager).toHaveBeenCalledWith({
autoRenewEnabled: false,
autoRenewOffset: MOCK_RENEW_OFFSET,
Expand Down Expand Up @@ -265,7 +271,7 @@ describe('License', () => {

await license.reinit();

expect(initSpy).toHaveBeenCalledWith('main', true);
expect(initSpy).toHaveBeenCalledWith(true);

expect(LicenseManager.prototype.reset).toHaveBeenCalled();
expect(LicenseManager.prototype.initialize).toHaveBeenCalled();
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/abstract-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ import { engine as expressHandlebars } from 'express-handlebars';
import { readFile } from 'fs/promises';
import type { Server } from 'http';
import isbot from 'isbot';
import type { InstanceType } from 'n8n-core';
import { Container, Service } from 'typedi';

import config from '@/config';
import { N8N_VERSION, TEMPLATES_DIR, inDevelopment, inTest } from '@/constants';
import * as Db from '@/db';
import { OnShutdown } from '@/decorators/on-shutdown';
import { ExternalHooks } from '@/external-hooks';
import { N8nInstanceType } from '@/interfaces';
import { Logger } from '@/logger';
import { rawBodyReader, bodyParser, corsMiddleware } from '@/middlewares';
import { send, sendErrorResponse } from '@/response-helper';
Expand Down Expand Up @@ -61,7 +61,7 @@ export abstract class AbstractServer {

readonly uniqueInstanceId: string;

constructor(instanceType: N8nInstanceType = 'main') {
constructor(instanceType: Exclude<InstanceType, 'worker'>) {
this.app = express();
this.app.disable('x-powered-by');

Expand Down
18 changes: 4 additions & 14 deletions packages/cli/src/commands/base-command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import { TelemetryEventRelay } from '@/events/telemetry-event-relay';
import { initExpressionEvaluator } from '@/expression-evaluator';
import { ExternalHooks } from '@/external-hooks';
import { ExternalSecretsManager } from '@/external-secrets/external-secrets-manager.ee';
import type { N8nInstanceType } from '@/interfaces';
import { License } from '@/license';
import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials';
import { Logger } from '@/logger';
Expand All @@ -33,9 +32,7 @@ export abstract class BaseCommand extends Command {

protected nodeTypes: NodeTypes;

protected instanceSettings: InstanceSettings;

private instanceType: N8nInstanceType = 'main';
protected instanceSettings: InstanceSettings = Container.get(InstanceSettings);

queueModeId: string;

Expand All @@ -62,9 +59,6 @@ export abstract class BaseCommand extends Command {
process.once('SIGTERM', this.onTerminationSignal('SIGTERM'));
process.once('SIGINT', this.onTerminationSignal('SIGINT'));

// Make sure the settings exist
this.instanceSettings = Container.get(InstanceSettings);

this.nodeTypes = Container.get(NodeTypes);
await Container.get(LoadNodesAndCredentials).init();

Expand Down Expand Up @@ -128,17 +122,13 @@ export abstract class BaseCommand extends Command {
await Container.get(TelemetryEventRelay).init();
}

protected setInstanceType(instanceType: N8nInstanceType) {
this.instanceType = instanceType;
config.set('generic.instanceType', instanceType);
}

protected setInstanceQueueModeId() {
if (config.get('redis.queueModeId')) {
this.queueModeId = config.get('redis.queueModeId');
return;
}
this.queueModeId = generateHostInstanceId(this.instanceType);
// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
this.queueModeId = generateHostInstanceId(this.instanceSettings.instanceType!);
config.set('redis.queueModeId', this.queueModeId);
}

Expand Down Expand Up @@ -278,7 +268,7 @@ export abstract class BaseCommand extends Command {

async initLicense(): Promise<void> {
this.license = Container.get(License);
await this.license.init(this.instanceType ?? 'main');
await this.license.init();

const activationKey = config.getEnv('license.activationKey');

Expand Down
1 change: 0 additions & 1 deletion packages/cli/src/commands/start.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ export class Start extends BaseCommand {

constructor(argv: string[], cmdConfig: Config) {
super(argv, cmdConfig);
this.setInstanceType('main');
this.setInstanceQueueModeId();
}

Expand Down
1 change: 0 additions & 1 deletion packages/cli/src/commands/webhook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ export class Webhook extends BaseCommand {

constructor(argv: string[], cmdConfig: Config) {
super(argv, cmdConfig);
this.setInstanceType('webhook');
if (this.queueModeId) {
this.logger.debug(`Webhook Instance queue mode id: ${this.queueModeId}`);
}
Expand Down
1 change: 0 additions & 1 deletion packages/cli/src/commands/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ export class Worker extends BaseCommand {
);
}

this.setInstanceType('worker');
this.setInstanceQueueModeId();
}

Expand Down
6 changes: 0 additions & 6 deletions packages/cli/src/config/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,12 +175,6 @@ export const schema = {
env: 'GENERIC_TIMEZONE',
},

instanceType: {
doc: 'Type of n8n instance',
format: ['main', 'webhook', 'worker'] as const,
default: 'main',
},

releaseChannel: {
doc: 'N8N release channel',
format: ['stable', 'beta', 'nightly', 'dev'] as const,
Expand Down
5 changes: 2 additions & 3 deletions packages/cli/src/databases/utils/generators.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
import type { InstanceType } from 'n8n-core';
import { ALPHABET } from 'n8n-workflow';
import { customAlphabet } from 'nanoid';

import type { N8nInstanceType } from '@/interfaces';

const nanoid = customAlphabet(ALPHABET, 16);

export function generateNanoId() {
return nanoid();
}

export function generateHostInstanceId(instanceType: N8nInstanceType) {
export function generateHostInstanceId(instanceType: InstanceType) {
return `${instanceType}-${nanoid()}`;
}
2 changes: 0 additions & 2 deletions packages/cli/src/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -422,5 +422,3 @@ export abstract class SecretsProvider {
abstract hasSecret(name: string): boolean;
abstract getSecretNames(): string[];
}

export type N8nInstanceType = 'main' | 'webhook' | 'worker';
13 changes: 7 additions & 6 deletions packages/cli/src/license.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
SETTINGS_LICENSE_CERT_KEY,
UNLIMITED_LICENSE_QUOTA,
} from './constants';
import type { BooleanLicenseFeature, N8nInstanceType, NumericLicenseFeature } from './interfaces';
import type { BooleanLicenseFeature, NumericLicenseFeature } from './interfaces';
import type { RedisServicePubSubPublisher } from './services/redis/redis-service-pub-sub-publisher';
import { RedisService } from './services/redis.service';

Expand Down Expand Up @@ -46,8 +46,8 @@ export class License {
/**
* Whether this instance should renew the license - on init and periodically.
*/
private renewalEnabled(instanceType: N8nInstanceType) {
if (instanceType !== 'main') return false;
private renewalEnabled() {
if (this.instanceSettings.instanceType !== 'main') return false;

const autoRenewEnabled = config.getEnv('license.autoRenewEnabled');

Expand All @@ -63,7 +63,7 @@ export class License {
return autoRenewEnabled;
}

async init(instanceType: N8nInstanceType = 'main', forceRecreate = false) {
async init(forceRecreate = false) {
if (this.manager && !forceRecreate) {
this.logger.warn('License manager already initialized or shutting down');
return;
Expand All @@ -73,6 +73,7 @@ export class License {
return;
}

const { instanceType } = this.instanceSettings;
const isMainInstance = instanceType === 'main';
const server = config.getEnv('license.serverUrl');
const offlineMode = !isMainInstance;
Expand All @@ -90,7 +91,7 @@ export class License {
? async () => await this.licenseMetricsService.collectPassthroughData()
: async () => ({});

const renewalEnabled = this.renewalEnabled(instanceType);
const renewalEnabled = this.renewalEnabled();

try {
this.manager = new LicenseManager({
Expand Down Expand Up @@ -399,7 +400,7 @@ export class License {

async reinit() {
this.manager?.reset();
await this.init('main', true);
await this.init(true);
this.logger.debug('License reinitialized');
}
}
20 changes: 10 additions & 10 deletions packages/cli/src/scaling/__tests__/scaling.service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { InstanceSettings } from 'n8n-core';
import { ApplicationError } from 'n8n-workflow';
import Container from 'typedi';

import config from '@/config';
import type { OrchestrationService } from '@/services/orchestration.service';
import { mockInstance } from '@test/mocking';

Expand Down Expand Up @@ -70,7 +69,8 @@ describe('ScalingService', () => {

beforeEach(() => {
jest.clearAllMocks();
config.set('generic.instanceType', 'main');
// @ts-expect-error readonly property
instanceSettings.instanceType = 'main';
instanceSettings.markAsLeader();

scalingService = new ScalingService(
Expand Down Expand Up @@ -128,8 +128,8 @@ describe('ScalingService', () => {

describe('if worker', () => {
it('should set up queue + listeners', async () => {
// @ts-expect-error Private field
scalingService.instanceType = 'worker';
// @ts-expect-error readonly property
instanceSettings.instanceType = 'worker';

await scalingService.setupQueue();

Expand All @@ -141,8 +141,8 @@ describe('ScalingService', () => {

describe('webhook', () => {
it('should set up a queue + listeners', async () => {
// @ts-expect-error Private field
scalingService.instanceType = 'webhook';
// @ts-expect-error readonly property
instanceSettings.instanceType = 'webhook';

await scalingService.setupQueue();

Expand All @@ -155,8 +155,8 @@ describe('ScalingService', () => {

describe('setupWorker', () => {
it('should set up a worker with concurrency', async () => {
// @ts-expect-error Private field
scalingService.instanceType = 'worker';
// @ts-expect-error readonly property
instanceSettings.instanceType = 'worker';
await scalingService.setupQueue();
const concurrency = 5;

Expand All @@ -172,8 +172,8 @@ describe('ScalingService', () => {
});

it('should throw if called before queue is ready', async () => {
// @ts-expect-error Private field
scalingService.instanceType = 'worker';
// @ts-expect-error readonly property
instanceSettings.instanceType = 'worker';

expect(() => scalingService.setupWorker(5)).toThrow();
});
Expand Down
11 changes: 5 additions & 6 deletions packages/cli/src/scaling/scaling.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ import type {
export class ScalingService {
private queue: JobQueue;

private readonly instanceType = config.getEnv('generic.instanceType');

constructor(
private readonly logger: Logger,
private readonly activeExecutions: ActiveExecutions,
Expand Down Expand Up @@ -211,9 +209,10 @@ export class ScalingService {
throw error;
});

if (this.instanceType === 'main' || this.instanceType === 'webhook') {
const { instanceType } = this.instanceSettings;
if (instanceType === 'main' || instanceType === 'webhook') {
this.registerMainOrWebhookListeners();
} else if (this.instanceType === 'worker') {
} else if (instanceType === 'worker') {
this.registerWorkerListeners();
}
}
Expand Down Expand Up @@ -295,7 +294,7 @@ export class ScalingService {
}

private assertWorker() {
if (this.instanceType === 'worker') return;
if (this.instanceSettings.instanceType === 'worker') return;

throw new ApplicationError('This method must be called on a `worker` instance');
}
Expand All @@ -311,7 +310,7 @@ export class ScalingService {
get isQueueMetricsEnabled() {
return (
this.globalConfig.endpoints.metrics.includeQueueMetrics &&
this.instanceType === 'main' &&
this.instanceSettings.instanceType === 'main' &&
!this.orchestrationService.isMultiMainSetupEnabled
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ let queueModeId: string;

function setDefaultConfig() {
config.set('executions.mode', 'queue');
config.set('generic.instanceType', 'main');
}

const workerRestartEventBusResponse: RedisServiceWorkerResponseObject = {
Expand Down Expand Up @@ -73,6 +72,9 @@ describe('Orchestration Service', () => {
});
setDefaultConfig();
queueModeId = config.get('redis.queueModeId');

// @ts-expect-error readonly property
instanceSettings.instanceType = 'main';
});

beforeEach(() => {
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/services/orchestration.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { RedisService } from './redis.service';
export class OrchestrationService {
constructor(
private readonly logger: Logger,
private readonly instanceSettings: InstanceSettings,
protected readonly instanceSettings: InstanceSettings,
private readonly redisService: RedisService,
readonly multiMainSetup: MultiMainSetup,
) {}
Expand All @@ -31,7 +31,7 @@ export class OrchestrationService {
return (
config.getEnv('executions.mode') === 'queue' &&
config.getEnv('multiMainSetup.enabled') &&
config.getEnv('generic.instanceType') === 'main' &&
this.instanceSettings.instanceType === 'main' &&
this.isMultiMainSetupLicensed
);
}
Expand Down
Loading

0 comments on commit 25c8a32

Please sign in to comment.