avoid transient changes to MastershipStore from being posted as events
Change-Id: Id033cf50f865e44439138f5b3814ebaedb832b73
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);
}
}