Skip to content

Commit 6bd7cbc

Browse files
NeoPlaysNeoPlays
authored
FIX: expert mode option handling and add a Lodestar heap size dropdown (#2315)
1 parent 56dde36 commit 6bd7cbc

6 files changed

Lines changed: 291 additions & 75 deletions

File tree

‎launcher/src/backend/tests/unit/ExternalIpOption.test.js‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -107,17 +107,19 @@ test("flags containing dots are escaped in the pattern", () => {
107107
expect(" - --natXextip=80.249.121.1\n").not.toMatch(pattern);
108108
});
109109

110-
test("lodestar can be given node runtime flags", () => {
110+
test("lodestar offers its heap size as a dropdown", () => {
111111
for (const service of ["LodestarBeaconService", "LodestarValidatorService"]) {
112-
const option = optionsFor(service).find((o) => o.commands?.[0] === "NODE_OPTIONS: ");
112+
const option = optionsFor(service).find((o) => o.commands?.[0] === "--max-old-space-size=");
113113

114114
expect(option).toBeDefined();
115-
// an environment variable, not a command flag: ExpertWindow needs isENV to
116-
// put it in the env block instead of appending it to the command list
115+
expect(option.type).toMatch("select");
116+
// whole GB like Teku's -Xmx dropdown, while the flag is written in MB
117+
expect(option.unit).toMatch("GB");
118+
expect(option.unitFactor).toEqual(1024);
119+
expect(option.value).toContain(8); // the 8192 MB the beacon ships with
120+
// an environment variable, not a command flag: ExpertWindow needs isENV and
121+
// envName to put it in the env block instead of the command list
117122
expect(option.isENV).toBe(true);
118-
expect(option.type).toMatch("text");
119-
// the command carries its own ": " separator
120-
expect(option.commands[0].endsWith(": ")).toBe(true);
121-
expect(option.placeholder).toMatch("--max-old-space-size=8192");
123+
expect(option.envName).toMatch("NODE_OPTIONS");
122124
}
123125
});

‎launcher/src/backend/tests/unit/MaxHeapSizeOption.test.js‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
import { createPinia, setActivePinia } from "pinia";
2+
import { useServices } from "@/store/services";
3+
4+
// Lodestar's heap is set through NODE_OPTIONS in MB, and the option offers it in
5+
// whole GB. These cover the pattern's side of that conversion the way
6+
// ExpertWindow uses it: group 2 is read for the dropdown and replaced on write,
7+
// so everything else the variable holds has to survive untouched.
8+
9+
let option;
10+
11+
beforeEach(() => {
12+
setActivePinia(createPinia());
13+
option = useServices()
14+
.allServices.find((s) => s.service === "LodestarBeaconService")
15+
.expertOptions.find((o) => o.commands?.[0] === "--max-old-space-size=");
16+
});
17+
18+
const config = (env) => `service: LodestarBeaconService
19+
command:
20+
- beacon
21+
- --network=hoodi
22+
env:
23+
${env}
24+
image: chainsafe/lodestar:v1.46.0
25+
`;
26+
27+
// what ExpertWindow's select branch does, so the option is exercised as used
28+
const read = (yaml) => {
29+
const match = yaml.match(new RegExp(option.pattern[0]));
30+
return match ? Number(match[2]) / option.unitFactor : "";
31+
};
32+
const write = (yaml, gb) => yaml.replace(new RegExp(option.pattern[0]), `$1${gb * option.unitFactor}$3`);
33+
34+
test("the heap size is read as GB and written back as MB", () => {
35+
const yaml = config(" NODE_OPTIONS: --max-old-space-size=8192");
36+
37+
expect(read(yaml)).toEqual(8);
38+
expect(write(yaml, 16)).toContain("NODE_OPTIONS: --max-old-space-size=16384\n");
39+
});
40+
41+
test("a quoted value keeps its quotes", () => {
42+
const yaml = config(' NODE_OPTIONS: "--max-old-space-size=8192"');
43+
44+
expect(read(yaml)).toEqual(8);
45+
expect(write(yaml, 4)).toContain('NODE_OPTIONS: "--max-old-space-size=4096"\n');
46+
});
47+
48+
test("other node options are left alone", () => {
49+
const yaml = config(" NODE_OPTIONS: --dns-result-order=ipv4first --max-old-space-size=8192 --trace-warnings");
50+
51+
expect(read(yaml)).toEqual(8);
52+
expect(write(yaml, 12)).toContain("NODE_OPTIONS: --dns-result-order=ipv4first --max-old-space-size=12288 --trace-warnings\n");
53+
});
54+
55+
test("other env vars are left alone", () => {
56+
const yaml = config(' NODE_OPTIONS: --max-old-space-size=8192\n SOME_OTHER: "--max-old-space-size=1"');
57+
58+
expect(write(yaml, 2)).toContain("NODE_OPTIONS: --max-old-space-size=2048\n");
59+
expect(write(yaml, 2)).toContain('SOME_OTHER: "--max-old-space-size=1"');
60+
});
61+
62+
test("every offered size converts to whole MB", () => {
63+
for (const gb of option.value) {
64+
expect(gb * option.unitFactor).toEqual(Math.round(gb * option.unitFactor));
65+
}
66+
});
67+
68+
test("a configuration without the flag does not match, so it has to be added", () => {
69+
expect(read(config(" NODE_OPTIONS: --trace-warnings"))).toEqual("");
70+
expect(read(config(' DRY_RUN: "false"'))).toEqual("");
71+
expect("service: LodestarValidatorService\nenv: {}\n".match(new RegExp(option.pattern[0]))).toBeNull();
72+
});

‎launcher/src/backend/tests/unit/ServiceYaml.test.js‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
import { addCommandEntry, setEnvFlag } from "@/share/serviceYaml";
2+
3+
// Expert mode adds a flag the configuration does not carry yet behind the last
4+
// one in the command list. Looking for that last flag in the whole file put it
5+
// inside Lodestar's NODE_OPTIONS instead, because an env var can hold "--"
6+
// values of its own and the env block comes after the command list.
7+
8+
const lodestar = `service: LodestarBeaconService
9+
id: 6bc6f0d4-6df2-4d21-9a0d-b7a5e0b4a111
10+
configVersion: 1
11+
command:
12+
- beacon
13+
- --network=hoodi
14+
- --dataDir=/opt/app/beacon
15+
entrypoint:
16+
- node
17+
- ./packages/cli/bin/lodestar
18+
env:
19+
NODE_OPTIONS: --max-old-space-size=8192
20+
image: chainsafe/lodestar:v1.46.0
21+
`;
22+
23+
test("an entry is added behind the last one in the command list", () => {
24+
const result = addCommandEntry(lodestar, "--enr.ip=80.249.121.1");
25+
26+
expect(result).toContain(" - --dataDir=/opt/app/beacon\n - --enr.ip=80.249.121.1\nentrypoint:");
27+
});
28+
29+
test("an entry never lands inside an env var that holds flags of its own", () => {
30+
const result = addCommandEntry(lodestar, "--enr.ip=80.249.121.1");
31+
32+
expect(result).toContain("NODE_OPTIONS: --max-old-space-size=8192\n");
33+
expect(result).not.toMatch(/NODE_OPTIONS:.*--enr\.ip/);
34+
// and the command list is the only place it shows up
35+
expect(result.match(/--enr\.ip=80\.249\.121\.1/g)).toHaveLength(1);
36+
});
37+
38+
test("the indentation of the command list is kept", () => {
39+
const result = addCommandEntry("command:\n - --network=hoodi\nenv: {}\n", "--enr.ip=1.2.3.4");
40+
41+
expect(result).toEqual("command:\n - --network=hoodi\n - --enr.ip=1.2.3.4\nenv: {}\n");
42+
});
43+
44+
test("an entry that repeats an earlier one is still added at the end", () => {
45+
const result = addCommandEntry("command:\n - --metrics\n - --network=hoodi\n - --metrics\nenv: {}\n", "--enr.ip=1.2.3.4");
46+
47+
expect(result).toEqual("command:\n - --metrics\n - --network=hoodi\n - --metrics\n - --enr.ip=1.2.3.4\nenv: {}\n");
48+
});
49+
50+
test("entries are inserted as text, not as a pattern", () => {
51+
const result = addCommandEntry("command:\n - --Network.ExternalIp=1.2.3.4\nenv: {}\n", "--Pruning.Mode=Full");
52+
53+
expect(result).toContain(" - --Pruning.Mode=Full\n");
54+
});
55+
56+
test("a configuration without a command list cannot take an entry", () => {
57+
expect(addCommandEntry('service: ValidatorEjectorService\ncommand: []\nenv:\n DRY_RUN: "false"\n', "--x=1")).toBeNull();
58+
expect(addCommandEntry("service: ValidatorEjectorService\nenv: {}\n", "--x=1")).toBeNull();
59+
});
60+
61+
// A setting that lives inside an env var, such as Lodestar's heap size in
62+
// NODE_OPTIONS, has to be addable to a service that does not carry it yet.
63+
64+
test("a flag is added to an env var that is already there", () => {
65+
const result = setEnvFlag("env:\n NODE_OPTIONS: --trace-warnings\nimage: x\n", "NODE_OPTIONS", "--max-old-space-size=8192");
66+
67+
expect(result).toEqual('env:\n NODE_OPTIONS: "--trace-warnings --max-old-space-size=8192"\nimage: x\n');
68+
});
69+
70+
test("quotes around an existing value are not doubled", () => {
71+
const result = setEnvFlag('env:\n NODE_OPTIONS: "--trace-warnings"\n', "NODE_OPTIONS", "--max-old-space-size=8192");
72+
73+
expect(result).toEqual('env:\n NODE_OPTIONS: "--trace-warnings --max-old-space-size=8192"\n');
74+
});
75+
76+
test("an empty env block becomes a block mapping to take the variable", () => {
77+
const result = setEnvFlag("service: LodestarValidatorService\nenv: {}\nimage: x\n", "NODE_OPTIONS", "--max-old-space-size=4096");
78+
79+
expect(result).toEqual('service: LodestarValidatorService\nenv:\n NODE_OPTIONS: "--max-old-space-size=4096"\nimage: x\n');
80+
});
81+
82+
test("an env block with entries keeps them", () => {
83+
const result = setEnvFlag('env:\n DRY_RUN: "false"\nimage: x\n', "NODE_OPTIONS", "--max-old-space-size=4096");
84+
85+
expect(result).toEqual('env:\n NODE_OPTIONS: "--max-old-space-size=4096"\n DRY_RUN: "false"\nimage: x\n');
86+
});
87+
88+
test("a configuration without an env block cannot take the variable", () => {
89+
expect(setEnvFlag("service: LodestarValidatorService\nimage: x\n", "NODE_OPTIONS", "--max-old-space-size=4096")).toBeNull();
90+
});

‎launcher/src/components/UI/node-page/sections/ExpertWindow.vue‎

Lines changed: 28 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@
150150
<img
151151
v-if="option.autoDetect === 'externalIp'"
152152
class="buttonOn"
153-
:class="{ 'animate-spin pointer-events-none': option.detecting }"
153+
:class="{ 'animate-pulse animate-duration-1000 pointer-events-none': option.detecting }"
154154
src="/img/icon/service-setting-icons/external-ip-update.png"
155155
:title="option.detecting ? 'detecting...' : 'detect the external IP of this node'"
156156
alt="detect"
@@ -316,6 +316,7 @@ import { mapState, mapWritableState } from "pinia";
316316
import { useNodeStore } from "@/store/theNode";
317317
import { useRestartService } from "@/composables/services";
318318
import { useSetups } from "@/store/setups";
319+
import { addCommandEntry, setEnvFlag } from "@/share/serviceYaml";
319320
320321
export default {
321322
props: {
@@ -420,9 +421,10 @@ export default {
420421
this.item.expertOptions = this.item.expertOptions.map((option) => {
421422
switch (option.type) {
422423
case "select": {
423-
option.changeValue = this.item.yaml.match(new RegExp(option.pattern[0]))
424-
? [...this.item.yaml.match(new RegExp(option.pattern[0]))][2]
425-
: "";
424+
const match = this.item.yaml.match(new RegExp(option.pattern[0]));
425+
// a dropdown can offer another unit than the configuration stores:
426+
// Lodestar's heap is set in MB and offered in GB, see unitFactor
427+
option.changeValue = match ? (option.unitFactor ? Number(match[2]) / option.unitFactor : match[2]) : "";
426428
break;
427429
}
428430
case "text": {
@@ -476,30 +478,10 @@ export default {
476478
this.somethingIsChanged(option);
477479
} else if (this.item.service === "NethermindService") {
478480
if (!this.item.yaml.includes("Pruning.AvailableSpaceCheckEnabled=")) {
479-
const matchAllCommands = this.item.yaml.match(new RegExp(/--[\S]+/gm));
480-
const lastCommand = matchAllCommands[matchAllCommands.length - 1];
481-
const matchSpaces = this.item.yaml.match(new RegExp(`(\\s*- )${lastCommand}`));
482-
let spaces = " ";
483-
if (matchSpaces) {
484-
spaces = matchSpaces[1];
485-
}
486-
this.item.yaml = this.item.yaml.replace(
487-
new RegExp(`${lastCommand}`),
488-
lastCommand + spaces + "--Pruning.AvailableSpaceCheckEnabled=true"
489-
);
481+
this.item.yaml = addCommandEntry(this.item.yaml, "--Pruning.AvailableSpaceCheckEnabled=true") ?? this.item.yaml;
490482
}
491483
if (!this.item.yaml.includes("Pruning.FullPruningDisableLowPriorityWrites=")) {
492-
const matchAllCommands = this.item.yaml.match(new RegExp(/--[\S]+/gm));
493-
const lastCommand = matchAllCommands[matchAllCommands.length - 1];
494-
const matchSpaces = this.item.yaml.match(new RegExp(`(\\s*- )${lastCommand}`));
495-
let spaces = " ";
496-
if (matchSpaces) {
497-
spaces = matchSpaces[1];
498-
}
499-
this.item.yaml = this.item.yaml.replace(
500-
new RegExp(`${lastCommand}`),
501-
lastCommand + spaces + "--Pruning.FullPruningDisableLowPriorityWrites=false"
502-
);
484+
this.item.yaml = addCommandEntry(this.item.yaml, "--Pruning.FullPruningDisableLowPriorityWrites=false") ?? this.item.yaml;
503485
}
504486
} else {
505487
option.changeValue = false;
@@ -523,9 +505,17 @@ export default {
523505
if (option.changed) {
524506
switch (option.type) {
525507
case "select": {
508+
const value = option.unitFactor ? option.changeValue * option.unitFactor : option.changeValue;
526509
option.commands.forEach((command) => {
527-
if (option.changeValue && this.item.yaml.includes(command)) {
528-
this.item.yaml = this.item.yaml.replace(new RegExp(option.pattern[0]), `$1${option.changeValue}$3`);
510+
if (!option.changeValue) return;
511+
if (new RegExp(option.pattern[0]).test(this.item.yaml)) {
512+
this.item.yaml = this.item.yaml.replace(new RegExp(option.pattern[0]), `$1${value}$3`);
513+
} else if (option.isENV && option.envName) {
514+
// a service that does not carry the setting yet, such as a
515+
// Lodestar validator with an empty env block
516+
const withFlag = setEnvFlag(this.item.yaml, option.envName, `${command}${value}`);
517+
if (withFlag) this.item.yaml = withFlag;
518+
else console.error(`No env block in the configuration to add ${option.envName} to`);
529519
}
530520
});
531521
break;
@@ -543,14 +533,7 @@ export default {
543533
}
544534
}
545535
} else if (option.changeValue == true && !this.item.yaml.includes(command)) {
546-
const matchAllCommands = this.item.yaml.match(new RegExp(/--[\S]+/gm));
547-
const lastCommand = matchAllCommands[matchAllCommands.length - 1];
548-
const matchSpaces = this.item.yaml.match(new RegExp(`(\\s*- )${lastCommand}`));
549-
let spaces = " ";
550-
if (matchSpaces) {
551-
spaces = matchSpaces[1];
552-
}
553-
this.item.yaml = this.item.yaml.replace(new RegExp(`${lastCommand}`), lastCommand + spaces + command);
536+
this.item.yaml = addCommandEntry(this.item.yaml, command) ?? this.item.yaml;
554537
}
555538
});
556539
break;
@@ -621,19 +604,13 @@ export default {
621604
console.error(`No env block in the configuration to add ${command}to`);
622605
}
623606
} else if (option.changeValue && !this.item.yaml.includes(command)) {
624-
let matchAllCommands = this.item.yaml.match(new RegExp(/--[\S]+/gm));
625-
if (matchAllCommands) {
626-
const lastCommand = matchAllCommands[matchAllCommands.length - 1];
627-
const matchSpaces = this.item.yaml.match(new RegExp(`(\\s*- )${lastCommand}`));
628-
let spaces = " ";
629-
if (matchSpaces) {
630-
spaces = matchSpaces[1];
631-
}
632-
633-
this.item.yaml = this.item.yaml.replace(
634-
new RegExp(`${lastCommand}`),
635-
`${lastCommand}${spaces}${command}${option.noEqualSign ? "" : "="}${valuePrefix}${option.changeValue}`
636-
);
607+
// a flag belongs in the command list, and only that list is
608+
// searched for where to put it: an env var can hold flags of
609+
// its own, and Lodestar's NODE_OPTIONS used to catch these
610+
const entry = `${command}${option.noEqualSign ? "" : "="}${valuePrefix}${option.changeValue}`;
611+
const withEntry = addCommandEntry(this.item.yaml, entry);
612+
if (withEntry) {
613+
this.item.yaml = withEntry;
637614
} else {
638615
const matchENV = this.item.yaml.match(/env:([\s]+)/);
639616
this.item.yaml = this.item.yaml.replace(
@@ -1237,6 +1214,7 @@ input:checked + .slider:before {
12371214
height: 20px;
12381215
cursor: pointer;
12391216
justify-self: end;
1217+
align-self: center;
12401218
margin-right: 10px;
12411219
}
12421220
.toggleTextBox .buttonOff {
@@ -1245,6 +1223,7 @@ input:checked + .slider:before {
12451223
width: 20px;
12461224
height: 20px;
12471225
justify-self: end;
1226+
align-self: center;
12481227
margin-right: 10px;
12491228
cursor: pointer;
12501229
}

‎launcher/src/share/serviceYaml.js‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
/**
2+
* Adds an entry to a service configuration's command list.
3+
*
4+
* Only the command list is searched for the entry to hang the new one behind.
5+
* An env var can carry "--" values of its own - Lodestar's NODE_OPTIONS holds
6+
* "--max-old-space-size" - and the env block comes after the command list in a
7+
* configuration, so looking at the whole file would append the entry inside
8+
* that variable instead of the command list.
9+
*
10+
* @param {string} yaml a service configuration
11+
* @param {string} entry the entry to add, e.g. "--enr.ip=80.249.121.1"
12+
* @returns {string|null} the configuration with the entry added, or null when
13+
* there is no command list to add it to
14+
*/
15+
export function addCommandEntry(yaml, entry) {
16+
const block = /^command:[ \t]*\n((?:[ \t]*-[^\n]*\n?)+)/m.exec(yaml);
17+
if (!block) return null;
18+
19+
const entries = block[1].split("\n").filter((line) => line.trim());
20+
if (!entries.length) return null;
21+
22+
const last = entries[entries.length - 1];
23+
const indent = /^[ \t]*-[ \t]*/.exec(last)[0];
24+
const insertAt = block.index + block[0].lastIndexOf(last) + last.length;
25+
26+
return yaml.slice(0, insertAt) + "\n" + indent + entry + yaml.slice(insertAt);
27+
}
28+
29+
/**
30+
* Puts a flag into the env var that carries it.
31+
*
32+
* Whatever else the variable holds is kept - the flag is only one part of its
33+
* value - and a service that does not have the variable at all gets it added to
34+
* its env block, where "env: {}" has to become a block mapping first.
35+
*
36+
* @param {string} yaml a service configuration
37+
* @param {string} envName the env var the flag belongs to, e.g. "NODE_OPTIONS"
38+
* @param {string} flag the flag to put there, e.g. "--max-old-space-size=8192"
39+
* @returns {string|null} the configuration with the flag set, or null when it
40+
* has no env block to add the variable to
41+
*/
42+
export function setEnvFlag(yaml, envName, flag) {
43+
const line = new RegExp(`^([ \\t]*${envName}:[ \\t]*)(.*)$`, "m").exec(yaml);
44+
if (line) {
45+
const value = line[2].replace(/^["'`]|["'`]$/g, "").trim();
46+
return yaml.replace(line[0], `${line[1]}"${value ? value + " " : ""}${flag}"`);
47+
}
48+
49+
const entry = ` ${envName}: "${flag}"`;
50+
if (/^env:[ \t]*\{[ \t]*\}[ \t]*$/m.test(yaml)) {
51+
return yaml.replace(/^env:[ \t]*\{[ \t]*\}[ \t]*$/m, `env:\n${entry}`);
52+
}
53+
if (/^env:[ \t]*$/m.test(yaml)) {
54+
return yaml.replace(/^env:[ \t]*$/m, `env:\n${entry}`);
55+
}
56+
return null;
57+
}

0 commit comments

Comments
 (0)