[ONOS-7573][Test successful] Mark online does not notify peers device subsystem
Change-Id: Ie347241325047cb65c836c849bec6aebad758820
diff --git a/core/api/src/main/java/org/onosproject/net/device/DeviceStore.java b/core/api/src/main/java/org/onosproject/net/device/DeviceStore.java
index 54bdf17..bd59838 100644
--- a/core/api/src/main/java/org/onosproject/net/device/DeviceStore.java
+++ b/core/api/src/main/java/org/onosproject/net/device/DeviceStore.java
@@ -93,9 +93,9 @@
* Marks the device as available.
*
* @param deviceId device identifier
- * @return true if availability change request was accepted and changed the state
+ * @return ready to send event describing what occurred; null if no change
*/
- boolean markOnline(DeviceId deviceId);
+ DeviceEvent markOnline(DeviceId deviceId);
/**
* Updates the ports of the specified infrastructure device using the given
diff --git a/core/api/src/test/java/org/onosproject/net/device/DeviceStoreAdapter.java b/core/api/src/test/java/org/onosproject/net/device/DeviceStoreAdapter.java
index a5a2a90..63971b9 100644
--- a/core/api/src/test/java/org/onosproject/net/device/DeviceStoreAdapter.java
+++ b/core/api/src/test/java/org/onosproject/net/device/DeviceStoreAdapter.java
@@ -62,8 +62,8 @@
}
@Override
- public boolean markOnline(DeviceId deviceId) {
- return false;
+ public DeviceEvent markOnline(DeviceId deviceId) {
+ return null;
}
@Override
diff --git a/core/common/src/test/java/org/onosproject/store/trivial/SimpleDeviceStore.java b/core/common/src/test/java/org/onosproject/store/trivial/SimpleDeviceStore.java
index f4a7836..645b287 100644
--- a/core/common/src/test/java/org/onosproject/store/trivial/SimpleDeviceStore.java
+++ b/core/common/src/test/java/org/onosproject/store/trivial/SimpleDeviceStore.java
@@ -250,9 +250,9 @@
// implement differently if desired
@Override
- public boolean markOnline(DeviceId deviceId) {
+ public DeviceEvent markOnline(DeviceId deviceId) {
log.warn("Mark online not supported");
- return false;
+ return null;
}
@Override
diff --git a/core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java b/core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java
index 8a0e673..e103b7f 100644
--- a/core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java
+++ b/core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java
@@ -878,7 +878,7 @@
case MASTER:
final Device device = getDevice(did);
if ((device != null) && !isAvailable(did)) {
- store.markOnline(did);
+ post(store.markOnline(did));
}
// TODO: should apply role only if there is mismatch
log.debug("Applying role {} to {}", myNextRole, did);
diff --git a/core/store/dist/src/main/java/org/onosproject/store/device/impl/ECDeviceStore.java b/core/store/dist/src/main/java/org/onosproject/store/device/impl/ECDeviceStore.java
index 680f0a0..177c41ed 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/device/impl/ECDeviceStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/device/impl/ECDeviceStore.java
@@ -363,12 +363,14 @@
}
// FIXME publicization of markOnline -- trigger some action independently?
- public boolean markOnline(DeviceId deviceId) {
+ public DeviceEvent markOnline(DeviceId deviceId) {
if (devices.containsKey(deviceId)) {
- return availableDevices.add(deviceId);
+ if (availableDevices.add(deviceId)) {
+ return new DeviceEvent(DEVICE_AVAILABILITY_CHANGED, devices.get(deviceId), null);
+ }
}
log.warn("Device {} does not exist in store", deviceId);
- return false;
+ return null;
}
@Override
diff --git a/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStore.java b/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStore.java
index 818f0583..10f8f3aa 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStore.java
@@ -106,7 +106,7 @@
import static org.onosproject.net.device.DeviceEvent.Type.PORT_STATS_UPDATED;
import static org.onosproject.net.device.DeviceEvent.Type.PORT_UPDATED;
import static org.onosproject.store.device.impl.GossipDeviceStoreMessageSubjects.DEVICE_ADVERTISE;
-import static org.onosproject.store.device.impl.GossipDeviceStoreMessageSubjects.DEVICE_OFFLINE;
+import static org.onosproject.store.device.impl.GossipDeviceStoreMessageSubjects.DEVICE_STATUS_CHANGE;
import static org.onosproject.store.device.impl.GossipDeviceStoreMessageSubjects.DEVICE_REMOVED;
import static org.onosproject.store.device.impl.GossipDeviceStoreMessageSubjects.DEVICE_REMOVE_REQ;
import static org.onosproject.store.device.impl.GossipDeviceStoreMessageSubjects.DEVICE_UPDATE;
@@ -176,7 +176,7 @@
.register(DistributedStoreSerializers.STORE_COMMON)
.nextId(DistributedStoreSerializers.STORE_CUSTOM_BEGIN)
.register(new InternalDeviceEventSerializer(), InternalDeviceEvent.class)
- .register(new InternalDeviceOfflineEventSerializer(), InternalDeviceOfflineEvent.class)
+ .register(new InternalDeviceStatusChangeEventSerializer(), InternalDeviceStatusChangeEvent.class)
.register(InternalDeviceRemovedEvent.class)
.register(new InternalPortEventSerializer(), InternalPortEvent.class)
.register(new InternalPortStatusEventSerializer(), InternalPortStatusEvent.class)
@@ -201,7 +201,7 @@
newSingleThreadScheduledExecutor(minPriority(groupedThreads("onos/device", "bg-%d", log)));
addSubscriber(DEVICE_UPDATE, this::handleDeviceEvent);
- addSubscriber(DEVICE_OFFLINE, this::handleDeviceOfflineEvent);
+ addSubscriber(DEVICE_STATUS_CHANGE, this::handleDeviceStatusChangeEvent);
addSubscriber(DEVICE_REMOVE_REQ, this::handleRemoveRequest);
addSubscriber(DEVICE_REMOVED, this::handleDeviceRemovedEvent);
addSubscriber(PORT_UPDATE, this::handlePortEvent);
@@ -263,7 +263,7 @@
devicePorts.clear();
availableDevices.clear();
clusterCommunicator.removeSubscriber(DEVICE_UPDATE);
- clusterCommunicator.removeSubscriber(DEVICE_OFFLINE);
+ clusterCommunicator.removeSubscriber(DEVICE_STATUS_CHANGE);
clusterCommunicator.removeSubscriber(DEVICE_REMOVE_REQ);
clusterCommunicator.removeSubscriber(DEVICE_REMOVED);
clusterCommunicator.removeSubscriber(PORT_UPDATE);
@@ -400,7 +400,7 @@
providerId, oldDevice, newDevice);
if (!providerId.isAncillary()) {
- markOnline(newDevice.id(), timestamp);
+ markOnline(newDevice.id(), timestamp, false);
}
log.debug("Device {} added", newDevice.id());
@@ -439,11 +439,7 @@
}
if (!providerId.isAncillary() && forceAvailable) {
- boolean wasOnline = availableDevices.contains(newDevice.id());
- markOnline(newDevice.id(), newTimestamp);
- if (!wasOnline) {
- notifyDelegateIfNotNull(new DeviceEvent(DEVICE_AVAILABILITY_CHANGED, newDevice, null));
- }
+ notifyDelegateIfNotNull(markOnline(newDevice.id(), newTimestamp, false));
}
return event;
}
@@ -472,7 +468,7 @@
if (event != null) {
log.debug("Notifying peers of a device offline topology event for deviceId: {} {}",
deviceId, timestamp);
- notifyPeers(new InternalDeviceOfflineEvent(deviceId, timestamp));
+ notifyPeers(new InternalDeviceStatusChangeEvent(deviceId, timestamp, false));
}
return event;
}
@@ -512,22 +508,8 @@
}
@Override
- public boolean markOnline(DeviceId deviceId) {
- if (devices.containsKey(deviceId)) {
- final Timestamp timestamp = deviceClockService.getTimestamp(deviceId);
- Map<?, ?> deviceLock = getOrCreateDeviceDescriptionsMap(deviceId);
- synchronized (deviceLock) {
- if (markOnline(deviceId, timestamp)) {
- notifyDelegate(new DeviceEvent(DEVICE_AVAILABILITY_CHANGED, getDevice(deviceId), null));
- return true;
- } else {
- return false;
- }
- }
- }
- log.warn("Device {} does not exist in store", deviceId);
- return false;
-
+ public DeviceEvent markOnline(DeviceId deviceId) {
+ return markOnline(deviceId, deviceClockService.getTimestamp(deviceId), true);
}
/**
@@ -536,20 +518,40 @@
*
* @param deviceId identifier of the device
* @param timestamp of the event triggering this change.
- * @return true if availability change request was accepted and changed the state
+ * @param notifyPeers if the event needs to be notified to peers.
+ * @return ready to send event describing what occurred; null if no change
*/
- // Guarded by deviceDescs value (=Device lock)
- private boolean markOnline(DeviceId deviceId, Timestamp timestamp) {
- // accept on-line if given timestamp is newer than
- // the latest offline request Timestamp
- Timestamp offlineTimestamp = offline.get(deviceId);
- if (offlineTimestamp == null ||
- offlineTimestamp.compareTo(timestamp) < 0) {
-
- offline.remove(deviceId);
- return availableDevices.add(deviceId);
+ private DeviceEvent markOnline(DeviceId deviceId, Timestamp timestamp, boolean notifyPeers) {
+ final DeviceEvent event = markOnlineInternal(deviceId, timestamp);
+ if (event != null && notifyPeers) {
+ log.debug("Notifying peers of a device online topology event for deviceId: {} {}",
+ deviceId, timestamp);
+ notifyPeers(new InternalDeviceStatusChangeEvent(deviceId, timestamp, true));
}
- return false;
+ return event;
+ }
+
+ // Guarded by deviceDescs value (=Device lock)
+ private DeviceEvent markOnlineInternal(DeviceId deviceId, Timestamp timestamp) {
+ if (devices.containsKey(deviceId)) {
+ Map<?, ?> deviceLock = getOrCreateDeviceDescriptionsMap(deviceId);
+ synchronized (deviceLock) {
+ // accept on-line if given timestamp is newer than
+ // the latest offline request Timestamp
+ Timestamp offlineTimestamp = offline.get(deviceId);
+ if (offlineTimestamp == null ||
+ offlineTimestamp.compareTo(timestamp) < 0) {
+ offline.remove(deviceId);
+ Device device = devices.get(deviceId);
+ boolean add = availableDevices.add(deviceId);
+ if (add) {
+ return new DeviceEvent(DEVICE_AVAILABILITY_CHANGED, device, null);
+ }
+ }
+ }
+ }
+ log.warn("Device {} does not exist in store", deviceId);
+ return null;
}
@Override
@@ -1271,8 +1273,8 @@
broadcastMessage(DEVICE_UPDATE, event);
}
- private void notifyPeers(InternalDeviceOfflineEvent event) {
- broadcastMessage(GossipDeviceStoreMessageSubjects.DEVICE_OFFLINE, event);
+ private void notifyPeers(InternalDeviceStatusChangeEvent event) {
+ broadcastMessage(GossipDeviceStoreMessageSubjects.DEVICE_STATUS_CHANGE, event);
}
private void notifyPeers(InternalDeviceRemovedEvent event) {
@@ -1295,9 +1297,9 @@
}
}
- private void notifyPeer(NodeId recipient, InternalDeviceOfflineEvent event) {
+ private void notifyPeer(NodeId recipient, InternalDeviceStatusChangeEvent event) {
try {
- unicastMessage(recipient, GossipDeviceStoreMessageSubjects.DEVICE_OFFLINE, event);
+ unicastMessage(recipient, GossipDeviceStoreMessageSubjects.DEVICE_STATUS_CHANGE, event);
} catch (IOException e) {
log.error("Failed to send" + event + " to " + recipient, e);
}
@@ -1466,7 +1468,7 @@
Timestamp lOffline = offline.get(deviceId);
if (lOffline != null && rOffline == null) {
// locally offline, but remote is online, suggest offline
- notifyPeer(sender, new InternalDeviceOfflineEvent(deviceId, lOffline));
+ notifyPeer(sender, new InternalDeviceStatusChangeEvent(deviceId, lOffline, false));
}
// remove device offline Ad already processed
@@ -1575,14 +1577,19 @@
}
}
- private void handleDeviceOfflineEvent(InternalDeviceOfflineEvent event) {
+ private void handleDeviceStatusChangeEvent(InternalDeviceStatusChangeEvent event) {
DeviceId deviceId = event.deviceId();
Timestamp timestamp = event.timestamp();
+ Boolean available = event.available();
try {
- notifyDelegateIfNotNull(markOfflineInternal(deviceId, timestamp));
+ if (available) {
+ notifyDelegateIfNotNull(markOnlineInternal(deviceId, timestamp));
+ } else {
+ notifyDelegateIfNotNull(markOfflineInternal(deviceId, timestamp));
+ }
} catch (Exception e) {
- log.warn("Exception thrown handling device offline", e);
+ log.warn("Exception thrown handling device status change event", e);
}
}
diff --git a/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStoreMessageSubjects.java b/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStoreMessageSubjects.java
index 38ab9be..d970061 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStoreMessageSubjects.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStoreMessageSubjects.java
@@ -25,7 +25,7 @@
private GossipDeviceStoreMessageSubjects() {}
public static final MessageSubject DEVICE_UPDATE = new MessageSubject("peer-device-update");
- public static final MessageSubject DEVICE_OFFLINE = new MessageSubject("peer-device-offline");
+ public static final MessageSubject DEVICE_STATUS_CHANGE = new MessageSubject("peer-device-status-change");
public static final MessageSubject DEVICE_REMOVE_REQ = new MessageSubject("peer-device-remove-request");
public static final MessageSubject DEVICE_REMOVED = new MessageSubject("peer-device-removed");
public static final MessageSubject PORT_UPDATE = new MessageSubject("peer-port-update");
diff --git a/core/store/dist/src/main/java/org/onosproject/store/device/impl/InternalDeviceOfflineEvent.java b/core/store/dist/src/main/java/org/onosproject/store/device/impl/InternalDeviceStatusChangeEvent.java
similarity index 70%
rename from core/store/dist/src/main/java/org/onosproject/store/device/impl/InternalDeviceOfflineEvent.java
rename to core/store/dist/src/main/java/org/onosproject/store/device/impl/InternalDeviceStatusChangeEvent.java
index 028e43d..6763615 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/device/impl/InternalDeviceOfflineEvent.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/device/impl/InternalDeviceStatusChangeEvent.java
@@ -24,19 +24,22 @@
* Information published by GossipDeviceStore to notify peers of a device
* going offline.
*/
-public class InternalDeviceOfflineEvent {
+public class InternalDeviceStatusChangeEvent {
private final DeviceId deviceId;
private final Timestamp timestamp;
+ private final Boolean available;
/**
- * Creates a InternalDeviceOfflineEvent.
- * @param deviceId identifier of device going offline.
- * @param timestamp timestamp of when the device went offline.
+ * Creates a InternalDeviceStatusChangeEvent.
+ * @param deviceId identifier of device.
+ * @param timestamp timestamp of when the device status is changed.
+ * @param available device status is available or not.
*/
- public InternalDeviceOfflineEvent(DeviceId deviceId, Timestamp timestamp) {
+ public InternalDeviceStatusChangeEvent(DeviceId deviceId, Timestamp timestamp, Boolean available) {
this.deviceId = deviceId;
this.timestamp = timestamp;
+ this.available = available;
}
public DeviceId deviceId() {
@@ -47,18 +50,24 @@
return timestamp;
}
+ public Boolean available() {
+ return available;
+ }
+
@Override
public String toString() {
return MoreObjects.toStringHelper(getClass())
.add("deviceId", deviceId)
.add("timestamp", timestamp)
+ .add("available", available)
.toString();
}
// for serializer
@SuppressWarnings("unused")
- private InternalDeviceOfflineEvent() {
+ private InternalDeviceStatusChangeEvent() {
deviceId = null;
timestamp = null;
+ available = null;
}
}
diff --git a/core/store/dist/src/main/java/org/onosproject/store/device/impl/InternalDeviceOfflineEventSerializer.java b/core/store/dist/src/main/java/org/onosproject/store/device/impl/InternalDeviceStatusChangeEventSerializer.java
similarity index 64%
rename from core/store/dist/src/main/java/org/onosproject/store/device/impl/InternalDeviceOfflineEventSerializer.java
rename to core/store/dist/src/main/java/org/onosproject/store/device/impl/InternalDeviceStatusChangeEventSerializer.java
index 7c35587..b97d573 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/device/impl/InternalDeviceOfflineEventSerializer.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/device/impl/InternalDeviceStatusChangeEventSerializer.java
@@ -25,30 +25,32 @@
import com.esotericsoftware.kryo.io.Output;
/**
- * Kryo Serializer for {@link InternalDeviceOfflineEvent}.
+ * Kryo Serializer for {@link InternalDeviceStatusChangeEvent}.
*/
-public class InternalDeviceOfflineEventSerializer extends Serializer<InternalDeviceOfflineEvent> {
+public class InternalDeviceStatusChangeEventSerializer extends Serializer<InternalDeviceStatusChangeEvent> {
/**
- * Creates a serializer for {@link InternalDeviceOfflineEvent}.
+ * Creates a serializer for {@link InternalDeviceStatusChangeEvent}.
*/
- public InternalDeviceOfflineEventSerializer() {
+ public InternalDeviceStatusChangeEventSerializer() {
// does not accept null
super(false);
}
@Override
- public void write(Kryo kryo, Output output, InternalDeviceOfflineEvent event) {
+ public void write(Kryo kryo, Output output, InternalDeviceStatusChangeEvent event) {
kryo.writeObject(output, event.deviceId(), deviceIdSerializer());
kryo.writeClassAndObject(output, event.timestamp());
+ kryo.writeClassAndObject(output, event.available());
}
@Override
- public InternalDeviceOfflineEvent read(Kryo kryo, Input input,
- Class<InternalDeviceOfflineEvent> type) {
+ public InternalDeviceStatusChangeEvent read(Kryo kryo, Input input,
+ Class<InternalDeviceStatusChangeEvent> type) {
DeviceId deviceId = kryo.readObject(input, DeviceId.class, deviceIdSerializer());
Timestamp timestamp = (Timestamp) kryo.readClassAndObject(input);
+ Boolean available = (Boolean) kryo.readClassAndObject(input);
- return new InternalDeviceOfflineEvent(deviceId, timestamp);
+ return new InternalDeviceStatusChangeEvent(deviceId, timestamp, available);
}
}