URL: https://github.com/freeipa/freeipa/pull/1932
Author: pvoborni
 Title: #1932: test_server_del: fix TestServerDel suite
Action: opened

PR body:
"""
TestLastServices suite was fixed in PR #1913 . This PR completes ticket 
https://pagure.io/freeipa/issue/7517

The first patch does a bit of optimization so that this suite can be run on 
smaller topology, e.g. https://github.com/freeipa/freeipa-pr-ci/pull/165

Second fixes test_removal_of_replica1_disconnects_domain_topology, 
test_removal_of_replica2_disconnects_ca_topology. Details in commit message.

Third fixes test_removal_of_master_disconnects_both_topologies. Details in 
commit message.

Temp commit will be removed on successful run.

I think that the suite is still bit unstable as 
test_ignore_topology_disconnect_replica{1,2} reinstall master in disconnected 
topology so bad things might in theory happen. It might not be hit though, so 
let's try if it will work as is. 

"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/1932/head:pr1932
git checkout pr1932
From b466ac6c4013bb4f6f460d2005a33dae8d9b9ff6 Mon Sep 17 00:00:00 2001
From: Petr Vobornik <pvobo...@redhat.com>
Date: Mon, 14 May 2018 19:44:34 +0200
Subject: [PATCH 1/4] test_server_del: remove usage of client from the test

Client was used only for issuing API commands. It was installed with
--server option so it pointed always to master. Feature wise running
the CLI command directly on master is the same. By removing usage of
client we can make the required host for the topology smaller and fit
on smaller CI runner.

https://pagure.io/freeipa/issue/7517

Signed-off-by: Petr Vobornik <pvobo...@redhat.com>
---
 ipatests/test_integration/test_server_del.py | 30 +++++++++++++---------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/ipatests/test_integration/test_server_del.py b/ipatests/test_integration/test_server_del.py
index c35bcb87d3..076e7585a8 100644
--- a/ipatests/test_integration/test_server_del.py
+++ b/ipatests/test_integration/test_server_del.py
@@ -71,15 +71,13 @@ def check_removal_disconnects_topology(
 
 class ServerDelBase(IntegrationTest):
     num_replicas = 2
-    num_clients = 1
+    num_clients = 0
     domain_level = DOMAIN_LEVEL_1
     topology = 'star'
 
     @classmethod
     def install(cls, mh):
         super(ServerDelBase, cls).install(mh)
-
-        cls.client = cls.clients[0]
         cls.replica1 = cls.replicas[0]
         cls.replica2 = cls.replicas[1]
 
@@ -103,19 +101,19 @@ def install(cls, mh):
         #                    \
         #   replica1------- replica2
 
-        tasks.create_segment(cls.client, cls.replica1, cls.replica2)
-        tasks.create_segment(cls.client, cls.replica1, cls.replica2,
+        tasks.create_segment(cls.master, cls.replica1, cls.replica2)
+        tasks.create_segment(cls.master, cls.replica1, cls.replica2,
                              suffix=CA_SUFFIX_NAME)
 
         # try to delete all relevant segment connecting master and replica1/2
         segment_name_fmt = '{p[0].hostname}-to-{p[1].hostname}'
         for domain_pair in permutations((cls.master, cls.replica2)):
             tasks.destroy_segment(
-                cls.client, segment_name_fmt.format(p=domain_pair))
+                cls.master, segment_name_fmt.format(p=domain_pair))
 
         for ca_pair in permutations((cls.master, cls.replica1)):
             tasks.destroy_segment(
-                cls.client, segment_name_fmt.format(p=ca_pair),
+                cls.master, segment_name_fmt.format(p=ca_pair),
                 suffix=CA_SUFFIX_NAME)
 
     def test_removal_of_nonexistent_master_raises_error(self):
@@ -125,7 +123,7 @@ def test_removal_of_nonexistent_master_raises_error(self):
         hostname = u'bogus-master.bogus.domain'
         err_message = "{}: server not found".format(hostname)
         tasks.assert_error(
-            tasks.run_server_del(self.client, hostname),
+            tasks.run_server_del(self.master, hostname),
             err_message,
             returncode=2
         )
@@ -136,7 +134,7 @@ def test_forced_removal_of_nonexistent_master(self):
         an error
         """
         hostname = u'bogus-master.bogus.domain'
