feat(core): Add command to trigger license refresh on workers #7184

flipswitchingmonkey · 2023-09-15T13:20:58Z

This PR implements the updated license SDK so that worker and webhook instances do not auto-renew licenses any more.

Instead, they receive a reloadLicense command via the Redis client that will fetch the updated license after it was saved on the main instance

This also contains some refactoring with moving redis sub and pub clients into the event bus directly, to prevent cyclic dependency issues.

github-actions · 2023-09-15T13:21:15Z

Great PR! Please pay attention to the following items before merging:

Files matching packages/**:

If fixing bug, added test to cover scenario.
If addressing forum or Github issue, added link to description.

Files matching packages/**/*.ts:

Added unit tests to cover new or updated functionality.

Make sure to check off this list before asking for review.

codecov · 2023-09-15T13:40:30Z

Codecov Report

Patch coverage: 43.60% and project coverage change: -0.01% ⚠️

Comparison is base (d317e09) 32.66% compared to head (fc908ec) 32.65%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7184      +/-   ##
==========================================
- Coverage   32.66%   32.65%   -0.01%     
==========================================
  Files        3337     3340       +3     
  Lines      199394   199463      +69     
  Branches    21833    21849      +16     
==========================================
+ Hits        65129    65132       +3     
- Misses     133189   133251      +62     
- Partials     1076     1080       +4

Files Changed	Coverage Δ
packages/cli/src/Server.ts	`0.00% <0.00%> (ø)`
packages/cli/src/commands/BaseCommand.ts	`47.05% <0.00%> (ø)`
packages/cli/src/commands/start.ts	`0.00% <0.00%> (ø)`
packages/cli/src/commands/webhook.ts	`0.00% <0.00%> (ø)`
packages/cli/src/controllers/e2e.controller.ts	`0.00% <0.00%> (ø)`
...es/cli/src/controllers/orchestration.controller.ts	`0.00% <0.00%> (ø)`
...src/services/redis/RedisServicePubSubSubscriber.ts	`47.22% <0.00%> (-35.39%)`	⬇️
...li/src/eventbus/MessageEventBus/MessageEventBus.ts	`39.74% <30.61%> (-5.90%)`	⬇️
packages/cli/src/services/orchestration.service.ts	`55.31% <42.85%> (-10.07%)`	⬇️
packages/cli/src/worker/workerCommandHandler.ts	`22.58% <50.00%> (+1.89%)`	⬆️
... and 6 more

... and 4 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

csuermann · 2023-09-15T13:25:10Z

packages/cli/src/License.ts

