Add a helper for nonconflicting, multihost-safe filenames

tmt/steps/__init__.py

@@ -1796,7 +1796,7 @@ def to_dict(self) -> dict[str, Any]:
 
         return data
 
-    def save_yaml(self, dirpath: Path, filename: Optional[str] = None) -> Path:
+    def save_yaml(self, dirpath: Path, filename: Optional[Path] = None) -> Path:
         """
         Save the topology in a YAML file.
 
@@ -1807,7 +1807,7 @@ def save_yaml(self, dirpath: Path, filename: Optional[str] = None) -> Path:
         :returns: path to the saved file.
         """
 
-        filename = filename or f'{TEST_TOPOLOGY_FILENAME_BASE}.yaml'
+        filename = filename or Path(f'{TEST_TOPOLOGY_FILENAME_BASE}.yaml')
         filepath = dirpath / filename
 
         serialized = self.to_dict()
@@ -1822,7 +1822,7 @@ def save_yaml(self, dirpath: Path, filename: Optional[str] = None) -> Path:
 
         return filepath
 
-    def save_bash(self, dirpath: Path, filename: Optional[str] = None) -> Path:
+    def save_bash(self, dirpath: Path, filename: Optional[Path] = None) -> Path:
         """
         Save the topology in a Bash-sourceable file.
 
@@ -1833,7 +1833,7 @@ def save_bash(self, dirpath: Path, filename: Optional[str] = None) -> Path:
         :returns: path to the saved file.
         """
 
-        filename = filename or f'{TEST_TOPOLOGY_FILENAME_BASE}.sh'
+        filename = filename or Path(f'{TEST_TOPOLOGY_FILENAME_BASE}.sh')
         filepath = dirpath / filename
 
         lines: list[str] = []
