Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): Prevent occassional 429s on license init in multi-main setup #9284

Merged
merged 3 commits into from
May 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 30 additions & 4 deletions packages/cli/src/License.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,26 @@ export class License {
private readonly usageMetricsService: UsageMetricsService,
) {}

/**
* Whether this instance should renew the license - on init and periodically.
*/
private renewalEnabled(instanceType: N8nInstanceType) {
if (instanceType !== 'main') return false;

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

/**
* In multi-main setup, all mains start off with `unset` status and so renewal disabled.
* On becoming leader or follower, each will enable or disable renewal, respectively.
* 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;
}

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

const isMainInstance = instanceType === 'main';
const server = config.getEnv('license.serverUrl');
const autoRenewEnabled = isMainInstance && config.getEnv('license.autoRenewEnabled');
const offlineMode = !isMainInstance;
const autoRenewOffset = config.getEnv('license.autoRenewOffset');
const saveCertStr = isMainInstance
Expand All @@ -66,13 +85,15 @@ export class License {
? async () => await this.usageMetricsService.collectUsageMetrics()
: async () => [];

const renewalEnabled = this.renewalEnabled(instanceType);

try {
this.manager = new LicenseManager({
server,
tenantId: config.getEnv('license.tenantId'),
productIdentifier: `n8n-${N8N_VERSION}`,
autoRenewEnabled,
renewOnInit: autoRenewEnabled,
autoRenewEnabled: renewalEnabled,
renewOnInit: renewalEnabled,
autoRenewOffset,
offlineMode,
logger: this.logger,
Expand Down Expand Up @@ -126,7 +147,7 @@ export class License {

if (this.orchestrationService.isMultiMainSetupEnabled && !isMultiMainLicensed) {
this.logger.debug(
'[Multi-main setup] License changed with no support for multi-main setup - no new followers will be allowed to init. To restore multi-main setup, please upgrade to a license that supporst this feature.',
'[Multi-main setup] License changed with no support for multi-main setup - no new followers will be allowed to init. To restore multi-main setup, please upgrade to a license that supports this feature.',
);
}
}
Expand Down Expand Up @@ -335,4 +356,9 @@ export class License {
isWithinUsersLimit() {
return this.getUsersLimit() === UNLIMITED_LICENSE_QUOTA;
}

async reinit() {
this.manager?.reset();
await this.init();
}
}
10 changes: 6 additions & 4 deletions packages/cli/src/commands/BaseCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ export abstract class BaseCommand extends Command {

protected shutdownService: ShutdownService = Container.get(ShutdownService);

protected license: License;

/**
* How long to wait for graceful shutdown before force killing the process.
*/
Expand Down Expand Up @@ -269,21 +271,21 @@ export abstract class BaseCommand extends Command {
}

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

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

if (activationKey) {
const hasCert = (await license.loadCertStr()).length > 0;
const hasCert = (await this.license.loadCertStr()).length > 0;

if (hasCert) {
return this.logger.debug('Skipping license activation');
}

try {
this.logger.debug('Attempting license activation');
await license.activate(activationKey);
await this.license.activate(activationKey);
this.logger.debug('License init complete');
} catch (e) {
this.logger.error('Could not activate license', e as Error);
Expand Down
2 changes: 2 additions & 0 deletions packages/cli/src/commands/start.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,9 +211,11 @@ export class Start extends BaseCommand {

orchestrationService.multiMainSetup
.on('leader-stepdown', async () => {
await this.license.reinit(); // to disable renewal
await this.activeWorkflowRunner.removeAllTriggerAndPollerBasedWorkflows();
})
.on('leader-takeover', async () => {
await this.license.reinit(); // to enable renewal
await this.activeWorkflowRunner.addAllTriggerAndPollerBasedWorkflows();
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,10 @@ export class MultiMainSetup extends EventEmitter {

config.set('multiMainSetup.instanceType', 'follower');

this.emit('leader-stepdown'); // lost leadership - stop triggers, pollers, pruning
/**
* Lost leadership - stop triggers, pollers, pruning, wait tracking, license renewal
*/
this.emit('leader-stepdown');

await this.tryBecomeLeader();
}
Expand All @@ -97,7 +100,10 @@ export class MultiMainSetup extends EventEmitter {

await this.redisPublisher.setExpiration(this.leaderKey, this.leaderKeyTtl);

this.emit('leader-takeover'); // gained leadership - start triggers, pollers, pruning, wait-tracking
/**
* Gained leadership - start triggers, pollers, pruning, wait-tracking, license renewal
*/
this.emit('leader-takeover');
} else {
config.set('multiMainSetup.instanceType', 'follower');
}
Expand Down
78 changes: 78 additions & 0 deletions packages/cli/test/unit/License.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,3 +175,81 @@ describe('License', () => {
expect(mainPlan).toBeUndefined();
});
});

describe('License', () => {
beforeEach(() => {
config.load(config.default);
});

describe('init', () => {
describe('in single-main setup', () => {
describe('with `license.autoRenewEnabled` enabled', () => {
it('should enable renewal', async () => {
config.set('multiMainSetup.enabled', false);

await new License(mock(), mock(), mock(), mock(), mock()).init();

expect(LicenseManager).toHaveBeenCalledWith(
expect.objectContaining({ autoRenewEnabled: true, renewOnInit: true }),
);
});
});

describe('with `license.autoRenewEnabled` disabled', () => {
it('should disable renewal', async () => {
config.set('license.autoRenewEnabled', false);

await new License(mock(), mock(), mock(), mock(), mock()).init();

expect(LicenseManager).toHaveBeenCalledWith(
expect.objectContaining({ autoRenewEnabled: false, renewOnInit: false }),
);
});
});
});

describe('in multi-main setup', () => {
describe('with `license.autoRenewEnabled` disabled', () => {
test.each(['unset', 'leader', 'follower'])(
'if %s status, should disable removal',
async (status) => {
config.set('multiMainSetup.enabled', true);
config.set('multiMainSetup.instanceType', status);
config.set('license.autoRenewEnabled', false);

await new License(mock(), mock(), mock(), mock(), mock()).init();

expect(LicenseManager).toHaveBeenCalledWith(
expect.objectContaining({ autoRenewEnabled: false, renewOnInit: false }),
);
},
);
});

describe('with `license.autoRenewEnabled` enabled', () => {
test.each(['unset', 'follower'])('if %s status, should disable removal', async (status) => {
config.set('multiMainSetup.enabled', true);
config.set('multiMainSetup.instanceType', status);
config.set('license.autoRenewEnabled', false);

await new License(mock(), mock(), mock(), mock(), mock()).init();

expect(LicenseManager).toHaveBeenCalledWith(
expect.objectContaining({ autoRenewEnabled: false, renewOnInit: false }),
);
});

it('if leader status, should enable renewal', async () => {
config.set('multiMainSetup.enabled', true);
config.set('multiMainSetup.instanceType', 'leader');

await new License(mock(), mock(), mock(), mock(), mock()).init();

expect(LicenseManager).toHaveBeenCalledWith(
expect.objectContaining({ autoRenewEnabled: true, renewOnInit: true }),
);
});
});
});
});
});
Loading