Some class of constraints should not be used for link cost evaluation.
ONOS-6021
Current ConnectivityIntentCompiler simply picks first non-negative cost as Link cost value.
Some class of constraints are used to express Path viability or IntentCompiler's behavior.
Those constraints tend to returns fixed arbitrary non-negative link cost, which probably is not the best option to be used as Link cost during path computation.
This patch will:
- Introduce base class for constraints which should not influence Link cost.
- Introduce base class for constraints which should not influence Link cost or Path viability.
- Exclude above classes from link cost computation in ConnectivityIntentCompiler
MarkerConstraint
base class for Constraints,
which is not meant to influence Link cost or Path viability.
PathViablityConstraint
base class for Constraints,
which is not meant to influence Link cost.
Change-Id: Ice8b83a18cfe3bf5a68c25a853667bfaedb2b1a1
diff --git a/core/api/src/main/java/org/onosproject/net/intent/constraint/AsymmetricPathConstraint.java b/core/api/src/main/java/org/onosproject/net/intent/constraint/AsymmetricPathConstraint.java
index c42163b..e2bb6e2 100644
--- a/core/api/src/main/java/org/onosproject/net/intent/constraint/AsymmetricPathConstraint.java
+++ b/core/api/src/main/java/org/onosproject/net/intent/constraint/AsymmetricPathConstraint.java
@@ -16,51 +16,11 @@
package org.onosproject.net.intent.constraint;
import com.google.common.annotations.Beta;
-import org.onosproject.net.Link;
-import org.onosproject.net.Path;
-import org.onosproject.net.intent.Constraint;
-import org.onosproject.net.intent.ResourceContext;
-
-import java.util.Objects;
-
-import static com.google.common.base.MoreObjects.toStringHelper;
/**
* Constraint that serves as a request for asymmetric bi-directional path.
*/
@Beta
-public class AsymmetricPathConstraint implements Constraint {
+public final class AsymmetricPathConstraint extends MarkerConstraint {
- // doesn't use LinkResourceService
- @Override
- public double cost(Link link, ResourceContext context) {
- return 1;
- }
-
- // doesn't use LinkResourceService
- @Override
- public boolean validate(Path path, ResourceContext context) {
- return true;
- }
-
- @Override
- public int hashCode() {
- return Objects.hashCode(true);
- }
-
- @Override
- public boolean equals(Object obj) {
- if (this == obj) {
- return true;
- }
- if (obj == null || getClass() != obj.getClass()) {
- return false;
- }
- return true;
- }
-
- @Override
- public String toString() {
- return toStringHelper(this).toString();
- }
}
diff --git a/core/api/src/main/java/org/onosproject/net/intent/constraint/HashedPathSelectionConstraint.java b/core/api/src/main/java/org/onosproject/net/intent/constraint/HashedPathSelectionConstraint.java
index 3cb120c..2ce6252 100644
--- a/core/api/src/main/java/org/onosproject/net/intent/constraint/HashedPathSelectionConstraint.java
+++ b/core/api/src/main/java/org/onosproject/net/intent/constraint/HashedPathSelectionConstraint.java
@@ -15,42 +15,9 @@
*/
package org.onosproject.net.intent.constraint;
-import org.onosproject.net.Link;
-import org.onosproject.net.Path;
-import org.onosproject.net.intent.Constraint;
-import org.onosproject.net.intent.ResourceContext;
-
/**
* A constraint for intent.hashCode() based path selection.
*/
-public class HashedPathSelectionConstraint implements Constraint {
+public final class HashedPathSelectionConstraint extends MarkerConstraint {
- @Override
- public double cost(Link link, ResourceContext context) {
- return 1;
- }
-
- @Override
- public boolean validate(Path path, ResourceContext context) {
- return true;
- }
-
- @Override
- public int hashCode() {
- return 1;
- }
-
- @Override
- public boolean equals(Object obj) {
- if (this == obj) {
- return true;
- }
-
- return (obj != null && getClass().equals(obj.getClass()));
- }
-
- @Override
- public String toString() {
- return "HashedPathSelectionConstraint";
- }
}
diff --git a/core/api/src/main/java/org/onosproject/net/intent/constraint/MarkerConstraint.java b/core/api/src/main/java/org/onosproject/net/intent/constraint/MarkerConstraint.java
new file mode 100644
index 0000000..bc2c107
--- /dev/null
+++ b/core/api/src/main/java/org/onosproject/net/intent/constraint/MarkerConstraint.java
@@ -0,0 +1,57 @@
+/*
+ * Copyright 2016-present 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.Constraint;
+import org.onosproject.net.intent.ResourceContext;
+
+/**
+ * Abstract Constraint for constraints not intended to influence
+ * individual link cost or path validity.
+ */
+public abstract class MarkerConstraint implements Constraint {
+
+ @Override
+ public final double cost(Link link, ResourceContext context) {
+ // random value, should never be used.
+ return 1.0;
+ }
+
+ @Override
+ public final boolean validate(Path path, ResourceContext context) {
+ return true;
+ }
+
+ @Override
+ public int hashCode() {
+ return this.getClass().hashCode();
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ return obj != null && this.getClass() == obj.getClass();
+ }
+
+ @Override
+ public String toString() {
+ return getClass().getSimpleName().replace("Constraint", "");
+ }
+}
diff --git a/core/api/src/main/java/org/onosproject/net/intent/constraint/PartialFailureConstraint.java b/core/api/src/main/java/org/onosproject/net/intent/constraint/PartialFailureConstraint.java
index 787d126..9be6809 100644
--- a/core/api/src/main/java/org/onosproject/net/intent/constraint/PartialFailureConstraint.java
+++ b/core/api/src/main/java/org/onosproject/net/intent/constraint/PartialFailureConstraint.java
@@ -15,30 +15,15 @@
*/
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.intent.ResourceContext;
/**
* 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 {
- // doesn't use LinkResourceService
- @Override
- public double cost(Link link, ResourceContext context) {
- return 1;
- }
-
- // doesn't use LinkResourceService
- @Override
- public boolean validate(Path path, ResourceContext context) {
- return true;
- }
+public final class PartialFailureConstraint extends MarkerConstraint {
public static boolean intentAllowsPartialFailure(Intent intent) {
if (intent instanceof ConnectivityIntent) {
@@ -50,22 +35,6 @@
}
@Override
- public int hashCode() {
- return 1;
- }
-
- @Override
- public boolean equals(Object obj) {
- if (this == obj) {
- return true;
- }
- if (obj == null || getClass() != obj.getClass()) {
- return false;
- }
- return true;
- }
-
- @Override
public String toString() {
return "PartialFailureConstraint";
}
diff --git a/core/api/src/main/java/org/onosproject/net/intent/constraint/PathViabilityConstraint.java b/core/api/src/main/java/org/onosproject/net/intent/constraint/PathViabilityConstraint.java
new file mode 100644
index 0000000..bb30a5a
--- /dev/null
+++ b/core/api/src/main/java/org/onosproject/net/intent/constraint/PathViabilityConstraint.java
@@ -0,0 +1,35 @@
+/*
+ * Copyright 2017-present 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.intent.Constraint;
+import org.onosproject.net.intent.ResourceContext;
+
+/**
+ * Abstract Constraint for constraints intended to influence
+ * only path viability and not influence individual link cost
+ * during path computation.
+ */
+public abstract class PathViabilityConstraint implements Constraint {
+
+ @Override
+ public final double cost(Link link, ResourceContext context) {
+ // random value, should never be used.
+ return 1.0;
+ }
+
+}
diff --git a/core/api/src/main/java/org/onosproject/net/intent/constraint/ProtectionConstraint.java b/core/api/src/main/java/org/onosproject/net/intent/constraint/ProtectionConstraint.java
index e4dce01..bceebcd 100644
--- a/core/api/src/main/java/org/onosproject/net/intent/constraint/ProtectionConstraint.java
+++ b/core/api/src/main/java/org/onosproject/net/intent/constraint/ProtectionConstraint.java
@@ -17,32 +17,17 @@
package org.onosproject.net.intent.constraint;
import com.google.common.annotations.Beta;
-import org.onosproject.net.Link;
-import org.onosproject.net.Path;
-import org.onosproject.net.intent.Constraint;
import org.onosproject.net.intent.Intent;
import org.onosproject.net.intent.PointToPointIntent;
-import org.onosproject.net.intent.ResourceContext;
/**
* Constraint that determines whether to employ path protection.
*/
@Beta
-public class ProtectionConstraint implements Constraint {
+public final class ProtectionConstraint extends MarkerConstraint {
+
private static final ProtectionConstraint PROTECTION_CONSTRAINT = new ProtectionConstraint();
- // doesn't use LinkResourceService
- @Override
- public double cost(Link link, ResourceContext context) {
- return 1;
- }
-
- // doesn't use LinkResourceService
- @Override
- public boolean validate(Path path, ResourceContext context) {
- return true;
- }
-
/**
* Determines whether to utilize path protection for the given intent.
*
diff --git a/core/api/src/main/java/org/onosproject/net/intent/constraint/WaypointConstraint.java b/core/api/src/main/java/org/onosproject/net/intent/constraint/WaypointConstraint.java
index c768820..a0e78ef 100644
--- a/core/api/src/main/java/org/onosproject/net/intent/constraint/WaypointConstraint.java
+++ b/core/api/src/main/java/org/onosproject/net/intent/constraint/WaypointConstraint.java
@@ -21,7 +21,6 @@
import org.onosproject.net.DeviceId;
import org.onosproject.net.Link;
import org.onosproject.net.Path;
-import org.onosproject.net.intent.Constraint;
import org.onosproject.net.intent.ResourceContext;
import java.util.Collections;
@@ -36,7 +35,7 @@
* Constraint that evaluates elements passed through in order.
*/
@Beta
-public class WaypointConstraint implements Constraint {
+public final class WaypointConstraint extends PathViabilityConstraint {
private final List<DeviceId> waypoints;
@@ -62,13 +61,6 @@
// doesn't use LinkResourceService
@Override
- public double cost(Link link, ResourceContext context) {
- // Always consider the number of hops
- return 1;
- }
-
- // doesn't use LinkResourceService
- @Override
public boolean validate(Path path, ResourceContext context) {
// explicitly call a method not depending on LinkResourceService
return validate(path);
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/ConnectivityIntentCompiler.java b/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/ConnectivityIntentCompiler.java
index 0489c85..5ab65be 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/ConnectivityIntentCompiler.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/ConnectivityIntentCompiler.java
@@ -37,6 +37,8 @@
import org.onosproject.net.intent.IntentExtensionService;
import org.onosproject.net.intent.constraint.BandwidthConstraint;
import org.onosproject.net.intent.constraint.HashedPathSelectionConstraint;
+import org.onosproject.net.intent.constraint.MarkerConstraint;
+import org.onosproject.net.intent.constraint.PathViabilityConstraint;
import org.onosproject.net.intent.impl.PathNotFoundException;
import org.onosproject.net.provider.ProviderId;
import org.onosproject.net.resource.Resource;
@@ -360,7 +362,10 @@
// iterate over all constraints in order and return the weight of
// the first one with fast fail over the first failure
- Iterator<Constraint> it = constraints.iterator();
+ Iterator<Constraint> it = constraints.stream()
+ .filter(c -> !(c instanceof MarkerConstraint))
+ .filter(c -> !(c instanceof PathViabilityConstraint))
+ .iterator();
if (!it.hasNext()) {
return DEFAULT_HOP_WEIGHT;
@@ -369,10 +374,11 @@
double cost = it.next().cost(edge.link(), resourceService::isAvailable);
while (it.hasNext() && cost > 0) {
if (it.next().cost(edge.link(), resourceService::isAvailable) < 0) {
+ // TODO shouldn't this be non-viable?
cost = -1;
}
}
- return new ScalarWeight(cost);
+ return ScalarWeight.toWeight(cost);
}
}