From ed464dab6cd43469f1a6838057bd478d898e1054 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= Date: Tue, 3 Sep 2024 14:12:53 +0200 Subject: [PATCH 1/6] topology-marker: TopologyMark._CreateFromArgs was made public And therefore rename to CreateFromArgs. --- sssd_test_framework/config.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sssd_test_framework/config.py b/sssd_test_framework/config.py index eb0a378e..e84549d0 100644 --- a/sssd_test_framework/config.py +++ b/sssd_test_framework/config.py @@ -99,9 +99,9 @@ def export(self) -> dict: return d @classmethod - def _CreateFromArgs(cls, item: pytest.Function, args: Tuple, kwargs: Mapping[str, Any]) -> TopologyMark: + def CreateFromArgs(cls, item: pytest.Function, args: Tuple, kwargs: Mapping[str, Any]) -> TopologyMark: """ - Create :class:`TopologyMark` from pytest marker arguments. + Create :class:`TopologyMark` from pytest.mark.topology arguments. .. warning:: From 46986ad11dfb4e8a9420ddf580415871a2f82c27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= Date: Tue, 3 Sep 2024 14:17:53 +0200 Subject: [PATCH 2/6] topology-marker: TopologyMark.CreateFromArgs now returns Self --- sssd_test_framework/config.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sssd_test_framework/config.py b/sssd_test_framework/config.py index e84549d0..9b0adfdc 100644 --- a/sssd_test_framework/config.py +++ b/sssd_test_framework/config.py @@ -1,6 +1,6 @@ from __future__ import annotations -from typing import Any, Mapping, Tuple, Type +from typing import Any, Mapping, Self, Tuple, Type import pytest from pytest_mh import ( @@ -99,7 +99,7 @@ def export(self) -> dict: return d @classmethod - def CreateFromArgs(cls, item: pytest.Function, args: Tuple, kwargs: Mapping[str, Any]) -> TopologyMark: + def CreateFromArgs(cls, item: pytest.Function, args: Tuple, kwargs: Mapping[str, Any]) -> Self: """ Create :class:`TopologyMark` from pytest.mark.topology arguments. @@ -113,7 +113,7 @@ def CreateFromArgs(cls, item: pytest.Function, args: Tuple, kwargs: Mapping[str, :type item: pytest.Function :raises ValueError: If the marker is invalid. :return: Instance of TopologyMark. - :rtype: TopologyMark + :rtype: Self """ # First three parameters are positional, the rest are keyword arguments. if len(args) != 2 and len(args) != 3: From 25790bbd52991824adf2f5fbf562edf58c41a179 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= Date: Thu, 5 Sep 2024 14:47:47 +0200 Subject: [PATCH 3/6] topology-controller: TopologyController._init was made public And therefore renamed to init(). --- sssd_test_framework/topology_controllers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sssd_test_framework/topology_controllers.py b/sssd_test_framework/topology_controllers.py index 77ae0fc7..c91fbd62 100644 --- a/sssd_test_framework/topology_controllers.py +++ b/sssd_test_framework/topology_controllers.py @@ -58,8 +58,8 @@ def __init__(self) -> None: self.backup_data: dict[BaseBackupHost, Any | None] = {} self.provisioned: bool = False - def _init(self, *args, **kwargs): - super()._init(*args, **kwargs) + def init(self, *args, **kwargs): + super().init(*args, **kwargs) self.provisioned = self.name in self.multihost.provisioned_topologies def restore(self, hosts: dict[BaseBackupHost, Any | None]) -> None: From 039ad15a87a5dc859397dc4b1a19483f5cf0331a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= Date: Thu, 5 Sep 2024 14:46:48 +0200 Subject: [PATCH 4/6] hosts: use built-in MultihostBackupHost instead of BaseBackupHost This functionality has been now included in pytest-mh. --- sssd_test_framework/hosts/base.py | 112 ++------------------ sssd_test_framework/hosts/client.py | 4 +- sssd_test_framework/hosts/nfs.py | 4 +- sssd_test_framework/topology_controllers.py | 15 ++- 4 files changed, 18 insertions(+), 117 deletions(-) diff --git a/sssd_test_framework/hosts/base.py b/sssd_test_framework/hosts/base.py index 77a597c1..6dd9622b 100644 --- a/sssd_test_framework/hosts/base.py +++ b/sssd_test_framework/hosts/base.py @@ -2,14 +2,11 @@ from __future__ import annotations -from abc import ABC, abstractmethod -from pathlib import PurePath from typing import Any import ldap from ldap.ldapobject import ReconnectLDAPObject -from pytest_mh import MultihostHost -from pytest_mh.conn import Powershell +from pytest_mh import MultihostBackupHost, MultihostHost from pytest_mh.utils.fs import LinuxFileSystem from pytest_mh.utils.services import SystemdServices @@ -18,17 +15,20 @@ __all__ = [ "BaseHost", - "BaseBackupHost", "BaseDomainHost", "BaseLDAPDomainHost", ] -class BaseHost(MultihostHost[SSSDMultihostDomain]): +class BaseHost(MultihostBackupHost[SSSDMultihostDomain]): """ Base class for all SSSD hosts. """ + def __init__(self, *args, **kwargs) -> None: + # restore is handled in topology controllers + super().__init__(*args, auto_restore=False, **kwargs) + @property def features(self) -> dict[str, bool]: """ @@ -37,105 +37,7 @@ def features(self) -> dict[str, bool]: return {} -class BaseBackupHost(BaseHost, ABC): - """ - Base class for all hosts that supports automatic backup and restore. - - A backup of the host is created before starting a test case and all changes - done in the test case to the host are automatically reverted when the test - run is finished. - - .. warning:: - - There might be some limitations on what data can and can not be restored - that depends on particular host. See the documentation of each host - class to learn if a full or partial restoration is done. - """ - - def __init__(self, *args, **kwargs) -> None: - super().__init__(*args, **kwargs) - - self.backup_data: Any | None = None - """Backup data of vanilla state of this host.""" - - def pytest_setup(self) -> None: - # Make sure required services are running - try: - self.start() - except NotImplementedError: - pass - - # Create backup of initial state - self.backup_data = self.backup() - - def pytest_teardown(self) -> None: - self.remove_backup(self.backup_data) - - def remove_backup(self, backup_data: Any | None) -> None: - """ - Remove backup data from the host. - - :param backup_data: Backup data. - :type backup_data: Any | None - """ - if backup_data is None: - return - - if isinstance(backup_data, PurePath): - path = str(backup_data) - else: - raise TypeError(f"Only PurePath is supported as backup_data, got {type(backup_data)}") - - if isinstance(self.conn.shell, Powershell): - self.conn.exec(["Remove-Item", "-Force", "-Recurse", path]) - else: - self.conn.exec(["rm", "-fr", path]) - - @abstractmethod - def start(self) -> None: - """ - Start required services. - - :raises NotImplementedError: If start operation is not supported. - """ - pass - - @abstractmethod - def stop(self) -> None: - """ - Stop required services. - - :raises NotImplementedError: If stop operation is not supported. - """ - pass - - @abstractmethod - def backup(self) -> Any: - """ - Backup backend data. - - Returns directory or file path where the backup is stored (as PurePath) - or any Python data relevant for the backup. This data is passed to - :meth:`restore` which will use this information to restore the host to - its original state. - - :return: Backup data. - :rtype: Any - """ - pass - - @abstractmethod - def restore(self, backup_data: Any | None) -> None: - """ - Restore backend data. - - :param backup_data: Backup data. - :type backup_data: Any | None - """ - pass - - -class BaseDomainHost(BaseBackupHost): +class BaseDomainHost(BaseHost): """ Base class for all domain (backend) hosts. diff --git a/sssd_test_framework/hosts/client.py b/sssd_test_framework/hosts/client.py index 75729e12..e19e0494 100644 --- a/sssd_test_framework/hosts/client.py +++ b/sssd_test_framework/hosts/client.py @@ -7,14 +7,14 @@ from pytest_mh.conn import ProcessLogLevel -from .base import BaseBackupHost, BaseLinuxHost +from .base import BaseHost, BaseLinuxHost __all__ = [ "ClientHost", ] -class ClientHost(BaseBackupHost, BaseLinuxHost): +class ClientHost(BaseHost, BaseLinuxHost): """ SSSD client host object. diff --git a/sssd_test_framework/hosts/nfs.py b/sssd_test_framework/hosts/nfs.py index 258e3fa9..e08e32d4 100644 --- a/sssd_test_framework/hosts/nfs.py +++ b/sssd_test_framework/hosts/nfs.py @@ -7,14 +7,14 @@ from pytest_mh.conn import ProcessLogLevel -from .base import BaseBackupHost, BaseLinuxHost +from .base import BaseHost, BaseLinuxHost __all__ = [ "NFSHost", ] -class NFSHost(BaseBackupHost, BaseLinuxHost): +class NFSHost(BaseHost, BaseLinuxHost): """ NFS server host object. diff --git a/sssd_test_framework/topology_controllers.py b/sssd_test_framework/topology_controllers.py index c91fbd62..a0c1829c 100644 --- a/sssd_test_framework/topology_controllers.py +++ b/sssd_test_framework/topology_controllers.py @@ -3,12 +3,11 @@ from functools import partial, wraps from typing import Any -from pytest_mh import TopologyController +from pytest_mh import MultihostBackupHost, TopologyController from pytest_mh.conn import ProcessResult from .config import SSSDMultihostConfig from .hosts.ad import ADHost -from .hosts.base import BaseBackupHost from .hosts.client import ClientHost from .hosts.ipa import IPAHost from .hosts.nfs import NFSHost @@ -55,17 +54,17 @@ class BackupTopologyController(TopologyController[SSSDMultihostConfig]): def __init__(self) -> None: super().__init__() - self.backup_data: dict[BaseBackupHost, Any | None] = {} + self.backup_data: dict[MultihostBackupHost, Any | None] = {} self.provisioned: bool = False def init(self, *args, **kwargs): super().init(*args, **kwargs) self.provisioned = self.name in self.multihost.provisioned_topologies - def restore(self, hosts: dict[BaseBackupHost, Any | None]) -> None: + def restore(self, hosts: dict[MultihostBackupHost, Any | None]) -> None: errors = [] for host, backup_data in hosts.items(): - if not isinstance(host, BaseBackupHost): + if not isinstance(host, MultihostBackupHost): continue try: @@ -77,10 +76,10 @@ def restore(self, hosts: dict[BaseBackupHost, Any | None]) -> None: raise ExceptionGroup("Some hosts failed to restore to original state", errors) def restore_vanilla(self) -> None: - restore_data: dict[BaseBackupHost, Any | None] = {} + restore_data: dict[MultihostBackupHost, Any | None] = {} for host in self.hosts: - if not isinstance(host, BaseBackupHost): + if not isinstance(host, MultihostBackupHost): continue restore_data[host] = host.backup_data @@ -93,7 +92,7 @@ def topology_teardown(self) -> None: try: for host, backup_data in self.backup_data.items(): - if not isinstance(host, BaseBackupHost): + if not isinstance(host, MultihostBackupHost): continue host.remove_backup(backup_data) From b437594957604fc6af680fd3f7b4126c36fb2402 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= Date: Fri, 6 Sep 2024 12:30:12 +0200 Subject: [PATCH 5/6] controllers: use built-in BackupTopologyController instead of custom implementation This functionality has been now included in pytest-mh. --- sssd_test_framework/topology_controllers.py | 109 +++----------------- 1 file changed, 17 insertions(+), 92 deletions(-) diff --git a/sssd_test_framework/topology_controllers.py b/sssd_test_framework/topology_controllers.py index a0c1829c..911037f8 100644 --- a/sssd_test_framework/topology_controllers.py +++ b/sssd_test_framework/topology_controllers.py @@ -1,9 +1,6 @@ from __future__ import annotations -from functools import partial, wraps -from typing import Any - -from pytest_mh import MultihostBackupHost, TopologyController +from pytest_mh import BackupTopologyController from pytest_mh.conn import ProcessResult from .config import SSSDMultihostConfig @@ -24,29 +21,7 @@ ] -def restore_vanilla_on_error(method): - """ - Restore or hosts to its original state if an exception occurs - during method execution. - - :param method: Method to decorate. - :type method: _type_ - :return: _description_ - :rtype: _type_ - """ - - @wraps(method) - def wrapper(self: BackupTopologyController, *args, **kwargs): - try: - return self._invoke_with_args(partial(method, self)) - except Exception: - self.restore_vanilla() - raise - - return wrapper - - -class BackupTopologyController(TopologyController[SSSDMultihostConfig]): +class ProvisionedBackupTopologyController(BackupTopologyController[SSSDMultihostConfig]): """ Provide basic restore functionality for topologies. """ @@ -54,92 +29,48 @@ class BackupTopologyController(TopologyController[SSSDMultihostConfig]): def __init__(self) -> None: super().__init__() - self.backup_data: dict[MultihostBackupHost, Any | None] = {} self.provisioned: bool = False def init(self, *args, **kwargs): super().init(*args, **kwargs) self.provisioned = self.name in self.multihost.provisioned_topologies - def restore(self, hosts: dict[MultihostBackupHost, Any | None]) -> None: - errors = [] - for host, backup_data in hosts.items(): - if not isinstance(host, MultihostBackupHost): - continue - - try: - host.restore(backup_data) - except Exception as e: - errors.append(e) - - if errors: - raise ExceptionGroup("Some hosts failed to restore to original state", errors) - - def restore_vanilla(self) -> None: - restore_data: dict[MultihostBackupHost, Any | None] = {} - - for host in self.hosts: - if not isinstance(host, MultihostBackupHost): - continue - - restore_data[host] = host.backup_data - - self.restore(restore_data) - def topology_teardown(self) -> None: if self.provisioned: return - try: - for host, backup_data in self.backup_data.items(): - if not isinstance(host, MultihostBackupHost): - continue - - host.remove_backup(backup_data) - except Exception: - # This is not that important, we can just ignore - pass - - self.restore_vanilla() + super().topology_teardown() def teardown(self) -> None: if self.provisioned: self.restore_vanilla() return - self.restore(self.backup_data) + super().teardown() -class ClientTopologyController(BackupTopologyController): +class ClientTopologyController(ProvisionedBackupTopologyController): """ Client Topology Controller. """ - def topology_teardown(self) -> None: - pass - - def teardown(self) -> None: - self.restore_vanilla() + pass -class LDAPTopologyController(BackupTopologyController): +class LDAPTopologyController(ProvisionedBackupTopologyController): """ LDAP Topology Controller. """ - def topology_teardown(self) -> None: - pass - - def teardown(self) -> None: - self.restore_vanilla() + pass -class IPATopologyController(BackupTopologyController): +class IPATopologyController(ProvisionedBackupTopologyController): """ IPA Topology Controller. """ - @restore_vanilla_on_error + @BackupTopologyController.restore_vanilla_on_error def topology_setup(self, client: ClientHost, ipa: IPAHost, nfs: NFSHost) -> None: if self.provisioned: self.logger.info(f"Topology '{self.name}' is already provisioned") @@ -159,17 +90,15 @@ def topology_setup(self, client: ClientHost, ipa: IPAHost, nfs: NFSHost) -> None client.conn.exec(["realm", "join", ipa.domain], input=ipa.adminpw) # Backup so we can restore to this state after each test - self.backup_data[ipa] = ipa.backup() - self.backup_data[client] = client.backup() - self.backup_data[nfs] = nfs.backup() + super().topology_setup() -class ADTopologyController(BackupTopologyController): +class ADTopologyController(ProvisionedBackupTopologyController): """ AD Topology Controller. """ - @restore_vanilla_on_error + @BackupTopologyController.restore_vanilla_on_error def topology_setup(self, client: ClientHost, provider: ADHost | SambaHost, nfs: NFSHost) -> None: if self.provisioned: self.logger.info(f"Topology '{self.name}' is already provisioned") @@ -185,9 +114,7 @@ def topology_setup(self, client: ClientHost, provider: ADHost | SambaHost, nfs: client.conn.exec(["realm", "join", provider.domain], input=provider.adminpw) # Backup so we can restore to this state after each test - self.backup_data[provider] = provider.backup() - self.backup_data[client] = client.backup() - self.backup_data[nfs] = nfs.backup() + super().topology_setup() class SambaTopologyController(ADTopologyController): @@ -198,12 +125,12 @@ class SambaTopologyController(ADTopologyController): pass -class IPATrustADTopologyController(BackupTopologyController): +class IPATrustADTopologyController(ProvisionedBackupTopologyController): """ IPA trust AD Topology Controller. """ - @restore_vanilla_on_error + @BackupTopologyController.restore_vanilla_on_error def topology_setup(self, client: ClientHost, ipa: IPAHost, trusted: ADHost | SambaHost) -> None: if self.provisioned: self.logger.info(f"Topology '{self.name}' is already provisioned") @@ -230,9 +157,7 @@ def topology_setup(self, client: ClientHost, ipa: IPAHost, trusted: ADHost | Sam client.conn.exec(["realm", "join", ipa.domain], input=ipa.adminpw) # Backup so we can restore to this state after each test - self.backup_data[ipa] = ipa.backup() - self.backup_data[trusted] = trusted.backup() - self.backup_data[client] = client.backup() + super().topology_setup() # If this command is run on freshly started containers, it is possible the IPA is not yet # fully ready to create the trust. It takes a while for it to start working. From 81a17f1599b7616d5c3245e7cdf900d5d5e88ab4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= Date: Thu, 19 Sep 2024 11:22:28 +0200 Subject: [PATCH 6/6] requirements: bump pytest-mh minimum version to 1.0.19 --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index bbaaec0f..e8af5a0c 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,4 @@ jc pytest python-ldap -pytest-mh >= 1.0.18 +pytest-mh >= 1.0.19