Bug 4644: TerminationPoint not remove from switch

if termination point is added to the existing bridge
which was already present before OVSDB node got connected
to the controller. Plugin was not able to delete the termination
point because it was not able to find the connection client and
the parent bridge from the data store change notification on
termination point delete.

Change-Id: I1f720f77aa8005274674903ddb5fa1daf962af5d
Signed-off-by: Anil Vishnoi <vishnoianil@gmail.com>

diff --git a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/OvsdbDataChangeListener.java b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/OvsdbDataChangeListener.java
index b51b3ec920205d4fb0eee260afa36d86fbcae35b..81a7a34f44726c40eeed3b351e0538b5856ad14f 100644 (file)
--- a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/OvsdbDataChangeListener.java
+++ b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/OvsdbDataChangeListener.java
@@ -96,7 +96,7 @@ public class OvsdbDataChangeListener implements ClusteredDataChangeListener, Aut
             OvsdbConnectionInstance connectionInstance = connectionInstanceEntry.getValue();
             connectionInstance.transact(new TransactCommandAggregator(),
                     new BridgeOperationalState(db, changes),
-                    new DataChangesManagedByOvsdbNodeEvent(
+                    new DataChangesManagedByOvsdbNodeEvent(db,
                             connectionInstance.getInstanceIdentifier(),
                             changes));
         }
