ONOS-2513 Fix entire MP2SP intent failing on partial connectivity loss

* Added PartialFailureContraint to MP2SP intent to allow partial connectivity. This means the intent remains installed as long as at least one ingress point can reach the egress point. * Intents with this constraint are recompiled on ObjectiveTracker triggers even if not in FAILED state * MP2SP intent compiler can compute a partial tree if constraint is set * ObjectiveTracker recompiles intents on any link event * SDN-IP MP2SP intents now use PartialFailureConstraint Ported from onos-1.2 branch. Change-Id: I32eaa198fae1dfba021d9251c8f855573f0e1d7d

ONOS-2513 Fix entire MP2SP intent failing on partial connectivity loss
* Added PartialFailureContraint to MP2SP intent to allow partial connectivity. This means the intent remains installed as long as at least one ingress point can reach the egress point. * Intents with this constraint are recompiled on ObjectiveTracker triggers even if not in FAILED state * MP2SP intent compiler can compute a partial tree if constraint is set * ObjectiveTracker recompiles intents on any link event * SDN-IP MP2SP intents now use PartialFailureConstraint Ported from onos-1.2 branch. Change-Id: I32eaa198fae1dfba021d9251c8f855573f0e1d7d
Jonathan Hart · Gerrit Code Review
Commit 96c5a4ab2f7fefcbd83f30c332f8a4ee32dfc5c4 96c5a4ab 1 parent 78b300c7
Showing 9 changed files with 144 additions and 33 deletions
apps/sdnip/src/main/java/org/onosproject/sdnip/IntentSynchronizer.java
apps/sdnip/src/test/java/org/onosproject/sdnip/IntentSyncTest.java
cli/src/main/java/org/onosproject/cli/net/ConnectivityIntentCommand.java
core/api/src/main/java/org/onosproject/net/intent/constraint/PartialFailureConstraint.java
core/net/src/main/java/org/onosproject/net/intent/impl/IntentManager.java
core/net/src/main/java/org/onosproject/net/intent/impl/ObjectiveTracker.java
core/net/src/main/java/org/onosproject/net/intent/impl/compiler/MultiPointToSinglePointIntentCompiler.java
core/net/src/test/java/org/onosproject/net/intent/impl/compiler/MultiPointToSinglePointIntentCompilerTest.java
core/store/serializers/src/main/java/org/onosproject/store/serializers/KryoNamespaces.java
--- a/apps/sdnip/src/main/java/org/onosproject/sdnip/IntentSynchronizer.java
View file @96c5a4a
+++ b/apps/sdnip/src/main/java/org/onosproject/sdnip/IntentSynchronizer.java
View file @96c5a4a
@@ -15,19 +15,8 @@
  */
 package org.onosproject.sdnip;
-import java.util.Collection;
+import com.google.common.collect.ImmutableList;
-import java.util.HashMap;
+import com.google.common.util.concurrent.ThreadFactoryBuilder;
-import java.util.HashSet;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Map;
-import java.util.Objects;
-import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
-import java.util.concurrent.Semaphore;
-
 import org.onlab.packet.Ethernet;
 import org.onlab.packet.IpAddress;
 import org.onlab.packet.IpPrefix;
@@ -40,15 +29,17 @@ import org.onosproject.net.flow.DefaultTrafficSelector;
 import org.onosproject.net.flow.DefaultTrafficTreatment;
 import org.onosproject.net.flow.TrafficSelector;
 import org.onosproject.net.flow.TrafficTreatment;
-import org.onosproject.net.flow.criteria.IPCriterion;
 import org.onosproject.net.flow.criteria.Criterion;
+import org.onosproject.net.flow.criteria.IPCriterion;
 import org.onosproject.net.host.HostService;
+import org.onosproject.net.intent.Constraint;
 import org.onosproject.net.intent.Intent;
 import org.onosproject.net.intent.IntentService;
 import org.onosproject.net.intent.IntentState;
 import org.onosproject.net.intent.Key;
 import org.onosproject.net.intent.MultiPointToSinglePointIntent;
 import org.onosproject.net.intent.PointToPointIntent;
+import org.onosproject.net.intent.constraint.PartialFailureConstraint;
 import org.onosproject.routing.FibListener;
 import org.onosproject.routing.FibUpdate;
 import org.onosproject.routing.IntentRequestListener;
