From b82de23876bb85a394c6ddda329f8b9c0b62e08a Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Tue, 29 Sep 2020 01:01:19 +0200 Subject: [PATCH 1/2] Extend instance statuses Add the statuses unknown and unassigned for when the status has not yet been set by a slave and replace the initialized status with starting, creating_save and exporting_data. --- packages/lib/link/messages.js | 20 +++++++++---------- packages/lib/plugin.js | 21 ++++++++++++++++---- packages/master/master.js | 36 +++++++++++------------------------ packages/slave/slave.js | 32 ++++++++++++++++++------------- 4 files changed, 56 insertions(+), 53 deletions(-) diff --git a/packages/lib/link/messages.js b/packages/lib/link/messages.js index 9329b232..7174b3e6 100644 --- a/packages/lib/link/messages.js +++ b/packages/lib/link/messages.js @@ -804,7 +804,7 @@ messages.debugWsMessage = new Event({ messages.instanceInitialized = new Event({ type: "instance_initialized", - links: ["instance-slave", "slave-master"], + links: ["instance-slave"], eventProperties: { "instance_id": { type: "integer" }, "plugins": { @@ -813,18 +813,14 @@ messages.instanceInitialized = new Event({ }, }, }); -messages.instanceStarted = new Event({ - type: "instance_started", - links: ["instance-slave", "slave-master"], - eventProperties: { - "instance_id": { type: "integer" }, - }, -}); -messages.instanceStopped = new Event({ - type: "instance_stopped", +messages.instanceStatusChanged = new Event({ + type: "instance_status_changed", links: ["instance-slave", "slave-master"], eventProperties: { "instance_id": { type: "integer" }, + "status": { enum: [ + "stopped", "starting", "running", "creating_save", "exporting_data", + ]}, }, }); @@ -840,7 +836,9 @@ messages.updateInstances = new Event({ required: ["serialized_config", "status"], properties: { "serialized_config": { type: "object" }, - "status": { enum: ["stopped", "initialized", "running"] }, + "status": { enum: [ + "stopped", "starting", "running", "creating_save", "exporting_data", + ]}, }, }, }, diff --git a/packages/lib/plugin.js b/packages/lib/plugin.js index 659a5ee0..d4a1a6f5 100644 --- a/packages/lib/plugin.js +++ b/packages/lib/plugin.js @@ -226,10 +226,23 @@ class BaseMasterPlugin { * * Invoked when the master server has received notice from a slave that * the running status of an instance has changed. The possible statuses - * are `stopped` meaning it is not loaded or running, `initialized` - * meaning the instance has initialized and is starting up the Factorio - * server, and `running` which means the Factorio server it manages is - * up and running. + * that can be notified about are: + * - `stopped`: Instance is no longer running. + * - `starting`: Instance is in the process of starting up. + * - `running`: Instance startup completed and is now running. + * - `creating_save`: Instance is in the process of creating a save. + * - `exporting_data`: Instance is in the process of exporting item + * icons and locale data. + * + * On master startup all known instances gets the status `unknown` if + * they are assigned to a slave, when the slave then connects to the + * master and informs of the current status of its instances this hook + * is invoked for all those instances. You can detect this situation by + * checking if prev equals `unknown`. + * + * Note that it's possible for status change notification to get lost in + * the case of network outages if reconnect fails to re-establish the + * session between the master server and the slave. * * @param {Object} instance - the instance that changed. * @param {string} prev - the previous status of the instance. diff --git a/packages/master/master.js b/packages/master/master.js index c8efaa2b..b1aa838f 100755 --- a/packages/master/master.js +++ b/packages/master/master.js @@ -273,7 +273,8 @@ async function loadInstances(databaseDirectory, file) { for (let serializedConfig of serialized) { let instanceConfig = new config.InstanceConfig(); await instanceConfig.load(serializedConfig); - instances.set(instanceConfig.get("instance.id"), { config: instanceConfig }); + let status = instanceConfig.get("instance.assigned_slave") === null ? "unassigned" : "unknown"; + instances.set(instanceConfig.get("instance.id"), { config: instanceConfig, status }); } } catch (err) { @@ -625,7 +626,7 @@ class ControlConnection extends BaseConnection { if (db.instances.has(instanceId)) { throw new errors.RequestError(`Instance with ID ${instanceId} already exists`); } - db.instances.set(instanceId, { config: instanceConfig }); + db.instances.set(instanceId, { config: instanceConfig, status: "unassigned" }); } async deleteInstanceRequestHandler(message, request) { @@ -948,27 +949,11 @@ class SlaveConnection extends BaseConnection { } } - async instanceInitializedEventHandler(message, event) { + async instanceStatusChangedEventHandler(message, event) { let instance = db.instances.get(message.data.instance_id); let prev = instance.status; - instance.status = "initialized"; - console.log(`Clusterio | Instance ${instance.config.get("instance.name")} Initialized`); - await plugin.invokeHook(masterPlugins, "onInstanceStatusChanged", instance, prev); - } - - async instanceStartedEventHandler(message, event) { - let instance = db.instances.get(message.data.instance_id); - let prev = instance.status; - instance.status = "running"; - console.log(`Clusterio | Instance ${instance.config.get("instance.name")} Started`); - await plugin.invokeHook(masterPlugins, "onInstanceStatusChanged", instance, prev); - } - - async instanceStoppedEventHandler(message, event) { - let instance = db.instances.get(message.data.instance_id); - let prev = instance.status; - instance.status = "stopped"; - console.log(`Clusterio | Instance ${instance.config.get("instance.name")} Stopped`); + instance.status = message.data.status; + console.log(`Clusterio | Instance ${instance.config.get("instance.name")} State: ${instance.status}`); await plugin.invokeHook(masterPlugins, "onInstanceStatusChanged", instance, prev); } @@ -994,15 +979,16 @@ class SlaveConnection extends BaseConnection { if (masterInstance.status !== instance.status) { let prev = masterInstance.status; masterInstance.status = instance.status; - if (prev !== undefined) { - await plugin.invokeHook(masterPlugins, "onInstanceStatusChanged", instance, prev); - } + await plugin.invokeHook(masterPlugins, "onInstanceStatusChanged", instance, prev); } continue; } instanceConfig.set("instance.assigned_slave", this._id); - db.instances.set(instanceConfig.get("instance.id"), { config: instanceConfig }); + db.instances.set(instanceConfig.get("instance.id"), { + config: instanceConfig, + status: instance.status, + }); await link.messages.assignInstance.send(this, { instance_id: instanceConfig.get("instance.id"), serialized_config: instanceConfig.serialize(), diff --git a/packages/slave/slave.js b/packages/slave/slave.js index bb5d609c..b2947b08 100755 --- a/packages/slave/slave.js +++ b/packages/slave/slave.js @@ -176,9 +176,15 @@ class Instance extends link.Link{ instanceFactorioAutosaveSize.labels(String(this.config.get("instance.id"))).set(stat.size); } + notifyStatus(status) { + link.messages.instanceStatusChanged.send(this, { + instance_id: this.config.get("instance.id"), status, + }); + } + notifyExit() { this._running = false; - link.messages.instanceStopped.send(this, { instance_id: this.config.get("instance.id") }); + this.notifyStatus("stopped"); this.config.off("fieldChanged", this._configFieldChanged); @@ -461,7 +467,7 @@ class Instance extends link.Link{ await plugin.invokeHook(this.plugins, "onStart"); this._running = true; - link.messages.instanceStarted.send(this, { instance_id: this.config.get("instance.id") }); + this.notifyStatus("running"); } /** @@ -483,7 +489,7 @@ class Instance extends link.Link{ await plugin.invokeHook(this.plugins, "onStart"); this._running = true; - link.messages.instanceStarted.send(this, { instance_id: this.config.get("instance.id") }); + this.notifyStatus("running"); } /** @@ -642,6 +648,7 @@ class Instance extends link.Link{ } async createSaveRequestHandler() { + this.notifyStatus("creating_save"); try { console.log("Clusterio | Writing server-settings.json"); await this.writeServerSettings(); @@ -660,6 +667,7 @@ class Instance extends link.Link{ } async exportDataRequestHandler() { + this.notifyStatus("exporting_data"); try { console.log("Clusterio | Writing server-settings.json"); await this.writeServerSettings(); @@ -813,19 +821,14 @@ class InstanceConnection extends link.Link { } async instanceInitializedEventHandler(message, event) { - this.status = "initialized"; this.plugins = new Map(Object.entries(message.data.plugins)); - this.forwardEventToMaster(message, event); } - async instanceStartedEventHandler(message, event) { - this.status = "running"; - this.forwardEventToMaster(message, event); - } - - async instanceStoppedEventHandler(message, event) { - this.status = "stopped"; - this.slave.instanceConnections.delete(this.instanceId); + async instanceStatusChangedEventHandler(message, event) { + this.status = message.data.status; + if (this.status === "stopped") { + this.slave.instanceConnections.delete(this.instanceId); + } this.forwardEventToMaster(message, event); } } @@ -1108,6 +1111,9 @@ class Slave extends link.Link { path.join(instanceInfo.path, "instance.json"), JSON.stringify(warnedOutput, null, 4) ); + + // Notify master the instance is no longer usassigned + link.messages.instanceStatusChanged.send(this, { instance_id, status: "stopped" }); } /** From 003b035abdfc71514967fd1dd3d876ccb22696a3 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Tue, 29 Sep 2020 01:18:17 +0200 Subject: [PATCH 2/2] Send instance status in listInstances request Send the status of the instance in the repsonse to the listInstances request and use this status in the integration tests to check if the instance was started/stopped. --- packages/lib/link/messages.js | 5 ++++- packages/master/master.js | 1 + test/integration/clusterio.js | 28 +++++++++++++++++++++++----- test/lib/command.js | 2 +- 4 files changed, 29 insertions(+), 7 deletions(-) diff --git a/packages/lib/link/messages.js b/packages/lib/link/messages.js index 7174b3e6..b7951c90 100644 --- a/packages/lib/link/messages.js +++ b/packages/lib/link/messages.js @@ -290,11 +290,14 @@ messages.listInstances = new Request({ type: "array", items: { additionalProperties: false, - required: ["name", "id", "assigned_slave"], + required: ["name", "id", "assigned_slave", "status"], properties: { "name": { type: "string" }, "id": { type: "integer" }, "assigned_slave": { type: ["null", "integer"] }, + "status": { enum: [ + "unknown", "unassigned", "stopped", "starting", "running", "creating_save", "exporting_data", + ]}, }, }, }, diff --git a/packages/master/master.js b/packages/master/master.js index b1aa838f..e3832fba 100755 --- a/packages/master/master.js +++ b/packages/master/master.js @@ -612,6 +612,7 @@ class ControlConnection extends BaseConnection { id: instance.config.get("instance.id"), name: instance.config.get("instance.name"), assigned_slave: instance.config.get("instance.assigned_slave"), + status: instance.status, }); } return { list }; diff --git a/test/integration/clusterio.js b/test/integration/clusterio.js index 584164c0..3ec94b1e 100644 --- a/test/integration/clusterio.js +++ b/test/integration/clusterio.js @@ -8,6 +8,16 @@ const link = require("@clusterio/lib/link"); const { slowTest, get, execCtl, sendRcon, getControl, instancesDir } = require("./index"); +async function getInstances() { + let result = await link.messages.listInstances.send(getControl()); + return new Map(result.list.map(instance => [instance.id, instance])); +} + +async function checkInstanceStatus(id, status) { + let instances = await getInstances(); + assert.equal(instances.get(44).status, status, "incorrect instance status"); +} + describe("Integration of Clusterio", function() { describe("clusterioctl", function() { describe("slave list", function() { @@ -22,15 +32,19 @@ describe("Integration of Clusterio", function() { }); describe("instance create", function() { - it("runs", async function() { + it("creates the instance", async function() { await execCtl("instance create test --id 44"); + let instances = await getInstances(); + assert(instances.has(44), "instance was not created"); + assert.equal(instances.get(44).status, "unassigned", "incorrect instance status"); }); }); describe("instance assign", function() { it("creates the instance files", async function() { await execCtl("instance assign test 4"); - assert(await fs.exists(path.join(instancesDir, "test")), "Instance was not created"); + assert(await fs.exists(path.join(instancesDir, "test")), "Instance directory was not created"); + await checkInstanceStatus(44, "stopped"); }); }); @@ -38,6 +52,7 @@ describe("Integration of Clusterio", function() { it("creates a save", async function() { slowTest(this); await execCtl("instance create-save test"); + await checkInstanceStatus(44, "stopped"); }); }); @@ -46,6 +61,7 @@ describe("Integration of Clusterio", function() { slowTest(this); await execCtl("instance export-data test"); assert(await fs.exists(path.join("static", "export", "locale.json")), "Export was not created"); + await checkInstanceStatus(44, "stopped"); }); }); @@ -53,7 +69,7 @@ describe("Integration of Clusterio", function() { it("starts the instance", async function() { slowTest(this); await execCtl("instance start test"); - // TODO check that the instance actually started + await checkInstanceStatus(44, "running"); }); }); @@ -200,7 +216,7 @@ describe("Integration of Clusterio", function() { it("stops the instance", async function() { slowTest(this); await execCtl("instance stop test"); - // TODO check that the instance actually stopped + await checkInstanceStatus(44, "stopped"); }); }); @@ -208,7 +224,9 @@ describe("Integration of Clusterio", function() { it("deletes the instance", async function() { slowTest(this); await execCtl("instance delete test"); - assert(!await fs.exists(path.join(instancesDir, "test")), "Instance was not deleted"); + assert(!await fs.exists(path.join(instancesDir, "test")), "Instance files was not deleted"); + let instances = await getInstances(); + assert(!instances.has(44), "instance was not deleted from master"); }); }); diff --git a/test/lib/command.js b/test/lib/command.js index f09db22e..ceaa3a61 100644 --- a/test/lib/command.js +++ b/test/lib/command.js @@ -24,7 +24,7 @@ describe("lib/command", function() { seq: 1, type: "list_instances_response", data: { seq: message.seq, - list: [{ id: 57, assigned_slave: 4, name: "Test Instance" }], + list: [{ id: 57, assigned_slave: 4, name: "Test Instance", status: "stopped" }], }, }); } else if (message.type === "list_roles_request") {