Add check to prevent deletion of n/w resources when last VM/non-persistent network is deleted but there exists another n/w with overlapping vlan which is persistent

Author: Pearl Dsilva

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

@@ -40,7 +40,9 @@
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
+import com.cloud.api.query.dao.DomainRouterJoinDao;
 import com.cloud.api.query.dao.UserVmJoinDao;
+import com.cloud.api.query.vo.DomainRouterJoinVO;
 import com.cloud.api.query.vo.UserVmJoinVO;
 import com.cloud.deployasis.dao.UserVmDeployAsIsDetailsDao;
 import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao;
@@ -356,6 +358,8 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac
     private UserVmJoinDao userVmJoinDao;
     @Inject
     private NetworkOfferingDao networkOfferingDao;
+    @Inject
+    private DomainRouterJoinDao domainRouterJoinDao;
 
     VmWorkJobHandlerProxy _jobHandlerProxy = new VmWorkJobHandlerProxy(this);
 
@@ -1844,14 +1848,29 @@ private void orchestrateStop(final String vmUuid, final boolean cleanUpEvenIfUna
         advanceStop(vm, cleanUpEvenIfUnableToStop);
     }
 
+    private void getPersistenceMap(Map<String, Boolean> vlanToPersistenceMap, NetworkVO networkVO) {
+        NetworkOfferingVO offeringVO = networkOfferingDao.findById(networkVO.getNetworkOfferingId());
+        Pair<String, Boolean> data = getVMNetworkDetails(networkVO, offeringVO.isPersistent());
+        Boolean shouldDeleteNwResource = MapUtils.isNotEmpty(vlanToPersistenceMap) ? vlanToPersistenceMap.get(data.first()) : null;
+        if (shouldDeleteNwResource == null || shouldDeleteNwResource){
+            vlanToPersistenceMap.put(data.first(), data.second());
+        }
+    }
+
     private Map<String, Boolean> getVlanToPersistenceMapForVM(long vmId) {
         List<UserVmJoinVO> userVmJoinVOS = userVmJoinDao.searchByIds(vmId);
         Map<String, Boolean> vlanToPersistenceMap = new HashMap<>();
         for (UserVmJoinVO userVmJoinVO : userVmJoinVOS) {
             NetworkVO networkVO = _networkDao.findById(userVmJoinVO.getNetworkId());
-            NetworkOfferingVO offeringVO = networkOfferingDao.findById(networkVO.getNetworkOfferingId());
-            Pair<String, Boolean> data = getVMNetworkDetails(networkVO, offeringVO.isPersistent());
-            vlanToPersistenceMap.put(data.first(), data.second());
+            getPersistenceMap(vlanToPersistenceMap, networkVO);
+        }
+        if (userVmJoinVOS.isEmpty()) {
+            VMInstanceVO vmInstanceVO = _vmDao.findById(vmId);
+            if (vmInstanceVO.getType() == VirtualMachine.Type.DomainRouter) {
+                DomainRouterJoinVO routerVO = domainRouterJoinDao.findById(vmId);
+                NetworkVO networkVO = _networkDao.findById(routerVO.getNetworkId());
+                getPersistenceMap(vlanToPersistenceMap, networkVO);
+            }
         }
         return vlanToPersistenceMap;
     }
