diff --git a/src/timecapsulesmb/cli/flows.py b/src/timecapsulesmb/cli/flows.py index 40a5d45..12dce9b 100644 --- a/src/timecapsulesmb/cli/flows.py +++ b/src/timecapsulesmb/cli/flows.py @@ -7,7 +7,7 @@ from timecapsulesmb.cli.runtime import LogCallback, emit_progress from timecapsulesmb.core.net import extract_host from timecapsulesmb.core.errors import system_exit_message -from timecapsulesmb.deploy.executor import remote_request_reboot, remote_request_shutdown_reboot +from timecapsulesmb.deploy.executor import remote_request_reboot from timecapsulesmb.deploy.verify import ( managed_runtime_ready, render_managed_runtime_verification, @@ -26,6 +26,7 @@ REBOOT_UP_TIMEOUT_MESSAGE = "Timed out waiting for SSH after reboot." ACP_REBOOT_REQUEST_TIMEOUT_SECONDS = 10 +SSH_SHUTDOWN_REBOOT_PROGRESS_MESSAGE = "SSH: /bin/sync; /sbin/shutdown -r now (fallback /sbin/reboot)" def wait_for_tcp_port_state( @@ -167,8 +168,8 @@ def _request_reboot_via_ssh_shutdown( connection, command_context, log=log, - request_reboot=remote_request_shutdown_reboot, - progress_message="SSH: /sbin/shutdown -r now (fallback /sbin/reboot)", + request_reboot=remote_request_reboot, + progress_message=SSH_SHUTDOWN_REBOOT_PROGRESS_MESSAGE, ) @@ -178,7 +179,7 @@ def _request_reboot_via_ssh( *, log: LogCallback = None, request_reboot: Callable[[SshConnection], None] | None = None, - progress_message: str = "SSH: /sbin/reboot", + progress_message: str = SSH_SHUTDOWN_REBOOT_PROGRESS_MESSAGE, ) -> None: command_context.add_debug_fields(ssh_reboot_attempted=True) emit_progress(log, progress_message) diff --git a/src/timecapsulesmb/cli/fsck.py b/src/timecapsulesmb/cli/fsck.py index c3aa3e0..78cffad 100644 --- a/src/timecapsulesmb/cli/fsck.py +++ b/src/timecapsulesmb/cli/fsck.py @@ -8,6 +8,7 @@ from timecapsulesmb.cli.context import CommandContext from timecapsulesmb.cli.flows import observe_reboot_cycle from timecapsulesmb.cli.runtime import add_config_argument, load_env_config +from timecapsulesmb.deploy.executor import DETACHED_SHUTDOWN_REBOOT_COMMAND from timecapsulesmb.deploy.planner import DEFAULT_APPLE_MOUNT_WAIT_SECONDS from timecapsulesmb.device.processes import render_direct_pkill9_by_ucomm, render_direct_pkill9_watchdog from timecapsulesmb.identity import ensure_install_id @@ -89,7 +90,7 @@ def build_remote_fsck_script(device: str, mountpoint: str, *, reboot: bool) -> s lines.extend( [ "echo '--- reboot ---'", - "/sbin/reboot >/dev/null 2>&1 || true", + DETACHED_SHUTDOWN_REBOOT_COMMAND, ] ) return "\n".join(lines) diff --git a/src/timecapsulesmb/deploy/executor.py b/src/timecapsulesmb/deploy/executor.py index 646eaa3..db97d0e 100644 --- a/src/timecapsulesmb/deploy/executor.py +++ b/src/timecapsulesmb/deploy/executor.py @@ -10,11 +10,10 @@ from timecapsulesmb.transport.ssh import SshConnection, run_scp, run_ssh -DETACHED_REBOOT_COMMAND = "/bin/sh -c 'exec /dev/null 2>&1; (/bin/sleep 1; /sbin/reboot) & exit 0'" DETACHED_SHUTDOWN_REBOOT_COMMAND = ( "/bin/sh -c 'exec /dev/null 2>&1; " - "(/bin/sleep 1; " - "if [ -x /sbin/shutdown ]; then /sbin/shutdown -r now || /sbin/reboot; else /sbin/reboot; fi" + "(/bin/sync; /bin/sleep 1; " + "/sbin/shutdown -r now || /sbin/reboot" ") & exit 0'" ) REBOOT_REQUEST_TIMEOUT_SECONDS = 30 @@ -120,10 +119,6 @@ def run_remote_actions(connection: SshConnection, actions: Iterable[RemoteAction def remote_request_reboot(connection: SshConnection) -> None: - run_ssh(connection, DETACHED_REBOOT_COMMAND, check=False, timeout=REBOOT_REQUEST_TIMEOUT_SECONDS) - - -def remote_request_shutdown_reboot(connection: SshConnection) -> None: run_ssh(connection, DETACHED_SHUTDOWN_REBOOT_COMMAND, check=False, timeout=REBOOT_REQUEST_TIMEOUT_SECONDS) diff --git a/tests/test_cli.py b/tests/test_cli.py index 57d7e64..65e3a6f 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -902,14 +902,8 @@ def run_deploy_cli( mocks.verify_managed_runtime = stack.enter_context( mock.patch("timecapsulesmb.cli.flows.verify_managed_runtime", return_value=verify_runtime) ) - mocks.remote_request_shutdown_reboot = stack.enter_context( - mock.patch("timecapsulesmb.cli.flows.remote_request_shutdown_reboot", side_effect=reboot_side_effect) - ) mocks.remote_request_reboot = stack.enter_context( - mock.patch( - "timecapsulesmb.cli.flows.remote_request_reboot", - side_effect=AssertionError("deploy should not request legacy SSH reboot directly"), - ) + mock.patch("timecapsulesmb.cli.flows.remote_request_reboot", side_effect=reboot_side_effect) ) mocks.acp_reboot = stack.enter_context( mock.patch( @@ -4529,7 +4523,6 @@ def test_deploy_no_reboot_stops_after_upload_phase(self) -> None: ) self.assertEqual(result.rc, 0) - result.mocks.remote_request_shutdown_reboot.assert_not_called() result.mocks.remote_request_reboot.assert_not_called() self.assertEqual(result.mocks.verify_payload_home_conn.call_count, 2) result.mocks.flush_remote_filesystem_writes.assert_called_once() @@ -4546,7 +4539,6 @@ def test_deploy_payload_verification_failure_aborts_before_reboot(self) -> None: ) self.assertEqual(str(result.exception), "managed payload verification failed at /Volumes/dk2/.samba4: missing smbd") - result.mocks.remote_request_shutdown_reboot.assert_not_called() result.mocks.remote_request_reboot.assert_not_called() result.mocks.verify_payload_home_conn.assert_called_once() result.mocks.flush_remote_filesystem_writes.assert_not_called() @@ -4572,7 +4564,6 @@ def test_deploy_post_sync_payload_verification_failure_aborts_before_reboot(self "managed payload verification failed at /Volumes/dk2/.samba4: missing payload directory", ) result.mocks.flush_remote_filesystem_writes.assert_called_once() - result.mocks.remote_request_shutdown_reboot.assert_not_called() result.mocks.remote_request_reboot.assert_not_called() self.assertEqual(result.mocks.verify_payload_home_conn.call_count, 2) telemetry_error = self.telemetry_payload("deploy_finished")["error"] @@ -4591,7 +4582,6 @@ def test_deploy_declined_reboot_returns_without_rebooting(self) -> None: self.assertEqual(result.rc, 0) self.assertIn("Deployment complete without reboot.", result.text) - result.mocks.remote_request_shutdown_reboot.assert_not_called() result.mocks.remote_request_reboot.assert_not_called() self.assertEqual(result.mocks.verify_payload_home_conn.call_count, 2) result.mocks.flush_remote_filesystem_writes.assert_called_once() @@ -4610,8 +4600,7 @@ def test_deploy_reboot_timeout_returns_failure(self) -> None: self.assertEqual(result.rc, 1) self.assertIn("SSH reboot request timed out; checking whether the device is rebooting...", result.text) self.assertIn(deploy.REBOOT_NO_DOWN_MESSAGE, result.text) - result.mocks.remote_request_shutdown_reboot.assert_called_once() - result.mocks.remote_request_reboot.assert_not_called() + result.mocks.remote_request_reboot.assert_called_once() result.mocks.acp_reboot.assert_not_called() result.mocks.verify_managed_runtime.assert_not_called() @@ -4673,7 +4662,6 @@ def test_deploy_netbsd4_yes_runs_activation_and_skips_reboot(self) -> None: self.assertEqual(result.mocks.run_remote_actions.call_count, 3) self.assertEqual(result.mocks.verify_payload_home_conn.call_count, 2) result.mocks.flush_remote_filesystem_writes.assert_called_once() - result.mocks.remote_request_shutdown_reboot.assert_not_called() result.mocks.remote_request_reboot.assert_not_called() self.assertIn("Activating NetBSD4 payload without reboot.", result.text) self.assertIn("NetBSD4 activation complete.", result.text) @@ -7324,7 +7312,11 @@ def test_fsck_yes_reboots_and_waits_by_default(self) -> None: self.assertIn("^wcifsfs$", remote_cmd) self.assertIn("umount -f /Volumes/dk2", remote_cmd) self.assertIn("fsck_hfs -fy /dev/dk2", remote_cmd) + self.assertIn("exec /dev/null 2>&1", remote_cmd) + self.assertIn("/bin/sync; /bin/sleep 1;", remote_cmd) + self.assertIn("/sbin/shutdown -r now", remote_cmd) self.assertIn("/sbin/reboot", remote_cmd) + self.assertIn(") & exit 0", remote_cmd) self.assertEqual(wait_mock.call_args_list[0].kwargs, {"expected_up": False, "timeout_seconds": 90}) self.assertEqual(wait_mock.call_args_list[1].kwargs, {"expected_up": True, "timeout_seconds": 420}) text = output.getvalue() diff --git a/tests/test_cli_flows.py b/tests/test_cli_flows.py index 5028d1c..f86eb63 100644 --- a/tests/test_cli_flows.py +++ b/tests/test_cli_flows.py @@ -16,6 +16,7 @@ from timecapsulesmb.cli.flows import ( ACP_REBOOT_REQUEST_TIMEOUT_SECONDS, REBOOT_UP_TIMEOUT_MESSAGE, + SSH_SHUTDOWN_REBOOT_PROGRESS_MESSAGE, observe_reboot_cycle, request_deploy_reboot_and_wait, request_reboot_and_wait, @@ -152,10 +153,10 @@ def test_request_reboot_and_wait_succeeds_after_acp_reboot_request(self) -> None self.assertIn("ACP reboot requested.", output.getvalue()) self.assertIn("Waiting for the device to go down...", output.getvalue()) - def test_request_deploy_reboot_and_wait_uses_ssh_shutdown_request(self) -> None: + def test_request_deploy_reboot_and_wait_uses_ssh_reboot_request(self) -> None: command_context = FakeCommandContext() output = io.StringIO() - with mock.patch("timecapsulesmb.cli.flows.remote_request_shutdown_reboot") as shutdown_reboot_mock: + with mock.patch("timecapsulesmb.cli.flows.remote_request_reboot") as reboot_mock: with mock.patch("timecapsulesmb.cli.flows.acp_reboot", side_effect=AssertionError("deploy should not use ACP reboot")): with mock.patch("timecapsulesmb.cli.flows.wait_for_ssh_state_conn", side_effect=[True, True]) as wait_mock: with redirect_stdout(output): @@ -166,7 +167,7 @@ def test_request_deploy_reboot_and_wait_uses_ssh_shutdown_request(self) -> None: ) self.assertTrue(ok) - shutdown_reboot_mock.assert_called_once() + reboot_mock.assert_called_once() self.assertEqual(wait_mock.call_args_list[0].kwargs, {"expected_up": False, "timeout_seconds": 60}) self.assertEqual(wait_mock.call_args_list[1].kwargs, {"expected_up": True, "timeout_seconds": 240}) self.assertEqual(command_context.finish_fields["reboot_was_attempted"], True) @@ -268,7 +269,7 @@ def test_request_ssh_reboot_uses_ssh_only_strategy_and_progress_log(self) -> Non self.assertEqual(command_context.debug_fields["reboot_request_strategy"], "ssh") self.assertEqual(command_context.debug_fields["ssh_reboot_attempted"], True) self.assertEqual(command_context.debug_fields["ssh_reboot_succeeded"], True) - self.assertEqual(messages, ["SSH: /sbin/reboot"]) + self.assertEqual(messages, [SSH_SHUTDOWN_REBOOT_PROGRESS_MESSAGE]) self.assertIn("SSH reboot requested.", output.getvalue()) def test_request_ssh_reboot_records_timeout_without_raising(self) -> None: diff --git a/tests/test_deploy_modules.py b/tests/test_deploy_modules.py index cf63eef..d1a716d 100644 --- a/tests/test_deploy_modules.py +++ b/tests/test_deploy_modules.py @@ -40,14 +40,12 @@ ) from timecapsulesmb.deploy.dry_run import format_deployment_plan from timecapsulesmb.deploy.executor import ( - DETACHED_REBOOT_COMMAND, DETACHED_SHUTDOWN_REBOOT_COMMAND, FLUSH_REMOTE_FILESYSTEMS_COMMAND, FLUSH_REMOTE_FILESYSTEMS_TIMEOUT_SECONDS, REBOOT_REQUEST_TIMEOUT_SECONDS, flush_remote_filesystem_writes, remote_request_reboot, - remote_request_shutdown_reboot, remote_uninstall_payload, upload_deployment_payload, upload_flash_file, @@ -297,25 +295,18 @@ def test_remote_request_reboot_uses_explicit_reboot_timeout(self) -> None: connection = SshConnection("root@10.0.0.2", "pw", "-o foo") with mock.patch("timecapsulesmb.deploy.executor.run_ssh") as run_ssh_mock: remote_request_reboot(connection) - run_ssh_mock.assert_called_once_with( - connection, - DETACHED_REBOOT_COMMAND, - check=False, - timeout=REBOOT_REQUEST_TIMEOUT_SECONDS, - ) - - def test_remote_request_shutdown_reboot_uses_shutdown_with_reboot_fallback(self) -> None: - connection = SshConnection("root@10.0.0.2", "pw", "-o foo") - with mock.patch("timecapsulesmb.deploy.executor.run_ssh") as run_ssh_mock: - remote_request_shutdown_reboot(connection) run_ssh_mock.assert_called_once_with( connection, DETACHED_SHUTDOWN_REBOOT_COMMAND, check=False, timeout=REBOOT_REQUEST_TIMEOUT_SECONDS, ) + self.assertIn("exec /dev/null 2>&1", DETACHED_SHUTDOWN_REBOOT_COMMAND) + self.assertIn("/bin/sync; /bin/sleep 1;", DETACHED_SHUTDOWN_REBOOT_COMMAND) self.assertIn("/sbin/shutdown -r now", DETACHED_SHUTDOWN_REBOOT_COMMAND) self.assertIn("|| /sbin/reboot", DETACHED_SHUTDOWN_REBOOT_COMMAND) + self.assertNotIn("[ -x /sbin/shutdown ]", DETACHED_SHUTDOWN_REBOOT_COMMAND) + self.assertIn(") & exit 0", DETACHED_SHUTDOWN_REBOOT_COMMAND) def test_flush_remote_filesystem_writes_syncs_and_waits(self) -> None: connection = SshConnection("root@10.0.0.2", "pw", "-o foo")