fix: missing implementation

Author: dmadisetti

pkg/resources/pod_test.go

@@ -16,6 +16,8 @@ const (
 	testSetupVenv       = "setup-venv"
 	testSSHDContainer   = "sshd"
 	testSSHFSName       = "sshfs-0"
+	testCWSidecarName   = "cw-0"
+	testSSHPubkeyName   = "ssh-pubkey"
 )
 
 func TestBuildPod_BasicConfig(t *testing.T) {
@@ -1163,8 +1165,8 @@ func TestBuildPod_WithCWMounts(t *testing.T) {
 
 	// Second container should be cw sidecar
 	cwSidecar := pod.Spec.Containers[1]
-	if cwSidecar.Name != "cw-0" {
-		t.Errorf("expected sidecar name 'cw-0', got '%s'", cwSidecar.Name)
+	if cwSidecar.Name != testCWSidecarName {
+		t.Errorf("expected sidecar name '%s', got '%s'", testCWSidecarName, cwSidecar.Name)
 	}
 }
 
@@ -1208,8 +1210,8 @@ func TestExpandMounts_CW(t *testing.T) {
 	}
 
 	sidecar := sidecars[0]
-	if sidecar.Name != "cw-0" {
-		t.Errorf("expected name 'cw-0', got %q", sidecar.Name)
+	if sidecar.Name != testCWSidecarName {
+		t.Errorf("expected name '%s', got %q", testCWSidecarName, sidecar.Name)
 	}
 
 	if !strings.Contains(sidecar.Image, "s3fs") {
@@ -1335,7 +1337,7 @@ func TestBuildPod_MountPropagation_WithFUSESidecar(t *testing.T) {
 		if pod.Spec.Containers[i].Name == testMarimoContainer {
 			marimoContainer = &pod.Spec.Containers[i]
 		}
-		if pod.Spec.Containers[i].Name == "cw-0" {
+		if pod.Spec.Containers[i].Name == testCWSidecarName {
 			cwContainer = &pod.Spec.Containers[i]
 		}
 	}
@@ -1344,7 +1346,7 @@ func TestBuildPod_MountPropagation_WithFUSESidecar(t *testing.T) {
 		t.Fatal("marimo container not found")
 	}
 	if cwContainer == nil {
-		t.Fatal("cw-0 container not found")
+		t.Fatalf("%s container not found", testCWSidecarName)
 	}
 
 	// Check marimo has HostToContainer propagation on PVC mount
@@ -1442,16 +1444,16 @@ func TestBuildPod_SSHFSSidecar_SecretMount(t *testing.T) {
 	// Check ssh-pubkey volume exists
 	var foundSSHPubkeyVolume bool
 	for _, vol := range pod.Spec.Volumes {
-		if vol.Name == "ssh-pubkey" {
-			if vol.Secret == nil || vol.Secret.SecretName != "ssh-pubkey" {
-				t.Error("ssh-pubkey volume should reference ssh-pubkey secret")
+		if vol.Name == testSSHPubkeyName {
+			if vol.Secret == nil || vol.Secret.SecretName != testSSHPubkeyName {
+				t.Errorf("%s volume should reference %s secret", testSSHPubkeyName, testSSHPubkeyName)
 			}
 			foundSSHPubkeyVolume = true
 			break
 		}
 	}
 	if !foundSSHPubkeyVolume {
-		t.Error("expected ssh-pubkey volume to be present for sshfs sidecar")
+		t.Errorf("expected %s volume to be present for sshfs sidecar", testSSHPubkeyName)
 	}
 
 	// Find sshfs sidecar and check it has the secret mounted
@@ -1470,7 +1472,7 @@ func TestBuildPod_SSHFSSidecar_SecretMount(t *testing.T) {
 	// Check ssh-pubkey is mounted at /config/ssh-pubkey
 	var foundSSHPubkeyMount bool
 	for _, vm := range sshfsSidecar.VolumeMounts {
-		if vm.Name == "ssh-pubkey" && vm.MountPath == "/config/ssh-pubkey" && vm.ReadOnly {
+		if vm.Name == testSSHPubkeyName && vm.MountPath == "/config/"+testSSHPubkeyName && vm.ReadOnly {
 			foundSSHPubkeyMount = true
 			break
 		}
@@ -1500,8 +1502,8 @@ func TestBuildPod_NoSSHFSSidecar_NoSecretMount(t *testing.T) {
 
 	// Check ssh-pubkey volume does NOT exist
 	for _, vol := range pod.Spec.Volumes {
-		if vol.Name == "ssh-pubkey" {
-			t.Error("ssh-pubkey volume should NOT be present when no sshfs sidecar")
+		if vol.Name == testSSHPubkeyName {
+			t.Errorf("%s volume should NOT be present when no sshfs sidecar", testSSHPubkeyName)
 		}
 	}
 }

plugin/README.md

@@ -106,7 +106,7 @@ kubectl marimo delete [OPTIONS] FILE
 
 Options:
 - `-n, --namespace` - Kubernetes namespace
-- `--keep-pvc` - Preserve persistent storage
+- `--delete-pvc` - Also delete PersistentVolumeClaim (PVC is preserved by default)
 - `--no-sync` - Delete without syncing changes back
 
 ### status

plugin/examples/with-rsync.py

@@ -13,8 +13,6 @@
 
 @app.cell
 def check_sync():
-    import os
-    import marimo as mo
 
     return
 

plugin/kubectl_marimo/delete.py

@@ -6,7 +6,7 @@
 import click
 
 from .formats import parse_file
-from .k8s import delete_resource, exec_in_pod
+from .k8s import delete_resource, exec_in_pod, patch_resource
 from .resources import compute_hash, resource_name, detect_content_type
 from .swap import read_swap_file, delete_swap_file
 from .sync import sync_local_mounts
@@ -17,7 +17,7 @@ def delete_notebook(
     namespace: str | None = None,
     force: bool = False,
     no_sync: bool = False,
-    keep_pvc: bool = False,
+    delete_pvc: bool = False,
 ) -> None:
     """Delete notebook deployment from cluster."""
     path = Path(file_path)
@@ -77,14 +77,19 @@ def delete_notebook(
         if meta.local_mounts:
             sync_local_mounts(meta.name, namespace, meta.local_mounts)
 
-    # Delete the MarimoNotebook resource
-    # Note: PVC is deleted via owner reference unless keep_pvc is set
-    if keep_pvc:
-        click.echo("Note: --keep-pvc requires manual PVC deletion prevention")
-        click.echo(
-            f'      kubectl patch pvc -n {namespace} {name}-pvc -p \'{{"metadata":{{"ownerReferences":[]}}}}\''
-        )
+    # By default, preserve PVC by removing owner references before delete
+    # With --delete-pvc, skip patching so PVC is garbage collected
+    if not delete_pvc:
+        pvc_name = f"{name}-pvc"
+        patch_json = '{"metadata":{"ownerReferences":null}}'
+        if not patch_resource("pvc", pvc_name, namespace, patch_json):
+            click.echo(
+                "Warning: Could not patch PVC to remove owner references. "
+                "PVC may be deleted with the notebook.",
+                err=True,
+            )
 
+    # Delete the MarimoNotebook resource
     if not delete_resource("marimos.marimo.io", name, namespace):
         sys.exit(1)
 

plugin/kubectl_marimo/k8s.py

@@ -100,6 +100,39 @@ def get_pod_logs(pod_name: str, namespace: str) -> tuple[bool, str]:
         return False, "kubectl not found in PATH"
 
 
+def patch_resource(
+    kind: str,
+    name: str,
+    namespace: str,
+    patch: str,
+) -> bool:
+    """Patch a Kubernetes resource using kubectl.
+
+    Returns True on success, False on failure.
+    """
+    cmd = [
+        "kubectl",
+        "patch",
+        kind,
+        name,
+        "-n",
+        namespace,
+        "--type=merge",
+        "-p",
+        patch,
+    ]
+    try:
+        result = subprocess.run(cmd, capture_output=True, text=True)
+        if result.returncode != 0:
+            print(f"Error: {result.stderr}", file=sys.stderr)
+            return False
+        print(result.stdout, end="")
+        return True
+    except FileNotFoundError:
+        print("Error: kubectl not found in PATH", file=sys.stderr)
+        return False
+
+
 def get_resource(
     kind: str,
     name: str,

plugin/kubectl_marimo/main.py

@@ -108,15 +108,18 @@ def sync(file: str, namespace: str | None, force: bool):
     "-n", "--namespace", help="Kubernetes namespace (default: from swap file)"
 )
 @click.option(
-    "--keep-pvc", is_flag=True, help="Keep PersistentVolumeClaim (preserve data)"
+    "--delete-pvc",
+    is_flag=True,
+    help="Also delete PersistentVolumeClaim (destroys data)",
 )
 @click.option("--no-sync", is_flag=True, help="Delete without syncing changes back")
-def delete(file: str, namespace: str | None, keep_pvc: bool, no_sync: bool):
+def delete(file: str, namespace: str | None, delete_pvc: bool, no_sync: bool):
     """Sync changes, then delete cluster resources.
 
     FILE is the local notebook that was previously deployed.
+    PVC is preserved by default to protect your data.
     """
-    delete_notebook(file, namespace=namespace, keep_pvc=keep_pvc, no_sync=no_sync)
+    delete_notebook(file, namespace=namespace, delete_pvc=delete_pvc, no_sync=no_sync)
 
 
 @cli.command()