-        result = tasks.run_server_del(self.client, hostname, force=True)
+        result = tasks.run_server_del(self.master, hostname, force=True)
         assert result.returncode == 0
         assert ('Deleted IPA server "{}"'.format(hostname) in
                 result.stdout_text)
@@ -150,7 +148,7 @@ def test_removal_of_replica1_disconnects_domain_topology(self):
         """
 
         check_removal_disconnects_topology(
-            self.client,
+            self.master,
             self.replica1.hostname,
             affected_suffixes=(DOMAIN_SUFFIX_NAME,)
         )
@@ -162,7 +160,7 @@ def test_removal_of_replica2_disconnects_ca_topology(self):
         """
 
         check_removal_disconnects_topology(
-            self.client,
+            self.master,
             self.replica2.hostname,
             affected_suffixes=(CA_SUFFIX_NAME,)
         )
@@ -173,7 +171,7 @@ def test_ignore_topology_disconnect_replica1(self):
         destroys master for good
         """
         check_master_removal(
-            self.client,
+            self.master,
             self.replica1.hostname,
             ignore_topology_disconnect=True
         )
@@ -188,7 +186,7 @@ def test_ignore_topology_disconnect_replica2(self):
         destroys master for good
         """
         check_master_removal(
-            self.client,
+            self.master,
             self.replica2.hostname,
             ignore_topology_disconnect=True
         )
@@ -202,7 +200,7 @@ def test_removal_of_master_disconnects_both_topologies(self):
         tests that master removal will now raise errors in both suffixes.
         """
         check_removal_disconnects_topology(
-            self.client,
+            self.master,
             self.master.hostname,
             affected_suffixes=(CA_SUFFIX_NAME, DOMAIN_SUFFIX_NAME)
         )
@@ -212,7 +210,7 @@ def test_removal_of_replica1(self):
         tests the removal of replica1 which should now pass without errors
         """
         check_master_removal(
-            self.client,
+            self.master,
             self.replica1.hostname
         )
 
@@ -221,7 +219,7 @@ def test_removal_of_replica2(self):
         tests the removal of replica2 which should now pass without errors
         """
         check_master_removal(
-            self.client,
+            self.master,
             self.replica2.hostname
         )
 

From 910ae0a27418ad361e3fa5e7b216de3e83f6a9db Mon Sep 17 00:00:00 2001
From: Petr Vobornik <pvobo...@redhat.com>
Date: Mon, 14 May 2018 20:18:47 +0200
Subject: [PATCH 2/4] test_server_del: pass domain level of removed replica
 explicitely

When server-del is called on master replica HTTP service entry is
removed. WHen it manages to replicate to the replica, uninstall_master,
then cannot find out what domain level it gets and so uses 1. Which
means that it doesn't pass --ignore_topology_disconnect option and
thus the uninstall fails.

https://pagure.io/freeipa/issue/7517

Signed-off-by: Petr Vobornik <pvobo...@redhat.com>
---
 ipatests/pytest_plugins/integration/tasks.py | 12 ++++++++++--
 ipatests/test_integration/test_server_del.py | 16 ++++++++--------
 2 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/ipatests/pytest_plugins/integration/tasks.py b/ipatests/pytest_plugins/integration/tasks.py
index ddd70302a0..cb436ad5dc 100644
--- a/ipatests/pytest_plugins/integration/tasks.py
+++ b/ipatests/pytest_plugins/integration/tasks.py
@@ -717,11 +717,19 @@ def kinit_admin(host, raiseonerr=True):
 
 
 def uninstall_master(host, ignore_topology_disconnect=True,
-                     ignore_last_of_role=True, clean=True):
+                     ignore_last_of_role=True, clean=True,
+                     domain_level=None):
     host.collect_log(paths.IPASERVER_UNINSTALL_LOG)
     uninstall_cmd = ['ipa-server-install', '--uninstall', '-U']
 
-    host_domain_level = domainlevel(host)
+    # in some cases, like uninstalling of replica already removed by
+    # server-del where removal of principals was replicated, dynamic
+    # domain resolution doesn't work as connecting to the API on that
+    # server might no longer work
+    if domain_level is None:
+        host_domain_level = domainlevel(host)
+    else:
+        host_domain_level = domain_level
 
     if ignore_topology_disconnect and host_domain_level != DOMAIN_LEVEL_0:
         uninstall_cmd.append('--ignore-topology-disconnect')
diff --git a/ipatests/test_integration/test_server_del.py b/ipatests/test_integration/test_server_del.py
index 076e7585a8..26fcd3ebb4 100644
--- a/ipatests/test_integration/test_server_del.py
+++ b/ipatests/test_integration/test_server_del.py
@@ -167,8 +167,8 @@ def test_removal_of_replica2_disconnects_ca_topology(self):
 
     def test_ignore_topology_disconnect_replica1(self):
         """
