URL: https://github.com/freeipa/freeipa/pull/4499
Author: fcami
 Title: #4499: [Backport][ipa-4-8] ipatests: move ipa_backup to tasks
Action: opened

PR body:
"""
MANUAL BACKPORT of https://github.com/freeipa/freeipa/pull/4489 (straight 
cherry-pick, no merge required)

* tasks had an ipa_backup() method that was not used anywhere.
* test_backup_and_restore had a backup() method that used to return
  both the path to the backup and the whole result from run_command ;
  The path to the backup can be determined from the result.

Clean up:
* move test_backup_and_restore.backup to tasks.ipa_backup, replacing
  the unused method.
* add tasks.get_backup_dir(host) which runs ipa-backup on host and
  returns the path to the backup directory.
* adjust test_backup_and_restore and test_replica_promotion.

Related: https://pagure.io/freeipa/issue/8217
Signed-off-by: François Cami <fc...@redhat.com>
Reviewed-By: Michal Polovka <mpolo...@redhat.com>
Reviewed-By: Rob Crittenden <rcrit...@redhat.com>
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/4499/head:pr4499
git checkout pr4499
From 26e05a7c93f634ffe699e228364fffeff734fc68 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fran=C3=A7ois=20Cami?= <fc...@redhat.com>
Date: Thu, 2 Apr 2020 16:26:11 +0200
Subject: [PATCH] ipatests: move ipa_backup to tasks
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* tasks had an ipa_backup() method that was not used anywhere.
* test_backup_and_restore had a backup() method that used to return
  both the path to the backup and the whole result from run_command ;
  The path to the backup can be determined from the result.

Clean up:
* move test_backup_and_restore.backup to tasks.ipa_backup, replacing
  the unused method.
* add tasks.get_backup_dir(host) which runs ipa-backup on host and
  returns the path to the backup directory.
* adjust test_backup_and_restore and test_replica_promotion.

Related: https://pagure.io/freeipa/issue/8217
Signed-off-by: François Cami <fc...@redhat.com>
Reviewed-By: Michal Polovka <mpolo...@redhat.com>
Reviewed-By: Rob Crittenden <rcrit...@redhat.com>
---
 ipatests/pytest_ipa/integration/tasks.py      | 39 ++++++++++--
 .../test_backup_and_restore.py                | 61 +++++--------------
 .../test_replica_promotion.py                 |  3 +-
 3 files changed, 50 insertions(+), 53 deletions(-)

diff --git a/ipatests/pytest_ipa/integration/tasks.py b/ipatests/pytest_ipa/integration/tasks.py
index f19728cc62..9838c8ce61 100755
--- a/ipatests/pytest_ipa/integration/tasks.py
+++ b/ipatests/pytest_ipa/integration/tasks.py
@@ -1491,11 +1491,40 @@ def resolve_record(nameserver, query, rtype="SOA", retry=True, timeout=100):
         time.sleep(1)
 
 
-def ipa_backup(master):
-    result = master.run_command(["ipa-backup"])
-    path_re = re.compile("^Backed up to (?P<backup>.*)$", re.MULTILINE)
-    matched = path_re.search(result.stdout_text + result.stderr_text)
-    return matched.group("backup")
+def ipa_backup(host, disable_role_check=False, raiseonerr=True):
+    """Run backup on host and return the run_command result.
+    """
+    cmd = ['ipa-backup', '-v']
+    if disable_role_check:
+        cmd.append('--disable-role-check')
+    result = host.run_command(cmd, raiseonerr=raiseonerr)
+
+    # Test for ticket 7632: check that services are restarted
+    # before the backup is compressed
+    pattern = r'.*{}.*Starting IPA service.*'.format(paths.GZIP)
+    if (re.match(pattern, result.stderr_text, re.DOTALL)):
+        raise AssertionError('IPA Services are started after compression')
+
+    return result
+
+
+def get_backup_dir(host, raiseonerr=True):
+    """Wrapper around ipa_backup: returns the backup directory.
+    """
+    result = ipa_backup(host, raiseonerr)
+
+    # Get the backup location from the command's output
+    for line in result.stderr_text.splitlines():
+        prefix = 'ipaserver.install.ipa_backup: INFO: Backed up to '
+        if line.startswith(prefix):
+            backup_path = line[len(prefix):].strip()
+            logger.info('Backup path for %s is %s', host.hostname, backup_path)
+            return backup_path
+    else:
+        if raiseonerr:
+            raise AssertionError('Backup directory not found in output')
+        else:
+            return None
 
 
 def ipa_restore(master, backup_path):
diff --git a/ipatests/test_integration/test_backup_and_restore.py b/ipatests/test_integration/test_backup_and_restore.py
index c8d4278ba0..f0b08f0936 100644
--- a/ipatests/test_integration/test_backup_and_restore.py
+++ b/ipatests/test_integration/test_backup_and_restore.py
@@ -162,35 +162,6 @@ def restore_checker(host):
         assert_func(expected, got)
 
 
-def backup(host, disable_role_check=False, raiseonerr=True):
-    """Run backup on host and return a tuple:
-    (path to the backup directory, run_command result)
-    """
-    cmd = ['ipa-backup', '-v']
-    if disable_role_check:
-        cmd.append('--disable-role-check')
-    result = host.run_command(cmd, raiseonerr=raiseonerr)
-
-    # Test for ticket 7632: check that services are restarted
-    # before the backup is compressed
-    pattern = r'.*{}.*Starting IPA service.*'.format(paths.GZIP)
-    if (re.match(pattern, result.stderr_text, re.DOTALL)):
-        raise AssertionError('IPA Services are started after compression')
-
-    # Get the backup location from the command's output
-    for line in result.stderr_text.splitlines():
-        prefix = 'ipaserver.install.ipa_backup: INFO: Backed up to '
-        if line.startswith(prefix):
-            backup_path = line[len(prefix):].strip()
-            logger.info('Backup path for %s is %s', host.hostname, backup_path)
-            return backup_path, result
-    else:
-        if raiseonerr:
-            raise AssertionError('Backup directory not found in output')
-        else:
-            return None, result
-
-
 @pytest.yield_fixture(scope="function")
 def cert_sign_request(request):
     master = request.instance.master
@@ -215,7 +186,7 @@ class TestBackupAndRestore(IntegrationTest):
     def test_full_backup_and_restore(self):
         """backup, uninstall, restore"""
         with restore_checker(self.master):
-            backup_path, unused = backup(self.master)
+            backup_path = tasks.get_backup_dir(self.master)
 
             self.master.run_command(['ipa-server-install',
                                      '--uninstall',
@@ -241,7 +212,7 @@ def test_full_backup_and_restore(self):
     def test_full_backup_and_restore_with_removed_users(self):
         """regression test for https://fedorahosted.org/freeipa/ticket/3866""";
         with restore_checker(self.master):
