Merge pull request #295 from pgreyson/master

Dynamic update, link directions and various display improvements
diff --git a/build.xml b/build.xml
index f986cea..c7b6449 100644
--- a/build.xml
+++ b/build.xml
@@ -69,6 +69,7 @@
 	<include name="curator-client-1.3.5-SNAPSHOT.jar"/>
 	<include name="curator-framework-1.3.5-SNAPSHOT.jar"/>
 	<include name="curator-recipes-1.3.5-SNAPSHOT.jar"/>
+	<include name="curator-x-discovery-1.3.5-SNAPSHOT.jar"/>
 	<include name="zookeeper-3.4.5.jar"/>
 	<include name="ezmorph-1.0.6.jar"/>	
  	<include name="json-lib-2.4-jdk15.jar"/>
diff --git a/mastership-test.sh b/mastership-test.sh
deleted file mode 100755
index 209ffb7..0000000
--- a/mastership-test.sh
+++ /dev/null
@@ -1,2 +0,0 @@
-#java -Dlogback.configurationFile=logback.xml -cp target/floodlight-only.jar:lib/*:lib/titan/* net.onrc.onos.registry.controller.RegistryRunner $1
-java -Dlogback.configurationFile=logback.xml -cp target/floodlight-only.jar:lib/*:lib/titan/* net.floodlightcontroller.core.Main -cf onos.properties
diff --git a/onos.sh b/onos.sh
deleted file mode 100755
index 5667114..0000000
--- a/onos.sh
+++ /dev/null
@@ -1,59 +0,0 @@
-#!/bin/sh
-
-# Set paths
-FL_HOME=`dirname $0`
-FL_JAR="${FL_HOME}/target/floodlight.jar"
-FL_LOGBACK="${FL_HOME}/logback.xml"
-
-# Set JVM options
-JVM_OPTS=""
-#JVM_OPTS="$JVM_OPTS -server -d64"
-#JVM_OPTS="$JVM_OPTS -Xmx2g -Xms2g -Xmn800m"
-#JVM_OPTS="$JVM_OPTS -XX:+UseParallelGC -XX:+AggressiveOpts -XX:+UseFastAccessorMethods"
-#JVM_OPTS="$JVM_OPTS -XX:MaxInlineSize=8192 -XX:FreqInlineSize=8192"
-#JVM_OPTS="$JVM_OPTS -XX:CompileThreshold=1500 -XX:PreBlockSpin=8"
-#JVM_OPTS="$JVM_OPTS -Dpython.security.respectJavaAccessibility=false"
-
-# Set classpath to include titan libs
-CLASSPATH=`echo ${FL_HOME}/lib/*.jar ${FL_HOME}/lib/titan/*.jar | sed 's/ /:/g'`
-
-
-# Create a logback file if required
-cat <<EOF_LOGBACK >${FL_LOGBACK}
-<configuration scan="true" debug="true">
-  <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
-    <encoder>
-      <pattern>%level [%logger:%thread] %msg%n</pattern>
-    </encoder>
-  </appender>
-
-  <appender name="FILE" class="ch.qos.logback.core.FileAppender">
-    <file>onos.log</file>
-    <encoder>
-      <pattern>%date %level [%thread] %logger{10} [%file:%line] %msg%n</pattern>
-    </encoder>
-  </appender>
-
-  <logger name="org" level="WARN"/>
-  <logger name="LogService" level="WARN"/> <!-- Restlet access logging -->
-  <logger name="net.floodlightcontroller.logging" level="WARN"/>
-
-  <root level="DEBUG">
-    <appender-ref ref="STDOUT" />
-    <appender-ref ref="FILE" />
-  </root>
-</configuration>
-EOF_LOGBACK
-
-# Delete and recreate /tmp/netmap
-#rm -rf /tmp/cassandra.titan
-#mkdir /tmp/cassandra.titan
-
-# Clear logs
-rm onos.log
-
-# Run floodlight
-echo "Starting ONOS controller ..."
-echo 
-#java ${JVM_OPTS} -Dlogback.configurationFile=${FL_LOGBACK} -Xbootclasspath/a:$CLASSPATH -jar ${FL_JAR} -cf ./onos.properties
-java ${JVM_OPTS} -Dlogback.configurationFile=${FL_LOGBACK} -jar ${FL_JAR} -cf ./onos.properties
diff --git a/scripts/run-onos-simple.sh b/scripts/run-onos-simple.sh
new file mode 100755
index 0000000..41b2cc6
--- /dev/null
+++ b/scripts/run-onos-simple.sh
@@ -0,0 +1,13 @@
+#!/bin/sh
+
+basedir=`dirname $0`
+onosdir="${basedir}/.."
+
+logfile="${onosdir}/logback.xml"
+jarfile="${onosdir}/target/floodlight-only.jar"
+classpath="${jarfile}:${onosdir}/lib/*:${onosdir}/lib/titan/*"
+mainclass="net.floodlightcontroller.core.Main"
+propfile="${onosdir}/onos.properties"
+
+#java -Dlogback.configurationFile=logback.xml -cp target/floodlight-only.jar:lib/*:lib/titan/* net.floodlightcontroller.core.Main -cf onos.properties
+java -Dlogback.configurationFile=${logfile} -cp ${classpath} ${mainclass} -cf ${propfile}
diff --git a/src/main/java/net/floodlightcontroller/onoslistener/OnosPublisher.java b/src/main/java/net/floodlightcontroller/onoslistener/OnosPublisher.java
index 36cde95..964eff1 100644
--- a/src/main/java/net/floodlightcontroller/onoslistener/OnosPublisher.java
+++ b/src/main/java/net/floodlightcontroller/onoslistener/OnosPublisher.java
@@ -102,7 +102,7 @@
 				// TODO Auto-generated catch block
 				e.printStackTrace();
 			} catch (RegistryException e) {
-				// TODO Auto-generated catch block
+				log.debug("Caught RegistryException trying to requestControl in cleanup thread");
 				e.printStackTrace();
 			}			
 		}
diff --git a/src/main/java/net/onrc/onos/registry/controller/ControllerService.java b/src/main/java/net/onrc/onos/registry/controller/ControllerService.java
new file mode 100644
index 0000000..c74e85d
--- /dev/null
+++ b/src/main/java/net/onrc/onos/registry/controller/ControllerService.java
@@ -0,0 +1,26 @@
+package net.onrc.onos.registry.controller;
+
+
+
+//@JsonRootName("controller")
+public class ControllerService {
+
+	private String controllerId;
+	
+	public ControllerService(){
+		this("");
+	}
+	
+	public ControllerService(String controllerId) {
+		this.controllerId = controllerId;
+	}
+
+    public void setControllerId(String controllerId) {
+        this.controllerId = controllerId;
+    }
+
+    public String getControllerId() {
+        return controllerId;
+    }
+
+}
diff --git a/src/main/java/net/onrc/onos/registry/controller/RegistryRunner.java b/src/main/java/net/onrc/onos/registry/controller/RegistryRunner.java
deleted file mode 100644
index 164f328..0000000
--- a/src/main/java/net/onrc/onos/registry/controller/RegistryRunner.java
+++ /dev/null
@@ -1,82 +0,0 @@
-package net.onrc.onos.registry.controller;
-
-import net.floodlightcontroller.core.module.FloodlightModuleContext;
-import net.floodlightcontroller.core.module.FloodlightModuleException;
-import net.onrc.onos.registry.controller.IControllerRegistryService.ControlChangeCallback;
-
-import org.openflow.util.HexString;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-/**
- * Used for lightweight testing of the mastership module without having
- * to load up the entire ONOS.
- * @author jono
- *
- */
-public class RegistryRunner {
-	protected static Logger log = LoggerFactory.getLogger(RegistryRunner.class);
-
-	public static void main(String args[]){
-		FloodlightModuleContext fmc = new FloodlightModuleContext();
-		ZookeeperRegistry rm = new ZookeeperRegistry();
-		
-		fmc.addConfigParam(rm, "enableZookeeper", "true");
-		
-		String id = null;
-		if (args.length > 0){
-			id = args[0];
-			log.info("Using unique id: {}", id);
-		}
-		
-		try {
-			rm.init(fmc);
-			rm.startUp(fmc);
-			
-			if (id == null){
-				log.error("No unique ID supplied");
-				System.exit(1);
-			}
-			
-			rm.registerController(id);
-			//rm.setMastershipId(id);
-				
-			rm.requestControl(1L, 
-				new ControlChangeCallback(){
-					@Override
-					public void controlChanged(long dpid, boolean isMaster) {
-						if (isMaster){
-							log.debug("Callback for becoming master for {}", HexString.toHexString(dpid));
-						}
-						else {
-							log.debug("Callback for losing mastership for {}", HexString.toHexString(dpid));
-						}
-					}
-				});
-			
-			Thread.sleep(1000);
-			
-			/*
-			Map<String, List<ControllerRegistryEntry>> switches = rm.getAllSwitches();
-			for (List<ControllerRegistryEntry> ls : switches.values()){
-				for (ControllerRegistryEntry cre : ls){
-					log.debug("ctrlr: {}", cre.getControllerId());
-				}
-			}
-			*/
-			//"Server" loop
-			while (true) {
-				Thread.sleep(60000);
-			}
-			
-		} catch (FloodlightModuleException e) {
-			e.printStackTrace();
-		} catch (InterruptedException e) {
-			e.printStackTrace();
-		} catch (Exception e) {
-			e.printStackTrace();
-		}
-		
-		log.debug("is master: {}", rm.hasControl(1L));
-	}
-}
diff --git a/src/main/java/net/onrc/onos/registry/controller/SwitchRegistryResource.java b/src/main/java/net/onrc/onos/registry/controller/SwitchRegistryResource.java
index 0a7ac5d..599a1af 100644
--- a/src/main/java/net/onrc/onos/registry/controller/SwitchRegistryResource.java
+++ b/src/main/java/net/onrc/onos/registry/controller/SwitchRegistryResource.java
@@ -6,12 +6,8 @@
 
 import org.restlet.resource.Get;
 import org.restlet.resource.ServerResource;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 public class SwitchRegistryResource extends ServerResource {
-
-	protected static Logger log = LoggerFactory.getLogger(SwitchRegistryResource.class);
 	
 	@Get("json")
 	public Map<String, List<ControllerRegistryEntry>> getAllControllers(){
@@ -26,12 +22,6 @@
 			switches = new HashMap<String, List<ControllerRegistryEntry>>();
 		}
 		
-		/*for (List<ControllerRegistryEntry> list: switches.values()){
-			for (ControllerRegistryEntry en : list) {
-				log.debug("Controller id {}", en.getControllerId());
-			}
-		}*/
-		
 		return switches;
 	}
 }
