Improve handling for protocols config changes

Given the dependency of Peers of Rib.
When delete
 - remove first peers
 - then remove Rib
when modification
 - update rib
 - then update peers

JIRA: BGPCEP-799

Change-Id: I582398dab191842473f7cbbd1010b76fe9a9d9fd
Signed-off-by: Claudio D. Gasparini <claudio.gasparini@pantheon.tech>

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BGPClusterSingletonService.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BGPClusterSingletonService.java
index 6131bb3ca132f3134dd2eeeee2fe3306467bce81..2bffcbf57a7dc5f38c738b98f9cf4c4986481b44 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BGPClusterSingletonService.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BGPClusterSingletonService.java
@@ -137,6 +137,7 @@ public final class BGPClusterSingletonService implements ClusterSingletonService
                     LOG.debug("RIB instance removed {}", this.ribImpl);
                     closeAllBindedPeers();
                     closeRibService();
+                    this.ribImpl = null;
                 }
                 break;
             case SUBTREE_MODIFIED:

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpDeployerImpl.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpDeployerImpl.java
index c88c31dba84ed7c7be940707686a5527fb4225fb..26f79303cd25f6c1025be30361d541f3059ceafd 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpDeployerImpl.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpDeployerImpl.java
@@ -19,9 +19,11 @@ import com.google.common.util.concurrent.FutureCallback;
 import com.google.common.util.concurrent.MoreExecutors;
 import java.util.Collection;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.ExecutionException;
+import java.util.stream.Collectors;
 import javax.annotation.concurrent.GuardedBy;
 import org.opendaylight.controller.md.sal.binding.api.ClusteredDataTreeChangeListener;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
@@ -125,19 +127,72 @@ public final class BgpDeployerImpl implements ClusteredDataTreeChangeListener<Bg
             LOG.trace("BGP Deployer was already closed, skipping changes.");
             return;
         }
+
         for (final DataTreeModification<Bgp> dataTreeModification : changes) {
             final InstanceIdentifier<Bgp> rootIdentifier = dataTreeModification.getRootPath().getRootIdentifier();
             final DataObjectModification<Bgp> rootNode = dataTreeModification.getRootNode();
             LOG.trace("BGP configuration has changed: {}", rootNode);
-            for (final DataObjectModification<? extends DataObject> dataObjectModification :
-                    rootNode.getModifiedChildren()) {
-                if (dataObjectModification.getDataType().equals(Global.class)) {
-                    onGlobalChanged((DataObjectModification<Global>) dataObjectModification, rootIdentifier);
-                } else if (dataObjectModification.getDataType().equals(Neighbors.class)) {
-                    onNeighborsChanged((DataObjectModification<Neighbors>) dataObjectModification, rootIdentifier);
-                } else if (dataObjectModification.getDataType().equals(PeerGroups.class)) {
-                    rebootNeighbors((DataObjectModification<PeerGroups>) dataObjectModification);
-                }
+
+            final List<DataObjectModification<? extends DataObject>> deletedConfig
+                    = rootNode.getModifiedChildren().stream()
+                    .filter(mod -> mod.getModificationType() == DataObjectModification.ModificationType.DELETE)
+                    .collect(Collectors.toList());
+            final List<DataObjectModification<? extends DataObject>> changedConfig = rootNode.getModifiedChildren().stream()
+                    .filter(mod -> mod.getModificationType() != DataObjectModification.ModificationType.DELETE)
+                    .collect(Collectors.toList());
+            handleDeletions(deletedConfig, rootIdentifier);
+            handleModifications(changedConfig, rootIdentifier);
+        }
+    }
+
+    private void handleModifications(final List<DataObjectModification<? extends DataObject>> changedConfig,
+            final InstanceIdentifier<Bgp> rootIdentifier) {
+        final List<DataObjectModification<? extends DataObject>> globalMod = changedConfig.stream()
+                .filter(mod -> mod.getDataType().equals(Global.class))
+                .collect(Collectors.toList());
+        final List<DataObjectModification<? extends DataObject>> peerMod = changedConfig.stream()
+                .filter(mod -> !mod.getDataType().equals(Global.class))
+                .collect(Collectors.toList());
+        if (!globalMod.isEmpty()) {
+            handleGlobalChange(globalMod, rootIdentifier);
+        }
+        if (!peerMod.isEmpty()) {
+            handlePeersChange(peerMod, rootIdentifier);
+        }
+    }
+
+    private void handleDeletions(final List<DataObjectModification<? extends DataObject>> deletedConfig,
+            final InstanceIdentifier<Bgp> rootIdentifier) {
+        final List<DataObjectModification<? extends DataObject>> globalMod = deletedConfig.stream()
+                .filter(mod -> mod.getDataType().equals(Global.class))
+                .collect(Collectors.toList());
+        final List<DataObjectModification<? extends DataObject>> peerMod = deletedConfig.stream()
+                .filter(mod -> !mod.getDataType().equals(Global.class))
+                .collect(Collectors.toList());
+        if (!peerMod.isEmpty()) {
+            handleGlobalChange(peerMod, rootIdentifier);
+        }
+        if (!globalMod.isEmpty()) {
+            handlePeersChange(globalMod, rootIdentifier);
+        }
+    }
+
+    private void handleGlobalChange(
+            final List<DataObjectModification<? extends DataObject>> config,
+            final InstanceIdentifier<Bgp> rootIdentifier) {
+        for (final DataObjectModification<? extends DataObject> dataObjectModification : config) {
+            onGlobalChanged((DataObjectModification<Global>) dataObjectModification, rootIdentifier);
+        }
+    }
+
+    private void handlePeersChange(
+            final List<DataObjectModification<? extends DataObject>> config,
+            final InstanceIdentifier<Bgp> rootIdentifier) {
+        for (final DataObjectModification<? extends DataObject> dataObjectModification : config) {
+            if (dataObjectModification.getDataType().equals(Neighbors.class)) {
+                onNeighborsChanged((DataObjectModification<Neighbors>) dataObjectModification, rootIdentifier);
+            } else if (dataObjectModification.getDataType().equals(PeerGroups.class)) {
+                rebootNeighbors((DataObjectModification<PeerGroups>) dataObjectModification);
             }
         }
     }

diff --git a/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpDeployerImplTest.java b/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpDeployerImplTest.java
index 948911fc92e2a72d1a0de1bb0d99178fb86cb71d..fbb26ad60f9f29d41ebca782a2d75648dcc51ced 100644 (file)
--- a/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpDeployerImplTest.java
+++ b/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpDeployerImplTest.java
@@ -161,8 +161,8 @@ public class BgpDeployerImplTest extends DefaultRibPoliciesMockTest {
         verify(this.blueprintContainer).getComponentInstance(eq("ribImpl"));
         verify(this.bundleContext, times(2))
                 .registerService(eq(InstanceType.RIB.getServices()), any(), any(Dictionary.class));
-        verify(this.dataTreeRegistration, times(2)).close();
-        verify(this.registration, times(2)).unregister();
+        verify(this.dataTreeRegistration, times(1)).close();
+        verify(this.registration, times(1)).unregister();
 
         deployer.close();
     }
@@ -199,7 +199,7 @@ public class BgpDeployerImplTest extends DefaultRibPoliciesMockTest {
         //Delete existing Peer
         verify(this.bundleContext, times(2))
                 .registerService(eq(InstanceType.PEER.getServices()), any(BgpPeer.class), any(Dictionary.class));
-        verify(this.registration, times(2)).unregister();
+        verify(this.registration, times(3)).unregister();
 
         deployer.close();
     }