Secondary ip address and static nat

[Alena Prokharchyk]

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.