diff --git a/src/main/java/net/onrc/onos/registry/controller/ZookeeperRegistry.java b/src/main/java/net/onrc/onos/registry/controller/ZookeeperRegistry.java
index b666db7..d46b42e 100644
--- a/src/main/java/net/onrc/onos/registry/controller/ZookeeperRegistry.java
+++ b/src/main/java/net/onrc/onos/registry/controller/ZookeeperRegistry.java
@@ -1,10 +1,10 @@
 package net.onrc.onos.registry.controller;
 
 import java.io.IOException;
-import java.io.UnsupportedEncodingException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -16,9 +16,6 @@
 import net.floodlightcontroller.core.module.IFloodlightService;
 import net.floodlightcontroller.restserver.IRestApiService;
 
-import org.apache.zookeeper.CreateMode;
-import org.apache.zookeeper.WatchedEvent;
-import org.apache.zookeeper.Watcher;
 import org.openflow.util.HexString;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -35,9 +32,11 @@
 import com.netflix.curator.framework.recipes.leader.LeaderLatch;
 import com.netflix.curator.framework.recipes.leader.LeaderLatchEvent;
 import com.netflix.curator.framework.recipes.leader.LeaderLatchListener;
-import com.netflix.curator.framework.recipes.leader.Participant;
 import com.netflix.curator.retry.ExponentialBackoffRetry;
