NETVIRT-1134 : ModifiedNodeDoesNotExistException bgp/networks in CSIT 42/69442/3
authoreupakir <kiran.n.upadhyaya@ericsson.com>
Sat, 10 Mar 2018 16:23:13 +0000 (21:53 +0530)
committerSam Hague <shague@redhat.com>
Tue, 13 Mar 2018 23:04:53 +0000 (23:04 +0000)
This exception was being thrown when a delete was performed on a
non-existent node.
This was because of the data-remaining in the data-stores that were
created during the initial_create-networks on devstack bringup.

This patch aims at proper handling of deletion of those bgp routes that
have no nexthop.
It also prevents withdrawal of extra-routes from bgp when such the router
for the extra-routes is not associated to a bgpvpn

Change-Id: Ia666014c03b3f3454878487d3d42ef756a8bd2b2
Signed-off-by: eupakir <kiran.n.upadhyaya@ericsson.com>
Signed-off-by: Sam Hague <shague@redhat.com>
bgpmanager/api/src/main/java/org/opendaylight/netvirt/bgpmanager/api/IBgpManager.java
bgpmanager/impl/src/main/java/org/opendaylight/netvirt/bgpmanager/BgpManager.java
vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnInterfaceManager.java
vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnManagerImpl.java

index 5c00826267e18d971e2d7e404338e434b51b3d92..cada07b91df5e489529cdddabef07d0ae72e54ad 100644 (file)
@@ -74,6 +74,8 @@ public interface IBgpManager {
 
     void withdrawPrefix(String rd, String prefix);
 
+    void withdrawPrefixIfPresent(String rd, String prefix);
+
     String getDCGwIP();
 
     void sendNotificationEvent(int code, int subcode);
index 3d8463e352ff43768d8d67c3a56bd66ff1c9620e..e5ca26b2fb8435438dc24d7f1d206e0a38761cd9 100644 (file)
@@ -7,6 +7,9 @@
  */
 package org.opendaylight.netvirt.bgpmanager;
 
+import com.google.common.base.Optional;
+import com.google.common.util.concurrent.FutureCallback;
+import com.google.common.util.concurrent.Futures;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -17,6 +20,9 @@ import javax.annotation.PostConstruct;
 import javax.annotation.PreDestroy;
 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.ReadOnlyTransaction;
+import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.netvirt.bgpmanager.api.IBgpManager;
 import org.opendaylight.netvirt.bgpmanager.oam.BgpAlarmErrorCodes;
 import org.opendaylight.netvirt.bgpmanager.oam.BgpConstants;
@@ -27,7 +33,10 @@ import org.opendaylight.yang.gen.v1.urn.ericsson.params.xml.ns.yang.ebgp.rev1509
 import org.opendaylight.yang.gen.v1.urn.ericsson.params.xml.ns.yang.ebgp.rev150901.Bgp;
 import org.opendaylight.yang.gen.v1.urn.ericsson.params.xml.ns.yang.ebgp.rev150901.TcpMd5SignaturePasswordType;
 import org.opendaylight.yang.gen.v1.urn.ericsson.params.xml.ns.yang.ebgp.rev150901.bgp.Neighbors;
+import org.opendaylight.yang.gen.v1.urn.ericsson.params.xml.ns.yang.ebgp.rev150901.bgp.Networks;
+import org.opendaylight.yang.gen.v1.urn.ericsson.params.xml.ns.yang.ebgp.rev150901.bgp.NetworksKey;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.fibmanager.rev150330.vrfentries.VrfEntry;
+import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -37,12 +46,14 @@ public class BgpManager implements AutoCloseable, IBgpManager {
     private final BgpConfigurationManager bcm;
 
     private final FibDSWriter fibDSWriter;
+    private final DataBroker dataBroker;
     private volatile long qbgprestartTS = 0;
 
     @Inject
-    public BgpManager(final BgpConfigurationManager bcm, final FibDSWriter fibDSWriter) {
+    public BgpManager(final BgpConfigurationManager bcm, final FibDSWriter fibDSWriter, final DataBroker dataBroker) {
         this.bcm = bcm;
         this.fibDSWriter = fibDSWriter;
+        this.dataBroker = dataBroker;
     }
 
     @PostConstruct
@@ -162,6 +173,34 @@ public class BgpManager implements AutoCloseable, IBgpManager {
         LOG.info("WITHDRAW: Removed Prefix rd {} prefix {}", rd, prefix);
     }
 
+    @Override
+    public void withdrawPrefixIfPresent(String rd, String prefix) {
+        InstanceIdentifier<Networks> networksId = InstanceIdentifier.builder(Bgp.class).child(Networks.class,
+                new NetworksKey(rd, prefix)).build();
+        try (ReadOnlyTransaction tx = dataBroker.newReadOnlyTransaction()) {
+            Futures.addCallback(tx.read(LogicalDatastoreType.CONFIGURATION, networksId),
+                new FutureCallback<Optional<Networks>>() {
+                    @Override
+                    public void onSuccess(@Nullable Optional<Networks> networks) {
+                        if (networks != null && networks.isPresent()) {
+                            LOG.info("withdrawPrefixIfPresent: ebgp networks present for rd {} prefix {}"
+                                    + ". Withdrawing..", networks.get().getRd(), networks.get().getPrefixLen());
+                            withdrawPrefix(rd, prefix);
+                        } else {
+                            LOG.error("withdrawPrefixIfPresent: ebgp networks not found for rd {} prefix {}",
+                                    rd, prefix);
+                        }
+                    }
+
+                    @Override
+                    public void onFailure(Throwable throwable) {
+                        LOG.error("withdrwaPrefixIfPresent: Failed to retrieve ebgp networks for rd {} prefix {}",
+                                rd, prefix, throwable);
+                    }
+                });
+        }
+    }
+
     @Override
     public void setQbgpLog(String fileName, String debugLevel) {
         bcm.addLogging(fileName, debugLevel);
index d4d937fabd8b555057271c6f8087ab13d7a30216..05528c6d2550f93b602e97695c76209a1603fc05 100755 (executable)
@@ -778,10 +778,10 @@ public class VpnInterfaceManager extends AsyncDataTreeChangeListenerBase<VpnInte
                             arpResponderHandler.addArpResponderFlow(dpnId, lportTag, interfaceName,
                                     gatewayIp, gwMac.get());
                         } else {
-                            LOG.error("processVpnInterfaceAdjacencies: Gateway MAC for subnet could not be "
+                            LOG.error("processVpnInterfaceAdjacencies: Gateway MAC for subnet ID {} could not be "
                                 + "obtained, cannot create ARP responder flow for interface name {}, vpnName {}, "
                                 + "gwIp {}",
-                                interfaceName, vpnName, gatewayIp);
+                                subnetId, interfaceName, vpnName, gatewayIp);
                         }
                     }
                 } else {
@@ -1346,7 +1346,7 @@ public class VpnInterfaceManager extends AsyncDataTreeChangeListenerBase<VpnInte
                         if (rd.equals(vpnName)) {
                             //this is an internal vpn - the rd is assigned to the vpn instance name;
                             //remove from FIB directly
-                            nhList.forEach((nh) -> removeAdjacencyFromInternalVpn(nextHop, vpnName,
+                            nhList.forEach(removeAdjacencyFromInternalVpn(nextHop, vpnName,
                                     interfaceName, dpnId, writeConfigTxn));
                         } else {
                             removeAdjacencyFromBgpvpn(nextHop, nhList, vpnName, primaryRd, dpnId, rd, interfaceName,
@@ -1356,7 +1356,7 @@ public class VpnInterfaceManager extends AsyncDataTreeChangeListenerBase<VpnInte
                         LOG.error("removeAdjacenciesFromVpn: nextHop empty for ip {} rd {} adjacencyType {}"
                                         + " interface {}", nextHop.getIpAddress(), rd,
                                 nextHop.getAdjacencyType().toString(), interfaceName);
-                        bgpManager.withdrawPrefix(rd, nextHop.getIpAddress());
+                        bgpManager.withdrawPrefixIfPresent(rd, nextHop.getIpAddress());
                         fibManager.removeFibEntry(primaryRd, nextHop.getIpAddress(), writeConfigTxn);
                     }
                 }
index 6ddc4c5b48d64a2d6546be42c86695447a01a87f..da306f7e84d05f75c7209f908c88561580c64ff5 100644 (file)
@@ -336,7 +336,7 @@ public class VpnManagerImpl implements IVpnManager {
                     }
                 }
                 fibManager.removeOrUpdateFibEntry(primaryRd, prefix, tunnelIp, writeConfigTxn);
-                if (VpnUtil.isEligibleForBgp(rd, vpnName, dpnId, null /*networkName*/)) {
+                if (VpnUtil.isEligibleForBgp(primaryRd, vpnName, dpnId, null /*networkName*/)) {
                     // TODO: Might be needed to include nextHop here
                     bgpManager.withdrawPrefix(rd, prefix);
                 }