From fc3686b86567f4354c91de9899d931d29d99bf21 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Sat, 30 May 2020 14:18:19 +0200 Subject: [PATCH 1/5] Invoke plugin hooks via common function Use a common function to iterate through and invoke the given hook in every plugin, and ignore errors thrown by plugins from these hooks. --- lib/plugin.js | 25 +++++++++++++++++++++++++ master.js | 36 ++++++++++-------------------------- slave.js | 33 +++++++++------------------------ test/lib/plugin.js | 27 +++++++++++++++++++++++++++ 4 files changed, 71 insertions(+), 50 deletions(-) diff --git a/lib/plugin.js b/lib/plugin.js index 81a66330..3e08cd0f 100644 --- a/lib/plugin.js +++ b/lib/plugin.js @@ -334,6 +334,30 @@ function attachPluginMessages(link, pluginInfo, plugin) { } } +/** + * Invokes the given hook on all plugins + * + * @param {Map { link.messages.instanceOutput.send(this, { instance_id: this.config.get("instance.id"), output }) - for (let [name, plugin] of this.plugins) { - plugin.onOutput(output).catch(err => { - console.error(`Plugin ${name} raised error in onOutput:`, err); - }); - } + plugin.invokeHook(this.plugins, "onOutput", output); }); this.server.on("error", err => { @@ -337,9 +333,7 @@ class Instance extends link.Link{ await this.server.disableAchievements() await this.updateInstanceData(); - for (let pluginInstance of this.plugins.values()) { - await pluginInstance.onStart(); - } + await plugin.invokeHook(this.plugins, "onStart"); this._running = true; link.messages.instanceStarted.send(this, { instance_id: this.config.get("instance.id") }); @@ -362,35 +356,26 @@ class Instance extends link.Link{ // XXX this needs more thought to it if (this.server._state === "running") { - for (let pluginInstance of this.plugins.values()) { - await pluginInstance.onStop(); - } - + await plugin.invokeHook(this.plugins, "onStop"); await this.server.stop(); } } async masterConnectionEventEventHandler(message) { - for (let pluginInstance of this.plugins.values()) { - pluginInstance.onMasterConnectionEvent(message.data.event); - } + await plugin.invokeHook(this.plugins, "onMasterConnectionEvent", message.data.event); } async prepareMasterDisconnectRequestHandler() { - for (let pluginInstance of this.plugins.values()) { - await pluginInstance.onPrepareMasterDisconnect(); - } + await plugin.invokeHook(this.plugins, "onPrepareMasterDisconnect"); } async getMetricsRequestHandler() { let results = [] if (this._running) { - for (let pluginInstance of this.plugins.values()) { - let pluginResults = await pluginInstance.onMetrics(); - if (pluginResults !== undefined) { - for await (let result of pluginResults) { - results.push(prometheus.serializeResult(result)) - } + let pluginResults = await plugin.invokeHook(this.plugins, "onMetrics"); + for (let metricIterator of pluginResults) { + for await (let metric of metricIterator) { + results.push(prometheus.serializeResult(metric)) } } } diff --git a/test/lib/plugin.js b/test/lib/plugin.js index 5e915e8a..3e147065 100644 --- a/test/lib/plugin.js +++ b/test/lib/plugin.js @@ -119,4 +119,31 @@ describe("lib/plugin", function() { ); }); }); + + describe("invokeHook()", function() { + let betaTestCalled = false; + let plugins = new Map([ + ["alpha", { + test: async function() { }, + pass: async function(arg) { return arg; }, + error: async function() { throw new Error("Test"); }, + }], + ["beta", { + test: async function() { betaTestCalled = true; }, + pass: async function() { }, + error: async function() { }, + }], + ]); + it("should invoke the hook on the plugin", async function() { + await plugin.invokeHook(plugins, "test"); + assert(betaTestCalled, "Hook was not called"); + }); + it("should pass and return args", async function() { + let result = await plugin.invokeHook(plugins, "pass", 1234); + assert.deepEqual(result, [1234]); + }); + it("should ignore errors", async function() { + await plugin.invokeHook(plugins, "error"); + }); + }); }); From 67ca8dcc4db1082ef998a0aec580d8532b016944 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Sat, 30 May 2020 14:25:50 +0200 Subject: [PATCH 2/5] Implement config notification event Add fieldChanged event to config objects that is triggered when the value of a field in a group belonging to it is changed. --- lib/config/classes.js | 49 ++++++++++++++++++++++++++++-------- lib/plugin.js | 13 ++++++++++ slave.js | 9 ++++++- test/lib/config/classes.js | 51 ++++++++++++++++++++++++++++++-------- 4 files changed, 101 insertions(+), 21 deletions(-) diff --git a/lib/config/classes.js b/lib/config/classes.js index 13196729..4c5c0351 100644 --- a/lib/config/classes.js +++ b/lib/config/classes.js @@ -1,6 +1,9 @@ // Configuration classes "use strict"; +const events = require("events"); +const util = require("util"); + const { basicType } = require("lib/helpers"); @@ -55,10 +58,20 @@ function splitOn(separator, string) { * Represents a collection of ConfigGroup instances that can be * managed as one. * + * This is an EventEmitter with the following events: + * - fieldChanged - + * invoked after a field has been updated in one of the config groups + * belonging to this config. Takes three parameters: group, field, + * prev. + * + * @extends events.EventEmitter + * * @memberof module:lib/config */ -class Config { +class Config extends events.EventEmitter { constructor() { + super(); + if (!this.constructor._finalized) { throw new Error(`Cannot instantiate incomplete Config class ${this.constructor.name}`) } @@ -71,7 +84,7 @@ class Config { for (let [name, GroupClass] of this.constructor._groups) { if (!this._groups.has(name)) { let group = new GroupClass(); - await group.init(); + await group.init(this); this._groups.set(name, group); } } @@ -98,7 +111,7 @@ class Config { } let group = new GroupClass(); - await group.load(serializedGroup); + await group.load(this, serializedGroup); this._groups.set(GroupClass.groupName, group); } @@ -123,12 +136,12 @@ class Config { * Updates all groups in the config with groups in the serialized * config passed. */ - update(serializedConfig) { + update(serializedConfig, notify) { this._validate(serializedConfig); for (let serializedGroup of serializedConfig.groups) { let group = this._groups.get(serializedGroup.name); if (group) { - group.update(serializedGroup); + group.update(serializedGroup, notify); } else { this._unknownGroups.set(serializedGroup.name, serializedGroup); } @@ -226,6 +239,11 @@ class Config { /** * Collection of related config entries * + * Type checks and stores a collection of related config entries. Attempt + * to set invalid values for fields will result in an error thrown while + * loading a config with invalid values will result in those being replaced + * by the innitial value for those fields. + * * @memberof module:lib/config */ class ConfigGroup { @@ -242,6 +260,7 @@ class ConfigGroup { this._fields = new Map(); this._unknownFields = new Map(); + this._config = null; } /** @@ -250,7 +269,9 @@ class ConfigGroup { * Computes and assigns the initial values for all fields of the * config group. */ - async init() { + async init(config) { + this._config = config; + for (let [name, def] of this.constructor._definitions) { if (this._fields.has(name)) { continue; @@ -275,9 +296,9 @@ class ConfigGroup { * group and then initializes any possible missing fields from it to * their default values. */ - async load(serializedGroup) { - this.update(serializedGroup); - await this.init(); + async load(config, serializedGroup) { + this.update(serializedGroup, false); + await this.init(config); } /** @@ -345,7 +366,11 @@ class ConfigGroup { } } + let prev = this._fields.get(name); this._fields.set(name, value); + if (this._config && !util.isDeepStrictEqual(value, prev)) { + this._config.emit("fieldChanged", this, name, prev); + } } /** @@ -354,7 +379,7 @@ class ConfigGroup { * Overwrites the values of all the fields in this config group that * has a valid value in the passed serialized group with that value. */ - update(serializedGroup) { + update(serializedGroup, notify) { if (basicType(serializedGroup) !== "object") { throw new Error(`Expected object, not ${basicType(serializedGroup)} for ConfigGroup`); } @@ -388,7 +413,11 @@ class ConfigGroup { continue; } + let prev = this._fields.get(name); this._fields.set(name, value); + if (notify && this._config && !util.isDeepStrictEqual(value, prev)) { + this._config.emit("fieldChanged", this, name, prev); + } } } diff --git a/lib/plugin.js b/lib/plugin.js index 3e08cd0f..f2d2a080 100644 --- a/lib/plugin.js +++ b/lib/plugin.js @@ -42,6 +42,19 @@ class BaseInstancePlugin { */ async init() { } + /** + * Called when the value of a config field changed. + * + * Invoked after the value of the config field given by `field` has + * changed. + * + * @param {module:lib/config.ConfigGroup} group - + * The group who's field got changed on. + * @param {string} field - Name of the field that changed. + * @param prev - The previous value of the field. + */ + async onInstanceConfigFieldChanged(group, field, prev) { } + /** * Called before collecting Prometheus metrics * diff --git a/slave.js b/slave.js index 15390cb7..8932aacb 100644 --- a/slave.js +++ b/slave.js @@ -68,6 +68,11 @@ class Instance extends link.Link{ this.plugins = new Map(); this.config = instanceConfig; + this._configFieldChanged = (group, field, prev) => { + plugin.invokeHook(this.plugins, "onInstanceConfigFieldChanged", group, field, prev); + }; + this.config.on("fieldChanged", this._configFieldChanged); + let serverOptions = { version: this.config.get("factorio.version"), gamePort: this.config.get('factorio.game_port'), @@ -112,6 +117,8 @@ class Instance extends link.Link{ this._running = false; link.messages.instanceStopped.send(this, { instance_id: this.config.get("instance.id") }); + this.config.off("fieldChanged", this._configFieldChanged); + // Clear metrics this instance is exporting for (let collector of prometheus.defaultRegistry.collectors) { if ( @@ -756,7 +763,7 @@ class Slave extends link.Link { let { instance_id, serialized_config } = message.data; let instanceInfo = this.instanceInfos.get(instance_id); if (instanceInfo) { - instanceInfo.config.update(serialized_config); + instanceInfo.config.update(serialized_config, true); console.log(`Clusterio | Updated config for ${instanceInfo.path}`); // TODO: Notify of update if instance is running diff --git a/test/lib/config/classes.js b/test/lib/config/classes.js index 6e2f0bac..7f7c862c 100644 --- a/test/lib/config/classes.js +++ b/test/lib/config/classes.js @@ -177,7 +177,7 @@ describe("lib/config/classes", function() { describe(".load()", function() { it("should load a basic group", async function() { let testInstance = new TestGroup(); - await testInstance.load({ name: "test_group", fields: { + await testInstance.load(null, { name: "test_group", fields: { enum: "c", test: "blah", func: 22, @@ -193,7 +193,7 @@ describe("lib/config/classes", function() { }); it("should load defaults for missing serialized fields", async function() { let testInstance = new TestGroup(); - await testInstance.load({ name: "test_group", fields: { enum: "a" } }); + await testInstance.load(null, { name: "test_group", fields: { enum: "a" } }); assert.equal(testInstance.get("enum"), "a"); assert.equal(testInstance.get("test"), null); @@ -213,7 +213,7 @@ describe("lib/config/classes", function() { } let testInstance = new TestGroup(); - await testInstance.load({ name: "test_group", fields: testFields }); + await testInstance.load(null, { name: "test_group", fields: testFields }); assert.deepEqual(testInstance.serialize().fields, testFields); }); @@ -223,7 +223,7 @@ describe("lib/config/classes", function() { it("should skip updating invalid values", async function() { let testInstance = new TestGroup(); await testInstance.init(); - testInstance.update({ name: "test_group", fields: { enum: "wrong", test: 3, func: null }}); + testInstance.update({ name: "test_group", fields: { enum: "wrong", test: 3, func: null }}, false); assert.equal(testInstance.get("enum"), "b"); assert.equal(testInstance.get("test"), null); @@ -237,19 +237,19 @@ describe("lib/config/classes", function() { new Error("Expected object, not undefined for ConfigGroup") ); assert.throws( - () => testInstance.update({}), + () => testInstance.update({}, false), new Error("Expected group name test_group, not undefined") ); assert.throws( - () => testInstance.update({ name: "test_group" }), + () => testInstance.update({ name: "test_group" }, false), new Error("Expected fields to be an object, not undefined") ); assert.throws( - () => testInstance.update({ name: "test_group", fields: []}), + () => testInstance.update({ name: "test_group", fields: []}, false), new Error("Expected fields to be an object, not array") ); assert.throws( - () => testInstance.update({ name: "test_group", fields: null}), + () => testInstance.update({ name: "test_group", fields: null}, false), new Error("Expected fields to be an object, not null") ); }); @@ -261,7 +261,7 @@ describe("lib/config/classes", function() { testInstance = new TestGroup(); testInstance.update({ name: "test_group", fields: { enum: "a", test: "blah", func: 27, - }}); + }}, false); }); it("should throw if field does not exist", function() { @@ -500,7 +500,7 @@ describe("lib/config/classes", function() { name: "beta", fields: { bar: 30 }, } - ]}); + ]}, false); assert.deepEqual(testInstance.serialize(), { groups: [ @@ -563,5 +563,36 @@ describe("lib/config/classes", function() { }); }); + describe("fieldChanged event", function() { + it("should be called when setting a field", async function() { + let testInstance = new TestConfig(); + await testInstance.init(); + + let called = false; + testInstance.once("fieldChanged", (group, field, prev) => { + if (group.constructor.groupName === "alpha" && field === "foo") { + called = true; + } + }); + + testInstance.set("alpha.foo", "new value"); + assert(called, "fieldChanged was not called"); + }); + + it("should be called when updating a config", async function() { + let testInstance = new TestConfig(); + await testInstance.init(); + + let called = false; + testInstance.once("fieldChanged", (group, field, prev) => { + if (group.constructor.groupName === "alpha" && field === "foo") { + called = true; + } + }); + + testInstance.update({ groups: [{ name: "alpha", fields: { foo: "new value" }}]}, true); + assert(called, "fieldChanged was not called"); + }); + }); }); }); From 8c9daff0bbb991596439b4c9d3852938bddba865 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Sun, 31 May 2020 14:16:06 +0200 Subject: [PATCH 3/5] Add name shortcut for ConfigGroup instances Add .name shortcut to get the groupName class property of config group instances. --- lib/config/classes.js | 17 +++++++++++++---- test/lib/config/classes.js | 8 ++++++++ 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/lib/config/classes.js b/lib/config/classes.js index 4c5c0351..14ee1b8b 100644 --- a/lib/config/classes.js +++ b/lib/config/classes.js @@ -112,7 +112,7 @@ class Config extends events.EventEmitter { let group = new GroupClass(); await group.load(this, serializedGroup); - this._groups.set(GroupClass.groupName, group); + this._groups.set(group.name, group); } await this.init(); @@ -301,6 +301,15 @@ class ConfigGroup { await this.init(config); } + /** + * Name of the group + * + * Shortcut for `group.constructor.groupName` + */ + get name() { + return this.constructor.groupName; + } + /** * Get value for field */ @@ -384,8 +393,8 @@ class ConfigGroup { throw new Error(`Expected object, not ${basicType(serializedGroup)} for ConfigGroup`); } - if (serializedGroup.name !== this.constructor.groupName) { - throw new Error(`Expected group name ${this.constructor.groupName}, not ${serializedGroup.name}`); + if (serializedGroup.name !== this.name) { + throw new Error(`Expected group name ${this.name}, not ${serializedGroup.name}`); } if (basicType(serializedGroup.fields) !== "object") { @@ -435,7 +444,7 @@ class ConfigGroup { } return { - name: this.constructor.groupName, + name: this.name, fields, }; } diff --git a/test/lib/config/classes.js b/test/lib/config/classes.js index 7f7c862c..614a020b 100644 --- a/test/lib/config/classes.js +++ b/test/lib/config/classes.js @@ -156,6 +156,14 @@ describe("lib/config/classes", function() { }); }); + describe("get name", function() { + it("should give the name of the group", async function() { + let testInstance = new TestGroup(); + await testInstance.init(); + assert.equal(testInstance.name, "test_group"); + }); + }); + describe(".serialize()", function() { it("should serialize a basic group", async function() { let testInstance = new TestGroup(); From 5b043524b4b35580d1bb3167303b12b123b42eb8 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Sun, 31 May 2020 16:08:21 +0200 Subject: [PATCH 4/5] Implement setProp in configs Add setProp method for selectively changing one property of an object field in a config group. --- lib/config/classes.js | 45 ++++++++++++++++++- test/lib/config/classes.js | 89 ++++++++++++++++++++++++++------------ 2 files changed, 106 insertions(+), 28 deletions(-) diff --git a/lib/config/classes.js b/lib/config/classes.js index 14ee1b8b..1e3d1c39 100644 --- a/lib/config/classes.js +++ b/lib/config/classes.js @@ -164,6 +164,14 @@ class Config extends events.EventEmitter { return this.group(group).set(field, value); } + /** + * Set property of an object value for a config field + */ + setProp(name, prop, value) { + let [group, field] = splitOn('.', name); + return this.group(group).setProp(field, prop, value); + } + /** * Get the config group instance with the given name */ @@ -327,12 +335,13 @@ class ConfigGroup { * The field must be defined for the config group and the value is * type checked against the field definition. * + * @throws {module:lib/config.InvalidField} if field is not defined. * @throws {module:lib/config.InvalidValue} if value is not allowed for the field. */ set(name, value) { let def = this.constructor._definitions.get(name); if (!def) { - throw new Error(`No field named '${name}'`); + throw new InvalidField(`No field named '${name}'`); } // Empty strings are treates as null @@ -382,6 +391,40 @@ class ConfigGroup { } } + /** + * Set property of object field + * + * Update value of stored object field by setting the specified property + * on it. The field must be defined as an object for the config group. + * + * @throws {module:lib/config.InvalidField} if field is not defined. + * @throws {module:lib/config.InvalidValue} if field is not an object. + */ + setProp(name, prop, value) { + let def = this.constructor._definitions.get(name); + if (!def) { + throw new InvalidField(`No field named '${name}'`); + } + + if (def.type !== "object") { + throw new InvalidValue(`Cannot set property on non-object field '${name}'`); + } + + let prev = this._fields.get(name); + let updated = {}; + if (prev !== null) { + for (let [key, value] of Object.entries(prev)) { + updated[key] = value; + } + } + + updated[prop] = value; + this._fields.set(name, updated); + if (this._config && !util.isDeepStrictEqual(updated, prev)) { + this._config.emit("fieldChanged", this, name, prev); + } + } + /** * Update a config group from a serialized group * diff --git a/test/lib/config/classes.js b/test/lib/config/classes.js index 614a020b..e094e4a2 100644 --- a/test/lib/config/classes.js +++ b/test/lib/config/classes.js @@ -73,7 +73,7 @@ describe("lib/config/classes", function() { TestGroup.define({ name: "test", type: "string", optional: true }); TestGroup.define({ name: "func", type: "number", initial_value: () => 42 }); TestGroup.define({ name: "bool", type: "boolean", initial_value: false, optional: true }); - TestGroup.define({ name: "json", type: "object", initial_value: {} }); + TestGroup.define({ name: "json", type: "object", initial_value: {}, optional: true }); assert(TestGroup._definitions.has("test"), "field test was not defined"); assert(TestGroup._definitions.has("func"), "field func was not defined"); @@ -265,17 +265,17 @@ describe("lib/config/classes", function() { describe(".set()", function() { let testInstance; - before(function() { + before(async function() { testInstance = new TestGroup(); - testInstance.update({ name: "test_group", fields: { + await testInstance.load(null, { name: "test_group", fields: { enum: "a", test: "blah", func: 27, - }}, false); + }}); }); it("should throw if field does not exist", function() { assert.throws( () => testInstance.set("bar", 1), - new Error("No field named 'bar'") + new classes.InvalidField("No field named 'bar'") ); }) @@ -359,6 +359,42 @@ describe("lib/config/classes", function() { ); }); }); + + describe(".setProp()", function() { + let testInstance; + before(async function() { + testInstance = new TestGroup(); + await testInstance.load(null, { name: "test_group", fields: { + enum: "a", test: "blah", func: 27, + }}); + }); + + it("should throw if field does not exist", function() { + assert.throws( + () => testInstance.setProp("bar", 1), + new classes.InvalidField("No field named 'bar'") + ); + }) + + it("should throw if field is not an object", function() { + assert.throws( + () => testInstance.setProp("enum", "a"), + new classes.InvalidField("Cannot set property on non-object field 'enum'") + ); + }) + + it("should work if field is an object", function() { + testInstance.set("json", { prev: 32, test: false }); + testInstance.setProp("json", "test", true); + assert.deepEqual(testInstance.get("json"), { prev: 32, test: true }); + }); + + it("should handle field being null", function() { + testInstance.set("json", null); + testInstance.setProp("json", "test", true); + assert.deepEqual(testInstance.get("json"), { test: true }); + }); + }); }); describe("Config", function() { @@ -372,7 +408,7 @@ describe("lib/config/classes", function() { AlphaGroup.define({ name: "foo", type: "string", optional: true }); AlphaGroup.finalize(); TestConfig.registerGroup(AlphaGroup); - BetaGroup.define({ name: "bar", type: "number", initial_value: 2 }); + BetaGroup.define({ name: "bar", type: "object", initial_value: {} }); BetaGroup.finalize(); TestConfig.registerGroup(BetaGroup); }); @@ -424,7 +460,7 @@ describe("lib/config/classes", function() { let testInstance = new TestConfig(); await testInstance.init(); assert.equal(testInstance.get("alpha.foo"), null); - assert.equal(testInstance.get("beta.bar"), 2); + assert.deepEqual(testInstance.get("beta.bar"), {}); }) }); @@ -440,7 +476,7 @@ describe("lib/config/classes", function() { }, { name: "beta", - fields: { bar: 2 }, + fields: { bar: {} }, } ] }); @@ -469,7 +505,7 @@ describe("lib/config/classes", function() { await testInstance.load({ groups: [{ name: "alpha", fields: { foo: "a" }}] }); assert.equal(testInstance.get("alpha.foo"), "a"); - assert.equal(testInstance.get("beta.bar"), 2); + assert.deepEqual(testInstance.get("beta.bar"), {}); }); it("should preserve unknown groups when serialized back", async function() { @@ -484,7 +520,7 @@ describe("lib/config/classes", function() { }, { name: "beta", - fields: { bar: 20 }, + fields: { bar: { value: 20 }}, } ] @@ -506,7 +542,7 @@ describe("lib/config/classes", function() { }, { name: "beta", - fields: { bar: 30 }, + fields: { bar: { value: 30 }}, } ]}, false); @@ -522,7 +558,7 @@ describe("lib/config/classes", function() { }, { name: "beta", - fields: { bar: 30 }, + fields: { bar: { value: 30 }}, } ], }); @@ -572,33 +608,32 @@ describe("lib/config/classes", function() { }); describe("fieldChanged event", function() { - it("should be called when setting a field", async function() { - let testInstance = new TestConfig(); + let testInstance; + let called; + beforeEach(async function() { + testInstance = new TestConfig(); await testInstance.init(); - let called = false; + called = false; testInstance.once("fieldChanged", (group, field, prev) => { - if (group.constructor.groupName === "alpha" && field === "foo") { + if (group.name === "beta" && field === "bar") { called = true; } }); + }); - testInstance.set("alpha.foo", "new value"); + it("should be called when setting a field", async function() { + testInstance.set("beta.bar", { value: 1 }); assert(called, "fieldChanged was not called"); }); it("should be called when updating a config", async function() { - let testInstance = new TestConfig(); - await testInstance.init(); + testInstance.update({ groups: [{ name: "beta", fields: { bar: { value: 1 }}}]}, true); + assert(called, "fieldChanged was not called"); + }); - let called = false; - testInstance.once("fieldChanged", (group, field, prev) => { - if (group.constructor.groupName === "alpha" && field === "foo") { - called = true; - } - }); - - testInstance.update({ groups: [{ name: "alpha", fields: { foo: "new value" }}]}, true); + it("should be called when setting a prop", async function() { + testInstance.setProp("beta.bar", "value", 2); assert(called, "fieldChanged was not called"); }); }); From 931ad4816f143ddfbc156565944645863a9af214 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Sun, 31 May 2020 22:28:51 +0200 Subject: [PATCH 5/5] Update config of running Factorio instances Monitor the factorio.settings config for changes and apply them to running instances via the /config set command. --- clusterctl.js | 20 +++++++ lib/link/messages.js | 11 ++++ master.js | 33 ++++++---- slave.js | 109 +++++++++++++++++++++++++++++----- test/integration/clusterio.js | 87 ++++++++++++++++++++++++++- test/integration/index.js | 37 ++++++++++++ test/integration/link.js | 25 +------- test/slave.js | 3 +- 8 files changed, 274 insertions(+), 51 deletions(-) diff --git a/clusterctl.js b/clusterctl.js index 4680f6e8..f7dbee34 100644 --- a/clusterctl.js +++ b/clusterctl.js @@ -223,6 +223,26 @@ commands.push(new Command({ }, })); +commands.push(new Command({ + definition: ["set-instance-config-prop", "Set property of field in instance config", (yargs) => { + yargs.option({ + "instance": { describe: "Instance to set config on", nargs: 1, type: "string", demandOption: true }, + "field": { describe: "Field to set", nargs: 1, type: "string", demandOption: true }, + "prop": { describe: "Property to set", nargs: 1, type: "string", demandOption: true }, + "value": { describe: "JSON parsed value to set", nargs: 1, type: "string", demandOption: true }, + }); + }], + handler: async function(args, control) { + let instanceId = await resolveInstance(control, args.instance); + await link.messages.setInstanceConfigProp.send(control, { + instance_id: instanceId, + field: args.field, + prop: args.prop, + value: JSON.parse(args.value), + }); + }, +})); + commands.push(new Command({ definition: ['assign-instance', "Assign instance to a slave", (yargs) => { yargs.options({ diff --git a/lib/link/messages.js b/lib/link/messages.js index 408ae4b3..effda751 100644 --- a/lib/link/messages.js +++ b/lib/link/messages.js @@ -312,6 +312,17 @@ messages.setInstanceConfigField = new Request({ }, }); +messages.setInstanceConfigProp = new Request({ + type: "set_instance_config_prop", + links: ["control-master"], + requestProperties: { + "instance_id": { type: "integer" }, + "field": { type: "string" }, + "prop": { type: "string" }, + "value": {}, + }, +}); + messages.assignInstanceCommand = new Request({ type: 'assign_instance_command', links: ['control-master'], diff --git a/master.js b/master.js index 196b7c77..05a7340a 100644 --- a/master.js +++ b/master.js @@ -529,6 +529,19 @@ class ControlConnection extends BaseConnection { } } + async updateInstanceConfig(instance) { + let slaveId = instance.config.get("instance.assigned_slave"); + if (slaveId) { + let connection = slaveConnections.get(slaveId); + if (connection) { + await link.messages.assignInstance.send(connection, { + instance_id: instance.config.get("instance.id"), + serialized_config: instance.config.serialize(), + }); + } + } + } + async setInstanceConfigFieldRequestHandler(message) { let instance = db.instances.get(message.data.instance_id); if (!instance) { @@ -545,18 +558,18 @@ class ControlConnection extends BaseConnection { } instance.config.set(message.data.field, message.data.value); + await this.updateInstanceConfig(instance); + } - // Push updated config to slave - let slaveId = instance.config.get('instance.assigned_slave'); - if (slaveId) { - let connection = slaveConnections.get(slaveId); - if (connection) { - await link.messages.assignInstance.send(connection, { - instance_id: instance.config.get("instance.id"), - serialized_config: instance.config.serialize(), - }); - } + async setInstanceConfigPropRequestHandler(message) { + let instance = db.instances.get(message.data.instance_id); + if (!instance) { + throw new errors.RequestError(`Instance with ID ${message.data.instance_id} does not exist`); } + + let {field, prop, value} = message.data; + instance.config.setProp(field, prop, value); + await this.updateInstanceConfig(instance); } async assignInstanceCommandRequestHandler(message, request) { diff --git a/slave.js b/slave.js index 8932aacb..8b05fa4f 100644 --- a/slave.js +++ b/slave.js @@ -17,6 +17,7 @@ const events = require("events"); const pidusage = require("pidusage"); const setBlocking = require("set-blocking"); const phin = require("phin"); +const util = require("util"); const version = require("./package").version; // internal libraries @@ -55,6 +56,49 @@ const instanceFactorioAutosaveSize = new prometheus.Gauge( { labels: ["instance_id"] } ); +function applyAsConfig(name) { + return async function action(server, value) { + if (name === "tags" && value instanceof Array) { + // Replace spaces with non-break spaces and delimit by spaces. + // This does change the defined tags, but there doesn't seem to + // be a way to include a space into a tag from the console. + value = value.map(tag => tag.replace(/ /g, "\u00a0")).join(" "); + } + try { + await server.sendRcon(`/config set ${name} ${value}`); + } catch (err) { + console.error(`Error applying server setting ${name}`, err); + } + } +} + +const serverSettingsActions = { + "afk_autokick_interval": applyAsConfig("afk-auto-kick"), + "allow_commands": applyAsConfig("allow-commands"), + "autosave_interval": applyAsConfig("autosave-interval"), + "autosave_only_on_server": applyAsConfig("autosave-only-on-server"), + "description": applyAsConfig("description"), + "ignore_player_limit_for_returning_players": applyAsConfig("ignore-player-limit-for-returning-players"), + "max_players": applyAsConfig("max-players"), + "max_upload_slots": applyAsConfig("max-upload-slots"), + "max_upload_in_kilobytes_per_second": applyAsConfig("max-upload-speed"), + "name": applyAsConfig("name"), + "only_admins_can_pause_the_game": applyAsConfig("only-admins-can-pause"), + "game_password": applyAsConfig("password"), + "require_user_verification": applyAsConfig("require-user-verification"), + "tags": applyAsConfig("tags"), + "visibility": async function action(server, value) { + for (let scope of ["lan", "public", "steam"]) { + try { + let enabled = Boolean(value[scope]); + await server.sendRcon(`/config set visibility-${scope} ${enabled}`); + } catch (err) { + console.error(`Error applying visibility ${scope}`, err); + } + } + }, +}; + /** * Keeps track of the runtime parameters of an instance */ @@ -69,7 +113,13 @@ class Instance extends link.Link{ this.config = instanceConfig; this._configFieldChanged = (group, field, prev) => { - plugin.invokeHook(this.plugins, "onInstanceConfigFieldChanged", group, field, prev); + let hook = () => plugin.invokeHook(this.plugins, "onInstanceConfigFieldChanged", group, field, prev); + + if (group.name === "factorio" && field === "settings") { + this.updateFactorioSettings(group.get(field), prev).finally(hook); + } else { + hook(); + } }; this.config.on("fieldChanged", this._configFieldChanged); @@ -175,15 +225,16 @@ class Instance extends link.Link{ } /** - * Write the server-settings.json file + * Resolve the effective Factorio server settings * - * Generate the server-settings.json file from the example file in the - * data directory and override any settings configured in the instance's - * factorio_settings config entry. + * Use the example settings as the basis and override it with all the + * entries from the given settings object. + * + * @param {Object} overrides - Server settings to override. + * @returns server example settings with the given settings applied over it. */ - async writeServerSettings() { + async resolveServerSettings(overrides) { let serverSettings = await this.server.exampleSettings(); - let overrides = this.config.get('factorio.settings'); for (let [key, value] of Object.entries(overrides)) { if (!Object.hasOwnProperty.call(serverSettings, key)) { @@ -192,6 +243,18 @@ class Instance extends link.Link{ serverSettings[key] = value; } + return serverSettings; + } + + /** + * Write the server-settings.json file + * + * Generate the server-settings.json file from the example file in the + * data directory and override any settings configured in the instance's + * factorio_settings config entry. + */ + async writeServerSettings() { + let serverSettings = await this.resolveServerSettings(this.config.get("factorio.settings")); await fs.writeFile( this.server.writePath("server-settings.json"), JSON.stringify(serverSettings, null, 4) @@ -355,6 +418,17 @@ class Instance extends link.Link{ await this.server.sendRcon(`/sc clusterio_private.update_instance(${id}, "${name}")`, true); } + async updateFactorioSettings(current, previous) { + current = await this.resolveServerSettings(current); + previous = await this.resolveServerSettings(previous); + + for (let [key, action] of Object.entries(serverSettingsActions)) { + if (current[key] !== undefined && !util.isDeepStrictEqual(current[key], previous[key])) { + await action(this.server, current[key]); + } + } + } + /** * Stop the instance */ @@ -497,13 +571,15 @@ class Instance extends link.Link{ * Searches for instances in the provided directory * * Looks through all sub-dirs of the provided directory for valid - * instance definitions and returns a mapping of instance id to - * instance config objects. + * instance definitions and updates the mapping of instance id to + * instanceInfo objects. * - * @returns {Map} mapping between instance + * @param {Map} instanceInfos - + * mapping between instance id and information about this instance. + * @param {string} instancesDir - Directory containing instances + * @param logger - console like logging interface. */ -async function discoverInstances(instancesDir, logger) { - let instanceInfos = new Map(); +async function discoverInstances(instanceInfos, instancesDir, logger) { for (let entry of await fs.readdir(instancesDir, { withFileTypes: true })) { if (entry.isDirectory()) { let instanceConfig = new config.InstanceConfig(); @@ -521,6 +597,11 @@ async function discoverInstances(instancesDir, logger) { continue; } + // Ignore instances we have already discovered before. + if (instanceInfos.has(instanceConfig.get("instance.id"))) { + continue; + } + let instancePath = path.join(instancesDir, entry.name); logger.log(`found instance ${instanceConfig.get("instance.name")} in ${instancePath}`); instanceInfos.set(instanceConfig.get("instance.id"), { @@ -529,8 +610,6 @@ async function discoverInstances(instancesDir, logger) { }); } } - - return instanceInfos; } class InstanceConnection extends link.Link { @@ -891,7 +970,7 @@ class Slave extends link.Link { * the slave and master server with the new list of instances. */ async updateInstances() { - this.instanceInfos = await discoverInstances(this.config.get("slave.instances_directory"), console); + await discoverInstances(this.instanceInfos, this.config.get("slave.instances_directory"), console); let list = []; for (let [instanceId, instanceInfo] of this.instanceInfos) { let instanceConnection = this.instanceConnections.get(instanceId); diff --git a/test/integration/clusterio.js b/test/integration/clusterio.js index f48353b4..6b507dcb 100644 --- a/test/integration/clusterio.js +++ b/test/integration/clusterio.js @@ -4,7 +4,7 @@ const path = require("path"); const validateHTML = require('html5-validator'); const parallel = require('mocha.parallel'); -const { slowTest, get, exec, controlConfigPath, instancesDir } = require("./index"); +const { slowTest, get, exec, sendRcon, controlConfigPath, instancesDir } = require("./index"); describe("Integration of Clusterio", function() { @@ -82,6 +82,91 @@ describe("Integration of Clusterio", function() { }); }); + describe("set-instance-config-prop", function() { + it("applies factorio settings while running", async function() { + slowTest(this); + + let testConfigs = [ + // json name, value to set, + // /config name, expected result + [ + "afk_autokick_interval", 2, + "afk-auto-kick", "Kick if AFK for more than 2 minutes.", + ], + [ + "allow_commands", true, + "allow-commands", "Yes", + ], + [ + "autosave_interval", 17, + "autosave-interval", "Autosave every 17 minutes.", + ], + [ + "autosave_only_on_server", false, + "autosave-only-on-server", "Autosave only on server: false.", + ], + [ + "description", "A test server blah blah", + "description", "Server description: A test server blah blah", + ], + [ + "ignore_player_limit_for_returning_players", true, + "ignore-player-limit-for-returning-players", "Ignore player limit for returning players: true." + ], + [ + "max_players", 11, + "max-players", "11", + ], + [ + "max_upload_slots", 7, + "max-upload-slots", "7 slots.", + ], + [ + "max_upload_in_kilobytes_per_second", 123, + "max-upload-speed", "123 kilobytes per second.", + ], + [ + "name", "A test", + "name", "Server name: A test", + ], + [ + "only_admins_can_pause_the_game", false, + "only-admins-can-pause", "Only admins can pause: false.", + ], + [ + "game_password", "secret", + "password", "The server currently has a password.", + ], + [ + "tags", ["clusterio", "test-tag"], + "tags", "Server tags: clusterio test-tag", + ], + [ + "visibility", { lan: false, public: false }, + "visibility-lan", "LAN visibility: false.", + ], + + // Public visibility must be reset before verify can be reset + [ + "require_user_verification", false, + "require-user-verification", "Verify user identity: false.", + ], + ]; + + for (let [prop, value, , ] of testConfigs) { + value = `"'${JSON.stringify(value).replace(/"/g, process.platform === "win32" ? '""' : '\\"')}'"`; + let args = `--instance test --field factorio.settings --prop ${prop} --value ${value}`; + await exec(`node clusterctl --config ${controlConfigPath} set-instance-config-prop ${args}`); + } + + // Do this afterwards to leave enough to time for the + // changes to have propogated. + for (let [, , configName, expectedResult] of testConfigs) { + assert.equal(await sendRcon(44, `/config get ${configName}`), `${expectedResult}\n`); + } + }); + }); + describe("stop-instance", function() { it("stops the instance", async function() { slowTest(this); diff --git a/test/integration/index.js b/test/integration/index.js index 49546554..05bb92f5 100644 --- a/test/integration/index.js +++ b/test/integration/index.js @@ -6,9 +6,31 @@ const needle = require("needle"); const util = require("util"); const events = require("events"); +const link = require("lib/link"); const server = require("lib/factorio/server"); +class TestControl extends link.Link { + constructor(connector) { + super("control", "master", connector); + link.attachAllMessages(this); + } + + async prepareDisconnectRequestHandler(message, request) { + this.connector.setClosing(); + return await super.prepareDisconnectRequestHandler(message, request); + } + + async debugWsMessageEventHandler() { } + async instanceOutputEventHandler() { } +} + +class TestControlConnector extends link.WebSocketClientConnector { + register() { + this.sendHandshake("register_control", { token: this.token, agent: "clusterctl", version: "test" }); + } +} + // Mark that this test takes a lot of time, or depeneds on a test // that takes a lot of time. function slowTest(test) { @@ -29,6 +51,7 @@ async function get(path) { let masterProcess; let slaveProcess; +let control; let url = "https://localhost:4443/"; let token = jwt.sign({ id: "api" }, "TestSecretDoNotUse"); @@ -43,6 +66,11 @@ async function exec(...args) { return await util.promisify(child_process.exec)(...args); } +async function sendRcon(instanceId, command) { + let response = await link.messages.sendRcon.send(control, { instance_id: instanceId, command }) + return response.result; +} + function spawn(name, cmd, waitFor) { return new Promise((resolve, reject) => { console.log(cmd); @@ -94,6 +122,11 @@ before(async function() { masterProcess = await spawn("master:", `node master --config ${masterConfigPath} run`, "All plugins loaded"); slaveProcess = await spawn("slave:", `node slave --config ${slaveConfigPath} run`, "SOCKET | received ready from master"); + + let controlConnector = new TestControlConnector(url, 2); + controlConnector.token = token; + control = new TestControl(controlConnector); + await controlConnector.connect(); }); after(async function() { @@ -118,9 +151,13 @@ process.on("exit", () => { module.exports = { + TestControl, + TestControlConnector, slowTest, get, exec, + sendRcon, + control, url, token, diff --git a/test/integration/link.js b/test/integration/link.js index a5f2b95a..ad9f2056 100644 --- a/test/integration/link.js +++ b/test/integration/link.js @@ -1,31 +1,8 @@ const assert = require("assert").strict; const events = require("events"); -const link = require("lib/link"); +const { TestControlConnector, TestControl, get, exec, url, token } = require("./index"); -const { get, exec, url, token } = require("./index"); - - -class TestControl extends link.Link { - constructor(connector) { - super("control", "master", connector); - link.attachAllMessages(this); - } - - async prepareDisconnectRequestHandler(message, request) { - this.connector.setClosing(); - return await super.prepareDisconnectRequestHandler(message, request); - } - - async debugWsMessageEventHandler() { } - async instanceOutputEventHandler() { } -} - -class TestControlConnector extends link.WebSocketClientConnector { - register() { - this.sendHandshake("register_control", { token: this.token, agent: "clusterctl", version: "test" }); - } -} describe("Integration of lib/link", function() { let control; diff --git a/test/slave.js b/test/slave.js index d9309753..1ce8ed1c 100644 --- a/test/slave.js +++ b/test/slave.js @@ -157,7 +157,8 @@ describe("Slave testing", function() { it("should discover test instance", async function() { let logger = { log: () => {}, error: () => {} }; let instancePath = path.join("test", "file", "instances"); - let instanceInfos = await slave._discoverInstances(instancePath, logger); + let instanceInfos = new Map(); + await slave._discoverInstances(instanceInfos, instancePath, logger); let referenceConfig = new config.InstanceConfig(); await referenceConfig.init();