Skip to content

Commit 13ab8a0

Browse files
sureshanapartisureshanaparti
authored
Fix for Vlan doesn't match issue while adding IP range for the shared network without any IP range (apache#10837)
1 parent 1507a56 commit 13ab8a0

File tree

3 files changed

+76
-39
lines changed

3 files changed

+76
-39
lines changed

‎api/src/main/java/org/apache/cloudstack/api/command/admin/vlan/CreateVlanIpRangeCmd.java‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -155,9 +155,6 @@ public String getStartIp() {
155155
}
156156

157157
public String getVlan() {
158-
if (vlan == null || vlan.isEmpty()) {
159-
vlan = "untagged";
160-
}
161158
return vlan;
162159
}
163160

‎server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java‎

Lines changed: 70 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -4407,14 +4407,24 @@ public Vlan createVlanAndPublicIpRange(final CreateVlanIpRangeCmd cmd) throws In
44074407
String endIP = cmd.getEndIp();
44084408
final String newVlanGateway = cmd.getGateway();
44094409
final String newVlanNetmask = cmd.getNetmask();
4410+
Long networkId = cmd.getNetworkID();
4411+
Long physicalNetworkId = cmd.getPhysicalNetworkId();
4412+
4413+
// Verify that network exists
4414+
Network network = getNetwork(networkId);
4415+
if (network != null) {
4416+
zoneId = network.getDataCenterId();
4417+
physicalNetworkId = network.getPhysicalNetworkId();
4418+
}
4419+
44104420
String vlanId = cmd.getVlan();
4421+
vlanId = verifyAndUpdateVlanId(vlanId, network);
4422+
44114423
// TODO decide if we should be forgiving or demand a valid and complete URI
44124424
if (!(vlanId == null || "".equals(vlanId) || vlanId.startsWith(BroadcastDomainType.Vlan.scheme()))) {
44134425
vlanId = BroadcastDomainType.Vlan.toUri(vlanId).toString();
44144426
}
44154427
final Boolean forVirtualNetwork = cmd.isForVirtualNetwork();
4416-
Long networkId = cmd.getNetworkID();
4417-
Long physicalNetworkId = cmd.getPhysicalNetworkId();
44184428
final String accountName = cmd.getAccountName();
44194429
final Long projectId = cmd.getProjectId();
44204430
final Long domainId = cmd.getDomainId();
@@ -4487,18 +4497,6 @@ public Vlan createVlanAndPublicIpRange(final CreateVlanIpRangeCmd cmd) throws In
44874497
}
44884498
}
44894499

4490-
// Verify that network exists
4491-
Network network = null;
4492-
if (networkId != null) {
4493-
network = _networkDao.findById(networkId);
4494-
if (network == null) {
4495-
throw new InvalidParameterValueException("Unable to find network by id " + networkId);
4496-
} else {
4497-
zoneId = network.getDataCenterId();
4498-
physicalNetworkId = network.getPhysicalNetworkId();
4499-
}
4500-
}
4501-
45024500
// Verify that zone exists
45034501
final DataCenterVO zone = _zoneDao.findById(zoneId);
45044502
if (zone == null) {
@@ -4639,6 +4637,32 @@ public Vlan createVlanAndPublicIpRange(final CreateVlanIpRangeCmd cmd) throws In
46394637
ip6Cidr, domain, vlanOwner, network, sameSubnet);
46404638
}
46414639

