[Freeipa-devel] [Test][Patch-0047] Added a test for Ticket N 5964
Martin Basti
mbasti at redhat.com
Thu Nov 3 15:22:51 UTC 2016
almost ACK, but the ticket in commit message is closed as invalid. So
I'm quite puzzled now what to do.
On 03.11.2016 13:28, Oleg Fayans wrote:
> ping for review
>
> On 10/19/2016 04:54 PM, Oleg Fayans wrote:
>> Hi Martin,
>>
>> Thanks for the review. Fixed both issues.
>>
>> $ ipa-run-tests test_integration/test_topology.py -k TestCASpecificRUVs
>> WARNING: Couldn't write lextab module 'pycparser.lextab'. [Errno 13]
>> Permission denied: 'lextab.py'
>> WARNING: yacc table file version is out of date
>> WARNING: Couldn't create 'pycparser.yacctab'. [Errno 13] Permission
>> denied: 'yacctab.py'
>> ====================================================================================
>>
>> test session starts
>> =====================================================================================
>>
>>
>> platform linux2 -- Python 2.7.11, pytest-2.9.2, py-1.4.31, pluggy-0.3.1
>> rootdir: /usr/lib/python2.7/site-packages/ipatests, inifile: pytest.ini
>> plugins: sourceorder-0.5, multihost-1.0
>> collected 5 items
>>
>> test_integration/test_topology.py ..
>>
>> ================================================================================
>>
>> 2 passed in 2444.84 seconds
>> =================================================================================
>>
>>
>>
>>
>> On 10/17/2016 07:05 PM, Martin Basti wrote:
>>> 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