Relinquish mastership, tests, and few modifications to trivial MastershipStore

Change-Id: Iae29de010f13cb3ee02bcb316510cc254d5756fc

@@ -64,4 +64,14 @@ public interface MastershipStore extends Store<MastershipEvent, MastershipStoreD
      * @return the current master's ID and the term value for device, or null
      */
     MastershipTerm getTermFor(DeviceId deviceId);
+
+    /**
+     * Revokes a controller instance's mastership over a device and hands
+     * over mastership to another controller instance.
+     *
+     * @param nodeId   the controller instance identifier
+     * @param deviceId device to revoke mastership for
+     * @return a mastership event
+     */
+    MastershipEvent unsetMaster(NodeId nodeId, DeviceId deviceId);
 }

@@ -72,9 +72,20 @@ public class MastershipManager
         checkNotNull(nodeId, NODE_ID_NULL);
         checkNotNull(deviceId, DEVICE_ID_NULL);
         checkNotNull(role, ROLE_NULL);
-        //TODO figure out appropriate action for non-MASTER roles, if we even set those
+
+        MastershipRole current = store.getRole(nodeId, deviceId);
+        if (role.equals(current)) {
+            return;
+        } else {
+            MastershipEvent event = null;
             if (role.equals(MastershipRole.MASTER)) {
-            MastershipEvent event = store.setMaster(nodeId, deviceId);
+                //current was STANDBY, wanted MASTER
+                event = store.setMaster(nodeId, deviceId);
+            } else {
+                //current was MASTER, wanted STANDBY
+                event = store.unsetMaster(nodeId, deviceId);
+            }
+
             if (event != null) {
                 post(event);
             }
@@ -90,7 +101,18 @@ public class MastershipManager
     @Override
     public void relinquishMastership(DeviceId deviceId) {
         checkNotNull(deviceId, DEVICE_ID_NULL);
-        // FIXME: add method to store to give up mastership and trigger new master selection process
+
+        MastershipRole role = store.getRole(
+                clusterService.getLocalNode().id(), deviceId);
+        if (!role.equals(MastershipRole.MASTER)) {
+            return;
+        }
+
+        MastershipEvent event = store.unsetMaster(
+                clusterService.getLocalNode().id(), deviceId);
+        if (event != null) {
+            post(event);
+        }
     }
 
     @Override
@@ -159,10 +181,6 @@ public class MastershipManager
                      break;
                 case INSTANCE_REMOVED:
                 case INSTANCE_DEACTIVATED:
-                    for (DeviceId d : getDevicesOf(event.subject().id())) {
-                        //this method should be an admin iface?
-                        relinquishMastership(d);
-                    }
                     break;
                 default:
                     log.warn("unknown cluster event {}", event);

@@ -204,8 +204,11 @@ public class DeviceManager
             checkNotNull(deviceId, DEVICE_ID_NULL);
             checkValidity();
             DeviceEvent event = store.markOffline(deviceId);
+
+            //we're no longer capable of mastership.
             if (event != null) {
                 log.info("Device {} disconnected", deviceId);
+                mastershipService.relinquishMastership(deviceId);
                 post(event);
             }
         }

