Hi Martin,

Thanks for the review! Both patches were updated.

On 07/28/2016 04:11 PM, Martin Basti wrote:


On 08.07.2016 15:41, Oleg Fayans wrote:
Hi Martin,

Thanks for the review!

On 07/08/2016 02:18 PM, Martin Basti wrote:


On 27.06.2016 13:53, Oleg Fayans wrote:
Hi guys,

Is there a chance the patches NN 0047.1 and 0048.1 get reviewed before
4.4 release? They cover a good part of the Managed Topology 4.4
feature.

On 06/17/2016 11:18 AM, Oleg Fayans wrote:
One more test was added to the patch-0048

On 06/17/2016 09:43 AM, Oleg Fayans wrote:
Fixed a bug in the previous patch, automated 2 more testcases from
http://www.freeipa.org/page/V4/Manage_replication_topology_4_4/Test_Plan



On 06/16/2016 04:46 PM, Oleg Fayans wrote:






IIUC, this will turn off the machine completely, how is cleanup done
then.  AFAIK our tests cannot turn on machine again and run cleanup, so
you will not be able to run more tests on the same topology without
manual cleanup and manual start.

+        replica = self.replicas[0]
+        replica.run_command(['poweroff'])

IMO would be better to just call 'ipactl stop' instead of 'poweroff'

Agreed! Fixed.


Martin^2




*Automated ipa-replica-manage del tests*

1)
+        replica.run_command(['ipactl', 'stop'])
+        time.sleep(3)

Why do you need sleep here?

Removed, it was left from the old "poweroff" approach



2)
+        ruvid_re = re.compile(".*%s:389: (\d+).*" % replica.hostname)
+        replica_ruvs = ruvid_re.findall(result.stdout_text)
+        master.run_command(['ipa-replica-manage', 'clean-ruv', 'f',
+                            '-p', master.config.dirman_password,
+                            replica_ruvs[0]])

Because you are using re.findall(), without any match you will receive
IndexError here replica_ruvs[0]. IMO it deserves assert before

Implemented the assert which checks that the output contains enough replica RUVs


3)
assert(replica.hostname in result1.stdout_text)

I think that this is error prone. What if there is just error 'could not
connect to replica <replica hostname>', or something similar. instead of
listing/cleaning/whatever operation was executed. I think that it should
be more specific regexp than just finding a replica name substring  (Yes
In IPA we dont always print error so stderr)

I'm not sure, but probably there might be cases when non critical error
happen and exist status is still 0

Agree. Implemented a regex-based search


4)

+        replica.run_command(['poweroff'])
+        time.sleep(3)

There should not be poweroff, probably sleep could be removed too.

Gone



  *   Automated clean-ruv subcommand test*

1) PEP8, 2 new lines expected
./ipatests/test_integration/test_topology.py:163:1: E302 expected 2
blank lines, found 0
./ipatests/test_integration/test_topology.py:182:80: E501 line too long
(85 > 79 characters)

Fixed



2)
I dont like doing assert just with count of occurences of substring in
STDOUT, would be possible to improve this somehow?

Maybe, but frankly, I don't see how. In this case we are making sure that both simple and CA-specific RUVs of a replica are displayed. The format of the output is strict:
Replica Update Vectors:
replica1_hostname:389: RUV_id
replica2_hostname:389: RUV_id
Certificate Server Replica Update Vectors:
replica1_hostname:389: RUV_id
replica2_hostname:389: RUV_id
If we do not see 2 occurrences of the replica hostname than definitely something went wrong


3)
I'm not sure if clean-ruv is instant operations or there is some magic
happening in background (we have abort-clean-ruv). Maybe some sleep
should be there, but this needs investigation.

+        assert(replica.hostname in result2.stdout_text), (
+            "The wrong RUV was deleted")
+        result3 = master.run_command(['ipa-replica-manage', 'list-ruv',
+                                      '-p', master.config.dirman_password])
+        assert(result3.stdout_text.count(replica.hostname) == 1), (
+            "CA RUV of the replica is still displayed")


Based on my discussion with Stanislav Laznicka, I understood that by default clean-ruv does not return the shell until the operation is finished. You can force dropping into the shell by pressing CTRL+C, in which case the background job will still be running, but this is not the default behavior

