Fixing Protected P2PIntent Compiler issues
- Register ProtectionConstraint
- Workaround for NPE in P2PIntent Compiler
buildFailoverTreatment sometimes throw NPE,
when the Group was not available by the time building the head-end treatment.
- debug log and cosmetic fixes
This might be related to ONOS-5183
Change-Id: I5ffc78619951fd8c4a35e985b3b849a1702080e8
diff --git a/core/api/src/main/java/org/onosproject/net/group/DefaultGroupDescription.java b/core/api/src/main/java/org/onosproject/net/group/DefaultGroupDescription.java
index 1962e0a..7ef7dfb 100644
--- a/core/api/src/main/java/org/onosproject/net/group/DefaultGroupDescription.java
+++ b/core/api/src/main/java/org/onosproject/net/group/DefaultGroupDescription.java
@@ -203,6 +203,7 @@
.add("type", type)
.add("buckets", buckets)
.add("appId", appId)
+ .add("appCookie", appCookie)
.add("givenGroupId", givenGroupId)
.toString();
}
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/PointToPointIntentCompiler.java b/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/PointToPointIntentCompiler.java
index 0de641a..3b018c3 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/PointToPointIntentCompiler.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/PointToPointIntentCompiler.java
@@ -46,9 +46,11 @@
import org.onosproject.net.group.GroupBuckets;
import org.onosproject.net.group.GroupDescription;
import org.onosproject.net.group.GroupKey;
+import org.onosproject.net.group.GroupListener;
import org.onosproject.net.group.GroupService;
import org.onosproject.net.intent.FlowRuleIntent;
import org.onosproject.net.intent.Intent;
+import org.onosproject.net.intent.IntentCompilationException;
import org.onosproject.net.intent.IntentId;
import org.onosproject.net.intent.PathIntent;
import org.onosproject.net.intent.PointToPointIntent;
@@ -56,6 +58,7 @@
import org.onosproject.net.intent.impl.PathNotFoundException;
import org.onosproject.net.link.LinkService;
import org.onosproject.net.provider.ProviderId;
+import org.slf4j.Logger;
import java.nio.ByteBuffer;
import java.util.ArrayList;
@@ -63,9 +66,14 @@
import java.util.Iterator;
import java.util.List;
import java.util.ListIterator;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
import static java.util.Arrays.asList;
import static org.onosproject.net.DefaultEdgeLink.createEdgeLink;
+import static org.slf4j.LoggerFactory.getLogger;
/**
* An intent compiler for {@link org.onosproject.net.intent.PointToPointIntent}.
@@ -79,7 +87,13 @@
new ProviderId("core", "org.onosproject.core", true);
// TODO: consider whether the default cost is appropriate or not
public static final int DEFAULT_COST = 1;
+
protected static final int PRIORITY = Intent.DEFAULT_INTENT_PRIORITY;
+
+ private static final int GROUP_TIMEOUT = 5;
+
+ private final Logger log = getLogger(getClass());
+
protected boolean erasePrimary = false;
protected boolean eraseBackup = false;
@@ -104,7 +118,7 @@
@Override
public List<Intent> compile(PointToPointIntent intent, List<Intent> installable) {
-
+ log.trace("compiling {} {}", intent, installable);
ConnectPoint ingressPoint = intent.ingressPoint();
ConnectPoint egressPoint = intent.egressPoint();
@@ -121,6 +135,7 @@
// attempt to compute and implement backup path
return createProtectedIntent(ingressPoint, egressPoint, intent, installable);
} catch (PathNotFoundException e) {
+ log.warn("Could not find disjoint Path for {}", intent);
// no disjoint path extant -- maximum one path exists between devices
return createSinglePathIntent(ingressPoint, egressPoint, intent, installable);
}
@@ -155,6 +170,7 @@
ConnectPoint egressPoint,
PointToPointIntent intent,
List<Intent> installable) {
+ log.trace("createProtectedIntent");
DisjointPath path = getDisjointPath(intent, ingressPoint.deviceId(),
egressPoint.deviceId());
@@ -208,10 +224,12 @@
}
}
- intentList.add(createPathIntent(new DefaultPath(PID, links, path.cost(), path.annotations()),
+ intentList.add(createPathIntent(new DefaultPath(PID, links, path.cost(),
+ path.annotations()),
intent, PathIntent.ProtectionType.PRIMARY));
intentList.add(createPathIntent(new DefaultPath(PID, backupLinks, path.backup().cost(),
- path.backup().annotations()), intent, PathIntent.ProtectionType.BACKUP));
+ path.backup().annotations()),
+ intent, PathIntent.ProtectionType.BACKUP));
// Create fast failover flow rule intent or, if it already exists,
// add contents appropriately.
@@ -354,6 +372,7 @@
GroupDescription groupDesc = new DefaultGroupDescription(src.deviceId(), Group.Type.FAILOVER,
groupBuckets, makeGroupKey(intent.id()), null, intent.appId());
+ log.trace("adding failover group {}", groupDesc);
groupService.addGroup(groupDesc);
}
@@ -387,9 +406,75 @@
return flowRules;
}
+
+ /**
+ * Waits for specified group to appear maximum of {@value #GROUP_TIMEOUT} seconds.
+ *
+ * @param deviceId {@link DeviceId}
+ * @param groupKey {@link GroupKey} to wait for.
+ * @return {@link Group}
+ * @throws IntentCompilationException on any error.
+ */
+ private Group waitForGroup(DeviceId deviceId, GroupKey groupKey) {
+ return waitForGroup(deviceId, groupKey, GROUP_TIMEOUT, TimeUnit.SECONDS);
+ }
+
+ /**
+ * Waits for specified group to appear until timeout.
+ *
+ * @param deviceId {@link DeviceId}
+ * @param groupKey {@link GroupKey} to wait for.
+ * @param timeout timeout
+ * @param unit unit of timeout
+ * @return {@link Group}
+ * @throws IntentCompilationException on any error.
+ */
+ private Group waitForGroup(DeviceId deviceId, GroupKey groupKey, long timeout, TimeUnit unit) {
+ Group group = groupService.getGroup(deviceId, groupKey);
+ if (group != null) {
+ return group;
+ }
+
+ final CompletableFuture<Group> future = new CompletableFuture<>();
+ final GroupListener listener = event -> {
+ if (event.subject().deviceId() == deviceId &&
+ event.subject().appCookie().equals(groupKey)) {
+ future.complete(event.subject());
+ return;
+ }
+ };
+
+ groupService.addListener(listener);
+ try {
+ group = groupService.getGroup(deviceId, groupKey);
+ if (group != null) {
+ return group;
+ }
+ return future.get(timeout, unit);
+ } catch (InterruptedException e) {
+ log.debug("Interrupted", e);
+ Thread.currentThread().interrupt();
+ throw new IntentCompilationException("Interrupted", e);
+ } catch (ExecutionException e) {
+ log.debug("ExecutionException", e);
+ throw new IntentCompilationException("ExecutionException caught", e);
+ } catch (TimeoutException e) {
+ // one last try
+ group = groupService.getGroup(deviceId, groupKey);
+ if (group != null) {
+ return group;
+ } else {
+ log.debug("Timeout", e);
+ throw new IntentCompilationException("Timeout", e);
+ }
+ } finally {
+ groupService.removeListener(listener);
+ }
+ }
+
private TrafficTreatment buildFailoverTreatment(DeviceId srcDevice,
GroupKey groupKey) {
- Group group = groupService.getGroup(srcDevice, groupKey);
+ Group group = waitForGroup(srcDevice, groupKey);
TrafficTreatment.Builder tBuilder = DefaultTrafficTreatment.builder();
TrafficTreatment trafficTreatment = tBuilder.group(group.id()).build();
return trafficTreatment;
@@ -509,7 +594,7 @@
private void updateFailoverGroup(PointToPointIntent pointIntent) {
DeviceId deviceId = pointIntent.ingressPoint().deviceId();
GroupKey groupKey = makeGroupKey(pointIntent.id());
- Group group = groupService.getGroup(deviceId, groupKey);
+ Group group = waitForGroup(deviceId, groupKey);
Iterator<GroupBucket> groupIterator = group.buckets().buckets().iterator();
while (groupIterator.hasNext()) {
GroupBucket bucket = groupIterator.next();
diff --git a/core/store/serializers/src/main/java/org/onosproject/store/serializers/KryoNamespaces.java b/core/store/serializers/src/main/java/org/onosproject/store/serializers/KryoNamespaces.java
index 60030ee..64eda0e 100644
--- a/core/store/serializers/src/main/java/org/onosproject/store/serializers/KryoNamespaces.java
+++ b/core/store/serializers/src/main/java/org/onosproject/store/serializers/KryoNamespaces.java
@@ -189,6 +189,7 @@
import org.onosproject.net.intent.constraint.LinkTypeConstraint;
import org.onosproject.net.intent.constraint.ObstacleConstraint;
import org.onosproject.net.intent.constraint.PartialFailureConstraint;
+import org.onosproject.net.intent.constraint.ProtectionConstraint;
import org.onosproject.net.intent.constraint.WaypointConstraint;
import org.onosproject.net.link.DefaultLinkDescription;
import org.onosproject.net.meter.MeterId;
@@ -550,6 +551,7 @@
.register(DiscreteResourceCodec.class)
.register(new ImmutableByteSequenceSerializer(), ImmutableByteSequence.class)
.register(PathIntent.ProtectionType.class)
+ .register(ProtectionConstraint.class)
.build("API");
/**