-            backup_path, unused = backup(self.master)
+            backup_path = tasks.get_backup_dir(self.master)
 
             logger.info('Backup path for %s is %s', self.master, backup_path)
 
@@ -267,7 +238,7 @@ def test_full_backup_and_restore_with_removed_users(self):
     def test_full_backup_and_restore_with_selinux_booleans_off(self):
         """regression test for https://fedorahosted.org/freeipa/ticket/4157""";
         with restore_checker(self.master):
-            backup_path, unused = backup(self.master)
+            backup_path = tasks.get_backup_dir(self.master)
 
             logger.info('Backup path for %s is %s', self.master, backup_path)
 
@@ -320,7 +291,7 @@ def _full_backup_restore_with_DNS_zone(self, reinstall=False):
 
             tasks.resolve_record(self.master.ip, self.example_test_zone)
 
-            backup_path, unused = backup(self.master)
+            backup_path = tasks.get_backup_dir(self.master)
 
             self.master.run_command(['ipa-server-install',
                                      '--uninstall',
@@ -396,7 +367,7 @@ def _full_backup_and_restore_with_DNSSEC_zone(self, reinstall=False):
                     self.master.ip, self.example_test_zone)
             ), "Zone is not signed"
 
-            backup_path, unused = backup(self.master)
+            backup_path = tasks.get_backup_dir(self.master)
 
             self.master.run_command(['ipa-server-install',
                                      '--uninstall',
@@ -478,7 +449,7 @@ def _full_backup_restore_with_vault(self, reinstall=False):
                 "--password", self.vault_password,
             ])
 
