avoid transient changes to MastershipStore from being posted as events
Change-Id: Id033cf50f865e44439138f5b3814ebaedb832b73
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 342bd5b..5c9de15 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
@@ -372,4 +372,8 @@
// TODO Auto-generated method stub.
}
+ @Override
+ public boolean isReachable(Device 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 fff6333..8411b09 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
@@ -47,4 +47,11 @@
*/
void roleChanged(Device device, MastershipRole newRole);
+ /**
+ * Checks the reachability (connectivity) of a device from this provider.
+ *
+ * @param device device to check
+ * @return true if reachable, false otherwise
+ */
+ boolean isReachable(Device device);
}
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 d3cce22..6180ada 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
@@ -97,10 +97,20 @@
checkNotNull(role, ROLE_NULL);
MastershipEvent event = null;
- if (role.equals(MastershipRole.MASTER)) {
- event = store.setMaster(nodeId, deviceId);
- } else {
- event = store.setStandby(nodeId, deviceId);
+
+ switch (role) {
+ case MASTER:
+ event = store.setMaster(nodeId, deviceId);
+ break;
+ case STANDBY:
+ event = store.setStandby(nodeId, deviceId);
+ break;
+ case NONE:
+ event = store.relinquishRole(nodeId, deviceId);
+ break;
+ default:
+ log.info("Unknown role; ignoring");
+ return;
}
if (event != null) {
@@ -259,6 +269,10 @@
@Override
public void notify(MastershipEvent event) {
+ if (clusterService.getLocalNode().id().equals(event.roleInfo().master())) {
+ log.info("ignoring locally-generated event {}", event);
+ // return;
+ }
log.info("dispatching mastership event {}", event);
eventDispatcher.post(event);
}
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 3559c4b..e284052 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,32 +159,37 @@
// Applies the specified role to the device; ignores NONE
private void applyRole(DeviceId deviceId, MastershipRole newRole) {
- if (!newRole.equals(MastershipRole.NONE)) {
- Device device = store.getDevice(deviceId);
- // FIXME: Device might not be there yet. (eventual consistent)
- if (device == null) {
- return;
- }
- DeviceProvider provider = getProvider(device.providerId());
- if (provider != null) {
- provider.roleChanged(device, newRole);
-
- // only trigger event when request was sent to provider
- // TODO: consider removing this from Device event type?
- post(new DeviceEvent(DEVICE_MASTERSHIP_CHANGED, device));
- }
+ if (newRole.equals(MastershipRole.NONE)) {
+ return;
}
- }
- // Queries a device for port information.
- private void queryPortInfo(DeviceId deviceId) {
Device device = store.getDevice(deviceId);
// FIXME: Device might not be there yet. (eventual consistent)
if (device == null) {
return;
}
+
DeviceProvider provider = getProvider(device.providerId());
- provider.triggerProbe(device);
+ if (provider != null) {
+ provider.roleChanged(device, newRole);
+ // only trigger event when request was sent to provider
+ // TODO: consider removing this from Device event type?
+ post(new DeviceEvent(DEVICE_MASTERSHIP_CHANGED, device));
+
+ if (newRole.equals(MastershipRole.MASTER)) {
+ provider.triggerProbe(device);
+ }
+ }
+ }
+
+ // Check a device for control channel connectivity.
+ private boolean isReachable(Device device) {
+ // FIXME: Device might not be there yet. (eventual consistent)
+ if (device == null) {
+ return false;
+ }
+ DeviceProvider provider = getProvider(device.providerId());
+ return provider.isReachable(device);
}
@Override
@@ -236,7 +241,6 @@
log.info("Device {} connected", deviceId);
// check my Role
MastershipRole role = mastershipService.requestRoleFor(deviceId);
- log.info("requestedRole, became {} for {}", role, deviceId);
if (role != MastershipRole.MASTER) {
// TODO: Do we need to explicitly tell the Provider that
// this instance is no longer the MASTER? probably not
@@ -405,14 +409,16 @@
// only set the new term if I am the master
deviceClockProviderService.setMastershipTerm(did, term);
- // FIXME: we should check that the device is connected on our end.
- // currently, this is not straight forward as the actual switch
- // implementation is hidden from the registry. Maybe we can ask the
- // provider.
// if the device is null here, we are the first master to claim the
// device. No worries, the DeviceManager will create one soon.
Device device = getDevice(did);
if ((device != null) && !isAvailable(did)) {
+ if (!isReachable(device)) {
+ log.warn("Device {} has disconnected after this event", did);
+ mastershipService.relinquishMastership(did);
+ applyRole(did, MastershipRole.STANDBY);
+ return;
+ }
//flag the device as online. Is there a better way to do this?
DeviceEvent devEvent = store.createOrUpdateDevice(device.providerId(), did,
new DefaultDeviceDescription(
@@ -422,9 +428,11 @@
post(devEvent);
}
applyRole(did, MastershipRole.MASTER);
- // re-collect device information to fix potential staleness
- queryPortInfo(did);
} else if (event.roleInfo().backups().contains(myNodeId)) {
+ if (!isReachable(getDevice(did))) {
+ log.warn("Device {} has disconnected after this event", did);
+ mastershipService.relinquishMastership(did);
+ }
applyRole(did, MastershipRole.STANDBY);
}
}
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 d079378..25fa922 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
@@ -278,6 +278,11 @@
deviceReceived = device;
roleReceived = newRole;
}
+
+ @Override
+ public boolean isReachable(Device device) {
+ return false;
+ }
}
private static class TestListener implements DeviceListener {
diff --git a/core/store/dist/src/main/java/org/onlab/onos/store/mastership/impl/DistributedMastershipStore.java b/core/store/dist/src/main/java/org/onlab/onos/store/mastership/impl/DistributedMastershipStore.java
index 75c0405..d1917d6 100644
--- a/core/store/dist/src/main/java/org/onlab/onos/store/mastership/impl/DistributedMastershipStore.java
+++ b/core/store/dist/src/main/java/org/onlab/onos/store/mastership/impl/DistributedMastershipStore.java
@@ -272,6 +272,10 @@
switch (role) {
case MASTER:
event = reelect(nodeId, deviceId, rv);
+ if (event != null) {
+ Integer term = terms.get(deviceId);
+ terms.put(deviceId, ++term);
+ }
//fall through to reinforce relinquishment
case STANDBY:
//fall through to reinforce relinquishment
@@ -304,15 +308,11 @@
if (backup == null) {
log.info("{} giving up and going to NONE for {}", current, deviceId);
rv.remove(MASTER, current);
- roleMap.put(deviceId, rv);
return null;
} else {
log.info("{} trying to pass mastership for {} to {}", current, deviceId, backup);
rv.replace(current, backup, MASTER);
rv.reassign(backup, STANDBY, NONE);
- roleMap.put(deviceId, rv);
- Integer term = terms.get(deviceId);
- terms.put(deviceId, ++term);
return new MastershipEvent(MASTER_CHANGED, deviceId, rv.roleInfo());
}
}
@@ -366,7 +366,7 @@
@Override
public void entryUpdated(EntryEvent<DeviceId, RoleValue> event) {
-
+ // this subsumes entryAdded event
notifyDelegate(new MastershipEvent(
MASTER_CHANGED, event.getKey(), event.getValue().roleInfo()));
}
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 ecc50a3..6ba0fe3 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
@@ -103,22 +103,31 @@
LOG.info("Stopped");
}
+
+ @Override
+ public boolean isReachable(Device device) {
+ // FIXME if possible, we might want this to be part of
+ // OpenFlowSwitch interface so the driver interface isn't misused.
+ OpenFlowSwitch sw = controller.getSwitch(dpid(device.id().uri()));
+ if (sw == null || !((OpenFlowSwitchDriver) sw).isConnected()) {
+ return false;
+ }
+ return true;
+ //return checkChannel(device, sw);
+ }
+
@Override
public void triggerProbe(Device device) {
LOG.info("Triggering probe on device {}", device.id());
- // 1. check device liveness
- // FIXME if possible, we might want this to be part of
- // OpenFlowSwitch interface so the driver interface isn't misused.
OpenFlowSwitch sw = controller.getSwitch(dpid(device.id().uri()));
- if (sw == null ||
- !((OpenFlowSwitchDriver) sw).isConnected()) {
- LOG.error("Failed to probe device {} on sw={}", device, sw);
- providerService.deviceDisconnected(device.id());
- return;
- }
+ //if (!checkChannel(device, sw)) {
+ // LOG.error("Failed to probe device {} on sw={}", device, sw);
+ // providerService.deviceDisconnected(device.id());
+ //return;
+ //}
- // 2. Prompt an update of port information. Do we have an XID for this?
+ // Prompt an update of port information. We can use any XID for this.
OFFactory fact = sw.factory();
switch (fact.getVersion()) {
case OF_10:
@@ -132,6 +141,16 @@
}
}
+ // Checks if the OF channel is connected.
+ //private boolean checkChannel(Device device, OpenFlowSwitch sw) {
+ // FIXME if possible, we might want this to be part of
+ // OpenFlowSwitch interface so the driver interface isn't misused.
+ // if (sw == null || !((OpenFlowSwitchDriver) sw).isConnected()) {
+ // return false;
+ // }
+ // return true;
+ // }
+
@Override
public void roleChanged(Device device, MastershipRole newRole) {
switch (newRole) {
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 f560c83..08e3546 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
@@ -226,4 +226,9 @@
public ProviderId id() {
return PID;
}
+
+ @Override
+ public boolean isReachable(Device device) {
+ return false;
+ }
}