KVM: clean up persistent VXLAN network bridges on all hosts on delete

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

@@ -1515,9 +1515,10 @@ 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);
+    protected boolean networkMeetsPersistenceCriteria(NetworkVO network, NetworkOfferingVO offering, boolean cleanup) {
+        BroadcastDomainType broadcastDomainType = BroadcastDomainType.getSchemeValue(network.getBroadcastUri());
+        List<BroadcastDomainType> broadcastTypes = Arrays.asList(BroadcastDomainType.Vlan, BroadcastDomainType.Vxlan);
+        boolean criteriaMet = offering.isPersistent() && broadcastTypes.contains(broadcastDomainType);
         if (!cleanup) {
             return criteriaMet && network.getGuestType() == GuestType.L2;
         } else {

engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestratorTest.java

@@ -59,6 +59,7 @@
 import com.cloud.network.Network.GuestType;
 import com.cloud.network.Network.Service;
 import com.cloud.network.NetworkModel;
+import com.cloud.network.Networks.BroadcastDomainType;
 import com.cloud.network.Networks.TrafficType;
 import com.cloud.network.dao.IPAddressDao;
 import com.cloud.network.dao.IPAddressVO;
@@ -1010,4 +1011,33 @@ public void testImportNicWithIP4Address() throws Exception {
             assertEquals("testtag", nicProfile.getName());
         }
     }
+
+    private NetworkVO mockPersistentNetwork(URI broadcastUri, GuestType guestType) {
+        NetworkVO network = mock(NetworkVO.class);
+        when(network.getBroadcastUri()).thenReturn(broadcastUri);
+        when(network.getGuestType()).thenReturn(guestType);
+        return network;
+    }
+
+    @Test
+    public void testNetworkMeetsPersistenceCriteriaForVlanAndVxlan() {
+        NetworkOfferingVO offering = mock(NetworkOfferingVO.class);
+        when(offering.isPersistent()).thenReturn(true);
+
+        NetworkVO vlanNetwork = mockPersistentNetwork(BroadcastDomainType.Vlan.toUri(100), GuestType.Isolated);
+        NetworkVO vxlanNetwork = mockPersistentNetwork(BroadcastDomainType.Vxlan.toUri(5000), GuestType.Isolated);
+
+        assertTrue(testOrchestrator.networkMeetsPersistenceCriteria(vlanNetwork, offering, true));
+        assertTrue(testOrchestrator.networkMeetsPersistenceCriteria(vxlanNetwork, offering, true));
+    }
+
+    @Test
+    public void testNetworkMeetsPersistenceCriteriaForNonPersistentOffering() {
+        NetworkOfferingVO offering = mock(NetworkOfferingVO.class);
+        when(offering.isPersistent()).thenReturn(false);
+
+        NetworkVO vxlanNetwork = mockPersistentNetwork(BroadcastDomainType.Vxlan.toUri(5000), GuestType.Isolated);
+
+        assertFalse(testOrchestrator.networkMeetsPersistenceCriteria(vxlanNetwork, offering, true));
+    }
 }

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

@@ -343,7 +343,7 @@ private void createVnet(String vnetId, String pif, String brName, String protoco
         }
     }
 
