BUG-6378: Fix PeerExportGroup

PeerExportGroup contains all peers.
Fix By contain only peers correspending to same
PeerExportGroup Role

Change-Id: If3f795add270d598b74a62ee8e9597441bcea858
Signed-off-by: Claudio D. Gasparini <cgaspari@cisco.com>

diff --git a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/AddPathAbstractRouteEntry.java b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/AddPathAbstractRouteEntry.java
index 0514ae23e6056ed67007a238911bb79e688517d9..912349838b6308a1437dcdd7d3eedf1350d37b0d 100644 (file)
--- a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/AddPathAbstractRouteEntry.java
+++ b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/AddPathAbstractRouteEntry.java
@@ -110,14 +110,15 @@ public abstract class AddPathAbstractRouteEntry extends AbstractRouteEntry {
         if(this.bestPath != null) {
             this.bestPath.stream().filter(path -> filterRoutes(path.getPeerId(), destPeer, peerPT, localTK, discPeers) &&
                 peersSupportsAddPathOrIsFirstBestPath(destPeerSupAddPath, isFirstBestPath(this.bestPath.indexOf(path))))
-                .forEach(path -> writeRoutePath(destPeer, routeId, peerGroup, destPeerSupAddPath, path, rootPath, localTK, ribSup, tx));
+                .forEach(path -> writeRoutePath(destPeer, routeId, peerPT, peerGroup, destPeerSupAddPath, path, rootPath, localTK, ribSup, tx));
         }
     }
 
