You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by nitin-maharana <gi...@git.apache.org> on 2016/12/01 07:26:14 UTC
[GitHub] cloudstack pull request #1804: CLOUDSTACK-9639: Unable to create shared netw...
GitHub user nitin-maharana opened a pull request:
https://github.com/apache/cloudstack/pull/1804
CLOUDSTACK-9639: Unable to create shared network with vLan isolation
Description:
=========
Create shared network fails with Error.
While creating a shared network it fails to create with Error "The new IP range you have specified has overlapped with the guest network in the zone: XYZ. Please specify a different gateway/netmask"
Steps to Reproduce:
===============
1. Create an isolated network with a subnet eg: 10.1.1.0.24
2. Create a shared network with the same subnet but different VLAN, we should observe this issue.
Expected Behaviour:
===============
It shouldn't restrict the creation of the guest network with the same subnet as long as they are segmented by VLAN.
ACTUAL BEHAVIOR:
===============
It doesn't allow the creation of shared guest networks if there is any isolated guest network using the same subnet although it allows using the same subnet in multiple shared networks.
Cause:
=====
The issue is happening because, when Shared network is getting created we are checking if there is any guest network already implemented with the same CIDR and throwing the error without checking if they are having same VLAN also. Creating the same CIDR shared network with different VLAN should be allowed.
Fix:
===
When creating a shared network, if there is any existing guest network with the same CIDR, we check if they are having the same VLAN, if they are in same VLAN, then we don't allow creating it. If they are in the same CIDR with different VLAN then we allowing to create the network.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/Accelerite/cloudstack CLOUDSTACK-9639
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1804.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1804
----
commit fc4434846433f8df64998614deb6c1b4b40992da
Author: Nitin Kumar Maharana <ni...@accelerite.com>
Date: 2016-12-01T07:13:49Z
CLOUDSTACK-9639: Unable to create shared network with vLan isolation
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1804: CLOUDSTACK-9639: Unable to create shared network wit...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on the issue:
https://github.com/apache/cloudstack/pull/1804
@rhtyd looks like the Travis build is failing due to a timeout. Do you have any ideas what could be causing this timeout?
@nitin-maharana are their existing Marvin tests that verify this fix?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1804: CLOUDSTACK-9639: Unable to create shared network wit...
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1804
@nitin-maharana please push -f on the PR to re-kick travis, or close/open the PR
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1804: CLOUDSTACK-9639: Unable to create shared network wit...
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1804
@jburwell I'll have a look. I've fixed on issue with Travis component test.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1804: CLOUDSTACK-9639: Unable to create shared network wit...
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1804
@nitin-maharana this looks useful, can you change the base branch for the PR to 4.9, rebase your PR branch against 4.9? Can you add a marvin test for this?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1804: CLOUDSTACK-9639: Unable to create shared netw...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/1804
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1804: CLOUDSTACK-9639: Unable to create shared network wit...
Posted by nitin-maharana <gi...@git.apache.org>.
Github user nitin-maharana commented on the issue:
https://github.com/apache/cloudstack/pull/1804
@rhtyd : Changed the base branch to 4.9
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1804: CLOUDSTACK-9639: Unable to create shared netw...
Posted by nitin-maharana <gi...@git.apache.org>.
Github user nitin-maharana commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1804#discussion_r90584766
--- Diff: server/src/com/cloud/configuration/ConfigurationManagerImpl.java ---
@@ -3092,8 +3092,12 @@ public Vlan createVlanAndPublicIpRange(final long zoneId, final long networkId,
final String guestNetworkCidr = zone.getGuestNetworkCidr();
if (guestNetworkCidr != null) {
if (NetUtils.isNetworksOverlap(newCidr, guestNetworkCidr)) {
- throw new InvalidParameterValueException("The new IP range you have specified has overlapped with the guest network in zone: " + zone.getName()
- + ". Please specify a different gateway/netmask.");
+ // when adding shared network with same cidr of zone guest cidr,
+ // if the specified vlan is not present in zone, physical network, allow to create the network as the isolation is based on VLAN.
+ if (_zoneDao.findVnet(zoneId, physicalNetworkId, vlanId).size() > 0) {
--- End diff --
@jburwell : Thanks for your suggestion. I will consider this change. The effect would be same if we put all the conditions with && operator.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1804: CLOUDSTACK-9639: Unable to create shared netw...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1804#discussion_r90476049
--- Diff: server/src/com/cloud/configuration/ConfigurationManagerImpl.java ---
@@ -3092,8 +3092,12 @@ public Vlan createVlanAndPublicIpRange(final long zoneId, final long networkId,
final String guestNetworkCidr = zone.getGuestNetworkCidr();
if (guestNetworkCidr != null) {
if (NetUtils.isNetworksOverlap(newCidr, guestNetworkCidr)) {
- throw new InvalidParameterValueException("The new IP range you have specified has overlapped with the guest network in zone: " + zone.getName()
- + ". Please specify a different gateway/netmask.");
+ // when adding shared network with same cidr of zone guest cidr,
+ // if the specified vlan is not present in zone, physical network, allow to create the network as the isolation is based on VLAN.
+ if (_zoneDao.findVnet(zoneId, physicalNetworkId, vlanId).size() > 0) {
--- End diff --
Please consider using `isEmpty` rather than a size check to determine whether or not a list contains elements. It is more idiomatic/clear, but less error prone. Also, why isn't this `if` condition combined with the previous `if` block and an `&&` operator.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1804: CLOUDSTACK-9639: Unable to create shared network wit...
Posted by murali-reddy <gi...@git.apache.org>.
Github user murali-reddy commented on the issue:
https://github.com/apache/cloudstack/pull/1804
LGTM.
a small side note. User can create a guest VM with nic's in two networks with same CIDR but different VLAN isolations and mess-up routing.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---