[ONOS-7161] Flows stuck on pending_add after bring the link down
Change-Id: I8281b7cf9348056687ab9e30416170739d22953e
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/installer/FlowRuleIntentInstaller.java b/core/net/src/main/java/org/onosproject/net/intent/impl/installer/FlowRuleIntentInstaller.java
index 0cdd01d..8217255 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/installer/FlowRuleIntentInstaller.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/installer/FlowRuleIntentInstaller.java
@@ -120,13 +120,13 @@
flowRulesToInstall = Collections.emptyList();
}
- List<FlowRule> dontUninstall;
+ List<FlowRule> flowRuleToModify;
List<FlowRule> dontTouch;
// If both uninstall/install list contained equal (=match conditions are equal) FlowRules,
// omit it from remove list, since it will/should be overwritten by install
- dontUninstall = flowRulesToUninstall.stream()
- .filter(flowRule -> flowRulesToInstall.stream().anyMatch(flowRule::equals))
+ flowRuleToModify = flowRulesToInstall.stream()
+ .filter(flowRule -> flowRulesToUninstall.stream().anyMatch(flowRule::equals))
.collect(Collectors.toList());
// If both contained exactMatch-ing FlowRules, remove from both list,
@@ -135,11 +135,13 @@
.filter(flowRule -> flowRulesToUninstall.stream().anyMatch(flowRule::exactMatch))
.collect(Collectors.toList());
- flowRulesToUninstall.removeAll(dontUninstall);
+ flowRulesToUninstall.removeAll(flowRuleToModify);
flowRulesToUninstall.removeAll(dontTouch);
+ flowRulesToInstall.removeAll(flowRuleToModify);
flowRulesToInstall.removeAll(dontTouch);
+ flowRuleToModify.removeAll(dontTouch);
- if (flowRulesToInstall.isEmpty() && flowRulesToUninstall.isEmpty()) {
+ if (flowRulesToInstall.isEmpty() && flowRulesToUninstall.isEmpty() && flowRuleToModify.isEmpty()) {
// There is no flow rules to install/uninstall
intentInstallCoordinator.intentInstallSuccess(context);
return;
@@ -148,6 +150,8 @@
FlowRuleOperations.Builder builder = FlowRuleOperations.builder();
// Add flows
flowRulesToInstall.forEach(builder::add);
+ // Modify flows
+ flowRuleToModify.forEach(builder::modify);
// Remove flows
flowRulesToUninstall.forEach(builder::remove);
diff --git a/core/net/src/test/java/org/onosproject/net/intent/impl/installer/FlowRuleIntentInstallerTest.java b/core/net/src/test/java/org/onosproject/net/intent/impl/installer/FlowRuleIntentInstallerTest.java
index 09f48ca..f979f27 100644
--- a/core/net/src/test/java/org/onosproject/net/intent/impl/installer/FlowRuleIntentInstallerTest.java
+++ b/core/net/src/test/java/org/onosproject/net/intent/impl/installer/FlowRuleIntentInstallerTest.java
@@ -212,6 +212,7 @@
assertEquals(0, flowRuleService.flowRulesRemove.size());
assertEquals(0, flowRuleService.flowRulesAdd.size());
+ assertEquals(0, flowRuleService.flowRulesModify.size());
}
/**
@@ -242,6 +243,7 @@
assertEquals(0, flowRuleService.flowRulesRemove.size());
assertEquals(0, flowRuleService.flowRulesAdd.size());
+ assertEquals(0, flowRuleService.flowRulesModify.size());
}
/**
@@ -261,6 +263,7 @@
assertEquals(0, flowRuleService.flowRulesRemove.size());
assertEquals(0, flowRuleService.flowRulesAdd.size());
+ assertEquals(0, flowRuleService.flowRulesModify.size());
}
/**
@@ -290,6 +293,41 @@
}
/**
+ * Test intents with same match rules, should do modify instead of add.
+ */
+ @Test
+ public void testRuleModify() {
+ List<Intent> intentsToInstall = createFlowRuleIntents();
+ List<Intent> intentsToUninstall = createFlowRuleIntentsWithSameMatch();
+
+ IntentData toInstall = new IntentData(createP2PIntent(),
+ IntentState.INSTALLING,
+ new WallClockTimestamp());
+ toInstall = new IntentData(toInstall, intentsToInstall);
+ IntentData toUninstall = new IntentData(createP2PIntent(),
+ IntentState.INSTALLED,
+ new WallClockTimestamp());
+ toUninstall = new IntentData(toUninstall, intentsToUninstall);
+
+ IntentOperationContext<FlowRuleIntent> operationContext;
+ IntentInstallationContext context = new IntentInstallationContext(toUninstall, toInstall);
+ operationContext = new IntentOperationContext(intentsToUninstall, intentsToInstall, context);
+
+ installer.apply(operationContext);
+
+ IntentOperationContext successContext = intentInstallCoordinator.successContext;
+ assertEquals(successContext, operationContext);
+
+ assertEquals(0, flowRuleService.flowRulesRemove.size());
+ assertEquals(0, flowRuleService.flowRulesAdd.size());
+ assertEquals(1, flowRuleService.flowRulesModify.size());
+
+ FlowRuleIntent installedIntent = (FlowRuleIntent) intentsToInstall.get(0);
+ assertEquals(flowRuleService.flowRulesModify.size(), installedIntent.flowRules().size());
+ assertTrue(flowRuleService.flowRulesModify.containsAll(installedIntent.flowRules()));
+ }
+
+ /**
* Generates FlowRuleIntents for test.
*
* @return the FlowRuleIntents for test
@@ -327,6 +365,45 @@
}
/**
+ * Generates FlowRuleIntents for test. Flow rules in Intent should have same
+ * match as we created by createFlowRuleIntents method, but action will be
+ * different.
+ *
+ * @return the FlowRuleIntents for test
+ */
+ public List<Intent> createFlowRuleIntentsWithSameMatch() {
+ TrafficSelector selector = DefaultTrafficSelector.builder()
+ .matchInPhyPort(CP1.port())
+ .build();
+ TrafficTreatment treatment = DefaultTrafficTreatment.builder()
+ .punt()
+ .build();
+
+ FlowRule flowRule = DefaultFlowRule.builder()
+ .forDevice(CP1.deviceId())
+ .withSelector(selector)
+ .withTreatment(treatment)
+ .fromApp(APP_ID)
+ .withPriority(DEFAULT_PRIORITY)
+ .makePermanent()
+ .build();
+
+ List<NetworkResource> resources = ImmutableList.of(CP1.deviceId());
+
+ FlowRuleIntent intent = new FlowRuleIntent(APP_ID,
+ KEY1,
+ ImmutableList.of(flowRule),
+ resources,
+ PathIntent.ProtectionType.PRIMARY,
+ RG1);
+
+ List<Intent> flowRuleIntents = Lists.newArrayList();
+ flowRuleIntents.add(intent);
+
+ return flowRuleIntents;
+ }
+
+ /**
* Generates another different FlowRuleIntents for test.
*
* @return the FlowRuleIntents for test
@@ -370,6 +447,7 @@
class TestFlowRuleService extends FlowRuleServiceAdapter {
Set<FlowRule> flowRulesAdd = Sets.newHashSet();
+ Set<FlowRule> flowRulesModify = Sets.newHashSet();
Set<FlowRule> flowRulesRemove = Sets.newHashSet();
public void record(FlowRuleOperations ops) {
@@ -384,6 +462,8 @@
case REMOVE:
flowRulesRemove.add(op.rule());
break;
+ case MODIFY:
+ flowRulesModify.add(op.rule());
default:
break;
}
diff --git a/core/store/dist/src/main/java/org/onosproject/store/flow/impl/DistributedFlowRuleStore.java b/core/store/dist/src/main/java/org/onosproject/store/flow/impl/DistributedFlowRuleStore.java
index e2d485f..73db656 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/flow/impl/DistributedFlowRuleStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/flow/impl/DistributedFlowRuleStore.java
@@ -436,11 +436,10 @@
return Tools.allOf(operation.getOperations().stream().map(op -> {
switch (op.operator()) {
case ADD:
+ case MODIFY:
return addBatchEntry(op).thenApply(succeeded -> succeeded ? op : null);
case REMOVE:
return removeBatchEntry(op).thenApply(succeeded -> succeeded ? op : null);
- case MODIFY:
- return CompletableFuture.<FlowRuleBatchEntry>completedFuture(null);
default:
log.warn("Unknown flow operation operator: {}", op.operator());
return CompletableFuture.<FlowRuleBatchEntry>completedFuture(null);