ONOS-2947 Improvements to HostService
- Added ability to replace IPs for existing hosts to HostProviderService
- Moved createOrUpdateHost to use compute() (rather than get/set) in HostStore
Change-Id: I8ac035d010ae65f23158d2237f9fc97c8f811dd4
diff --git a/apps/dhcp/src/test/java/org/onosproject/dhcp/impl/DhcpManagerTest.java b/apps/dhcp/src/test/java/org/onosproject/dhcp/impl/DhcpManagerTest.java
index 682ab91..3ea3b1b 100644
--- a/apps/dhcp/src/test/java/org/onosproject/dhcp/impl/DhcpManagerTest.java
+++ b/apps/dhcp/src/test/java/org/onosproject/dhcp/impl/DhcpManagerTest.java
@@ -330,7 +330,7 @@
}
@Override
- public void hostDetected(HostId hostId, HostDescription hostDescription) {
+ public void hostDetected(HostId hostId, HostDescription hostDescription, boolean replaceIps) {
}
@Override
diff --git a/core/api/src/main/java/org/onosproject/net/host/HostProviderService.java b/core/api/src/main/java/org/onosproject/net/host/HostProviderService.java
index 8678a29..f7b7c49 100644
--- a/core/api/src/main/java/org/onosproject/net/host/HostProviderService.java
+++ b/core/api/src/main/java/org/onosproject/net/host/HostProviderService.java
@@ -30,7 +30,20 @@
* @param hostId id of the host that been detected
* @param hostDescription description of host and its location
*/
- void hostDetected(HostId hostId, HostDescription hostDescription);
+ @Deprecated
+ default void hostDetected(HostId hostId, HostDescription hostDescription) {
+ hostDetected(hostId, hostDescription, false);
+ }
+
+ /**
+ * Notifies the core when a host has been detected on a network along with
+ * information that identifies the host location.
+ *
+ * @param hostId id of the host that been detected
+ * @param hostDescription description of host and its location
+ * @param replaceIps replace IP set if true, merge IP set otherwise
+ */
+ void hostDetected(HostId hostId, HostDescription hostDescription, boolean replaceIps);
/**
* Notifies the core when a host is no longer detected on a network.
diff --git a/core/api/src/main/java/org/onosproject/net/host/HostStore.java b/core/api/src/main/java/org/onosproject/net/host/HostStore.java
index 35a2a8b..5894fe9 100644
--- a/core/api/src/main/java/org/onosproject/net/host/HostStore.java
+++ b/core/api/src/main/java/org/onosproject/net/host/HostStore.java
@@ -39,12 +39,13 @@
* @param providerId provider identification
* @param hostId host identification
* @param hostDescription host description data
+ * @param replaceIps replace IP set if true, merge IP set otherwise
* @return appropriate event or null if no change resulted
*/
HostEvent createOrUpdateHost(ProviderId providerId, HostId hostId,
- HostDescription hostDescription);
+ HostDescription hostDescription,
+ boolean replaceIps);
- // FIXME: API to remove only IpAddress is missing
/**
* Removes the specified host from the inventory.
*
diff --git a/core/common/src/test/java/org/onosproject/store/trivial/SimpleHostStore.java b/core/common/src/test/java/org/onosproject/store/trivial/SimpleHostStore.java
index 7095d7b..264d049 100644
--- a/core/common/src/test/java/org/onosproject/store/trivial/SimpleHostStore.java
+++ b/core/common/src/test/java/org/onosproject/store/trivial/SimpleHostStore.java
@@ -84,12 +84,14 @@
@Override
public HostEvent createOrUpdateHost(ProviderId providerId, HostId hostId,
- HostDescription hostDescription) {
+ HostDescription hostDescription,
+ boolean replaceIps) {
+ //TODO We need a way to detect conflicting changes and abort update.
StoredHost host = hosts.get(hostId);
if (host == null) {
return createHost(providerId, hostId, hostDescription);
}
- return updateHost(providerId, host, hostDescription);
+ return updateHost(providerId, host, hostDescription, replaceIps);
}
// creates a new host and sends HOST_ADDED
@@ -110,7 +112,7 @@
// checks for type of update to host, sends appropriate event
private HostEvent updateHost(ProviderId providerId, StoredHost host,
- HostDescription descr) {
+ HostDescription descr, boolean replaceIps) {
HostEvent event;
if (!host.location().equals(descr.location())) {
host.setLocation(descr.location());
@@ -122,8 +124,14 @@
return null;
}
- Set<IpAddress> addresses = new HashSet<>(host.ipAddresses());
- addresses.addAll(descr.ipAddress());
+ final Set<IpAddress> addresses;
+ if (replaceIps) {
+ addresses = ImmutableSet.copyOf(descr.ipAddress());
+ } else {
+ addresses = new HashSet<>(host.ipAddresses());
+ addresses.addAll(descr.ipAddress());
+ }
+
Annotations annotations = merge((DefaultAnnotations) host.annotations(),
descr.annotations());
StoredHost updated = new StoredHost(providerId, host.id(),
diff --git a/core/net/src/main/java/org/onosproject/net/host/impl/HostManager.java b/core/net/src/main/java/org/onosproject/net/host/impl/HostManager.java
index 9adba01..43f346b 100644
--- a/core/net/src/main/java/org/onosproject/net/host/impl/HostManager.java
+++ b/core/net/src/main/java/org/onosproject/net/host/impl/HostManager.java
@@ -207,12 +207,12 @@
}
@Override
- public void hostDetected(HostId hostId, HostDescription hostDescription) {
+ public void hostDetected(HostId hostId, HostDescription hostDescription, boolean replaceIps) {
checkNotNull(hostId, HOST_ID_NULL);
checkValidity();
hostDescription = validateHost(hostDescription, hostId);
HostEvent event = store.createOrUpdateHost(provider().id(), hostId,
- hostDescription);
+ hostDescription, replaceIps);
if (event != null) {
post(event);
}
diff --git a/core/store/dist/src/main/java/org/onosproject/store/host/impl/ECHostStore.java b/core/store/dist/src/main/java/org/onosproject/store/host/impl/ECHostStore.java
index 46c5fa2..d0b827c 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/host/impl/ECHostStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/host/impl/ECHostStore.java
@@ -16,9 +16,11 @@
package org.onosproject.store.host.impl;
import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
import static org.onosproject.net.DefaultAnnotations.merge;
import static org.onosproject.net.host.HostEvent.Type.HOST_ADDED;
import static org.onosproject.net.host.HostEvent.Type.HOST_REMOVED;
+import static org.onosproject.net.host.HostEvent.Type.HOST_MOVED;
import static org.onosproject.net.host.HostEvent.Type.HOST_UPDATED;
import static org.onosproject.store.service.EventuallyConsistentMapEvent.Type.PUT;
import static org.onosproject.store.service.EventuallyConsistentMapEvent.Type.REMOVE;
@@ -27,6 +29,7 @@
import java.util.Collection;
import java.util.Objects;
import java.util.Set;
+import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Predicate;
import java.util.stream.Collectors;
@@ -47,6 +50,7 @@
import org.onosproject.net.DeviceId;
import org.onosproject.net.Host;
import org.onosproject.net.HostId;
+import org.onosproject.net.HostLocation;
import org.onosproject.net.host.HostDescription;
import org.onosproject.net.host.HostEvent;
import org.onosproject.net.host.HostStore;
@@ -123,22 +127,67 @@
@Override
public HostEvent createOrUpdateHost(ProviderId providerId,
- HostId hostId,
- HostDescription hostDescription) {
- DefaultHost currentHost = hosts.get(hostId);
- if (currentHost == null) {
- DefaultHost newhost = new DefaultHost(
- providerId,
- hostId,
- hostDescription.hwAddress(),
- hostDescription.vlan(),
- hostDescription.location(),
- ImmutableSet.copyOf(hostDescription.ipAddress()),
- hostDescription.annotations());
- hosts.put(hostId, newhost);
- return new HostEvent(HOST_ADDED, newhost);
+ HostId hostId,
+ HostDescription hostDescription,
+ boolean replaceIPs) {
+ // TODO: We need a way to detect conflicting changes and abort update.
+ // (BOC) Compute might do this for us.
+
+ final AtomicReference<Type> eventType = new AtomicReference<>();
+ final AtomicReference<DefaultHost> oldHost = new AtomicReference<>();
+ DefaultHost host = hosts.compute(hostId, (id, existingHost) -> {
+ if (existingHost != null) {
+ oldHost.set(existingHost);
+ checkState(Objects.equals(hostDescription.hwAddress(), existingHost.mac()),
+ "Existing and new MAC addresses differ.");
+ checkState(Objects.equals(hostDescription.vlan(), existingHost.vlan()),
+ "Existing and new VLANs differ.");
+ }
+
+ // TODO do we ever want the existing location?
+ HostLocation location = hostDescription.location();
+
+ final Set<IpAddress> addresses;
+ if (existingHost == null || replaceIPs) {
+ addresses = ImmutableSet.copyOf(hostDescription.ipAddress());
+ } else {
+ addresses = Sets.newHashSet(existingHost.ipAddresses());
+ addresses.addAll(hostDescription.ipAddress());
+ }
+
+ final Annotations annotations;
+ if (existingHost != null) {
+ annotations = merge((DefaultAnnotations) existingHost.annotations(),
+ hostDescription.annotations());
+ } else {
+ annotations = hostDescription.annotations();
+ }
+
+ if (existingHost == null) {
+ eventType.set(HOST_ADDED);
+ } else if (!Objects.equals(existingHost.location(), hostDescription.location())) {
+ eventType.set(HOST_MOVED);
+ } else if (!existingHost.ipAddresses().containsAll(hostDescription.ipAddress()) ||
+ !hostDescription.annotations().keys().isEmpty()) {
+ eventType.set(HOST_UPDATED);
+ } // else, eventType == null; this means we don't send an event
+
+ return new DefaultHost(providerId,
+ hostId,
+ hostDescription.hwAddress(),
+ hostDescription.vlan(),
+ location,
+ addresses,
+ annotations);
+ });
+
+ if (oldHost.get() != null) {
+ DefaultHost old = oldHost.get();
+ locations.remove(old.location(), old);
}
- return updateHost(providerId, hostId, hostDescription, currentHost);
+ locations.put(host.location(), host);
+
+ return eventType.get() != null ? new HostEvent(eventType.get(), host) : null;
}
@Override
@@ -196,39 +245,6 @@
return collection.stream().filter(predicate).collect(Collectors.toSet());
}
- // checks for type of update to host, sends appropriate event
- private HostEvent updateHost(ProviderId providerId,
- HostId hostId,
- HostDescription descr,
- DefaultHost currentHost) {
-
- final boolean hostMoved = !currentHost.location().equals(descr.location());
- if (hostMoved ||
- !currentHost.ipAddresses().containsAll(descr.ipAddress()) ||
- !descr.annotations().keys().isEmpty()) {
-
- Set<IpAddress> addresses = Sets.newHashSet(currentHost.ipAddresses());
- addresses.addAll(descr.ipAddress());
- Annotations annotations = merge((DefaultAnnotations) currentHost.annotations(),
- descr.annotations());
-
- DefaultHost updatedHost = new DefaultHost(providerId, currentHost.id(),
- currentHost.mac(), currentHost.vlan(),
- descr.location(),
- addresses,
- annotations);
-
- // TODO: We need a way to detect conflicting changes and abort update.
- hosts.put(hostId, updatedHost);
- locations.remove(currentHost.location(), currentHost);
- locations.put(updatedHost.location(), updatedHost);
-
- HostEvent.Type eventType = hostMoved ? Type.HOST_MOVED : Type.HOST_UPDATED;
- return new HostEvent(eventType, updatedHost);
- }
- return null;
- }
-
private class HostLocationTracker implements EventuallyConsistentMapListener<HostId, DefaultHost> {
@Override
public void event(EventuallyConsistentMapEvent<HostId, DefaultHost> event) {
diff --git a/providers/host/src/test/java/org/onosproject/provider/host/impl/HostLocationProviderTest.java b/providers/host/src/test/java/org/onosproject/provider/host/impl/HostLocationProviderTest.java
index b9d9097..d010f17 100644
--- a/providers/host/src/test/java/org/onosproject/provider/host/impl/HostLocationProviderTest.java
+++ b/providers/host/src/test/java/org/onosproject/provider/host/impl/HostLocationProviderTest.java
@@ -243,7 +243,7 @@
}
@Override
- public void hostDetected(HostId hostId, HostDescription hostDescription) {
+ public void hostDetected(HostId hostId, HostDescription hostDescription, boolean replaceIps) {
DeviceId descr = hostDescription.location().deviceId();
if (added == null) {
added = descr;
diff --git a/providers/ovsdb/host/src/test/java/org/onosproject/ovsdb/provider/host/OvsdbHostProviderTest.java b/providers/ovsdb/host/src/test/java/org/onosproject/ovsdb/provider/host/OvsdbHostProviderTest.java
index bf18606..ad720c8 100644
--- a/providers/ovsdb/host/src/test/java/org/onosproject/ovsdb/provider/host/OvsdbHostProviderTest.java
+++ b/providers/ovsdb/host/src/test/java/org/onosproject/ovsdb/provider/host/OvsdbHostProviderTest.java
@@ -143,7 +143,7 @@
}
@Override
- public void hostDetected(HostId hostId, HostDescription hostDescription) {
+ public void hostDetected(HostId hostId, HostDescription hostDescription, boolean replaceIps) {
DeviceId descr = hostDescription.location().deviceId();
if (added == null) {
added = descr;