Include cleanup of n/w resources on n/w deletion + prevent n/w resource deletion on detroy/stop/unplug of VM from persistent n/w

Author: Pearl Dsilva

core/src/main/java/com/cloud/agent/api/CleanupPersistentNetworkResourceAnswer.java

@@ -0,0 +1,27 @@
+// 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.agent.api;
+
+public class CleanupPersistentNetworkResourceAnswer extends Answer{
+    public CleanupPersistentNetworkResourceAnswer() {
+    }
+
+    public CleanupPersistentNetworkResourceAnswer(CleanupPersistentNetworkResourceCommand cmd, boolean success, String result) {
+        super(cmd, success, result);
+    }
+}

core/src/main/java/com/cloud/agent/api/CleanupPersistentNetworkResourceCommand.java

@@ -0,0 +1,43 @@
+// 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.agent.api;
+
+import com.cloud.agent.api.to.NicTO;
+
+public class CleanupPersistentNetworkResourceCommand extends Command{
+    NicTO nicTO;
+
+    protected CleanupPersistentNetworkResourceCommand() {}
+
+    public CleanupPersistentNetworkResourceCommand(NicTO nicTO) {
+        this.nicTO = nicTO;
+    }
+
+    public NicTO getNicTO() {
+        return nicTO;
+    }
+
+    public void setNicTO(NicTO nicTO) {
+        this.nicTO = nicTO;
+    }
+
+    @Override
+    public boolean executeInSequence() {
+        return false;
+    }
+}

core/src/main/java/com/cloud/agent/api/UnPlugNicCommand.java

@@ -19,11 +19,14 @@
 
 package com.cloud.agent.api;
 