-            backup_path, unused = backup(self.master)
+            backup_path = tasks.get_backup_dir(self.master)
 
             self.master.run_command(['ipa-server-install',
                                      '--uninstall',
@@ -597,7 +568,7 @@ def test_full_backup_and_restore_with_replica(self, cert_sign_request):
         tasks.user_add(self.replica1, 'test1_replica')
 
         with restore_checker(self.master):
-            backup_path, unused = backup(self.master)
+            backup_path = tasks.get_backup_dir(self.master)
 
             # change data after backup
             self.master.run_command(['ipa', 'user-del', 'test1_master'])
@@ -721,7 +692,7 @@ def install(cls, mh):
     def test_userroot_ldif_files_ownership_and_permission(self):
         """backup, uninstall, restore, backup"""
         tasks.install_master(self.master)
-        backup_path, unused = backup(self.master)
+        backup_path = tasks.get_backup_dir(self.master)
 
         self.master.run_command(['ipa-server-install',
                                  '--uninstall',
@@ -787,7 +758,7 @@ class TestBackupAndRestoreDMPassword(IntegrationTest):
     def test_restore_bad_dm_password(self):
         """backup, uninstall, restore, wrong DM password (expect failure)"""
         with restore_checker(self.master):
-            backup_path, unused = backup(self.master)
+            backup_path = tasks.get_backup_dir(self.master)
 
             # No uninstall, just pure restore, the only case where
             # prompting for the DM password matters.
@@ -801,7 +772,7 @@ def test_restore_dirsrv_not_running(self):
 
         # Flying blind without the restore_checker so we can have
         # an error thrown when dirsrv is down.
-        backup_path, unused = backup(self.master)
+        backup_path = tasks.get_backup_dir(self.master)
 
         self.master.run_command(['ipactl', 'stop'])
 
@@ -836,7 +807,7 @@ def test_replica_install_after_restore(self):
         check_replication(master, replica1, "testuser1")
 
         # backup master.
-        backup_path, unused = backup(master)
+        backup_path = tasks.get_backup_dir(self.master)
 
         suffix = ipautil.realm_to_suffix(master.domain.realm)
         suffix = escape_dn_chars(str(suffix))
@@ -900,7 +871,7 @@ def test_restore_trust_pkg_before_restore(self):
                                  '--add-sids'])
 
         with restore_checker(self.master):
-            backup_path, unused = backup(self.master)
+            backup_path = tasks.get_backup_dir(self.master)
 
             self.master.run_command(['ipa-server-install',
                                      '--uninstall',
@@ -941,20 +912,18 @@ def install(cls, mh):
         tasks.install_master(cls.master, setup_dns=True)
 
     def _check_rolecheck_backup_failure(self, host):
-        unused, result = backup(host, raiseonerr=False)
+        result = tasks.ipa_backup(host, raiseonerr=False)
         assert result.returncode == 1
         assert "Error: Local roles" in result.stderr_text
         assert "do not match globally used roles" in result.stderr_text
 
-        unused, result = backup(
-            host, disable_role_check=True
-        )
+        result = tasks.ipa_backup(host, disable_role_check=True)
         assert result.returncode == 0
         assert "Warning: Local roles" in result.stderr_text
         assert "do not match globally used roles" in result.stderr_text
 
     def _check_rolecheck_backup_success(self, host):
-        unused, result = backup(host)
+        result = tasks.ipa_backup(host)
         assert result.returncode == 0
         assert "Local roles match globally used roles, proceeding." \
             in result.stderr_text
diff --git a/ipatests/test_integration/test_replica_promotion.py b/ipatests/test_integration/test_replica_promotion.py
index c17a3d137e..88a49bd34b 100644
--- a/ipatests/test_integration/test_replica_promotion.py
+++ b/ipatests/test_integration/test_replica_promotion.py
@@ -20,7 +20,6 @@
     DOMAIN_LEVEL_1, IPA_CA_NICKNAME, CA_SUFFIX_NAME)
 from ipaplatform.paths import paths
 from ipapython import certdb
-from ipatests.test_integration.test_backup_and_restore import backup
 from ipatests.test_integration.test_dns_locations import (
     resolve_records_from_server, IPA_DEFAULT_MASTER_SRV_REC
 )
@@ -928,7 +927,7 @@ def test_hidden_replica_backup_and_restore(self):
         """
         self._check_server_role(self.replicas[0], 'hidden')
         # backup
-        backup_path, unused = backup(self.replicas[0])
+        backup_path = tasks.get_backup_dir(self.replicas[0])
         # uninstall
         tasks.uninstall_master(self.replicas[0])
         # restore
_______________________________________________
FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org
To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/freeipa-devel@lists.fedorahosted.org

Reply via email to