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 1 commit
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
91 changes: 55 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,35 @@ 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 = 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();
}
}
Comment on lines +4410 to +4418

@DaanHoogland DaanHoogland May 14, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be a seperate method

@sureshanaparti sureshanaparti May 15, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


String vlanId = cmd.getVlan();
if (StringUtils.isBlank(vlanId)) {
vlanId = Vlan.UNTAGGED;
if (network != null & network.getTrafficType() == TrafficType.Guest) {
boolean connectivityWithoutVlan = isConnectivityWithoutVlan(network);
vlanId = getNetworkVlanId(network, connectivityWithoutVlan);
}
}

@DaanHoogland DaanHoogland May 14, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be an extra method

@sureshanaparti sureshanaparti May 15, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


// 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 +4508,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 @@ -4847,28 +4856,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 +4989,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