Skip to content

payload/rpm-ostree: Include program output in exception #6431

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 30, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions pyanaconda/core/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,26 @@ def execWithCapture(command, argv, stdin=None, root='/', env_prune=None, env_add
return _run_program(argv, stdin=stdin, root=root, log_output=log_output, env_prune=env_prune,
env_add=env_add, filter_stderr=filter_stderr, do_preexec=do_preexec)[1]

def execProgram(command, argv, stdin=None, root='/', env_prune=None, env_add=None,
log_output=True, filter_stderr=False, do_preexec=True):
""" Run an external program and capture standard out and err as well as the return code.

:param command: The command to run
:param argv: The argument list
:param stdin: The file object to read stdin from.
:param root: The directory to chroot to before running command.
:param env_prune: environment variable to remove before execution
:param env_add: environment variables added for the execution
:param log_output: Whether to log the output of command
:param filter_stderr: Whether stderr should be excluded from the returned output
:param do_preexec: whether to use the preexec function
:return: Tuple of the return code and the output of the command
"""
argv = [command] + argv

return _run_program(argv, stdin=stdin, root=root, log_output=log_output, env_prune=env_prune,
env_add=env_add, filter_stderr=filter_stderr, do_preexec=do_preexec)


def execWithCaptureAsLiveUser(command, argv, stdin=None, root='/', log_output=True,
filter_stderr=False, do_preexec=True):
Expand Down
39 changes: 20 additions & 19 deletions pyanaconda/modules/payloads/payload/rpm_ostree/installation.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
from pyanaconda.core.glib import GError, Variant, create_new_context, format_size_full
from pyanaconda.core.i18n import _
from pyanaconda.core.path import make_directories, set_system_root
from pyanaconda.core.util import execWithRedirect
from pyanaconda.core.util import execProgram, execWithRedirect
from pyanaconda.modules.common.constants.objects import BOOTLOADER, DEVICE_TREE
from pyanaconda.modules.common.constants.services import STORAGE
from pyanaconda.modules.common.errors.installation import (
Expand All @@ -47,16 +47,17 @@
log = get_module_logger(__name__)


def safe_exec_with_redirect(cmd, argv, successful_return_codes=(0,), **kwargs):
"""Like util.execWithRedirect, but treat errors as fatal.
def safe_exec_program(cmd, argv, successful_return_codes=(0,), **kwargs):
"""Like util.execProgram, but treat errors as fatal.

:raise: PayloadInstallationError if the call fails for any reason
"""
rc = execWithRedirect(cmd, argv, **kwargs)
rc, output = execProgram(cmd, argv, **kwargs)

if rc not in successful_return_codes:
raise PayloadInstallationError(
"The command '{}' exited with the code {}.".format(" ".join([cmd] + argv), rc)
"The command '{}' exited with the code {}:\n{}".format(" ".join([cmd] + argv), rc,
output)
)


Expand Down Expand Up @@ -170,8 +171,8 @@ def _setup_internal_bindmount(self, src, dest=None,
dest = os.path.realpath(dest)

if bind_ro:
safe_exec_with_redirect("mount", ["--bind", src, src])
safe_exec_with_redirect("mount", ["--bind", "-o", "remount,ro", src, src])
safe_exec_program("mount", ["--bind", src, src])
safe_exec_program("mount", ["--bind", "-o", "remount,ro", src, src])
else:
# Create missing directories for user defined mount points
if not os.path.exists(dest):
Expand All @@ -183,7 +184,7 @@ def _setup_internal_bindmount(self, src, dest=None,
bindopt = '--rbind'
else:
bindopt = '--bind'
safe_exec_with_redirect("mount", [bindopt, src, dest])
safe_exec_program("mount", [bindopt, src, dest])

self._internal_mounts.append(src if bind_ro else dest)

Expand Down Expand Up @@ -240,7 +241,7 @@ def _create_tmpfiles(self, path):
# Therefore we ignore error 65, since this is coming from
# the payload itself and the actual execution of it was fine

safe_exec_with_redirect(
safe_exec_program(
"systemd-tmpfiles", [
"--create",
"--boot",
Expand Down Expand Up @@ -377,7 +378,7 @@ def _run(self):
# doesn't, we're not on a UEFI system, so we don't want to copy the data.
if not fname == 'efi' or is_efi and os.path.isdir(os.path.join(physboot, fname)):
log.info("Copying bootloader data: %s", fname)
safe_exec_with_redirect('cp', ['-r', '-p', srcpath, physboot])
safe_exec_program('cp', ['-r', '-p', srcpath, physboot])

# Unfortunate hack, see https://github.com/rhinstaller/anaconda/issues/1188
efi_grubenv_link = physboot + '/grub2/grubenv'
Expand Down Expand Up @@ -405,7 +406,7 @@ def run(self):

This will create the repository as well.
"""
safe_exec_with_redirect(
safe_exec_program(
"ostree",
["admin",
"--sysroot=" + self._physroot,
Expand Down Expand Up @@ -582,12 +583,12 @@ def _set_kargs(self):

set_kargs_args.append("rw")

safe_exec_with_redirect("ostree", set_kargs_args, root=self._sysroot)
safe_exec_program("ostree", set_kargs_args, root=self._sysroot)

if arch.is_s390():
# Deployment was done. Enable ostree's zipl support; this is how things are currently done in e.g.
# https://github.com/coreos/coreos-assembler/blob/7d6fa376fc9f73625487adbb9386785bb09f1bb2/src/osbuild-manifests/coreos.osbuild.s390x.mpp.yaml#L261
safe_exec_with_redirect(
safe_exec_program(
"ostree",
["config",
"--repo=" + self._sysroot + "/ostree/repo",
Expand All @@ -607,7 +608,7 @@ def _set_kargs(self):
break

# pylint: disable=possibly-used-before-assignment
safe_exec_with_redirect(
safe_exec_program(
"zipl",
["-V",
"-i",
Expand Down Expand Up @@ -646,14 +647,14 @@ def run(self):
if arch.is_s390():
# Disable ostree's builtin zipl support; this is how things are currently done in e.g.
# https://github.com/coreos/coreos-assembler/blob/7d6fa376fc9f73625487adbb9386785bb09f1bb2/src/osbuild-manifests/coreos.osbuild.s390x.mpp.yaml#L168
safe_exec_with_redirect(
safe_exec_program(
"ostree",
["config",
"--repo=" + self._physroot + "/ostree/repo",
"set", "sysroot.bootloader", "none"]
)

safe_exec_with_redirect(
safe_exec_program(
"ostree",
["admin",
"--sysroot=" + self._physroot,
Expand All @@ -675,13 +676,13 @@ def run(self):
if not self._data.signature_verification_enabled:
args.append("--no-signature-verification")

safe_exec_with_redirect(
safe_exec_program(
"ostree",
args
)
else:
log.info("ostree admin deploy starting")
safe_exec_with_redirect(
safe_exec_program(
"ostree",
["admin",
"--sysroot=" + self._physroot,
Expand All @@ -692,7 +693,7 @@ def run(self):

log.info("ostree config set sysroot.readonly true")

safe_exec_with_redirect(
safe_exec_program(
"ostree",
["config",
"--repo=" + self._physroot + "/ostree/repo",
Expand Down
Loading