Merge remote-tracking branch 'origin/master'

@@ -55,6 +55,5 @@ public interface MastershipStore extends Store<MastershipEvent, MastershipStoreD
      * @param role     new role
      * @return a mastership event
      */
-    MastershipEvent setRole(NodeId nodeId, DeviceId deviceId,
-                            MastershipRole role);
+    MastershipEvent setMaster(NodeId nodeId, DeviceId deviceId);
 }

@@ -64,9 +64,12 @@ public class MastershipManager
         checkNotNull(nodeId, NODE_ID_NULL);
         checkNotNull(deviceId, DEVICE_ID_NULL);
         checkNotNull(role, ROLE_NULL);
-        MastershipEvent event = store.setRole(nodeId, deviceId, role);
-        if (event != null) {
-            post(event);
+        //TODO figure out appropriate action for non-MASTER roles, if we even set those
+        if (role.equals(MastershipRole.MASTER)) {
+            MastershipEvent event = store.setMaster(nodeId, deviceId);
+            if (event != null) {
+                post(event);
+            }
         }
     }
 

+package org.onlab.onos.cluster.impl;
+
+import java.util.Set;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.onlab.onos.cluster.ClusterEventListener;
+import org.onlab.onos.cluster.ClusterService;
+import org.onlab.onos.cluster.ControllerNode;
+import org.onlab.onos.cluster.ControllerNode.State;
+import org.onlab.onos.cluster.DefaultControllerNode;
+import org.onlab.onos.cluster.MastershipService;
+import org.onlab.onos.cluster.NodeId;
+import org.onlab.onos.event.impl.TestEventDispatcher;
+import org.onlab.onos.net.DeviceId;
+import org.onlab.onos.net.trivial.impl.SimpleMastershipStore;
+import org.onlab.packet.IpPrefix;
+
+import static org.junit.Assert.assertEquals;
+import static org.onlab.onos.net.MastershipRole.*;
+
+/**
+ * Test codifying the mastership service contracts.
+ */
+public class MastershipManagerTest {
+
+    private static final NodeId NID_LOCAL = new NodeId("local");
+    private static final NodeId NID_OTHER = new NodeId("foo");
+    private static final IpPrefix LOCALHOST = IpPrefix.valueOf("127.0.0.1");
+    private static final DeviceId DEV_MASTER = DeviceId.deviceId("of:1");
+    private static final DeviceId DEV_OTHER = DeviceId.deviceId("of:2");
+
+    private MastershipManager mgr;
+    protected MastershipService service;
+
+    @Before
+    public void setUp() {
+        mgr = new MastershipManager();
+        service = mgr;
+        mgr.store = new SimpleMastershipStore();
+        mgr.eventDispatcher = new TestEventDispatcher();
+        mgr.clusterService = new TestClusterService();
+        mgr.activate();
+    }
+
+    @After
+    public void tearDown() {
+        mgr.deactivate();
+        mgr.clusterService = null;
+        mgr.eventDispatcher = null;
+        mgr.store = null;
+    }
+
+    @Test
+    public void setRole() {
+        mgr.setRole(NID_OTHER, DEV_MASTER, MASTER);
+        assertEquals("wrong local role:", STANDBY, mgr.getLocalRole(DEV_MASTER));
+
+        //set to master
+        mgr.setRole(NID_LOCAL, DEV_MASTER, MASTER);
+        assertEquals("wrong local role:", MASTER, mgr.getLocalRole(DEV_MASTER));
+    }
+
+    @Test
+    public void relinquishMastership() {
+        //TODO
+    }
+
+    @Test
+    public void requestRoleFor() {
+        mgr.setRole(NID_LOCAL, DEV_MASTER, MASTER);
+        mgr.setRole(NID_OTHER, DEV_OTHER, MASTER);
+
+        //local should be master for one but standby for other
+        assertEquals("wrong role:", MASTER, mgr.requestRoleFor(DEV_MASTER));
+        assertEquals("wrong role:", STANDBY, mgr.requestRoleFor(DEV_OTHER));
+    }
+
+    @Test
+    public void getMasterFor() {
+        mgr.setRole(NID_LOCAL, DEV_MASTER, MASTER);
+        mgr.setRole(NID_OTHER, DEV_OTHER, MASTER);
+        assertEquals("wrong master:", NID_LOCAL, mgr.getMasterFor(DEV_MASTER));
+        assertEquals("wrong master:", NID_OTHER, mgr.getMasterFor(DEV_OTHER));
+
+        //have NID_OTHER hand over DEV_OTHER to NID_LOCAL
+        mgr.setRole(NID_LOCAL, DEV_OTHER, MASTER);
+        assertEquals("wrong master:", NID_LOCAL, mgr.getMasterFor(DEV_OTHER));
+    }
+
+    @Test
+    public void getDevicesOf() {
+        mgr.setRole(NID_LOCAL, DEV_MASTER, MASTER);
+        mgr.setRole(NID_LOCAL, DEV_OTHER, STANDBY);
+        assertEquals("should be one device:", 1, mgr.getDevicesOf(NID_LOCAL).size());
+
+        //hand both devices to NID_LOCAL
+        mgr.setRole(NID_LOCAL, DEV_OTHER, MASTER);
+        assertEquals("should be two devices:", 2, mgr.getDevicesOf(NID_LOCAL).size());
+    }
+
+    private final class TestClusterService implements ClusterService {
+
+        ControllerNode local = new DefaultControllerNode(NID_LOCAL, LOCALHOST);
+
+        @Override
+        public ControllerNode getLocalNode() {
+            return local;
+        }
+
+        @Override
+        public Set<ControllerNode> getNodes() {
+            return null;
+        }
+
+        @Override
+        public ControllerNode getNode(NodeId nodeId) {
+            return null;
+        }
+
+        @Override
+        public State getState(NodeId nodeId) {
+            return null;
+        }
+
+        @Override
+        public void addListener(ClusterEventListener listener) {
+        }
+
+        @Override
+        public void removeListener(ClusterEventListener listener) {
+        }
+
+    }
+}

