Skip to content

Commit

Permalink
CLOUDSTACK-9832: Remove public interface from VPC Virtual Router
Browse files Browse the repository at this point in the history 
Co-Authored-By: Prashanth Manthena <prashanth.manthena@nuagenetworks.net>
Co-Authored-By: Sigert Goeminne <sigert.goeminne@nuagenetworks.net>

Bug: https://issues.apache.org/jira/browse/CLOUDSTACK-9832

Detail:
When the VPC offering does not contain VpcVirtualRouter as a SourceNat provider,
then we will not add the interface in the public network to the VpcVR.

CLOUDSTACK-9832: Move isSrcNat check to VpcManager
  • Loading branch information
@fmaximus
fmaximus authored and Sigert Goeminne committed Oct 11, 2017
1 parent f1c01a5 commit 1d382e0
Show file tree
Hide file tree
Showing 11 changed files with 292 additions and 96 deletions.
5 changes: 5 additions & 0 deletions ...i/src/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java
View file
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
7 changes: 7 additions & 0 deletions engine/components-api/src/com/cloud/network/vpc/VpcManager.java
View file
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
View file
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
View file
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);
}
}
14 changes: 8 additions & 6 deletions server/src/org/cloud/network/router/deployment/VpcRouterDeploymentDefinition.java
View file
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
26 changes: 24 additions & 2 deletions server/test/org/cloud/network/router/deployment/VpcRouterDeploymentDefinitionTest.java
View file
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
View file
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

0 comments on commit 1d382e0

Please sign in to comment.