From 91fdfe226a30fa5aca463699b7389fde8b0d8cc2 Mon Sep 17 00:00:00 2001 From: dmadisetti Date: Tue, 6 Jan 2026 13:37:27 -0800 Subject: [PATCH 1/2] fix: release script --- plugin/uv.lock | 2 +- scripts/release.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/plugin/uv.lock b/plugin/uv.lock index 5a2faa6..141a2af 100644 --- a/plugin/uv.lock +++ b/plugin/uv.lock @@ -185,7 +185,7 @@ wheels = [ [[package]] name = "kubectl-marimo" -version = "0.2.1" +version = "0.2.2" source = { editable = "." } dependencies = [ { name = "click" }, diff --git a/scripts/release.sh b/scripts/release.sh index bc56680..1f83def 100755 --- a/scripts/release.sh +++ b/scripts/release.sh @@ -136,7 +136,7 @@ fi # Commit version change print_step "Committing version change" -git add plugin/kubectl_marimo/__init__.py plugin/pyproject.toml +git add plugin/kubectl_marimo/__init__.py plugin/pyproject.toml plugin/uv.lock git commit -m "release: v$NEW_VERSION" # Push changes From 72226e75d35d0cba56d0127a5297543e5567386c Mon Sep 17 00:00:00 2001 From: dmadisetti Date: Tue, 6 Jan 2026 14:56:49 -0800 Subject: [PATCH 2/2] fix: proper teardown on pod stop --- plugin/examples/basic.py | 7 +- plugin/examples/getting-started.py | 24 ++-- plugin/examples/with-cw.py | 8 +- plugin/examples/with-rsync.py | 4 - plugin/examples/with-sshfs.py | 3 +- plugin/examples/with-storage.py | 3 +- plugin/kubectl_marimo/deploy.py | 33 ++++- plugin/kubectl_marimo/k8s.py | 21 ++++ plugin/tests/test_delete.py | 58 +++++++++ plugin/tests/test_deploy.py | 189 +++++++++++++++++++++++++++++ 10 files changed, 317 insertions(+), 33 deletions(-) diff --git a/plugin/examples/basic.py b/plugin/examples/basic.py index 0f8ceaf..e2075a4 100644 --- a/plugin/examples/basic.py +++ b/plugin/examples/basic.py @@ -1,14 +1,9 @@ import marimo -__generated_with = "0.16.4" +__generated_with = "0.18.4" app = marimo.App() -@app.cell -def hello(): - return - - @app.cell def _(): print("In notebook.py") diff --git a/plugin/examples/getting-started.py b/plugin/examples/getting-started.py index 3a10ad1..75e55ac 100644 --- a/plugin/examples/getting-started.py +++ b/plugin/examples/getting-started.py @@ -11,17 +11,15 @@ app = marimo.App() -@app.cell -def _(): - import marimo as mo - +@app.cell(hide_code=True) +def _(mo): mo.md(""" # Welcome to marimo on Kubernetes! This notebook introduces marimo's **reactive execution model** — if you're coming from Jupyter, this is the key difference to understand. """) - return (mo,) + return @app.cell @@ -31,9 +29,10 @@ def _(mo): return (slider,) -@app.cell +@app.cell(hide_code=True) def _(mo, slider): - mo.md(f""" + mo.md( + f""" ## Reactive execution You picked **{slider.value}**. Try moving the slider above! @@ -48,7 +47,8 @@ def _(mo, slider): 2. **Cells run in dependency order** — Not top-to-bottom, but based on which variables each cell uses 3. **No cell numbers** — Order on the page doesn't determine execution order - """) + """ + ) return @@ -66,7 +66,7 @@ def _(mo, slider): return -@app.cell +@app.cell(hide_code=True) def _(mo): mo.md(""" ## Next steps @@ -80,5 +80,11 @@ def _(mo): return +@app.cell +def _(): + import marimo as mo + return (mo,) + + if __name__ == "__main__": app.run() diff --git a/plugin/examples/with-cw.py b/plugin/examples/with-cw.py index cddbc52..1ef2752 100644 --- a/plugin/examples/with-cw.py +++ b/plugin/examples/with-cw.py @@ -1,8 +1,8 @@ # /// script # dependencies = ["marimo"] -# /// # [tool.marimo.k8s] # mounts = ["cw://operator-bucket"] +# /// # Note: cw-credentials secret is auto-created from ~/.s3cfg by the plugin import marimo @@ -33,13 +33,11 @@ def check_mount(): result = subprocess.run(["ps", "aux"], capture_output=True, text=True) if "s3fs" in result.stdout: print("\ns3fs process is running!") - return + return (os,) @app.cell -def read_test_file(): - import os - +def read_test_file(os): test_file = "/home/marimo/notebooks/mounts/cw-0/test-data.txt" if os.path.exists(test_file): with open(test_file) as f: diff --git a/plugin/examples/with-rsync.py b/plugin/examples/with-rsync.py index 1b47c89..c725bde 100644 --- a/plugin/examples/with-rsync.py +++ b/plugin/examples/with-rsync.py @@ -11,11 +11,7 @@ app = marimo.App() -@app.cell -def _(): - import marimo as mo - return if __name__ == "__main__": diff --git a/plugin/examples/with-sshfs.py b/plugin/examples/with-sshfs.py index 84026b9..26955a2 100644 --- a/plugin/examples/with-sshfs.py +++ b/plugin/examples/with-sshfs.py @@ -14,11 +14,10 @@ @app.cell def check_mount(): import os - import marimo as mo mount_path = "/home/marimo/notebooks/mounts/sshfs-0" exists = os.path.exists(mount_path) - files = os.listdir(mount_path) if exists else [] + os.listdir(mount_path) if exists else [] return diff --git a/plugin/examples/with-storage.py b/plugin/examples/with-storage.py index 2263b71..d17d894 100644 --- a/plugin/examples/with-storage.py +++ b/plugin/examples/with-storage.py @@ -13,10 +13,9 @@ @app.cell def check(): import os - import marimo as mo path = "/home/marimo/notebooks" - files = os.listdir(path) if os.path.exists(path) else [] + os.listdir(path) if os.path.exists(path) else [] return diff --git a/plugin/kubectl_marimo/deploy.py b/plugin/kubectl_marimo/deploy.py index b555a3f..0fb614a 100644 --- a/plugin/kubectl_marimo/deploy.py +++ b/plugin/kubectl_marimo/deploy.py @@ -13,9 +13,9 @@ import click from .formats import parse_file -from .k8s import apply_resource +from .k8s import apply_resource, delete_resource, resource_exists from .resources import build_marimo_notebook, resource_name, compute_hash, to_yaml -from .swap import read_swap_file, write_swap_file, create_swap_meta +from .swap import read_swap_file, write_swap_file, create_swap_meta, delete_swap_file from .sync import sync_notebook @@ -401,9 +401,17 @@ def deploy_notebook( click.echo("Error: SSH pubkey required for sshfs mounts", err=True) sys.exit(1) - # Apply to cluster - if not apply_resource(resource): - sys.exit(1) + # Check if resource already exists + already_exists = resource_exists("marimos.marimo.io", name, namespace) + + if already_exists: + click.echo( + click.style(f"Pod '{name}' already exists. Reconnecting...", fg="yellow") + ) + else: + # Apply to cluster + if not apply_resource(resource): + sys.exit(1) # Handle rsync mounts - need to wait for pod ready first if rsync_mounts: @@ -573,6 +581,21 @@ def open_notebook( sync_notebook(file_path, namespace=namespace, force=True) except Exception as e: click.echo(f"Warning: Sync failed: {e}", err=True) + + # Prompt user about teardown + keep_running = click.confirm("Keep pod running?", default=False) + if not keep_running: + click.echo("Tearing down pod...") + try: + delete_resource("marimos.marimo.io", name, namespace) + delete_swap_file(file_path) + except Exception as e: + click.echo(f"Warning: Teardown failed: {e}", err=True) + else: + click.echo( + f"Pod '{name}' left running. Use 'kubectl-marimo delete' to remove later." + ) + click.echo("Done") diff --git a/plugin/kubectl_marimo/k8s.py b/plugin/kubectl_marimo/k8s.py index de4e4a6..3c1623c 100644 --- a/plugin/kubectl_marimo/k8s.py +++ b/plugin/kubectl_marimo/k8s.py @@ -61,6 +61,27 @@ def delete_resource( return False +def resource_exists(kind: str, name: str, namespace: str | None = None) -> bool: + """Check if a Kubernetes resource exists. + + Args: + kind: Resource kind (e.g., "marimos.marimo.io", "pod") + name: Resource name + namespace: Kubernetes namespace (None = use kubectl context) + + Returns: + True if resource exists, False otherwise + """ + cmd = ["kubectl", "get", kind, name, "-o", "name"] + if namespace is not None: + cmd.extend(["-n", namespace]) + try: + result = subprocess.run(cmd, capture_output=True, text=True) + return result.returncode == 0 + except FileNotFoundError: + return False + + def exec_in_pod( pod_name: str, namespace: str | None, diff --git a/plugin/tests/test_delete.py b/plugin/tests/test_delete.py index 3bb7bd9..38f9d55 100644 --- a/plugin/tests/test_delete.py +++ b/plugin/tests/test_delete.py @@ -82,6 +82,64 @@ def test_continues_if_patch_fails(self, notebook_file, mock_k8s, mock_swap, mock mock_k8s["delete_resource"].assert_called_once() +class TestResourceExists: + """Tests for resource_exists function.""" + + def test_resource_exists_returns_true(self, mocker): + """Returns True when resource exists.""" + from kubectl_marimo.k8s import resource_exists + + mock_run = mocker.patch("kubectl_marimo.k8s.subprocess.run") + mock_run.return_value.returncode = 0 + + result = resource_exists("marimos.marimo.io", "test-notebook", "default") + + assert result is True + args = mock_run.call_args[0][0] + assert "kubectl" in args + assert "get" in args + assert "marimos.marimo.io" in args + assert "test-notebook" in args + assert "-n" in args + assert "default" in args + + def test_resource_exists_returns_false_not_found(self, mocker): + """Returns False when resource doesn't exist.""" + from kubectl_marimo.k8s import resource_exists + + mock_run = mocker.patch("kubectl_marimo.k8s.subprocess.run") + mock_run.return_value.returncode = 1 + + result = resource_exists("marimos.marimo.io", "nonexistent", "default") + + assert result is False + + def test_resource_exists_no_namespace(self, mocker): + """Works without namespace argument.""" + from kubectl_marimo.k8s import resource_exists + + mock_run = mocker.patch("kubectl_marimo.k8s.subprocess.run") + mock_run.return_value.returncode = 0 + + result = resource_exists("marimos.marimo.io", "test-notebook") + + assert result is True + args = mock_run.call_args[0][0] + assert "-n" not in args + + def test_resource_exists_kubectl_not_found(self, mocker): + """Returns False when kubectl not in PATH.""" + from kubectl_marimo.k8s import resource_exists + + mocker.patch( + "kubectl_marimo.k8s.subprocess.run", side_effect=FileNotFoundError() + ) + + result = resource_exists("marimos.marimo.io", "test-notebook", "default") + + assert result is False + + class TestPatchResource: """Tests for patch_resource function.""" diff --git a/plugin/tests/test_deploy.py b/plugin/tests/test_deploy.py index 4431799..d7163aa 100644 --- a/plugin/tests/test_deploy.py +++ b/plugin/tests/test_deploy.py @@ -2,6 +2,7 @@ import socket +import pytest from kubectl_marimo.deploy import find_available_port, get_access_token @@ -71,3 +72,191 @@ def test_returns_none_on_empty_output(self, mocker): token = get_access_token("test", "default") assert token is None + + +class TestReconnectionAlert: + """Tests for reconnection alert when pod already exists.""" + + @pytest.fixture + def notebook_file(self, tmp_path): + """Create a temporary notebook file.""" + nb = tmp_path / "test_notebook.py" + nb.write_text( + "import marimo\napp = marimo.App()\n@app.cell\ndef _():\n pass" + ) + return nb + + @pytest.fixture + def mock_deploy_deps(self, mocker): + """Mock all deploy dependencies.""" + mocks = { + "apply_resource": mocker.patch( + "kubectl_marimo.deploy.apply_resource", return_value=True + ), + "resource_exists": mocker.patch( + "kubectl_marimo.deploy.resource_exists", return_value=False + ), + "wait_for_ready": mocker.patch( + "kubectl_marimo.deploy.wait_for_ready", return_value=True + ), + "write_swap_file": mocker.patch("kubectl_marimo.deploy.write_swap_file"), + "read_swap_file": mocker.patch( + "kubectl_marimo.deploy.read_swap_file", return_value=None + ), + "echo": mocker.patch("kubectl_marimo.deploy.click.echo"), + "style": mocker.patch( + "kubectl_marimo.deploy.click.style", side_effect=lambda x, **kw: x + ), + } + return mocks + + def test_shows_reconnection_alert_when_pod_exists( + self, notebook_file, mock_deploy_deps, mocker + ): + """Shows yellow reconnection message when pod already exists.""" + from kubectl_marimo.deploy import deploy_notebook + + mock_deploy_deps["resource_exists"].return_value = True + + deploy_notebook(str(notebook_file), namespace="default", headless=True) + + # Should show reconnection message + mock_deploy_deps["style"].assert_any_call(mocker.ANY, fg="yellow") + # Check the styled message contains "Reconnecting" + style_calls = mock_deploy_deps["style"].call_args_list + reconnect_calls = [c for c in style_calls if "Reconnecting" in str(c)] + assert len(reconnect_calls) > 0 + + # Should NOT call apply_resource when reconnecting + mock_deploy_deps["apply_resource"].assert_not_called() + + def test_creates_new_pod_when_not_exists(self, notebook_file, mock_deploy_deps): + """Creates new pod when resource doesn't exist.""" + from kubectl_marimo.deploy import deploy_notebook + + mock_deploy_deps["resource_exists"].return_value = False + + deploy_notebook(str(notebook_file), namespace="default", headless=True) + + # Should call apply_resource for new pod + mock_deploy_deps["apply_resource"].assert_called_once() + + +class TestTeardownPrompt: + """Tests for teardown prompt on Ctrl-C.""" + + @pytest.fixture + def mock_open_notebook_deps(self, mocker): + """Mock dependencies for open_notebook.""" + mocks = { + "wait_for_ready": mocker.patch( + "kubectl_marimo.deploy.wait_for_ready", return_value=True + ), + "get_access_token": mocker.patch( + "kubectl_marimo.deploy.get_access_token", return_value="token123" + ), + "find_available_port": mocker.patch( + "kubectl_marimo.deploy.find_available_port", return_value=2718 + ), + "webbrowser_open": mocker.patch("kubectl_marimo.deploy.webbrowser.open"), + "subprocess_run": mocker.patch( + "kubectl_marimo.deploy.subprocess.run", + side_effect=KeyboardInterrupt(), + ), + "sync_notebook": mocker.patch("kubectl_marimo.deploy.sync_notebook"), + "delete_resource": mocker.patch( + "kubectl_marimo.deploy.delete_resource", return_value=True + ), + "delete_swap_file": mocker.patch( + "kubectl_marimo.deploy.delete_swap_file", return_value=True + ), + "confirm": mocker.patch("kubectl_marimo.deploy.click.confirm"), + "echo": mocker.patch("kubectl_marimo.deploy.click.echo"), + } + return mocks + + def test_teardown_on_no_response(self, mock_open_notebook_deps, tmp_path): + """Tears down pod when user responds No (default).""" + from kubectl_marimo.deploy import open_notebook + + mock_open_notebook_deps["confirm"].return_value = False + notebook_file = tmp_path / "test.py" + notebook_file.write_text("# test") + + open_notebook("test-notebook", "default", 2718, str(notebook_file)) + + # Should sync changes + mock_open_notebook_deps["sync_notebook"].assert_called_once() + + # Should prompt user + mock_open_notebook_deps["confirm"].assert_called_once_with( + "Keep pod running?", default=False + ) + + # Should delete resource and swap file + mock_open_notebook_deps["delete_resource"].assert_called_once_with( + "marimos.marimo.io", "test-notebook", "default" + ) + mock_open_notebook_deps["delete_swap_file"].assert_called_once() + + def test_keeps_pod_on_yes_response(self, mock_open_notebook_deps, tmp_path): + """Keeps pod running when user responds Yes.""" + from kubectl_marimo.deploy import open_notebook + + mock_open_notebook_deps["confirm"].return_value = True + notebook_file = tmp_path / "test.py" + notebook_file.write_text("# test") + + open_notebook("test-notebook", "default", 2718, str(notebook_file)) + + # Should sync changes + mock_open_notebook_deps["sync_notebook"].assert_called_once() + + # Should prompt user + mock_open_notebook_deps["confirm"].assert_called_once() + + # Should NOT delete resource or swap file + mock_open_notebook_deps["delete_resource"].assert_not_called() + mock_open_notebook_deps["delete_swap_file"].assert_not_called() + + # Should show message about keeping pod running + echo_calls = [str(c) for c in mock_open_notebook_deps["echo"].call_args_list] + keep_running_msgs = [c for c in echo_calls if "left running" in c] + assert len(keep_running_msgs) > 0 + + def test_handles_teardown_failure_gracefully( + self, mock_open_notebook_deps, tmp_path + ): + """Continues gracefully if teardown fails.""" + from kubectl_marimo.deploy import open_notebook + + mock_open_notebook_deps["confirm"].return_value = False + mock_open_notebook_deps["delete_resource"].side_effect = Exception("k8s error") + notebook_file = tmp_path / "test.py" + notebook_file.write_text("# test") + + # Should not raise exception + open_notebook("test-notebook", "default", 2718, str(notebook_file)) + + # Should show warning + echo_calls = [str(c) for c in mock_open_notebook_deps["echo"].call_args_list] + warning_msgs = [c for c in echo_calls if "Warning" in c] + assert len(warning_msgs) > 0 + + def test_handles_sync_failure_gracefully(self, mock_open_notebook_deps, tmp_path): + """Continues to teardown prompt even if sync fails.""" + from kubectl_marimo.deploy import open_notebook + + mock_open_notebook_deps["sync_notebook"].side_effect = Exception("sync error") + mock_open_notebook_deps["confirm"].return_value = False + notebook_file = tmp_path / "test.py" + notebook_file.write_text("# test") + + # Should not raise exception + open_notebook("test-notebook", "default", 2718, str(notebook_file)) + + # Should still prompt for teardown + mock_open_notebook_deps["confirm"].assert_called_once() + + # Should still attempt teardown + mock_open_notebook_deps["delete_resource"].assert_called_once()