MastershipManagerTest plus tweaks to MastershipStore
Change-Id: Ie5d3201ce2297ae68cdafac4439168989dd804c5
Showing
5 changed files
with
187 additions
and
25 deletions
... | @@ -54,6 +54,5 @@ public interface MastershipStore { | ... | @@ -54,6 +54,5 @@ public interface MastershipStore { |
54 | * @param role new role | 54 | * @param role new role |
55 | * @return a mastership event | 55 | * @return a mastership event |
56 | */ | 56 | */ |
57 | - MastershipEvent setRole(NodeId nodeId, DeviceId deviceId, | 57 | + MastershipEvent setMaster(NodeId nodeId, DeviceId deviceId); |
58 | - MastershipRole role); | ||
59 | } | 58 | } | ... | ... |
... | @@ -64,9 +64,12 @@ public class MastershipManager | ... | @@ -64,9 +64,12 @@ public class MastershipManager |
64 | checkNotNull(nodeId, NODE_ID_NULL); | 64 | checkNotNull(nodeId, NODE_ID_NULL); |
65 | checkNotNull(deviceId, DEVICE_ID_NULL); | 65 | checkNotNull(deviceId, DEVICE_ID_NULL); |
66 | checkNotNull(role, ROLE_NULL); | 66 | checkNotNull(role, ROLE_NULL); |
67 | - MastershipEvent event = store.setRole(nodeId, deviceId, role); | 67 | + //TODO figure out appropriate action for non-MASTER roles, if we even set those |
68 | - if (event != null) { | 68 | + if (role.equals(MastershipRole.MASTER)) { |
69 | - post(event); | 69 | + MastershipEvent event = store.setMaster(nodeId, deviceId); |
70 | + if (event != null) { | ||
71 | + post(event); | ||
72 | + } | ||
70 | } | 73 | } |
71 | } | 74 | } |
72 | 75 | ... | ... |
1 | +package org.onlab.onos.cluster.impl; | ||
2 | + | ||
3 | +import java.util.Set; | ||
4 | + | ||
5 | +import org.junit.After; | ||
6 | +import org.junit.Before; | ||
7 | +import org.junit.Test; | ||
8 | +import org.onlab.onos.cluster.ClusterEventListener; | ||
9 | +import org.onlab.onos.cluster.ClusterService; | ||
10 | +import org.onlab.onos.cluster.ControllerNode; | ||
11 | +import org.onlab.onos.cluster.ControllerNode.State; | ||
12 | +import org.onlab.onos.cluster.DefaultControllerNode; | ||
13 | +import org.onlab.onos.cluster.MastershipService; | ||
14 | +import org.onlab.onos.cluster.NodeId; | ||
15 | +import org.onlab.onos.event.impl.TestEventDispatcher; | ||
16 | +import org.onlab.onos.net.DeviceId; | ||
17 | +import org.onlab.onos.net.trivial.impl.SimpleMastershipStore; | ||
18 | +import org.onlab.packet.IpPrefix; | ||
19 | + | ||
20 | +import static org.junit.Assert.assertEquals; | ||
21 | +import static org.onlab.onos.net.MastershipRole.*; | ||
22 | + | ||
23 | +/** | ||
24 | + * Test codifying the mastership service contracts. | ||
25 | + */ | ||
26 | +public class MastershipManagerTest { | ||
27 | + | ||
28 | + private static final NodeId NID_LOCAL = new NodeId("local"); | ||
29 | + private static final NodeId NID_OTHER = new NodeId("foo"); | ||
30 | + private static final IpPrefix LOCALHOST = IpPrefix.valueOf("127.0.0.1"); | ||
31 | + private static final DeviceId DEV_MASTER = DeviceId.deviceId("of:1"); | ||
32 | + private static final DeviceId DEV_OTHER = DeviceId.deviceId("of:2"); | ||
33 | + | ||
34 | + private MastershipManager mgr; | ||
35 | + protected MastershipService service; | ||
36 | + | ||
37 | + @Before | ||
38 | + public void setUp() { | ||
39 | + mgr = new MastershipManager(); | ||
40 | + service = mgr; | ||
41 | + mgr.store = new SimpleMastershipStore(); | ||
42 | + mgr.eventDispatcher = new TestEventDispatcher(); | ||
43 | + mgr.clusterService = new TestClusterService(); | ||
44 | + mgr.activate(); | ||
45 | + } | ||
46 | + | ||
47 | + @After | ||
48 | + public void tearDown() { | ||
49 | + mgr.deactivate(); | ||
50 | + mgr.clusterService = null; | ||
51 | + mgr.eventDispatcher = null; | ||
52 | + mgr.store = null; | ||
53 | + } | ||
54 | + | ||
55 | + @Test | ||
56 | + public void setRole() { | ||
57 | + mgr.setRole(NID_OTHER, DEV_MASTER, MASTER); | ||
58 | + assertEquals("wrong local role:", STANDBY, mgr.getLocalRole(DEV_MASTER)); | ||
59 | + | ||
60 | + //set to master | ||
61 | + mgr.setRole(NID_LOCAL, DEV_MASTER, MASTER); | ||
62 | + assertEquals("wrong local role:", MASTER, mgr.getLocalRole(DEV_MASTER)); | ||
63 | + } | ||
64 | + | ||
65 | + @Test | ||
66 | + public void relinquishMastership() { | ||
67 | + //TODO | ||
68 | + } | ||
69 | + | ||
70 | + @Test | ||
71 | + public void requestRoleFor() { | ||
72 | + mgr.setRole(NID_LOCAL, DEV_MASTER, MASTER); | ||
73 | + mgr.setRole(NID_OTHER, DEV_OTHER, MASTER); | ||
74 | + | ||
75 | + //local should be master for one but standby for other | ||
76 | + assertEquals("wrong role:", MASTER, mgr.requestRoleFor(DEV_MASTER)); | ||
77 | + assertEquals("wrong role:", STANDBY, mgr.requestRoleFor(DEV_OTHER)); | ||
78 | + } | ||
79 | + | ||
80 | + @Test | ||
81 | + public void getMasterFor() { | ||
82 | + mgr.setRole(NID_LOCAL, DEV_MASTER, MASTER); | ||
83 | + mgr.setRole(NID_OTHER, DEV_OTHER, MASTER); | ||
84 | + assertEquals("wrong master:", NID_LOCAL, mgr.getMasterFor(DEV_MASTER)); | ||
85 | + assertEquals("wrong master:", NID_OTHER, mgr.getMasterFor(DEV_OTHER)); | ||
86 | + | ||
87 | + //have NID_OTHER hand over DEV_OTHER to NID_LOCAL | ||
88 | + mgr.setRole(NID_LOCAL, DEV_OTHER, MASTER); | ||
89 | + assertEquals("wrong master:", NID_LOCAL, mgr.getMasterFor(DEV_OTHER)); | ||
90 | + } | ||
91 | + | ||
92 | + @Test | ||
93 | + public void getDevicesOf() { | ||
94 | + mgr.setRole(NID_LOCAL, DEV_MASTER, MASTER); | ||
95 | + mgr.setRole(NID_LOCAL, DEV_OTHER, STANDBY); | ||
96 | + assertEquals("should be one device:", 1, mgr.getDevicesOf(NID_LOCAL).size()); | ||
97 | + | ||
98 | + //hand both devices to NID_LOCAL | ||
99 | + mgr.setRole(NID_LOCAL, DEV_OTHER, MASTER); | ||
100 | + assertEquals("should be two devices:", 2, mgr.getDevicesOf(NID_LOCAL).size()); | ||
101 | + } | ||
102 | + | ||
103 | + private final class TestClusterService implements ClusterService { | ||
104 | + | ||
105 | + ControllerNode local = new DefaultControllerNode(NID_LOCAL, LOCALHOST); | ||
106 | + | ||
107 | + @Override | ||
108 | + public ControllerNode getLocalNode() { | ||
109 | + return local; | ||
110 | + } | ||
111 | + | ||
112 | + @Override | ||
113 | + public Set<ControllerNode> getNodes() { | ||
114 | + return null; | ||
115 | + } | ||
116 | + | ||
117 | + @Override | ||
118 | + public ControllerNode getNode(NodeId nodeId) { | ||
119 | + return null; | ||
120 | + } | ||
121 | + | ||
122 | + @Override | ||
123 | + public State getState(NodeId nodeId) { | ||
124 | + return null; | ||
125 | + } | ||
126 | + | ||
127 | + @Override | ||
128 | + public void addListener(ClusterEventListener listener) { | ||
129 | + } | ||
130 | + | ||
131 | + @Override | ||
132 | + public void removeListener(ClusterEventListener listener) { | ||
133 | + } | ||
134 | + | ||
135 | + } | ||
136 | +} |
... | @@ -40,6 +40,7 @@ public class DistributedMastershipStore extends AbstractDistributedStore | ... | @@ -40,6 +40,7 @@ public class DistributedMastershipStore extends AbstractDistributedStore |
40 | @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY) | 40 | @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY) |
41 | protected ClusterService clusterService; | 41 | protected ClusterService clusterService; |
42 | 42 | ||
43 | + @Override | ||
43 | @Activate | 44 | @Activate |
44 | public void activate() { | 45 | public void activate() { |
45 | super.activate(); | 46 | super.activate(); |
... | @@ -59,10 +60,10 @@ public class DistributedMastershipStore extends AbstractDistributedStore | ... | @@ -59,10 +60,10 @@ public class DistributedMastershipStore extends AbstractDistributedStore |
59 | } | 60 | } |
60 | 61 | ||
61 | @Override | 62 | @Override |
62 | - public MastershipEvent setRole(NodeId nodeId, DeviceId deviceId, MastershipRole role) { | 63 | + public MastershipEvent setMaster(NodeId nodeId, DeviceId deviceId) { |
63 | synchronized (this) { | 64 | synchronized (this) { |
64 | NodeId currentMaster = getMaster(deviceId); | 65 | NodeId currentMaster = getMaster(deviceId); |
65 | - if (role == MastershipRole.MASTER && Objects.equals(currentMaster, nodeId)) { | 66 | + if (Objects.equals(currentMaster, nodeId)) { |
66 | return null; | 67 | return null; |
67 | } | 68 | } |
68 | 69 | ||
... | @@ -92,7 +93,7 @@ public class DistributedMastershipStore extends AbstractDistributedStore | ... | @@ -92,7 +93,7 @@ public class DistributedMastershipStore extends AbstractDistributedStore |
92 | @Override | 93 | @Override |
93 | public MastershipRole requestRole(DeviceId deviceId) { | 94 | public MastershipRole requestRole(DeviceId deviceId) { |
94 | // FIXME: for now we are 'selecting' as master whoever asks | 95 | // FIXME: for now we are 'selecting' as master whoever asks |
95 | - setRole(clusterService.getLocalNode().id(), deviceId, MastershipRole.MASTER); | 96 | + setMaster(clusterService.getLocalNode().id(), deviceId); |
96 | return MastershipRole.MASTER; | 97 | return MastershipRole.MASTER; |
97 | } | 98 | } |
98 | 99 | ... | ... |
... | @@ -3,6 +3,8 @@ package org.onlab.onos.net.trivial.impl; | ... | @@ -3,6 +3,8 @@ package org.onlab.onos.net.trivial.impl; |
3 | import static org.slf4j.LoggerFactory.getLogger; | 3 | import static org.slf4j.LoggerFactory.getLogger; |
4 | 4 | ||
5 | import java.util.Collections; | 5 | import java.util.Collections; |
6 | +import java.util.HashSet; | ||
7 | +import java.util.Map; | ||
6 | import java.util.Set; | 8 | import java.util.Set; |
7 | import java.util.concurrent.ConcurrentHashMap; | 9 | import java.util.concurrent.ConcurrentHashMap; |
8 | import java.util.concurrent.ConcurrentMap; | 10 | import java.util.concurrent.ConcurrentMap; |
... | @@ -25,24 +27,25 @@ import static org.onlab.onos.cluster.MastershipEvent.Type.*; | ... | @@ -25,24 +27,25 @@ import static org.onlab.onos.cluster.MastershipEvent.Type.*; |
25 | 27 | ||
26 | /** | 28 | /** |
27 | * Manages inventory of controller mastership over devices using | 29 | * Manages inventory of controller mastership over devices using |
28 | - * trivial in-memory structures implementation. | 30 | + * trivial, non-distributed in-memory structures implementation. |
29 | */ | 31 | */ |
30 | @Component(immediate = true) | 32 | @Component(immediate = true) |
31 | @Service | 33 | @Service |
32 | public class SimpleMastershipStore implements MastershipStore { | 34 | public class SimpleMastershipStore implements MastershipStore { |
33 | 35 | ||
34 | - public static final IpPrefix LOCALHOST = IpPrefix.valueOf("127.0.0.1"); | ||
35 | - | ||
36 | private final Logger log = getLogger(getClass()); | 36 | private final Logger log = getLogger(getClass()); |
37 | 37 | ||
38 | - private ControllerNode instance; | 38 | + public static final IpPrefix LOCALHOST = IpPrefix.valueOf("127.0.0.1"); |
39 | + | ||
40 | + private ControllerNode instance = | ||
41 | + new DefaultControllerNode(new NodeId("local"), LOCALHOST); | ||
39 | 42 | ||
40 | - protected final ConcurrentMap<DeviceId, MastershipRole> roleMap = | 43 | + //devices mapped to their masters, to emulate multiple nodes |
44 | + protected final ConcurrentMap<DeviceId, NodeId> masterMap = | ||
41 | new ConcurrentHashMap<>(); | 45 | new ConcurrentHashMap<>(); |
42 | 46 | ||
43 | @Activate | 47 | @Activate |
44 | public void activate() { | 48 | public void activate() { |
45 | - instance = new DefaultControllerNode(new NodeId("local"), LOCALHOST); | ||
46 | log.info("Started"); | 49 | log.info("Started"); |
47 | } | 50 | } |
48 | 51 | ||
... | @@ -52,23 +55,36 @@ public class SimpleMastershipStore implements MastershipStore { | ... | @@ -52,23 +55,36 @@ public class SimpleMastershipStore implements MastershipStore { |
52 | } | 55 | } |
53 | 56 | ||
54 | @Override | 57 | @Override |
55 | - public MastershipEvent setRole(NodeId nodeId, DeviceId deviceId, | 58 | + public MastershipEvent setMaster(NodeId nodeId, DeviceId deviceId) { |
56 | - MastershipRole role) { | 59 | + |
57 | - if (roleMap.get(deviceId) == null) { | 60 | + NodeId node = masterMap.get(deviceId); |
61 | + if (node == null) { | ||
62 | + masterMap.put(deviceId, nodeId); | ||
63 | + return new MastershipEvent(MASTER_CHANGED, deviceId, nodeId); | ||
64 | + } | ||
65 | + | ||
66 | + if (node.equals(nodeId)) { | ||
58 | return null; | 67 | return null; |
68 | + } else { | ||
69 | + masterMap.put(deviceId, nodeId); | ||
70 | + return new MastershipEvent(MASTER_CHANGED, deviceId, nodeId); | ||
59 | } | 71 | } |
60 | - roleMap.put(deviceId, role); | ||
61 | - return new MastershipEvent(MASTER_CHANGED, deviceId, nodeId); | ||
62 | } | 72 | } |
63 | 73 | ||
64 | @Override | 74 | @Override |
65 | public NodeId getMaster(DeviceId deviceId) { | 75 | public NodeId getMaster(DeviceId deviceId) { |
66 | - return instance.id(); | 76 | + return masterMap.get(deviceId); |
67 | } | 77 | } |
68 | 78 | ||
69 | @Override | 79 | @Override |
70 | public Set<DeviceId> getDevices(NodeId nodeId) { | 80 | public Set<DeviceId> getDevices(NodeId nodeId) { |
71 | - return Collections.unmodifiableSet(roleMap.keySet()); | 81 | + Set<DeviceId> ids = new HashSet<>(); |
82 | + for (Map.Entry<DeviceId, NodeId> d : masterMap.entrySet()) { | ||
83 | + if (d.getValue().equals(nodeId)) { | ||
84 | + ids.add(d.getKey()); | ||
85 | + } | ||
86 | + } | ||
87 | + return Collections.unmodifiableSet(ids); | ||
72 | } | 88 | } |
73 | 89 | ||
74 | @Override | 90 | @Override |
... | @@ -78,11 +94,18 @@ public class SimpleMastershipStore implements MastershipStore { | ... | @@ -78,11 +94,18 @@ public class SimpleMastershipStore implements MastershipStore { |
78 | 94 | ||
79 | @Override | 95 | @Override |
80 | public MastershipRole getRole(NodeId nodeId, DeviceId deviceId) { | 96 | public MastershipRole getRole(NodeId nodeId, DeviceId deviceId) { |
81 | - MastershipRole role = roleMap.get(deviceId); | 97 | + NodeId node = masterMap.get(deviceId); |
82 | - if (role == null) { | 98 | + MastershipRole role; |
83 | - //say MASTER. If clustered, we'd figure out if anyone's got dibs here. | 99 | + if (node != null) { |
100 | + if (node.equals(nodeId)) { | ||
101 | + role = MastershipRole.MASTER; | ||
102 | + } else { | ||
103 | + role = MastershipRole.STANDBY; | ||
104 | + } | ||
105 | + } else { | ||
106 | + //masterMap doesn't contain it. | ||
84 | role = MastershipRole.MASTER; | 107 | role = MastershipRole.MASTER; |
85 | - roleMap.put(deviceId, role); | 108 | + masterMap.put(deviceId, nodeId); |
86 | } | 109 | } |
87 | return role; | 110 | return role; |
88 | } | 111 | } | ... | ... |
-
Please register or login to post a comment