CLOUDSTACK-9106 - As a Developer I want the Redundant VPC private gateway feature fixed#1179
Closed
wilderrodrigues wants to merge 3 commits into
Closed
CLOUDSTACK-9106 - As a Developer I want the Redundant VPC private gateway feature fixed#1179wilderrodrigues wants to merge 3 commits into
wilderrodrigues wants to merge 3 commits into
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.
Contributor
Author
|
Ping @remibergsma @DaanHoogland @bhaisaab @karuturi @borisroman @miguelaferreira Could you please review this PR? I will execute more tests now.
|
Contributor
|
I like the scrum style title but just to be nitpicking: isn't this more of a network engineer feature instead of a developer tool? |
Contributor
There was a problem hiding this comment.
will we forget all intermediate results and only return the last one?
Contributor
Author
|
Agree... ;) Sometimes I find difficult to wear different hats, although I could do it just fine. Should I change the title here and on the issue? Cheers, |
Contributor
Author
There was a problem hiding this comment.
I already said why I didn't do it and want to change the code. So, what's the point in going on and comment on every thing? We could just accept this and improve before the 4.7.0 feature freeze.
Contributor
There was a problem hiding this comment.
Sorry, i did not notice. Where did you say that?
I saw some other places where you bail on the first fail. I don't like that solution:
If the first fails nothing is applied.
If the second fails the first router has the rules applied.
with &= it is symmetrical.
Contributor
Author
There was a problem hiding this comment.
Yep... I know. That's what I said when you first mentioned it. I will change it now, but we have to revisit many places in the Java side as well, and not only the classes I changed, but the VpcManagerImpl.java, for instance.
I saw that in some places it checks the result inside the loop and throw an exception. I will replace it with the &= assignment.
Contributor
|
Wilder can this one be on 4.6 please? |
Contributor
Author
|
More tests... On the same environment, but with hardware TRUE. |
Contributor
|
LGTM based on these tests: Result: And: Result: Will also run the test you mentioned above! |
Contributor
|
@remibergsma I think I am being captain obvious (as my new colleagues like to call each other) but let's add them to the standard run. |
remibergsma
added a commit
to MissionCriticalCloud/bubble-toolkit
that referenced
this pull request
Dec 6, 2015
After this PR is merged: apache/cloudstack#1179
Contributor
|
@DaanHoogland Yes, sir! See linked PR above. |
Contributor
|
Run this test: Result: Nice work @wilderrodrigues ! |
Contributor
Author
|
I'm closing this PR and will create a new one against 4.6. Cheers, |
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
3 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 fixes the Private Gateway feature when using Redundant VPCs.
In order to get it to work, I had to refactor some of the Java code in order to reduce the number of iterations we had with the routers list. It caused an issue when trying to configure ACL rules in a router (the backup one) that did not have the interface setup yet.
The rVPC Pvt GW integration test is not being skipped anymore and is 100% green!