MastershipManagerTest plus tweaks to MastershipStore
Change-Id: Ie5d3201ce2297ae68cdafac4439168989dd804c5
diff --git a/core/api/src/main/java/org/onlab/onos/cluster/MastershipStore.java b/core/api/src/main/java/org/onlab/onos/cluster/MastershipStore.java
index 3e2ee03..b3dc696 100644
--- a/core/api/src/main/java/org/onlab/onos/cluster/MastershipStore.java
+++ b/core/api/src/main/java/org/onlab/onos/cluster/MastershipStore.java
@@ -54,6 +54,5 @@
* @param role new role
* @return a mastership event
*/
- MastershipEvent setRole(NodeId nodeId, DeviceId deviceId,
- MastershipRole role);
+ MastershipEvent setMaster(NodeId nodeId, DeviceId deviceId);
}
diff --git a/core/net/src/main/java/org/onlab/onos/cluster/impl/MastershipManager.java b/core/net/src/main/java/org/onlab/onos/cluster/impl/MastershipManager.java
index 4ac6052..255830c 100644
--- a/core/net/src/main/java/org/onlab/onos/cluster/impl/MastershipManager.java
+++ b/core/net/src/main/java/org/onlab/onos/cluster/impl/MastershipManager.java
@@ -64,9 +64,12 @@
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);
+ }
}
}
diff --git a/core/net/src/test/java/org/onlab/onos/cluster/impl/MastershipManagerTest.java b/core/net/src/test/java/org/onlab/onos/cluster/impl/MastershipManagerTest.java
new file mode 100644
index 0000000..40902f2
--- /dev/null
+++ b/core/net/src/test/java/org/onlab/onos/cluster/impl/MastershipManagerTest.java
@@ -0,0 +1,136 @@
+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) {
+ }
+
+ }
+}
diff --git a/core/store/src/main/java/org/onlab/onos/store/cluster/impl/DistributedMastershipStore.java b/core/store/src/main/java/org/onlab/onos/store/cluster/impl/DistributedMastershipStore.java
index 92d6880..c15d6aa 100644
--- a/core/store/src/main/java/org/onlab/onos/store/cluster/impl/DistributedMastershipStore.java
+++ b/core/store/src/main/java/org/onlab/onos/store/cluster/impl/DistributedMastershipStore.java
@@ -40,6 +40,7 @@
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
protected ClusterService clusterService;
+ @Override
@Activate
public void activate() {
super.activate();
@@ -59,10 +60,10 @@
}
@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;
}
@@ -92,7 +93,7 @@
@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;
}
diff --git a/core/trivial/src/main/java/org/onlab/onos/net/trivial/impl/SimpleMastershipStore.java b/core/trivial/src/main/java/org/onlab/onos/net/trivial/impl/SimpleMastershipStore.java
index 24480c6..829f140 100644
--- a/core/trivial/src/main/java/org/onlab/onos/net/trivial/impl/SimpleMastershipStore.java
+++ b/core/trivial/src/main/java/org/onlab/onos/net/trivial/impl/SimpleMastershipStore.java
@@ -3,6 +3,8 @@
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;
@@ -25,24 +27,25 @@
/**
* Manages inventory of controller mastership over devices using
- * trivial in-memory structures implementation.
+ * trivial, non-distributed in-memory structures implementation.
*/
@Component(immediate = true)
@Service
public class SimpleMastershipStore 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");
- protected final ConcurrentMap<DeviceId, MastershipRole> roleMap =
+ private ControllerNode instance =
+ new DefaultControllerNode(new NodeId("local"), LOCALHOST);
+
+ //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");
}
@@ -52,23 +55,36 @@
}
@Override
- public MastershipEvent setRole(NodeId nodeId, DeviceId deviceId,
- MastershipRole role) {
- if (roleMap.get(deviceId) == null) {
- return 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);
}
- roleMap.put(deviceId, role);
- 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);
+ }
}
@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
@@ -78,11 +94,18 @@
@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;
}