Intents are now removed after being withdrawn.
Change-Id: I7574fe94add00abf58c71c6122bb3dc5aafa0f79
diff --git a/cli/src/main/java/org/onlab/onos/cli/net/IntentPushTestCommand.java b/cli/src/main/java/org/onlab/onos/cli/net/IntentPushTestCommand.java
index 4d597ca..f3c6b41 100644
--- a/cli/src/main/java/org/onlab/onos/cli/net/IntentPushTestCommand.java
+++ b/cli/src/main/java/org/onlab/onos/cli/net/IntentPushTestCommand.java
@@ -15,6 +15,7 @@
*/
package org.onlab.onos.cli.net;
+import com.google.common.collect.Lists;
import org.apache.karaf.shell.commands.Argument;
import org.apache.karaf.shell.commands.Command;
import org.onlab.onos.cli.AbstractShellCommand;
@@ -35,6 +36,7 @@
import org.onlab.packet.Ethernet;
import org.onlab.packet.MacAddress;
+import java.util.List;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
@@ -89,39 +91,44 @@
add = true;
latch = new CountDownLatch(count);
- IntentOperations operations = generateIntents(ingress, egress);
+ List<Intent> operations = generateIntents(ingress, egress);
submitIntents(operations);
add = false;
latch = new CountDownLatch(count);
- operations = generateIntents(ingress, egress);
submitIntents(operations);
service.removeListener(this);
}
- private IntentOperations generateIntents(ConnectPoint ingress, ConnectPoint egress) {
+ private List<Intent> generateIntents(ConnectPoint ingress, ConnectPoint egress) {
TrafficSelector.Builder selector = DefaultTrafficSelector.builder()
.matchEthType(Ethernet.TYPE_IPV4);
TrafficTreatment treatment = DefaultTrafficTreatment.builder().build();
- IntentOperations.Builder ops = IntentOperations.builder();
+ List<Intent> intents = Lists.newArrayList();
for (int i = 1; i <= count; i++) {
TrafficSelector s = selector
.matchEthSrc(MacAddress.valueOf(i))
.build();
- Intent intent = new PointToPointIntent(appId(), s, treatment,
- ingress, egress);
- if (add) {
- ops.addSubmitOperation(intent);
- } else {
- ops.addWithdrawOperation(intent.id());
- }
+ intents.add(new PointToPointIntent(appId(), s, treatment,
+ ingress, egress));
+
}
- return ops.build();
+ return intents;
}
- private void submitIntents(IntentOperations ops) {
+ private void submitIntents(List<Intent> intents) {
+ IntentOperations.Builder builder = IntentOperations.builder();
+ for (Intent intent : intents) {
+ if (add) {
+ builder.addSubmitOperation(intent);
+ } else {
+ builder.addWithdrawOperation(intent.id());
+ }
+ }
+ IntentOperations ops = builder.build();
+
start = System.currentTimeMillis();
service.execute(ops);
try {
diff --git a/core/api/src/main/java/org/onlab/onos/net/intent/IntentStore.java b/core/api/src/main/java/org/onlab/onos/net/intent/IntentStore.java
index a2248a2..3af59c1 100644
--- a/core/api/src/main/java/org/onlab/onos/net/intent/IntentStore.java
+++ b/core/api/src/main/java/org/onlab/onos/net/intent/IntentStore.java
@@ -42,9 +42,8 @@
* Removes the specified intent from the inventory.
*
* @param intentId intent identification
- * @return removed state transition event or null if intent was not found
*/
- IntentEvent removeIntent(IntentId intentId);
+ void removeIntent(IntentId intentId);
/**
* Returns the number of intents in the store.
@@ -103,8 +102,6 @@
*/
List<Intent> getInstallableIntents(IntentId intentId);
- // TODO: this should be triggered from with the store as a result of removeIntent call
-
/**
* Removes any installable intents which resulted from compilation of the
* specified original intent.
diff --git a/core/net/src/main/java/org/onlab/onos/net/intent/impl/IntentManager.java b/core/net/src/main/java/org/onlab/onos/net/intent/impl/IntentManager.java
index fb6aa40..da2fe70 100644
--- a/core/net/src/main/java/org/onlab/onos/net/intent/impl/IntentManager.java
+++ b/core/net/src/main/java/org/onlab/onos/net/intent/impl/IntentManager.java
@@ -694,6 +694,10 @@
if (event != null) {
eventDispatcher.post(event);
}
+
+ if (newState == WITHDRAWN) {
+ store.removeIntent(intent.id());
+ }
}
Map<Intent, IntentState> stateMap() {
diff --git a/core/net/src/test/java/org/onlab/onos/net/intent/impl/IntentManagerTest.java b/core/net/src/test/java/org/onlab/onos/net/intent/impl/IntentManagerTest.java
index dc7383b..20240d4 100644
--- a/core/net/src/test/java/org/onlab/onos/net/intent/impl/IntentManagerTest.java
+++ b/core/net/src/test/java/org/onlab/onos/net/intent/impl/IntentManagerTest.java
@@ -155,7 +155,8 @@
listener.setLatch(1, Type.WITHDRAWN);
service.withdraw(intent);
listener.await(Type.WITHDRAWN);
- assertEquals(1L, service.getIntentCount());
+ delay(10); //FIXME this is a race
+ assertEquals(0L, service.getIntentCount());
assertEquals(0L, flowRuleService.getFlowRuleCount());
}
@@ -176,7 +177,8 @@
listener.await(Type.INSTALLED);
listener.await(Type.WITHDRAWN);
- assertEquals(1L, service.getIntentCount());
+ delay(10); //FIXME this is a race
+ assertEquals(0L, service.getIntentCount());
assertEquals(0L, flowRuleService.getFlowRuleCount());
}
@@ -198,7 +200,8 @@
service.replace(intent.id(), intent2);
listener.await(Type.WITHDRAWN);
listener.await(Type.INSTALLED);
- assertEquals(2L, service.getIntentCount());
+ delay(10); //FIXME this is a race
+ assertEquals(1L, service.getIntentCount());
assertEquals(1L, manager.flowRuleService.getFlowRuleCount());
assertEquals(intent2.number().intValue(),
flowRuleService.flows.iterator().next().priority());
diff --git a/core/store/dist/src/main/java/org/onlab/onos/store/intent/impl/DistributedIntentStore.java b/core/store/dist/src/main/java/org/onlab/onos/store/intent/impl/DistributedIntentStore.java
index 5974b62..8da72cf 100644
--- a/core/store/dist/src/main/java/org/onlab/onos/store/intent/impl/DistributedIntentStore.java
+++ b/core/store/dist/src/main/java/org/onlab/onos/store/intent/impl/DistributedIntentStore.java
@@ -50,6 +50,7 @@
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
+import static com.google.common.base.Preconditions.checkState;
import static org.onlab.onos.net.intent.IntentState.*;
import static org.slf4j.LoggerFactory.getLogger;
import static org.onlab.metrics.MetricsUtil.*;
@@ -174,20 +175,15 @@
}
@Override
- public IntentEvent removeIntent(IntentId intentId) {
+ public void removeIntent(IntentId intentId) {
Context timer = startTimer(removeIntentTimer);
+ checkState(getIntentState(intentId) == WITHDRAWN,
+ "Intent state for {} is not WITHDRAWN.", intentId);
try {
- Intent intent = intents.remove(intentId);
- installable.remove(intentId);
- if (intent == null) {
- // was already removed
- return null;
- }
- IntentEvent event = this.setState(intent, WITHDRAWN);
+ intents.remove(intentId);
states.remove(intentId);
transientStates.remove(intentId);
- // TODO: Should we callremoveInstalledIntents if this Intent was
- return event;
+ installable.remove(intentId);
} finally {
stopTimer(timer);
}
diff --git a/core/store/dist/src/main/java/org/onlab/onos/store/intent/impl/HazelcastIntentStore.java b/core/store/dist/src/main/java/org/onlab/onos/store/intent/impl/HazelcastIntentStore.java
index 001ab7e..29bf15f 100644
--- a/core/store/dist/src/main/java/org/onlab/onos/store/intent/impl/HazelcastIntentStore.java
+++ b/core/store/dist/src/main/java/org/onlab/onos/store/intent/impl/HazelcastIntentStore.java
@@ -52,6 +52,7 @@
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
+import static com.google.common.base.Preconditions.checkState;
import static org.onlab.onos.net.intent.IntentState.*;
import static org.slf4j.LoggerFactory.getLogger;
import static org.onlab.metrics.MetricsUtil.*;
@@ -177,20 +178,15 @@
}
@Override
- public IntentEvent removeIntent(IntentId intentId) {
+ public void removeIntent(IntentId intentId) {
Context timer = startTimer(removeIntentTimer);
+ checkState(getIntentState(intentId) == WITHDRAWN,
+ "Intent state for {} is not WITHDRAWN.", intentId);
try {
- Intent intent = intents.remove(intentId);
+ intents.remove(intentId);
installable.remove(intentId);
- if (intent == null) {
- // was already removed
- return null;
- }
- IntentEvent event = this.setState(intent, WITHDRAWN);
states.remove(intentId);
transientStates.remove(intentId);
- // TODO: Should we callremoveInstalledIntents if this Intent was
- return event;
} finally {
stopTimer(timer);
}
diff --git a/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/SimpleIntentStore.java b/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/SimpleIntentStore.java
index 5a91276..51ee166 100644
--- a/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/SimpleIntentStore.java
+++ b/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/SimpleIntentStore.java
@@ -33,6 +33,7 @@
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
+import static com.google.common.base.Preconditions.checkState;
import static org.onlab.onos.net.intent.IntentState.WITHDRAWN;
import static org.slf4j.LoggerFactory.getLogger;
@@ -68,16 +69,12 @@
}
@Override
- public IntentEvent removeIntent(IntentId intentId) {
- Intent intent = intents.remove(intentId);
+ public void removeIntent(IntentId intentId) {
+ checkState(getIntentState(intentId) == WITHDRAWN,
+ "Intent state for {} is not WITHDRAWN.", intentId);
+ intents.remove(intentId);
installable.remove(intentId);
- if (intent == null) {
- // was already removed
- return null;
- }
- IntentEvent event = this.setState(intent, WITHDRAWN);
states.remove(intentId);
- return event;
}
@Override