Bugfix in route handler in occasion of route update event
Change-Id: Ia0a0cc7d5522d41cad5f3d992c589c9f5cbad244
diff --git a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/RouteHandler.java b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/RouteHandler.java
index c80bc09..21870ee 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/RouteHandler.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/RouteHandler.java
@@ -161,25 +161,20 @@
log.debug("RouteUpdated. populateSubnet {}, {}", allLocations, allPrefixes);
srManager.defaultRoutingHandler.populateSubnet(allLocations, allPrefixes);
-
Set<ResolvedRoute> toBeRemoved = Sets.difference(oldRoutes, routes).immutableCopy();
Set<ResolvedRoute> toBeAdded = Sets.difference(routes, oldRoutes).immutableCopy();
toBeRemoved.forEach(route -> {
srManager.nextHopLocations(route).forEach(oldLocation -> {
- if (toBeAdded.stream().map(srManager::nextHopLocations)
- .flatMap(Set::stream).map(ConnectPoint::deviceId)
- .noneMatch(deviceId -> deviceId.equals(oldLocation.deviceId()))) {
- IpPrefix prefix = route.prefix();
- MacAddress nextHopMac = route.nextHopMac();
- VlanId nextHopVlan = route.nextHopVlan();
-
- log.debug("RouteUpdated. removeSubnet {}, {}", oldLocation, prefix);
- srManager.deviceConfiguration.removeSubnet(oldLocation, prefix);
- log.debug("RouteUpdated. revokeRoute {}, {}, {}, {}", oldLocation, prefix, nextHopMac, nextHopVlan);
- srManager.defaultRoutingHandler.revokeRoute(oldLocation.deviceId(), prefix,
- nextHopMac, nextHopVlan, oldLocation.port());
- }
+ if (toBeAdded.stream().map(srManager::nextHopLocations)
+ .flatMap(Set::stream).map(ConnectPoint::deviceId)
+ .noneMatch(deviceId -> deviceId.equals(oldLocation.deviceId()))) {
+ IpPrefix prefix = route.prefix();
+ log.debug("RouteUpdated. removeSubnet {}, {}", oldLocation, prefix);
+ srManager.deviceConfiguration.removeSubnet(oldLocation, prefix);
+ // We don't remove the flow on the old location in occasion of two next hops becoming one
+ // since the populateSubnet will point the old location to the new location via spine.
+ }
});
});
@@ -197,7 +192,6 @@
nextHopMac, nextHopVlan, location.port());
});
});
-
}
void processRouteRemoved(RouteEvent event) {
diff --git a/apps/segmentrouting/src/test/java/org/onosproject/segmentrouting/RouteHandlerTest.java b/apps/segmentrouting/src/test/java/org/onosproject/segmentrouting/RouteHandlerTest.java
index 775e5ee..843ba6b 100644
--- a/apps/segmentrouting/src/test/java/org/onosproject/segmentrouting/RouteHandlerTest.java
+++ b/apps/segmentrouting/src/test/java/org/onosproject/segmentrouting/RouteHandlerTest.java
@@ -49,12 +49,18 @@
import java.util.Map;
import java.util.Set;
+import static org.easymock.EasyMock.reset;
import static org.junit.Assert.*;
+import static org.easymock.EasyMock.createMock;
+import static org.easymock.EasyMock.expectLastCall;
+import static org.easymock.EasyMock.replay;
+import static org.easymock.EasyMock.verify;
/**
* Unit test for {@link RouteHandler}.
*/
public class RouteHandlerTest {
+ private SegmentRoutingManager srManager;
private RouteHandler routeHandler;
private HostService hostService;
@@ -134,9 +140,9 @@
mockNetworkConfigRegistry.applyConfig(dev2Config);
// Initialize Segment Routing Manager
- SegmentRoutingManager srManager = new MockSegmentRoutingManager(NEXT_TABLE);
+ srManager = new MockSegmentRoutingManager(NEXT_TABLE);
srManager.cfgService = new NetworkConfigRegistryAdapter();
- srManager.deviceConfiguration = new DeviceConfiguration(srManager);
+ srManager.deviceConfiguration = createMock(DeviceConfiguration.class);
srManager.flowObjectiveService = new MockFlowObjectiveService(BRIDGING_TABLE, NEXT_TABLE);
srManager.routingRulePopulator = new MockRoutingRulePopulator(srManager, ROUTING_TABLE);
srManager.defaultRoutingHandler = new MockDefaultRoutingHandler(srManager, SUBNET_TABLE);
@@ -180,6 +186,11 @@
@Test
public void processRouteAdded() throws Exception {
+ reset(srManager.deviceConfiguration);
+ srManager.deviceConfiguration.addSubnet(CP1, P1);
+ expectLastCall().once();
+ replay(srManager.deviceConfiguration);
+
RouteEvent re = new RouteEvent(RouteEvent.Type.ROUTE_ADDED, RR1, Sets.newHashSet(RR1));
routeHandler.processRouteAdded(re);
@@ -191,17 +202,28 @@
assertEquals(1, SUBNET_TABLE.size());
assertTrue(SUBNET_TABLE.get(CP1).contains(P1));
+
+ verify(srManager.deviceConfiguration);
}
@Test
public void processRouteUpdated() throws Exception {
processRouteAdded();
+ reset(srManager.deviceConfiguration);
+ srManager.deviceConfiguration.removeSubnet(CP1, P1);
+ expectLastCall().once();
+ srManager.deviceConfiguration.addSubnet(CP2, P1);
+ expectLastCall().once();
+ replay(srManager.deviceConfiguration);
+
RouteEvent re = new RouteEvent(RouteEvent.Type.ROUTE_UPDATED, RR2, RR1, Sets.newHashSet(RR2),
Sets.newHashSet(RR1));
routeHandler.processRouteUpdated(re);
- assertEquals(1, ROUTING_TABLE.size());
+ // Note: We shouldn't remove the old nexthop during the occasion of route update
+ // since the populate subnet will take care of it and point it to an ECMP group
+ assertEquals(2, ROUTING_TABLE.size());
MockRoutingTableValue rtv2 = ROUTING_TABLE.get(new MockRoutingTableKey(CP2.deviceId(), P1));
assertEquals(M2, rtv2.macAddress);
assertEquals(V2, rtv2.vlanId);
@@ -209,21 +231,37 @@
assertEquals(1, SUBNET_TABLE.size());
assertTrue(SUBNET_TABLE.get(CP2).contains(P1));
+
+ verify(srManager.deviceConfiguration);
}
@Test
public void processRouteRemoved() throws Exception {
processRouteAdded();
+ reset(srManager.deviceConfiguration);
+ srManager.deviceConfiguration.removeSubnet(CP1, P1);
+ expectLastCall().once();
+ replay(srManager.deviceConfiguration);
+
RouteEvent re = new RouteEvent(RouteEvent.Type.ROUTE_REMOVED, RR1, Sets.newHashSet(RR1));
routeHandler.processRouteRemoved(re);
assertEquals(0, ROUTING_TABLE.size());
assertEquals(0, SUBNET_TABLE.size());
+
+ verify(srManager.deviceConfiguration);
}
@Test
public void testTwoSingleHomedAdded() throws Exception {
+ reset(srManager.deviceConfiguration);
+ srManager.deviceConfiguration.addSubnet(CP1, P1);
+ expectLastCall().once();
+ srManager.deviceConfiguration.addSubnet(CP2, P1);
+ expectLastCall().once();
+ replay(srManager.deviceConfiguration);
+
RouteEvent re = new RouteEvent(RouteEvent.Type.ROUTE_ADDED, RR1, Sets.newHashSet(RR1, RR2));
routeHandler.processRouteAdded(re);
@@ -240,10 +278,19 @@
assertEquals(2, SUBNET_TABLE.size());
assertTrue(SUBNET_TABLE.get(CP1).contains(P1));
assertTrue(SUBNET_TABLE.get(CP2).contains(P1));
+
+ verify(srManager.deviceConfiguration);
}
@Test
public void testOneDualHomedAdded() throws Exception {
+ reset(srManager.deviceConfiguration);
+ srManager.deviceConfiguration.addSubnet(CP1, P1);
+ expectLastCall().once();
+ srManager.deviceConfiguration.addSubnet(CP2, P1);
+ expectLastCall().once();
+ replay(srManager.deviceConfiguration);
+
RouteEvent re = new RouteEvent(RouteEvent.Type.ROUTE_ADDED, RR3, Sets.newHashSet(RR3));
routeHandler.processRouteAdded(re);
@@ -260,12 +307,19 @@
assertEquals(2, SUBNET_TABLE.size());
assertTrue(SUBNET_TABLE.get(CP1).contains(P1));
assertTrue(SUBNET_TABLE.get(CP2).contains(P1));
+
+ verify(srManager.deviceConfiguration);
}
@Test
public void testOneSingleHomedToTwoSingleHomed() throws Exception {
processRouteAdded();
+ reset(srManager.deviceConfiguration);
+ srManager.deviceConfiguration.addSubnet(CP2, P1);
+ expectLastCall().once();
+ replay(srManager.deviceConfiguration);
+
RouteEvent re = new RouteEvent(RouteEvent.Type.ALTERNATIVE_ROUTES_CHANGED, RR1, null,
Sets.newHashSet(RR1, RR2), Sets.newHashSet(RR1));
routeHandler.processAlternativeRoutesChanged(re);
@@ -283,17 +337,26 @@
assertEquals(2, SUBNET_TABLE.size());
assertTrue(SUBNET_TABLE.get(CP1).contains(P1));
assertTrue(SUBNET_TABLE.get(CP2).contains(P1));
+
+ verify(srManager.deviceConfiguration);
}
@Test
public void testTwoSingleHomedToOneSingleHomed() throws Exception {
testTwoSingleHomedAdded();
+ reset(srManager.deviceConfiguration);
+ srManager.deviceConfiguration.removeSubnet(CP2, P1);
+ expectLastCall().once();
+ replay(srManager.deviceConfiguration);
+
RouteEvent re = new RouteEvent(RouteEvent.Type.ALTERNATIVE_ROUTES_CHANGED, RR1, null,
Sets.newHashSet(RR1), Sets.newHashSet(RR1, RR2));
routeHandler.processAlternativeRoutesChanged(re);
- assertEquals(1, ROUTING_TABLE.size());
+ // Note: We shouldn't remove the old nexthop during the occasion of route update
+ // since the populate subnet will take care of it and point it to an ECMP group
+ assertEquals(2, ROUTING_TABLE.size());
MockRoutingTableValue rtv1 = ROUTING_TABLE.get(new MockRoutingTableKey(CP1.deviceId(), P1));
assertEquals(M1, rtv1.macAddress);
assertEquals(V1, rtv1.vlanId);
@@ -301,8 +364,12 @@
assertEquals(1, SUBNET_TABLE.size());
assertTrue(SUBNET_TABLE.get(CP1).contains(P1));
+
+ verify(srManager.deviceConfiguration);
}
+ // TODO Add test cases for two single homed next hop at same location
+
@Test
public void testDualHomedSingleLocationFail() throws Exception {
testOneDualHomedAdded();
@@ -332,11 +399,20 @@
hostService = new MockHostService(HOSTS_ONE_FAIL);
+ reset(srManager.deviceConfiguration);
+ srManager.deviceConfiguration.removeSubnet(CP1, P1);
+ expectLastCall().once();
+ srManager.deviceConfiguration.removeSubnet(CP2, P1);
+ expectLastCall().once();
+ replay(srManager.deviceConfiguration);
+
RouteEvent re = new RouteEvent(RouteEvent.Type.ROUTE_REMOVED, RR3, Sets.newHashSet(RR3));
routeHandler.processRouteRemoved(re);
assertEquals(0, ROUTING_TABLE.size());
assertEquals(0, SUBNET_TABLE.size());
+
+ verify(srManager.deviceConfiguration);
}
@Test
@@ -345,21 +421,39 @@
hostService = new MockHostService(HOSTS_BOTH_FAIL);
+ reset(srManager.deviceConfiguration);
+ srManager.deviceConfiguration.removeSubnet(CP1, P1);
+ expectLastCall().once();
+ srManager.deviceConfiguration.removeSubnet(CP2, P1);
+ expectLastCall().once();
+ replay(srManager.deviceConfiguration);
+
RouteEvent re = new RouteEvent(RouteEvent.Type.ROUTE_REMOVED, RR1, Sets.newHashSet(RR1, RR2));
routeHandler.processRouteRemoved(re);
assertEquals(0, ROUTING_TABLE.size());
assertEquals(0, SUBNET_TABLE.size());
+
+ verify(srManager.deviceConfiguration);
}
@Test
public void testOneDualHomeRemoved() throws Exception {
testOneDualHomedAdded();
+ reset(srManager.deviceConfiguration);
+ srManager.deviceConfiguration.removeSubnet(CP1, P1);
+ expectLastCall().once();
+ srManager.deviceConfiguration.removeSubnet(CP2, P1);
+ expectLastCall().once();
+ replay(srManager.deviceConfiguration);
+
RouteEvent re = new RouteEvent(RouteEvent.Type.ROUTE_REMOVED, RR3, Sets.newHashSet(RR3));
routeHandler.processRouteRemoved(re);
assertEquals(0, ROUTING_TABLE.size());
assertEquals(0, SUBNET_TABLE.size());
+
+ verify(srManager.deviceConfiguration);
}
}
\ No newline at end of file