Vpcr Marvin test and some fixes for vprc#558
Conversation
|
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, |
|
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 terminatedCheers, |
|
@isoutham @wilderrodrigues is there a way to trigger the build again? |
|
Only if something get pushed to the branch which generated the PR. Only 1 job out of 9 timed out... I wouldn't bother. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
|
Hi @isoutham Unfortunately the test is failing. Details about my environment and results below: Management Server running on CentOS 7.1 It all starts fine: rVPC, tiers and VMs are created. After this... self.delete_nat_rules() All the tests work. However, the call to start the router doesnt self.start_router() 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 ls /bin boot dev etc home lib lib64 linuxrc lost+found media mnt opt proc root run sbin sys tmp usr var dateMon 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, |
|
@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, |
There was a problem hiding this comment.
can we remove these lines and not leave code in comment?
There was a problem hiding this comment.
+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?
:)
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
ok, clear. you got my LGTM
|
L reasonably GTM |
|
Thanks, @DaanHoogland Will proceed from here. |
…led (#558) * Fix bug missing Security Group in Advanced Zone 1. Add a new step security group in a zone with SG enabled. 2. Fix error of displaying sshKeypair data incorrectly * remove `this.hypervisor!=='KVM'` condition. Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
Improve logs in avoid planner Closes apache#558 See merge request scclouds/scclouds!223
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.
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.