Right you are! I am sorry.

On 10/13/2016 06:10 PM, Martin Basti wrote:
I think that you forgot to squash commits. Patch 47 doesn't apply


On 13.10.2016 14:01, Oleg Fayans wrote:
Hi Martin,

Thanks for the review.
With disabling directory server it works as well, thanks for the hint.
Also I moved the cleanup logic to the test itself for the sake of
simplicity. Patch-0048 was not changed

On 10/12/2016 02:35 PM, Martin Basti wrote:
1)

Can you just turn off dirsrv on replica instead of doing iptables magic?


2) NACK

No more eval() ever in code, use 'getattr', 'get' or whatever in the
object that can be used.

+                evalhost = eval("args[0].%s" % host)

Martin^2

On 12.10.2016 14:03, Oleg Fayans wrote:
Hi Martin,

After extensive discussion with Ludwig, I finally got the clue on how
does this feature work. When we uninstall the replica, the master
cleans the replication agreements with this replica and automatically
cleans all replica's RUVs.
If we clean replica's RUVs on master without uninstalling the replica,
the replica's RUVs get recreated on master (replication works!). So,
the only way to test the clean-ruv subcommand is to turn off the
replica, or block the traffic on it so it gets inaccessible to updates
from master.
The testcases were updated, see [1] and [2]

The updated versions of the patches are attached

[1]
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



[2]
http://www.freeipa.org/page/V4/Manage_replication_topology_4_4/Test_Plan#Test_case:_clean-ruv_subcommand



On 08/05/2016 06:36 PM, Martin Basti wrote:


On 03.08.2016 14:45, Oleg Fayans wrote:
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

Test failed:
        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")
E       AssertionError: replica's RUV is still displayed
E       assert 'replica3.ipa.test' not in 'Replica Update
V...ipa.test:389: 8\n'
E         'replica3.ipa.test' is contained here:
E           Replica Update Vectors:
E           \tmaster.ipa.test:389: 4
E           \treplica3.ipa.test:389: 3
E           \treplica2.ipa.test:389: 7
E           Certificate Server Replica Update Vectors:
E           \tmaster.ipa.test:389: 6
E           \treplica2.ipa.test:389: 8


[root@master ~]# ipa topologysegment-find
Suffix name: domain
------------------
2 segments matched
------------------
  Segment name: master.ipa.test-to-replica2.ipa.test
  Left node: master.ipa.test
  Right node: replica2.ipa.test
  Connectivity: both

  Segment name: master.ipa.test-to-replica3.ipa.test
  Left node: master.ipa.test
  Right node: replica3.ipa.test
  Connectivity: both
----------------------------
Number of entries returned 2
----------------------------
[root@master ~]# ipa-replica-manage list-ruv
Directory Manager password:

Replica Update Vectors:
    master.ipa.test:389: 4
    replica2.ipa.test:389: 7
    replica3.ipa.test:389: 3
Certificate Server Replica Update Vectors:
    master.ipa.test:389: 6
    replica2.ipa.test:389: 8
[root@master ~]#

Then I tried manually to clean RUV 3, and it behaves somehow odd

[root@master ~]# 'ipa-replica-manage' 'clean-ruv' '3' '-p'
'Secret123' '-f'
Clean the Replication Update Vector for replica3.ipa.test:389
Background task created to clean replication data. This may take a
while.
This may be safely interrupted with Ctrl+C
Cleanup task created
[root@master ~]# less /var/log/dirsrv/slapd-IPA-TEST/errors
[root@master ~]# ipa-replica-manage list-ruv
Directory Manager password:

Replica Update Vectors:
    master.ipa.test:389: 4
    replica2.ipa.test:389: 7
    replica3.ipa.test:389: 3
Certificate Server Replica Update Vectors:
    master.ipa.test:389: 6
    replica2.ipa.test:389: 8
[root@master ~]# 'ipa-replica-manage' 'clean-ruv' '3' '-p'
'Secret123' '-f'
Clean the Replication Update Vector for replica3.ipa.test:389
CLEANALLRUV task for replica id 3 already exists.
This may be safely interrupted with Ctrl+C
Cleanup task created

[root@master ~]# ipa-replica-manage list-clean-ruv -p Secret123
No CLEANALLRUV tasks running

