Bug 6745 Do not skip first data for reconciliation

 - we can accept first modification since all statistics
   are intentionally collected in one step on startup
 - added test
 - updated comments

Change-Id: I94590fb268accb0237260a09d184c2cecd1fb324
Signed-off-by: Andrej Leitner <andrej.leitner@pantheon.tech>

diff --git a/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/SimplifiedOperationalListener.java b/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/SimplifiedOperationalListener.java
index 3e9014de104ff0595913164eb837f2476bfa0ed7..2e8b40cf4dd36e7bf42ad62ce3f21034567813ee 100644 (file)
--- a/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/SimplifiedOperationalListener.java
+++ b/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/SimplifiedOperationalListener.java
@@ -79,11 +79,15 @@ public class SimplifiedOperationalListener extends AbstractFrmSyncListener<Node>
         final NodeId nodeId = ModificationUtil.nodeId(modification);
         updateCache(modification);
 
-        if (isAdd(modification) || isAddLogical(modification)) {
+        final boolean isAdd = isAdd(modification) || isAddLogical(modification);
+
+        if (isAdd) {
             deviceMastershipManager.onDeviceConnected(nodeId);
         }
 
-        if (reconciliationRegistry.isRegistered(nodeId) && isConsistentForReconcile(modification)) {
+        // if node is registered for reconcile we need consistent data from operational DS (skip partial collections)
+        // but we can accept first modification since all statistics are intentionally collected in one step on startup
+        if (reconciliationRegistry.isRegistered(nodeId) && (isAdd || isConsistentForReconcile(modification))) {
             return reconciliation(modification);
         } else {
             return skipModification(modification);
@@ -115,9 +119,6 @@ public class SimplifiedOperationalListener extends AbstractFrmSyncListener<Node>
         return Optional.absent();
     }
 
-    /**
-     * ModificationType.DELETE.
-     */
     private boolean isDelete(final DataTreeModification<Node> modification) {
         return ModificationType.DELETE == modification.getRootNode().getModificationType();
     }
@@ -170,6 +171,12 @@ public class SimplifiedOperationalListener extends AbstractFrmSyncListener<Node>
         }
     }
 
+    /**
+     * Check if modification is consistent for reconciliation. We need fresh data, which means that current statistics
+     * were collected after registration for reconcile and whole bunch of statistics was collected successfully.
+     * @param modification from DS
+     * @return status of modification
+     */
     private boolean isConsistentForReconcile(final DataTreeModification<Node> modification) {
         final NodeId nodeId = PathUtil.digNodeId(modification.getRootPath().getRootIdentifier());
         final FlowCapableStatisticsGatheringStatus gatheringStatus = modification.getRootNode().getDataAfter()

diff --git a/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/util/ReconciliationRegistry.java b/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/util/ReconciliationRegistry.java
index e9fac578706b12db3f1ace7c9051cc27c80ac253..594ec7aea20f5a645db345de9b941aac79795b51 100644 (file)
--- a/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/util/ReconciliationRegistry.java
+++ b/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/util/ReconciliationRegistry.java
@@ -25,7 +25,7 @@ public class ReconciliationRegistry {
     public Date register(NodeId nodeId) {
         Date timestamp = new Date();
         registration.put(nodeId, timestamp);
-        LOG.debug("Registered for next consistent operational: {}", nodeId.getValue());
+        LOG.debug("Registered for reconciliation: {}", nodeId.getValue());
         // TODO  elicit statistics gathering if not running actually
         return timestamp;
     }
@@ -33,7 +33,7 @@ public class ReconciliationRegistry {
     public Date unregisterIfRegistered(NodeId nodeId) {
         Date timestamp = registration.remove(nodeId);
         if (timestamp != null) {
-            LOG.debug("Unregistered for next consistent operational: {}", nodeId.getValue());
+            LOG.debug("Unregistered for reconciliation: {}", nodeId.getValue());
         }
         return timestamp;
     }

diff --git a/applications/forwardingrules-sync/src/test/java/org/opendaylight/openflowplugin/applications/frsync/impl/SimplifiedOperationalListenerTest.java b/applications/forwardingrules-sync/src/test/java/org/opendaylight/openflowplugin/applications/frsync/impl/SimplifiedOperationalListenerTest.java
index 15790d97869418018058f1599deba0391a14ca70..10441849daf78a358bce9d39e62a7aca8583036b 100644 (file)
--- a/applications/forwardingrules-sync/src/test/java/org/opendaylight/openflowplugin/applications/frsync/impl/SimplifiedOperationalListenerTest.java
+++ b/applications/forwardingrules-sync/src/test/java/org/opendaylight/openflowplugin/applications/frsync/impl/SimplifiedOperationalListenerTest.java
@@ -215,6 +215,20 @@ public class SimplifiedOperationalListenerTest {
         Mockito.verify(roTx).close();
     }
 
+
+    @Test
+    public void testOnDataTreeChangedReconcileAndFreshOperationalNotPresentButAdd() throws ParseException {
+        Mockito.when(reconciliationRegistry.isRegistered(NODE_ID)).thenReturn(true);
+        operationalAdd();
+        prepareFreshOperational(false);
+        final SyncupEntry syncupEntry = loadConfigDSAndPrepareSyncupEntry(configNode, configDS, fcOperationalNode, operationalDS);
+
+        nodeListenerOperational.onDataTreeChanged(Collections.singleton(dataTreeModification));
+
+        Mockito.verify(reactor).syncup(fcNodePath, syncupEntry);
+        Mockito.verify(roTx).close();
+    }
+
     @Test
     public void testOnDataTreeChangedReconcileAndConfigNotPresent() throws Exception {
         // Related to bug 5920 -> https://bugs.opendaylight.org/show_bug.cgi?id=5920