From 73cc019d2f2b49db753498ef64037176be9851d8 Mon Sep 17 00:00:00 2001 From: Cooldude2606 <25043174+Cooldude2606@users.noreply.github.com> Date: Sat, 1 Nov 2025 16:11:11 +0000 Subject: [PATCH] Sets restart required when a plugin is updated (#794) * Template type for load plugin config * Add plugin update checking * Add restart icon to instance list * Add tests * Log package name when failing to read package json --- packages/controller/src/ControlConnection.ts | 4 +- packages/controller/src/Controller.ts | 55 ++++++++++++----- packages/controller/src/routes.ts | 4 +- packages/host/src/Host.ts | 55 ++++++++++++----- packages/host/src/Instance.ts | 2 +- packages/lib/src/config/definitions.ts | 3 + .../web_ui/src/components/ControllerPage.tsx | 7 ++- .../web_ui/src/components/HostViewPage.tsx | 7 ++- .../web_ui/src/components/InstanceList.tsx | 20 +++--- test/controller/Controller.js | 61 ++++++++++++++++++- test/host.js | 59 ++++++++++++++++++ 11 files changed, 228 insertions(+), 49 deletions(-) diff --git a/packages/controller/src/ControlConnection.ts b/packages/controller/src/ControlConnection.ts index 8b6f4f28..c54a3c45 100644 --- a/packages/controller/src/ControlConnection.ts +++ b/packages/controller/src/ControlConnection.ts @@ -1079,9 +1079,7 @@ export default class ControlConnection extends BaseConnection { return this._controller.pluginInfos.map(pluginInfo => lib.PluginDetails.fromNodeEnvInfo( pluginInfo, this._controller.plugins.has(pluginInfo.name), - this._controller.config.get( - `${pluginInfo.name}.load_plugin` as keyof lib.ControllerConfigFields - ) as boolean, + this._controller.config.get(`${pluginInfo.name}.load_plugin`), )); } diff --git a/packages/controller/src/Controller.ts b/packages/controller/src/Controller.ts index 5d2b8f07..e3d13d02 100644 --- a/packages/controller/src/Controller.ts +++ b/packages/controller/src/Controller.ts @@ -549,19 +549,8 @@ export default class Controller { } requests.push(hostConnection.send(new lib.SystemInfoRequest())); } - if (!this.config.restartRequired) { - // If a restart isn't already required, then test if a new version is installed - try { - const runningVersion = this.config.get("controller.version"); - const packageJson = await fs.readJSON(path.join(__dirname, "..", "package.json")); - if (runningVersion !== packageJson.version) { - this.config.restartRequired = true; - } - } catch (err: any) { - logger.warn(`Failed to read package json:\n${err.stack ?? err.message}`); - } - } - requests.push(lib.gatherSystemInfo("controller", this.canRestart, this.config.restartRequired)); + const restartRequired = await this.checkRestartRequired(); + requests.push(lib.gatherSystemInfo("controller", this.canRestart, restartRequired)); const newMetrics = await Promise.all(requests); for (const metric of newMetrics) { this.systems.set(metric); @@ -571,6 +560,44 @@ export default class Controller { } } + async checkRestartRequired() { + if (this.config.restartRequired) { + return true; + } + + let packageName = "@clusterio/controller"; + try { + // First check the clusterio version + const runningVersion = this.config.get("controller.version"); + const packageJson = await fs.readJSON(path.join(__dirname, "..", "package.json")); + if (runningVersion !== packageJson.version) { + this.config.restartRequired = true; + return true; + } + + // Second check plugin versions + for (const pluginInfo of this.pluginInfos) { + if ( + !pluginInfo.controllerEntrypoint && !pluginInfo.webEntrypoint + || !this.config.get(`${pluginInfo.name}.load_plugin`) + ) { + continue; + } + + packageName = pluginInfo.npmPackage ?? pluginInfo.name; + const pluginPackageJson = await fs.readJSON(path.join(pluginInfo.requirePath, "package.json")); + if (pluginInfo.version !== pluginPackageJson.version) { + this.config.restartRequired = true; + return true; + } + } + } catch (err: any) { + logger.warn(`Failed to read package json for ${packageName}:\n${err.stack ?? err.message}`); + } + + return false; + } + onAutosaveIntervalChanged() { if (this.autosaveInterval) { clearInterval(this.autosaveInterval); @@ -1127,7 +1154,7 @@ export default class Controller { logger.warn(`Unable to load dist/web/manifest.json for plugin ${pluginInfo.name}`); } - if (!this.config.get(`${pluginInfo.name}.load_plugin` as keyof lib.ControllerConfigFields)) { + if (!this.config.get(`${pluginInfo.name}.load_plugin`)) { continue; } diff --git a/packages/controller/src/routes.ts b/packages/controller/src/routes.ts index 303059d4..5826f37c 100644 --- a/packages/controller/src/routes.ts +++ b/packages/controller/src/routes.ts @@ -109,9 +109,7 @@ function getPlugins(req: Request, res: Response) { for (let pluginInfo of req.app.locals.controller.pluginInfos) { let name = pluginInfo.name; let loaded = req.app.locals.controller.plugins.has(name); - let enabled = loaded && req.app.locals.controller.config.get( - `${pluginInfo.name}.load_plugin` as keyof lib.ControllerConfigFields, - ) as unknown as boolean; + let enabled = loaded && req.app.locals.controller.config.get(`${pluginInfo.name}.load_plugin`); // Note: Cast through unknown is needed because load_plugin is // defined at runtime and no other fields in the controller config // currently have the boolean type. diff --git a/packages/host/src/Host.ts b/packages/host/src/Host.ts index 839fac5d..ae4bd37b 100644 --- a/packages/host/src/Host.ts +++ b/packages/host/src/Host.ts @@ -410,7 +410,7 @@ export default class Host extends lib.Link { for (let pluginInfo of this.pluginInfos) { if ( !pluginInfo.hostEntrypoint && !pluginInfo.instanceEntrypoint - || !this.config.get(`${pluginInfo.name}.load_plugin` as keyof lib.HostConfigFields) + || !this.config.get(`${pluginInfo.name}.load_plugin`) ) { continue; } @@ -805,19 +805,46 @@ export default class Host extends lib.Link { } async handleSystemInfoRequest() { - if (!this.config.restartRequired) { - // If a restart isn't already required, then test if a new version is installed - try { - const runningVersion = this.config.get("host.version"); - const packageJson = await fs.readJSON(path.join(__dirname, "..", "package.json")); - if (runningVersion !== packageJson.version) { - this.config.restartRequired = true; - } - } catch (err: any) { - logger.warn(`Failed to read package json:\n${err.stack ?? err.message}`); - } + const restartRequired = await this.checkRestartRequired(); + return lib.gatherSystemInfo(this.config.get("host.id"), this.canRestart, restartRequired); + } + + async checkRestartRequired() { + if (this.config.restartRequired) { + return true; } - return lib.gatherSystemInfo(this.config.get("host.id"), this.canRestart, this.config.restartRequired); + + let packageName = "@clusterio/host"; + try { + // First check the clusterio version + const runningVersion = this.config.get("host.version"); + const packageJson = await fs.readJSON(path.join(__dirname, "..", "package.json")); + if (runningVersion !== packageJson.version) { + this.config.restartRequired = true; + return true; + } + + // Second check plugin versions + for (const pluginInfo of this.pluginInfos) { + if ( + !pluginInfo.hostEntrypoint && !pluginInfo.instanceEntrypoint + || !this.config.get(`${pluginInfo.name}.load_plugin`) + ) { + continue; + } + + packageName = pluginInfo.npmPackage ?? pluginInfo.name; + const pluginPackageJson = await fs.readJSON(path.join(pluginInfo.requirePath, "package.json")); + if (pluginInfo.version !== pluginPackageJson.version) { + this.config.restartRequired = true; + return true; + } + } + } catch (err: any) { + logger.warn(`Failed to read package json for ${packageName}:\n${err.stack ?? err.message}`); + } + + return false; } async handleHostMetricsRequest() { @@ -1087,7 +1114,7 @@ export default class Host extends lib.Link { return this.pluginInfos.map(pluginInfo => lib.PluginDetails.fromNodeEnvInfo( pluginInfo, this.plugins.has(pluginInfo.name), - this.config.get(`${pluginInfo.name}.load_plugin` as keyof lib.HostConfigFields) as boolean, + this.config.get(`${pluginInfo.name}.load_plugin`), )); } diff --git a/packages/host/src/Instance.ts b/packages/host/src/Instance.ts index 0a47c0b2..26815bbf 100644 --- a/packages/host/src/Instance.ts +++ b/packages/host/src/Instance.ts @@ -651,7 +651,7 @@ end`.replace(/\r?\n/g, " "); if ( !pluginInfo.instanceEntrypoint || !this._host.serverPlugins.has(pluginInfo.name) - || !this.config.get(`${pluginInfo.name}.load_plugin` as keyof lib.InstanceConfigFields) + || !this.config.get(`${pluginInfo.name}.load_plugin`) ) { continue; } diff --git a/packages/lib/src/config/definitions.ts b/packages/lib/src/config/definitions.ts index 138220fe..b948a04c 100644 --- a/packages/lib/src/config/definitions.ts +++ b/packages/lib/src/config/definitions.ts @@ -35,6 +35,7 @@ export interface ControllerConfigFields { "controller.allow_remote_updates": boolean; "controller.allow_plugin_updates": boolean; "controller.allow_plugin_install": boolean; + [key: `${string}.load_plugin`]: boolean; } /** @@ -267,6 +268,7 @@ export interface HostConfigFields { "host.allow_remote_updates": boolean; "host.allow_plugin_updates": boolean; "host.allow_plugin_install": boolean; + [key: `${string}.load_plugin`]: boolean; } /** @@ -399,6 +401,7 @@ export interface InstanceConfigFields { "instance.assigned_host": number | null; "instance.auto_start": boolean; "instance.exclude_from_start_all": boolean; + [key: `${string}.load_plugin`]: boolean; "factorio.version": string; "factorio.executable_path": string | null; diff --git a/packages/web_ui/src/components/ControllerPage.tsx b/packages/web_ui/src/components/ControllerPage.tsx index 5284a5e6..4a65b415 100644 --- a/packages/web_ui/src/components/ControllerPage.tsx +++ b/packages/web_ui/src/components/ControllerPage.tsx @@ -34,9 +34,10 @@ function ControllerControlButton({ canRestart, restartRequired }: ControllerCont actions.push({ key: "restart", label: ( - - Restart {restartRequired ? : undefined} - + !restartRequired ? "Restart" : + + Restart + ), onClick: () => { control.send( diff --git a/packages/web_ui/src/components/HostViewPage.tsx b/packages/web_ui/src/components/HostViewPage.tsx index d05115fb..78450966 100644 --- a/packages/web_ui/src/components/HostViewPage.tsx +++ b/packages/web_ui/src/components/HostViewPage.tsx @@ -37,9 +37,10 @@ function HostControlButton({ hostId, canRestart, restartRequired }: HostControlB actions.push({ key: "restart", label: ( - - Restart {restartRequired ? : undefined} - + !restartRequired ? "Restart" : + + Restart + ), onClick: () => { control.sendTo( diff --git a/packages/web_ui/src/components/InstanceList.tsx b/packages/web_ui/src/components/InstanceList.tsx index fe299f00..4e756b84 100644 --- a/packages/web_ui/src/components/InstanceList.tsx +++ b/packages/web_ui/src/components/InstanceList.tsx @@ -1,12 +1,14 @@ import React from "react"; import { useNavigate } from "react-router-dom"; -import { message, Button, Table } from "antd"; +import { message, Button, Table, Space } from "antd"; import CopyOutlined from "@ant-design/icons/CopyOutlined"; import type { SizeType } from "antd/es/config-provider/SizeContext"; import type { ColumnsType } from "antd/es/table"; import { useAccount } from "../model/account"; +import { useSystems } from "../model/system"; import { useHosts } from "../model/host"; +import { RestartRequired } from "./system_metrics"; import InstanceStatusTag from "./InstanceStatusTag"; import InstanceControlButton, { InstanceControlButtonPermissions } from "./InstanceControlButton"; import * as lib from "@clusterio/lib"; @@ -25,6 +27,7 @@ export default function InstanceList(props: InstanceListProps) { let account = useAccount(); let navigate = useNavigate(); let [hosts] = useHosts(); + const [systems] = useSystems(); function hostName(hostId?: number) { if (hostId === undefined) { @@ -50,12 +53,15 @@ export default function InstanceList(props: InstanceListProps) { { title: "Assigned Host", key: "assignedHost", - render: (_, instance) => e.stopPropagation()} - > - {hostName(instance.assignedHost)} - , + render: (_, instance) => + e.stopPropagation()} + > + {hostName(instance.assignedHost)} + + + , sorter: (a, b) => strcmp(hostName(a.assignedHost), hostName(b.assignedHost)), responsive: ["sm"], }, diff --git a/test/controller/Controller.js b/test/controller/Controller.js index f3d7e7ab..aa3a82d7 100644 --- a/test/controller/Controller.js +++ b/test/controller/Controller.js @@ -1,4 +1,5 @@ "use strict"; +const path = require("node:path"); const assert = require("assert").strict; const { Controller, HostInfo, InstanceInfo } = require("@clusterio/controller"); const { ControllerConfig, Address, RequestError, InstanceConfig, SystemInfo, Role } = require("@clusterio/lib"); @@ -27,8 +28,10 @@ describe("controller/src/Controller", function() { describe("class Controller", function() { /** @type {Controller} */ let controller, mockInstanceConfig; + const controllerVersion = require("@clusterio/controller/package.json").version; before(async function() { - controller = new Controller({}, [], new ControllerConfig("controller")); + const controllerConfig = new ControllerConfig("controller", { "controller.version": controllerVersion }); + controller = new Controller({}, [], controllerConfig); mockInstanceConfig = new MockInstanceConfig(new Map([ ["instance.id", 100], ["instance.name", "test"], ["factorio.settings", []], ])); @@ -233,6 +236,62 @@ describe("controller/src/Controller", function() { assert(instanceInfo.updatedAtMs === 0, "updatedAtMs incremented"); }); }); + describe(".checkRestartRequired()", function() { + beforeEach(function() { + // Would not be needed if controller setup was beforeEach + controller.config.restartRequired = false; + controller.config.set("controller.version", controllerVersion); + controller.config.set("global_chat.load_plugin", true); + controller.pluginInfos[0] = { + name: "global_chat", + requirePath: path.dirname(require.resolve("@clusterio/controller/package.json")), + version: controllerVersion, + }; + }); + it("returns false when no changes are present", async function() { + controller.pluginInfos[0].webEntrypoint = true; + controller.pluginInfos[0].controllerEntrypoint = true; + const result = await controller.checkRestartRequired(); + assert.equal(result, false); + assert.equal(controller.config.restartRequired, false); + }); + it("returns false when an unloaded plugin version changes", async function() { + controller.pluginInfos[0].version = "0.0.0"; + controller.pluginInfos[0].webEntrypoint = true; + controller.pluginInfos[0].controllerEntrypoint = true; + controller.config.set("global_chat.load_plugin", false); + controller.config.restartRequired = false; // Setting load_plugin requires a restart + const result = await controller.checkRestartRequired(); + assert.equal(result, false); + assert.equal(controller.config.restartRequired, false); + }); + it("returns true when the config flag is set", async function() { + controller.config.restartRequired = true; + const result = await controller.checkRestartRequired(); + assert.equal(result, true); + assert.equal(controller.config.restartRequired, true); + }); + it("returns true when clusterio version changes", async function() { + controller.config.set("controller.version", "0.0.0"); + const result = await controller.checkRestartRequired(); + assert.equal(result, true); + assert.equal(controller.config.restartRequired, true); + }); + it("returns true when a controller plugin version changes", async function() { + controller.pluginInfos[0].version = "0.0.0"; + controller.pluginInfos[0].controllerEntrypoint = true; + const result = await controller.checkRestartRequired(); + assert.equal(result, true); + assert.equal(controller.config.restartRequired, true); + }); + it("returns true when a web plugin version changes", async function() { + controller.pluginInfos[0].version = "0.0.0"; + controller.pluginInfos[0].webEntrypoint = true; + const result = await controller.checkRestartRequired(); + assert.equal(result, true); + assert.equal(controller.config.restartRequired, true); + }); + }); }); describe("migrations", function() { describe("SystemInfo", function() { diff --git a/test/host.js b/test/host.js index 54fd8ffe..fe50b04d 100644 --- a/test/host.js +++ b/test/host.js @@ -6,6 +6,7 @@ const lib = require("@clusterio/lib"); const Host = require("@clusterio/host/dist/node/src/Host").default; const Instance = require("@clusterio/host/dist/node/src/Instance").default; const { _discoverInstances } = require("@clusterio/host/dist/node/src/Host"); +const { HostConnector } = require("@clusterio/host/dist/node/host"); describe("Host testing", function() { describe("discoverInstances()", function() { @@ -115,5 +116,63 @@ describe("Host testing", function() { ]); }); }); + describe(".checkRestartRequired()", function() { + let host; + beforeEach(function() { + // This can be used more generally, but i did not want to interfere with handleSyncUserListsEvent + const hostVersion = require("@clusterio/host/package.json").version; + const pluginInfos = [{ + name: "global_chat", + requirePath: path.dirname(require.resolve("@clusterio/host/package.json")), + version: hostVersion, + }]; + const hostConfig = new lib.HostConfig("host", { "host.version": hostVersion }); + const hostConnector = new HostConnector(hostConfig, undefined, pluginInfos); + host = new Host(hostConnector, hostConfig, undefined, pluginInfos); + }); + it("returns false when no changes are present", async function() { + host.pluginInfos[0].hostEntrypoint = true; + host.pluginInfos[0].instanceEntrypoint = true; + const result = await host.checkRestartRequired(); + assert.equal(result, false); + assert.equal(host.config.restartRequired, false); + }); + it("returns false when an unloaded plugin version changes", async function() { + host.pluginInfos[0].version = "0.0.0"; + host.pluginInfos[0].hostEntrypoint = true; + host.pluginInfos[0].instanceEntrypoint = true; + host.config.set("global_chat.load_plugin", false); + host.config.restartRequired = false; // Setting load_plugin requires a restart + const result = await host.checkRestartRequired(); + assert.equal(result, false); + assert.equal(host.config.restartRequired, false); + }); + it("returns true when the config flag is set", async function() { + host.config.restartRequired = true; + const result = await host.checkRestartRequired(); + assert.equal(result, true); + assert.equal(host.config.restartRequired, true); + }); + it("returns true when clusterio version changes", async function() { + host.config.set("host.version", "0.0.0"); + const result = await host.checkRestartRequired(); + assert.equal(result, true); + assert.equal(host.config.restartRequired, true); + }); + it("returns true when a host plugin version changes", async function() { + host.pluginInfos[0].version = "0.0.0"; + host.pluginInfos[0].hostEntrypoint = true; + const result = await host.checkRestartRequired(); + assert.equal(result, true); + assert.equal(host.config.restartRequired, true); + }); + it("returns true when an instance plugin version changes", async function() { + host.pluginInfos[0].version = "0.0.0"; + host.pluginInfos[0].instanceEntrypoint = true; + const result = await host.checkRestartRequired(); + assert.equal(result, true); + assert.equal(host.config.restartRequired, true); + }); + }); }); });