Bug 1327 : Fixing a few Connection handling issues

Since the library and plugin layers are seperated out into its own OSGi bundles and the initial low-level
OVSDB connection is managed by the library layer, there are cases in which the plugin layer might become
ACTIVE at a later point in time after the actual Passive OVSDB connection is established with the Library.
Also, the ConnectionManager and higher application layers can independently be restarted.

This fix brings in the robustness in the Connection management interactions between Library, Plugin, AD-SAL
and Connection Manager NSF.

In turn this makes the printCache command to work consistently which addresses the issue raised in Bug 1327

Change-Id: Ifc004f01e0306f7e71f908c063472a0b38bb572c
Signed-off-by: Madhu Venugopal <mavenugo@gmail.com>

diff --git a/library/src/main/java/org/opendaylight/ovsdb/lib/OvsdbConnection.java b/library/src/main/java/org/opendaylight/ovsdb/lib/OvsdbConnection.java
index f60a09699b5718e7da606bcb1b73bd1989853ad4..7d6a3088b0572a99f9a4b05a48e3c0053c3a874f 100644 (file)
--- a/library/src/main/java/org/opendaylight/ovsdb/lib/OvsdbConnection.java
+++ b/library/src/main/java/org/opendaylight/ovsdb/lib/OvsdbConnection.java
@@ -11,6 +11,7 @@
 package org.opendaylight.ovsdb.lib;
 
 import java.net.InetAddress;
+import java.util.Collection;
 
 /**
  * OvsDBConnection Interface provides OVSDB connection management APIs which includes
@@ -46,4 +47,11 @@ public interface OvsdbConnection {
      * @param listener Passive Connection listener interested in Passive OVSDB connection requests.
      */
     public void registerForPassiveConnection(OvsdbConnectionListener listener);
+
+    /**
+     * Returns a Collection of all the active OVSDB Connections.
+     *
+     * @return Collection of all the active OVSDB Connections
+     */
+    public Collection<OvsdbClient> getConnections();
 }

diff --git a/library/src/main/java/org/opendaylight/ovsdb/lib/impl/OvsdbConnectionService.java b/library/src/main/java/org/opendaylight/ovsdb/lib/impl/OvsdbConnectionService.java
index cf647d34978151a75727e00d719b7bc62c1f492d..006852352b1ebab9b1520bb99ef6ad8c344473c0 100644 (file)
--- a/library/src/main/java/org/opendaylight/ovsdb/lib/impl/OvsdbConnectionService.java
+++ b/library/src/main/java/org/opendaylight/ovsdb/lib/impl/OvsdbConnectionService.java
@@ -28,6 +28,7 @@ import io.netty.handler.logging.LoggingHandler;
 import io.netty.util.CharsetUtil;
 
 import java.net.InetAddress;
+import java.util.Collection;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ExecutorService;
@@ -234,4 +235,8 @@ public class OvsdbConnectionService implements OvsdbConnection {
             listener.disconnected(client);
         }
     }
+    @Override
+    public Collection<OvsdbClient> getConnections() {
+        return connections.keySet();
+    }
 }

