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 <olivier.gayot@canonical.com>

subiquity/server/controllers/filesystem.py

@@ -20,6 +20,7 @@ import json
 import logging
 import os
 import pathlib
+import subprocess
 import time
 from typing import Any, Callable, Dict, List, Optional, Union
 
@@ -1532,6 +1533,15 @@ class FilesystemController(SubiquityController, FilesystemManipulator):
 
     async def _pre_shutdown(self):
         if not self.reset_partition_only:
+            # /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"])

subiquity/server/controllers/tests/test_filesystem.py

@@ -14,6 +14,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 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 @@ class TestSubiquityControllerFilesystem(IsolatedAsyncioTestCase):
         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 = [