--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From 1003531981483bc63ebdad56139ac6815026b711 Mon Sep 17 00:00:00 2001
From: Oleg Fayans <ofay...@redhat.com>
Date: Wed, 3 Aug 2016 09:17:10 +0200
Subject: [PATCH] Automated clean-ruv subcommand test

https://fedorahosted.org/freeipa/ticket/5964
---
 ipatests/test_integration/test_topology.py | 53 ++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/ipatests/test_integration/test_topology.py b/ipatests/test_integration/test_topology.py
index e956563c27eafd84deed5786274a73d0d3594642..87f78565e133de66251476e2c9d6ccbb368d000d 100644
--- a/ipatests/test_integration/test_topology.py
+++ b/ipatests/test_integration/test_topology.py
@@ -159,3 +159,56 @@ class TestTopologyOptions(IntegrationTest):
         assert err == "", err
         returncode, error = tasks.destroy_segment(self.master, "%s-to-%s" % replicas)
         assert returncode == 0, error
+
+
+@pytest.mark.skipif(config.domain_level == 0, reason=reasoning)
+class TestCASpecificRUVs(IntegrationTest):
+    num_replicas = 2
+    topology = 'star'
+    username = 'testuser'
+    user_firstname = 'test'
+    user_lastname = 'user'
+
+    def test_ca_specific_ruvs(self):
+        """
+        http://www.freeipa.org/page/V4/Manage_replication_topology_4_4/Test_Plan
+        #Test_case:_.2A-ruv_subcommands_of_ipa-replica-manage_are_extended
+        _to_handle_CA-specific_RUVs
+        """
+        replica = self.replicas[0]
+        master = self.master
+        res = master.run_command(['ipa-replica-manage', 'list-ruv', '-p',
+                                  master.config.dirman_password]).stdout_text
+        assert(res.count(replica.hostname) == 2 and
+               "Certificate Server Replica Update Vectors" in res), (
+            "CA-specific RUVs are not displayed")
+        ruvid_re = re.compile(".*%s:389: (\d+).*" % replica.hostname)
+        replica_ruvs = ruvid_re.findall(res)
+        # Find out the number of RUVids
+        assert(len(replica_ruvs) == 2), (
+            "The output should display 2 RUV ids of the selected replica")
+        result1 = master.run_command(['ipa-replica-manage', 'clean-ruv',
+                                      replica_ruvs[1], '-p',
+                                      master.config.dirman_password, '-f'])
+        assert(replica.hostname in result1.stdout_text), (
+            "The wrong RUV was deleted")
+        result2 = master.run_command(['ipa-replica-manage', 'list-ruv',
+                                      '-p', master.config.dirman_password])
+        assert(result2.stdout_text.count(replica.hostname) == 1), (
+            "CA RUV of the replica is still displayed")
+        result3 = master.run_command(['ipa-replica-manage', 'clean-ruv',
+                                      replica_ruvs[0], '-p',
+                                      master.config.dirman_password, '-f'])
+        assert(replica.hostname in result3.stdout_text), (
+            "The wrong RUV was deleted")
+        result4 = master.run_command(['ipa-replica-manage', 'list-ruv',
+                                      '-p', master.config.dirman_password])
+        assert(replica.hostname not in result4.stdout_text), (
+            "replica's RUV is still displayed")
+        master.run_command(['ipa', 'user-add', self.username,
+                            "--first=%s" % self.user_firstname,
+                            "--last=%s" % self.user_lastname])
+        result5 = replica.run_command(['ipa', 'user-show', self.username],
+                                      raiseonerr=False)
+        assert(result5.returncode > 0), (
+            'Replication still works after all RUVs were deleted')
-- 
1.8.3.1

From 132c246383fd93f88c52e7c1143c6c096125a15e Mon Sep 17 00:00:00 2001
From: Oleg Fayans <ofay...@redhat.com>
Date: Wed, 3 Aug 2016 14:17:50 +0200
Subject: [PATCH] Automated ipa-replica-manage del tests

---
 ipatests/test_integration/test_topology.py | 72 ++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/ipatests/test_integration/test_topology.py b/ipatests/test_integration/test_topology.py
