Skip to content

Commit 883333c

Browse files
murali-reddymurali-reddy
committed
CLOUDSTACK-2700:on network/vpc delete, portable IP should be still
associated with account Unlike public ip which gets dis-associated (released) with the account on network/VPC delete, portable IP should continue to be associated with the account even when the network/VPC with which it is currently associated in deleted. This fix ensures portable IP are associated to account even after network/vpc is deleted.
1 parent ad48c83 commit 883333c

9 files changed

Lines changed: 56 additions & 24 deletions

File tree

‎api/src/com/cloud/network/NetworkService.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ IpAddress allocateIP(Account ipOwner, long zoneId, Long networkId) throws Resour
5555
IpAddress allocatePortableIP(Account ipOwner, int regionId, Long zoneId, Long networkId, Long vpcId) throws ResourceAllocationException,
5656
InsufficientAddressCapacityException, ConcurrentOperationException;
5757

58-
boolean releasePortableIpAddress(long ipAddressId) throws InsufficientAddressCapacityException;
58+
boolean releasePortableIpAddress(long ipAddressId);
5959

6060
Network createGuestNetwork(CreateNetworkCmd cmd) throws InsufficientCapacityException, ConcurrentOperationException,
6161
ResourceAllocationException;

‎server/src/com/cloud/network/NetworkManager.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,11 @@ NicProfile prepareNic(VirtualMachineProfile<? extends VMInstanceVO> vmProfile, D
268268
IPAddressVO associateIPToGuestNetwork(long ipAddrId, long networkId, boolean releaseOnFailure) throws ResourceAllocationException, ResourceUnavailableException,
269269
InsufficientAddressCapacityException, ConcurrentOperationException;
270270

271+
IpAddress allocatePortableIp(Account ipOwner, Account caller, long dcId, Long networkId, Long vpcID)
272+
throws ConcurrentOperationException, ResourceAllocationException, InsufficientAddressCapacityException;
273+
274+
boolean releasePortableIpAddress(long addrId);
275+
271276
IPAddressVO associatePortableIPToGuestNetwork(long ipAddrId, long networkId, boolean releaseOnFailure) throws ResourceAllocationException, ResourceUnavailableException,
272277
InsufficientAddressCapacityException, ConcurrentOperationException;
273278

@@ -362,10 +367,6 @@ void implementNetworkElementsAndResources(DeployDestination dest,
362367
IpAddress allocateIp(Account ipOwner, boolean isSystem, Account caller, long callerId,
363368
DataCenter zone) throws ConcurrentOperationException, ResourceAllocationException, InsufficientAddressCapacityException;
364369

365-
366-
IpAddress allocatePortableIp(Account ipOwner, Account caller, long dcId, Long networkId, Long vpcID)
367-
throws ConcurrentOperationException, ResourceAllocationException, InsufficientAddressCapacityException;
368-
369370
Map<String, String> finalizeServicesAndProvidersForNetwork(NetworkOffering offering,
370371
Long physicalNetworkId);
371372

‎server/src/com/cloud/network/NetworkManagerImpl.java‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1118,7 +1118,8 @@ public boolean disassociatePublicIpAddress(long addrId, long userId, Account cal
11181118
}
11191119

11201120
@DB
1121-
private void releasePortableIpAddress(long addrId) {
1121+
@Override
1122+
public boolean releasePortableIpAddress(long addrId) {
11221123
Transaction txn = Transaction.currentTxn();
11231124
GlobalLock portableIpLock = GlobalLock.getInternLock("PortablePublicIpRange");
11241125

@@ -1133,12 +1134,13 @@ private void releasePortableIpAddress(long addrId) {
11331134

11341135
// removed the provisioned vlan
11351136
VlanVO vlan = _vlanDao.findById(ip.getVlanId());
1136-
_vlanDao.expunge(vlan.getId());
1137+
_vlanDao.remove(vlan.getId());
11371138

11381139
// remove the provisioned public ip address
1139-
_ipAddressDao.expunge(ip.getId());
1140+
_ipAddressDao.remove(ip.getId());
11401141

11411142
txn.commit();
1143+
return true;
11421144
} finally {
11431145
portableIpLock.releaseRef();
11441146
}
@@ -3537,8 +3539,16 @@ private boolean cleanupNetworkResources(long networkId, Account caller, long cal
35373539
List<IPAddressVO> ipsToRelease = _ipAddressDao.listByAssociatedNetwork(networkId, null);
35383540
for (IPAddressVO ipToRelease : ipsToRelease) {
35393541
if (ipToRelease.getVpcId() == null) {
3540-
IPAddressVO ip = markIpAsUnavailable(ipToRelease.getId());
3541-
assert (ip != null) : "Unable to mark the ip address id=" + ipToRelease.getId() + " as unavailable.";
3542+
if (!ipToRelease.isPortable()) {
3543+
IPAddressVO ip = markIpAsUnavailable(ipToRelease.getId());
3544+
assert (ip != null) : "Unable to mark the ip address id=" + ipToRelease.getId() + " as unavailable.";
3545+
} else {
3546+
// portable IP address are associated with owner, until explicitly requested to be disassociated
3547+
// so as part of network clean up just break IP association with guest network
3548+
ipToRelease.setAssociatedWithNetworkId(null);
3549+
_ipAddressDao.update(ipToRelease.getId(), ipToRelease);
3550+
s_logger.debug("Portable IP address " + ipToRelease + " is no longer associated with any network");
3551+
}
35423552
} else {
35433553
_vpcMgr.unassignIPFromVpcNetwork(ipToRelease.getId(), network.getId());
35443554
}

‎server/src/com/cloud/network/NetworkServiceImpl.java‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -591,8 +591,12 @@ public IpAddress allocatePortableIP(Account ipOwner, int regionId, Long zoneId,
591591

592592
@Override
593593
@ActionEvent(eventType = EventTypes.EVENT_PORTABLE_IP_RELEASE, eventDescription = "disassociating portable Ip", async = true)
594-
public boolean releasePortableIpAddress(long ipAddressId) throws InsufficientAddressCapacityException {
595-
return releaseIpAddressInternal(ipAddressId);
594+
public boolean releasePortableIpAddress(long ipAddressId) {
595+
try {
596+
return releaseIpAddressInternal(ipAddressId);
597+
} catch (Exception e) {
598+
return false;
599+
}
596600
}
597601

598602
@Override
@@ -880,7 +884,7 @@ private boolean releaseIpAddressInternal(long ipAddressId) throws InsufficientAd
880884
boolean success = _networkMgr.disassociatePublicIpAddress(ipAddressId, userId, caller);
881885

882886
if (success) {
883-
if (!ipVO.isPortable()) {
887+
if (ipVO.isPortable()) {
884888
return success;
885889
}
886890
Long networkId = ipVO.getAssociatedWithNetworkId();

‎server/src/com/cloud/network/rules/RulesManagerImpl.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -490,10 +490,9 @@ private boolean enableStaticNat(long ipId, long vmId, long networkId, boolean is
490490
"a part of enable static nat");
491491
return false;
492492
}
493-
performedIpAssoc = true;
494493
} else if (ipAddress.isPortable()) {
495-
s_logger.info("Portable IP " + ipAddress.getUuid() + " is not associated with the network, so" +
496-
"associate IP with the network " + networkId);
494+
s_logger.info("Portable IP " + ipAddress.getUuid() + " is not associated with the network yet "
495+
+ " so associate IP with the network " + networkId);
497496
try {
498497
// check if StaticNat service is enabled in the network
499498
_networkModel.checkIpForService(ipAddress, Service.StaticNat, networkId);
@@ -504,13 +503,12 @@ private boolean enableStaticNat(long ipId, long vmId, long networkId, boolean is
504503
}
505504

506505
// associate portable IP with guest network
507-
_networkMgr.associatePortableIPToGuestNetwork(ipId, networkId, false);
506+
ipAddress = _networkMgr.associatePortableIPToGuestNetwork(ipId, networkId, false);
508507
} catch (Exception e) {
509508
s_logger.warn("Failed to associate portable id=" + ipId + " to network id=" + networkId + " as " +
510509
"a part of enable static nat");
511510
return false;
512511
}
513-
performedIpAssoc = true;
514512
}
515513
} else if (ipAddress.getAssociatedWithNetworkId() != networkId) {
516514
if (ipAddress.isPortable()) {
@@ -520,14 +518,16 @@ private boolean enableStaticNat(long ipId, long vmId, long networkId, boolean is
520518
// check if portable IP can be transferred across the networks
521519
if (_networkMgr.isPortableIpTransferableFromNetwork(ipId, ipAddress.getAssociatedWithNetworkId() )) {
522520
try {
521+
// transfer the portable IP and refresh IP details
523522
_networkMgr.transferPortableIP(ipId, ipAddress.getAssociatedWithNetworkId(), networkId);
523+
ipAddress = _ipAddressDao.findById(ipId);
524524
} catch (Exception e) {
525525
s_logger.warn("Failed to associate portable id=" + ipId + " to network id=" + networkId + " as " +
526526
"a part of enable static nat");
527527
return false;
528528
}
529529
} else {
530-
throw new InvalidParameterValueException("Portable IP: " + ipId + " has associated services" +
530+
throw new InvalidParameterValueException("Portable IP: " + ipId + " has associated services " +
531531
"in network " + ipAddress.getAssociatedWithNetworkId() + " so can not be transferred to " +
532532
" network " + networkId);
533533
}

‎server/src/com/cloud/network/vpc/VpcManagerImpl.java‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1217,9 +1217,18 @@ public boolean cleanupVpcResources(long vpcId, Account caller, long callerUserId
12171217
List<IPAddressVO> ipsToRelease = _ipAddressDao.listByAssociatedVpc(vpcId, null);
12181218
s_logger.debug("Releasing ips for vpc id=" + vpcId + " as a part of vpc cleanup");
12191219
for (IPAddressVO ipToRelease : ipsToRelease) {
1220-
success = success && _ntwkMgr.disassociatePublicIpAddress(ipToRelease.getId(), callerUserId, caller);
1221-
if (!success) {
1222-
s_logger.warn("Failed to cleanup ip " + ipToRelease + " as a part of vpc id=" + vpcId + " cleanup");
1220+
if (ipToRelease.isPortable()) {
1221+
// portable IP address are associated with owner, until explicitly requested to be disassociated.
1222+
// so as part of VPC clean up just break IP association with VPC
1223+
ipToRelease.setVpcId(null);
1224+
ipToRelease.setAssociatedWithNetworkId(null);
1225+
_ipAddressDao.update(ipToRelease.getId(), ipToRelease);
1226+
s_logger.debug("Portable IP address " + ipToRelease + " is no longer associated with any VPC");
1227+
} else {
1228+
success = success && _ntwkMgr.disassociatePublicIpAddress(ipToRelease.getId(), callerUserId, caller);
1229+
if (!success) {
1230+
s_logger.warn("Failed to cleanup ip " + ipToRelease + " as a part of vpc id=" + vpcId + " cleanup");
1231+
}
12231232
}
12241233
}
12251234

‎server/src/com/cloud/user/AccountManagerImpl.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -731,6 +731,14 @@ public boolean cleanupAccount(AccountVO account, long callerUserId, Account call
731731
_resourceCountDao.removeEntriesByOwner(accountId, ResourceOwnerType.Account);
732732
_resourceLimitDao.removeEntriesByOwner(accountId, ResourceOwnerType.Account);
733733

734+
// release account specific acquired portable IP's. Since all the portable IP's must have been already
735+
// disassociated with VPC/guest network (due to deletion), so just mark portable IP as free.
736+
List<? extends IpAddress> portableIpsToRelease = _ipAddressDao.listByAccount(accountId);
737+
for (IpAddress ip : portableIpsToRelease) {
738+
s_logger.debug("Releasing portable ip " + ip + " as a part of account id=" + accountId + " cleanup");
739+
_networkMgr.releasePortableIpAddress(ip.getId());
740+
}
741+
734742
return true;
735743
} catch (Exception ex) {
736744
s_logger.warn("Failed to cleanup account " + account + " due to ", ex);

‎server/test/com/cloud/network/MockNetworkManagerImpl.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -887,7 +887,7 @@ public IpAddress allocatePortableIP(Account ipOwner, int regionId, Long zoneId,
887887
}
888888

889889
@Override
890-
public boolean releasePortableIpAddress(long ipAddressId) throws InsufficientAddressCapacityException {
890+
public boolean releasePortableIpAddress(long ipAddressId) {
891891
return false;// TODO Auto-generated method stub
892892
}
893893

‎server/test/com/cloud/vpc/MockNetworkManagerImpl.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ public IpAddress allocatePortableIP(Account ipOwner, int regionId, Long zoneId,
210210
}
211211

212212
@Override
213-
public boolean releasePortableIpAddress(long ipAddressId) throws InsufficientAddressCapacityException {
213+
public boolean releasePortableIpAddress(long ipAddressId) {
214214
return false;// TODO Auto-generated method stub
215215
}
216216

0 commit comments

Comments
 (0)