@@ -1869,9 +1888,15 @@ private Pair<String, Boolean> getVMNetworkDetails(NetworkVO networkVO, boolean i
         URI broadcastUri = networkVO.getBroadcastUri();
         String scheme = broadcastUri.getScheme();
         String vlanId = Networks.BroadcastDomainType.getValue(broadcastUri);
-        Boolean shouldDelete = !((networkVO.getGuestType() == Network.GuestType.L2 || networkVO.getGuestType() == Network.GuestType.Isolated) &&
+        boolean shouldDelete = !((networkVO.getGuestType() == Network.GuestType.L2 || networkVO.getGuestType() == Network.GuestType.Isolated) &&
                 (scheme.equalsIgnoreCase("vlan"))
                 && isPersistent);
+        if (shouldDelete) {
+            int persistentNetworksCount = _networkDao.getOtherPersistentNetworksCount(networkVO.getId(), networkVO.getBroadcastUri().toString(), true);
+            if (persistentNetworksCount > 0) {
+                shouldDelete = false;
+            }
+        }
         return new Pair<>(vlanId, shouldDelete);
     }
 

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

@@ -2962,7 +2962,8 @@ private void cleanupPersistentnNetworkResources(NetworkVO network) {
         long networkOfferingId = network.getNetworkOfferingId();
         NetworkOfferingVO offering = _networkOfferingDao.findById(networkOfferingId);
         if (offering != null) {
-            if (networkMeetsPersistenceCriteria(network, offering, true)) {
+            if (networkMeetsPersistenceCriteria(network, offering, true) &&
+                    _networksDao.getOtherPersistentNetworksCount(network.getId(), network.getBroadcastUri().toString(), offering.isPersistent()) == 0) {
                 List<HostVO> hosts = resourceManager.listAllUpAndEnabledHostsInOneZoneByType(Host.Type.Routing, network.getDataCenterId());
                 for (HostVO host : hosts) {
                     try {

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

@@ -45,6 +45,8 @@ public interface NetworkDao extends GenericDao<NetworkVO, Long>, StateDao<State,
 
     List<NetworkVO> getNetworksForOffering(long offeringId, long dataCenterId, long accountId);
 
+    int getOtherPersistentNetworksCount(long id, String broadcastURI, boolean isPersistent);
+
     @Override
     @Deprecated
     NetworkVO persist(NetworkVO vo);

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

@@ -79,6 +79,7 @@ public class NetworkDaoImpl extends GenericDaoBase<NetworkVO, Long>implements Ne
     SearchBuilder<NetworkVO> SourceNATSearch;
     GenericSearchBuilder<NetworkVO, Long> VpcNetworksCount;
     SearchBuilder<NetworkVO> OfferingAccountNetworkSearch;
+    SearchBuilder<NetworkVO> PersistentNetworkSearch;
 
     GenericSearchBuilder<NetworkVO, Long> GarbageCollectedSearch;
     SearchBuilder<NetworkVO> PrivateNetworkSearch;
@@ -181,6 +182,16 @@ protected void init() {
         CountBy.join("offerings", ntwkOffJoin, CountBy.entity().getNetworkOfferingId(), ntwkOffJoin.entity().getId(), JoinBuilder.JoinType.INNER);
         CountBy.done();
 
+        PersistentNetworkSearch = createSearchBuilder();
+        PersistentNetworkSearch.and("id", PersistentNetworkSearch.entity().getId(), Op.NEQ);
+        PersistentNetworkSearch.and("guestType", PersistentNetworkSearch.entity().getGuestType(), Op.IN);
+        PersistentNetworkSearch.and("broadcastUri", PersistentNetworkSearch.entity().getBroadcastUri(), Op.EQ);
+        PersistentNetworkSearch.and("removed", PersistentNetworkSearch.entity().getRemoved(), Op.NULL);
+        final SearchBuilder<NetworkOfferingVO> persistentNtwkOffJoin = _ntwkOffDao.createSearchBuilder();
+        persistentNtwkOffJoin.and("persistent", persistentNtwkOffJoin.entity().isPersistent(), Op.EQ);
+        PersistentNetworkSearch.join("persistent", persistentNtwkOffJoin, PersistentNetworkSearch.entity().getNetworkOfferingId(), persistentNtwkOffJoin.entity().getId(), JoinType.INNER);
+        PersistentNetworkSearch.done();
+
         PhysicalNetworkSearch = createSearchBuilder();
         PhysicalNetworkSearch.and("physicalNetworkId", PhysicalNetworkSearch.entity().getPhysicalNetworkId(), Op.EQ);
         PhysicalNetworkSearch.done();
@@ -265,6 +276,8 @@ protected void init() {
         join10.and("vpc", join10.entity().getVpcId(), Op.EQ);
         PrivateNetworkSearch.join("vpcgateways", join10, PrivateNetworkSearch.entity().getId(), join10.entity().getNetworkId(), JoinBuilder.JoinType.INNER);
         PrivateNetworkSearch.done();
+
+
     }
 
     @Override
@@ -390,6 +403,18 @@ public List<NetworkVO> getNetworksForOffering(final long offeringId, final long
         return search(sc, null);
     }
 
+    @Override
+    public int getOtherPersistentNetworksCount(long id, String broadcastURI, boolean isPersistent) {
+        Object[] guestTypes = {"Isolated", "L2"};
+        final SearchCriteria<NetworkVO> sc = PersistentNetworkSearch.create();
+        sc.setParameters("id", id);
+        sc.setParameters("broadcastUri", broadcastURI);
+        sc.setParameters("guestType", guestTypes);
+        sc.setJoinParameters("persistent", "persistent", isPersistent);
+        List<NetworkVO> persistentNetworks = search(sc, null);
+        return persistentNetworks.size();
+    }
+
     @Override
     public String getNextAvailableMacAddress(final long networkConfigId, Integer zoneMacIdentifier) {
         final SequenceFetcher fetch = SequenceFetcher.getInstance();

plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java

@@ -20,7 +20,6 @@
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.HashSet;
-import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -68,7 +67,7 @@ public boolean start() {
         if (s_apiNameDiscoveryResponseMap == null) {
             long startTime = System.nanoTime();
             s_apiNameDiscoveryResponseMap = new HashMap<String, ApiDiscoveryResponse>();
-            Set<Class<?>> cmdClasses = new LinkedHashSet<Class<?>>();
+            Set<Class<?>> cmdClasses = new HashSet<Class<?>>();
             for (PluggableService service : _services) {
                 s_logger.debug(String.format("getting api commands of service: %s", service.getClass().getName()));
                 cmdClasses.addAll(service.getCommands());

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java

@@ -289,7 +289,7 @@ private String generateVxnetBrName(String pifName, String vnetId) {
         return "brvx-" + vnetId;
     }
 
-    public String createVnetBr(String vNetId, String pifKey, String protocol) throws InternalErrorException {
+    private String createVnetBr(String vNetId, String pifKey, String protocol) throws InternalErrorException {
         String nic = _pifs.get(pifKey);
         if (nic == null) {
             // if not found in bridge map, maybe traffic label refers to pif already?

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java

@@ -305,7 +305,7 @@ private boolean shouldDeleteBridge(Map<String, Boolean> vlanToPersistenceMap, St
         if (MapUtils.isNotEmpty(vlanToPersistenceMap) && vlanId != null && vlanToPersistenceMap.containsKey(vlanId)) {
             return vlanToPersistenceMap.get(vlanId);
         }
-        return false;
+        return true;
     }
 
     /**

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnPlugNicCommandWrapper.java

@@ -103,6 +103,6 @@ private boolean shouldDeleteBridge(Map<String, Boolean> vlanToPersistenceMap, St
         if (MapUtils.isNotEmpty(vlanToPersistenceMap) && vlanId != null && vlanToPersistenceMap.containsKey(vlanId)) {
             return vlanToPersistenceMap.get(vlanId);
         }
-        return false;
+        return true;
     }
 }

server/src/test/java/com/cloud/vpc/dao/MockNetworkDaoImpl.java

@@ -74,6 +74,11 @@ public List<NetworkVO> getNetworksForOffering(final long offeringId, final long
         return null;
     }
 
+    @Override
+    public int getOtherPersistentNetworksCount(long id, String broadcastURI, boolean isPersistent) {
+        return 0;
+    }
+
     @Override
     public String getNextAvailableMacAddress(final long networkConfigId, Integer zoneMacIdentifier) {
         return null;