Bug-5500: Check Application Peer's bgp-rib-id config attribute against BGP Peer's...

- inject BGPPeerRegistry into BGPApplicationPeerModule as optional dependency
- insert ApplicationPeer into BGPPeerRegistry during BGPApplicationPeerModule to check for address conflict with existing BGP peers
- update BGPApplicationPeerModule unit-test

Change-Id: I8f612cf9eed02cf2a6b5e4832d2c8f9940d415a8
Signed-off-by: Ajay <ajayl.bro@gmail.com>

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/controller/config/yang/bgp/rib/impl/BGPApplicationPeerModule.java b/bgp/rib-impl/src/main/java/org/opendaylight/controller/config/yang/bgp/rib/impl/BGPApplicationPeerModule.java
old mode 100644 (file)
new mode 100755 (executable)
index e6303f6..92b60a9
--- a/bgp/rib-impl/src/main/java/org/opendaylight/controller/config/yang/bgp/rib/impl/BGPApplicationPeerModule.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/controller/config/yang/bgp/rib/impl/BGPApplicationPeerModule.java
@@ -8,6 +8,7 @@
 package org.opendaylight.controller.config.yang.bgp.rib.impl;
 
 import com.google.common.base.Optional;
+import java.util.Collections;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.controller.md.sal.dom.api.DOMDataTreeChangeService;
 import org.opendaylight.controller.md.sal.dom.api.DOMDataTreeIdentifier;
@@ -17,9 +18,17 @@ import org.opendaylight.protocol.bgp.openconfig.spi.BGPOpenconfigMapper;
 import org.opendaylight.protocol.bgp.openconfig.spi.InstanceConfigurationIdentifier;
 import org.opendaylight.protocol.bgp.openconfig.spi.pojo.BGPAppPeerInstanceConfiguration;
 import org.opendaylight.protocol.bgp.rib.impl.ApplicationPeer;
+import org.opendaylight.protocol.bgp.rib.impl.BGPPeer;
 import org.opendaylight.protocol.bgp.rib.impl.RIBImpl;
+import org.opendaylight.protocol.bgp.rib.impl.StrictBGPPeerRegistry;
+import org.opendaylight.protocol.bgp.rib.impl.spi.BGPPeerRegistry;
+import org.opendaylight.protocol.bgp.rib.impl.spi.BGPSessionPreferences;
+import org.opendaylight.protocol.bgp.rib.impl.spi.RIB;
+import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev100924.IpAddress;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.ApplicationRib;
+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.rib.Tables;
+import org.opendaylight.yangtools.concepts.ListenerRegistration;
 import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 