-        tests that removal of replica1 with '--ignore-topology-disconnect'
-        destroys master for good
+        tests that server-del of replica1 with '--ignore-topology-disconnect'
+        passes
         """
         check_master_removal(
             self.master,
@@ -177,13 +177,13 @@ def test_ignore_topology_disconnect_replica1(self):
         )
 
         # reinstall the replica
-        tasks.uninstall_master(self.replica1)
+        tasks.uninstall_master(self.replica1, domain_level=DOMAIN_LEVEL_1)
         tasks.install_replica(self.master, self.replica1, setup_ca=True)
 
     def test_ignore_topology_disconnect_replica2(self):
         """
-        tests that removal of replica2 with '--ignore-topology-disconnect'
-        destroys master for good
+        tests that server-del of replica2 with '--ignore-topology-disconnect'
+        passes
         """
         check_master_removal(
             self.master,
@@ -192,7 +192,7 @@ def test_ignore_topology_disconnect_replica2(self):
         )
 
         # reinstall the replica
-        tasks.uninstall_master(self.replica2)
+        tasks.uninstall_master(self.replica2, domain_level=DOMAIN_LEVEL_1)
         tasks.install_replica(self.master, self.replica2, setup_ca=True)
 
     def test_removal_of_master_disconnects_both_topologies(self):
@@ -207,7 +207,7 @@ def test_removal_of_master_disconnects_both_topologies(self):
 
     def test_removal_of_replica1(self):
         """
-        tests the removal of replica1 which should now pass without errors
+        tests the server-del of replica1 which should now pass without errors
         """
         check_master_removal(
             self.master,
@@ -216,7 +216,7 @@ def test_removal_of_replica1(self):
 
     def test_removal_of_replica2(self):
         """
-        tests the removal of replica2 which should now pass without errors
+        tests the server-del of replica2 which should now pass without errors
         """
         check_master_removal(
             self.master,

From ea2f1f371705a5d74930399c9de44afd63325cb6 Mon Sep 17 00:00:00 2001
From: Petr Vobornik <pvobo...@redhat.com>
Date: Tue, 15 May 2018 10:57:39 +0200
Subject: [PATCH 3/4] test_server_del: install second DNS in topology to allow
 other testing

test_removal_of_master_disconnects_both_topologies failed on server-del
check where it checked for removing the only DNS server in topology.
This is tested in suite TestLastServices so DNS server can be installed to
actually run the topology disconnect check as intended in
test_removal_of_master_disconnects_both_topologies

https://pagure.io/freeipa/issue/7517

Signed-off-by: Petr Vobornik <pvobo...@redhat.com>
---
 ipatests/test_integration/test_server_del.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ipatests/test_integration/test_server_del.py b/ipatests/test_integration/test_server_del.py
index 26fcd3ebb4..a0ce053069 100644
--- a/ipatests/test_integration/test_server_del.py
+++ b/ipatests/test_integration/test_server_del.py
@@ -115,6 +115,7 @@ def install(cls, mh):
             tasks.destroy_segment(
                 cls.master, segment_name_fmt.format(p=ca_pair),
                 suffix=CA_SUFFIX_NAME)
+        tasks.install_dns(cls.replica1)
 
     def test_removal_of_nonexistent_master_raises_error(self):
         """

