[Freeipa-devel] [Test][Patch-0047] Added a test for Ticket N 5964
Oleg Fayans
ofayans at redhat.com
Thu Oct 13 12:01:52 UTC 2016
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 at 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 at 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 at master ~]#
>>>
>>> Then I tried manually to clean RUV 3, and it behaves somehow odd
>>>
>>> [root at 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 at master ~]# less /var/log/dirsrv/slapd-IPA-TEST/errors
>>> [root at 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 at 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 at master ~]# ipa-replica-manage list-clean-ruv -p Secret123
>>> No CLEANALLRUV tasks running
>>>
>>> No abort CLEANALLRUV tasks running
>>> [root at 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 at master ~]# ipa-replica-manage list-clean-ruv -p Secret123
>>> CLEANALLRUV tasks
>>> RID 3: Successfully cleaned rid(3).
>>>
>>> No abort CLEANALLRUV tasks running
>>> [root at 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-ofayans-0047.4-Automated-clean-ruv-subcommand-tests.patch
Type: text/x-patch
Size: 5671 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20161013/be0cbeb2/attachment.bin>
More information about the Freeipa-devel
mailing list