index 87f78565e133de66251476e2c9d6ccbb368d000d..101a6d434e9042b5215c94a82211bb4065a501de 100644
--- a/ipatests/test_integration/test_topology.py
+++ b/ipatests/test_integration/test_topology.py
@@ -212,3 +212,75 @@ class TestCASpecificRUVs(IntegrationTest):
                                       raiseonerr=False)
         assert(result5.returncode > 0), (
             'Replication still works after all RUVs were deleted')
+
+
+class TestReplicaManageDel(IntegrationTest):
+    domain_level = 0
+    topology = 'star'
+    num_replicas = 3
+
+    def test_replica_managed_del_domlevel0(self):
+        """
+        http://www.freeipa.org/page/V4/Manage_replication_topology_4_4/
+        Test_Plan#Test_case:_ipa-replica-manage_del_with_turned_off_replica
+        _under_domain_level_0_keeps_ca-related_RUVs
+        """
+        master = self.master
+        replica = self.replicas[0]
+        replica.run_command(['ipactl', 'stop'])
+        master.run_command(['ipa-replica-manage', 'del', '-f', '-p',
+                            master.config.dirman_password, replica.hostname])
+        result = master.run_command(['ipa-replica-manage', 'list-ruv',
+                                     '-p', master.config.dirman_password])
+        num_ruvs = result.stdout_text.count(replica.hostname)
+        assert(num_ruvs == 1), ("Expected to find 1 replica's RUV, found %s" %
+                                num_ruvs)
+        ruvid_re = re.compile(".*%s:389: (\d+).*" % replica.hostname)
+        replica_ruvs = ruvid_re.findall(result.stdout_text)
+        master.run_command(['ipa-replica-manage', 'clean-ruv', '-f',
+                            '-p', master.config.dirman_password,
+                            replica_ruvs[0]])
+        result2 = master.run_command(['ipa-replica-manage', 'list-ruv',
+                                      '-p', master.config.dirman_password])
+        assert(replica.hostname not in result2.stdout_text), (
+            "Replica's RUV was not properly removed")
+
+    def test_clean_dangling_ruv_multi_ca(self):
+        """
+        http://www.freeipa.org/page/V4/Manage_replication_topology_4_4/
+        Test_Plan#Test_case:_ipa-replica-manage_clean-dangling-ruv_in_a
+        _multi-CA_setup
+        """
+        master = self.master
+        replica = self.replicas[1]
+        replica.run_command(['ipa-server-install', '--uninstall', '-U'])
+        master.run_command(['ipa-replica-manage', 'del', '-f', '-p',
+                            master.config.dirman_password, replica.hostname])
+        result1 = master.run_command(['ipa-replica-manage', 'list-ruv', '-p',
+                                      master.config.dirman_password])
+        ruvid_re = re.compile(".*%s:389: (\d+).*" % replica.hostname)
+        assert(ruvid_re.search(result1.stdout_text)), (
+            "Replica's RUV should not be removed under domain level 0")
+        master.run_command(['ipa-replica-manage', 'clean-dangling-ruv', '-p',
+                            master.config.dirman_password], stdin_text="yes\n")
+        result2 = master.run_command(['ipa-replica-manage', 'list-ruv', '-p',
+                                      master.config.dirman_password])
+        assert(replica.hostname not in result2.stdout_text), (
+            "Replica's RUV was not removed by a clean-dangling-ruv command")
+
+    def test_replica_managed_del_domlevel1(self):
+        """
+        http://www.freeipa.org/page/V4/Manage_replication_topology_4_4/
+        Test_Plan#Test_case:_ipa-replica-manage_del_with_turned_off_replica
+        _under_domain_level_1_removes_ca-related_RUVs
+        """
+        master = self.master
+        replica = self.replicas[2]
+        master.run_command(['ipa', 'domainlevel-set', '1'])
+        replica.run_command(['ipactl', 'stop'])
+        master.run_command(['ipa-replica-manage', 'del', '-f', '-p',
+                            master.config.dirman_password, replica.hostname])
+        result = master.run_command(['ipa-replica-manage', 'list-ruv',
+                                     '-p', master.config.dirman_password])
+        assert(replica.hostname not in result.stdout_text), (
+            "Replica's RUV was not properly removed")
-- 
1.8.3.1

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to