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