CORD-352 Refactoring SegmentRoutingConfig
- Add Javadoc and fix function name convention
- Add setAdjancencySids method
- Change return value of getAdjacencySids from List to ImmutableSet
- Validate config value
- Add unit test for SegmentRoutingConfig
Change-Id: Ic43ac31a49da8a9d62131d7803930280cf9994d2
diff --git a/pom.xml b/pom.xml
index 83ae76d..d170a7a 100644
--- a/pom.xml
+++ b/pom.xml
@@ -89,6 +89,11 @@
<groupId>org.osgi</groupId>
<artifactId>org.osgi.core</artifactId>
</dependency>
+ <dependency>
+ <groupId>org.onosproject</groupId>
+ <artifactId>onlab-junit</artifactId>
+ <scope>test</scope>
+ </dependency>
</dependencies>
<build>
diff --git a/src/main/java/org/onosproject/segmentrouting/TunnelHandler.java b/src/main/java/org/onosproject/segmentrouting/TunnelHandler.java
index b86adad..5a82e71 100644
--- a/src/main/java/org/onosproject/segmentrouting/TunnelHandler.java
+++ b/src/main/java/org/onosproject/segmentrouting/TunnelHandler.java
@@ -158,7 +158,7 @@
private int createGroupsForTunnel(Tunnel tunnel) {
- List<Integer> portNumbers;
+ Set<Integer> portNumbers;
final int groupError = -1;
DeviceId deviceId = config.getDeviceId(tunnel.labelIds().get(0));
diff --git a/src/main/java/org/onosproject/segmentrouting/config/DeviceConfiguration.java b/src/main/java/org/onosproject/segmentrouting/config/DeviceConfiguration.java
index 0ad0067..dbac596 100644
--- a/src/main/java/org/onosproject/segmentrouting/config/DeviceConfiguration.java
+++ b/src/main/java/org/onosproject/segmentrouting/config/DeviceConfiguration.java
@@ -16,7 +16,6 @@
package org.onosproject.segmentrouting.config;
import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Lists;
import org.onlab.packet.Ip4Address;
import org.onlab.packet.Ip4Prefix;
import org.onlab.packet.MacAddress;
@@ -26,7 +25,6 @@
import org.onosproject.net.ConnectPoint;
import org.onosproject.net.config.NetworkConfigRegistry;
import org.onosproject.net.host.InterfaceIpAddress;
-import org.onosproject.segmentrouting.config.SegmentRoutingConfig.AdjacencySid;
import org.onosproject.net.DeviceId;
import org.onosproject.net.PortNumber;
import org.slf4j.Logger;
@@ -34,6 +32,7 @@
import java.util.ArrayList;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -60,7 +59,7 @@
boolean isEdge;
HashMap<PortNumber, Ip4Address> gatewayIps;
HashMap<PortNumber, Ip4Prefix> subnets;
- List<AdjacencySid> adjacencySids;
+ Map<Integer, Set<Integer>> adjacencySids;
public SegmentRouterInfo() {
this.gatewayIps = new HashMap<>();
@@ -83,11 +82,11 @@
cfgService.getConfig(subject, SegmentRoutingConfig.class);
SegmentRouterInfo info = new SegmentRouterInfo();
info.deviceId = subject;
- info.nodeSid = config.getSid();
- info.ip = config.getIp();
- info.mac = config.getMac();
+ info.nodeSid = config.nodeSid();
+ info.ip = config.routerIp();
+ info.mac = config.routerMac();
info.isEdge = config.isEdgeRouter();
- info.adjacencySids = config.getAdjacencySids();
+ info.adjacencySids = config.adjacencySids();
this.deviceConfigMap.put(info.deviceId, info);
this.allSegmentIds.add(info.nodeSid);
@@ -410,19 +409,13 @@
*
* @param deviceId device identification of the router
* @param sid adjacency Sid
- * @return list of port numbers
+ * @return set of port numbers
*/
- public List<Integer> getPortsForAdjacencySid(DeviceId deviceId, int sid) {
+ public Set<Integer> getPortsForAdjacencySid(DeviceId deviceId, int sid) {
SegmentRouterInfo srinfo = deviceConfigMap.get(deviceId);
- if (srinfo != null) {
- for (AdjacencySid asid : srinfo.adjacencySids) {
- if (asid.getAsid() == sid) {
- return asid.getPorts();
- }
- }
- }
-
- return Lists.newArrayList();
+ return srinfo != null ?
+ ImmutableSet.copyOf(srinfo.adjacencySids.get(sid)) :
+ ImmutableSet.copyOf(new HashSet<>());
}
/**
@@ -435,20 +428,6 @@
*/
public boolean isAdjacencySid(DeviceId deviceId, int sid) {
SegmentRouterInfo srinfo = deviceConfigMap.get(deviceId);
- if (srinfo != null) {
- if (srinfo.adjacencySids.isEmpty()) {
- return false;
- } else {
- for (AdjacencySid asid:
- srinfo.adjacencySids) {
- if (asid.getAsid() == sid) {
- return true;
- }
- }
- return false;
- }
- }
-
- return false;
+ return srinfo != null && srinfo.adjacencySids.containsKey(sid);
}
}
\ No newline at end of file
diff --git a/src/main/java/org/onosproject/segmentrouting/config/SegmentRoutingConfig.java b/src/main/java/org/onosproject/segmentrouting/config/SegmentRoutingConfig.java
index 6dc3f0d..f788925 100644
--- a/src/main/java/org/onosproject/segmentrouting/config/SegmentRoutingConfig.java
+++ b/src/main/java/org/onosproject/segmentrouting/config/SegmentRoutingConfig.java
@@ -16,113 +16,210 @@
package org.onosproject.segmentrouting.config;
+import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.ArrayNode;
+import com.fasterxml.jackson.databind.node.ObjectNode;
+import com.google.common.collect.ImmutableMap;
import org.onlab.packet.Ip4Address;
import org.onlab.packet.MacAddress;
import org.onosproject.net.DeviceId;
import org.onosproject.net.config.Config;
-import org.onosproject.net.config.basics.BasicElementConfig;
-import java.util.ArrayList;
-import java.util.List;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
import java.util.Optional;
+import java.util.Set;
/**
* Configuration object for Segment Routing Application.
*/
public class SegmentRoutingConfig extends Config<DeviceId> {
- private static final String NAME = "name";
- private static final String IP = "routerIp";
- private static final String MAC = "routerMac";
- private static final String SID = "nodeSid";
- private static final String EDGE = "isEdgeRouter";
- private static final String ADJSID = "adjacencySids";
+ public static final String NAME = "name";
+ public static final String IP = "routerIp";
+ public static final String MAC = "routerMac";
+ public static final String SID = "nodeSid";
+ public static final String EDGE = "isEdgeRouter";
+ public static final String ADJSIDS = "adjacencySids";
+ public static final String ADJSID = "adjSid";
+ public static final String PORTS = "ports";
- public Optional<String> getName() {
+ @Override
+ public boolean isValid() {
+ return hasOnlyFields(NAME, IP, MAC, SID, EDGE, ADJSIDS, ADJSID, PORTS) &&
+ this.name() != null &&
+ this.routerIp() != null &&
+ this.routerMac() != null &&
+ this.nodeSid() != -1 &&
+ this.isEdgeRouter() != null &&
+ this.adjacencySids() != null;
+ }
+
+ /**
+ * Gets the name of the router.
+ *
+ * @return Optional name of the router. May be empty if not configured.
+ */
+ public Optional<String> name() {
String name = get(NAME, null);
return name != null ? Optional.of(name) : Optional.empty();
}
- public BasicElementConfig setName(String name) {
- return (BasicElementConfig) setOrClear(NAME, name);
+ /**
+ * Sets the name of the router.
+ *
+ * @param name name of the router.
+ * @return the config of the router.
+ */
+ public SegmentRoutingConfig setName(String name) {
+ return (SegmentRoutingConfig) setOrClear(NAME, name);
}
- public Ip4Address getIp() {
+ /**
+ * Gets the IP address of the router.
+ *
+ * @return IP address of the router. Or null if not configured.
+ */
+ public Ip4Address routerIp() {
String ip = get(IP, null);
return ip != null ? Ip4Address.valueOf(ip) : null;
}
- public BasicElementConfig setIp(String ip) {
- return (BasicElementConfig) setOrClear(IP, ip);
+ /**
+ * Sets the IP address of the router.
+ *
+ * @param ip IP address of the router.
+ * @return the config of the router.
+ */
+ public SegmentRoutingConfig setRouterIp(String ip) {
+ return (SegmentRoutingConfig) setOrClear(IP, ip);
}
- public MacAddress getMac() {
+ /**
+ * Gets the MAC address of the router.
+ *
+ * @return MAC address of the router. Or null if not configured.
+ */
+ public MacAddress routerMac() {
String mac = get(MAC, null);
return mac != null ? MacAddress.valueOf(mac) : null;
}
- public BasicElementConfig setMac(String mac) {
- return (BasicElementConfig) setOrClear(MAC, mac);
+ /**
+ * Sets the MAC address of the router.
+ *
+ * @param mac MAC address of the router.
+ * @return the config of the router.
+ */
+ public SegmentRoutingConfig setRouterMac(String mac) {
+ return (SegmentRoutingConfig) setOrClear(MAC, mac);
}
- public int getSid() {
+ /**
+ * Gets the node SID of the router.
+ *
+ * @return node SID of the router. Or -1 if not configured.
+ */
+ public int nodeSid() {
return get(SID, -1);
}
- public BasicElementConfig setSid(int sid) {
- return (BasicElementConfig) setOrClear(SID, sid);
+ /**
+ * Sets the node SID of the router.
+ *
+ * @param sid node SID of the router.
+ * @return the config of the router.
+ */
+ public SegmentRoutingConfig setNodeSid(int sid) {
+ return (SegmentRoutingConfig) setOrClear(SID, sid);
}
- public boolean isEdgeRouter() {
- return get(EDGE, false);
+ /**
+ * Checks if the router is an edge router.
+ *
+ * @return true if the router is an edge router.
+ * false if the router is not an edge router.
+ * null if the value is not configured.
+ */
+ public Boolean isEdgeRouter() {
+ String isEdgeRouter = get(EDGE, null);
+ return isEdgeRouter != null ?
+ Boolean.valueOf(isEdgeRouter) :
+ null;
}
- public BasicElementConfig setEdgeRouter(boolean isEdgeRouter) {
- return (BasicElementConfig) setOrClear(EDGE, isEdgeRouter);
+ /**
+ * Specifies if the router is an edge router.
+ *
+ * @param isEdgeRouter true if the router is an edge router.
+ * @return the config of the router.
+ */
+ public SegmentRoutingConfig setIsEdgeRouter(boolean isEdgeRouter) {
+ return (SegmentRoutingConfig) setOrClear(EDGE, isEdgeRouter);
}
- public List<AdjacencySid> getAdjacencySids() {
- ArrayList<AdjacencySid> adjacencySids = new ArrayList<>();
-
- if (!object.has(ADJSID)) {
- return adjacencySids;
+ /**
+ * Gets the adjacency SIDs of the router.
+ *
+ * @return adjacency SIDs of the router. Or null if not configured.
+ */
+ public Map<Integer, Set<Integer>> adjacencySids() {
+ if (!object.has(ADJSIDS)) {
+ return null;
}
- ArrayNode adjacencySidNodes = (ArrayNode) object.path(ADJSID);
- adjacencySidNodes.forEach(adjacencySidNode -> {
- int asid = adjacencySidNode.path(AdjacencySid.ASID).asInt();
+ Map<Integer, Set<Integer>> adjacencySids = new HashMap<>();
+ ArrayNode adjacencySidsNode = (ArrayNode) object.path(ADJSIDS);
+ for (JsonNode adjacencySidNode : adjacencySidsNode) {
+ int asid = adjacencySidNode.path(ADJSID).asInt(-1);
+ if (asid == -1) {
+ return null;
+ }
- ArrayList<Integer> ports = new ArrayList<Integer>();
- ArrayNode portsNodes = (ArrayNode) adjacencySidNode.path(AdjacencySid.PORTS);
- portsNodes.forEach(portNode -> {
- ports.add(portNode.asInt());
+ HashSet<Integer> ports = new HashSet<>();
+ ArrayNode portsNode = (ArrayNode) adjacencySidNode.path(PORTS);
+ for (JsonNode portNode : portsNode) {
+ int port = portNode.asInt(-1);
+ if (port == -1) {
+ return null;
+ }
+ ports.add(port);
+ }
+ adjacencySids.put(asid, ports);
+ }
+
+ return ImmutableMap.copyOf(adjacencySids);
+ }
+
+ /**
+ * Sets the adjacency SIDs of the router.
+ *
+ * @param adjacencySids adjacency SIDs of the router.
+ * @return the config of the router.
+ */
+ public SegmentRoutingConfig setAdjacencySids(Map<Integer, Set<Integer>> adjacencySids) {
+ if (adjacencySids == null) {
+ object.remove(ADJSIDS);
+ } else {
+ ArrayNode adjacencySidsNode = mapper.createArrayNode();
+
+ adjacencySids.forEach((sid, ports) -> {
+ ObjectNode adjacencySidNode = mapper.createObjectNode();
+
+ adjacencySidNode.put(ADJSID, sid);
+
+ ArrayNode portsNode = mapper.createArrayNode();
+ ports.forEach(port -> {
+ portsNode.add(port.toString());
+ });
+ adjacencySidNode.set(PORTS, portsNode);
+
+ adjacencySidsNode.add(adjacencySidNode);
});
- AdjacencySid adjacencySid = new AdjacencySid(asid, ports);
- adjacencySids.add(adjacencySid);
- });
-
- return adjacencySids;
- }
-
- public class AdjacencySid {
- private static final String ASID = "adjSid";
- private static final String PORTS = "ports";
-
- int asid;
- List<Integer> ports;
-
- public AdjacencySid(int asid, List<Integer> ports) {
- this.asid = asid;
- this.ports = ports;
+ object.set(ADJSIDS, adjacencySidsNode);
}
- public int getAsid() {
- return asid;
- }
-
- public List<Integer> getPorts() {
- return ports;
- }
+ return this;
}
}
\ No newline at end of file
diff --git a/src/test/java/org/onosproject/segmentrouting/config/SegmentRoutingConfigTest.java b/src/test/java/org/onosproject/segmentrouting/config/SegmentRoutingConfigTest.java
new file mode 100644
index 0000000..3e5daa5
--- /dev/null
+++ b/src/test/java/org/onosproject/segmentrouting/config/SegmentRoutingConfigTest.java
@@ -0,0 +1,157 @@
+/*
+ * Copyright 2014-2015 Open Networking Laboratory
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.onosproject.segmentrouting.config;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.junit.Before;
+import org.junit.Test;
+import org.onlab.packet.IpAddress;
+import org.onlab.packet.MacAddress;
+import org.onosproject.net.DeviceId;
+import org.onosproject.net.config.Config;
+import org.onosproject.net.config.ConfigApplyDelegate;
+
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+import static org.junit.Assert.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Tests for class {@link SegmentRoutingConfig}.
+ */
+public class SegmentRoutingConfigTest {
+ private SegmentRoutingConfig config;
+ private Map<Integer, Set<Integer>> adjacencySids1;
+ private Map<Integer, Set<Integer>> adjacencySids2;
+
+ @Before
+ public void setUp() throws Exception {
+ String jsonString = "{" +
+ "\"name\" : \"Leaf-R1\"," +
+ "\"nodeSid\" : 101," +
+ "\"routerIp\" : \"10.0.1.254\"," +
+ "\"routerMac\" : \"00:00:00:00:01:80\"," +
+ "\"isEdgeRouter\" : true," +
+ "\"adjacencySids\" : [" +
+ " { \"adjSid\" : 100, \"ports\" : [2, 3] }," +
+ " { \"adjSid\" : 200, \"ports\" : [4, 5] }" +
+ "]}";
+
+ adjacencySids1 = new HashMap<>();
+ Set<Integer> ports1 = new HashSet<>();
+ ports1.add(2);
+ ports1.add(3);
+ adjacencySids1.put(100, ports1);
+ Set<Integer> ports2 = new HashSet<>();
+ ports2.add(4);
+ ports2.add(5);
+ adjacencySids1.put(200, ports2);
+
+ adjacencySids2 = new HashMap<>();
+ Set<Integer> ports3 = new HashSet<>();
+ ports3.add(6);
+ adjacencySids2.put(300, ports3);
+
+ DeviceId subject = DeviceId.deviceId("of:0000000000000001");
+ String key = "org.onosproject.segmentrouting";
+ ObjectMapper mapper = new ObjectMapper();
+ JsonNode jsonNode = mapper.readTree(jsonString);
+ ConfigApplyDelegate delegate = new MockDelegate();
+
+ config = new SegmentRoutingConfig();
+ config.init(subject, key, jsonNode, mapper, delegate);
+ }
+
+ @Test
+ public void testName() throws Exception {
+ assertTrue(config.name().isPresent());
+ assertThat(config.name().get(), is("Leaf-R1"));
+ }
+
+ @Test
+ public void testSetName() throws Exception {
+ config.setName("Spine-R1");
+ assertTrue(config.name().isPresent());
+ assertThat(config.name().get(), is("Spine-R1"));
+ }
+
+ @Test
+ public void testRouterIp() throws Exception {
+ assertThat(config.routerIp(), is(IpAddress.valueOf("10.0.1.254")));
+ }
+
+ @Test
+ public void testSetRouterIp() throws Exception {
+ config.setRouterIp("10.0.2.254");
+ assertThat(config.routerIp(), is(IpAddress.valueOf("10.0.2.254")));
+ }
+
+ @Test
+ public void testRouterMac() throws Exception {
+ assertThat(config.routerMac(), is(MacAddress.valueOf("00:00:00:00:01:80")));
+ }
+
+ @Test
+ public void testSetRouterMac() throws Exception {
+ config.setRouterMac("00:00:00:00:02:80");
+ assertThat(config.routerMac(), is(MacAddress.valueOf("00:00:00:00:02:80")));
+ }
+
+ @Test
+ public void testNodeSid() throws Exception {
+ assertThat(config.nodeSid(), is(101));
+ }
+
+ @Test
+ public void testSetNodeSid() throws Exception {
+ config.setNodeSid(200);
+ assertThat(config.nodeSid(), is(200));
+ }
+
+ @Test
+ public void testIsEdgeRouter() throws Exception {
+ assertThat(config.isEdgeRouter(), is(true));
+ }
+
+ @Test
+ public void testSetIsEdgeRouter() throws Exception {
+ config.setIsEdgeRouter(false);
+ assertThat(config.isEdgeRouter(), is(false));
+ }
+
+ @Test
+ public void testAdjacencySids() throws Exception {
+ assertThat(config.adjacencySids(), is(adjacencySids1));
+ }
+
+ @Test
+ public void testSetAdjacencySids() throws Exception {
+ config.setAdjacencySids(adjacencySids2);
+ assertThat(config.adjacencySids(), is(adjacencySids2));
+ }
+
+ private class MockDelegate implements ConfigApplyDelegate {
+ @Override
+ public void onApply(Config config) {
+ }
+ }
+}
\ No newline at end of file