Skip to content

Fix for Vlan doesn't match issue while adding IP range for the shared network without any IP range #10837

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DaanHoogland merged 2 commits into from
May 16, 2025
Merged

Fix for Vlan doesn't match issue while adding IP range for the shared network without any IP range #10837

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4efa3af
Fix for Vlan doesn't match issue while adding IP range for the shared…
sureshanaparti May 8, 2025
deb77a1
review comments / code refactoring
sureshanaparti May 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions api/src/main/java/org/apache/cloudstack/api/command/admin/vlan/CreateVlanIpRangeCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,9 +155,6 @@ public String getStartIp() {
}

public String getVlan() {
if (vlan == null || vlan.isEmpty()) {
vlan = "untagged";
}
return vlan;
}

Expand Down
106 changes: 70 additions & 36 deletions server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4407,14 +4407,24 @@ public Vlan createVlanAndPublicIpRange(final CreateVlanIpRangeCmd cmd) throws In
String endIP = cmd.getEndIp();
final String newVlanGateway = cmd.getGateway();
final String newVlanNetmask = cmd.getNetmask();
Long networkId = cmd.getNetworkID();
Long physicalNetworkId = cmd.getPhysicalNetworkId();

// Verify that network exists
Network network = getNetwork(networkId);
if (network != null) {
zoneId = network.getDataCenterId();
physicalNetworkId = network.getPhysicalNetworkId();
}

String vlanId = cmd.getVlan();
vlanId = verifyAndUpdateVlanId(vlanId, network);

// TODO decide if we should be forgiving or demand a valid and complete URI
if (!(vlanId == null || "".equals(vlanId) || vlanId.startsWith(BroadcastDomainType.Vlan.scheme()))) {
vlanId = BroadcastDomainType.Vlan.toUri(vlanId).toString();
}
final Boolean forVirtualNetwork = cmd.isForVirtualNetwork();
Comment thread
sureshanaparti marked this conversation as resolved.
Long networkId = cmd.getNetworkID();
Long physicalNetworkId = cmd.getPhysicalNetworkId();
final String accountName = cmd.getAccountName();
final Long projectId = cmd.getProjectId();
final Long domainId = cmd.getDomainId();
Expand Down Expand Up @@ -4487,18 +4497,6 @@ public Vlan createVlanAndPublicIpRange(final CreateVlanIpRangeCmd cmd) throws In
}
}

// Verify that network exists
Network network = null;
if (networkId != null) {
network = _networkDao.findById(networkId);
if (network == null) {
throw new InvalidParameterValueException("Unable to find network by id " + networkId);
} else {
zoneId = network.getDataCenterId();
physicalNetworkId = network.getPhysicalNetworkId();
}
}

// Verify that zone exists
final DataCenterVO zone = _zoneDao.findById(zoneId);
if (zone == null) {
Expand Down Expand Up @@ -4639,6 +4637,32 @@ public Vlan createVlanAndPublicIpRange(final CreateVlanIpRangeCmd cmd) throws In
ip6Cidr, domain, vlanOwner, network, sameSubnet);
}

private Network getNetwork(Long networkId) {
if (networkId == null) {
return null;
}

Network network = _networkDao.findById(networkId);
if (network == null) {
throw new InvalidParameterValueException("Unable to find network by id " + networkId);
}

return network;
}

private String verifyAndUpdateVlanId(String vlanId, Network network) {
if (!StringUtils.isBlank(vlanId)) {
return vlanId;
}

if (network == null || network.getTrafficType() != TrafficType.Guest) {
return Vlan.UNTAGGED;
}

boolean connectivityWithoutVlan = isConnectivityWithoutVlan(network);
return getNetworkVlanId(network, connectivityWithoutVlan);
}

