From abef05178c4e60bbe259a7371e2a97e3821669fc Mon Sep 17 00:00:00 2001 From: Olivier Gayot Date: Mon, 2 Oct 2023 11:38:35 +0200 Subject: [PATCH] shutdown: do not try to unmount /target if install was not started If we ask for reboot before the installation has started (i.e., if curtin install was not invoked at least once), the following call fails and prevents the system from rebooting. $ umount --recursive /target Make sure we check that /target exists and is mounted before calling umount. Another approach would be to check the return value of umount but the values are not documented. Signed-off-by: Olivier Gayot --- subiquity/server/controllers/filesystem.py | 12 +++++++- .../controllers/tests/test_filesystem.py | 28 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/subiquity/server/controllers/filesystem.py b/subiquity/server/controllers/filesystem.py index b9f091ddd..b792426c0 100644 --- a/subiquity/server/controllers/filesystem.py +++ b/subiquity/server/controllers/filesystem.py @@ -20,6 +20,7 @@ import logging import os import pathlib +import subprocess import time from typing import Any, Callable, Dict, List, Optional, Union @@ -1532,6 +1533,15 @@ def make_autoinstall(self): async def _pre_shutdown(self): if not self.reset_partition_only: - await self.app.command_runner.run(["umount", "--recursive", "/target"]) + # /target is mounted only if the installation was actually started. + try: + await self.app.command_runner.run(["mountpoint", "/target"]) + except subprocess.CalledProcessError: + log.debug( + "/target does not exist or is not mounted," + " skipping call to umount --recursive" + ) + else: + await self.app.command_runner.run(["umount", "--recursive", "/target"]) if len(self.model._all(type="zpool")) > 0: await self.app.command_runner.run(["zpool", "export", "-a"]) diff --git a/subiquity/server/controllers/tests/test_filesystem.py b/subiquity/server/controllers/tests/test_filesystem.py index cf6cb6bba..a48e82d84 100644 --- a/subiquity/server/controllers/tests/test_filesystem.py +++ b/subiquity/server/controllers/tests/test_filesystem.py @@ -14,6 +14,7 @@ # along with this program. If not, see . import copy +import subprocess import uuid from unittest import IsolatedAsyncioTestCase, mock @@ -89,6 +90,7 @@ class TestSubiquityControllerFilesystem(IsolatedAsyncioTestCase): def setUp(self): self.app = make_app() self.app.opts.bootloader = "UEFI" + self.app.command_runner = mock.AsyncMock() self.app.report_start_event = mock.Mock() self.app.report_finish_event = mock.Mock() self.app.prober = mock.Mock() @@ -343,6 +345,32 @@ async def test_v2_edit_partition_POST(self): self.assertTrue(self.fsc.locked_probe_data) handler.assert_called_once() + async def test__pre_shutdown_install_started(self): + self.fsc.reset_partition_only = False + run = mock.patch.object(self.app.command_runner, "run") + _all = mock.patch.object(self.fsc.model, "_all") + with run as mock_run, _all: + await self.fsc._pre_shutdown() + mock_run.assert_has_calls( + [ + mock.call(["mountpoint", "/target"]), + mock.call(["umount", "--recursive", "/target"]), + ] + ) + self.assertEqual(len(mock_run.mock_calls), 2) + + async def test__pre_shutdown_install_not_started(self): + async def fake_run(cmd, **kwargs): + if cmd == ["mountpoint", "/target"]: + raise subprocess.CalledProcessError(cmd=cmd, returncode=1) + + self.fsc.reset_partition_only = False + run = mock.patch.object(self.app.command_runner, "run", side_effect=fake_run) + _all = mock.patch.object(self.fsc.model, "_all") + with run as mock_run, _all: + await self.fsc._pre_shutdown() + mock_run.assert_called_once_with(["mountpoint", "/target"]) + class TestGuided(IsolatedAsyncioTestCase): boot_expectations = [