diff --git a/plugin/src/main/java/org/opendaylight/ovsdb/plugin/Activator.java b/plugin/src/main/java/org/opendaylight/ovsdb/plugin/Activator.java
index d2264e0e12fe5ccb1f1bf1277d86d36e493bebd3..96e8102abb80073d0e5bfe5888e5ef995e85c6eb 100644 (file)
--- a/plugin/src/main/java/org/opendaylight/ovsdb/plugin/Activator.java
+++ b/plugin/src/main/java/org/opendaylight/ovsdb/plugin/Activator.java
@@ -113,6 +113,7 @@ public class Activator extends ComponentActivatorAbstractBase {
         if (imp.equals(InventoryService.class)) {
             Dictionary<String, Object> props = new Hashtable<String, Object>();
             props.put(GlobalConstants.PROTOCOLPLUGINTYPE.toString(), "OVS");
+            props.put("scope", "Global");
             c.setInterface(
                     new String[] {IPluginInInventoryService.class.getName(),
                             InventoryServiceInternal.class.getName()}, props);

diff --git a/plugin/src/main/java/org/opendaylight/ovsdb/plugin/Connection.java b/plugin/src/main/java/org/opendaylight/ovsdb/plugin/Connection.java
index 4088af1b8b1a710711b617dbd340f8b98141ecdb..2c7ae15e0be10da7744c5a82d4a78ae1be99c20c 100644 (file)
--- a/plugin/src/main/java/org/opendaylight/ovsdb/plugin/Connection.java
+++ b/plugin/src/main/java/org/opendaylight/ovsdb/plugin/Connection.java
@@ -11,8 +11,6 @@ package org.opendaylight.ovsdb.plugin;
 
 import org.opendaylight.controller.sal.core.ConstructionException;
 import org.opendaylight.controller.sal.core.Node;
-import org.opendaylight.controller.sal.utils.Status;
-import org.opendaylight.controller.sal.utils.StatusCode;
 import org.opendaylight.ovsdb.lib.OvsdbClient;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -72,8 +70,8 @@ public class Connection {
         this.node = node;
     }
 
-    public Status disconnect() {
-        return new Status(StatusCode.SUCCESS);
+    public void disconnect() {
+        client.disconnect();
     }
 
     @Override

diff --git a/plugin/src/main/java/org/opendaylight/ovsdb/plugin/ConnectionService.java b/plugin/src/main/java/org/opendaylight/ovsdb/plugin/ConnectionService.java
index 0bd775517a5ade31e56186e1e5cb6489e88da832..30b4ad5667bad468f6d6068788d580e1339cf1a9 100644 (file)
--- a/plugin/src/main/java/org/opendaylight/ovsdb/plugin/ConnectionService.java
+++ b/plugin/src/main/java/org/opendaylight/ovsdb/plugin/ConnectionService.java
@@ -9,12 +9,12 @@
  */
 package org.opendaylight.ovsdb.plugin;
 
-import io.netty.channel.Channel;
 import io.netty.channel.ChannelHandler;
 
 import java.io.IOException;
 import java.net.InetAddress;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -27,7 +27,6 @@ import org.opendaylight.controller.sal.connection.ConnectionConstants;
 import org.opendaylight.controller.sal.connection.IPluginInConnectionService;
 import org.opendaylight.controller.sal.core.Node;
 import org.opendaylight.controller.sal.core.Property;
-import org.opendaylight.controller.sal.utils.ServiceHelper;
 import org.opendaylight.controller.sal.utils.Status;
 import org.opendaylight.controller.sal.utils.StatusCode;
 import org.opendaylight.ovsdb.lib.MonitorCallBack;
@@ -56,15 +55,12 @@ public class ConnectionService implements IPluginInConnectionService, IConnectio
     protected static final Logger logger = LoggerFactory.getLogger(ConnectionService.class);
 
     // Properties that can be set in config.ini
-    private static final String OVSDB_LISTENPORT = "ovsdb.listenPort";
     private static final Integer defaultOvsdbPort = 6640;
 
     private OvsdbConnection connectionLib;
-    private static Integer ovsdbListenPort = defaultOvsdbPort;
-    private ConcurrentMap<String, Connection> ovsdbConnections;
+    private ConcurrentMap<String, Connection> ovsdbConnections = new ConcurrentHashMap<String, Connection>();
     private List<ChannelHandler> handlers = null;
     private InventoryServiceInternal inventoryServiceInternal;
-    private Channel serverListenChannel = null;
 
     public InventoryServiceInternal getInventoryServiceInternal() {
         return inventoryServiceInternal;
@@ -93,13 +89,6 @@ public class ConnectionService implements IPluginInConnectionService, IConnectio
     }
 
     public void init() {
-        ovsdbConnections = new ConcurrentHashMap<String, Connection>();
-        int listenPort = defaultOvsdbPort;
-        String portString = System.getProperty(OVSDB_LISTENPORT);
-        if (portString != null) {
-            listenPort = Integer.decode(portString).intValue();
-        }
-        ovsdbListenPort = listenPort;
     }
 
     /**
@@ -115,6 +104,10 @@ public class ConnectionService implements IPluginInConnectionService, IConnectio
      * the services provided by the class are registered in the service registry
      */
     void start() {
+        Collection<OvsdbClient> connections = connectionLib.getConnections();
+        for (OvsdbClient client : connections) {
+            this.connected(client);
+        }
     }
 
     /**
@@ -126,7 +119,6 @@ public class ConnectionService implements IPluginInConnectionService, IConnectio
         for (Connection connection : ovsdbConnections.values()) {
             connection.disconnect();
         }
-        serverListenChannel.disconnect();
     }
 
     @Override
@@ -135,7 +127,9 @@ public class ConnectionService implements IPluginInConnectionService, IConnectio
         Connection connection = ovsdbConnections.get(identifier);
         if (connection != null) {
             ovsdbConnections.remove(identifier);
-            return connection.disconnect();
+            connection.disconnect();
+            inventoryServiceInternal.removeNode(node);
+            return new Status(StatusCode.SUCCESS);
         } else {
             return new Status(StatusCode.NOTFOUND);
         }
@@ -268,7 +262,7 @@ public class ConnectionService implements IPluginInConnectionService, IConnectio
         Connection connection = ovsdbConnections.get(identifier);
         OvsdbClient client = connection.getClient();
         if (dbSchema == null) {
-            logger.error("Unable to get Database Schema for the ovsdb connection : {} , database : {}", client.getConnectionInfo(), dbSchema.getName());
+            logger.error("Unable to get Database Schema for the ovsdb connection : {}", client.getConnectionInfo());
             return null;
         }
         Set<String> tables = dbSchema.getTables();
@@ -332,8 +326,7 @@ public class ConnectionService implements IPluginInConnectionService, IConnectio
     public void connected(OvsdbClient client) {
         String identifier = getConnectionIdentifier(client);
         try {
-            ConnectionService connection = (ConnectionService)ServiceHelper.getGlobalInstance(IConnectionServiceInternal.class, this);
-            Node node = connection.handleNewConnection(identifier, client);
+            this.handleNewConnection(identifier, client);
         } catch (InterruptedException | ExecutionException e) {
             e.printStackTrace();
         }
@@ -341,5 +334,8 @@ public class ConnectionService implements IPluginInConnectionService, IConnectio
 
     @Override
     public void disconnected(OvsdbClient client) {
+        Connection connection = ovsdbConnections.get(this.getConnectionIdentifier(client));
+        if (connection == null) return;
+        this.disconnect(connection.getNode());
     }
 }

diff --git a/plugin/src/main/java/org/opendaylight/ovsdb/plugin/InventoryService.java b/plugin/src/main/java/org/opendaylight/ovsdb/plugin/InventoryService.java
index d8359e0e02ab36bfd95b7388af464bbba2fb1d0b..0f9c565f384e6c1bb423273a5a4663597a595f7b 100644 (file)
--- a/plugin/src/main/java/org/opendaylight/ovsdb/plugin/InventoryService.java
+++ b/plugin/src/main/java/org/opendaylight/ovsdb/plugin/InventoryService.java
@@ -51,8 +51,8 @@ public class InventoryService implements IPluginInInventoryService, InventorySer
             .getLogger(InventoryService.class);
     private final Set<IPluginOutInventoryService> pluginOutInventoryServices =
             new CopyOnWriteArraySet<IPluginOutInventoryService>();
-    private ConcurrentMap<Node, Map<String, Property>> nodeProps;
-    private ConcurrentMap<NodeConnector, Map<String, Property>> nodeConnectorProps;
+    private ConcurrentMap<Node, Map<String, Property>> nodeProps = new ConcurrentHashMap<Node, Map<String, Property>>();
+    private ConcurrentMap<NodeConnector, Map<String, Property>> nodeConnectorProps = new ConcurrentHashMap<NodeConnector, Map<String, Property>>();
     private ConcurrentMap<Node, NodeDB> dbCache = Maps.newConcurrentMap();
     private ScheduledExecutorService executor;
     private OVSDBConfigService configurationService;
@@ -63,8 +63,6 @@ public class InventoryService implements IPluginInInventoryService, InventorySer
      *
      */
     public void init() {
-        nodeProps = new ConcurrentHashMap<Node, Map<String, Property>>();
-        nodeConnectorProps = new ConcurrentHashMap<NodeConnector, Map<String, Property>>();
         Node.NodeIDType.registerIDType("OVS", String.class);
         NodeConnector.NodeConnectorIDType.registerIDType("OVS", String.class, "OVS");
         this.executor = Executors.newSingleThreadScheduledExecutor();
@@ -113,17 +111,11 @@ public class InventoryService implements IPluginInInventoryService, InventorySer
         configurationService = null;
     }
 
-    /**
-     * Retrieve nodes from openflow
-     */
     @Override
     public ConcurrentMap<Node, Map<String, Property>> getNodeProps() {
         return nodeProps;
     }
 
-    /**
-     * Retrieve nodeConnectors from openflow
-     */
     @Override
     public ConcurrentMap<NodeConnector, Map<String, Property>> getNodeConnectorProps(
             Boolean refresh) {