[Freeipa-devel] [Test][Patch-0047] Added a test for Ticket N 5964
Martin Basti
mbasti at redhat.com
Fri Aug 5 16:36:34 UTC 2016
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