From 0dc5acf2f37858c4210ab9508881faca9cdd8360 Mon Sep 17 00:00:00 2001 From: Maksim Davydov Date: Wed, 12 Aug 2026 15:41:29 +0300 Subject: [PATCH 01/15] IGNITE-28976 [ducktests] Add optional demo breakpoints to pause a running scenario for inspection --- .gitignore | 1 + modules/ducktests/README.md | 40 ++ .../tests/checks/utils/check_pause.py | 209 +++++++++ .../ducktests/tests/docker/demo_console.py | 184 ++++++++ .../ignitetest/services/mdc/mdc_cluster.py | 29 ++ .../services/network_group/manager.py | 30 +- .../tests/mdc/partition_resilience_test.py | 10 + .../tests/ignitetest/utils/ignite_test.py | 23 + .../ducktests/tests/ignitetest/utils/pause.py | 427 ++++++++++++++++++ 9 files changed, 947 insertions(+), 6 deletions(-) create mode 100644 modules/ducktests/tests/checks/utils/check_pause.py create mode 100644 modules/ducktests/tests/docker/demo_console.py create mode 100644 modules/ducktests/tests/ignitetest/utils/pause.py diff --git a/.gitignore b/.gitignore index 1e684823db175..8ab90f323cfa8 100644 --- a/.gitignore +++ b/.gitignore @@ -90,6 +90,7 @@ CMakeSettings.json #Ducktape /results +/.ducktests-demo .ducktape *.pyc /tests/venv diff --git a/modules/ducktests/README.md b/modules/ducktests/README.md index 3d56ca33f292d..f9f6571d1b6ca 100644 --- a/modules/ducktests/README.md +++ b/modules/ducktests/README.md @@ -236,6 +236,46 @@ You can target specific cross-product version compatibility combinations inside --global-json '{"safepoint_log_enabled": true}' ``` +### Demo Mode (Breakpoints) + +Scenarios can be frozen at named breakpoints, so a cluster can be shown to an audience in exactly that state and then resumed. Ducktape runs the test inside `ducker01` with stdin closed, so the keyboard lives in a second terminal. + +Terminal 1 - run the test with the `demo_pause` global: +```bash +./docker/run_tests.sh -n 10 -gj '{"demo_pause": "*"}' \ + -t ./ignitetest/tests/mdc/majority_partition_test.py::MdcMajorityPartitionTest.test_minority_dc_isolation +``` + +Terminal 2 - drive the breakpoints: +```bash +python docker/demo_console.py +``` + +At every breakpoint the console prints the step, the elapsed time, every service node with its liveness, the live network state (netem delays and partition drops as they are actually applied), and ready-to-paste commands for entering nodes and reading their logs and configs. `Enter` continues, `c` runs the rest unattended, `a` aborts the test. + +While paused the cluster keeps running and any network impairment stays in effect, so nodes can be inspected freely: +```bash +./docker/ducker-ignite ssh ducker03 +docker exec ducker03 bash -c "tail -n 50 /mnt/service/logs/ignite*.log" +docker exec ducker03 cat /mnt/service/config/ignite-config.xml +``` + +The console is optional - the test communicates through files under `/.ducktests-demo`, which is shared with the host by the same bind mount that carries the repository into the containers: +```bash +cat .ducktests-demo/paused.txt # the banner of the breakpoint currently held +touch .ducktests-demo/continue-3 # resume breakpoint 3 +touch .ducktests-demo/continue-all # resume and skip the remaining breakpoints +touch .ducktests-demo/abort # fail the test and tear down +``` + +Breakpoints are added to a test with `self.pause("name", mdc, net)` and cost nothing when the global is absent, which is how they stay in the tests without affecting CI. Run one test at a time in demo mode (no `--max-parallel`): a single control directory holds one breakpoint at a time. + +| Global Parameter Key | Definition | Example Configuration | +|---------------------|------------|----------------------| +| **demo_pause** | Which breakpoints stop the scenario. Absent or `false` disables them all (the default); `true` or `"*"` stops at every one; a list or comma separated string stops only at the named ones. | ```{"demo_pause": "split-brain,healed"}``` | +| **demo_pause_timeout_sec** | How long one breakpoint may hold the scenario before it resumes on its own. Default is 1800. | ```{"demo_pause_timeout_sec": 3600}``` | +| **demo_pause_dir** | Control directory shared with the host. Default is `/.ducktests-demo`. | ```{"demo_pause_dir": "/opt/ignite-dev/.demo"}``` | + ### Security Settings ```bash # Enable built-in authentication overrides diff --git a/modules/ducktests/tests/checks/utils/check_pause.py b/modules/ducktests/tests/checks/utils/check_pause.py new file mode 100644 index 0000000000000..39cee52a38284 --- /dev/null +++ b/modules/ducktests/tests/checks/utils/check_pause.py @@ -0,0 +1,209 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +""" +Checks demo breakpoints. +""" + +import json +import os +import threading + +import pytest + +from ignitetest.utils.pause import ALL, CONTINUE_ALL, ABORT, DemoPause, STATUS_JSON, STATUS_TXT, \ + continue_file, parse_selector + + +class FakeLogger: + """ + Collects what a paused test would have logged. + """ + def __init__(self): + self.messages = [] + + def info(self, msg): + """Records an info message.""" + self.messages.append(msg) + + def warn(self, msg): + """Records a warning.""" + self.messages.append(msg) + + debug = info + error = warn + + +def _pause(control_dir, **test_globals): + return DemoPause(FakeLogger(), test_globals, "check.CheckPause.check_something", control_dir=str(control_dir)) + + +def _resume_with(control_dir, name, delay_sec=.05): + """ + Creates a resume file from another thread, the way the host does while the test blocks. + """ + timer = threading.Timer(delay_sec, lambda: open(os.path.join(str(control_dir), name), "w").close()) + timer.daemon = True + timer.start() + + return timer + + +def check_selector_parsing(): + """ + Check that every shape the demo_pause global can arrive in is understood: -g passes it as + a string, -gj as whatever the json holds. + """ + for disabled in (None, False, "", "false", "off", "0", [], " "): + assert parse_selector(disabled) is None, disabled + + for every in (True, "*", "all", "true", "ON", "1"): + assert parse_selector(every) == ALL, every + + assert parse_selector("split-brain") == {"split-brain"} + assert parse_selector("split-brain, healed ,") == {"split-brain", "healed"} + assert parse_selector(["split-brain", "healed"]) == {"split-brain", "healed"} + + +def check_disabled_leaves_no_trace(tmp_path): + """ + Check that without the global a breakpoint is a plain return: it must not block, and it + must not even create the control directory, since every test carries breakpoints in CI. + """ + control_dir = tmp_path / "control" + + demo = _pause(control_dir) + + assert not demo.enabled + + demo.pause("split-brain") + + assert not os.path.exists(str(control_dir)) + assert demo.seq == 0 + + +def check_selected_breakpoints_only(tmp_path): + """ + Check that only the named breakpoints stop the scenario. + """ + demo = _pause(tmp_path, demo_pause="split-brain") + + demo.pause("cluster-up") + demo.pause("healed") + + assert demo.seq == 0, "an unnamed breakpoint must not stop the scenario" + + _resume_with(tmp_path, continue_file(1)) + + demo.pause("split-brain") + + assert demo.seq == 1 + + +def check_publishes_and_consumes_status(tmp_path): + """ + Check the published breakpoint - what the host reads - and that the test cleans it up + once resumed, so a stale banner never outlives the pause it describes. + """ + demo = _pause(tmp_path, demo_pause=True) + + published = {} + + def resume(): + with open(str(tmp_path / STATUS_JSON), encoding="utf-8") as file: + published.update(json.load(file)) + + open(str(tmp_path / continue_file(1)), "w").close() + + timer = threading.Timer(.05, resume) + timer.daemon = True + timer.start() + + demo.pause("split-brain", services=[]) + + assert published["seq"] == 1 + assert published["name"] == "split-brain" + assert published["test"] == "check.CheckPause.check_something" + assert any("PAUSED 1 split-brain" in line for line in published["banner"]) + + for leftover in (STATUS_JSON, STATUS_TXT, continue_file(1)): + assert not os.path.exists(str(tmp_path / leftover)), leftover + + +def check_continue_all_skips_the_rest(tmp_path): + """ + Check that continue-all resumes the current breakpoint and disables every later one, so + a demo can be cut short without restarting the scenario. + """ + demo = _pause(tmp_path, demo_pause=ALL) + + _resume_with(tmp_path, CONTINUE_ALL) + + demo.pause("split-brain") + + assert demo.seq == 1 + assert not demo.enabled + + demo.pause("healed") + + assert demo.seq == 1, "breakpoints after continue-all must not stop the scenario" + assert not os.path.exists(str(tmp_path / CONTINUE_ALL)) + + +def check_abort_fails_the_test(tmp_path): + """ + Check that abort ends the scenario through an assertion, so ducktape tears the cluster + down instead of leaving it running. + """ + demo = _pause(tmp_path, demo_pause=ALL) + + _resume_with(tmp_path, ABORT) + + with pytest.raises(AssertionError, match="split-brain"): + demo.pause("split-brain") + + assert not os.path.exists(str(tmp_path / ABORT)) + assert not os.path.exists(str(tmp_path / STATUS_JSON)) + + +def check_stale_resume_file_is_cleared(tmp_path): + """ + Check that a resume file left by a previous run does not skip the first breakpoint of + this one - the control directory outlives a test, its contents must not. + """ + open(str(tmp_path / continue_file(1)), "w").close() + open(str(tmp_path / STATUS_TXT), "w").close() + + demo = _pause(tmp_path, demo_pause=ALL, demo_pause_timeout_sec=.3) + + demo.pause("split-brain") + + assert demo.seq == 1 + assert any("timed out" in msg for msg in demo.logger.messages), \ + "the stale file must have been cleared, leaving the breakpoint to time out" + + +def check_timeout_resumes_on_its_own(tmp_path): + """ + Check that a forgotten breakpoint gives up rather than holding the scenario until + ducktape kills it. + """ + demo = _pause(tmp_path, demo_pause=ALL, demo_pause_timeout_sec=.3) + + demo.pause("split-brain") + + assert demo.seq == 1 + assert demo.enabled, "a timed out breakpoint must not disable the later ones" + assert not os.path.exists(str(tmp_path / STATUS_JSON)) diff --git a/modules/ducktests/tests/docker/demo_console.py b/modules/ducktests/tests/docker/demo_console.py new file mode 100644 index 0000000000000..f33a7f44a320a --- /dev/null +++ b/modules/ducktests/tests/docker/demo_console.py @@ -0,0 +1,184 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +""" +Host side of the demo breakpoints - run it in a second terminal, next to the one running +``run_tests.sh``, when a test is started with the ``demo_pause`` global: + + ./docker/run_tests.sh -gj '{"demo_pause": "*"}' -t ./ignitetest/tests/ + + python docker/demo_console.py + +Ducktape runs the test with stdin on /dev/null inside the ``ducker01`` container, so this is +where the keyboard lives. The console itself is deliberately dumb: the test renders the +banner and this only prints it and writes back a resume file. Everything it does can be done +by hand instead - ``cat .ducktests-demo/paused.txt``, then ``touch .ducktests-demo/continue-3``. + +Standard library only: it runs on the host, outside the ducktests virtualenv. +""" + +import argparse +import importlib.util +import json +import os +import sys +import time + +# Reach into the framework for the protocol constants rather than restating them. The host +# has no ducktape and no installed ignitetest, so the module is loaded by path: importing +# ignitetest.utils.pause would pull in the package __init__ chain and its ducktape imports. +_TESTS_DIR = os.path.abspath(os.path.join(os.path.dirname(os.path.abspath(__file__)), os.pardir)) +_PAUSE_PY = os.path.join(_TESTS_DIR, "ignitetest", "utils", "pause.py") + +_SPEC = importlib.util.spec_from_file_location("ignitetest_pause", _PAUSE_PY) +pause = importlib.util.module_from_spec(_SPEC) +_SPEC.loader.exec_module(pause) + +POLL_SEC = .3 + +KEYS = """ + [Enter] continue [c] continue, skipping the rest [a] abort the test + [q] leave the console (the test stays paused) +""" + + +def read_status(control_dir): + """ + :return: The published breakpoint, or None when the scenario is not paused. A missing or + half written file simply reads as "not paused" and is retried. + """ + path = os.path.join(control_dir, pause.STATUS_JSON) + + try: + with open(path, encoding="utf-8") as file: + return json.load(file) + except (OSError, ValueError): + return None + + +def clear_stale(control_dir): + """ + Removes resume files left behind by an earlier run, which would otherwise skip the first + breakpoint of this one. The test clears them too, on its side, at its first breakpoint. + """ + if not os.path.isdir(control_dir): + return + + for name in os.listdir(control_dir): + if name.startswith(pause.CONTINUE_PREFIX) or name == pause.ABORT: + try: + os.remove(os.path.join(control_dir, name)) + except OSError: + pass + + +def resume(control_dir, name): + """ + Writes a resume file. The test consumes and removes it. + """ + with open(os.path.join(control_dir, name), "w", encoding="utf-8") as file: + file.write("") + + +def prompt(control_dir, seq): + """ + Asks what to do with the breakpoint that is currently published. + + :return: False when the console should stop, True to wait for the next breakpoint. + """ + while True: + try: + answer = input(" > ").strip().lower() + except EOFError: + return False + + if answer in ("", "n", "next"): + resume(control_dir, pause.continue_file(seq)) + + return True + + if answer in ("c", "continue", "all"): + resume(control_dir, pause.CONTINUE_ALL) + + print(" continuing, remaining breakpoints skipped") + + return False + + if answer in ("a", "abort"): + resume(control_dir, pause.ABORT) + + print(" aborting the test") + + return False + + if answer in ("q", "quit", "exit"): + print(f" leaving the test paused, resume it with:\n" + f" touch {os.path.join(control_dir, pause.continue_file(seq))}") + + return False + + print(KEYS) + + +def main(): + """ + Waits for breakpoints and drives them until the test is resumed for good. + """ + parser = argparse.ArgumentParser(description="Drives the ducktests demo breakpoints.") + parser.add_argument("-d", "--control-dir", default=pause.default_control_dir(), + help="control directory shared with the test, defaults to " + f"/{pause.CONTROL_DIR_NAME}") + + args = parser.parse_args() + control_dir = args.control_dir + + clear_stale(control_dir) + + print(f"Demo console, watching {control_dir}") + print("Waiting for the first breakpoint... (Ctrl-C to leave)") + + last_seq, resumed_at = 0, None + + while True: + status = read_status(control_dir) + + if status is None or status.get("seq") == last_seq: + time.sleep(POLL_SEC) + + continue + + last_seq = status.get("seq") + + print() + + if resumed_at is not None: + print(f" ({time.monotonic() - resumed_at:.0f}s since the previous breakpoint)") + + print("\n".join(status.get("banner", []))) + print(KEYS) + + if not prompt(control_dir, last_seq): + return + + resumed_at = time.monotonic() + + print(" resumed, waiting for the next breakpoint...") + + +if __name__ == "__main__": + try: + main() + except KeyboardInterrupt: + sys.exit(130) diff --git a/modules/ducktests/tests/ignitetest/services/mdc/mdc_cluster.py b/modules/ducktests/tests/ignitetest/services/mdc/mdc_cluster.py index d37b9c045fa1c..1f492b501b014 100644 --- a/modules/ducktests/tests/ignitetest/services/mdc/mdc_cluster.py +++ b/modules/ducktests/tests/ignitetest/services/mdc/mdc_cluster.py @@ -195,6 +195,35 @@ def network_registry(self) -> Dict[str, List]: return registry + def describe(self) -> List[str]: + """ + Describes the cluster per data center for a demo breakpoint banner + (see :meth:`ignitetest.utils.ignite_test.IgniteTest.pause`). The generic banner sees + a flat list of services, which is where the DC each node belongs to gets lost. + + Structure only - which node is up is what the banner's own service section reports, + and it pays an SSH probe per node to find out. + + :return: Section lines, the first one being the section title. + """ + lines = ["DATA CENTERS"] + + for dc in self.dcs: + main = " (main)" if dc == self.main_dc and len(self.dcs) % 2 == 0 else "" + + lines.append(f" {dc}{main}") + + for label, services in (("server", [self.servers[dc]] if dc in self.servers else []), + ("runner", self.runners.get(dc, [])), + ("loader", self.loaders.get(dc, [])), + ("extra", self.extras.get(dc, []))): + hosts = [node.account.hostname for svc in services for node in svc.nodes] + + if hosts: + lines.append(f" {label:<7} {' '.join(hosts)}") + + return lines + def thin_client_addresses(self) -> List[str]: """ :return: Thin client addresses of all server nodes across all DCs. diff --git a/modules/ducktests/tests/ignitetest/services/network_group/manager.py b/modules/ducktests/tests/ignitetest/services/network_group/manager.py index 3f90b7f9450a1..c06884b37267b 100644 --- a/modules/ducktests/tests/ignitetest/services/network_group/manager.py +++ b/modules/ducktests/tests/ignitetest/services/network_group/manager.py @@ -297,6 +297,29 @@ def _log_network(self, log_tag: str): """ self.logger.debug(f"Network State Overview: [START][{log_tag}]") + # The per-node SSH probes flood the debug log with their own command output. Collect + # first, print contiguously after: the overview must stay readable as one block. + for node_status in self._probe_network(): + self.logger.debug(node_status) + + self.logger.debug(f"Network State Overview: [END][{log_tag}]") + + def describe(self): + """ + Describes the live network state for a demo breakpoint banner + (see :meth:`ignitetest.utils.ignite_test.IgniteTest.pause`). + + :return: Section lines, the first one being the section title. + """ + return ["NETWORK"] + [f" {status}" for status in self._probe_network()] + + def _probe_network(self): + """ + Probes the actual network state of every node - the applied netem constraints, what + they are filtered onto, and the partition drops - rather than what was asked for. + + :return: One line per node, grouped by network group. + """ entries = [(group, svc, node) for group, services in self.network_group_registry.items() for svc in services @@ -322,12 +345,7 @@ def _log_network(self, log_tag: str): node_statuses.append(f"[{group:<4}] {svc.who_am_i(node):<45}[{node_ip}] : " f"{constraints}{targets_str}{partition_str}") - # The per-node SSH probes above flood the debug log with their own command output. - # Collect first, print contiguously after: the overview must stay readable as one block. - for node_status in node_statuses: - self.logger.debug(node_status) - - self.logger.debug(f"Network State Overview: [END][{log_tag}]") + return node_statuses def _to_network_probe_cmd(self, node) -> str: """ diff --git a/modules/ducktests/tests/ignitetest/tests/mdc/partition_resilience_test.py b/modules/ducktests/tests/ignitetest/tests/mdc/partition_resilience_test.py index 2ac3929805558..e67d807caeaa0 100644 --- a/modules/ducktests/tests/ignitetest/tests/mdc/partition_resilience_test.py +++ b/modules/ducktests/tests/ignitetest/tests/mdc/partition_resilience_test.py @@ -63,17 +63,23 @@ def test_mdc_cluster_partition_resilience(self, ignite_version, cross_dc_latency with cross_dc_network(self.logger, mdc, delay_ms=cross_dc_latency_ms) as net: mdc.start_servers() + self.pause("cluster-up", mdc, net) + mdc.generate_data(DC_1, CACHE_NAME, 0, 100, backups=BACKUPS) mdc.generate_data(DC_2, CACHE_NAME, 100, 200, backups=BACKUPS) mdc.verify_cache_distribution(CACHE_NAME, copies_per_dc=1) + self.pause("data-loaded", mdc, net) + net.enable_network_partition(DC_1, DC_2) sleep(SPLIT_SETTLE_SECS) mdc.verify_split_brain() + self.pause("split-brain", mdc, net) + # All data written before the split is readable in both halves. mdc.check_data(DC_1, CACHE_NAME, 0, 200) mdc.check_data(DC_2, CACHE_NAME, 0, 200) @@ -82,6 +88,8 @@ def test_mdc_cluster_partition_resilience(self, ignite_version, cross_dc_latency mdc.check_put_admissibility(DC_1, CACHE_NAME, True) mdc.check_put_admissibility(DC_2, CACHE_NAME, False) + self.pause("secondary-read-only", mdc, net) + net.disable_network_partition(DC_1, DC_2) # Split-brain does not self-heal: the read-only half rejoins via restart. @@ -94,6 +102,8 @@ def test_mdc_cluster_partition_resilience(self, ignite_version, cross_dc_latency mdc.control(DC_1).idle_verify(CACHE_NAME) + self.pause("healed", mdc, net) + mdc.verify_servers_log_clean() mdc.stop_servers() diff --git a/modules/ducktests/tests/ignitetest/utils/ignite_test.py b/modules/ducktests/tests/ignitetest/utils/ignite_test.py index 1feda7859ea99..29c7bd25381b0 100644 --- a/modules/ducktests/tests/ignitetest/utils/ignite_test.py +++ b/modules/ducktests/tests/ignitetest/utils/ignite_test.py @@ -23,6 +23,7 @@ from ducktape.tests.test import Test, TestContext from ignitetest.services.utils.ducktests_service import DucktestsService +from ignitetest.utils.pause import DemoPause # globals: JFR_ENABLED = "jfr_enabled" @@ -66,6 +67,28 @@ def __init__(self, test_context): super().__init__(test_context=test_context) + self.__demo_pause = None + + def pause(self, name, *describers): + """ + Holds the scenario at a named demo breakpoint until it is resumed from the host, so + that the cluster can be shown in exactly this state. Does nothing at all unless the + `demo_pause` global names this breakpoint - see :mod:`ignitetest.utils.pause`. + + Must be called from the test body: :meth:`tearDown` kills every service, so a + breakpoint placed after the body would only ever show a dead cluster. + + :param name: Breakpoint name, matched against the `demo_pause` global. + :param describers: Objects exposing `describe() -> list of str`, each contributing a + section to the banner shown while paused, on top of the service list every + breakpoint reports. Any fixture a test drives can implement it. + """ + if self.__demo_pause is None: + self.__demo_pause = DemoPause(self.logger, self.test_context.globals, self.test_context.test_name) + + # noinspection PyProtectedMember + self.__demo_pause.pause(name, describers, self.test_context.services._services.values()) + @property def available_cluster_size(self): # noinspection PyUnresolvedReferences diff --git a/modules/ducktests/tests/ignitetest/utils/pause.py b/modules/ducktests/tests/ignitetest/utils/pause.py new file mode 100644 index 0000000000000..6ed01e0c38bce --- /dev/null +++ b/modules/ducktests/tests/ignitetest/utils/pause.py @@ -0,0 +1,427 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +""" +Demo breakpoints: freezing a scenario at a named point so a live audience can be shown the +cluster in that exact state. + +Ducktape runs the test inside the ``ducker01`` container with stdin on /dev/null, so a test +cannot read a keypress. What it can do is share files with the host: ``ducker-ignite`` bind +mounts the whole Ignite repository into every container, so a control directory below the +repository root is visible to the test and to the host at the same time. + +The protocol over that directory is one sided on purpose - the test is the only party that +deletes anything, so no step can race with the host: + + - the test publishes ``paused.txt`` (a rendered banner) and ``paused.json`` (the same + content as data) and then blocks; + - the host creates ``continue-``, ``continue-all`` or ``abort``; + - the test consumes that file, removes it along with its own status files, and proceeds. + +``docker/demo_console.py`` is the host side of it, but nothing depends on it: reading +``paused.txt`` and touching ``continue-`` by hand works just as well. + +Globals: + + demo_pause - absent or false disables every breakpoint (the default, so tests are + unaffected in CI); true or "*" stops at all of them; a list or a comma separated string + stops only at the named ones. + + demo_pause_timeout_sec - how long a single breakpoint may hold the scenario before it + resumes on its own, 1800 by default. + + demo_pause_dir - control directory, ``/.ducktests-demo`` by default. +""" + +import json +import os +import time + +# globals: +DEMO_PAUSE = "demo_pause" +DEMO_PAUSE_TIMEOUT_SEC = "demo_pause_timeout_sec" +DEMO_PAUSE_DIR = "demo_pause_dir" + +DEFAULT_TIMEOUT_SEC = 1800 + +CONTROL_DIR_NAME = ".ducktests-demo" + +STATUS_TXT = "paused.txt" +STATUS_JSON = "paused.json" +CONTINUE_PREFIX = "continue-" +CONTINUE_ALL = "continue-all" +ABORT = "abort" + +# The control directory is polled rather than watched: it is a bind mount shared with the +# host, where inotify is not dependable. +POLL_SEC = .5 + +# A paused test is silent, and ducktape kills a test client that stays silent for too long +# (--test-runner-timeout). The heartbeat keeps the client alive and timestamps the demo in +# the test log. +HEARTBEAT_SEC = 15 + +# Every breakpoint name matches. +ALL = "*" + +_WIDTH = 100 + + +def repo_root(): + """ + :return: Path of the Ignite repository root, derived from this module's own location + (``/modules/ducktests/tests/ignitetest/utils/pause.py``), so that a fork + checked out elsewhere resolves its own root. + """ + return os.path.abspath(os.path.join(os.path.dirname(__file__), *[os.pardir] * 5)) + + +def default_control_dir(): + """ + :return: Path of the control directory shared between the test and the host. + """ + return os.path.join(repo_root(), CONTROL_DIR_NAME) + + +def continue_file(seq): + """ + :return: Name of the file that resumes the breakpoint with the given sequence number. + """ + return f"{CONTINUE_PREFIX}{seq}" + + +def parse_selector(value): + """ + Interprets the ``demo_pause`` global. + + :return: None when demo pausing is disabled, :data:`ALL` to stop at every breakpoint, + or the set of breakpoint names to stop at. + """ + if value is None or value is False: + return None + + if value is True: + return ALL + + if isinstance(value, (list, tuple, set, frozenset)): + names = {str(name).strip() for name in value} + names.discard("") + + return names or None + + if isinstance(value, str): + text = value.strip().lower() + + if text in ("", "false", "no", "off", "0"): + return None + + if text in (ALL, "all", "true", "yes", "on", "1"): + return ALL + + names = {name.strip() for name in value.split(",")} + names.discard("") + + return names or None + + return ALL if value else None + + +def _fmt_duration(seconds): + """ + :return: Duration as mm:ss, or hh:mm:ss once it no longer fits. + """ + seconds = max(int(seconds), 0) + + if seconds >= 3600: + return f"{seconds // 3600}:{seconds // 60 % 60:02d}:{seconds % 60:02d}" + + return f"{seconds // 60:02d}:{seconds % 60:02d}" + + +def _node_addr(node): + """ + :return: The node's routable address when it adds anything to the name the banner already + carries - under ducker the two are the same string. + + Deliberately not resolved to an IP: a name that does not resolve costs a DNS round trip + per node, and a breakpoint that takes seconds to print its banner defeats the point. The + NETWORK section resolves addresses where they actually matter. + """ + addr = node.account.externally_routable_ip + + return "" if addr == node.account.hostname else f"[{addr}]" + + +def _node_state(service, node): + """ + :return: Liveness of the node as far as its service can tell, "?" when the probe itself + failed - a breakpoint must never fail the scenario it is only observing. + """ + alive = getattr(service, "alive", None) + + if alive is None: + return "" + + try: + return "UP" if alive(node) else "DOWN" + except Exception: # pylint: disable=broad-except + return "?" + + +class DemoPause: + """ + Holds a scenario at named breakpoints. + + Disabled unless the ``demo_pause`` global says otherwise, in which case :meth:`pause` is + a plain return and nothing is written anywhere. + """ + def __init__(self, logger, test_globals, test_name, control_dir=None): + self.logger = logger + self.test_name = test_name + + self.names = parse_selector(test_globals.get(DEMO_PAUSE)) + self.timeout_sec = float(test_globals.get(DEMO_PAUSE_TIMEOUT_SEC, DEFAULT_TIMEOUT_SEC)) + + self.control_dir = control_dir or test_globals.get(DEMO_PAUSE_DIR) or default_control_dir() + + self.seq = 0 + + self._started_at = time.monotonic() + self._prepared = False + self._continue_all = False + + @property + def enabled(self): + """ + :return: Whether any breakpoint of this test can stop the scenario. + """ + return self.names is not None and not self._continue_all + + def pause(self, name, describers=(), services=()): + """ + Blocks the scenario at the named breakpoint until the host resumes it. + + :param name: Breakpoint name, matched against the ``demo_pause`` global. + :param describers: Objects exposing ``describe() -> list of str``, each contributing + a section to the banner. The first line of a section is its title. + :param services: Services to list in the banner, normally the test's whole registry. + """ + if not self._stops_at(name): + return + + self._prepare() + + self.seq += 1 + + banner = self._render(name, describers, services) + + self._publish(name, banner) + + self.logger.info(f"Demo breakpoint reached [seq={self.seq}, name={name}, dir={self.control_dir}]") + + self._await_resume(name) + + def _stops_at(self, name): + if not self.enabled: + return False + + return self.names == ALL or name in self.names + + def _prepare(self): + """ + Creates the control directory and clears anything a previous run left behind: a + stale resume file would skip the very first breakpoint of this one. + """ + if self._prepared: + return + + os.makedirs(self.control_dir, exist_ok=True) + + for name in os.listdir(self.control_dir): + if name.startswith(CONTINUE_PREFIX) or name.startswith(STATUS_TXT) \ + or name.startswith(STATUS_JSON) or name == ABORT: + self._remove(name) + + self._prepared = True + + def _render(self, name, describers, services): + """ + :return: The banner as a list of lines. + """ + elapsed = f" t+{_fmt_duration(time.monotonic() - self._started_at)} since test start" + auto = f"auto-continue in {_fmt_duration(self.timeout_sec)} " + + lines = [ + "=" * _WIDTH, + f" PAUSED {self.seq} {name}", + f" test {self.test_name}", + (elapsed + auto.rjust(max(_WIDTH - len(elapsed), 1))).rstrip(), + ] + + for section in [self._services_section(services)] + [self._section(d) for d in describers]: + if section: + lines.append("-" * _WIDTH) + lines.extend(section) + + lines.append("-" * _WIDTH) + lines.extend(self._hints_section(services)) + + lines.append("-" * _WIDTH) + lines.append(" continue: [Enter] in the demo console") + lines.append(f" or touch {os.path.join(self.control_dir, continue_file(self.seq))}") + lines.append("=" * _WIDTH) + + return lines + + def _section(self, describer): + try: + return list(describer.describe()) + except Exception as ex: # pylint: disable=broad-except + self.logger.warn(f"Demo breakpoint describer failed [describer={describer}, error={ex}]") + + return [] + + @staticmethod + def _services_section(services): + lines = ["SERVICES"] + + for service in services: + for node in service.nodes: + who = service.who_am_i(node) + addr = _node_addr(node) + + lines.append(f" {who:<58} {addr:<17} {_node_state(service, node)}".rstrip()) + + if len(lines) == 1: + lines.append(" (none)") + + return lines + + @staticmethod + def _hints_section(services): + """ + Node logs live only on the nodes while the test runs - ducktape copies them into the + results directory at teardown - so every hint goes through the node container. + """ + hosts = sorted({node.account.hostname for service in services for node in service.nodes}) + + log_dir = "/mnt/service/logs" + config_file = "/mnt/service/config/ignite-config.xml" + + for service in services: + try: + log_dir = getattr(service, "log_dir", None) or log_dir + config_file = getattr(service, "config_file", None) or config_file + except Exception: # pylint: disable=broad-except + pass + + break + + # Node paths, joined by hand: they are always POSIX, os.path.join is not when the + # control machine happens to be Windows. + ignite_log = f"{log_dir.rstrip('/')}/ignite*.log" + console_log = f"{log_dir.rstrip('/')}/console.log" + + return [ + f" nodes {' '.join(hosts) if hosts else '(none)'}", + " shell ./docker/ducker-ignite ssh ", + f" logs docker exec bash -c \"tail -n 50 {ignite_log}\"", + f" console docker exec tail -n 50 {console_log}", + f" config docker exec cat {config_file}", + ] + + def _publish(self, name, banner): + """ + Publishes the breakpoint for the host, banner first as text and then as data: both + files are replaced atomically so the console never reads a half written one. + """ + text = "\n".join(banner) + "\n" + + self._write(STATUS_TXT, text) + + self._write(STATUS_JSON, json.dumps({ + "seq": self.seq, + "name": name, + "test": self.test_name, + "elapsed_sec": round(time.monotonic() - self._started_at, 1), + "timeout_sec": self.timeout_sec, + "banner": banner + }, indent=2)) + + def _await_resume(self, name): + deadline = time.monotonic() + self.timeout_sec + heartbeat = time.monotonic() + HEARTBEAT_SEC + + while True: + if self._exists(ABORT): + self._consume(ABORT) + + raise AssertionError(f"Demo aborted at breakpoint [seq={self.seq}, name={name}]") + + if self._exists(CONTINUE_ALL): + self._continue_all = True + + self._consume(CONTINUE_ALL) + self.logger.info(f"Demo resumed, remaining breakpoints skipped [seq={self.seq}, name={name}]") + + return + + if self._exists(continue_file(self.seq)): + self._consume(continue_file(self.seq)) + self.logger.info(f"Demo resumed [seq={self.seq}, name={name}]") + + return + + now = time.monotonic() + + if now >= deadline: + self._consume() + self.logger.warn(f"Demo breakpoint timed out after {self.timeout_sec}s, resuming " + f"[seq={self.seq}, name={name}]") + + return + + if now >= heartbeat: + heartbeat = now + HEARTBEAT_SEC + + self.logger.info(f"Still paused at demo breakpoint [seq={self.seq}, name={name}, " + f"waiting={_fmt_duration(self.timeout_sec - (deadline - now))}]") + + time.sleep(POLL_SEC) + + def _consume(self, *names): + """ + Clears the published breakpoint along with the resume files that ended it. + """ + for name in names + (STATUS_TXT, STATUS_JSON): + self._remove(name) + + def _exists(self, name): + return os.path.exists(os.path.join(self.control_dir, name)) + + def _remove(self, name): + try: + os.remove(os.path.join(self.control_dir, name)) + except OSError: + pass + + def _write(self, name, content): + path = os.path.join(self.control_dir, name) + tmp = path + ".tmp" + + with open(tmp, "w", encoding="utf-8") as file: + file.write(content) + + os.replace(tmp, path) From f6b0fe9a7bd0d128e9bd7b22ae54433a67dde300 Mon Sep 17 00:00:00 2001 From: Maksim Davydov Date: Wed, 12 Aug 2026 19:01:50 +0300 Subject: [PATCH 02/15] IGNITE-28976 [ducktests] Add optional demo breakpoints to pause a running scenario for inspection --- modules/ducktests/README.md | 8 +- .../tests/checks/utils/check_pause.py | 155 ++++++++++++++++-- .../ducktests/tests/docker/demo_console.py | 31 +++- modules/ducktests/tests/docker/run_tests.sh | 9 + .../ignitetest/services/mdc/mdc_cluster.py | 6 +- .../tests/ignitetest/utils/ignite_test.py | 21 ++- .../ducktests/tests/ignitetest/utils/pause.py | 120 +++++++++++--- 7 files changed, 301 insertions(+), 49 deletions(-) diff --git a/modules/ducktests/README.md b/modules/ducktests/README.md index f9f6571d1b6ca..b0e1b4d5af72f 100644 --- a/modules/ducktests/README.md +++ b/modules/ducktests/README.md @@ -270,10 +270,16 @@ touch .ducktests-demo/abort # fail the test and tear down Breakpoints are added to a test with `self.pause("name", mdc, net)` and cost nothing when the global is absent, which is how they stay in the tests without affecting CI. Run one test at a time in demo mode (no `--max-parallel`): a single control directory holds one breakpoint at a time. +A held test reports nothing back to ducktape, which kills a session it has heard nothing from for `--test-runner-timeout` (30 minutes by default) - and that budget is spent by the whole test, setup included, not by the breakpoint alone. Breakpoints therefore auto-continue while the runner is still waiting, shortening themselves below `demo_pause_timeout_sec` when there is not enough of the budget left and saying so in the test log. For a demo that needs longer, raise the runner timeout too (milliseconds): +```bash +./docker/run_tests.sh --test-runner-timeout 7200000 -gj '{"demo_pause": "*", "demo_pause_timeout_sec": 1800}' \ + -t ./ignitetest/tests/mdc/majority_partition_test.py +``` + | Global Parameter Key | Definition | Example Configuration | |---------------------|------------|----------------------| | **demo_pause** | Which breakpoints stop the scenario. Absent or `false` disables them all (the default); `true` or `"*"` stops at every one; a list or comma separated string stops only at the named ones. | ```{"demo_pause": "split-brain,healed"}``` | -| **demo_pause_timeout_sec** | How long one breakpoint may hold the scenario before it resumes on its own. Default is 1800. | ```{"demo_pause_timeout_sec": 3600}``` | +| **demo_pause_timeout_sec** | How long one breakpoint may hold the scenario before it resumes on its own. Default is 600, and it is capped by what is left of `--test-runner-timeout`. | ```{"demo_pause_timeout_sec": 1800}``` | | **demo_pause_dir** | Control directory shared with the host. Default is `/.ducktests-demo`. | ```{"demo_pause_dir": "/opt/ignite-dev/.demo"}``` | ### Security Settings diff --git a/modules/ducktests/tests/checks/utils/check_pause.py b/modules/ducktests/tests/checks/utils/check_pause.py index 39cee52a38284..0e974398f1bcf 100644 --- a/modules/ducktests/tests/checks/utils/check_pause.py +++ b/modules/ducktests/tests/checks/utils/check_pause.py @@ -20,11 +20,14 @@ import json import os import threading +import time +from types import SimpleNamespace import pytest -from ignitetest.utils.pause import ALL, CONTINUE_ALL, ABORT, DemoPause, STATUS_JSON, STATUS_TXT, \ - continue_file, parse_selector +from ignitetest.services.utils.path import IgnitePathAware +from ignitetest.utils.pause import ALL, CONTINUE_ALL, ABORT, DemoPause, RUNNER_TIMEOUT_MARGIN_SEC, STATUS_JSON, \ + STATUS_TXT, continue_file, parse_selector class FakeLogger: @@ -46,8 +49,68 @@ def warn(self, msg): error = warn -def _pause(control_dir, **test_globals): - return DemoPause(FakeLogger(), test_globals, "check.CheckPause.check_something", control_dir=str(control_dir)) +def _fake_nodes(*hostnames): + return [SimpleNamespace(account=SimpleNamespace(hostname=host, externally_routable_ip=host)) + for host in hostnames] + + +class FakeService: + """ + Stands in for a non-Ignite service of the test registry, e.g. a zookeeper one: it carries + paths of its own, which the banner must not hand out for Ignite nodes. + """ + log_dir = "/mnt/service/zk-logs" + config_file = "/mnt/service/zookeeper.properties" + + def __init__(self, *hostnames): + self.nodes = _fake_nodes(*hostnames) + + +class FakeIgniteService(IgnitePathAware): + """ + Stands in for an Ignite service, with the real path layout behind it. + """ + def __init__(self, *hostnames): + self.nodes = _fake_nodes(*hostnames) + + @property + def product(self): + return "ignite-dev" + + @property + def globals(self): + return {} + + +def _pause(control_dir, started_at=None, runner_timeout_sec=None, **test_globals): + return DemoPause(FakeLogger(), test_globals, "check.CheckPause.check_something", control_dir=str(control_dir), + started_at=started_at, runner_timeout_sec=runner_timeout_sec) + + +def _read_published(control_dir, resume_with=None, delay_sec=.05): + """ + Reads the published breakpoint while the test blocks on it, the way the host console does, + and optionally resumes it. + + :return: The dict that is filled in once the breakpoint has been published. + """ + published = {} + + def act(): + try: + with open(os.path.join(str(control_dir), STATUS_JSON), encoding="utf-8") as file: + published.update(json.load(file)) + except (OSError, ValueError): + pass + + if resume_with: + open(os.path.join(str(control_dir), resume_with), "w").close() + + timer = threading.Timer(delay_sec, act) + timer.daemon = True + timer.start() + + return published def _resume_with(control_dir, name, delay_sec=.05): @@ -119,21 +182,12 @@ def check_publishes_and_consumes_status(tmp_path): """ demo = _pause(tmp_path, demo_pause=True) - published = {} - - def resume(): - with open(str(tmp_path / STATUS_JSON), encoding="utf-8") as file: - published.update(json.load(file)) - - open(str(tmp_path / continue_file(1)), "w").close() - - timer = threading.Timer(.05, resume) - timer.daemon = True - timer.start() + published = _read_published(tmp_path, resume_with=continue_file(1)) demo.pause("split-brain", services=[]) assert published["seq"] == 1 + assert published["run"] == demo.run assert published["name"] == "split-brain" assert published["test"] == "check.CheckPause.check_something" assert any("PAUSED 1 split-brain" in line for line in published["banner"]) @@ -207,3 +261,74 @@ def check_timeout_resumes_on_its_own(tmp_path): assert demo.seq == 1 assert demo.enabled, "a timed out breakpoint must not disable the later ones" assert not os.path.exists(str(tmp_path / STATUS_JSON)) + + +def check_timeout_stays_within_the_runner_budget(tmp_path): + """ + Check that a breakpoint gives up while ducktape's runner is still waiting: it hears + nothing from a paused test, and killing the client takes the whole session down instead + of just cutting the demo short. The requested timeout only ever shrinks. + """ + demo = _pause(tmp_path, demo_pause=ALL, demo_pause_timeout_sec=3600, + runner_timeout_sec=RUNNER_TIMEOUT_MARGIN_SEC + .3) + + demo.pause("split-brain") + + assert demo.seq == 1 + assert any("timed out" in msg for msg in demo.logger.messages), \ + "the breakpoint must not outsit the runner budget it was given" + assert any("--test-runner-timeout" in msg for msg in demo.logger.messages), \ + "shortening a breakpoint must say what to raise to keep it" + + +def check_timeout_is_left_alone_within_the_runner_budget(tmp_path): + """ + Check that the budget only ever caps the requested timeout - a demo that fits must be + held for exactly as long as it asked for. + """ + demo = _pause(tmp_path, demo_pause=ALL, demo_pause_timeout_sec=.3, runner_timeout_sec=1800) + + published = _read_published(tmp_path) + + demo.pause("split-brain") + + assert published["timeout_sec"] == .3 + assert not any("--test-runner-timeout" in msg for msg in demo.logger.messages) + + +def check_elapsed_is_counted_from_test_start(tmp_path): + """ + Check that the banner counts from the start of the test rather than from the first + breakpoint: the setup phase of a multi-node scenario is minutes long, and a demo that + reports t+00:00 after it hides exactly the part worth showing. + """ + demo = _pause(tmp_path, started_at=time.monotonic() - 600, demo_pause=ALL, demo_pause_timeout_sec=.3) + + published = _read_published(tmp_path) + + demo.pause("split-brain") + + assert published["elapsed_sec"] >= 600 + assert any("t+10:00 since test start" in line for line in published["banner"]) + + +def check_hints_follow_the_ignite_services(): + """ + Check that the copy-pasteable commands name the Ignite paths even when a service of + another kind was registered first, as the zookeeper discovery scenarios do. + """ + # noinspection PyProtectedMember + hints = "\n".join(DemoPause._hints_section([FakeService("ducker02"), # pylint: disable=protected-access + FakeIgniteService("ducker03")])) + + # The service paths come from os.path.join, which follows the control machine rather than + # the nodes - a check that runs on Windows would otherwise see its separators. + hints = hints.replace("\\", "/") + + assert "/mnt/service/config/ignite-config.xml" in hints + assert "/mnt/service/logs/ignite*.log" in hints + assert "zookeeper.properties" not in hints + assert "zk-logs" not in hints + + # Every node is still offered, whichever service it belongs to. + assert "ducker02 ducker03" in hints diff --git a/modules/ducktests/tests/docker/demo_console.py b/modules/ducktests/tests/docker/demo_console.py index f33a7f44a320a..08dbf03989daf 100644 --- a/modules/ducktests/tests/docker/demo_console.py +++ b/modules/ducktests/tests/docker/demo_console.py @@ -68,10 +68,23 @@ def read_status(control_dir): return None +def breakpoint_key(status): + """ + :return: What identifies the published breakpoint. Not the sequence number on its own: + that one is per test, so it restarts at 1 for every test of a session, and a + run that died while paused leaves behind a banner numbered like a live one. + """ + return status.get("run"), status.get("seq") + + def clear_stale(control_dir): """ Removes resume files left behind by an earlier run, which would otherwise skip the first breakpoint of this one. The test clears them too, on its side, at its first breakpoint. + + The published breakpoint itself is left alone: a console is just as likely to be started + against a test that is already holding one, and a stale banner is told apart by its run + id anyway. """ if not os.path.isdir(control_dir): return @@ -149,17 +162,27 @@ def main(): print(f"Demo console, watching {control_dir}") print("Waiting for the first breakpoint... (Ctrl-C to leave)") - last_seq, resumed_at = 0, None + last_key, resumed_at = None, None while True: status = read_status(control_dir) - if status is None or status.get("seq") == last_seq: + if status is None: + # The test removes its status files as it resumes, so this is also what tells the + # console that the breakpoint it has just driven is over and the next one - which + # may well repeat its number, in the next test of the session - is a new one. + last_key = None + + time.sleep(POLL_SEC) + + continue + + if breakpoint_key(status) == last_key: time.sleep(POLL_SEC) continue - last_seq = status.get("seq") + last_key = breakpoint_key(status) print() @@ -169,7 +192,7 @@ def main(): print("\n".join(status.get("banner", []))) print(KEYS) - if not prompt(control_dir, last_seq): + if not prompt(control_dir, status.get("seq")): return resumed_at = time.monotonic() diff --git a/modules/ducktests/tests/docker/run_tests.sh b/modules/ducktests/tests/docker/run_tests.sh index 511c106701a29..b23ca856dcd7b 100755 --- a/modules/ducktests/tests/docker/run_tests.sh +++ b/modules/ducktests/tests/docker/run_tests.sh @@ -84,6 +84,10 @@ The options are as follows: --image Set custom docker image to run tests on. +--test-runner-timeout + Milliseconds ducktape waits for a sign of life from a running test before killing the + session, 1800000 by default. + EOF exit 0 } @@ -131,6 +135,7 @@ while [[ $# -ge 1 ]]; do --subnet) SUBNET="--subnet $2"; shift 2;; --jdk) JDK_VERSION="$2"; shift 2;; --image) IMAGE_NAME="$2"; shift 2;; + --test-runner-timeout) TEST_RUNNER_TIMEOUT="$2"; shift 2;; -f|--force) FORCE=$1; shift;; *) break;; esac @@ -169,5 +174,9 @@ if [[ -n "$REPEAT" ]]; then DUCKTAPE_OPTIONS="$DUCKTAPE_OPTIONS --repeat $REPEAT" fi +if [[ -n "$TEST_RUNNER_TIMEOUT" ]]; then + DUCKTAPE_OPTIONS="$DUCKTAPE_OPTIONS --test-runner-timeout $TEST_RUNNER_TIMEOUT" +fi + "$SCRIPT_DIR"/ducker-ignite test $TC_PATHS "$DUCKTAPE_OPTIONS" \ || die "ducker-ignite test failed" diff --git a/modules/ducktests/tests/ignitetest/services/mdc/mdc_cluster.py b/modules/ducktests/tests/ignitetest/services/mdc/mdc_cluster.py index 1f492b501b014..1afeef52e10bd 100644 --- a/modules/ducktests/tests/ignitetest/services/mdc/mdc_cluster.py +++ b/modules/ducktests/tests/ignitetest/services/mdc/mdc_cluster.py @@ -208,10 +208,8 @@ def describe(self) -> List[str]: """ lines = ["DATA CENTERS"] - for dc in self.dcs: - main = " (main)" if dc == self.main_dc and len(self.dcs) % 2 == 0 else "" - - lines.append(f" {dc}{main}") + for dc in DCS: + lines.append(f" {dc}") for label, services in (("server", [self.servers[dc]] if dc in self.servers else []), ("runner", self.runners.get(dc, [])), diff --git a/modules/ducktests/tests/ignitetest/utils/ignite_test.py b/modules/ducktests/tests/ignitetest/utils/ignite_test.py index 29c7bd25381b0..72444a8f9a9b9 100644 --- a/modules/ducktests/tests/ignitetest/utils/ignite_test.py +++ b/modules/ducktests/tests/ignitetest/utils/ignite_test.py @@ -69,6 +69,11 @@ def __init__(self, test_context): self.__demo_pause = None + # Stamped here rather than at the first breakpoint: it is what demo breakpoints count + # their elapsed time from, and what they measure the runner budget against, and both + # of those mean the start of the test - setup included. + self.__started_at = monotonic() + def pause(self, name, *describers): """ Holds the scenario at a named demo breakpoint until it is resumed from the host, so @@ -84,11 +89,25 @@ def pause(self, name, *describers): breakpoint reports. Any fixture a test drives can implement it. """ if self.__demo_pause is None: - self.__demo_pause = DemoPause(self.logger, self.test_context.globals, self.test_context.test_name) + self.__demo_pause = DemoPause(self.logger, self.test_context.globals, self.test_context.test_name, + started_at=self.__started_at, + runner_timeout_sec=self.__runner_timeout_sec()) # noinspection PyProtectedMember self.__demo_pause.pause(name, describers, self.test_context.services._services.values()) + def __runner_timeout_sec(self): + """ + :return: Ducktape's `--test-runner-timeout` in seconds, None when the session carries + none. A breakpoint has to give up inside it, since the runner kills a test + client it hears nothing from for that long - see + :meth:`ignitetest.utils.pause.DemoPause._budgeted_timeout`. + """ + session_context = getattr(self.test_context, "session_context", None) + timeout_ms = getattr(session_context, "test_runner_timeout", None) + + return timeout_ms / 1000 if timeout_ms else None + @property def available_cluster_size(self): # noinspection PyUnresolvedReferences diff --git a/modules/ducktests/tests/ignitetest/utils/pause.py b/modules/ducktests/tests/ignitetest/utils/pause.py index 6ed01e0c38bce..172fc77248c54 100644 --- a/modules/ducktests/tests/ignitetest/utils/pause.py +++ b/modules/ducktests/tests/ignitetest/utils/pause.py @@ -40,7 +40,8 @@ stops only at the named ones. demo_pause_timeout_sec - how long a single breakpoint may hold the scenario before it - resumes on its own, 1800 by default. + resumes on its own, 600 by default. Capped by what is left of ducktape's + ``--test-runner-timeout``, see :meth:`DemoPause._budgeted_timeout`. demo_pause_dir - control directory, ``/.ducktests-demo`` by default. """ @@ -54,7 +55,13 @@ DEMO_PAUSE_TIMEOUT_SEC = "demo_pause_timeout_sec" DEMO_PAUSE_DIR = "demo_pause_dir" -DEFAULT_TIMEOUT_SEC = 1800 +# Well below ducktape's own --test-runner-timeout (1800s), which a breakpoint must not +# outsit - see _budgeted_timeout(). +DEFAULT_TIMEOUT_SEC = 600 + +# Kept free of the runner budget, so that resuming a breakpoint at the very last moment +# still leaves the scenario time to reach its next event. +RUNNER_TIMEOUT_MARGIN_SEC = 60 CONTROL_DIR_NAME = ".ducktests-demo" @@ -68,9 +75,11 @@ # host, where inotify is not dependable. POLL_SEC = .5 -# A paused test is silent, and ducktape kills a test client that stays silent for too long -# (--test-runner-timeout). The heartbeat keeps the client alive and timestamps the demo in -# the test log. +# Timestamps the demo in the test log, so that a run can be read back afterwards and it is +# visible that the scenario is held rather than stuck. Deliberately NOT a keepalive towards +# ducktape: the test logger writes to files and to stdout only, while the runner listens for +# zmq events that just the runner client itself emits - _budgeted_timeout() is what keeps a +# paused test within the runner's patience. HEARTBEAT_SEC = 15 # Every breakpoint name matches. @@ -187,18 +196,32 @@ class DemoPause: Disabled unless the ``demo_pause`` global says otherwise, in which case :meth:`pause` is a plain return and nothing is written anywhere. """ - def __init__(self, logger, test_globals, test_name, control_dir=None): + def __init__(self, logger, test_globals, test_name, control_dir=None, started_at=None, + runner_timeout_sec=None): + """ + :param started_at: Monotonic timestamp the test itself started at, which is both what + the banner counts from and what the runner budget is spent from. Defaults to + now, which is only right when the first breakpoint is the start of the test. + :param runner_timeout_sec: Ducktape's ``--test-runner-timeout`` in seco nds, None when + unknown, in which case no breakpoint is cut short by it. + """ self.logger = logger self.test_name = test_name self.names = parse_selector(test_globals.get(DEMO_PAUSE)) self.timeout_sec = float(test_globals.get(DEMO_PAUSE_TIMEOUT_SEC, DEFAULT_TIMEOUT_SEC)) + self.runner_timeout_sec = runner_timeout_sec self.control_dir = control_dir or test_globals.get(DEMO_PAUSE_DIR) or default_control_dir() self.seq = 0 - self._started_at = time.monotonic() + # Identifies this run of this test to the host console, which has no other way of + # telling a breakpoint of the current run from one left published by a run that + # died while paused: seq alone restarts at 1 for every test. + self.run = f"{os.getpid()}-{int(time.time())}" + + self._started_at = time.monotonic() if started_at is None else started_at self._prepared = False self._continue_all = False @@ -225,13 +248,15 @@ def pause(self, name, describers=(), services=()): self.seq += 1 - banner = self._render(name, describers, services) + timeout_sec = self._budgeted_timeout() + + banner = self._render(name, describers, services, timeout_sec) - self._publish(name, banner) + self._publish(name, banner, timeout_sec) self.logger.info(f"Demo breakpoint reached [seq={self.seq}, name={name}, dir={self.control_dir}]") - self._await_resume(name) + self._await_resume(name, timeout_sec) def _stops_at(self, name): if not self.enabled: @@ -239,6 +264,40 @@ def _stops_at(self, name): return self.names == ALL or name in self.names + def _budgeted_timeout(self): + """ + :return: How long this breakpoint may actually hold the scenario. + + ``demo_pause_timeout_sec`` is what the demo asks for, the runner budget is what it is + allowed. Ducktape's runner kills a test client it has received no event from for + ``--test-runner-timeout`` and takes the whole session down with it, and a paused test + sends no events - so a breakpoint has to give up while the runner is still waiting. + The budget is spent from the start of the test rather than from the breakpoint, hence + a long setup, or a long earlier pause, leaves less of it for this one. + """ + if self.runner_timeout_sec is None: + return self.timeout_sec + + left = self.runner_timeout_sec - (time.monotonic() - self._started_at) - RUNNER_TIMEOUT_MARGIN_SEC + + if left >= self.timeout_sec: + return self.timeout_sec + + self.logger.warn(f"Demo breakpoint held for at most {_fmt_duration(max(left, 0))} instead of the requested " + f"{_fmt_duration(self.timeout_sec)}: what is left of ducktape's --test-runner-timeout " + f"({_fmt_duration(self.runner_timeout_sec)}) after {_fmt_duration(self.elapsed_sec)} of " + f"this test. Raise --test-runner-timeout for a longer demo " + f"[seq={self.seq}, test={self.test_name}]") + + return max(left, 0.0) + + @property + def elapsed_sec(self): + """ + :return: Seconds since the test started. + """ + return time.monotonic() - self._started_at + def _prepare(self): """ Creates the control directory and clears anything a previous run left behind: a @@ -256,12 +315,12 @@ def _prepare(self): self._prepared = True - def _render(self, name, describers, services): + def _render(self, name, describers, services, timeout_sec): """ :return: The banner as a list of lines. """ - elapsed = f" t+{_fmt_duration(time.monotonic() - self._started_at)} since test start" - auto = f"auto-continue in {_fmt_duration(self.timeout_sec)} " + elapsed = f" t+{_fmt_duration(self.elapsed_sec)} since test start" + auto = f"auto-continue in {_fmt_duration(timeout_sec)} " lines = [ "=" * _WIDTH, @@ -320,12 +379,24 @@ def _hints_section(services): log_dir = "/mnt/service/logs" config_file = "/mnt/service/config/ignite-config.xml" - for service in services: + # One set of copy-pasteable commands for a service list that is not homogeneous, so + # they follow the Ignite services: a ZookeeperService or a KafkaService registered + # ahead of them - which the discovery and CDC scenarios do - carries paths of its own + # and would have the banner name a zookeeper.properties for nodes that never had one. + # + # Imported here rather than at module level: docker/demo_console.py loads this module + # by path, on a host that has neither ducktape nor ignitetest installed. + from ignitetest.services.utils.path import IgnitePathAware # pylint: disable=import-outside-toplevel + + for service in [s for s in services if isinstance(s, IgnitePathAware)] or list(services): try: - log_dir = getattr(service, "log_dir", None) or log_dir - config_file = getattr(service, "config_file", None) or config_file + svc_log_dir = getattr(service, "log_dir", None) + svc_config_file = getattr(service, "config_file", None) except Exception: # pylint: disable=broad-except - pass + continue + + log_dir = svc_log_dir or log_dir + config_file = svc_config_file or config_file break @@ -342,7 +413,7 @@ def _hints_section(services): f" config docker exec cat {config_file}", ] - def _publish(self, name, banner): + def _publish(self, name, banner, timeout_sec): """ Publishes the breakpoint for the host, banner first as text and then as data: both files are replaced atomically so the console never reads a half written one. @@ -352,16 +423,17 @@ def _publish(self, name, banner): self._write(STATUS_TXT, text) self._write(STATUS_JSON, json.dumps({ + "run": self.run, "seq": self.seq, "name": name, "test": self.test_name, - "elapsed_sec": round(time.monotonic() - self._started_at, 1), - "timeout_sec": self.timeout_sec, + "elapsed_sec": round(self.elapsed_sec, 1), + "timeout_sec": timeout_sec, "banner": banner }, indent=2)) - def _await_resume(self, name): - deadline = time.monotonic() + self.timeout_sec + def _await_resume(self, name, timeout_sec): + deadline = time.monotonic() + timeout_sec heartbeat = time.monotonic() + HEARTBEAT_SEC while True: @@ -388,7 +460,7 @@ def _await_resume(self, name): if now >= deadline: self._consume() - self.logger.warn(f"Demo breakpoint timed out after {self.timeout_sec}s, resuming " + self.logger.warn(f"Demo breakpoint timed out after {timeout_sec}s, resuming " f"[seq={self.seq}, name={name}]") return @@ -397,7 +469,7 @@ def _await_resume(self, name): heartbeat = now + HEARTBEAT_SEC self.logger.info(f"Still paused at demo breakpoint [seq={self.seq}, name={name}, " - f"waiting={_fmt_duration(self.timeout_sec - (deadline - now))}]") + f"waiting={_fmt_duration(timeout_sec - (deadline - now))}]") time.sleep(POLL_SEC) From 83783550f2bfc3b44bfb2238624b35074753d08f Mon Sep 17 00:00:00 2001 From: Maksim Davydov Date: Wed, 12 Aug 2026 19:47:28 +0300 Subject: [PATCH 03/15] IGNITE-28976 [ducktests] Add optional demo breakpoints to pause a running scenario for inspection --- modules/ducktests/README.md | 11 +- .../tests/checks/utils/check_pause.py | 101 +++++++++++++++--- .../ducktests/tests/docker/demo_console.py | 25 +++-- .../ignitetest/services/mdc/mdc_cluster.py | 19 ++-- .../tests/ignitetest/utils/ignite_test.py | 3 +- .../ducktests/tests/ignitetest/utils/pause.py | 28 +++-- 6 files changed, 143 insertions(+), 44 deletions(-) diff --git a/modules/ducktests/README.md b/modules/ducktests/README.md index b0e1b4d5af72f..cf8537677ff53 100644 --- a/modules/ducktests/README.md +++ b/modules/ducktests/README.md @@ -242,8 +242,8 @@ Scenarios can be frozen at named breakpoints, so a cluster can be shown to an au Terminal 1 - run the test with the `demo_pause` global: ```bash -./docker/run_tests.sh -n 10 -gj '{"demo_pause": "*"}' \ - -t ./ignitetest/tests/mdc/majority_partition_test.py::MdcMajorityPartitionTest.test_minority_dc_isolation +./docker/run_tests.sh -n 12 -gj '{"demo_pause": "*"}' \ + -t ./ignitetest/tests/mdc/partition_resilience_test.py::MdcPartitionResilienceTest.test_mdc_cluster_partition_resilience ``` Terminal 2 - drive the breakpoints: @@ -272,13 +272,14 @@ Breakpoints are added to a test with `self.pause("name", mdc, net)` and cost not A held test reports nothing back to ducktape, which kills a session it has heard nothing from for `--test-runner-timeout` (30 minutes by default) - and that budget is spent by the whole test, setup included, not by the breakpoint alone. Breakpoints therefore auto-continue while the runner is still waiting, shortening themselves below `demo_pause_timeout_sec` when there is not enough of the budget left and saying so in the test log. For a demo that needs longer, raise the runner timeout too (milliseconds): ```bash -./docker/run_tests.sh --test-runner-timeout 7200000 -gj '{"demo_pause": "*", "demo_pause_timeout_sec": 1800}' \ - -t ./ignitetest/tests/mdc/majority_partition_test.py +./docker/run_tests.sh -n 12 --test-runner-timeout 7200000 \ + -gj '{"demo_pause": "*", "demo_pause_timeout_sec": 1800}' \ + -t ./ignitetest/tests/mdc/partition_resilience_test.py ``` | Global Parameter Key | Definition | Example Configuration | |---------------------|------------|----------------------| -| **demo_pause** | Which breakpoints stop the scenario. Absent or `false` disables them all (the default); `true` or `"*"` stops at every one; a list or comma separated string stops only at the named ones. | ```{"demo_pause": "split-brain,healed"}``` | +| **demo_pause** | Which breakpoints stop the scenario. Absent or `false` disables them all (the default); `true` or `"*"` stops at every one; a list or comma separated string stops only at the named ones, matched case insensitively. | ```{"demo_pause": "split-brain,healed"}``` | | **demo_pause_timeout_sec** | How long one breakpoint may hold the scenario before it resumes on its own. Default is 600, and it is capped by what is left of `--test-runner-timeout`. | ```{"demo_pause_timeout_sec": 1800}``` | | **demo_pause_dir** | Control directory shared with the host. Default is `/.ducktests-demo`. | ```{"demo_pause_dir": "/opt/ignite-dev/.demo"}``` | diff --git a/modules/ducktests/tests/checks/utils/check_pause.py b/modules/ducktests/tests/checks/utils/check_pause.py index 0e974398f1bcf..84edc0cb888a1 100644 --- a/modules/ducktests/tests/checks/utils/check_pause.py +++ b/modules/ducktests/tests/checks/utils/check_pause.py @@ -65,6 +65,10 @@ class FakeService: def __init__(self, *hostnames): self.nodes = _fake_nodes(*hostnames) + def who_am_i(self, node): + """Names the node the way a ducktape service does.""" + return f"{self.__class__.__name__}-{node.account.hostname}" + class FakeIgniteService(IgnitePathAware): """ @@ -73,6 +77,10 @@ class FakeIgniteService(IgnitePathAware): def __init__(self, *hostnames): self.nodes = _fake_nodes(*hostnames) + def who_am_i(self, node): + """Names the node the way a ducktape service does.""" + return f"{self.__class__.__name__}-{node.account.hostname}" + @property def product(self): return "ignite-dev" @@ -82,35 +90,56 @@ def globals(self): return {} +class FakeRegistry: + """ + Stands in for ducktape's ServiceRegistry, which is what a test hands the breakpoint: it is + iterable and nothing else, so a banner may not index it or ask it for a length. + """ + def __init__(self, *services): + self._services = services + + def __iter__(self): + return iter(self._services) + + def _pause(control_dir, started_at=None, runner_timeout_sec=None, **test_globals): return DemoPause(FakeLogger(), test_globals, "check.CheckPause.check_something", control_dir=str(control_dir), started_at=started_at, runner_timeout_sec=runner_timeout_sec) -def _read_published(control_dir, resume_with=None, delay_sec=.05): +def _read_published(control_dir, resume_with=None, timeout_sec=30): """ Reads the published breakpoint while the test blocks on it, the way the host console does, and optionally resumes it. - :return: The dict that is filled in once the breakpoint has been published. + Polls for the file rather than reading it once after a fixed delay: a breakpoint that is + only held for a fraction of a second - which is what these checks hold them for - would + otherwise be a race against the machine the checks happen to run on. + + :return: The dict that is filled in once the breakpoint has been published, and the reader + to join before reading it. """ published = {} def act(): - try: - with open(os.path.join(str(control_dir), STATUS_JSON), encoding="utf-8") as file: - published.update(json.load(file)) - except (OSError, ValueError): - pass + deadline = time.monotonic() + timeout_sec + + while time.monotonic() < deadline: + try: + with open(os.path.join(str(control_dir), STATUS_JSON), encoding="utf-8") as file: + published.update(json.load(file)) + + break + except (OSError, ValueError): + time.sleep(.01) if resume_with: open(os.path.join(str(control_dir), resume_with), "w").close() - timer = threading.Timer(delay_sec, act) - timer.daemon = True - timer.start() + reader = threading.Thread(target=act, daemon=True) + reader.start() - return published + return published, reader def _resume_with(control_dir, name, delay_sec=.05): @@ -139,6 +168,23 @@ def check_selector_parsing(): assert parse_selector("split-brain, healed ,") == {"split-brain", "healed"} assert parse_selector(["split-brain", "healed"]) == {"split-brain", "healed"} + # The global is typed by hand, the names live in the test source - the two meet case insensitively. + assert parse_selector("Split-Brain, HEALED") == {"split-brain", "healed"} + assert parse_selector(["Split-Brain"]) == {"split-brain"} + + +def check_names_are_matched_case_insensitively(tmp_path): + """ + Check that a breakpoint is found however the global spells it. + """ + demo = _pause(tmp_path, demo_pause="Split-Brain") + + _resume_with(tmp_path, continue_file(1)) + + demo.pause("split-brain") + + assert demo.seq == 1, "the global must not have to repeat the case of the name in the test" + def check_disabled_leaves_no_trace(tmp_path): """ @@ -182,10 +228,12 @@ def check_publishes_and_consumes_status(tmp_path): """ demo = _pause(tmp_path, demo_pause=True) - published = _read_published(tmp_path, resume_with=continue_file(1)) + published, reader = _read_published(tmp_path, resume_with=continue_file(1)) demo.pause("split-brain", services=[]) + reader.join(30) + assert published["seq"] == 1 assert published["run"] == demo.run assert published["name"] == "split-brain" @@ -288,10 +336,12 @@ def check_timeout_is_left_alone_within_the_runner_budget(tmp_path): """ demo = _pause(tmp_path, demo_pause=ALL, demo_pause_timeout_sec=.3, runner_timeout_sec=1800) - published = _read_published(tmp_path) + published, reader = _read_published(tmp_path) demo.pause("split-brain") + reader.join(30) + assert published["timeout_sec"] == .3 assert not any("--test-runner-timeout" in msg for msg in demo.logger.messages) @@ -304,14 +354,37 @@ def check_elapsed_is_counted_from_test_start(tmp_path): """ demo = _pause(tmp_path, started_at=time.monotonic() - 600, demo_pause=ALL, demo_pause_timeout_sec=.3) - published = _read_published(tmp_path) + published, reader = _read_published(tmp_path) demo.pause("split-brain") + reader.join(30) + assert published["elapsed_sec"] >= 600 assert any("t+10:00 since test start" in line for line in published["banner"]) +def check_banner_is_rendered_from_the_service_registry(tmp_path): + """ + Check that the banner is built by iterating the services alone: what a test passes is + ducktape's ServiceRegistry, which supports nothing else. + """ + demo = _pause(tmp_path, demo_pause=ALL, demo_pause_timeout_sec=.3) + + published, reader = _read_published(tmp_path) + + demo.pause("split-brain", services=FakeRegistry(FakeService("ducker02"), FakeIgniteService("ducker03"))) + + reader.join(30) + + banner = "\n".join(published["banner"]).replace("\\", "/") + + assert "FakeService-ducker02" in banner + assert "FakeIgniteService-ducker03" in banner + assert "/mnt/service/logs/ignite*.log" in banner, "the hints must still follow the Ignite service" + assert "ducker02 ducker03" in banner + + def check_hints_follow_the_ignite_services(): """ Check that the copy-pasteable commands name the Ignite paths even when a service of diff --git a/modules/ducktests/tests/docker/demo_console.py b/modules/ducktests/tests/docker/demo_console.py index 08dbf03989daf..4647f56ec0b58 100644 --- a/modules/ducktests/tests/docker/demo_console.py +++ b/modules/ducktests/tests/docker/demo_console.py @@ -82,12 +82,19 @@ def clear_stale(control_dir): Removes resume files left behind by an earlier run, which would otherwise skip the first breakpoint of this one. The test clears them too, on its side, at its first breakpoint. - The published breakpoint itself is left alone: a console is just as likely to be started - against a test that is already holding one, and a stale banner is told apart by its run - id anyway. + Only ever done while nothing is published: a console is just as likely to be started + against a test that is already holding a breakpoint, and a resume file that was written + for that one - by hand, or by a console that has just been closed - is the host's answer + to it rather than a leftover. The published breakpoint itself is left alone either way, + a stale banner is told apart by its run id. + + :return: Whether the sweep was performed. """ if not os.path.isdir(control_dir): - return + return True + + if read_status(control_dir) is not None: + return False for name in os.listdir(control_dir): if name.startswith(pause.CONTINUE_PREFIX) or name == pause.ABORT: @@ -96,6 +103,8 @@ def clear_stale(control_dir): except OSError: pass + return True + def resume(control_dir, name): """ @@ -157,10 +166,14 @@ def main(): args = parser.parse_args() control_dir = args.control_dir - clear_stale(control_dir) + swept = clear_stale(control_dir) print(f"Demo console, watching {control_dir}") - print("Waiting for the first breakpoint... (Ctrl-C to leave)") + + if swept: + print("Waiting for the first breakpoint... (Ctrl-C to leave)") + else: + print("A breakpoint is already held, joining it as it is (Ctrl-C to leave)") last_key, resumed_at = None, None diff --git a/modules/ducktests/tests/ignitetest/services/mdc/mdc_cluster.py b/modules/ducktests/tests/ignitetest/services/mdc/mdc_cluster.py index 1afeef52e10bd..a9ccbe4496d3c 100644 --- a/modules/ducktests/tests/ignitetest/services/mdc/mdc_cluster.py +++ b/modules/ducktests/tests/ignitetest/services/mdc/mdc_cluster.py @@ -209,16 +209,19 @@ def describe(self) -> List[str]: lines = ["DATA CENTERS"] for dc in DCS: - lines.append(f" {dc}") + roles = [(label, [node.account.hostname for svc in services for node in svc.nodes]) + for label, services in (("server", [self.servers[dc]] if dc in self.servers else []), + ("runner", self.runners.get(dc, [])), + ("loader", self.loaders.get(dc, [])), + ("extra", self.extras.get(dc, [])))] - for label, services in (("server", [self.servers[dc]] if dc in self.servers else []), - ("runner", self.runners.get(dc, [])), - ("loader", self.loaders.get(dc, [])), - ("extra", self.extras.get(dc, []))): - hosts = [node.account.hostname for svc in services for node in svc.nodes] + # A DC that holds nothing is not named at all: an empty header reads as a DC whose + # nodes have gone, which is exactly what a partition demo is being watched for. + if not any(hosts for _, hosts in roles): + continue - if hosts: - lines.append(f" {label:<7} {' '.join(hosts)}") + lines.append(f" {dc}") + lines.extend(f" {label:<7} {' '.join(hosts)}" for label, hosts in roles if hosts) return lines diff --git a/modules/ducktests/tests/ignitetest/utils/ignite_test.py b/modules/ducktests/tests/ignitetest/utils/ignite_test.py index 72444a8f9a9b9..82562ef5941bf 100644 --- a/modules/ducktests/tests/ignitetest/utils/ignite_test.py +++ b/modules/ducktests/tests/ignitetest/utils/ignite_test.py @@ -93,8 +93,7 @@ def pause(self, name, *describers): started_at=self.__started_at, runner_timeout_sec=self.__runner_timeout_sec()) - # noinspection PyProtectedMember - self.__demo_pause.pause(name, describers, self.test_context.services._services.values()) + self.__demo_pause.pause(name, describers, self.test_context.services) def __runner_timeout_sec(self): """ diff --git a/modules/ducktests/tests/ignitetest/utils/pause.py b/modules/ducktests/tests/ignitetest/utils/pause.py index 172fc77248c54..1134dee098e4f 100644 --- a/modules/ducktests/tests/ignitetest/utils/pause.py +++ b/modules/ducktests/tests/ignitetest/utils/pause.py @@ -22,14 +22,20 @@ mounts the whole Ignite repository into every container, so a control directory below the repository root is visible to the test and to the host at the same time. -The protocol over that directory is one sided on purpose - the test is the only party that -deletes anything, so no step can race with the host: +The protocol over that directory is one sided while a breakpoint is held - the host only +ever creates files, the test is the only party that deletes them, so no step of a held +breakpoint can race with the host: - the test publishes ``paused.txt`` (a rendered banner) and ``paused.json`` (the same content as data) and then blocks; - the host creates ``continue-``, ``continue-all`` or ``abort``; - the test consumes that file, removes it along with its own status files, and proceeds. +Between breakpoints both sides sweep the directory for files an earlier run left behind, +which would otherwise skip the next breakpoint: the test at its first one +(see :meth:`DemoPause._prepare`), the console at startup - and only while nothing is +published, so a resume file meant for a breakpoint that is currently held is never swept. + ``docker/demo_console.py`` is the host side of it, but nothing depends on it: reading ``paused.txt`` and touching ``continue-`` by hand works just as well. @@ -37,7 +43,7 @@ demo_pause - absent or false disables every breakpoint (the default, so tests are unaffected in CI); true or "*" stops at all of them; a list or a comma separated string - stops only at the named ones. + stops only at the named ones, matched case insensitively. demo_pause_timeout_sec - how long a single breakpoint may hold the scenario before it resumes on its own, 600 by default. Capped by what is left of ducktape's @@ -115,8 +121,11 @@ def parse_selector(value): """ Interprets the ``demo_pause`` global. + Names are matched case insensitively, both here and in :meth:`DemoPause._stops_at`: the + global is typed by hand next to a test whose breakpoint names are written in the source. + :return: None when demo pausing is disabled, :data:`ALL` to stop at every breakpoint, - or the set of breakpoint names to stop at. + or the set of breakpoint names to stop at, lower cased. """ if value is None or value is False: return None @@ -125,7 +134,7 @@ def parse_selector(value): return ALL if isinstance(value, (list, tuple, set, frozenset)): - names = {str(name).strip() for name in value} + names = {str(name).strip().lower() for name in value} names.discard("") return names or None @@ -139,7 +148,7 @@ def parse_selector(value): if text in (ALL, "all", "true", "yes", "on", "1"): return ALL - names = {name.strip() for name in value.split(",")} + names = {name.strip() for name in text.split(",")} names.discard("") return names or None @@ -202,7 +211,7 @@ def __init__(self, logger, test_globals, test_name, control_dir=None, started_at :param started_at: Monotonic timestamp the test itself started at, which is both what the banner counts from and what the runner budget is spent from. Defaults to now, which is only right when the first breakpoint is the start of the test. - :param runner_timeout_sec: Ducktape's ``--test-runner-timeout`` in seco nds, None when + :param runner_timeout_sec: Ducktape's ``--test-runner-timeout`` in seconds, None when unknown, in which case no breakpoint is cut short by it. """ self.logger = logger @@ -262,7 +271,7 @@ def _stops_at(self, name): if not self.enabled: return False - return self.names == ALL or name in self.names + return self.names == ALL or name.strip().lower() in self.names def _budgeted_timeout(self): """ @@ -469,7 +478,8 @@ def _await_resume(self, name, timeout_sec): heartbeat = now + HEARTBEAT_SEC self.logger.info(f"Still paused at demo breakpoint [seq={self.seq}, name={name}, " - f"waiting={_fmt_duration(timeout_sec - (deadline - now))}]") + f"held={_fmt_duration(timeout_sec - (deadline - now))}, " + f"left={_fmt_duration(deadline - now)}]") time.sleep(POLL_SEC) From 7832fd9a11cf20e72d079e9ec570019f1ec9b815 Mon Sep 17 00:00:00 2001 From: Maksim Davydov Date: Fri, 21 Aug 2026 13:26:06 +0300 Subject: [PATCH 04/15] IGNITE-28976 [ducktests] Split the demo breakpoint checks into focused files and shared helpers check_pause.py held four unrelated concerns plus a set of ducktape test doubles that are not pause specific at all. Split it by concern into check_pause_selector, check_pause_control, check_pause_timeout and check_pause_banner, and move the reusable parts out of checks/utils - which mirrors ignitetest/utils and is meant for check files alone - into checks/support, where any check can pick them up. checks/ carries no __init__.py on purpose, so setup.py's find_packages() keeps it out of the distribution; a conftest.py at the tests root is what puts that root on sys.path and lets checks.support resolve as a namespace package. Same checks, same assertions, no framework code touched. --- .../checks/support/demo_pause_control.py | 96 +++++ .../tests/checks/support/ducktape_doubles.py | 102 +++++ .../tests/checks/utils/check_pause.py | 407 ------------------ .../tests/checks/utils/check_pause_banner.py | 81 ++++ .../tests/checks/utils/check_pause_control.py | 100 +++++ .../checks/utils/check_pause_selector.py | 93 ++++ .../tests/checks/utils/check_pause_timeout.py | 71 +++ modules/ducktests/tests/conftest.py | 30 ++ 8 files changed, 573 insertions(+), 407 deletions(-) create mode 100644 modules/ducktests/tests/checks/support/demo_pause_control.py create mode 100644 modules/ducktests/tests/checks/support/ducktape_doubles.py delete mode 100644 modules/ducktests/tests/checks/utils/check_pause.py create mode 100644 modules/ducktests/tests/checks/utils/check_pause_banner.py create mode 100644 modules/ducktests/tests/checks/utils/check_pause_control.py create mode 100644 modules/ducktests/tests/checks/utils/check_pause_selector.py create mode 100644 modules/ducktests/tests/checks/utils/check_pause_timeout.py create mode 100644 modules/ducktests/tests/conftest.py diff --git a/modules/ducktests/tests/checks/support/demo_pause_control.py b/modules/ducktests/tests/checks/support/demo_pause_control.py new file mode 100644 index 0000000000000..1e26a5ed45259 --- /dev/null +++ b/modules/ducktests/tests/checks/support/demo_pause_control.py @@ -0,0 +1,96 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +""" +The host side of a demo breakpoint, for checks that drive one. + +A held breakpoint blocks the thread that reached it, so everything the host does - reading the +published banner, dropping a resume file - has to happen from another one while the check +itself sits inside :meth:`ignitetest.utils.pause.DemoPause.pause`. + +See :mod:`ignitetest.utils.pause` for the protocol these helpers speak. +""" + +import json +import os +import threading +import time +from contextlib import contextmanager + +from ignitetest.utils.pause import DemoPause, STATUS_JSON + +from checks.support.ducktape_doubles import FakeLogger + +# Stands for the test a breakpoint was reached in; breakpoints report it to the host. +TEST_NAME = "check.CheckPause.check_something" + + +def new_demo_pause(control_dir, started_at=None, runner_timeout_sec=None, **test_globals): + """ + :return: A DemoPause over the given control directory, logging into a FakeLogger its + ``logger`` attribute hands back to the check. + """ + return DemoPause(FakeLogger(), test_globals, TEST_NAME, control_dir=str(control_dir), + started_at=started_at, runner_timeout_sec=runner_timeout_sec) + + +def resume_with(control_dir, name, delay_sec=.05): + """ + Creates a resume file from another thread, the way the host does while the test blocks. + """ + timer = threading.Timer(delay_sec, lambda: open(os.path.join(str(control_dir), name), "w").close()) + timer.daemon = True + timer.start() + + +@contextmanager +def published_status(control_dir, resume=None, timeout_sec=30): + """ + Reads the published breakpoint while the check blocks on it, the way the host console + does, and optionally resumes it. + + Polls for the file rather than reading it once after a fixed delay: a breakpoint that is + only held for a fraction of a second - which is what these checks hold them for - would + otherwise be a race against the machine the checks happen to run on. + + :param resume: Name of the resume file to create once the breakpoint has been read, None + to leave it held. + :return: A dict, empty on entry and filled with the published breakpoint by the time the + block is left. + """ + published = {} + + def read(): + deadline = time.monotonic() + timeout_sec + + while time.monotonic() < deadline: + try: + with open(os.path.join(str(control_dir), STATUS_JSON), encoding="utf-8") as file: + published.update(json.load(file)) + + break + except (OSError, ValueError): + time.sleep(.01) + + if resume: + open(os.path.join(str(control_dir), resume), "w").close() + + reader = threading.Thread(target=read, daemon=True) + reader.start() + + try: + yield published + finally: + reader.join(timeout_sec) diff --git a/modules/ducktests/tests/checks/support/ducktape_doubles.py b/modules/ducktests/tests/checks/support/ducktape_doubles.py new file mode 100644 index 0000000000000..0d06221521783 --- /dev/null +++ b/modules/ducktests/tests/checks/support/ducktape_doubles.py @@ -0,0 +1,102 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +""" +Stand-ins for the ducktape objects a test is handed - a logger, nodes, services and the +registry they are collected in - for checks of framework code that only reads them. + +Each double is as poor as the real thing is at the point of use, so that a check fails on +code reaching for more than a test actually offers it. +""" + +from types import SimpleNamespace + +from ignitetest.services.utils.path import IgnitePathAware + + +class FakeLogger: + """ + Collects what the code under check would have logged. + """ + def __init__(self): + self.messages = [] + + def info(self, msg): + """Records an info message.""" + self.messages.append(msg) + + def warn(self, msg): + """Records a warning.""" + self.messages.append(msg) + + debug = info + error = warn + + +def fake_nodes(*hostnames): + """ + :return: Nodes carrying the account attributes that ducktape's do. + """ + return [SimpleNamespace(account=SimpleNamespace(hostname=host, externally_routable_ip=host)) + for host in hostnames] + + +class FakeService: + """ + Stands in for a non-Ignite service of the test registry, e.g. a zookeeper one: it carries + paths of its own, which code following the Ignite services must not hand out for Ignite + nodes. + """ + log_dir = "/mnt/service/zk-logs" + config_file = "/mnt/service/zookeeper.properties" + + def __init__(self, *hostnames): + self.nodes = fake_nodes(*hostnames) + + def who_am_i(self, node): + """Names the node the way a ducktape service does.""" + return f"{self.__class__.__name__}-{node.account.hostname}" + + +class FakeIgniteService(IgnitePathAware): + """ + Stands in for an Ignite service, with the real path layout behind it. + """ + def __init__(self, *hostnames): + self.nodes = fake_nodes(*hostnames) + + def who_am_i(self, node): + """Names the node the way a ducktape service does.""" + return f"{self.__class__.__name__}-{node.account.hostname}" + + @property + def product(self): + return "ignite-dev" + + @property + def globals(self): + return {} + + +class FakeRegistry: + """ + Stands in for ducktape's ServiceRegistry, which is what a test hands to the framework: it + is iterable and nothing else, so code reading it may not index it or ask it for a length. + """ + def __init__(self, *services): + self._services = services + + def __iter__(self): + return iter(self._services) diff --git a/modules/ducktests/tests/checks/utils/check_pause.py b/modules/ducktests/tests/checks/utils/check_pause.py deleted file mode 100644 index 84edc0cb888a1..0000000000000 --- a/modules/ducktests/tests/checks/utils/check_pause.py +++ /dev/null @@ -1,407 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one or more -# contributor license agreements. See the NOTICE file distributed with -# this work for additional information regarding copyright ownership. -# The ASF licenses this file to You under the Apache License, Version 2.0 -# (the "License"); you may not use this file except in compliance with -# the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -""" -Checks demo breakpoints. -""" - -import json -import os -import threading -import time -from types import SimpleNamespace - -import pytest - -from ignitetest.services.utils.path import IgnitePathAware -from ignitetest.utils.pause import ALL, CONTINUE_ALL, ABORT, DemoPause, RUNNER_TIMEOUT_MARGIN_SEC, STATUS_JSON, \ - STATUS_TXT, continue_file, parse_selector - - -class FakeLogger: - """ - Collects what a paused test would have logged. - """ - def __init__(self): - self.messages = [] - - def info(self, msg): - """Records an info message.""" - self.messages.append(msg) - - def warn(self, msg): - """Records a warning.""" - self.messages.append(msg) - - debug = info - error = warn - - -def _fake_nodes(*hostnames): - return [SimpleNamespace(account=SimpleNamespace(hostname=host, externally_routable_ip=host)) - for host in hostnames] - - -class FakeService: - """ - Stands in for a non-Ignite service of the test registry, e.g. a zookeeper one: it carries - paths of its own, which the banner must not hand out for Ignite nodes. - """ - log_dir = "/mnt/service/zk-logs" - config_file = "/mnt/service/zookeeper.properties" - - def __init__(self, *hostnames): - self.nodes = _fake_nodes(*hostnames) - - def who_am_i(self, node): - """Names the node the way a ducktape service does.""" - return f"{self.__class__.__name__}-{node.account.hostname}" - - -class FakeIgniteService(IgnitePathAware): - """ - Stands in for an Ignite service, with the real path layout behind it. - """ - def __init__(self, *hostnames): - self.nodes = _fake_nodes(*hostnames) - - def who_am_i(self, node): - """Names the node the way a ducktape service does.""" - return f"{self.__class__.__name__}-{node.account.hostname}" - - @property - def product(self): - return "ignite-dev" - - @property - def globals(self): - return {} - - -class FakeRegistry: - """ - Stands in for ducktape's ServiceRegistry, which is what a test hands the breakpoint: it is - iterable and nothing else, so a banner may not index it or ask it for a length. - """ - def __init__(self, *services): - self._services = services - - def __iter__(self): - return iter(self._services) - - -def _pause(control_dir, started_at=None, runner_timeout_sec=None, **test_globals): - return DemoPause(FakeLogger(), test_globals, "check.CheckPause.check_something", control_dir=str(control_dir), - started_at=started_at, runner_timeout_sec=runner_timeout_sec) - - -def _read_published(control_dir, resume_with=None, timeout_sec=30): - """ - Reads the published breakpoint while the test blocks on it, the way the host console does, - and optionally resumes it. - - Polls for the file rather than reading it once after a fixed delay: a breakpoint that is - only held for a fraction of a second - which is what these checks hold them for - would - otherwise be a race against the machine the checks happen to run on. - - :return: The dict that is filled in once the breakpoint has been published, and the reader - to join before reading it. - """ - published = {} - - def act(): - deadline = time.monotonic() + timeout_sec - - while time.monotonic() < deadline: - try: - with open(os.path.join(str(control_dir), STATUS_JSON), encoding="utf-8") as file: - published.update(json.load(file)) - - break - except (OSError, ValueError): - time.sleep(.01) - - if resume_with: - open(os.path.join(str(control_dir), resume_with), "w").close() - - reader = threading.Thread(target=act, daemon=True) - reader.start() - - return published, reader - - -def _resume_with(control_dir, name, delay_sec=.05): - """ - Creates a resume file from another thread, the way the host does while the test blocks. - """ - timer = threading.Timer(delay_sec, lambda: open(os.path.join(str(control_dir), name), "w").close()) - timer.daemon = True - timer.start() - - return timer - - -def check_selector_parsing(): - """ - Check that every shape the demo_pause global can arrive in is understood: -g passes it as - a string, -gj as whatever the json holds. - """ - for disabled in (None, False, "", "false", "off", "0", [], " "): - assert parse_selector(disabled) is None, disabled - - for every in (True, "*", "all", "true", "ON", "1"): - assert parse_selector(every) == ALL, every - - assert parse_selector("split-brain") == {"split-brain"} - assert parse_selector("split-brain, healed ,") == {"split-brain", "healed"} - assert parse_selector(["split-brain", "healed"]) == {"split-brain", "healed"} - - # The global is typed by hand, the names live in the test source - the two meet case insensitively. - assert parse_selector("Split-Brain, HEALED") == {"split-brain", "healed"} - assert parse_selector(["Split-Brain"]) == {"split-brain"} - - -def check_names_are_matched_case_insensitively(tmp_path): - """ - Check that a breakpoint is found however the global spells it. - """ - demo = _pause(tmp_path, demo_pause="Split-Brain") - - _resume_with(tmp_path, continue_file(1)) - - demo.pause("split-brain") - - assert demo.seq == 1, "the global must not have to repeat the case of the name in the test" - - -def check_disabled_leaves_no_trace(tmp_path): - """ - Check that without the global a breakpoint is a plain return: it must not block, and it - must not even create the control directory, since every test carries breakpoints in CI. - """ - control_dir = tmp_path / "control" - - demo = _pause(control_dir) - - assert not demo.enabled - - demo.pause("split-brain") - - assert not os.path.exists(str(control_dir)) - assert demo.seq == 0 - - -def check_selected_breakpoints_only(tmp_path): - """ - Check that only the named breakpoints stop the scenario. - """ - demo = _pause(tmp_path, demo_pause="split-brain") - - demo.pause("cluster-up") - demo.pause("healed") - - assert demo.seq == 0, "an unnamed breakpoint must not stop the scenario" - - _resume_with(tmp_path, continue_file(1)) - - demo.pause("split-brain") - - assert demo.seq == 1 - - -def check_publishes_and_consumes_status(tmp_path): - """ - Check the published breakpoint - what the host reads - and that the test cleans it up - once resumed, so a stale banner never outlives the pause it describes. - """ - demo = _pause(tmp_path, demo_pause=True) - - published, reader = _read_published(tmp_path, resume_with=continue_file(1)) - - demo.pause("split-brain", services=[]) - - reader.join(30) - - assert published["seq"] == 1 - assert published["run"] == demo.run - assert published["name"] == "split-brain" - assert published["test"] == "check.CheckPause.check_something" - assert any("PAUSED 1 split-brain" in line for line in published["banner"]) - - for leftover in (STATUS_JSON, STATUS_TXT, continue_file(1)): - assert not os.path.exists(str(tmp_path / leftover)), leftover - - -def check_continue_all_skips_the_rest(tmp_path): - """ - Check that continue-all resumes the current breakpoint and disables every later one, so - a demo can be cut short without restarting the scenario. - """ - demo = _pause(tmp_path, demo_pause=ALL) - - _resume_with(tmp_path, CONTINUE_ALL) - - demo.pause("split-brain") - - assert demo.seq == 1 - assert not demo.enabled - - demo.pause("healed") - - assert demo.seq == 1, "breakpoints after continue-all must not stop the scenario" - assert not os.path.exists(str(tmp_path / CONTINUE_ALL)) - - -def check_abort_fails_the_test(tmp_path): - """ - Check that abort ends the scenario through an assertion, so ducktape tears the cluster - down instead of leaving it running. - """ - demo = _pause(tmp_path, demo_pause=ALL) - - _resume_with(tmp_path, ABORT) - - with pytest.raises(AssertionError, match="split-brain"): - demo.pause("split-brain") - - assert not os.path.exists(str(tmp_path / ABORT)) - assert not os.path.exists(str(tmp_path / STATUS_JSON)) - - -def check_stale_resume_file_is_cleared(tmp_path): - """ - Check that a resume file left by a previous run does not skip the first breakpoint of - this one - the control directory outlives a test, its contents must not. - """ - open(str(tmp_path / continue_file(1)), "w").close() - open(str(tmp_path / STATUS_TXT), "w").close() - - demo = _pause(tmp_path, demo_pause=ALL, demo_pause_timeout_sec=.3) - - demo.pause("split-brain") - - assert demo.seq == 1 - assert any("timed out" in msg for msg in demo.logger.messages), \ - "the stale file must have been cleared, leaving the breakpoint to time out" - - -def check_timeout_resumes_on_its_own(tmp_path): - """ - Check that a forgotten breakpoint gives up rather than holding the scenario until - ducktape kills it. - """ - demo = _pause(tmp_path, demo_pause=ALL, demo_pause_timeout_sec=.3) - - demo.pause("split-brain") - - assert demo.seq == 1 - assert demo.enabled, "a timed out breakpoint must not disable the later ones" - assert not os.path.exists(str(tmp_path / STATUS_JSON)) - - -def check_timeout_stays_within_the_runner_budget(tmp_path): - """ - Check that a breakpoint gives up while ducktape's runner is still waiting: it hears - nothing from a paused test, and killing the client takes the whole session down instead - of just cutting the demo short. The requested timeout only ever shrinks. - """ - demo = _pause(tmp_path, demo_pause=ALL, demo_pause_timeout_sec=3600, - runner_timeout_sec=RUNNER_TIMEOUT_MARGIN_SEC + .3) - - demo.pause("split-brain") - - assert demo.seq == 1 - assert any("timed out" in msg for msg in demo.logger.messages), \ - "the breakpoint must not outsit the runner budget it was given" - assert any("--test-runner-timeout" in msg for msg in demo.logger.messages), \ - "shortening a breakpoint must say what to raise to keep it" - - -def check_timeout_is_left_alone_within_the_runner_budget(tmp_path): - """ - Check that the budget only ever caps the requested timeout - a demo that fits must be - held for exactly as long as it asked for. - """ - demo = _pause(tmp_path, demo_pause=ALL, demo_pause_timeout_sec=.3, runner_timeout_sec=1800) - - published, reader = _read_published(tmp_path) - - demo.pause("split-brain") - - reader.join(30) - - assert published["timeout_sec"] == .3 - assert not any("--test-runner-timeout" in msg for msg in demo.logger.messages) - - -def check_elapsed_is_counted_from_test_start(tmp_path): - """ - Check that the banner counts from the start of the test rather than from the first - breakpoint: the setup phase of a multi-node scenario is minutes long, and a demo that - reports t+00:00 after it hides exactly the part worth showing. - """ - demo = _pause(tmp_path, started_at=time.monotonic() - 600, demo_pause=ALL, demo_pause_timeout_sec=.3) - - published, reader = _read_published(tmp_path) - - demo.pause("split-brain") - - reader.join(30) - - assert published["elapsed_sec"] >= 600 - assert any("t+10:00 since test start" in line for line in published["banner"]) - - -def check_banner_is_rendered_from_the_service_registry(tmp_path): - """ - Check that the banner is built by iterating the services alone: what a test passes is - ducktape's ServiceRegistry, which supports nothing else. - """ - demo = _pause(tmp_path, demo_pause=ALL, demo_pause_timeout_sec=.3) - - published, reader = _read_published(tmp_path) - - demo.pause("split-brain", services=FakeRegistry(FakeService("ducker02"), FakeIgniteService("ducker03"))) - - reader.join(30) - - banner = "\n".join(published["banner"]).replace("\\", "/") - - assert "FakeService-ducker02" in banner - assert "FakeIgniteService-ducker03" in banner - assert "/mnt/service/logs/ignite*.log" in banner, "the hints must still follow the Ignite service" - assert "ducker02 ducker03" in banner - - -def check_hints_follow_the_ignite_services(): - """ - Check that the copy-pasteable commands name the Ignite paths even when a service of - another kind was registered first, as the zookeeper discovery scenarios do. - """ - # noinspection PyProtectedMember - hints = "\n".join(DemoPause._hints_section([FakeService("ducker02"), # pylint: disable=protected-access - FakeIgniteService("ducker03")])) - - # The service paths come from os.path.join, which follows the control machine rather than - # the nodes - a check that runs on Windows would otherwise see its separators. - hints = hints.replace("\\", "/") - - assert "/mnt/service/config/ignite-config.xml" in hints - assert "/mnt/service/logs/ignite*.log" in hints - assert "zookeeper.properties" not in hints - assert "zk-logs" not in hints - - # Every node is still offered, whichever service it belongs to. - assert "ducker02 ducker03" in hints diff --git a/modules/ducktests/tests/checks/utils/check_pause_banner.py b/modules/ducktests/tests/checks/utils/check_pause_banner.py new file mode 100644 index 0000000000000..91df9118fefa8 --- /dev/null +++ b/modules/ducktests/tests/checks/utils/check_pause_banner.py @@ -0,0 +1,81 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +""" +Checks the banner a held demo breakpoint renders: how long the scenario has been running, the +nodes it is made of, and the commands offered for looking into them. +""" + +import time + +from ignitetest.utils.pause import ALL, DemoPause + +from checks.support.demo_pause_control import new_demo_pause, published_status +from checks.support.ducktape_doubles import FakeIgniteService, FakeRegistry, FakeService + + +def check_elapsed_is_counted_from_test_start(tmp_path): + """ + Check that the banner counts from the start of the test rather than from the first + breakpoint: the setup phase of a multi-node scenario is minutes long, and a demo that + reports t+00:00 after it hides exactly the part worth showing. + """ + demo = new_demo_pause(tmp_path, started_at=time.monotonic() - 600, demo_pause=ALL, demo_pause_timeout_sec=.3) + + with published_status(tmp_path) as published: + demo.pause("split-brain") + + assert published["elapsed_sec"] >= 600 + assert any("t+10:00 since test start" in line for line in published["banner"]) + + +def check_banner_is_rendered_from_the_service_registry(tmp_path): + """ + Check that the banner is built by iterating the services alone: what a test passes is + ducktape's ServiceRegistry, which supports nothing else. + """ + demo = new_demo_pause(tmp_path, demo_pause=ALL, demo_pause_timeout_sec=.3) + + with published_status(tmp_path) as published: + demo.pause("split-brain", services=FakeRegistry(FakeService("ducker02"), FakeIgniteService("ducker03"))) + + banner = "\n".join(published["banner"]).replace("\\", "/") + + assert "FakeService-ducker02" in banner + assert "FakeIgniteService-ducker03" in banner + assert "/mnt/service/logs/ignite*.log" in banner, "the hints must still follow the Ignite service" + assert "ducker02 ducker03" in banner + + +def check_hints_follow_the_ignite_services(): + """ + Check that the copy-pasteable commands name the Ignite paths even when a service of + another kind was registered first, as the zookeeper discovery scenarios do. + """ + # noinspection PyProtectedMember + hints = "\n".join(DemoPause._hints_section([FakeService("ducker02"), # pylint: disable=protected-access + FakeIgniteService("ducker03")])) + + # The service paths come from os.path.join, which follows the control machine rather than + # the nodes - a check that runs on Windows would otherwise see its separators. + hints = hints.replace("\\", "/") + + assert "/mnt/service/config/ignite-config.xml" in hints + assert "/mnt/service/logs/ignite*.log" in hints + assert "zookeeper.properties" not in hints + assert "zk-logs" not in hints + + # Every node is still offered, whichever service it belongs to. + assert "ducker02 ducker03" in hints diff --git a/modules/ducktests/tests/checks/utils/check_pause_control.py b/modules/ducktests/tests/checks/utils/check_pause_control.py new file mode 100644 index 0000000000000..6592ed16213b9 --- /dev/null +++ b/modules/ducktests/tests/checks/utils/check_pause_control.py @@ -0,0 +1,100 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +""" +Checks the file protocol a demo breakpoint and the host speak over the control directory: +what a held breakpoint publishes, what resumes it, and what is swept up afterwards. +""" + +import os + +import pytest + +from ignitetest.utils.pause import ABORT, ALL, CONTINUE_ALL, STATUS_JSON, STATUS_TXT, continue_file + +from checks.support.demo_pause_control import TEST_NAME, new_demo_pause, published_status, resume_with + + +def check_publishes_and_consumes_status(tmp_path): + """ + Check the published breakpoint - what the host reads - and that the test cleans it up + once resumed, so a stale banner never outlives the pause it describes. + """ + demo = new_demo_pause(tmp_path, demo_pause=True) + + with published_status(tmp_path, resume=continue_file(1)) as published: + demo.pause("split-brain", services=[]) + + assert published["seq"] == 1 + assert published["run"] == demo.run + assert published["name"] == "split-brain" + assert published["test"] == TEST_NAME + assert any("PAUSED 1 split-brain" in line for line in published["banner"]) + + for leftover in (STATUS_JSON, STATUS_TXT, continue_file(1)): + assert not os.path.exists(str(tmp_path / leftover)), leftover + + +def check_continue_all_skips_the_rest(tmp_path): + """ + Check that continue-all resumes the current breakpoint and disables every later one, so + a demo can be cut short without restarting the scenario. + """ + demo = new_demo_pause(tmp_path, demo_pause=ALL) + + resume_with(tmp_path, CONTINUE_ALL) + + demo.pause("split-brain") + + assert demo.seq == 1 + assert not demo.enabled + + demo.pause("healed") + + assert demo.seq == 1, "breakpoints after continue-all must not stop the scenario" + assert not os.path.exists(str(tmp_path / CONTINUE_ALL)) + + +def check_abort_fails_the_test(tmp_path): + """ + Check that abort ends the scenario through an assertion, so ducktape tears the cluster + down instead of leaving it running. + """ + demo = new_demo_pause(tmp_path, demo_pause=ALL) + + resume_with(tmp_path, ABORT) + + with pytest.raises(AssertionError, match="split-brain"): + demo.pause("split-brain") + + assert not os.path.exists(str(tmp_path / ABORT)) + assert not os.path.exists(str(tmp_path / STATUS_JSON)) + + +def check_stale_resume_file_is_cleared(tmp_path): + """ + Check that a resume file left by a previous run does not skip the first breakpoint of + this one - the control directory outlives a test, its contents must not. + """ + open(str(tmp_path / continue_file(1)), "w").close() + open(str(tmp_path / STATUS_TXT), "w").close() + + demo = new_demo_pause(tmp_path, demo_pause=ALL, demo_pause_timeout_sec=.3) + + demo.pause("split-brain") + + assert demo.seq == 1 + assert any("timed out" in msg for msg in demo.logger.messages), \ + "the stale file must have been cleared, leaving the breakpoint to time out" diff --git a/modules/ducktests/tests/checks/utils/check_pause_selector.py b/modules/ducktests/tests/checks/utils/check_pause_selector.py new file mode 100644 index 0000000000000..18860261584bc --- /dev/null +++ b/modules/ducktests/tests/checks/utils/check_pause_selector.py @@ -0,0 +1,93 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +""" +Checks which demo breakpoints stop a scenario: how the ``demo_pause`` global is read, and +which of the breakpoints in the test it then selects. +""" + +import os + +from ignitetest.utils.pause import ALL, continue_file, parse_selector + +from checks.support.demo_pause_control import new_demo_pause, resume_with + + +def check_selector_parsing(): + """ + Check that every shape the demo_pause global can arrive in is understood: -g passes it as + a string, -gj as whatever the json holds. + """ + for disabled in (None, False, "", "false", "off", "0", [], " "): + assert parse_selector(disabled) is None, disabled + + for every in (True, "*", "all", "true", "ON", "1"): + assert parse_selector(every) == ALL, every + + assert parse_selector("split-brain") == {"split-brain"} + assert parse_selector("split-brain, healed ,") == {"split-brain", "healed"} + assert parse_selector(["split-brain", "healed"]) == {"split-brain", "healed"} + + # The global is typed by hand, the names live in the test source - the two meet case insensitively. + assert parse_selector("Split-Brain, HEALED") == {"split-brain", "healed"} + assert parse_selector(["Split-Brain"]) == {"split-brain"} + + +def check_names_are_matched_case_insensitively(tmp_path): + """ + Check that a breakpoint is found however the global spells it. + """ + demo = new_demo_pause(tmp_path, demo_pause="Split-Brain") + + resume_with(tmp_path, continue_file(1)) + + demo.pause("split-brain") + + assert demo.seq == 1, "the global must not have to repeat the case of the name in the test" + + +def check_disabled_leaves_no_trace(tmp_path): + """ + Check that without the global a breakpoint is a plain return: it must not block, and it + must not even create the control directory, since every test carries breakpoints in CI. + """ + control_dir = tmp_path / "control" + + demo = new_demo_pause(control_dir) + + assert not demo.enabled + + demo.pause("split-brain") + + assert not os.path.exists(str(control_dir)) + assert demo.seq == 0 + + +def check_selected_breakpoints_only(tmp_path): + """ + Check that only the named breakpoints stop the scenario. + """ + demo = new_demo_pause(tmp_path, demo_pause="split-brain") + + demo.pause("cluster-up") + demo.pause("healed") + + assert demo.seq == 0, "an unnamed breakpoint must not stop the scenario" + + resume_with(tmp_path, continue_file(1)) + + demo.pause("split-brain") + + assert demo.seq == 1 diff --git a/modules/ducktests/tests/checks/utils/check_pause_timeout.py b/modules/ducktests/tests/checks/utils/check_pause_timeout.py new file mode 100644 index 0000000000000..211f0f33d143e --- /dev/null +++ b/modules/ducktests/tests/checks/utils/check_pause_timeout.py @@ -0,0 +1,71 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +""" +Checks how long a demo breakpoint may hold a scenario: the timeout the demo asks for, and +ducktape's --test-runner-timeout budget that only ever shortens it. +""" + +import os + +from ignitetest.utils.pause import ALL, RUNNER_TIMEOUT_MARGIN_SEC, STATUS_JSON + +from checks.support.demo_pause_control import new_demo_pause, published_status + + +def check_timeout_resumes_on_its_own(tmp_path): + """ + Check that a forgotten breakpoint gives up rather than holding the scenario until + ducktape kills it. + """ + demo = new_demo_pause(tmp_path, demo_pause=ALL, demo_pause_timeout_sec=.3) + + demo.pause("split-brain") + + assert demo.seq == 1 + assert demo.enabled, "a timed out breakpoint must not disable the later ones" + assert not os.path.exists(str(tmp_path / STATUS_JSON)) + + +def check_timeout_stays_within_the_runner_budget(tmp_path): + """ + Check that a breakpoint gives up while ducktape's runner is still waiting: it hears + nothing from a paused test, and killing the client takes the whole session down instead + of just cutting the demo short. The requested timeout only ever shrinks. + """ + demo = new_demo_pause(tmp_path, demo_pause=ALL, demo_pause_timeout_sec=3600, + runner_timeout_sec=RUNNER_TIMEOUT_MARGIN_SEC + .3) + + demo.pause("split-brain") + + assert demo.seq == 1 + assert any("timed out" in msg for msg in demo.logger.messages), \ + "the breakpoint must not outsit the runner budget it was given" + assert any("--test-runner-timeout" in msg for msg in demo.logger.messages), \ + "shortening a breakpoint must say what to raise to keep it" + + +def check_timeout_is_left_alone_within_the_runner_budget(tmp_path): + """ + Check that the budget only ever caps the requested timeout - a demo that fits must be + held for exactly as long as it asked for. + """ + demo = new_demo_pause(tmp_path, demo_pause=ALL, demo_pause_timeout_sec=.3, runner_timeout_sec=1800) + + with published_status(tmp_path) as published: + demo.pause("split-brain") + + assert published["timeout_sec"] == .3 + assert not any("--test-runner-timeout" in msg for msg in demo.logger.messages) diff --git a/modules/ducktests/tests/conftest.py b/modules/ducktests/tests/conftest.py new file mode 100644 index 0000000000000..c928ed7b5397a --- /dev/null +++ b/modules/ducktests/tests/conftest.py @@ -0,0 +1,30 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +""" +Deliberately empty - pytest imports this file for its side effect alone. + +The framework checks live in ``checks/``, whose helper modules sit apart from the check files +themselves (``checks/support/``), and pytest on its own puts only each check file's own +directory on ``sys.path`` - never this one, which is what ``checks.support`` has to be reached +through. + +Importing a conftest is what adds its directory, so this file is what makes +``from checks.support... import ...`` resolve, as a namespace package. Nothing below ``checks/`` +carries an ``__init__.py`` on purpose: ``setup.py`` collects the distribution with +``find_packages()``, which only finds directories that have one, so the checks and their +helpers stay out of the installed ``ignitetest`` package without ``setup.py`` having to name +them. +""" From 660f954dee31d69fe8bf5281a54af3d57b993c4a Mon Sep 17 00:00:00 2001 From: Maksim Davydov Date: Fri, 21 Aug 2026 13:36:08 +0300 Subject: [PATCH 05/15] IGNITE-28976 [ducktests] Group the demo breakpoint checks under checks/utils/pause Four of the thirteen files in checks/utils were check_pause_*, for a feature whose source is a single ignitetest/utils/pause.py. A pause/ directory mirrors that module and gives the directory back its one-file-per-source reading; the names drop the prefix the directory now carries. Plain rename - no content changes, and no __init__.py, so collection works the way it does everywhere else under checks/. --- .../checks/utils/{check_pause_banner.py => pause/check_banner.py} | 0 .../utils/{check_pause_control.py => pause/check_control.py} | 0 .../utils/{check_pause_selector.py => pause/check_selector.py} | 0 .../utils/{check_pause_timeout.py => pause/check_timeout.py} | 0 4 files changed, 0 insertions(+), 0 deletions(-) rename modules/ducktests/tests/checks/utils/{check_pause_banner.py => pause/check_banner.py} (100%) rename modules/ducktests/tests/checks/utils/{check_pause_control.py => pause/check_control.py} (100%) rename modules/ducktests/tests/checks/utils/{check_pause_selector.py => pause/check_selector.py} (100%) rename modules/ducktests/tests/checks/utils/{check_pause_timeout.py => pause/check_timeout.py} (100%) diff --git a/modules/ducktests/tests/checks/utils/check_pause_banner.py b/modules/ducktests/tests/checks/utils/pause/check_banner.py similarity index 100% rename from modules/ducktests/tests/checks/utils/check_pause_banner.py rename to modules/ducktests/tests/checks/utils/pause/check_banner.py diff --git a/modules/ducktests/tests/checks/utils/check_pause_control.py b/modules/ducktests/tests/checks/utils/pause/check_control.py similarity index 100% rename from modules/ducktests/tests/checks/utils/check_pause_control.py rename to modules/ducktests/tests/checks/utils/pause/check_control.py diff --git a/modules/ducktests/tests/checks/utils/check_pause_selector.py b/modules/ducktests/tests/checks/utils/pause/check_selector.py similarity index 100% rename from modules/ducktests/tests/checks/utils/check_pause_selector.py rename to modules/ducktests/tests/checks/utils/pause/check_selector.py diff --git a/modules/ducktests/tests/checks/utils/check_pause_timeout.py b/modules/ducktests/tests/checks/utils/pause/check_timeout.py similarity index 100% rename from modules/ducktests/tests/checks/utils/check_pause_timeout.py rename to modules/ducktests/tests/checks/utils/pause/check_timeout.py From 1be89ea7c2e23e64da2abb2b361dde21ece5dfb5 Mon Sep 17 00:00:00 2001 From: Maksim Davydov Date: Fri, 21 Aug 2026 16:30:22 +0300 Subject: [PATCH 06/15] IGNITE-28976 [ducktests] Make the demo breakpoint checks runnable under tox The pause checks are the first ones to use pytest's tmp_path, which builds its root below a per user directory. getpass.getuser() reads the name from the environment and falls back to the password database, which a Windows control machine does not have - and tox passes neither USER nor USERNAME by default, so all twelve errored at setup there while passing outside tox. Also bound the breakpoints a check expects to be resumed: they inherited the framework default of 600s, so a resume that never arrived would have held the suite for ten minutes instead of failing the check that expected it. --- .../ducktests/tests/checks/support/demo_pause_control.py | 9 ++++++++- modules/ducktests/tests/tox.ini | 6 ++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/modules/ducktests/tests/checks/support/demo_pause_control.py b/modules/ducktests/tests/checks/support/demo_pause_control.py index 1e26a5ed45259..804d4d27ce43c 100644 --- a/modules/ducktests/tests/checks/support/demo_pause_control.py +++ b/modules/ducktests/tests/checks/support/demo_pause_control.py @@ -29,19 +29,26 @@ import time from contextlib import contextmanager -from ignitetest.utils.pause import DemoPause, STATUS_JSON +from ignitetest.utils.pause import DEMO_PAUSE_TIMEOUT_SEC, DemoPause, STATUS_JSON from checks.support.ducktape_doubles import FakeLogger # Stands for the test a breakpoint was reached in; breakpoints report it to the host. TEST_NAME = "check.CheckPause.check_something" +# Far longer than the fraction of a second a check actually holds a breakpoint for, and far +# shorter than the framework's own default: a resume that never arrives has to fail the check +# that expected it rather than hold the suite for ten minutes. +RESUME_TIMEOUT_SEC = 30 + def new_demo_pause(control_dir, started_at=None, runner_timeout_sec=None, **test_globals): """ :return: A DemoPause over the given control directory, logging into a FakeLogger its ``logger`` attribute hands back to the check. """ + test_globals.setdefault(DEMO_PAUSE_TIMEOUT_SEC, RESUME_TIMEOUT_SEC) + return DemoPause(FakeLogger(), test_globals, TEST_NAME, control_dir=str(control_dir), started_at=started_at, runner_timeout_sec=runner_timeout_sec) diff --git a/modules/ducktests/tests/tox.ini b/modules/ducktests/tests/tox.ini index e0fa4a74df8aa..c40e563eb269c 100644 --- a/modules/ducktests/tests/tox.ini +++ b/modules/ducktests/tests/tox.ini @@ -21,6 +21,12 @@ usedevelop = True envdir = {toxworkdir}/.virtualenvs/ignite-ducktests-{envname} deps = -r{toxinidir}/docker/requirements-dev.txt install_command = pip install --extra-index-url https://pypi.org/simple {opts} {packages} +# pytest builds its tmp_path root below a per user directory, and getpass.getuser() has no +# password database to fall back on when the name is missing from the environment - which is +# what a Windows control machine runs into, since tox passes neither variable by default. +passenv = + USER + USERNAME commands = pytest {env:PYTESTARGS:} {posargs} [testenv:codestyle] From 6dda92d7359639c1866fa3dedbbf15e88546804d Mon Sep 17 00:00:00 2001 From: Maksim Davydov Date: Fri, 21 Aug 2026 16:35:03 +0300 Subject: [PATCH 07/15] IGNITE-28976 [ducktests] Say which side of the bind mount demo_pause_dir names The control directory is reached from a container and from the host, and only the default is found by both on its own: the global is read by the test, where the repository is /opt/ignite-dev, so overriding it leaves the console looking at the default until it is pointed at the host side of the same directory. --- modules/ducktests/README.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/modules/ducktests/README.md b/modules/ducktests/README.md index cf8537677ff53..aeafbec2f4cca 100644 --- a/modules/ducktests/README.md +++ b/modules/ducktests/README.md @@ -268,6 +268,13 @@ touch .ducktests-demo/continue-all # resume and skip the remaining breakpoints touch .ducktests-demo/abort # fail the test and tear down ``` +Both sides find that directory on their own, so by default nothing has to be configured. `demo_pause_dir` overrides it - but the two sides name the same directory differently, since the test runs inside `ducker01`, where the repository is mounted at `/opt/ignite-dev`, while the console runs on the host. Override it and the console has to be pointed at the host side of it: +```bash +./docker/run_tests.sh -gj '{"demo_pause": "*", "demo_pause_dir": "/opt/ignite-dev/.demo"}' -t ./ignitetest/tests/ + +python docker/demo_console.py -d .demo +``` + Breakpoints are added to a test with `self.pause("name", mdc, net)` and cost nothing when the global is absent, which is how they stay in the tests without affecting CI. Run one test at a time in demo mode (no `--max-parallel`): a single control directory holds one breakpoint at a time. A held test reports nothing back to ducktape, which kills a session it has heard nothing from for `--test-runner-timeout` (30 minutes by default) - and that budget is spent by the whole test, setup included, not by the breakpoint alone. Breakpoints therefore auto-continue while the runner is still waiting, shortening themselves below `demo_pause_timeout_sec` when there is not enough of the budget left and saying so in the test log. For a demo that needs longer, raise the runner timeout too (milliseconds): @@ -281,7 +288,7 @@ A held test reports nothing back to ducktape, which kills a session it has heard |---------------------|------------|----------------------| | **demo_pause** | Which breakpoints stop the scenario. Absent or `false` disables them all (the default); `true` or `"*"` stops at every one; a list or comma separated string stops only at the named ones, matched case insensitively. | ```{"demo_pause": "split-brain,healed"}``` | | **demo_pause_timeout_sec** | How long one breakpoint may hold the scenario before it resumes on its own. Default is 600, and it is capped by what is left of `--test-runner-timeout`. | ```{"demo_pause_timeout_sec": 1800}``` | -| **demo_pause_dir** | Control directory shared with the host. Default is `/.ducktests-demo`. | ```{"demo_pause_dir": "/opt/ignite-dev/.demo"}``` | +| **demo_pause_dir** | Control directory shared with the host, named as the test sees it - inside the containers the repository is `/opt/ignite-dev`. Default is `/.ducktests-demo`; anything else has to be passed to the console as well, with `-d` and the host path. | ```{"demo_pause_dir": "/opt/ignite-dev/.demo"}``` | ### Security Settings ```bash From a08bdc45fcf6c2b526c96ee1b2f36e25c0393087 Mon Sep 17 00:00:00 2001 From: Maksim Davydov Date: Fri, 21 Aug 2026 16:41:41 +0300 Subject: [PATCH 08/15] IGNITE-28976 [ducktests] Let a service that cannot answer degrade the banner, not fail it _node_state already refused to let a probe failure out, on the grounds that a breakpoint must never fail the scenario it is only observing - but the two readings either side of it were unguarded, and who_am_i() is the one that realistically throws: it resolves the node through idx(), which raises for a node the service no longer owns. Rendering the banner would then fail the test at exactly the point the demo was added to show. Each node line now degrades to the name that can be read without its service, and the hints drop a node they cannot name instead of losing the rest with it. --- .../tests/checks/support/ducktape_doubles.py | 14 ++++++++ .../tests/checks/utils/pause/check_banner.py | 22 +++++++++++- .../ducktests/tests/ignitetest/utils/pause.py | 36 ++++++++++++++++--- 3 files changed, 66 insertions(+), 6 deletions(-) diff --git a/modules/ducktests/tests/checks/support/ducktape_doubles.py b/modules/ducktests/tests/checks/support/ducktape_doubles.py index 0d06221521783..7a8621e583106 100644 --- a/modules/ducktests/tests/checks/support/ducktape_doubles.py +++ b/modules/ducktests/tests/checks/support/ducktape_doubles.py @@ -90,6 +90,20 @@ def globals(self): return {} +class FakeBrokenService: + """ + Stands in for a service that can no longer answer for the nodes it still holds, the way a + ducktape one does once a node has been freed from it: ``who_am_i`` goes through ``idx()``, + which raises for a node the service does not own. + """ + def __init__(self, *hostnames): + self.nodes = fake_nodes(*hostnames) + + def who_am_i(self, node): + """Fails the way a ducktape service does for a node it does not own.""" + raise RuntimeError(f"Could not find node {node}") + + class FakeRegistry: """ Stands in for ducktape's ServiceRegistry, which is what a test hands to the framework: it diff --git a/modules/ducktests/tests/checks/utils/pause/check_banner.py b/modules/ducktests/tests/checks/utils/pause/check_banner.py index 91df9118fefa8..6057a9652b988 100644 --- a/modules/ducktests/tests/checks/utils/pause/check_banner.py +++ b/modules/ducktests/tests/checks/utils/pause/check_banner.py @@ -23,7 +23,7 @@ from ignitetest.utils.pause import ALL, DemoPause from checks.support.demo_pause_control import new_demo_pause, published_status -from checks.support.ducktape_doubles import FakeIgniteService, FakeRegistry, FakeService +from checks.support.ducktape_doubles import FakeBrokenService, FakeIgniteService, FakeRegistry, FakeService def check_elapsed_is_counted_from_test_start(tmp_path): @@ -59,6 +59,26 @@ def check_banner_is_rendered_from_the_service_registry(tmp_path): assert "ducker02 ducker03" in banner +def check_a_service_that_cannot_answer_is_degraded_not_raised(tmp_path): + """ + Check that a service which cannot answer for its nodes costs the banner those lines and + nothing more. A breakpoint only observes the cluster, so one that throws while rendering + would fail the scenario at exactly the point the demo was added to show. + """ + demo = new_demo_pause(tmp_path, demo_pause=ALL, demo_pause_timeout_sec=.3) + + with published_status(tmp_path) as published: + demo.pause("split-brain", + services=FakeRegistry(FakeBrokenService("ducker02"), FakeIgniteService("ducker03"))) + + banner = "\n".join(published["banner"]) + + assert "FakeBrokenService-ducker02" in banner, "the node must still be named, without asking its service" + assert "RuntimeError" in banner, "and what could not be read must say so" + assert "FakeIgniteService-ducker03" in banner, "a service after the broken one must still be listed" + assert "ducker02 ducker03" in banner, "and every node must still be offered by the hints" + + def check_hints_follow_the_ignite_services(): """ Check that the copy-pasteable commands name the Ignite paths even when a service of diff --git a/modules/ducktests/tests/ignitetest/utils/pause.py b/modules/ducktests/tests/ignitetest/utils/pause.py index 1134dee098e4f..dcb6208230272 100644 --- a/modules/ducktests/tests/ignitetest/utils/pause.py +++ b/modules/ducktests/tests/ignitetest/utils/pause.py @@ -168,6 +168,13 @@ def _fmt_duration(seconds): return f"{seconds // 60:02d}:{seconds % 60:02d}" +def _node_host(node): + """ + :return: Hostname of the node, None when it carries no account to read one from. + """ + return getattr(getattr(node, "account", None), "hostname", None) + + def _node_addr(node): """ :return: The node's routable address when it adds anything to the name the banner already @@ -198,6 +205,25 @@ def _node_state(service, node): return "?" +def _node_line(service, node): + """ + :return: The node's line of the SERVICES section, degraded to the name that can be read + without asking the service when the service itself cannot answer for the node - + ``who_am_i`` goes through ``idx()``, which raises for a node the service no + longer owns. + + Like :func:`_node_state`, this lets no reading failure out: a breakpoint must never fail + the scenario it is only observing, least of all while rendering the banner it was added + for. + """ + try: + return f" {service.who_am_i(node):<58} {_node_addr(node):<17} {_node_state(service, node)}".rstrip() + except Exception as ex: # pylint: disable=broad-except + name = f"{type(service).__name__}-{_node_host(node) or '?'}" + + return f" {name:<58} ({type(ex).__name__})" + + class DemoPause: """ Holds a scenario at named breakpoints. @@ -367,10 +393,7 @@ def _services_section(services): for service in services: for node in service.nodes: - who = service.who_am_i(node) - addr = _node_addr(node) - - lines.append(f" {who:<58} {addr:<17} {_node_state(service, node)}".rstrip()) + lines.append(_node_line(service, node)) if len(lines) == 1: lines.append(" (none)") @@ -383,7 +406,10 @@ def _hints_section(services): Node logs live only on the nodes while the test runs - ducktape copies them into the results directory at teardown - so every hint goes through the node container. """ - hosts = sorted({node.account.hostname for service in services for node in service.nodes}) + # A node the banner could not name is left out rather than allowed to fail the hints: + # the commands are offered for the nodes that can be entered, and one unreadable node + # must not cost the demo the rest of them. + hosts = sorted({_node_host(node) for service in services for node in service.nodes} - {None}) log_dir = "/mnt/service/logs" config_file = "/mnt/service/config/ignite-config.xml" From b34b4c5d99670c1e64078c6fa1b56ad6a42d51c7 Mon Sep 17 00:00:00 2001 From: Maksim Davydov Date: Fri, 21 Aug 2026 16:48:12 +0300 Subject: [PATCH 09/15] IGNITE-28976 [ducktests] Make the gc log check hold off Linux too check_colon_options wrote out the expected path with a forward slash, while __get_default_jvm_opts builds it with os.path.join, which follows the control machine - so the check only held where the separator happened to match, and failed on a Windows one. Join the expectation the same way the code under check joins it, as check_get_ssl_params_from_globals already does for the key store paths. The option was also spelled out twice, once to assert it is there and once to assert what it comes before; it is a local now. --- .../tests/checks/utils/check_ignite_spec.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/modules/ducktests/tests/checks/utils/check_ignite_spec.py b/modules/ducktests/tests/checks/utils/check_ignite_spec.py index a3c8e52104939..b60fce333e18f 100644 --- a/modules/ducktests/tests/checks/utils/check_ignite_spec.py +++ b/modules/ducktests/tests/checks/utils/check_ignite_spec.py @@ -16,6 +16,7 @@ """ Checks Spec class that describes config and command line to start Ignite-aware service. """ +import os from unittest.mock import Mock import pytest @@ -90,10 +91,15 @@ def check_boolean_options__go_after_default_ones_and_overwrite_them__if_passed_v def check_colon_options__go_after_default_ones_and_overwrite_them__if_passed_via_jvm_opt(service): service.log_dir = "/default-path" + + # The default is built with os.path.join, which follows the control machine rather than the + # node - so the expectation is joined the same way instead of being written out, or the check + # would only hold where the separator happens to be "/". + default_gc_log = ("-Xlog:gc*=debug,gc+stats*=debug,gc+ergo*=debug:" + f"{os.path.join(service.log_dir, 'gc.log')}:uptime,time,level,tags") + spec = IgniteApplicationSpec(service, jvm_opts=["-Xlog:gc:/some-non-default-path/gc.log"]) + assert "-Xlog:gc:/some-non-default-path/gc.log" in spec.jvm_opts - assert "-Xlog:gc*=debug,gc+stats*=debug,gc+ergo*=debug:/default-path/gc.log:uptime,time,level,tags" \ - in spec.jvm_opts - assert spec.jvm_opts.index("-Xlog:gc:/some-non-default-path/gc.log") > \ - spec.jvm_opts.index( - "-Xlog:gc*=debug,gc+stats*=debug,gc+ergo*=debug:/default-path/gc.log:uptime,time,level,tags") + assert default_gc_log in spec.jvm_opts + assert spec.jvm_opts.index("-Xlog:gc:/some-non-default-path/gc.log") > spec.jvm_opts.index(default_gc_log) From 4b47a4c6ef57406d273a6f68f81abcf9e3ef96e3 Mon Sep 17 00:00:00 2001 From: Maksim Davydov Date: Fri, 21 Aug 2026 16:54:04 +0300 Subject: [PATCH 10/15] IGNITE-28976 [ducktests] Add tox to the development requirements The checks are run through tox, so the environment that runs them should carry it: README asks for it in a pip install of its own, after the requirements file the same section has just installed. --- modules/ducktests/tests/docker/requirements-dev.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/ducktests/tests/docker/requirements-dev.txt b/modules/ducktests/tests/docker/requirements-dev.txt index f868e43d67466..8ce0758663a4a 100644 --- a/modules/ducktests/tests/docker/requirements-dev.txt +++ b/modules/ducktests/tests/docker/requirements-dev.txt @@ -16,3 +16,4 @@ -r requirements.txt pytest==6.2.5 flake8==6.1.0 +tox From 6ad0c80f436f781c765c2cf1ab6e797607963440 Mon Sep 17 00:00:00 2001 From: Maksim Davydov Date: Fri, 21 Aug 2026 16:55:38 +0300 Subject: [PATCH 11/15] IGNITE-28976 [ducktests] Drop the separate tox install from the README The section installs docker/requirements-dev.txt a few steps earlier, and that file now carries tox. --- modules/ducktests/README.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/modules/ducktests/README.md b/modules/ducktests/README.md index aeafbec2f4cca..822a0c6352cc7 100644 --- a/modules/ducktests/README.md +++ b/modules/ducktests/README.md @@ -129,9 +129,8 @@ To locally simulate validation matrices across distinct target runtimes (e.g., P pyenv install 3.9 pyenv shell 3.8 3.9 ``` -3. Install `tox` and run the validation suite: +3. Run the validation suite: ```bash - pip install tox tox tox -r -e codestyle,py3 ``` From f75af86818a86a10fdfa3b101857a05935ed03e0 Mon Sep 17 00:00:00 2001 From: Maksim Davydov Date: Fri, 21 Aug 2026 17:25:42 +0300 Subject: [PATCH 12/15] IGNITE-28976 [ducktests] Give the demo breakpoint control directory a class of its own Review feedback: the file system polling reads as a set of methods rather than as the thing it is. It is worse than that - the protocol was written out three times. pause.py swept stale files in DemoPause._prepare and demo_console.py swept them again in clear_stale; the console read the published status and the checks read it a third way; all three joined control file paths by hand. The three copies have to agree file for file or a breakpoint silently never resumes. ControlDir now owns the mechanics - paths, atomic writes, sweeping, polling - and all three sides go through it. What the files mean stays with the caller: DemoPause still decides that abort ends the scenario and logs the heartbeat, which await_any reports through a tick callback so the protocol never learns what a logger is. It lives in its own module because demo_console.py loads it by path on a host with neither ducktape nor ignitetest: pause_control.py is standard library only, where pause.py - 509 lines of banner rendering the console never wanted - was not. --- .../checks/support/demo_pause_control.py | 27 +- .../tests/checks/utils/pause/check_control.py | 3 +- .../checks/utils/pause/check_control_dir.py | 166 +++++++++++++ .../checks/utils/pause/check_selector.py | 3 +- .../tests/checks/utils/pause/check_timeout.py | 3 +- .../ducktests/tests/docker/demo_console.py | 72 ++---- .../ducktests/tests/ignitetest/utils/pause.py | 166 ++++--------- .../tests/ignitetest/utils/pause_control.py | 234 ++++++++++++++++++ 8 files changed, 487 insertions(+), 187 deletions(-) create mode 100644 modules/ducktests/tests/checks/utils/pause/check_control_dir.py create mode 100644 modules/ducktests/tests/ignitetest/utils/pause_control.py diff --git a/modules/ducktests/tests/checks/support/demo_pause_control.py b/modules/ducktests/tests/checks/support/demo_pause_control.py index 804d4d27ce43c..97d5d31b676e4 100644 --- a/modules/ducktests/tests/checks/support/demo_pause_control.py +++ b/modules/ducktests/tests/checks/support/demo_pause_control.py @@ -20,16 +20,17 @@ published banner, dropping a resume file - has to happen from another one while the check itself sits inside :meth:`ignitetest.utils.pause.DemoPause.pause`. -See :mod:`ignitetest.utils.pause` for the protocol these helpers speak. +The protocol itself is spoken through :class:`ignitetest.utils.pause_control.ControlDir`, the +same class the test and ``docker/demo_console.py`` use - a check that hand rolled the file +names would stop checking the protocol and start checking its own copy of it. """ -import json -import os import threading import time from contextlib import contextmanager -from ignitetest.utils.pause import DEMO_PAUSE_TIMEOUT_SEC, DemoPause, STATUS_JSON +from ignitetest.utils.pause import DEMO_PAUSE_TIMEOUT_SEC, DemoPause +from ignitetest.utils.pause_control import ControlDir from checks.support.ducktape_doubles import FakeLogger @@ -57,7 +58,9 @@ def resume_with(control_dir, name, delay_sec=.05): """ Creates a resume file from another thread, the way the host does while the test blocks. """ - timer = threading.Timer(delay_sec, lambda: open(os.path.join(str(control_dir), name), "w").close()) + control = ControlDir(control_dir) + + timer = threading.Timer(delay_sec, lambda: control.resume(name)) timer.daemon = True timer.start() @@ -77,22 +80,24 @@ def published_status(control_dir, resume=None, timeout_sec=30): :return: A dict, empty on entry and filled with the published breakpoint by the time the block is left. """ + control = ControlDir(control_dir) published = {} def read(): deadline = time.monotonic() + timeout_sec while time.monotonic() < deadline: - try: - with open(os.path.join(str(control_dir), STATUS_JSON), encoding="utf-8") as file: - published.update(json.load(file)) + status = control.read_status() + + if status is not None: + published.update(status) break - except (OSError, ValueError): - time.sleep(.01) + + time.sleep(.01) if resume: - open(os.path.join(str(control_dir), resume), "w").close() + control.resume(resume) reader = threading.Thread(target=read, daemon=True) reader.start() diff --git a/modules/ducktests/tests/checks/utils/pause/check_control.py b/modules/ducktests/tests/checks/utils/pause/check_control.py index 6592ed16213b9..af01dceb98998 100644 --- a/modules/ducktests/tests/checks/utils/pause/check_control.py +++ b/modules/ducktests/tests/checks/utils/pause/check_control.py @@ -22,7 +22,8 @@ import pytest -from ignitetest.utils.pause import ABORT, ALL, CONTINUE_ALL, STATUS_JSON, STATUS_TXT, continue_file +from ignitetest.utils.pause import ALL +from ignitetest.utils.pause_control import ABORT, CONTINUE_ALL, STATUS_JSON, STATUS_TXT, continue_file from checks.support.demo_pause_control import TEST_NAME, new_demo_pause, published_status, resume_with diff --git a/modules/ducktests/tests/checks/utils/pause/check_control_dir.py b/modules/ducktests/tests/checks/utils/pause/check_control_dir.py new file mode 100644 index 0000000000000..8bed65bec31d6 --- /dev/null +++ b/modules/ducktests/tests/checks/utils/pause/check_control_dir.py @@ -0,0 +1,166 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +""" +Checks the control directory itself - the files a paused test and the host exchange, and the +polling that waits for them - with no breakpoint on top of it. +""" + +import os +import time + +from ignitetest.utils.pause_control import ABORT, CONTINUE_ALL, ControlDir, STATUS_JSON, STATUS_TXT, continue_file + + +def check_taking_a_file_consumes_it(tmp_path): + """ + Check that taking a control file reports it and removes it: the test is the only party + that deletes what the host wrote, so taking one is what acknowledges it. + """ + control = ControlDir(tmp_path) + + assert not control.take(ABORT), "nothing was written yet" + + control.resume(ABORT) + + assert control.exists(ABORT) + assert control.take(ABORT) + assert not control.exists(ABORT), "a taken file must not be left for the next breakpoint" + assert not control.take(ABORT) + + +def check_sweep_spares_a_held_breakpoint(tmp_path): + """ + Check the asymmetry the two sides need: both clear the resume files an earlier run left, + but only the test drops the banner as well. The console is just as likely to have been + started against a test that is already holding one. + """ + control = ControlDir(tmp_path) + + control.publish(["PAUSED 1 split-brain"], {"seq": 1}) + control.resume(continue_file(3)) + control.resume(CONTINUE_ALL) + + control.sweep() + + assert not control.exists(continue_file(3)) + assert not control.exists(CONTINUE_ALL) + assert control.read_status() is not None, "the console must not withdraw a breakpoint it did not hold" + + control.sweep(status=True) + + assert control.read_status() is None + assert not control.exists(STATUS_TXT) + + +def check_sweeping_a_directory_that_is_not_there(tmp_path): + """ + Check that sweeping is safe before anything has been created: without the global no + breakpoint ever makes the directory, and the console may well be started first. + """ + control = ControlDir(tmp_path / "never-created") + + control.sweep(status=True) + + assert control.read_status() is None + assert not os.path.exists(control.path), "sweeping must not create what it was asked to clean" + + +def check_publishing_round_trips(tmp_path): + """ + Check that what is published is what a reader gets back, and that withdrawing it leaves + nothing of either file behind. + """ + control = ControlDir(tmp_path) + + control.publish(["PAUSED 1 split-brain", " test some.Test"], {"seq": 1, "name": "split-brain"}) + + assert control.read_status() == {"seq": 1, "name": "split-brain"} + + with open(control.file(STATUS_TXT), encoding="utf-8") as file: + assert file.read() == "PAUSED 1 split-brain\n test some.Test\n" + + control.clear_status() + + assert control.read_status() is None + assert not control.exists(STATUS_TXT) + assert not control.exists(STATUS_JSON) + + +def check_unreadable_status_reads_as_nothing_published(tmp_path): + """ + Check that a status file caught half written reads as "not paused" rather than raising: + the host polls this in a loop and simply comes back. + """ + control = ControlDir(tmp_path) + + control.write(STATUS_JSON, '{"seq": 1') + + assert control.read_status() is None + + +def check_awaiting_takes_the_file_that_arrived(tmp_path): + """ + Check the wait ends on the file that appears, and hands back which one it was. + """ + control = ControlDir(tmp_path) + + control.resume(continue_file(1)) + + assert control.await_any([ABORT, continue_file(1)], 5) == continue_file(1) + assert not control.exists(continue_file(1)), "the wait must consume what ended it" + + +def check_awaiting_honours_the_order_it_was_given(tmp_path): + """ + Check that the first name wins when several are already there: abort has to beat a + continue that landed in the same interval, or a demo would be resumed instead of ended. + """ + control = ControlDir(tmp_path) + + control.resume(CONTINUE_ALL) + control.resume(ABORT) + + assert control.await_any([ABORT, CONTINUE_ALL], 5) == ABORT + assert control.exists(CONTINUE_ALL), "only the file that ended the wait may be consumed" + + +def check_awaiting_gives_up(tmp_path): + """ + Check that a wait nobody answers ends on its own rather than holding the scenario until + ducktape kills it. + """ + control = ControlDir(tmp_path) + + started_at = time.monotonic() + + assert control.await_any([ABORT], .3) is None + assert time.monotonic() - started_at >= .3, "it must have waited for what it was given" + + +def check_awaiting_reports_that_it_is_still_waiting(tmp_path): + """ + Check that the caller is ticked while the wait runs, which is how a held breakpoint says + in the test log that it is paused rather than stuck - without this class having to know + what a log is. + """ + control = ControlDir(tmp_path) + + ticks = [] + + assert control.await_any([ABORT], 1.2, tick=ticks.append, tick_sec=.01) is None + assert ticks, "a wait longer than the tick interval must report itself" + assert all(0 < left <= 1.2 for left in ticks), ticks + assert ticks == sorted(ticks, reverse=True), "each tick must report less time left than the last" diff --git a/modules/ducktests/tests/checks/utils/pause/check_selector.py b/modules/ducktests/tests/checks/utils/pause/check_selector.py index 18860261584bc..f01c568562693 100644 --- a/modules/ducktests/tests/checks/utils/pause/check_selector.py +++ b/modules/ducktests/tests/checks/utils/pause/check_selector.py @@ -20,7 +20,8 @@ import os -from ignitetest.utils.pause import ALL, continue_file, parse_selector +from ignitetest.utils.pause import ALL, parse_selector +from ignitetest.utils.pause_control import continue_file from checks.support.demo_pause_control import new_demo_pause, resume_with diff --git a/modules/ducktests/tests/checks/utils/pause/check_timeout.py b/modules/ducktests/tests/checks/utils/pause/check_timeout.py index 211f0f33d143e..e47b922281f39 100644 --- a/modules/ducktests/tests/checks/utils/pause/check_timeout.py +++ b/modules/ducktests/tests/checks/utils/pause/check_timeout.py @@ -20,7 +20,8 @@ import os -from ignitetest.utils.pause import ALL, RUNNER_TIMEOUT_MARGIN_SEC, STATUS_JSON +from ignitetest.utils.pause import ALL, RUNNER_TIMEOUT_MARGIN_SEC +from ignitetest.utils.pause_control import STATUS_JSON from checks.support.demo_pause_control import new_demo_pause, published_status diff --git a/modules/ducktests/tests/docker/demo_console.py b/modules/ducktests/tests/docker/demo_console.py index 4647f56ec0b58..6f4b6c8c6a85a 100644 --- a/modules/ducktests/tests/docker/demo_console.py +++ b/modules/ducktests/tests/docker/demo_console.py @@ -31,18 +31,22 @@ import argparse import importlib.util -import json import os import sys import time -# Reach into the framework for the protocol constants rather than restating them. The host -# has no ducktape and no installed ignitetest, so the module is loaded by path: importing -# ignitetest.utils.pause would pull in the package __init__ chain and its ducktape imports. +# Speak the protocol through the framework's own ControlDir rather than restating it here: +# the two sides of a shared directory have to agree file for file, and a second copy of it +# is a second thing to keep in step. +# +# The host has no ducktape and no installed ignitetest, so the module is loaded by path - +# importing ignitetest.utils.pause_control would pull in the package __init__ chain and its +# ducktape imports. pause_control itself is standard library only, which is what makes it +# loadable like this; ignitetest.utils.pause, which holds what the files mean, is not. _TESTS_DIR = os.path.abspath(os.path.join(os.path.dirname(os.path.abspath(__file__)), os.pardir)) -_PAUSE_PY = os.path.join(_TESTS_DIR, "ignitetest", "utils", "pause.py") +_PAUSE_CONTROL_PY = os.path.join(_TESTS_DIR, "ignitetest", "utils", "pause_control.py") -_SPEC = importlib.util.spec_from_file_location("ignitetest_pause", _PAUSE_PY) +_SPEC = importlib.util.spec_from_file_location("ignitetest_pause_control", _PAUSE_CONTROL_PY) pause = importlib.util.module_from_spec(_SPEC) _SPEC.loader.exec_module(pause) @@ -54,20 +58,6 @@ """ -def read_status(control_dir): - """ - :return: The published breakpoint, or None when the scenario is not paused. A missing or - half written file simply reads as "not paused" and is retried. - """ - path = os.path.join(control_dir, pause.STATUS_JSON) - - try: - with open(path, encoding="utf-8") as file: - return json.load(file) - except (OSError, ValueError): - return None - - def breakpoint_key(status): """ :return: What identifies the published breakpoint. Not the sequence number on its own: @@ -77,7 +67,7 @@ def breakpoint_key(status): return status.get("run"), status.get("seq") -def clear_stale(control_dir): +def clear_stale(control): """ Removes resume files left behind by an earlier run, which would otherwise skip the first breakpoint of this one. The test clears them too, on its side, at its first breakpoint. @@ -90,31 +80,15 @@ def clear_stale(control_dir): :return: Whether the sweep was performed. """ - if not os.path.isdir(control_dir): - return True - - if read_status(control_dir) is not None: + if control.read_status() is not None: return False - for name in os.listdir(control_dir): - if name.startswith(pause.CONTINUE_PREFIX) or name == pause.ABORT: - try: - os.remove(os.path.join(control_dir, name)) - except OSError: - pass + control.sweep() return True -def resume(control_dir, name): - """ - Writes a resume file. The test consumes and removes it. - """ - with open(os.path.join(control_dir, name), "w", encoding="utf-8") as file: - file.write("") - - -def prompt(control_dir, seq): +def prompt(control, seq): """ Asks what to do with the breakpoint that is currently published. @@ -127,19 +101,19 @@ def prompt(control_dir, seq): return False if answer in ("", "n", "next"): - resume(control_dir, pause.continue_file(seq)) + control.resume(pause.continue_file(seq)) return True if answer in ("c", "continue", "all"): - resume(control_dir, pause.CONTINUE_ALL) + control.resume(pause.CONTINUE_ALL) print(" continuing, remaining breakpoints skipped") return False if answer in ("a", "abort"): - resume(control_dir, pause.ABORT) + control.resume(pause.ABORT) print(" aborting the test") @@ -147,7 +121,7 @@ def prompt(control_dir, seq): if answer in ("q", "quit", "exit"): print(f" leaving the test paused, resume it with:\n" - f" touch {os.path.join(control_dir, pause.continue_file(seq))}") + f" touch {control.file(pause.continue_file(seq))}") return False @@ -164,11 +138,11 @@ def main(): f"/{pause.CONTROL_DIR_NAME}") args = parser.parse_args() - control_dir = args.control_dir + control = pause.ControlDir(args.control_dir) - swept = clear_stale(control_dir) + swept = clear_stale(control) - print(f"Demo console, watching {control_dir}") + print(f"Demo console, watching {control.path}") if swept: print("Waiting for the first breakpoint... (Ctrl-C to leave)") @@ -178,7 +152,7 @@ def main(): last_key, resumed_at = None, None while True: - status = read_status(control_dir) + status = control.read_status() if status is None: # The test removes its status files as it resumes, so this is also what tells the @@ -205,7 +179,7 @@ def main(): print("\n".join(status.get("banner", []))) print(KEYS) - if not prompt(control_dir, status.get("seq")): + if not prompt(control, status.get("seq")): return resumed_at = time.monotonic() diff --git a/modules/ducktests/tests/ignitetest/utils/pause.py b/modules/ducktests/tests/ignitetest/utils/pause.py index dcb6208230272..46eeea1187460 100644 --- a/modules/ducktests/tests/ignitetest/utils/pause.py +++ b/modules/ducktests/tests/ignitetest/utils/pause.py @@ -22,19 +22,15 @@ mounts the whole Ignite repository into every container, so a control directory below the repository root is visible to the test and to the host at the same time. -The protocol over that directory is one sided while a breakpoint is held - the host only -ever creates files, the test is the only party that deletes them, so no step of a held -breakpoint can race with the host: - - - the test publishes ``paused.txt`` (a rendered banner) and ``paused.json`` (the same - content as data) and then blocks; - - the host creates ``continue-``, ``continue-all`` or ``abort``; - - the test consumes that file, removes it along with its own status files, and proceeds. +The directory itself, and the file protocol over it, are +:class:`ignitetest.utils.pause_control.ControlDir`. This module holds only what those files +*mean*: which breakpoint stops the scenario, what the banner says, and that ``abort`` ends +the test. Between breakpoints both sides sweep the directory for files an earlier run left behind, -which would otherwise skip the next breakpoint: the test at its first one -(see :meth:`DemoPause._prepare`), the console at startup - and only while nothing is -published, so a resume file meant for a breakpoint that is currently held is never swept. +which would otherwise skip the next breakpoint: the test at its first one (see +:meth:`DemoPause._prepare`), the console at startup - and only while nothing is published, so +a resume file meant for a breakpoint that is currently held is never swept. ``docker/demo_console.py`` is the host side of it, but nothing depends on it: reading ``paused.txt`` and touching ``continue-`` by hand works just as well. @@ -52,10 +48,11 @@ demo_pause_dir - control directory, ``/.ducktests-demo`` by default. """ -import json import os import time +from ignitetest.utils.pause_control import ABORT, CONTINUE_ALL, ControlDir, continue_file, default_control_dir + # globals: DEMO_PAUSE = "demo_pause" DEMO_PAUSE_TIMEOUT_SEC = "demo_pause_timeout_sec" @@ -69,18 +66,6 @@ # still leaves the scenario time to reach its next event. RUNNER_TIMEOUT_MARGIN_SEC = 60 -CONTROL_DIR_NAME = ".ducktests-demo" - -STATUS_TXT = "paused.txt" -STATUS_JSON = "paused.json" -CONTINUE_PREFIX = "continue-" -CONTINUE_ALL = "continue-all" -ABORT = "abort" - -# The control directory is polled rather than watched: it is a bind mount shared with the -# host, where inotify is not dependable. -POLL_SEC = .5 - # Timestamps the demo in the test log, so that a run can be read back afterwards and it is # visible that the scenario is held rather than stuck. Deliberately NOT a keepalive towards # ducktape: the test logger writes to files and to stdout only, while the runner listens for @@ -94,29 +79,6 @@ _WIDTH = 100 -def repo_root(): - """ - :return: Path of the Ignite repository root, derived from this module's own location - (``/modules/ducktests/tests/ignitetest/utils/pause.py``), so that a fork - checked out elsewhere resolves its own root. - """ - return os.path.abspath(os.path.join(os.path.dirname(__file__), *[os.pardir] * 5)) - - -def default_control_dir(): - """ - :return: Path of the control directory shared between the test and the host. - """ - return os.path.join(repo_root(), CONTROL_DIR_NAME) - - -def continue_file(seq): - """ - :return: Name of the file that resumes the breakpoint with the given sequence number. - """ - return f"{CONTINUE_PREFIX}{seq}" - - def parse_selector(value): """ Interprets the ``demo_pause`` global. @@ -247,7 +209,7 @@ def __init__(self, logger, test_globals, test_name, control_dir=None, started_at self.timeout_sec = float(test_globals.get(DEMO_PAUSE_TIMEOUT_SEC, DEFAULT_TIMEOUT_SEC)) self.runner_timeout_sec = runner_timeout_sec - self.control_dir = control_dir or test_globals.get(DEMO_PAUSE_DIR) or default_control_dir() + self.control = ControlDir(control_dir or test_globals.get(DEMO_PAUSE_DIR) or default_control_dir()) self.seq = 0 @@ -289,7 +251,7 @@ def pause(self, name, describers=(), services=()): self._publish(name, banner, timeout_sec) - self.logger.info(f"Demo breakpoint reached [seq={self.seq}, name={name}, dir={self.control_dir}]") + self.logger.info(f"Demo breakpoint reached [seq={self.seq}, name={name}, dir={self.control.path}]") self._await_resume(name, timeout_sec) @@ -335,18 +297,13 @@ def elapsed_sec(self): def _prepare(self): """ - Creates the control directory and clears anything a previous run left behind: a - stale resume file would skip the very first breakpoint of this one. + Readies the control directory, once per test: a resume file left by a previous run + would skip the very first breakpoint of this one. """ if self._prepared: return - os.makedirs(self.control_dir, exist_ok=True) - - for name in os.listdir(self.control_dir): - if name.startswith(CONTINUE_PREFIX) or name.startswith(STATUS_TXT) \ - or name.startswith(STATUS_JSON) or name == ABORT: - self._remove(name) + self.control.prepare() self._prepared = True @@ -374,7 +331,7 @@ def _render(self, name, describers, services, timeout_sec): lines.append("-" * _WIDTH) lines.append(" continue: [Enter] in the demo console") - lines.append(f" or touch {os.path.join(self.control_dir, continue_file(self.seq))}") + lines.append(f" or touch {self.control.file(continue_file(self.seq))}") lines.append("=" * _WIDTH) return lines @@ -450,14 +407,10 @@ def _hints_section(services): def _publish(self, name, banner, timeout_sec): """ - Publishes the breakpoint for the host, banner first as text and then as data: both - files are replaced atomically so the console never reads a half written one. + Publishes the breakpoint for the host: the banner to print, plus what a reader needs + to tell this pause from any other. """ - text = "\n".join(banner) + "\n" - - self._write(STATUS_TXT, text) - - self._write(STATUS_JSON, json.dumps({ + self.control.publish(banner, { "run": self.run, "seq": self.seq, "name": name, @@ -465,71 +418,36 @@ def _publish(self, name, banner, timeout_sec): "elapsed_sec": round(self.elapsed_sec, 1), "timeout_sec": timeout_sec, "banner": banner - }, indent=2)) + }) def _await_resume(self, name, timeout_sec): - deadline = time.monotonic() + timeout_sec - heartbeat = time.monotonic() + HEARTBEAT_SEC - - while True: - if self._exists(ABORT): - self._consume(ABORT) - - raise AssertionError(f"Demo aborted at breakpoint [seq={self.seq}, name={name}]") - - if self._exists(CONTINUE_ALL): - self._continue_all = True - - self._consume(CONTINUE_ALL) - self.logger.info(f"Demo resumed, remaining breakpoints skipped [seq={self.seq}, name={name}]") - - return - - if self._exists(continue_file(self.seq)): - self._consume(continue_file(self.seq)) - self.logger.info(f"Demo resumed [seq={self.seq}, name={name}]") - - return - - now = time.monotonic() - - if now >= deadline: - self._consume() - self.logger.warn(f"Demo breakpoint timed out after {timeout_sec}s, resuming " - f"[seq={self.seq}, name={name}]") - - return - - if now >= heartbeat: - heartbeat = now + HEARTBEAT_SEC - - self.logger.info(f"Still paused at demo breakpoint [seq={self.seq}, name={name}, " - f"held={_fmt_duration(timeout_sec - (deadline - now))}, " - f"left={_fmt_duration(deadline - now)}]") - - time.sleep(POLL_SEC) - - def _consume(self, *names): """ - Clears the published breakpoint along with the resume files that ended it. + Holds the scenario until the host resumes the breakpoint, or until it gives up on its + own. + + What each resume file means lives here rather than in the control directory: it is the + only part of the protocol that knows there is a scenario to end. """ - for name in names + (STATUS_TXT, STATUS_JSON): - self._remove(name) + def still_waiting(left_sec): + self.logger.info(f"Still paused at demo breakpoint [seq={self.seq}, name={name}, " + f"held={_fmt_duration(timeout_sec - left_sec)}, " + f"left={_fmt_duration(left_sec)}]") - def _exists(self, name): - return os.path.exists(os.path.join(self.control_dir, name)) + taken = self.control.await_any([ABORT, CONTINUE_ALL, continue_file(self.seq)], timeout_sec, + tick=still_waiting, tick_sec=HEARTBEAT_SEC) - def _remove(self, name): - try: - os.remove(os.path.join(self.control_dir, name)) - except OSError: - pass + # Whatever ended the wait, the banner describes a breakpoint that is over. + self.control.clear_status() - def _write(self, name, content): - path = os.path.join(self.control_dir, name) - tmp = path + ".tmp" + if taken == ABORT: + raise AssertionError(f"Demo aborted at breakpoint [seq={self.seq}, name={name}]") - with open(tmp, "w", encoding="utf-8") as file: - file.write(content) + if taken == CONTINUE_ALL: + self._continue_all = True - os.replace(tmp, path) + self.logger.info(f"Demo resumed, remaining breakpoints skipped [seq={self.seq}, name={name}]") + elif taken is None: + self.logger.warn(f"Demo breakpoint timed out after {timeout_sec}s, resuming " + f"[seq={self.seq}, name={name}]") + else: + self.logger.info(f"Demo resumed [seq={self.seq}, name={name}]") diff --git a/modules/ducktests/tests/ignitetest/utils/pause_control.py b/modules/ducktests/tests/ignitetest/utils/pause_control.py new file mode 100644 index 0000000000000..cc42a131482b8 --- /dev/null +++ b/modules/ducktests/tests/ignitetest/utils/pause_control.py @@ -0,0 +1,234 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +""" +The directory a paused test and the host talk over, and the file protocol they talk in. + +The protocol is one sided while a breakpoint is held - the host only ever creates files, the +test is the only party that deletes them - so no step of a held breakpoint can race with the +host: + + - the test publishes ``paused.txt`` (a rendered banner) and ``paused.json`` (the same + content as data) and then blocks; + - the host creates ``continue-``, ``continue-all`` or ``abort``; + - the test consumes that file, removes it along with its own status files, and proceeds. + +Both sides drive it through :class:`ControlDir`, which owns the mechanics alone - paths, +atomic writes, sweeping, polling. What a file *means* is the caller's business: +:mod:`ignitetest.utils.pause` decides that ``abort`` fails the scenario, and +``docker/demo_console.py`` decides which one to write. + +Standard library only, and deliberately free of every ignitetest import: the console loads +this module by path, on a host that has neither ducktape nor ignitetest installed. +""" + +import json +import os +import time + +CONTROL_DIR_NAME = ".ducktests-demo" + +STATUS_TXT = "paused.txt" +STATUS_JSON = "paused.json" +CONTINUE_PREFIX = "continue-" +CONTINUE_ALL = "continue-all" +ABORT = "abort" + +# The control directory is polled rather than watched: it is a bind mount shared with the +# host, where inotify is not dependable. +POLL_SEC = .5 + + +def repo_root(): + """ + :return: Path of the Ignite repository root, derived from this module's own location + (``/modules/ducktests/tests/ignitetest/utils/pause_control.py``), so that a + fork checked out elsewhere resolves its own root. + """ + return os.path.abspath(os.path.join(os.path.dirname(__file__), *[os.pardir] * 5)) + + +def default_control_dir(): + """ + :return: Path of the control directory shared between the test and the host. + """ + return os.path.join(repo_root(), CONTROL_DIR_NAME) + + +def continue_file(seq): + """ + :return: Name of the file that resumes the breakpoint with the given sequence number. + """ + return f"{CONTINUE_PREFIX}{seq}" + + +class ControlDir: + """ + One directory, shared between a paused test and the host driving it. + + Every method here is mechanics. Nothing in this class knows what a breakpoint is, which + is what lets the test, the console and the checks all speak the protocol through the same + code rather than through three copies of it that have to agree. + """ + def __init__(self, path): + self._path = str(path) + + @property + def path(self): + """ + :return: Path of the directory itself, as both sides name it. + """ + return self._path + + def file(self, name): + """ + :return: Path of a control file, for reporting it to someone who will type it. + """ + return os.path.join(self._path, name) + + def exists(self, name): + """ + :return: Whether the control file is there. + """ + return os.path.exists(self.file(name)) + + def take(self, name): + """ + Consumes a control file: the test is the only party that removes what the host wrote, + so taking it is what acknowledges it. + + :return: Whether the file was there to take. + """ + if not self.exists(name): + return False + + self.remove(name) + + return True + + def remove(self, name): + """ + Removes a control file, if it is still there - the other side may have just swept it. + """ + try: + os.remove(self.file(name)) + except OSError: + pass + + def write(self, name, content): + """ + Writes a control file whole: it lands through a temporary name, so the other side + never reads a half written one. + """ + path = self.file(name) + tmp = path + ".tmp" + + with open(tmp, "w", encoding="utf-8") as file: + file.write(content) + + os.replace(tmp, path) + + def prepare(self): + """ + Creates the directory and clears anything an earlier run left in it: a stale resume + file would skip the very first breakpoint of this one. + """ + os.makedirs(self._path, exist_ok=True) + + self.sweep(status=True) + + def sweep(self, status=False): + """ + Removes what an earlier run left behind. + + :param status: Whether to drop a published banner as well. The test does, at its first + breakpoint. The console must not: it is just as likely to have been started + against a test that is already holding one. + """ + if not os.path.isdir(self._path): + return + + for name in os.listdir(self._path): + stale = name.startswith(CONTINUE_PREFIX) or name == ABORT + + if status: + stale = stale or name.startswith(STATUS_TXT) or name.startswith(STATUS_JSON) + + if stale: + self.remove(name) + + def publish(self, banner, payload): + """ + Publishes a held breakpoint, banner first as text and then as data, so that whoever + polls for the data never finds it ahead of the text it describes. + """ + self.write(STATUS_TXT, "\n".join(banner) + "\n") + self.write(STATUS_JSON, json.dumps(payload, indent=2)) + + def read_status(self): + """ + :return: The published breakpoint, or None when nothing is published. A missing or + half written file simply reads as "not paused" and is retried by the caller. + """ + try: + with open(self.file(STATUS_JSON), encoding="utf-8") as file: + return json.load(file) + except (OSError, ValueError): + return None + + def clear_status(self): + """ + Withdraws the published breakpoint, so a stale banner never outlives the pause it + describes. + """ + for name in (STATUS_TXT, STATUS_JSON): + self.remove(name) + + def resume(self, name): + """ + Writes a resume file. The test consumes and removes it. + """ + self.write(name, "") + + def await_any(self, names, timeout_sec, tick=None, tick_sec=None): + """ + Polls until one of the named files appears, and takes it. + + :param names: Names to watch, in priority order - the first one present wins when + several land between two polls. + :param tick: Called with the seconds left, every ``tick_sec`` that passes without a + file. This is how a caller reports that it is still waiting without this class + having to know what it would report to. + :return: The name that ended the wait, None when the timeout ran out first. + """ + deadline = time.monotonic() + timeout_sec + next_tick = time.monotonic() + tick_sec if tick else None + + while True: + for name in names: + if self.take(name): + return name + + now = time.monotonic() + + if now >= deadline: + return None + + if next_tick is not None and now >= next_tick: + next_tick = now + tick_sec + + tick(deadline - now) + + time.sleep(POLL_SEC) From 3b82e4379eec8e0bad3f9ceaf97d3d8885f00774 Mon Sep 17 00:00:00 2001 From: Maksim Davydov Date: Fri, 21 Aug 2026 17:59:33 +0300 Subject: [PATCH 13/15] IGNITE-28976 [ducktests] Fix what the review found in the demo breakpoints The resume command in the banner named a path only the test could use. It is rendered inside ducker01, where the repository is mounted at /opt/ignite-dev, and demo_console.py prints the banner verbatim on the host, where that directory does not exist - so the one command offered to a reader without the console was one that reader could not run. Worse, the console's own "q" branch printed the correct host path for the same file, on the same screen. Named relative to the repository root it is one string for both sides, and the form README documents. A sweep matched abort by name while every other control file was matched by prefix. That was harmless until resume() started going through the atomic write(), which made abort.tmp a real transient that no sweep would ever clear. Resume files are empty, so they are created outright now - and the sweep matches abort by prefix too, for whatever an interrupted write leaves behind. await_any() guarded its heartbeat on tick but read tick_sec, which defaulted independently to None, so asking only for a tick raised a TypeError from inside a held breakpoint. It defaults to the poll interval now. The deferred import of IgnitePathAware was justified by the console loading this module by path. It no longer does - it loads pause_control - so the import goes back to the top where the next one added here will not be misled by it. --- .../tests/checks/utils/pause/check_banner.py | 32 +++++++++++++- .../checks/utils/pause/check_control_dir.py | 42 +++++++++++++++++++ .../ducktests/tests/ignitetest/utils/pause.py | 37 ++++++++++++---- .../tests/ignitetest/utils/pause_control.py | 16 +++++-- 4 files changed, 116 insertions(+), 11 deletions(-) diff --git a/modules/ducktests/tests/checks/utils/pause/check_banner.py b/modules/ducktests/tests/checks/utils/pause/check_banner.py index 6057a9652b988..d32443388dee0 100644 --- a/modules/ducktests/tests/checks/utils/pause/check_banner.py +++ b/modules/ducktests/tests/checks/utils/pause/check_banner.py @@ -18,9 +18,11 @@ nodes it is made of, and the commands offered for looking into them. """ +import os import time -from ignitetest.utils.pause import ALL, DemoPause +from ignitetest.utils.pause import ALL, DemoPause, _resume_command +from ignitetest.utils.pause_control import ControlDir, repo_root from checks.support.demo_pause_control import new_demo_pause, published_status from checks.support.ducktape_doubles import FakeBrokenService, FakeIgniteService, FakeRegistry, FakeService @@ -79,6 +81,34 @@ def check_a_service_that_cannot_answer_is_degraded_not_raised(tmp_path): assert "ducker02 ducker03" in banner, "and every node must still be offered by the hints" +def check_the_resume_command_is_named_where_it_will_be_typed(): + """ + Check that the command a breakpoint offers is one its reader can actually run. The banner + is rendered inside ducker01, where the repository is mounted at /opt/ignite-dev, and the + console prints it verbatim on the host, where that path does not exist - so the shared + repository root is the only anchor the two sides have in common. + """ + inside = ControlDir(os.path.join(repo_root(), ".ducktests-demo")) + + command = _resume_command(inside, 3) + + assert command == "touch .ducktests-demo/continue-3 (from the repository root)" + assert repo_root() not in command, "an absolute path here names a directory the reader may not have" + + +def check_the_resume_command_falls_back_to_the_whole_path(): + """ + Check that a control directory outside the repository is still named in full: there is no + shared anchor left to make it relative to, so a whole path is the honest answer. + """ + outside = ControlDir(os.path.join(os.path.dirname(repo_root()), "elsewhere", "demo")) + + command = _resume_command(outside, 3) + + assert command.endswith(os.path.join("elsewhere", "demo", "continue-3")) + assert "from the repository root" not in command + + def check_hints_follow_the_ignite_services(): """ Check that the copy-pasteable commands name the Ignite paths even when a service of diff --git a/modules/ducktests/tests/checks/utils/pause/check_control_dir.py b/modules/ducktests/tests/checks/utils/pause/check_control_dir.py index 8bed65bec31d6..e5bb01c1ab6da 100644 --- a/modules/ducktests/tests/checks/utils/pause/check_control_dir.py +++ b/modules/ducktests/tests/checks/utils/pause/check_control_dir.py @@ -78,6 +78,34 @@ def check_sweeping_a_directory_that_is_not_there(tmp_path): assert not os.path.exists(control.path), "sweeping must not create what it was asked to clean" +def check_sweep_clears_what_an_interrupted_write_left(tmp_path): + """ + Check that the temporary name a write goes through is swept too, whichever file it + belonged to. Every control file is matched by prefix for this reason: one matched exactly + would leave its ".tmp" in a directory that nothing else ever visits. + """ + control = ControlDir(tmp_path) + + for interrupted in (continue_file(1), CONTINUE_ALL, ABORT, STATUS_TXT, STATUS_JSON): + control.write(interrupted + ".tmp", "") + + control.sweep(status=True) + + assert os.listdir(control.path) == [] + + +def check_a_resume_file_lands_without_a_temporary(tmp_path): + """ + Check that resuming creates the file and nothing besides: a resume file is empty, so an + atomic replace would buy nothing and leave a transient behind for the sweep to know about. + """ + control = ControlDir(tmp_path) + + control.resume(ABORT) + + assert os.listdir(control.path) == [ABORT] + + def check_publishing_round_trips(tmp_path): """ Check that what is published is what a reader gets back, and that withdrawing it leaves @@ -150,6 +178,20 @@ def check_awaiting_gives_up(tmp_path): assert time.monotonic() - started_at >= .3, "it must have waited for what it was given" +def check_awaiting_ticks_without_being_told_how_often(tmp_path): + """ + Check that a caller who wants to hear that the wait is still running need not also pick an + interval - asking for one and getting a TypeError out of a held breakpoint would be a poor + way to find out that the two arguments go together. + """ + control = ControlDir(tmp_path) + + ticks = [] + + assert control.await_any([ABORT], .6, tick=ticks.append) is None + assert ticks, "a wait longer than one poll must report itself" + + def check_awaiting_reports_that_it_is_still_waiting(tmp_path): """ Check that the caller is ticked while the wait runs, which is how a held breakpoint says diff --git a/modules/ducktests/tests/ignitetest/utils/pause.py b/modules/ducktests/tests/ignitetest/utils/pause.py index 46eeea1187460..04496da9dfcc9 100644 --- a/modules/ducktests/tests/ignitetest/utils/pause.py +++ b/modules/ducktests/tests/ignitetest/utils/pause.py @@ -51,7 +51,9 @@ import os import time -from ignitetest.utils.pause_control import ABORT, CONTINUE_ALL, ControlDir, continue_file, default_control_dir +from ignitetest.services.utils.path import IgnitePathAware +from ignitetest.utils.pause_control import ABORT, CONTINUE_ALL, ControlDir, continue_file, default_control_dir, \ + repo_root # globals: DEMO_PAUSE = "demo_pause" @@ -130,6 +132,32 @@ def _fmt_duration(seconds): return f"{seconds // 60:02d}:{seconds % 60:02d}" +def _resume_command(control, seq): + """ + :return: The command that resumes a breakpoint by hand, named where it will be typed. + + Relative to the repository root rather than absolute: the banner is rendered inside + ``ducker01``, where the repository is mounted at ``/opt/ignite-dev``, and read on the host, + where that path does not exist - so an absolute one would be a command that cannot work + for the person it is offered to. Both sides see the same repository through the bind + mount, which makes the relative form the one string that holds for both, and it is the + form README documents. + """ + path = control.file(continue_file(seq)) + + try: + relative = os.path.relpath(path, repo_root()) + except ValueError: + # A control directory on another Windows drive than the repository: nothing shared to + # be relative to, so the full path is all there is to offer. + relative = path + + if relative.startswith(os.pardir) or relative == path: + return f"touch {path}" + + return f"touch {relative.replace(os.sep, '/')} (from the repository root)" + + def _node_host(node): """ :return: Hostname of the node, None when it carries no account to read one from. @@ -331,7 +359,7 @@ def _render(self, name, describers, services, timeout_sec): lines.append("-" * _WIDTH) lines.append(" continue: [Enter] in the demo console") - lines.append(f" or touch {self.control.file(continue_file(self.seq))}") + lines.append(f" or {_resume_command(self.control, self.seq)}") lines.append("=" * _WIDTH) return lines @@ -375,11 +403,6 @@ def _hints_section(services): # they follow the Ignite services: a ZookeeperService or a KafkaService registered # ahead of them - which the discovery and CDC scenarios do - carries paths of its own # and would have the banner name a zookeeper.properties for nodes that never had one. - # - # Imported here rather than at module level: docker/demo_console.py loads this module - # by path, on a host that has neither ducktape nor ignitetest installed. - from ignitetest.services.utils.path import IgnitePathAware # pylint: disable=import-outside-toplevel - for service in [s for s in services if isinstance(s, IgnitePathAware)] or list(services): try: svc_log_dir = getattr(service, "log_dir", None) diff --git a/modules/ducktests/tests/ignitetest/utils/pause_control.py b/modules/ducktests/tests/ignitetest/utils/pause_control.py index cc42a131482b8..9c9a679bca60e 100644 --- a/modules/ducktests/tests/ignitetest/utils/pause_control.py +++ b/modules/ducktests/tests/ignitetest/utils/pause_control.py @@ -160,8 +160,11 @@ def sweep(self, status=False): if not os.path.isdir(self._path): return + # Matched by prefix rather than by name: an interrupted write leaves the temporary + # ``.tmp`` it goes through behind, and a sweep is the only thing that ever + # visits this directory without knowing what it expects to find. for name in os.listdir(self._path): - stale = name.startswith(CONTINUE_PREFIX) or name == ABORT + stale = name.startswith(CONTINUE_PREFIX) or name.startswith(ABORT) if status: stale = stale or name.startswith(STATUS_TXT) or name.startswith(STATUS_JSON) @@ -199,10 +202,15 @@ def clear_status(self): def resume(self, name): """ Writes a resume file. The test consumes and removes it. + + Created outright rather than through :meth:`write`: a resume file is empty, so there + is no half written state for an atomic replace to hide, and the temporary name that + replace goes through would be one more transient to leave lying about. """ - self.write(name, "") + with open(self.file(name), "w", encoding="utf-8"): + pass - def await_any(self, names, timeout_sec, tick=None, tick_sec=None): + def await_any(self, names, timeout_sec, tick=None, tick_sec=POLL_SEC): """ Polls until one of the named files appears, and takes it. @@ -211,6 +219,8 @@ def await_any(self, names, timeout_sec, tick=None, tick_sec=None): :param tick: Called with the seconds left, every ``tick_sec`` that passes without a file. This is how a caller reports that it is still waiting without this class having to know what it would report to. + :param tick_sec: How often to do that, by default as often as the directory is looked + at - which is as often as there is anything new to say. :return: The name that ended the wait, None when the timeout ran out first. """ deadline = time.monotonic() + timeout_sec From b44a01e941b5b30c2f316ad237af4c79e8360311 Mon Sep 17 00:00:00 2001 From: Maksim Davydov Date: Fri, 21 Aug 2026 18:00:56 +0300 Subject: [PATCH 14/15] IGNITE-28976 [ducktests] Say what else the tests conftest is holding up It reads as a file about checks.support, which invites moving it into checks/ - where it would still add a directory to sys.path, just not the one that makes the name checks resolve. And it would take import ignitetest under tox with it: skipsdist drops the usedevelop install there, so the checkout is reached through this path entry alone, and every check file fails at collection without it. Both reasons are written down now, including that the second one is a workaround for the packaging and stops applying once that is fixed. --- modules/ducktests/tests/conftest.py | 37 ++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/modules/ducktests/tests/conftest.py b/modules/ducktests/tests/conftest.py index c928ed7b5397a..76fa9af5b7e85 100644 --- a/modules/ducktests/tests/conftest.py +++ b/modules/ducktests/tests/conftest.py @@ -14,17 +14,32 @@ # limitations under the License. """ -Deliberately empty - pytest imports this file for its side effect alone. +Deliberately empty - pytest imports this file for its side effect alone, which is to put the +directory it sits in on ``sys.path``. -The framework checks live in ``checks/``, whose helper modules sit apart from the check files -themselves (``checks/support/``), and pytest on its own puts only each check file's own -directory on ``sys.path`` - never this one, which is what ``checks.support`` has to be reached -through. +That is the whole of it: importing a conftest adds *its own* directory, and pytest on its own +adds only each check file's. Two things depend on this one being the directory that gets +added, so moving this file down into ``checks/`` - which is where it looks like it belongs - +breaks both: -Importing a conftest is what adds its directory, so this file is what makes -``from checks.support... import ...`` resolve, as a namespace package. Nothing below ``checks/`` -carries an ``__init__.py`` on purpose: ``setup.py`` collects the distribution with -``find_packages()``, which only finds directories that have one, so the checks and their -helpers stay out of the installed ``ignitetest`` package without ``setup.py`` having to name -them. + - ``from checks.support... import ...``. The checks live in ``checks/``, their helper + modules apart from them in ``checks/support/``, and the name ``checks`` resolves only + while its parent is on the path. A conftest in ``checks/`` would add ``checks/`` and the + import would have to become a bare ``support...`` instead. + + - ``import ignitetest``, under tox. ``[tox] skipsdist = True`` reads to tox 4 as + ``no_package``, which silently drops the ``usedevelop`` install, so the tox environment + has no installed ignitetest at all and reaches the checkout through this path entry + alone. Take this file away and every check file fails at collection, not just the ones + importing ``checks.support``. Outside tox the editable install covers it, which is why + the breakage only shows in one of the two ways the checks are run. + +The second reason is a workaround, not a design: once the packaging is fixed - a +``pyproject.toml`` and a PEP 517 editable build, so ``usedevelop`` works again - only the +first remains, and this file could then reasonably move into ``checks/``. + +Nothing below ``checks/`` carries an ``__init__.py`` on purpose: ``setup.py`` collects the +distribution with ``find_packages()``, which only finds directories that have one, so the +checks and their helpers stay out of the installed ``ignitetest`` package without ``setup.py`` +having to name them, and ``checks.support`` resolves as a namespace package instead. """ From 7b1f08782e31b362fc465acf7a57641360cc1f23 Mon Sep 17 00:00:00 2001 From: Maksim Davydov Date: Fri, 21 Aug 2026 18:04:55 +0300 Subject: [PATCH 15/15] IGNITE-28976 [ducktests] Do not let an unusable control directory fail the scenario prepare() and publish() were the last unguarded filesystem calls on the breakpoint path, while everything around them - _node_state, _node_line, _section - already refuses to let a reading failure out, on the grounds that a breakpoint observes a scenario and must not be what ends one. The directory is a bind mount of the host repository, so it can be read only, be owned by another user or be full, and any of those turned a run that was about to pass into a failure with nothing to do with the cluster. Blocking instead of raising would have been no better: a breakpoint whose banner never reached the host holds the scenario for its whole timeout with nothing on screen to resume it. So the breakpoint is skipped and the later ones with it - they would fail the same way, and one warning beats one per breakpoint. abort still ends a scenario, because that one is asked for. --- .../tests/checks/utils/pause/check_control.py | 38 +++++++++++++++++ .../ducktests/tests/ignitetest/utils/pause.py | 42 ++++++++++++++++--- 2 files changed, 74 insertions(+), 6 deletions(-) diff --git a/modules/ducktests/tests/checks/utils/pause/check_control.py b/modules/ducktests/tests/checks/utils/pause/check_control.py index af01dceb98998..e84c83478a62a 100644 --- a/modules/ducktests/tests/checks/utils/pause/check_control.py +++ b/modules/ducktests/tests/checks/utils/pause/check_control.py @@ -84,6 +84,44 @@ def check_abort_fails_the_test(tmp_path): assert not os.path.exists(str(tmp_path / STATUS_JSON)) +def check_a_control_directory_that_cannot_be_made_does_not_fail_the_scenario(tmp_path): + """ + Check that a control directory which cannot even be created costs the demo and nothing + else. It is a bind mount of the host repository, so it can be read only or owned by + another user - neither of which says anything about the cluster under test, and a + breakpoint must not be what turns a passing run red. + """ + # A file where the directory should go: the portable way to make os.makedirs fail. + (tmp_path / "in-the-way").write_text("not a directory", encoding="utf-8") + + demo = new_demo_pause(tmp_path / "in-the-way" / "control", demo_pause=ALL) + + demo.pause("split-brain") + + assert demo.seq == 0, "the scenario must have carried straight on" + assert not demo.enabled, "and the later breakpoints must not try it again" + assert any("control directory cannot be used" in msg for msg in demo.logger.messages), demo.logger.messages + + +def check_a_breakpoint_that_cannot_be_published_is_skipped(tmp_path): + """ + Check the same where the directory exists but the banner cannot be written. Blocking would + be no better than raising here: nothing reached the host, so there would be nothing on + screen to resume, and the scenario would sit there for the whole timeout. + """ + # A directory where the banner goes makes the write fail wherever these checks run. The + # sweep cannot remove it either, so it is still in the way when the breakpoint publishes. + os.mkdir(str(tmp_path / STATUS_TXT)) + + demo = new_demo_pause(tmp_path, demo_pause=ALL) + + demo.pause("split-brain") + + assert not demo.enabled + assert not os.path.exists(str(tmp_path / STATUS_JSON)), "half a breakpoint must not be left published" + assert any("control directory cannot be used" in msg for msg in demo.logger.messages), demo.logger.messages + + def check_stale_resume_file_is_cleared(tmp_path): """ Check that a resume file left by a previous run does not skip the first breakpoint of diff --git a/modules/ducktests/tests/ignitetest/utils/pause.py b/modules/ducktests/tests/ignitetest/utils/pause.py index 04496da9dfcc9..05fd17eb78757 100644 --- a/modules/ducktests/tests/ignitetest/utils/pause.py +++ b/modules/ducktests/tests/ignitetest/utils/pause.py @@ -249,18 +249,22 @@ def __init__(self, logger, test_globals, test_name, control_dir=None, started_at self._started_at = time.monotonic() if started_at is None else started_at self._prepared = False self._continue_all = False + self._unusable = False @property def enabled(self): """ :return: Whether any breakpoint of this test can stop the scenario. """ - return self.names is not None and not self._continue_all + return self.names is not None and not self._continue_all and not self._unusable def pause(self, name, describers=(), services=()): """ Blocks the scenario at the named breakpoint until the host resumes it. + A control directory that cannot be used costs the demo and nothing else - see + :meth:`_give_up`. + :param name: Breakpoint name, matched against the ``demo_pause`` global. :param describers: Objects exposing ``describe() -> list of str``, each contributing a section to the banner. The first line of a section is its title. @@ -269,20 +273,46 @@ def pause(self, name, describers=(), services=()): if not self._stops_at(name): return - self._prepare() + try: + self._prepare() + + self.seq += 1 - self.seq += 1 + timeout_sec = self._budgeted_timeout() - timeout_sec = self._budgeted_timeout() + banner = self._render(name, describers, services, timeout_sec) - banner = self._render(name, describers, services, timeout_sec) + self._publish(name, banner, timeout_sec) + except OSError as ex: + self._give_up(name, ex) - self._publish(name, banner, timeout_sec) + return self.logger.info(f"Demo breakpoint reached [seq={self.seq}, name={name}, dir={self.control.path}]") self._await_resume(name, timeout_sec) + def _give_up(self, name, error): + """ + Turns the remaining breakpoints off, after the control directory turned out to be + unusable. + + A breakpoint observes a scenario; it must not be what ends one. The directory is a + bind mount of the host repository, so it can be read only, be owned by another user or + be full - none of which says anything about the cluster under test, and all of which + would otherwise fail a run that was about to pass. Blocking would be no better than + raising: a breakpoint whose banner never reached the host would hold the scenario for + its whole timeout with nothing on screen to resume it. + + Every later breakpoint would fail the same way, so they are dropped here rather than + reported again at each one. + """ + self._unusable = True + + self.logger.warn(f"Demo breakpoints disabled, the control directory cannot be used " + f"[dir={self.control.path}, error={error}, seq={self.seq}, name={name}, " + f"test={self.test_name}]") + def _stops_at(self, name): if not self.enabled: return False