@@ -58,7 +49,18 @@ import org.onosproject.routing.config.RoutingConfigurationService;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import com.google.common.util.concurrent.ThreadFactoryBuilder;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Semaphore;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
@@ -70,6 +72,8 @@ import static com.google.common.base.Preconditions.checkNotNull;
 public class IntentSynchronizer implements FibListener, IntentRequestListener {
     private static final int PRIORITY_OFFSET = 100;
     private static final int PRIORITY_MULTIPLIER = 5;
+    protected static final ImmutableList<Constraint> CONSTRAINTS
+            = ImmutableList.of(new PartialFailureConstraint());
     private static final Logger log =
         LoggerFactory.getLogger(IntentSynchronizer.class);
@@ -375,6 +379,7 @@ public class IntentSynchronizer implements FibListener, IntentRequestListener {
                 .ingressPoints(ingressPorts)
                 .egressPoint(egressPort)
                 .priority(priority)
+                .constraints(CONSTRAINTS)
                 .build();
     }
@@ -425,6 +430,7 @@ public class IntentSynchronizer implements FibListener, IntentRequestListener {
                 .ingressPoints(ingressPoints)
                 .egressPoint(egressPoint)
                 .priority(priority)
+                .constraints(CONSTRAINTS)
                 .build();
         log.trace("Generates ConnectivityInternetToHost intent {}", intent);
@@ -923,6 +929,7 @@ public class IntentSynchronizer implements FibListener, IntentRequestListener {
                 .ingressPoints(ingressPoints)
                 .egressPoint(dstConnectPoint)
                 .priority(priority)
+                .constraints(CONSTRAINTS)
                 .build();
         log.trace("Generates ConnectivityHostToHost = {} ", intent);
@@ -950,6 +957,7 @@ public class IntentSynchronizer implements FibListener, IntentRequestListener {
                         .ingressPoints(ingressPoints)
                         .egressPoint(existingIntent.egressPoint())
                         .priority(existingIntent.priority())
+                        .constraints(CONSTRAINTS)
                         .build();
                 log.trace("Update an existing MultiPointToSinglePointIntent "
--- a/apps/sdnip/src/test/java/org/onosproject/sdnip/IntentSyncTest.java
View file @96c5a4a
+++ b/apps/sdnip/src/test/java/org/onosproject/sdnip/IntentSyncTest.java
View file @96c5a4a
@@ -239,6 +239,7 @@ public class IntentSyncTest extends AbstractIntentTest {
                         .treatment(treatmentBuilder.build())
                         .ingressPoints(ingressPoints)
                         .egressPoint(SW1_ETH1)
+                        .constraints(IntentSynchronizer.CONSTRAINTS)
                         .build();
         // Setup the expected intents
@@ -301,6 +302,7 @@ public class IntentSyncTest extends AbstractIntentTest {
                         .treatment(treatmentBuilder.build())
                         .ingressPoints(ingressPoints)
                         .egressPoint(SW4_ETH1)
+                        .constraints(IntentSynchronizer.CONSTRAINTS)
                         .build();
         // Setup the expected intents
@@ -371,6 +373,7 @@ public class IntentSyncTest extends AbstractIntentTest {
                         .treatment(treatmentBuilderNew.build())
                         .ingressPoints(ingressPointsNew)
                         .egressPoint(SW2_ETH1)
+                        .constraints(IntentSynchronizer.CONSTRAINTS)
                         .build();
         // Set up test expectation
@@ -609,6 +612,7 @@ public class IntentSyncTest extends AbstractIntentTest {
                         .treatment(treatmentBuilder.build())
                         .ingressPoints(ingressPoints)
                         .egressPoint(egressPoint)
+                        .constraints(IntentSynchronizer.CONSTRAINTS)
                         .build();
         return intent;
     }
--- a/cli/src/main/java/org/onosproject/cli/net/ConnectivityIntentCommand.java
View file @96c5a4a
+++ b/cli/src/main/java/org/onosproject/cli/net/ConnectivityIntentCommand.java
View file @96c5a4a
@@ -40,6 +40,7 @@ import org.onosproject.net.intent.Key;
 import org.onosproject.net.intent.constraint.BandwidthConstraint;
 import org.onosproject.net.intent.constraint.LambdaConstraint;
 import org.onosproject.net.intent.constraint.LinkTypeConstraint;
+import org.onosproject.net.intent.constraint.PartialFailureConstraint;
 import org.onosproject.net.resource.link.BandwidthResource;
 import org.onlab.packet.IpPrefix;
 import org.onlab.packet.MacAddress;
@@ -130,6 +131,10 @@ public abstract class ConnectivityIntentCommand extends AbstractShellCommand {
             required = false, multiValued = false)
     private String intentKey = null;
+    @Option(name = "--partial", description = "Allow partial installation",
+            required = false, multiValued = false)
+    private boolean partial = false;
+
     // Treatments
     @Option(name = "--setEthSrc", description = "Rewrite Source MAC Address",
@@ -350,6 +355,10 @@ public abstract class ConnectivityIntentCommand extends AbstractShellCommand {
         }
         constraints.add(new LinkTypeConstraint(lambda, Link.Type.OPTICAL));
+        if (partial) {
+            constraints.add(new PartialFailureConstraint());
+        }
+
         return constraints;
     }
--- a/core/api/src/main/java/org/onosproject/net/intent/constraint/PartialFailureConstraint.java 0 → 100644
View file @96c5a4a
+++ b/core/api/src/main/java/org/onosproject/net/intent/constraint/PartialFailureConstraint.java 0 → 100644
View file @96c5a4a
+/*
+ * 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.constraint;
+
+import org.onosproject.net.Link;
+import org.onosproject.net.Path;
+import org.onosproject.net.intent.ConnectivityIntent;
+import org.onosproject.net.intent.Constraint;
+import org.onosproject.net.intent.Intent;
+import org.onosproject.net.resource.link.LinkResourceService;
+
+/**
+ * A constraint that allows intents that can only be partially compiled
+ * (i.e. MultiPointToSinglePointIntent or SinglePointToMultiPointIntent)
+ * to be installed when some endpoints or paths are not found.
+ */
+public class PartialFailureConstraint implements Constraint {
+    @Override
+    public double cost(Link link, LinkResourceService resourceService) {
+        return 1;
+    }
+
+    @Override
+    public boolean validate(Path path, LinkResourceService resourceService) {
+        return true;
+    }
+
+    public static boolean intentAllowsPartialFailure(Intent intent) {
+        if (intent instanceof ConnectivityIntent) {
+            ConnectivityIntent connectivityIntent = (ConnectivityIntent) intent;
+            return connectivityIntent.constraints().stream()
+                    .anyMatch(c -> c instanceof PartialFailureConstraint);
+        }
+        return false;
+    }
+}
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/IntentManager.java
View file @96c5a4a
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/IntentManager.java
View file @96c5a4a
@@ -63,6 +63,7 @@ import static java.util.concurrent.Executors.newFixedThreadPool;
 import static java.util.concurrent.Executors.newSingleThreadExecutor;
 import static org.onlab.util.Tools.groupedThreads;
 import static org.onosproject.net.intent.IntentState.*;
+import static org.onosproject.net.intent.constraint.PartialFailureConstraint.intentAllowsPartialFailure;
 import static org.onosproject.net.intent.impl.phase.IntentProcessPhase.newInitialPhase;
 import static org.onosproject.security.AppGuard.checkPermission;
 import static org.slf4j.LoggerFactory.getLogger;
@@ -85,6 +86,8 @@ public class IntentManager
     private static final EnumSet<IntentState> RECOMPILE
             = EnumSet.of(INSTALL_REQ, FAILED, WITHDRAW_REQ);
+    private static final EnumSet<IntentState> WITHDRAW
+            = EnumSet.of(WITHDRAW_REQ, WITHDRAWING, WITHDRAWN);
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
     protected CoreService coreService;
@@ -252,19 +255,18 @@ public class IntentManager
             submit(intent);
         }
-        if (compileAllFailed) {
         // If required, compile all currently failed intents.
         for (Intent intent : getIntents()) {
             IntentState state = getIntentState(intent.key());
-                if (RECOMPILE.contains(state)) {
+            if ((compileAllFailed && RECOMPILE.contains(state))
-                    if (state == WITHDRAW_REQ) {
+                    || intentAllowsPartialFailure(intent)) {
+                if (WITHDRAW.contains(state)) {
                     withdraw(intent);
                 } else {
                     submit(intent);
                 }
             }
         }
-        }
         //FIXME
 //        for (ApplicationId appId : batches.keySet()) {
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/ObjectiveTracker.java
View file @96c5a4a
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/ObjectiveTracker.java
View file @96c5a4a
@@ -276,31 +276,28 @@ public class ObjectiveTracker implements ObjectiveTrackerService {
                 delegate.triggerCompile(Collections.emptySet(), true);
             } else {
-                Set<Key> toBeRecompiled = new HashSet<>();
+                Set<Key> intentsToRecompile = new HashSet<>();
-                boolean recompileOnly = true;
+                boolean dontRecompileAllFailedIntents = true;
                 // Scan through the list of reasons and keep accruing all
                 // intents that need to be recompiled.
                 for (Event reason : event.reasons()) {
                     if (reason instanceof LinkEvent) {
                         LinkEvent linkEvent = (LinkEvent) reason;
-                        if (linkEvent.type() == LINK_REMOVED
-                                || (linkEvent.type() == LINK_UPDATED &&
-                                        linkEvent.subject().isDurable())) {
                         final LinkKey linkKey = linkKey(linkEvent.subject());
                         synchronized (intentsByLink) {
                             Set<Key> intentKeys = intentsByLink.get(linkKey);
-                                log.debug("recompile triggered by LinkDown {} {}", linkKey, intentKeys);
+                            log.debug("recompile triggered by LinkEvent {} ({}) for {}",
-                                toBeRecompiled.addAll(intentKeys);
+                                    linkKey, linkEvent.type(), intentKeys);
+                            intentsToRecompile.addAll(intentKeys);
                         }
-                        }
+                        dontRecompileAllFailedIntents = dontRecompileAllFailedIntents &&
-                        recompileOnly = recompileOnly &&
                                 (linkEvent.type() == LINK_REMOVED ||
                                 (linkEvent.type() == LINK_UPDATED &&
                                 linkEvent.subject().isDurable()));
                     }
                 }
-                delegate.triggerCompile(toBeRecompiled, !recompileOnly);
+                delegate.triggerCompile(intentsToRecompile, !dontRecompileAllFailedIntents);
             }
         }
     }
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/MultiPointToSinglePointIntentCompiler.java
View file @96c5a4a
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/MultiPointToSinglePointIntentCompiler.java
View file @96c5a4a
@@ -26,13 +26,14 @@ import org.onosproject.net.ConnectPoint;
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.Link;
 import org.onosproject.net.Path;
+import org.onosproject.net.device.DeviceService;
 import org.onosproject.net.intent.Intent;
 import org.onosproject.net.intent.IntentCompiler;
+import org.onosproject.net.intent.IntentException;
 import org.onosproject.net.intent.IntentExtensionService;
 import org.onosproject.net.intent.LinkCollectionIntent;
 import org.onosproject.net.intent.MultiPointToSinglePointIntent;
 import org.onosproject.net.intent.PointToPointIntent;
-import org.onosproject.net.intent.impl.PathNotFoundException;
 import org.onosproject.net.resource.link.LinkResourceAllocations;
 import org.onosproject.net.topology.PathService;
@@ -42,6 +43,9 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import static org.onosproject.net.intent.constraint.PartialFailureConstraint.intentAllowsPartialFailure;
+
+
 /**
  * An intent compiler for
  * {@link org.onosproject.net.intent.MultiPointToSinglePointIntent}.
@@ -56,6 +60,9 @@ public class MultiPointToSinglePointIntentCompiler
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
     protected PathService pathService;
+    @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
+    protected DeviceService deviceService;
+
     @Activate
     public void activate() {
         intentManager.registerCompiler(MultiPointToSinglePointIntent.class, this);
@@ -72,11 +79,24 @@ public class MultiPointToSinglePointIntentCompiler
         Map<DeviceId, Link> links = new HashMap<>();
         ConnectPoint egressPoint = intent.egressPoint();
+        final boolean allowMissingPaths = intentAllowsPartialFailure(intent);
+        boolean partialTree = false;
+        boolean anyMissingPaths = false;
         for (ConnectPoint ingressPoint : intent.ingressPoints()) {
             if (ingressPoint.deviceId().equals(egressPoint.deviceId())) {
+                if (deviceService.isAvailable(ingressPoint.deviceId())) {
+                    partialTree = true;
+                } else {
+                    anyMissingPaths = true;
+                }
+
                 continue;
             }
+
             Path path = getPath(ingressPoint, intent.egressPoint());
+            if (path != null) {
+                partialTree = true;
+
                 for (Link link : path.links()) {
                     if (links.containsKey(link.src().deviceId())) {
                         // We've already reached the existing tree with the first
@@ -86,9 +106,17 @@ public class MultiPointToSinglePointIntentCompiler
                         links.put(link.src().deviceId(), link);
                         break;
                     }
-
                     links.put(link.src().deviceId(), link);
                 }
+            } else {
+                anyMissingPaths = true;
+            }
+        }
+
+        if (!partialTree) {
+            throw new IntentException("Could not find any paths between ingress and egress points.");
+        } else if (!allowMissingPaths && anyMissingPaths) {
+            throw new IntentException("Missing some paths between ingress and egress ports.");
         }
         Intent result = LinkCollectionIntent.builder()
@@ -111,12 +139,11 @@ public class MultiPointToSinglePointIntentCompiler
      * @param one start of the path
      * @param two end of the path
      * @return Path between the two
-     * @throws org.onosproject.net.intent.impl.PathNotFoundException if a path cannot be found
      */
     private Path getPath(ConnectPoint one, ConnectPoint two) {
         Set<Path> paths = pathService.getPaths(one.deviceId(), two.deviceId());
         if (paths.isEmpty()) {
-            throw new PathNotFoundException(one.elementId(), two.elementId());
+            return null;
         }
         // TODO: let's be more intelligent about this eventually
         return paths.iterator().next();
--- a/core/net/src/test/java/org/onosproject/net/intent/impl/compiler/MultiPointToSinglePointIntentCompilerTest.java
View file @96c5a4a
+++ b/core/net/src/test/java/org/onosproject/net/intent/impl/compiler/MultiPointToSinglePointIntentCompilerTest.java
View file @96c5a4a
@@ -20,8 +20,10 @@ import org.junit.Test;
 import org.onosproject.TestApplicationId;
 import org.onosproject.core.ApplicationId;
 import org.onosproject.net.ConnectPoint;
+import org.onosproject.net.DeviceId;
 import org.onosproject.net.ElementId;
 import org.onosproject.net.Path;
+import org.onosproject.net.device.DeviceServiceAdapter;
 import org.onosproject.net.flow.TrafficSelector;
 import org.onosproject.net.flow.TrafficTreatment;
 import org.onosproject.net.intent.AbstractIntentTest;
@@ -92,6 +94,16 @@ public class MultiPointToSinglePointIntentCompilerTest extends AbstractIntentTes
     }
     /**
+     * Mocks the device service so that a device appears available in the test.
+     */
+    private static class MockDeviceService extends DeviceServiceAdapter {
+        @Override
+        public boolean isAvailable(DeviceId deviceId) {
+            return true;
+        }
+    }
+
+    /**
      * Creates a MultiPointToSinglePoint intent for a group of ingress points
      * and an egress point.
      *
@@ -126,6 +138,7 @@ public class MultiPointToSinglePointIntentCompilerTest extends AbstractIntentTes
         MultiPointToSinglePointIntentCompiler compiler =
                 new MultiPointToSinglePointIntentCompiler();
         compiler.pathService = new MockPathService(hops);
+        compiler.deviceService = new MockDeviceService();
         return compiler;
     }
--- a/core/store/serializers/src/main/java/org/onosproject/store/serializers/KryoNamespaces.java
View file @96c5a4a
+++ b/core/store/serializers/src/main/java/org/onosproject/store/serializers/KryoNamespaces.java
View file @96c5a4a
@@ -157,6 +157,7 @@ import org.onosproject.net.intent.constraint.LambdaConstraint;
 import org.onosproject.net.intent.constraint.LatencyConstraint;
 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.WaypointConstraint;
 import org.onosproject.net.link.DefaultLinkDescription;
 import org.onosproject.net.newresource.DefaultResource;
@@ -412,6 +413,7 @@ public final class KryoNamespaces {
                     ObstacleConstraint.class,
                     AnnotationConstraint.class,
                     BooleanConstraint.class,
+                    PartialFailureConstraint.class,
                     IntentOperation.class,
                     FlowRuleExtPayLoad.class,
                     Frequency.class,