-    private void writeRoutePath(final PeerId destPeer, final PathArgument routeId, final PeerExportGroup peerGroup, final boolean destPeerSupAddPath,
+    private void writeRoutePath(final PeerId destPeer, final PathArgument routeId, final ExportPolicyPeerTracker peerPT,
+        final PeerExportGroup peerGroup, final boolean destPeerSupAddPath,
         final BestPath path, final YangInstanceIdentifier rootPath, final TablesKey localTK, final RIBSupport ribSup, final DOMDataWriteTransaction tx) {
         final PathArgument routeIdAddPath = ribSup.getRouteIdAddPath(path.getPathId(), routeId);
-        final ContainerNode effectiveAttributes = peerGroup.effectiveAttributes(path.getPeerId(), path.getAttributes());
+        final ContainerNode effectiveAttributes = peerGroup.effectiveAttributes(getRoutePeerIdRole(peerPT,path.getPeerId()), path.getAttributes());
         if (destPeerSupAddPath) {
             writeRoute(destPeer, getAdjRibOutYII(ribSup, rootPath, routeIdAddPath, localTK), effectiveAttributes, createValue(routeIdAddPath, path), ribSup, tx);
         } else {
@@ -164,7 +165,7 @@ public abstract class AddPathAbstractRouteEntry extends AbstractRouteEntry {
         for (final PeerRole role : PeerRole.values()) {
             final PeerExportGroup peerGroup = peerPT.getPeerGroup(role);
             if (peerGroup != null) {
-                final ContainerNode effectiveAttributes = peerGroup.effectiveAttributes(routePeerId, attributes);
+                final ContainerNode effectiveAttributes = peerGroup.effectiveAttributes(getRoutePeerIdRole(peerPT, routePeerId), attributes);
                 for (final Map.Entry<PeerId, PeerExporTuple> pid : peerGroup.getPeers()) {
                     final PeerId destPeer = pid.getKey();
                     final boolean destPeerSupAddPath = peerPT.isAddPathSupportedByPeer(destPeer);

diff --git a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/base/BaseAbstractRouteEntry.java b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/base/BaseAbstractRouteEntry.java
index f8fc1cc867b82d60bbd1175e557206407ea6b5f5..9d984ee7e0edca0917ec94a4002a61d91818d22c 100644 (file)
--- a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/base/BaseAbstractRouteEntry.java
+++ b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/base/BaseAbstractRouteEntry.java
@@ -120,7 +120,7 @@ abstract class BaseAbstractRouteEntry extends AbstractRouteEntry {
         if (this.bestPath != null) {
             final BaseBestPath path = this.bestPath;
             if (filterRoutes(path.getPeerId(), destPeer, peerPT, localTK, discPeers)) {
-                final ContainerNode effAttrib = peerGroup.effectiveAttributes(path.getPeerId(), path.getAttributes());
+                final ContainerNode effAttrib = peerGroup.effectiveAttributes(getRoutePeerIdRole(peerPT,path.getPeerId()), path.getAttributes());
                 writeRoute(destPeer, getAdjRibOutYII(ribSup, rootPath, routeId, localTK), effAttrib, createValue(routeId, path), ribSup, tx);
             }
         }
@@ -179,7 +179,7 @@ abstract class BaseAbstractRouteEntry extends AbstractRouteEntry {
         for (final PeerRole role : PeerRole.values()) {
             final PeerExportGroup peerGroup = peerPT.getPeerGroup(role);
             if (peerGroup != null) {
-                final ContainerNode effAttrib = peerGroup.effectiveAttributes(routePeerId, attributes);
+                final ContainerNode effAttrib = peerGroup.effectiveAttributes(getRoutePeerIdRole(peerPT,routePeerId), attributes);
                 peerGroup.getPeers().stream()
                     .filter(pid -> filterRoutes(routePeerId, pid.getKey(), peerPT, localTK, discPeers))
                     .forEach(pid -> update(pid.getKey(), getAdjRibOutYII(ribSup, pid.getValue().getYii(), routeId, localTK), effAttrib, value, ribSup, tx));

diff --git a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/spi/AbstractRouteEntry.java b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/spi/AbstractRouteEntry.java
index 93328d24dbd18151cf9716ab957a145a82dbd893..1bc617d1108a3c3c8fb54b04f8d8565b27125b51 100644 (file)
--- a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/spi/AbstractRouteEntry.java
+++ b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/spi/AbstractRouteEntry.java
@@ -13,9 +13,11 @@ import org.opendaylight.controller.md.sal.dom.api.DOMDataWriteTransaction;
 import org.opendaylight.protocol.bgp.mode.api.RouteEntry;
 import org.opendaylight.protocol.bgp.rib.spi.CacheDisconnectedPeers;
 import org.opendaylight.protocol.bgp.rib.spi.ExportPolicyPeerTracker;
+import org.opendaylight.protocol.bgp.rib.spi.PeerExportGroup;
 import org.opendaylight.protocol.bgp.rib.spi.RIBSupport;
 import org.opendaylight.protocol.bgp.rib.spi.RibSupportUtils;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.PeerId;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.PeerRole;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.bgp.rib.rib.peer.AdjRibOut;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.rib.Tables;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.rib.TablesKey;
@@ -71,4 +73,14 @@ public abstract class AbstractRouteEntry implements RouteEntry {
         return ribSup.routePath(rootPath.node(AdjRibOut.QNAME).node(Tables.QNAME).node(RibSupportUtils.toYangTablesKey(localTK))
             .node(ROUTES_IDENTIFIER), routeId);
     }
+
+    protected PeerRole getRoutePeerIdRole(final ExportPolicyPeerTracker peerPT, final PeerId routePeerId) {
+        for (final PeerRole role : PeerRole.values()) {
+            final PeerExportGroup peerGroup = peerPT.getPeerGroup(role);
+            if(peerGroup != null && peerGroup.containsPeer(routePeerId)) {
+                return role;
+            }
+        }
+        return null;
+    }
 }

diff --git a/bgp/path-selection-mode/src/test/java/org/opendaylight/protocol/bgp/mode/impl/AbstractRouteEntryTest.java b/bgp/path-selection-mode/src/test/java/org/opendaylight/protocol/bgp/mode/impl/AbstractRouteEntryTest.java
index bad58b64398ed0b8bf63a7fb28cebe8917eacb87..85fe15e6d47f0f2319ddeb39b310e9ad2e0a4c85 100644 (file)
--- a/bgp/path-selection-mode/src/test/java/org/opendaylight/protocol/bgp/mode/impl/AbstractRouteEntryTest.java
+++ b/bgp/path-selection-mode/src/test/java/org/opendaylight/protocol/bgp/mode/impl/AbstractRouteEntryTest.java
@@ -170,11 +170,12 @@ public class AbstractRouteEntryTest {
     }
 
     private void mockExportGroup() {
-        Mockito.doReturn(this.attributes).when(this.peg).effectiveAttributes(Mockito.any(PeerId.class), Mockito.any(ContainerNode.class));
-        Mockito.doReturn(null).when(this.pegNot).effectiveAttributes(Mockito.any(PeerId.class), Mockito.any(ContainerNode.class));
+        Mockito.doReturn(this.attributes).when(this.peg).effectiveAttributes(Mockito.any(PeerRole.class), Mockito.any(ContainerNode.class));
+        Mockito.doReturn(null).when(this.pegNot).effectiveAttributes(Mockito.any(PeerRole.class), Mockito.any(ContainerNode.class));
 
         Map<PeerId, PeerExportGroup.PeerExporTuple> peers = new HashMap<>();
         Mockito.doReturn(ImmutableList.copyOf(peers.entrySet())).when(this.pegNot).getPeers();
+        Mockito.doReturn(true).when(this.pegNot).containsPeer(Mockito.any(PeerId.class));
 
         peers.put(PEER_ID, new PeerExportGroup.PeerExporTuple(PEER_YII, PeerRole.Ibgp));
         peers.put(PEER_ID2, new PeerExportGroup.PeerExporTuple(PEER_YII2, PeerRole.Ibgp));

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/ExportPolicyPeerTrackerImpl.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/ExportPolicyPeerTrackerImpl.java
index 197c9c793fcff3f0ad48f962b4096f2945ef4fc2..f7eec81f32e7c754313382800f2db7178d9bc8de 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/ExportPolicyPeerTrackerImpl.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/ExportPolicyPeerTrackerImpl.java
@@ -64,12 +64,13 @@ final class ExportPolicyPeerTrackerImpl implements ExportPolicyPeerTracker {
             return Collections.emptyMap();
         }
 
-        final Map<PeerId, PeerExporTuple> immutablePeers = ImmutableMap.copyOf(peerPathRoles.entrySet().stream()
-            .collect(toMap(peer -> IdentifierUtils.peerKeyToPeerId(peer.getKey()), peer -> new PeerExporTuple(peer.getKey(), peer.getValue()))));
+        final Map<PeerRole, Map<PeerId, PeerExporTuple>> immutablePeers = peerPathRoles.entrySet().stream()
+            .collect(Collectors.groupingBy(Map.Entry::getValue, toMap(peer -> IdentifierUtils.peerKeyToPeerId(peer
+            .getKey()), peer -> new PeerExporTuple(peer.getKey(), peer.getValue()))));
 
         final Map<PeerRole, PeerExportGroup> ret = peerPathRoles.values().stream().collect(Collectors.toSet()).stream().filter(role -> role != PeerRole.Internal)
-            .collect(toMap(identity(), role -> new PeerExportGroupImpl(immutablePeers, this.policyDatabase.exportPolicyForRole(role)),
-                (oldKey, newKey) -> oldKey, () -> new EnumMap<>(PeerRole.class)));
+            .collect(toMap(identity(), role -> new PeerExportGroupImpl(ImmutableMap.copyOf(immutablePeers.get(role)),
+                this.policyDatabase.exportPolicyForRole(role)), (oldKey, newKey) -> oldKey, () -> new EnumMap<>(PeerRole.class)));
 
         return ret;
     }

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/PeerExportGroupImpl.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/PeerExportGroupImpl.java
index 53bdbdc32203ff0e5150ae4d76434cd190935fe8..69ba399ce140a321f6d0538264f1a0aeb490e512 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/PeerExportGroupImpl.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/PeerExportGroupImpl.java
@@ -12,6 +12,7 @@ import java.util.Collection;
 import java.util.Map;
 import org.opendaylight.protocol.bgp.rib.spi.PeerExportGroup;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.PeerId;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.PeerRole;
 import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode;
 
 final class PeerExportGroupImpl implements PeerExportGroup {
@@ -24,13 +25,17 @@ final class PeerExportGroupImpl implements PeerExportGroup {
     }
 
     @Override
-    public ContainerNode effectiveAttributes(final PeerId sourcePeerId, final ContainerNode attributes) {
-        final PeerExporTuple peer = this.peers.get(sourcePeerId);
-        return attributes == null || peer == null ? null : policy.effectiveAttributes(peer.getRole(), attributes);
+    public ContainerNode effectiveAttributes(final PeerRole role, final ContainerNode attributes) {
+        return attributes == null || role == null ? null : policy.effectiveAttributes(role, attributes);
     }
 
     @Override
     public Collection<Map.Entry<PeerId, PeerExporTuple>> getPeers() {
         return peers.entrySet();
     }
+
+    @Override
+    public boolean containsPeer(final PeerId routePeerId) {
+        return this.peers.containsKey(routePeerId);
+    }
 }
\ No newline at end of file

diff --git a/bgp/rib-spi/src/main/java/org/opendaylight/protocol/bgp/rib/spi/PeerExportGroup.java b/bgp/rib-spi/src/main/java/org/opendaylight/protocol/bgp/rib/spi/PeerExportGroup.java
index b9c82878987bce113287e8fc4f4128d2d43924f1..d8c538e0e799e6ce45df5a3000db855d05b6bbf9 100644 (file)
--- a/bgp/rib-spi/src/main/java/org/opendaylight/protocol/bgp/rib/spi/PeerExportGroup.java
+++ b/bgp/rib-spi/src/main/java/org/opendaylight/protocol/bgp/rib/spi/PeerExportGroup.java
@@ -40,14 +40,16 @@ public interface PeerExportGroup {
     /**
      * Transform outgoing attributes according to policy per Peer
      *
-     * @param sourcePeerId root Peer
+     * @param role root Peer role
      * @param attributes attributes container
      * @return return attributes container after apply policy
      */
-    ContainerNode effectiveAttributes(PeerId sourcePeerId, ContainerNode attributes);
+    ContainerNode effectiveAttributes(PeerRole role, ContainerNode attributes);
 
     /**
      * @return map of peer
      */
     Collection<Map.Entry<PeerId, PeerExporTuple>> getPeers();
+
+    boolean containsPeer(PeerId routePeerId);
 }