Skip to content

Add detached reboot for deploy/uninstall to prevent timeouts #67

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jamesyc merged 1 commit into from
Apr 29, 2026
Merged

Add detached reboot for deploy/uninstall to prevent timeouts #67

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/timecapsulesmb/cli/deploy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from timecapsulesmb.deploy.executor import (
remote_ensure_adisk_uuid,
remote_install_auth_files,
remote_request_reboot,
run_remote_actions,
upload_deployment_payload,
)
Expand All @@ -32,7 +33,6 @@
from timecapsulesmb.device.compat import is_netbsd4_payload_family, payload_family_description, render_compatibility_message
from timecapsulesmb.device.probe import build_device_paths, discover_volume_root_conn, wait_for_ssh_state_conn
from timecapsulesmb.telemetry import TelemetryClient
from timecapsulesmb.transport.ssh import run_ssh
from timecapsulesmb.cli.util import NETBSD4_REBOOT_FOLLOWUP, NETBSD4_REBOOT_GUIDANCE, color_green, color_red


Expand Down Expand Up @@ -233,8 +233,8 @@ def main(argv: Optional[list[str]] = None) -> int:
return 0

command_context.set_stage("reboot")
run_ssh(connection, "/sbin/reboot", check=False)
command_context.update_fields(reboot_was_attempted=True)
remote_request_reboot(connection)
print("Reboot requested. Waiting for the device to go down...")
command_context.set_stage("wait_for_reboot_down")
wait_for_ssh_state_conn(connection, expected_up=False, timeout_seconds=60)
Expand Down
5 changes: 2 additions & 3 deletions src/timecapsulesmb/cli/uninstall.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,10 @@
from timecapsulesmb.cli.runtime import load_env_values, resolve_env_connection
from timecapsulesmb.core.config import require_valid_config
from timecapsulesmb.deploy.dry_run import format_uninstall_plan, uninstall_plan_to_jsonable
from timecapsulesmb.deploy.executor import remote_uninstall_payload
from timecapsulesmb.deploy.executor import remote_request_reboot, remote_uninstall_payload
from timecapsulesmb.deploy.planner import build_uninstall_plan
from timecapsulesmb.deploy.verify import verify_post_uninstall
from timecapsulesmb.device.probe import build_device_paths, discover_volume_root_conn, wait_for_ssh_state_conn
from timecapsulesmb.transport.ssh import run_ssh


def main(argv: Optional[list[str]] = None) -> int:
Expand Down Expand Up @@ -58,7 +57,7 @@ def main(argv: Optional[list[str]] = None) -> int:
print("Skipped reboot. The Time Capsule may need a manual reboot to fully clear running processes.")
return 0

run_ssh(connection, "/sbin/reboot", check=False)
remote_request_reboot(connection)
print("Reboot requested. Waiting for the device to go down...")
wait_for_ssh_state_conn(connection, expected_up=False, timeout_seconds=60)
print("Waiting for the device to come back up...")
Expand Down
22 changes: 22 additions & 0 deletions src/timecapsulesmb/deploy/dry_run.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,27 @@

from timecapsulesmb.cli.util import NETBSD4_REBOOT_FOLLOWUP, NETBSD4_REBOOT_GUIDANCE
from timecapsulesmb.deploy.commands import render_remote_actions
from timecapsulesmb.deploy.executor import DETACHED_REBOOT_COMMAND
from timecapsulesmb.deploy.planner import ActivationPlan, DeploymentPlan, UninstallPlan


def _append_reboot_request(lines: list[str], reboot_required: bool) -> None:
if not reboot_required:
return
lines.append(f" request: {DETACHED_REBOOT_COMMAND}")
lines.append(" follow-up: wait for SSH down, then SSH up")


def _add_reboot_request_json(data: dict[str, object], reboot_required: bool) -> None:
if not reboot_required:
return
data["reboot_request"] = {
"mode": "detached_ssh",
"command": DETACHED_REBOOT_COMMAND,
"follow_up": ["wait_for_ssh_down", "wait_for_ssh_up"],
}