@@ -218,12 +218,9 @@ public class OvsdbDataChangeListener implements ClusteredDataChangeListener, Aut
                     if (ovsNode != null && ovsNode.getConnectionInfo() != null) {
                         client = cm.getConnectionInstance(ovsNode.getConnectionInfo());
                     } else {
-                        List<TerminationPoint> terminationPoint = ((Node)created.getValue()).getTerminationPoint();
-                        if (!terminationPoint.isEmpty()) {
-                            InstanceIdentifier<Node> nodeIid = SouthboundMapper.
-                                    createInstanceIdentifier(((Node)created.getValue()).getNodeId());
-                            client = cm.getConnectionInstance(nodeIid);
-                        }
+                        InstanceIdentifier<Node> nodeIid = SouthboundMapper.
+                                createInstanceIdentifier(((Node)created.getValue()).getNodeId());
+                        client = cm.getConnectionInstance(nodeIid);
                     }
                 }
                 if (client != null) {

diff --git a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/DataChangesManagedByOvsdbNodeEvent.java b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/DataChangesManagedByOvsdbNodeEvent.java
index 5b4acd4c00e5a7884fb23ce1b2cc01e4dfcac074..c718e8c9ace3aafbe06732231650ade385f379a3 100644 (file)
--- a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/DataChangesManagedByOvsdbNodeEvent.java
+++ b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/DataChangesManagedByOvsdbNodeEvent.java
@@ -14,7 +14,10 @@ import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
 
+import com.google.common.base.Optional;
+import org.opendaylight.controller.md.sal.binding.api.DataBroker;
 import org.opendaylight.controller.md.sal.common.api.data.AsyncDataChangeEvent;
+import org.opendaylight.ovsdb.southbound.SouthboundUtil;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.rev150105.OvsdbBridgeAugmentation;
 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.OvsdbTerminationPointAugmentation;
@@ -26,14 +29,16 @@ public class DataChangesManagedByOvsdbNodeEvent implements
     AsyncDataChangeEvent<InstanceIdentifier<?>, DataObject> {
 
     private InstanceIdentifier<?> iid;
+    private DataBroker db;
     private AsyncDataChangeEvent<InstanceIdentifier<?>, DataObject> event;
     private Map<InstanceIdentifier<?>, DataObject> createdData = null;
     private Map<InstanceIdentifier<?>, DataObject> updatedData = null;
     private Map<InstanceIdentifier<?>, DataObject> originalData = null;
     private Set<InstanceIdentifier<?>> removedPaths;
 
-    public DataChangesManagedByOvsdbNodeEvent(InstanceIdentifier<?> iid,
-            AsyncDataChangeEvent<InstanceIdentifier<?>, DataObject> event) {
+    public DataChangesManagedByOvsdbNodeEvent(DataBroker dataBroker, InstanceIdentifier<?> iid,
+                                              AsyncDataChangeEvent<InstanceIdentifier<?>, DataObject> event) {
+        this.db = dataBroker;
         this.iid = iid;
         this.event = event;
     }
@@ -105,10 +110,30 @@ public class DataChangesManagedByOvsdbNodeEvent implements
         if (managedBy != null && managedBy.equals(iid)) {
             return true;
         }
+
+        managedBy = getManagedByIidFromOperDS(bridgeIid);
+        if (managedBy != null && managedBy.equals(iid)) {
+            return true;
+        }
         return false;
 
     }
 
+    private InstanceIdentifier<?> getManagedByIidFromOperDS(InstanceIdentifier<?> bridgeIid) {
+        // Get the InstanceIdentifier of the containing node
+        InstanceIdentifier<Node> nodeEntryIid = bridgeIid.firstIdentifierOf(Node.class);
+
+        Optional<?> bridgeNode =  SouthboundUtil.readNode(db.newReadWriteTransaction(),nodeEntryIid);
+        if (bridgeNode.isPresent() && bridgeNode.get() instanceof Node) {
+            Node node = (Node)bridgeNode.get();
+            OvsdbBridgeAugmentation bridge = node.getAugmentation(OvsdbBridgeAugmentation.class);
+            if (bridge != null && bridge.getManagedBy() != null) {
+                return bridge.getManagedBy().getValue();
+            }
+        }
+        return null;
+    }
+
     private InstanceIdentifier<?> getManagedByIid(Map<InstanceIdentifier<?>, DataObject> map,
             InstanceIdentifier<?> iidToCheck) {
         // Get the InstanceIdentifier of the containing node

diff --git a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/TerminationPointDeleteCommand.java b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/TerminationPointDeleteCommand.java
index 672169e2470e711d4133796c60f77114684a9882..b59e6db15028fb3959e7c0b5205ec6a9b461d7e4 100644 (file)
--- a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/TerminationPointDeleteCommand.java
+++ b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/TerminationPointDeleteCommand.java
@@ -56,8 +56,18 @@ public class TerminationPointDeleteCommand implements TransactCommand {
             Node originalNode = originalNodes.get(removedTpIid.firstIdentifierOf(Node.class));
             OvsdbBridgeAugmentation originalOvsdbBridgeAugmentation =
                     originalNode.getAugmentation(OvsdbBridgeAugmentation.class);
-            String bridgeName = originalOvsdbBridgeAugmentation != null
-                     ? originalOvsdbBridgeAugmentation.getBridgeName().getValue() : "Bridge name not found";
+            String bridgeName = null;
+            if(originalOvsdbBridgeAugmentation != null) {
+                bridgeName = originalOvsdbBridgeAugmentation.getBridgeName().getValue();
+            } else {
+                Optional<OvsdbBridgeAugmentation> bridgeAug = state.getOvsdbBridgeAugmentation(removedTpIid);
+                if(bridgeAug.isPresent()) {
+                    bridgeName = bridgeAug.get().getBridgeName().getValue();
+                } else {
+                    LOG.error("Bridge does not exist for termination point {}", removedTpIid);
+                }
+            }
+
             Port port = TyperUtils.getTypedRowWrapper(transaction.getDatabaseSchema(), Port.class,null);
             Optional<OvsdbTerminationPointAugmentation> tpAugmentation =
                     state.getOvsdbTerminationPointAugmentation(removedTpIid);

diff --git a/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/OvsdbDataChangeListenerTest.java b/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/OvsdbDataChangeListenerTest.java
index 9cf70ae0dbb99515eed6a1447f3afefd9e81d490..3db9470fe0037d079cc4a5128f4c411e68556e31 100644 (file)
--- a/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/OvsdbDataChangeListenerTest.java
+++ b/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/OvsdbDataChangeListenerTest.java
@@ -120,7 +120,11 @@ public class OvsdbDataChangeListenerTest {
         when(ovsdbDataChangeListener.connectionInstancesFromChanges(any(AsyncDataChangeEvent.class))).thenReturn(map);
         BridgeOperationalState bridgeOperationalState = mock(BridgeOperationalState.class);
         DataChangesManagedByOvsdbNodeEvent dataChangesManagedByOvsdbNodeEvent = mock(DataChangesManagedByOvsdbNodeEvent.class);
-        PowerMockito.whenNew(DataChangesManagedByOvsdbNodeEvent.class).withArguments(any(InstanceIdentifier.class), any(AsyncDataChangeEvent.class)).thenReturn(dataChangesManagedByOvsdbNodeEvent);
+        PowerMockito.whenNew(DataChangesManagedByOvsdbNodeEvent.class).withArguments(
+                any(DataBroker.class),
+                any(InstanceIdentifier.class),
+                any(AsyncDataChangeEvent.class)).thenReturn(dataChangesManagedByOvsdbNodeEvent);
+
         PowerMockito.whenNew(BridgeOperationalState.class).withArguments(any(DataBroker.class), any(AsyncDataChangeEvent.class)).thenReturn(bridgeOperationalState);
 
         when(connectionInstance.getInstanceIdentifier()).thenReturn(iid);
@@ -255,7 +259,6 @@ public class OvsdbDataChangeListenerTest {
         when(SouthboundMapper.createInstanceIdentifier(any(NodeId.class))).thenReturn(nodeIid);
         when(cm.getConnectionInstance(any(InstanceIdentifier.class))).thenReturn(client);
         assertEquals("Error returning correct Map", testResultMap, ovsdbDataChangeListener.connectionInstancesFromMap(map));
-        verify(node).getTerminationPoint();
         verify(cm).getConnectionInstance(any(InstanceIdentifier.class));
     }
 }

diff --git a/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/DataChangesManagedByOvsdbNodeEventTest.java b/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/DataChangesManagedByOvsdbNodeEventTest.java
index 1d4b052b6074ed8370d50ed31840a9a33d041ebb..693b83d5e6b0107139e26f0f7efe818e7b22d20a 100644 (file)
--- a/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/DataChangesManagedByOvsdbNodeEventTest.java
+++ b/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/DataChangesManagedByOvsdbNodeEventTest.java
@@ -22,6 +22,7 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.Mockito;
+import org.opendaylight.controller.md.sal.binding.api.DataBroker;
 import org.opendaylight.controller.md.sal.common.api.data.AsyncDataChangeEvent;
 import org.opendaylight.yangtools.yang.binding.DataObject;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
@@ -35,6 +36,7 @@ import org.powermock.reflect.Whitebox;
 public class DataChangesManagedByOvsdbNodeEventTest {
 
     @Mock private InstanceIdentifier<?> iid;
+    @Mock private DataBroker db;
     @Mock private AsyncDataChangeEvent<InstanceIdentifier<?>, DataObject> event;
     private Set<InstanceIdentifier<?>> removedPaths;
     private DataChangesManagedByOvsdbNodeEvent dataChangesManagedByOvsdbNodeEvent;
@@ -47,7 +49,8 @@ public class DataChangesManagedByOvsdbNodeEventTest {
 
     @Test
     public void testDataChangesManagedByOvsdbNodeEvent() {
-        DataChangesManagedByOvsdbNodeEvent dataChangesManagedByOvsdbNodeEvent1 = new DataChangesManagedByOvsdbNodeEvent(iid, event);
+        DataChangesManagedByOvsdbNodeEvent dataChangesManagedByOvsdbNodeEvent1 = new
+                DataChangesManagedByOvsdbNodeEvent(db, iid, event);
         assertEquals(iid, Whitebox.getInternalState(dataChangesManagedByOvsdbNodeEvent1, "iid"));
         assertEquals(event, Whitebox.getInternalState(dataChangesManagedByOvsdbNodeEvent1, "event"));
     }