Vpcr Marvin test and some fixes for vprc

[isoutham]

A combined commit because otherwise it would not make sense.

If I commit the fixes alone there is not test to verify them
If I commit the tests alone it will not work because it found bugs

So both together

The test test_vpc_redundant.py is added (requires hardware) together with the following fixes.

1. Fix incorrect SNAT configuration (was adding SNAT to interfaces not marked as nat in the json config).
2. Fix interface used for keepalived multicast traffic to be lowest guest i/f. This stops some unnecessary keepalived flip-flops
3. If all nat rules are removed and re-added public interfaces would stay down (because master status does not change and CS deletes the interface)
4. Small change to Marvin to allow a test to change the retries count (is downwardly compatible).
5. A couple of pep8 things.
6. Improved locking during master/backup transitions.

The test still very occasionally fails with a (correct) double master detection. When I have worked out why, I will produce another PR. It appears to be a timing issue and could be tricky to find.

[wilderrodrigues]

Nice one, @isoutham !

I will test it this week and also try to get someone else to have a look so we get the 2 LGTM needed to merge.

Thanks!

Cheers,
Wilder

[wilderrodrigues]

Travis failed due timeout... so, that's not relevant and won't be taken into account when reviewing the PR:

---

No output has been received in the last 10 minutes, this potentially indicates a stalled build or something wrong with the build itself.

The build has been terminated

Cheers,
Wilder

[remibergsma]

@isoutham @wilderrodrigues is there a way to trigger the build again?

[wilderrodrigues]

Only if something get pushed to the branch which generated the PR.

Only 1 job out of 9 timed out... I wouldn't bother.

[sanju1010]

Can you remove this Services class and leverage the test data available @ tools/marvin/marvin/config/test_data.py file? You can refer to any tests available under test/integration/smoke/*.py on how to read test data from test_data.py config file.

[wilderrodrigues]

Hi @sanju1010

@isoutham is currently on holidays and I would like to test and get this PR through before he is back.

I agree with the improvement you suggested, however it's not a blocking thing for the PR because it's only related to the marvin test.

Once his PR get's through, I will proceed with removing the Services class and use test_data.py instead. A new PR will be created afterwards.

Cheers,
Wilder

[wilderrodrigues]

Hi @isoutham

Unfortunately the test is failing. Details about my environment and results below:

Management Server running on CentOS 7.1
Multi-host zone: 2 XenServer 6.2 hosts

It all starts fine: rVPC, tiers and VMs are created. After this...

self.delete_nat_rules()
self.check_master_status(1)
self.do_vpc_test(True)

All the tests work. However, the call to start the router doesnt

self.start_router()
self.add_nat_rules()
time.sleep(60)
self.check_master_status(2)
self.do_vpc_test(False)

The router never starts and the rules are not applied. I tried to give it a chance and restarted it manually (yes, deliberate action). After the router was running again, we got timeouts.

I checked the IPs and found out they had no PF rules to port 22, although the ACLs were properly configured. I added the PF rules and after a while I got:

===SSH to Host 192.168.23.8 port : 22 SUCCESSFUL===

I also connected to the VM manually"

Last login: Mon Jul 6 12:13:11 on ttys004
sbpltk1zffh04:asf_cloudstack wrodrigues$ ping 192.168.23.7
PING 192.168.23.7 (192.168.23.7): 56 data bytes
64 bytes from 192.168.23.7: icmp_seq=0 ttl=63 time=4.225 ms
64 bytes from 192.168.23.7: icmp_seq=1 ttl=63 time=4.085 ms
^C
--- 192.168.23.7 ping statistics ---
2 packets transmitted, 2 packets received, 0.0% packet loss
round-trip min/avg/max/stddev = 4.085/4.155/4.225/0.070 ms
sbpltk1zffh04:asf_cloudstack wrodrigues$ ssh [email protected]
The authenticity of host '192.168.23.7 (192.168.23.7)' can't be established.
RSA key fingerprint is c5:d3:aa:6e:09:b2:c7:6d:41:dd:ea:6c:4c:22:72:80.
Are you sure you want to continue connecting (yes/no)? yes
Warning: Permanently added '192.168.23.7' (RSA) to the list of known hosts.
[email protected]'s password:

ls /

bin boot dev etc home lib lib64 linuxrc lost+found media mnt opt proc root run sbin sys tmp usr var

date

Mon Jul 6 13:48:23 UTC 2015

Due to the previous failures, the test failed anyway.

I will have a closer look at the changes and try to get it to work - will probably be part of our next Sprint.

Cheers,
Wilder

[wilderrodrigues]

@DaanHoogland @remibergsma @karuturi

It LGTM.

There is still 1 issue, that was also mentioned by Ian, but I'm fixing that.

The details can be found here: https://issues.apache.org/jira/browse/CLOUDSTACK-8616

Since Ian is on holidays, I would like to get this PR merged so I can apply the fixes based on his work and create a separate PR.

Would one of you help me with that? :)

Thanks in advance.

Cheers,
Wilder

[DaanHoogland]

can we remove these lines and not leave code in comment?

[wilderrodrigues]

+1

Can I do all that in the new PR along with the fixes?

I mean... I will have 1 commit removing all the commented out lines. Is it okay?

:)

[DaanHoogland]

You can but you can also make a new pr inlcuding @isoutham code (with attribution intact) and add ammending commits
My worry is: will this break any existing tests. prod code is safe so I won't -1, just looking for loopholes

[wilderrodrigues]

The idea is to get his PR merged, then I will apply the fixes, remove the commented lines, test and create a new PR.

No, the changes above won't break any existing tests. The redundant VPC has just 1 test till now, which tests the offering and creation of rVPC - it was added by @afornie. @isoutham is adding the second test from scratch, so that won't break anything.

Cheers,
Wilder

[DaanHoogland]

ok, clear. you got my LGTM

[DaanHoogland]

L reasonably GTM

[wilderrodrigues]

Thanks, @DaanHoogland

Will proceed from here.