bug 8674 fix port vlan bindings reconciliation 87/58787/5
authorK.V Suneelu Verma <k.v.suneelu.verma@ericsson.com>
Tue, 13 Jun 2017 06:58:44 +0000 (12:28 +0530)
committerK.V Suneelu Verma <k.v.suneelu.verma@ericsson.com>
Fri, 30 Jun 2017 07:00:00 +0000 (12:30 +0530)
1. When controller is restarted after clearing the config by
removing journals and snapshots, it does not have any config data
for ports.

Do not fire delete of port during reconciliation.
Fire port update to clear the vlan bindings.
Port addition/deletion is always managed by southbound not by controller.
Only port update is managed by controller.

2. Sometimes logical switch does not get deleted from device due to some
local ucast/mcast references to it. Cleanup the references also while
deleting the logical switch. It can still fail if vlan binding reference
is not removed.

3. Make sure that mcast and ucast programming during reconciliation
uses proper physical locators.

Change-Id: I81f095a6e030e05d6702997600ced012d4666c36
Signed-off-by: K.V Suneelu Verma <k.v.suneelu.verma@ericsson.com>
hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/reconciliation/configuration/HwvtepReconciliationTask.java
hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/HwvtepOperationalState.java
hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/LogicalSwitchRemoveCommand.java
hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/PhysicalPortRemoveCommand.java
hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/TransactUtils.java
hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/transact/UcastMacsRemoteUpdateCommand.java
hwvtepsouthbound/hwvtepsouthbound-impl/src/test/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepDataChangeListenerTest.java

index 97dfe5b902cf29b31a4f7e64c35186a79da9cab6..aa8f092a80ae450154ed122567da74864534806d 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016 Ericsson India Global Services Pvt Ltd. and others.  All rights reserved.
+ * Copyright (c) 2016, 2017 Ericsson India Global Services Pvt Ltd. and others.  All rights reserved.
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Public License v1.0 which accompanies this distribution,
@@ -52,8 +52,11 @@ public class HwvtepReconciliationTask extends ReconciliationTask {
         this.mdsalUtils = new MdsalUtils(db);
     }
 
-    private void transactChangesToDevice(Collection<DataTreeModification<Node>> changes) {
-        HwvtepOperationalState hwvtepOperationalState = new HwvtepOperationalState(db, connectionInstance, changes);
+    private void transactChangesToDevice(final Collection<DataTreeModification<Node>> changes,
+                                         final Node globalOperNode,
+                                         final Node psNode) {
+        HwvtepOperationalState hwvtepOperationalState = new HwvtepOperationalState(db, connectionInstance, changes,
+                globalOperNode, psNode);
         hwvtepOperationalState.setInReconciliation(true);
         connectionInstance.transact(new TransactCommandAggregator(hwvtepOperationalState,changes));
     }
@@ -88,7 +91,7 @@ public class HwvtepReconciliationTask extends ReconciliationTask {
                 }
             }
         }
-        transactChangesToDevice(changes);
+        transactChangesToDevice(changes, globalOpNode, psNode);
         return true;
     }
 
index 3a89c516ee4fe245b5b76e630e1d965eef0f4b22..4f6b09c15d673ba9e9d26a90fc117f1bca6ed29f 100644 (file)
@@ -105,6 +105,22 @@ public class HwvtepOperationalState {
         }
     }
 
