southbound should only monitor Open_vSwitch tables 69/31369/4
authorSam Hague <shague@redhat.com>
Tue, 15 Dec 2015 19:13:58 +0000 (14:13 -0500)
committerSam Hague <shague@redhat.com>
Wed, 16 Dec 2015 18:12:55 +0000 (13:12 -0500)
The patch also adds more clear logging to indicate connection events and indicate which direction the events happened.

Patch Set 4: Review comments. Remove the filter since it doesn't make sense because the caller is asking explictly for the table to monitor.
Patch Set 2, 3: fix unit tests and update commit message

Change-Id: I4ea511906911bf9296e78f57f1b63dc2608c9501
Signed-off-by: Sam Hague <shague@redhat.com>
hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepConnectionInstance.java
hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepConnectionManager.java
hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepSchemaConstants.java
hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepSouthboundUtil.java
southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionInstance.java
southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionManager.java
southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/SouthboundConstants.java
southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/SouthboundUtil.java
southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionInstanceTest.java
southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionManagerTest.java

index 7eec7e18a55ae15b2d880116f0917a54ac7d2cf9..04ec20fa8bedbb5667bacc8a4844c8dae366fd56 100644 (file)
@@ -88,18 +88,17 @@ public class HwvtepConnectionInstance implements OvsdbClient{
             }
 
             try {
-                List<String> databases = getDatabases().get();
-                this.callback = new HwvtepMonitorCallback(this,txInvoker);
-                for (String database : databases) {
-                    DatabaseSchema dbSchema = getSchema(database).get();
-                    if (dbSchema != null) {
-                        monitorAllTables(database, dbSchema, HwvtepSchemaConstants.databaseName);
-                    } else {
-                        LOG.warn("No schema reported for database {} for key {}",database,connectionInfo);
-                    }
+                String database = HwvtepSchemaConstants.HARDWARE_VTEP;
+                DatabaseSchema dbSchema = getSchema(database).get();
+                if (dbSchema != null) {
+                    LOG.info("Monitoring database: {}", database);
+                    callback = new HwvtepMonitorCallback(this, txInvoker);
+                    monitorAllTables(database, dbSchema);
+                } else {
+                    LOG.info("No database {} found on {}", database, connectionInfo);
                 }
             } catch (InterruptedException | ExecutionException e) {
-                LOG.warn("Exception attempting to registerCallbacks {}: {}",connectionInfo,e);
+                LOG.warn("Exception attempting to registerCallbacks {}: ", connectionInfo, e);
             }
         }
     }
