Install essential requirements with its own order (#3334)

It turns out essential requirements must be installed before regular
phases start running, because these phases may need their requirements
to even begin. In particular, we noticed `prepare/ansible` requires a
Python interpreter, but the default `order` puts it before installation
of (collected) requirements.

spec/plans/prepare.fmf

@@ -14,9 +14,10 @@ description: |
 
     Use the ``order`` attribute to select in which order the
     preparation should happen if there are multiple configs.
-    Default order is ``50``. For installation of required packages
-    gathered from the :ref:`/spec/tests/require` attribute of
-    individual tests order ``70`` is used, for recommended
+    Default order is ``50``. For installation of essential plugin
+    and check requirements ``30`` is used, for installation of
+    required packages gathered from the :ref:`/spec/tests/require`
+    attribute of individual tests order ``70`` is used, for recommended
     packages it is ``75``. The Dist-git prepare happens before those,
     with order ``60``.
 

tests/multihost/complete/test.sh

@@ -42,16 +42,25 @@ rlJournalStart
     rlPhaseStartTest "Prepare"
         rlRun -s "tmt -vvv run --scratch --id $run discover provision prepare"
 
-        rlRun "grep '^        queued prepare task #1: default-0 on client-1 (client) and client-2 (client)' $rlRun_LOG"
-        rlRun "grep '^        queued prepare task #2: default-1 on server (server)' $rlRun_LOG"
-
-        rlRun "grep '^        prepare task #1: default-0 on client-1 (client) and client-2 (client)' $rlRun_LOG"
+        rlRun "grep '^        queued prepare task #1: essential-requires on client-1 (client), client-2 (client) and server (server)' $rlRun_LOG"
+        rlRun "grep '^        queued prepare task #2: default-0 on client-1 (client) and client-2 (client)' $rlRun_LOG"
+        rlRun "grep '^        queued prepare task #3: default-1 on server (server)' $rlRun_LOG"
+
+        rlRun "grep '^        prepare task #1: essential-requires on client-1 (client), client-2 (client) and server (server)' $rlRun_LOG"
+        rlRun "grep '^\\[client-1 (client)\\]         how: install' $rlRun_LOG"
+        rlRun "grep '^\\[client-1 (client)\\]         summary: Install essential required packages' $rlRun_LOG"
+        rlRun "grep '^\\[client-2 (client)\\]         how: install' $rlRun_LOG"
+        rlRun "grep '^\\[client-2 (client)\\]         summary: Install essential required packages' $rlRun_LOG"
+        rlRun "grep '^\\[server (server)\\]           how: install' $rlRun_LOG"
+        rlRun "grep '^\\[server (server)\\]           summary: Install essential required packages' $rlRun_LOG"
+
+        rlRun "grep '^        prepare task #2: default-0 on client-1 (client) and client-2 (client)' $rlRun_LOG"
         rlRun "grep '^\\[client-1 (client)\\]         how: shell' $rlRun_LOG"
         rlRun "grep '^\\[client-1 (client)\\]         overview: 3 scripts found' $rlRun_LOG"
         rlRun "grep '^\\[client-2 (client)\\]         how: shell' $rlRun_LOG"
         rlRun "grep '^\\[client-2 (client)\\]         overview: 3 scripts found' $rlRun_LOG"
 
-        rlRun "grep '^        prepare task #2: default-1 on server (server)' $rlRun_LOG"
+        rlRun "grep '^        prepare task #3: default-1 on server (server)' $rlRun_LOG"
         rlRun "grep '^        how: shell' $rlRun_LOG"
         rlRun "grep '^        guest: server' $rlRun_LOG"
         rlRun "grep '^        overview: 3 scripts found' $rlRun_LOG"

tests/prepare/require/test.sh

@@ -26,7 +26,7 @@ rlJournalStart
 
         rlPhaseStartTest "Require an available package ($image)"
             rlRun -s "$tmt plan --name available"
-            rlAssertGrep '1 preparation applied' $rlRun_LOG
+            rlAssertGrep '2 preparations applied' $rlRun_LOG
         rlPhaseEnd
 
         rlPhaseStartTest "Require a missing package ($image)"

tmt/steps/prepare/__init__.py

@@ -1,6 +1,6 @@
 import copy
 import dataclasses
-from typing import TYPE_CHECKING, Any, Optional, TypeVar, Union, cast
+from typing import TYPE_CHECKING, Any, Literal, Optional, TypeVar, Union, cast
 
 import click
 import fmf
@@ -100,6 +100,34 @@ def go(
         return []
 
 
+# Required & recommended packages
+#
+# Structures and code for collecting requirements for different guests
+# or their groups to avoid installing all test requirements on all
+# guests. For example, a test running on the "server" guest might
+# require package `foo` while the test running on the "client" might
+# require package `bar`, and `foo` and `bar` cannot be installed at the
+# sametime.
+
+
+@dataclasses.dataclass
+class DependencyCollection:
+    """ Bundle guests and packages to install on them """
+
+    # Guest*s*, not a guest. The list will start with just one guest at
+    # first, but when grouping guests by same requirements, we'd start
+    # adding guests to the list when spotting same set of dependencies.
+    guests: list[Guest]
+    dependencies: list['tmt.base.DependencySimple'] = dataclasses.field(default_factory=list)
+
+    @property
+    def as_key(self) -> 'DependencyCollectionKey':
+        return frozenset(self.dependencies)
+
+
+DependencyCollectionKey = frozenset['tmt.base.DependencySimple']
+
+
 class Prepare(tmt.steps.Step):
     """
     Prepare the environment for testing.
@@ -169,31 +197,6 @@ def go(self, force: bool = False) -> None:
 
         import tmt.base
 
-        # Required & recommended packages
-        #
-        # Take care of collecting requirements for different guests or their
-        # groups to avoid installing all test requirements on all guests. For
-        # example, a test running on the "server" guest might require
-        # package `foo` while the test running on the "client" might require
-        # package `bar`, and `foo` and `bar` cannot be installed at the same
-        # time.
-        @dataclasses.dataclass
-        class DependencyCollection:
-            """ Bundle guests and packages to install on them """
-
-            # Guest*s*, not a guest. The list will start with just one guest in
-            # `collected_* dicts, but when grouping guests by same requirements,
-            # we'd start adding guests to the list when spotting same set of
-            # dependencies.
-            guests: list[Guest]
-            dependencies: list['tmt.base.DependencySimple'] \
-                = dataclasses.field(default_factory=list)
-
-            @property
-            def as_key(self) -> frozenset['tmt.base.DependencySimple']:
-                # ignore[attr-defined]: mypy doesn't seem to understand collection as `frozenset`
-                return frozenset(collection.dependencies)  # type: ignore[has-type]
-
         # All phases from all steps.
         phases = [
             phase
@@ -209,30 +212,42 @@ def as_key(self) -> frozenset['tmt.base.DependencySimple']:
         # All provisioned guests.
         guests = self.plan.provision.guests()
 
+        # 1. collect all requirements, per guest. For each phase, test,
+        # check and so on, find out on which guest it needs to run, and
+        # add its requirements to a guest-specific collection.
+
+        # Collecting all essential requirements, per guest.
+        collected_essential_requires = {
+            guest: DependencyCollection(guests=[guest]) for guest in guests
+            }
+
         # Collecting all required packages, per guest.
-        collected_requires: dict[Guest, DependencyCollection] = {
+        collected_requires = {
             guest: DependencyCollection(guests=[guest]) for guest in guests
             }
 
         # Collecting all recommended packages, per guest.
-        collected_recommends: dict[Guest, DependencyCollection] = {
+        collected_recommends = {
             guest: DependencyCollection(guests=[guest]) for guest in guests
             }
 
-        # For each guest and phase pair, check whether the phase is supposed to
-        # run on the guest. If so, just add its requirements on top of the
-        # guest's pile of requirements.
+        # For each guest, check everything that can run, and if enabled
+        # for the given guest, add requirements into the correct
+        # collection.
         for guest in guests:
+            # First, check phases - plugins have their own requirements,
+            # the essential requirements.
             for phase in phases:
                 if not phase.enabled_on_guest(guest):
                     continue
 
-                collected_requires[guest].dependencies += tmt.base.assert_simple_dependencies(
-                    # ignore[attr-defined]: mypy thinks that phase is Phase type, while its
-                    # actually PluginClass
-                    phase.essential_requires(),  # type: ignore[attr-defined]
-                    'After beakerlib processing, tests may have only simple requirements',
-                    self._logger)
+                collected_essential_requires[guest].dependencies \
+                    += tmt.base.assert_simple_dependencies(
+                        # ignore[attr-defined]: mypy thinks that phase is Phase type, while its
+                        # actually PluginClass
+                        phase.essential_requires(),  # type: ignore[attr-defined]
+                        'After beakerlib processing, tests may have only simple requirements',
+                        self._logger)
 
             # The `discover` step is different: no phases, just query tests
             # collected by the step itself. Maybe we could iterate over
@@ -254,17 +269,19 @@ def as_key(self) -> frozenset['tmt.base.DependencySimple']:
                     'After beakerlib processing, tests may have only simple requirements',
                     self._logger)
 
-                collected_requires[guest].dependencies += test.test_framework.get_requirements(
-                    test,
-                    self._logger)
-
-                for check in test.check:
-                    collected_requires[guest].dependencies += check.plugin.essential_requires(
-                        guest,
+                collected_essential_requires[guest].dependencies \
+                    += test.test_framework.get_requirements(
                         test,
                         self._logger)
 
-        # Now we have guests and all their requirements. There can be
+                for check in test.check:
+                    collected_essential_requires[guest].dependencies \
+                        += check.plugin.essential_requires(
+                            guest,
+                            test,
+                            self._logger)
+
+        # 2. Now we have guests and all their requirements. There can be
         # duplicities, multiple tests requesting the same package, but also
         # some guests may share the set of packages to be installed on them.
         # Let's say N guests share a `role`, all their tests would add the same
@@ -274,58 +291,70 @@ def as_key(self) -> frozenset['tmt.base.DependencySimple']:
         #
         # 1. make the list of requirements unique,
         # 2. group guests with same requirements.
-        from tmt.steps.prepare.install import PrepareInstallData
+        def _prune_collections(
+                collections: dict[Guest, DependencyCollection]) -> list[DependencyCollection]:
 
-        pruned_requires: dict[frozenset[tmt.base.DependencySimple], DependencyCollection] = {}
-        pruned_recommends: dict[frozenset[tmt.base.DependencySimple], DependencyCollection] = {}
+            pruned: dict[DependencyCollectionKey, DependencyCollection] = {}
 
-        for guest, collection in collected_requires.items():
-            collection.dependencies = uniq(collection.dependencies)
+            for guest, collection in collections.items():
+                collection.dependencies = uniq(collection.dependencies)
 
-            if collection.as_key in pruned_requires:
-                pruned_requires[collection.as_key].guests.append(guest)
+                if collection.as_key in pruned:
+                    pruned[collection.as_key].guests.append(guest)
 
-            else:
-                pruned_requires[collection.as_key] = collection
+                else:
+                    pruned[collection.as_key] = collection
 
-        for guest, collection in collected_recommends.items():
-            collection.dependencies = uniq(collection.dependencies)
+            return list(pruned.values())
 
-            if collection.as_key in pruned_recommends:
-                pruned_recommends[collection.as_key].guests.append(guest)
+        pruned_essential_requires = _prune_collections(collected_essential_requires)
+        pruned_requires = _prune_collections(collected_requires)
+        pruned_recommends = _prune_collections(collected_recommends)
 
-            else:
-                pruned_recommends[collection.as_key] = collection
+        # 3. for each collection, which now groups a set of packages and
+        # all guests they need to be installed on, add new phase that
+        # would take care of installation.
+        def _emit_phase(
+                pruned_collections: list[DependencyCollection],
+                name: str,
+                summary: str,
+                order: int,
+                missing: Literal['skip', 'fail'] = 'fail') -> None:
+            from tmt.steps.prepare.install import PrepareInstallData
 
-        for collection in pruned_requires.values():
-            if not collection.dependencies:
-                continue
-
-            data = PrepareInstallData(
-                name='requires',
-                how='install',
-                summary='Install required packages',
-                order=tmt.utils.DEFAULT_PLUGIN_ORDER_REQUIRES,
-                where=[guest.name for guest in collection.guests],
-                package=collection.dependencies
-                )
-
-            self._phases.append(PreparePlugin.delegate(self, data=data))
-
-        for collection in pruned_recommends.values():
-            if not collection.dependencies:
-                continue
-
-            data = PrepareInstallData(
-                how='install',
-                name='recommends',
-                summary='Install recommended packages',
-                order=tmt.utils.DEFAULT_PLUGIN_ORDER_RECOMMENDS,
-                where=[guest.name for guest in collection.guests],
-                package=collection.dependencies,
-                missing='skip')
+            for collection in pruned_collections:
+                if not collection.dependencies:
+                    continue
 
-            self._phases.append(PreparePlugin.delegate(self, data=data))
+                data = PrepareInstallData(
+                    name=name,
+                    how='install',
+                    summary=summary,
+                    order=order,
+                    where=[guest.name for guest in collection.guests],
+                    package=collection.dependencies,
+                    missing=missing)
+
+                self._phases.append(PreparePlugin.delegate(self, data=data))
+
+        _emit_phase(
+            pruned_essential_requires,
+            'essential-requires',
+            'Install essential required packages',
+            tmt.utils.DEFAULT_PLUGIN_ORDER_ESSENTIAL_REQUIRES)
+
+        _emit_phase(
+            pruned_requires,
+            'requires',
+            'Install required packages',
+            tmt.utils.DEFAULT_PLUGIN_ORDER_REQUIRES)
+
+        _emit_phase(
+            pruned_recommends,
+            'recommends',
+            'Install recommended packages',
+            tmt.utils.DEFAULT_PLUGIN_ORDER_RECOMMENDS,
+            missing='skip')
 
         # Prepare guests (including workdir sync)
         guest_copies: list[Guest] = []

tmt/steps/prepare/ansible.py

@@ -10,6 +10,7 @@
 import tmt.options
 import tmt.steps
 import tmt.steps.prepare
+import tmt.steps.provision
 import tmt.utils
 from tmt.result import PhaseResult
 from tmt.steps.provision import Guest
@@ -183,6 +184,4 @@ def essential_requires(self) -> list[tmt.base.Dependency]:
         :returns: a list of requirements.
         """
 
-        return [
-            tmt.base.DependencySimple('/usr/bin/python3')
-            ]
+        return tmt.steps.provision.essential_ansible_requires()

tmt/steps/prepare/feature.py

@@ -7,6 +7,7 @@
 import tmt.options
 import tmt.steps
 import tmt.steps.prepare
+import tmt.steps.provision
 import tmt.utils
 from tmt.result import PhaseResult
 from tmt.steps.provision import Guest
@@ -144,3 +145,15 @@ def go(
                 raise tmt.utils.GeneralError(f"Unsupported feature '{feature_key}'.")
 
         return results
+
+    def essential_requires(self) -> list[tmt.base.Dependency]:
+        """
+        Collect all essential requirements of the plugin.
+
+        Essential requirements of a plugin are necessary for the plugin to
+        perform its basic functionality.
+
+        :returns: a list of requirements.
+        """
+
+        return tmt.steps.provision.essential_ansible_requires()

tmt/steps/provision/__init__.py

@@ -141,6 +141,17 @@ def configure_ssh_options() -> tmt.utils.RawCommand:
 STAT_BTIME_PATTERN = re.compile(r'btime\s+(\d+)')
 
 
+# Note: returns a static list, but we cannot make it a mere list,
+# because `tmt.base` needs to be imported and that creates a circular
+# import loop.
+def essential_ansible_requires() -> list['tmt.base.Dependency']:
+    """ Return essential requirements for running Ansible modules """
+
+    return [
+        tmt.base.DependencySimple('/usr/bin/python3')
+        ]
+
+
 def format_guest_full_name(name: str, role: Optional[str]) -> str:
     """ Render guest's full name, i.e. name and its role """
 

tmt/utils/__init__.py

@@ -142,6 +142,7 @@ def configure_constant(default: int, envvar: str) -> int:
 DEFAULT_NAME = 'default'
 DEFAULT_PLUGIN_ORDER = 50
 DEFAULT_PLUGIN_ORDER_MULTIHOST = 10
+DEFAULT_PLUGIN_ORDER_ESSENTIAL_REQUIRES = 30
 DEFAULT_PLUGIN_ORDER_REQUIRES = 70
 DEFAULT_PLUGIN_ORDER_RECOMMENDS = 75