[SDFAB-6] Intermittent failures in the DynamicConfig tests
shouldProgram is now enforced also on dynamic interface config.
Change-Id: I70a877065dc6e5f030c1d93bf1c1f6e73ef3ffdd
diff --git a/impl/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java b/impl/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
index e085c63..1dbc701 100644
--- a/impl/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
+++ b/impl/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
@@ -1314,17 +1314,23 @@
}
/**
- * Revoke rules of given subnet in all edge switches.
+ * Revoke rules of given subnet in all edge switches. Use the
+ * destination switch (if it is provided) to provide coordination
+ * among the instances. Otherwise, only the leader of the target
+ * switch can remove this subnet.
*
* @param subnets subnet being removed
+ * @param destSw destination switch. It is null when it is called from RouteHandler,
+ * in this context we don't have a way to remember the old locations.
* @return true if succeed
*/
- protected boolean revokeSubnet(Set<IpPrefix> subnets) {
+ protected boolean revokeSubnet(Set<IpPrefix> subnets, DeviceId destSw) {
DeviceId targetSw;
List<Future<Boolean>> futures = Lists.newArrayList();
for (Device sw : srManager.deviceService.getAvailableDevices()) {
targetSw = sw.id();
- if (shouldProgram(targetSw)) {
+ // In some calls, we dont know anymore the destination switch
+ if ((destSw != null && shouldProgram(destSw)) || shouldProgram(targetSw)) {
futures.add(routePopulators.submit(new RevokeSubnet(targetSw, subnets)));
} else {
futures.add(CompletableFuture.completedFuture(true));
diff --git a/impl/src/main/java/org/onosproject/segmentrouting/RouteHandler.java b/impl/src/main/java/org/onosproject/segmentrouting/RouteHandler.java
index dc1a25a..dae673d 100644
--- a/impl/src/main/java/org/onosproject/segmentrouting/RouteHandler.java
+++ b/impl/src/main/java/org/onosproject/segmentrouting/RouteHandler.java
@@ -154,7 +154,8 @@
// Revoke subnet from all locations and reset oldRoutes such that system will be reprogrammed from scratch
if (oldRoutes.size() > 2) {
log.info("Revoke subnet {} and reset oldRoutes");
- srManager.defaultRoutingHandler.revokeSubnet(allPrefixes);
+ // FIXME remove routes more precisely by memorizing the old locations
+ srManager.defaultRoutingHandler.revokeSubnet(allPrefixes, null);
oldRoutes = Sets.newHashSet();
}
@@ -213,7 +214,8 @@
allPrefixes.add(route.prefix());
});
log.debug("RouteRemoved. revokeSubnet {}", allPrefixes);
- srManager.defaultRoutingHandler.revokeSubnet(allPrefixes);
+ // FIXME remove routes more precisely by memorizing the old locations
+ srManager.defaultRoutingHandler.revokeSubnet(allPrefixes, null);
routes.forEach(route -> {
IpPrefix prefix = route.prefix();
diff --git a/impl/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java b/impl/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
index c81ee56..5d86487 100644
--- a/impl/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
+++ b/impl/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
@@ -2155,9 +2155,15 @@
Interface intf = intfs.stream().findFirst().get();
Interface prevIntf = prevIntfs.stream().findFirst().get();
-
DeviceId deviceId = intf.connectPoint().deviceId();
PortNumber portNum = intf.connectPoint().port();
+
+ if (!shouldProgram(deviceId)) {
+ log.debug("Not leading the programming of {} skip update interface {}",
+ deviceId, intf);
+ return;
+ }
+
// We need to do nexthop update al least one time for each
// interface config change. There is no difference when it is done;
boolean updateNexthop = false;
@@ -2394,7 +2400,7 @@
if (!subnetsToBeRevoked.isEmpty()) {
log.debug("Removing subnets for connectPoint: {}, subnets: {}", cp, subnetsToBeRevoked);
- defaultRoutingHandler.revokeSubnet(subnetsToBeRevoked);
+ defaultRoutingHandler.revokeSubnet(subnetsToBeRevoked, cp.deviceId());
}
// 2. Interface IP punts
@@ -2436,13 +2442,13 @@
if (!subnetsToBePopulated.isEmpty()) {
log.debug("Adding subnets for connectPoint: {}, subnets: {}", cp, subnetsToBePopulated);
- // check if pair-device has the same subnet configured?
+ // check if pair-device has the same subnet configured
Optional<DeviceId> pairDevice = getPairDeviceId(cp.deviceId());
if (pairDevice.isPresent()) {
Set<IpPrefix> pairDeviceIpPrefix = getDeviceSubnetMap().get(pairDevice.get());
Set<IpPrefix> subnetsToBePopulatedAsDualHomed = subnetsToBePopulated.stream()
- .filter(ipPrefix -> pairDeviceIpPrefix.contains(ipPrefix))
+ .filter(pairDeviceIpPrefix::contains)
.collect(Collectors.toSet());
Set<IpPrefix> subnetsToBePopulatedAsSingleHomed = Sets.difference(subnetsToBePopulated,
subnetsToBePopulatedAsDualHomed);
diff --git a/impl/src/main/java/org/onosproject/segmentrouting/mcast/McastHandler.java b/impl/src/main/java/org/onosproject/segmentrouting/mcast/McastHandler.java
index 8c4968a..7d62c68 100644
--- a/impl/src/main/java/org/onosproject/segmentrouting/mcast/McastHandler.java
+++ b/impl/src/main/java/org/onosproject/segmentrouting/mcast/McastHandler.java
@@ -1656,7 +1656,13 @@
// Iterates over the route and updates properly the filtering objective on the source device.
srManager.multicastRouteService.getRoutes().forEach(mcastRoute -> {
log.debug("Update filter for {}", mcastRoute.group());
- if (!mcastUtils.isLeader(mcastRoute.group())) {
+ /*
+ * Only skip filtering objective programming when current instance is
+ * not the leader of the Multicast group nor the leader of the device
+ * FIXME revisit multicast group programming responsibility or
+ * call updateFilterToDeviceInternal from a different context
+ */
+ if (!mcastUtils.isLeader(mcastRoute.group()) && !srManager.shouldProgram(deviceId)) {
log.debug("Skip {} due to lack of leadership", mcastRoute.group());
return;
}
diff --git a/impl/src/test/java/org/onosproject/segmentrouting/MockDefaultRoutingHandler.java b/impl/src/test/java/org/onosproject/segmentrouting/MockDefaultRoutingHandler.java
index d82908c..6cea390 100644
--- a/impl/src/test/java/org/onosproject/segmentrouting/MockDefaultRoutingHandler.java
+++ b/impl/src/test/java/org/onosproject/segmentrouting/MockDefaultRoutingHandler.java
@@ -56,7 +56,7 @@
}
@Override
- protected boolean revokeSubnet(Set<IpPrefix> subnets) {
+ protected boolean revokeSubnet(Set<IpPrefix> subnets, DeviceId deviceId) {
for (Map.Entry<ConnectPoint, Set<IpPrefix>> entry : subnetTable.entrySet()) {
entry.getValue().removeAll(subnets);
if (entry.getValue().isEmpty()) {