[Freeipa-devel] [Test][Patch-0047] Added a test for Ticket N 5964
Martin Basti
mbasti at redhat.com
Mon Oct 17 17:05:04 UTC 2016
1)
you don't need to disable/enable dirsrv, just stop/start. Please remove
disable/enable parts
2)
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> traceback >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
self = <ipatests.test_integration.test_topology.TestCASpecificRUVs
object at 0x7f6a502eec90>
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")
E TypeError: argument of type 'SSHCommand' is not iterable
test_integration/test_topology.py:215: TypeError
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> entering PDB >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>
/usr/lib/python2.7/site-packages/ipatests/test_integration/test_topology.py(215)test_delete_ruvs()
-> assert(res1.stdout_text.count(replica.hostname) == 2 and
On 14.10.2016 11:36, Oleg Fayans wrote:
> 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 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.
>>>>>
>>>>
>>>
>>
>
More information about the Freeipa-devel
mailing list