[Vmware] Enable PVLAN support on L2 networks (apache#3732)

* Enable PVLAN support on L2 networks

* Fix prevent null pointer on details

* Add marvin tests

* Fixes from comments

* Fix: missing pvlan type on plugniccommand

* Fix checks on network creation for vlans overlap

* Fix remove prefix from secondary vlan id

* Improve checks on physical network for pvlans

* Fix compatibility with previous pvlan creation

* Fix shared networks backwards pvlan compatibility

* Add ui fix for pvlan type not passed to api

* Add check for isolated vlan id overlap

* Include check for dynamic vlan reserved for secondary vlan

* Fix marvin tests errors

* Fix redundant imports

* Skip marvin test for pvlan if dvswitch is not present

* spelling

Co-authored-by: Andrija Panic <[email protected]>

Author: nvazquez

api/src/main/java/com/cloud/network/Network.java

@@ -21,6 +21,8 @@
 import java.util.ArrayList;
 import java.util.List;
 
+import com.cloud.exception.InvalidParameterValueException;
+import org.apache.commons.lang.StringUtils;
 import org.apache.commons.lang.builder.ToStringBuilder;
 import org.apache.commons.lang.builder.ToStringStyle;
 
@@ -44,6 +46,24 @@ enum GuestType {
         Shared, Isolated, L2
     }
 
+    enum PVlanType {
+        Community, Isolated, Promiscuous;
+
+        static PVlanType fromValue(String type) {
+            if (StringUtils.isBlank(type)) {
+                return null;
+            } else if (type.equalsIgnoreCase("promiscuous") || type.equalsIgnoreCase("p")) {
+                return Promiscuous;
+            } else if (type.equalsIgnoreCase("community") || type.equalsIgnoreCase("c")) {
+                return Community;
+            } else if (type.equalsIgnoreCase("isolated") || type.equalsIgnoreCase("i")) {
+                return Isolated;
+            } else {
+                throw new InvalidParameterValueException("Unexpected Private VLAN type: " + type);
+            }
+        }
+    }
+
     String updatingInSequence = "updatingInSequence";
     String hideIpAddressUsage = "hideIpAddressUsage";
 
@@ -416,4 +436,6 @@ public void setIp6Address(String ip6Address) {
     boolean isStrechedL2Network();
 
     String getExternalId();
+
+    PVlanType getPvlanType();
 }

api/src/main/java/com/cloud/network/NetworkProfile.java

@@ -314,4 +314,9 @@ public String getExternalId() {
         return externalId;
     }
 
+    @Override
+    public PVlanType getPvlanType() {
+        return null;
+    }
+
 }

api/src/main/java/com/cloud/offering/NetworkOffering.java

@@ -38,7 +38,7 @@ public enum State {
     }
 
     public enum Detail {
-        InternalLbProvider, PublicLbProvider, servicepackageuuid, servicepackagedescription, PromiscuousMode, MacAddressChanges, ForgedTransmits, RelatedNetworkOffering, domainid, zoneid
+        InternalLbProvider, PublicLbProvider, servicepackageuuid, servicepackagedescription, PromiscuousMode, MacAddressChanges, ForgedTransmits, RelatedNetworkOffering, domainid, zoneid, pvlanType
     }
 
     public final static String SystemPublicNetwork = "System-Public-Network";

api/src/main/java/org/apache/cloudstack/api/command/user/network/CreateNetworkCmd.java

