diff --git a/docs/configuration.md b/docs/configuration.md index ca4886d4..5c5e4e79 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -23,7 +23,7 @@ need to re-export `KINETIC_*` env vars each time you switch. | `KINETIC_OUTPUT_DIR` | CLI + remote pod | `gs://{bucket}/outputs/{job_id}` | Per-job durable artifact prefix. See [Checkpointing](guides/checkpointing.md). | | `KINETIC_RESERVATION` | `kinetic pool add` | _(unset)_ | GCP capacity reservation to consume. Pool-level config, not a per-job setting. | | `KINETIC_LOG_LEVEL` | Library | `INFO` | `DEBUG`, `INFO`, `WARNING`, `ERROR`, `FATAL`. | -| `KINETIC_DEBUG_WAIT_TIMEOUT` | Library + remote pod | `600` | Seconds the remote pod waits for a debugger client to attach when `debug=True`. Applies on both sides (local `debug_attach()` and the pod's debugpy server). | +| `KINETIC_DEBUG_WAIT_TIMEOUT` | Library + remote pod | `600` | Seconds that Kinetic waits for a debugger to attach when `debug=True`. Kinetic reads the variable when you submit the job, and applies the value to the local wait and to the pod. Set the variable before you submit. The value must be a positive whole number of seconds. If the value is not valid, Kinetic uses `600`. See [Interactive Debugging](guides/debugging.md). | | `KINETIC_PACKAGE_ROOT` | Library (submit) | _(auto-detected)_ | Set this variable to the directory that Kinetic puts into `context.zip`. The directory must exist. The directory must also contain the directory that defines your function. If one of these conditions is not true, Kinetic raises a `ValueError` at submit time. See [What Ships to the Pod](guides/packaging.md). | | `KINETIC_NO_DEFAULT_EXCLUDES`| Library (submit) | _(unset)_ | Set this variable to `1` to turn the default exclusions off. Kinetic then puts `.venv`, `node_modules`, and the cache directories into `context.zip`. Kinetic always excludes `.git` and `__pycache__`. | | `KINETIC_CONTEXT_SIZE_WARN_MB`| Library (submit) | `100` | Warning threshold in megabytes for `context.zip`. Above the threshold, Kinetic logs a warning and lists the five largest files. Set the value to `0` to turn the warning off. | diff --git a/docs/guides/debugging.md b/docs/guides/debugging.md index 263a89d3..43e7213d 100644 --- a/docs/guides/debugging.md +++ b/docs/guides/debugging.md @@ -121,27 +121,48 @@ match. ## Path mappings and source files -Kinetic fills in `pathMappings` in the printed `launch.json` so -breakpoints set in your local files hit the matching remote files — -no "unverified breakpoint" warnings, no file mismatch. - -If you attach from a directory that isn't your project root, pass -`working_dir=` to `debug_attach()` (or replace `${workspaceFolder}` in -the printed snippet) so the mapping points at the sources you actually -have open. +Your source files have the same path on the pod and on your machine. +The runner extracts the workspace into a temporary directory. Then the +runner makes a symbolic link at the path of your client working +directory. That link points at the workspace. + +The two paths are the same, so the snippet sets `localRoot` and +`remoteRoot` to that one directory. A breakpoint in a local file stops +the program in the same file on the pod. VS Code does not show an +"unverified breakpoint" warning. + +`kinetic jobs debug ` does not know your client working +directory. For that command, Kinetic prints no `pathMappings` entry. +debugpy then uses the remote paths without a change. This result is +correct, because the paths are the same. + +Add a mapping only if you open your sources from a different directory +than the directory that you submitted from. Set `localRoot` to the +directory that you have open. Set `remoteRoot` to the directory that +you submitted from. ## Timeouts and the attach window -The pod waits up to 10 minutes for a debugger client to attach. If no -one connects in that window, it proceeds with your function running -normally — the job does not hang indefinitely. To extend or shorten -that window, set `KINETIC_DEBUG_WAIT_TIMEOUT` (seconds) in your local -environment before submitting: +The pod waits 10 minutes for a debugger to attach. If no debugger +attaches in that time, the pod runs your function as usual. The job +does not wait longer than the window. + +To change the length of the window, set `KINETIC_DEBUG_WAIT_TIMEOUT` +in your local environment. The unit is seconds. ```bash export KINETIC_DEBUG_WAIT_TIMEOUT=1800 # 30 minutes ``` +Kinetic reads the variable when you submit the job. Kinetic then puts +the value in the pod. The client and the pod wait for the same time. + +Set the variable before you submit the job. A change after that time +has no effect on a job that is already in the cluster. + +The value must be a positive whole number of seconds. If the value is +not valid, Kinetic writes a warning to the log and uses 10 minutes. + ## Multi-host debugging On multi-host TPU slices (Pathways backend), you attach once to the @@ -150,6 +171,11 @@ distributed runtime doesn't start until you're ready. `jax.process_index()` semantics stay predictable, and you don't need to attach to each host separately. +Kinetic gives the leader and the workers the same attach window. If +you set `KINETIC_DEBUG_WAIT_TIMEOUT`, the new value applies to all the +hosts. Each worker waits a short time more than the leader, so that +normal write latency does not fail the job. + :::{warning} **Avoid `spot=True` with `debug=True`.** Preemption mid-session terminates the pod, dropping your debug connection. Kinetic warns at @@ -159,15 +185,19 @@ work. ## Automated environments -`@kinetic.run(debug=True)` requires an interactive terminal — if -`stdin` isn't a TTY (CI, `nohup`, piped input), the local client -raises `RuntimeError` before submission so your job doesn't silently -hang waiting for someone to attach. +A blocking call to `@kinetic.run(debug=True)` needs an interactive +terminal. If `stdin` is not a TTY (CI, `nohup`, or piped input), the +client raises a `RuntimeError`. The client raises the error before it +submits the job, so no job starts in the cluster. Without this check, +the job waits the full window for a debugger that cannot attach. Then +the job runs your function without a debugger. + +To override the check, set `KINETIC_NO_TTY_DEBUG=1`. This variable is +for automated tests. -For async submission there's no TTY requirement — -`@kinetic.run(debug=True)` works fine in any environment, and -`kinetic jobs debug` from an interactive shell attaches whenever -you're ready. +`run_async()` has no TTY requirement. Submit the job from any +environment. Then attach with `kinetic jobs debug ` from an +interactive shell when you are ready. ## Related pages diff --git a/kinetic/backend/gke_client.py b/kinetic/backend/gke_client.py index dcd17042..7d8a3d30 100644 --- a/kinetic/backend/gke_client.py +++ b/kinetic/backend/gke_client.py @@ -13,7 +13,7 @@ from kinetic.backend.log_streaming import LogStreamer from kinetic.cli.constants import KINETIC_KSA_NAME from kinetic.credentials import invalidate_credential_cache -from kinetic.debug import DEBUG_WAIT_TIMEOUT, DEBUGPY_PORT +from kinetic.debug import DEBUGPY_PORT, resolve_debug_wait_timeout from kinetic.job_status import JobStatus # Guards the last-seen kubeconfig context used to invalidate the @@ -427,7 +427,7 @@ def _create_job_spec( client.V1EnvVar(name="PYTHONBREAKPOINT", value="debugpy.breakpoint"), client.V1EnvVar( name="KINETIC_DEBUG_WAIT_TIMEOUT", - value=str(DEBUG_WAIT_TIMEOUT), + value=str(resolve_debug_wait_timeout()), ), client.V1EnvVar(name="KINETIC_DEBUG_PORT", value=str(DEBUGPY_PORT)), ] diff --git a/kinetic/backend/gke_client_test.py b/kinetic/backend/gke_client_test.py index 4eac9a3d..c53b43b3 100644 --- a/kinetic/backend/gke_client_test.py +++ b/kinetic/backend/gke_client_test.py @@ -1,5 +1,6 @@ """Tests for kinetic.backend.gke_client — K8s job submission and monitoring.""" +import os from unittest import mock from unittest.mock import MagicMock @@ -22,6 +23,7 @@ GCSFUSE_CSI_DRIVER, GCSFUSE_VOLUMES_ANNOTATION, ) +from kinetic.debug import DEBUG_WAIT_TIMEOUT_ENV from kinetic.job_status import JobStatus @@ -246,6 +248,40 @@ def test_fuse_single_file_mounts_parent_dir(self): self.assertIn("only-dir=data", vol.csi.volume_attributes["mountOptions"]) self.assertNotIn("weights.h5", vol.csi.volume_attributes["mountOptions"]) + def _debug_env(self, job): + container = job.spec.template.spec.containers[0] + return {e.name: e.value for e in container.env} + + def _make_debug_job(self): + return _create_job_spec( + job_name="debug-job", + container_uri="img", + accel_config=self._make_cpu_config(), + job_id="j", + bucket_name="b", + namespace="ns", + debug=True, + ) + + def test_debug_wait_timeout_defaults_to_ten_minutes(self): + with mock.patch.dict(os.environ, {}, clear=False): + os.environ.pop(DEBUG_WAIT_TIMEOUT_ENV, None) + env = self._debug_env(self._make_debug_job()) + + self.assertEqual(env["KINETIC_DEBUG_WAIT_TIMEOUT"], "600") + + def test_debug_wait_timeout_propagates_user_value_to_pod(self): + """A client-side override must reach the pod, or the two disagree. + + The pod stops waiting at its own window. If it kept the default + while the user asked for longer, the pod would run the function + before the user finished attaching. + """ + with mock.patch.dict(os.environ, {DEBUG_WAIT_TIMEOUT_ENV: "1800"}): + env = self._debug_env(self._make_debug_job()) + + self.assertEqual(env["KINETIC_DEBUG_WAIT_TIMEOUT"], "1800") + class TestWaitForJob(absltest.TestCase): def setUp(self): diff --git a/kinetic/backend/pathways_client.py b/kinetic/backend/pathways_client.py index 1c933f67..bd244445 100644 --- a/kinetic/backend/pathways_client.py +++ b/kinetic/backend/pathways_client.py @@ -13,7 +13,7 @@ from kinetic.cli.constants import KINETIC_KSA_NAME from kinetic.core import accelerators from kinetic.credentials import invalidate_credential_cache -from kinetic.debug import DEBUG_WAIT_TIMEOUT, DEBUGPY_PORT +from kinetic.debug import DEBUGPY_PORT, resolve_debug_wait_timeout from kinetic.job_status import JobStatus LWS_GROUP = "leaderworkerset.x-k8s.io" @@ -594,6 +594,10 @@ def _create_lws_spec( # hang trying to join JAX's distributed runtime while the leader is # paused at debugpy. if debug: + # Resolved once so the leader and its workers agree on the window + # even if the environment changes underneath a later call. + wait_timeout = str(resolve_debug_wait_timeout()) + leader_template = copy.deepcopy(pod_template) leader_container = leader_template["spec"]["containers"][0] leader_container["env"].extend( @@ -602,7 +606,7 @@ def _create_lws_spec( {"name": "PYTHONBREAKPOINT", "value": "debugpy.breakpoint"}, { "name": "KINETIC_DEBUG_WAIT_TIMEOUT", - "value": str(DEBUG_WAIT_TIMEOUT), + "value": wait_timeout, }, {"name": "KINETIC_DEBUG_PORT", "value": str(DEBUGPY_PORT)}, ] @@ -618,7 +622,7 @@ def _create_lws_spec( {"name": "KINETIC_DEBUG_WAIT_LEADER", "value": "1"}, { "name": "KINETIC_DEBUG_WAIT_TIMEOUT", - "value": str(DEBUG_WAIT_TIMEOUT), + "value": wait_timeout, }, ] ) diff --git a/kinetic/backend/pathways_client_test.py b/kinetic/backend/pathways_client_test.py index 56bc8020..0528b4e8 100644 --- a/kinetic/backend/pathways_client_test.py +++ b/kinetic/backend/pathways_client_test.py @@ -1,5 +1,6 @@ """Tests for kinetic.backend.pathways_client — LWS job submission and monitoring.""" +import os from unittest import mock from unittest.mock import MagicMock @@ -26,6 +27,7 @@ from kinetic.backend.pathways_client import ( list_jobs as list_pathways_jobs, ) +from kinetic.debug import DEBUG_WAIT_TIMEOUT_ENV from kinetic.job_status import JobStatus _MODULE = "kinetic.backend.pathways_client" @@ -393,6 +395,34 @@ def test_non_debug_has_no_debug_contract(self): self.assertNotIn("KINETIC_DEBUG", env) self.assertNotIn("KINETIC_DEBUG_WAIT_LEADER", env) + def _debug_wait_timeouts(self): + lws = self._make_spec(debug=True)["spec"]["leaderWorkerTemplate"] + return ( + self._env(lws["leaderTemplate"])["KINETIC_DEBUG_WAIT_TIMEOUT"], + self._env(lws["workerTemplate"])["KINETIC_DEBUG_WAIT_TIMEOUT"], + ) + + def test_debug_wait_timeout_defaults_to_ten_minutes(self): + with mock.patch.dict(os.environ, {}, clear=False): + os.environ.pop(DEBUG_WAIT_TIMEOUT_ENV, None) + leader, worker = self._debug_wait_timeouts() + + self.assertEqual(leader, "600") + self.assertEqual(worker, "600") + + def test_debug_wait_timeout_propagates_user_value_to_both_roles(self): + """Leader and workers must read the same window from one resolve. + + A worker waits the leader's window plus a buffer. If the two roles + disagreed, the workers would give up while the user was still + attached to the leader, and fail the job. + """ + with mock.patch.dict(os.environ, {DEBUG_WAIT_TIMEOUT_ENV: "1800"}): + leader, worker = self._debug_wait_timeouts() + + self.assertEqual(leader, "1800") + self.assertEqual(worker, "1800") + class TestSubmitPathwaysJob(absltest.TestCase): def setUp(self): diff --git a/kinetic/core/core.py b/kinetic/core/core.py index bfed6d3e..278dc2e5 100644 --- a/kinetic/core/core.py +++ b/kinetic/core/core.py @@ -232,9 +232,10 @@ def _require_interactive_terminal(): ``run(debug=True)`` blocks waiting for a VS Code debugger to attach. Without a TTY (CI, cron, nohup, piped input), no one can attach and - the job hangs for ``DEBUG_WAIT_TIMEOUT`` before falling through. - Fail fast with a clear message instead. Set - ``KINETIC_NO_TTY_DEBUG=1`` to override (useful for automated tests). + the job burns the whole attach window before falling through. Called + before the job is submitted so nothing lands on the cluster; fails + fast with a clear message instead. Set ``KINETIC_NO_TTY_DEBUG=1`` to + override (useful for automated tests). """ if os.environ.get("KINETIC_NO_TTY_DEBUG") == "1": return @@ -291,6 +292,12 @@ def _make_decorator( def decorator(func): @functools.wraps(func) def wrapper(*args, **kwargs): + # Checked before anything is submitted: a blocking debug call that + # nobody can attach to would otherwise leave a job on the cluster + # that waits out the whole attach window and then runs anyway. + if sync and debug: + _require_interactive_terminal() + env_vars = _capture_env(capture_env_vars) resolved_backend = _resolve_backend_name(accelerator, backend, spot=spot) @@ -334,7 +341,6 @@ def wrapper(*args, **kwargs): if sync: if debug: - _require_interactive_terminal() pf_proc = handle.debug_attach(working_dir=ctx.working_dir) try: return handle.result(stream_logs=False, cleanup=False) diff --git a/kinetic/core/core_test.py b/kinetic/core/core_test.py index acc10306..2a8324bc 100644 --- a/kinetic/core/core_test.py +++ b/kinetic/core/core_test.py @@ -316,7 +316,7 @@ def test_run_debug_raises_when_stdin_not_tty(self): mock.patch( "kinetic.core.core.submit_remote", return_value=mock_handle, - ), + ) as mock_submit, mock.patch( "kinetic.core.core.JobContext.from_params", return_value=MagicMock(), @@ -335,9 +335,43 @@ def func(): ): func() + # Nothing may reach the cluster: a submitted debug job that nobody + # can attach to sits there for the whole attach window, then runs. + mock_submit.assert_not_called() + # The debug attach path must not have been invoked. mock_handle.debug_attach.assert_not_called() + def test_run_async_debug_submits_without_tty(self): + """Only the blocking path needs a TTY; run_async() attaches later.""" + mock_handle = MagicMock() + with ( + mock.patch.dict( + os.environ, + _isolate_profile_env({"KINETIC_PROJECT": "proj"}), + clear=False, + ), + mock.patch( + "kinetic.core.core.submit_remote", + return_value=mock_handle, + ) as mock_submit, + mock.patch( + "kinetic.core.core.JobContext.from_params", + return_value=MagicMock(), + ), + mock.patch("sys.stdin.isatty", return_value=False), + ): + os.environ.pop("KINETIC_NO_TTY_DEBUG", None) + + @run(accelerator="cpu", debug=True) + def func(): + pass + + handle = func.run_async() + + self.assertIs(handle, mock_handle) + mock_submit.assert_called_once() + def test_run_debug_allowed_when_stdin_is_tty(self): mock_handle = MagicMock() mock_handle.result.return_value = 7 diff --git a/kinetic/debug.py b/kinetic/debug.py index d39f70e8..15bb9930 100644 --- a/kinetic/debug.py +++ b/kinetic/debug.py @@ -5,6 +5,7 @@ """ import contextlib +import json import os import subprocess import tempfile @@ -23,11 +24,12 @@ # of the box. DEBUGPY_PORT = 5678 -# Single source of truth for the debugger attach timeout (seconds). -# Covers pod scheduling + debugpy install + time for the user to -# attach. Propagated to the pod via the KINETIC_DEBUG_WAIT_TIMEOUT -# env var so remote_runner.py uses the same value. -DEBUG_WAIT_TIMEOUT = 600 +# Environment variable that overrides the debugger attach window. +DEBUG_WAIT_TIMEOUT_ENV = "KINETIC_DEBUG_WAIT_TIMEOUT" + +# Default debugger attach timeout (seconds). Covers pod scheduling + +# debugpy install + time for the user to attach. +DEFAULT_DEBUG_WAIT_TIMEOUT = 600 _TERMINAL_STATUSES = frozenset( {JobStatus.SUCCEEDED, JobStatus.FAILED, JobStatus.NOT_FOUND} @@ -37,6 +39,37 @@ _PORT_FORWARD_STARTUP_SECONDS = 2 +def resolve_debug_wait_timeout(): + """Return the debugger attach window in seconds. + + Single source of truth for both sides of a debug session: the client + uses the value for its own wait in ``wait_for_debug_server``, and the + backends put it in the pod's ``KINETIC_DEBUG_WAIT_TIMEOUT`` env var so + remote_runner.py waits for exactly as long. + + Returns: + The value of ``KINETIC_DEBUG_WAIT_TIMEOUT`` when it is a positive + whole number of seconds, ``DEFAULT_DEBUG_WAIT_TIMEOUT`` otherwise. + """ + raw = os.environ.get(DEBUG_WAIT_TIMEOUT_ENV) + if not raw: + return DEFAULT_DEBUG_WAIT_TIMEOUT + try: + seconds = int(raw) + except ValueError: + seconds = 0 + if seconds > 0: + return seconds + logging.warning( + "Ignoring invalid %s=%r (expected a positive whole number of seconds); " + "using %ds.", + DEBUG_WAIT_TIMEOUT_ENV, + raw, + DEFAULT_DEBUG_WAIT_TIMEOUT, + ) + return DEFAULT_DEBUG_WAIT_TIMEOUT + + def start_port_forward(pod_name, namespace, local_port, remote_port): """Start kubectl port-forward as a background subprocess. @@ -115,12 +148,42 @@ def start_port_forward(pod_name, namespace, local_port, remote_port): def print_attach_instructions(local_port, working_dir=None): """Print VS Code launch.json snippet for attaching to the remote debugger. + The pod reconstructs the client's own working-directory path (the + runner symlinks it to the extracted workspace), so source files carry + identical paths on both sides and the mapping is the identity. When + the working directory is unknown the mapping is left out entirely, + which is what debugpy already does with unmapped paths. + Args: local_port: Local port where debugpy is forwarded. working_dir: Local working directory for path mappings. If None, - uses a placeholder. + no pathMappings entry is emitted. """ - local_root = working_dir or "${workspaceFolder}" + config_lines = [ + ' "name": "Kinetic Debug",', + ' "type": "debugpy",', + ' "request": "attach",', + f' "connect": {{"host": "localhost", "port": {local_port}}}', + ] + if working_dir: + # os.fspath because debug_attach() is public API and a caller can + # hand it a pathlib.Path, which json.dumps cannot serialize. This + # runs after start_port_forward(), so raising here would leak the + # kubectl subprocess. json.dumps then keeps a path that holds a + # backslash or a quote valid in the printed snippet. + root = json.dumps(os.fspath(working_dir)) + config_lines[-1] += "," + config_lines.extend( + [ + ' "pathMappings": [', + " {", + f' "localRoot": {root},', + f' "remoteRoot": {root}', + " }", + " ]", + ] + ) + # Use print() rather than logging.info() — these are user-facing # instructions that must appear exactly once on stdout. The logging # subsystem can duplicate messages when multiple handlers are @@ -138,31 +201,45 @@ def print_attach_instructions(local_port, working_dir=None): '"create a launch.json file", then replace its contents.', "", " {", - ' "name": "Kinetic Debug",', - ' "type": "debugpy",', - ' "request": "attach",', - f' "connect": {{"host": "localhost", "port": {local_port}}},', - ' "pathMappings": [', - " {", - f' "localRoot": "{local_root}",', - ' "remoteRoot": "/tmp/workspace"', - " }", - " ]", + *config_lines, " }", "", - "Set your breakpoints, then start debugging with F5 or", - "via the menu: Run > Start Debugging.", - "", - "The debugger will pause inside the Kinetic runner before", - "your function is called. Press Step Into (F11) to enter", - "your function, or Step Over (F10) to run it directly.", - "=" * 50, - "", ] + if working_dir: + lines.extend( + [ + "Kinetic mirrors your local paths on the pod, so localRoot", + "and remoteRoot are the same directory. Edit both if you", + "open the sources from somewhere else.", + "", + ] + ) + else: + lines.extend( + [ + "The remote paths match your local ones, so no pathMappings", + "entry is needed. If your sources are somewhere else, add a", + 'mapping with "localRoot" set to that directory and', + '"remoteRoot" set to the directory you submitted from.', + "", + ] + ) + lines.extend( + [ + "Set your breakpoints, then start debugging with F5 or", + "via the menu: Run > Start Debugging.", + "", + "The debugger will pause inside the Kinetic runner before", + "your function is called. Press Step Into (F11) to enter", + "your function, or Step Over (F10) to run it directly.", + "=" * 50, + "", + ] + ) print("\n".join(lines)) # noqa: T201 -def wait_for_debug_server(handle, timeout=DEBUG_WAIT_TIMEOUT, poll_interval=5): +def wait_for_debug_server(handle, timeout=None, poll_interval=5): """Poll GCS sentinel until the debugpy server confirms readiness. Logs progress as the job transitions through states so the user @@ -170,13 +247,22 @@ def wait_for_debug_server(handle, timeout=DEBUG_WAIT_TIMEOUT, poll_interval=5): Args: handle: A JobHandle instance. - timeout: Maximum seconds to wait. + timeout: Maximum seconds to wait. Defaults to the window from + ``resolve_debug_wait_timeout()``, so raising + ``KINETIC_DEBUG_WAIT_TIMEOUT`` extends this wait and the pod's + attach wait together rather than only one of the two. The two + waits are consecutive: this one covers pod scheduling and the + debugpy install, and ends when the pod publishes its readiness + sentinel; the pod's own window for the debugger to attach + starts there. poll_interval: Seconds between log polls. Raises: TimeoutError: If the signal is not found within timeout. RuntimeError: If the job reaches a terminal state before the signal. """ + if timeout is None: + timeout = resolve_debug_wait_timeout() deadline = time.monotonic() + timeout last_status = None while time.monotonic() < deadline: diff --git a/kinetic/debug_test.py b/kinetic/debug_test.py new file mode 100644 index 00000000..28aca5db --- /dev/null +++ b/kinetic/debug_test.py @@ -0,0 +1,168 @@ +"""Tests for kinetic.debug — attach instructions and the attach window.""" + +import contextlib +import io +import json +import os +import pathlib +from unittest import mock + +from absl.testing import absltest, parameterized + +from kinetic.debug import ( + DEBUG_WAIT_TIMEOUT_ENV, + DEFAULT_DEBUG_WAIT_TIMEOUT, + print_attach_instructions, + resolve_debug_wait_timeout, +) + + +def _capture(local_port=5678, working_dir=None): + """Return the text print_attach_instructions() writes to stdout.""" + buf = io.StringIO() + with contextlib.redirect_stdout(buf): + print_attach_instructions(local_port, working_dir) + return buf.getvalue() + + +def _launch_config(output): + """Parse the launch.json object out of the printed instructions. + + The snippet is printed for the user to paste into VS Code, so it has + to be valid JSON. Parsing it here is the assertion: a malformed + snippet fails the test instead of failing in the user's editor. + """ + lines = output.splitlines() + start = lines.index(" {") + end = lines.index(" }", start) + return json.loads("\n".join(lines[start : end + 1])) + + +class TestPrintAttachInstructions(parameterized.TestCase): + """The printed pathMappings must match what the runner actually does. + + Regression test for the snippet that hardcoded + ``"remoteRoot": "/tmp/workspace"``. Since the runner extracts the + workspace under a ``tempfile.mkdtemp(prefix="kinetic-run-")`` + directory and symlinks the client's own working_dir at it, that + literal path is never where the sources are, and breakpoints set + against it never bind. + """ + + def test_working_dir_gives_identity_mapping(self): + working_dir = "/Users/dev/project" + config = _launch_config(_capture(working_dir=working_dir)) + + self.assertEqual( + config["pathMappings"], + [{"localRoot": working_dir, "remoteRoot": working_dir}], + ) + + def test_never_prints_stale_tmp_workspace_root(self): + """The pod has no /tmp/workspace; it must not appear anywhere.""" + self.assertNotIn("/tmp/workspace", _capture(working_dir="/Users/dev/proj")) + self.assertNotIn("/tmp/workspace", _capture()) + + def test_no_working_dir_omits_path_mappings(self): + """`kinetic jobs debug` has no working_dir; emit no mapping at all. + + An unmapped path is what debugpy already assumes, so leaving the + entry out is better than printing a root that is a guess. + """ + output = _capture() + config = _launch_config(output) + + self.assertNotIn("pathMappings", config) + # The old snippet fell back to this VS Code variable, which resolves + # to whatever folder happens to be open — not the submit directory. + self.assertNotIn("${workspaceFolder}", output) + + @parameterized.named_parameters( + ("backslash", "/home/dev/we\\ird"), + ("double_quote", '/home/dev/we"ird'), + ("windows_style", r"C:\Users\dev\project"), + ) + def test_path_needing_escapes_stays_valid_json(self, working_dir): + """A raw path would break the snippet the user has to paste. + + POSIX allows a backslash and a quote in a directory name, so this + is not only about Windows clients. + """ + config = _launch_config(_capture(working_dir=working_dir)) + + self.assertEqual( + config["pathMappings"], + [{"localRoot": working_dir, "remoteRoot": working_dir}], + ) + + def test_pathlib_working_dir_is_accepted(self): + """debug_attach() is public API, so a caller can pass a Path. + + json.dumps() cannot serialize a Path. A raise here would abort + debug_attach() after start_port_forward() launched kubectl, which + leaks the port-forward subprocess with no handle to clean it up. + """ + working_dir = pathlib.Path("/Users/dev/project") + config = _launch_config(_capture(working_dir=working_dir)) + + self.assertEqual( + config["pathMappings"], + [{"localRoot": str(working_dir), "remoteRoot": str(working_dir)}], + ) + + def test_snippet_carries_the_forwarded_port(self): + for working_dir in (None, "/Users/dev/project"): + with self.subTest(working_dir=working_dir): + config = _launch_config(_capture(4242, working_dir)) + + self.assertEqual(config["connect"], {"host": "localhost", "port": 4242}) + self.assertEqual(config["type"], "debugpy") + self.assertEqual(config["request"], "attach") + + +@contextlib.contextmanager +def _attach_window(value): + """Set (or unset, when value is None) the attach-window env var.""" + with mock.patch.dict(os.environ, {}, clear=False): + if value is None: + os.environ.pop(DEBUG_WAIT_TIMEOUT_ENV, None) + else: + os.environ[DEBUG_WAIT_TIMEOUT_ENV] = value + yield + + +class TestResolveDebugWaitTimeout(parameterized.TestCase): + """KINETIC_DEBUG_WAIT_TIMEOUT is read on the client, not just the pod.""" + + def test_default_is_ten_minutes(self): + """Locks the value documented in docs/configuration.md.""" + self.assertEqual(DEFAULT_DEBUG_WAIT_TIMEOUT, 600) + + def test_unset_uses_default(self): + with _attach_window(None): + self.assertEqual(resolve_debug_wait_timeout(), 600) + + @parameterized.named_parameters( + ("half_a_minute", "30", 30), + ("thirty_minutes", "1800", 1800), + ("surrounding_whitespace", " 900 ", 900), + ) + def test_positive_value_is_honored(self, raw, expected): + with _attach_window(raw): + self.assertEqual(resolve_debug_wait_timeout(), expected) + + @parameterized.named_parameters( + ("empty", ""), + ("zero", "0"), + ("negative", "-59"), + ("not_a_number", "ten minutes"), + ("fractional", "12.5"), + ) + def test_invalid_value_falls_back_to_default(self, raw): + """A bad value must not disable the wait or crash the submit.""" + with _attach_window(raw): + self.assertEqual(resolve_debug_wait_timeout(), DEFAULT_DEBUG_WAIT_TIMEOUT) + + +if __name__ == "__main__": + absltest.main() diff --git a/kinetic/jobs.py b/kinetic/jobs.py index 39d9d82f..45112344 100644 --- a/kinetic/jobs.py +++ b/kinetic/jobs.py @@ -372,7 +372,7 @@ def tail(self, n: int = 100) -> str: def debug_attach( self, local_port: int = DEBUGPY_PORT, - working_dir: str | None = None, + working_dir: str | os.PathLike[str] | None = None, ) -> subprocess.Popen: """Wait for debugpy, start port-forward, and print VS Code config. @@ -382,7 +382,8 @@ def debug_attach( Args: local_port: Local port to forward debugpy traffic to. working_dir: Local working directory for VS Code path mappings. - If None, a placeholder is used. + The pod mirrors this path, so the printed mapping is the + identity. If None, no pathMappings entry is printed. Returns: The ``subprocess.Popen`` handle for the kubectl port-forward diff --git a/kinetic/runner/remote_runner.py b/kinetic/runner/remote_runner.py index 37091d91..7ae1aabc 100644 --- a/kinetic/runner/remote_runner.py +++ b/kinetic/runner/remote_runner.py @@ -800,8 +800,10 @@ def _install_debugger(): # Fallback if KINETIC_DEBUG_WAIT_TIMEOUT env var is not set. -# The pod spec normally propagates DEBUG_WAIT_TIMEOUT from -# kinetic.debug as the env var, keeping both sides in sync. +# The pod spec normally sets the env var from kinetic.debug's +# resolve_debug_wait_timeout(), so the client and the pod wait out +# the same window. Keep this in sync with DEFAULT_DEBUG_WAIT_TIMEOUT +# there; the runner cannot import kinetic on the pod. _DEBUG_WAIT_TIMEOUT_DEFAULT = 600