Routing/bridging rules on the same leaf pair should always be programmed by the same ONOS instance
Main change:
- Implement new logic for shouldHandleRouting, along with corresponding unit tests. Also rename it to shouldProgram
Side changes:
- Refactor revokeSubnet such that it is only invoked on the instance that should handle routing change
- Move the following methods to RoutingRulePopulator and follow the same design pattern as populate/revoke subnet/route
- populateBridging, revokeBridging, updateBridging
- updateFwdObj
- Make sure the following methods in RoutingRulePopulator are always invoked by DefaultRoutingHandler with shouldProgram check
- populateRoute, revokeRoute
- populateSubnet, revokeSubnet
- populateBridging, revokeBridging, updateBridging
- updateFwdObj
Change-Id: I903129271ede91c45ebf0d973e06faeae46c157a
diff --git a/app/src/test/java/org/onosproject/segmentrouting/DefaultRoutingHandlerTest.java b/app/src/test/java/org/onosproject/segmentrouting/DefaultRoutingHandlerTest.java
new file mode 100644
index 0000000..05abcfc
--- /dev/null
+++ b/app/src/test/java/org/onosproject/segmentrouting/DefaultRoutingHandlerTest.java
@@ -0,0 +1,339 @@
+/*
+ * Copyright 2018-present Open Networking Foundation
+ *
+ * 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;
+
+import com.google.common.collect.Sets;
+import org.junit.Before;
+import org.junit.Test;
+import org.onlab.packet.IpAddress;
+import org.onosproject.cluster.ClusterService;
+import org.onosproject.cluster.DefaultControllerNode;
+import org.onosproject.cluster.NodeId;
+import org.onosproject.mastership.MastershipService;
+import org.onosproject.net.DeviceId;
+import org.onosproject.net.device.DeviceService;
+import org.onosproject.segmentrouting.config.DeviceConfiguration;
+import org.onosproject.store.service.StorageService;
+import org.onosproject.store.service.TestConsistentMap;
+
+import java.util.Optional;
+
+import static org.easymock.EasyMock.createMock;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.replay;
+import static org.easymock.EasyMock.reset;
+import static org.junit.Assert.*;
+
+public class DefaultRoutingHandlerTest {
+ private SegmentRoutingManager srManager;
+ private DefaultRoutingHandler dfh;
+
+ private static final DeviceId DEV1A = DeviceId.deviceId("of:1a");
+ private static final DeviceId DEV1B = DeviceId.deviceId("of:1b");
+ private static final DeviceId DEV2 = DeviceId.deviceId("of:2");
+
+ private static final NodeId NODE1 = NodeId.nodeId("192.168.1.1");
+ private static final NodeId NODE2 = NodeId.nodeId("192.168.1.2");
+ private static final NodeId NODE3 = NodeId.nodeId("192.168.1.3");
+ private static final IpAddress IP1 = IpAddress.valueOf("192.168.1.1");
+ private static final IpAddress IP2 = IpAddress.valueOf("192.168.1.2");
+ private static final IpAddress IP3 = IpAddress.valueOf("192.168.1.3");
+
+ @Before
+ public void setUp() {
+ srManager = createMock(SegmentRoutingManager.class);
+ srManager.storageService = createMock(StorageService.class);
+ expect(srManager.storageService.consistentMapBuilder()).andReturn(new TestConsistentMap.Builder<>()).anyTimes();
+ replay(srManager.storageService);
+ srManager.routingRulePopulator = createMock(RoutingRulePopulator.class);
+ srManager.deviceService = createMock(DeviceService.class);
+ srManager.deviceConfiguration = createMock(DeviceConfiguration.class);
+ srManager.mastershipService = createMock(MastershipService.class);
+ srManager.clusterService = createMock(ClusterService.class);
+ dfh = new DefaultRoutingHandler(srManager);
+ }
+
+ // Node 1 is the master of switch 1A, 1B, and 2
+ @Test
+ public void testShouldHandleRoutingCase1() {
+ expect(srManager.mastershipService.getMasterFor(DEV1A)).andReturn(NODE1).anyTimes();
+ expect(srManager.mastershipService.getMasterFor(DEV1B)).andReturn(NODE1).anyTimes();
+ expect(srManager.mastershipService.getMasterFor(DEV2)).andReturn(NODE1).anyTimes();
+ replay(srManager.mastershipService);
+
+ expect(srManager.getPairDeviceId(DEV1A)).andReturn(Optional.of(DEV1B)).anyTimes();
+ expect(srManager.getPairDeviceId(DEV1B)).andReturn(Optional.of(DEV1A)).anyTimes();
+ expect(srManager.getPairDeviceId(DEV2)).andReturn(Optional.empty()).anyTimes();
+ replay(srManager);
+
+ // Node 1 should program every device
+ expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE1, IP1)).anyTimes();
+ replay(srManager.clusterService);
+ assertTrue(dfh.shouldProgram(DEV1A));
+ assertTrue(dfh.shouldProgram(DEV1B));
+ assertTrue(dfh.shouldProgram(DEV2));
+
+ reset(srManager.clusterService);
+
+ // Node 2 should program no device
+ expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE2, IP2)).anyTimes();
+ replay(srManager.clusterService);
+ assertFalse(dfh.shouldProgram(DEV1A));
+ assertFalse(dfh.shouldProgram(DEV1B));
+ assertFalse(dfh.shouldProgram(DEV2));
+
+ reset(srManager.clusterService);
+
+ // Node 3 should program no device
+ expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE3, IP3)).anyTimes();
+ replay(srManager.clusterService);
+ assertFalse(dfh.shouldProgram(DEV1A));
+ assertFalse(dfh.shouldProgram(DEV1B));
+ assertFalse(dfh.shouldProgram(DEV2));
+ }
+
+ // Node 1 is the master of switch 1A, 1B
+ // Node 2 is the master of switch 2
+ @Test
+ public void testShouldHandleRoutingCase2() {
+ expect(srManager.mastershipService.getMasterFor(DEV1A)).andReturn(NODE1).anyTimes();
+ expect(srManager.mastershipService.getMasterFor(DEV1B)).andReturn(NODE1).anyTimes();
+ expect(srManager.mastershipService.getMasterFor(DEV2)).andReturn(NODE2).anyTimes();
+ replay(srManager.mastershipService);
+
+ expect(srManager.getPairDeviceId(DEV1A)).andReturn(Optional.of(DEV1B)).anyTimes();
+ expect(srManager.getPairDeviceId(DEV1B)).andReturn(Optional.of(DEV1A)).anyTimes();
+ expect(srManager.getPairDeviceId(DEV2)).andReturn(Optional.empty()).anyTimes();
+ replay(srManager);
+
+ // Node 1 should program 1A, 1B
+ expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE1, IP1)).anyTimes();
+ replay(srManager.clusterService);
+ assertTrue(dfh.shouldProgram(DEV1A));
+ assertTrue(dfh.shouldProgram(DEV1B));
+ assertFalse(dfh.shouldProgram(DEV2));
+
+ reset(srManager.clusterService);
+
+ // Node 2 should program 2
+ expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE2, IP2)).anyTimes();
+ replay(srManager.clusterService);
+ assertFalse(dfh.shouldProgram(DEV1A));
+ assertFalse(dfh.shouldProgram(DEV1B));
+ assertTrue(dfh.shouldProgram(DEV2));
+
+ reset(srManager.clusterService);
+
+ // Node 3 should program no device
+ expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE3, IP3)).anyTimes();
+ replay(srManager.clusterService);
+ assertFalse(dfh.shouldProgram(DEV1A));
+ assertFalse(dfh.shouldProgram(DEV1B));
+ assertFalse(dfh.shouldProgram(DEV2));
+ }
+
+ // Node 1 is the master of switch 1A
+ // Node 2 is the master of switch 1B
+ // Node 3 is the master of switch 2
+ @Test
+ public void testShouldHandleRoutingCase3() {
+ expect(srManager.mastershipService.getMasterFor(DEV1A)).andReturn(NODE1).anyTimes();
+ expect(srManager.mastershipService.getMasterFor(DEV1B)).andReturn(NODE2).anyTimes();
+ expect(srManager.mastershipService.getMasterFor(DEV2)).andReturn(NODE3).anyTimes();
+ replay(srManager.mastershipService);
+
+ expect(srManager.getPairDeviceId(DEV1A)).andReturn(Optional.of(DEV1B)).anyTimes();
+ expect(srManager.getPairDeviceId(DEV1B)).andReturn(Optional.of(DEV1A)).anyTimes();
+ expect(srManager.getPairDeviceId(DEV2)).andReturn(Optional.empty()).anyTimes();
+ replay(srManager);
+
+ // Node 1 should program 1A, 1B
+ expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE1, IP1)).anyTimes();
+ replay(srManager.clusterService);
+ assertTrue(dfh.shouldProgram(DEV1A));
+ assertTrue(dfh.shouldProgram(DEV1B));
+ assertFalse(dfh.shouldProgram(DEV2));
+
+ reset(srManager.clusterService);
+
+ // Node 2 should program no device
+ expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE2, IP2)).anyTimes();
+ replay(srManager.clusterService);
+ assertFalse(dfh.shouldProgram(DEV1A));
+ assertFalse(dfh.shouldProgram(DEV1B));
+ assertFalse(dfh.shouldProgram(DEV2));
+
+ reset(srManager.clusterService);
+
+ // Node 3 should program 2
+ expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE3, IP3)).anyTimes();
+ replay(srManager.clusterService);
+ assertFalse(dfh.shouldProgram(DEV1A));
+ assertFalse(dfh.shouldProgram(DEV1B));
+ assertTrue(dfh.shouldProgram(DEV2));
+ }
+
+ // Node 3 is the master of switch 1A, 1B, 2
+ // Later on, node 1 becomes the master of 1A; Node 2 becomes the master of 1B.
+ @Test
+ public void testShouldHandleRoutingCase4() {
+ expect(srManager.mastershipService.getMasterFor(DEV1A)).andReturn(NODE3).anyTimes();
+ expect(srManager.mastershipService.getMasterFor(DEV1B)).andReturn(NODE3).anyTimes();
+ expect(srManager.mastershipService.getMasterFor(DEV2)).andReturn(NODE3).anyTimes();
+ replay(srManager.mastershipService);
+
+ expect(srManager.getPairDeviceId(DEV1A)).andReturn(Optional.of(DEV1B)).anyTimes();
+ expect(srManager.getPairDeviceId(DEV1B)).andReturn(Optional.of(DEV1A)).anyTimes();
+ expect(srManager.getPairDeviceId(DEV2)).andReturn(Optional.empty()).anyTimes();
+ replay(srManager);
+
+ // Node 1 should program no device
+ expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE1, IP1)).anyTimes();
+ replay(srManager.clusterService);
+ assertFalse(dfh.shouldProgram(DEV1A));
+ assertFalse(dfh.shouldProgram(DEV1B));
+ assertFalse(dfh.shouldProgram(DEV2));
+
+ reset(srManager.clusterService);
+
+ // Node 2 should program no device
+ expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE2, IP2)).anyTimes();
+ replay(srManager.clusterService);
+ assertFalse(dfh.shouldProgram(DEV1A));
+ assertFalse(dfh.shouldProgram(DEV1B));
+ assertFalse(dfh.shouldProgram(DEV2));
+
+ reset(srManager.clusterService);
+
+ // Node 3 should program 1A, 1B and 2
+ expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE3, IP3)).anyTimes();
+ replay(srManager.clusterService);
+ assertTrue(dfh.shouldProgram(DEV1A));
+ assertTrue(dfh.shouldProgram(DEV1B));
+ assertTrue(dfh.shouldProgram(DEV2));
+
+ // Mastership of switch 1A moves to Node 1
+ reset(srManager.mastershipService);
+ expect(srManager.mastershipService.getMasterFor(DEV1A)).andReturn(NODE1).anyTimes();
+ expect(srManager.mastershipService.getMasterFor(DEV1B)).andReturn(NODE2).anyTimes();
+ expect(srManager.mastershipService.getMasterFor(DEV2)).andReturn(NODE3).anyTimes();
+ replay(srManager.mastershipService);
+
+ reset(srManager.clusterService);
+
+ // Node 1 should program 1A, 1B
+ expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE1, IP1)).anyTimes();
+ replay(srManager.clusterService);
+ assertTrue(dfh.shouldProgram(DEV1A));
+ assertTrue(dfh.shouldProgram(DEV1B));
+ assertFalse(dfh.shouldProgram(DEV2));
+
+ reset(srManager.clusterService);
+
+ // Node 2 should program no device
+ expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE2, IP2)).anyTimes();
+ replay(srManager.clusterService);
+ assertFalse(dfh.shouldProgram(DEV1A));
+ assertFalse(dfh.shouldProgram(DEV1B));
+ assertFalse(dfh.shouldProgram(DEV2));
+
+ reset(srManager.clusterService);
+
+ // Node 3 should program 2
+ expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE3, IP3)).anyTimes();
+ replay(srManager.clusterService);
+ assertFalse(dfh.shouldProgram(DEV1A));
+ assertFalse(dfh.shouldProgram(DEV1B));
+ assertTrue(dfh.shouldProgram(DEV2));
+ }
+
+ // Node 1 is the master of 1A. 1B has no master
+ // Node 2 becomes the master of 1B later
+ @Test
+ public void testShouldHandleRoutingCase5() {
+ expect(srManager.mastershipService.getMasterFor(DEV1A)).andReturn(NODE1).anyTimes();
+ expect(srManager.mastershipService.getMasterFor(DEV1B)).andReturn(null).anyTimes();
+ replay(srManager.mastershipService);
+
+ expect(srManager.getPairDeviceId(DEV1A)).andReturn(Optional.of(DEV1B)).anyTimes();
+ expect(srManager.getPairDeviceId(DEV1B)).andReturn(Optional.of(DEV1A)).anyTimes();
+ replay(srManager);
+
+ // Node 1 should program both 1A and 1B
+ expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE1, IP1)).anyTimes();
+ replay(srManager.clusterService);
+ assertTrue(dfh.shouldProgram(DEV1A));
+ assertTrue(dfh.shouldProgram(DEV1B));
+
+ reset(srManager.clusterService);
+
+ // Node 2 should program no device
+ expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE2, IP2)).anyTimes();
+ replay(srManager.clusterService);
+ assertFalse(dfh.shouldProgram(DEV1A));
+ assertFalse(dfh.shouldProgram(DEV1B));
+
+ // Mastership of switch 1B moves to Node 2
+ reset(srManager.mastershipService);
+ expect(srManager.mastershipService.getMasterFor(DEV1A)).andReturn(NODE1).anyTimes();
+ expect(srManager.mastershipService.getMasterFor(DEV1B)).andReturn(NODE2).anyTimes();
+ replay(srManager.mastershipService);
+
+ reset(srManager.clusterService);
+
+ // Node 1 should program 1A, 1B
+ expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE1, IP1)).anyTimes();
+ replay(srManager.clusterService);
+ assertTrue(dfh.shouldProgram(DEV1A));
+ assertTrue(dfh.shouldProgram(DEV1B));
+
+ reset(srManager.clusterService);
+
+ // Node 2 should program no device
+ expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE2, IP2)).anyTimes();
+ replay(srManager.clusterService);
+ assertFalse(dfh.shouldProgram(DEV1A));
+ assertFalse(dfh.shouldProgram(DEV1B));
+ }
+
+ // Neither 1A or 1B has master
+ @Test
+ public void testShouldHandleRoutingCase6() {
+ expect(srManager.mastershipService.getMasterFor(DEV1A)).andReturn(null).anyTimes();
+ expect(srManager.mastershipService.getMasterFor(DEV1B)).andReturn(null).anyTimes();
+ replay(srManager.mastershipService);
+
+ expect(srManager.getPairDeviceId(DEV1A)).andReturn(Optional.of(DEV1B)).anyTimes();
+ expect(srManager.getPairDeviceId(DEV1B)).andReturn(Optional.of(DEV1A)).anyTimes();
+ replay(srManager);
+
+ // Node 1 should program no device
+ expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE1, IP1)).anyTimes();
+ replay(srManager.clusterService);
+ assertFalse(dfh.shouldProgram(DEV1A));
+ assertFalse(dfh.shouldProgram(DEV1B));
+
+ reset(srManager.clusterService);
+
+ // Node 2 should program no device
+ expect(srManager.clusterService.getLocalNode()).andReturn(new DefaultControllerNode(NODE2, IP2)).anyTimes();
+ replay(srManager.clusterService);
+ assertFalse(dfh.shouldProgram(DEV1A));
+ assertFalse(dfh.shouldProgram(DEV1B));
+
+ assertFalse(dfh.shouldProgram.containsKey(Sets.newHashSet(DEV1A, DEV1B)));
+ }
+}
\ No newline at end of file
diff --git a/app/src/test/java/org/onosproject/segmentrouting/HostHandlerTest.java b/app/src/test/java/org/onosproject/segmentrouting/HostHandlerTest.java
index b6ec070..3605727 100644
--- a/app/src/test/java/org/onosproject/segmentrouting/HostHandlerTest.java
+++ b/app/src/test/java/org/onosproject/segmentrouting/HostHandlerTest.java
@@ -44,10 +44,15 @@
import org.onosproject.net.provider.ProviderId;
import org.onosproject.segmentrouting.config.DeviceConfiguration;
import org.onosproject.segmentrouting.config.SegmentRoutingDeviceConfig;
+import org.onosproject.store.service.StorageService;
+import org.onosproject.store.service.TestConsistentMap;
import java.util.Map;
import java.util.Set;
+import static org.easymock.EasyMock.createMock;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.replay;
import static org.junit.Assert.*;
/**r
@@ -201,6 +206,9 @@
// Initialize Segment Routing Manager
SegmentRoutingManager srManager = new MockSegmentRoutingManager(NEXT_TABLE);
+ srManager.storageService = createMock(StorageService.class);
+ expect(srManager.storageService.consistentMapBuilder()).andReturn(new TestConsistentMap.Builder<>()).anyTimes();
+ replay(srManager.storageService);
srManager.cfgService = new NetworkConfigRegistryAdapter();
srManager.deviceConfiguration = new DeviceConfiguration(srManager);
srManager.flowObjectiveService = new MockFlowObjectiveService(BRIDGING_TABLE, NEXT_TABLE);
@@ -846,10 +854,4 @@
assertNotNull(BRIDGING_TABLE.get(new MockBridgingTableKey(DEV1, HOST_MAC, INTF_VLAN_UNTAGGED)));
assertNotNull(BRIDGING_TABLE.get(new MockBridgingTableKey(DEV2, HOST_MAC, INTF_VLAN_UNTAGGED)));
}
-
- @Test
- public void testBridgingFwdObjBuilder() throws Exception {
- assertNotNull(hostHandler.bridgingFwdObjBuilder(DEV2, HOST_MAC, HOST_VLAN_UNTAGGED, P1, false));
- assertNull(hostHandler.bridgingFwdObjBuilder(DEV2, HOST_MAC, HOST_VLAN_UNTAGGED, P3, false));
- }
}
diff --git a/app/src/test/java/org/onosproject/segmentrouting/MockDefaultRoutingHandler.java b/app/src/test/java/org/onosproject/segmentrouting/MockDefaultRoutingHandler.java
index d31be2f..0002948 100644
--- a/app/src/test/java/org/onosproject/segmentrouting/MockDefaultRoutingHandler.java
+++ b/app/src/test/java/org/onosproject/segmentrouting/MockDefaultRoutingHandler.java
@@ -18,6 +18,7 @@
import org.onlab.packet.IpPrefix;
import org.onosproject.net.ConnectPoint;
+import org.onosproject.net.DeviceId;
import java.util.Map;
import java.util.Set;
@@ -62,4 +63,9 @@
routingTable.entrySet().removeIf(e -> subnets.contains(e.getKey().ipPrefix));
return true;
}
+
+ @Override
+ protected boolean shouldProgram(DeviceId deviceId) {
+ return true;
+ }
}
\ No newline at end of file
diff --git a/app/src/test/java/org/onosproject/segmentrouting/RouteHandlerTest.java b/app/src/test/java/org/onosproject/segmentrouting/RouteHandlerTest.java
index 36a9e79..a7388d1 100644
--- a/app/src/test/java/org/onosproject/segmentrouting/RouteHandlerTest.java
+++ b/app/src/test/java/org/onosproject/segmentrouting/RouteHandlerTest.java
@@ -45,10 +45,13 @@
import org.onosproject.routeservice.RouteEvent;
import org.onosproject.segmentrouting.config.DeviceConfiguration;
import org.onosproject.segmentrouting.config.SegmentRoutingDeviceConfig;
+import org.onosproject.store.service.StorageService;
+import org.onosproject.store.service.TestConsistentMap;
import java.util.Map;
import java.util.Set;
+import static org.easymock.EasyMock.expect;
import static org.easymock.EasyMock.reset;
import static org.junit.Assert.*;
import static org.easymock.EasyMock.createMock;
@@ -141,6 +144,9 @@
// Initialize Segment Routing Manager
srManager = new MockSegmentRoutingManager(NEXT_TABLE);
+ srManager.storageService = createMock(StorageService.class);
+ expect(srManager.storageService.consistentMapBuilder()).andReturn(new TestConsistentMap.Builder<>()).anyTimes();
+ replay(srManager.storageService);
srManager.cfgService = new NetworkConfigRegistryAdapter();
srManager.deviceConfiguration = createMock(DeviceConfiguration.class);
srManager.flowObjectiveService = new MockFlowObjectiveService(BRIDGING_TABLE, NEXT_TABLE);