def format_deployment_plan(plan: DeploymentPlan) -> str:
lines: list[str] = []
lines.append("Dry run: deployment plan")
Expand Down Expand Up @@ -42,6 +60,7 @@ def format_deployment_plan(plan: DeploymentPlan) -> str:
lines.append("")
lines.append("Reboot:")
lines.append(f" {'yes' if plan.reboot_required else 'no'}")
_append_reboot_request(lines, plan.reboot_required)
if plan.activation_actions:
lines.append(" Deploy will activate Samba immediately without rebooting.")
lines.append(f" {NETBSD4_REBOOT_GUIDANCE}")
Expand All @@ -61,6 +80,7 @@ def deployment_plan_to_jsonable(plan: DeploymentPlan) -> dict[str, object]:
data["smbd_path"] = str(plan.smbd_path)
data["mdns_path"] = str(plan.mdns_path)
data["nbns_path"] = str(plan.nbns_path)
_add_reboot_request_json(data, plan.reboot_required)
return data


Expand Down Expand Up @@ -99,6 +119,7 @@ def format_uninstall_plan(plan: UninstallPlan) -> str:
lines.append("")
lines.append("Reboot:")
lines.append(f" {'yes' if plan.reboot_required else 'no'}")
_append_reboot_request(lines, plan.reboot_required)
lines.append("")
lines.append("Post-uninstall checks:")
if plan.post_uninstall_checks:
Expand All @@ -111,4 +132,5 @@ def format_uninstall_plan(plan: UninstallPlan) -> str:

def uninstall_plan_to_jsonable(plan: UninstallPlan) -> dict[str, object]:
data = asdict(plan)
_add_reboot_request_json(data, plan.reboot_required)
return data
7 changes: 7 additions & 0 deletions src/timecapsulesmb/deploy/executor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
from timecapsulesmb.transport.ssh import SshConnection, run_scp, run_ssh


DETACHED_REBOOT_COMMAND = "/bin/sh -c '(/bin/sleep 1; /sbin/reboot) >/dev/null 2>&1 &'"


def remote_prepare_dirs(connection: SshConnection, payload_dir: str) -> None:
run_ssh(connection, render_remote_action(prepare_dirs_action(payload_dir)))

Expand Down Expand Up @@ -94,6 +97,10 @@ def run_remote_actions(connection: SshConnection, actions) -> None:
run_ssh(connection, command)


def remote_request_reboot(connection: SshConnection) -> None:
run_ssh(connection, DETACHED_REBOOT_COMMAND, check=False, timeout=10)
Comment thread
jamesyc marked this conversation as resolved.


def remote_uninstall_payload(connection: SshConnection, plan: UninstallPlan) -> None:
# Use for loop to avoid rc=255 bug on NetBSD 4 Time Capsules
for command in render_remote_actions(plan.remote_actions):
Expand Down
46 changes: 35 additions & 11 deletions tests/test_cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
RemoteInterfaceCandidatesProbeResult,
RemoteInterfaceProbeResult,
)
from timecapsulesmb.deploy.executor import DETACHED_REBOOT_COMMAND
from timecapsulesmb.deploy.templates import DEFAULT_APPLE_MOUNT_WAIT_SECONDS
from timecapsulesmb.transport.ssh import SshConnection
from timecapsulesmb.discovery.bonjour import BonjourDiscoverySnapshot, BonjourServiceInstance, Discovered
Expand Down Expand Up @@ -3970,6 +3971,8 @@ def test_deploy_dry_run_prints_target_host(self) -> None:
self.assertIn("volume root: /Volumes/dk2", text)
self.assertIn(f"Apple mount wait: {DEFAULT_APPLE_MOUNT_WAIT_SECONDS}s", text)
self.assertIn("generated smbpasswd", text)
self.assertIn(f"request: {DETACHED_REBOOT_COMMAND}", text)
self.assertIn("follow-up: wait for SSH down, then SSH up", text)
self.assertIn("SSH goes down after reboot request", text)
self.assertIn("SSH returns after reboot", text)
self.assertIn("managed runtime smb.conf is present", text)
Expand All @@ -3994,6 +3997,8 @@ def test_deploy_dry_run_no_reboot_matches_no_reboot_execution_path(self) -> None
text = output.getvalue()
self.assertIn("Reboot:\n no", text)
self.assertIn("Post-deploy checks:\n none", text)
self.assertNotIn(DETACHED_REBOOT_COMMAND, text)
self.assertNotIn("detached_ssh", text)
self.assertNotIn("SSH returns after reboot", text)
self.assertNotIn("smbd is bound to TCP 445", text)

