Skip to content

Allow CIDR update for the shared network when the network IPs are not in use #10839

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.

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

Open
wants to merge 1 commit into
base: 4.19
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions engine/schema/src/main/java/com/cloud/vm/dao/NicDao.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ public interface NicDao extends GenericDao<NicVO, Long> {

List<NicVO> listByNetworkId(long networkId);

List<NicVO> listNonDeallocatedByNetworkId(long networkId);

NicVO findByNtwkIdAndInstanceId(long networkId, long instanceId);

NicVO findByInstanceIdAndNetworkIdIncludingRemoved(long networkId, long instanceId);
Expand Down
10 changes: 10 additions & 0 deletions engine/schema/src/main/java/com/cloud/vm/dao/NicDaoImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@
NonReleasedSearch.and("instance", NonReleasedSearch.entity().getInstanceId(), Op.EQ);
NonReleasedSearch.and("network", NonReleasedSearch.entity().getNetworkId(), Op.EQ);
NonReleasedSearch.and("state", NonReleasedSearch.entity().getState(), Op.NOTIN);
NonReleasedSearch.and("strategy", NonReleasedSearch.entity().getReservationStrategy(), Op.NEQ);
NonReleasedSearch.done();

deviceIdSearch = createSearchBuilder(Integer.class);
Expand Down Expand Up @@ -151,6 +152,15 @@
return listBy(sc);
}

@Override
public List<NicVO> listNonDeallocatedByNetworkId(long networkId) {
SearchCriteria<NicVO> sc = NonReleasedSearch.create();
sc.setParameters("network", networkId);
sc.setParameters("state", Nic.State.Deallocating);
sc.setParameters("strategy", Nic.ReservationStrategy.PlaceHolder.toString());
return listBy(sc);
}

Check warning on line 162 in engine/schema/src/main/java/com/cloud/vm/dao/NicDaoImpl.java

View check run for this annotation

Codecov / codecov/patch

engine/schema/src/main/java/com/cloud/vm/dao/NicDaoImpl.java#L156-L162

Added lines #L156 - L162 were not covered by tests

