From 42127c1a5053071389047e885e9bc09973e62190 Mon Sep 17 00:00:00 2001 From: Chetan Arakere Gowdru Date: Tue, 17 Mar 2020 08:08:50 +0530 Subject: [PATCH] NPE Exception while processing Interfaces 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 Change-Id: I8e55733bccfa8dae013dda985678514e1ceb98d5 Signed-off-by: Chetan Arakere Gowdru --- .../southbound/OvsdbConnectionInstance.java | 28 +++++++++++++++++ .../md/OvsdbPortRemoveCommand.java | 12 +++++--- .../md/OvsdbPortUpdateCommand.java | 30 ++++++++++++++----- .../md/OvsdbPortUpdateCommandTest.java | 5 ++++ 4 files changed, 64 insertions(+), 11 deletions(-) diff --git a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionInstance.java b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionInstance.java index 00f8c2df5..797e6bba2 100644 --- a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionInstance.java +++ b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionInstance.java @@ -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> ports = new ConcurrentHashMap<>(); + private Map> portInterfaces = new ConcurrentHashMap<>(); OvsdbConnectionInstance(final ConnectionInfo key, final OvsdbClient client, final TransactionInvoker txInvoker, final InstanceIdentifier iid) { @@ -87,6 +91,30 @@ public class OvsdbConnectionInstance { this.instanceIdentifier = iid; } + public void updatePort(UUID uuid, InstanceIdentifier iid) { + ports.put(uuid, iid); + } + + public void removePort(UUID uuid) { + ports.remove(uuid); + } + + public InstanceIdentifier getPort(UUID uuid) { + return ports.get(uuid); + } + + public void updatePortInterface(String name, InstanceIdentifier iid) { + portInterfaces.put(name, iid); + } + + public void removePortInterface(String name) { + portInterfaces.remove(name); + } + + public InstanceIdentifier getPortInterface(String name) { + return portInterfaces.get(name); + } + /** * Apply the given command to the given events, based on the given bridge state. * diff --git a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/transactions/md/OvsdbPortRemoveCommand.java b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/transactions/md/OvsdbPortRemoveCommand.java index a156fa6bc..ed45a2ec7 100644 --- a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/transactions/md/OvsdbPortRemoveCommand.java +++ b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/transactions/md/OvsdbPortRemoveCommand.java @@ -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 portRemovedRows = TyperUtils.extractRowsRemoved( - Port.class, getUpdates(), getDbSchema()).values(); + Map portRemovedRows = TyperUtils.extractRowsRemoved( + Port.class, getUpdates(), getDbSchema()); Map portUpdatedRows = TyperUtils.extractRowsUpdated( Port.class, getUpdates(), getDbSchema()); Map bridgeUpdatedRows = TyperUtils.extractRowsUpdated( Bridge.class, getUpdates(), getDbSchema()); Map bridgeUpdatedOldRows = TyperUtils.extractRowsOld( Bridge.class, getUpdates(), getDbSchema()); - for (Port port : portRemovedRows) { + for (Entry 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); } } } diff --git a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/transactions/md/OvsdbPortUpdateCommand.java b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/transactions/md/OvsdbPortUpdateCommand.java index 00a1a6121..70e111d5c 100644 --- a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/transactions/md/OvsdbPortUpdateCommand.java +++ b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/transactions/md/OvsdbPortUpdateCommand.java @@ -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 interfaceUpdate : interfaceUpdatedRows.entrySet()) { String interfaceName = null; + Optional> bridgeIid = Optional.absent(); interfaceName = interfaceUpdatedRows.get(interfaceUpdate.getKey()).getNameColumn().getData(); - Optional> 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> getTerminationPointBridge(UUID portUuid) { - for (Entry 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 entry : this.bridgeUpdatedRows.entrySet()) { + UUID bridgeUuid = entry.getKey(); + if (this.bridgeUpdatedRows.get(bridgeUuid).getPortsColumn().getData() + .contains(portUuid)) { + InstanceIdentifier 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(); } diff --git a/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/transactions/md/OvsdbPortUpdateCommandTest.java b/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/transactions/md/OvsdbPortUpdateCommandTest.java index 3584b6583..e9e82a3ce 100644 --- a/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/transactions/md/OvsdbPortUpdateCommandTest.java +++ b/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/transactions/md/OvsdbPortUpdateCommandTest.java @@ -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 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()); -- 2.36.6