Committed by
Gerrit Code Review
remove dependency on Devices for handling MastershipEvents
Change-Id: Ie1670e77d3eee5dbd597f574ebe16d687e7b551e
Showing
8 changed files
with
18 additions
and
26 deletions
| ... | @@ -358,7 +358,7 @@ public class OpticalConfigProvider extends AbstractProvider implements DevicePro | ... | @@ -358,7 +358,7 @@ public class OpticalConfigProvider extends AbstractProvider implements DevicePro |
| 358 | } | 358 | } |
| 359 | 359 | ||
| 360 | @Override | 360 | @Override |
| 361 | - public void triggerProbe(Device device) { | 361 | + public void triggerProbe(DeviceId deviceId) { |
| 362 | // TODO We may want to consider re-reading config files and publishing them based on this event. | 362 | // TODO We may want to consider re-reading config files and publishing them based on this event. |
| 363 | } | 363 | } |
| 364 | 364 | ... | ... |
| ... | @@ -15,7 +15,6 @@ | ... | @@ -15,7 +15,6 @@ |
| 15 | */ | 15 | */ |
| 16 | package org.onlab.onos.net.device; | 16 | package org.onlab.onos.net.device; |
| 17 | 17 | ||
| 18 | -import org.onlab.onos.net.Device; | ||
| 19 | import org.onlab.onos.net.DeviceId; | 18 | import org.onlab.onos.net.DeviceId; |
| 20 | import org.onlab.onos.net.MastershipRole; | 19 | import org.onlab.onos.net.MastershipRole; |
| 21 | import org.onlab.onos.net.provider.Provider; | 20 | import org.onlab.onos.net.provider.Provider; |
| ... | @@ -35,9 +34,9 @@ public interface DeviceProvider extends Provider { | ... | @@ -35,9 +34,9 @@ public interface DeviceProvider extends Provider { |
| 35 | * {@link org.onlab.onos.net.device.DeviceProviderService#deviceDisconnected} | 34 | * {@link org.onlab.onos.net.device.DeviceProviderService#deviceDisconnected} |
| 36 | * at some later point in time. | 35 | * at some later point in time. |
| 37 | * | 36 | * |
| 38 | - * @param device device to be probed | 37 | + * @param deviceId ID of device to be probed |
| 39 | */ | 38 | */ |
| 40 | - void triggerProbe(Device device); | 39 | + void triggerProbe(DeviceId deviceId); |
| 41 | 40 | ||
| 42 | /** | 41 | /** |
| 43 | * Notifies the provider of a mastership role change for the specified | 42 | * Notifies the provider of a mastership role change for the specified | ... | ... |
| ... | @@ -182,6 +182,9 @@ public class DeviceManager | ... | @@ -182,6 +182,9 @@ public class DeviceManager |
| 182 | 182 | ||
| 183 | // Check a device for control channel connectivity. | 183 | // Check a device for control channel connectivity. |
| 184 | private boolean isReachable(DeviceId deviceId) { | 184 | private boolean isReachable(DeviceId deviceId) { |
| 185 | + if (deviceId == null) { | ||
| 186 | + return false; | ||
| 187 | + } | ||
| 185 | DeviceProvider provider = getProvider(deviceId); | 188 | DeviceProvider provider = getProvider(deviceId); |
| 186 | if (provider != null) { | 189 | if (provider != null) { |
| 187 | return provider.isReachable(deviceId); | 190 | return provider.isReachable(deviceId); |
| ... | @@ -370,7 +373,6 @@ public class DeviceManager | ... | @@ -370,7 +373,6 @@ public class DeviceManager |
| 370 | checkNotNull(portDescriptions, | 373 | checkNotNull(portDescriptions, |
| 371 | "Port descriptions list cannot be null"); | 374 | "Port descriptions list cannot be null"); |
| 372 | checkValidity(); | 375 | checkValidity(); |
| 373 | - | ||
| 374 | if (!deviceClockProviderService.isTimestampAvailable(deviceId)) { | 376 | if (!deviceClockProviderService.isTimestampAvailable(deviceId)) { |
| 375 | // Never been a master for this device | 377 | // Never been a master for this device |
| 376 | // any update will be ignored. | 378 | // any update will be ignored. |
| ... | @@ -475,15 +477,7 @@ public class DeviceManager | ... | @@ -475,15 +477,7 @@ public class DeviceManager |
| 475 | return true; | 477 | return true; |
| 476 | } | 478 | } |
| 477 | 479 | ||
| 478 | - Device device = store.getDevice(deviceId); | 480 | + DeviceProvider provider = getProvider(deviceId); |
| 479 | - // FIXME: Device might not be there yet. (eventual consistent) | ||
| 480 | - // FIXME relinquish role | ||
| 481 | - if (device == null) { | ||
| 482 | - log.warn("{} was not there. Cannot apply role {}", deviceId, newRole); | ||
| 483 | - return false; | ||
| 484 | - } | ||
| 485 | - | ||
| 486 | - DeviceProvider provider = getProvider(device.providerId()); | ||
| 487 | if (provider == null) { | 481 | if (provider == null) { |
| 488 | log.warn("Provider for {} was not found. Cannot apply role {}", deviceId, newRole); | 482 | log.warn("Provider for {} was not found. Cannot apply role {}", deviceId, newRole); |
| 489 | return false; | 483 | return false; |
| ... | @@ -492,10 +486,7 @@ public class DeviceManager | ... | @@ -492,10 +486,7 @@ public class DeviceManager |
| 492 | 486 | ||
| 493 | if (newRole.equals(MastershipRole.MASTER)) { | 487 | if (newRole.equals(MastershipRole.MASTER)) { |
| 494 | // only trigger event when request was sent to provider | 488 | // only trigger event when request was sent to provider |
| 495 | - // TODO: consider removing this from Device event type? | 489 | + provider.triggerProbe(deviceId); |
| 496 | - post(new DeviceEvent(DEVICE_MASTERSHIP_CHANGED, device)); | ||
| 497 | - | ||
| 498 | - provider.triggerProbe(device); | ||
| 499 | } | 490 | } |
| 500 | return true; | 491 | return true; |
| 501 | } | 492 | } | ... | ... |
| ... | @@ -259,7 +259,7 @@ public class DeviceManagerTest { | ... | @@ -259,7 +259,7 @@ public class DeviceManagerTest { |
| 259 | } | 259 | } |
| 260 | 260 | ||
| 261 | @Override | 261 | @Override |
| 262 | - public void triggerProbe(Device device) { | 262 | + public void triggerProbe(DeviceId deviceId) { |
| 263 | } | 263 | } |
| 264 | 264 | ||
| 265 | @Override | 265 | @Override | ... | ... |
| ... | @@ -235,7 +235,10 @@ public class SimpleDeviceStore | ... | @@ -235,7 +235,10 @@ public class SimpleDeviceStore |
| 235 | DeviceId deviceId, | 235 | DeviceId deviceId, |
| 236 | List<PortDescription> portDescriptions) { | 236 | List<PortDescription> portDescriptions) { |
| 237 | Device device = devices.get(deviceId); | 237 | Device device = devices.get(deviceId); |
| 238 | - checkArgument(device != null, DEVICE_NOT_FOUND, deviceId); | 238 | + if (device == null) { |
| 239 | + log.debug("Device {} doesn't exist or hasn't been initialized yet", deviceId); | ||
| 240 | + return Collections.emptyList(); | ||
| 241 | + } | ||
| 239 | 242 | ||
| 240 | Map<ProviderId, DeviceDescriptions> descsMap = deviceDescs.get(deviceId); | 243 | Map<ProviderId, DeviceDescriptions> descsMap = deviceDescs.get(deviceId); |
| 241 | checkArgument(descsMap != null, DEVICE_NOT_FOUND, deviceId); | 244 | checkArgument(descsMap != null, DEVICE_NOT_FOUND, deviceId); | ... | ... |
| ... | @@ -57,7 +57,7 @@ class RoleManager implements RoleHandler { | ... | @@ -57,7 +57,7 @@ class RoleManager implements RoleHandler { |
| 57 | 57 | ||
| 58 | private static Logger log = LoggerFactory.getLogger(RoleManager.class); | 58 | private static Logger log = LoggerFactory.getLogger(RoleManager.class); |
| 59 | 59 | ||
| 60 | - // The time until cached XID is evicted. Arbitray for now. | 60 | + // The time until cached XID is evicted. Arbitrary for now. |
| 61 | private final int pendingXidTimeoutSeconds = 60; | 61 | private final int pendingXidTimeoutSeconds = 60; |
| 62 | 62 | ||
| 63 | // The cache for pending expected RoleReplies keyed on expected XID | 63 | // The cache for pending expected RoleReplies keyed on expected XID | ... | ... |
| ... | @@ -127,17 +127,16 @@ public class OpenFlowDeviceProvider extends AbstractProvider implements DevicePr | ... | @@ -127,17 +127,16 @@ public class OpenFlowDeviceProvider extends AbstractProvider implements DevicePr |
| 127 | } | 127 | } |
| 128 | 128 | ||
| 129 | @Override | 129 | @Override |
| 130 | - public void triggerProbe(Device device) { | 130 | + public void triggerProbe(DeviceId deviceId) { |
| 131 | - final DeviceId deviceId = device.id(); | ||
| 132 | LOG.info("Triggering probe on device {}", deviceId); | 131 | LOG.info("Triggering probe on device {}", deviceId); |
| 133 | 132 | ||
| 134 | final Dpid dpid = dpid(deviceId.uri()); | 133 | final Dpid dpid = dpid(deviceId.uri()); |
| 135 | OpenFlowSwitch sw = controller.getSwitch(dpid); | 134 | OpenFlowSwitch sw = controller.getSwitch(dpid); |
| 136 | if (sw == null || !sw.isConnected()) { | 135 | if (sw == null || !sw.isConnected()) { |
| 137 | - LOG.error("Failed to probe device {} on sw={}", device, sw); | 136 | + LOG.error("Failed to probe device {} on sw={}", deviceId, sw); |
| 138 | providerService.deviceDisconnected(deviceId); | 137 | providerService.deviceDisconnected(deviceId); |
| 139 | } else { | 138 | } else { |
| 140 | - LOG.trace("Confirmed device {} connection", device); | 139 | + LOG.trace("Confirmed device {} connection", deviceId); |
| 141 | // FIXME require something like below to match javadoc description | 140 | // FIXME require something like below to match javadoc description |
| 142 | // but this starts infinite loop with current DeviceManager | 141 | // but this starts infinite loop with current DeviceManager |
| 143 | // final ChassisId cId = new ChassisId(dpid.value()); | 142 | // final ChassisId cId = new ChassisId(dpid.value()); | ... | ... |
| ... | @@ -240,7 +240,7 @@ class ConfigProvider implements DeviceProvider, LinkProvider, HostProvider { | ... | @@ -240,7 +240,7 @@ class ConfigProvider implements DeviceProvider, LinkProvider, HostProvider { |
| 240 | } | 240 | } |
| 241 | 241 | ||
| 242 | @Override | 242 | @Override |
| 243 | - public void triggerProbe(Device device) { | 243 | + public void triggerProbe(DeviceId deviceId) { |
| 244 | } | 244 | } |
| 245 | 245 | ||
| 246 | @Override | 246 | @Override | ... | ... |
-
Please register or login to post a comment