Move BGP listen port configuration to BgpSessionManager component

Change-Id: I445bff180740fededacaa59441fc53332cc3d482

@@ -24,9 +24,8 @@ public interface BgpService {
      * Starts the BGP service.
      *
      * @param routeListener listener to send route updates to
-     * @param bgpPort port number to listen on
      */
-    void start(RouteListener routeListener, int bgpPort);
+    void start(RouteListener routeListener);
 
     /**
      * Stops the BGP service.

@@ -55,6 +55,11 @@
         </dependency>
 
         <dependency>
+            <groupId>org.osgi</groupId>
+            <artifactId>org.osgi.compendium</artifactId>
+        </dependency>
+
+        <dependency>
             <groupId>org.apache.karaf.shell</groupId>
             <artifactId>org.apache.karaf.shell.console</artifactId>
         </dependency>

@@ -124,7 +124,7 @@ public class Router implements RoutingService {
         this.fibComponent = checkNotNull(listener);
         this.hostService.addListener(hostListener);
 
-        bgpService.start(new InternalRouteListener(), 2000);
+        bgpService.start(new InternalRouteListener());
 
         bgpUpdatesExecutor.execute(new Runnable() {
             @Override

@@ -15,7 +15,11 @@
  */
 package org.onosproject.routing.bgp;
 
+import org.osgi.service.component.ComponentContext;
+import org.apache.felix.scr.annotations.Activate;
 import org.apache.felix.scr.annotations.Component;
+import org.apache.felix.scr.annotations.Deactivate;
+import org.apache.felix.scr.annotations.Modified;
 import org.apache.felix.scr.annotations.Service;
 import org.jboss.netty.bootstrap.ServerBootstrap;
 import org.jboss.netty.channel.Channel;
@@ -40,6 +44,7 @@ import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 import java.util.Collection;
+import java.util.Dictionary;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 
@@ -72,6 +77,47 @@ public class BgpSessionManager implements BgpInfoService, BgpService {
 
     private RouteListener routeListener;
 
+    private static final int DEFAULT_BGP_PORT = 2000;
+    private int bgpPort;
+
+    @Activate
+    protected void activate(ComponentContext context) {
+        readComponentConfiguration(context);
+        log.info("BgpSessionManager started");
+    }
+
+    @Deactivate
+    protected void deactivate() {
+        log.info("BgpSessionManager stopped");
+    }
+
+    /**
+     * Extracts properties from the component configuration context.
+     *
+     * @param context the component context
+     */
+    private void readComponentConfiguration(ComponentContext context) {
+        Dictionary<?, ?> properties = context.getProperties();
+        try {
+            String strPort = (String) properties.get("bgpPort");
+            if (strPort != null) {
+                bgpPort = Integer.parseInt(strPort);
+            } else {
+                bgpPort = DEFAULT_BGP_PORT;
+            }
+        } catch (Exception e) {
+            bgpPort = DEFAULT_BGP_PORT;
+        }
+        log.debug("BGP port is set to {}", bgpPort);
+    }
+
+    @Modified
+    public void modified(ComponentContext context) {
+        // Blank @Modified method to catch modifications to the context.
+        // If no @Modified method exists, it seems @Activate is called again
+        // when the context is modified.
+    }
+
     /**
      * Checks whether the BGP Session Manager is shutdown.
      *
@@ -245,7 +291,7 @@ public class BgpSessionManager implements BgpInfoService, BgpService {
     }
 
     @Override
-    public void start(RouteListener routeListener, int listenPortNumber) {
+    public void start(RouteListener routeListener) {
         log.debug("BGP Session Manager start.");
         isShutdown = false;
 
@@ -271,7 +317,7 @@ public class BgpSessionManager implements BgpInfoService, BgpService {
             }
         };
         InetSocketAddress listenAddress =
-                new InetSocketAddress(listenPortNumber);
+                new InetSocketAddress(bgpPort);
 
         serverBootstrap = new ServerBootstrap(channelFactory);
         // serverBootstrap.setOptions("reuseAddr", true);

@@ -78,7 +78,7 @@ public class RouterAsyncArpTest {
         hostService = createMock(HostService.class);
 
         BgpService bgpService = createMock(BgpService.class);
-        bgpService.start(anyObject(RouteListener.class), anyInt());
+        bgpService.start(anyObject(RouteListener.class));
         bgpService.stop();
         replay(bgpService);
 

@@ -84,7 +84,7 @@ public class RouterTest {
         setUpHostService();
 
         BgpService bgpService = createMock(BgpService.class);
-        bgpService.start(anyObject(RouteListener.class), anyInt());
+        bgpService.start(anyObject(RouteListener.class));
         bgpService.stop();
         replay(bgpService);
 

@@ -35,16 +35,21 @@ import org.onlab.packet.Ip4Address;
 import org.onlab.packet.Ip4Prefix;
 import org.onosproject.routingapi.RouteListener;
 import org.onosproject.routingapi.RouteUpdate;
+import org.osgi.service.component.ComponentContext;
 
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Dictionary;
 import java.util.LinkedList;
 import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
 
+import static org.easymock.EasyMock.createMock;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.replay;
 import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.notNullValue;
@@ -252,7 +257,14 @@ public class BgpSessionManagerTest {
         //
         bgpSessionManager = new BgpSessionManager();
         // NOTE: We use port 0 to bind on any available port
-        bgpSessionManager.start(dummyRouteListener, 0);
+        ComponentContext componentContext = createMock(ComponentContext.class);
+        Dictionary<String, String> dictionary = createMock(Dictionary.class);
+        expect(dictionary.get("bgpPort")).andReturn("0");
+        replay(dictionary);
+        expect(componentContext.getProperties()).andReturn(dictionary);
+        replay(componentContext);
+        bgpSessionManager.activate(componentContext);
+        bgpSessionManager.start(dummyRouteListener);
 
         // Get the port number the BGP Session Manager is listening on
         Channel serverChannel = TestUtils.getField(bgpSessionManager,

@@ -55,11 +55,6 @@
     </dependency>
 
     <dependency>
-      <groupId>org.osgi</groupId>
-      <artifactId>org.osgi.compendium</artifactId>
-    </dependency>
-
-    <dependency>
       <groupId>org.onosproject</groupId>
       <artifactId>onlab-misc</artifactId>
     </dependency>

@@ -18,7 +18,6 @@ package org.onosproject.sdnip;
 import org.apache.felix.scr.annotations.Activate;
 import org.apache.felix.scr.annotations.Component;
 import org.apache.felix.scr.annotations.Deactivate;
-import org.apache.felix.scr.annotations.Modified;
 import org.apache.felix.scr.annotations.Reference;
 import org.apache.felix.scr.annotations.ReferenceCardinality;
 import org.apache.felix.scr.annotations.Service;
@@ -34,11 +33,8 @@ import org.onosproject.net.host.HostService;
 import org.onosproject.net.intent.IntentService;
 import org.onosproject.routingapi.RoutingService;
 import org.onosproject.sdnip.config.SdnIpConfigurationReader;
-import org.osgi.service.component.ComponentContext;
 import org.slf4j.Logger;
 
-import java.util.Dictionary;
-
 import static org.slf4j.LoggerFactory.getLogger;
 
 /**
@@ -77,9 +73,6 @@ public class SdnIp implements SdnIpService {
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
     protected NetworkConfigService networkConfigService;
 
-    private static final int DEFAULT_BGP_PORT = 2000;
-    private int bgpPort;
-
     private IntentSynchronizer intentSynchronizer;
     private SdnIpConfigurationReader config;
     private PeerConnectivityManager peerConnectivity;
@@ -90,9 +83,8 @@ public class SdnIp implements SdnIpService {
     private ControllerNode localControllerNode;
 
     @Activate
-    protected void activate(ComponentContext context) {
+    protected void activate() {
         log.info("SDN-IP started");
-        readComponentConfiguration(context);
 
         appId = coreService.registerApplication(SDN_IP_APP);
         config = new SdnIpConfigurationReader();
@@ -117,9 +109,6 @@ public class SdnIp implements SdnIpService {
 
         leadershipService.addListener(leadershipEventListener);
         leadershipService.runForLeadership(appId.name());
-
-        log.info("Starting BGP with port {}", bgpPort);
-        // TODO feed port information through to the BgpService
     }
 
     @Deactivate
@@ -134,33 +123,6 @@ public class SdnIp implements SdnIpService {
         log.info("SDN-IP Stopped");
     }
 
-    /**
-     * Extracts properties from the component configuration context.
-     *
-     * @param context the component context
-     */
-    private void readComponentConfiguration(ComponentContext context) {
-        Dictionary<?, ?> properties = context.getProperties();
-        try {
-            String strPort = (String) properties.get("bgpPort");
-            if (strPort != null) {
-                bgpPort = Integer.parseInt(strPort);
-            } else {
-                bgpPort = DEFAULT_BGP_PORT;
-            }
-        } catch (Exception e) {
-            bgpPort = DEFAULT_BGP_PORT;
-        }
-        log.debug("BGP port is set to {}", bgpPort);
-    }
-
-    @Modified
-    public void modified(ComponentContext context) {
-        // Blank @Modified method to catch modifications to the context.
-        // If no @Modified method exists, it seems @Activate is called again
-        // when the context is modified.
-    }
-
     @Override
     public void modifyPrimary(boolean isPrimary) {
         intentSynchronizer.leaderChanged(isPrimary);