CLOUDSTACK-9106 - As a Developer I want the Redundant VPC private gateway feature fixed#1184
Merged
asfgit merged 6 commits intoDec 7, 2015
Conversation
…s of a VPC - It was causing problems because Nics were expected to be plugged before they actually exist. Only in rVPC cases. - Applies ACL items to routers only after the Pvt GW is setup.
…thod
- Use the router to retrieve the instance ID
- Check if the VPC is redundant in order to reuse the private gateway address.
- Brings the private gateways interfaces up.
- Checks the result of a call against the previous result. Either both are true or the method returns false.
- Do not thrown exceptions because some calls are not handling/rethrowing them. It would cause runtime problems.
- When doing a list.addAll(Arrays.asList(String[]{}) will cause problems when trying to cast the list.toArray() into an aray of String
It would only work if instead of calling addAll() I would pass it straight into the constructor:
e.g. List<String> l = new ArrayList(Arrays.asList(new String[]{});
Stirng [] s = (String[]) l.toArray();
But I did not like that implementation because it would require 2 arrays of string and combine them at the end.
…rs() method - Changed the NetworkTopologyContext class just to make the private member accessible from the test - Added a test class to cover the positive scenario of the VpcVirtualRouterElementTest.applyVpnUsers() method. - Covering when there is either no VPC or no routers.
Contributor
Author
|
Ping @DaanHoogland @remibergsma @borisroman I will continue the tests, but PR is ready to be reviewed.
|
Contributor
Author
|
Jenkins known issue:
|
Contributor
Author
|
Ping @remibergsma @miguelaferreira @DaanHoogland @borisroman Problem with my PR! I haven’t seen that one failing before. Will investigate.
|
Contributor
Author
|
Ping @remibergsma @DaanHoogland @miguelaferreira @borisroman Deployed another DC and followed the same step as the test, but did it manually: Result:
Waiting for your feedback on your tests. Cheers, |
Contributor
Author
|
Although it worked manually, I'm running the test again... in an brand new DC. |
Contributor
|
Went through the code. Looks Good To Me. |
Contributor
|
LGTM based on these tests: |
Contributor
Author
|
Ping @remibergsma @miguelaferreira @DaanHoogland Test passed as expected! I will post more results soon! |
Contributor
Author
Ping @remibergsma @DaanHoogland @miguelaferreira I executed all the steps of the tests that failed manually, in a new DC, and everything worked fine. I also executed the test itself, with nose - see comment above - , and all worked as expected. Cheers, |
Contributor
|
@wilderrodrigues I think we're fine. That test also passes here: |
asfgit
pushed a commit
that referenced
this pull request
Dec 7, 2015
…9106 CLOUDSTACK-9106 - As a Developer I want the Redundant VPC private gateway feature fixedThis PR contains the same fixes from PR #1179, which was created against the master branch. In addition, the points mentioned by @DaanHoogland were handled in this new PR: * Made the code more consistent - result = result && methodCall(), instead of throwing exceptions in some places or not checking 2 consecutive returns - in case of rVPC. * Added an unit test to cover changes in the VpcRouterElementImpl.applyVpnUsers() method. The method returns an array of String, so I had to make sure it would contain the users from 2 consecutive calls. There are 2 tests to cover negative scenarios. * pr/1184: CLOUDSTACK-9106 - Makes Enum name compliant with Java code conventions. CLOUDSTACK-9106 - Adds a test to cover the changes in the applyVpnUsers() method CLOUDSTACK-9106 - Makes the router commands call more consistent. CLOUDSTACK-9106 - Enables private gateway tests on Redundant VPCs CLOUDSTACK-9106 - Refactor the createPrivateNicProfileForGateway() method CLOUDSTACK-9106 - Reduces the amount of iterations through the routers of a VPC Signed-off-by: Remi Bergsma <github@remi.nl>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR contains the same fixes from PR #1179, which was created against the master branch.
In addition, the points mentioned by @DaanHoogland were handled in this new PR: