From 01cf789f5d6d96ea417265012fc2b32a33573c7a Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Wed, 28 Oct 2026 22:09:06 +0000 Subject: [PATCH] Add base instances for instance config inheritance Instances can be marked as a base with instance.is_base and other instances inherit config values from it through instance.base_id. The controller copies base values into inheriting configs and keeps track of inherited fields in the hidden instance.inherited_fields, so hosts receive resolved configs and need no changes. Setting a field stops inheriting it, `instance config inherit` undoes that. Bases cannot be assigned to a host, are hidden from `instance list` unless --bases is given, and cannot be deleted or unmarked while other instances inherit from them. Chains of bases are allowed and cycles are rejected. Part of #200 Co-Authored-By: Claude Opus 5.5 --- docs/configuration.md | 22 ++ docs/managing-a-cluster.md | 17 +- packages/controller/src/ControlConnection.ts | 38 +++- packages/controller/src/InstanceManager.ts | 207 ++++++++++++++++++- packages/controller/src/InstanceRecord.ts | 1 + packages/ctl/src/commands_instance.ts | 53 ++++- packages/lib/src/config/definitions.ts | 44 ++++ packages/lib/src/data/InstanceDetails.ts | 10 +- packages/lib/src/data/messages_instance.ts | 22 ++ packages/lib/src/link/messages.ts | 1 + test/controller/ControlConnection.test.js | 99 +++++++++ test/controller/Controller.js | 4 + test/controller/InstanceManager.test.js | 175 ++++++++++++++++ test/integration/clusterio.js | 57 +++++ test/integration/routing.js | 3 +- test/lib/config/definitions.js | 42 ++++ 16 files changed, 769 insertions(+), 26 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 89650ca9..54e1f015 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -521,6 +521,28 @@ This allows you to prevent certain instances from being started when using the b Defaults to false. +### instance.is_base + +If enabled this instance is a base other instances can inherit config values from. +Base instances cannot be assigned to a host and therefore never start. +Hidden from `instance list` in clusterioctl unless `--bases` is passed. +An instance cannot stop being a base while other instances inherit from it. + +Defaults to false. + + +### instance.base_id + +ID of the base instance to inherit config values from. +Inherited fields take their value from the base and follow it when it changes, setting a field on this instance stops inheriting it. +When set on an existing instance only fields with the same value as the base become inherited. +`instance.name`, `instance.is_base`, `instance.base_id` and hidden fields are never inherited. +A base may itself inherit from another base, but not from itself through a chain of bases. +The fields currently inherited are stored in the hidden `instance.inherited_fields` field. + +Defaults to null meaning no base. + + ### factorio.version Version of Factorio to use for this instance. diff --git a/docs/managing-a-cluster.md b/docs/managing-a-cluster.md index 51982131..14073d9d 100644 --- a/docs/managing-a-cluster.md +++ b/docs/managing-a-cluster.md @@ -79,15 +79,16 @@ Marks the host as requiring a restart, the same way a plugin update does. Use it ### List Instances - ctl> instance list + ctl> instance list [--bases] List instances. +Base instances are only listed if `--bases` is passed. ### Create instance - ctl> instance create [--id ] + ctl> instance create [--id ] [--from ] [--base ] [--is-base] -Create a new instance. `name` is the instance name. `--id` is the optional id. +Create a new instance. `name` is the instance name. `--id` is the optional id. `--from` copies the config of an existing instance. `--base` makes the new instance inherit all config values from the given base instance. `--is-base` creates the instance as a base, see `instance.is_base` in the [configuration docs](configuration.md). ### Kill instance @@ -120,7 +121,7 @@ Most notably Clusterio records wall clock time while the save time is recorded i ctl> instance config list -List config values for an instance. `instance` is the instance whose config values to list. +List config values for an instance. `instance` is the instance whose config values to list. Values inherited from a base instance are marked with `(inherited)`. #### Set config value @@ -132,11 +133,17 @@ Set field in instance config. `instance` is instance to set. `field` is field to To be written. +#### Inherit config value + + ctl> instance config inherit + +Make a field in the instance config take its value from the base instance again, undoing a previous `set`. `instance` is the instance to update, `field` is the field to inherit. + #### Edit instance config ctl> instance config edit [editor] -Edit instance config in a text editor. Once file is saved and editor has been exited, config values will be read back in. `instance` is the instance to edit, `editor` is the editor to use. If `editor` is not given, clusterioctl will read from the `EDITOR` or `VISUAL` enviroment variables. +Edit instance config in a text editor. Once file is saved and editor has been exited, changed config values will be read back in. `instance` is the instance to edit, `editor` is the editor to use. If `editor` is not given, clusterioctl will read from the `EDITOR` or `VISUAL` enviroment variables. ### Assign instance to host diff --git a/packages/controller/src/ControlConnection.ts b/packages/controller/src/ControlConnection.ts index 43af122d..72b8ae88 100644 --- a/packages/controller/src/ControlConnection.ts +++ b/packages/controller/src/ControlConnection.ts @@ -8,6 +8,7 @@ import { logFilter, logger } from "@clusterio/lib"; import BaseConnection from "./BaseConnection.js"; import User from "./User.js"; import UserRecord from "./UserRecord.js"; +import type InstanceRecord from "./InstanceRecord.js"; import * as routes from "./routes.js"; import Controller from "./Controller.js"; @@ -94,6 +95,7 @@ export default class ControlConnection extends BaseConnection { this.handle(lib.InstanceConfigSetRequest, this.handleInstanceConfigSetRequest.bind(this)); this.handle(lib.InstanceConfigSetFieldRequest, this.handleInstanceConfigSetFieldRequest.bind(this)); this.handle(lib.InstanceConfigSetPropRequest, this.handleInstanceConfigSetPropRequest.bind(this)); + this.handle(lib.InstanceConfigInheritFieldRequest, this.handleInstanceConfigInheritFieldRequest.bind(this)); this.handle(lib.InstanceAssignRequest, this.handleInstanceAssignRequest.bind(this)); this.handle(lib.InstanceSaveDetailsListRequest, this.handleInstanceSaveDetailsListRequest.bind(this)); this.handle(lib.InstanceRenameSaveRequest, controller.sendRequestToHostByInstanceId.bind(controller)); @@ -315,6 +317,12 @@ export default class ControlConnection extends BaseConnection { instanceConfig.set("instance.assigned_host", null); // New instances are unassigned } instanceConfig.update(request.config, false, "control"); + if (Object.hasOwn(request.config, "instance.base_id")) { + this._controller.instances.checkInheritance( + instanceConfig, instanceConfig.get("instance.is_base"), instanceConfig.get("instance.base_id") + ); + this._controller.instances.applyBase(instanceConfig, Object.keys(request.config)); + } await this._controller.instances.createInstance(instanceConfig); } @@ -355,17 +363,36 @@ export default class ControlConnection extends BaseConnection { } } + this.checkStagedInheritance(instance); instance.config.commitStaging(); + this._controller.instances.overrideFields(instance, Object.keys(request.fields)); await this._controller.instanceConfigUpdated(instance); } finally { instance.config.revertStaging(); } } + private checkStagedInheritance(instance: InstanceRecord) { + const staged = instance.config.staging; + this._controller.instances.checkInheritance( + instance.config, staged.get("instance.is_base"), staged.get("instance.base_id") + ); + } + async handleInstanceConfigSetFieldRequest(request: lib.InstanceConfigSetFieldRequest) { const instance = this._controller.instances.getForRequest(request.instanceId); - ControlConnection.validateInstanceConfigSetField(request.field); - instance.config.set(request.field as keyof lib.InstanceConfigFields, request.value, "control"); + const field = request.field as keyof lib.InstanceConfigFields; + ControlConnection.validateInstanceConfigSetField(field); + if (["instance.is_base", "instance.base_id"].includes(field)) { + try { + instance.config.stage(field, request.value, "control"); + this.checkStagedInheritance(instance); + } finally { + instance.config.revertStaging(); + } + } + instance.config.set(field, request.value, "control"); + this._controller.instances.overrideFields(instance, [field]); await this._controller.instanceConfigUpdated(instance); } @@ -374,9 +401,16 @@ export default class ControlConnection extends BaseConnection { let { field, prop, value } = request; ControlConnection.validateInstanceConfigSetField(field); instance.config.setProp(field as keyof lib.InstanceConfigFields, prop, value, "control"); + this._controller.instances.overrideFields(instance, [field]); await this._controller.instanceConfigUpdated(instance); } + async handleInstanceConfigInheritFieldRequest(request: lib.InstanceConfigInheritFieldRequest) { + ControlConnection.validateInstanceConfigSetField(request.field); + this._controller.instances.inheritField(request.instanceId, request.field); + await this._controller.instanceConfigUpdated(this._controller.instances.getForRequest(request.instanceId)); + } + async handleInstanceAssignRequest(request: lib.InstanceAssignRequest) { await this._controller.instances.assignInstance(request.instanceId, request.hostId ?? null); } diff --git a/packages/controller/src/InstanceManager.ts b/packages/controller/src/InstanceManager.ts index 4323d59a..38802a96 100644 --- a/packages/controller/src/InstanceManager.ts +++ b/packages/controller/src/InstanceManager.ts @@ -1,4 +1,6 @@ +import { isDeepStrictEqual } from "node:util"; import * as lib from "@clusterio/lib"; +import { logger } from "@clusterio/lib"; import type Controller from "./Controller.js"; import type HostConnection from "./HostConnection.js"; @@ -24,6 +26,9 @@ export default class InstanceManager { non_blocking_saving: false, } as const; + /** Non-zero while base values are being copied into inheriting instances */ + private _inheriting = 0; + constructor( public readonly records: lib.SubscribableDatastore, private readonly _controller: Controller, @@ -106,11 +111,190 @@ export default class InstanceManager { */ addInstanceHooks(instance: InstanceRecord): void { instance.config.on("fieldChanged", (field: string, curr: unknown, prev: unknown) => { + if (field === "instance.base_id") { + this._rebase(instance); + } else if (!this._inheriting && instance.config.isInherited(field)) { + this._setInherited(instance, [field], false); + } this.records.set(instance); this._notifyConfigFieldChanged(instance, field, curr, prev); + this._propagate(instance, field, curr); }); } + /* ------------------------------------------------------------------------- */ + /* Inheritance */ + /* ------------------------------------------------------------------------- */ + + /** + * Get the instances that inherit directly from the given base. + * + * @param baseId - ID of the base instance. + * @returns Instances with `instance.base_id` set to baseId. + */ + inheritorsOf(baseId: number): InstanceRecord[] { + return [...this.records.values()].filter( + instance => instance.config.get("instance.base_id") === baseId + ); + } + + /** + * Check that new values for `instance.is_base` and `instance.base_id` are allowed. + * + * @param config - Current config of the instance. + * @param isBase - New value for `instance.is_base`. + * @param baseId - New value for `instance.base_id`. + * @throws {module:lib.ValidationError} if the base is not a base or would create a cycle. + * @throws {module:lib.ConflictError} if the instance is assigned or still has inheritors. + */ + checkInheritance(config: lib.InstanceConfig, isBase: boolean, baseId: number | null): void { + const instanceId = config.get("instance.id"); + if (isBase && config.get("instance.assigned_host") !== null) { + throw new lib.ConflictError("Instance must be unassigned before it can be made a base"); + } + if (!isBase && config.get("instance.is_base")) { + const count = this.inheritorsOf(instanceId).length; + if (count) { + throw new lib.ConflictError(`Instance is the base of ${count} other instance(s)`); + } + } + if (baseId === null) { + return; + } + const base = this.records.get(baseId); + if (!base) { + throw new lib.NotFoundError(`Instance with ID ${baseId} does not exist`); + } + if (!base.config.get("instance.is_base")) { + throw new lib.ValidationError(`Instance ${base.config.get("instance.name")} is not a base`); + } + const visited = new Set(); + for (let id: number | null = baseId; id !== null && !visited.has(id);) { + if (id === instanceId) { + throw new lib.ValidationError("Base instances cannot inherit from themselves"); + } + visited.add(id); + id = this.records.get(id)?.config.get("instance.base_id") ?? null; + } + } + + /** + * Copy all inheritable values from the base set in `instance.base_id` + * into a config that is not yet part of the cluster and mark them inherited. + * + * @param config - Config of the instance to be created. + * @param keep - Fields to keep the current value of. + */ + applyBase(config: lib.InstanceConfig, keep: string[] = []): void { + const base = this.records.get(config.get("instance.base_id")!); + if (!base) { + return; + } + const values: Record = {}; + const inherited: Record = {}; + for (const name of Object.keys(lib.InstanceConfig.fieldDefinitions)) { + if (lib.InstanceConfig.isInheritable(name) && !keep.includes(name)) { + values[name] = structuredClone(base.config.get(name as keyof lib.InstanceConfigFields)); + inherited[name] = true; + } + } + config.update(values, false); + config.set("instance.inherited_fields", inherited); + } + + /** + * Make a field take its value from the base instance again. + * + * @param instanceId - ID of the instance to update. + * @param field - Name of the field to inherit. + * @throws {module:lib.ValidationError} if the instance has no base or the field cannot be inherited. + */ + inheritField(instanceId: number, field: string): void { + const instance = this.getForRequest(instanceId); + const baseId = instance.config.get("instance.base_id"); + const base = baseId !== null ? this.records.get(baseId) : undefined; + if (!base) { + throw new lib.ValidationError("Instance does not have a base"); + } + if (!lib.InstanceConfig.isInheritable(field)) { + throw new lib.ValidationError(`Field ${field} cannot be inherited`); + } + this._setInherited(instance, [field], true); + this._inheriting += 1; + try { + instance.config.update({ + [field]: structuredClone(base.config.get(field as keyof lib.InstanceConfigFields)), + }, true); + } finally { + this._inheriting -= 1; + } + } + + /** + * Stop inheriting the given fields, used when a value is explicitly set. + * + * @param instance - Instance the fields were set on. + * @param fields - Names of fields set. + */ + overrideFields(instance: InstanceRecord, fields: string[]): void { + const inherited = fields.filter(field => instance.config.isInherited(field)); + if (inherited.length) { + this._setInherited(instance, inherited, false); + } + } + + private _setInherited(instance: InstanceRecord, fields: string[], inherited: boolean): void { + const value = { ...instance.config.get("instance.inherited_fields") }; + for (const field of fields) { + if (inherited) { + value[field] = true; + } else { + delete value[field]; + } + } + instance.config.set("instance.inherited_fields", value); + } + + /** Recompute inherited fields after base_id changed, values equal to the base are inherited */ + private _rebase(instance: InstanceRecord): void { + const baseId = instance.config.get("instance.base_id"); + const base = baseId !== null ? this.records.get(baseId) : undefined; + const inherited: Record = {}; + if (base) { + for (const name of Object.keys(lib.InstanceConfig.fieldDefinitions)) { + const field = name as keyof lib.InstanceConfigFields; + if ( + lib.InstanceConfig.isInheritable(name) + && isDeepStrictEqual(instance.config.get(field), base.config.get(field)) + ) { + inherited[name] = true; + } + } + } + instance.config.set("instance.inherited_fields", inherited); + } + + /** Copy a changed value from a base to the instances inheriting it */ + private _propagate(base: InstanceRecord, field: string, value: unknown): void { + if (!lib.InstanceConfig.isInheritable(field)) { + return; + } + for (const instance of this.inheritorsOf(base.id)) { + if (!instance.config.isInherited(field)) { + continue; + } + this._inheriting += 1; + try { + instance.config.update({ [field]: structuredClone(value) as any }, true); + } finally { + this._inheriting -= 1; + } + this._controller.instanceConfigUpdated(instance).catch(err => { + logger.error(`Failed to send config of instance ${instance.id} to its host:\n${err.stack}`); + }); + } + } + /* ------------------------------------------------------------------------- */ /* Lifecycle */ /* ------------------------------------------------------------------------- */ @@ -136,13 +320,15 @@ export default class InstanceManager { throw new lib.ConflictError(`Instance with ID ${instanceId} already exists`); } - const controllerName = this._controller.config.get("controller.name"); - instanceConfig.set("factorio.settings", { - name: `${controllerName} - ${instanceConfig.get("instance.name")}`, - description: `Clusterio instance for ${controllerName}`, - ...InstanceManager.DefaultFactorioSettings, - ...instanceConfig.get("factorio.settings"), - }); + if (!instanceConfig.isInherited("factorio.settings")) { + const controllerName = this._controller.config.get("controller.name"); + instanceConfig.set("factorio.settings", { + name: `${controllerName} - ${instanceConfig.get("instance.name")}`, + description: `Clusterio instance for ${controllerName}`, + ...InstanceManager.DefaultFactorioSettings, + ...instanceConfig.get("factorio.settings"), + }); + } const instance = new InstanceRecord(instanceConfig, "unassigned"); this.addInstanceHooks(instance); @@ -186,6 +372,9 @@ export default class InstanceManager { if (currentAssignedHost === hostId) { return; } + if (hostId !== null && instance.config.get("instance.is_base")) { + throw new lib.ValidationError("Base instances cannot be assigned to a host"); + } let newHostConnection: HostConnection | undefined; if (hostId !== null) { @@ -252,6 +441,10 @@ export default class InstanceManager { async deleteInstance(instanceId: number): Promise { const instance = this.getForRequest(instanceId); const hostId = instance.config.get("instance.assigned_host"); + const inheritors = this.inheritorsOf(instanceId).length; + if (inheritors) { + throw new lib.ConflictError(`Instance is the base of ${inheritors} other instance(s)`); + } if (hostId !== null) { await this._controller.sendTo( diff --git a/packages/controller/src/InstanceRecord.ts b/packages/controller/src/InstanceRecord.ts index 9ad0db58..a441a88a 100644 --- a/packages/controller/src/InstanceRecord.ts +++ b/packages/controller/src/InstanceRecord.ts @@ -83,6 +83,7 @@ export default class InstanceRecord { this.startedAtMs, this.updatedAtMs, this.config.get("instance.exclude_from_start_all"), + this.config.get("instance.is_base"), ); } diff --git a/packages/ctl/src/commands_instance.ts b/packages/ctl/src/commands_instance.ts index 63440875..c575827b 100644 --- a/packages/ctl/src/commands_instance.ts +++ b/packages/ctl/src/commands_instance.ts @@ -20,10 +20,14 @@ export const instanceCommands = new lib.CommandTree({ name: "instance", alias: ["i"], description: "Instance management", }); instanceCommands.add(new lib.Command({ - definition: [["list", "l"], "List instances known to the controller"], - handler: async function(args: object, ctl: Ctl) { + definition: [["list", "l"], "List instances known to the controller", (yargs) => { + yargs.options({ + "bases": { describe: "Include base instances", nargs: 0, type: "boolean", default: false }, + }); + }], + handler: async function(args: { bases: boolean }, ctl: Ctl) { let list = await ctl.send(new lib.InstanceDetailsListRequest()); - print(asTable(list)); + print(asTable(list.filter(instance => args.bases || !instance.isBase))); }, })); @@ -34,18 +38,30 @@ instanceCommands.add(new lib.Command({ yargs.options({ "id": { type: "number", nargs: 1, describe: "Instance id" }, "from": { type: "number", nargs: 1, describe: "Clone config from instance id" }, + "base": { type: "string", nargs: 1, describe: "Base instance to inherit config from" }, + "is-base": { type: "boolean", nargs: 0, describe: "Create the instance as a base" }, }); }], - handler: async function(args: { name: string, id?: number, from?: number, }, ctl: Ctl) { + handler: async function( + args: { name: string, id?: number, from?: number, base?: string, isBase?: boolean }, + ctl: Ctl, + ) { let instanceConfig = new lib.InstanceConfig("control"); + let fields: (keyof lib.InstanceConfigFields)[] = ["instance.id", "instance.name"]; if (args.id !== undefined) { instanceConfig.set("instance.id", args.id); } instanceConfig.set("instance.name", args.name); + if (args.base !== undefined) { + instanceConfig.set("instance.base_id", await lib.resolveInstance(ctl, args.base)); + fields.push("instance.base_id"); + } + if (args.isBase) { + instanceConfig.set("instance.is_base", true); + fields.push("instance.is_base"); + } await ctl.send(new lib.InstanceCreateRequest( - instanceConfig.toRemote("controller", [ - "instance.id", "instance.name", - ]), + instanceConfig.toRemote("controller", fields), args.from )); }, @@ -61,13 +77,30 @@ instanceConfigCommands.add(new lib.Command({ handler: async function(args: { instance: string }, ctl: Ctl) { let instanceId = await lib.resolveInstance(ctl, args.instance); let config = await ctl.send(new lib.InstanceConfigGetRequest(instanceId)); + const inherited = config["instance.base_id"] !== null + ? config["instance.inherited_fields"] as Record ?? {} + : {}; for (let [name, value] of Object.entries(config)) { - print(`${name} ${JSON.stringify(value)}`); + if (name === "instance.inherited_fields") { + continue; + } + print(`${name} ${JSON.stringify(value)}${inherited[name] ? " (inherited)" : ""}`); } }, })); +instanceConfigCommands.add(new lib.Command({ + definition: ["inherit ", "Make field in instance config use the value of its base", (yargs) => { + yargs.positional("instance", { describe: "Instance to update config on", type: "string" }); + yargs.positional("field", { describe: "Field to inherit", type: "string" }); + }], + handler: async function(args: { instance: string, field: string }, ctl: Ctl) { + let instanceId = await lib.resolveInstance(ctl, args.instance); + await ctl.send(new lib.InstanceConfigInheritFieldRequest(instanceId, args.field)); + }, +})); + instanceConfigCommands.add(new lib.Command({ definition: ["set [value]", "Set field in instance config", (yargs) => { yargs.positional("instance", { describe: "Instance to set config on", type: "string" }); @@ -170,8 +203,10 @@ instanceConfigCommands.add(new lib.Command({ await events.once(editorSpawn, "exit"); const data = await fs.readFile(tmpFile, "utf8"); const final = await configToKeyVal(data); + const original = await configToKeyVal(allConfigElements); for (let index in final) { - if (index in final) { + // Unchanged fields are skipped so they keep inheriting from the base + if (index in final && final[index] !== original[index]) { try { await ctl.send(new lib.InstanceConfigSetFieldRequest( instanceId, diff --git a/packages/lib/src/config/definitions.ts b/packages/lib/src/config/definitions.ts index b28db160..061d24dd 100644 --- a/packages/lib/src/config/definitions.ts +++ b/packages/lib/src/config/definitions.ts @@ -488,6 +488,9 @@ export interface InstanceConfigFields { "instance.assigned_host": number | null; "instance.auto_start": boolean; "instance.exclude_from_start_all": boolean; + "instance.is_base": boolean; + "instance.base_id": number | null; + "instance.inherited_fields": Record; [key: `${string}.load_plugin`]: boolean; [key: `${string}.version`]: string; @@ -539,6 +542,23 @@ export class InstanceConfig extends classes.Config { return config; } + /** Fields that are never inherited from a base instance */ + static uninheritableFields = ["instance.name", "instance.is_base", "instance.base_id"]; + + /** True if the field can take its value from a base instance */ + static isInheritable(name: string) { + const def = this.fieldDefinitions[name as keyof InstanceConfigFields]; + return Boolean(def) + && !def.hidden + && (def.access ?? this.defaultAccess).includes("controller") + && !this.uninheritableFields.includes(name); + } + + /** True if the field currently takes its value from the base instance */ + isInherited(name: string) { + return this.get("instance.base_id") !== null && Boolean(this.get("instance.inherited_fields")[name]); + } + static fieldDefinitions: classes.ConfigDefs = { "instance.name": { type: "string", @@ -566,6 +586,30 @@ export class InstanceConfig extends classes.Config { type: "boolean", initialValue: false, }, + "instance.is_base": { + description: + "Mark this instance as a base other instances can inherit config from. " + + "Bases cannot be assigned to a host or started.", + access: ["controller", "control"], + type: "boolean", + initialValue: false, + }, + "instance.base_id": { + title: "Base Instance", + description: "ID of the base instance to inherit config values from", + access: ["controller", "control"], + type: "number", + optional: true, + // Checked by the controller + }, + "instance.inherited_fields": { + description: "Fields whose value comes from the base instance", + access: ["controller", "control"], + readonly: ["controller"], + type: "object", + initialValue: {}, + hidden: true, + }, "factorio.version": { description: "Version of the game to run. Use 'latest' for the latest installed version, a release " + diff --git a/packages/lib/src/data/InstanceDetails.ts b/packages/lib/src/data/InstanceDetails.ts index adf0b9d8..ce008c9b 100644 --- a/packages/lib/src/data/InstanceDetails.ts +++ b/packages/lib/src/data/InstanceDetails.ts @@ -41,7 +41,8 @@ export default class InstanceDetails { public startedAtMs = 0, /** Millisecond Unix timestamp this entry was last updated at */ public updatedAtMs = 0, - public excludeFromStartAll = false + public excludeFromStartAll = false, + public isBase = false, ) {} get isDeleted() { @@ -61,6 +62,7 @@ export default class InstanceDetails { "startedAtMs": Type.Optional(Type.Number()), "updatedAtMs": Type.Optional(Type.Number()), "excludeFromStartAll": Type.Optional(Type.Boolean()), + "isBase": Type.Optional(Type.Boolean()), }); static fromJSON(json: Static) { @@ -73,7 +75,8 @@ export default class InstanceDetails { json.factorioVersion, json.startedAtMs, json.updatedAtMs, - json.excludeFromStartAll + json.excludeFromStartAll, + json.isBase, ); } @@ -102,6 +105,9 @@ export default class InstanceDetails { if (this.excludeFromStartAll !== false) { json.excludeFromStartAll = this.excludeFromStartAll; } + if (this.isBase !== false) { + json.isBase = this.isBase; + } return json; } diff --git a/packages/lib/src/data/messages_instance.ts b/packages/lib/src/data/messages_instance.ts index d107632c..5cde39b7 100644 --- a/packages/lib/src/data/messages_instance.ts +++ b/packages/lib/src/data/messages_instance.ts @@ -176,6 +176,28 @@ export class InstanceConfigSetPropRequest { } } +export class InstanceConfigInheritFieldRequest { + declare ["constructor"]: typeof InstanceConfigInheritFieldRequest; + static type = "request" as const; + static src = "control" as const; + static dst = "controller" as const; + static permission = "core.instance.update_config" as const; + + constructor( + public instanceId: number, + public field: string, + ) { } + + static jsonSchema = Type.Object({ + "instanceId": Type.Integer(), + "field": Type.String(), + }); + + static fromJSON(json: Static) { + return new this(json.instanceId, json.field); + } +} + export class InstanceAssignRequest { declare ["constructor"]: typeof InstanceAssignRequest; static type = "request" as const; diff --git a/packages/lib/src/link/messages.ts b/packages/lib/src/link/messages.ts index f4e15976..c0ea4940 100644 --- a/packages/lib/src/link/messages.ts +++ b/packages/lib/src/link/messages.ts @@ -71,6 +71,7 @@ export const dataClasses: (RequestClass | EventClass) instance.InstanceConfigSetRequest, instance.InstanceConfigSetFieldRequest, instance.InstanceConfigSetPropRequest, + instance.InstanceConfigInheritFieldRequest, instance.InstanceAssignRequest, instance.InstanceMetricsRequest, instance.InstanceStartRequest, diff --git a/test/controller/ControlConnection.test.js b/test/controller/ControlConnection.test.js index 6311c2b1..e4d9efde 100644 --- a/test/controller/ControlConnection.test.js +++ b/test/controller/ControlConnection.test.js @@ -156,6 +156,105 @@ describe("controller/src/ControlConnection", function() { assert.equal(clone.config.get("instance.name"), "clone"); assert.equal(clone.config.get("instance.assigned_host"), null); }); + + it("creates an instance inheriting from a base", async function() { + await create({ "instance.id": 4001, "instance.name": "base", "instance.is_base": true }); + mockController.instances.getMutable(4001).config.set("factorio.version", "2.0"); + + await create({ "instance.id": 4002, "instance.name": "child", "instance.base_id": 4001 }); + const child = mockController.instances.get(4002); + assert.equal(child.config.get("factorio.version"), "2.0"); + assert.equal(child.config.isInherited("factorio.version"), true); + assert.equal(child.config.isInherited("instance.name"), false); + }); + + it("rejects a base that is not a base", async function() { + await create({ "instance.id": 4001, "instance.name": "plain" }); + await assert.rejects( + create({ "instance.id": 4002, "instance.name": "child", "instance.base_id": 4001 }), + new lib.ValidationError("Instance plain is not a base"), + ); + assert.equal(mockController.instances.has(4002), false); + }); + }); + + describe("instance config set handlers", function() { + let mockController; + let connection; + + beforeEach(async function() { + mockController = new MockController(); + mockController.mockConfigEntries.set("controller.name", "Test"); + mockController.instances = new InstanceManager(new lib.SubscribableDatastore(), mockController); + mockController.instanceConfigUpdated = async () => {}; + connection = { + _controller: mockController, + checkStagedInheritance: ControlConnection.prototype.checkStagedInheritance, + }; + for (const [id, fields] of [ + [1, { "instance.is_base": true, "factorio.version": "2.0" }], + [2, { "instance.base_id": 1 }], + ]) { + const config = new lib.InstanceConfig("controller", { + "instance.id": id, "instance.name": `i${id}`, ...fields, + }); + if (fields["instance.base_id"]) { + mockController.instances.applyBase(config, ["instance.id", "instance.name", "instance.base_id"]); + } + await mockController.instances.createInstance(config); + } + }); + + function call(handler, request) { + return ControlConnection.prototype[handler].call(connection, request); + } + const setField = (...args) => new lib.InstanceConfigSetFieldRequest(...args); + const setFields = (...args) => new lib.InstanceConfigSetRequest(...args); + + it("stops inheriting a field set to the inherited value", async function() { + await call("handleInstanceConfigSetFieldRequest", setField(2, "factorio.version", "2.0")); + assert.equal(mockController.instances.get(2).config.isInherited("factorio.version"), false); + }); + + it("stops inheriting a field set through set request", async function() { + await call("handleInstanceConfigSetRequest", setFields(2, { "factorio.version": "2.0" })); + assert.equal(mockController.instances.get(2).config.isInherited("factorio.version"), false); + }); + + it("stops inheriting a field changed through set-prop", async function() { + await call( + "handleInstanceConfigSetPropRequest", + new lib.InstanceConfigSetPropRequest(2, "factorio.settings", "max_players", 0), + ); + assert.equal(mockController.instances.get(2).config.isInherited("factorio.settings"), false); + }); + + it("rejects base_id pointing at a non-base", async function() { + await assert.rejects( + call("handleInstanceConfigSetFieldRequest", setField(1, "instance.base_id", "2")), + new lib.ValidationError("Instance i2 is not a base"), + ); + assert.equal(mockController.instances.get(1).config.get("instance.base_id"), null); + }); + + it("rejects unmarking a base with inheritors through set request", async function() { + await assert.rejects( + call("handleInstanceConfigSetRequest", setFields(1, { "instance.is_base": "false" })), + new lib.ConflictError("Instance is the base of 1 other instance(s)"), + ); + assert.equal(mockController.instances.get(1).config.get("instance.is_base"), true); + }); + + it("inherits a field again", async function() { + await call("handleInstanceConfigSetFieldRequest", setField(2, "factorio.version", "2.1")); + await call( + "handleInstanceConfigInheritFieldRequest", + new lib.InstanceConfigInheritFieldRequest(2, "factorio.version"), + ); + const config = mockController.instances.get(2).config; + assert.equal(config.get("factorio.version"), "2.0"); + assert.equal(config.isInherited("factorio.version"), true); + }); }); describe("close cleanup", function() { diff --git a/test/controller/Controller.js b/test/controller/Controller.js index 732feceb..9a5908c0 100644 --- a/test/controller/Controller.js +++ b/test/controller/Controller.js @@ -34,6 +34,10 @@ class MockInstanceConfig extends EventEmitter { this._data.set(field, value); this.emit("fieldChanged", field, value, prev); } + + isInherited() { + return false; + } } describe("controller/src/Controller", function() { diff --git a/test/controller/InstanceManager.test.js b/test/controller/InstanceManager.test.js index dc254629..fc6e262b 100644 --- a/test/controller/InstanceManager.test.js +++ b/test/controller/InstanceManager.test.js @@ -393,4 +393,179 @@ describe("controller/InstanceManager", function () { assert.equal(sentRequest.instanceId, 1); }); }); + + describe("inheritance", function () { + /** @type {InstanceRecord} */ + let base; + /** @type {InstanceRecord} */ + let child; + let updated; + + async function createInheritor(id, baseId, fields = {}) { + const config = new lib.InstanceConfig("controller", { + "instance.id": id, "instance.name": `Instance ${id}`, "instance.base_id": baseId, ...fields, + }); + instances.applyBase(config, ["instance.id", "instance.name", "instance.base_id", ...Object.keys(fields)]); + return await instances.createInstance(config); + } + + beforeEach(async function () { + base = await instances.createInstance(new lib.InstanceConfig("controller", { + "instance.id": 10, + "instance.name": "Base", + "instance.is_base": true, + "factorio.version": "2.0", + "factorio.settings": { name: "Base server" }, + })); + child = await createInheritor(11, 10, { "factorio.game_port": 34197 }); + updated = []; + controller.instanceConfigUpdated = async (inst) => { updated.push(inst.id); }; + }); + + describe(".applyBase()", function () { + it("should copy base values and mark them inherited", function () { + assert.equal(child.config.get("factorio.version"), "2.0"); + assert.equal(child.config.isInherited("factorio.version"), true); + assert.equal(child.config.get("factorio.game_port"), 34197); + assert.equal(child.config.isInherited("factorio.game_port"), false); + assert.equal(child.config.isInherited("instance.name"), false); + }); + it("should keep inherited factorio.settings in createInstance", function () { + assert.deepEqual(child.config.get("factorio.settings"), base.config.get("factorio.settings")); + assert.notEqual(child.config.get("factorio.settings"), base.config.get("factorio.settings")); + }); + }); + + describe("base changes", function () { + it("should propagate to inherited fields and send the config to the host", function () { + base.config.set("factorio.version", "2.1"); + assert.equal(child.config.get("factorio.version"), "2.1"); + assert.equal(child.config.isInherited("factorio.version"), true); + assert.deepEqual(updated, [11]); + }); + it("should mark the inheritor as requiring restart", function () { + child.config.restartRequired = false; + base.config.set("factorio.version", "2.1"); + assert.equal(child.config.restartRequired, true); + }); + it("should not touch fields the inheritor has set", function () { + base.config.set("factorio.game_port", 1234); + assert.equal(child.config.get("factorio.game_port"), 34197); + assert.deepEqual(updated, []); + }); + it("should propagate through several levels", async function () { + child.config.set("instance.is_base", true); + const grandchild = await createInheritor(12, 11); + base.config.setProp("factorio.settings", "max_players", 10); + assert.equal(grandchild.config.get("factorio.settings").max_players, 10); + assert.deepEqual(updated.sort(), [11, 12]); + }); + }); + + describe("inheritor changes", function () { + it("should stop inheriting a field that is changed", function () { + child.config.set("factorio.version", "2.1"); + assert.equal(child.config.isInherited("factorio.version"), false); + base.config.set("factorio.version", "2.0.1"); + assert.equal(child.config.get("factorio.version"), "2.1"); + }); + it("should stop inheriting with overrideFields() even if the value is unchanged", function () { + instances.overrideFields(child, ["factorio.version", "factorio.game_port"]); + assert.equal(child.config.isInherited("factorio.version"), false); + }); + }); + + describe(".inheritField()", function () { + it("should restore the base value", function () { + child.config.set("factorio.version", "2.1"); + instances.inheritField(11, "factorio.version"); + assert.equal(child.config.get("factorio.version"), "2.0"); + assert.equal(child.config.isInherited("factorio.version"), true); + }); + it("should throw if the instance has no base", function () { + assert.throws( + () => instances.inheritField(1, "factorio.version"), + new lib.ValidationError("Instance does not have a base"), + ); + }); + it("should throw for fields that cannot be inherited", function () { + assert.throws( + () => instances.inheritField(11, "instance.name"), + new lib.ValidationError("Field instance.name cannot be inherited"), + ); + }); + }); + + describe("base_id changes", function () { + it("should inherit fields equal to the new base", function () { + instance.config.set("factorio.version", "2.0"); + instance.config.set("instance.base_id", 10); + assert.equal(instance.config.isInherited("factorio.version"), true); + assert.equal(instance.config.isInherited("factorio.settings"), false); + }); + it("should clear inherited fields when the base is removed", function () { + child.config.set("instance.base_id", null); + assert.deepEqual(child.config.get("instance.inherited_fields"), {}); + assert.equal(child.config.get("factorio.version"), "2.0"); + }); + }); + + describe(".checkInheritance()", function () { + it("should accept a valid base", function () { + instances.checkInheritance(instance.config, false, 10); + }); + it("should reject a missing base", function () { + assert.throws( + () => instances.checkInheritance(instance.config, false, 99), + new lib.NotFoundError("Instance with ID 99 does not exist"), + ); + }); + it("should reject an instance that is not a base", function () { + assert.throws( + () => instances.checkInheritance(instance.config, false, 11), + new lib.ValidationError("Instance Instance 11 is not a base"), + ); + }); + it("should reject cycles", function () { + child.config.set("instance.is_base", true); + assert.throws( + () => instances.checkInheritance(base.config, true, 11), + new lib.ValidationError("Base instances cannot inherit from themselves"), + ); + }); + it("should reject making an assigned instance a base", function () { + instance.config.set("instance.assigned_host", 5); + assert.throws( + () => instances.checkInheritance(instance.config, true, null), + new lib.ConflictError("Instance must be unassigned before it can be made a base"), + ); + }); + it("should reject unmarking a base that has inheritors", function () { + assert.throws( + () => instances.checkInheritance(base.config, false, null), + new lib.ConflictError("Instance is the base of 1 other instance(s)"), + ); + }); + }); + + it("should not assign a base to a host", async function () { + controller.wsServer.hostConnections.set(5, { send: async () => {}, connector: { closing: false } }); + await assert.rejects( + instances.assignInstance(10, 5), + new lib.ValidationError("Base instances cannot be assigned to a host"), + ); + }); + + it("should not delete a base that has inheritors", async function () { + await assert.rejects( + instances.deleteInstance(10), + new lib.ConflictError("Instance is the base of 1 other instance(s)"), + ); + }); + + it("should report bases in instance details", function () { + assert.equal(base.toInstanceDetails().isBase, true); + assert.equal(child.toInstanceDetails().isBase, false); + }); + }); }); diff --git a/test/integration/clusterio.js b/test/integration/clusterio.js index 54dcd29b..d25fc29b 100644 --- a/test/integration/clusterio.js +++ b/test/integration/clusterio.js @@ -971,6 +971,63 @@ describe("Integration of Clusterio", function() { }); }); + describe("instance base inheritance", function() { + after(async function() { + for (const id of [4402, 4401, 4400]) { + await getControl().send(new lib.InstanceDeleteRequest(id)).catch(() => {}); + } + }); + it("creates a base and an instance inheriting from it", async function() { + await execCtl("instance create inheritBase --id 4400 --is-base"); + await execCtl("instance config set inheritBase factorio.version 2.0"); + await execCtl("instance create inheritChild --id 4401 --base inheritBase"); + const config = await getControl().send(new lib.InstanceConfigGetRequest(4401)); + assert.equal(config["instance.base_id"], 4400); + assert.equal(config["factorio.version"], "2.0"); + assert.equal(config["instance.inherited_fields"]["factorio.version"], true); + assert.equal((await getInstances()).get(4400).isBase, true); + }); + it("propagates base changes", async function() { + await execCtl("instance config set inheritBase factorio.version 2.1"); + const config = await getControl().send(new lib.InstanceConfigGetRequest(4401)); + assert.equal(config["factorio.version"], "2.1"); + }); + it("rejects inheriting from an instance that is not a base", async function() { + await assert.rejects( + execCtl("instance create inheritBad --id 4402 --base inheritChild"), + new lib.ValidationError("Instance inheritChild is not a base"), + ); + }); + it("rejects assigning a base to a host", async function() { + await assert.rejects( + execCtl("instance assign inheritBase 4"), + new lib.ValidationError("Base instances cannot be assigned to a host"), + ); + }); + describe("instance list --bases", function() { + it("runs", async function() { + await execCtl("instance list --bases"); + }); + }); + describe("instance config list", function() { + it("runs on an inheriting instance", async function() { + await execCtl("instance config list inheritChild"); + }); + }); + describe("instance config inherit", function() { + it("runs", async function() { + await execCtl("instance config set inheritChild factorio.version 2.0"); + let config = await getControl().send(new lib.InstanceConfigGetRequest(4401)); + assert.equal(config["instance.inherited_fields"]["factorio.version"], undefined); + + await execCtl("instance config inherit inheritChild factorio.version"); + config = await getControl().send(new lib.InstanceConfigGetRequest(4401)); + assert.equal(config["factorio.version"], "2.1"); + assert.equal(config["instance.inherited_fields"]["factorio.version"], true); + }); + }); + }); + describe("instance assign", function() { it("creates the instance files", async function() { await execCtl(`instance assign test ${hostId}`); diff --git a/test/integration/routing.js b/test/integration/routing.js index 19c73dfa..4bfc7834 100644 --- a/test/integration/routing.js +++ b/test/integration/routing.js @@ -41,7 +41,8 @@ function connectInstance(controller, host, instanceId) { instanceConfig.set("instance.id", instanceId); instanceConfig.set("instance.assigned_host", host.config.get("host.id")); host.assignedInstances.set(instanceId, { path: "invalid", config: instanceConfig }); - controller.instances.records.set(new InstanceRecord(instanceConfig, "stopped")); + const controllerConfig = new lib.InstanceConfig("controller", instanceConfig.toRemote("controller")); + controller.instances.records.set(new InstanceRecord(controllerConfig, "stopped")); const instance = new Instance(host, instanceSide, "invalid", instanceConfig, new MockServer()); const instanceConnection = new InstanceConnection(hostSide, host, instance); host.instanceConnections.set(instanceId, instanceConnection); diff --git a/test/lib/config/definitions.js b/test/lib/config/definitions.js index a406f579..33cc097e 100644 --- a/test/lib/config/definitions.js +++ b/test/lib/config/definitions.js @@ -4,6 +4,48 @@ import * as lib from "@clusterio/lib"; describe("lib/config/definitions", function() { + describe("InstanceConfig", function() { + describe("isInheritable()", function() { + it("should allow regular fields", function() { + assert.equal(lib.InstanceConfig.isInheritable("factorio.version"), true); + assert.equal(lib.InstanceConfig.isInheritable("instance.auto_start"), true); + }); + it("should reject identity, hidden, host only and unknown fields", function() { + for (const name of [ + "instance.name", "instance.id", "instance.assigned_host", "instance.is_base", + "instance.base_id", "instance.inherited_fields", "factorio.host_assigned_game_port", "no.field", + ]) { + assert.equal(lib.InstanceConfig.isInheritable(name), false, name); + } + }); + }); + describe(".isInherited()", function() { + it("should only report fields as inherited when a base is set", function() { + const config = new lib.InstanceConfig("controller", { + "instance.inherited_fields": { "factorio.version": true }, + }); + assert.equal(config.isInherited("factorio.version"), false); + config.set("instance.base_id", 1); + assert.equal(config.isInherited("factorio.version"), true); + assert.equal(config.isInherited("factorio.game_port"), false); + }); + }); + it("should not send inheritance fields to the host", function() { + const config = new lib.InstanceConfig("controller", { "instance.base_id": 1 }); + const remote = config.toRemote("host"); + for (const name of ["instance.is_base", "instance.base_id", "instance.inherited_fields"]) { + assert(!Object.hasOwn(remote, name), name); + } + }); + it("should not let control write inherited_fields", function() { + const config = new lib.InstanceConfig("controller"); + assert.throws( + () => config.set("instance.inherited_fields", { "factorio.version": true }, "control"), + new lib.InvalidAccessError("Field 'instance.inherited_fields' is not accessible from control"), + ); + }); + }); + describe("addPluginConfigFields()", function() { after(function() { delete lib.CtlConfig.fieldDefinitions["ctl_test.field"];