Make plan environment file guest-specific

docs/overview.rst

@@ -662,6 +662,9 @@ TMT_PLAN_ENVIRONMENT_FILE
     **not** allowed, use ``TMT_PLAN_SOURCE_SCRIPT`` instead to include
     other bash commands.
 
+    Note that this is not shared between guests of the given plan, each
+    guest has its own dedicated file.
+
     Example of the file content::
 
         COUNT=1

docs/releases/pending/4772.fmf

@@ -0,0 +1,7 @@
+description: |
+    The :ref:`plan environment file <step-variables>` is now
+    guest-specific, it is no longer shared by all guests. Being shared
+    made the file open to race conditions, as tmt would fetch it from
+    all guests, saving all input files under the same name - the last
+    one saved wins. The filename is now guest specific, files do not
+    overwrite each other.

tests/unit/test_guest.py

@@ -154,6 +154,11 @@ def test_execute_no_connection_closed(
         logger=root_logger, parent=step, name='foo', data=GuestSshData(primary_address='bar')
     )
 
+    monkeypatch.setattr(
+        guest,
+        '_prepare_environment',
+        MagicMock(return_value=Environment()),
+    )
     monkeypatch.setattr(
         guest,
         '_run_guest_command',

tmt/base/plan.py

@@ -471,7 +471,6 @@ def _environment_from_intrinsics(self) -> Environment:
 
         if self.my_run:
             environment['TMT_PLAN_DATA'] = EnvVarValue(self.data_directory)
-            environment['TMT_PLAN_ENVIRONMENT_FILE'] = EnvVarValue(self.plan_environment_file)
             environment['TMT_PLAN_SOURCE_SCRIPT'] = EnvVarValue(self.plan_source_script)
 
         return environment
@@ -504,25 +503,6 @@ def _environment_from_cli(self) -> Environment:
             logger=self._logger,
         )
 
-    @property
-    def _environment_from_plan_environment_file(self) -> Environment:
-        """
-        Environment sourced from the :ref:`plan environment file <step-variables>`.
-        """
-
-        if (
-            self.my_run
-            and self.plan_environment_file.exists()
-            and self.plan_environment_file.stat().st_size > 0
-        ):
-            return tmt.utils.Environment.from_file(
-                filename=self.plan_environment_file.name,
-                root=self.plan_environment_file.parent,
-                logger=self._logger,
-            )
-
-        return Environment()
-
     @property
     def environment(self) -> Environment:
         """
@@ -531,7 +511,6 @@ def environment(self) -> Environment:
         Contains all environment variables collected from multiple
         sources (in the following order):
 
-        * :ref:`plan environment file <step-variables>`,
         * plan's ``environment`` and ``environment-file`` keys,
         * importing plan's environment,
         * ``--environment`` and ``--environment-file`` options,
@@ -542,7 +521,6 @@ def environment(self) -> Environment:
         if self.my_run:
             return Environment(
                 {
-                    **self._environment_from_plan_environment_file,
                     **self._environment_from_fmf,
                     **self._environment_from_importing,
                     **self._environment_from_cli,
@@ -711,17 +689,6 @@ def _initialize_data_directory(self) -> None:
         self.debug(f"Create the data directory '{self.data_directory}'.", level=2)
         self.data_directory.mkdir(exist_ok=True, parents=True)
 
-    @functools.cached_property
-    def plan_environment_file(self) -> Path:
-        assert self.data_directory is not None  # narrow type
-
-        plan_environment_file_path = self.data_directory / "variables.env"
-        plan_environment_file_path.touch(exist_ok=True)
-
-        self.debug(f"Create the environment file '{plan_environment_file_path}'.", level=2)
-
-        return plan_environment_file_path
-
     @functools.cached_property
     def plan_source_script(self) -> Path:
         assert self.data_directory is not None  # narrow type

tmt/guest/__init__.py

@@ -58,6 +58,8 @@
 )
 from tmt.utils import (
     Command,
+    Environment,
+    EnvVarValue,
     GeneralError,
     OnProcessEndCallback,
     OnProcessStartCallback,
@@ -78,6 +80,9 @@
 
 T = TypeVar('T')
 
+#: Name of the :ref:`plan environment file <step-variables>`.
+PLAN_ENVIRONMENT_FILENAME = 'variables.env'
+
 #: How many seconds to wait for a connection to succeed after guest boot.
 #: This is the default value tmt would use unless told otherwise.
 DEFAULT_CONNECT_TIMEOUT = 2 * 60
@@ -1823,6 +1828,41 @@ def scripts_path(self) -> Path:
             else tmt.steps.scripts.DEFAULT_SCRIPTS_DEST_DIR
         )
 
+    @functools.cached_property
+    def plan_environment_path(self) -> Optional[Path]:
+        """
+        A path to the :ref:`plan environment file <step-variables>` file.
+        """
+
+        if not isinstance(self.parent, tmt.steps.provision.Provision):
+            return None
+
+        path = self.parent.plan.data_directory / f'{PLAN_ENVIRONMENT_FILENAME}-{self.safe_name}'
+        path.touch(exist_ok=True)
+
+        self.debug(f"Create the environment file '{path}'.", level=2)
+
+        return path
+
+    @property
+    def plan_environment(self) -> Environment:
+        """
+        Environment sourced from the :ref:`plan environment file <step-variables>`.
+        """
+
+        if (
+            self.plan_environment_path
+            and self.plan_environment_path.exists()
+            and self.plan_environment_path.stat().st_size > 0
+        ):
+            return tmt.utils.Environment.from_file(
+                filename=self.plan_environment_path.name,
+                root=self.plan_environment_path.parent,
+                logger=self._logger,
+            )
+
+        return Environment()
+
     @classmethod
     def options(cls, how: Optional[str] = None) -> list[tmt.options.ClickOptionDecoratorType]:
         """