From 211675424a1366d52a86e3246787ef2a46fde4ad Mon Sep 17 00:00:00 2001
From: Petr Vobornik <pvobo...@redhat.com>
Date: Mon, 14 May 2018 20:37:55 +0200
Subject: [PATCH 4/4] temp commit: test_server_del::TestServerDel

Signed-off-by: Petr Vobornik <pvobo...@redhat.com>
---
 .freeipa-pr-ci.yaml | 185 ++++------------------------------------------------
 1 file changed, 11 insertions(+), 174 deletions(-)

diff --git a/.freeipa-pr-ci.yaml b/.freeipa-pr-ci.yaml
index 88d34d58fc..d4d77e3371 100644
--- a/.freeipa-pr-ci.yaml
+++ b/.freeipa-pr-ci.yaml
@@ -3,15 +3,10 @@ topologies:
     name: build
     cpu: 2
     memory: 3800
-  master_1repl: &master_1repl
-    name: master_1repl
+  master_2repl_1client: &master_2repl_1client
+    name: master_2repl_1client
     cpu: 4
-    memory: 5750
-  master_1repl_1client: &master_1repl_1client
-    name: master_1repl_1client
-    cpu: 4
-    memory: 6700
-
+    memory: 9100
 jobs:
   fedora-27/build:
     requires: []
@@ -27,183 +22,25 @@ jobs:
         timeout: 1800
         topology: *build
 
