Do not override configured hosts
- When multiple host providers add the same host, the configured one will always win
- A provider can only remove a host provided by itself
Change-Id: I38d95cceed6f36c71b8417ce6812bbd5add10e57
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 3f5d643..8a3cf7d 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
@@ -399,9 +399,6 @@
});
}
-
-
-
// returns a HostDescription made from the union of the BasicHostConfig
// annotations if it exists
private HostDescription validateHost(HostDescription hostDescription, HostId hostId) {
@@ -416,6 +413,12 @@
checkNotNull(hostId, HOST_ID_NULL);
checkValidity();
Host host = store.getHost(hostId);
+
+ // Disallow removing inexistent host or host provided by others
+ if (host == null || !host.providerId().equals(provider().id())) {
+ return;
+ }
+
if (monitorHosts) {
host.ipAddresses().forEach(ip -> {
monitor.stopMonitoring(ip);
diff --git a/core/net/src/test/java/org/onosproject/net/host/impl/HostManagerTest.java b/core/net/src/test/java/org/onosproject/net/host/impl/HostManagerTest.java
index 6a71f43..81bff46 100644
--- a/core/net/src/test/java/org/onosproject/net/host/impl/HostManagerTest.java
+++ b/core/net/src/test/java/org/onosproject/net/host/impl/HostManagerTest.java
@@ -45,6 +45,7 @@
import org.onosproject.net.provider.ProviderId;
import org.onosproject.store.trivial.SimpleHostStore;
+import java.util.Collections;
import java.util.Dictionary;
import java.util.Hashtable;
import java.util.List;
@@ -66,7 +67,8 @@
*/
public class HostManagerTest {
- private static final ProviderId PID = new ProviderId("of", "foo");
+ private static final ProviderId PID = new ProviderId("host", "foo");
+ private static final ProviderId PID2 = new ProviderId("host2", "foo2");
private static final VlanId VLAN1 = VlanId.vlanId((short) 1);
private static final VlanId VLAN2 = VlanId.vlanId((short) 2);
@@ -107,7 +109,9 @@
protected TestListener listener = new TestListener();
protected HostProviderRegistry registry;
protected TestHostProvider provider;
+ protected TestHostProvider provider2;
protected HostProviderService providerService;
+ protected HostProviderService providerService2;
@Before
public void setUp() {
@@ -122,17 +126,24 @@
mgr.addListener(listener);
- provider = new TestHostProvider();
+ provider = new TestHostProvider(PID);
+ provider2 = new TestHostProvider(PID2);
providerService = registry.register(provider);
+ providerService2 = registry.register(provider2);
assertTrue("provider should be registered",
registry.getProviders().contains(provider.id()));
+ assertTrue("provider2 should be registered",
+ registry.getProviders().contains(provider2.id()));
}
@After
public void tearDown() {
registry.unregister(provider);
+ registry.unregister(provider2);
assertFalse("provider should not be registered",
registry.getProviders().contains(provider.id()));
+ assertFalse("provider2 should not be registered",
+ registry.getProviders().contains(provider2.id()));
mgr.removeListener(listener);
mgr.deactivate();
@@ -146,6 +157,13 @@
assertNotNull("host should be found", mgr.getHost(hid));
}
+ private void configured(HostId hid, MacAddress mac, VlanId vlan,
+ HostLocation loc, IpAddress ip) {
+ HostDescription descr = new DefaultHostDescription(mac, vlan, loc, Collections.singleton(ip), true);
+ providerService2.hostDetected(hid, descr, false);
+ assertNotNull("host should be found", mgr.getHost(hid));
+ }
+
private void validateEvents(Enum... types) {
TestTools.assertAfter(100, () -> {
int i = 0;
@@ -202,6 +220,26 @@
assertEquals("only two hosts should be found", 2, mgr.getHostCount());
}
+ /**
+ * If configured host and learnt host are both provided, we should always use
+ * the configured one.
+ */
+ @Test
+ public void hostDetectedWithMultipleProviders() {
+ detect(HID1, MAC1, VLAN1, LOC1, IP1);
+ assertEquals("Expect ProviderId to be PID", PID, mgr.getHost(HID1).providerId());
+ assertTrue("Expect IP to be IP1", mgr.getHost(HID1).ipAddresses().contains(IP1));
+ assertEquals("Expect 1 host in the store", 1, mgr.getHostCount());
+ configured(HID1, MAC1, VLAN1, LOC1, IP2);
+ assertEquals("Expect ProviderId get overridden by PID2", PID2, mgr.getHost(HID1).providerId());
+ assertTrue("Expect IP to be IP2", mgr.getHost(HID1).ipAddresses().contains(IP2));
+ assertEquals("Expect 1 hosts in the store", 1, mgr.getHostCount());
+ detect(HID1, MAC1, VLAN1, LOC1, IP1);
+ assertEquals("Expect ProviderId doesn't get overridden by PID", PID2, mgr.getHost(HID1).providerId());
+ assertTrue("Expect IP to be IP2", mgr.getHost(HID1).ipAddresses().contains(IP2));
+ assertEquals("Expect 1 host in the store", 1, mgr.getHostCount());
+ }
+
@Test
public void hostVanished() {
detect(HID1, MAC1, VLAN1, LOC1, IP1);
@@ -220,6 +258,27 @@
assertNull("host should have been removed", mgr.getHost(HID3));
}
+ /**
+ * Providers should only be able to remove a host provided by itself.
+ */
+ @Test
+ public void hostVanishedWithMultipleProviders() {
+ detect(HID1, MAC1, VLAN1, LOC1, IP1);
+ configured(HID2, MAC2, VLAN2, LOC2, IP2);
+
+ providerService2.hostVanished(HID1);
+ assertNotNull("host should not be removed", mgr.getHost(HID1));
+
+ providerService.hostVanished(HID2);
+ assertNotNull("host should not be removed", mgr.getHost(HID2));
+
+ providerService.hostVanished(HID1);
+ assertNull("host should be removed", mgr.getHost(HID1));
+
+ providerService2.hostVanished(HID2);
+ assertNull("host should be removed", mgr.getHost(HID2));
+ }
+
private void validateHosts(
String msg, Iterable<Host> hosts, HostId... ids) {
Set<HostId> hids = Sets.newHashSet(ids);
@@ -258,13 +317,8 @@
private static class TestHostProvider extends AbstractProvider
implements HostProvider {
- protected TestHostProvider() {
- super(PID);
- }
-
- @Override
- public ProviderId id() {
- return PID;
+ protected TestHostProvider(ProviderId pid) {
+ super(pid);
}
@Override
diff --git a/core/store/dist/src/main/java/org/onosproject/store/host/impl/DistributedHostStore.java b/core/store/dist/src/main/java/org/onosproject/store/host/impl/DistributedHostStore.java
index 356df9c..bfb60c7 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/host/impl/DistributedHostStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/host/impl/DistributedHostStore.java
@@ -119,6 +119,11 @@
return true;
}
+ // Avoid overriding configured hosts
+ if (existingHost.configured()) {
+ return false;
+ }
+
if (!Objects.equals(existingHost.providerId(), providerId) ||
!Objects.equals(existingHost.mac(), hostDescription.hwAddress()) ||
!Objects.equals(existingHost.vlan(), hostDescription.vlan()) ||