No abort CLEANALLRUV tasks running
[root@master ~]# 'ipa-replica-manage' 'clean-ruv' '3' '-p'
'Secret123' '-f'
Clean the Replication Update Vector for replica3.ipa.test:389
Background task created to clean replication data. This may take a
while.
This may be safely interrupted with Ctrl+C
Cleanup task created
[root@master ~]# ipa-replica-manage list-clean-ruv -p Secret123
CLEANALLRUV tasks
RID 3: Successfully cleaned rid(3).

No abort CLEANALLRUV tasks running
[root@master ~]# ipa-replica-manage list-ruv -p Secret123
Replica Update Vectors:
    master.ipa.test:389: 4
    replica2.ipa.test:389: 7
Certificate Server Replica Update Vectors:
    master.ipa.test:389: 6
    replica2.ipa.test:389: 8


I'm not sure if this behavior is right, Ludwig may know.





--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From fdb5ae60a2c80d5ef51ae3905b023dbf48251116 Mon Sep 17 00:00:00 2001
From: Oleg Fayans <ofay...@redhat.com>
Date: Fri, 14 Oct 2016 11:34:49 +0200
Subject: [PATCH] Automated clean-ruv subcommand tests

https://fedorahosted.org/freeipa/ticket/5964
---
 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 c095812c47121b15168ce2ef2db7e06bc9d07e28..e1a2a3ad6998801f5a206c7546d76d6309b7589d 100644
--- a/ipatests/test_integration/test_topology.py
+++ b/ipatests/test_integration/test_topology.py
@@ -193,3 +193,75 @@ 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_delete_ruvs(self):
+        """
+        http://www.freeipa.org/page/V4/Manage_replication_topology_4_4/
+        Test_Plan#Test_case:_clean-ruv_subcommand
+        """
+        replica = self.replicas[0]
+        master = self.master
+        res1 = master.run_command(['ipa-replica-manage', 'list-ruv', '-p',
+                                  master.config.dirman_password])
+        assert(res1.stdout_text.count(replica.hostname) == 2 and
+               "Certificate Server Replica Update Vectors" in res1), (
+            "CA-specific RUVs are not displayed")
+        ruvid_re = re.compile(".*%s:389: (\d+).*" % replica.hostname)
+        replica_ruvs = ruvid_re.findall(res1.stdout_text)
+        # Find out the number of RUVids
+        assert(len(replica_ruvs) == 2), (
+            "The output should display 2 RUV ids of the selected replica")
+
+        # Block replication to preserve replica-specific RUVs
+        dashed_domain = master.domain.realm.replace(".", '-')
+        dirsrv_service = "dirsrv@%s.service" % dashed_domain
+        replica.run_command(['systemctl', 'disable', dirsrv_service])
+        replica.run_command(['systemctl', 'stop', dirsrv_service])
+        try:
+            master.run_command(['ipa-replica-manage', 'clean-ruv',
+                                replica_ruvs[1], '-p',
+                                master.config.dirman_password, '-f'])
+            res2 = master.run_command(['ipa-replica-manage',
+                                       'list-ruv', '-p',
+                                       master.config.dirman_password])
+
+            assert(res2.stdout_text.count(replica.hostname) == 1), (
+                "CA RUV of the replica is still displayed")
+            master.run_command(['ipa-replica-manage', 'clean-ruv',
+                                replica_ruvs[0], '-p',
+                                master.config.dirman_password, '-f'])
+            res3 = master.run_command(['ipa-replica-manage', 'list-ruv', '-p',
+                                       master.config.dirman_password])
+            assert(replica.hostname not in res3.stdout_text), (
+                "replica's RUV is still displayed")
+        finally:
+            replica.run_command(['systemctl', 'enable', dirsrv_service])
+            replica.run_command(['systemctl', 'start', dirsrv_service])
+
+    def test_replica_uninstall_deletes_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
+        """
+        master = self.master
+        replica = self.replicas[1]
+        res1 = master.run_command(['ipa-replica-manage', 'list-ruv', '-p',
+                                  master.config.dirman_password]).stdout_text
+        assert(res1.count(replica.hostname) == 2), (
+            "Did not find proper number of replica hostname (%s) occurrencies"
+            " in the command output: %s" % (replica.hostname, res1))
+        tasks.uninstall_master(replica)
+        res2 = master.run_command(['ipa-replica-manage', 'list-ruv', '-p',
+                                  master.config.dirman_password]).stdout_text
+        assert(replica.hostname not in res2), (
+            "Replica RUVs were not clean during replica uninstallation")
-- 
2.7.4

-- 
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