From a0e45e7e8faf6f78bb388e8366c11db9db9e8ff7 Mon Sep 17 00:00:00 2001 From: Miroslav Vadkerti Date: Fri, 20 Sep 2024 01:40:14 +0200 Subject: [PATCH 01/17] Support Fedora Image Mode To support Fedora Image mode the following changes were required: * Fix `rsync` installation. The tool is not included in the distribution and the installation method was completely broken, i.e. `rpm-ostree` has no `--install-root` option. We did not hit it as there is no good testing environment available that would cover this. * Introduce `TMT_SCRIPTS_DEST_DIR` environment variable set by default to `/var/tmp/tmt/bin` which hosts the `tmt` scripts. The original path is not writable on these systems. The `PATH` is set system wide via a new `/etc/profile.d/tmt.sh` script. Signed-off-by: Miroslav Vadkerti --- .github/workflows/shellcheck.yml | 1 + tmt/steps/execute/__init__.py | 91 ++++++++++++++++++++++++----- tmt/steps/execute/scripts/tmt.sh.j2 | 4 ++ tmt/steps/provision/__init__.py | 18 +----- 4 files changed, 83 insertions(+), 31 deletions(-) create mode 100644 tmt/steps/execute/scripts/tmt.sh.j2 diff --git a/.github/workflows/shellcheck.yml b/.github/workflows/shellcheck.yml index 40a31c0d4e..a29671f37f 100644 --- a/.github/workflows/shellcheck.yml +++ b/.github/workflows/shellcheck.yml @@ -28,6 +28,7 @@ jobs: exclude-path: | tests/** examples/**/test.sh + tmt/steps/execute/scripts/tmt.sh.j2 tmt/templates/** token: ${{ secrets.GITHUB_TOKEN }} diff --git a/tmt/steps/execute/__init__.py b/tmt/steps/execute/__init__.py index 407b4ac096..56a89467db 100644 --- a/tmt/steps/execute/__init__.py +++ b/tmt/steps/execute/__init__.py @@ -5,6 +5,7 @@ import os import signal as _signal import subprocess +import tempfile import threading from contextlib import suppress from dataclasses import dataclass @@ -27,6 +28,7 @@ from tmt.steps.discover import Discover, DiscoverPlugin, DiscoverStepData from tmt.steps.provision import Guest from tmt.utils import ( + Command, Path, ShellScript, Stopwatch, @@ -34,6 +36,7 @@ format_duration, format_timestamp, ) +from tmt.utils.templates import render_template_file if TYPE_CHECKING: import tmt.cli @@ -58,6 +61,9 @@ # Scripts source directory SCRIPTS_SRC_DIR = tmt.utils.resource_files('steps/execute/scripts') +#: The default scripts destination directory +SCRIPTS_DEST_DIR = Path("/var/tmp/tmt/bin") # noqa: S108 insecure usage of temporary dir + @dataclass class Script: @@ -75,12 +81,36 @@ class ScriptCreatingFile(Script): created_file: str +@dataclass +class ScriptTemplate(Script): + """ + Represents a Jinja2 templated script. + The source filename must have a ``.j2`` suffix. + """ + + context: dict[str, str] + + +def effective_scripts_dest_dir() -> Path: + """ + Find out what the actual scripts destination directory is. + + If ``TMT_SCRIPTS_DEST_DIR`` variable is set, it is used as the scripts destination + directory. Otherwise, the default of :py:data:`SCRIPTS_DEST_DIR` is used. + """ + + if 'TMT_SCRIPTS_DEST_DIR' in os.environ: + return Path(os.environ['TMT_SCRIPTS_DEST_DIR']) + + return SCRIPTS_DEST_DIR + + # Script handling reboots, in restraint compatible fashion TMT_REBOOT_SCRIPT = ScriptCreatingFile( - path=Path("/usr/local/bin/tmt-reboot"), + path=effective_scripts_dest_dir() / 'tmt-reboot', aliases=[ - Path("/usr/local/bin/rstrnt-reboot"), - Path("/usr/local/bin/rhts-reboot")], + effective_scripts_dest_dir() / 'rstrnt-reboot', + effective_scripts_dest_dir() / 'rhts-reboot'], related_variables=[ "TMT_REBOOT_COUNT", "REBOOTCOUNT", @@ -89,43 +119,54 @@ class ScriptCreatingFile(Script): ) TMT_REBOOT_CORE_SCRIPT = Script( - path=Path("/usr/local/bin/tmt-reboot-core"), + path=effective_scripts_dest_dir() / 'tmt-reboot-core', aliases=[], related_variables=[]) # Script handling result reporting, in restraint compatible fashion TMT_REPORT_RESULT_SCRIPT = ScriptCreatingFile( - path=Path("/usr/local/bin/tmt-report-result"), + path=effective_scripts_dest_dir() / 'tmt-report-result', aliases=[ - Path("/usr/local/bin/rstrnt-report-result"), - Path("/usr/local/bin/rhts-report-result")], + effective_scripts_dest_dir() / 'rstrnt-report-result', + effective_scripts_dest_dir() / 'rhts-report-result'], related_variables=[], created_file="tmt-report-results.yaml" ) # Script for archiving a file, usable for BEAKERLIB_COMMAND_SUBMIT_LOG TMT_FILE_SUBMIT_SCRIPT = Script( - path=Path("/usr/local/bin/tmt-file-submit"), + path=effective_scripts_dest_dir() / 'tmt-file-submit', aliases=[ - Path("/usr/local/bin/rstrnt-report-log"), - Path("/usr/local/bin/rhts-submit-log"), - Path("/usr/local/bin/rhts_submit_log")], + effective_scripts_dest_dir() / 'rstrnt-report-log', + effective_scripts_dest_dir() / 'rhts-submit-log', + effective_scripts_dest_dir() / 'rhts_submit_log'], related_variables=[] ) # Script handling text execution abortion, in restraint compatible fashion TMT_ABORT_SCRIPT = ScriptCreatingFile( - path=Path("/usr/local/bin/tmt-abort"), + path=effective_scripts_dest_dir() / 'tmt-abort', aliases=[ - Path("/usr/local/bin/rstrnt-abort"), - Path("/usr/local/bin/rhts-abort")], + effective_scripts_dest_dir() / 'rstrnt-abort', + effective_scripts_dest_dir() / 'rhts-abort'], related_variables=[], created_file="abort" ) +# Profile script for adding SCRIPTS_DEST_DIR to executable pats system-wide +TMT_ETC_PROFILE_D = ScriptTemplate( + path=Path("/etc/profile.d/tmt.sh"), + aliases=[], + related_variables=[], + context={ + 'scripts_dest_dir': str(effective_scripts_dest_dir()) + }) + + # List of all available scripts SCRIPTS = ( TMT_ABORT_SCRIPT, + TMT_ETC_PROFILE_D, TMT_FILE_SUBMIT_SCRIPT, TMT_REBOOT_SCRIPT, TMT_REBOOT_CORE_SCRIPT, @@ -594,12 +635,30 @@ def prepare_tests(self, guest: Guest, logger: tmt.log.Logger) -> list[TestInvoca return invocations + def _render_script_template(self, source: Path, context: dict[str, str]) -> Path: + """ Render script template with given context """ + + with tempfile.NamedTemporaryFile(mode='w', delete=False) as rendered_script: + rendered_script.write(render_template_file(source, None, **context)) + + return Path(rendered_script.name) + def prepare_scripts(self, guest: "tmt.steps.provision.Guest") -> None: """ Prepare additional scripts for testing """ + # Create scripts directory + guest.execute(Command("mkdir", "-p", str(SCRIPTS_DEST_DIR))) + # Install all scripts on guest for script in self.scripts: source = SCRIPTS_SRC_DIR / script.path.name + # Render script template + if isinstance(script, ScriptTemplate): + source = self._render_script_template( + SCRIPTS_SRC_DIR / f"{script.path.name}.j2", + context=script.context + ) + for dest in [script.path, *script.aliases]: guest.push( source=source, @@ -607,6 +666,10 @@ def prepare_scripts(self, guest: "tmt.steps.provision.Guest") -> None: options=["-p", "--chmod=755"], superuser=guest.facts.is_superuser is not True) + # Remove script template source + if isinstance(script, ScriptTemplate): + os.unlink(source) + def _tmt_report_results_filepath(self, invocation: TestInvocation) -> Path: """ Create path to test's ``tmt-report-result`` file """ diff --git a/tmt/steps/execute/scripts/tmt.sh.j2 b/tmt/steps/execute/scripts/tmt.sh.j2 new file mode 100644 index 0000000000..a916794965 --- /dev/null +++ b/tmt/steps/execute/scripts/tmt.sh.j2 @@ -0,0 +1,4 @@ +# shellcheck shell=bash + +# tmt provides executable scripts under this path +export PATH={{ scripts_dest_dir }}:$PATH diff --git a/tmt/steps/provision/__init__.py b/tmt/steps/provision/__init__.py index 0d33ad6048..6cbb65faa0 100644 --- a/tmt/steps/provision/__init__.py +++ b/tmt/steps/provision/__init__.py @@ -1306,23 +1306,7 @@ def _check_rsync(self) -> CheckRsyncOutcome: except tmt.utils.RunError: pass - # Install under '/root/pkg' for read-only distros - # (for now the check is based on 'rpm-ostree' presence) - # FIXME: Find a better way how to detect read-only distros - # self.debug("Check for a read-only distro.") - if self.facts.package_manager == 'rpm-ostree': - self.package_manager.install( - Package('rsync'), - options=tmt.package_managers.Options( - install_root=Path('/root/pkg'), - release_version='/' - ) - ) - - self.execute(Command('ln', '-sf', '/root/pkg/bin/rsync', '/usr/local/bin/rsync')) - - else: - self.package_manager.install(Package('rsync')) + self.package_manager.install(Package('rsync')) return CheckRsyncOutcome.INSTALLED From d3319726c8108cb518906bc993a8c363ca334a37 Mon Sep 17 00:00:00 2001 From: Miroslav Vadkerti Date: Tue, 1 Oct 2024 20:39:11 +0200 Subject: [PATCH 02/17] Support Fedora Image Mode To support Fedora Image mode the following changes were required: * Fix `rsync` installation. The tool is not included in the distribution and the installation method was completely broken, i.e. `rpm-ostree` has no `--install-root` option. We did not hit it as there is no good testing environment available that would cover this. * Introduce `TMT_SCRIPTS_DEST_DIR` environment variable set by default to `/var/tmp/tmt/bin` which hosts the `tmt` scripts. The original path is not writable on these systems. The `PATH` is set system wide via a new `/etc/profile.d/tmt.sh` script. Signed-off-by: Miroslav Vadkerti --- .github/workflows/shellcheck.yml | 2 +- tmt/steps/execute/__init__.py | 96 ++++++++++++++++++----------- tmt/steps/execute/scripts/tmt.sh.j2 | 2 +- 3 files changed, 61 insertions(+), 39 deletions(-) diff --git a/.github/workflows/shellcheck.yml b/.github/workflows/shellcheck.yml index a29671f37f..6af26cdf77 100644 --- a/.github/workflows/shellcheck.yml +++ b/.github/workflows/shellcheck.yml @@ -28,7 +28,7 @@ jobs: exclude-path: | tests/** examples/**/test.sh - tmt/steps/execute/scripts/tmt.sh.j2 + tmt/steps/execute/scripts/*.sh.j2 tmt/templates/** token: ${{ secrets.GITHUB_TOKEN }} diff --git a/tmt/steps/execute/__init__.py b/tmt/steps/execute/__init__.py index 56a89467db..a85e4eb137 100644 --- a/tmt/steps/execute/__init__.py +++ b/tmt/steps/execute/__init__.py @@ -5,10 +5,10 @@ import os import signal as _signal import subprocess -import tempfile import threading from contextlib import suppress from dataclasses import dataclass +from tempfile import NamedTemporaryFile from typing import TYPE_CHECKING, Any, Optional, TypeVar, Union, cast import click @@ -58,7 +58,7 @@ # Metadata file with details about the current test TEST_METADATA_FILENAME = 'metadata.yaml' -# Scripts source directory +#: Scripts source directory SCRIPTS_SRC_DIR = tmt.utils.resource_files('steps/execute/scripts') #: The default scripts destination directory @@ -67,16 +67,35 @@ @dataclass class Script: - """ Represents a script provided by the internal executor """ + """ + Represents a script provided by the internal executor. + + Must be used as a context manager. The context manager returns + the source file path. + + The source file name matches the name of the file specified via + the ``path` attribute and must be located in the directory specified + via :py:data:`SCRIPTS_SRC_DIR` variable. + """ path: Path aliases: list[Path] related_variables: list[str] + def __enter__(self) -> Path: + return SCRIPTS_SRC_DIR / self.path.name + + def __exit__(self, *args: object) -> None: + pass + @dataclass class ScriptCreatingFile(Script): - """ Represents a script which creates a file """ + """ + Represents a script which creates a file. + + See :py:class:`Script` for more details. + """ created_file: str @@ -85,18 +104,41 @@ class ScriptCreatingFile(Script): class ScriptTemplate(Script): """ Represents a Jinja2 templated script. - The source filename must have a ``.j2`` suffix. + + Must be used as a context manager. The context manager returns + the source file path. + + The source file name is constructed from the name of the file specified + via the ``path` attribute that is extended with the ``.j2`` suffix. + The template file must be located in the directory specified + via :py:data:`SCRIPTS_SRC_DIR` variable. """ context: dict[str, str] + _rendered_script_path: Optional[Path] = None + + def __enter__(self) -> Path: + with NamedTemporaryFile(mode='w', delete=False) as rendered_script: + rendered_script.write(render_template_file( + SCRIPTS_SRC_DIR / f"{self.path.name}.j2", None, **self.context)) + + self._rendered_script_path = Path(rendered_script.name) + + return self._rendered_script_path + + def __exit__(self, *args: object) -> None: + assert self._rendered_script_path + os.unlink(self._rendered_script_path) + def effective_scripts_dest_dir() -> Path: """ Find out what the actual scripts destination directory is. - If ``TMT_SCRIPTS_DEST_DIR`` variable is set, it is used as the scripts destination - directory. Otherwise, the default of :py:data:`SCRIPTS_DEST_DIR` is used. + If the ``TMT_SCRIPTS_DEST_DIR`` variable is set, it is used as the scripts + destination directory. Otherwise, the default of :py:data:`SCRIPTS_DEST_DIR` + is used. """ if 'TMT_SCRIPTS_DEST_DIR' in os.environ: @@ -153,13 +195,13 @@ def effective_scripts_dest_dir() -> Path: created_file="abort" ) -# Profile script for adding SCRIPTS_DEST_DIR to executable pats system-wide +# Profile script for adding SCRIPTS_DEST_DIR to executable paths system-wide TMT_ETC_PROFILE_D = ScriptTemplate( path=Path("/etc/profile.d/tmt.sh"), aliases=[], related_variables=[], context={ - 'scripts_dest_dir': str(effective_scripts_dest_dir()) + 'SCRIPTS_DEST_DIR': str(effective_scripts_dest_dir()) }) @@ -635,40 +677,20 @@ def prepare_tests(self, guest: Guest, logger: tmt.log.Logger) -> list[TestInvoca return invocations - def _render_script_template(self, source: Path, context: dict[str, str]) -> Path: - """ Render script template with given context """ - - with tempfile.NamedTemporaryFile(mode='w', delete=False) as rendered_script: - rendered_script.write(render_template_file(source, None, **context)) - - return Path(rendered_script.name) - def prepare_scripts(self, guest: "tmt.steps.provision.Guest") -> None: """ Prepare additional scripts for testing """ # Create scripts directory - guest.execute(Command("mkdir", "-p", str(SCRIPTS_DEST_DIR))) + guest.execute(Command("mkdir", "-p", str(effective_scripts_dest_dir()))) # Install all scripts on guest for script in self.scripts: - source = SCRIPTS_SRC_DIR / script.path.name - - # Render script template - if isinstance(script, ScriptTemplate): - source = self._render_script_template( - SCRIPTS_SRC_DIR / f"{script.path.name}.j2", - context=script.context - ) - - for dest in [script.path, *script.aliases]: - guest.push( - source=source, - destination=dest, - options=["-p", "--chmod=755"], - superuser=guest.facts.is_superuser is not True) - - # Remove script template source - if isinstance(script, ScriptTemplate): - os.unlink(source) + with script as source: + for dest in [script.path, *script.aliases]: + guest.push( + source=source, + destination=dest, + options=["-p", "--chmod=755"], + superuser=guest.facts.is_superuser is not True) def _tmt_report_results_filepath(self, invocation: TestInvocation) -> Path: """ Create path to test's ``tmt-report-result`` file """ diff --git a/tmt/steps/execute/scripts/tmt.sh.j2 b/tmt/steps/execute/scripts/tmt.sh.j2 index a916794965..bb8023e8f5 100644 --- a/tmt/steps/execute/scripts/tmt.sh.j2 +++ b/tmt/steps/execute/scripts/tmt.sh.j2 @@ -1,4 +1,4 @@ # shellcheck shell=bash # tmt provides executable scripts under this path -export PATH={{ scripts_dest_dir }}:$PATH +export PATH={{ SCRIPTS_DEST_DIR }}:$PATH From cbd50b6bf2078235e6de6fedcf22f5e2ad02e83d Mon Sep 17 00:00:00 2001 From: Miroslav Vadkerti Date: Tue, 1 Oct 2024 20:50:19 +0200 Subject: [PATCH 03/17] Fix grammar --- tmt/steps/execute/__init__.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tmt/steps/execute/__init__.py b/tmt/steps/execute/__init__.py index a85e4eb137..4adf38f935 100644 --- a/tmt/steps/execute/__init__.py +++ b/tmt/steps/execute/__init__.py @@ -74,7 +74,7 @@ class Script: the source file path. The source file name matches the name of the file specified via - the ``path` attribute and must be located in the directory specified + the ``path`` attribute and must be located in the directory specified via :py:data:`SCRIPTS_SRC_DIR` variable. """ @@ -109,7 +109,7 @@ class ScriptTemplate(Script): the source file path. The source file name is constructed from the name of the file specified - via the ``path` attribute that is extended with the ``.j2`` suffix. + via the ``path`` attribute, with the ``.j2`` suffix appended. The template file must be located in the directory specified via :py:data:`SCRIPTS_SRC_DIR` variable. """ @@ -136,9 +136,9 @@ def effective_scripts_dest_dir() -> Path: """ Find out what the actual scripts destination directory is. - If the ``TMT_SCRIPTS_DEST_DIR`` variable is set, it is used as the scripts - destination directory. Otherwise, the default of :py:data:`SCRIPTS_DEST_DIR` - is used. + If the ``TMT_SCRIPTS_DEST_DIR`` environment variable is set, it is used + as the scripts destination directory. Otherwise, the default + of :py:data:`SCRIPTS_DEST_DIR` is used. """ if 'TMT_SCRIPTS_DEST_DIR' in os.environ: From 1a8e95bfff87471da483f33703244c1e2a4e3553 Mon Sep 17 00:00:00 2001 From: Miroslav Vadkerti Date: Wed, 2 Oct 2024 12:51:57 +0200 Subject: [PATCH 04/17] Update tmt/steps/execute/scripts/tmt.sh.j2 --- tmt/steps/execute/scripts/tmt.sh.j2 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tmt/steps/execute/scripts/tmt.sh.j2 b/tmt/steps/execute/scripts/tmt.sh.j2 index bb8023e8f5..19afcf078a 100644 --- a/tmt/steps/execute/scripts/tmt.sh.j2 +++ b/tmt/steps/execute/scripts/tmt.sh.j2 @@ -1,4 +1,4 @@ # shellcheck shell=bash # tmt provides executable scripts under this path -export PATH={{ SCRIPTS_DEST_DIR }}:$PATH +export PATH="{{ SCRIPTS_DEST_DIR }}:$PATH" From 3326c2d895f1cb549ba61130c0061b83b4db2b48 Mon Sep 17 00:00:00 2001 From: Miroslav Vadkerti Date: Wed, 2 Oct 2024 13:33:37 +0200 Subject: [PATCH 05/17] Added docs Signed-off-by: Miroslav Vadkerti --- docs/overview.rst | 10 ++++++++++ spec/plans/execute.fmf | 13 +++++++++++++ 2 files changed, 23 insertions(+) diff --git a/docs/overview.rst b/docs/overview.rst index 6d19584817..72865429da 100644 --- a/docs/overview.rst +++ b/docs/overview.rst @@ -466,6 +466,16 @@ TMT_REBOOT_TIMEOUT How many seconds to wait for a connection to succeed after guest reboot. By default, it is 10 minutes. + +TMT_SCRIPTS_DEST_DIR + Destination directory for storing ``tmt`` scripts on the guest. + By default ``/var/tmp/tmt/bin`` is used. For more information + see the `tmt internal test executor`__ documentation. + +__ https://tmt.readthedocs.io/en/stable/spec/plans.html#tmt + + .. versionadded:: 1.38 + TMT_SSH_* Every environment variable in this format would be treated as an SSH option, and passed to the ``-o`` option of ``ssh`` command. See diff --git a/spec/plans/execute.fmf b/spec/plans/execute.fmf index 93bdb2cfeb..eb8f73ca63 100644 --- a/spec/plans/execute.fmf +++ b/spec/plans/execute.fmf @@ -200,6 +200,19 @@ description: | For more information see the :ref:`/stories/features/abort` feature documentation. + The scripts are hosted by default in the ``/var/tmp/tmt/bin`` + directory. The directory can be changed using + the ``TMT_SCRIPTS_DEST_DIR`` environment variable. This directory + works across all supported operating systems, including those using + ``rpm-ostree`` and ``bootc``. The path is added to executable paths + using the system-wide ``/etc/profile.d/tmt.sh`` profile script. + + .. warning:: + + Please be aware that the provided scripts will only be available + in a shell that loads the profile scripts. This is the default + for ``bash``-like shells, but might not work for others. + example: | execute: how: tmt From 03807fba3f788789afafb1a144dc82655296784b Mon Sep 17 00:00:00 2001 From: Miroslav Vadkerti Date: Wed, 16 Oct 2024 13:38:19 +0200 Subject: [PATCH 06/17] Add rpm-ostree detection, install to special path only in image mode Signed-off-by: Miroslav Vadkerti --- docs/overview.rst | 6 +- spec/plans/execute.fmf | 20 +++-- tmt/frameworks/beakerlib.py | 2 +- tmt/steps/execute/__init__.py | 141 +++++++++++++++++++++----------- tmt/steps/provision/__init__.py | 16 ++++ 5 files changed, 127 insertions(+), 58 deletions(-) diff --git a/docs/overview.rst b/docs/overview.rst index 72865429da..98f34ee196 100644 --- a/docs/overview.rst +++ b/docs/overview.rst @@ -469,8 +469,10 @@ TMT_REBOOT_TIMEOUT TMT_SCRIPTS_DEST_DIR Destination directory for storing ``tmt`` scripts on the guest. - By default ``/var/tmp/tmt/bin`` is used. For more information - see the `tmt internal test executor`__ documentation. + By default ``/usr/local/bin`` is used, except for guests using + ``rpm-ostree``, where ``/var/lib/tmt/scripts`` is used. See the + `tmt internal test executor`__ documentation for more details + on the scripts installed on the guest. __ https://tmt.readthedocs.io/en/stable/spec/plans.html#tmt diff --git a/spec/plans/execute.fmf b/spec/plans/execute.fmf index eb8f73ca63..bf1f758efd 100644 --- a/spec/plans/execute.fmf +++ b/spec/plans/execute.fmf @@ -200,18 +200,20 @@ description: | For more information see the :ref:`/stories/features/abort` feature documentation. - The scripts are hosted by default in the ``/var/tmp/tmt/bin`` - directory. The directory can be changed using - the ``TMT_SCRIPTS_DEST_DIR`` environment variable. This directory - works across all supported operating systems, including those using - ``rpm-ostree`` and ``bootc``. The path is added to executable paths - using the system-wide ``/etc/profile.d/tmt.sh`` profile script. + The scripts are hosted by default in the ``/usr/local/bin`` + directory, except for guests using ``rpm-ostree``, where + ``/var/lib/tmt/scripts`` is used. The directory can be forced + using the ``TMT_SCRIPTS_DEST_DIR`` environment variable. + Note that for guests using ``rpm-ostree``, the directory is added + to executable paths using the system-wide ``/etc/profile.d/tmt.sh`` + profile script. .. warning:: - Please be aware that the provided scripts will only be available - in a shell that loads the profile scripts. This is the default - for ``bash``-like shells, but might not work for others. + Please be aware that for guests using ``rpm-ostree`` + the provided scripts will only be available in a shell that + loads the profile scripts. This is the default for + ``bash``-like shells, but might not work for others. example: | execute: diff --git a/tmt/frameworks/beakerlib.py b/tmt/frameworks/beakerlib.py index a722f0b82b..507dd0f5d0 100644 --- a/tmt/frameworks/beakerlib.py +++ b/tmt/frameworks/beakerlib.py @@ -36,7 +36,7 @@ def get_environment_variables( return Environment({ 'BEAKERLIB_DIR': EnvVarValue(invocation.path), 'BEAKERLIB_COMMAND_SUBMIT_LOG': EnvVarValue( - f'bash {tmt.steps.execute.TMT_FILE_SUBMIT_SCRIPT.path}') + f'bash {tmt.steps.execute.TMT_FILE_SUBMIT_SCRIPT.guest_path(invocation.guest)}') }) @classmethod diff --git a/tmt/steps/execute/__init__.py b/tmt/steps/execute/__init__.py index 4adf38f935..a15c6374d1 100644 --- a/tmt/steps/execute/__init__.py +++ b/tmt/steps/execute/__init__.py @@ -9,7 +9,7 @@ from contextlib import suppress from dataclasses import dataclass from tempfile import NamedTemporaryFile -from typing import TYPE_CHECKING, Any, Optional, TypeVar, Union, cast +from typing import TYPE_CHECKING, Any, Callable, Optional, TypeVar, Union, cast import click import fmf @@ -62,7 +62,13 @@ SCRIPTS_SRC_DIR = tmt.utils.resource_files('steps/execute/scripts') #: The default scripts destination directory -SCRIPTS_DEST_DIR = Path("/var/tmp/tmt/bin") # noqa: S108 insecure usage of temporary dir +SCRIPTS_DEST_DIR = Path("/usr/local/bin") + +#: The default scripts destination directory for rpm-ostree based distributions, https://github.com/teemtee/tmt/discussions/3260 +SCRIPTS_DEST_DIR_OSTREE = Path("/var/lib/tmt/scripts") + +#: The default tmt environment variable name for forcing ``SCRIPTS_DEST_DIR`` +SCRIPTS_DEST_DIR_VARIABLE = 'TMT_SCRIPTS_DEST_DIR' @dataclass @@ -73,21 +79,43 @@ class Script: Must be used as a context manager. The context manager returns the source file path. - The source file name matches the name of the file specified via - the ``path`` attribute and must be located in the directory specified - via :py:data:`SCRIPTS_SRC_DIR` variable. + The source file is defined by the ``source_filename`` attribute and its + location is relative to the directory specified via the :py:data:`SCRIPTS_SRC_DIR` + variable. All scripts must be located in this directory. + + The default destination directory of the scripts is :py:data:`SCRIPTS_DEST_DIR`. + On ``rpm-ostree`` distributions like Fedora CoreOS, the default destination + directory is :py:data:``SCRIPTS_DEST_DIR_OSTREE``. The destination directory + of the scripts can be forced by the script using ``destination_path`` attribute. + + The destination directory can be overridden using the environment variable defined + by the :py:data:`SCRIPTS_DEST_DIR_VARIABLE` variable. + + The ``enabled`` attribute can specify a function which is called with :py:class:`Guest` + instance to evaluate if the script is enabled. This can be useful to optionally disable + a script for specific guests. """ - path: Path - aliases: list[Path] + source_filename: str + destination_path: Optional[Path] + aliases: list[str] related_variables: list[str] + enabled: Callable[[Guest], bool] def __enter__(self) -> Path: - return SCRIPTS_SRC_DIR / self.path.name + return SCRIPTS_SRC_DIR / self.source_filename def __exit__(self, *args: object) -> None: pass + def guest_path(self, guest: Guest, alias: Optional[str] = None) -> Path: + """ Return path of the script or the script alias on a guest """ + filename = alias or self.source_filename + scripts_dest_dir = effective_scripts_dest_dir( + default=SCRIPTS_DEST_DIR_OSTREE if guest.facts.is_ostree else SCRIPTS_DEST_DIR + ) + return self.destination_path if self.destination_path else scripts_dest_dir / filename + @dataclass class ScriptCreatingFile(Script): @@ -108,8 +136,8 @@ class ScriptTemplate(Script): Must be used as a context manager. The context manager returns the source file path. - The source file name is constructed from the name of the file specified - via the ``path`` attribute, with the ``.j2`` suffix appended. + The source filename is constructed from the name of the file specified + via the ``source_filename`` attribute, with the ``.j2`` suffix appended. The template file must be located in the directory specified via :py:data:`SCRIPTS_SRC_DIR` variable. """ @@ -121,7 +149,7 @@ class ScriptTemplate(Script): def __enter__(self) -> Path: with NamedTemporaryFile(mode='w', delete=False) as rendered_script: rendered_script.write(render_template_file( - SCRIPTS_SRC_DIR / f"{self.path.name}.j2", None, **self.context)) + SCRIPTS_SRC_DIR / f"{self.source_filename}.j2", None, **self.context)) self._rendered_script_path = Path(rendered_script.name) @@ -132,77 +160,91 @@ def __exit__(self, *args: object) -> None: os.unlink(self._rendered_script_path) -def effective_scripts_dest_dir() -> Path: +def effective_scripts_dest_dir(default: Path = SCRIPTS_DEST_DIR) -> Path: """ Find out what the actual scripts destination directory is. If the ``TMT_SCRIPTS_DEST_DIR`` environment variable is set, it is used - as the scripts destination directory. Otherwise, the default - of :py:data:`SCRIPTS_DEST_DIR` is used. + as the scripts destination directory. Otherwise, the ``default`` + is returned. """ - if 'TMT_SCRIPTS_DEST_DIR' in os.environ: - return Path(os.environ['TMT_SCRIPTS_DEST_DIR']) + if SCRIPTS_DEST_DIR_VARIABLE in os.environ: + return Path(os.environ[SCRIPTS_DEST_DIR_VARIABLE]) - return SCRIPTS_DEST_DIR + return default # Script handling reboots, in restraint compatible fashion TMT_REBOOT_SCRIPT = ScriptCreatingFile( - path=effective_scripts_dest_dir() / 'tmt-reboot', + source_filename='tmt-reboot', + destination_path=None, aliases=[ - effective_scripts_dest_dir() / 'rstrnt-reboot', - effective_scripts_dest_dir() / 'rhts-reboot'], + 'rstrnt-reboot', + 'rhts-reboot'], related_variables=[ "TMT_REBOOT_COUNT", "REBOOTCOUNT", "RSTRNT_REBOOTCOUNT"], - created_file="reboot-request" + created_file="reboot-request", + enabled=lambda _: True ) TMT_REBOOT_CORE_SCRIPT = Script( - path=effective_scripts_dest_dir() / 'tmt-reboot-core', + source_filename='tmt-reboot-core', + destination_path=None, aliases=[], - related_variables=[]) + related_variables=[], + enabled=lambda _: True + ) # Script handling result reporting, in restraint compatible fashion TMT_REPORT_RESULT_SCRIPT = ScriptCreatingFile( - path=effective_scripts_dest_dir() / 'tmt-report-result', + source_filename='tmt-report-result', + destination_path=None, aliases=[ - effective_scripts_dest_dir() / 'rstrnt-report-result', - effective_scripts_dest_dir() / 'rhts-report-result'], + 'rstrnt-report-result', + 'rhts-report-result'], related_variables=[], - created_file="tmt-report-results.yaml" + created_file="tmt-report-results.yaml", + enabled=lambda _: True ) # Script for archiving a file, usable for BEAKERLIB_COMMAND_SUBMIT_LOG TMT_FILE_SUBMIT_SCRIPT = Script( - path=effective_scripts_dest_dir() / 'tmt-file-submit', + source_filename='tmt-file-submit', + destination_path=None, aliases=[ - effective_scripts_dest_dir() / 'rstrnt-report-log', - effective_scripts_dest_dir() / 'rhts-submit-log', - effective_scripts_dest_dir() / 'rhts_submit_log'], - related_variables=[] + 'rstrnt-report-log', + 'rhts-submit-log', + 'rhts_submit_log'], + related_variables=[], + enabled=lambda _: True ) # Script handling text execution abortion, in restraint compatible fashion TMT_ABORT_SCRIPT = ScriptCreatingFile( - path=effective_scripts_dest_dir() / 'tmt-abort', + source_filename='tmt-abort', + destination_path=None, aliases=[ - effective_scripts_dest_dir() / 'rstrnt-abort', - effective_scripts_dest_dir() / 'rhts-abort'], + 'rstrnt-abort', + 'rhts-abort'], related_variables=[], - created_file="abort" + created_file="abort", + enabled=lambda _: True ) # Profile script for adding SCRIPTS_DEST_DIR to executable paths system-wide +# Used only for rpm-ostree based distributions TMT_ETC_PROFILE_D = ScriptTemplate( - path=Path("/etc/profile.d/tmt.sh"), + source_filename='tmt.sh', + destination_path=Path("/etc/profile.d/tmt.sh"), aliases=[], related_variables=[], context={ - 'SCRIPTS_DEST_DIR': str(effective_scripts_dest_dir()) - }) + 'SCRIPTS_DEST_DIR': str(effective_scripts_dest_dir(default=SCRIPTS_DEST_DIR_OSTREE)) + }, + enabled=lambda guest: guest.facts.is_ostree is True) # List of all available scripts @@ -679,18 +721,25 @@ def prepare_tests(self, guest: Guest, logger: tmt.log.Logger) -> list[TestInvoca def prepare_scripts(self, guest: "tmt.steps.provision.Guest") -> None: """ Prepare additional scripts for testing """ + # For rpm-ostree based distributions use a different default destination directory + scripts_dest_dir = effective_scripts_dest_dir( + default=SCRIPTS_DEST_DIR_OSTREE if guest.facts.is_ostree else SCRIPTS_DEST_DIR) + # Create scripts directory - guest.execute(Command("mkdir", "-p", str(effective_scripts_dest_dir()))) + guest.execute(Command("mkdir", "-p", str(scripts_dest_dir))) # Install all scripts on guest for script in self.scripts: with script as source: - for dest in [script.path, *script.aliases]: - guest.push( - source=source, - destination=dest, - options=["-p", "--chmod=755"], - superuser=guest.facts.is_superuser is not True) + for dest in [script.source_filename, *script.aliases]: + if script.enabled(guest): + guest.push( + source=source, + destination=script.guest_path(guest, dest), + options=[ + "-p", + "--chmod=755"], + superuser=guest.facts.is_superuser is not True) def _tmt_report_results_filepath(self, invocation: TestInvocation) -> Path: """ Create path to test's ``tmt-report-result`` file """ diff --git a/tmt/steps/provision/__init__.py b/tmt/steps/provision/__init__.py index 6cbb65faa0..258180fbbd 100644 --- a/tmt/steps/provision/__init__.py +++ b/tmt/steps/provision/__init__.py @@ -191,6 +191,7 @@ class GuestFacts(SerializableContainer): has_selinux: Optional[bool] = None is_superuser: Optional[bool] = None + is_ostree: Optional[bool] = None #: Various Linux capabilities and whether they are permitted to #: commands executed on this guest. @@ -437,6 +438,20 @@ def _query_is_superuser(self, guest: 'Guest') -> Optional[bool]: return output.stdout.strip() == 'root' + def _query_is_ostree(self, guest: 'Guest') -> Optional[bool]: + # https://github.com/vrothberg/chkconfig/commit/538dc7edf0da387169d83599fe0774ea080b4a37#diff-562b9b19cb1cd12a7343ce5c739745ebc8f363a195276ca58e926f22927238a5R1334 + output = self._execute( + guest, + Command( + 'bash', + '-c', + 'if [ -e /run/ostree-booted ] || [ -L /ostree ]; then echo yes; else echo no; fi')) + + if output is None or output.stdout is None: + return None + + return output.stdout.strip() == 'yes' + def _query_capabilities(self, guest: 'Guest') -> dict[GuestCapability, bool]: # TODO: there must be a canonical way of getting permitted capabilities. # For now, we're interested in whether we can access kernel message buffer. @@ -457,6 +472,7 @@ def sync(self, guest: 'Guest') -> None: self.package_manager = self._query_package_manager(guest) self.has_selinux = self._query_has_selinux(guest) self.is_superuser = self._query_is_superuser(guest) + self.is_ostree = self._query_is_ostree(guest) self.capabilities = self._query_capabilities(guest) self.in_sync = True From 8d77bf52d2218f22669bd84844b42dbbbd10191d Mon Sep 17 00:00:00 2001 From: Miroslav Vadkerti Date: Wed, 16 Oct 2024 17:34:44 +0200 Subject: [PATCH 07/17] Add tests Signed-off-by: Miroslav Vadkerti --- tests/execute/tmt-scripts/data/.fmf/version | 1 + tests/execute/tmt-scripts/data/plan.fmf | 6 + tests/execute/tmt-scripts/main.fmf | 146 ++++++++++++++++++++ tests/execute/tmt-scripts/test.sh | 28 ++++ 4 files changed, 181 insertions(+) create mode 100644 tests/execute/tmt-scripts/data/.fmf/version create mode 100644 tests/execute/tmt-scripts/data/plan.fmf create mode 100644 tests/execute/tmt-scripts/main.fmf create mode 100755 tests/execute/tmt-scripts/test.sh diff --git a/tests/execute/tmt-scripts/data/.fmf/version b/tests/execute/tmt-scripts/data/.fmf/version new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/tests/execute/tmt-scripts/data/.fmf/version @@ -0,0 +1 @@ +1 diff --git a/tests/execute/tmt-scripts/data/plan.fmf b/tests/execute/tmt-scripts/data/plan.fmf new file mode 100644 index 0000000000..320429b2e5 --- /dev/null +++ b/tests/execute/tmt-scripts/data/plan.fmf @@ -0,0 +1,6 @@ +provision: + how: container + image: $IMAGE +execute: + how: tmt + script: ls $PATHS diff --git a/tests/execute/tmt-scripts/main.fmf b/tests/execute/tmt-scripts/main.fmf new file mode 100644 index 0000000000..e1aeec2a72 --- /dev/null +++ b/tests/execute/tmt-scripts/main.fmf @@ -0,0 +1,146 @@ +summary: Verify correct location of tmt scripts + +/fedora: + environment: + IMAGE: registry.fedoraproject.org/fedora:rawhide + FOUND: | + /usr/local/bin/rhts-abort + /usr/local/bin/rhts-reboot + /usr/local/bin/rhts-report-result + /usr/local/bin/rhts-submit-log + /usr/local/bin/rhts_submit_log + /usr/local/bin/rstrnt-abort + /usr/local/bin/rstrnt-reboot + /usr/local/bin/rstrnt-report-log + /usr/local/bin/rstrnt-report-result + /usr/local/bin/tmt-abort + /usr/local/bin/tmt-file-submit + /usr/local/bin/tmt-reboot + /usr/local/bin/tmt-reboot-core + /usr/local/bin/tmt-report-result + NOT_FOUND: + /etc/profile.d/tmt.sh + /var/lib/tmt/scripts + +/fedora-force: + environment: + IMAGE: registry.fedoraproject.org/fedora:rawhide + TMT_SCRIPTS_DEST_DIR: /usr/bin + FOUND: | + /usr/bin/rhts-abort + /usr/bin/rhts-reboot + /usr/bin/rhts-report-result + /usr/bin/rhts-submit-log + /usr/bin/rhts_submit_log + /usr/bin/rstrnt-abort + /usr/bin/rstrnt-reboot + /usr/bin/rstrnt-report-log + /usr/bin/rstrnt-report-result + /usr/bin/tmt-abort + /usr/bin/tmt-file-submit + /usr/bin/tmt-reboot + /usr/bin/tmt-reboot-core + /usr/bin/tmt-report-result + NOT_FOUND: + /etc/profile.d/tmt.sh + /usr/local/bin/rhts-abort + /usr/local/bin/rhts-reboot + /usr/local/bin/rhts-report-result + /usr/local/bin/rhts-submit-log + /usr/local/bin/rhts_submit_log + /usr/local/bin/rstrnt-abort + /usr/local/bin/rstrnt-reboot + /usr/local/bin/rstrnt-report-log + /usr/local/bin/rstrnt-report-result + /usr/local/bin/tmt-abort + /usr/local/bin/tmt-file-submit + /usr/local/bin/tmt-reboot + /usr/local/bin/tmt-reboot-core + /usr/local/bin/tmt-report-result + /var/lib/tmt/scripts + +/fedora-bootc: + environment: + IMAGE: quay.io/fedora/fedora-bootc:rawhide + FOUND: | + /var/lib/tmt/scripts/rhts-abort + /var/lib/tmt/scripts/rhts-reboot + /var/lib/tmt/scripts/rhts-report-result + /var/lib/tmt/scripts/rhts-submit-log + /var/lib/tmt/scripts/rhts_submit_log + /var/lib/tmt/scripts/rstrnt-abort + /var/lib/tmt/scripts/rstrnt-reboot + /var/lib/tmt/scripts/rstrnt-report-log + /var/lib/tmt/scripts/rstrnt-report-result + /var/lib/tmt/scripts/tmt-abort + /var/lib/tmt/scripts/tmt-file-submit + /var/lib/tmt/scripts/tmt-reboot + /var/lib/tmt/scripts/tmt-reboot-core + /var/lib/tmt/scripts/tmt-report-result + /etc/profile.d/tmt.sh + NOT_FOUND: | + /usr/local/bin/rhts-abort + /usr/local/bin/rhts-reboot + /usr/local/bin/rhts-report-result + /usr/local/bin/rhts-submit-log + /usr/local/bin/rhts_submit_log + /usr/local/bin/rstrnt-abort + /usr/local/bin/rstrnt-reboot + /usr/local/bin/rstrnt-report-log + /usr/local/bin/rstrnt-report-result + /usr/local/bin/tmt-abort + /usr/local/bin/tmt-file-submit + /usr/local/bin/tmt-reboot + /usr/local/bin/tmt-reboot-core + /usr/local/bin/tmt-report-result + + +/fedora-bootc-force: + environment: + IMAGE: quay.io/fedora/fedora-bootc:rawhide + TMT_SCRIPTS_DEST_DIR: /var/tmp/tmt/bin + FOUND: | + /etc/profile.d/tmt.sh + /var/tmp/tmt/bin/rhts-abort + /var/tmp/tmt/bin/rhts-reboot + /var/tmp/tmt/bin/rhts-report-result + /var/tmp/tmt/bin/rhts-submit-log + /var/tmp/tmt/bin/rhts_submit_log + /var/tmp/tmt/bin/rstrnt-abort + /var/tmp/tmt/bin/rstrnt-reboot + /var/tmp/tmt/bin/rstrnt-report-log + /var/tmp/tmt/bin/rstrnt-report-result + /var/tmp/tmt/bin/tmt-abort + /var/tmp/tmt/bin/tmt-file-submit + /var/tmp/tmt/bin/tmt-reboot + /var/tmp/tmt/bin/tmt-reboot-core + /var/tmp/tmt/bin/tmt-report-result + NOT_FOUND: | + /usr/local/bin/rhts-abort + /usr/local/bin/rhts-reboot + /usr/local/bin/rhts-report-result + /usr/local/bin/rhts-submit-log + /usr/local/bin/rhts_submit_log + /usr/local/bin/rstrnt-abort + /usr/local/bin/rstrnt-reboot + /usr/local/bin/rstrnt-report-log + /usr/local/bin/rstrnt-report-result + /usr/local/bin/tmt-abort + /usr/local/bin/tmt-file-submit + /usr/local/bin/tmt-reboot + /usr/local/bin/tmt-reboot-core + /usr/local/bin/tmt-report-result + /var/lib/tmt/scripts/rhts-abort + /var/lib/tmt/scripts/rhts-reboot + /var/lib/tmt/scripts/rhts-report-result + /var/lib/tmt/scripts/rhts-submit-log + /var/lib/tmt/scripts/rhts_submit_log + /var/lib/tmt/scripts/rstrnt-abort + /var/lib/tmt/scripts/rstrnt-reboot + /var/lib/tmt/scripts/rstrnt-report-log + /var/lib/tmt/scripts/rstrnt-report-result + /var/lib/tmt/scripts/tmt-abort + /var/lib/tmt/scripts/tmt-file-submit + /var/lib/tmt/scripts/tmt-reboot + /var/lib/tmt/scripts/tmt-reboot-core + /var/lib/tmt/scripts/tmt-report-result diff --git a/tests/execute/tmt-scripts/test.sh b/tests/execute/tmt-scripts/test.sh new file mode 100755 index 0000000000..0442188d01 --- /dev/null +++ b/tests/execute/tmt-scripts/test.sh @@ -0,0 +1,28 @@ +#!/bin/bash +. /usr/share/beakerlib/beakerlib.sh || exit 1 + +rlJournalStart + rlPhaseStartSetup + rlRun "run=\$(mktemp -d)" 0 "Create run directory" + rlRun "pushd data" + rlPhaseEnd + + rlPhaseStartTest + PATHS=$(echo $FOUND $NOT_FOUND) + + rlRun -s "tmt run -vvv -e IMAGE=$IMAGE -e \"PATHS='$PATHS'\" --id $run" 2 "Run the plan" + + for FOUND_PATH in $FOUND; do + rlAssertGrep "out: $FOUND_PATH" $rlRun_LOG + done + + for NOT_FOUND_PATH in $NOT_FOUND; do + rlAssertGrep "ls: cannot access '$NOT_FOUND_PATH': No such file or directory" $rlRun_LOG + done + rlPhaseEnd + + rlPhaseStartCleanup + rlRun "popd" + rlRun "rm -rf $run" 0 "Remove run directory" + rlPhaseEnd +rlJournalEnd From 97765f27d42503d25b45a088bb9af86d1501a9a5 Mon Sep 17 00:00:00 2001 From: Miroslav Vadkerti Date: Wed, 16 Oct 2024 18:33:40 +0200 Subject: [PATCH 08/17] Fix review comments Signed-off-by: Miroslav Vadkerti --- tests/execute/tmt-scripts/test.sh | 1 + tmt/steps/execute/__init__.py | 45 ++++++++++++++++++------------- tmt/steps/provision/__init__.py | 2 +- 3 files changed, 28 insertions(+), 20 deletions(-) diff --git a/tests/execute/tmt-scripts/test.sh b/tests/execute/tmt-scripts/test.sh index 0442188d01..3eac5ad5fb 100755 --- a/tests/execute/tmt-scripts/test.sh +++ b/tests/execute/tmt-scripts/test.sh @@ -8,6 +8,7 @@ rlJournalStart rlPhaseEnd rlPhaseStartTest + # List of paths to check, on a single line PATHS=$(echo $FOUND $NOT_FOUND) rlRun -s "tmt run -vvv -e IMAGE=$IMAGE -e \"PATHS='$PATHS'\" --id $run" 2 "Run the plan" diff --git a/tmt/steps/execute/__init__.py b/tmt/steps/execute/__init__.py index a15c6374d1..4ea9703ca3 100644 --- a/tmt/steps/execute/__init__.py +++ b/tmt/steps/execute/__init__.py @@ -62,12 +62,12 @@ SCRIPTS_SRC_DIR = tmt.utils.resource_files('steps/execute/scripts') #: The default scripts destination directory -SCRIPTS_DEST_DIR = Path("/usr/local/bin") +DEFAULT_SCRIPTS_DEST_DIR = Path("/usr/local/bin") #: The default scripts destination directory for rpm-ostree based distributions, https://github.com/teemtee/tmt/discussions/3260 -SCRIPTS_DEST_DIR_OSTREE = Path("/var/lib/tmt/scripts") +DEFAULT_SCRIPTS_DEST_DIR_OSTREE = Path("/var/lib/tmt/scripts") -#: The default tmt environment variable name for forcing ``SCRIPTS_DEST_DIR`` +#: The tmt environment variable name for forcing ``SCRIPTS_DEST_DIR`` SCRIPTS_DEST_DIR_VARIABLE = 'TMT_SCRIPTS_DEST_DIR' @@ -83,13 +83,13 @@ class Script: location is relative to the directory specified via the :py:data:`SCRIPTS_SRC_DIR` variable. All scripts must be located in this directory. - The default destination directory of the scripts is :py:data:`SCRIPTS_DEST_DIR`. + The default destination directory of the scripts is :py:data:`DEFAULT_SCRIPTS_DEST_DIR`. On ``rpm-ostree`` distributions like Fedora CoreOS, the default destination - directory is :py:data:``SCRIPTS_DEST_DIR_OSTREE``. The destination directory + directory is :py:data:``DEFAULT_SCRIPTS_DEST_DIR_OSTREE``. The destination directory of the scripts can be forced by the script using ``destination_path`` attribute. The destination directory can be overridden using the environment variable defined - by the :py:data:`SCRIPTS_DEST_DIR_VARIABLE` variable. + by the :py:data:`DEFAULT_SCRIPTS_DEST_DIR_VARIABLE` variable. The ``enabled`` attribute can specify a function which is called with :py:class:`Guest` instance to evaluate if the script is enabled. This can be useful to optionally disable @@ -108,13 +108,18 @@ def __enter__(self) -> Path: def __exit__(self, *args: object) -> None: pass - def guest_path(self, guest: Guest, alias: Optional[str] = None) -> Path: - """ Return path of the script or the script alias on a guest """ - filename = alias or self.source_filename + def guest_path(self, guest: Guest, filename: Optional[str] = None) -> Path: + """ Return path of the script on the guest """ + filename = filename or self.source_filename + + if self.destination_path: + return self.destination_path + scripts_dest_dir = effective_scripts_dest_dir( - default=SCRIPTS_DEST_DIR_OSTREE if guest.facts.is_ostree else SCRIPTS_DEST_DIR - ) - return self.destination_path if self.destination_path else scripts_dest_dir / filename + default=DEFAULT_SCRIPTS_DEST_DIR_OSTREE + if guest.facts.is_ostree else DEFAULT_SCRIPTS_DEST_DIR) + + return scripts_dest_dir / filename @dataclass @@ -160,7 +165,7 @@ def __exit__(self, *args: object) -> None: os.unlink(self._rendered_script_path) -def effective_scripts_dest_dir(default: Path = SCRIPTS_DEST_DIR) -> Path: +def effective_scripts_dest_dir(default: Path = DEFAULT_SCRIPTS_DEST_DIR) -> Path: """ Find out what the actual scripts destination directory is. @@ -242,8 +247,9 @@ def effective_scripts_dest_dir(default: Path = SCRIPTS_DEST_DIR) -> Path: aliases=[], related_variables=[], context={ - 'SCRIPTS_DEST_DIR': str(effective_scripts_dest_dir(default=SCRIPTS_DEST_DIR_OSTREE)) - }, + 'SCRIPTS_DEST_DIR': str( + effective_scripts_dest_dir( + default=DEFAULT_SCRIPTS_DEST_DIR_OSTREE))}, enabled=lambda guest: guest.facts.is_ostree is True) @@ -723,19 +729,20 @@ def prepare_scripts(self, guest: "tmt.steps.provision.Guest") -> None: """ Prepare additional scripts for testing """ # For rpm-ostree based distributions use a different default destination directory scripts_dest_dir = effective_scripts_dest_dir( - default=SCRIPTS_DEST_DIR_OSTREE if guest.facts.is_ostree else SCRIPTS_DEST_DIR) + default=DEFAULT_SCRIPTS_DEST_DIR_OSTREE + if guest.facts.is_ostree else DEFAULT_SCRIPTS_DEST_DIR) - # Create scripts directory + # Make sure scripts directory exists guest.execute(Command("mkdir", "-p", str(scripts_dest_dir))) # Install all scripts on guest for script in self.scripts: with script as source: - for dest in [script.source_filename, *script.aliases]: + for filename in [script.source_filename, *script.aliases]: if script.enabled(guest): guest.push( source=source, - destination=script.guest_path(guest, dest), + destination=script.guest_path(guest, filename), options=[ "-p", "--chmod=755"], diff --git a/tmt/steps/provision/__init__.py b/tmt/steps/provision/__init__.py index 258180fbbd..fe5ddf7664 100644 --- a/tmt/steps/provision/__init__.py +++ b/tmt/steps/provision/__init__.py @@ -443,7 +443,7 @@ def _query_is_ostree(self, guest: 'Guest') -> Optional[bool]: output = self._execute( guest, Command( - 'bash', + tmt.utils.DEFAULT_SHELL, '-c', 'if [ -e /run/ostree-booted ] || [ -L /ostree ]; then echo yes; else echo no; fi')) From e24b8e4c88443d93d77ed973912d07dda3e447de Mon Sep 17 00:00:00 2001 From: Miroslav Vadkerti Date: Thu, 17 Oct 2024 00:45:13 +0200 Subject: [PATCH 09/17] Add suggestion from review Signed-off-by: Miroslav Vadkerti --- tmt/frameworks/beakerlib.py | 2 +- tmt/steps/execute/__init__.py | 22 +++------------------- tmt/steps/provision/__init__.py | 11 +++++++++++ 3 files changed, 15 insertions(+), 20 deletions(-) diff --git a/tmt/frameworks/beakerlib.py b/tmt/frameworks/beakerlib.py index 507dd0f5d0..4a70c05b91 100644 --- a/tmt/frameworks/beakerlib.py +++ b/tmt/frameworks/beakerlib.py @@ -36,7 +36,7 @@ def get_environment_variables( return Environment({ 'BEAKERLIB_DIR': EnvVarValue(invocation.path), 'BEAKERLIB_COMMAND_SUBMIT_LOG': EnvVarValue( - f'bash {tmt.steps.execute.TMT_FILE_SUBMIT_SCRIPT.guest_path(invocation.guest)}') + f'bash {invocation.guest.scripts_path / tmt.steps.execute.TMT_FILE_SUBMIT_SCRIPT.source_filename}') # noqa: E501 }) @classmethod diff --git a/tmt/steps/execute/__init__.py b/tmt/steps/execute/__init__.py index 4ea9703ca3..f2c83ec550 100644 --- a/tmt/steps/execute/__init__.py +++ b/tmt/steps/execute/__init__.py @@ -108,19 +108,6 @@ def __enter__(self) -> Path: def __exit__(self, *args: object) -> None: pass - def guest_path(self, guest: Guest, filename: Optional[str] = None) -> Path: - """ Return path of the script on the guest """ - filename = filename or self.source_filename - - if self.destination_path: - return self.destination_path - - scripts_dest_dir = effective_scripts_dest_dir( - default=DEFAULT_SCRIPTS_DEST_DIR_OSTREE - if guest.facts.is_ostree else DEFAULT_SCRIPTS_DEST_DIR) - - return scripts_dest_dir / filename - @dataclass class ScriptCreatingFile(Script): @@ -727,13 +714,10 @@ def prepare_tests(self, guest: Guest, logger: tmt.log.Logger) -> list[TestInvoca def prepare_scripts(self, guest: "tmt.steps.provision.Guest") -> None: """ Prepare additional scripts for testing """ - # For rpm-ostree based distributions use a different default destination directory - scripts_dest_dir = effective_scripts_dest_dir( - default=DEFAULT_SCRIPTS_DEST_DIR_OSTREE - if guest.facts.is_ostree else DEFAULT_SCRIPTS_DEST_DIR) # Make sure scripts directory exists - guest.execute(Command("mkdir", "-p", str(scripts_dest_dir))) + guest.execute(Command("sudo" if guest.facts.is_superuser else "", + "mkdir", "-p", str(guest.scripts_path))) # Install all scripts on guest for script in self.scripts: @@ -742,7 +726,7 @@ def prepare_scripts(self, guest: "tmt.steps.provision.Guest") -> None: if script.enabled(guest): guest.push( source=source, - destination=script.guest_path(guest, filename), + destination=(script.destination_path or guest.scripts_path) / filename, options=[ "-p", "--chmod=755"], diff --git a/tmt/steps/provision/__init__.py b/tmt/steps/provision/__init__.py index fe5ddf7664..de43937e18 100644 --- a/tmt/steps/provision/__init__.py +++ b/tmt/steps/provision/__init__.py @@ -844,6 +844,17 @@ def package_manager(self) -> 'tmt.package_managers.PackageManager': return tmt.package_managers.find_package_manager( self.facts.package_manager)(guest=self, logger=self._logger) + @functools.cached_property + def scripts_path(self) -> Path: + """ Absolute path to tmt scripts directory """ + + import tmt.steps.execute + + # For rpm-ostree based distributions use a different default destination directory + return tmt.steps.execute.effective_scripts_dest_dir( + default=tmt.steps.execute.DEFAULT_SCRIPTS_DEST_DIR_OSTREE + if self.facts.is_ostree else tmt.steps.execute.DEFAULT_SCRIPTS_DEST_DIR) + @classmethod def options(cls, how: Optional[str] = None) -> list[tmt.options.ClickOptionDecoratorType]: """ Prepare command line options related to guests """ From ae169630fd49b10e6eba0a4ef5b25a065a1d1b5c Mon Sep 17 00:00:00 2001 From: Miroslav Vadkerti Date: Thu, 17 Oct 2024 13:35:27 +0200 Subject: [PATCH 10/17] Update tmt/steps/execute/__init__.py --- tmt/steps/execute/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tmt/steps/execute/__init__.py b/tmt/steps/execute/__init__.py index f2c83ec550..a90f96514e 100644 --- a/tmt/steps/execute/__init__.py +++ b/tmt/steps/execute/__init__.py @@ -716,7 +716,7 @@ def prepare_scripts(self, guest: "tmt.steps.provision.Guest") -> None: """ Prepare additional scripts for testing """ # Make sure scripts directory exists - guest.execute(Command("sudo" if guest.facts.is_superuser else "", + guest.execute(Command("sudo" if not guest.facts.is_superuser else "", "mkdir", "-p", str(guest.scripts_path))) # Install all scripts on guest From 5065eb0c5a0e40159d3c1be6a3d70e569c915f3e Mon Sep 17 00:00:00 2001 From: Miroslav Vadkerti Date: Thu, 17 Oct 2024 17:53:40 +0200 Subject: [PATCH 11/17] Correct sudo usage Signed-off-by: Miroslav Vadkerti --- tmt/steps/execute/__init__.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tmt/steps/execute/__init__.py b/tmt/steps/execute/__init__.py index a90f96514e..af1e3ad902 100644 --- a/tmt/steps/execute/__init__.py +++ b/tmt/steps/execute/__init__.py @@ -716,8 +716,10 @@ def prepare_scripts(self, guest: "tmt.steps.provision.Guest") -> None: """ Prepare additional scripts for testing """ # Make sure scripts directory exists - guest.execute(Command("sudo" if not guest.facts.is_superuser else "", - "mkdir", "-p", str(guest.scripts_path))) + guest.execute(Command( + ShellScript( + "sudo " if not guest.facts.is_superuser else "" + f"mkdir -p {guest.scripts_path!s}").to_element())) # Install all scripts on guest for script in self.scripts: From 727c955dca65124ef4861e51791f059bc9a414fe Mon Sep 17 00:00:00 2001 From: Miroslav Vadkerti Date: Thu, 17 Oct 2024 18:58:08 +0200 Subject: [PATCH 12/17] Update tmt/steps/execute/__init__.py Co-authored-by: Petr Matyas --- tmt/steps/execute/__init__.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/tmt/steps/execute/__init__.py b/tmt/steps/execute/__init__.py index af1e3ad902..a8d2f3f76c 100644 --- a/tmt/steps/execute/__init__.py +++ b/tmt/steps/execute/__init__.py @@ -161,10 +161,7 @@ def effective_scripts_dest_dir(default: Path = DEFAULT_SCRIPTS_DEST_DIR) -> Path is returned. """ - if SCRIPTS_DEST_DIR_VARIABLE in os.environ: - return Path(os.environ[SCRIPTS_DEST_DIR_VARIABLE]) - - return default + return Path(os.environ.get(SCRIPTS_DEST_DIR_VARIABLE, default)) # Script handling reboots, in restraint compatible fashion @@ -716,10 +713,12 @@ def prepare_scripts(self, guest: "tmt.steps.provision.Guest") -> None: """ Prepare additional scripts for testing """ # Make sure scripts directory exists - guest.execute(Command( - ShellScript( - "sudo " if not guest.facts.is_superuser else "" - f"mkdir -p {guest.scripts_path!s}").to_element())) + command = Command("mkdir", "-p", "{guest.scripts_path}") + + if not guest.facts.is_superuser: + command = Command("sudo") + command + + guest.execute(command) # Install all scripts on guest for script in self.scripts: From 1bd0078074e39dce7846547d3e070d5d115d2b31 Mon Sep 17 00:00:00 2001 From: Miroslav Vadkerti Date: Thu, 17 Oct 2024 19:31:21 +0200 Subject: [PATCH 13/17] Fix 2 issues introduced lately Signed-off-by: Miroslav Vadkerti --- tmt/steps/execute/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tmt/steps/execute/__init__.py b/tmt/steps/execute/__init__.py index a8d2f3f76c..d54e600edc 100644 --- a/tmt/steps/execute/__init__.py +++ b/tmt/steps/execute/__init__.py @@ -713,7 +713,7 @@ def prepare_scripts(self, guest: "tmt.steps.provision.Guest") -> None: """ Prepare additional scripts for testing """ # Make sure scripts directory exists - command = Command("mkdir", "-p", "{guest.scripts_path}") + command = Command("mkdir", "-p", f"{guest.scripts_path}") if not guest.facts.is_superuser: command = Command("sudo") + command @@ -727,7 +727,7 @@ def prepare_scripts(self, guest: "tmt.steps.provision.Guest") -> None: if script.enabled(guest): guest.push( source=source, - destination=(script.destination_path or guest.scripts_path) / filename, + destination=script.destination_path or guest.scripts_path / filename, options=[ "-p", "--chmod=755"], From 0367b3db6704a51fd67dccbbc4327bec7127105d Mon Sep 17 00:00:00 2001 From: Miroslav Vadkerti Date: Thu, 17 Oct 2024 19:55:23 +0200 Subject: [PATCH 14/17] Fix docstrings and add release notes Signed-off-by: Miroslav Vadkerti --- docs/releases.rst | 9 +++++++++ tmt/steps/execute/__init__.py | 11 ++++------- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/docs/releases.rst b/docs/releases.rst index 841eaed9e6..c2acf536e6 100644 --- a/docs/releases.rst +++ b/docs/releases.rst @@ -29,6 +29,15 @@ A race condition in the thus multihost tests using this provision method should now work reliably without unexpected connection failures. +Support for RHEL-like operating systems in `Image Mode`__ has been +added. The destination directory of the scripts added by ``tmt`` +on these operating systems is ``/var/lib/tmt/scripts``. For +all others the ``/usr/local/bin`` destination directory is used. +A new environment variable ``TMT_SCRIPTS_DIR`` is available +to override the default locations. + +__ https://www.redhat.com/en/technologies/linux-platforms/enterprise-linux/image-mode + tmt-1.37.0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/tmt/steps/execute/__init__.py b/tmt/steps/execute/__init__.py index d54e600edc..229d5c92ee 100644 --- a/tmt/steps/execute/__init__.py +++ b/tmt/steps/execute/__init__.py @@ -77,7 +77,7 @@ class Script: Represents a script provided by the internal executor. Must be used as a context manager. The context manager returns - the source file path. + the source filename. The source file is defined by the ``source_filename`` attribute and its location is relative to the directory specified via the :py:data:`SCRIPTS_SRC_DIR` @@ -125,9 +125,6 @@ class ScriptTemplate(Script): """ Represents a Jinja2 templated script. - Must be used as a context manager. The context manager returns - the source file path. - The source filename is constructed from the name of the file specified via the ``source_filename`` attribute, with the ``.j2`` suffix appended. The template file must be located in the directory specified @@ -158,7 +155,7 @@ def effective_scripts_dest_dir(default: Path = DEFAULT_SCRIPTS_DEST_DIR) -> Path If the ``TMT_SCRIPTS_DEST_DIR`` environment variable is set, it is used as the scripts destination directory. Otherwise, the ``default`` - is returned. + parameter path is returned. """ return Path(os.environ.get(SCRIPTS_DEST_DIR_VARIABLE, default)) @@ -223,8 +220,8 @@ def effective_scripts_dest_dir(default: Path = DEFAULT_SCRIPTS_DEST_DIR) -> Path enabled=lambda _: True ) -# Profile script for adding SCRIPTS_DEST_DIR to executable paths system-wide -# Used only for rpm-ostree based distributions +# Profile script for adding SCRIPTS_DEST_DIR to executable paths system-wide. +# Used only for distributions using rpm-ostree. TMT_ETC_PROFILE_D = ScriptTemplate( source_filename='tmt.sh', destination_path=Path("/etc/profile.d/tmt.sh"), From 9f5d40af50a2253f47f392b8c8ed5320246e0636 Mon Sep 17 00:00:00 2001 From: Miroslav Vadkerti Date: Fri, 18 Oct 2024 18:29:15 +0200 Subject: [PATCH 15/17] fix failing test Signed-off-by: Miroslav Vadkerti --- tests/execute/nonroot/data/main.fmf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/execute/nonroot/data/main.fmf b/tests/execute/nonroot/data/main.fmf index 5c7fdccb13..602e855fca 100644 --- a/tests/execute/nonroot/data/main.fmf +++ b/tests/execute/nonroot/data/main.fmf @@ -12,6 +12,6 @@ /command_found: test: command -v tmt-file-submit /full_path_exists: - test: test -e /usr/local/bin/tmt-file-submit + test: test -e /var/lib/tmt/scripts/tmt-file-submit /command_works: test: tmt-file-submit XXXX main.fmf From cbd06b540f5ed772d4856222ec2dd8675850d390 Mon Sep 17 00:00:00 2001 From: Miroslav Vadkerti Date: Mon, 21 Oct 2024 14:08:13 +0200 Subject: [PATCH 16/17] Address comments from the review Signed-off-by: Miroslav Vadkerti --- docs/overview.rst | 6 +++--- spec/plans/execute.fmf | 2 +- tests/execute/tmt-scripts/main.fmf | 4 ++-- tmt/steps/execute/__init__.py | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/overview.rst b/docs/overview.rst index 98f34ee196..4824e4c687 100644 --- a/docs/overview.rst +++ b/docs/overview.rst @@ -467,17 +467,17 @@ TMT_REBOOT_TIMEOUT guest reboot. By default, it is 10 minutes. -TMT_SCRIPTS_DEST_DIR +TMT_SCRIPTS_DIR Destination directory for storing ``tmt`` scripts on the guest. By default ``/usr/local/bin`` is used, except for guests using ``rpm-ostree``, where ``/var/lib/tmt/scripts`` is used. See the `tmt internal test executor`__ documentation for more details on the scripts installed on the guest. -__ https://tmt.readthedocs.io/en/stable/spec/plans.html#tmt - .. versionadded:: 1.38 +__ https://tmt.readthedocs.io/en/stable/spec/plans.html#tmt + TMT_SSH_* Every environment variable in this format would be treated as an SSH option, and passed to the ``-o`` option of ``ssh`` command. See diff --git a/spec/plans/execute.fmf b/spec/plans/execute.fmf index bf1f758efd..fc3d2d3b43 100644 --- a/spec/plans/execute.fmf +++ b/spec/plans/execute.fmf @@ -203,7 +203,7 @@ description: | The scripts are hosted by default in the ``/usr/local/bin`` directory, except for guests using ``rpm-ostree``, where ``/var/lib/tmt/scripts`` is used. The directory can be forced - using the ``TMT_SCRIPTS_DEST_DIR`` environment variable. + using the ``TMT_SCRIPTS_DIR`` environment variable. Note that for guests using ``rpm-ostree``, the directory is added to executable paths using the system-wide ``/etc/profile.d/tmt.sh`` profile script. diff --git a/tests/execute/tmt-scripts/main.fmf b/tests/execute/tmt-scripts/main.fmf index e1aeec2a72..dd05ae991b 100644 --- a/tests/execute/tmt-scripts/main.fmf +++ b/tests/execute/tmt-scripts/main.fmf @@ -25,7 +25,7 @@ summary: Verify correct location of tmt scripts /fedora-force: environment: IMAGE: registry.fedoraproject.org/fedora:rawhide - TMT_SCRIPTS_DEST_DIR: /usr/bin + TMT_SCRIPTS_DIR: /usr/bin FOUND: | /usr/bin/rhts-abort /usr/bin/rhts-reboot @@ -98,7 +98,7 @@ summary: Verify correct location of tmt scripts /fedora-bootc-force: environment: IMAGE: quay.io/fedora/fedora-bootc:rawhide - TMT_SCRIPTS_DEST_DIR: /var/tmp/tmt/bin + TMT_SCRIPTS_DIR: /var/tmp/tmt/bin FOUND: | /etc/profile.d/tmt.sh /var/tmp/tmt/bin/rhts-abort diff --git a/tmt/steps/execute/__init__.py b/tmt/steps/execute/__init__.py index 229d5c92ee..b963093a97 100644 --- a/tmt/steps/execute/__init__.py +++ b/tmt/steps/execute/__init__.py @@ -68,7 +68,7 @@ DEFAULT_SCRIPTS_DEST_DIR_OSTREE = Path("/var/lib/tmt/scripts") #: The tmt environment variable name for forcing ``SCRIPTS_DEST_DIR`` -SCRIPTS_DEST_DIR_VARIABLE = 'TMT_SCRIPTS_DEST_DIR' +SCRIPTS_DEST_DIR_VARIABLE = 'TMT_SCRIPTS_DIR' @dataclass @@ -153,7 +153,7 @@ def effective_scripts_dest_dir(default: Path = DEFAULT_SCRIPTS_DEST_DIR) -> Path """ Find out what the actual scripts destination directory is. - If the ``TMT_SCRIPTS_DEST_DIR`` environment variable is set, it is used + If the ``TMT_SCRIPTS_DIR`` environment variable is set, it is used as the scripts destination directory. Otherwise, the ``default`` parameter path is returned. """ From a7f039860428afdc2baa60629d1dc2516b262fd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20=C5=A0pl=C3=ADchal?= Date: Mon, 21 Oct 2024 15:04:26 +0200 Subject: [PATCH 17/17] Drop an extra line --- docs/overview.rst | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/overview.rst b/docs/overview.rst index 4824e4c687..b878270fe5 100644 --- a/docs/overview.rst +++ b/docs/overview.rst @@ -466,7 +466,6 @@ TMT_REBOOT_TIMEOUT How many seconds to wait for a connection to succeed after guest reboot. By default, it is 10 minutes. - TMT_SCRIPTS_DIR Destination directory for storing ``tmt`` scripts on the guest. By default ``/usr/local/bin`` is used, except for guests using