@@ -97,6 +97,10 @@ public class CreateNetworkCmd extends BaseCmd implements UserCmd {
     @Parameter(name = ApiConstants.ISOLATED_PVLAN, type = CommandType.STRING, description = "the isolated private VLAN for this network")
     private String isolatedPvlan;
 
+    @Parameter(name = ApiConstants.ISOLATED_PVLAN_TYPE, type = CommandType.STRING,
+            description = "the isolated private VLAN type for this network")
+    private String isolatedPvlanType;
+
     @Parameter(name = ApiConstants.NETWORK_DOMAIN, type = CommandType.STRING, description = "network domain")
     private String networkDomain;
 
@@ -217,6 +221,10 @@ public String getExternalId() {
         return externalId;
     }
 
+    public String getIsolatedPvlanType() {
+        return isolatedPvlanType;
+    }
+
     @Override
     public boolean isDisplay() {
         if(displayNetwork == null)

engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java

@@ -180,7 +180,7 @@ void prepare(VirtualMachineProfile profile, DeployDestination dest, ReservationC
 
     Network createGuestNetwork(long networkOfferingId, String name, String displayText, String gateway, String cidr, String vlanId, boolean bypassVlanOverlapCheck, String networkDomain, Account owner,
                                Long domainId, PhysicalNetwork physicalNetwork, long zoneId, ACLType aclType, Boolean subdomainAccess, Long vpcId, String ip6Gateway, String ip6Cidr,
-                               Boolean displayNetworkEnabled, String isolatedPvlan, String externalId) throws ConcurrentOperationException, InsufficientCapacityException, ResourceAllocationException;
+                               Boolean displayNetworkEnabled, String isolatedPvlan, Network.PVlanType isolatedPvlanType, String externalId) throws ConcurrentOperationException, InsufficientCapacityException, ResourceAllocationException;
 
     UserDataServiceProvider getPasswordResetProvider(Network network);
 

engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java

@@ -39,6 +39,8 @@
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
+import com.cloud.network.dao.NetworkDetailVO;
+import com.cloud.network.dao.NetworkDetailsDao;
 import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao;
 import org.apache.cloudstack.api.ApiConstants;
 import org.apache.cloudstack.api.command.admin.vm.MigrateVMCmd;
@@ -329,6 +331,8 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac
     private StorageManager storageMgr;
     @Inject
     private NetworkOfferingDetailsDao networkOfferingDetailsDao;
+    @Inject
+    private NetworkDetailsDao networkDetailsDao;
 
     VmWorkJobHandlerProxy _jobHandlerProxy = new VmWorkJobHandlerProxy(this);
 
@@ -4059,6 +4063,13 @@ public boolean plugNic(final Network network, final NicTO nic, final VirtualMach
         final VMInstanceVO router = _vmDao.findById(vm.getId());
         if (router.getState() == State.Running) {
             try {
+                NetworkDetailVO pvlanTypeDetail = networkDetailsDao.findDetail(network.getId(), ApiConstants.ISOLATED_PVLAN_TYPE);
+                if (pvlanTypeDetail != null) {
+                    Map<NetworkOffering.Detail, String> nicDetails = nic.getDetails() == null ? new HashMap<>() : nic.getDetails();
+                    s_logger.debug("Found PVLAN type: " + pvlanTypeDetail.getValue() + " on network details, adding it as part of the PlugNicCommand");
+                    nicDetails.putIfAbsent(NetworkOffering.Detail.pvlanType, pvlanTypeDetail.getValue());
+                    nic.setDetails(nicDetails);
+                }
                 final PlugNicCommand plugNicCmd = new PlugNicCommand(nic, vm.getName(), vm.getType(), vm.getDetails());
                 final Commands cmds = new Commands(Command.OnError.Stop);
                 cmds.addCommand("plugnic", plugNicCmd);

engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java

@@ -38,7 +38,10 @@
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
+import com.cloud.network.dao.NetworkDetailVO;
+import com.cloud.network.dao.NetworkDetailsDao;
 import org.apache.cloudstack.acl.ControlledEntity.ACLType;
+import org.apache.cloudstack.api.ApiConstants;
 import org.apache.cloudstack.context.CallContext;
 import org.apache.cloudstack.engine.cloud.entity.api.db.VMNetworkMapVO;
 import org.apache.cloudstack.engine.cloud.entity.api.db.dao.VMNetworkMapDao;
@@ -222,6 +225,8 @@
 import com.cloud.vm.dao.VMInstanceDao;
 import com.google.common.base.Strings;
 
+import static org.apache.commons.lang.StringUtils.isNotBlank;
+
 /**
  * NetworkManagerImpl implements NetworkManager.
  */
@@ -251,6 +256,8 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra
     @Inject
     NetworkDao _networksDao;
     @Inject
+    NetworkDetailsDao networkDetailsDao;
+    @Inject
     NicDao _nicDao;
     @Inject
     RulesManager _rulesMgr;
@@ -698,6 +705,11 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
                                 finalizeServicesAndProvidersForNetwork(offering, plan.getPhysicalNetworkId()));
                         networks.add(networkPersisted);
 
+                        if (network.getPvlanType() != null) {
+                            NetworkDetailVO detailVO = new NetworkDetailVO(networkPersisted.getId(), ApiConstants.ISOLATED_PVLAN_TYPE, network.getPvlanType().toString(), true);
+                            networkDetailsDao.persist(detailVO);
+                        }
+
                         if (predefined instanceof NetworkVO && guru instanceof NetworkGuruAdditionalFunctions){
                             final NetworkGuruAdditionalFunctions functions = (NetworkGuruAdditionalFunctions) guru;
                             functions.finalizeNetworkDesign(networkPersisted.getId(), ((NetworkVO)predefined).getVlanIdAsUUID());
@@ -2168,7 +2180,7 @@ public void expungeNics(final VirtualMachineProfile vm) {
     public Network createGuestNetwork(final long networkOfferingId, final String name, final String displayText, final String gateway, final String cidr, String vlanId,
                                       boolean bypassVlanOverlapCheck, String networkDomain, final Account owner, final Long domainId, final PhysicalNetwork pNtwk,
                                       final long zoneId, final ACLType aclType, Boolean subdomainAccess, final Long vpcId, final String ip6Gateway, final String ip6Cidr,
-                                      final Boolean isDisplayNetworkEnabled, final String isolatedPvlan, String externalId) throws ConcurrentOperationException, InsufficientCapacityException, ResourceAllocationException {
+                                      final Boolean isDisplayNetworkEnabled, final String isolatedPvlan, Network.PVlanType isolatedPvlanType, String externalId) throws ConcurrentOperationException, InsufficientCapacityException, ResourceAllocationException {
 
         final NetworkOfferingVO ntwkOff = _networkOfferingDao.findById(networkOfferingId);
         final DataCenterVO zone = _dcDao.findById(zoneId);
@@ -2280,16 +2292,25 @@ public Network createGuestNetwork(final long networkOfferingId, final String nam
 
         if (vlanSpecified) {
             URI uri = BroadcastDomainType.fromString(vlanId);
+            // Aux: generate secondary URI for secondary VLAN ID (if provided) for performing checks
+            URI secondaryUri = isNotBlank(isolatedPvlan) ? BroadcastDomainType.fromString(isolatedPvlan) : null;
             //don't allow to specify vlan tag used by physical network for dynamic vlan allocation
             if (!(bypassVlanOverlapCheck && ntwkOff.getGuestType() == GuestType.Shared) && _dcDao.findVnet(zoneId, pNtwk.getId(), BroadcastDomainType.getValue(uri)).size() > 0) {
                 throw new InvalidParameterValueException("The VLAN tag " + vlanId + " is already being used for dynamic vlan allocation for the guest network in zone "
                         + zone.getName());
             }
+            if (secondaryUri != null && !(bypassVlanOverlapCheck && ntwkOff.getGuestType() == GuestType.Shared) &&
+                    _dcDao.findVnet(zoneId, pNtwk.getId(), BroadcastDomainType.getValue(secondaryUri)).size() > 0) {
+                throw new InvalidParameterValueException("The VLAN tag " + isolatedPvlan + " is already being used for dynamic vlan allocation for the guest network in zone "
+                        + zone.getName());
+            }
             if (! UuidUtils.validateUUID(vlanId)){
                 // For Isolated and L2 networks, don't allow to create network with vlan that already exists in the zone
-                if (ntwkOff.getGuestType() == GuestType.Isolated || !hasGuestBypassVlanOverlapCheck(bypassVlanOverlapCheck, ntwkOff)) {
+                if (ntwkOff.getGuestType() == GuestType.Isolated || ntwkOff.getGuestType() == GuestType.L2 || !hasGuestBypassVlanOverlapCheck(bypassVlanOverlapCheck, ntwkOff)) {
                     if (_networksDao.listByZoneAndUriAndGuestType(zoneId, uri.toString(), null).size() > 0) {
                         throw new InvalidParameterValueException("Network with vlan " + vlanId + " already exists or overlaps with other network vlans in zone " + zoneId);
+                    } else if (secondaryUri != null && _networksDao.listByZoneAndUriAndGuestType(zoneId, secondaryUri.toString(), null).size() > 0) {
+                        throw new InvalidParameterValueException("Network with vlan " + isolatedPvlan + " already exists or overlaps with other network vlans in zone " + zoneId);
                     } else {
                         final List<DataCenterVnetVO> dcVnets = _datacenterVnetDao.findVnet(zoneId, BroadcastDomainType.getValue(uri));
                         //for the network that is created as part of private gateway,
@@ -2436,8 +2457,15 @@ public Network doInTransaction(final TransactionStatus status) {
                         if (vlanIdFinal.equalsIgnoreCase(Vlan.UNTAGGED)) {
                             throw new InvalidParameterValueException("Cannot support pvlan with untagged primary vlan!");
                         }
-                        userNetwork.setBroadcastUri(NetUtils.generateUriForPvlan(vlanIdFinal, isolatedPvlan));
+                        URI uri = NetUtils.generateUriForPvlan(vlanIdFinal, isolatedPvlan);
+                        if (_networksDao.listByPhysicalNetworkPvlan(physicalNetworkId, uri.toString(), isolatedPvlanType).size() > 0) {
+                            throw new InvalidParameterValueException("Network with primary vlan " + vlanIdFinal +
+                                    " and secondary vlan " + isolatedPvlan + " type " + isolatedPvlanType +
+                                    " already exists or overlaps with other network pvlans in zone " + zoneId);
+                        }
+                        userNetwork.setBroadcastUri(uri);
                         userNetwork.setBroadcastDomainType(BroadcastDomainType.Pvlan);
+                        userNetwork.setPvlanType(isolatedPvlanType);
                     }
                 }
 

engine/schema/src/main/java/com/cloud/network/dao/NetworkDao.java

@@ -122,4 +122,6 @@ public interface NetworkDao extends GenericDao<NetworkVO, Long>, StateDao<State,
     List<NetworkVO> listNetworkVO(List<Long> idset);
 
     List<NetworkVO> listByAccountIdNetworkName(long accountId, String name);
+
+    List<NetworkVO> listByPhysicalNetworkPvlan(long physicalNetworkId, String broadcastUri, Network.PVlanType pVlanType);
 }

engine/schema/src/main/java/com/cloud/network/dao/NetworkDaoImpl.java

@@ -26,7 +26,9 @@
 import javax.inject.Inject;
 import javax.persistence.TableGenerator;
 
+import com.cloud.utils.exception.CloudRuntimeException;
 import org.apache.cloudstack.acl.ControlledEntity.ACLType;
+import org.apache.cloudstack.api.ApiConstants;
 import org.springframework.stereotype.Component;
 
 import com.cloud.network.Network;
@@ -92,6 +94,8 @@ public class NetworkDaoImpl extends GenericDaoBase<NetworkVO, Long>implements Ne
     NetworkOfferingDao _ntwkOffDao;
     @Inject
     NetworkOpDao _ntwkOpDao;
+    @Inject
+    NetworkDetailsDao networkDetailsDao;
 
     TableGenerator _tgMacAddress;
 
@@ -707,4 +711,72 @@ public List<NetworkVO> listByAccountIdNetworkName(final long accountId, final St
 
         return listBy(sc, null);
     }
+
+    /**
+     * True when a requested PVLAN pair overlaps with any existing PVLAN pair within the same physical network, i.e when:
+     *      - The requested exact PVLAN pair exists
+     *      - The requested secondary VLAN ID is secondary VLAN ID of an existing PVLAN pair
+     *      - The requested secondary VLAN ID is primary VLAN ID of an existing PVLAN pair
+     */
+    protected boolean isNetworkOverlappingRequestedPvlan(Integer existingPrimaryVlan, Integer existingSecondaryVlan, Network.PVlanType existingPvlanType,
+                                                         Integer requestedPrimaryVlan, Integer requestedSecondaryVlan, Network.PVlanType requestedPvlanType) {
+        if (existingPrimaryVlan == null || existingSecondaryVlan == null || requestedPrimaryVlan == null || requestedSecondaryVlan == null) {
+            throw new CloudRuntimeException(String.format("Missing VLAN ID while checking PVLAN pair (%s, %s)" +
+                    " against existing pair (%s, %s)", existingPrimaryVlan, existingSecondaryVlan, requestedPrimaryVlan, requestedSecondaryVlan));
+        }
+        boolean exactMatch = existingPrimaryVlan.equals(requestedPrimaryVlan) && existingSecondaryVlan.equals(requestedSecondaryVlan);
+        boolean secondaryVlanUsed = requestedPvlanType != Network.PVlanType.Promiscuous && requestedSecondaryVlan.equals(existingPrimaryVlan) || requestedSecondaryVlan.equals(existingSecondaryVlan);
+        boolean isolatedMax = false;
+        boolean promiscuousMax = false;
+        if (requestedPvlanType == Network.PVlanType.Isolated && existingPrimaryVlan.equals(requestedPrimaryVlan) && existingPvlanType.equals(Network.PVlanType.Isolated)) {
+            isolatedMax = true;
+        } else if (requestedPvlanType == Network.PVlanType.Promiscuous && existingPrimaryVlan.equals(requestedPrimaryVlan) && existingPvlanType == Network.PVlanType.Promiscuous) {
+            promiscuousMax = true;
+        }
+        return exactMatch || secondaryVlanUsed || isolatedMax || promiscuousMax;
+    }
+
+    protected Network.PVlanType getNetworkPvlanType(long networkId, List<Integer> existingPvlan) {
+        Network.PVlanType existingPvlanType = null;
+        NetworkDetailVO pvlanTypeDetail = networkDetailsDao.findDetail(networkId, ApiConstants.ISOLATED_PVLAN_TYPE);
+        if (pvlanTypeDetail != null) {
+            existingPvlanType = Network.PVlanType.valueOf(pvlanTypeDetail.getValue());
+        } else {
+            existingPvlanType = existingPvlan.get(0).equals(existingPvlan.get(1)) ? Network.PVlanType.Promiscuous : Network.PVlanType.Isolated;
+        }
+        return existingPvlanType;
+    }
+
+    @Override
+    public List<NetworkVO> listByPhysicalNetworkPvlan(long physicalNetworkId, String broadcastUri, Network.PVlanType pVlanType) {
+        final URI searchUri = BroadcastDomainType.fromString(broadcastUri);
+        if (!searchUri.getScheme().equalsIgnoreCase("pvlan")) {
+            throw new CloudRuntimeException("PVLAN requested but URI is not in the expected format: " + searchUri.toString());
+        }
+        final String searchRange = BroadcastDomainType.getValue(searchUri);
+        final List<Integer> searchVlans = UriUtils.expandPvlanUri(searchRange);
+        final List<NetworkVO> overlappingNetworks = new ArrayList<>();
+
+        final SearchCriteria<NetworkVO> sc = PhysicalNetworkSearch.create();
+        sc.setParameters("physicalNetworkId", physicalNetworkId);
+
+        for (final NetworkVO network : listBy(sc)) {
+            if (network.getBroadcastUri() == null || !network.getBroadcastUri().getScheme().equalsIgnoreCase("pvlan")) {
+                continue;
+            }
+            final String networkVlanRange = BroadcastDomainType.getValue(network.getBroadcastUri());
+            if (networkVlanRange == null || networkVlanRange.isEmpty()) {
+                continue;
+            }
+            List<Integer> existingPvlan = UriUtils.expandPvlanUri(networkVlanRange);
+            Network.PVlanType existingPvlanType = getNetworkPvlanType(network.getId(), existingPvlan);
+            if (isNetworkOverlappingRequestedPvlan(existingPvlan.get(0), existingPvlan.get(1), existingPvlanType,
+                    searchVlans.get(0), searchVlans.get(1), pVlanType)) {
+                overlappingNetworks.add(network);
+                break;
+            }
+        }
+
+        return overlappingNetworks;
+    }
 }

engine/schema/src/main/java/com/cloud/network/dao/NetworkVO.java

@@ -181,6 +181,9 @@ public class NetworkVO implements Network {
     @Transient
     boolean rollingRestart = false;
 
+    @Transient
+    PVlanType pVlanType;
+
     public NetworkVO() {
         uuid = UUID.randomUUID().toString();
     }
@@ -661,4 +664,12 @@ public boolean isRollingRestart() {
     public void setRollingRestart(boolean rollingRestart) {
         this.rollingRestart = rollingRestart;
     }
+
+    public PVlanType getPvlanType() {
+        return pVlanType;
+    }
+
+    public void setPvlanType(PVlanType pvlanType) {
+        this.pVlanType = pvlanType;
+    }
 }