@@ -2139,15 +2179,19 @@ def _prepare_command_environment(
         """
 
         if environment is None:
-            # narrow type
-            assert isinstance(self.parent, tmt.steps.Step)
-
             environment = tmt.utils.Environment()
 
-            environment.update(
-                self.environment,
-                self.parent.plan.environment,
-            )
+            environment.update(self.environment)
+
+            if isinstance(self.parent, tmt.steps.Step):
+                environment.update(self.parent.plan.environment)
+
+            # TODO: this was owned by plan, but at wrong position, and it will
+            # be owned by plan again once the dust of environment untangling
+            # settles. Follow https://github.com/teemtee/tmt/issues/4241 for
+            # more.
+            if self.plan_environment_path:
+                environment['TMT_PLAN_ENVIRONMENT_FILE'] = EnvVarValue(self.plan_environment_path)
 
         else:
             # Create a copy of given environment - this prevents any

tmt/steps/execute/__init__.py

@@ -365,6 +365,7 @@ def environment(self) -> Environment:
             environment.update(
                 self.guest.environment,
                 self.test.environment,
+                self.guest.plan_environment,
                 self.phase.parent.plan.environment,
             )
 
@@ -384,6 +385,12 @@ def environment(self) -> Environment:
         else:
             environment = self._environment
 
+        # TODO: this was owned by plan, but at wrong position, and it will
+        # be owned by plan again once the dust of environment untangling
+        # settles. Follow https://github.com/teemtee/tmt/issues/4241 for
+        # more.
+        environment['TMT_PLAN_ENVIRONMENT_FILE'] = EnvVarValue(self.guest.plan_environment_path)
+
         environment.update(
             # Add variables from invocation contexts
             self.abort,

tmt/steps/provision/local.py

@@ -132,10 +132,7 @@ def execute(
         sourced_files = sourced_files or []
 
         # Prepare the environment (plan/cli variables override)
-        environment = tmt.utils.Environment()
-        environment.update(env or {})
-        if self.parent:
-            environment.update(self.parent.plan.environment)
+        environment = self._prepare_command_environment(environment=env)
 
         if tty:
             self.warn("Ignoring requested tty, not supported by the 'local' provision plugin.")