[Freeipa-devel] [Test][Patch-0047] Added a test for Ticket N 5964
Oleg Fayans
ofayans at redhat.com
Wed Oct 19 14:54:16 UTC 2016
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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-ofayans-0047.6-Automated-clean-ruv-subcommand-tests.patch
Type: text/x-patch
Size: 4418 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20161019/3025158a/attachment.bin>
More information about the Freeipa-devel
mailing list