-    private void deleteVnetBr(String brName, boolean deleteBr) {
+    protected void deleteVnetBr(String brName, boolean deleteBr) {
         synchronized (_vnetBridgeMonitor) {
             String cmdout = Script.runSimpleBashScript("ls /sys/class/net/" + brName);
             if (cmdout == null)
@@ -383,6 +383,11 @@ private void deleteVnetBr(String brName, boolean deleteBr) {
             String scriptPath = null;
             if (cmdout != null && cmdout.contains("vxlan")) {
                 scriptPath = _modifyVxlanPath;
+                // Read the pif from the VXLAN device.
+                String vxlanPif = getVxlanPif(vNetId);
+                if (vxlanPif != null) {
+                    pName = vxlanPif;
+                }
             } else {
                 scriptPath = _modifyVlanPath;
             }
@@ -403,6 +408,10 @@ private void deleteVnetBr(String brName, boolean deleteBr) {
         }
     }
 
+    protected String getVxlanPif(String vxlanId) {
+        return Script.runSimpleBashScript("ip -d link show vxlan" + vxlanId + " | grep -o 'dev [^ ]*' | cut -d' ' -f2");
+    }
+
     private void deleteExistingLinkLocalRouteTable(String linkLocalBr) {
         Script command = new Script("/bin/bash", _timeout);
         command.add("-c");
@@ -458,6 +467,14 @@ public boolean isExistingBridge(String bridgeName) {
 
     @Override
     public void deleteBr(NicTO nic) {
+        if (Networks.BroadcastDomainType.getSchemeValue(nic.getBroadcastUri()) == Networks.BroadcastDomainType.Vxlan) {
+            // VXLAN bridges are named after the VNI alone. deleteVnetBr reads the pif from the VXLAN device.
+            String vxlanId = Networks.BroadcastDomainType.getValue(nic.getBroadcastUri());
+            if (vxlanId != null) {
+                deleteVnetBr(generateVxnetBrName(null, vxlanId), true);
+            }
+            return;
+        }
         String vlanId = Networks.BroadcastDomainType.getValue(nic.getBroadcastUri());
         String trafficLabel = nic.getName();
         String pifName = _pifs.get(trafficLabel);

plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriverTest.java

@@ -18,18 +18,22 @@
 
 import java.net.URI;
 import java.net.URISyntaxException;
+import java.util.HashMap;
 
 import org.junit.Assert;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.InjectMocks;
+import org.mockito.MockedConstruction;
+import org.mockito.MockedStatic;
 import org.mockito.Mockito;
 import org.mockito.Spy;
 import org.mockito.junit.MockitoJUnitRunner;
 
 import com.cloud.agent.api.to.NicTO;
 import com.cloud.exception.InternalErrorException;
 import com.cloud.network.Networks;
+import com.cloud.utils.script.Script;
 
 @RunWith(MockitoJUnitRunner.class)
 public class BridgeVifDriverTest {
@@ -100,4 +104,62 @@ public void createStorageVnetBridgeIfNeededCreatesVnetBridgeWhenUntaggedVlan() t
         String result = driver.createStorageVnetBridgeIfNeeded(nic, "trafficLabel", BRIDGE_NAME);
         Assert.assertEquals(BRIDGE_NAME, result);
     }
+
+    private String createAndDeleteBridge(String vNetId, String trafficLabel, Networks.BroadcastDomainType broadcastType) throws InternalErrorException {
+        Mockito.doNothing().when(driver).deleteVnetBr(Mockito.anyString(), Mockito.anyBoolean());
+        String createdBridge;
+        try (MockedConstruction<Script> ignored = Mockito.mockConstruction(Script.class)) {
+            createdBridge = driver.createVnetBr(vNetId, trafficLabel, broadcastType.scheme());
+        }
+
+        NicTO nic = new NicTO();
+        nic.setName(trafficLabel);
+        nic.setBroadcastUri(broadcastType.toUri(vNetId));
+        driver.deleteBr(nic);
+        return createdBridge;
+    }
+
+    @Test
+    public void deleteBrRemovesTheBridgeCreatedForVxlan() throws InternalErrorException {
+        driver._pifs = new HashMap<>();
+
+        String createdBridge = createAndDeleteBridge("5000", "unknownTrafficLabel", Networks.BroadcastDomainType.Vxlan);
+
+        Assert.assertEquals("brvx-5000", createdBridge);
+        Mockito.verify(driver).deleteVnetBr(createdBridge, true);
+    }
+
+    @Test
+    public void deleteBrRemovesTheBridgeCreatedForVlan() throws InternalErrorException {
+        driver._pifs = new HashMap<>();
+        driver._pifs.put("guestLabel", "eth1");
+
+        String createdBridge = createAndDeleteBridge("100", "guestLabel", Networks.BroadcastDomainType.Vlan);
+
+        Assert.assertEquals("breth1-100", createdBridge);
+        Mockito.verify(driver).deleteVnetBr(createdBridge, true);
+    }
+
+    private Script deleteVxlanBridge(String vxlanPif) {
+        Mockito.doReturn(vxlanPif).when(driver).getVxlanPif("5000");
+        try (MockedStatic<Script> script = Mockito.mockStatic(Script.class);
+             MockedConstruction<Script> commands = Mockito.mockConstruction(Script.class)) {
+            script.when(() -> Script.runSimpleBashScript("ls /sys/class/net/brvx-5000")).thenReturn("brif");
+            script.when(() -> Script.runSimpleBashScript("ls /sys/class/net/brvx-5000/brif | tr '\n' ' '")).thenReturn("vxlan5000 ");
+            driver.deleteVnetBr("brvx-5000", true);
+            return commands.constructed().get(0);
+        }
+    }
+
+    @Test
+    public void deleteVnetBrPassesThePifFromTheVxlanDevice() {
+        Script command = deleteVxlanBridge("eth0");
+        Mockito.verify(command).add("-p", "eth0");
+    }
+
+    @Test
+    public void deleteVnetBrKeepsTheParsedPifWhenTheVxlanDeviceHasNone() {
+        Script command = deleteVxlanBridge(null);
+        Mockito.verify(command).add("-p", "vx");
+    }
 }