[Freeipa-devel] [Test][Patch-0047] Added a test for Ticket N 5964
Oleg Fayans
ofayans at redhat.com
Thu Nov 3 12:28:01 UTC 2016
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.
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
>
>
--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
More information about the Freeipa-devel
mailing list