URL: https://github.com/freeipa/freeipa/pull/1943 Author: pvoborni Title: #1943: [Backport][ipa-4-6] Fix test_server_del::TestLastServices Action: opened
PR body: """ This PR was opened manually because PR #1913 was pushed to master and backport to ipa-4-6 is required. """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/1943/head:pr1943 git checkout pr1943
From bfd3fa34e5fff5a4d463479017e99f7e169389d6 Mon Sep 17 00:00:00 2001 From: Petr Vobornik <pvobo...@redhat.com> Date: Thu, 10 May 2018 10:02:16 +0200 Subject: [PATCH 1/2] server-del do not return early if CA renewal master cannot be changed Early return prevented adding last warning message in the method: "Ignoring these warnings and proceeding with removal" And thus `check_master_removal` in `test_server_del` did not work. https://pagure.io/freeipa/issue/7517 Signed-off-by: Petr Vobornik <pvobo...@redhat.com> Reviewed-By: Florence Blanc-Renaud <fren...@redhat.com> --- ipaserver/plugins/server.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/ipaserver/plugins/server.py b/ipaserver/plugins/server.py index 59e611fc4c..4ea6f5b4b4 100644 --- a/ipaserver/plugins/server.py +++ b/ipaserver/plugins/server.py @@ -523,16 +523,13 @@ def handler(msg, ignore_last_of_role): "leave your installation without a CA."), ignore_last_of_role) + # change the renewal master if there is other master with CA if ca_renewal_master == hostname: other_cas = [ca for ca in ca_servers if ca != hostname] - # if this is the last CA there is no other server to become - # renewal master - if not other_cas: - return - - self.api.Command.config_mod( - ca_renewal_master_server=other_cas[0]) + if other_cas: + self.api.Command.config_mod( + ca_renewal_master_server=other_cas[0]) if ignore_last_of_role: self.add_message( From a575151434dbd4ba8349c7788e426c828e7c5e67 Mon Sep 17 00:00:00 2001 From: Petr Vobornik <pvobo...@redhat.com> Date: Thu, 10 May 2018 12:53:20 +0200 Subject: [PATCH 2/2] Fix test_server_del::TestLastServices The reason why the test started to fail is probably commit be3ad1e where the checks were reordered. TestLastServices relies on execution of tests in a specific order. So it fails given that checks were changed but tests weren't. Given that master is installed with DNS and CA and replica with anything and given that checks in server-del command are in order: DNS, DNSSec, CA, KRA then the test should be something like: * install master (with DNS, CA) * install replica * test test_removal_of_master_raises_error_about_last_dns * test_install_dns_on_replica1_and_dnssec_on_master (installing DNS and DNSSec will allow DNSSec check) * test_removal_of_master_raises_error_about_dnssec * test_disable_dnssec_on_master (will allow CA check) * test_removal_of_master_raises_error_about_last_ca * test_forced_removal_of_master https://pagure.io/freeipa/issue/7517 Signed-off-by: Petr Vobornik <pvobo...@redhat.com> Reviewed-By: Florence Blanc-Renaud <fren...@redhat.com> --- ipatests/test_integration/test_server_del.py | 43 +++++++++++++++++----------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/ipatests/test_integration/test_server_del.py b/ipatests/test_integration/test_server_del.py index f5738a3a59..c35bcb87d3 100644 --- a/ipatests/test_integration/test_server_del.py +++ b/ipatests/test_integration/test_server_del.py @@ -241,23 +241,6 @@ def install(cls, mh): cls.topology, cls.master, cls.replicas, [], domain_level=cls.domain_level, setup_replica_cas=False) - def test_removal_of_master_raises_error_about_last_ca(self): - """ - test that removal of master fails on the last - """ - tasks.assert_error( - tasks.run_server_del(self.replicas[0], self.master.hostname), - "Deleting this server is not allowed as it would leave your " - "installation without a CA.", - 1 - ) - - def test_install_ca_on_replica1(self): - """ - Install CA on replica so that we can test DNS-related checks - """ - tasks.install_ca(self.replicas[0], domain_level=self.domain_level) - def test_removal_of_master_raises_error_about_last_dns(self): """ Now server-del should complain about the removal of last DNS server @@ -291,6 +274,32 @@ def test_removal_of_master_raises_error_about_dnssec(self): 1 ) + def test_disable_dnssec_on_master(self): + """ + Disable DNSSec master so that it is not tested anymore. Normal way + would be to move the DNSSec master to replica, but that is tested in + DNSSec tests. + """ + args = [ + "ipa-dns-install", + "--disable-dnssec-master", + "--forwarder", self.master.config.dns_forwarder, + "--force", + "-U", + ] + self.master.run_command(args) + + def test_removal_of_master_raises_error_about_last_ca(self): + """ + test that removal of master fails on the last + """ + tasks.assert_error( + tasks.run_server_del(self.replicas[0], self.master.hostname), + "Deleting this server is not allowed as it would leave your " + "installation without a CA.", + 1 + ) + def test_forced_removal_of_master(self): """ Tests that we can still force remove the master using
_______________________________________________ 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://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/freeipa-devel@lists.fedorahosted.org/message/44VNVIJ7S2XKHURPOYC5Y5NVRYML6RSM/
