Better error handling in pseudowire implementation.
I refactored the pw handler to return meaningful very specific
errors for failures. As a result, I modified also the cli and rest
api implementations to use these fine grain errors accordingly.
Change-Id: I2429532f747c4560378c40be325b039ca0f5c925
diff --git a/apps/segmentrouting/web/src/main/java/org/onosproject/segmentrouting/web/PseudowireCodec.java b/apps/segmentrouting/web/src/main/java/org/onosproject/segmentrouting/web/PseudowireCodec.java
index fba6ff9..755cb68 100644
--- a/apps/segmentrouting/web/src/main/java/org/onosproject/segmentrouting/web/PseudowireCodec.java
+++ b/apps/segmentrouting/web/src/main/java/org/onosproject/segmentrouting/web/PseudowireCodec.java
@@ -15,7 +15,9 @@
*/
package org.onosproject.segmentrouting.web;
+import com.fasterxml.jackson.databind.node.ArrayNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
+import javafx.util.Pair;
import org.onlab.packet.MplsLabel;
import org.onlab.packet.VlanId;
import org.onosproject.codec.CodecContext;
@@ -28,6 +30,8 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import java.util.List;
+
import static org.onosproject.segmentrouting.pwaas.PwaasUtil.*;
/**
@@ -47,6 +51,11 @@
private static final String SERVICE_DELIM_TAG = "serviceTag";
private static final String PW_LABEL = "pwLabel";
+ // JSON field names for error in return
+ private static final String FAILED_PWS = "failedPws";
+ private static final String FAILED_PW = "pw";
+ private static final String REASON = "reason";
+
private static Logger log = LoggerFactory
.getLogger(PseudowireCodec.class);
@@ -73,6 +82,44 @@
}
/**
+ * Encoded in an Object Node the pseudowire and the specificError it failed.
+ *
+ * @param failedPW The failed pseudowire
+ * @param specificError The specificError it failed
+ * @param context Our context
+ * @return A node containing the information we provided
+ */
+ public ObjectNode encodeError(DefaultL2TunnelDescription failedPW, String specificError,
+ CodecContext context) {
+ ObjectNode result = context.mapper().createObjectNode();
+
+ ObjectNode pw = encode(failedPW, context);
+ result.set(FAILED_PW, pw);
+ result.put(REASON, specificError);
+
+ return result;
+ }
+
+ /**
+ * Returns a JSON containing the failed pseudowires and the reason that its one failed.
+ *
+ * @param failedPws Pairs of pws and reasons.
+ * @param context The context
+ * @return ObjectNode representing the json to return
+ */
+ public ObjectNode encodeFailedPseudowires(
+ List<Pair<DefaultL2TunnelDescription, String>> failedPws,
+ CodecContext context) {
+
+ ArrayNode failedNodes = context.mapper().createArrayNode();
+ failedPws.stream()
+ .forEach(failed -> failedNodes.add(encodeError(failed.getKey(), failed.getValue(), context)));
+ final ObjectNode toReturn = context.mapper().createObjectNode();
+ toReturn.set(FAILED_PWS, failedNodes);
+ return toReturn;
+ }
+
+ /**
* Decodes a json containg a single field with the pseudowire id.
*
* @param json Json to decode.
diff --git a/apps/segmentrouting/web/src/main/java/org/onosproject/segmentrouting/web/PseudowireWebResource.java b/apps/segmentrouting/web/src/main/java/org/onosproject/segmentrouting/web/PseudowireWebResource.java
index 1d8bb76..e20d282 100644
--- a/apps/segmentrouting/web/src/main/java/org/onosproject/segmentrouting/web/PseudowireWebResource.java
+++ b/apps/segmentrouting/web/src/main/java/org/onosproject/segmentrouting/web/PseudowireWebResource.java
@@ -19,6 +19,7 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.node.ArrayNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
+import javafx.util.Pair;
import org.onlab.util.ItemNotFoundException;
import org.onosproject.rest.AbstractWebResource;
import org.onosproject.segmentrouting.SegmentRoutingService;
@@ -116,18 +117,19 @@
return Response.serverError().status(Response.Status.BAD_REQUEST).build();
}
- log.debug("Creating pseudowire {} from rest api!", pseudowire.l2Tunnel().tunnelId());
+ long tunId = pseudowire.l2Tunnel().tunnelId();
+ log.debug("Creating pseudowire {} from rest api!", tunId);
L2TunnelHandler.Result res = srService.addPseudowire(pseudowire);
switch (res) {
- case ADDITION_ERROR:
- log.error("Pseudowire {} could not be added, error in configuration," +
- " please check logs for more details!",
- pseudowire.l2Tunnel().tunnelId());
+ case WRONG_PARAMETERS:
+ case CONFIGURATION_ERROR:
+ case PATH_NOT_FOUND:
+ case INTERNAL_ERROR:
+ log.error("Pseudowire {} could not be added : {}", tunId, res.getSpecificError());
return Response.serverError().status(Response.Status.INTERNAL_SERVER_ERROR).build();
-
case SUCCESS:
- log.debug("Pseudowire {} succesfully deployed!", pseudowire.l2Tunnel().tunnelId());
+ log.info("Pseudowire {} succesfully deployed!", pseudowire.l2Tunnel().tunnelId());
return Response.ok().build();
default:
return Response.ok().build();
@@ -160,20 +162,24 @@
}
log.debug("Creating pseudowires {} from rest api!", pseudowires);
+ List<Pair<DefaultL2TunnelDescription, String>> failed = new ArrayList<>();
- L2TunnelHandler.Result res = srService.addPseudowiresBulk(pseudowires);
- switch (res) {
- case ADDITION_ERROR:
- log.error("Bulk of pseudowires {} could not be added, error in configuration," +
- " please check logs for more details!",
- pseudowires);
- return Response.serverError().status(Response.Status.INTERNAL_SERVER_ERROR).build();
+ for (DefaultL2TunnelDescription pw : pseudowires) {
+ L2TunnelHandler.Result res = srService.addPseudowire(pw);
+ if (!(res == L2TunnelHandler.Result.SUCCESS)) {
+ log.trace("Could not create pseudowire {} : {}", pw.l2Tunnel().tunnelId(), res.getSpecificError());
+ failed.add(new Pair<>(pw, res.getSpecificError()));
+ }
+ }
- case SUCCESS:
- log.debug("Bulk of pseudowires {} succesfully deployed!", pseudowires);
- return Response.ok().build();
- default:
- return Response.ok().build();
+ if (failed.size() == 0) {
+ // all pseudowires were instantiated
+ return Response.ok().build();
+ } else {
+ PseudowireCodec pwCodec = new PseudowireCodec();
+ // some failed, need to report them to user
+ ObjectNode result = pwCodec.encodeFailedPseudowires(failed, this);
+ return Response.serverError().entity(result).build();
}
}
@@ -202,11 +208,10 @@
L2TunnelHandler.Result res = srService.removePseudowire(pseudowireId);
switch (res) {
- case REMOVAL_ERROR:
- log.error("Pseudowire {} could not be removed, error in configuration," +
- " please check logs for more details!",
- pseudowireId);
-
+ case WRONG_PARAMETERS:
+ case INTERNAL_ERROR:
+ log.error("Pseudowire {} could not be removed : {}",
+ pseudowireId, res.getSpecificError());
return Response.noContent().build();
case SUCCESS:
log.debug("Pseudowire {} was removed succesfully!", pseudowireId);
@@ -257,10 +262,10 @@
for (Integer pseudowireId : ids) {
L2TunnelHandler.Result res = srService.removePseudowire(pseudowireId);
switch (res) {
- case REMOVAL_ERROR:
- log.error("Pseudowire {} could not be removed, error in configuration," +
- " please check logs for more details!",
- pseudowireId);
+ case WRONG_PARAMETERS:
+ case INTERNAL_ERROR:
+ log.error("Pseudowire {} could not be removed, internal error : {}",
+ pseudowireId, res.getSpecificError());
break;
case SUCCESS:
log.debug("Pseudowire {} was removed succesfully!", pseudowireId);