BgpManager commit for code refactoring & bug fixes

Change-Id: If9b1d5d49b7b527827a4c55fdd9558046383d2b8
Signed-off-by: Manisha Malla <manisha.malla@ericsson.com>

diff --git a/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/BgpConfigurationManager.java b/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/BgpConfigurationManager.java
index b12f9278dc15add82d3233022c198ce5af77a8fb..21e4cb0139216f283118526d923b6a74a49d9bcc 100644 (file)
--- a/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/BgpConfigurationManager.java
+++ b/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/BgpConfigurationManager.java
@@ -40,6 +40,9 @@ public class BgpConfigurationManager {
     private BgpManager bgpManager;
     private final DataBroker broker;
     private static final int MAX_RETRIES_BGP_COMMUNICATION = 1;
+    private enum BgpOp {
+        START_BGP, ADD_NGHBR, DEL_NGHBR
+    }
 
     public BgpConfigurationManager(final DataBroker db, BgpConfiguration bgpCfg, BgpManager bgpMgr) {
         broker = db;
@@ -68,6 +71,10 @@ public class BgpConfigurationManager {
 
         private synchronized void removeBgpRouter(BgpRouter del)
         {
+            bgpManager.disconnect();
+
+            bgpConfiguration.setRouterId("");
+            bgpConfiguration.setAsNum(0);
 
         }
 
@@ -93,23 +100,7 @@ public class BgpConfigurationManager {
             }
 
             if(bgpConfiguration.isConfigUpdated()) {
-                int retryCount = 0;
-                boolean retry = false;
-                do {
-                    try {
-                        if(retry)
-                            LOG.info("Retrying BgpService start.." + "retry count:" + retryCount);
-                        //bgpManager.waitForBgpInit();
-                        bgpManager.startBgpService();
-                        LOG.info("BgpService start done..");
-                        retry = false;
-                    } catch (TException t) {
-                        LOG.info("BgpService start failed..");
-                        retry = true;
-                        retryCount++;
-                    }
-                } while(retry && retryCount <= MAX_RETRIES_BGP_COMMUNICATION);
-
+                configureBgpServer(BgpOp.START_BGP);
                 bgpConfiguration.unsetConfigUpdated();
             }
 
@@ -135,19 +126,7 @@ public class BgpConfigurationManager {
             if(value.getLocalAsNumber() == null || value.getLocalAsIdentifier() == null)
                 return;
 
-            int retryCount = 0;
-            boolean retry = false;
-
-            do {
-                try {
-                    //bgpManager.waitForBgpInit();
-                    bgpManager.startBgpService();
-                    retry = false;
-                } catch (TException t) {
-                    retry = true;
-                    retryCount++;
-                }
-            } while(retry && retryCount <= MAX_RETRIES_BGP_COMMUNICATION);
+            configureBgpServer(BgpOp.START_BGP);
         }
 
         @Override
@@ -203,21 +182,11 @@ public class BgpConfigurationManager {
                 if ((gateway.getPeerAddressType() != null) && (gateway.getPeerAddressType() instanceof org.opendaylight.yang.gen.v1.urn.cisco.params.xml.ns.yang.bgp.rev130715.bgp.neighbors.bgp.neighbor.peer.address.type.IpAddress)) {
                     IpAddress neighborIPAddr = ((org.opendaylight.yang.gen.v1.urn.cisco.params.xml.ns.yang.bgp.rev130715.bgp.neighbors.bgp.neighbor.peer.address.type.IpAddress) gateway.getPeerAddressType()).getIpAddress();
                     LOG.info("Bgp Neighbor IP Address " + neighborIPAddr.getIpv4Address().getValue());
-                    bgpConfiguration.setNeighbourIp("");
-                    bgpConfiguration.setNeighbourAsNum(0);
 
-                    int retryCount = 0;
-                    boolean retry = false;
+                    configureBgpServer(BgpOp.DEL_NGHBR);
 
-                    do {
-                        try {
-                            bgpManager.deleteNeighbor(neighborIPAddr.getIpv4Address().getValue());
-                            retry = false;
-                        } catch (TException t) {
-                            retry = true;
-                            retryCount++;
-                        }
-                    } while(retry && retryCount <= MAX_RETRIES_BGP_COMMUNICATION);
+                    bgpConfiguration.setNeighbourIp("");
+                    bgpConfiguration.setNeighbourAsNum(0);
 
                 }
             }
@@ -233,12 +202,24 @@ public class BgpConfigurationManager {
         }
 
         private synchronized void updateBgpNeighbors(BgpNeighbors original, BgpNeighbors update) {
+
             List<BgpNeighbor> bgpNeighborList = update.getBgpNeighbor();
 
+            //handle the case where there are no neighbors configured - single neighbor entry has been deleted
+            if(bgpNeighborList.isEmpty()) {
+                configureBgpServer(BgpOp.DEL_NGHBR);
+                return;
+            }
+
             //We will always consider the first element of this list, since there can be just one DC Gateway
             BgpNeighbor gateway = bgpNeighborList.get(0);
 
             if(gateway != null) {
+                if(gateway.getAsNumber() != null ||
+                    ((gateway.getPeerAddressType() != null) && (gateway.getPeerAddressType() instanceof org.opendaylight.yang.gen.v1.urn.cisco.params.xml.ns.yang.bgp.rev130715.bgp.neighbors.bgp.neighbor.peer.address.type.IpAddress))) {
+                    //there is an updated neighbor, so we need to delete the old neighbor
+                    configureBgpServer(BgpOp.DEL_NGHBR);
+                }
                 if(gateway.getAsNumber() != null) {
                     LOG.info("Bgp Neighbor AS number " + gateway.getAsNumber());
                     if(bgpConfiguration.getNeighbourAsNum() != gateway.getAsNumber()) {
@@ -257,33 +238,8 @@ public class BgpConfigurationManager {
                 }
             }
             if(bgpConfiguration.isConfigUpdated()) {
-                //delete old neighbor
-                BgpNeighbor old = original.getBgpNeighbor().get(0);
-                IpAddress oldIPAddr = ((org.opendaylight.yang.gen.v1.urn.cisco.params.xml.ns.yang.bgp.rev130715.bgp.neighbors.bgp.neighbor.peer.address.type.IpAddress)old.getPeerAddressType()).getIpAddress();
-                int retryCount = 0;
-                boolean retry = false;
-                do {
-                    try {
-                        bgpManager.deleteNeighbor(oldIPAddr.getIpv4Address().getValue());
-                        retry = false;
-                    } catch (TException t) {
-                        retry = true;
-                        retryCount++;
-                    }
-                } while(retry && retryCount <= MAX_RETRIES_BGP_COMMUNICATION);
-
                 //add the newly configured neighbor
-                retryCount = 0;
-                retry = false;
-                do {
-                    try {
-                        bgpManager.addNeighbor(bgpConfiguration.getNeighbourIp(), bgpConfiguration.getNeighbourAsNum());
-                        retry = false;
-                    } catch (TException t) {
-                        retry = true;
-                        retryCount++;
-                    }
-                } while(retry && retryCount <= MAX_RETRIES_BGP_COMMUNICATION);
+                configureBgpServer(BgpOp.ADD_NGHBR);
             }
         }
 
@@ -315,17 +271,7 @@ public class BgpConfigurationManager {
 
                 }
                 if(bgpConfiguration.getNeighbourAsNum() != 0  && bgpConfiguration.getNeighbourIp() != null) {
-                    int retryCount = 0;
-                    boolean retry = false;
-                    do {
-                        try {
-                            bgpManager.addNeighbor(bgpConfiguration.getNeighbourIp(), bgpConfiguration.getNeighbourAsNum());
-                            retry = false;
-                        } catch (TException t) {
-                            retry = true;
-                            retryCount++;
-                        }
-                    } while(retry && retryCount <= MAX_RETRIES_BGP_COMMUNICATION);
+                    configureBgpServer(BgpOp.ADD_NGHBR);
                 }
             }
 
@@ -344,6 +290,7 @@ public class BgpConfigurationManager {
             return InstanceIdentifier.create(BgpNeighbors.class);
         }
 
+
         @Override
         public void close() throws Exception {
             if (listenerRegistration != null) {
@@ -358,4 +305,31 @@ public class BgpConfigurationManager {
         }
 
     }
+
+    private void configureBgpServer(BgpOp bgpOp) {
+        int retryCount = 0;
+        boolean retry = false;
+        do {
+            try {
+                switch(bgpOp) {
+                    case START_BGP:
+                        bgpManager.startBgpService();
+                        break;
+                    case ADD_NGHBR:
+                        bgpManager.addNeighbor(bgpConfiguration.getNeighbourIp(), bgpConfiguration.getNeighbourAsNum());
+                        break;
+                    case DEL_NGHBR:
+                        bgpManager.deleteNeighbor(bgpConfiguration.getNeighbourIp());
+                        break;
+                    default:
+                        LOG.info("Invalid configuration option");
+                }
+
+                retry = false;
+            } catch (TException t) {
+                retry = true;
+                retryCount++;
+            }
+        } while(retry && retryCount <= MAX_RETRIES_BGP_COMMUNICATION);
+    }
 }

diff --git a/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/BgpManager.java b/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/BgpManager.java
index d4fa53cc69bf6edc48f9a17fecd326d1cb4978d4..cbcaa2e5d9e3a2048f9d6b0cba28f7dca0fd1896 100644 (file)
--- a/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/BgpManager.java
+++ b/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/BgpManager.java
@@ -120,12 +120,13 @@ public class BgpManager implements BindingAwareProvider, AutoCloseable, IBgpMana
             }
 
         }  catch (TException t) {
-            //s_logger.error("Transport error while starting bgp server ", t);
             s_logger.error("Could not set up thrift connection with bgp server");
+            //s_logger.trace("Transport error while starting bgp server ", t);
             reInitConn();
             throw t;
         } catch (Exception e) {
-            s_logger.error("Error while starting bgp server", e);
+            s_logger.error("Error while starting bgp server");
+            //s_logger.trace("Bgp Service not started due to exception", e);
             return;
         }
 
@@ -259,13 +260,12 @@ public class BgpManager implements BindingAwareProvider, AutoCloseable, IBgpMana
 
     @Override
     public void addPrefix(String rd, String prefix, String nextHop, int vpnLabel) throws Exception {
-        if(bgpThriftClient == null) {
-            s_logger.info("Add BGP prefix - bgpThriftClient is null. Unable to add BGP prefix.");
-            return;
-        }
-        if(!hasBgpServiceStarted) {
+
+        if(bgpThriftClient == null || !hasBgpServiceStarted) {
             fibDSWriter.addFibEntryToDS(rd, prefix, nextHop, vpnLabel);
+            return;
         }
+
         try {
             bgpThriftClient.addPrefix(rd, prefix, nextHop, vpnLabel);
         } catch (BgpRouterException b) {
@@ -287,13 +287,11 @@ public class BgpManager implements BindingAwareProvider, AutoCloseable, IBgpMana
 
     @Override
     public void deletePrefix(String rd, String prefix) throws Exception {
-        if(bgpThriftClient == null) {
-            s_logger.info("Delete BGP prefix - bgpThriftClient is null. Unable to delete BGP prefix.");
-            return;
-        }
-        if(!hasBgpServiceStarted) {
+        if(bgpThriftClient == null || !hasBgpServiceStarted) {
             fibDSWriter.removeFibEntryFromDS(rd, prefix);
+            return;
         }
+
         try {
             bgpThriftClient.delPrefix(rd, prefix);
         } catch (BgpRouterException b) {
@@ -326,14 +324,14 @@ public class BgpManager implements BindingAwareProvider, AutoCloseable, IBgpMana
             s_logger.info("Connected to BGP server " + host + " on port " + port);
         } catch (BgpRouterException b) {
             s_logger.error("Failed to connect to BGP server " + host + " on port " + port + " due to BgpRouter Exception number " + b.getErrorCode());
-            s_logger.error("BgpRouterException trace ", b);
+            //_logger.error("BgpRouterException trace ", b);
             throw b;
         } catch (TException t) {
-            s_logger.error("Failed to initialize BGP Connection due to Transport error ", t);
+            s_logger.error("Failed to initialize BGP Connection due to Transport error ");
             throw t;
         }
         catch (Exception e) {
-            s_logger.error("Failed to initialize BGP Connection ", e);
+            s_logger.error("Failed to initialize BGP Connection ");
             throw e;
         }
     }
@@ -521,29 +519,6 @@ public class BgpManager implements BindingAwareProvider, AutoCloseable, IBgpMana
     public void disconnect() {
         bgpThriftClient.disconnect();
     }
-/*
-    public void setRoute(Route r) {
-        s_logger.info("Setting route in VPN Manager");
-        //l3Manager.getVpnInstanceManager().addRoute(r.getRd(), r.getPrefix(), r.getNexthop(), r.getLabel());
-    }*/
-
-    /* For testing purposes */
-    /*public String ribGet() {
-        String family = "ipv4";
-        String format = "json";
-
-        try {
-            List<Route> routeList = bgpThriftClient.getRoutes();
-            Iterator<Route> iter = routeList.iterator();
-            while(iter.hasNext()) {
-                Route r = iter.next();
-                System.out.println("Route:: vrf:" + r.getRd() + " Prefix: " + r.getPrefix() + " Nexthop: " + r.getNexthop() + "Label: " + r.getLabel());
-            }
-        } catch (Exception e) {
-            s_logger.error("Failed getting bgp routes ", e);
-        }
-        return null;
-    }*/
 
 
 }

diff --git a/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/FibDSWriter.java b/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/FibDSWriter.java
index 6737385a8569e8798f2357a81552f31d293bcaf0..8cab61572c9553386d0f581f695e8fd4b2fc51b5 100644 (file)
--- a/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/FibDSWriter.java
+++ b/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/FibDSWriter.java
@@ -48,7 +48,6 @@ public class FibDSWriter {
         InstanceIdentifierBuilder<VrfTables> idBuilder =
             InstanceIdentifier.builder(FibEntries.class).child(VrfTables.class, new VrfTablesKey(rd));
 
-        logger.info("Created idBuilder for VrfTables");
 
         InstanceIdentifier<VrfTables> vrfTableId = idBuilder.build();
         Optional<VrfTables> vrfTable = read(LogicalDatastoreType.CONFIGURATION, vrfTableId);
@@ -69,11 +68,10 @@ public class FibDSWriter {
             VrfTables vrfTableNew = new VrfTablesBuilder().setRouteDistinguisher(rd).
                 setVrfEntry(vrfEntryList).build();
 
-            logger.info("Created VrfTables");
 
             InstanceIdentifier<VrfTables> vrfTableNewId = InstanceIdentifier.builder(FibEntries.class)
                 .child(VrfTables.class, new VrfTablesKey(rd)).build();
-            logger.info("Created idBuilder for new VrfTables");
+
             write(LogicalDatastoreType.CONFIGURATION, vrfTableNewId, vrfTableNew);
         }
 

diff --git a/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/globals/BgpConfiguration.java b/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/globals/BgpConfiguration.java
index ae8bfb4bcd32b9b9daf51128c34cae0ea3e4ee0c..65d4f0a5cd596a675ccc30e9751db2949ff78a18 100644 (file)
--- a/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/globals/BgpConfiguration.java
+++ b/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/globals/BgpConfiguration.java
@@ -10,7 +10,7 @@ package org.opendaylight.bgpmanager.globals;
 
 public class BgpConfiguration {
 
-    long asNum;
+    long asNum = 0;
     String bgpServer = "";
     int bgpPort;
     String routerId = "";

diff --git a/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/thrift/client/implementation/BgpRouter.java b/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/thrift/client/implementation/BgpRouter.java
index 82ac98d8c303b35a60516a9ed9cb0b01b4077904..181e266427071f85f542d432040d882c8ccc6d45 100644 (file)
--- a/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/thrift/client/implementation/BgpRouter.java
+++ b/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/thrift/client/implementation/BgpRouter.java
@@ -78,7 +78,7 @@ public class BgpRouter {
             logger.info("Connecting to BGP Server " + bgpHost + " on port " + bgpPort);
             reInit();
         } catch (Exception e) {
-            logger.error("Failed connecting to BGP server ", e);
+            logger.error("Failed connecting to BGP server ");
             throw e;
         }
     }

diff --git a/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/thrift/server/implementation/BgpUpdateHandler.java b/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/thrift/server/implementation/BgpUpdateHandler.java
index d0825d9ded0f62e356ba49d7db903e2df952b86e..a9aecccc9fd5edd6ccb423cc858a7c0a2add7ecc 100644 (file)
--- a/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/thrift/server/implementation/BgpUpdateHandler.java
+++ b/bgpmanager/bgpmanager-impl/src/main/java/org/opendaylight/bgpmanager/thrift/server/implementation/BgpUpdateHandler.java
@@ -15,14 +15,6 @@ class BgpUpdateHandler implements BgpUpdater.Iface {
     public BgpUpdateHandler(BgpManager bgpMgr, FibDSWriter dsWriter) {
         bgpManager = bgpMgr;
         fibDSWriter = dsWriter;
-
-        //Test
-        onUpdatePushRoute("5", "10.1.1.2", 32, "1.2.3.4", 200);
-        onUpdatePushRoute("5", "10.1.1.3", 32, "1.2.3.5", 400);
-        onUpdatePushRoute("10", "10.10.0.10", 32, "5.4.3.2", 600);
-        onUpdateWithdrawRoute("5", "10.1.1.3", 32);
-
-
     }
 
     public void onUpdatePushRoute(String rd, String prefix, int plen,