FIX: expert mode option handling and add a Lodestar heap size dropdown

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

@@ -107,17 +107,19 @@ test("flags containing dots are escaped in the pattern", () => {
   expect("  - --natXextip=80.249.121.1\n").not.toMatch(pattern);
 });
 
-test("lodestar can be given node runtime flags", () => {
+test("lodestar offers its heap size as a dropdown", () => {
   for (const service of ["LodestarBeaconService", "LodestarValidatorService"]) {
-    const option = optionsFor(service).find((o) => o.commands?.[0] === "NODE_OPTIONS: ");
+    const option = optionsFor(service).find((o) => o.commands?.[0] === "--max-old-space-size=");
 
     expect(option).toBeDefined();
-    // an environment variable, not a command flag: ExpertWindow needs isENV to
-    // put it in the env block instead of appending it to the command list
+    expect(option.type).toMatch("select");
+    // whole GB like Teku's -Xmx dropdown, while the flag is written in MB
+    expect(option.unit).toMatch("GB");
+    expect(option.unitFactor).toEqual(1024);
+    expect(option.value).toContain(8); // the 8192 MB the beacon ships with
+    // an environment variable, not a command flag: ExpertWindow needs isENV and
+    // envName to put it in the env block instead of the command list
     expect(option.isENV).toBe(true);
-    expect(option.type).toMatch("text");
-    // the command carries its own ": " separator
-    expect(option.commands[0].endsWith(": ")).toBe(true);
-    expect(option.placeholder).toMatch("--max-old-space-size=8192");
+    expect(option.envName).toMatch("NODE_OPTIONS");
   }
 });

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

@@ -0,0 +1,72 @@
+import { createPinia, setActivePinia } from "pinia";
+import { useServices } from "@/store/services";
+
+// Lodestar's heap is set through NODE_OPTIONS in MB, and the option offers it in
+// whole GB. These cover the pattern's side of that conversion the way
+// ExpertWindow uses it: group 2 is read for the dropdown and replaced on write,
+// so everything else the variable holds has to survive untouched.
+
+let option;
+
+beforeEach(() => {
+  setActivePinia(createPinia());
+  option = useServices()
+    .allServices.find((s) => s.service === "LodestarBeaconService")
+    .expertOptions.find((o) => o.commands?.[0] === "--max-old-space-size=");
+});
+
+const config = (env) => `service: LodestarBeaconService
+command:
+  - beacon
+  - --network=hoodi
+env:
+${env}
+image: chainsafe/lodestar:v1.46.0
+`;
+
+// what ExpertWindow's select branch does, so the option is exercised as used
+const read = (yaml) => {
+  const match = yaml.match(new RegExp(option.pattern[0]));
+  return match ? Number(match[2]) / option.unitFactor : "";
+};
+const write = (yaml, gb) => yaml.replace(new RegExp(option.pattern[0]), `$1${gb * option.unitFactor}$3`);
+
+test("the heap size is read as GB and written back as MB", () => {
+  const yaml = config("  NODE_OPTIONS: --max-old-space-size=8192");
+
+  expect(read(yaml)).toEqual(8);
+  expect(write(yaml, 16)).toContain("NODE_OPTIONS: --max-old-space-size=16384\n");
+});
+
+test("a quoted value keeps its quotes", () => {
+  const yaml = config('  NODE_OPTIONS: "--max-old-space-size=8192"');
+
+  expect(read(yaml)).toEqual(8);
+  expect(write(yaml, 4)).toContain('NODE_OPTIONS: "--max-old-space-size=4096"\n');
+});
+
+test("other node options are left alone", () => {
+  const yaml = config("  NODE_OPTIONS: --dns-result-order=ipv4first --max-old-space-size=8192 --trace-warnings");
+
+  expect(read(yaml)).toEqual(8);
+  expect(write(yaml, 12)).toContain("NODE_OPTIONS: --dns-result-order=ipv4first --max-old-space-size=12288 --trace-warnings\n");
+});
+
+test("other env vars are left alone", () => {
+  const yaml = config('  NODE_OPTIONS: --max-old-space-size=8192\n  SOME_OTHER: "--max-old-space-size=1"');
+
+  expect(write(yaml, 2)).toContain("NODE_OPTIONS: --max-old-space-size=2048\n");
+  expect(write(yaml, 2)).toContain('SOME_OTHER: "--max-old-space-size=1"');
+});
+
+test("every offered size converts to whole MB", () => {
+  for (const gb of option.value) {
+    expect(gb * option.unitFactor).toEqual(Math.round(gb * option.unitFactor));
+  }
+});
+
+test("a configuration without the flag does not match, so it has to be added", () => {
+  expect(read(config("  NODE_OPTIONS: --trace-warnings"))).toEqual("");
+  expect(read(config('  DRY_RUN: "false"'))).toEqual("");
+  expect("service: LodestarValidatorService\nenv: {}\n".match(new RegExp(option.pattern[0]))).toBeNull();
+});

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

@@ -0,0 +1,90 @@
+import { addCommandEntry, setEnvFlag } from "@/share/serviceYaml";
+
+// Expert mode adds a flag the configuration does not carry yet behind the last
+// one in the command list. Looking for that last flag in the whole file put it
+// inside Lodestar's NODE_OPTIONS instead, because an env var can hold "--"
+// values of its own and the env block comes after the command list.
+
+const lodestar = `service: LodestarBeaconService
+id: 6bc6f0d4-6df2-4d21-9a0d-b7a5e0b4a111
+configVersion: 1
+command:
+  - beacon
+  - --network=hoodi
+  - --dataDir=/opt/app/beacon
+entrypoint:
+  - node
+  - ./packages/cli/bin/lodestar
+env:
+  NODE_OPTIONS: --max-old-space-size=8192
+image: chainsafe/lodestar:v1.46.0
+`;
+
+test("an entry is added behind the last one in the command list", () => {
+  const result = addCommandEntry(lodestar, "--enr.ip=80.249.121.1");
+
+  expect(result).toContain("  - --dataDir=/opt/app/beacon\n  - --enr.ip=80.249.121.1\nentrypoint:");
+});
+
+test("an entry never lands inside an env var that holds flags of its own", () => {
+  const result = addCommandEntry(lodestar, "--enr.ip=80.249.121.1");
+
+  expect(result).toContain("NODE_OPTIONS: --max-old-space-size=8192\n");
+  expect(result).not.toMatch(/NODE_OPTIONS:.*--enr\.ip/);
+  // and the command list is the only place it shows up
+  expect(result.match(/--enr\.ip=80\.249\.121\.1/g)).toHaveLength(1);
+});
+
+test("the indentation of the command list is kept", () => {
+  const result = addCommandEntry("command:\n    - --network=hoodi\nenv: {}\n", "--enr.ip=1.2.3.4");
+
+  expect(result).toEqual("command:\n    - --network=hoodi\n    - --enr.ip=1.2.3.4\nenv: {}\n");
+});
+
+test("an entry that repeats an earlier one is still added at the end", () => {
+  const result = addCommandEntry("command:\n  - --metrics\n  - --network=hoodi\n  - --metrics\nenv: {}\n", "--enr.ip=1.2.3.4");
+
+  expect(result).toEqual("command:\n  - --metrics\n  - --network=hoodi\n  - --metrics\n  - --enr.ip=1.2.3.4\nenv: {}\n");
+});
+
+test("entries are inserted as text, not as a pattern", () => {
+  const result = addCommandEntry("command:\n  - --Network.ExternalIp=1.2.3.4\nenv: {}\n", "--Pruning.Mode=Full");
+
+  expect(result).toContain("  - --Pruning.Mode=Full\n");
+});
+
+test("a configuration without a command list cannot take an entry", () => {
+  expect(addCommandEntry('service: ValidatorEjectorService\ncommand: []\nenv:\n  DRY_RUN: "false"\n', "--x=1")).toBeNull();
+  expect(addCommandEntry("service: ValidatorEjectorService\nenv: {}\n", "--x=1")).toBeNull();
+});
+
+// A setting that lives inside an env var, such as Lodestar's heap size in
+// NODE_OPTIONS, has to be addable to a service that does not carry it yet.
+
+test("a flag is added to an env var that is already there", () => {
+  const result = setEnvFlag("env:\n  NODE_OPTIONS: --trace-warnings\nimage: x\n", "NODE_OPTIONS", "--max-old-space-size=8192");
+
+  expect(result).toEqual('env:\n  NODE_OPTIONS: "--trace-warnings --max-old-space-size=8192"\nimage: x\n');
+});
+
+test("quotes around an existing value are not doubled", () => {
+  const result = setEnvFlag('env:\n  NODE_OPTIONS: "--trace-warnings"\n', "NODE_OPTIONS", "--max-old-space-size=8192");
+
+  expect(result).toEqual('env:\n  NODE_OPTIONS: "--trace-warnings --max-old-space-size=8192"\n');
+});
+
+test("an empty env block becomes a block mapping to take the variable", () => {
+  const result = setEnvFlag("service: LodestarValidatorService\nenv: {}\nimage: x\n", "NODE_OPTIONS", "--max-old-space-size=4096");
+
+  expect(result).toEqual('service: LodestarValidatorService\nenv:\n  NODE_OPTIONS: "--max-old-space-size=4096"\nimage: x\n');
+});
+
+test("an env block with entries keeps them", () => {
+  const result = setEnvFlag('env:\n  DRY_RUN: "false"\nimage: x\n', "NODE_OPTIONS", "--max-old-space-size=4096");
+
+  expect(result).toEqual('env:\n  NODE_OPTIONS: "--max-old-space-size=4096"\n  DRY_RUN: "false"\nimage: x\n');
+});
+
+test("a configuration without an env block cannot take the variable", () => {
+  expect(setEnvFlag("service: LodestarValidatorService\nimage: x\n", "NODE_OPTIONS", "--max-old-space-size=4096")).toBeNull();
+});

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

@@ -150,7 +150,7 @@
             <img
               v-if="option.autoDetect === 'externalIp'"
               class="buttonOn"
-              :class="{ 'animate-spin pointer-events-none': option.detecting }"
+              :class="{ 'animate-pulse animate-duration-1000 pointer-events-none': option.detecting }"
               src="/img/icon/service-setting-icons/external-ip-update.png"
               :title="option.detecting ? 'detecting...' : 'detect the external IP of this node'"
               alt="detect"
@@ -316,6 +316,7 @@ import { mapState, mapWritableState } from "pinia";
 import { useNodeStore } from "@/store/theNode";
 import { useRestartService } from "@/composables/services";
 import { useSetups } from "@/store/setups";
+import { addCommandEntry, setEnvFlag } from "@/share/serviceYaml";
 
 export default {
   props: {
@@ -420,9 +421,10 @@ export default {
       this.item.expertOptions = this.item.expertOptions.map((option) => {
         switch (option.type) {
           case "select": {
-            option.changeValue = this.item.yaml.match(new RegExp(option.pattern[0]))
-              ? [...this.item.yaml.match(new RegExp(option.pattern[0]))][2]
-              : "";
+            const match = this.item.yaml.match(new RegExp(option.pattern[0]));
+            // a dropdown can offer another unit than the configuration stores:
+            // Lodestar's heap is set in MB and offered in GB, see unitFactor
+            option.changeValue = match ? (option.unitFactor ? Number(match[2]) / option.unitFactor : match[2]) : "";
             break;
           }
           case "text": {
@@ -476,30 +478,10 @@ export default {
                 this.somethingIsChanged(option);
               } else if (this.item.service === "NethermindService") {
                 if (!this.item.yaml.includes("Pruning.AvailableSpaceCheckEnabled=")) {
-                  const matchAllCommands = this.item.yaml.match(new RegExp(/--[\S]+/gm));
-                  const lastCommand = matchAllCommands[matchAllCommands.length - 1];
-                  const matchSpaces = this.item.yaml.match(new RegExp(`(\\s*- )${lastCommand}`));
-                  let spaces = " ";
-                  if (matchSpaces) {
-                    spaces = matchSpaces[1];
-                  }
-                  this.item.yaml = this.item.yaml.replace(
-                    new RegExp(`${lastCommand}`),
-                    lastCommand + spaces + "--Pruning.AvailableSpaceCheckEnabled=true"
-                  );
+                  this.item.yaml = addCommandEntry(this.item.yaml, "--Pruning.AvailableSpaceCheckEnabled=true") ?? this.item.yaml;
                 }
                 if (!this.item.yaml.includes("Pruning.FullPruningDisableLowPriorityWrites=")) {
-                  const matchAllCommands = this.item.yaml.match(new RegExp(/--[\S]+/gm));
-                  const lastCommand = matchAllCommands[matchAllCommands.length - 1];
-                  const matchSpaces = this.item.yaml.match(new RegExp(`(\\s*- )${lastCommand}`));
-                  let spaces = " ";
-                  if (matchSpaces) {
-                    spaces = matchSpaces[1];
-                  }
-                  this.item.yaml = this.item.yaml.replace(
-                    new RegExp(`${lastCommand}`),
-                    lastCommand + spaces + "--Pruning.FullPruningDisableLowPriorityWrites=false"
-                  );
+                  this.item.yaml = addCommandEntry(this.item.yaml, "--Pruning.FullPruningDisableLowPriorityWrites=false") ?? this.item.yaml;
                 }
               } else {
                 option.changeValue = false;
@@ -523,9 +505,17 @@ export default {
         if (option.changed) {
           switch (option.type) {
             case "select": {
+              const value = option.unitFactor ? option.changeValue * option.unitFactor : option.changeValue;
               option.commands.forEach((command) => {
-                if (option.changeValue && this.item.yaml.includes(command)) {
-                  this.item.yaml = this.item.yaml.replace(new RegExp(option.pattern[0]), `$1${option.changeValue}$3`);
+                if (!option.changeValue) return;
+                if (new RegExp(option.pattern[0]).test(this.item.yaml)) {
+                  this.item.yaml = this.item.yaml.replace(new RegExp(option.pattern[0]), `$1${value}$3`);
+                } else if (option.isENV && option.envName) {
+                  // a service that does not carry the setting yet, such as a
+                  // Lodestar validator with an empty env block
+                  const withFlag = setEnvFlag(this.item.yaml, option.envName, `${command}${value}`);
+                  if (withFlag) this.item.yaml = withFlag;
+                  else console.error(`No env block in the configuration to add ${option.envName} to`);
                 }
               });
               break;
@@ -543,14 +533,7 @@ export default {
                     }
                   }
                 } else if (option.changeValue == true && !this.item.yaml.includes(command)) {
-                  const matchAllCommands = this.item.yaml.match(new RegExp(/--[\S]+/gm));
-                  const lastCommand = matchAllCommands[matchAllCommands.length - 1];
-                  const matchSpaces = this.item.yaml.match(new RegExp(`(\\s*- )${lastCommand}`));
-                  let spaces = " ";
-                  if (matchSpaces) {
-                    spaces = matchSpaces[1];
-                  }
-                  this.item.yaml = this.item.yaml.replace(new RegExp(`${lastCommand}`), lastCommand + spaces + command);
+                  this.item.yaml = addCommandEntry(this.item.yaml, command) ?? this.item.yaml;
                 }
               });
               break;
@@ -621,19 +604,13 @@ export default {
                     console.error(`No env block in the configuration to add ${command}to`);
                   }
                 } else if (option.changeValue && !this.item.yaml.includes(command)) {
-                  let matchAllCommands = this.item.yaml.match(new RegExp(/--[\S]+/gm));
-                  if (matchAllCommands) {
-                    const lastCommand = matchAllCommands[matchAllCommands.length - 1];
-                    const matchSpaces = this.item.yaml.match(new RegExp(`(\\s*- )${lastCommand}`));
-                    let spaces = " ";
-                    if (matchSpaces) {
-                      spaces = matchSpaces[1];
-                    }
-
-                    this.item.yaml = this.item.yaml.replace(
-                      new RegExp(`${lastCommand}`),
-                      `${lastCommand}${spaces}${command}${option.noEqualSign ? "" : "="}${valuePrefix}${option.changeValue}`
-                    );
+                  // a flag belongs in the command list, and only that list is
+                  // searched for where to put it: an env var can hold flags of
+                  // its own, and Lodestar's NODE_OPTIONS used to catch these
+                  const entry = `${command}${option.noEqualSign ? "" : "="}${valuePrefix}${option.changeValue}`;
+                  const withEntry = addCommandEntry(this.item.yaml, entry);
+                  if (withEntry) {
+                    this.item.yaml = withEntry;
                   } else {
                     const matchENV = this.item.yaml.match(/env:([\s]+)/);
                     this.item.yaml = this.item.yaml.replace(
@@ -1237,6 +1214,7 @@ input:checked + .slider:before {
   height: 20px;
   cursor: pointer;
   justify-self: end;
+  align-self: center;
   margin-right: 10px;
 }
 .toggleTextBox .buttonOff {
@@ -1245,6 +1223,7 @@ input:checked + .slider:before {
   width: 20px;
   height: 20px;
   justify-self: end;
+  align-self: center;
   margin-right: 10px;
   cursor: pointer;
 }

launcher/src/share/serviceYaml.js

@@ -0,0 +1,57 @@
+/**
+ * Adds an entry to a service configuration's command list.
+ *
+ * Only the command list is searched for the entry to hang the new one behind.
+ * An env var can carry "--" values of its own - Lodestar's NODE_OPTIONS holds
+ * "--max-old-space-size" - and the env block comes after the command list in a
+ * configuration, so looking at the whole file would append the entry inside
+ * that variable instead of the command list.
+ *
+ * @param {string} yaml a service configuration
+ * @param {string} entry the entry to add, e.g. "--enr.ip=80.249.121.1"
+ * @returns {string|null} the configuration with the entry added, or null when
+ *   there is no command list to add it to
+ */
+export function addCommandEntry(yaml, entry) {
+  const block = /^command:[ \t]*\n((?:[ \t]*-[^\n]*\n?)+)/m.exec(yaml);
+  if (!block) return null;
+
+  const entries = block[1].split("\n").filter((line) => line.trim());
+  if (!entries.length) return null;
+
+  const last = entries[entries.length - 1];
+  const indent = /^[ \t]*-[ \t]*/.exec(last)[0];
+  const insertAt = block.index + block[0].lastIndexOf(last) + last.length;
+
+  return yaml.slice(0, insertAt) + "\n" + indent + entry + yaml.slice(insertAt);
+}
+
+/**
+ * Puts a flag into the env var that carries it.
+ *
+ * Whatever else the variable holds is kept - the flag is only one part of its
+ * value - and a service that does not have the variable at all gets it added to
+ * its env block, where "env: {}" has to become a block mapping first.
+ *
+ * @param {string} yaml a service configuration
+ * @param {string} envName the env var the flag belongs to, e.g. "NODE_OPTIONS"
+ * @param {string} flag the flag to put there, e.g. "--max-old-space-size=8192"
+ * @returns {string|null} the configuration with the flag set, or null when it
+ *   has no env block to add the variable to
+ */
+export function setEnvFlag(yaml, envName, flag) {
+  const line = new RegExp(`^([ \\t]*${envName}:[ \\t]*)(.*)$`, "m").exec(yaml);
+  if (line) {
+    const value = line[2].replace(/^["'`]|["'`]$/g, "").trim();
+    return yaml.replace(line[0], `${line[1]}"${value ? value + " " : ""}${flag}"`);
+  }
+
+  const entry = `  ${envName}: "${flag}"`;
+  if (/^env:[ \t]*\{[ \t]*\}[ \t]*$/m.test(yaml)) {
+    return yaml.replace(/^env:[ \t]*\{[ \t]*\}[ \t]*$/m, `env:\n${entry}`);
+  }
+  if (/^env:[ \t]*$/m.test(yaml)) {
+    return yaml.replace(/^env:[ \t]*$/m, `env:\n${entry}`);
+  }
+  return null;
+}