@@ -42,6 +42,7 @@ public class DistributedMastershipStore
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
     protected ClusterService clusterService;
 
+    @Override
     @Activate
     public void activate() {
         super.activate();
@@ -61,10 +62,10 @@ public class DistributedMastershipStore
     }
 
     @Override
-    public MastershipEvent setRole(NodeId nodeId, DeviceId deviceId, MastershipRole role) {
+    public MastershipEvent setMaster(NodeId nodeId, DeviceId deviceId) {
         synchronized (this) {
             NodeId currentMaster = getMaster(deviceId);
-            if (role == MastershipRole.MASTER && Objects.equals(currentMaster, nodeId)) {
+            if (Objects.equals(currentMaster, nodeId)) {
                 return null;
             }
 
@@ -94,7 +95,7 @@ public class DistributedMastershipStore
     @Override
     public MastershipRole requestRole(DeviceId deviceId) {
         // FIXME: for now we are 'selecting' as master whoever asks
-        setRole(clusterService.getLocalNode().id(), deviceId, MastershipRole.MASTER);
+        setMaster(clusterService.getLocalNode().id(), deviceId);
         return MastershipRole.MASTER;
     }
 

@@ -3,6 +3,8 @@ package org.onlab.onos.net.trivial.impl;
 import static org.slf4j.LoggerFactory.getLogger;
 
 import java.util.Collections;
+import java.util.HashSet;
+import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
@@ -27,7 +29,7 @@ import static org.onlab.onos.cluster.MastershipEvent.Type.*;
 
 /**
  * Manages inventory of controller mastership over devices using
- * trivial in-memory structures implementation.
+ * trivial, non-distributed in-memory structures implementation.
  */
 @Component(immediate = true)
 @Service
@@ -35,18 +37,19 @@ public class SimpleMastershipStore
         extends AbstractStore<MastershipEvent, MastershipStoreDelegate>
         implements MastershipStore {
 
-    public static final IpPrefix LOCALHOST = IpPrefix.valueOf("127.0.0.1");
-
     private final Logger log = getLogger(getClass());
 
-    private ControllerNode instance;
+    public static final IpPrefix LOCALHOST = IpPrefix.valueOf("127.0.0.1");
+
+    private ControllerNode instance =
+            new DefaultControllerNode(new NodeId("local"), LOCALHOST);
 
-    protected final ConcurrentMap<DeviceId, MastershipRole> roleMap =
+    //devices mapped to their masters, to emulate multiple nodes
+    protected final ConcurrentMap<DeviceId, NodeId> masterMap =
             new ConcurrentHashMap<>();
 
     @Activate
     public void activate() {
-        instance = new DefaultControllerNode(new NodeId("local"), LOCALHOST);
         log.info("Started");
     }
 
@@ -56,23 +59,36 @@ public class SimpleMastershipStore
     }
 
     @Override
-    public MastershipEvent setRole(NodeId nodeId, DeviceId deviceId,
-                                   MastershipRole role) {
-        if (roleMap.get(deviceId) == null) {
+    public MastershipEvent setMaster(NodeId nodeId, DeviceId deviceId) {
+
+        NodeId node = masterMap.get(deviceId);
+        if (node == null) {
+            masterMap.put(deviceId, nodeId);
+            return new MastershipEvent(MASTER_CHANGED, deviceId, nodeId);
+        }
+
+        if (node.equals(nodeId)) {
             return null;
+        } else {
+            masterMap.put(deviceId, nodeId);
+            return new MastershipEvent(MASTER_CHANGED, deviceId, nodeId);
         }
-        roleMap.put(deviceId, role);
-        return new MastershipEvent(MASTER_CHANGED, deviceId, nodeId);
     }
 
     @Override
     public NodeId getMaster(DeviceId deviceId) {
-        return instance.id();
+        return masterMap.get(deviceId);
     }
 
     @Override
     public Set<DeviceId> getDevices(NodeId nodeId) {
-        return Collections.unmodifiableSet(roleMap.keySet());
+        Set<DeviceId> ids = new HashSet<>();
+        for (Map.Entry<DeviceId, NodeId> d : masterMap.entrySet()) {
+            if (d.getValue().equals(nodeId)) {
+                ids.add(d.getKey());
+            }
+        }
+        return Collections.unmodifiableSet(ids);
     }
 
     @Override
@@ -82,11 +98,18 @@ public class SimpleMastershipStore
 
     @Override
     public MastershipRole getRole(NodeId nodeId, DeviceId deviceId) {
-        MastershipRole role = roleMap.get(deviceId);
-        if (role == null) {
-            //say MASTER. If clustered, we'd figure out if anyone's got dibs here.
+        NodeId node = masterMap.get(deviceId);
+        MastershipRole role;
+        if (node != null) {
+            if (node.equals(nodeId)) {
+                role = MastershipRole.MASTER;
+            } else {
+                role = MastershipRole.STANDBY;
+            }
+        } else {
+            //masterMap doesn't contain it.
             role = MastershipRole.MASTER;
-            roleMap.put(deviceId, role);
+            masterMap.put(deviceId, nodeId);
         }
         return role;
     }