[Blackbird] Fixing Intent purge case (ONOS-1207)
Master needs to remove from the current map.
Change-Id: I30eccbe188997949ef2d63d6dbd37b0d8d4b3f5e
diff --git a/cli/src/main/java/org/onosproject/cli/net/IntentRemoveCommand.java b/cli/src/main/java/org/onosproject/cli/net/IntentRemoveCommand.java
index ef42c32..2095a80 100644
--- a/cli/src/main/java/org/onosproject/cli/net/IntentRemoveCommand.java
+++ b/cli/src/main/java/org/onosproject/cli/net/IntentRemoveCommand.java
@@ -85,35 +85,61 @@
Intent intent = intentService.getIntent(key);
if (intent != null) {
- // set up latch and listener to track uninstall progress
- CountDownLatch latch = new CountDownLatch(1);
- IntentListener listener = (IntentEvent event) -> {
- if (Objects.equals(event.subject().key(), key) &&
- (event.type() == IntentEvent.Type.WITHDRAWN ||
- event.type() == IntentEvent.Type.FAILED)) {
- latch.countDown();
- }
- };
- intentService.addListener(listener);
+ IntentListener listener = null;
+ final CountDownLatch withdrawLatch, purgeLatch;
+ if (purgeAfterRemove || sync) {
+ // set up latch and listener to track uninstall progress
+ withdrawLatch = new CountDownLatch(1);
+ purgeLatch = purgeAfterRemove ? new CountDownLatch(1) : null;
+ listener = (IntentEvent event) -> {
+ if (Objects.equals(event.subject().key(), key)) {
+ if (event.type() == IntentEvent.Type.WITHDRAWN ||
+ event.type() == IntentEvent.Type.FAILED) {
+ withdrawLatch.countDown();
+ } else if (purgeAfterRemove &&
+ event.type() == IntentEvent.Type.PURGED) {
+ purgeLatch.countDown();
+ }
+ }
+ };
+ intentService.addListener(listener);
+ } else {
+ purgeLatch = null;
+ withdrawLatch = null;
+ }
// request the withdraw
intentService.withdraw(intent);
if (purgeAfterRemove || sync) {
- try {
- latch.await(5, TimeUnit.SECONDS);
+ try { // wait for withdraw event
+ withdrawLatch.await(5, TimeUnit.SECONDS);
} catch (InterruptedException e) {
- print("Timed out waiting for intent {}", key);
+ print("Timed out waiting for intent {} withdraw", key);
}
// double check the state
IntentState state = intentService.getIntentState(key);
if (purgeAfterRemove && (state == WITHDRAWN || state == FAILED)) {
- intentService.purge(key);
+ intentService.purge(intent);
+ }
+ if (sync) { // wait for purge event
+ /* TODO
+ Technically, the event comes before map.remove() is called.
+ If we depend on sync and purge working together, we will
+ need to address this.
+ */
+ try {
+ purgeLatch.await(5, TimeUnit.SECONDS);
+ } catch (InterruptedException e) {
+ print("Timed out waiting for intent {} purge", key);
+ }
}
}
- // clean up the listener
- intentService.removeListener(listener);
- }
+ if (listener != null) {
+ // clean up the listener
+ intentService.removeListener(listener);
+ }
+ }
}
}
diff --git a/core/api/src/main/java/org/onosproject/net/intent/IntentEvent.java b/core/api/src/main/java/org/onosproject/net/intent/IntentEvent.java
index 3b7c6c6..792787c 100644
--- a/core/api/src/main/java/org/onosproject/net/intent/IntentEvent.java
+++ b/core/api/src/main/java/org/onosproject/net/intent/IntentEvent.java
@@ -46,7 +46,12 @@
/**
* Signifies that an intent has been withdrawn from the system.
*/
- WITHDRAWN
+ WITHDRAWN,
+
+ /**
+ * Signifies that an intent has been purged from the system.
+ */
+ PURGED
}
/**
@@ -110,6 +115,9 @@
case FAILED:
type = Type.FAILED;
break;
+ case PURGE_REQ:
+ type = Type.PURGED;
+ break;
// fallthrough to default from here
case COMPILING:
diff --git a/core/api/src/main/java/org/onosproject/net/intent/IntentService.java b/core/api/src/main/java/org/onosproject/net/intent/IntentService.java
index b86d793..aed7ad0 100644
--- a/core/api/src/main/java/org/onosproject/net/intent/IntentService.java
+++ b/core/api/src/main/java/org/onosproject/net/intent/IntentService.java
@@ -43,6 +43,14 @@
void withdraw(Intent intent);
/**
+ * Purges a specific intent from the system if it is <b>FAILED</b> or
+ * <b>WITHDRAWN</b>. Otherwise, the intent remains in its current state.
+ *
+ * @param intent intent to purge
+ */
+ void purge(Intent intent);
+
+ /**
* Fetches an intent based on its key.
*
* @param key key of the intent
@@ -112,11 +120,4 @@
* @param listener listener to be removed
*/
void removeListener(IntentListener listener);
-
- /**
- * Purges a specific intent from the system if it is FAILED or WITHDRAWN.
- *
- * @param key key of the intent to purge
- */
- void purge(Key key);
}
diff --git a/core/api/src/main/java/org/onosproject/net/intent/IntentState.java b/core/api/src/main/java/org/onosproject/net/intent/IntentState.java
index 73b337c..555fe4b 100644
--- a/core/api/src/main/java/org/onosproject/net/intent/IntentState.java
+++ b/core/api/src/main/java/org/onosproject/net/intent/IntentState.java
@@ -92,5 +92,14 @@
* Signifies that the intent has failed compiling, installing or
* recompiling states.
*/
- FAILED //TODO consider renaming to UNSAT.
+ FAILED, //TODO consider renaming to UNSAT.
+
+ /**
+ * Indicates that the intent should be purged from the database.
+ * <p>
+ * Note: This operation will only be performed if the intent is already
+ * in WITHDRAWN or FAILED.
+ * </p>
+ */
+ PURGE_REQ
}
diff --git a/core/api/src/main/java/org/onosproject/net/intent/IntentStore.java b/core/api/src/main/java/org/onosproject/net/intent/IntentStore.java
index 2bf1ef1..5cfae17 100644
--- a/core/api/src/main/java/org/onosproject/net/intent/IntentStore.java
+++ b/core/api/src/main/java/org/onosproject/net/intent/IntentStore.java
@@ -109,10 +109,4 @@
* @return pending intents
*/
Iterable<Intent> getPending();
-
- /** Purges a specific intent from the system if it is FAILED or WITHDRAWN.
- *
- * @param key key of the intent to purge
- */
- void purge(Key key);
}
diff --git a/core/api/src/test/java/org/onosproject/net/intent/FakeIntentManager.java b/core/api/src/test/java/org/onosproject/net/intent/FakeIntentManager.java
index f232cbc..dca55d1 100644
--- a/core/api/src/test/java/org/onosproject/net/intent/FakeIntentManager.java
+++ b/core/api/src/test/java/org/onosproject/net/intent/FakeIntentManager.java
@@ -186,6 +186,17 @@
}
@Override
+ public void purge(Intent intent) {
+ IntentState currentState = intentStates.get(intent.key());
+ if (currentState == IntentState.WITHDRAWN
+ || currentState == IntentState.FAILED) {
+ intents.remove(intent.key());
+ installables.remove(intent.key());
+ intentStates.remove(intent.key());
+ }
+ }
+
+ @Override
public Set<Intent> getIntents() {
return Collections.unmodifiableSet(new HashSet<>(intents.values()));
}
@@ -230,11 +241,6 @@
listeners.remove(listener);
}
- @Override
- public void purge(Key key) {
- // FIXME implement this
- }
-
private void dispatch(IntentEvent event) {
for (IntentListener listener : listeners) {
listener.event(event);
diff --git a/core/api/src/test/java/org/onosproject/net/intent/IntentServiceAdapter.java b/core/api/src/test/java/org/onosproject/net/intent/IntentServiceAdapter.java
index 2d912eb..3a70fcf 100644
--- a/core/api/src/test/java/org/onosproject/net/intent/IntentServiceAdapter.java
+++ b/core/api/src/test/java/org/onosproject/net/intent/IntentServiceAdapter.java
@@ -33,6 +33,11 @@
}
@Override
+ public void purge(Intent intent) {
+
+ }
+
+ @Override
public Iterable<Intent> getIntents() {
return null;
}
@@ -76,9 +81,4 @@
public void removeListener(IntentListener listener) {
}
-
- @Override
- public void purge(Key key) {
-
- }
}
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/IntentManager.java b/core/net/src/main/java/org/onosproject/net/intent/impl/IntentManager.java
index ed1552a..982aa0c 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/IntentManager.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/IntentManager.java
@@ -151,6 +151,13 @@
}
@Override
+ public void purge(Intent intent) {
+ checkNotNull(intent, INTENT_NULL);
+ IntentData data = new IntentData(intent, IntentState.PURGE_REQ, null);
+ store.addPending(data);
+ }
+
+ @Override
public Intent getIntent(Key key) {
return store.getIntent(key);
}
@@ -193,11 +200,6 @@
}
@Override
- public void purge(Key key) {
- store.purge(key);
- }
-
- @Override
public <T extends Intent> void registerCompiler(Class<T> cls, IntentCompiler<T> compiler) {
compilerRegistry.registerCompiler(cls, compiler);
}
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/phase/IntentProcessPhase.java b/core/net/src/main/java/org/onosproject/net/intent/impl/phase/IntentProcessPhase.java
index 5f7412b..dfa9d11 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/phase/IntentProcessPhase.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/phase/IntentProcessPhase.java
@@ -56,6 +56,8 @@
} else {
return new WithdrawRequest(processor, data, current);
}
+ case PURGE_REQ:
+ return new PurgeRequest(data, current);
default:
// illegal state
return new CompilingFailed(data);
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/phase/PurgeRequest.java b/core/net/src/main/java/org/onosproject/net/intent/impl/phase/PurgeRequest.java
new file mode 100644
index 0000000..b04e05e
--- /dev/null
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/phase/PurgeRequest.java
@@ -0,0 +1,60 @@
+/*
+ * Copyright 2015 Open Networking Laboratory
+ *
+ * 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.net.intent.impl.phase;
+
+import org.onosproject.net.intent.IntentData;
+import org.onosproject.net.intent.IntentState;
+import org.slf4j.Logger;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+import static org.slf4j.LoggerFactory.getLogger;
+
+/**
+ * Represents a phase of requesting a purge of an intent.
+ * <p>
+ * Note: The purge will only succeed if the intent is FAILED or WITHDRAWN.
+ * </p>
+ */
+final class PurgeRequest extends FinalIntentProcessPhase {
+
+ private static final Logger log = getLogger(PurgeRequest.class);
+
+ private final IntentData pending;
+ private final IntentData current;
+
+ PurgeRequest(IntentData intentData, IntentData current) {
+ this.pending = checkNotNull(intentData);
+ this.current = checkNotNull(current);
+ }
+
+ private boolean shouldAcceptPurge() {
+ if (current.state() == IntentState.WITHDRAWN
+ || current.state() == IntentState.FAILED) {
+ return true;
+ }
+ log.info("Purge for intent {} is rejected", pending.key());
+ return false;
+ }
+
+ @Override
+ public IntentData data() {
+ if (shouldAcceptPurge()) {
+ return pending;
+ } else {
+ return current;
+ }
+ }
+}
diff --git a/core/store/dist/src/main/java/org/onosproject/store/ecmap/EventuallyConsistentMapImpl.java b/core/store/dist/src/main/java/org/onosproject/store/ecmap/EventuallyConsistentMapImpl.java
index 2987529..8fe0120 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/ecmap/EventuallyConsistentMapImpl.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/ecmap/EventuallyConsistentMapImpl.java
@@ -412,6 +412,11 @@
peerUpdateFunction.apply(key, value));
notifyListeners(new EventuallyConsistentMapEvent<>(
EventuallyConsistentMapEvent.Type.REMOVE, key, value));
+ } else {
+ // TODO remove this extra call when ONOS-1207 is resolved
+ Timestamped<V> latest = (Timestamped) items.get(key);
+ log.info("Remove of intent {} failed; request time {} vs. latest time {}",
+ key, timestamp, latest.timestamp());
}
}
diff --git a/core/store/dist/src/main/java/org/onosproject/store/intent/impl/GossipIntentStore.java b/core/store/dist/src/main/java/org/onosproject/store/intent/impl/GossipIntentStore.java
index 8ff462e..43e184f 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/intent/impl/GossipIntentStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/intent/impl/GossipIntentStore.java
@@ -224,6 +224,8 @@
}
return true;
+ case PURGE_REQ:
+ return true;
case COMPILING:
case RECOMPILING:
@@ -241,7 +243,11 @@
if (isUpdateAcceptable(currentData, newData)) {
// Only the master is modifying the current state. Therefore assume
// this always succeeds
- currentMap.put(newData.key(), copyData(newData));
+ if (newData.state() == PURGE_REQ) {
+ currentMap.remove(newData.key(), newData);
+ } else {
+ currentMap.put(newData.key(), copyData(newData));
+ }
// if current.put succeeded
pendingMap.remove(newData.key(), newData);
@@ -325,14 +331,6 @@
.collect(Collectors.toList());
}
- @Override
- public void purge(Key key) {
- IntentData data = currentMap.get(key);
- if (data.state() == WITHDRAWN || data.state() == FAILED) {
- currentMap.remove(key, data);
- }
- }
-
private void notifyDelegateIfNotNull(IntentEvent event) {
if (event != null) {
notifyDelegate(event);
diff --git a/core/store/trivial/src/main/java/org/onosproject/store/trivial/impl/SimpleIntentStore.java b/core/store/trivial/src/main/java/org/onosproject/store/trivial/impl/SimpleIntentStore.java
index 25ff1fb..06da986 100644
--- a/core/store/trivial/src/main/java/org/onosproject/store/trivial/impl/SimpleIntentStore.java
+++ b/core/store/trivial/src/main/java/org/onosproject/store/trivial/impl/SimpleIntentStore.java
@@ -180,7 +180,11 @@
IntentData pendingData = pending.get(newData.key());
if (isUpdateAcceptable(currentData, newData)) {
- current.put(newData.key(), copyData(newData));
+ if (pendingData.state() == PURGE_REQ) {
+ current.remove(newData.key(), newData);
+ } else {
+ current.put(newData.key(), copyData(newData));
+ }
if (pendingData != null
// pendingData version is less than or equal to newData's
@@ -253,12 +257,4 @@
.map(IntentData::intent)
.collect(Collectors.toList());
}
-
- @Override
- public void purge(Key key) {
- IntentData data = current.get(key);
- if (data.state() == IntentState.WITHDRAWN || data.state() == IntentState.FAILED) {
- current.remove(key, data);
- }
- }
}