Expand Down Expand Up @@ -4175,7 +4180,7 @@ def test_deploy_no_reboot_stops_after_upload_phase(self) -> None:
):
with mock.patch("timecapsulesmb.cli.deploy.upload_deployment_payload"):
with mock.patch("timecapsulesmb.cli.deploy.remote_install_auth_files"):
with mock.patch("timecapsulesmb.cli.deploy.run_ssh") as run_ssh_mock:
with mock.patch("timecapsulesmb.cli.deploy.remote_request_reboot") as run_ssh_mock:
with redirect_stdout(output):
rc = deploy.main(["--no-reboot"])
self.assertEqual(rc, 0)
Expand Down Expand Up @@ -4238,7 +4243,7 @@ def test_deploy_declined_reboot_returns_without_rebooting(self) -> None:
with mock.patch("timecapsulesmb.cli.deploy.upload_deployment_payload"):
with mock.patch("timecapsulesmb.cli.deploy.remote_install_auth_files"):
with mock.patch("builtins.input", return_value="n"):
with mock.patch("timecapsulesmb.cli.deploy.run_ssh") as run_ssh_mock:
with mock.patch("timecapsulesmb.cli.deploy.remote_request_reboot") as run_ssh_mock:
with redirect_stdout(output):
rc = deploy.main([])
self.assertEqual(rc, 0)
Expand Down Expand Up @@ -4273,7 +4278,7 @@ def test_deploy_reboot_timeout_returns_failure(self) -> None:
with mock.patch("timecapsulesmb.cli.deploy.upload_deployment_payload"):
with mock.patch("timecapsulesmb.cli.deploy.remote_install_auth_files"):
with mock.patch("builtins.input", return_value="y"):
with mock.patch("timecapsulesmb.cli.deploy.run_ssh"):
with mock.patch("timecapsulesmb.cli.deploy.remote_request_reboot"):
with mock.patch("timecapsulesmb.cli.deploy.wait_for_ssh_state_conn", side_effect=[True, False]) as wait_mock:
with redirect_stdout(output):
rc = deploy.main([])
Expand Down Expand Up @@ -4311,7 +4316,7 @@ def test_deploy_waits_for_managed_smbd_before_verifying(self) -> None:
):
with mock.patch("timecapsulesmb.cli.deploy.upload_deployment_payload"):
with mock.patch("timecapsulesmb.cli.deploy.remote_install_auth_files"):
with mock.patch("timecapsulesmb.cli.deploy.run_ssh"):
with mock.patch("timecapsulesmb.cli.deploy.remote_request_reboot"):
with mock.patch("timecapsulesmb.cli.deploy.wait_for_ssh_state_conn", side_effect=[True, True]):
with mock.patch("timecapsulesmb.cli.deploy.verify_managed_runtime", return_value=True) as verify_runtime_mock:
with mock.patch("builtins.input", return_value="y"):
Expand Down Expand Up @@ -4355,7 +4360,7 @@ def test_deploy_returns_failure_when_managed_smbd_never_becomes_ready(self) -> N
):
with mock.patch("timecapsulesmb.cli.deploy.upload_deployment_payload"):
with mock.patch("timecapsulesmb.cli.deploy.remote_install_auth_files"):
with mock.patch("timecapsulesmb.cli.deploy.run_ssh"):
with mock.patch("timecapsulesmb.cli.deploy.remote_request_reboot"):
with mock.patch("timecapsulesmb.cli.deploy.wait_for_ssh_state_conn", side_effect=[True, True]):
with mock.patch("timecapsulesmb.cli.deploy.verify_managed_runtime", return_value=False) as verify_runtime_mock:
with mock.patch("builtins.input", return_value="y"):
Expand Down Expand Up @@ -4392,7 +4397,7 @@ def test_deploy_returns_failure_when_managed_mdns_never_becomes_ready(self) -> N
):
with mock.patch("timecapsulesmb.cli.deploy.upload_deployment_payload"):
with mock.patch("timecapsulesmb.cli.deploy.remote_install_auth_files"):
with mock.patch("timecapsulesmb.cli.deploy.run_ssh"):
with mock.patch("timecapsulesmb.cli.deploy.remote_request_reboot"):
with mock.patch("timecapsulesmb.cli.deploy.wait_for_ssh_state_conn", side_effect=[True, True]):
with mock.patch("timecapsulesmb.cli.deploy.verify_managed_runtime", return_value=False) as verify_runtime_mock:
with mock.patch("builtins.input", return_value="y"):
Expand Down Expand Up @@ -4701,7 +4706,7 @@ def test_deploy_netbsd4_yes_runs_activation_and_skips_reboot(self) -> None:
with mock.patch("timecapsulesmb.cli.deploy.upload_deployment_payload"):
with mock.patch("timecapsulesmb.cli.deploy.remote_install_auth_files"):
with mock.patch("timecapsulesmb.cli.deploy.verify_managed_runtime", return_value=True) as verify_mock:
with mock.patch("timecapsulesmb.cli.deploy.run_ssh") as run_ssh_mock:
with mock.patch("timecapsulesmb.cli.deploy.remote_request_reboot") as run_ssh_mock:
with redirect_stdout(output):
rc = deploy.main(["--yes"])
self.assertEqual(rc, 0)
Expand Down Expand Up @@ -4746,7 +4751,7 @@ def test_deploy_netbsd4_no_reboot_still_runs_activation(self) -> None:
with mock.patch("timecapsulesmb.cli.deploy.upload_deployment_payload"):
with mock.patch("timecapsulesmb.cli.deploy.remote_install_auth_files"):
with mock.patch("timecapsulesmb.cli.deploy.verify_managed_runtime", return_value=True):
with mock.patch("timecapsulesmb.cli.deploy.run_ssh") as run_ssh_mock:
with mock.patch("timecapsulesmb.cli.deploy.remote_request_reboot") as run_ssh_mock:
with redirect_stdout(output):
rc = deploy.main(["--yes", "--no-reboot"])
self.assertEqual(rc, 0)
Expand Down Expand Up @@ -5008,6 +5013,14 @@ def test_deploy_dry_run_json_outputs_plan(self) -> None:
self.assertEqual(payload["apple_mount_wait_seconds"], DEFAULT_APPLE_MOUNT_WAIT_SECONDS)
self.assertTrue(payload["nbns_path"].endswith("/bin/nbns/nbns-advertiser"))
self.assertEqual(payload["payload_targets"]["nbns-advertiser"], f"/Volumes/dk2/{values['TC_PAYLOAD_DIR_NAME']}/nbns-advertiser")
self.assertEqual(
payload["reboot_request"],
{
"mode": "detached_ssh",
"command": DETACHED_REBOOT_COMMAND,
"follow_up": ["wait_for_ssh_down", "wait_for_ssh_up"],
},
)
self.assertEqual(
[check["id"] for check in payload["post_deploy_checks"]],
[
Expand Down Expand Up @@ -5295,6 +5308,8 @@ def test_uninstall_dry_run_prints_target_host(self) -> None:
self.assertIn("Dry run: uninstall plan", text)
self.assertIn("host: root@10.0.0.2", text)
self.assertIn(f"payload dir: /Volumes/dk2/{values['TC_PAYLOAD_DIR_NAME']}", text)
self.assertIn(f"request: {DETACHED_REBOOT_COMMAND}", text)
self.assertIn("follow-up: wait for SSH down, then SSH up", text)

def test_uninstall_dry_run_no_reboot_matches_no_reboot_execution_path(self) -> None:
output = io.StringIO()
Expand All @@ -5312,6 +5327,7 @@ def test_uninstall_dry_run_no_reboot_matches_no_reboot_execution_path(self) -> N
text = output.getvalue()
self.assertIn("Reboot:\n no", text)
self.assertIn("Post-uninstall checks:\n none", text)
self.assertNotIn(DETACHED_REBOOT_COMMAND, text)
self.assertNotIn("SSH returns after reboot", text)

def test_uninstall_validates_only_host_and_payload_dir(self) -> None:
Expand Down Expand Up @@ -5356,6 +5372,14 @@ def test_uninstall_json_outputs_plan(self) -> None:
payload = json.loads(output.getvalue())
self.assertEqual(payload["host"], "root@10.0.0.2")
self.assertEqual(payload["volume_root"], "/Volumes/dk2")
self.assertEqual(
payload["reboot_request"],
{
"mode": "detached_ssh",
"command": DETACHED_REBOOT_COMMAND,
"follow_up": ["wait_for_ssh_down", "wait_for_ssh_up"],
},
)
self.assertEqual(
[check["id"] for check in payload["post_uninstall_checks"]],
[
Expand All @@ -5376,7 +5400,7 @@ def test_uninstall_yes_reboots_and_verifies(self) -> None:
with mock.patch("timecapsulesmb.cli.uninstall.load_env_values", return_value=values):
with mock.patch("timecapsulesmb.cli.uninstall.discover_volume_root_conn", return_value="/Volumes/dk2"):
with mock.patch("timecapsulesmb.cli.uninstall.remote_uninstall_payload") as uninstall_mock:
with mock.patch("timecapsulesmb.cli.uninstall.run_ssh") as run_ssh_mock:
with mock.patch("timecapsulesmb.cli.uninstall.remote_request_reboot") as run_ssh_mock:
with mock.patch("timecapsulesmb.cli.uninstall.wait_for_ssh_state_conn", side_effect=[True, True]) as wait_mock:
with mock.patch("timecapsulesmb.cli.uninstall.verify_post_uninstall", return_value=True) as verify_mock:
with redirect_stdout(output):
Expand All @@ -5402,7 +5426,7 @@ def test_uninstall_no_reboot_skips_reboot_and_returns_success(self) -> None:
with mock.patch("timecapsulesmb.cli.uninstall.load_env_values", return_value=values):
with mock.patch("timecapsulesmb.cli.uninstall.discover_volume_root_conn", return_value="/Volumes/dk2"):
with mock.patch("timecapsulesmb.cli.uninstall.remote_uninstall_payload") as uninstall_mock:
with mock.patch("timecapsulesmb.cli.uninstall.run_ssh") as run_ssh_mock:
with mock.patch("timecapsulesmb.cli.uninstall.remote_request_reboot") as run_ssh_mock:
with mock.patch("timecapsulesmb.cli.uninstall.verify_post_uninstall") as verify_mock:
with redirect_stdout(output):
rc = uninstall.main(["--no-reboot"])
Expand All @@ -5424,7 +5448,7 @@ def test_uninstall_declined_reboot_skips_reboot_and_returns_success(self) -> Non
with mock.patch("timecapsulesmb.cli.uninstall.discover_volume_root_conn", return_value="/Volumes/dk2"):
with mock.patch("timecapsulesmb.cli.uninstall.remote_uninstall_payload"):
with mock.patch("builtins.input", return_value="n"):
with mock.patch("timecapsulesmb.cli.uninstall.run_ssh") as run_ssh_mock:
with mock.patch("timecapsulesmb.cli.uninstall.remote_request_reboot") as run_ssh_mock:
with redirect_stdout(output):
rc = uninstall.main([])
self.assertEqual(rc, 0)
Expand Down
Loading