Clean up VPN data on VPN delete.

1. When a VRFTable is not created in FIB due to VPN addition, the VRFTable will
not be available during VPN deletion.
2. The prefix-to-interface, l3-nexthop datastores will not have any data for
a vpn, until a vpn-interface is up and running in the network.
Under such conditions VPN deletion causes
ModifiedNodeDoesNotExistException on trying to delete such datastores
which causes entire transaction which handles clean-up to fail, leave
stale-data in the system.

This further causes new VPN-create with same RDs to fail. This review
fixes the issue.

Change-Id: I3011d81907eb32693de3079bd118cfaaf1b9dfcc
Signed-off-by: Deepthi V V <deepthi.v.v@ericsson.com>

diff --git a/fibmanager/api/src/main/java/org/opendaylight/netvirt/fibmanager/api/IFibManager.java b/fibmanager/api/src/main/java/org/opendaylight/netvirt/fibmanager/api/IFibManager.java
index c65587727767dbbadd50838da5c05749f263dc65..4984f89bfcfafc031123641cc950ce8e95973673 100644 (file)
--- a/fibmanager/api/src/main/java/org/opendaylight/netvirt/fibmanager/api/IFibManager.java
+++ b/fibmanager/api/src/main/java/org/opendaylight/netvirt/fibmanager/api/IFibManager.java
@@ -65,6 +65,8 @@ public interface IFibManager {
     void updateRoutePathForFibEntry(String rd, String prefix, String nextHop,
                                     long label, boolean nextHopAdd, WriteTransaction writeConfigTxn);
 
+    void addVrfTable(String rd, WriteTransaction writeConfigTxn);
+
     void removeVrfTable(String rd, WriteTransaction writeConfigTxn);
 
     void removeInterVPNLinkRouteFlows(String interVpnLinkName,

diff --git a/fibmanager/impl/src/main/java/org/opendaylight/netvirt/fibmanager/FibManagerImpl.java b/fibmanager/impl/src/main/java/org/opendaylight/netvirt/fibmanager/FibManagerImpl.java
index 71ff1f7ef4c4fe7f9a8c187db952a5ae05d2cf9f..cd4beb335ca2b5d5239629ec7ee7f7861d790e64 100755 (executable)
--- a/fibmanager/impl/src/main/java/org/opendaylight/netvirt/fibmanager/FibManagerImpl.java
+++ b/fibmanager/impl/src/main/java/org/opendaylight/netvirt/fibmanager/FibManagerImpl.java
@@ -157,6 +157,11 @@ public class FibManagerImpl implements IFibManager {
         fibUtil.removeVrfTable(rd, writeConfigTxn);
     }
 
+    @Override
+    public void addVrfTable(String rd, WriteTransaction writeConfigTxn) {
+        fibUtil.addVrfTable(rd, writeConfigTxn);
+    }
+
     @Override
     public boolean isVPNConfigured() {
         return this.vpnmanager.isVPNConfigured();

diff --git a/fibmanager/impl/src/main/java/org/opendaylight/netvirt/fibmanager/FibUtil.java b/fibmanager/impl/src/main/java/org/opendaylight/netvirt/fibmanager/FibUtil.java
index 9a8543cb612ab150ad4ee057d4b651b0ce06e1ff..4be3c7b795f3aba7fb9619f7c53835473e20e1f7 100644 (file)
--- a/fibmanager/impl/src/main/java/org/opendaylight/netvirt/fibmanager/FibUtil.java
+++ b/fibmanager/impl/src/main/java/org/opendaylight/netvirt/fibmanager/FibUtil.java
@@ -70,6 +70,7 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.N
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.fibmanager.rev150330.FibEntries;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.fibmanager.rev150330.RouterInterface;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.fibmanager.rev150330.fibentries.VrfTables;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.fibmanager.rev150330.fibentries.VrfTablesBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.fibmanager.rev150330.fibentries.VrfTablesKey;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.fibmanager.rev150330.vrfentries.VrfEntry;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.fibmanager.rev150330.vrfentries.VrfEntryBuilder;
@@ -481,6 +482,21 @@ public class FibUtil {
         }
     }
 
+    public void addVrfTable(String rd, WriteTransaction writeConfigTxn) {
+        LOG.debug("Adding vrf table for rd {}", rd);
+        InstanceIdentifier.InstanceIdentifierBuilder<VrfTables> idBuilder =
+            InstanceIdentifier.builder(FibEntries.class).child(VrfTables.class, new VrfTablesKey(rd));
+        InstanceIdentifier<VrfTables> vrfTableId = idBuilder.build();
+        VrfTablesBuilder vrfTablesBuilder = new VrfTablesBuilder().setKey(new VrfTablesKey(rd))
+            .setRouteDistinguisher(rd).setVrfEntry(new ArrayList<VrfEntry>());
+        if (writeConfigTxn != null) {
+            writeConfigTxn.merge(LogicalDatastoreType.CONFIGURATION, vrfTableId, vrfTablesBuilder.build());
+        } else {
+            MDSALUtil.syncUpdate(dataBroker, LogicalDatastoreType.CONFIGURATION,
+                                 vrfTableId, vrfTablesBuilder.build());
+        }
+    }
+
     public void removeVrfTable(String rd, WriteTransaction writeConfigTxn) {
         LOG.debug("Removing vrf table for rd {}", rd);
         InstanceIdentifier.InstanceIdentifierBuilder<VrfTables> idBuilder =

diff --git a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnInstanceListener.java b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnInstanceListener.java
index 816a821779f6d49ff1c7d9fe82986caeb48461fc..d6250e5b8092fbdbfaa1bd136c83d43b3cde4130 100644 (file)
--- a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnInstanceListener.java
+++ b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnInstanceListener.java
@@ -375,6 +375,9 @@ public class VpnInstanceListener extends AsyncDataTreeChangeListenerBase<VpnInst
                         this.vpnName, primaryRd);
                 return false;
             }
+            synchronized (vpnName.intern()) {
+                fibManager.addVrfTable(primaryRd, null);
+            }
             vpnInterfaceManager.handleVpnsExportingRoutes(this.vpnName, primaryRd);
             return true;
         }

