Skip to content
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

Support Fedora Image Mode #3229

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
1 change: 1 addition & 0 deletions .github/workflows/shellcheck.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ jobs:
exclude-path: |
tests/**
examples/**/test.sh
tmt/steps/execute/scripts/tmt.sh.j2
tmt/templates/**
token: ${{ secrets.GITHUB_TOKEN }}

Expand Down
91 changes: 77 additions & 14 deletions tmt/steps/execute/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import os
import signal as _signal
import subprocess
import tempfile
import threading
from contextlib import suppress
from dataclasses import dataclass
Expand All @@ -27,13 +28,15 @@
from tmt.steps.discover import Discover, DiscoverPlugin, DiscoverStepData
from tmt.steps.provision import Guest
from tmt.utils import (
Command,
Path,
ShellScript,
Stopwatch,
field,
format_duration,
format_timestamp,
)
from tmt.utils.templates import render_template_file

if TYPE_CHECKING:
import tmt.cli
Expand All @@ -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:
Expand All @@ -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",
Expand All @@ -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())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd recommend using upper case for variable names, which seems to be common in the Jinja world in general, and definitely true in our own templates.

})


# List of all available scripts
SCRIPTS = (
TMT_ABORT_SCRIPT,
TMT_ETC_PROFILE_D,
TMT_FILE_SUBMIT_SCRIPT,
TMT_REBOOT_SCRIPT,
TMT_REBOOT_CORE_SCRIPT,
Expand Down Expand Up @@ -595,19 +636,41 @@ 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:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like the implementation of a script class leaked into the plugin.

I'd suggest extending Script to become a context manager, i.e. adding __enter__ and __exit__ methods. The default implementation of __enter__ would return self.path, but ScriptTemplate class would do the magic we have here, render the template and write down the path to temporary file. Then do with script as path below, and ScriptTemplate's __exit__ would remove the temporary file when leaving the context. Plugin does not need to be concerned about itnernals of various script implementations, all it needs is a local path to a file, and the rest shouldn't be its responsibility.

""" 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,
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)

def _tmt_report_results_filepath(self, invocation: TestInvocation) -> Path:
""" Create path to test's ``tmt-report-result`` file """

Expand Down
4 changes: 4 additions & 0 deletions tmt/steps/execute/scripts/tmt.sh.j2
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# shellcheck shell=bash

# tmt provides executable scripts under this path
export PATH={{ scripts_dest_dir }}:$PATH
Fixed Show fixed Hide fixed
Fixed Show fixed Hide fixed
Fixed Show fixed Hide fixed
Fixed Show fixed Hide fixed
Fixed Show fixed Hide fixed
18 changes: 1 addition & 17 deletions tmt/steps/provision/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1303,23 +1303,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

Expand Down
Loading