diff --git a/.github/scripts/test-edge-ops.py b/.github/scripts/test-edge-ops.py index 42c6a33c8fe5e..29204804e53b3 100644 --- a/.github/scripts/test-edge-ops.py +++ b/.github/scripts/test-edge-ops.py @@ -24,8 +24,11 @@ import os from pathlib import Path import shutil +import signal import subprocess +import sys import tempfile +import time import unittest import xml.etree.ElementTree as ET @@ -411,6 +414,234 @@ def test_windows_force_stop_returns_without_pausing(self): self.assertNotIn("Press any key", result.stdout) +@unittest.skipIf(WINDOWS, "Unix stop-edge.sh") +class EdgeStopTest(unittest.TestCase): + """Run the real stop-edge.sh against stand-in Edge processes that only sleep. + + start-edge.sh needs a JVM, so each stand-in carries the -DIOTDB_HOME that + start-edge.sh would pass to it: the value initEdgeHome derives for it. + """ + + def setUp(self): + temporary = tempfile.TemporaryDirectory(prefix="iotdb-edge-stop-") + self.addCleanup(temporary.cleanup) + self.root = Path(temporary.name).resolve() + # Identifies this test's stand-ins, so that cleanup never touches others. + self.marker = "-Dedge.stop.test=" + self.root.as_posix() + self.env = os.environ.copy() + self.env.pop("IOTDB_HOME", None) + self.env.pop("CDPATH", None) + + def install(self, relative): + home = self.root / relative + for script in ( + "sbin/start-edge.sh", + "sbin/stop-edge.sh", + "conf/iotdb-common.sh", + ): + (home / script).parent.mkdir(parents=True, exist_ok=True) + shutil.copyfile(REPOSITORY / "scripts" / script, home / script) + return home + + def link(self, relative, target): + path = self.root / relative + if path.is_symlink(): + path.unlink() + path.symlink_to(target, target_is_directory=True) + + def symlink_layout(self): + # x/link/../edge is y/edge physically, while x/edge is another installation. + (self.root / "y/sub").mkdir(parents=True) + self.install("y/edge") + self.install("x/edge") + self.link("x/link", "../y/sub") + + def bash(self, arguments, iotdb_home=None, cwd=None, **variables): + env = dict(self.env, **variables) + if iotdb_home is not None: + env["IOTDB_HOME"] = iotdb_home + return subprocess.run( + ["bash", *arguments], + env=env, + cwd=cwd or self.root, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + errors="replace", + timeout=60, + ) + + def started_home(self, script, iotdb_home=None, **variables): + """The -DIOTDB_HOME that start-edge.sh at this path passes to the JVM.""" + result = self.bash( + ( + "-c", + 'source "$(dirname "$1")/../conf/iotdb-common.sh"' + ' && initEdgeHome "$1" && printf "%s" "$IOTDB_HOME"', + "bash", + script, + ), + iotdb_home, + **variables, + ) + self.assertEqual(result.returncode, 0, result.stderr) + return result.stdout + + def stop(self, script, iotdb_home=None, cwd=None, **variables): + result = self.bash((script,), iotdb_home, cwd, **variables) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + return result + + def spawn(self, command, cwd=None): + # Detached through a short-lived parent, like the nohup'ed JVM, so that + # the process is reaped once it exits: a zombie child of this test would + # still look alive to stop-edge.sh. + launcher = subprocess.run( + [ + sys.executable, + "-c", + "import subprocess, sys; print(subprocess.Popen(sys.argv[1:]," + " stdin=subprocess.DEVNULL, stdout=subprocess.DEVNULL," + " stderr=subprocess.DEVNULL).pid)", + *command, + ], + cwd=cwd or self.root, + stdout=subprocess.PIPE, + text=True, + check=True, + timeout=30, + ) + pid = int(launcher.stdout) + self.addCleanup(self.kill, pid) + return pid + + def start(self, home, cwd=None): + """Start a stand-in for the JVM and write edge.pid where start-edge.sh does.""" + cwd = cwd or self.root + pid = self.spawn( + [ + sys.executable, + "-c", + "import time; time.sleep(600)", + self.marker, + "-DIOTDB_HOME=" + home, + "-DCONFIGNODE_HOME=" + home, + "org.apache.iotdb.edge.EdgeNode", + "-s", + ], + cwd, + ) + (Path(cwd) / home / "edge.pid").write_text("{}\n".format(pid)) + return pid + + def command(self, pid): + return subprocess.run( + ["ps", "-ww", "-p", str(pid), "-o", "command="], + stdout=subprocess.PIPE, + text=True, + errors="replace", + ).stdout + + def running(self, pid): + return self.marker in self.command(pid) + + def kill(self, pid): + if self.running(pid): + os.kill(pid, signal.SIGKILL) + + def test_symlinked_home_is_stopped_through_the_real_path(self): + # apache/iotdb#18545 + self.install("edge") + self.link("link", "edge") + home = self.started_home("edge/sbin/start-edge.sh", str(self.root / "link")) + pid = self.start(home) + self.stop("edge/sbin/stop-edge.sh") + self.assertFalse(self.running(pid)) + + def test_stop_after_the_symlink_is_re_pointed(self): + self.install("edge-v1") + self.install("edge-v2") + for iotdb_home in (None, str(self.root / "iotdb")): + with self.subTest(IOTDB_HOME=iotdb_home): + self.link("iotdb", "edge-v1") + pid = self.start( + self.started_home("iotdb/sbin/start-edge.sh", iotdb_home) + ) + self.link("iotdb", "edge-v2") + self.stop("iotdb/sbin/stop-edge.sh", iotdb_home) + self.assertFalse(self.running(pid)) + + def test_relative_home_of_another_installation_is_left_alone(self): + # Recorded as-is by the start-edge.sh of earlier releases. + pid = self.start(".", cwd=self.install("edge-a")) + self.install("edge-b") + for iotdb_home in (None, "."): + with self.subTest(IOTDB_HOME=iotdb_home): + self.stop("sbin/stop-edge.sh", iotdb_home, cwd=self.root / "edge-b") + self.assertTrue(self.running(pid)) + + def test_symlink_followed_by_dot_dot_in_the_home(self): + self.symlink_layout() + given = str(self.root / "x/link/../edge") + home = self.started_home("y/edge/sbin/start-edge.sh", given) + self.assertEqual(home, given) + pid = self.start(home) + self.stop("x/edge/sbin/stop-edge.sh") + self.assertTrue(self.running(pid)) + self.stop("y/edge/sbin/stop-edge.sh") + self.assertFalse(self.running(pid)) + + def test_symlink_followed_by_dot_dot_in_the_script_path(self): + self.symlink_layout() + pid = self.start(self.started_home("x/link/../edge/sbin/start-edge.sh")) + self.stop("x/link/../edge/sbin/stop-edge.sh") + self.assertFalse(self.running(pid)) + + def test_process_without_iotdb_home_is_not_matched(self): + home = self.install("edge") + # Everything before the first " -D" names the installation, and edge.pid + # points at the process. + pid = self.spawn( + [ + "perl", + "-e", + "$0 = shift; sleep 600", + "{} -Dfoo=1 {} org.apache.iotdb.edge.EdgeNode -s".format( + home, self.marker + ), + ] + ) + for _ in range(100): + if self.command(pid).startswith(str(home)): + break + time.sleep(0.05) + (home / "edge.pid").write_text("{}\n".format(pid)) + self.stop("edge/sbin/stop-edge.sh") + self.assertTrue(self.running(pid)) + + def test_cdpath_is_not_used(self): + self.install("edge") + (self.root / "elsewhere/edge/sbin").mkdir(parents=True) + cdpath = {"CDPATH": str(self.root / "elsewhere")} + home = self.started_home("edge/sbin/start-edge.sh", **cdpath) + self.assertEqual(home, str(self.root / "edge")) + pid = self.start(home) + self.stop("edge/sbin/stop-edge.sh", **cdpath) + self.assertFalse(self.running(pid)) + + def test_home_of_another_installation_is_reported(self): + self.install("edge-a") + own = self.install("edge-b") + result = self.stop("edge-b/sbin/stop-edge.sh", str(self.root / "edge-a")) + self.assertIn("belongs to {}".format(own), result.stderr) + + def test_missing_home_is_an_error(self): + self.install("edge") + result = self.bash(("edge/sbin/stop-edge.sh",), str(self.root / "missing")) + self.assertNotEqual(result.returncode, 0, result.stdout) + self.assertIn("not an accessible directory", result.stderr) + + class AssemblyTest(unittest.TestCase): @staticmethod def packaged(descriptor, path): diff --git a/AGENTS.md b/AGENTS.md index cb1d54cae571a..220b5676cb6b9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -83,7 +83,7 @@ Keep IT-specific rules and commands in that module-level file. - `EdgeNode` (`iotdb-core/confignode/src/main/java/org/apache/iotdb/edge/EdgeNode.java`) starts the ConfigNode and DataNode in one JVM for resource-constrained single-node deployments. - `distribution/src/assembly/edge.xml` produces the additional `apache-iotdb--edge-bin.zip` artifact. Keep its `edge-bin` assembly id unique so it cannot replace an existing attached artifact. - Edge-specific system defaults live under `iotdb-core/node-commons/src/assembly/resources/conf/edge/`. Standard node, all-in-one, and integration-test assemblies must continue to exclude `edge/**`. -- Edge launchers must reuse the configuration-aware port checks in `scripts/conf/iotdb-common.sh`. Stop scripts must match both the `EdgeNode` main class and the exact `IOTDB_HOME`; never fall back to a global process-name kill. +- Edge launchers must reuse the configuration-aware port checks in `scripts/conf/iotdb-common.sh`. Start and stop scripts must derive `IOTDB_HOME` with the same shared function (`initEdgeHome`). Stop scripts must match both the `EdgeNode` main class and the `IOTDB_HOME` recorded at start, either exactly or, for an absolute recorded path, after resolving symlinks on both sides; never fall back to a global process-name kill. - Package node-independent tools that work with the combined process. Exclude scripts that require standalone node launchers or environment files, including destructive, daemon-management, and health-check scripts. - After packaging changes, run a clean distribution build, check that every existing distribution artifact is still produced, and inspect the Edge zip contents and startup/shutdown behavior. diff --git a/integration-test/src/test/java/org/apache/iotdb/edge/it/IoTDBEdgeBasicIT.java b/integration-test/src/test/java/org/apache/iotdb/edge/it/IoTDBEdgeBasicIT.java index 07ab860655d6d..b47742c8c7548 100644 --- a/integration-test/src/test/java/org/apache/iotdb/edge/it/IoTDBEdgeBasicIT.java +++ b/integration-test/src/test/java/org/apache/iotdb/edge/it/IoTDBEdgeBasicIT.java @@ -133,6 +133,7 @@ public static void tearDown() throws Exception { if (edgeHome != null && Files.isRegularFile(edgeHome.resolve("sbin/stop-edge.sh"))) { try { runScript(edgeHome.resolve("sbin/stop-edge.sh"), STOP_SCRIPT_LOG); + assertEdgeProcessExited(); } catch (Exception | AssertionError e) { stopFailure = new AssertionError("Failed to stop IoTDB Edge with stop-edge.sh", e); } @@ -541,6 +542,21 @@ private static void runScript(final Path script, final Path outputFile) throws E } } + private static void assertEdgeProcessExited() throws InterruptedException { + // stop-edge.sh also exits 0 when it finds no matching process, so check the process itself. + // The short wait covers its "kill -9" fallback, which does not wait for the exit. + if (edgePid <= 0) { + return; + } + final ProcessHandle process = ProcessHandle.of(edgePid).orElse(null); + for (int i = 0; i < 10 && process != null && process.isAlive(); i++) { + Thread.sleep(500); + } + assertFalse( + "IoTDB Edge process " + edgePid + " is still running after stop-edge.sh returned", + process != null && process.isAlive()); + } + private static void stopProcessForciblyIfNeeded() throws InterruptedException { if (edgePid <= 0) { return; diff --git a/scripts/conf/iotdb-common.sh b/scripts/conf/iotdb-common.sh index 6a6b589e5235e..1ace4c62dbb4e 100755 --- a/scripts/conf/iotdb-common.sh +++ b/scripts/conf/iotdb-common.sh @@ -508,3 +508,42 @@ calculate_memory_sizes() ON_HEAP_MEMORY="${on_heap_memory_size_in_mb}M" OFF_HEAP_MEMORY="${off_heap_memory_size_in_mb}M" } + +# Prints the physical absolute path of directory $1, or nothing if it cannot be +# entered. "cd -P" is required: a plain "cd" collapses ".." logically, which +# would resolve "/../x" against the symlink's parent instead of its +# target. CDPATH is cleared so that a relative name is not looked up in it. +resolvePhysicalDir() +{ + [ -n "$1" ] || return 1 + (CDPATH= cd -P -- "$1" 2>/dev/null && pwd -P) +} + +# Sets IOTDB_HOME for start-edge.sh and stop-edge.sh, which both call this with +# their own "$0". start-edge.sh hands the value to the JVM as -DIOTDB_HOME and +# stop-edge.sh finds the process by it, so the two must derive it the same way. +# The value is only made absolute; symlinks are deliberately not resolved, so +# that an installation started through a symlink keeps that identity after the +# symlink is re-pointed to another version. +initEdgeHome() +{ + if [ -z "${IOTDB_HOME}" ]; then + IOTDB_HOME="$(CDPATH= cd -- "$(dirname "$1")"/.. && pwd)" || return 1 + fi + case "${IOTDB_HOME}" in + /*) ;; + *) IOTDB_HOME="$(pwd)/${IOTDB_HOME}" ;; + esac + local physical_home + local script_home + physical_home="$(resolvePhysicalDir "${IOTDB_HOME}")" + if [ -z "${physical_home}" ]; then + echo "IOTDB_HOME ${IOTDB_HOME} is not an accessible directory." >&2 + return 1 + fi + script_home="$(resolvePhysicalDir "$(dirname "$1")/..")" + if [ "${physical_home}" != "${script_home}" ]; then + echo "Warning: IOTDB_HOME is ${IOTDB_HOME}, but $(basename "$1") belongs to ${script_home}." >&2 + fi + export IOTDB_HOME +} diff --git a/scripts/sbin/start-edge.sh b/scripts/sbin/start-edge.sh index be1a70ea6f795..20e0cdebbbf87 100644 --- a/scripts/sbin/start-edge.sh +++ b/scripts/sbin/start-edge.sh @@ -20,9 +20,9 @@ # Start IoTDB Edge: ConfigNode + DataNode in one JVM process. -if [ -z "${IOTDB_HOME}" ]; then - export IOTDB_HOME="$(cd "$(dirname "$0")"/.. && pwd)" -fi +source "$(dirname "$0")/../conf/iotdb-common.sh" +# stop-edge.sh derives IOTDB_HOME with the same function and matches on it. +initEdgeHome "$0" || exit 1 if [ -z "${IOTDB_CONF}" ]; then export IOTDB_CONF=${IOTDB_HOME}/conf fi @@ -30,7 +30,6 @@ export IOTDB_DATA_HOME=${IOTDB_DATA_HOME:-${IOTDB_HOME}} export IOTDB_LOG_DIR=${IOTDB_LOG_DIR:-${IOTDB_HOME}/logs} mkdir -p "${IOTDB_LOG_DIR}" -source "$(dirname "$0")/../conf/iotdb-common.sh" export CONFIGNODE_HOME=${IOTDB_HOME} export CONFIGNODE_DATA_HOME=${IOTDB_DATA_HOME} export CONFIGNODE_CONF=${IOTDB_CONF} diff --git a/scripts/sbin/stop-edge.sh b/scripts/sbin/stop-edge.sh index 8f8f98ac64b14..6359364a18031 100644 --- a/scripts/sbin/stop-edge.sh +++ b/scripts/sbin/stop-edge.sh @@ -20,7 +20,13 @@ # Stop IoTDB Edge (the merged ConfigNode + DataNode process). -IOTDB_HOME="$(cd "$(dirname "$0")"/.. && pwd)" +source "$(dirname "$0")/../conf/iotdb-common.sh" +# Same derivation as start-edge.sh, which records the result as -DIOTDB_HOME. +initEdgeHome "$0" || exit 1 + +# The physical path, so that the same installation reached through a different +# path still compares equal in the fallback below. +IOTDB_HOME_RESOLVED="$(resolvePhysicalDir "${IOTDB_HOME}")" PID_FILE="${IOTDB_HOME}/edge.pid" @@ -30,10 +36,28 @@ is_same_edge_home() { *"-DIOTDB_HOME=${IOTDB_HOME} "*|*"-DIOTDB_HOME=${IOTDB_HOME}") return 0 ;; + *-DIOTDB_HOME=*) + ;; *) return 1 ;; esac + # Fall back to comparing resolved paths, so that a process started with + # IOTDB_HOME pointing at a symlink is still recognised when stopped through + # the real path, and the other way round. The value is delimited by the next + # " -D", which start-edge.sh always emits after -DIOTDB_HOME. If a hand-built + # command line ends with -DIOTDB_HOME, the extraction keeps the trailing + # arguments, the resolution below fails and the process is not matched. + local home="${command_line#*-DIOTDB_HOME=}" + home="${home%% -D*}" + # Only absolute values can be resolved from here. A relative one such as "." + # is meaningful in the started process's working directory, not in ours, so + # resolving it here could match an unrelated installation. + case "$home" in + /*) ;; + *) return 1 ;; + esac + [ "$(resolvePhysicalDir "$home")" = "${IOTDB_HOME_RESOLVED}" ] } is_edge_process() {