+    public HwvtepOperationalState(final DataBroker db,
+                                  final HwvtepConnectionInstance connectionInstance,
+                                  final Collection<DataTreeModification<Node>> changes,
+                                  final Node globalOperNode,
+                                  final Node psNode) {
+        this(db, connectionInstance, changes);
+        operationalNodes.put(connectionInstance.getInstanceIdentifier(), globalOperNode);
+        HwvtepGlobalAugmentation globalAugmentation = globalOperNode.getAugmentation(HwvtepGlobalAugmentation.class);
+        if (globalAugmentation != null) {
+            if (!HwvtepSouthboundUtil.isEmpty(globalAugmentation.getSwitches())) {
+                operationalNodes.put((InstanceIdentifier<Node>)
+                        globalAugmentation.getSwitches().get(0).getSwitchRef().getValue(), psNode);
+            }
+        }
+    }
+
     public void readOperationalNodes() {
         if (inReconciliation) {
             return;
index cdc25f81d7b6169f53909dfb1468cbfe6006a4f5..7ba2d7b1555dbaafde7d1c7cbee7954d6de66d4e 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015 China Telecom Beijing Research Institute and others.  All rights reserved.
+ * Copyright (c) 2015, 2017 China Telecom Beijing Research Institute and others.  All rights reserved.
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Public License v1.0 which accompanies this distribution,
@@ -17,11 +17,16 @@ import java.util.Map.Entry;
 import java.util.Objects;
 
 import org.opendaylight.controller.md.sal.binding.api.DataTreeModification;
+import org.opendaylight.ovsdb.hwvtepsouthbound.HwvtepDeviceInfo;
 import org.opendaylight.ovsdb.hwvtepsouthbound.HwvtepSouthboundUtil;
 import org.opendaylight.ovsdb.lib.notation.UUID;
 import org.opendaylight.ovsdb.lib.operations.TransactionBuilder;
 import org.opendaylight.ovsdb.lib.schema.typed.TyperUtils;
 import org.opendaylight.ovsdb.schema.hardwarevtep.LogicalSwitch;
+import org.opendaylight.ovsdb.schema.hardwarevtep.McastMacsLocal;
+import org.opendaylight.ovsdb.schema.hardwarevtep.McastMacsRemote;
+import org.opendaylight.ovsdb.schema.hardwarevtep.UcastMacsLocal;
+import org.opendaylight.ovsdb.schema.hardwarevtep.UcastMacsRemote;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.hwvtep.rev150901.HwvtepGlobalAugmentation;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.hwvtep.rev150901.hwvtep.global.attributes.LogicalSwitches;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.hwvtep.rev150901.hwvtep.global.attributes.RemoteMcastMacs;
@@ -99,17 +104,35 @@ public class LogicalSwitchRemoveCommand extends AbstractTransactCommand<LogicalS
                                     final LogicalSwitches lswitch,
                                     final InstanceIdentifier lsKey,
                                     final Object... extraData) {
-            LOG.debug("Removing logcial switch named: {}", lswitch.getHwvtepNodeName().getValue());
-            Optional<LogicalSwitches> operationalSwitchOptional =
-                    getOperationalState().getLogicalSwitches(instanceIdentifier, lswitch.getKey());
+            LOG.debug("Removing logical switch named: {}", lswitch.getHwvtepNodeName().getValue());
+            HwvtepDeviceInfo.DeviceData deviceData  = getOperationalState().getDeviceInfo().getDeviceOperData(
+                    LogicalSwitches.class, lsKey);
             LogicalSwitch logicalSwitch = TyperUtils.getTypedRowWrapper(transaction.getDatabaseSchema(), LogicalSwitch.class, null);
 
-            if (operationalSwitchOptional.isPresent() &&
-                    operationalSwitchOptional.get().getLogicalSwitchUuid() != null) {
-                UUID logicalSwitchUuid = new UUID(operationalSwitchOptional.get().getLogicalSwitchUuid().getValue());
+            if (deviceData != null && deviceData.getUuid() != null) {
+                UUID logicalSwitchUuid = deviceData.getUuid();
                 transaction.add(op.delete(logicalSwitch.getSchema())
                         .where(logicalSwitch.getUuidColumn().getSchema().opEqual(logicalSwitchUuid)).build());
-                transaction.add(op.comment("Logical Switch: Deleting " + lswitch.getHwvtepNodeName().getValue()));
+
+                UcastMacsRemote ucastMacsRemote = TyperUtils.getTypedRowWrapper(transaction.getDatabaseSchema(),
+                        UcastMacsRemote.class, null);
+                transaction.add(op.delete(ucastMacsRemote.getSchema())
+                        .where(ucastMacsRemote.getLogicalSwitchColumn().getSchema().opEqual(logicalSwitchUuid)).build());
+
+                UcastMacsLocal ucastMacsLocal = TyperUtils.getTypedRowWrapper(transaction.getDatabaseSchema(),
+                        UcastMacsLocal.class, null);
+                transaction.add(op.delete(ucastMacsLocal.getSchema())
+                        .where(ucastMacsLocal.getLogicalSwitchColumn().getSchema().opEqual(logicalSwitchUuid)).build());
+
+                McastMacsRemote mcastMacsRemote = TyperUtils.getTypedRowWrapper(transaction.getDatabaseSchema(),
+                        McastMacsRemote.class, null);
+                transaction.add(op.delete(mcastMacsRemote.getSchema())
+                        .where(mcastMacsRemote.getLogicalSwitchColumn().getSchema().opEqual(logicalSwitchUuid)).build());
+
+                McastMacsLocal mcastMacsLocal = TyperUtils.getTypedRowWrapper(transaction.getDatabaseSchema(),
+                        McastMacsLocal.class, null);
+                transaction.add(op.delete(mcastMacsLocal.getSchema())
+                        .where(mcastMacsLocal.getLogicalSwitchColumn().getSchema().opEqual(logicalSwitchUuid)).build());
                 getOperationalState().getDeviceInfo().markKeyAsInTransit(RemoteMcastMacs.class, lsKey);
             } else {
                 LOG.warn("Unable to delete logical switch {} because it was not found in the operational store",
index daa89081407d3a1fdeaf54d34f89323b9fa2cd63..f412c2b275dd2a8e13892cf58b97596ce3e7c2b1 100644 (file)
@@ -12,7 +12,6 @@ import static org.opendaylight.ovsdb.lib.operations.Operations.op;
 
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -20,12 +19,9 @@ import java.util.Map.Entry;
 
 import org.opendaylight.controller.md.sal.binding.api.DataObjectModification;
 import org.opendaylight.controller.md.sal.binding.api.DataTreeModification;
-import org.opendaylight.ovsdb.lib.notation.Mutator;
-import org.opendaylight.ovsdb.lib.notation.UUID;
 import org.opendaylight.ovsdb.lib.operations.TransactionBuilder;
 import org.opendaylight.ovsdb.lib.schema.typed.TyperUtils;
 import org.opendaylight.ovsdb.schema.hardwarevtep.PhysicalPort;
-import org.opendaylight.ovsdb.schema.hardwarevtep.PhysicalSwitch;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.hwvtep.rev150901.HwvtepPhysicalPortAugmentation;
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.Node;
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.node.TerminationPoint;
@@ -51,34 +47,33 @@ public class PhysicalPortRemoveCommand extends AbstractTransactCommand {
         if (!removeds.isEmpty()) {
             for (Entry<InstanceIdentifier<Node>, List<HwvtepPhysicalPortAugmentation>> removed:
                 removeds.entrySet()) {
-                removePhysicalPort(transaction,  removed.getKey(), removed.getValue());
+                updatePhysicalPort(transaction, removed.getKey(), removed.getValue());
             }
         }
     }
 
-    private void removePhysicalPort(TransactionBuilder transaction,
-            InstanceIdentifier<Node> psNodeiid,
-            List<HwvtepPhysicalPortAugmentation> listPort) {
+    private void updatePhysicalPort(final TransactionBuilder transaction,
+                                    final InstanceIdentifier<Node> psNodeiid,
+                                    final List<HwvtepPhysicalPortAugmentation> listPort) {
         for (HwvtepPhysicalPortAugmentation port : listPort) {
-            LOG.debug("Removing a physical port named: {}", port.getHwvtepNodeName().getValue());
+            LOG.debug("Updating a physical port named: {}", port.getHwvtepNodeName().getValue());
             Optional<HwvtepPhysicalPortAugmentation> operationalPhysicalPortOptional =
                     getOperationalState().getPhysicalPortAugmentation(psNodeiid, port.getHwvtepNodeName());
-            PhysicalPort physicalPort = TyperUtils.getTypedRowWrapper(transaction.getDatabaseSchema(), PhysicalPort.class, null);
-            //get managing global node of physicalSwitchBelong
-            //InstanceIdentifier<?> globalNodeIid = physicalSwitchBelong.getManagedBy().getValue();
             if (operationalPhysicalPortOptional.isPresent()) {
-                UUID physicalPortUuid = new UUID(operationalPhysicalPortOptional.get().getPhysicalPortUuid().getValue());
-                PhysicalSwitch physicalSwitch = TyperUtils.getTypedRowWrapper(transaction.getDatabaseSchema(),
-                        PhysicalSwitch.class, null);
-                transaction.add(op.delete(physicalPort.getSchema())
-                        .where(physicalPort.getUuidColumn().getSchema().opEqual(physicalPortUuid)).build());
-                transaction.add(op.comment("Physical Port: Deleting " + port.getHwvtepNodeName().getValue()));
-                transaction.add(op.mutate(physicalSwitch.getSchema())
-                        .addMutation(physicalSwitch.getPortsColumn().getSchema(), Mutator.DELETE,
-                                Collections.singleton(physicalPortUuid)));
-                transaction.add(op.comment("Physical Switch: Mutating " + port.getHwvtepNodeName().getValue() + " " + physicalPortUuid));
+                PhysicalPort physicalPort = TyperUtils.getTypedRowWrapper(
+                        transaction.getDatabaseSchema(),PhysicalPort.class);
+                physicalPort.setVlanBindings(new HashMap<>());
+                HwvtepPhysicalPortAugmentation updatedPhysicalPort = operationalPhysicalPortOptional.get();
+                String existingPhysicalPortName = updatedPhysicalPort.getHwvtepNodeName().getValue();
+                PhysicalPort extraPhyscialPort =
+                        TyperUtils.getTypedRowWrapper(transaction.getDatabaseSchema(), PhysicalPort.class);
+                extraPhyscialPort.setName("");
+                LOG.trace("execute: updating physical port: {}", physicalPort);
+                transaction.add(op.update(physicalPort)
+                        .where(extraPhyscialPort.getNameColumn().getSchema().opEqual(existingPhysicalPortName))
+                        .build());
             } else {
-                LOG.warn("Unable to delete logical switch {} because it was not found in the operational store, "
+                LOG.warn("Unable to update physical port {} because it was not found in the operational store, "
                         + "and thus we cannot retrieve its UUID", port.getHwvtepNodeName().getValue());
             }
         }
index 903aae29940e5f4dd5fd97a3deb3c69a301206bd..76cdce1977dc2fb2ec17a6a0c28ff6c85d698457 100644 (file)
@@ -22,6 +22,7 @@ import org.opendaylight.controller.md.sal.binding.api.ReadWriteTransaction;
 import org.opendaylight.controller.md.sal.binding.api.DataObjectModification.ModificationType;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.controller.md.sal.common.api.data.ReadFailedException;
+import org.opendaylight.ovsdb.hwvtepsouthbound.HwvtepDeviceInfo;
 import org.opendaylight.ovsdb.hwvtepsouthbound.HwvtepSouthboundConstants;
 import org.opendaylight.ovsdb.hwvtepsouthbound.HwvtepSouthboundMapper;
 import org.opendaylight.ovsdb.lib.notation.UUID;
@@ -139,24 +140,11 @@ public class TransactUtils {
     }
 
     public static UUID createPhysicalLocatorSet(HwvtepOperationalState hwvtepOperationalState, TransactionBuilder transaction, List<LocatorSet> locatorList) {
-        Set<UUID> locators = new HashSet<>();
+        Set<UUID> locators = new HashSet<UUID>();
         for (LocatorSet locator: locatorList) {
-            UUID locatorUuid = null;
             @SuppressWarnings("unchecked")
             InstanceIdentifier<TerminationPoint> iid =(InstanceIdentifier<TerminationPoint>) locator.getLocatorRef().getValue();
-            //try to find locator in operational DS
-            Optional<HwvtepPhysicalLocatorAugmentation> operationalLocatorOptional =
-                    hwvtepOperationalState.getPhysicalLocatorAugmentation(iid);
-            if (operationalLocatorOptional.isPresent()) {
-                //if exist, get uuid
-                HwvtepPhysicalLocatorAugmentation locatorAugmentation = operationalLocatorOptional.get();
-                locatorUuid = new UUID(locatorAugmentation.getPhysicalLocatorUuid().getValue());
-            } else {
-                locatorUuid = hwvtepOperationalState.getUUIDFromCurrentTx(TerminationPoint.class, iid);
-                if (locatorUuid == null) {
-                    locatorUuid = createPhysicalLocator(transaction, hwvtepOperationalState, iid);
-                }
-            }
+            UUID locatorUuid = createPhysicalLocator(transaction, hwvtepOperationalState, iid);
             if (locatorUuid != null) {
                 locators.add(locatorUuid);
             }
@@ -170,20 +158,25 @@ public class TransactUtils {
 
     public static UUID createPhysicalLocator(TransactionBuilder transaction, HwvtepOperationalState operationalState,
                                              InstanceIdentifier<TerminationPoint> iid) {
-        Optional<TerminationPoint> configLocatorOptional = TransactUtils.readNodeFromConfig(
-                operationalState.getReadWriteTransaction(), iid);
+        UUID locatorUuid = null;
+        HwvtepDeviceInfo.DeviceData deviceData = operationalState.getDeviceInfo().getDeviceOperData(
+                TerminationPoint.class, iid);
+        if (deviceData != null && deviceData.getUuid() != null) {
+            locatorUuid = deviceData.getUuid();
+            return locatorUuid;
+        }
+        locatorUuid = operationalState.getUUIDFromCurrentTx(TerminationPoint.class, iid);
+        if (locatorUuid != null) {
+            return locatorUuid;
+        }
         HwvtepPhysicalLocatorAugmentationBuilder builder = new HwvtepPhysicalLocatorAugmentationBuilder();
         HwvtepPhysicalLocatorAugmentation locatorAugmentation = null;
-        if (configLocatorOptional.isPresent()) {
-            locatorAugmentation = configLocatorOptional.get().getAugmentation(HwvtepPhysicalLocatorAugmentation.class);
-        } else {
-            builder.setEncapsulationType(EncapsulationTypeVxlanOverIpv4.class);
-            String tepKey = iid.firstKeyOf(TerminationPoint.class).getTpId().getValue();
-            String ip = tepKey.substring(tepKey.indexOf(":")+1);
-            builder.setDstIp(new IpAddress(ip.toCharArray()));
-            locatorAugmentation = builder.build();
-        }
-        UUID locatorUuid = TransactUtils.createPhysicalLocator(transaction, locatorAugmentation);
+        builder.setEncapsulationType(EncapsulationTypeVxlanOverIpv4.class);
+        String tepKey = iid.firstKeyOf(TerminationPoint.class).getTpId().getValue();
+        String ip = tepKey.substring(tepKey.indexOf(":")+1);
+        builder.setDstIp(new IpAddress(ip.toCharArray()));
+        locatorAugmentation = builder.build();
+        locatorUuid = TransactUtils.createPhysicalLocator(transaction, locatorAugmentation);
         operationalState.updateCurrentTxData(TerminationPoint.class, iid, locatorUuid);
         operationalState.getDeviceInfo().markKeyAsInTransit(TerminationPoint.class, iid);
         return locatorUuid;
index 20ad6745ff3f7a843f1d3611a520915630599569..13665e2b8f5d4cadf08bca999f7e0ed37791a83d 100644 (file)
@@ -126,22 +126,10 @@ public class UcastMacsRemoteUpdateCommand extends AbstractTransactCommand<Remote
     private void setLocator(TransactionBuilder transaction, UcastMacsRemote ucastMacsRemote, RemoteUcastMacs inputMac) {
         //get UUID by locatorRef
         if (inputMac.getLocatorRef() != null) {
-            UUID locatorUuid = null;
             @SuppressWarnings("unchecked")
-            InstanceIdentifier<TerminationPoint> iid = (InstanceIdentifier<TerminationPoint>) inputMac.getLocatorRef().getValue();
-            //try to find locator in operational DS
-            HwvtepDeviceInfo.DeviceData deviceData = getOperationalState().getDeviceInfo().getDeviceOperData(TerminationPoint.class, iid);
-            if (deviceData != null) {
-                //if exist, get uuid
-                locatorUuid = deviceData.getUuid();
-            } else {
-                locatorUuid = getOperationalState().getUUIDFromCurrentTx(TerminationPoint.class, iid);
-                if (locatorUuid == null) {
-                    locatorUuid = TransactUtils.createPhysicalLocator(transaction, getOperationalState(),
-                            (InstanceIdentifier<TerminationPoint>) inputMac.getLocatorRef().getValue());
-                    updateCurrentTxData(TerminationPoint.class, iid, locatorUuid, null);
-                }
-            }
+            InstanceIdentifier<TerminationPoint> iid = (InstanceIdentifier<TerminationPoint>)
+                    inputMac.getLocatorRef().getValue();
+            UUID locatorUuid = TransactUtils.createPhysicalLocator(transaction, getOperationalState(), iid);
             if (locatorUuid != null) {
                 ucastMacsRemote.setLocator(locatorUuid);
             }
index c7189b7bbf3adfa849a2ddbc26400f95e99f78fd..edb6f1271fc7ed470b72564fec2b82409038e4dc 100644 (file)
@@ -132,7 +132,7 @@ public class HwvtepDataChangeListenerTest extends DataChangeListenerTestBase {
         addData(OPERATIONAL, LogicalSwitches.class, logicalSwitches);
         resetOperations();
         deleteData(CONFIGURATION, LogicalSwitches.class, logicalSwitches);
-        verify(Operations.op,  times(2)).delete(any());
+        verify(Operations.op,  times(10)).delete(any());
     }
 
     @Test