tmt icon indicating copy to clipboard operation
tmt copied to clipboard

Add `epel` prepare plugin using `feature` into `tmt try`

Open falconizmi opened this issue 1 year ago • 4 comments

Add option --epel in tmt try to enable epel repository.

Pull Request Checklist

  • [x] implement the feature
  • [x] write the documentation
  • [x] extend the test coverage
  • [x] adjust plugin docstring
  • [x] mention the version
  • [x] include a release note

falconizmi avatar Jun 04 '24 10:06 falconizmi

Adding blocked label, to get https://github.com/teemtee/tmt/pull/2986 in first to simplify this patch a bit.

happz avatar Jun 25 '24 12:06 happz

@falconizmi commit message and PR $SUBJ would deserve a small tweak: "Add epel prepare plugin using feature" is technically correct, but it looks like the epel support is added in general, which is not true; "Add epel prepare plugin using feature into tmt try" would be more precise, or something along the lines of extending tmt try rather than adding epel to prepare/feature, because we already have that.

happz avatar Jun 25 '24 12:06 happz

@falconizmi it does nto work for me, testing with tmt try --epel CentOS-Stream-9

tmt try --epel CentOS-Stream-9

fails with

        prepare task #1: tmt-try-epel on default-0
        how: feature
        name: tmt-try-epel
        order: 50
        Enable EPEL
        fail: 

prepare step failed

The exception was caused by 1 earlier exceptions

Cause number 1:

thrix avatar Jul 30 '24 13:07 thrix

diff --git a/tmt/steps/prepare/ansible.py b/tmt/steps/prepare/ansible.py
index d3745cc4..008050b7 100644
--- a/tmt/steps/prepare/ansible.py
+++ b/tmt/steps/prepare/ansible.py
@@ -164,4 +164,4 @@ class PrepareAnsible(tmt.steps.prepare.PreparePlugin[PrepareAnsibleData]):
 
                 logger.info('playbook-path', playbook_path, 'green')
 
-            guest.ansible(playbook_path, extra_args=self.data.extra_args)
+            guest.ansible(playbook_path, playbook_root=self.step.plan.fmf_root, extra_args=self.data.extra_args)
diff --git a/tmt/steps/prepare/feature.py b/tmt/steps/prepare/feature.py
index 4209d52a..115d16f6 100644
--- a/tmt/steps/prepare/feature.py
+++ b/tmt/steps/prepare/feature.py
@@ -58,7 +58,7 @@ class ToggleableFeature(Feature):
                 f"{op.capitalize()} {self.NAME.upper()} is not supported on this guest.")
 
         self.info(f'{op.capitalize()} {self.NAME.upper()}')
-        self.guest.ansible(playbook_path.relative_to(self.get_root_path()))
+        self.guest.ansible(playbook_path)
 
     def _enable(self, playbook_filename: str) -> None:
         self._run_playbook('enable', playbook_filename)
diff --git a/tmt/steps/provision/__init__.py b/tmt/steps/provision/__init__.py
index fb5defba..43f02ac4 100644
--- a/tmt/steps/provision/__init__.py
+++ b/tmt/steps/provision/__init__.py
@@ -951,14 +951,18 @@ class Guest(tmt.utils.Common):
                 tasks = fmf.utils.listed(matched.group(1), 'task')
                 self.verbose(key, tasks, 'green')
 
-    def _ansible_playbook_path(self, playbook: Path) -> Path:
+    def _ansible_playbook_path(self, playbook: Path, playbook_root: Path) -> Path:
         """ Prepare full ansible playbook path """
-        self.debug(f"Applying playbook '{playbook}' on guest '{self.primary_address}'.")
+        # self.debug(f"Applying playbook '{playbook}' on guest '{self.primary_address}'.")
         # FIXME: cast() - https://github.com/teemtee/tmt/issues/1372