@@ -1882,7 +1882,7 @@ def save(
             self,
             *,
             dirpath: Path,
-            filename_base: Optional[str] = None) -> list[Path]:
+            filename_base: Optional[Path] = None) -> list[Path]:
         """
         Save the topology in files.
 
@@ -1893,16 +1893,19 @@ def save(
         """
 
         return [
-            self.save_yaml(dirpath, filename=(f'{filename_base}.yaml' if filename_base else None)),
-            self.save_bash(dirpath, filename=(f'{filename_base}.sh' if filename_base else None))
-            ]
+            self.save_yaml(
+                dirpath,
+                filename=Path(f'{filename_base}.yaml') if filename_base else None),
+            self.save_bash(
+                dirpath,
+                filename=Path(f'{filename_base}.sh') if filename_base else None)]
 
     def push(
             self,
             *,
             dirpath: Path,
             guest: 'Guest',
-            filename_base: Optional[str] = None,
+            filename_base: Optional[Path] = None,
             logger: tmt.log.Logger) -> EnvironmentType:
         """
         Save and push topology to a given guest.
@@ -2075,3 +2078,36 @@ def sync_with_guests(
         # Shall be fixed with https://github.com/teemtee/tmt/pull/2094
         raise tmt.utils.GeneralError(f'{step.__class__.__name__.lower()} step failed') \
             from failed_actions[0].exc
+
+
+def safe_filename(basename: str, phase: Phase, guest: 'Guest') -> Path:
+    """
+    Construct a non-conflicting filename safe for parallel tasks.
+
+    Function adds enough uniqueness to the starting base name by adding a phase
+    name and a guest name that the eventual filename would be safe against
+    conflicting access from a phase running on multiple guests, and against
+    re-use when created by the same plugin in different phases.
+
+    At first glance, the name might be an overkill: at one moment, there is
+    just one phase running on the given guest, why bother? No other phase
+    would touch the file on the guest. But:
+
+    1. filenames are created locally. One phase, running against multiple
+       guests, still needs the filename to be unique **on the tmt runner**.
+       Otherwise, phase running in different threads would contest a single
+       file.
+    2. while the scenario is unlikely, user may eventually convince tmt to
+       recognize two different names for the same machine, via ``-h connect
+       --guest $IP``. Therefore it may happen that one phase, running
+       against two guests, would actually run on the very same HW. Therefore
+       even the remote per-guest uniqueness is needed.
+    3. the phase name is included to avoid re-use of the filename by different
+       phases. A plugin may be invoked by multiple phases, and it might use a
+       "constant" name for the file. That would lead to the filename being
+       re-used by different plugin executions. Adding the phase name should
+       lower confusion: it would be immediately clear which phase used which
+       filename, or whether a filename was or was not created by given phase.
+    """
+
+    return Path(f'{basename}-{phase.safe_name}-{guest.safe_name}')

tmt/steps/execute/internal.py

@@ -17,6 +17,7 @@
 import tmt.utils
 from tmt.base import Test
 from tmt.result import BaseResult, CheckResult, Result, ResultOutcome
+from tmt.steps import safe_filename
 from tmt.steps.execute import SCRIPTS, TEST_OUTPUT_FILENAME, TMT_REBOOT_SCRIPT
 from tmt.steps.provision import Guest
 from tmt.utils import EnvironmentType, Path, ShellScript, Stopwatch, field
@@ -269,7 +270,7 @@ def execute(
         # the same `discover` phase for different guests at the same time, and
         # must keep them isolated. The wrapper script, while being prepared, is
         # a shared global state, and we must prevent race conditions.
-        test_wrapper_filename = f'{TEST_WRAPPER_FILENAME}.{guest.name}'
+        test_wrapper_filename = safe_filename(TEST_WRAPPER_FILENAME, self, guest)
         test_wrapper_filepath = workdir / test_wrapper_filename
 
         logger.debug('test wrapper', test_wrapper_filepath)

tmt/steps/finish/shell.py

@@ -7,6 +7,7 @@
 import tmt.steps
 import tmt.steps.finish
 import tmt.utils
+from tmt.steps import safe_filename
 from tmt.steps.provision import Guest
 from tmt.utils import ShellScript, field
 
@@ -71,7 +72,8 @@ def go(
 
         workdir = self.step.plan.worktree
         assert workdir is not None  # narrow type
-        finish_wrapper_filename = f'{FINISH_WRAPPER_FILENAME}-{self.safe_name}-{guest.safe_name}'
+
+        finish_wrapper_filename = safe_filename(FINISH_WRAPPER_FILENAME, self, guest)
         finish_wrapper_path = workdir / finish_wrapper_filename
 
         logger.debug('finish wrapper', finish_wrapper_path, level=3)

tmt/steps/prepare/shell.py

@@ -8,6 +8,7 @@
 import tmt.steps
 import tmt.steps.prepare
 import tmt.utils
+from tmt.steps import safe_filename
 from tmt.steps.provision import Guest
 from tmt.utils import ShellScript, field
 
@@ -80,18 +81,15 @@ def go(
             topology = tmt.steps.Topology(self.step.plan.provision.guests())
             topology.guest = tmt.steps.GuestTopology(guest)
 
-            # Since we do not have the test data dir at hand, we must make the topology
-            # filename unique on our own, and include the phase name and guest name.
-            filename_base = f'{tmt.steps.TEST_TOPOLOGY_FILENAME_BASE}-{self.safe_name}-{guest.safe_name}'  # noqa: E501
-
             environment.update(
                 topology.push(
                     dirpath=workdir,
                     guest=guest,
                     logger=logger,
-                    filename_base=filename_base))
+                    filename_base=safe_filename(tmt.steps.TEST_TOPOLOGY_FILENAME_BASE, self, guest)
+                    ))
 
-        prepare_wrapper_filename = f'{PREPARE_WRAPPER_FILENAME}-{self.safe_name}-{guest.safe_name}'
+        prepare_wrapper_filename = safe_filename(PREPARE_WRAPPER_FILENAME, self, guest)
         prepare_wrapper_path = workdir / prepare_wrapper_filename
 
         logger.debug('prepare wrapper', prepare_wrapper_path, level=3)