@@ -45,9 +54,48 @@ public class BGPApplicationPeerModule extends org.opendaylight.controller.config
 
     @Override
     public java.lang.AutoCloseable createInstance() {
+        // add to peer-registry to catch any conflicting peer addresses
+        addToPeerRegistry();
+
         final YangInstanceIdentifier id = YangInstanceIdentifier.builder().node(ApplicationRib.QNAME).nodeWithKey(ApplicationRib.QNAME, APP_ID_QNAME, getApplicationRibId().getValue()).node(Tables.QNAME).node(Tables.QNAME).build();
         final DOMDataTreeChangeService service = (DOMDataTreeChangeService) getDataBrokerDependency().getSupportedExtensions().get(DOMDataTreeChangeService.class);
-        return service.registerDataTreeChangeListener(new DOMDataTreeIdentifier(LogicalDatastoreType.CONFIGURATION, id), new ApplicationPeer(getApplicationRibId(), getBgpPeerId(), (RIBImpl) getTargetRibDependency(), new AppPeerModuleTracker(getTargetRibDependency().getOpenConfigProvider())));
+        final ListenerRegistration<ApplicationPeer> listenerRegistration = service.registerDataTreeChangeListener(new DOMDataTreeIdentifier(LogicalDatastoreType.CONFIGURATION, id), new ApplicationPeer(getApplicationRibId(), getBgpPeerId(), (RIBImpl) getTargetRibDependency(), new AppPeerModuleTracker(getTargetRibDependency().getOpenConfigProvider())));
+
+        return new CloseableNoEx() {
+            @Override
+            public void close() {
+                listenerRegistration.close();
+                removeFromPeerRegistry();
+            }
+        };
+    }
+
+    private interface CloseableNoEx extends AutoCloseable {
+        @Override
+        void close();
+    }
+
+    private void addToPeerRegistry() {
+        final RIB r = getTargetRibDependency();
+
+        final IpAddress bgpPeerId = new IpAddress(getBgpPeerId());
+        final BGPPeer bgpClientPeer = new BGPPeer(bgpPeerId.getIpv4Address().getValue(), r, PeerRole.Internal);
+
+        final BGPSessionPreferences prefs = new BGPSessionPreferences(r.getLocalAs(), 0, r.getBgpIdentifier(),
+            r.getLocalAs(), Collections.emptyList());
+
+        final BGPPeerRegistry peerRegistry = getPeerRegistryBackwards();
+        peerRegistry.addPeer(bgpPeerId, bgpClientPeer, prefs);
+    }
+
+    private void removeFromPeerRegistry() {
+        final IpAddress bgpPeerId = new IpAddress(getBgpPeerId());
+        final BGPPeerRegistry peerRegistry = getPeerRegistryBackwards();
+        peerRegistry.removePeer(bgpPeerId);
+    }
+
+    private BGPPeerRegistry getPeerRegistryBackwards() {
+        return getPeerRegistry() == null ? StrictBGPPeerRegistry.GLOBAL : getPeerRegistryDependency();
     }
 
     private final class AppPeerModuleTracker implements BGPConfigModuleTracker {

diff --git a/bgp/rib-impl/src/main/yang/odl-bgp-rib-impl-cfg.yang b/bgp/rib-impl/src/main/yang/odl-bgp-rib-impl-cfg.yang
index 15717caa48c5bec4a9d641df29bcf1c2710ebea5..616527b50c60172c1518cb89659430178c43ae1a 100644 (file)
--- a/bgp/rib-impl/src/main/yang/odl-bgp-rib-impl-cfg.yang
+++ b/bgp/rib-impl/src/main/yang/odl-bgp-rib-impl-cfg.yang
@@ -157,7 +157,7 @@ module odl-bgp-rib-impl-cfg {
     }
 
 
-     augment "/config:modules/config:module/config:configuration" {
+    augment "/config:modules/config:module/config:configuration" {
         case strict-bgp-peer-registry {
             when "/config:modules/config:module/config:type = 'strict-bgp-peer-registry'";
         }
@@ -245,6 +245,17 @@ module odl-bgp-rib-impl-cfg {
                 }
             }
 
+            container peer-registry {
+                description "BGP peer registry where current instance of BGP peer will be registered.";
+                uses config:service-ref {
+                    refine type {
+                        // FIXME backwards compatibility. If not configured, GLOBAL instance is used
+                        mandatory false;
+                        config:required-identity bgp-peer-registry;
+                    }
+                }
+            }
+
             leaf application-rib-id {
                 type rib:application-rib-id;
                 mandatory true;

diff --git a/bgp/rib-impl/src/test/java/org/opendaylight/controller/config/yang/bgp/rib/impl/BGPApplicationPeerModuleTest.java b/bgp/rib-impl/src/test/java/org/opendaylight/controller/config/yang/bgp/rib/impl/BGPApplicationPeerModuleTest.java
old mode 100644 (file)
new mode 100755 (executable)
index 498fce9..989a85f
--- a/bgp/rib-impl/src/test/java/org/opendaylight/controller/config/yang/bgp/rib/impl/BGPApplicationPeerModuleTest.java
+++ b/bgp/rib-impl/src/test/java/org/opendaylight/controller/config/yang/bgp/rib/impl/BGPApplicationPeerModuleTest.java
@@ -9,10 +9,15 @@
 package org.opendaylight.controller.config.yang.bgp.rib.impl;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.util.List;
+
 import javax.management.InstanceNotFoundException;
 import javax.management.ObjectName;
+
 import org.junit.Test;
 import org.opendaylight.controller.config.api.jmx.CommitStatus;
 import org.opendaylight.controller.config.spi.ModuleFactory;
@@ -22,10 +27,14 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.
 public class BGPApplicationPeerModuleTest extends AbstractRIBImplModuleTest {
 
     private static final String INSTANCE_NAME = "application-peer-instance";
+    private static final String INSTANCE_NAME2 = "application-peer-instance-2";
     private static final String FACTORY_NAME = BGPApplicationPeerModuleFactory.NAME;
     private static final ApplicationRibId APP_RIB_ID = new ApplicationRibId("application-peer-test");
     private static final ApplicationRibId NEW_APP_RIB_ID = new ApplicationRibId("new-application-peer-name");
 
+    private ObjectName dataBroker = null;
+    private ObjectName ribModule = null;
+
     @Override
     protected List<ModuleFactory> getModuleFactories() {
         final List<ModuleFactory> moduleFactories = super.getModuleFactories();
@@ -64,15 +73,40 @@ public class BGPApplicationPeerModuleTest extends AbstractRIBImplModuleTest {
         assertEquals(NEW_APP_RIB_ID, getApplicationRibId());
     }
 
+    @Test
+    public void testConflictingPeerAddress() throws Exception {
+        createApplicationPeerInstance();
+        try {
+            createApplicationPeerInstance(INSTANCE_NAME2);
+            fail();
+        } catch (IllegalStateException e) {
+            assertTrue(e.getMessage().contains("getInstance() failed for ModuleIdentifier"));
+            final Throwable ex = e.getCause();
+            assertNotNull(ex);
+            assertTrue(ex.getMessage().contains("Peer for IpAddress"));
+            assertTrue(ex.getMessage().contains("already present"));
+        }
+    }
+
     private CommitStatus createApplicationPeerInstance() throws Exception {
+        return createApplicationPeerInstance(INSTANCE_NAME);
+    }
+
+    private CommitStatus createApplicationPeerInstance(final String instanceName) throws Exception {
         final ConfigTransactionJMXClient transaction = this.configRegistryClient.createTransaction();
-        final ObjectName objName = transaction.createModule(BGPApplicationPeerModuleFactory.NAME, INSTANCE_NAME);
+        final ObjectName objName = transaction.createModule(BGPApplicationPeerModuleFactory.NAME, instanceName);
         final BGPApplicationPeerModuleMXBean mxBean = transaction.newMXBeanProxy(objName, BGPApplicationPeerModuleMXBean.class);
         final ObjectName dataBrokerON = lookupDomAsyncDataBroker(transaction);
         mxBean.setDataBroker(dataBrokerON);
         mxBean.setBgpPeerId(BGP_ID);
         mxBean.setApplicationRibId(APP_RIB_ID);
-        mxBean.setTargetRib(createRIBImplModuleInstance(transaction, createAsyncDataBrokerInstance(transaction)));
+        if (this.dataBroker == null) {
+            this.dataBroker = createAsyncDataBrokerInstance(transaction);
+        }
+        if (this.ribModule == null) {
+            this.ribModule = createRIBImplModuleInstance(transaction, this.dataBroker);
+        }
+        mxBean.setTargetRib(this.ribModule);
         return transaction.commit();
     }