-  fedora-27/simple_replication:
-    requires: [fedora-27/build]
-    priority: 50
-    job:
-      class: RunPytest
-      args:
-        build_url: '{fedora-27/build_url}'
-        test_suite: test_integration/test_simple_replication.py
-        template: *ci-master-f27
-        timeout: 3600
-        topology: *master_1repl
-
-  fedora-27/caless:
-    requires: [fedora-27/build]
-    priority: 50
-    job:
-      class: RunPytest
-      args:
-        build_url: '{fedora-27/build_url}'
-        test_suite: test_integration/test_caless.py::TestServerReplicaCALessToCAFull
-        template: *ci-master-f27
-        timeout: 3600
-        topology: *master_1repl
-
-  fedora-27/external_ca:
-    requires: [fedora-27/build]
-    priority: 50
-    job:
-      class: RunPytest
-      args:
-        build_url: '{fedora-27/build_url}'
-        test_suite: test_integration/test_external_ca.py::TestExternalCA test_integration/test_external_ca.py::TestSelfExternalSelf test_integration/test_external_ca.py::TestExternalCAInstall
-        template: *ci-master-f27
-        timeout: 3600
-        topology: *master_1repl
-
-  fedora-27/test_topologies:
-    requires: [fedora-27/build]
-    priority: 50
-    job:
-      class: RunPytest
-      args:
-        build_url: '{fedora-27/build_url}'
-        test_suite: test_integration/test_topologies.py
-        template: *ci-master-f27
-        timeout: 3600
-        topology: *master_1repl
-
-  fedora-27/test_sudo:
-    requires: [fedora-27/build]
-    priority: 50
-    job:
-      class: RunPytest
-      args:
-        build_url: '{fedora-27/build_url}'
-        test_suite: test_integration/test_sudo.py
-        template: *ci-master-f27
-        timeout: 3600
-        topology: *master_1repl_1client
-
-  fedora-27/test_ipa_cli:
-    requires: [fedora-27/build]
-    priority: 50
-    job:
-      class: RunPytest
-      args:
-        build_url: '{fedora-27/build_url}'
-        test_suite: test_integration/test_ipa_cli.py
-        template: *ci-master-f27
-        timeout: 3600
-        topology: *master_1repl
-
-  fedora-27/test_kerberos_flags:
-    requires: [fedora-27/build]
-    priority: 50
-    job:
-      class: RunPytest
-      args:
-        build_url: '{fedora-27/build_url}'
-        test_suite: test_integration/test_kerberos_flags.py
-        template: *ci-master-f27
-        timeout: 3600
-        topology: *master_1repl_1client
-
-  fedora-27/test_http_kdc_proxy:
-    requires: [fedora-27/build]
-    priority: 50
-    job:
-      class: RunPytest
-      args:
-        build_url: '{fedora-27/build_url}'
-        test_suite: test_integration/test_http_kdc_proxy.py
-        template: *ci-master-f27
-        timeout: 3600
-        topology: *master_1repl_1client
-
-  fedora-27/test_forced_client_enrolment:
-    requires: [fedora-27/build]
-    priority: 50
-    job:
-      class: RunPytest
-      args:
-        build_url: '{fedora-27/build_url}'
-        test_suite: test_integration/test_forced_client_reenrollment.py
-        template: *ci-master-f27
-        timeout: 3600
-        topology: *master_1repl_1client
-
-  fedora-27/test_advise:
-    requires: [fedora-27/build]
-    priority: 50
-    job:
-      class: RunPytest
-      args:
-        build_url: '{fedora-27/build_url}'
-        test_suite: test_integration/test_advise.py
-        template: *ci-master-f27
-        timeout: 3600
-        topology: *master_1repl
-
-  fedora-27/test_testconfig:
-    requires: [fedora-27/build]
-    priority: 50
-    job:
-      class: RunPytest
-      args:
-        build_url: '{fedora-27/build_url}'
-        test_suite: test_integration/test_testconfig.py
-        template: *ci-master-f27
-        timeout: 3600
-        topology: *master_1repl
-
-  fedora-27/test_service_permissions:
+  fedora-27/server_del:
     requires: [fedora-27/build]
     priority: 50
     job:
       class: RunPytest
       args:
         build_url: '{fedora-27/build_url}'
-        test_suite: test_integration/test_service_permissions.py
+        test_suite: test_integration/test_server_del.py::TestServerDel
         template: *ci-master-f27
-        timeout: 3600
-        topology: *master_1repl
-
-  fedora-27/test_netgroup:
+        timeout: 7200
+        topology: *master_2repl_1client
+  fedora-27/server_del_last_services:
     requires: [fedora-27/build]
     priority: 50
     job:
       class: RunPytest
       args:
         build_url: '{fedora-27/build_url}'
-        test_suite: test_integration/test_netgroup.py
+        test_suite: test_integration/test_server_del.py::TestLastServices
         template: *ci-master-f27
-        timeout: 3600
-        topology: *master_1repl
-
-  fedora-27/test_vault:
-    requires: [fedora-27/build]
-    priority: 50
-    job:
-      class: RunPytest
-      args:
-        build_url: '{fedora-27/build_url}'
-        test_suite: test_integration/test_vault.py
-        template: *ci-master-f27
-        timeout: 3600
-        topology: *master_1repl
-
-  fedora-27/test_authconfig:
-    requires: [fedora-27/build]
-    priority: 50
-    job:
-      class: RunPytest
-      args:
-        build_url: '{fedora-27/build_url}'
-        test_suite: test_integration/test_authselect.py
-        template: *ci-master-f27
-        timeout: 3600
-        topology: *master_1repl_1client
-
+        timeout: 7200
+        topology: *master_2repl_1client
_______________________________________________
FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org
To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org

Reply via email to