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 1b7fdee621ea1a34ac65acd84a5bb42c9c15f11a..48f1c6e9a7f4d0dc5c3bc4295dae2eefb854049d 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
@@ -125,6 +125,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 2b8bf6594d723f7de64b210c952bb2fc3ed8944e..327c3fbe461e67ae4a9b6c757e4337330edcebf4 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
@@ -17,7 +17,9 @@ import com.google.common.util.concurrent.ListenableFuture;
 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.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;
@@ -105,14 +107,62 @@ public final class BgpDeployerImpl implements ClusteredDataTreeChangeListener<Bg
             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);
-                }
-            }
+            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) {
+            onNeighborsChanged((DataObjectModification<Neighbors>) dataObjectModification, rootIdentifier);
         }
     }
 

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 8dd5d97612a30e30a66505a6d4881be1c0c1cedd..894d493ccdfead94df39f4e692caa4c1ff886848 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
@@ -271,17 +271,18 @@ public class BgpDeployerImplTest {
         verify(spyDeployer, times(3)).onDataTreeChanged(any(Collection.class));
 
         //Delete for existing rib
+        doReturn(createNeighborExpected(IPV4UNICAST.class)).when(this.dObject).getDataBefore();
         doReturn(DELETE).when(this.dObject).getModificationType();
 
         spyDeployer.onDataTreeChanged(this.collection);
-        verifyPrivate(spyDeployer, times(4)).invoke("onGlobalChanged",
+        verifyPrivate(spyDeployer, times(3)).invoke("onGlobalChanged",
                 any(DataObjectModification.class), any(InstanceIdentifier.class));
         verify(this.blueprintContainer).getComponentInstance(eq("ribImpl"));
         verify(this.bundleContext, times(2))
                 .registerService(eq(InstanceType.RIB.getServices()), any(), any(Dictionary.class));
         verify(spyDeployer, times(4)).onDataTreeChanged(any(Collection.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();
         verify(spyDeployer, times(4)).onDataTreeChanged(any(Collection.class));
 
         deployer.close();
@@ -366,8 +367,8 @@ public class BgpDeployerImplTest {
 
         spyDeployer.onDataTreeChanged(this.collection);
         verify(spyDeployer, times(5)).onDataTreeChanged(any(Collection.class));
-        verify(this.dObject).getDataBefore();
-        verifyPrivate(spyDeployer, times(4)).invoke("onNeighborsChanged",
+       // verify(this.dObject).getDataBefore();
+        verifyPrivate(spyDeployer, times(3)).invoke("onNeighborsChanged",
                 any(DataObjectModification.class), any(InstanceIdentifier.class));
         verify(this.bundleContext, times(2))
                 .registerService(eq(InstanceType.PEER.getServices()), any(BgpPeer.class), any(Dictionary.class));