From a4b0759c5267c74aceb543bdde6919b6b046144f Mon Sep 17 00:00:00 2001 From: Murali reddy Date: Mon, 21 May 2012 20:08:46 +0530 Subject: [PATCH] bug CS-14862: EIP/ELB - SSVM and CPVM should be given an ip address from the public ip address range. With this fix both SSVM and CPVM will get public IP's in case of basic zone with EIP service. A static NAT rule is implicitly configured on the EIP service provider to map public IP to a guest IP address associated with SSVM/CPVM --- .../api/commands/AssociateIPAddrCmd.java | 2 +- .../api/commands/EnableStaticNatCmd.java | 2 +- api/src/com/cloud/network/NetworkService.java | 2 +- .../com/cloud/network/rules/RulesService.java | 2 +- .../consoleproxy/ConsoleProxyManagerImpl.java | 36 +++++++++++++++++- .../com/cloud/network/NetworkManagerImpl.java | 12 +++++- .../com/cloud/network/rules/RulesManager.java | 3 +- .../cloud/network/rules/RulesManagerImpl.java | 25 +++++++------ .../SecondaryStorageManagerImpl.java | 34 ++++++++++++++++- .../storage/upload/UploadMonitorImpl.java | 37 +++++++++++-------- 10 files changed, 119 insertions(+), 36 deletions(-) diff --git a/api/src/com/cloud/api/commands/AssociateIPAddrCmd.java b/api/src/com/cloud/api/commands/AssociateIPAddrCmd.java index e0d721359d8..2c9737efdb3 100644 --- a/api/src/com/cloud/api/commands/AssociateIPAddrCmd.java +++ b/api/src/com/cloud/api/commands/AssociateIPAddrCmd.java @@ -168,7 +168,7 @@ public class AssociateIPAddrCmd extends BaseAsyncCreateCmd { @Override public void create() throws ResourceAllocationException{ try { - IpAddress ip = _networkService.allocateIP(getNetworkId(), _accountService.getAccount(getEntityOwnerId()), false); + IpAddress ip = _networkService.allocateIP(getNetworkId(), _accountService.getAccount(getEntityOwnerId())); if (ip != null) { this.setEntityId(ip.getId()); } else { diff --git a/api/src/com/cloud/api/commands/EnableStaticNatCmd.java b/api/src/com/cloud/api/commands/EnableStaticNatCmd.java index e62d963c097..9e649ec9d08 100644 --- a/api/src/com/cloud/api/commands/EnableStaticNatCmd.java +++ b/api/src/com/cloud/api/commands/EnableStaticNatCmd.java @@ -78,7 +78,7 @@ public class EnableStaticNatCmd extends BaseCmd{ @Override public void execute() throws ResourceUnavailableException{ try { - boolean result = _rulesService.enableStaticNat(ipAddressId, virtualMachineId); + boolean result = _rulesService.enableStaticNat(ipAddressId, virtualMachineId, false); if (result) { SuccessResponse response = new SuccessResponse(getCommandName()); this.setResponseObject(response); diff --git a/api/src/com/cloud/network/NetworkService.java b/api/src/com/cloud/network/NetworkService.java index 162104417ee..048d7145b6f 100755 --- a/api/src/com/cloud/network/NetworkService.java +++ b/api/src/com/cloud/network/NetworkService.java @@ -37,7 +37,7 @@ public interface NetworkService { List getIsolatedNetworksOwnedByAccountInZone(long zoneId, Account owner); - IpAddress allocateIP(long networkId, Account ipOwner, boolean isSystem) throws ResourceAllocationException, InsufficientAddressCapacityException, ConcurrentOperationException; + IpAddress allocateIP(long networkId, Account ipOwner) throws ResourceAllocationException, InsufficientAddressCapacityException, ConcurrentOperationException; /** * Associates a public IP address for a router. diff --git a/api/src/com/cloud/network/rules/RulesService.java b/api/src/com/cloud/network/rules/RulesService.java index 137c10c35fc..cde28dbfd52 100644 --- a/api/src/com/cloud/network/rules/RulesService.java +++ b/api/src/com/cloud/network/rules/RulesService.java @@ -60,7 +60,7 @@ public interface RulesService { boolean applyPortForwardingRules(long ipAdddressId, Account caller) throws ResourceUnavailableException; - boolean enableStaticNat(long ipAddressId, long vmId) throws NetworkRuleConflictException, ResourceUnavailableException; + boolean enableStaticNat(long ipAddressId, long vmId, boolean isSystemVm) throws NetworkRuleConflictException, ResourceUnavailableException; PortForwardingRule getPortForwardigRule(long ruleId); diff --git a/server/src/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java b/server/src/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java index 9bd2d9cfeaa..b6c1057a61a 100755 --- a/server/src/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java +++ b/server/src/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java @@ -81,10 +81,13 @@ import com.cloud.info.RunningHostInfoAgregator.ZoneHostInfo; import com.cloud.keystore.KeystoreDao; import com.cloud.keystore.KeystoreManager; import com.cloud.keystore.KeystoreVO; +import com.cloud.network.IPAddressVO; import com.cloud.network.NetworkManager; import com.cloud.network.NetworkVO; import com.cloud.network.Networks.TrafficType; +import com.cloud.network.dao.IPAddressDao; import com.cloud.network.dao.NetworkDao; +import com.cloud.network.rules.RulesManager; import com.cloud.offering.ServiceOffering; import com.cloud.offerings.NetworkOfferingVO; import com.cloud.offerings.dao.NetworkOfferingDao; @@ -108,6 +111,7 @@ import com.cloud.storage.dao.VMTemplateHostDao; import com.cloud.user.Account; import com.cloud.user.AccountManager; import com.cloud.user.User; +import com.cloud.user.UserContext; import com.cloud.utils.DateUtil; import com.cloud.utils.NumbersUtil; import com.cloud.utils.Pair; @@ -129,6 +133,7 @@ import com.cloud.uuididentity.dao.IdentityDao; import com.cloud.vm.ConsoleProxyVO; import com.cloud.vm.NicProfile; import com.cloud.vm.ReservationContext; +import com.cloud.vm.SecondaryStorageVmVO; import com.cloud.vm.SystemVmLoadScanHandler; import com.cloud.vm.SystemVmLoadScanner; import com.cloud.vm.SystemVmLoadScanner.AfterScanAction; @@ -214,7 +219,11 @@ public class ConsoleProxyManagerImpl implements ConsoleProxyManager, ConsoleProx IdentityDao _identityDao; @Inject NetworkDao _networkDao; - + @Inject + RulesManager _rulesMgr; + @Inject + IPAddressDao _ipAddressDao; + private ConsoleProxyListener _listener; private ServiceOfferingVO _serviceOffering; @@ -1677,6 +1686,21 @@ public class ConsoleProxyManagerImpl implements ConsoleProxyManager, ConsoleProx return false; } + try { + //get system ip and create static nat rule for the vm in case of basic networking with EIP/ELB + _rulesMgr.getSystemIpAndEnableStaticNatForVm(profile.getVirtualMachine(), false); + IPAddressVO ipaddr = _ipAddressDao.findByAssociatedVmId(profile.getVirtualMachine().getId()); + if (ipaddr != null && ipaddr.getSystem()) { + ConsoleProxyVO consoleVm = profile.getVirtualMachine(); + // override CPVM guest IP with EIP, so that console url's will be prepared with EIP + consoleVm.setPublicIpAddress(ipaddr.getAddress().addr()); + _consoleProxyDao.update(consoleVm.getId(), consoleVm); + } + } catch (Exception ex) { + s_logger.warn("Failed to get system ip and enable static nat for the vm " + profile.getVirtualMachine() + " due to exception ", ex); + return false; + } + return true; } @@ -1757,6 +1781,16 @@ public class ConsoleProxyManagerImpl implements ConsoleProxyManager, ConsoleProx @Override public void finalizeStop(VirtualMachineProfile profile, StopAnswer answer) { + //release elastic IP here if assigned + IPAddressVO ip = _ipAddressDao.findByAssociatedVmId(profile.getId()); + if (ip != null && ip.getSystem()) { + UserContext ctx = UserContext.current(); + try { + _rulesMgr.disableStaticNat(ip.getId(), ctx.getCaller(), ctx.getCallerUserId(), true); + } catch (Exception ex) { + s_logger.warn("Failed to disable static nat and release system ip " + ip + " as a part of vm " + profile.getVirtualMachine() + " stop due to exception ", ex); + } + } } @Override diff --git a/server/src/com/cloud/network/NetworkManagerImpl.java b/server/src/com/cloud/network/NetworkManagerImpl.java index 235771baf87..fca654eb57c 100755 --- a/server/src/com/cloud/network/NetworkManagerImpl.java +++ b/server/src/com/cloud/network/NetworkManagerImpl.java @@ -972,8 +972,12 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag } @Override - @DB @ActionEvent(eventType = EventTypes.EVENT_NET_IP_ASSIGN, eventDescription = "allocating Ip", create = true) + public IpAddress allocateIP(long networkId, Account ipOwner) throws ResourceAllocationException, InsufficientAddressCapacityException, ConcurrentOperationException { + return allocateIP(networkId, ipOwner, false); + } + + @DB public IpAddress allocateIP(long networkId, Account ipOwner, boolean isSystem) throws ResourceAllocationException, InsufficientAddressCapacityException, ConcurrentOperationException { Account caller = UserContext.current().getCaller(); long userId = UserContext.current().getCallerUserId(); @@ -1080,9 +1084,13 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag } @Override - @DB @ActionEvent(eventType = EventTypes.EVENT_NET_IP_ASSIGN, eventDescription = "associating Ip", async = true) public IpAddress associateIP(long ipId) throws ResourceAllocationException, ResourceUnavailableException, InsufficientAddressCapacityException, ConcurrentOperationException { + return associateIP(ipId, false); + } + + @DB + private IpAddress associateIP(long ipId, boolean isSystem) throws ResourceAllocationException, ResourceUnavailableException, InsufficientAddressCapacityException, ConcurrentOperationException { Account caller = UserContext.current().getCaller(); Account owner = null; diff --git a/server/src/com/cloud/network/rules/RulesManager.java b/server/src/com/cloud/network/rules/RulesManager.java index c9c7dec6a6b..a188bd1e776 100644 --- a/server/src/com/cloud/network/rules/RulesManager.java +++ b/server/src/com/cloud/network/rules/RulesManager.java @@ -20,6 +20,7 @@ import com.cloud.exception.ResourceUnavailableException; import com.cloud.network.IpAddress; import com.cloud.user.Account; import com.cloud.uservm.UserVm; +import com.cloud.vm.VirtualMachine; /** * Rules Manager manages the network rules created for different networks. @@ -69,7 +70,7 @@ public interface RulesManager extends RulesService { boolean applyStaticNatsForNetwork(long networkId, boolean continueOnError, Account caller); - void getSystemIpAndEnableStaticNatForVm(UserVm vm, boolean getNewIp) throws InsufficientAddressCapacityException; + void getSystemIpAndEnableStaticNatForVm(VirtualMachine vm, boolean getNewIp) throws InsufficientAddressCapacityException; boolean disableStaticNat(long ipAddressId, Account caller, long callerUserId, boolean releaseIpIfElastic) throws ResourceUnavailableException; diff --git a/server/src/com/cloud/network/rules/RulesManagerImpl.java b/server/src/com/cloud/network/rules/RulesManagerImpl.java index e3d9d323d01..f9397840a1a 100755 --- a/server/src/com/cloud/network/rules/RulesManagerImpl.java +++ b/server/src/com/cloud/network/rules/RulesManagerImpl.java @@ -69,6 +69,7 @@ import com.cloud.utils.AnnotationHelper; import com.cloud.vm.Nic; import com.cloud.vm.UserVmVO; import com.cloud.vm.VirtualMachine; +import com.cloud.vm.VirtualMachine.Type; import com.cloud.vm.dao.NicDao; import com.cloud.vm.dao.UserVmDao; @@ -322,23 +323,24 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { } @Override - public boolean enableStaticNat(long ipId, long vmId) throws NetworkRuleConflictException, ResourceUnavailableException { + public boolean enableStaticNat(long ipId, long vmId, boolean isSystemVm) throws NetworkRuleConflictException, ResourceUnavailableException { UserContext ctx = UserContext.current(); Account caller = ctx.getCaller(); - // Verify input parameters - UserVmVO vm = _vmDao.findById(vmId); - if (vm == null) { - throw new InvalidParameterValueException("Can't enable static nat for the address id=" + ipId + ", invalid virtual machine id specified (" + vmId + ")."); - } - IPAddressVO ipAddress = _ipAddressDao.findById(ipId); if (ipAddress == null) { throw new InvalidParameterValueException("Unable to find ip address by id " + ipId); } - // Check permissions - checkIpAndUserVm(ipAddress, vm, caller); + // Verify input parameters + if (!isSystemVm) { + UserVmVO vm = _vmDao.findById(vmId); + if (vm == null) { + throw new InvalidParameterValueException("Can't enable static nat for the address id=" + ipId + ", invalid virtual machine id specified (" + vmId + ")."); + } + // Check permissions + checkIpAndUserVm(ipAddress, vm, caller); + } // Verify that the ip is associated with the network and static nat service is supported for the network Long networkId = ipAddress.getAssociatedWithNetworkId(); @@ -1188,7 +1190,7 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { } @Override - public void getSystemIpAndEnableStaticNatForVm(UserVm vm, boolean getNewIp) throws InsufficientAddressCapacityException { + public void getSystemIpAndEnableStaticNatForVm(VirtualMachine vm, boolean getNewIp) throws InsufficientAddressCapacityException { boolean success = true; // enable static nat if eIp capability is supported @@ -1212,8 +1214,9 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { s_logger.debug("Allocated system ip " + ip + ", now enabling static nat on it for vm " + vm); + boolean isSystemVM = (vm.getType() == Type.ConsoleProxy || vm.getType() == Type.SecondaryStorageVm); try { - success = enableStaticNat(ip.getId(), vm.getId()); + success = enableStaticNat(ip.getId(), vm.getId(), isSystemVM); } catch (NetworkRuleConflictException ex) { s_logger.warn("Failed to enable static nat as a part of enabling elasticIp and staticNat for vm " + vm + " in guest network " + guestNetwork + " due to exception ", ex); success = false; diff --git a/server/src/com/cloud/storage/secondary/SecondaryStorageManagerImpl.java b/server/src/com/cloud/storage/secondary/SecondaryStorageManagerImpl.java index e6bb237bedf..29a978244ee 100755 --- a/server/src/com/cloud/storage/secondary/SecondaryStorageManagerImpl.java +++ b/server/src/com/cloud/storage/secondary/SecondaryStorageManagerImpl.java @@ -68,10 +68,13 @@ import com.cloud.info.RunningHostCountInfo; import com.cloud.info.RunningHostInfoAgregator; import com.cloud.info.RunningHostInfoAgregator.ZoneHostInfo; import com.cloud.keystore.KeystoreManager; +import com.cloud.network.IPAddressVO; import com.cloud.network.NetworkManager; import com.cloud.network.NetworkVO; import com.cloud.network.Networks.TrafficType; +import com.cloud.network.dao.IPAddressDao; import com.cloud.network.dao.NetworkDao; +import com.cloud.network.rules.RulesManager; import com.cloud.offering.ServiceOffering; import com.cloud.offerings.NetworkOfferingVO; import com.cloud.offerings.dao.NetworkOfferingDao; @@ -96,6 +99,7 @@ import com.cloud.storage.template.TemplateConstants; import com.cloud.user.Account; import com.cloud.user.AccountService; import com.cloud.user.User; +import com.cloud.user.UserContext; import com.cloud.utils.DateUtil; import com.cloud.utils.NumbersUtil; import com.cloud.utils.Pair; @@ -215,8 +219,11 @@ public class SecondaryStorageManagerImpl implements SecondaryStorageVmManager, V NetworkDao _networkDao; @Inject NetworkOfferingDao _networkOfferingDao; + @Inject + protected IPAddressDao _ipAddressDao = null; + @Inject + protected RulesManager _rulesMgr; - @Inject KeystoreManager _keystoreMgr; private long _capacityScanInterval = DEFAULT_CAPACITY_SCAN_INTERVAL; @@ -1181,11 +1188,36 @@ public class SecondaryStorageManagerImpl implements SecondaryStorageVmManager, V return false; } + try { + //get system ip and create static nat rule for the vm in case of basic networking with EIP/ELB + _rulesMgr.getSystemIpAndEnableStaticNatForVm(profile.getVirtualMachine(), false); + IPAddressVO ipaddr = _ipAddressDao.findByAssociatedVmId(profile.getVirtualMachine().getId()); + if (ipaddr != null && ipaddr.getSystem()) { + SecondaryStorageVmVO secVm = profile.getVirtualMachine(); + // override SSVM guest IP with EIP, so that download url's with be prepared with EIP + secVm.setPublicIpAddress(ipaddr.getAddress().addr()); + _secStorageVmDao.update(secVm.getId(), secVm); + } + } catch (Exception ex) { + s_logger.warn("Failed to get system ip and enable static nat for the vm " + profile.getVirtualMachine() + " due to exception ", ex); + return false; + } + return true; } @Override public void finalizeStop(VirtualMachineProfile profile, StopAnswer answer) { + //release elastic IP here + IPAddressVO ip = _ipAddressDao.findByAssociatedVmId(profile.getId()); + if (ip != null && ip.getSystem()) { + UserContext ctx = UserContext.current(); + try { + _rulesMgr.disableStaticNat(ip.getId(), ctx.getCaller(), ctx.getCallerUserId(), true); + } catch (Exception ex) { + s_logger.warn("Failed to disable static nat and release system ip " + ip + " as a part of vm " + profile.getVirtualMachine() + " stop due to exception ", ex); + } + } } @Override diff --git a/server/src/com/cloud/storage/upload/UploadMonitorImpl.java b/server/src/com/cloud/storage/upload/UploadMonitorImpl.java index 5e11ba5decd..5f19943ecf0 100755 --- a/server/src/com/cloud/storage/upload/UploadMonitorImpl.java +++ b/server/src/com/cloud/storage/upload/UploadMonitorImpl.java @@ -279,12 +279,7 @@ public class UploadMonitorImpl implements UploadMonitor { uploadJob.setUploadState(Status.DOWNLOAD_URL_NOT_CREATED); uploadJob.setLastUpdated(new Date()); _uploadDao.update(uploadJob.getId(), uploadJob); - - List ssVms = _secStorageVmDao.getSecStorageVmListInStates(SecondaryStorageVm.Role.templateProcessor, dataCenterId, State.Running); - if (ssVms.size() == 0){ - errorString = "Couldnt find a running SSVM in the zone" + dataCenterId+ ". Couldnt create the extraction URL."; - throw new CloudRuntimeException(errorString); - } + // Create Symlink at ssvm String uuid = UUID.randomUUID().toString() + path.substring(path.length() - 4) ; // last 4 characters of the path specify the format like .vhd HostVO secStorage = ApiDBUtils.findHostById(ApiDBUtils.findUploadById(uploadId).getHostId()); @@ -303,16 +298,26 @@ public class UploadMonitorImpl implements UploadMonitor { throw new CloudRuntimeException(errorString); } - //Construct actual URL locally now that the symlink exists at SSVM - String extractURL = generateCopyUrl(ssvm.getPublicIpAddress(), uuid); - UploadVO vo = _uploadDao.createForUpdate(); - vo.setLastUpdated(new Date()); - vo.setUploadUrl(extractURL); - vo.setUploadState(Status.DOWNLOAD_URL_CREATED); - _uploadDao.update(uploadId, vo); - success = true; - return; - + List ssVms = _secStorageVmDao.getSecStorageVmListInStates(SecondaryStorageVm.Role.templateProcessor, dataCenterId, State.Running); + if (ssVms.size() > 0) { + SecondaryStorageVmVO ssVm = ssVms.get(0); + if (ssVm.getPublicIpAddress() == null) { + errorString = "A running secondary storage vm has a null public ip?"; + s_logger.error(errorString); + throw new CloudRuntimeException(errorString); + } + //Construct actual URL locally now that the symlink exists at SSVM + String extractURL = generateCopyUrl(ssVm.getPublicIpAddress(), uuid); + UploadVO vo = _uploadDao.createForUpdate(); + vo.setLastUpdated(new Date()); + vo.setUploadUrl(extractURL); + vo.setUploadState(Status.DOWNLOAD_URL_CREATED); + _uploadDao.update(uploadId, vo); + success = true; + return; + } + errorString = "Couldnt find a running SSVM in the zone" + dataCenterId+ ". Couldnt create the extraction URL."; + throw new CloudRuntimeException(errorString); }finally{ if(!success){ UploadVO uploadJob = _uploadDao.createForUpdate(uploadId);