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/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 {

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);

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

@@ -37,7 +37,7 @@ public interface NetworkService {
 
     List<? extends Network> 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.

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);
 

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<ConsoleProxyVO> 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

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;
 

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;
 

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
+        // Verify input parameters
-        checkIpAndUserVm(ipAddress, vm, caller);
+        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;

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<SecondaryStorageVmVO> 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

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<SecondaryStorageVmVO> 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
+            List<SecondaryStorageVmVO> ssVms = _secStorageVmDao.getSecStorageVmListInStates(SecondaryStorageVm.Role.templateProcessor, dataCenterId, State.Running);
-            String extractURL = generateCopyUrl(ssvm.getPublicIpAddress(), uuid);
+    	    if (ssVms.size() > 0) {
-            UploadVO vo = _uploadDao.createForUpdate();
+                SecondaryStorageVmVO ssVm = ssVms.get(0);
-            vo.setLastUpdated(new Date());
+                if (ssVm.getPublicIpAddress() == null) {
-            vo.setUploadUrl(extractURL);
+                    errorString = "A running secondary storage vm has a null public ip?";
-            vo.setUploadState(Status.DOWNLOAD_URL_CREATED);
+                    s_logger.error(errorString);
-            _uploadDao.update(uploadId, vo);
+                    throw new CloudRuntimeException(errorString);
-            success = true;
+                }
-            return;
+                //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);