@@ -19,6 +19,7 @@ import org.onlab.onos.net.trivial.impl.SimpleMastershipStore;
 import org.onlab.packet.IpPrefix;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
 import static org.onlab.onos.net.MastershipRole.*;
 
 /**
@@ -65,7 +66,24 @@ public class MastershipManagerTest {
 
     @Test
     public void relinquishMastership() {
-        //TODO
+        //no backups - should turn to standby and no master for device
+        mgr.setRole(NID_LOCAL, DEV_MASTER, MASTER);
+        assertEquals("wrong role:", MASTER, mgr.getLocalRole(DEV_MASTER));
+        mgr.relinquishMastership(DEV_MASTER);
+        assertNull("wrong master:", mgr.getMasterFor(DEV_OTHER));
+        assertEquals("wrong role:", STANDBY, mgr.getLocalRole(DEV_MASTER));
+
+        //not master, nothing should happen
+        mgr.setRole(NID_LOCAL, DEV_OTHER, STANDBY);
+        mgr.relinquishMastership(DEV_OTHER);
+        assertNull("wrong role:", mgr.getMasterFor(DEV_OTHER));
+
+        //provide NID_OTHER as backup and relinquish
+        mgr.setRole(NID_LOCAL, DEV_MASTER, MASTER);
+        assertEquals("wrong master:", NID_LOCAL, mgr.getMasterFor(DEV_MASTER));
+        mgr.setRole(NID_OTHER, DEV_MASTER, STANDBY);
+        mgr.relinquishMastership(DEV_MASTER);
+        assertEquals("wrong master:", NID_OTHER, mgr.getMasterFor(DEV_MASTER));
     }
 
     @Test

@@ -122,4 +122,10 @@ public class DistributedMastershipStore
         return null;
     }
 
+    @Override
+    public MastershipEvent unsetMaster(NodeId nodeId, DeviceId deviceId) {
+        // TODO Auto-generated method stub
+        return null;
+    }
+
 }

@@ -5,6 +5,7 @@ import static org.slf4j.LoggerFactory.getLogger;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
@@ -26,6 +27,8 @@ import org.onlab.onos.store.AbstractStore;
 import org.onlab.packet.IpPrefix;
 import org.slf4j.Logger;
 
+import com.google.common.collect.Lists;
+
 import static org.onlab.onos.cluster.MastershipEvent.Type.*;
 
 /**
@@ -47,8 +50,10 @@ public class SimpleMastershipStore
 
     //devices mapped to their masters, to emulate multiple nodes
     protected final Map<DeviceId, NodeId> masterMap = new HashMap<>();
+    //emulate backups
+    protected final Map<DeviceId, List<NodeId>> backupMap = new HashMap<>();
+    //terms
     protected final Map<DeviceId, AtomicInteger> termMap = new HashMap<>();
-    protected final Set<NodeId> masters = new HashSet<>();
 
     @Activate
     public void activate() {
@@ -63,24 +68,38 @@ public class SimpleMastershipStore
     @Override
     public MastershipEvent setMaster(NodeId nodeId, DeviceId deviceId) {
 
-        NodeId node = masterMap.get(deviceId);
-        if (node == null) {
+        NodeId current = masterMap.get(deviceId);
+        List<NodeId> backups = backupMap.get(deviceId);
+
+        if (current == null) {
+            if (backups == null) {
+                //add new mapping to everything
                 synchronized (this) {
                     masterMap.put(deviceId, nodeId);
+                    backups = Lists.newLinkedList();
+                    backupMap.put(deviceId, backups);
                     termMap.put(deviceId, new AtomicInteger());
                 }
-            return new MastershipEvent(MASTER_CHANGED, deviceId, nodeId);
-        }
-
-        if (node.equals(nodeId)) {
-            return null;
             } else {
+                //set master to new node and remove from backups if there
                 synchronized (this) {
                     masterMap.put(deviceId, nodeId);
+                    backups.remove(nodeId);
                     termMap.get(deviceId).incrementAndGet();
-                return new MastershipEvent(MASTER_CHANGED, deviceId, nodeId);
                 }
             }
+        } else if (current.equals(nodeId)) {
+            return null;
+        } else {
+            //add current to backup, set master to new node
+            masterMap.put(deviceId, nodeId);
+            backups.add(current);
+            backups.remove(nodeId);
+            termMap.get(deviceId).incrementAndGet();
+        }
+
+        updateStandby(nodeId, deviceId);
+        return new MastershipEvent(MASTER_CHANGED, deviceId, nodeId);
     }
 
     @Override
@@ -106,29 +125,97 @@ public class SimpleMastershipStore
 
     @Override
     public MastershipRole getRole(NodeId nodeId, DeviceId deviceId) {
-        NodeId node = masterMap.get(deviceId);
-        MastershipRole role;
-        if (node != null) {
-            if (node.equals(nodeId)) {
-                role = MastershipRole.MASTER;
-            } else {
-                role = MastershipRole.STANDBY;
+        NodeId current = masterMap.get(deviceId);
+        List<NodeId> backups = backupMap.get(deviceId);
+
+        if (current == null) {
+            //masterMap or backup doesn't contain device. Say new node is MASTER
+            if (backups == null) {
+                synchronized (this) {
+                    masterMap.put(deviceId, nodeId);
+                    backups = Lists.newLinkedList();
+                    backupMap.put(deviceId, backups);
+                    termMap.put(deviceId, new AtomicInteger());
+                }
+                updateStandby(nodeId, deviceId);
+                return MastershipRole.MASTER;
+            }
+
+            //device once existed, but got removed, and is now getting a backup.
+            if (!backups.contains(nodeId)) {
+                synchronized (this) {
+                    backups.add(nodeId);
+                    termMap.put(deviceId, new AtomicInteger());
                 }
+                updateStandby(nodeId, deviceId);
+            }
+
+        } else if (current.equals(nodeId)) {
+            return  MastershipRole.MASTER;
         } else {
-            //masterMap doesn't contain it.
-            role = MastershipRole.MASTER;
-            masterMap.put(deviceId, nodeId);
+            //once created, a device never has a null backups list.
+            if (!backups.contains(nodeId)) {
+                //we must have requested STANDBY setting
+                synchronized (this) {
+                    backups.add(nodeId);
+                    termMap.put(deviceId, new AtomicInteger());
+                }
+                updateStandby(nodeId, deviceId);
+            }
         }
-        return role;
+
+        return MastershipRole.STANDBY;
     }
 
     @Override
     public MastershipTerm getTermFor(DeviceId deviceId) {
-        if (masterMap.get(deviceId) == null) {
+        if ((masterMap.get(deviceId) == null) ||
+                (termMap.get(deviceId) == null)) {
             return null;
         }
         return MastershipTerm.of(
                 masterMap.get(deviceId), termMap.get(deviceId).get());
     }
 
+    @Override
+    public MastershipEvent unsetMaster(NodeId nodeId, DeviceId deviceId) {
+        NodeId node = masterMap.get(deviceId);
+
+        //TODO case where node is completely removed from the cluster?
+        if (node.equals(nodeId)) {
+            synchronized (this) {
+                //pick new node.
+                List<NodeId> backups = backupMap.get(deviceId);
+
+                //no backups, so device is hosed
+                if (backups.isEmpty()) {
+                    masterMap.remove(deviceId);
+                    backups.add(nodeId);
+                    return null;
+                }
+                NodeId backup = backups.remove(0);
+                masterMap.put(deviceId, backup);
+                backups.add(nodeId);
+                return new MastershipEvent(MASTER_CHANGED, deviceId, backup);
+            }
+        }
+        return null;
+    }
+
+    //add node as STANDBY to maps un-scalably.
+    private void updateStandby(NodeId nodeId, DeviceId deviceId) {
+        for (Map.Entry<DeviceId, List<NodeId>> e : backupMap.entrySet()) {
+            DeviceId dev = e.getKey();
+            if (dev.equals(deviceId)) {
+                continue;
+            }
+            synchronized (this) {
+                List<NodeId> nodes = e.getValue();
+                if (!nodes.contains(nodeId)) {
+                    nodes.add(nodeId);
+                }
+            }
+        }
+    }
+
 }