@@ -108,7 +107,7 @@ public class HwvtepConnectionInstance implements OvsdbClient{
         if (transactInvokers == null) {
             try {
                 transactInvokers = new HashMap<>();
-                DatabaseSchema dbSchema = getSchema(HwvtepSchemaConstants.databaseName).get();
+                DatabaseSchema dbSchema = getSchema(HwvtepSchemaConstants.HARDWARE_VTEP).get();
                 if(dbSchema != null) {
                     transactInvokers.put(dbSchema, new TransactInvokerImpl(this,dbSchema));
                 }
@@ -118,11 +117,7 @@ public class HwvtepConnectionInstance implements OvsdbClient{
         }
     }
 
-    private void monitorAllTables(String database, DatabaseSchema dbSchema, String filter) {
-        if((filter != null) && (!dbSchema.getName().equals(filter))) {
-            LOG.debug("Not monitoring tables in {}, filter: {}", dbSchema.getName(), filter);
-            return;
-        }
+    private void monitorAllTables(String database, DatabaseSchema dbSchema) {
         Set<String> tables = dbSchema.getTables();
         if (tables != null) {
             List<MonitorRequest> monitorRequests = Lists.newArrayList();
index dfceda097df0ae00b5750d6d948d8bb5aa690c0a..f8e733a62f8d77d4e980cdb0fd311ed240a283ad 100644 (file)
@@ -88,16 +88,24 @@ public class HwvtepConnectionManager implements OvsdbConnectionListener, AutoClo
 
     @Override
     public void connected(@Nonnull final OvsdbClient client) {
+        LOG.info("Library connected {} from {}:{} to {}:{}",
+                client.getConnectionInfo().getType(),
+                client.getConnectionInfo().getRemoteAddress(),
+                client.getConnectionInfo().getRemotePort(),
+                client.getConnectionInfo().getLocalAddress(),
+                client.getConnectionInfo().getLocalPort());
         HwvtepConnectionInstance hwClient = connectedButCallBacksNotRegistered(client);
         registerEntityForOwnership(hwClient);
-        LOG.trace("connected client: {}", client);
     }
 
     @Override
     public void disconnected(OvsdbClient client) {
-        LOG.info("HWVTEP Disconnected from {}:{}. Cleaning up the operational data store"
-                        ,client.getConnectionInfo().getRemoteAddress(),
-                        client.getConnectionInfo().getRemotePort());
+        LOG.info("Library disconnected {} from {}:{} to {}:{}. Cleaning up the operational data store",
+                client.getConnectionInfo().getType(),
+                client.getConnectionInfo().getRemoteAddress(),
+                client.getConnectionInfo().getRemotePort(),
+                client.getConnectionInfo().getLocalAddress(),
+                client.getConnectionInfo().getLocalPort());
         ConnectionInfo key = HwvtepSouthboundMapper.createConnectionInfo(client);
         HwvtepConnectionInstance hwvtepConnectionInstance = getConnectionInstance(key);
         if (hwvtepConnectionInstance != null) {
@@ -110,10 +118,11 @@ public class HwvtepConnectionManager implements OvsdbConnectionListener, AutoClo
         } else {
             LOG.warn("HWVTEP disconnected event did not find connection instance for {}", key);
         }
-        LOG.trace("disconnected client: {}", client);
+        LOG.trace("HwvtepConnectionManager exit disconnected client: {}", client);
     }
 
     public OvsdbClient connect(InstanceIdentifier<Node> iid, HwvtepGlobalAugmentation hwvtepGlobal) throws UnknownHostException {
+        LOG.info("Connecting to {}", HwvtepSouthboundUtil.connectionInfoToString(hwvtepGlobal.getConnectionInfo()));
         InetAddress ip = HwvtepSouthboundMapper.createInetAddress(hwvtepGlobal.getConnectionInfo().getRemoteIp());
         OvsdbClient client = OvsdbConnectionService.getService()
                         .connect(ip, hwvtepGlobal.getConnectionInfo().getRemotePort().getValue());
@@ -131,6 +140,7 @@ public class HwvtepConnectionManager implements OvsdbConnectionListener, AutoClo
         return client;
     }
     public void disconnect(HwvtepGlobalAugmentation ovsdbNode) throws UnknownHostException {
+        LOG.info("Diconnecting from {}", HwvtepSouthboundUtil.connectionInfoToString(ovsdbNode.getConnectionInfo()));
         HwvtepConnectionInstance client = getConnectionInstance(ovsdbNode.getConnectionInfo());
         if (client != null) {
             client.disconnect();
@@ -272,10 +282,10 @@ public class HwvtepConnectionManager implements OvsdbConnectionListener, AutoClo
         Global globalRow = null;
 
         try {
-            dbSchema = connectionInstance.getSchema(HwvtepSchemaConstants.databaseName).get();
+            dbSchema = connectionInstance.getSchema(HwvtepSchemaConstants.HARDWARE_VTEP).get();
         } catch (InterruptedException | ExecutionException e) {
             LOG.warn("Not able to fetch schema for database {} from device {}",
-                    HwvtepSchemaConstants.databaseName,connectionInstance.getConnectionInfo(),e);
+                    HwvtepSchemaConstants.HARDWARE_VTEP,connectionInstance.getConnectionInfo(),e);
         }
 
         if (dbSchema != null) {
index 3cb45904c66713f9b40c425bd4d76d021accccb6..31b70251a88495ccc53882373ba81db7726c0b76 100644 (file)
@@ -9,7 +9,7 @@
 package org.opendaylight.ovsdb.hwvtepsouthbound;
 
 public class HwvtepSchemaConstants {
-    public static final String databaseName = "hardware_vtep";
+    public static final String HARDWARE_VTEP = "hardware_vtep";
     public enum HWVTEPSCHEMATABLES {
         GLOBAL("Global", null, null),
         MANAGER("Manager","Global","managers"),
index 84388736089e1193ae85d7280f2764f469bbd4a6..0c1e3dc8ec6384ea16426cba0b313632f7cfad4c 100644 (file)
@@ -18,6 +18,7 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.hw
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.hwvtep.rev150901.HwvtepGlobalRef;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.hwvtep.rev150901.HwvtepLogicalSwitchAttributes;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.hwvtep.rev150901.HwvtepPhysicalSwitchAttributes;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.hwvtep.rev150901.hwvtep.global.attributes.ConnectionInfo;
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.Node;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.impl.codec.DeserializationException;
@@ -138,5 +139,8 @@ public class HwvtepSouthboundUtil {
             return Optional.absent();
         }
     }
-
+    public static String connectionInfoToString(final ConnectionInfo connectionInfo) {
+        return String.valueOf(
+                connectionInfo.getRemoteIp().getValue()) + ":" + connectionInfo.getRemotePort().getValue();
+    }
 }
index 605f15f8f6b236337d2f161ca15aef06cb0ed0d3..d3a7a05c347636fc4e0a2f0fe4dd0f6cea0ca118 100644 (file)
@@ -98,18 +98,17 @@ public class OvsdbConnectionInstance implements OvsdbClient {
             }
 
             try {
-                List<String> databases = getDatabases().get();
-                this.callback = new OvsdbMonitorCallback(this,txInvoker);
-                for (String database : databases) {
-                    DatabaseSchema dbSchema = getSchema(database).get();
-                    if (dbSchema != null) {
-                        monitorAllTables(database, dbSchema);
-                    } else {
-                        LOG.warn("No schema reported for database {} for key {}",database,connectionInfo);
-                    }
+                String database = SouthboundConstants.OPEN_V_SWITCH;
+                DatabaseSchema dbSchema = getSchema(database).get();
+                if (dbSchema != null) {
+                    LOG.info("Monitoring database: {}", database);
+                    callback = new OvsdbMonitorCallback(this, txInvoker);
+                    monitorAllTables(database, dbSchema);
+                } else {
+                    LOG.info("No database {} found on {}", database, connectionInfo);
                 }
             } catch (InterruptedException | ExecutionException e) {
-                LOG.warn("Exception attempting to registerCallbacks {}: {}",connectionInfo,e);
+                LOG.warn("Exception attempting to registerCallbacks {}: ", connectionInfo, e);
             }
         }
     }
@@ -136,6 +135,7 @@ public class OvsdbConnectionInstance implements OvsdbClient {
         if (tables != null) {
             List<MonitorRequest> monitorRequests = Lists.newArrayList();
             for (String tableName : tables) {
+                LOG.info("Southbound monitoring table {} in {}", tableName, dbSchema.getName());
                 GenericTableSchema tableSchema = dbSchema.table(tableName, GenericTableSchema.class);
                 Set<String> columns = tableSchema.getColumns();
                 MonitorRequestBuilder<GenericTableSchema> monitorBuilder = MonitorRequestBuilder.builder(tableSchema);
index 762efafbfd2ea5ca09e59c7e7a1b66503bbfab88..cc8f2a3b88c289a7ef09f4b696689f2b3e512b45 100644 (file)
@@ -86,7 +86,12 @@ public class OvsdbConnectionManager implements OvsdbConnectionListener, AutoClos
 
     @Override
     public void connected(@Nonnull final OvsdbClient externalClient) {
-
+        LOG.info("Library connected {} from {}:{} to {}:{}",
+                externalClient.getConnectionInfo().getType(),
+                externalClient.getConnectionInfo().getRemoteAddress(),
+                externalClient.getConnectionInfo().getRemotePort(),
+                externalClient.getConnectionInfo().getLocalAddress(),
+                externalClient.getConnectionInfo().getLocalPort());
         OvsdbConnectionInstance client = connectedButCallBacksNotRegistered(externalClient);
 
         // Register Cluster Ownership for ConnectionInfo
@@ -125,9 +130,12 @@ public class OvsdbConnectionManager implements OvsdbConnectionListener, AutoClos
 
     @Override
     public void disconnected(OvsdbClient client) {
-        LOG.info("OVSDB Disconnected from {}:{}. Cleaning up the operational data store"
-                ,client.getConnectionInfo().getRemoteAddress(),
-                client.getConnectionInfo().getRemotePort());
+        LOG.info("Library disconnected {} from {}:{} to {}:{}. Cleaning up the operational data store",
+                client.getConnectionInfo().getType(),
+                client.getConnectionInfo().getRemoteAddress(),
+                client.getConnectionInfo().getRemotePort(),
+                client.getConnectionInfo().getLocalAddress(),
+                client.getConnectionInfo().getLocalPort());
         ConnectionInfo key = SouthboundMapper.createConnectionInfo(client);
         OvsdbConnectionInstance ovsdbConnectionInstance = getConnectionInstance(key);
         if (ovsdbConnectionInstance != null) {
@@ -143,11 +151,13 @@ public class OvsdbConnectionManager implements OvsdbConnectionListener, AutoClos
         } else {
             LOG.warn("disconnected : Connection instance not found for OVSDB Node {} ", key);
         }
-        LOG.trace("OvsdbConnectionManager: disconnected exit");
+        LOG.trace("OvsdbConnectionManager: exit disconnected client: {}", client);
     }
 
     public OvsdbClient connect(InstanceIdentifier<Node> iid,
             OvsdbNodeAugmentation ovsdbNode) throws UnknownHostException {
+        LOG.info("Connecting to {}", SouthboundUtil.connectionInfoToString(ovsdbNode.getConnectionInfo()));
+
         // TODO handle case where we already have a connection
         // TODO use transaction chains to handle ordering issues between disconnected
         // TODO and connected when writing to the operational store
@@ -170,6 +180,7 @@ public class OvsdbConnectionManager implements OvsdbConnectionListener, AutoClos
     }
 
     public void disconnect(OvsdbNodeAugmentation ovsdbNode) throws UnknownHostException {
+        LOG.info("Disconnecting from {}", SouthboundUtil.connectionInfoToString(ovsdbNode.getConnectionInfo()));
         OvsdbConnectionInstance client = getConnectionInstance(ovsdbNode.getConnectionInfo());
         if (client != null) {
             // Unregister Cluster Onwership for ConnectionInfo
index 854d5b2febddd1fb03a980c7606d19bf793048e7..26dde438c9591fabd90cd1e1213febcbf5913475 100755 (executable)
@@ -43,6 +43,7 @@ import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.
 import com.google.common.collect.ImmutableBiMap;
 
 public class SouthboundConstants {
+    public static final String OPEN_V_SWITCH = "Open_vSwitch";
     public static final TopologyId OVSDB_TOPOLOGY_ID = new TopologyId(new Uri("ovsdb:1"));
     public static final String OVSDB_URI_PREFIX = "ovsdb";
     public static final String BRIDGE_URI_PREFIX = "bridge";
index b2a1ed7d66349316a749b706b11026f1da47ab24..2b80cdd3adcaad2d8e93c4d45568baff6b079f24 100644 (file)
@@ -172,4 +172,9 @@ public class SouthboundUtil {
 
         return target;
     }
+
+    public static String connectionInfoToString(final ConnectionInfo connectionInfo) {
+        return String.valueOf(
+                connectionInfo.getRemoteIp().getValue()) + ":" + connectionInfo.getRemotePort().getValue();
+    }
 }
index 14b0acb1f98690dabb5671d0186b07dd30daa403..1088386ecc85df8494ec12eaa2479a5638837145 100644 (file)
@@ -129,7 +129,7 @@ public class OvsdbConnectionInstanceTest {
 
         MemberModifier.suppress(MemberMatcher.method(OvsdbConnectionInstance.class, "monitorAllTables", String.class,DatabaseSchema.class));
         ovsdbConnectionInstance.registerCallbacks();
-        PowerMockito.verifyPrivate(ovsdbConnectionInstance, times(2)).invoke("monitorAllTables", anyString(), any(DatabaseSchema.class));
+        PowerMockito.verifyPrivate(ovsdbConnectionInstance, times(1)).invoke("monitorAllTables", anyString(), any(DatabaseSchema.class));
     }
 
     @SuppressWarnings({ "unchecked", "rawtypes" })
@@ -171,6 +171,7 @@ public class OvsdbConnectionInstanceTest {
         tables.add("tableName1");
         tables.add("tableName2");
         DatabaseSchema dbSchema = mock(DatabaseSchema.class);
+        when(dbSchema.getName()).thenReturn(SouthboundConstants.OPEN_V_SWITCH);
         when(dbSchema.getTables()).thenReturn(tables);
         GenericTableSchema tableSchema = mock(GenericTableSchema.class);
         when(dbSchema.table(anyString(), eq(GenericTableSchema.class))).thenReturn(tableSchema);
index 5a915d97aea128fe4b7079a9406dfe3f9897e273..c49a3daec20a2e1053b425143252770189dbd663 100644 (file)
@@ -39,6 +39,7 @@ import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.rev150105.OvsdbBridgeAttributes;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.rev150105.OvsdbNodeAugmentation;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.rev150105.ovsdb.node.attributes.ConnectionInfo;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.rev150105.ovsdb.node.attributes.ConnectionInfoBuilder;
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.Node;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 import org.powermock.api.mockito.PowerMockito;
@@ -59,6 +60,7 @@ public class OvsdbConnectionManagerTest {
     @Mock private TransactionInvoker txInvoker;
     @Mock private EntityOwnershipService entityOwnershipService;
     @Mock private OvsdbConnection ovsdbConnection;
+    @Mock private OvsdbClient externalClient;
     private Map<ConnectionInfo,OvsdbConnectionInstance> clients;
     private Map<ConnectionInfo,InstanceIdentifier<Node>> instanceIdentifiers;
     private Map<Entity, OvsdbConnectionInstance> entityConnectionMap;
@@ -74,11 +76,19 @@ public class OvsdbConnectionManagerTest {
         MemberModifier.field(OvsdbConnectionManager.class, "ovsdbConnection")
                 .set(ovsdbConnectionManager, ovsdbConnection);
         entityConnectionMap = new ConcurrentHashMap<>();
+
+        externalClient = mock(OvsdbClient.class, Mockito.RETURNS_DEEP_STUBS);
+        when(externalClient.getConnectionInfo().getRemoteAddress()).thenReturn(mock(InetAddress.class));
+        when(externalClient.getConnectionInfo().getRemotePort()).thenReturn(8080);
+        when(externalClient.getConnectionInfo().getLocalAddress()).thenReturn(mock(InetAddress.class));
+        when(externalClient.getConnectionInfo().getLocalPort()).thenReturn(8080);
+
+        PowerMockito.mockStatic(SouthboundUtil.class);
+        when(SouthboundUtil.connectionInfoToString(any(ConnectionInfo.class))).thenReturn("192.18.120.31:8080");
     }
     @Test
     public void testConnected() throws Exception {
         OvsdbConnectionInstance client = mock(OvsdbConnectionInstance.class);
-        OvsdbClient externalClient = mock(OvsdbClient.class);
         MemberModifier.suppress(MemberMatcher.method(OvsdbConnectionManager.class, "connectedButCallBacksNotRegistered", OvsdbClient.class));
         when(ovsdbConnectionManager.connectedButCallBacksNotRegistered(any(OvsdbClient.class))).thenReturn(client);
         doNothing().when(client).registerCallbacks();
@@ -97,15 +107,9 @@ public class OvsdbConnectionManagerTest {
     @SuppressWarnings("unchecked")
     @Test
     public void testConnectedButCallBacksNotRegistered() throws Exception {
-        OvsdbClient externalClient = mock(OvsdbClient.class, Mockito.RETURNS_DEEP_STUBS);
         OvsdbConnectionInstance client = mock(OvsdbConnectionInstance.class);
-
-        InetAddress ip = mock(InetAddress.class);
-
-        when(externalClient.getConnectionInfo().getRemoteAddress()).thenReturn(ip);
-        when(externalClient.getConnectionInfo().getRemotePort()).thenReturn(8080);
-
         ConnectionInfo key = mock(ConnectionInfo.class);
+
         PowerMockito.mockStatic(SouthboundMapper.class);
         when(SouthboundMapper.createConnectionInfo(any(OvsdbClient.class))).thenReturn(key);
 
@@ -127,11 +131,7 @@ public class OvsdbConnectionManagerTest {
 
     @Test
     public void testDisconnected() throws Exception {
-        OvsdbClient client = mock(OvsdbClient.class, Mockito.RETURNS_DEEP_STUBS);
         OvsdbConnectionInstance ovsdbConnectionInstance = mock(OvsdbConnectionInstance.class);
-        InetAddress ip = mock(InetAddress.class);
-        when(client.getConnectionInfo().getRemoteAddress()).thenReturn(ip);
-        when(client.getConnectionInfo().getRemotePort()).thenReturn(8080);
 
         ConnectionInfo key = mock(ConnectionInfo.class);
         PowerMockito.mockStatic(SouthboundMapper.class);
@@ -149,7 +149,7 @@ public class OvsdbConnectionManagerTest {
 
       //TODO: Write unit tests for EntityOwnershipService
         MemberModifier.suppress(MemberMatcher.method(OvsdbConnectionManager.class, "unregisterEntityForOwnership", OvsdbConnectionInstance.class));
-        ovsdbConnectionManager.disconnected(client);
+        ovsdbConnectionManager.disconnected(externalClient);
         Map<ConnectionInfo,OvsdbConnectionInstance> testClients = Whitebox.getInternalState(ovsdbConnectionManager, "clients");
         assertEquals("Error, size of the hashmap is incorrect", 0, testClients.size());
     }