CLOUDSTACK-9317: Enable/disable static NAT associates only relevant IPs.

server/src/com/cloud/network/IpAddressManagerImpl.java

@@ -1698,6 +1698,22 @@ public String acquireGuestIpAddress(Network network, String requestedIp) {
 
     Random _rand = new Random(System.currentTimeMillis());
 
+    /**
+     * Get the list of public IPs that need to be applied for a static NAT enable/disable operation.
+     * Manipulating only these ips prevents concurrency issues when disabling static nat at the same time.
+     * @param staticNats
+     * @return The list of IPs that need to be applied for the static NAT to work.
+     */
+    public List<IPAddressVO> getStaticNatSourceIps(List<? extends StaticNat> staticNats) {
+        List<IPAddressVO> userIps = new ArrayList<>();
+
+        for (StaticNat snat : staticNats) {
+            userIps.add(_ipAddressDao.findById(snat.getSourceIpAddressId()));
+        }
+
+        return userIps;
+    }
+
     @Override
     public boolean applyStaticNats(List<? extends StaticNat> staticNats, boolean continueOnError, boolean forRevoke) throws ResourceUnavailableException {
         if (staticNats == null || staticNats.size() == 0) {
@@ -1714,8 +1730,8 @@ public boolean applyStaticNats(List<? extends StaticNat> staticNats, boolean con
             return true;
         }
 
-        // get the list of public ip's owned by the network
-        List<IPAddressVO> userIps = _ipAddressDao.listByAssociatedNetwork(network.getId(), null);
+        List<IPAddressVO> userIps = getStaticNatSourceIps(staticNats);
+
         List<PublicIp> publicIps = new ArrayList<PublicIp>();
         if (userIps != null && !userIps.isEmpty()) {
             for (IPAddressVO userIp : userIps) {

server/src/com/cloud/network/router/CommandSetupHelper.java

@@ -780,10 +780,11 @@ public int compare(final PublicIpAddress o1, final PublicIpAddress o2) {
 
                 final boolean add = ipAddr.getState() == IpAddress.State.Releasing ? false : true;
                 boolean sourceNat = ipAddr.isSourceNat();
-                /* enable sourceNAT for the first ip of the public interface */
-                if (firstIP) {
-                    sourceNat = true;
+                /* enable sourceNAT for the first ip of the public interface as long as it's source nat. */
+                if (firstIP && !sourceNat) {
+                    firstIP = false;
                 }
+
                 final String vlanId = ipAddr.getVlanTag();
                 final String vlanGateway = ipAddr.getGateway();
                 final String vlanNetmask = ipAddr.getNetmask();

server/test/com/cloud/network/IpAddressManagerTest.java

@@ -0,0 +1,71 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package com.cloud.network;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+
+import com.cloud.network.dao.IPAddressDao;
+import com.cloud.network.dao.IPAddressVO;
+import com.cloud.network.rules.StaticNat;
+import com.cloud.network.rules.StaticNatImpl;
+import com.cloud.utils.net.Ip;
+
+import static org.mockito.Mockito.when;
+
+import java.util.Collections;
+import java.util.List;
+
+import static org.mockito.Mockito.anyLong;
+import static org.mockito.Mockito.mock;
+
+public class IpAddressManagerTest {
+
+    @Mock
+    IPAddressDao _ipAddrDao;
+
+    @InjectMocks
+    IpAddressManagerImpl _ipManager;
+
+    @Before
+    public void setup() {
+        MockitoAnnotations.initMocks(this);
+    }
+
+    @Test
+    public void testGetStaticNatSourceIps() {
+        String publicIpAddress = "192.168.1.3";
+        IPAddressVO vo = mock(IPAddressVO.class);
+        when(vo.getAddress()).thenReturn(new Ip(publicIpAddress));
+        when(vo.getId()).thenReturn(1l);
+
+        when(_ipAddrDao.findById(anyLong())).thenReturn(vo);
+        StaticNat snat = new StaticNatImpl(1, 1, 1, 1, publicIpAddress, false);
+
+        List<IPAddressVO> ips = _ipManager.getStaticNatSourceIps(Collections.singletonList(snat));
+        Assert.assertNotNull(ips);
+        Assert.assertEquals(1, ips.size());
+
+        IPAddressVO returnedVO = ips.get(0);
+        Assert.assertEquals(vo, returnedVO);
+    }
+}