Merge pull request #815 from Cooldude2606/feature/database-lock

Add extra startup warnings to CLI: User count and Lock file

test/integration/clusterio.js

@@ -13,8 +13,8 @@ const { wait } = lib;
 const testStrings = require("../lib/factorio/test_strings");
 const {
 	TestControl, TestControlConnector, url, controlToken, slowTest,
-	exec, execCtl, execCtlProcess, execHost, sendRcon, getControl,
-	spawnNode, instancesDir, factorioDir,
+	execCtl, execCtlProcess, execController, execHost, sendRcon, getControl,
+	spawnNode, instancesDir, factorioDir, databaseDir, controllerConfigPath,
 } = require("./index");
 
 
@@ -109,16 +109,59 @@ describe("Integration of Clusterio", function() {
 	describe("clusteriocontroller", function() {
 		describe("bootstrap generate-user-token", function() {
 			it("work for existing user", async function() {
-				await exec("node ../../packages/controller bootstrap generate-user-token test");
+				await execController("bootstrap generate-user-token test");
 			});
 
 			it("fails if user does not exist", async function() {
 				await assert.rejects(
-					exec("node ../../packages/controller bootstrap generate-user-token invalid")
+					execController("bootstrap generate-user-token invalid")
 				);
 			});
 		});
 
+		describe("bootstrap create-admin", function() {
+			it("should refuse to modify the database files when locked", async function() {
+				await assert.rejects(
+					execController("bootstrap create-admin BootstrapAdminTest")
+				);
+			});
+			it("should modify the database files when locked if bypass option is given", async function() {
+				const lockFilePath = `${controllerConfigPath}.lock`;
+				await fs.copyFile(lockFilePath, "temp.lock");
+				try {
+					await execController("bootstrap create-admin BootstrapAdminTest2 --bypass-lock-file");
+				} finally {
+					await fs.move("temp.lock", lockFilePath); // Replace the lockfile after it is deleted
+				}
+
+				const json = await fs.readJSON(path.join(databaseDir, "users.json"));
+				assert.equal(Object.values(json).some(user => user.name === "BootstrapAdminTest2"), true);
+			});
+		});
+
+		describe("config", function() {
+			it("can read the config file", async function() {
+				await execController("config list");
+			});
+			it("should refuse to modify the config file while locked", async function() {
+				await assert.rejects(
+					execController("config set controller.name ConfigEditTest")
+				);
+			});
+			it("should modify the config file while locked if bypass option is given", async function() {
+				const lockFilePath = `${controllerConfigPath}.lock`;
+				await fs.copyFile(lockFilePath, "temp.lock");
+				try {
+					await execController("config set controller.name ConfigEditTest2 --bypass-lock-file");
+				} finally {
+					await fs.move("temp.lock", lockFilePath); // Replace the lockfile after it is deleted
+				}
+
+				const json = await fs.readJSON(path.join(controllerConfigPath));
+				assert.equal(json["controller.name"], "ConfigEditTest2");
+			});
+		});
+
 		describe("run", function() {
 			it("should handle resume of an active connection", async function() {
 				slowTest(this);
@@ -143,6 +186,38 @@ describe("Integration of Clusterio", function() {
 				await events.once(connectorB, "resume");
 				await connectorB.close(1000, "");
 			});
+			it("should refuse to start a second process while locked", async function() {
+				await assert.rejects(execController("run"));
+			});
+			it("should start a second process while locked if bypass option is given", async function() {
+				slowTest(this);
+				const lockFilePath = `${controllerConfigPath}.lock`;
+				await fs.copyFile(lockFilePath, "temp.lock");
+				try {
+					await execController("run --bypass-lock-file");
+				} catch (err) {
+					assert.equal(/Error: Server listening failed/.test(err.stderr), true);
+				}
+				await fs.move("temp.lock", lockFilePath); // Replace the lockfile after it is deleted
+			});
+			it("should refuse to start when no users are loaded", async function() {
+				const dir = path.join("temp", "test", "empty_controller");
+				await fs.emptyDir(dir);
+				await assert.rejects(execController("run", { cwd: dir }));
+			});
+			it("should start when no users are loaded if bypass option given", async function() {
+				slowTest(this);
+				const dir = path.join("temp", "test", "empty_controller");
+				await fs.emptyDir(dir);
+				const child = await spawnNode(
+					"altController",
+					"../../../packages/controller run --no-check-user-count",
+					/Started controller/,
+					{ cwd: dir },
+				);
+				assert.ok(child.kill());
+				assert.equal(child.killed, true);
+			});
 		});
 
 		describe("queryLogRequestHandler", function() {
@@ -245,6 +320,67 @@ describe("Integration of Clusterio", function() {
 				await stopAltHost(hostProcess);
 			}
 		});
+		describe("config", function() {
+			it("can read the config file", async function() {
+				await execHost("config list");
+			});
+			it("should refuse to modify the config file while locked", async function() {
+				slowTest(this);
+				let hostProcess; // Alt host used since the process could get killed
+				const config = "alt-host-config.json";
+				try {
+					hostProcess = await startAltHost();
+					await assert.rejects(
+						execHost(`--config ${config} config set host.name ConfigEditTest`)
+					);
+				} finally {
+					await stopAltHost(hostProcess);
+				}
+			});
+			it("should modify the config file while locked if bypass option is given", async function() {
+				slowTest(this);
+				let hostProcess; // Alt host used since the process could get killed
+				const config = "alt-host-config.json";
+				try {
+					hostProcess = await startAltHost();
+					await execHost(`--config ${config} config set host.name ConfigEditTest2 --bypass-lock-file`);
+				} finally {
+					await stopAltHost(hostProcess);
+				}
+
+				const json = await fs.readJSON(path.join("temp", "test", config));
+				assert.equal(json["host.name"], "ConfigEditTest2");
+			});
+		});
+		describe("run", function() {
+			it("should refuse to start a second process while locked", async function() {
+				slowTest(this);
+				let hostProcess; // Alt host used since the process could get killed
+				const config = "alt-host-config.json";
+				try {
+					hostProcess = await startAltHost();
+					await assert.rejects(
+						execHost(`--config ${config} run`)
+					);
+				} finally {
+					await stopAltHost(hostProcess);
+				}
+			});
+			it("should start a second process while locked if bypass option is given", async function() {
+				slowTest(this);
+				let hostProcessA, hostProcessB; // Alt host used since the process could get killed
+				const config = "alt-host-config.json";
+				try {
+					hostProcessA = await startAltHost();
+					hostProcessB = await spawnNode(
+						"alt-host-b:", `../../packages/host run --config ${config} --bypass-lock-file`, /Started host/
+					);
+				} finally {
+					await stopAltHost(hostProcessA);
+					await stopAltHost(hostProcessB);
+				}
+			});
+		});
 	});
 
 	describe("clusterioctl", function() {

test/integration/index.js

@@ -173,26 +173,26 @@ let controllerConfigPath = path.join("temp", "test", "config-controller.json");
 let hostConfigPath = path.join("temp", "test", "config-host.json");
 let controlConfigPath = path.join("temp", "test", "config-control.json");
 
-async function exec(command, options = {}) {
+function exec(command, options = {}) {
 	// Uncomment to show commands run in tests
 	// console.log(command);
 	options = { cwd: path.join("temp", "test"), ...options };
-	return await util.promisify(child_process.exec)(command, options);
+	return util.promisify(child_process.exec)(command, options);
 }
 
-async function execController(...args) {
+function execController(...args) {
 	args[0] = `node --enable-source-maps ../../packages/controller ${args[0]}`;
-	return await exec(...args);
+	return exec(...args);
 }
 
-async function execHost(...args) {
+function execHost(...args) {
 	args[0] = `node --enable-source-maps ../../packages/host ${args[0]}`;
-	return await exec(...args);
+	return exec(...args);
 }
 
-async function execCtlProcess(...args) {
+function execCtlProcess(...args) {
 	args[0] = `node --enable-source-maps ../../packages/ctl ${args[0]}`;
-	return await exec(...args);
+	return exec(...args);
 }
 
 let inExecCtl = false;
@@ -229,7 +229,8 @@ function getControl() {
 	return control;
 }
 
-function spawn(name, cmd, waitFor) {
+/** @returns {Promise<child_process.ChildProcess>} */
+function spawn(name, cmd, waitFor, options = {}) {
 
 	const silent = process.env.SILENT_TEST;
 	const bootstrap = !controllerProcess || !hostProcess;
@@ -242,7 +243,7 @@ function spawn(name, cmd, waitFor) {
 	return new Promise((resolve, reject) => {
 		log(cmd);
 		let parts = cmd.split(" ");
-		let process = child_process.spawn(parts[0], parts.slice(1), { cwd: path.join("temp", "test") });
+		let process = child_process.spawn(parts[0], parts.slice(1), { cwd: path.join("temp", "test"), ...options });
 		let stdout = new LineSplitter({ readableObjectMode: true });
 		let stderr = new LineSplitter({ readableObjectMode: true });
 		let onDataOut = line => {
@@ -266,8 +267,8 @@ function spawn(name, cmd, waitFor) {
 	});
 }
 
-async function spawnNode(name, cmd, waitFor) {
-	return await spawn(name, `node --enable-source-maps ${cmd}`, waitFor);
+function spawnNode(name, cmd, waitFor, options) {
+	return spawn(name, `node --enable-source-maps ${cmd}`, waitFor, options);
 }
 
 before(async function() {
@@ -394,6 +395,7 @@ module.exports = {
 	TestControl,
 	TestControlConnector,
 	TestHostConnector,
+	execController,
 	execCtlProcess,
 	slowTest,
 	get,

test/lib/LockFile.test.js

@@ -0,0 +1,133 @@
+/* eslint-disable node/no-sync */
+"use strict";
+const assert = require("assert").strict;
+const fs = require("fs-extra");
+const path = require("path");
+
+const { LockFile } = require("@clusterio/lib");
+
+const baseDir = path.join("temp", "test", "lock_file");
+
+describe("class LockFile", function () {
+	/** @type {string} */
+	let filePath;
+	/** @type {LockFile} */
+	let lock;
+
+	before(async function () {
+		await fs.ensureDir(baseDir);
+	});
+
+	beforeEach(function() {
+		filePath = path.join(baseDir, `${Date.now()}-${Math.random()}.lock`);
+		lock = new LockFile(filePath);
+	});
+
+	afterEach(async function () {
+		await fs.unlink(filePath).catch(() => {});
+	});
+
+	describe("acquire()", function () {
+		it("should create the lock file when acquiring", async function () {
+			await lock.acquire();
+			assert.ok(fs.existsSync(filePath));
+		});
+
+		it("should throw LockFileBusyError if called when not idle", async function () {
+			await lock.acquire();
+			await assert.rejects(lock.acquire(), { name: "LockFileBusyError" });
+		});
+
+		it("should remove stale lock file and acquire it", async function () {
+			await fs.writeFile(filePath, "999999\n"); // stale lock
+			await lock.acquire();
+			assert.ok(fs.existsSync(filePath));
+		});
+
+		it("should throw if existing lock file is not stale", async function () {
+			await fs.writeFile(filePath, `${process.pid}\n`);
+			await assert.rejects(lock.acquire(), { name: "LockFileExistsError" });
+		});
+	});
+
+	describe("release()", function () {
+		it("should remove the lock file when releasing", async function () {
+			await lock.acquire();
+			await lock.release();
+			assert.ok(!fs.existsSync(filePath));
+		});
+
+		it("should throw LockFileBusyError if called when not acquired", async function () {
+			await assert.rejects(lock.release(), { name: "LockFileBusyError" });
+		});
+	});
+
+	describe("isHeld()", function () {
+		it("should return true only when lock file contains current PID", async function () {
+			await lock.acquire();
+			const held = await lock.isHeld();
+			assert.equal(held, true);
+		});
+
+		it("should return false when lock file does not exist", async function () {
+			const held = await lock.isHeld();
+			assert.equal(held, false);
+		});
+
+		it("should return false if lock file contains different PID", async function () {
+			await fs.writeFile(filePath, "999999\n"); // simulate another process
+			const held = await lock.isHeld();
+			assert.equal(held, false);
+		});
+
+		it("should return false if lock file was deleted while held", async function() {
+			await lock.acquire();
+			await fs.unlink(filePath);
+			const held = await lock.isHeld();
+			assert.equal(held, false);
+		});
+	});
+
+	describe("isStale()", function () {
+		it("should return true if lock file does not exist", async function () {
+			const stale = await lock.isStale();
+			assert.equal(stale, true);
+		});
+
+		it("should return true for invalid PID in lock file", async function () {
+			await fs.writeFile(filePath, "abc\n"); // non-numeric PID
+			const stale = await lock.isStale();
+			assert.equal(stale, true);
+		});
+
+		it("should return true for non-existent process PID", async function () {
+			await fs.writeFile(filePath, "999999\n"); // assume PID does not exist
+			const stale = await lock.isStale();
+			assert.equal(stale, true);
+		});
+
+		it("should return false for current process PID", async function () {
+			await fs.writeFile(filePath, `${process.pid}\n`);
+			const stale = await lock.isStale();
+			assert.equal(stale, false);
+		});
+	});
+
+	describe("attach/detach exit handlers", function () {
+		it("should attach exit handler on acquire and detach on release", async function () {
+			await lock.acquire();
+			const listenersBefore = process.listeners("exit").slice();
+			assert.ok(listenersBefore.includes(lock.handleExitSync));
+
+			await lock.release();
+			const listenersAfter = process.listeners("exit").slice();
+			assert.ok(!listenersAfter.includes(lock.handleExitSync));
+		});
+	});
+
+	describe("handleExitSync()", function () {
+		it("should not throw if file already removed", function () {
+			assert.doesNotThrow(() => lock.handleExitSync()); // simulate non-existent file
+		});
+	});
+});