BUG-7364: BGP Peer Acceptor Rework

- Decouple BGP Peer acceptor from BGPAcceptorConfig
- Add missing destroy method close to BP

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

diff --git a/bgp/peer-acceptor/src/main/java/org/opendaylight/protocol/bgp/peer/acceptor/BGPPeerAcceptorImpl.java b/bgp/peer-acceptor/src/main/java/org/opendaylight/protocol/bgp/peer/acceptor/BGPPeerAcceptorImpl.java
index 9c930d614ae99037d4feed6277175efdd6b695ab..c6a809a6316278ba58a88958e91039ba7039111f 100644 (file)
--- a/bgp/peer-acceptor/src/main/java/org/opendaylight/protocol/bgp/peer/acceptor/BGPPeerAcceptorImpl.java
+++ b/bgp/peer-acceptor/src/main/java/org/opendaylight/protocol/bgp/peer/acceptor/BGPPeerAcceptorImpl.java
@@ -28,37 +28,72 @@ import org.opendaylight.protocol.concepts.KeyMapping;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.IetfInetUtil;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.IpAddress;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.PortNumber;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.peer.acceptor.config.rev161003.BgpPeerAcceptorConfig;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 public final class BGPPeerAcceptorImpl implements AutoCloseable {
     private static final Logger LOG = LoggerFactory.getLogger(BGPPeerAcceptorImpl.class);
     private static final int PRIVILEGED_PORTS = 1024;
-    private final ChannelFuture futureChannel;
+    private final BGPPeerRegistry peerRegistry;
+    private final BGPDispatcher bgpDispatcher;
+    private final InetSocketAddress address;
+    private ChannelFuture futureChannel;
     private AutoCloseable listenerRegistration;
 
-    public BGPPeerAcceptorImpl(final BgpPeerAcceptorConfig config, final BGPPeerRegistry peerRegistry, final BGPDispatcher bgpDispatcher) {
-        final PortNumber portNumber = config.getBindingPort();
-        final IpAddress bindingAddress = config.getBindingAddress();
-        LOG.debug("Instantiating BGP Peer Acceptor : {}/{}", bindingAddress, portNumber);
-
-        if (!PlatformDependent.isWindows() && !PlatformDependent.isRoot() && portNumber.getValue() < PRIVILEGED_PORTS) {
-            throw new AccessControlException("Unable to bind port " + portNumber.getValue() + " while running as non-root user.");
+    public BGPPeerAcceptorImpl(final IpAddress bindingAddress, final PortNumber portNumber,
+        final BGPPeerRegistry peerRegistry, final BGPDispatcher bgpDispatcher) {
+        this.peerRegistry = Preconditions.checkNotNull(peerRegistry);
+        this.bgpDispatcher = Preconditions.checkNotNull(bgpDispatcher);
+        this.address = getAddress(Preconditions.checkNotNull(bindingAddress), Preconditions.checkNotNull(portNumber));
+        if (!PlatformDependent.isWindows() && !PlatformDependent.isRoot()
+            && portNumber.getValue() < PRIVILEGED_PORTS) {
+            throw new AccessControlException("Unable to bind port " + portNumber.getValue() +
+                " while running as non-root user.");
         }
+    }
 
-        this.futureChannel = bgpDispatcher.createServer(peerRegistry, getAddress(bindingAddress, portNumber));
+    public void start() {
+        LOG.debug("Instantiating BGP Peer Acceptor : {}", this.address);
 
+        this.futureChannel = this.bgpDispatcher.createServer(this.peerRegistry, this.address);
         // Validate future success
         this.futureChannel.addListener(future -> {
-            Preconditions.checkArgument(future.isSuccess(), "Unable to start bgp server on %s", getAddress(bindingAddress, portNumber), future.cause());
+            Preconditions.checkArgument(future.isSuccess(), "Unable to start bgp server on %s",
+                this.address, future.cause());
             final Channel channel = this.futureChannel.channel();
             if (Epoll.isAvailable()) {
-                BGPPeerAcceptorImpl.this.listenerRegistration = peerRegistry.registerPeerRegisterListener(new BGPPeerAcceptorImpl.PeerRegistryListenerImpl(channel.config()));
+                this.listenerRegistration = this.peerRegistry.registerPeerRegisterListener(
+                    new BGPPeerAcceptorImpl.PeerRegistryListenerImpl(channel.config()));
             }
         });
     }
 
+    private InetSocketAddress getAddress(final IpAddress ipAddress, final PortNumber portNumber) {
+        final InetAddress inetAddr;
+        try {
+            inetAddr = InetAddress.getByName(ipAddress.getIpv4Address() != null ?
+                ipAddress.getIpv4Address().getValue() : ipAddress.getIpv6Address().getValue());
+        } catch (final UnknownHostException e) {
+            throw new IllegalArgumentException("Illegal binding address " + ipAddress, e);
+        }
+        return new InetSocketAddress(inetAddr, portNumber.getValue());
+    }
+
+    /**
+     * This closes the acceptor and no new bgp connections will be accepted
+     * Connections already established will be preserved
+     *
+     * @throws Exception
+     */
+    @Override
+    public void close() throws Exception {
+        this.futureChannel.cancel(true);
+        this.futureChannel.channel().close();
+        if (this.listenerRegistration != null) {
+            this.listenerRegistration.close();
+        }
+    }
+
     private static final class PeerRegistryListenerImpl implements PeerRegistryListener {
         private final ChannelConfig channelConfig;
         private final KeyMapping keys;
@@ -83,29 +118,4 @@ public final class BGPPeerAcceptorImpl implements AutoCloseable {
             }
         }
     }
-
-    private InetSocketAddress getAddress(final IpAddress ipAddress, final PortNumber portNumber) {
-        final InetAddress inetAddr;
-        try {
-            inetAddr = InetAddress.getByName(ipAddress.getIpv4Address() != null ? ipAddress.getIpv4Address().getValue() : ipAddress.getIpv6Address().getValue());
-        } catch (final UnknownHostException e) {
-            throw new IllegalArgumentException("Illegal binding address " + ipAddress, e);
-        }
-        return new InetSocketAddress(inetAddr, portNumber.getValue());
-    }
-
-    /**
-     * This closes the acceptor and no new bgp connections will be accepted
-     * Connections already established will be preserved
-     *
-     * @throws Exception
-     */
-    @Override
-    public void close() throws Exception {
-        this.futureChannel.cancel(true);
-        this.futureChannel.channel().close();
-        if (BGPPeerAcceptorImpl.this.listenerRegistration != null) {
-            BGPPeerAcceptorImpl.this.listenerRegistration.close();
-        }
-    }
 }

diff --git a/bgp/peer-acceptor/src/main/resources/org/opendaylight/blueprint/bgp-acceptor.xml b/bgp/peer-acceptor/src/main/resources/org/opendaylight/blueprint/bgp-acceptor.xml
index 17ba242f7aba71245f3168c42eb4c42e11aff410..f68747613d638d266e083f0c9ef027ae085e9155 100644 (file)
--- a/bgp/peer-acceptor/src/main/resources/org/opendaylight/blueprint/bgp-acceptor.xml
+++ b/bgp/peer-acceptor/src/main/resources/org/opendaylight/blueprint/bgp-acceptor.xml
@@ -5,11 +5,11 @@
     <reference id="BGPDispatcher" interface="org.opendaylight.protocol.bgp.rib.impl.spi.BGPDispatcher"/>
 
     <odl:clustered-app-config id="bgpPeerAcceptorConfig"
-                              binding-class="org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.peer.acceptor.config.rev161003.BgpPeerAcceptorConfig"
+                              binding-class="org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.odl.bgp.peer.acceptor.config.rev161003.BgpPeerAcceptorConfig"
                               list-key-value="default">
         <odl:default-config>
             <![CDATA[
-            <bgp-peer-acceptor-config xmlns="urn:opendaylight:params:xml:ns:yang:bgp-peer-acceptor-config">
+            <bgp-peer-acceptor-config xmlns="urn:opendaylight:params:xml:ns:yang:odl-bgp-peer-acceptor-config">
               <config-name>default</config-name>
               <binding-port>1790</binding-port>
             </bgp-peer-acceptor-config>
@@ -17,8 +17,14 @@
         </odl:default-config>
     </odl:clustered-app-config>
 
-    <bean id="bgpPeerAcceptor" class="org.opendaylight.protocol.bgp.peer.acceptor.BGPPeerAcceptorImpl">
-        <argument ref="bgpPeerAcceptorConfig"/>
+    <bean id="bgpPeerAcceptor" class="org.opendaylight.protocol.bgp.peer.acceptor.BGPPeerAcceptorImpl"
+          init-method="start" destroy-method="close">
+        <argument>
+            <bean factory-ref="bgpPeerAcceptorConfig" factory-method="getBindingAddress"/>
+        </argument>
+        <argument>
+            <bean factory-ref="bgpPeerAcceptorConfig" factory-method="getBindingPort"/>
+        </argument>
         <argument ref="BGPPeerRegistry"/>
         <argument ref="BGPDispatcher"/>
     </bean>

diff --git a/bgp/peer-acceptor/src/main/yang/bgp-peer-acceptor-config.yang b/bgp/peer-acceptor/src/main/yang/odl-bgp-peer-acceptor-config.yang
similarity index 84%
rename from bgp/peer-acceptor/src/main/yang/bgp-peer-acceptor-config.yang
rename to bgp/peer-acceptor/src/main/yang/odl-bgp-peer-acceptor-config.yang
index 40165ab10a4c5f17733c486e5381682502a4f54b..ce76d8c6f83345e1c8a1b95523dc46fbed7439a2 100644 (file)
--- a/bgp/peer-acceptor/src/main/yang/bgp-peer-acceptor-config.yang
+++ b/bgp/peer-acceptor/src/main/yang/odl-bgp-peer-acceptor-config.yang
@@ -1,6 +1,6 @@
-module bgp-peer-acceptor-config {
+module odl-bgp-peer-acceptor-config {
     yang-version 1;
-    namespace "urn:opendaylight:params:xml:ns:yang:bgp-peer-acceptor-config";
+    namespace "urn:opendaylight:params:xml:ns:yang:odl-bgp-peer-acceptor-config";
     prefix bgp-peer-acceptor-config;
 
     import ietf-inet-types { prefix inet; revision-date 2013-07-15; }
@@ -8,7 +8,7 @@ module bgp-peer-acceptor-config {
     description
         "This module contains the base YANG definitions for
          BGP Peer Acceptor Configuration.
-         Copyright (c)2013 Cisco Systems, Inc. All rights reserved.;
+         Copyright (c)2016 Cisco Systems, Inc. All rights reserved.;
 
          This program and the accompanying materials are made available
          under the terms of the Eclipse Public License v1.0 which

diff --git a/bgp/peer-acceptor/src/test/java/org/opendaylight/protocol/bgp/peer/acceptor/BGPPeerAcceptorImplTest.java b/bgp/peer-acceptor/src/test/java/org/opendaylight/protocol/bgp/peer/acceptor/BGPPeerAcceptorImplTest.java
index c9a012caa640dabbdd42f498127a88e81079ff65..98f81c40086e68762b93ca57cc914b33b93b5edf 100644 (file)
--- a/bgp/peer-acceptor/src/test/java/org/opendaylight/protocol/bgp/peer/acceptor/BGPPeerAcceptorImplTest.java
+++ b/bgp/peer-acceptor/src/test/java/org/opendaylight/protocol/bgp/peer/acceptor/BGPPeerAcceptorImplTest.java
@@ -12,7 +12,6 @@ import static org.opendaylight.protocol.bgp.rib.impl.CheckUtil.waitFutureSuccess
 import com.google.common.collect.Sets;
 import io.netty.util.concurrent.Future;
 import java.net.InetSocketAddress;
-import java.util.concurrent.ExecutionException;
 import org.junit.Assert;
 import org.junit.Test;
 import org.opendaylight.protocol.bgp.rib.impl.AbstractBGPDispatcherTest;
@@ -22,21 +21,21 @@ import org.opendaylight.protocol.util.InetSocketAddressUtil;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.IpAddress;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.Ipv4Address;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.PortNumber;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.peer.acceptor.config.rev161003.BgpPeerAcceptorConfig;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.peer.acceptor.config.rev161003.BgpPeerAcceptorConfigBuilder;
 
 
 public class BGPPeerAcceptorImplTest extends AbstractBGPDispatcherTest {
     @Test
-    public void testBGPPeerAcceptorImpl() throws InterruptedException, ExecutionException {
+    public void testBGPPeerAcceptorImpl() throws Exception {
         final InetSocketAddress inetServerAddress = InetSocketAddressUtil.getRandomLoopbackInetSocketAddress();
         final IpAddress serverIpAddress = new IpAddress(new Ipv4Address(InetSocketAddressUtil.toHostAndPort(inetServerAddress).getHostText()));
-        final BgpPeerAcceptorConfig config = new BgpPeerAcceptorConfigBuilder().setBindingAddress(serverIpAddress)
-            .setBindingPort(new PortNumber(InetSocketAddressUtil.toHostAndPort(inetServerAddress).getPort())).build();
+        final PortNumber portNumber = new PortNumber(InetSocketAddressUtil.toHostAndPort(inetServerAddress).getPort());
         this.registry.addPeer(serverIpAddress, this.serverListener, createPreferences(inetServerAddress));
 
-        final BGPPeerAcceptorImpl bgpPeerAcceptor = new BGPPeerAcceptorImpl(config, this.registry, this.serverDispatcher);
-        final Future<BGPSessionImpl> futureClient = this.clientDispatcher.createClient(this.clientAddress, inetServerAddress, this.registry, 2, true);
+        final BGPPeerAcceptorImpl bgpPeerAcceptor = new BGPPeerAcceptorImpl(serverIpAddress, portNumber,
+            this.registry, this.serverDispatcher);
+        bgpPeerAcceptor.start();
+        final Future<BGPSessionImpl> futureClient = this.clientDispatcher
+            .createClient(this.clientAddress, inetServerAddress, this.registry, 2, true);
         waitFutureSuccess(futureClient);
         final BGPSessionImpl session = futureClient.get();
         Assert.assertEquals(State.UP, this.clientListener.getState());
@@ -44,5 +43,7 @@ public class BGPPeerAcceptorImplTest extends AbstractBGPDispatcherTest {
         Assert.assertEquals(Sets.newHashSet(IPV_4_TT), session.getAdvertisedTableTypes());
         session.close();
         checkIdleState(this.clientListener);
+
+        bgpPeerAcceptor.close();
     }
 }
\ No newline at end of file