From 002ae5f50230020cabd19d5f3f904abd11f9973d Mon Sep 17 00:00:00 2001 From: Ismail Ibrahim Quwarah Date: Thu, 10 Oct 2024 14:04:56 +0200 Subject: [PATCH 1/3] draft for epel feature as feature plugin --- tmt/plugins/__init__.py | 1 + tmt/steps/prepare/feature.py | 79 ++++++++++++++++--------------- tmt/steps/prepare/feature/epel.py | 20 ++++++++ 3 files changed, 62 insertions(+), 38 deletions(-) create mode 100644 tmt/steps/prepare/feature/epel.py diff --git a/tmt/plugins/__init__.py b/tmt/plugins/__init__.py index fd7e42fa51..d168a9b297 100644 --- a/tmt/plugins/__init__.py +++ b/tmt/plugins/__init__.py @@ -73,6 +73,7 @@ def _discover_packages() -> list[tuple[str, Path]]: ('tmt.frameworks', Path('frameworks')), ('tmt.checks', Path('checks')), ('tmt.package_managers', Path('package_managers')), + ('tmt.steps.prepare.feature', Path('steps/prepare/feature')), ] diff --git a/tmt/steps/prepare/feature.py b/tmt/steps/prepare/feature.py index 7d0af56923..adc7473a7a 100644 --- a/tmt/steps/prepare/feature.py +++ b/tmt/steps/prepare/feature.py @@ -1,5 +1,5 @@ import dataclasses -from typing import Optional, cast +from typing import Callable, Optional, cast import tmt import tmt.base @@ -9,12 +9,34 @@ import tmt.steps.prepare import tmt.steps.provision import tmt.utils +from tmt.plugins import PluginRegistry from tmt.result import PhaseResult from tmt.steps.provision import Guest from tmt.utils import Path, field FEATURE_PLAYEBOOK_DIRECTORY = tmt.utils.resource_files('steps/prepare/feature') +FeatureClass = type['Feature'] +_FEATURE_PLUGIN_REGISTRY: PluginRegistry[FeatureClass] = PluginRegistry() + + +def provides_feature( + feature: str) -> Callable[[FeatureClass], FeatureClass]: + """ + A decorator for registering feature plugins. + Decorate a feature plugin class to register a feature. + """ + + def _provides_feature(feature_cls: FeatureClass) -> FeatureClass: + _FEATURE_PLUGIN_REGISTRY.register_plugin( + plugin_id=feature, + plugin=feature_cls, + logger=tmt.log.Logger.get_bootstrap_logger()) + + return feature_cls + + return _provides_feature + class Feature(tmt.utils.Common): """ Base class for ``feature`` prepare plugin implementations """ @@ -32,6 +54,12 @@ def __init__( self.guest = guest + def enable(self) -> None: + raise NotImplementedError + + def disable(self) -> None: + raise NotImplementedError + def _find_playbook(self, filename: str) -> Optional[Path]: filepath = FEATURE_PLAYEBOOK_DIRECTORY / filename if filepath.exists(): @@ -40,8 +68,6 @@ def _find_playbook(self, filename: str) -> Optional[Path]: self.warn(f"Cannot find any suitable playbook for '{filename}'.") return None - -class ToggleableFeature(Feature): def _run_playbook(self, op: str, playbook_filename: str) -> None: playbook_path = self._find_playbook(playbook_filename) if not playbook_path: @@ -51,33 +77,6 @@ def _run_playbook(self, op: str, playbook_filename: str) -> None: self.info(f'{op.capitalize()} {self.NAME.upper()}') self.guest.ansible(playbook_path) - def _enable(self, playbook_filename: str) -> None: - self._run_playbook('enable', playbook_filename) - - def _disable(self, playbook_filename: str) -> None: - self._run_playbook('disable', playbook_filename) - - def enable(self) -> None: - raise NotImplementedError - - def disable(self) -> None: - raise NotImplementedError - - -class EPEL(ToggleableFeature): - NAME = 'epel' - - def enable(self) -> None: - self._enable('epel-enable.yaml') - - def disable(self) -> None: - self._disable('epel-disable.yaml') - - -_FEATURES: dict[str, type[Feature]] = { - EPEL.NAME: EPEL - } - @dataclasses.dataclass class PrepareFeatureData(tmt.steps.prepare.PrepareStepData): @@ -126,14 +125,20 @@ def go( if self.opt('dry'): return [] - # Enable or disable epel - for feature_key in _FEATURES: - value = cast(Optional[str], getattr(self.data, feature_key, None)) + print("### DEBUG IZMI ###") + print(f"obsah registru: {list(_FEATURE_PLUGIN_REGISTRY.iter_plugins())}") + print_value = cast(Optional[str], getattr(self.data, "epel", None)) + print(f"obsah value: {print_value}") + + for feature_id in _FEATURE_PLUGIN_REGISTRY.iter_plugin_ids(): + feature = _FEATURE_PLUGIN_REGISTRY.get_plugin(feature_id) + + assert feature is not None # narrow type + + value = cast(Optional[str], getattr(self.data, feature.NAME, None)) if value is None: continue - - feature = _FEATURES[feature_key](parent=self, guest=guest, logger=logger) - if isinstance(feature, ToggleableFeature): + if isinstance(feature, Feature): value = value.lower() if value == 'enabled': feature.enable() @@ -141,8 +146,6 @@ def go( feature.disable() else: raise tmt.utils.GeneralError(f"Unknown feature setting '{value}'.") - else: - raise tmt.utils.GeneralError(f"Unsupported feature '{feature_key}'.") return results diff --git a/tmt/steps/prepare/feature/epel.py b/tmt/steps/prepare/feature/epel.py new file mode 100644 index 0000000000..00cc2e780c --- /dev/null +++ b/tmt/steps/prepare/feature/epel.py @@ -0,0 +1,20 @@ +from typing import TYPE_CHECKING, Any + +if TYPE_CHECKING: + pass + +from tmt.steps.prepare.feature import Feature, provides_feature + + +@provides_feature('epel') +class Epel(Feature): + NAME = "epel" + + def __init__(self, *args: Any, **kwargs: Any) -> None: + super().__init__(*args, **kwargs) + + def enable(self) -> None: + self._run_playbook('enable', "epel-enable.yaml") + + def disable(self) -> None: + self._run_playbook('disable', "epel-disable.yaml") From ba52d6864416585896489435478c3a6f194b3894 Mon Sep 17 00:00:00 2001 From: Ismail Ibrahim Quwarah Date: Wed, 6 Nov 2024 11:31:04 +0100 Subject: [PATCH 2/3] rewrite epel feature as feature plugin --- pyproject.toml | 2 +- .../{feature.py => feature/__init__.py} | 68 ++++++++++++------- tmt/steps/prepare/feature/epel.py | 13 ++-- 3 files changed, 53 insertions(+), 30 deletions(-) rename tmt/steps/prepare/{feature.py => feature/__init__.py} (69%) diff --git a/pyproject.toml b/pyproject.toml index 0f125de355..2e8eef10bf 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -246,7 +246,7 @@ ignore = [ "tmt/steps/discover/*.py", "tmt/steps/execute/*.py", "tmt/steps/finish/*.py", - "tmt/steps/prepare/feature.py", + "tmt/steps/prepare/feature/__init__.py", "tmt/steps/prepare/__init__.py", "tmt/steps/prepare/install.py", "tmt/steps/provision/*.py", diff --git a/tmt/steps/prepare/feature.py b/tmt/steps/prepare/feature/__init__.py similarity index 69% rename from tmt/steps/prepare/feature.py rename to tmt/steps/prepare/feature/__init__.py index adc7473a7a..50e53f398c 100644 --- a/tmt/steps/prepare/feature.py +++ b/tmt/steps/prepare/feature/__init__.py @@ -38,6 +38,22 @@ def _provides_feature(feature_cls: FeatureClass) -> FeatureClass: return _provides_feature +def find_plugin(name: str) -> 'FeatureClass': + """ + Find a plugin by its name. + + :raises GeneralError: when the plugin does not exist. + """ + + plugin = _FEATURE_PLUGIN_REGISTRY.get_plugin(name) + + if plugin is None: + raise tmt.utils.GeneralError( + f"Feature plugin '{name}' was not found in feature registry.") + + return plugin + + class Feature(tmt.utils.Common): """ Base class for ``feature`` prepare plugin implementations """ @@ -54,28 +70,37 @@ def __init__( self.guest = guest - def enable(self) -> None: + @classmethod + def enable(cls, guest: Guest, logger: tmt.log.Logger) -> None: raise NotImplementedError - def disable(self) -> None: + @classmethod + def disable(cls, guest: Guest, logger: tmt.log.Logger) -> None: raise NotImplementedError - def _find_playbook(self, filename: str) -> Optional[Path]: + @classmethod + def _find_playbook(cls, filename: str, logger: tmt.log.Logger) -> Optional[Path]: filepath = FEATURE_PLAYEBOOK_DIRECTORY / filename if filepath.exists(): return filepath - self.warn(f"Cannot find any suitable playbook for '{filename}'.") + logger.warning(f"Cannot find any suitable playbook for '{filename}'.", 0) return None - def _run_playbook(self, op: str, playbook_filename: str) -> None: - playbook_path = self._find_playbook(playbook_filename) + @classmethod + def _run_playbook( + cls, + op: str, + playbook_filename: str, + guest: Guest, + logger: tmt.log.Logger) -> None: + playbook_path = cls._find_playbook(playbook_filename, logger) if not playbook_path: raise tmt.utils.GeneralError( - f"{op.capitalize()} {self.NAME.upper()} is not supported on this guest.") + f"{op.capitalize()} {cls.NAME.upper()} is not supported on this guest.") - self.info(f'{op.capitalize()} {self.NAME.upper()}') - self.guest.ansible(playbook_path) + logger.info(f'{op.capitalize()} {cls.NAME.upper()}') + guest.ansible(playbook_path) @dataclasses.dataclass @@ -125,27 +150,20 @@ def go( if self.opt('dry'): return [] - print("### DEBUG IZMI ###") - print(f"obsah registru: {list(_FEATURE_PLUGIN_REGISTRY.iter_plugins())}") - print_value = cast(Optional[str], getattr(self.data, "epel", None)) - print(f"obsah value: {print_value}") - for feature_id in _FEATURE_PLUGIN_REGISTRY.iter_plugin_ids(): - feature = _FEATURE_PLUGIN_REGISTRY.get_plugin(feature_id) - - assert feature is not None # narrow type + feature = find_plugin(feature_id) value = cast(Optional[str], getattr(self.data, feature.NAME, None)) if value is None: continue - if isinstance(feature, Feature): - value = value.lower() - if value == 'enabled': - feature.enable() - elif value == 'disabled': - feature.disable() - else: - raise tmt.utils.GeneralError(f"Unknown feature setting '{value}'.") + + value = value.lower() + if value == 'enabled': + feature.enable(guest, logger) + elif value == 'disabled': + feature.disable(guest, logger) + else: + raise tmt.utils.GeneralError(f"Unknown feature setting '{value}'.") return results diff --git a/tmt/steps/prepare/feature/epel.py b/tmt/steps/prepare/feature/epel.py index 00cc2e780c..1f4b3c6a3c 100644 --- a/tmt/steps/prepare/feature/epel.py +++ b/tmt/steps/prepare/feature/epel.py @@ -1,5 +1,8 @@ from typing import TYPE_CHECKING, Any +import tmt.log +from tmt.steps.provision import Guest + if TYPE_CHECKING: pass @@ -13,8 +16,10 @@ class Epel(Feature): def __init__(self, *args: Any, **kwargs: Any) -> None: super().__init__(*args, **kwargs) - def enable(self) -> None: - self._run_playbook('enable', "epel-enable.yaml") + @classmethod + def enable(cls, guest: Guest, logger: tmt.log.Logger) -> None: + cls._run_playbook('enable', "epel-enable.yaml", guest, logger) - def disable(self) -> None: - self._run_playbook('disable', "epel-disable.yaml") + @classmethod + def disable(cls, guest: Guest, logger: tmt.log.Logger) -> None: + cls._run_playbook('disable', "epel-disable.yaml", guest, logger) From 213e6fa643a29141684968e60745bed250455734 Mon Sep 17 00:00:00 2001 From: Ismail Ibrahim Quwarah Date: Wed, 6 Nov 2024 14:55:44 +0100 Subject: [PATCH 3/3] Update doc with new file name --- stories/cli/try.fmf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stories/cli/try.fmf b/stories/cli/try.fmf index f36ea23634..97d2ac5cbe 100644 --- a/stories/cli/try.fmf +++ b/stories/cli/try.fmf @@ -145,7 +145,7 @@ link: link: - implemented-by: /tmt/trying.py - - implemented-by: /tmt/steps/prepare/feature.py + - implemented-by: /tmt/steps/prepare/feature/__init__.py /install: story: