NPE Exception while processing Interfaces 49/88449/8
authorChetan Arakere Gowdru <chetan.arakere@altencalsoftlabs.com>
Tue, 17 Mar 2020 02:38:50 +0000 (08:08 +0530)
committerChetan Arakere Gowdru <chetan.arakere@altencalsoftlabs.com>
Mon, 13 Apr 2020 05:14:11 +0000 (05:14 +0000)
Description: Its been observed that if the port is created first
followed by setting the external-ids, the termination points are
not updated properly with is external-ids.

Changes are done to cache the bridge Identifier and refer this cache
during interface-update is obtained for the port.

Signed-off-by: Chetan Arakere Gowdru <chetan.arakere@altencalsoftlabs.com>
Change-Id: I8e55733bccfa8dae013dda985678514e1ceb98d5
Signed-off-by: Chetan Arakere Gowdru <chetan.arakere@altencalsoftlabs.com>
southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionInstance.java
southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/transactions/md/OvsdbPortRemoveCommand.java
southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/transactions/md/OvsdbPortUpdateCommand.java
southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/transactions/md/OvsdbPortUpdateCommandTest.java

index 00f8c2df519ce6bec85547d374c03046ba4f18b1..797e6bba26750709d84d74d6ca05e2c84368e986 100644 (file)
@@ -19,6 +19,7 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ExecutionException;
 import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.controller.md.sal.binding.api.DataTreeModification;
@@ -36,6 +37,7 @@ import org.opendaylight.ovsdb.lib.message.MonitorSelect;
 import org.opendaylight.ovsdb.lib.message.TableUpdates;
 import org.opendaylight.ovsdb.lib.notation.Mutator;
 import org.opendaylight.ovsdb.lib.notation.Row;
+import org.opendaylight.ovsdb.lib.notation.UUID;
 import org.opendaylight.ovsdb.lib.operations.Mutate;
 import org.opendaylight.ovsdb.lib.operations.Operation;
 import org.opendaylight.ovsdb.lib.operations.OperationResult;
@@ -77,6 +79,8 @@ public class OvsdbConnectionInstance {
     private Entity connectedEntity;
     private EntityOwnershipCandidateRegistration deviceOwnershipCandidateRegistration;
     private OvsdbNodeAugmentation initialCreateData = null;
+    private Map<UUID, InstanceIdentifier<Node>> ports = new ConcurrentHashMap<>();
+    private Map<String, InstanceIdentifier<Node>> portInterfaces = new ConcurrentHashMap<>();
 
     OvsdbConnectionInstance(final ConnectionInfo key, final OvsdbClient client, final TransactionInvoker txInvoker,
                             final InstanceIdentifier<Node> iid) {
@@ -87,6 +91,30 @@ public class OvsdbConnectionInstance {
         this.instanceIdentifier = iid;
     }
 
+    public void updatePort(UUID uuid, InstanceIdentifier<Node> iid) {
+        ports.put(uuid, iid);
+    }
+
+    public void removePort(UUID uuid) {
+        ports.remove(uuid);
+    }
+
+    public InstanceIdentifier<Node> getPort(UUID uuid) {
+        return ports.get(uuid);
+    }
+
+    public void updatePortInterface(String name, InstanceIdentifier<Node> iid) {
+        portInterfaces.put(name, iid);
+    }
+
+    public void removePortInterface(String name) {
+        portInterfaces.remove(name);
+    }
+
+    public InstanceIdentifier<Node> getPortInterface(String name) {
+        return portInterfaces.get(name);
+    }
+
     /**
      * Apply the given command to the given events, based on the given bridge state.
      *
index a156fa6bc71209b32ae6aa045ca4f166e86e70a1..ed45a2ec78a4db94f71dcec052663ddf1b8c6142 100644 (file)
@@ -7,7 +7,6 @@
  */
 package org.opendaylight.ovsdb.southbound.transactions.md;
 
-import java.util.Collection;
 import java.util.Map;
 import java.util.Map.Entry;
 import org.opendaylight.controller.md.sal.binding.api.ReadWriteTransaction;
@@ -41,15 +40,17 @@ public class OvsdbPortRemoveCommand extends AbstractTransactionCommand {
 
     @Override
     public void execute(final ReadWriteTransaction transaction) {
-        Collection<Port> portRemovedRows = TyperUtils.extractRowsRemoved(
-                Port.class, getUpdates(), getDbSchema()).values();
+        Map<UUID, Port> portRemovedRows = TyperUtils.extractRowsRemoved(
+            Port.class, getUpdates(), getDbSchema());
         Map<UUID, Port> portUpdatedRows = TyperUtils.extractRowsUpdated(
                 Port.class, getUpdates(), getDbSchema());
         Map<UUID,Bridge> bridgeUpdatedRows = TyperUtils.extractRowsUpdated(
                 Bridge.class, getUpdates(), getDbSchema());
         Map<UUID,Bridge> bridgeUpdatedOldRows = TyperUtils.extractRowsOld(
                 Bridge.class, getUpdates(), getDbSchema());
-        for (Port port : portRemovedRows) {
+        for (Entry<UUID, Port> portRemoved: portRemovedRows.entrySet()) {
+            final UUID portUuid = portRemoved.getKey();
+            final Port port = portRemoved.getValue();
             final String portName = port.getName();
             boolean isPortInUpdatedRows = portUpdatedRows.values()
                 .stream().anyMatch(updatedPort -> portName.equals(updatedPort.getName()));
@@ -80,6 +81,9 @@ public class OvsdbPortRemoveCommand extends AbstractTransactionCommand {
                 instanceIdentifierCodec, getOvsdbConnectionInstance(), bridgeData)
                     .child(TerminationPoint.class, new TerminationPointKey(new TpId(portName)));
             transaction.delete(LogicalDatastoreType.OPERATIONAL, nodePath);
+            // Remove from OvsdbConnection Instance cache
+            getOvsdbConnectionInstance().removePort(portUuid);
+            getOvsdbConnectionInstance().removePortInterface(portName);
         }
     }
 }
index 00a1a6121847371d191c2a9c0f816d4edd0aa7e6..70e111d5cbc79d47f7c14b79233fd0162e5cd560 100644 (file)
@@ -139,6 +139,7 @@ public class OvsdbPortUpdateCommand extends AbstractTransactionCommand {
             if (bridgeIid.isPresent()) {
                 NodeId bridgeId = SouthboundMapper.createManagedNodeId(bridgeIid.get());
                 TerminationPointKey tpKey = new TerminationPointKey(new TpId(portName));
+                getOvsdbConnectionInstance().updatePortInterface(portName, bridgeIid.get());
                 TerminationPointBuilder tpBuilder = new TerminationPointBuilder();
                 tpBuilder.withKey(tpKey);
                 tpBuilder.setTpId(tpKey.getTpId());
@@ -167,8 +168,14 @@ public class OvsdbPortUpdateCommand extends AbstractTransactionCommand {
         }
         for (Entry<UUID, Interface> interfaceUpdate : interfaceUpdatedRows.entrySet()) {
             String interfaceName = null;
+            Optional<InstanceIdentifier<Node>> bridgeIid = Optional.absent();
             interfaceName = interfaceUpdatedRows.get(interfaceUpdate.getKey()).getNameColumn().getData();
-            Optional<InstanceIdentifier<Node>> bridgeIid = getTerminationPointBridge(transaction, node, interfaceName);
+            if (getOvsdbConnectionInstance().getPortInterface(interfaceName) != null) {
+                bridgeIid = Optional.of(getOvsdbConnectionInstance().getPortInterface(interfaceName));
+            }
+            if (!bridgeIid.isPresent()) {
+                bridgeIid = getTerminationPointBridge(transaction, node, interfaceName);
+            }
             if (bridgeIid.isPresent()) {
                 TerminationPointKey tpKey = new TerminationPointKey(new TpId(interfaceName));
                 TerminationPointBuilder tpBuilder = new TerminationPointBuilder();
@@ -236,14 +243,23 @@ public class OvsdbPortUpdateCommand extends AbstractTransactionCommand {
     }
 
     private Optional<InstanceIdentifier<Node>> getTerminationPointBridge(UUID portUuid) {
-        for (Entry<UUID, Bridge> entry : this.bridgeUpdatedRows.entrySet()) {
-            UUID bridgeUuid = entry.getKey();
-            if (entry.getValue().getPortsColumn().getData().contains(portUuid)) {
-                return Optional.of(
-                        SouthboundMapper.createInstanceIdentifier(instanceIdentifierCodec, getOvsdbConnectionInstance(),
-                                this.bridgeUpdatedRows.get(bridgeUuid)));
+
+        if (bridgeUpdatedRows != null) {
+            for (Entry<UUID, Bridge> entry : this.bridgeUpdatedRows.entrySet()) {
+                UUID bridgeUuid = entry.getKey();
+                if (this.bridgeUpdatedRows.get(bridgeUuid).getPortsColumn().getData()
+                    .contains(portUuid)) {
+                    InstanceIdentifier<Node> iid = SouthboundMapper.createInstanceIdentifier(
+                        instanceIdentifierCodec, getOvsdbConnectionInstance(),
+                        this.bridgeUpdatedRows.get(bridgeUuid));
+                    getOvsdbConnectionInstance().updatePort(portUuid, iid);
+                    return Optional.of(iid);
+                }
             }
         }
+        if (getOvsdbConnectionInstance().getPort(portUuid) != null) {
+            return Optional.of(getOvsdbConnectionInstance().getPort(portUuid));
+        }
         return Optional.absent();
     }
 
index 3584b65839cd1a469cefc1d421009018143ddf61..e9e82a3ce6e86e958e800a8df77b3d954e9b02e1 100644 (file)
@@ -39,6 +39,7 @@ import org.mockito.Matchers;
 import org.mockito.Mockito;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
+import org.opendaylight.controller.md.sal.binding.api.ReadTransaction;
 import org.opendaylight.controller.md.sal.binding.api.ReadWriteTransaction;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.controller.md.sal.common.api.data.ReadFailedException;
@@ -164,6 +165,9 @@ public class OvsdbPortUpdateCommandTest {
         doNothing().when(ovsdbPortUpdateCommand).updateTerminationPoints(any(ReadWriteTransaction.class),
             any(Node.class));
         ReadWriteTransaction transaction = mock(ReadWriteTransaction.class);
+        PowerMockito.mockStatic(SouthboundUtil.class);
+        PowerMockito.when(SouthboundUtil.readNode(any(ReadTransaction.class), any(InstanceIdentifier.class)))
+            .thenReturn(node);
         ovsdbPortUpdateCommand.execute(transaction);
         verify(ovsdbConnectionInstance).getInstanceIdentifier();
         verify(ovsdbPortUpdateCommand).updateTerminationPoints(any(ReadWriteTransaction.class), any(Node.class));
@@ -247,6 +251,7 @@ public class OvsdbPortUpdateCommandTest {
         Column<GenericTableSchema, String> interfaceColumn = mock(Column.class);
         when(interfaceUpdate.getNameColumn()).thenReturn(interfaceColumn);
         when(interfaceColumn.getData()).thenReturn(INTERFACE_NAME);
+        when(ovsdbPortUpdateCommand.getOvsdbConnectionInstance()).thenReturn(mock(OvsdbConnectionInstance.class));
 
         PowerMockito.doReturn(bridgeIid).when(ovsdbPortUpdateCommand, "getTerminationPointBridge",
                 any(ReadWriteTransaction.class), any(Node.class), anyString());