Changed argument from Device -> DeviceId
- roleChanged(), isReachable()
Eventually consistent nature of Device store was
interfering with mastership control.
Change-Id: I9c0dd846a4e30863f922f6706c6cb62fd7c83f29
diff --git a/apps/optical/src/main/java/org/onlab/onos/optical/cfg/OpticalConfigProvider.java b/apps/optical/src/main/java/org/onlab/onos/optical/cfg/OpticalConfigProvider.java
index 5c9de15..4b9bc9f 100644
--- a/apps/optical/src/main/java/org/onlab/onos/optical/cfg/OpticalConfigProvider.java
+++ b/apps/optical/src/main/java/org/onlab/onos/optical/cfg/OpticalConfigProvider.java
@@ -368,12 +368,12 @@
}
@Override
- public void roleChanged(Device device, MastershipRole newRole) {
+ public void roleChanged(DeviceId device, MastershipRole newRole) {
// TODO Auto-generated method stub.
}
@Override
- public boolean isReachable(Device device) {
+ public boolean isReachable(DeviceId device) {
return false;
}
}
diff --git a/core/api/src/main/java/org/onlab/onos/net/device/DeviceProvider.java b/core/api/src/main/java/org/onlab/onos/net/device/DeviceProvider.java
index 8411b09..8dd405a 100644
--- a/core/api/src/main/java/org/onlab/onos/net/device/DeviceProvider.java
+++ b/core/api/src/main/java/org/onlab/onos/net/device/DeviceProvider.java
@@ -16,6 +16,7 @@
package org.onlab.onos.net.device;
import org.onlab.onos.net.Device;
+import org.onlab.onos.net.DeviceId;
import org.onlab.onos.net.MastershipRole;
import org.onlab.onos.net.provider.Provider;
@@ -42,16 +43,16 @@
* Notifies the provider of a mastership role change for the specified
* device as decided by the core.
*
- * @param device affected device
+ * @param deviceId device identifier
* @param newRole newly determined mastership role
*/
- void roleChanged(Device device, MastershipRole newRole);
+ void roleChanged(DeviceId deviceId, MastershipRole newRole);
/**
* Checks the reachability (connectivity) of a device from this provider.
*
- * @param device device to check
+ * @param deviceId device identifier
* @return true if reachable, false otherwise
*/
- boolean isReachable(Device device);
+ boolean isReachable(DeviceId deviceId);
}
diff --git a/core/net/src/main/java/org/onlab/onos/net/device/impl/DeviceManager.java b/core/net/src/main/java/org/onlab/onos/net/device/impl/DeviceManager.java
index 258f57c..68665e4 100644
--- a/core/net/src/main/java/org/onlab/onos/net/device/impl/DeviceManager.java
+++ b/core/net/src/main/java/org/onlab/onos/net/device/impl/DeviceManager.java
@@ -159,13 +159,14 @@
}
// Check a device for control channel connectivity.
- private boolean isReachable(Device device) {
- // FIXME: Device might not be there yet. (eventual consistent)
- if (device == null) {
+ private boolean isReachable(DeviceId deviceId) {
+ DeviceProvider provider = getProvider(deviceId);
+ if (provider != null) {
+ return provider.isReachable(deviceId);
+ } else {
+ log.error("Provider not found for {}", deviceId);
return false;
}
- DeviceProvider provider = getProvider(device.providerId());
- return provider.isReachable(device);
}
@Override
@@ -226,15 +227,9 @@
log.warn("Provider for {} was not found. Cannot apply role {}", deviceId, newRole);
return false;
}
- // FIXME roleChanged should take DeviceId instead of Device
- Device device = store.getDevice(deviceId);
- if (device == null) {
- log.warn("{} was not there. Cannot apply role {}", deviceId, newRole);
- return false;
- }
- provider.roleChanged(device, newRole);
+ provider.roleChanged(deviceId, newRole);
+ // not triggering probe when triggered by provider service event
- // not triggering prove when triggered by provider service
return true;
}
@@ -442,6 +437,7 @@
Device device = store.getDevice(deviceId);
// FIXME: Device might not be there yet. (eventual consistent)
+ // FIXME relinquish role
if (device == null) {
log.warn("{} was not there. Cannot apply role {}", deviceId, newRole);
return false;
@@ -452,8 +448,7 @@
log.warn("Provider for {} was not found. Cannot apply role {}", deviceId, newRole);
return false;
}
- // FIXME roleChanged should take DeviceId instead of Device
- provider.roleChanged(device, newRole);
+ provider.roleChanged(deviceId, newRole);
if (newRole.equals(MastershipRole.MASTER)) {
// only trigger event when request was sent to provider
@@ -495,8 +490,7 @@
}
- final Device device = getDevice(did);
- final boolean isReachable = isReachable(device);
+ final boolean isReachable = isReachable(did);
if (!isReachable) {
// device is not connected to this node
if (myNextRole != NONE) {
@@ -505,6 +499,7 @@
+ "Relinquishing role. ",
myNextRole, did);
mastershipService.relinquishMastership(did);
+ // FIXME disconnect?
}
return;
}
@@ -523,6 +518,7 @@
switch (myNextRole) {
case MASTER:
+ final Device device = getDevice(did);
if ((device != null) && !isAvailable(did)) {
//flag the device as online. Is there a better way to do this?
DefaultDeviceDescription deviceDescription
diff --git a/core/net/src/test/java/org/onlab/onos/net/device/impl/DeviceManagerTest.java b/core/net/src/test/java/org/onlab/onos/net/device/impl/DeviceManagerTest.java
index 79e2f10..d278c24 100644
--- a/core/net/src/test/java/org/onlab/onos/net/device/impl/DeviceManagerTest.java
+++ b/core/net/src/test/java/org/onlab/onos/net/device/impl/DeviceManagerTest.java
@@ -251,7 +251,7 @@
private class TestProvider extends AbstractProvider implements DeviceProvider {
- private Device deviceReceived;
+ private DeviceId deviceReceived;
private MastershipRole roleReceived;
public TestProvider() {
@@ -263,13 +263,13 @@
}
@Override
- public void roleChanged(Device device, MastershipRole newRole) {
+ public void roleChanged(DeviceId device, MastershipRole newRole) {
deviceReceived = device;
roleReceived = newRole;
}
@Override
- public boolean isReachable(Device device) {
+ public boolean isReachable(DeviceId device) {
return false;
}
}
diff --git a/providers/openflow/device/src/main/java/org/onlab/onos/provider/of/device/impl/OpenFlowDeviceProvider.java b/providers/openflow/device/src/main/java/org/onlab/onos/provider/of/device/impl/OpenFlowDeviceProvider.java
index aaa6117..c0cc416 100644
--- a/providers/openflow/device/src/main/java/org/onlab/onos/provider/of/device/impl/OpenFlowDeviceProvider.java
+++ b/providers/openflow/device/src/main/java/org/onlab/onos/provider/of/device/impl/OpenFlowDeviceProvider.java
@@ -111,8 +111,8 @@
@Override
- public boolean isReachable(Device device) {
- OpenFlowSwitch sw = controller.getSwitch(dpid(device.id().uri()));
+ public boolean isReachable(DeviceId deviceId) {
+ OpenFlowSwitch sw = controller.getSwitch(dpid(deviceId.uri()));
if (sw == null || !sw.isConnected()) {
return false;
}
@@ -170,22 +170,22 @@
// }
@Override
- public void roleChanged(Device device, MastershipRole newRole) {
+ public void roleChanged(DeviceId deviceId, MastershipRole newRole) {
switch (newRole) {
case MASTER:
- controller.setRole(dpid(device.id().uri()), RoleState.MASTER);
+ controller.setRole(dpid(deviceId.uri()), RoleState.MASTER);
break;
case STANDBY:
- controller.setRole(dpid(device.id().uri()), RoleState.EQUAL);
+ controller.setRole(dpid(deviceId.uri()), RoleState.EQUAL);
break;
case NONE:
- controller.setRole(dpid(device.id().uri()), RoleState.SLAVE);
+ controller.setRole(dpid(deviceId.uri()), RoleState.SLAVE);
break;
default:
LOG.error("Unknown Mastership state : {}", newRole);
}
- LOG.info("Accepting mastership role change for device {}", device.id());
+ LOG.info("Accepting mastership role change for device {}", deviceId);
}
private class InternalDeviceProvider implements OpenFlowSwitchListener {
diff --git a/providers/openflow/device/src/test/java/org/onlab/onos/provider/of/device/impl/OpenFlowDeviceProviderTest.java b/providers/openflow/device/src/test/java/org/onlab/onos/provider/of/device/impl/OpenFlowDeviceProviderTest.java
index 839de7d..88c68a9 100644
--- a/providers/openflow/device/src/test/java/org/onlab/onos/provider/of/device/impl/OpenFlowDeviceProviderTest.java
+++ b/providers/openflow/device/src/test/java/org/onlab/onos/provider/of/device/impl/OpenFlowDeviceProviderTest.java
@@ -105,11 +105,11 @@
@Test
public void roleChanged() {
- provider.roleChanged(DEV1, MASTER);
+ provider.roleChanged(DID1, MASTER);
assertEquals("Should be MASTER", RoleState.MASTER, controller.roleMap.get(DPID1));
- provider.roleChanged(DEV1, STANDBY);
+ provider.roleChanged(DID1, STANDBY);
assertEquals("Should be EQUAL", RoleState.EQUAL, controller.roleMap.get(DPID1));
- provider.roleChanged(DEV1, NONE);
+ provider.roleChanged(DID1, NONE);
assertEquals("Should be SLAVE", RoleState.SLAVE, controller.roleMap.get(DPID1));
}
diff --git a/web/api/src/main/java/org/onlab/onos/rest/ConfigProvider.java b/web/api/src/main/java/org/onlab/onos/rest/ConfigProvider.java
index 57e247f..e4a0763 100644
--- a/web/api/src/main/java/org/onlab/onos/rest/ConfigProvider.java
+++ b/web/api/src/main/java/org/onlab/onos/rest/ConfigProvider.java
@@ -244,7 +244,7 @@
}
@Override
- public void roleChanged(Device device, MastershipRole newRole) {
+ public void roleChanged(DeviceId device, MastershipRole newRole) {
}
@Override
@@ -257,7 +257,7 @@
}
@Override
- public boolean isReachable(Device device) {
+ public boolean isReachable(DeviceId device) {
return false;
}
}