-        parent = cast(Provision, self.parent)
-        assert parent.plan.fmf_root is not None  # narrow type
+        # parent = cast(Provision, self.parent)
+        # assert parent.plan.fmf_root is not None  # narrow type
+
         # Playbook paths should be relative to the metadata tree root
-        playbook = parent.plan.fmf_root / playbook.unrooted()
+        playbook = playbook_root / playbook.unrooted()
+        if not playbook.exists() or not playbook.is_relative_to(playbook_root):
+            raise Exception()
+
         self.debug(f"Playbook full path: '{playbook}'", level=2)
         return playbook
 
@@ -1040,6 +1044,7 @@ class Guest(tmt.utils.Common):
     def _run_ansible(
             self,
             playbook: Path,
+            playbook_root: Optional[Path] = None,
             extra_args: Optional[str] = None,
             friendly_command: Optional[str] = None,
             log: Optional[tmt.log.LoggingFunction] = None,
@@ -1066,6 +1071,7 @@ class Guest(tmt.utils.Common):
     def ansible(
             self,
             playbook: Path,
+            playbook_root: Optional[Path] = None,
             extra_args: Optional[str] = None,
             friendly_command: Optional[str] = None,
             log: Optional[tmt.log.LoggingFunction] = None,
@@ -1089,6 +1095,7 @@ class Guest(tmt.utils.Common):
 
         output = self._run_ansible(
             playbook,
+            playbook_root=playbook_root,
             extra_args=extra_args,
             friendly_command=friendly_command,
             log=log if log else self._command_verbose_logger,
@@ -1512,6 +1519,7 @@ class GuestSsh(Guest):
     def _run_ansible(
             self,
             playbook: Path,
+            playbook_root: Optional[Path] = None,
             extra_args: Optional[str] = None,
             friendly_command: Optional[str] = None,
             log: Optional[tmt.log.LoggingFunction] = None,
@@ -1532,7 +1540,8 @@ class GuestSsh(Guest):
         :param silent: if set, logging of steps taken by this function would be
             reduced.
         """
-        playbook = self._ansible_playbook_path(playbook)
+        if playbook_root:
+            playbook = self._ansible_playbook_path(playbook, playbook_root)
 
         ansible_command = Command('ansible-playbook', *self._ansible_verbosity())
 
diff --git a/tmt/steps/provision/local.py b/tmt/steps/provision/local.py
index c25cc3e1..7b7ee505 100644
--- a/tmt/steps/provision/local.py
+++ b/tmt/steps/provision/local.py
@@ -29,6 +29,7 @@ class GuestLocal(tmt.Guest):
     def _run_ansible(
             self,
             playbook: Path,
+            playbook_root: Optional[Path] = None,
             extra_args: Optional[str] = None,
             friendly_command: Optional[str] = None,
             log: Optional[tmt.log.LoggingFunction] = None,
diff --git a/tmt/utils/__init__.py b/tmt/utils/__init__.py
index 916f2ea7..966a0935 100644
--- a/tmt/utils/__init__.py
+++ b/tmt/utils/__init__.py
@@ -4524,6 +4524,13 @@ def git_add(*, path: Path, logger: tmt.log.Logger) -> None:
         raise GeneralError(f"Failed to add path '{path}' to git index.") from error
 
 
+def git_ignore(*, root: Path, logger: tmt.log.Logger) -> list[Path]:
+    output = Command('git', 'ls-files', '--exclude-standard', '-oi', '--directory') \
+        .run(cwd=root, logger=logger)
+
+    return [Path(line.strip()) for line in output.stdout.splitlines()] if output.stdout else []
+
+
 def default_branch(
         *,
         repository: Path,

happz avatar Jul 31 '24 12:07 happz

/packit build

falconizmi avatar Sep 18 '24 08:09 falconizmi

Thanks for implementing this! Looks good. The only thing is the extra option-handling action which should not be needed. Tried to remove in 4d5bf16 and everything seems to be working just fine. @falconizmi, could you please have a look if the change is ok?

Looks good to me

falconizmi avatar Sep 24 '24 08:09 falconizmi