-import com.netflix.curator.utils.ZKPaths;
+import com.netflix.curator.x.discovery.ServiceCache;
+import com.netflix.curator.x.discovery.ServiceDiscovery;
+import com.netflix.curator.x.discovery.ServiceDiscoveryBuilder;
+import com.netflix.curator.x.discovery.ServiceInstance;
 
 /**
  * A registry service that uses Zookeeper. All data is stored in Zookeeper,
@@ -57,11 +56,12 @@
 	
 	private final String namespace = "onos";
 	private final String switchLatchesPath = "/switches";
-	private final String controllerPath = "/controllers";
+
+	private final String SERVICES_PATH = "/"; //i.e. the root of our namespace
+	private final String CONTROLLER_SERVICE_NAME = "controllers";
 	
 	protected CuratorFramework client;
 	
-	protected PathChildrenCache controllerCache;
 	protected PathChildrenCache switchCache;
 
 	protected ConcurrentHashMap<String, SwitchLeadershipData> switches;
@@ -96,7 +96,8 @@
 						HexString.toLong(dpid), latch.hasLeadership());
 			}
 			else {
-				log.debug("Latch for {} has changed", dpid);
+				log.debug("Latch for {} has changed: old latch {} - new latch {}", 
+						new Object[]{dpid, latch, swData.getLatch()});
 			}
 		}
 	}
@@ -123,26 +124,35 @@
 			case CHILD_ADDED:
 			case CHILD_UPDATED:
 				//Check we have a PathChildrenCache for this child, add one if not
-				if (switchPathCaches.get(strSwitch) == null){
-					PathChildrenCache pc = new PathChildrenCache(client, 
-							event.getData().getPath(), true);
-					pc.start(StartMode.NORMAL);
-					switchPathCaches.put(strSwitch, pc);
+				synchronized (switchPathCaches){
+					if (switchPathCaches.get(strSwitch) == null){
+						PathChildrenCache pc = new PathChildrenCache(client, 
+								event.getData().getPath(), true);
+						pc.start(StartMode.NORMAL);
+						switchPathCaches.put(strSwitch, pc);
+					}
 				}
 				break;
 			case CHILD_REMOVED:
 				//Remove our PathChildrenCache for this child
-				PathChildrenCache pc = switchPathCaches.remove(strSwitch);
-				pc.close();
+				PathChildrenCache pc = null;
+				synchronized(switchPathCaches){
+					pc = switchPathCaches.remove(strSwitch);
+				}
+				if (pc != null){
+					pc.close();
+				}
 				break;
 			default:
-				//All other events are connection status events. We need to do anything
-				//as the path cache handles these on its own.
+				//All other events are connection status events. We don't need to 
+				//do anything as the path cache handles these on its own.
 				break;
 			}
 			
 		}
 	};
+	protected ServiceDiscovery<ControllerService> serviceDiscovery;
+	protected ServiceCache<ControllerService> serviceCache;
 
 	
 	@Override
@@ -206,6 +216,8 @@
 
 		LeaderLatch latch = swData.getLatch();
 		
+		latch.removeAllListeners();
+		
 		try {
 			latch.close();
 		} catch (IOException e) {
@@ -238,17 +250,13 @@
 		log.debug("Getting all controllers");
 		
 		List<String> controllers = new ArrayList<String>();
-		for (ChildData data : controllerCache.getCurrentData()){
-
-			String d = null;
-			try {
-				d = new String(data.getData(), "UTF-8");
-			} catch (UnsupportedEncodingException e) {
-				throw new RegistryException("Error encoding string", e);
+		for (ServiceInstance<ControllerService> instance : serviceCache.getInstances()){
+			String id = instance.getPayload().getControllerId();
+			if (!controllers.contains(id)){
+				controllers.add(id);
 			}
-
-			controllers.add(d);
 		}
+
 		return controllers;
 	}
 
@@ -261,78 +269,50 @@
 		
 		controllerId = id;
 		
-		byte bytes[] = id.getBytes(Charsets.UTF_8);
-		String path = ZKPaths.makePath(controllerPath, controllerId);
-		
-		log.info("Registering controller with id {}", id);
-		
 		try {
-			//We need to set a watch to recreate the node in the controller
-			//registry if it gets deleted - e.g. on Zookeeper connection loss.
-			Watcher watcher = new Watcher(){
-				@Override
-				public void process(WatchedEvent event) {
-					log.debug("got any watch event {} ", event);
-					
-					String path = ZKPaths.makePath(controllerPath, controllerId);
-					byte bytes[] = controllerId.getBytes(Charsets.UTF_8);
-					
-					try {
-						if (event.getType() == Event.EventType.NodeDeleted){
-							log.debug("got a node deleted event");
-							
-							
-							client.create().withMode(CreateMode.EPHEMERAL)
-								.forPath(path, bytes);
-						}
-					} catch (Exception e) {
-						log.warn("Error recreating controller node for {}: {}",
-								controllerId, e.getMessage());
-					} finally {
-						try {
-							client.checkExists().usingWatcher(this).forPath(path);
-						} catch (Exception e2){
-							log.warn("Error resetting watch for {}: {}", 
-									controllerId, e2.getMessage());
-						}
-					}
-				}
-			};
+			ServiceInstance<ControllerService> thisInstance = ServiceInstance.<ControllerService>builder()
+			        .name(CONTROLLER_SERVICE_NAME)
+			        .payload(new ControllerService(controllerId))
+			        //.port((int)(65535 * Math.random())) // in a real application, you'd use a common port
+			        //.uriSpec(uriSpec)
+			        .build();
 			
-			//Create ephemeral node in controller registry
-			//TODO Use protection
-			client.create().withMode(CreateMode.EPHEMERAL)
-					.forPath(path, bytes);
-			client.checkExists().usingWatcher(watcher).forPath(path);
+			serviceDiscovery.registerService(thisInstance);
 		} catch (Exception e) {
-			throw new RegistryException("Error contacting the Zookeeper service", e);
+			// TODO Auto-generated catch block
+			e.printStackTrace();
 		}
+		
 	}
 	
 	@Override
 	public String getControllerForSwitch(long dpid) throws RegistryException {
-		// TODO work out synchronization
-		
 		String dpidStr = HexString.toHexString(dpid);
-
-		SwitchLeadershipData swData = switches.get(dpidStr);
-		//LeaderLatch latch = (switches.get(dpidStr) != null)?switches.get(dpidStr).getLatch():null;
 		
-		if (swData == null){
+		PathChildrenCache switchCache = switchPathCaches.get(dpidStr);
+		
+		if (switchCache == null){
 			log.warn("Tried to get controller for non-existent switch");
 			return null;
 		}
 		
-		LeaderLatch latch = swData.getLatch();
+		List<ChildData> sortedData = new ArrayList<ChildData>(switchCache.getCurrentData()); 
 		
-		Participant leader = null;
-		try {
-			leader = latch.getLeader();
-		} catch (Exception e) {
-			throw new RegistryException("Error contacting the Zookeeper service", e);
-		}
+		Collections.sort(
+				sortedData,
+				new Comparator<ChildData>(){
+					private String getSequenceNumber(String path){
+						return path.substring(path.lastIndexOf('-') + 1);
+					}
+					@Override
+					public int compare(ChildData lhs, ChildData rhs) {
+						return getSequenceNumber(lhs.getPath()).
+								compareTo(getSequenceNumber(rhs.getPath()));
+					}
+				}
+			);
 		
-		return leader.getId();
+		return new String(sortedData.get(0).getData(), Charsets.UTF_8);
 	}
 	
 	@Override
@@ -421,7 +401,8 @@
 		restApi = context.getServiceImpl(IRestApiService.class);
 
 		switches = new ConcurrentHashMap<String, SwitchLeadershipData>();
-		switchPathCaches = new HashMap<String, PathChildrenCache>();
+		//switchPathCaches = new HashMap<String, PathChildrenCache>();
+		switchPathCaches = new ConcurrentHashMap<String, PathChildrenCache>();
 		
 		RetryPolicy retryPolicy = new ExponentialBackoffRetry(1000, 3);
 		client = CuratorFrameworkFactory.newClient(this.connectionString, 
@@ -431,12 +412,22 @@
 		client = client.usingNamespace(namespace);
 
 		
-		controllerCache = new PathChildrenCache(client, controllerPath, true);
 		switchCache = new PathChildrenCache(client, switchLatchesPath, true);
 		switchCache.getListenable().addListener(switchPathCacheListener);
 		
+		//Build the service discovery object
+	    serviceDiscovery = ServiceDiscoveryBuilder.builder(ControllerService.class)
+	            .client(client).basePath(SERVICES_PATH).build();
+	    
+	    //We read the list of services very frequently (GUI periodically queries them)
+	    //so we'll cache them to cut down on Zookeeper queries.
+	    serviceCache = serviceDiscovery.serviceCacheBuilder()
+				.name(CONTROLLER_SERVICE_NAME).build();
+	    
+	    
 		try {
-			controllerCache.start(StartMode.BUILD_INITIAL_CACHE);
+			serviceDiscovery.start();
+			serviceCache.start();
 			
 			//Don't prime the cache, we want a notification for each child node in the path
 			switchCache.start(StartMode.NORMAL);
diff --git a/start.sh b/start.sh
deleted file mode 100755
index d6aa010..0000000
--- a/start.sh
+++ /dev/null
@@ -1,9 +0,0 @@
-#! /bin/sh
-if [ -f floodlight.onos1vpc.log ]; then
- mv floodlight.onos1vpc.log floodlight.onos1vpc.log.1
-fi
-ppid=`ps -edalf |grep java |grep floodlight.jar | awk '{print $4}'`
-if [ x$ppid != "x" ]; then
-  sudo kill -KILL $ppid
-fi
-java -jar target/floodlight.jar > floodlight.onos1vpc.log 2>&1 &