runner: make runner.start() forward kwargs to astart_command()

In a recent patch, we switched the mechanism used for running some
shutdown related commands. We used to lean on run_command() and we now
lean on runner.run().

runner.run() works a bit differently because it runs the command through
systemd-run - and by default does not capture the output (it goes into
the journal).

Another difference is that runner.run() would not forward the keyword
arguments (like stdout, stderr) to subprocess. Well, it would forward
those arguments to runner.start() which would raise an exception:

  File "subiquity/server/controllers/shutdown.py", line 118, in copy_logs_to_target
    await self.app.command_runner.run(
  File "subiquity/server/runner.py", line 98, in run
    proc = await self.start(cmd, **opts)
                 ^^^^^^^^^^^^^^^^^^^^^^^
TypeError: DryRunCommandRunner.start() got an unexpected keyword argument 'stdout'

We now ensure that those keyword arguments are properly forwarded to
subprocess (through astart_command).

Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>

subiquity/server/runner.py

@@ -70,12 +70,17 @@ class LoggedCommandRunner:
         return prefix + cmd
 
     async def start(
-        self, cmd: List[str], *, private_mounts: bool = False, capture: bool = False
+        self,
+        cmd: List[str],
+        *,
+        private_mounts: bool = False,
+        capture: bool = False,
+        **astart_kwargs,
     ) -> asyncio.subprocess.Process:
         forged: List[str] = self._forge_systemd_cmd(
             cmd, private_mounts=private_mounts, capture=capture
         )
-        proc = await astart_command(forged)
+        proc = await astart_command(forged, **astart_kwargs)
         proc.args = forged
         return proc
 
@@ -128,10 +133,17 @@ class DryRunCommandRunner(LoggedCommandRunner):
             return self.delay
 
     async def start(
-        self, cmd: List[str], *, private_mounts: bool = False, capture: bool = False
+        self,
+        cmd: List[str],
+        *,
+        private_mounts: bool = False,
+        capture: bool = False,
+        **astart_kwargs,
     ) -> asyncio.subprocess.Process:
         delay = self._get_delay_for_cmd(cmd)
-        proc = await super().start(cmd, private_mounts=private_mounts, capture=capture)
+        proc = await super().start(
+            cmd, private_mounts=private_mounts, capture=capture, **astart_kwargs
+        )
         await asyncio.sleep(delay)
         return proc
 

subiquity/server/tests/test_runner.py

@@ -14,7 +14,8 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import os
-from unittest.mock import patch
+import subprocess
+from unittest.mock import ANY, patch
 
 from subiquity.server.runner import DryRunCommandRunner, LoggedCommandRunner
 from subiquitycore.tests import SubiTestCase
@@ -103,6 +104,15 @@ class TestLoggedCommandRunner(SubiTestCase):
         ]
         self.assertEqual(cmd, expected)
 
+    async def test_start(self):
+        runner = LoggedCommandRunner(ident="my-id", use_systemd_user=False)
+
+        with patch("subiquity.server.runner.astart_command") as astart_mock:
+            await runner.start(["/bin/ls"], stdout=subprocess.PIPE)
+
+        expected_cmd = ANY
+        astart_mock.assert_called_once_with(expected_cmd, stdout=subprocess.PIPE)
+
 
 class TestDryRunCommandRunner(SubiTestCase):
     def setUp(self):