+import java.util.Map;
+
 import com.cloud.agent.api.to.NicTO;
 
 public class UnPlugNicCommand extends Command {
     NicTO nic;
     String instanceName;
+    Map<String, Boolean> vlanToPersistenceMap;
 
     public NicTO getNic() {
         return nic;
@@ -45,4 +48,12 @@ public UnPlugNicCommand(NicTO nic, String instanceName) {
     public String getVmName() {
         return instanceName;
     }
+
+    public Map<String, Boolean> getVlanToPersistenceMap() {
+        return vlanToPersistenceMap;
+    }
+
+    public void setVlanToPersistenceMap(Map<String, Boolean> vlanToPersistenceMap) {
+        this.vlanToPersistenceMap = vlanToPersistenceMap;
+    }
 }

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

@@ -1974,7 +1974,6 @@ private void advanceStop(final VMInstanceVO vm, final boolean cleanUpEvenIfUnabl
         vmGuru.prepareStop(profile);
 
         Map<String, Boolean> vlanToPersistenceMap = getVlanToPersistenceMapForVM(vm.getId());
-
         final StopCommand stop = new StopCommand(vm, getExecuteInSequence(vm.getHypervisorType()), false, cleanUpEvenIfUnableToStop);
         stop.setControlIp(getControlNicIpForVM(vm));
         if (MapUtils.isNotEmpty(vlanToPersistenceMap)) {
@@ -4419,6 +4418,10 @@ public boolean unplugNic(final Network network, final NicTO nic, final VirtualMa
             try {
                 final Commands cmds = new Commands(Command.OnError.Stop);
                 final UnPlugNicCommand unplugNicCmd = new UnPlugNicCommand(nic, vm.getName());
+                Map<String, Boolean> vlanToPersistenceMap = getVlanToPersistenceMapForVM(vm.getId());
+                if (MapUtils.isNotEmpty(vlanToPersistenceMap)) {
+                    unplugNicCmd.setVlanToPersistenceMap(vlanToPersistenceMap);
+                }
                 cmds.addCommand("unplugnic", unplugNicCmd);
                 _agentMgr.send(dest.getHost().getId(), cmds);
 

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

@@ -40,6 +40,8 @@
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
+import com.cloud.agent.api.CleanupPersistentNetworkResourceAnswer;
+import com.cloud.agent.api.CleanupPersistentNetworkResourceCommand;
 import com.cloud.agent.api.SetupPersistentNetworkAnswer;
 import com.cloud.agent.api.SetupPersistentNetworkCommand;
 import com.cloud.dc.ClusterVO;
@@ -1223,7 +1225,7 @@ private Pair<Boolean, NicTO> isNtwConfiguredInCluster(HostVO hostVO, Map<Long, L
         if (hosts == null) {
             hosts = new ArrayList<>();
         }
-        if (hostVO.getHypervisorType() == HypervisorType.KVM) {
+        if (hostVO.getHypervisorType() == HypervisorType.KVM || hostVO.getHypervisorType() == HypervisorType.XenServer ) {
             hosts.add(hostVO.getId());
             clusterToHostsMap.put(clusterId, hosts);
             return new Pair<>(false, createNicTOFromNetworkAndOffering(networkVO, networkOfferingVO, hostVO));
@@ -1267,6 +1269,15 @@ private void setupPersistentNetwork(NetworkVO network, NetworkOfferingVO offerin
         }
     }
 
+    private boolean networkMeetsPersistenceCriteria(NetworkVO network, NetworkOfferingVO offering, boolean cleanup) {
+        boolean criteriaMet = offering.isPersistent() &&
+                (network.getBroadcastUri() != null && BroadcastDomainType.getSchemeValue(network.getBroadcastUri()) == BroadcastDomainType.Vlan);
+        if (!cleanup) {
+            return criteriaMet && network.getGuestType() == GuestType.L2;
+        } else {
+            return criteriaMet && (network.getGuestType() == GuestType.L2 || network.getGuestType() == GuestType.Isolated);
+        }
+    }
     @Override
     @DB
     public Pair<NetworkGuru, NetworkVO> implementNetwork(final long networkId, final DeployDestination dest, final ReservationContext context) throws ConcurrentOperationException,
@@ -1326,8 +1337,7 @@ public Pair<NetworkGuru, NetworkVO> implementNetwork(final long networkId, final
             implementNetworkElementsAndResources(dest, context, network, offering);
 
             long dcId = dest.getDataCenter().getId();
-            if (network.getGuestType() == GuestType.L2 && offering.isPersistent() &&
-                    (network.getBroadcastUri() != null && BroadcastDomainType.getSchemeValue(network.getBroadcastUri()) == BroadcastDomainType.Vlan)) {
+            if (networkMeetsPersistenceCriteria(network,offering, false)) {
                 setupPersistentNetwork(network, offering, dcId);
             }
             if (isSharedNetworkWithServices(network)) {
@@ -2948,6 +2958,32 @@ public boolean shutdownNetworkElementsAndResources(final ReservationContext cont
         return success;
     }
 
+    private void cleanupPersistentnNetworkResources(NetworkVO network) {
+        long networkOfferingId = network.getNetworkOfferingId();
+        NetworkOfferingVO offering = _networkOfferingDao.findById(networkOfferingId);
+        if (offering != null) {
+            if (networkMeetsPersistenceCriteria(network, offering, true)) {
+                List<HostVO> hosts = resourceManager.listAllUpAndEnabledHostsInOneZoneByType(Host.Type.Routing, network.getDataCenterId());
+                for (HostVO host : hosts) {
+                    try {
+                        NicTO to = createNicTOFromNetworkAndOffering(network, offering, host);
+                        CleanupPersistentNetworkResourceCommand cmd = new CleanupPersistentNetworkResourceCommand(to);
+                        CleanupPersistentNetworkResourceAnswer answer = (CleanupPersistentNetworkResourceAnswer) _agentMgr.send(host.getId(), cmd);
+                        if (answer == null) {
+                            s_logger.warn("Unable to get an answer to the CleanupPersistentNetworkResourceCommand from agent:" + host.getId());
+                        }
+
+                        if (!answer.getResult()) {
+                            s_logger.warn("Unable to setup agent " + host.getId() + " due to " + answer.getDetails());
+                        }
+                    } catch (Exception e) {
+                        s_logger.warn("Failed to cleanup network resources");
+                    }
+                }
+            }
+        }
+    }
+
     @Override
     @DB
     public boolean destroyNetwork(final long networkId, final ReservationContext context, final boolean forced) {
@@ -2987,6 +3023,8 @@ public boolean destroyNetwork(final long networkId, final ReservationContext con
             }
         }
 
+        cleanupPersistentnNetworkResources(network);
+
         // Shutdown network first
         shutdownNetwork(networkId, context, false);
 

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

@@ -20,6 +20,7 @@
 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;
@@ -67,7 +68,7 @@ public boolean start() {
         if (s_apiNameDiscoveryResponseMap == null) {
             long startTime = System.nanoTime();
             s_apiNameDiscoveryResponseMap = new HashMap<String, ApiDiscoveryResponse>();
-            Set<Class<?>> cmdClasses = new HashSet<Class<?>>();
+            Set<Class<?>> cmdClasses = new LinkedHashSet<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

@@ -437,4 +437,20 @@ public boolean isExistingBridge(String bridgeName) {
             return false;
         }
     }
+
+    @Override
+    public void deleteBr(NicTO nic) {
+        String vlanId = Networks.BroadcastDomainType.getValue(nic.getBroadcastUri());
+        String trafficLabel = nic.getName();
+        String pifName = _pifs.get(trafficLabel);
+        if (pifName == null) {
+            // if not found in bridge map, maybe traffic label refers to pif already?
+            File pif = new File("/sys/class/net/" + trafficLabel);
+            if (pif.isDirectory()) {
+                pifName = trafficLabel;
+            }
+        }
+        String brName = generateVnetBrName(pifName, vlanId);
+        deleteVnetBr(brName, true);
+    }
 }

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

@@ -80,4 +80,7 @@ public void detach(LibvirtVMDef.InterfaceDef iface) {
     public void createControlNetwork(String privBrName) {
     }
 
+    @Override
+    public void deleteBr(NicTO nic) {
+    }
 }

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

@@ -287,6 +287,10 @@ public void createControlNetwork(String privBrName) {
         }
     }
 
+    @Override
+    public void deleteBr(NicTO nic) {
+    }
+
     private boolean isBridgeExists(String bridgeName) {
         File f = new File("/sys/devices/virtual/net/" + bridgeName);
         if (f.exists()) {

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

@@ -260,4 +260,8 @@ public boolean isExistingBridge(String bridgeName) {
             return false;
         }
     }
+
+    @Override
+    public void deleteBr(NicTO nic) {
+    }
 }