diff --git a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnOpStatusListener.java b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnOpStatusListener.java
index 155bbf6f0c80731cddbb8a405e8c985665612818..031f5d64b73917a3b89f45c11ecee179fae2e6e1 100644 (file)
--- a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnOpStatusListener.java
+++ b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnOpStatusListener.java
@@ -8,16 +8,21 @@
 package org.opendaylight.netvirt.vpnmanager;
 
 import com.google.common.base.Optional;
+import com.google.common.util.concurrent.CheckedFuture;
+import com.google.common.util.concurrent.FutureCallback;
+import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.concurrent.ExecutionException;
 import javax.annotation.PostConstruct;
 import javax.inject.Inject;
 import javax.inject.Singleton;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
 import org.opendaylight.controller.md.sal.binding.api.WriteTransaction;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
+import org.opendaylight.controller.md.sal.common.api.data.TransactionCommitFailedException;
 import org.opendaylight.genius.datastoreutils.AsyncDataTreeChangeListenerBase;
 import org.opendaylight.genius.mdsalutil.interfaces.IMdsalApiManager;
 import org.opendaylight.genius.utils.SystemPropertyReader;
@@ -27,7 +32,11 @@ import org.opendaylight.netvirt.fibmanager.api.IFibManager;
 import org.opendaylight.netvirt.vpnmanager.api.VpnExtraRouteHelper;
 import org.opendaylight.yang.gen.v1.urn.ericsson.params.xml.ns.yang.ebgp.rev150901.AddressFamily;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.idmanager.rev160406.IdManagerService;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3nexthop.rev150409.L3nexthop;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3nexthop.rev150409.l3nexthop.VpnNexthops;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3nexthop.rev150409.l3nexthop.VpnNexthopsKey;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3vpn.rev130911.VpnInstanceOpData;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3vpn.rev130911.prefix.to._interface.VpnIds;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3vpn.rev130911.vpn.instance.op.data.VpnInstanceOpDataEntry;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3vpn.rev130911.vpn.instance.op.data.vpn.instance.op.data.entry.vpntargets.VpnTarget;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3vpn.rev130911.vpn.to.extraroutes.Vpn;