private Vlan commitVlan(final Long zoneId, final Long podId, final String startIP, final String endIP, final String newVlanGatewayFinal, final String newVlanNetmaskFinal,
final String vlanId, final Boolean forVirtualNetwork, final Boolean forSystemVms, final Long networkId, final Long physicalNetworkId, final String startIPv6, final String endIPv6,
final String ip6Gateway, final String ip6Cidr, final Domain domain, final Account vlanOwner, final Network network, final Pair<Boolean, Pair<String, String>> sameSubnet) {
Expand Down Expand Up @@ -4847,28 +4871,8 @@ public Vlan createVlanAndPublicIpRange(final long zoneId, final long networkId,
// same as network's vlan
// 2) if vlan is missing, default it to the guest network's vlan
if (network.getTrafficType() == TrafficType.Guest) {
String networkVlanId = null;
boolean connectivityWithoutVlan = false;
if (_networkModel.areServicesSupportedInNetwork(network.getId(), Service.Connectivity)) {
Map<Capability, String> connectivityCapabilities = _networkModel.getNetworkServiceCapabilities(network.getId(), Service.Connectivity);
connectivityWithoutVlan = MapUtils.isNotEmpty(connectivityCapabilities) && connectivityCapabilities.containsKey(Capability.NoVlan);
}

final URI uri = network.getBroadcastUri();
if (connectivityWithoutVlan) {
networkVlanId = network.getBroadcastDomainType().toUri(network.getUuid()).toString();
} else if (uri != null) {
// Do not search for the VLAN tag when the network doesn't support VLAN
if (uri.toString().startsWith("vlan")) {
final String[] vlan = uri.toString().split("vlan:\\/\\/");
networkVlanId = vlan[1];
// For pvlan
if (network.getBroadcastDomainType() != BroadcastDomainType.Vlan) {
networkVlanId = networkVlanId.split("-")[0];
}
}
}

boolean connectivityWithoutVlan = isConnectivityWithoutVlan(network);
String networkVlanId = getNetworkVlanId(network, connectivityWithoutVlan);
if (vlanId != null && !connectivityWithoutVlan) {
// if vlan is specified, throw an error if it's not equal to
// network's vlanId
Expand Down Expand Up @@ -5000,6 +5004,36 @@ public Vlan createVlanAndPublicIpRange(final long zoneId, final long networkId,
return vlan;
}

private boolean isConnectivityWithoutVlan(Network network) {
boolean connectivityWithoutVlan = false;
if (_networkModel.areServicesSupportedInNetwork(network.getId(), Service.Connectivity)) {
Map<Capability, String> connectivityCapabilities = _networkModel.getNetworkServiceCapabilities(network.getId(), Service.Connectivity);
connectivityWithoutVlan = MapUtils.isNotEmpty(connectivityCapabilities) && connectivityCapabilities.containsKey(Capability.NoVlan);
}
return connectivityWithoutVlan;
}

private String getNetworkVlanId(Network network, boolean connectivityWithoutVlan) {
String networkVlanId = null;
if (connectivityWithoutVlan) {
return network.getBroadcastDomainType().toUri(network.getUuid()).toString();
}

final URI uri = network.getBroadcastUri();
if (uri != null) {
// Do not search for the VLAN tag when the network doesn't support VLAN
if (uri.toString().startsWith("vlan")) {
final String[] vlan = uri.toString().split("vlan:\\/\\/");
networkVlanId = vlan[1];
// For pvlan
if (network.getBroadcastDomainType() != BroadcastDomainType.Vlan) {
networkVlanId = networkVlanId.split("-")[0];
}
}
}
return networkVlanId;
}

private void checkZoneVlanIpOverlap(DataCenterVO zone, Network network, String newCidr, String vlanId, String vlanGateway, String vlanNetmask, String startIP, String endIP) {
// Throw an exception if this subnet overlaps with subnet on other VLAN,
// if this is ip range extension, gateway, network mask should be same and ip range should not overlap
Expand Down
6 changes: 6 additions & 0 deletions server/src/test/java/com/cloud/configuration/ConfigurationManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@
import com.cloud.network.dao.IPAddressDao;
import com.cloud.network.dao.IPAddressVO;
import com.cloud.network.dao.Ipv6GuestPrefixSubnetNetworkMapDao;
import com.cloud.network.dao.NetworkDao;
import com.cloud.network.dao.NetworkVO;
import com.cloud.network.dao.PhysicalNetworkDao;
import com.cloud.network.dao.PhysicalNetworkVO;
import com.cloud.offering.DiskOffering;
Expand Down Expand Up @@ -189,6 +191,8 @@ public class ConfigurationManagerTest {
@Mock
HostPodDao _podDao;
@Mock
NetworkDao _networkDao;
@Mock
PhysicalNetworkDao _physicalNetworkDao;
@Mock
ImageStoreDao _imageStoreDao;
Expand Down Expand Up @@ -1325,6 +1329,8 @@ public void testDisabledConfigCreateIpv6NetworkOffering() {
public void testWrongIpv6CreateVlanAndPublicIpRange() {
CreateVlanIpRangeCmd cmd = Mockito.mock(CreateVlanIpRangeCmd.class);
Mockito.when(cmd.getIp6Cidr()).thenReturn("fd17:5:8a43:e2a4:c000::/66");
NetworkVO network = Mockito.mock(NetworkVO.class);
Mockito.when(_networkDao.findById(Mockito.anyLong())).thenReturn(network);
try {
configurationMgr.createVlanAndPublicIpRange(cmd);
} catch (InsufficientCapacityException | ResourceUnavailableException | ResourceAllocationException e) {
Expand Down