Merge pull request #1185 from ogayot/systemd-run

Add option to run commands with private mounts and use it for curtin in-target
This commit is contained in:
Michael Hudson-Doyle 2022-03-08 09:34:41 +13:00 committed by GitHub
commit e6d2c8bdb8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 281 additions and 55 deletions

View File

@ -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')

View File

@ -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

View File

@ -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()

View File

@ -14,28 +14,68 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
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

View File

@ -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 <http://www.gnu.org/licenses/>.
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)

View File

@ -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"])

View File

@ -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.

View File

@ -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!