diff --git a/subiquity/server/apt.py b/subiquity/server/apt.py
index c03e9d55..4bca31cf 100644
--- a/subiquity/server/apt.py
+++ b/subiquity/server/apt.py
@@ -149,7 +149,7 @@ class AptConfigurer:
if type is not None:
opts.extend(['-t', type])
await self.app.command_runner.run(
- ['mount'] + opts + [device, mountpoint])
+ ['mount'] + opts + [device, mountpoint], private_mounts=False)
m = Mountpoint(mountpoint=mountpoint)
self._mounts.append(m)
return m
@@ -157,7 +157,9 @@ class AptConfigurer:
async def unmount(self, mountpoint: Mountpoint, remove=True):
if remove:
self._mounts.remove(mountpoint)
- await self.app.command_runner.run(['umount', mountpoint.mountpoint])
+ await self.app.command_runner.run(
+ ['umount', mountpoint.mountpoint],
+ private_mounts=False)
async def setup_overlay(self, lowers: List[Lower]) -> OverlayMountpoint:
tdir = self.tdir()
@@ -194,7 +196,7 @@ class AptConfigurer:
await run_curtin_command(
self.app, context, 'apt-config', '-t', self.configured_tree.p(),
- config=config_location)
+ config=config_location, private_mounts=True)
async def configure_for_install(self, context):
assert self.configured_tree is not None
@@ -223,7 +225,7 @@ class AptConfigurer:
await run_curtin_command(
self.app, context, "in-target", "-t", self.install_tree.p(),
- "--", "apt-get", "update")
+ "--", "apt-get", "update", private_mounts=True)
return self.install_tree.p()
@@ -270,7 +272,7 @@ class AptConfigurer:
if self.app.base_model.network.has_network:
await run_curtin_command(
self.app, context, "in-target", "-t", target_mnt.p(),
- "--", "apt-get", "update")
+ "--", "apt-get", "update", private_mounts=True)
else:
await _restore_dir('var/lib/apt/lists')
diff --git a/subiquity/server/controllers/install.py b/subiquity/server/controllers/install.py
index 46ed4463..56574758 100644
--- a/subiquity/server/controllers/install.py
+++ b/subiquity/server/controllers/install.py
@@ -118,7 +118,8 @@ class InstallController(SubiquityController):
@with_context(description="umounting /target dir")
async def unmount_target(self, *, context, target):
- await run_curtin_command(self.app, context, 'unmount', '-t', target)
+ await run_curtin_command(self.app, context, 'unmount', '-t', target,
+ private_mounts=False)
if not self.app.opts.dry_run:
shutil.rmtree(target)
@@ -132,8 +133,9 @@ class InstallController(SubiquityController):
@with_context(
description="installing system", level="INFO", childlevel="DEBUG")
async def curtin_install(self, *, context, source):
- await run_curtin_command(
- self.app, context, 'install', source, config=self.write_config())
+ await run_curtin_command(self.app, context, 'install', source,
+ config=self.write_config(),
+ private_mounts=False)
@with_context()
async def install(self, *, context):
@@ -224,7 +226,8 @@ class InstallController(SubiquityController):
async def install_package(self, *, context, package):
await run_curtin_command(
self.app, context, 'system-install', '-t', self.tpath(),
- '--', package)
+ '--', package,
+ private_mounts=False)
@with_context(description="restoring apt configuration")
async def restore_apt_config(self, context):
@@ -250,7 +253,8 @@ class InstallController(SubiquityController):
self.unattended_upgrades_ctx = context
self.unattended_upgrades_cmd = await start_curtin_command(
self.app, context, "in-target", "-t", self.tpath(),
- "--", "unattended-upgrades", "-v")
+ "--", "unattended-upgrades", "-v",
+ private_mounts=True)
await self.unattended_upgrades_cmd.wait()
self.unattended_upgrades_cmd = None
self.unattended_upgrades_ctx = None
diff --git a/subiquity/server/curtin.py b/subiquity/server/curtin.py
index d704f93a..093cb793 100644
--- a/subiquity/server/curtin.py
+++ b/subiquity/server/curtin.py
@@ -18,7 +18,9 @@ import asyncio
import json
import logging
import os
+import subprocess
import sys
+from typing import List
from curtin.commands.install import (
INSTALL_LOG,
@@ -37,7 +39,8 @@ class _CurtinCommand:
_count = 0
- def __init__(self, opts, runner, command, *args, config=None):
+ def __init__(self, opts, runner, command: str, *args: str,
+ config=None, private_mounts: bool):
self.opts = opts
self.runner = runner
self._event_contexts = {}
@@ -47,6 +50,7 @@ class _CurtinCommand:
self._fd = None
self.proc = None
self._cmd = self.make_command(command, *args, config=config)
+ self.private_mounts = private_mounts
def _event(self, event):
e = {
@@ -81,7 +85,7 @@ class _CurtinCommand:
if curtin_ctx is not None:
curtin_ctx.exit(result=status)
- def make_command(self, command, *args, config=None):
+ def make_command(self, command: str, *args: str, config=None) -> List[str]:
reporting_conf = {
'subiquity': {
'type': 'journald',
@@ -107,7 +111,8 @@ class _CurtinCommand:
# first couple of events.
await asyncio.sleep(0)
self._event_contexts[''] = context
- self.proc = await self.runner.start(self._cmd, **opts)
+ self.proc = await self.runner.start(
+ self._cmd, **opts, private_mounts=self.private_mounts)
async def wait(self):
result = await self.runner.wait(self.proc)
@@ -147,8 +152,10 @@ class _FailingDryRunCurtinCommand(_DryRunCurtinCommand):
event_file = 'examples/curtin-events-fail.json'
-async def start_curtin_command(
- app, context, command, *args, config=None, **opts):
+async def start_curtin_command(app, context,
+ command: str, *args: str,
+ config=None, private_mounts: bool,
+ **opts):
if app.opts.dry_run:
if 'install-fail' in app.debug_flags:
cls = _FailingDryRunCurtinCommand
@@ -157,13 +164,18 @@ async def start_curtin_command(
else:
cls = _CurtinCommand
curtin_cmd = cls(app.opts, app.command_runner, command, *args,
- config=config)
+ config=config, private_mounts=private_mounts)
await curtin_cmd.start(context, **opts)
return curtin_cmd
async def run_curtin_command(
- app, context, command, *args, config=None, **opts):
+ app, context,
+ command: str, *args: str,
+ config=None,
+ private_mounts: bool,
+ **opts) -> subprocess.CompletedProcess:
cmd = await start_curtin_command(
- app, context, command, *args, config=config, **opts)
+ app, context, command, *args,
+ config=config, private_mounts=private_mounts, **opts)
return await cmd.wait()
diff --git a/subiquity/server/runner.py b/subiquity/server/runner.py
index f8e9f821..06e59909 100644
--- a/subiquity/server/runner.py
+++ b/subiquity/server/runner.py
@@ -14,28 +14,68 @@
# along with this program. If not, see .
import asyncio
+from contextlib import suppress
+import os
import subprocess
+from typing import List, Optional
from subiquitycore.utils import astart_command
class LoggedCommandRunner:
-
- def __init__(self, ident):
+ """ Class that executes commands using systemd-run. """
+ def __init__(self, ident,
+ *, use_systemd_user: Optional[bool] = None) -> None:
self.ident = ident
+ self.env_whitelist = [
+ "PATH", "PYTHONPATH",
+ "PYTHON",
+ "TARGET_MOUNT_POINT",
+ "SNAP",
+ ]
+ if use_systemd_user is not None:
+ self.use_systemd_user = use_systemd_user
+ else:
+ self.use_systemd_user = os.geteuid() != 0
- async def start(self, cmd, *, capture=False):
- if not capture:
- cmd = [
- 'systemd-cat',
- '--level-prefix=false',
- '--identifier='+self.ident,
- ] + cmd
- proc = await astart_command(cmd)
- proc.args = cmd
+ def _forge_systemd_cmd(self, cmd: List[str],
+ private_mounts: bool, capture: bool) -> List[str]:
+ """ Return the supplied command prefixed with the systemd-run stuff.
+ """
+ prefix = [
+ "systemd-run",
+ "--wait", "--same-dir",
+ "--property", f"SyslogIdentifier={self.ident}",
+ ]
+ if private_mounts:
+ prefix.extend(("--property", "PrivateMounts=yes"))
+ if self.use_systemd_user:
+ prefix.append("--user")
+ if capture:
+ # NOTE Using --pipe seems to be the simplest way to capture the
+ # output of the child process. However, let's keep in mind that
+ # --pipe also opens a pipe on stdin. This will effectively make the
+ # child process behave differently if it reads from stdin.
+ prefix.append("--pipe")
+ for key in self.env_whitelist:
+ with suppress(KeyError):
+ prefix.extend(("--setenv", f"{key}={os.environ[key]}"))
+
+ prefix.append("--")
+
+ return prefix + cmd
+
+ async def start(self, cmd: List[str],
+ *, private_mounts: bool = False, capture: bool = False) \
+ -> asyncio.subprocess.Process:
+ forged: List[str] = self._forge_systemd_cmd(
+ cmd, private_mounts=private_mounts, capture=capture)
+ proc = await astart_command(forged)
+ proc.args = forged
return proc
- async def wait(self, proc):
+ async def wait(self, proc: asyncio.subprocess.Process) \
+ -> subprocess.CompletedProcess:
stdout, stderr = await proc.communicate()
if proc.returncode != 0:
raise subprocess.CalledProcessError(proc.returncode, proc.args)
@@ -43,27 +83,45 @@ class LoggedCommandRunner:
return subprocess.CompletedProcess(
proc.args, proc.returncode, stdout=stdout, stderr=stderr)
- async def run(self, cmd, **opts):
+ async def run(self, cmd: List[str], **opts) -> subprocess.CompletedProcess:
proc = await self.start(cmd, **opts)
return await self.wait(proc)
class DryRunCommandRunner(LoggedCommandRunner):
- def __init__(self, ident, delay):
- super().__init__(ident)
+ def __init__(self, ident, delay,
+ *, use_systemd_user: Optional[bool] = None) -> None:
+ super().__init__(ident, use_systemd_user=use_systemd_user)
self.delay = delay
- async def start(self, cmd, *, capture=False):
- if 'scripts/replay-curtin-log.py' in cmd:
- delay = 0
+ def _forge_systemd_cmd(self, cmd: List[str],
+ private_mounts: bool, capture: bool) -> List[str]:
+ if "scripts/replay-curtin-log.py" in cmd:
+ # We actually want to run this command
+ prefixed_command = cmd
else:
- cmd = ['echo', 'not running:'] + cmd
- if 'unattended-upgrades' in cmd:
- delay = 3*self.delay
- else:
- delay = self.delay
- proc = await super().start(cmd, capture=capture)
+ prefixed_command = ["echo", "not running:"] + cmd
+
+ return super()._forge_systemd_cmd(prefixed_command,
+ private_mounts=private_mounts,
+ capture=capture)
+
+ def _get_delay_for_cmd(self, cmd: List[str]) -> float:
+ if 'scripts/replay-curtin-log.py' in cmd:
+ return 0
+ elif 'unattended-upgrades' in cmd:
+ return 3 * self.delay
+ else:
+ return self.delay
+
+ async def start(self, cmd: List[str],
+ *, private_mounts: bool = False, capture: bool = False) \
+ -> asyncio.subprocess.Process:
+ delay = self._get_delay_for_cmd(cmd)
+ proc = await super().start(cmd,
+ private_mounts=private_mounts,
+ capture=capture)
await asyncio.sleep(delay)
return proc
diff --git a/subiquity/server/tests/test_runner.py b/subiquity/server/tests/test_runner.py
new file mode 100644
index 00000000..2989acb7
--- /dev/null
+++ b/subiquity/server/tests/test_runner.py
@@ -0,0 +1,144 @@
+# Copyright 2022 Canonical, Ltd.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as
+# published by the Free Software Foundation, either version 3 of the
+# License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program. If not, see .
+
+import os
+from unittest.mock import patch
+
+from subiquitycore.tests import SubiTestCase
+from subiquity.server.runner import (
+ LoggedCommandRunner,
+ DryRunCommandRunner,
+ )
+
+
+class TestLoggedCommandRunner(SubiTestCase):
+ def test_init(self):
+ with patch("os.geteuid", return_value=0):
+ runner = LoggedCommandRunner(ident="my-identifier")
+ self.assertEqual(runner.ident, "my-identifier")
+ self.assertEqual(runner.use_systemd_user, False)
+
+ with patch("os.geteuid", return_value=1000):
+ runner = LoggedCommandRunner(ident="my-identifier")
+ self.assertEqual(runner.use_systemd_user, True)
+
+ runner = LoggedCommandRunner(ident="my-identifier",
+ use_systemd_user=True)
+ self.assertEqual(runner.use_systemd_user, True)
+
+ runner = LoggedCommandRunner(ident="my-identifier",
+ use_systemd_user=False)
+ self.assertEqual(runner.use_systemd_user, False)
+
+ def test_forge_systemd_cmd(self):
+ runner = LoggedCommandRunner(ident="my-id", use_systemd_user=False)
+ environ = {
+ "PATH": "/snap/subiquity/x1/bin",
+ "PYTHONPATH": "/usr/lib/python3.8/site-packages",
+ "PYTHON": "/snap/subiquity/x1/usr/bin/python3.8",
+ "TARGET_MOUNT_POINT": "/target",
+ "SNAP": "/snap/subiquity/x1",
+ "DUMMY": "should-not-be-exported",
+ }
+
+ with patch.dict(os.environ, environ):
+ cmd = runner._forge_systemd_cmd(
+ ["/bin/ls", "/root"],
+ private_mounts=True, capture=False)
+
+ expected = [
+ "systemd-run",
+ "--wait", "--same-dir",
+ "--property", "SyslogIdentifier=my-id",
+ "--property", "PrivateMounts=yes",
+ "--setenv", "PATH=/snap/subiquity/x1/bin",
+ "--setenv", "PYTHONPATH=/usr/lib/python3.8/site-packages",
+ "--setenv", "PYTHON=/snap/subiquity/x1/usr/bin/python3.8",
+ "--setenv", "TARGET_MOUNT_POINT=/target",
+ "--setenv", "SNAP=/snap/subiquity/x1",
+ "--",
+ "/bin/ls", "/root",
+ ]
+ self.assertEqual(cmd, expected)
+
+ runner = LoggedCommandRunner(ident="my-id", use_systemd_user=True)
+ # Make sure unset variables are ignored
+ environ = {
+ "PYTHONPATH": "/usr/lib/python3.8/site-packages",
+ }
+ with patch.dict(os.environ, environ, clear=True):
+ cmd = runner._forge_systemd_cmd(
+ ["/bin/ls", "/root"],
+ private_mounts=False, capture=True)
+
+ expected = [
+ "systemd-run",
+ "--wait", "--same-dir",
+ "--property", "SyslogIdentifier=my-id",
+ "--user",
+ "--pipe",
+ "--setenv", "PYTHONPATH=/usr/lib/python3.8/site-packages",
+ "--",
+ "/bin/ls", "/root",
+ ]
+ self.assertEqual(cmd, expected)
+
+
+class TestDryRunCommandRunner(SubiTestCase):
+ def setUp(self):
+ self.runner = DryRunCommandRunner(ident="my-identifier",
+ delay=10, use_systemd_user=True)
+
+ def test_init(self):
+ self.assertEqual(self.runner.ident, "my-identifier")
+ self.assertEqual(self.runner.delay, 10)
+ self.assertEqual(self.runner.use_systemd_user, True)
+
+ @patch.object(LoggedCommandRunner, "_forge_systemd_cmd")
+ def test_forge_systemd_cmd(self, mock_super):
+ self.runner._forge_systemd_cmd(
+ ["/bin/ls", "/root"],
+ private_mounts=True, capture=True)
+ mock_super.assert_called_once_with(
+ ["echo", "not running:", "/bin/ls", "/root"],
+ private_mounts=True, capture=True)
+
+ mock_super.reset_mock()
+ # Make sure exceptions are handled.
+ self.runner._forge_systemd_cmd(["scripts/replay-curtin-log.py"],
+ private_mounts=True, capture=False)
+ mock_super.assert_called_once_with(
+ ["scripts/replay-curtin-log.py"],
+ private_mounts=True, capture=False)
+
+ def test_get_delay_for_cmd(self):
+ # Most commands use the default delay
+ delay = self.runner._get_delay_for_cmd(["/bin/ls", "/root"])
+ self.assertEqual(delay, 10)
+
+ # Commands containing "unattended-upgrades" use delay * 3
+ delay = self.runner._get_delay_for_cmd([
+ "python3", "-m", "curtin",
+ "in-target",
+ "-t", "/target",
+ "--",
+ "unattended-upgrades", "-v",
+ ])
+ self.assertEqual(delay, 30)
+
+ # Commands having scripts/replay will actually be executed - no delay.
+ delay = self.runner._get_delay_for_cmd(
+ ["scripts/replay-curtin-log.py"])
+ self.assertEqual(delay, 0)
diff --git a/subiquity/server/tests/test_ubuntu_drivers.py b/subiquity/server/tests/test_ubuntu_drivers.py
index e869985c..4c04bcd6 100644
--- a/subiquity/server/tests/test_ubuntu_drivers.py
+++ b/subiquity/server/tests/test_ubuntu_drivers.py
@@ -62,7 +62,8 @@ class TestUbuntuDriversInterface(unittest.TestCase):
self.app, "installing third-party drivers",
"in-target", "-t", "/target",
"--",
- "ubuntu-drivers", "install"
+ "ubuntu-drivers", "install",
+ private_mounts=True,
)
@patch.multiple(UbuntuDriversInterface, __abstractmethods__=set())
@@ -130,6 +131,6 @@ nvidia-driver-510 linux-modules-nvidia-510-generic-hwe-20.04
"in-target", "-t", "/target",
"--",
"ubuntu-drivers", "list", "--recommended",
- capture=True)
+ capture=True, private_mounts=True)
self.assertEqual(drivers, ["nvidia-driver-510"])
diff --git a/subiquity/server/ubuntu_drivers.py b/subiquity/server/ubuntu_drivers.py
index aa5728bb..2b02f70f 100644
--- a/subiquity/server/ubuntu_drivers.py
+++ b/subiquity/server/ubuntu_drivers.py
@@ -59,7 +59,8 @@ class UbuntuDriversInterface(ABC):
async def install_drivers(self, root_dir: str, context) -> None:
await run_curtin_command(
self.app, context,
- "in-target", "-t", root_dir, "--", *self.install_drivers_cmd)
+ "in-target", "-t", root_dir, "--", *self.install_drivers_cmd,
+ private_mounts=True)
def _drivers_from_output(self, output: str) -> List[str]:
""" Parse the output of ubuntu-drivers list --recommended and return a
@@ -98,7 +99,7 @@ class UbuntuDriversClientInterface(UbuntuDriversInterface):
result = await run_curtin_command(
self.app, context,
"in-target", "-t", root_dir, "--", *self.list_drivers_cmd,
- capture=True)
+ capture=True, private_mounts=True)
# Currently we have no way to specify universal_newlines=True or
# encoding="utf-8" to run_curtin_command so we need to decode the
# output.
diff --git a/subiquitycore/utils.py b/subiquitycore/utils.py
index 34b5e9c4..e790cf8d 100644
--- a/subiquitycore/utils.py
+++ b/subiquitycore/utils.py
@@ -19,6 +19,7 @@ import logging
import os
import random
import subprocess
+from typing import List
log = logging.getLogger("subiquitycore.utils")
@@ -34,9 +35,9 @@ def _clean_env(env):
return env
-def run_command(cmd, *, input=None, stdout=subprocess.PIPE,
+def run_command(cmd: List[str], *, input=None, stdout=subprocess.PIPE,
stderr=subprocess.PIPE, encoding='utf-8', errors='replace',
- env=None, **kw):
+ env=None, **kw) -> subprocess.CompletedProcess:
"""A wrapper around subprocess.run with logging and different defaults.
We never ever want a subprocess to inherit our file descriptors!
@@ -62,9 +63,11 @@ def run_command(cmd, *, input=None, stdout=subprocess.PIPE,
return cp
-async def arun_command(cmd, *, stdout=subprocess.PIPE, stderr=subprocess.PIPE,
+async def arun_command(cmd: List[str], *,
+ stdout=subprocess.PIPE, stderr=subprocess.PIPE,
encoding='utf-8', input=None, errors='replace',
- env=None, check=False, **kw):
+ env=None, check=False, **kw) \
+ -> subprocess.CompletedProcess:
if input is None:
if 'stdin' not in kw:
kw['stdin'] = subprocess.DEVNULL
@@ -88,23 +91,24 @@ async def arun_command(cmd, *, stdout=subprocess.PIPE, stderr=subprocess.PIPE,
cmd, proc.returncode, stdout, stderr)
-async def astart_command(cmd, *, stdout=subprocess.PIPE,
+async def astart_command(cmd: List[str], *, stdout=subprocess.PIPE,
stderr=subprocess.PIPE, stdin=subprocess.DEVNULL,
- env=None, **kw):
+ env=None, **kw) -> asyncio.subprocess.Process:
log.debug("astart_command called: %s", cmd)
return await asyncio.create_subprocess_exec(
*cmd, stdout=stdout, stderr=stderr,
env=_clean_env(env), **kw)
-async def split_cmd_output(cmd, split_on):
+async def split_cmd_output(cmd: List[str], split_on: str) -> List[str]:
cp = await arun_command(cmd, check=True)
return cp.stdout.split(split_on)
-def start_command(cmd, *, stdin=subprocess.DEVNULL, stdout=subprocess.PIPE,
+def start_command(cmd: List[str], *,
+ stdin=subprocess.DEVNULL, stdout=subprocess.PIPE,
stderr=subprocess.PIPE, encoding='utf-8', errors='replace',
- env=None, **kw):
+ env=None, **kw) -> subprocess.Popen:
"""A wrapper around subprocess.Popen with logging and different defaults.
We never ever want a subprocess to inherit our file descriptors!