Secondary ip address and static nat

[Alena Prokharchyk <Al...@citrix.com>]

Hi Jayapal,

I have a question to you regarding your commit aedb8c47 for secondary ip address in terms of StaticNat feature. Before this checkin, we didn’t allow Static nat to be enabled on the 1 VM and multiple public Ips. After the checkin, you can create multiple static nat rules per same vm/different public Ips, as long as each rule is assigned to a different secondary ip address of the vm.

Here is the check that you do when see if the public ip is ready for static nat:


Method in RulesManagerImpl: protected void isIpReadyForStaticNat(long vmId, IPAddressVO ipAddress, String vmIp, Account caller, long callerUserId)

Before the fix:

IPAddressVO oldIP = _ipAddressDao.findByAssociatedVmId(vmId);

After the fix:

//check wether the vm ip is alreday associated with any public ip address
IPAddressVO oldIP = _ipAddressDao.findByAssociatedVmIdAndVmIp(vmId, vmIp);


Just wanted to confirm if that’s the expected behavior. If it is, we have a lot of regression bugs in the ip addresses cleanup when static nat ip is referenced just by vmId. For instance one of them is https://issues.apache.org/jira/browse/CLOUDSTACK-7218. During the vm expunge when we cleanup vm’s resources, we expect only one static nat enabled public ip address for vm:

    private boolean cleanupVmResources(long vmId) {

        // If vm is assigned to static nat, disable static nat for the ip
        // address and disassociate ip if elasticIP is enabled
        IPAddressVO ip = _ipAddressDao.findByAssociatedVmId(vmId);
        try {
             if (ip != null) {
                if (_rulesMgr.disableStaticNat(ip.getId(), _accountMgr.getAccount(Account.ACCOUNT_ID_SYSTEM), User.UID_SYSTEM, true)) {
                    s_logger.debug("Disabled 1-1 nat for ip address " + ip + " as a part of vm id=" + vmId + " expunge”);



And I suspect that the same kind of bugs will happen in all the places where you make a call to _ipAddressDao.findByAssociatedVmId(vmId) returning only one entry, where there can be more than one per vm/public ip.

Or if you say that we still shouldn’t allow more than one Static Nat enabled Public IP per single VM, then the isIpReadyForStaticNat should be fixed by disallowing enabling static nat on the vm that is already associated with the public ip address, disregarding the vmIp, and considering the vmId only when make a search.


Can you please take a look and elaborate.

Thank you,
-Alena.