Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CLOUDSTACK-9832: Do not assign public IP NIC to the VPC VR when the VPC offering does not contain VpcVirtualRouter as a SourceNat provider #2004

Merged
merged 1 commit into from
Nov 2, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,11 @@ NicProfile prepareNic(VirtualMachineProfile vmProfile, DeployDestination dest, R
throws InsufficientVirtualNetworkCapacityException, InsufficientAddressCapacityException, ConcurrentOperationException, InsufficientCapacityException,
ResourceUnavailableException;

/**
* Removes the provided nic from the given vm
* @param vm
* @param nic
*/
void removeNic(VirtualMachineProfile vm, Nic nic);

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,4 +165,11 @@ public interface VpcManager {
validateNtwkOffForNtwkInVpc(Long networkId, long newNtwkOffId, String newCidr, String newNetworkDomain, Vpc vpc, String gateway, Account networkOwner, Long aclId);

List<PrivateGateway> getVpcPrivateGateways(long vpcId);

/**
* Checks if the specified offering needs a public src nat ip or not.
* @param vpcOfferingId
* @return
*/
boolean isSrcNatIpRequired(long vpcOfferingId);
}
77 changes: 45 additions & 32 deletions server/src/com/cloud/network/router/VpcNetworkHelperImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
import java.util.Arrays;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.TreeSet;

import javax.annotation.PostConstruct;
Expand Down Expand Up @@ -83,7 +85,10 @@ public void reallocateRouterNetworks(final RouterDeploymentDefinition vpcRouterD
throws ConcurrentOperationException, InsufficientCapacityException {

final TreeSet<String> publicVlans = new TreeSet<String>();
publicVlans.add(vpcRouterDeploymentDefinition.getSourceNatIP().getVlanTag());
if (vpcRouterDeploymentDefinition.isPublicNetwork()) {
publicVlans.add(vpcRouterDeploymentDefinition.getSourceNatIP()
.getVlanTag());
}

//1) allocate nic for control and source nat public ip
final LinkedHashMap<Network, List<? extends NicProfile>> networks = configureDefaultNics(vpcRouterDeploymentDefinition);
Expand Down Expand Up @@ -115,43 +120,51 @@ public void reallocateRouterNetworks(final RouterDeploymentDefinition vpcRouterD
final List<IPAddressVO> ips = _ipAddressDao.listByAssociatedVpc(vpcId, false);
final List<NicProfile> publicNics = new ArrayList<NicProfile>();
Network publicNetwork = null;
final Map<Network.Service, Set<Network.Provider>> vpcOffSvcProvidersMap = vpcMgr.getVpcOffSvcProvidersMap(vpcRouterDeploymentDefinition.getVpc().getVpcOfferingId());

boolean vpcIsStaticNatProvider = vpcOffSvcProvidersMap.get(Network.Service.StaticNat) != null &&
vpcOffSvcProvidersMap.get(Network.Service.StaticNat).contains(Network.Provider.VPCVirtualRouter);

final ServiceOfferingVO routerOffering = _serviceOfferingDao.findById(vpcRouterDeploymentDefinition.getServiceOfferingId());

for (final IPAddressVO ip : ips) {
final PublicIp publicIp = PublicIp.createFromAddrAndVlan(ip, _vlanDao.findById(ip.getVlanId()));
if ((ip.getState() == IpAddress.State.Allocated || ip.getState() == IpAddress.State.Allocating) && vpcMgr.isIpAllocatedToVpc(ip) &&
!publicVlans.contains(publicIp.getVlanTag())) {
s_logger.debug("Allocating nic for router in vlan " + publicIp.getVlanTag());
final NicProfile publicNic = new NicProfile();
publicNic.setDefaultNic(false);
publicNic.setIPv4Address(publicIp.getAddress().addr());
publicNic.setIPv4Gateway(publicIp.getGateway());
publicNic.setIPv4Netmask(publicIp.getNetmask());
publicNic.setMacAddress(publicIp.getMacAddress());
publicNic.setBroadcastType(BroadcastDomainType.Vlan);
publicNic.setBroadcastUri(BroadcastDomainType.Vlan.toUri(publicIp.getVlanTag()));
publicNic.setIsolationUri(IsolationType.Vlan.toUri(publicIp.getVlanTag()));
final NetworkOffering publicOffering = _networkModel.getSystemAccountNetworkOfferings(NetworkOffering.SystemPublicNetwork).get(0);
if (publicNetwork == null) {
final List<? extends Network> publicNetworks = _networkMgr.setupNetwork(s_systemAccount, publicOffering, vpcRouterDeploymentDefinition.getPlan(), null, null, false);
publicNetwork = publicNetworks.get(0);
if (vpcIsStaticNatProvider || !ip.isOneToOneNat()) {
final PublicIp publicIp = PublicIp.createFromAddrAndVlan(ip, _vlanDao.findById(ip.getVlanId()));
if ((ip.getState() == IpAddress.State.Allocated || ip.getState() == IpAddress.State.Allocating)
&& vpcMgr.isIpAllocatedToVpc(ip)
&& !publicVlans.contains(publicIp.getVlanTag())) {
s_logger.debug("Allocating nic for router in vlan " + publicIp.getVlanTag());
final NicProfile publicNic = new NicProfile();
publicNic.setDefaultNic(false);
publicNic.setIPv4Address(publicIp.getAddress()
.addr());
publicNic.setIPv4Gateway(publicIp.getGateway());
publicNic.setIPv4Netmask(publicIp.getNetmask());
publicNic.setMacAddress(publicIp.getMacAddress());
publicNic.setBroadcastType(BroadcastDomainType.Vlan);
publicNic.setBroadcastUri(BroadcastDomainType.Vlan.toUri(publicIp.getVlanTag()));
publicNic.setIsolationUri(IsolationType.Vlan.toUri(publicIp.getVlanTag()));
final NetworkOffering publicOffering = _networkModel.getSystemAccountNetworkOfferings(NetworkOffering.SystemPublicNetwork)
.get(0);
if (publicNetwork == null) {
final List<? extends Network> publicNetworks = _networkMgr.setupNetwork(s_systemAccount, publicOffering, vpcRouterDeploymentDefinition.getPlan(), null, null, false);
publicNetwork = publicNetworks.get(0);
}
publicNics.add(publicNic);
publicVlans.add(publicIp.getVlanTag());
}
publicNics.add(publicNic);
publicVlans.add(publicIp.getVlanTag());
}
}
if (publicNetwork != null) {
if (networks.get(publicNetwork) != null) {
@SuppressWarnings("unchecked")
final
List<NicProfile> publicNicProfiles = (List<NicProfile>)networks.get(publicNetwork);
publicNicProfiles.addAll(publicNics);
networks.put(publicNetwork, publicNicProfiles);
} else {
networks.put(publicNetwork, publicNics);
if (publicNetwork != null) {
if (networks.get(publicNetwork) != null) {
@SuppressWarnings("unchecked") final List<NicProfile> publicNicProfiles = (List<NicProfile>)networks.get(publicNetwork);
publicNicProfiles.addAll(publicNics);
networks.put(publicNetwork, publicNicProfiles);
} else {
networks.put(publicNetwork, publicNics);
}
}
}

final ServiceOfferingVO routerOffering = _serviceOfferingDao.findById(vpcRouterDeploymentDefinition.getServiceOfferingId());

_itMgr.allocate(router.getInstanceName(), template, routerOffering, networks, vpcRouterDeploymentDefinition.getPlan(), hType);
}

Expand Down
33 changes: 7 additions & 26 deletions server/src/com/cloud/network/vpc/VpcManagerImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,12 @@
import org.apache.cloudstack.api.command.user.vpc.ListStaticRoutesCmd;
import org.apache.cloudstack.context.CallContext;
import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
import org.apache.cloudstack.framework.config.ConfigDepot;
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
import org.apache.cloudstack.managed.context.ManagedContextRunnable;
import org.apache.commons.collections.CollectionUtils;
import org.apache.log4j.Logger;

import com.cloud.configuration.Config;
import com.cloud.configuration.ConfigurationManager;
import com.cloud.configuration.Resource.ResourceType;
import com.cloud.dc.DataCenter;
import com.cloud.dc.DataCenterVO;
Expand Down Expand Up @@ -88,8 +86,6 @@
import com.cloud.network.dao.IPAddressVO;
import com.cloud.network.dao.NetworkDao;
import com.cloud.network.dao.NetworkVO;
import com.cloud.network.dao.PhysicalNetworkDao;
import com.cloud.network.dao.Site2SiteVpnGatewayDao;
import com.cloud.network.element.NetworkElement;
import com.cloud.network.element.StaticNatServiceProvider;
import com.cloud.network.element.VpcProvider;
Expand All @@ -108,7 +104,6 @@
import com.cloud.offerings.dao.NetworkOfferingServiceMapDao;
import com.cloud.org.Grouping;
import com.cloud.projects.Project.ListProjectResourcesCriteria;
import com.cloud.server.ConfigurationServer;
import com.cloud.server.ResourceTag.ResourceObjectType;
import com.cloud.tags.ResourceTagVO;
import com.cloud.tags.dao.ResourceTagDao;
Expand Down Expand Up @@ -140,7 +135,6 @@
import com.cloud.utils.net.NetUtils;
import com.cloud.vm.ReservationContext;
import com.cloud.vm.ReservationContextImpl;
import com.cloud.vm.dao.DomainRouterDao;

public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvisioningService, VpcService {
private static final Logger s_logger = Logger.getLogger(VpcManagerImpl.class);
Expand All @@ -162,8 +156,6 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis
@Inject
ConfigurationDao _configDao;
@Inject
ConfigurationManager _configMgr;
@Inject
AccountManager _accountMgr;
@Inject
NetworkDao _ntwkDao;
Expand All @@ -176,8 +168,6 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis
@Inject
IPAddressDao _ipAddressDao;
@Inject
DomainRouterDao _routerDao;
@Inject
VpcGatewayDao _vpcGatewayDao;
@Inject
PrivateIpDao _privateIpDao;
Expand All @@ -188,14 +178,10 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis
@Inject
VpcOfferingServiceMapDao _vpcOffServiceDao;
@Inject
PhysicalNetworkDao _pNtwkDao;
@Inject
ResourceTagDao _resourceTagDao;
@Inject
FirewallRulesDao _firewallDao;
@Inject
Site2SiteVpnGatewayDao _vpnGatewayDao;
@Inject
Site2SiteVpnManager _s2sVpnMgr;
@Inject
VlanDao _vlanDao = null;
Expand All @@ -206,17 +192,11 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis
@Inject
DataCenterDao _dcDao;
@Inject
ConfigurationServer _configServer;
@Inject
NetworkACLDao _networkAclDao;
@Inject
NetworkACLItemDao _networkACLItemDao;
@Inject
NetworkACLManager _networkAclMgr;
@Inject
IpAddressManager _ipAddrMgr;
@Inject
ConfigDepot _configDepot;

@Inject
private VpcPrivateGatewayTransactionCallable vpcTxCallable;
Expand Down Expand Up @@ -2266,14 +2246,9 @@ public IpAddress associateIPToVpc(final long ipId, final long vpcId) throws Reso
// check permissions
_accountMgr.checkAccess(caller, null, true, owner, vpc);

boolean isSourceNat = false;
if (getExistingSourceNatInVpc(owner.getId(), vpcId) == null) {
isSourceNat = true;
}

s_logger.debug("Associating ip " + ipToAssoc + " to vpc " + vpc);

final boolean isSourceNatFinal = isSourceNat;
final boolean isSourceNatFinal = isSrcNatIpRequired(vpc.getVpcOfferingId()) && getExistingSourceNatInVpc(owner.getId(), vpcId) == null;
Transaction.execute(new TransactionCallbackNoReturn() {
@Override
public void doInTransactionWithoutResult(final TransactionStatus status) {
Expand Down Expand Up @@ -2449,4 +2424,10 @@ public boolean applyStaticRoute(final long routeId) throws ResourceUnavailableEx
final StaticRoute route = _staticRouteDao.findById(routeId);
return applyStaticRoutesForVpc(route.getVpcId());
}

@Override
public boolean isSrcNatIpRequired(long vpcOfferingId) {
final Map<Network.Service, Set<Network.Provider>> vpcOffSvcProvidersMap = getVpcOffSvcProvidersMap(vpcOfferingId);
return vpcOffSvcProvidersMap.get(Network.Service.SourceNat).contains(Network.Provider.VPCVirtualRouter);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,6 @@ public boolean isVpcRouter() {
return true;
}

@Override
public boolean isPublicNetwork() {
return true;
}

@Override
protected void lock() {
final Vpc vpcLock = vpcDao.acquireInLockTable(vpc.getId());
Expand Down Expand Up @@ -115,12 +110,19 @@ protected List<DeployDestination> findDestinations() {
*/
@Override
protected boolean prepareDeployment() {
//Check if the VR is the src NAT provider...
isPublicNetwork = vpcMgr.isSrcNatIpRequired(vpc.getVpcOfferingId());

// Check if public network has to be set on VR
return true;
}

@Override
protected void findSourceNatIP() throws InsufficientAddressCapacityException, ConcurrentOperationException {
sourceNatIp = vpcMgr.assignSourceNatIpAddressToVpc(owner, vpc);
sourceNatIp = null;
if (isPublicNetwork) {
sourceNatIp = vpcMgr.assignSourceNatIpAddressToVpc(owner, vpc);
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,30 @@ public void testGetNumberOfRoutersToDeploy() {
assertEquals("If there is already a router found, there is no need to deploy more", 0, deployment.getNumberOfRoutersToDeploy());
}

protected void driveTestPrepareDeployment(final boolean isRedundant, final boolean isPublicNw) {
// Prepare
when(vpcMgr.isSrcNatIpRequired(mockVpc.getVpcOfferingId())).thenReturn(isPublicNw);

// Execute
final boolean canProceedDeployment = deployment.prepareDeployment();
// Assert
assertTrue("There are no preconditions for Vpc Deployment, thus it should always pass", canProceedDeployment);
assertEquals(isPublicNw, deployment.isPublicNetwork());
}

@Test
public void testPrepareDeploymentPublicNw() {
driveTestPrepareDeployment(true, true);
}

@Test
public void testPrepareDeploymentNonRedundant() {
driveTestPrepareDeployment(false, true);
}

@Test
public void testPrepareDeployment() {
assertTrue("There are no preconditions for Vpc Deployment, thus it should always pass", deployment.prepareDeployment());
public void testPrepareDeploymentRedundantNonPublicNw() {
driveTestPrepareDeployment(true, false);
}

@Test
Expand Down Expand Up @@ -246,6 +267,7 @@ public void testFindSourceNatIP() throws InsufficientAddressCapacityException, C
// Prepare
final PublicIp publicIp = mock(PublicIp.class);
when(vpcMgr.assignSourceNatIpAddressToVpc(mockOwner, mockVpc)).thenReturn(publicIp);
deployment.isPublicNetwork = true;

// Execute
deployment.findSourceNatIP();
Expand Down
32 changes: 30 additions & 2 deletions test/integration/plugins/nuagevsp/nuageTestCase.py
Original file line number Diff line number Diff line change
Expand Up @@ -848,6 +848,34 @@ def validate_PublicIPAddress(self, public_ip, network, static_nat=False,
self.debug("Successfully validated the assignment and state of public "
"IP address - %s" % public_ip.ipaddress.ipaddress)

# verify_VRWithoutPublicIPNIC - Verifies that the given Virtual Router has
# no public IP and NIC
def verify_VRWithoutPublicIPNIC(self, vr):
"""Verifies VR without Public IP and NIC"""
self.debug("Verifies that there is no public IP and NIC in Virtual "
"Router - %s" % vr.name)
self.assertEqual(vr.publicip, None,
"Virtual router has public IP"
)
for nic in vr.nic:
self.assertNotEqual(nic.traffictype, "Public",
"Virtual router has public NIC"
)
self.debug("Successfully verified that there is no public IP and NIC "
"in Virtual Router - %s" % vr.name)

def verify_vpc_has_no_src_nat(self, vpc, account=None):
if not account:
account = self.account
self.debug("Verify that there is no src NAT ip address "
"allocated for the vpc")
src_nat_ip = PublicIPAddress.list(
self.api_client,
vpcid=vpc.id,
issourcenat=True,
account=account.name)
self.assertEqual(src_nat_ip, None, "VPC has a source NAT ip!")

# VSD verifications; VSD is a programmable policy and analytics engine of
# Nuage VSP SDN platform

Expand Down Expand Up @@ -984,10 +1012,10 @@ def verify_vsd_object_status(self, cs_object, stopped):
expected_status = cs_object.state.upper() if not stopped \
else "DELETE_PENDING"
tries = 0
while (vsd_object.status != expected_status) and (tries < 10):
while (vsd_object.status != expected_status) and (tries < 120):
self.debug("Waiting for the CloudStack object " + cs_object.name +
" to be fully resolved in VSD...")
time.sleep(30)
time.sleep(5)
self.debug("Rechecking the CloudStack object " + cs_object.name +
" status in VSD...")
vsd_object = self.vsd.get_vm(
Expand Down
Loading