@@ -95,13 +104,18 @@ public class VpnOpStatusListener extends AsyncDataTreeChangeListenerBase<VpnInst
             String primaryRd = update.getVrfId();
             final long vpnId = VpnUtil.getVpnId(dataBroker, vpnName);
             jobCoordinator.enqueueJob("VPN-" + update.getVpnInstanceName(), () -> {
-                WriteTransaction writeTxn = dataBroker.newWriteOnlyTransaction();
+                WriteTransaction writeConfigTxn = dataBroker.newWriteOnlyTransaction();
+                WriteTransaction writeOperTxn = dataBroker.newWriteOnlyTransaction();
                 // Clean up VpnInstanceToVpnId from Config DS
-                VpnUtil.removeVpnIdToVpnInstance(dataBroker, vpnId, writeTxn);
-                VpnUtil.removeVpnInstanceToVpnId(dataBroker, vpnName, writeTxn);
+                VpnUtil.removeVpnIdToVpnInstance(dataBroker, vpnId, writeConfigTxn);
+                VpnUtil.removeVpnInstanceToVpnId(dataBroker, vpnName, writeConfigTxn);
                 LOG.trace("Removed vpnIdentifier for  rd{} vpnname {}", primaryRd, vpnName);
+
                 // Clean up FIB Entries Config DS
-                fibManager.removeVrfTable(primaryRd, null);
+                synchronized (vpnName.intern()) {
+                    fibManager.removeVrfTable(primaryRd, null);
+                }
+
                 // Clean up VPNExtraRoutes Operational DS
                 if (VpnUtil.isBgpVpn(vpnName, primaryRd)) {
                     if (update.getType() == VpnInstanceOpDataEntry.Type.L2) {
@@ -118,26 +132,49 @@ public class VpnOpStatusListener extends AsyncDataTreeChangeListenerBase<VpnInst
                 Optional<Vpn> optVpnToExtraroute = VpnUtil.read(dataBroker,
                         LogicalDatastoreType.OPERATIONAL, vpnToExtraroute);
                 if (optVpnToExtraroute.isPresent()) {
-                    VpnUtil.removeVpnExtraRouteForVpn(dataBroker, vpnName, writeTxn);
+                    VpnUtil.removeVpnExtraRouteForVpn(dataBroker, vpnName, writeOperTxn);
                 }
 
                 if (VpnUtil.isL3VpnOverVxLan(update.getL3vni())) {
                     VpnUtil.removeExternalTunnelDemuxFlows(vpnName, dataBroker, mdsalManager);
                 }
 
-                // Clean up VPNInstanceOpDataEntry
-                VpnUtil.removeVpnOpInstance(dataBroker, primaryRd, writeTxn);
                 // Clean up PrefixToInterface Operational DS
-                VpnUtil.removePrefixToInterfaceForVpnId(dataBroker, vpnId, writeTxn);
+                Optional<VpnIds> optPrefixToIntf = VpnUtil.read(dataBroker, LogicalDatastoreType.OPERATIONAL,
+                                                                VpnUtil.getPrefixToInterfaceIdentifier(vpnId));
+                if (optPrefixToIntf.isPresent()) {
+                    VpnUtil.removePrefixToInterfaceForVpnId(dataBroker, vpnId, writeOperTxn);
+                }
 
                 // Clean up L3NextHop Operational DS
-                VpnUtil.removeL3nexthopForVpnId(dataBroker, vpnId, writeTxn);
+                InstanceIdentifier<VpnNexthops> vpnNextHops = InstanceIdentifier.builder(L3nexthop.class).child(
+                    VpnNexthops.class, new VpnNexthopsKey(vpnId)).build();
+                Optional<VpnNexthops> optL3nexthopForVpnId = VpnUtil.read(dataBroker,
+                                                                          LogicalDatastoreType.OPERATIONAL,
+                                                                          vpnNextHops);
+                if (optL3nexthopForVpnId.isPresent()) {
+                    VpnUtil.removeL3nexthopForVpnId(dataBroker, vpnId, writeOperTxn);
+                }
+
+                // Clean up VPNInstanceOpDataEntry
+                VpnUtil.removeVpnOpInstance(dataBroker, primaryRd, writeOperTxn);
 
-                // Release the ID used for this VPN back to IdManager
-                VpnUtil.releaseId(idManager, VpnConstants.VPN_IDPOOL_NAME, vpnName);
+                // Note: Release the of VpnId will happen in PostDeleteVpnInstancWorker only if
+                // operationalTxn/Config succeeds.
 
+                CheckedFuture<Void, TransactionCommitFailedException> checkFutures = writeOperTxn.submit();
+                try {
+                    checkFutures.get();
+                } catch (InterruptedException | ExecutionException e) {
+                    LOG.error("Error deleting vpn {} ", vpnName);
+                    writeConfigTxn.cancel();
+                    throw new RuntimeException(e);
+                }
                 List<ListenableFuture<Void>> futures = new ArrayList<>();
-                futures.add(writeTxn.submit());
+                futures.add(writeConfigTxn.submit());
+                ListenableFuture<List<Void>> listenableFuture = Futures.allAsList(futures);
+                Futures.addCallback(listenableFuture, new VpnOpStatusListener.PostDeleteVpnInstanceWorker(vpnName));
+                LOG.info("Removed vpn data for vpnname {}", vpnName);
                 return futures;
             }, SystemPropertyReader.getDataStoreJobCoordinatorMaxRetries());
         } else if (update.getVpnState() == VpnInstanceOpDataEntry.VpnState.Created) {
@@ -217,4 +254,32 @@ public class VpnOpStatusListener extends AsyncDataTreeChangeListenerBase<VpnInst
                        final VpnInstanceOpDataEntry value) {
         LOG.debug("add: Ignoring vpn Op {} with rd {}", value.getVpnInstanceName(), value.getVrfId());
     }
+
+    private class PostDeleteVpnInstanceWorker implements FutureCallback<List<Void>> {
+        private final Logger log = LoggerFactory.getLogger(VpnOpStatusListener.PostDeleteVpnInstanceWorker.class);
+        String vpnName;
+
+        PostDeleteVpnInstanceWorker(String vpnName)  {
+            this.vpnName = vpnName;
+        }
+
+        /**
+         * This implies that all the future instances have returned success.
+         * Release the ID used for VPN back to IdManager
+         */
+        @Override
+        public void onSuccess(List<Void> voids) {
+            VpnUtil.releaseId(idManager, VpnConstants.VPN_IDPOOL_NAME, vpnName);
+            log.info("onSuccess: VpnId for VpnName {} is released to IdManager successfully.", vpnName);
+        }
+
+        /**
+         * This method is used to handle failure callbacks.
+         */
+        @Override
+        public void onFailure(Throwable throwable) {
+            log.error("onFailure: Job for vpnInstance: {} failed with exception:",
+                      vpnName , throwable);
+        }
+    }
 }