CORD-180 Changed the way how learnt hosts are suppressed in gerrit 9195
Change-Id: I086ba82147ef716c076cb6140b03da2886515c32
diff --git a/BUCK b/BUCK
index 7cb8d69..f532bea 100644
--- a/BUCK
+++ b/BUCK
@@ -5,7 +5,6 @@
'//cli:onos-cli',
'//core/store/serializers:onos-core-serializers',
'//incubator/api:onos-incubator-api',
- '//providers/netcfghost:onos-providers-netcfghost',
'//utils/rest:onlab-rest',
]
diff --git a/pom.xml b/pom.xml
index a80cbe9..ffc2362 100644
--- a/pom.xml
+++ b/pom.xml
@@ -76,11 +76,6 @@
<version>${project.version}</version>
</dependency>
<dependency>
- <groupId>org.onosproject</groupId>
- <artifactId>onos-netcfg-host-provider</artifactId>
- <version>${project.version}</version>
- </dependency>
- <dependency>
<groupId>javax.ws.rs</groupId>
<artifactId>javax.ws.rs-api</artifactId>
<version>2.0.1</version>
diff --git a/src/main/java/org/onosproject/segmentrouting/HostHandler.java b/src/main/java/org/onosproject/segmentrouting/HostHandler.java
index 132bff5..d5c93fd 100644
--- a/src/main/java/org/onosproject/segmentrouting/HostHandler.java
+++ b/src/main/java/org/onosproject/segmentrouting/HostHandler.java
@@ -38,7 +38,6 @@
import org.onosproject.net.flowobjective.ObjectiveContext;
import org.onosproject.net.host.HostEvent;
import org.onosproject.net.host.HostService;
-import org.onosproject.provider.netcfghost.NetworkConfigHostProvider;
import org.onosproject.segmentrouting.config.SegmentRoutingAppConfig;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -351,16 +350,12 @@
* @return true if segment routing accepts the host
*/
private boolean accepted(Host host) {
- // Always accept configured hosts
- if (host.providerId().equals(NetworkConfigHostProvider.PROVIDER_ID)) {
- return true;
- }
-
SegmentRoutingAppConfig appConfig = srManager.cfgService
.getConfig(srManager.appId, SegmentRoutingAppConfig.class);
- boolean accepted = appConfig != null &&
- appConfig.hostLearning() &&
- !appConfig.suppressHost().contains(host.location());
+
+ boolean accepted = appConfig == null ||
+ (!appConfig.suppressHostByProvider().contains(host.providerId().id()) &&
+ !appConfig.suppressHostByPort().contains(host.location()));
if (!accepted) {
log.info("Ignore suppressed host {}", host.id());
}
diff --git a/src/main/java/org/onosproject/segmentrouting/config/SegmentRoutingAppConfig.java b/src/main/java/org/onosproject/segmentrouting/config/SegmentRoutingAppConfig.java
index 9bbcaa6..0eb2c8e 100644
--- a/src/main/java/org/onosproject/segmentrouting/config/SegmentRoutingAppConfig.java
+++ b/src/main/java/org/onosproject/segmentrouting/config/SegmentRoutingAppConfig.java
@@ -37,15 +37,17 @@
private static final String VROUTER_MACS = "vRouterMacs";
private static final String VROUTER_ID = "vRouterId";
private static final String SUPPRESS_SUBNET = "suppressSubnet";
- private static final String SUPPRESS_HOST = "suppressHost";
- private static final String HOST_LEARNING = "hostLearning";
+ private static final String SUPPRESS_HOST_BY_PORT = "suppressHostByPort";
+ // TODO We might want to move SUPPRESS_HOST_BY_PROVIDER to Component Config
+ private static final String SUPPRESS_HOST_BY_PROVIDER = "suppressHostByProvider";
@Override
public boolean isValid() {
return hasOnlyFields(VROUTER_MACS, VROUTER_ID, SUPPRESS_SUBNET,
- SUPPRESS_HOST, HOST_LEARNING) &&
+ SUPPRESS_HOST_BY_PORT, SUPPRESS_HOST_BY_PROVIDER) &&
vRouterMacs() != null && vRouterId() != null &&
- suppressSubnet() != null && suppressHost() != null;
+ suppressSubnet() != null && suppressHostByPort() != null &&
+ suppressHostByProvider() != null;
}
/**
@@ -181,18 +183,18 @@
}
/**
- * Gets names of ports to which SegmentRouting does not push host rules.
+ * Gets connect points to which SegmentRouting does not push host rules.
*
- * @return Set of port names, empty if not specified, or null
+ * @return Set of connect points, empty if not specified, or null
* if not valid
*/
- public Set<ConnectPoint> suppressHost() {
- if (!object.has(SUPPRESS_HOST)) {
+ public Set<ConnectPoint> suppressHostByPort() {
+ if (!object.has(SUPPRESS_HOST_BY_PORT)) {
return ImmutableSet.of();
}
ImmutableSet.Builder<ConnectPoint> builder = ImmutableSet.builder();
- ArrayNode arrayNode = (ArrayNode) object.path(SUPPRESS_HOST);
+ ArrayNode arrayNode = (ArrayNode) object.path(SUPPRESS_HOST_BY_PORT);
for (JsonNode jsonNode : arrayNode) {
String portName = jsonNode.asText(null);
if (portName == null) {
@@ -208,42 +210,61 @@
}
/**
- * Sets names of ports to which SegmentRouting does not push host rules.
+ * Sets connect points to which SegmentRouting does not push host rules.
*
- * @param suppressHost names of ports to which SegmentRouting does not push
+ * @param connectPoints connect points to which SegmentRouting does not push
* host rules
* @return this {@link SegmentRoutingAppConfig}
*/
- public SegmentRoutingAppConfig setSuppressHost(Set<ConnectPoint> suppressHost) {
- if (suppressHost == null) {
- object.remove(SUPPRESS_HOST);
+ public SegmentRoutingAppConfig setSuppressHostByPort(Set<ConnectPoint> connectPoints) {
+ if (connectPoints == null) {
+ object.remove(SUPPRESS_HOST_BY_PORT);
} else {
ArrayNode arrayNode = mapper.createArrayNode();
- suppressHost.forEach(connectPoint -> {
+ connectPoints.forEach(connectPoint -> {
arrayNode.add(connectPoint.deviceId() + "/" + connectPoint.port());
});
- object.set(SUPPRESS_HOST, arrayNode);
+ object.set(SUPPRESS_HOST_BY_PORT, arrayNode);
}
return this;
}
/**
- * Gets whether host learning is enabled or not.
+ * Gets provider names from which SegmentRouting does not learn host info.
*
- * @return true if enabled. false if disabled or not configured
+ * @return array of provider names that need to be ignored
*/
- public boolean hostLearning() {
- return object.has(HOST_LEARNING) && object.path(HOST_LEARNING).asBoolean();
+ public Set<String> suppressHostByProvider() {
+ if (!object.has(SUPPRESS_HOST_BY_PROVIDER)) {
+ return ImmutableSet.of();
+ }
+
+ ImmutableSet.Builder<String> builder = ImmutableSet.builder();
+ ArrayNode arrayNode = (ArrayNode) object.path(SUPPRESS_HOST_BY_PROVIDER);
+ for (JsonNode jsonNode : arrayNode) {
+ String providerName = jsonNode.asText(null);
+ if (providerName == null) {
+ return null;
+ }
+ builder.add(providerName);
+ }
+ return builder.build();
}
/**
- * Sets whether host learning is enabled or not.
+ * Sets provider names from which SegmentRouting does not learn host info.
*
- * @param enabled true if enabled
+ * @param providers set of provider names
* @return this {@link SegmentRoutingAppConfig}
*/
- public SegmentRoutingAppConfig setHostLearning(boolean enabled) {
- object.put(HOST_LEARNING, enabled);
+ public SegmentRoutingAppConfig setSuppressHostByProvider(Set<String> providers) {
+ if (providers == null) {
+ object.remove(SUPPRESS_HOST_BY_PROVIDER);
+ } else {
+ ArrayNode arrayNode = mapper.createArrayNode();
+ providers.forEach(arrayNode::add);
+ object.set(SUPPRESS_HOST_BY_PROVIDER, arrayNode);
+ }
return this;
}
@@ -253,8 +274,8 @@
.add("vRouterMacs", vRouterMacs())
.add("vRouterId", vRouterId())
.add("suppressSubnet", suppressSubnet())
- .add("suppressHost", suppressHost())
- .add("hostLearning", hostLearning())
+ .add("suppressHostByPort", suppressHostByPort())
+ .add("suppressHostByProvider", suppressHostByProvider())
.toString();
}
}
diff --git a/src/test/java/org/onosproject/segmentrouting/config/SegmentRoutingAppConfigTest.java b/src/test/java/org/onosproject/segmentrouting/config/SegmentRoutingAppConfigTest.java
index d987247..e6f4912 100644
--- a/src/test/java/org/onosproject/segmentrouting/config/SegmentRoutingAppConfigTest.java
+++ b/src/test/java/org/onosproject/segmentrouting/config/SegmentRoutingAppConfigTest.java
@@ -55,6 +55,9 @@
private static final ConnectPoint PORT_3 = ConnectPoint.deviceConnectPoint("of:1/3");
private static final DeviceId VROUTER_ID_1 = DeviceId.deviceId("of:1");
private static final DeviceId VROUTER_ID_2 = DeviceId.deviceId("of:2");
+ private static final String PROVIDER_1 = "org.onosproject.provider.host";
+ private static final String PROVIDER_2 = "org.onosproject.netcfghost";
+ private static final String PROVIDER_3 = "org.onosproject.anotherprovider";
/**
* Initialize test related variables.
@@ -180,55 +183,65 @@
}
/**
- * Tests suppressHost getter.
+ * Tests suppressHostByPort getter.
*
* @throws Exception
*/
@Test
- public void testSuppressHost() throws Exception {
- Set<ConnectPoint> suppressHost = config.suppressHost();
- assertNotNull("suppressHost should not be null", suppressHost);
- assertThat(suppressHost.size(), is(2));
- assertTrue(suppressHost.contains(PORT_1));
- assertTrue(suppressHost.contains(PORT_2));
+ public void testSuppressHostByPort() throws Exception {
+ Set<ConnectPoint> suppressHostByPort = config.suppressHostByPort();
+ assertNotNull("suppressHostByPort should not be null", suppressHostByPort);
+ assertThat(suppressHostByPort.size(), is(2));
+ assertTrue(suppressHostByPort.contains(PORT_1));
+ assertTrue(suppressHostByPort.contains(PORT_2));
}
/**
- * Tests suppressHost setter.
+ * Tests suppressHostByPort setter.
*
* @throws Exception
*/
@Test
- public void testSetSuppressHost() throws Exception {
+ public void testSetSuppressHostByPort() throws Exception {
ImmutableSet.Builder<ConnectPoint> builder = ImmutableSet.builder();
builder.add(PORT_3);
- config.setSuppressHost(builder.build());
+ config.setSuppressHostByPort(builder.build());
- Set<ConnectPoint> suppressHost = config.suppressHost();
- assertNotNull("suppressHost should not be null", suppressHost);
- assertThat(suppressHost.size(), is(1));
- assertTrue(suppressHost.contains(PORT_3));
+ Set<ConnectPoint> suppressHostByPort = config.suppressHostByPort();
+ assertNotNull("suppressHostByPort should not be null", suppressHostByPort);
+ assertThat(suppressHostByPort.size(), is(1));
+ assertTrue(suppressHostByPort.contains(PORT_3));
}
/**
- * Tests hostLearning getter.
+ * Tests suppressHostByProvider getter.
*
* @throws Exception
*/
@Test
- public void testHostLearning() throws Exception {
- assertFalse(config.hostLearning());
+ public void testSuppressHostByProvider() throws Exception {
+ Set<String> supprsuppressHostByProvider = config.suppressHostByProvider();
+ assertNotNull("suppressHostByProvider should not be null", supprsuppressHostByProvider);
+ assertThat(supprsuppressHostByProvider.size(), is(2));
+ assertTrue(supprsuppressHostByProvider.contains(PROVIDER_1));
+ assertTrue(supprsuppressHostByProvider.contains(PROVIDER_2));
}
/**
- * Tests hostLearning setter.
+ * Tests suppressHostByProvider setter.
*
* @throws Exception
*/
@Test
public void testSetHostLearning() throws Exception {
- config.setHostLearning(true);
- assertTrue(config.hostLearning());
+ ImmutableSet.Builder<String> builder = ImmutableSet.builder();
+ builder.add(PROVIDER_3);
+ config.setSuppressHostByProvider(builder.build());
+
+ Set<String> supprsuppressHostByProvider = config.suppressHostByProvider();
+ assertNotNull("suppressHostByProvider should not be null", supprsuppressHostByProvider);
+ assertThat(supprsuppressHostByProvider.size(), is(1));
+ assertTrue(supprsuppressHostByProvider.contains(PROVIDER_3));
}
private class MockDelegate implements ConfigApplyDelegate {
diff --git a/src/test/resources/sr-app-config-invalid.json b/src/test/resources/sr-app-config-invalid.json
index 72def14..89b2286 100644
--- a/src/test/resources/sr-app-config-invalid.json
+++ b/src/test/resources/sr-app-config-invalid.json
@@ -8,9 +8,9 @@
"of:1/1",
"of:1/2"
],
- "suppressHost" : [
+ "suppressHostByPort" : [
"of:1/1",
"wrongPort"
],
- "hostLearning" : false
+ "suppressHostByProvider" : []
}
diff --git a/src/test/resources/sr-app-config.json b/src/test/resources/sr-app-config.json
index de6d7fc..ce5a086 100644
--- a/src/test/resources/sr-app-config.json
+++ b/src/test/resources/sr-app-config.json
@@ -8,9 +8,12 @@
"of:1/1",
"of:1/2"
],
- "suppressHost" : [
+ "suppressHostByPort" : [
"of:1/1",
"of:1/2"
],
- "hostLearning" : false
+ "suppressHostByProvider" : [
+ "org.onosproject.provider.host",
+ "org.onosproject.netcfghost"
+ ]
}