4640+
private Network getNetwork(Long networkId) {
4641+
if (networkId == null) {
4642+
return null;
4643+
}
4644+
4645+
Network network = _networkDao.findById(networkId);
4646+
if (network == null) {
4647+
throw new InvalidParameterValueException("Unable to find network by id " + networkId);
4648+
}
4649+
4650+
return network;
4651+
}
4652+
4653+
private String verifyAndUpdateVlanId(String vlanId, Network network) {
4654+
if (!StringUtils.isBlank(vlanId)) {
4655+
return vlanId;
4656+
}
4657+
4658+
if (network == null || network.getTrafficType() != TrafficType.Guest) {
4659+
return Vlan.UNTAGGED;
4660+
}
4661+
4662+
boolean connectivityWithoutVlan = isConnectivityWithoutVlan(network);
4663+
return getNetworkVlanId(network, connectivityWithoutVlan);
4664+
}
4665+
46424666
private Vlan commitVlan(final Long zoneId, final Long podId, final String startIP, final String endIP, final String newVlanGatewayFinal, final String newVlanNetmaskFinal,
46434667
final String vlanId, final Boolean forVirtualNetwork, final Boolean forSystemVms, final Long networkId, final Long physicalNetworkId, final String startIPv6, final String endIPv6,
46444668
final String ip6Gateway, final String ip6Cidr, final Domain domain, final Account vlanOwner, final Network network, final Pair<Boolean, Pair<String, String>> sameSubnet) {
@@ -4847,28 +4871,8 @@ public Vlan createVlanAndPublicIpRange(final long zoneId, final long networkId,
48474871
// same as network's vlan
48484872
// 2) if vlan is missing, default it to the guest network's vlan
48494873
if (network.getTrafficType() == TrafficType.Guest) {
4850-
String networkVlanId = null;
4851-
boolean connectivityWithoutVlan = false;
4852-
if (_networkModel.areServicesSupportedInNetwork(network.getId(), Service.Connectivity)) {
4853-
Map<Capability, String> connectivityCapabilities = _networkModel.getNetworkServiceCapabilities(network.getId(), Service.Connectivity);
4854-
connectivityWithoutVlan = MapUtils.isNotEmpty(connectivityCapabilities) && connectivityCapabilities.containsKey(Capability.NoVlan);
4855-
}
4856-
4857-
final URI uri = network.getBroadcastUri();
4858-
if (connectivityWithoutVlan) {
4859-
networkVlanId = network.getBroadcastDomainType().toUri(network.getUuid()).toString();
4860-
} else if (uri != null) {
4861-
// Do not search for the VLAN tag when the network doesn't support VLAN
4862-
if (uri.toString().startsWith("vlan")) {
4863-
final String[] vlan = uri.toString().split("vlan:\\/\\/");
4864-
networkVlanId = vlan[1];
4865-
// For pvlan
4866-
if (network.getBroadcastDomainType() != BroadcastDomainType.Vlan) {
4867-
networkVlanId = networkVlanId.split("-")[0];
4868-
}
4869-
}
4870-
}
4871-
4874+
boolean connectivityWithoutVlan = isConnectivityWithoutVlan(network);
4875+
String networkVlanId = getNetworkVlanId(network, connectivityWithoutVlan);
48724876
if (vlanId != null && !connectivityWithoutVlan) {
48734877
// if vlan is specified, throw an error if it's not equal to
48744878
// network's vlanId
@@ -5000,6 +5004,36 @@ public Vlan createVlanAndPublicIpRange(final long zoneId, final long networkId,
50005004
return vlan;
50015005
}
50025006

5007+
private boolean isConnectivityWithoutVlan(Network network) {
5008+
boolean connectivityWithoutVlan = false;
5009+
if (_networkModel.areServicesSupportedInNetwork(network.getId(), Service.Connectivity)) {
5010+
Map<Capability, String> connectivityCapabilities = _networkModel.getNetworkServiceCapabilities(network.getId(), Service.Connectivity);
5011+
connectivityWithoutVlan = MapUtils.isNotEmpty(connectivityCapabilities) && connectivityCapabilities.containsKey(Capability.NoVlan);
5012+
}
5013+
return connectivityWithoutVlan;
5014+
}
5015+
5016+
private String getNetworkVlanId(Network network, boolean connectivityWithoutVlan) {
5017+
String networkVlanId = null;
5018+
if (connectivityWithoutVlan) {
5019+
return network.getBroadcastDomainType().toUri(network.getUuid()).toString();
5020+
}
5021+
5022+
final URI uri = network.getBroadcastUri();
5023+
if (uri != null) {
5024+
// Do not search for the VLAN tag when the network doesn't support VLAN
5025+
if (uri.toString().startsWith("vlan")) {
5026+
final String[] vlan = uri.toString().split("vlan:\\/\\/");
5027+
networkVlanId = vlan[1];
5028+
// For pvlan
5029+
if (network.getBroadcastDomainType() != BroadcastDomainType.Vlan) {
5030+
networkVlanId = networkVlanId.split("-")[0];
5031+
}
5032+
}
5033+
}
5034+
return networkVlanId;
5035+
}
5036+
50035037
private void checkZoneVlanIpOverlap(DataCenterVO zone, Network network, String newCidr, String vlanId, String vlanGateway, String vlanNetmask, String startIP, String endIP) {
50045038
// Throw an exception if this subnet overlaps with subnet on other VLAN,
50055039
// if this is ip range extension, gateway, network mask should be same and ip range should not overlap

‎server/src/test/java/com/cloud/configuration/ConfigurationManagerTest.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@
5454
import com.cloud.network.dao.IPAddressDao;
5555
import com.cloud.network.dao.IPAddressVO;
5656
import com.cloud.network.dao.Ipv6GuestPrefixSubnetNetworkMapDao;
57+
import com.cloud.network.dao.NetworkDao;
58+
import com.cloud.network.dao.NetworkVO;
5759
import com.cloud.network.dao.PhysicalNetworkDao;
5860
import com.cloud.network.dao.PhysicalNetworkVO;
5961
import com.cloud.offering.DiskOffering;
@@ -189,6 +191,8 @@ public class ConfigurationManagerTest {
189191
@Mock
190192
HostPodDao _podDao;
191193
@Mock
194+
NetworkDao _networkDao;
195+
@Mock
192196
PhysicalNetworkDao _physicalNetworkDao;
193197
@Mock
194198
ImageStoreDao _imageStoreDao;
@@ -1325,6 +1329,8 @@ public void testDisabledConfigCreateIpv6NetworkOffering() {
13251329
public void testWrongIpv6CreateVlanAndPublicIpRange() {
13261330
CreateVlanIpRangeCmd cmd = Mockito.mock(CreateVlanIpRangeCmd.class);
13271331
Mockito.when(cmd.getIp6Cidr()).thenReturn("fd17:5:8a43:e2a4:c000::/66");
1332+
NetworkVO network = Mockito.mock(NetworkVO.class);
1333+
Mockito.when(_networkDao.findById(Mockito.anyLong())).thenReturn(network);
13281334
try {
13291335
configurationMgr.createVlanAndPublicIpRange(cmd);
13301336
} catch (InsufficientCapacityException | ResourceUnavailableException | ResourceAllocationException e) {

0 commit comments

Comments
 (0)