diff --git a/server/src/com/cloud/network/IpAddressManagerImpl.java b/server/src/com/cloud/network/IpAddressManagerImpl.java index d4da5fae24c2..bd59c66df3ab 100644 --- a/server/src/com/cloud/network/IpAddressManagerImpl.java +++ b/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 getStaticNatSourceIps(List staticNats) { + List userIps = new ArrayList<>(); + + for (StaticNat snat : staticNats) { + userIps.add(_ipAddressDao.findById(snat.getSourceIpAddressId())); + } + + return userIps; + } + @Override public boolean applyStaticNats(List staticNats, boolean continueOnError, boolean forRevoke) throws ResourceUnavailableException { if (staticNats == null || staticNats.size() == 0) { @@ -1714,8 +1730,8 @@ public boolean applyStaticNats(List staticNats, boolean con return true; } - // get the list of public ip's owned by the network - List userIps = _ipAddressDao.listByAssociatedNetwork(network.getId(), null); + List userIps = getStaticNatSourceIps(staticNats); + List publicIps = new ArrayList(); if (userIps != null && !userIps.isEmpty()) { for (IPAddressVO userIp : userIps) { diff --git a/server/src/com/cloud/network/router/CommandSetupHelper.java b/server/src/com/cloud/network/router/CommandSetupHelper.java index 7208b2568139..500fc49a0c0d 100644 --- a/server/src/com/cloud/network/router/CommandSetupHelper.java +++ b/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(); diff --git a/server/test/com/cloud/network/IpAddressManagerTest.java b/server/test/com/cloud/network/IpAddressManagerTest.java new file mode 100644 index 000000000000..0bf92ee2f692 --- /dev/null +++ b/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 ips = _ipManager.getStaticNatSourceIps(Collections.singletonList(snat)); + Assert.assertNotNull(ips); + Assert.assertEquals(1, ips.size()); + + IPAddressVO returnedVO = ips.get(0); + Assert.assertEquals(vo, returnedVO); + } +}