@Override
public NicVO findByNtwkIdAndInstanceId(long networkId, long instanceId) {
SearchCriteria<NicVO> sc = AllFieldsSearch.create();
Expand Down
66 changes: 55 additions & 11 deletions server/src/main/java/com/cloud/network/NetworkServiceImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -3090,15 +3090,18 @@
if (dc.getNetworkType() == NetworkType.Basic) {
throw new InvalidParameterValueException("Guest VM CIDR can't be specified for zone with " + NetworkType.Basic + " networking");
}
if (network.getGuestType() != GuestType.Isolated) {
throw new InvalidParameterValueException("Can only allow IP Reservation in networks with guest type " + GuestType.Isolated);
if (network.getGuestType() != GuestType.Isolated && network.getGuestType() != GuestType.Shared) {

Check warning on line 3093 in server/src/main/java/com/cloud/network/NetworkServiceImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/NetworkServiceImpl.java#L3093

Added line #L3093 was not covered by tests
throw new InvalidParameterValueException("Can only allow IP Reservation in networks with guest types: " + GuestType.Isolated + " or " + GuestType.Shared);
}
if (networkOfferingChanged) {
throw new InvalidParameterValueException("Cannot specify this network offering change and guestVmCidr at same time. Specify only one.");
}
if (network.getState() != Network.State.Implemented && network.getState() != Network.State.Allocated) {
throw new InvalidParameterValueException(String.format("The network must be in %s or %s state. IP Reservation cannot be applied in %s state",
Network.State.Implemented, Network.State.Allocated, network.getState()));
if (network.getGuestType() != GuestType.Isolated && network.getGuestType() != GuestType.Shared) {
throw new InvalidParameterValueException("Can only allow IP Reservation in networks with guest types: " + GuestType.Isolated + " or " + GuestType.Shared);

Check warning on line 3100 in server/src/main/java/com/cloud/network/NetworkServiceImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/NetworkServiceImpl.java#L3100

Added line #L3100 was not covered by tests
}
if (network.getState() != Network.State.Implemented && network.getState() != Network.State.Allocated && network.getState() != Network.State.Setup) {
throw new InvalidParameterValueException(String.format("The network must be in %s, $s or %s state. IP Reservation cannot be applied in %s state",

Check warning on line 3103 in server/src/main/java/com/cloud/network/NetworkServiceImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/NetworkServiceImpl.java#L3103

Added line #L3103 was not covered by tests
Network.State.Implemented, Network.State.Allocated, Network.State.Setup, network.getState()));
}
if (!NetUtils.isValidIp4Cidr(guestVmCidr)) {
throw new InvalidParameterValueException("Invalid format of Guest VM CIDR.");
Expand All @@ -3111,34 +3114,57 @@
// But in case networkCidr is a non null value (IP reservation already exists), it implies network cidr is networkCidr
if (networkCidr != null) {
if (!NetUtils.isNetworkAWithinNetworkB(guestVmCidr, networkCidr)) {
throw new InvalidParameterValueException("Invalid value of Guest VM CIDR. For IP Reservation, Guest VM CIDR should be a subset of network CIDR : " + networkCidr);
throw new InvalidParameterValueException("Invalid value of Guest VM CIDR. For IP Reservation, Guest VM CIDR should be a subset of network CIDR: " + networkCidr);
}
} else {
if (!NetUtils.isNetworkAWithinNetworkB(guestVmCidr, network.getCidr())) {
throw new InvalidParameterValueException("Invalid value of Guest VM CIDR. For IP Reservation, Guest VM CIDR should be a subset of network CIDR : " + network.getCidr());
throw new InvalidParameterValueException("Invalid value of Guest VM CIDR. For IP Reservation, Guest VM CIDR should be a subset of network CIDR: " + network.getCidr());
}
}

// This check makes sure there are no active IPs existing outside the guestVmCidr in the network
String[] guestVmCidrPair = guestVmCidr.split("\\/");
Long size = Long.valueOf(guestVmCidrPair[1]);
List<NicVO> nicsPresent = _nicDao.listByNetworkId(networkId);

String cidrIpRange[] = NetUtils.getIpRangeFromCidr(guestVmCidrPair[0], size);
s_logger.info("The start IP of the specified guest vm cidr is: " + cidrIpRange[0] + " and end IP is: " + cidrIpRange[1]);
long startIp = NetUtils.ip2Long(cidrIpRange[0]);
long endIp = NetUtils.ip2Long(cidrIpRange[1]);
long range = endIp - startIp + 1;
s_logger.info("The specified guest vm cidr has " + range + " IPs");

for (NicVO nic : nicsPresent) {
List<NicVO> nonDellocatedNicsPresent = _nicDao.listNonDeallocatedByNetworkId(networkId);
if (network.getGuestType() == GuestType.Shared) {
if (CollectionUtils.isNotEmpty(nonDellocatedNicsPresent)) {
throw new InvalidParameterValueException("IPs are in use, cannot apply reservation");
}

Check warning on line 3139 in server/src/main/java/com/cloud/network/NetworkServiceImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/NetworkServiceImpl.java#L3135-L3139

Added lines #L3135 - L3139 were not covered by tests
List<VlanVO> vlans = _vlanDao.listVlansByNetworkId(networkId);
if (CollectionUtils.isNotEmpty(vlans)) {

Check warning on line 3141 in server/src/main/java/com/cloud/network/NetworkServiceImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/NetworkServiceImpl.java#L3141

Added line #L3141 was not covered by tests
for (VlanVO vlan : vlans) {
if (vlan == null) {
continue;

Check warning on line 3144 in server/src/main/java/com/cloud/network/NetworkServiceImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/NetworkServiceImpl.java#L3144

Added line #L3144 was not covered by tests
}
String vlanIpRange = vlan.getIpRange();

Check warning on line 3146 in server/src/main/java/com/cloud/network/NetworkServiceImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/NetworkServiceImpl.java#L3146

Added line #L3146 was not covered by tests
if (vlanIpRange == null) {
continue;
}
String[] vlanRange = vlanIpRange.split("-");

Check warning on line 3150 in server/src/main/java/com/cloud/network/NetworkServiceImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/NetworkServiceImpl.java#L3150

Added line #L3150 was not covered by tests
String vlanStartIP = vlanRange[0];
String vlanEndIP = vlanRange[1];

Check warning on line 3152 in server/src/main/java/com/cloud/network/NetworkServiceImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/NetworkServiceImpl.java#L3152

Added line #L3152 was not covered by tests
if (!NetUtils.isIpWithInCidrRange(vlanStartIP, guestVmCidr) || !NetUtils.isIpWithInCidrRange(vlanEndIP, guestVmCidr)) {
throw new InvalidParameterValueException(String.format("CIDR doesn't include the IP range %s, cannot apply reservation", vlanIpRange));

Check warning on line 3154 in server/src/main/java/com/cloud/network/NetworkServiceImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/NetworkServiceImpl.java#L3154

Added line #L3154 was not covered by tests
}
}
}
}

Check warning on line 3158 in server/src/main/java/com/cloud/network/NetworkServiceImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/NetworkServiceImpl.java#L3156-L3158

Added lines #L3156 - L3158 were not covered by tests

for (NicVO nic : nonDellocatedNicsPresent) {

Check warning on line 3160 in server/src/main/java/com/cloud/network/NetworkServiceImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/NetworkServiceImpl.java#L3160

Added line #L3160 was not covered by tests
if (nic.getIPv4Address() == null) {
continue;
}
long nicIp = NetUtils.ip2Long(nic.getIPv4Address());
//check if nic IP is outside the guest vm cidr
if ((nicIp < startIp || nicIp > endIp) && nic.getState() != Nic.State.Deallocating) {
throw new InvalidParameterValueException("Active IPs like " + nic.getIPv4Address() + " exist outside the Guest VM CIDR. Cannot apply reservation ");
throw new InvalidParameterValueException("Active IPs like " + nic.getIPv4Address() + " exist outside the Guest VM CIDR. Cannot apply reservation");
}
}

Expand Down Expand Up @@ -3171,6 +3197,24 @@
s_logger.warn("Guest VM CIDR and Network CIDR both are same, reservation will reset.");
network.setNetworkCidr(null);
}

// Remove any placeholder nic before updating the cidr
final List<NicVO> placeholderNics = _nicDao.listPlaceholderNicsByNetworkId(network.getId());
if (CollectionUtils.isNotEmpty(placeholderNics)) {
for (NicVO nic : placeholderNics) {

Check warning on line 3204 in server/src/main/java/com/cloud/network/NetworkServiceImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/NetworkServiceImpl.java#L3203-L3204

Added lines #L3203 - L3204 were not covered by tests
if (nic.getIPv4Address() != null) {
s_logger.debug("Releasing ip " + nic.getIPv4Address() + " of placeholder nic " + nic);
IPAddressVO ip = _ipAddressDao.findByIpAndSourceNetworkId(nic.getNetworkId(), nic.getIPv4Address());
if (ip != null) {

Check warning on line 3208 in server/src/main/java/com/cloud/network/NetworkServiceImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/NetworkServiceImpl.java#L3208

Added line #L3208 was not covered by tests
_ipAddrMgr.markIpAsUnavailable(ip.getId());
_ipAddressDao.unassignIpAddress(ip.getId());
}
}
s_logger.debug("Removing placeholder nic " + nic);

Check warning on line 3213 in server/src/main/java/com/cloud/network/NetworkServiceImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/NetworkServiceImpl.java#L3212-L3213

Added lines #L3212 - L3213 were not covered by tests
_nicDao.remove(nic.getId());
}
}

Check warning on line 3216 in server/src/main/java/com/cloud/network/NetworkServiceImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/NetworkServiceImpl.java#L3215-L3216

Added lines #L3215 - L3216 were not covered by tests

// Finally update "cidr" with the guestVmCidr
// which becomes the effective address space for CloudStack guest VMs
network.setCidr(guestVmCidr);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,7 @@
nic.setIpv4AllocationRaceCheck(true);
}
if (guestIp == null && network.getGuestType() != GuestType.L2 && !_networkModel.listNetworkOfferingServices(network.getNetworkOfferingId()).isEmpty()) {
throw new InsufficientVirtualNetworkCapacityException("Unable to acquire Guest IP" + " address for network " + network, DataCenter.class,
throw new InsufficientVirtualNetworkCapacityException("Unable to acquire Guest IP address for network " + network, DataCenter.class,

Check warning on line 447 in server/src/main/java/com/cloud/network/guru/GuestNetworkGuru.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/guru/GuestNetworkGuru.java#L447

Added line #L447 was not covered by tests
dc.getId());
}
}
Expand Down
Loading