-		if (this.manager) {
+	async init(instanceId: string, instanceType: N8nInstanceType = 'main') {
+		const isMainInstance = instanceType === 'main';
+		if (this.manager && isMainInstance) {


why is && isMainInstance needed here?

because we dont want to redo the init() on the main instances (since they were the onces who actually did the license renewal), only on the workers. so if the manager was already initialised on main, just return

if we can use the reload() instead of init(), maybe we don't even need all this

csuermann · 2023-09-15T13:36:16Z

packages/cli/src/services/orchestration/handleCommandMessage.ts

+			case 'reloadLicense':
+				const instanceId = Container.get(License).instanceId;
+				if (instanceId) {
+					await Container.get(License).init(instanceId, 'main');


can we use the License SDK's reload() function instead?

If that effectively does the same, sure!

…resh-on-workers

csuermann

LGTM

cypress · 2023-09-17T08:11:31Z

1 flaky test on run #2190 ↗︎

0	238	3	0	1

Details:

🌳 pay-827-add-command-to-trigger-license-refresh-on-workers 🖥️ browsers:node18...
Project: n8n	Commit: `fc908ecd6b`
Status: Passed	Duration: 07:38 💡
Started: Sep 17, 2023 8:03 AM	Ended: Sep 17, 2023 8:11 AM

cypress/e2e/24-ndv-paired-item.cy.ts • 1 flaky test

View Output Video

Test		Artifacts
NDV > resolves expression with default item when input node is not parent, while still pairing items		`Output` `Screenshots` `Video`

_{This comment has been generated by cypress-bot as a result of this
project's GitHub integration settings.}

github-actions · 2023-09-17T08:11:36Z

✅ All Cypress E2E specs passed

* master: feat(core): Add command to trigger license refresh on workers (#7184) fix(X (Formerly Twitter) Node): Rename Twitter to X (keep Twitter alias) (#7179)

# [1.8.0](https://github.com/n8n-io/n8n/compare/n8n@1.7.0...n8n@1.8.0) (2023-09-20) ### Bug Fixes * **core:** Make parsing of content-type and content-disposition headers more flexible ([#7217](#7217)) ([d41546b](d41546b)), closes [#7149](#7149) * **core:** Resolve domains to IPv4 first ([#7206](#7206)) ([e9ce531](e9ce531)) * **editor:** Add ssh key type selection to source control settings when regenerating key ([#7172](#7172)) ([54bf66d](54bf66d)) * **editor:** No need to add click emitting click events, VUE delegates the handler to the root element of the component ([#7182](#7182)) ([3c055e4](3c055e4)) * **editor:** Prevent duplicate creation of credential for OAuth2 ([#7163](#7163)) ([07a6417](07a6417)) * **editor:** Testing flaky resource mapper feature in e2e tests ([#7165](#7165)) ([aaf87c3](aaf87c3)) * **HTML Node:** Add pairedItem support for 'Convert to HTML Table' operation ([#7196](#7196)) ([6bc477b](6bc477b)) * **HTTP Request Node:** Decrease default timeout to 5min ([#7177](#7177)) ([321780d](321780d)) * **seven Node:** Rename sms77 to seven, fix credentials test ([#7180](#7180)) ([cf776b8](cf776b8)) * **X (Formerly Twitter) Node:** Rename Twitter to X (keep Twitter alias) ([#7179](#7179)) ([d317e09](d317e09)) ### Features * **core:** Add command to trigger license refresh on workers ([#7184](#7184)) ([9f797b9](9f797b9)) * **core:** Add rsa option to ssh key generation ([#7154](#7154)) ([fdac2c8](fdac2c8)) * **Linear Node:** Add support for OAuth2 ([#7201](#7201)) ([12a3168](12a3168)) * **Microsoft Outlook Node:** Node overhaul ([#4449](#4449)) ([556a613](556a613)) * **Set Node:** Overhaul ([#6348](#6348)) ([3a47455](3a47455)) Co-authored-by: krynble <krynble@users.noreply.github.com>

This reverts commit 9f797b9.

## [1.8.1](https://github.com/n8n-io/n8n/compare/n8n@1.8.0...n8n@1.8.1) (2023-09-21) ### Bug Fixes * **Airtable Node:** Attachments field type fix ([#7227](#7227)) ([ebfcc87](ebfcc87)) * **core:** Handle filename* with quotes in Content-Disposition header ([#7229](#7229)) ([fb36b0a](fb36b0a)) * Ensure new Set node is on top of search list ([#7215](#7215)) ([c8fb06f](c8fb06f)) * Issue enforcing user limits on start plan ([#7188](#7188)) ([50a4c5e](50a4c5e)) ### Reverts * **core:** Add command to trigger license refresh on workers ([#7184](#7184)) ([6ac19a5](6ac19a5)) Co-authored-by: netroy <netroy@users.noreply.github.com>

janober · 2023-09-21T16:37:53Z

Got released with n8n@1.8.1

flipswitchingmonkey added 4 commits September 15, 2023 11:08

add reloadLicense

40db942

remove cyclic deps and handle events

bf9ceb8

fix eventbus cleanup and calling reload twice

13dac38

remove debug function

421377e

flipswitchingmonkey marked this pull request as ready for review September 15, 2023 13:23

n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team labels Sep 15, 2023

csuermann reviewed Sep 15, 2023

View reviewed changes

use reload from sdk instead of init

4226a2b

flipswitchingmonkey requested a review from csuermann September 15, 2023 14:20

flipswitchingmonkey added 2 commits September 15, 2023 16:53

Merge branch 'master' into pay-827-add-command-to-trigger-license-ref…

00fd91a

…resh-on-workers

fix test

fc908ec

flipswitchingmonkey requested a review from krynble September 15, 2023 15:49

csuermann approved these changes Sep 17, 2023

View reviewed changes

flipswitchingmonkey merged commit 9f797b9 into master Sep 17, 2023
52 of 53 checks passed

flipswitchingmonkey deleted the pay-827-add-command-to-trigger-license-refresh-on-workers branch September 17, 2023 09:05

github-actions bot mentioned this pull request Sep 20, 2023

🚀 Release 1.8.0 #7219

Merged

netroy added a commit that referenced this pull request Sep 21, 2023

revert(core): Add command to trigger license refresh on workers (#7184)

4ce85c1

This reverts commit 9f797b9.

netroy added a commit that referenced this pull request Sep 21, 2023

revert(core): Add command to trigger license refresh on workers (#7184)

6ac19a5

This reverts commit 9f797b9.

github-actions bot mentioned this pull request Sep 21, 2023

🚀 Release 1.8.1 #7231

Merged

janober added the Released label Sep 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(core): Add command to trigger license refresh on workers #7184

feat(core): Add command to trigger license refresh on workers #7184

flipswitchingmonkey commented Sep 15, 2023

github-actions bot commented Sep 15, 2023

codecov bot commented Sep 15, 2023 •

edited

csuermann Sep 15, 2023

flipswitchingmonkey Sep 15, 2023

csuermann Sep 15, 2023

flipswitchingmonkey Sep 15, 2023

csuermann left a comment

cypress bot commented Sep 17, 2023

github-actions bot commented Sep 17, 2023

janober commented Sep 21, 2023

feat(core): Add command to trigger license refresh on workers #7184

feat(core): Add command to trigger license refresh on workers #7184

Conversation

flipswitchingmonkey commented Sep 15, 2023

github-actions bot commented Sep 15, 2023

codecov bot commented Sep 15, 2023 • edited

Codecov Report

csuermann Sep 15, 2023

Choose a reason for hiding this comment

flipswitchingmonkey Sep 15, 2023

Choose a reason for hiding this comment

csuermann Sep 15, 2023

Choose a reason for hiding this comment

flipswitchingmonkey Sep 15, 2023

Choose a reason for hiding this comment

csuermann left a comment

Choose a reason for hiding this comment

cypress bot commented Sep 17, 2023

1 flaky test on run #2190 ↗︎

github-actions bot commented Sep 17, 2023

janober commented Sep 21, 2023

codecov bot commented Sep 15, 2023 •

edited