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/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
index 88a81cc..79dd832 100644
--- a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
+++ b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
@@ -103,9 +103,9 @@
import org.onosproject.segmentrouting.mcast.McastHandler;
import org.onosproject.segmentrouting.mcast.McastRole;
import org.onosproject.segmentrouting.pwaas.DefaultL2Tunnel;
+import org.onosproject.segmentrouting.pwaas.DefaultL2TunnelDescription;
import org.onosproject.segmentrouting.pwaas.DefaultL2TunnelHandler;
import org.onosproject.segmentrouting.pwaas.DefaultL2TunnelPolicy;
-import org.onosproject.segmentrouting.pwaas.DefaultL2TunnelDescription;
import org.onosproject.segmentrouting.pwaas.L2Tunnel;
import org.onosproject.segmentrouting.pwaas.L2TunnelHandler;
@@ -569,7 +569,7 @@
return l2TunnelHandler.getL2Policies();
}
- @Override
+ @Deprecated
public L2TunnelHandler.Result addPseudowiresBulk(List<DefaultL2TunnelDescription> bulkPseudowires) {
// get both added and pending pseudowires
@@ -580,20 +580,17 @@
Set<L2TunnelDescription> newPseudowires = new HashSet(bulkPseudowires);
- // check global validity for all the new pseudowires, if it fails
- // do not add any of them
- log.debug("Verifying set of pseudowires {}", pseudowires);
- boolean res = configurationValidity(pseudowires);
- if (res) {
- log.debug("Pseudowire configuration is valid, deploying pseudowires!");
- l2TunnelHandler.deploy(newPseudowires);
-
- return L2TunnelHandler.Result.SUCCESS;
- } else {
- log.error("Bulk pseudowires {} can not be added, error in global configuration!",
- newPseudowires);
- return L2TunnelHandler.Result.ADDITION_ERROR;
+ L2TunnelHandler.Result retRes = L2TunnelHandler.Result.SUCCESS;
+ L2TunnelHandler.Result res;
+ for (DefaultL2TunnelDescription pw : bulkPseudowires) {
+ res = addPseudowire(pw);
+ if (res != L2TunnelHandler.Result.SUCCESS) {
+ log.error("Pseudowire with id {} can not be instantiated !", res);
+ retRes = res;
+ }
}
+
+ return retRes;
}
@Override
@@ -607,19 +604,15 @@
// add the new pseudowire to the List
newPseudowires.add(l2TunnelDescription);
// validate the new list of pseudowires
- boolean res = configurationValidity(newPseudowires);
- if (res) {
- // deploy a set with ONLY the new pseudowire
- Set<L2TunnelDescription> pwToDeploy = new HashSet<>();
- pwToDeploy.add(l2TunnelDescription);
- l2TunnelHandler.deploy(pwToDeploy);
-
- log.info("Pseudowire with {} deployment started, check log for any errors in this process!",
+ L2TunnelHandler.Result res = configurationValidity(newPseudowires);
+ if (res == L2TunnelHandler.Result.SUCCESS) {
+ log.debug("Pseudowire with {} deployment started, check log for any errors in this process!",
l2TunnelDescription.l2Tunnel().tunnelId());
- return L2TunnelHandler.Result.SUCCESS;
+ return l2TunnelHandler.deployPseudowire(l2TunnelDescription, false);
} else {
- log.error("Pseudowire with {} can not be added!", l2TunnelDescription.l2Tunnel().tunnelId());
- return L2TunnelHandler.Result.ADDITION_ERROR;
+ log.error("Pseudowire with {} can not be added, error in global validity!",
+ l2TunnelDescription.l2Tunnel().tunnelId());
+ return res;
}
}
@@ -638,16 +631,17 @@
if ((pendingPseudowires.size() == 0) && (pseudowires.size() == 0)) {
log.error("Pseudowire with id {} does not exist", pwId);
- return L2TunnelHandler.Result.REMOVAL_ERROR;
+ return L2TunnelHandler.Result.WRONG_PARAMETERS
+ .appendError("Pseudowire does not exist.");
}
if (pendingPseudowires.size() != 0) {
log.info("Remove pseudowire from pending store!");
// will fill when we implement failure mechanism detection.
}
if (pseudowires.size() != 0) {
- l2TunnelHandler.tearDown(new HashSet<>(pseudowires));
log.info("Removal of pseudowire with {} started, check log for any errors in this process!",
pwId);
+ return l2TunnelHandler.tearDownPseudowire(pwId);
}
return L2TunnelHandler.Result.SUCCESS;
diff --git a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingService.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingService.java
index c329d3d..b688726 100644
--- a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingService.java
+++ b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingService.java
@@ -128,11 +128,15 @@
L2TunnelHandler.Result addPseudowire(L2TunnelDescription tunnel);
/**
- * Adss a set of pseudowires.
+ * Adds a set of pseudowires.
+ *
+ *
* @param l2TunnelDescriptions The pseudowires to add.
* @return SUCCESS if ALL pseudowires can be instantiated and are deployed, or a
* a descriptive error otherwise, without deploying any pseudowire.
+ * @deprecated onos-1.12 use addPseudowire instead
*/
+ @Deprecated
L2TunnelHandler.Result addPseudowiresBulk(List<DefaultL2TunnelDescription> l2TunnelDescriptions);
/**
diff --git a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/cli/PseudowireAddCommand.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/cli/PseudowireAddCommand.java
index 4e2c4b7..277a1c3 100644
--- a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/cli/PseudowireAddCommand.java
+++ b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/cli/PseudowireAddCommand.java
@@ -119,14 +119,26 @@
L2TunnelDescription pw = new DefaultL2TunnelDescription(tun, policy);
L2TunnelHandler.Result res = srService.addPseudowire(pw);
switch (res) {
- case ADDITION_ERROR:
- print("Pseudowire could not be added!");
+ case WRONG_PARAMETERS:
+ print("Pseudowire could not be added , error in the parameters : \n\t%s",
+ res.getSpecificError());
+ break;
+ case CONFIGURATION_ERROR:
+ print("Pseudowire could not be added, configuration error : \n\t%s",
+ res.getSpecificError());
+ break;
+ case PATH_NOT_FOUND:
+ print("Pseudowire path not found : \n\t%s",
+ res.getSpecificError());
+ break;
+ case INTERNAL_ERROR:
+ print("Pseudowire could not be added, internal error : \n\t%s",
+ res.getSpecificError());
break;
case SUCCESS:
break;
default:
break;
}
-
}
}
\ No newline at end of file
diff --git a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/cli/PseudowireRemoveCommand.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/cli/PseudowireRemoveCommand.java
index 305f0be..362ebbb 100644
--- a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/cli/PseudowireRemoveCommand.java
+++ b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/cli/PseudowireRemoveCommand.java
@@ -56,11 +56,15 @@
L2TunnelHandler.Result res = mngr.removePseudowire(pwIntId);
switch (res) {
- case REMOVAL_ERROR:
- error("Error in deletion, pseudowire not found!");
+ case WRONG_PARAMETERS:
+ error("Pseudowire could not be removed , wrong parameters: \n\t %s\n",
+ res.getSpecificError());
break;
- case CONFIG_NOT_FOUND:
- error("Could not fetch pseudowire class configuration!");
+ case INTERNAL_ERROR:
+ error("Pseudowire could not be removed, internal error : \n\t %s\n",
+ res.getSpecificError());
+ break;
+ case SUCCESS:
break;
default:
break;
diff --git a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/pwaas/DefaultL2TunnelHandler.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/pwaas/DefaultL2TunnelHandler.java
index f7c64f6..faabdfe 100644
--- a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/pwaas/DefaultL2TunnelHandler.java
+++ b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/pwaas/DefaultL2TunnelHandler.java
@@ -406,7 +406,7 @@
} else {
for (short i = transportVlanUpper; i > transportVlanLower; i--) {
- VlanId vlanToUse = VlanId.vlanId((short) i);
+ VlanId vlanToUse = VlanId.vlanId(i);
if (!vlanStore.contains(vlanToUse)) {
vlanStore.add(vlanToUse);
@@ -428,7 +428,7 @@
* @return The devices on the path with the order they
* are traversed.
*/
- List<DeviceId> getDevicesOnPath(List<Link> path) {
+ private List<DeviceId> getDevicesOnPath(List<Link> path) {
// iterate over links and get all devices in the order
// we find them
@@ -534,7 +534,7 @@
* @param removeFromPending if to remove the pseudowire from the pending store
* @return result of pseudowire deployment
*/
- private Result deployPseudowire(L2TunnelDescription pw, boolean removeFromPending) {
+ public Result deployPseudowire(L2TunnelDescription pw, boolean removeFromPending) {
Result result;
long l2TunnelId;
@@ -543,7 +543,8 @@
// The tunnel id cannot be 0.
if (l2TunnelId == 0) {
log.warn("Tunnel id id must be > 0");
- return Result.ADDITION_ERROR;
+ return Result.WRONG_PARAMETERS
+ .appendError("Tunnel id id must be > 0");
}
// leafSpinePw determines if this is a leaf-leaf
@@ -556,7 +557,8 @@
if (!srManager.deviceConfiguration().isEdgeDevice(cp1.deviceId()) &&
!srManager.deviceConfiguration().isEdgeDevice(cp2.deviceId())) {
log.error("Can not deploy pseudowire from spine to spine!");
- return Result.INTERNAL_ERROR;
+ return Result.WRONG_PARAMETERS
+ .appendError("Can not deploy pseudowire from spine to spine!");
} else if (srManager.deviceConfiguration().isEdgeDevice(cp1.deviceId()) &&
srManager.deviceConfiguration().isEdgeDevice(cp2.deviceId())) {
leafSpinePw = false;
@@ -564,16 +566,17 @@
leafSpinePw = true;
}
} catch (DeviceConfigNotFoundException e) {
- log.error("A device for pseudowire connection points does not exist.");
- return Result.INTERNAL_ERROR;
+ log.error("Device for pseudowire connection points does not exist in the configuration");
+ return Result.INTERNAL_ERROR
+ .appendError("Device for pseudowire connection points does not exist in the configuration");
}
// get path here, need to use the same for fwd and rev direction
List<Link> path = getPath(pw.l2TunnelPolicy().cP1(),
pw.l2TunnelPolicy().cP2());
if (path == null) {
- log.info("Deploying process : No path between the connection points for pseudowire {}", l2TunnelId);
- return WRONG_PARAMETERS;
+ log.error("Deploying process : No path between the connection points for pseudowire {}", l2TunnelId);
+ return PATH_NOT_FOUND.appendError("No path between the connection points for pseudowire!");
}
Link fwdNextHop;
@@ -581,7 +584,7 @@
if (!isValidPath(path, leafSpinePw)) {
log.error("Deploying process : Path for pseudowire {} is not valid",
l2TunnelId);
- return INTERNAL_ERROR;
+ return INTERNAL_ERROR.appendError("Internal error : path for pseudowire is not valid!");
}
// oneHope flag is used to determine if we need to
@@ -616,7 +619,7 @@
egressVlan);
if (result != SUCCESS) {
log.info("Deploying process : Error in deploying pseudowire initiation for CP1");
- return Result.ADDITION_ERROR;
+ return Result.INTERNAL_ERROR.appendError("Error in deploying pseudowire initiation for CP1");
}
// We create the policy.
@@ -628,7 +631,7 @@
result.nextId);
if (result != SUCCESS) {
log.info("Deploying process : Error in deploying pseudowire policy for CP1");
- return Result.ADDITION_ERROR;
+ return Result.INTERNAL_ERROR.appendError("Error in deploying pseudowire policy for CP1");
}
// We terminate the tunnel
@@ -641,8 +644,7 @@
if (result != SUCCESS) {
log.info("Deploying process : Error in deploying pseudowire termination for CP1");
- return Result.ADDITION_ERROR;
-
+ return Result.INTERNAL_ERROR.appendError("Error in deploying pseudowire termination for CP1");
}
log.info("Deploying process : Establishing reverse direction for pseudowire {}", l2TunnelId);
@@ -663,7 +665,8 @@
egressVlan);
if (result != SUCCESS) {
log.info("Deploying process : Error in deploying pseudowire initiation for CP2");
- return Result.ADDITION_ERROR;
+ return Result.INTERNAL_ERROR
+ .appendError("Error in deploying pseudowire initiation for CP2");
}
@@ -675,7 +678,8 @@
result.nextId);
if (result != SUCCESS) {
log.info("Deploying process : Error in deploying policy for CP2");
- return Result.ADDITION_ERROR;
+ return Result.INTERNAL_ERROR
+ .appendError("Deploying process : Error in deploying policy for CP2");
}
result = deployPseudoWireTerm(pw.l2Tunnel(),
@@ -687,13 +691,13 @@
if (result != SUCCESS) {
log.info("Deploying process : Error in deploying pseudowire termination for CP2");
- return Result.ADDITION_ERROR;
+ return Result.INTERNAL_ERROR.appendError("Error in deploying pseudowire termination for CP2");
}
result = manageIntermediateFiltering(pw, leafSpinePw);
if (result != SUCCESS) {
- log.info("Deploying process : Error in installing intermediate rules for tagged transport!");
- return Result.ADDITION_ERROR;
+ log.info("Deploying process : Error in installing intermediate rules for tagged transport");
+ return Result.INTERNAL_ERROR.appendError("Error in installing intermediate rules for tagged transport");
}
log.info("Deploying process : Updating relevant information for pseudowire {}", l2TunnelId);
@@ -719,255 +723,14 @@
}
/**
- * To deploy a number of pseudo wires.
- *
- * @param pwToAdd the set of pseudo wires to add
- */
- public void deploy(Set<L2TunnelDescription> pwToAdd) {
-
- Result result;
-
- for (L2TunnelDescription currentL2Tunnel : pwToAdd) {
- // add pseudowires one by one
- long tunnelId = currentL2Tunnel.l2TunnelPolicy().tunnelId();
- result = deployPseudowire(currentL2Tunnel, false);
- switch (result) {
- case INTERNAL_ERROR:
- log.warn("Could not deploy pseudowire {}, internal error!", tunnelId);
- break;
- case WRONG_PARAMETERS:
- log.warn("Could not deploy pseudowire {}, wrong parameters!", tunnelId);
- break;
- case ADDITION_ERROR:
- log.warn("Could not deploy pseudowire {}, error in populating rules!", tunnelId);
- break;
- default:
- log.info("Pseudowire with {} succesfully deployed!", tunnelId);
- break;
- }
- }
- }
-
- /**
- * Helper function to update a pw.
- * <p>
- * Called upon configuration changes that update existing pseudowires and
- * when links fail. Checking of mastership for CP1 is mandatory because it is
- * called in multiple instances for both cases.
- * <p>
- * Meant to call asynchronously for various events, thus this call can not block and need
- * to perform asynchronous operations.
- * <p>
- * For this reason error checking is omitted.
- *
- * @param oldPw the pseudo wire to remove
- * @param newPw the pseudo wire to add
- */
- /*
- private void updatePw(L2TunnelDescription oldPw,
- L2TunnelDescription newPw) {
- ConnectPoint oldCp1 = oldPw.l2TunnelPolicy().cP1();
- long tunnelId = oldPw.l2Tunnel().tunnelId();
-
- // only determine if the new pseudowire is leaf-spine, because
- // removal process is the same for both leaf-leaf and leaf-spine pws
- boolean newPwSpine;
- try {
- newPwSpine = !srManager.deviceConfiguration().isEdgeDevice(newPw.l2TunnelPolicy().cP1().deviceId()) ||
- !srManager.deviceConfiguration().isEdgeDevice(newPw.l2TunnelPolicy().cP2().deviceId());
- } catch (DeviceConfigNotFoundException e) {
- // if exception is caught treat the new pw as leaf-leaf
- newPwSpine = false;
- }
-
- // copy the variable here because we need to use it in lambda thus it needs to be final
- boolean finalNewPwSpine = newPwSpine;
-
- log.info("Updating pseudowire {}", oldPw.l2Tunnel().tunnelId());
-
- // The async tasks to orchestrate the next and forwarding update
- CompletableFuture<ObjectiveError> fwdInitNextFuture = new CompletableFuture<>();
- CompletableFuture<ObjectiveError> revInitNextFuture = new CompletableFuture<>();
- CompletableFuture<ObjectiveError> fwdTermNextFuture = new CompletableFuture<>();
- CompletableFuture<ObjectiveError> revTermNextFuture = new CompletableFuture<>();
- CompletableFuture<ObjectiveError> fwdPwFuture = new CompletableFuture<>();
- CompletableFuture<ObjectiveError> revPwFuture = new CompletableFuture<>();
-
- // first delete all information from our stores, we can not do it asynchronously
- l2PolicyStore.remove(Long.toString(tunnelId));
-
- // grab the old l2 tunnel from the store, since it carries information which is not exposed
- // to the user configuration and set it to oldPw.
- oldPw.setL2Tunnel(l2TunnelStore.get(Long.toString(tunnelId)).value());
- VlanId transportVlan = l2TunnelStore.get(Long.toString(tunnelId)).value().transportVlan();
- l2TunnelStore.remove(Long.toString(tunnelId));
-
- // remove the reserved transport vlan, if one is used
- if (!transportVlan.equals(UNTAGGED_TRANSPORT_VLAN)) {
- vlanStore.remove(transportVlan);
- }
-
- // First we remove both policy.
- log.debug("Start deleting fwd policy for {}", tunnelId);
- VlanId egressVlan = determineEgressVlan(oldPw.l2TunnelPolicy().cP1OuterTag(),
- oldPw.l2TunnelPolicy().cP1InnerTag(),
- oldPw.l2TunnelPolicy().cP2OuterTag(),
- oldPw.l2TunnelPolicy().cP2InnerTag());
- deletePolicy(tunnelId, oldPw.l2TunnelPolicy().cP1(),
- oldPw.l2TunnelPolicy().cP1InnerTag(),
- oldPw.l2TunnelPolicy().cP1OuterTag(),
- egressVlan,
- fwdInitNextFuture,
- FWD);
-
- deletePolicy(tunnelId, oldPw.l2TunnelPolicy().cP2(),
- oldPw.l2TunnelPolicy().cP2InnerTag(),
- oldPw.l2TunnelPolicy().cP2OuterTag(),
- egressVlan, revInitNextFuture,
- REV);
-
- // Finally we remove both the tunnels.
- fwdInitNextFuture.thenAcceptAsync(status -> {
- if (status == null) {
- log.debug("Update process : Fwd policy removed. " +
- "Now remove fwd {} for {}", INITIATION, tunnelId);
- tearDownPseudoWireInit(tunnelId, oldPw.l2TunnelPolicy().cP1(), fwdTermNextFuture, FWD);
- }
- });
- revInitNextFuture.thenAcceptAsync(status -> {
- if (status == null) {
- log.debug("Update process : Rev policy removed. " +
- "Now remove rev {} for {}", INITIATION, tunnelId);
- tearDownPseudoWireInit(tunnelId, oldPw.l2TunnelPolicy().cP2(), revTermNextFuture, REV);
- }
- });
- fwdTermNextFuture.thenAcceptAsync(status -> {
- if (status == null) {
- log.debug("Update process : Fwd {} removed. " +
- "Now remove fwd {} for {}", INITIATION, TERMINATION, tunnelId);
- tearDownPseudoWireTerm(oldPw.l2Tunnel(), oldPw.l2TunnelPolicy().cP2(), fwdPwFuture, FWD);
- }
- });
- revTermNextFuture.thenAcceptAsync(status -> {
- if (status == null) {
- log.debug("Update process : Rev {} removed. " +
- "Now remove rev {} for {}", INITIATION, TERMINATION, tunnelId);
- tearDownPseudoWireTerm(oldPw.l2Tunnel(), oldPw.l2TunnelPolicy().cP1(), revPwFuture, REV);
- }
- });
-
- // get path here, need to use the same for fwd and rev direction
- List<Link> path = getPath(newPw.l2TunnelPolicy().cP1(),
- newPw.l2TunnelPolicy().cP2());
- if (path == null) {
- log.error("Update process : " +
- "No path between the connection points for pseudowire {}", newPw.l2Tunnel().tunnelId());
- return;
- }
-
- Link fwdNextHop, revNextHop;
- if (!isValidPathSize(path.size())) {
- log.error("Deploying process : Path size for pseudowire should be of one of the following sizes" +
- " = [1, 2, 3, 4], for pseudowire {}",
- newPw.l2Tunnel().tunnelId());
- return;
- }
-
- // spinePw signifies if we have a leaf-spine pw
- // thus only one label should be pushed (that of pw)
- // if size>1 we need to push intermediate labels also.
- boolean oneHope = true;
- if (path.size() > 1) {
- oneHope = false;
- }
- final boolean oneHopeFinal = oneHope;
-
- fwdNextHop = path.get(0);
- revNextHop = reverseLink(path.get(path.size() - 1));
-
- // set new path and transport vlan.
- newPw.l2Tunnel().setPath(path);
- newPw.l2Tunnel().setTransportVlan(determineTransportVlan(newPwSpine));
-
- // At the end we install the updated PW.
- fwdPwFuture.thenAcceptAsync(status -> {
- if (status == null) {
-
- // Upgrade stores and book keeping information, need to move this here
- // cause this call is asynchronous.
- l2PolicyStore.put(Long.toString(tunnelId), newPw.l2TunnelPolicy());
- l2TunnelStore.put(Long.toString(tunnelId), newPw.l2Tunnel());
-
- VlanId egressVlanId = determineEgressVlan(newPw.l2TunnelPolicy().cP1OuterTag(),
- newPw.l2TunnelPolicy().cP1InnerTag(),
- newPw.l2TunnelPolicy().cP2OuterTag(),
- newPw.l2TunnelPolicy().cP2InnerTag());
-
- log.debug("Update process : Deploying new fwd pw for {}", tunnelId);
- Result lamdaResult = deployPseudoWireInit(newPw.l2Tunnel(), newPw.l2TunnelPolicy().cP1(),
- newPw.l2TunnelPolicy().cP2(), FWD,
- fwdNextHop, finalNewPwSpine, oneHopeFinal, egressVlanId);
- if (lamdaResult != SUCCESS) {
- return;
- }
-
- lamdaResult = deployPolicy(tunnelId, newPw.l2TunnelPolicy().cP1(),
- newPw.l2TunnelPolicy().cP1InnerTag(),
- newPw.l2TunnelPolicy().cP1OuterTag(),
- egressVlanId, lamdaResult.nextId);
- if (lamdaResult != SUCCESS) {
- return;
- }
- deployPseudoWireTerm(newPw.l2Tunnel(), newPw.l2TunnelPolicy().cP2(),
- egressVlanId, FWD, finalNewPwSpine, oneHopeFinal);
-
- }
- });
- revPwFuture.thenAcceptAsync(status -> {
- if (status == null) {
-
- log.debug("Update process : Deploying new rev pw for {}", tunnelId);
-
- VlanId egressVlanId = determineEgressVlan(newPw.l2TunnelPolicy().cP2OuterTag(),
- newPw.l2TunnelPolicy().cP2InnerTag(),
- newPw.l2TunnelPolicy().cP1OuterTag(),
- newPw.l2TunnelPolicy().cP1InnerTag());
-
- Result lamdaResult = deployPseudoWireInit(newPw.l2Tunnel(),
- newPw.l2TunnelPolicy().cP2(),
- newPw.l2TunnelPolicy().cP1(),
- REV,
- revNextHop, finalNewPwSpine, oneHopeFinal, egressVlanId);
- if (lamdaResult != SUCCESS) {
- return;
- }
-
- lamdaResult = deployPolicy(tunnelId,
- newPw.l2TunnelPolicy().cP2(),
- newPw.l2TunnelPolicy().cP2InnerTag(),
- newPw.l2TunnelPolicy().cP2OuterTag(),
- egressVlanId,
- lamdaResult.nextId);
- if (lamdaResult != SUCCESS) {
- return;
- }
- deployPseudoWireTerm(newPw.l2Tunnel(),
- newPw.l2TunnelPolicy().cP1(),
- egressVlanId,
- REV, finalNewPwSpine, oneHopeFinal);
- }
- });
- }
- */
-
- /**
* Tears down connection points of pseudowires. We can either tear down both connection points,
* or each one of them.
*
* @param l2TunnelId The tunnel id for this pseudowire.
* @param tearDownFirst Boolean, true if we want to tear down cp1
* @param tearDownSecond Boolean, true if we want to tear down cp2
- * @return
+ * @return Result of tearing down the pseudowire, SUCCESS if everything was ok
+ * WRONG_PARAMETERS or INTERNAL_ERROR otherwise
*/
private Result tearDownConnectionPoints(long l2TunnelId, boolean tearDownFirst, boolean tearDownSecond) {
@@ -979,7 +742,7 @@
if (l2TunnelId == 0) {
log.warn("Removal process : Tunnel id cannot be 0");
- return Result.WRONG_PARAMETERS;
+ return Result.WRONG_PARAMETERS.appendError("Pseudowire id can not be 0.");
}
// check existence of tunnels/policy in the store, if one is missing abort!
@@ -987,7 +750,8 @@
Versioned<L2TunnelPolicy> l2TunnelPolicyVersioned = l2PolicyStore.get(Long.toString(l2TunnelId));
if ((l2TunnelVersioned == null) || (l2TunnelPolicyVersioned == null)) {
log.warn("Removal process : Policy and/or tunnel missing for tunnel id {}", l2TunnelId);
- return Result.REMOVAL_ERROR;
+ return Result.INTERNAL_ERROR
+ .appendError("Policy and/or tunnel missing for pseudowire!");
}
L2TunnelDescription pwToRemove = new DefaultL2TunnelDescription(l2TunnelVersioned.value(),
@@ -1086,11 +850,11 @@
* @return Returns SUCCESS if no error is obeserved or an appropriate
* error on a failure
*/
- private Result tearDownPseudowire(long l2TunnelId) {
+ public Result tearDownPseudowire(long l2TunnelId) {
return tearDownConnectionPoints(l2TunnelId, true, true);
}
- @Override
+ @Deprecated
public void tearDown(Set<L2TunnelDescription> pwToRemove) {
for (L2TunnelDescription currentL2Tunnel : pwToRemove) {
@@ -1099,16 +863,8 @@
log.info("Removing pseudowire {}", tunnelId);
Result result = tearDownPseudowire(tunnelId);
- switch (result) {
- case WRONG_PARAMETERS:
- log.error("Error in supplied parameters for the pseudowire removal with tunnel id {}!",
- tunnelId);
- break;
- case REMOVAL_ERROR:
- log.error("Error in pseudowire removal with tunnel id {}!", tunnelId);
- break;
- default:
- log.info("Pseudowire with tunnel id {} was removed successfully", tunnelId);
+ if (result != Result.SUCCESS) {
+ log.error("Could not remove pseudowire {}!", tunnelId);
}
}
}
@@ -1602,12 +1358,6 @@
* @return the path
*/
private List<Link> getPath(ConnectPoint srcCp, ConnectPoint dstCp) {
- /* TODO We retrieve a set of paths in case of a link failure, what happens
- * if the TopologyService gets the link notification AFTER us and has not updated the paths?
- *
- * TODO This has the potential to act on old topology.
- * Maybe we should make SRManager be a listener on topology events instead raw link events.
- */
Set<Path> paths = srManager.topologyService.getPaths(
srManager.topologyService.currentTopology(),
srcCp.deviceId(), dstCp.deviceId());
@@ -1713,12 +1463,12 @@
}
return;
}
- NextObjective nextObjective = l2InitiationNextObjStore.get(key).value();
// un-comment in case you want to delete groups used by the pw
// however, this will break the update of pseudowires cause the L2 interface group can
// not be deleted (it is referenced by other groups)
/*
+ NextObjective nextObjective = l2InitiationNextObjStore.get(key).value();
ObjectiveContext context = new ObjectiveContext() {
@Override
public void onSuccess(Objective objective) {
diff --git a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/pwaas/L2TunnelHandler.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/pwaas/L2TunnelHandler.java
index daabb07..347d7f0 100644
--- a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/pwaas/L2TunnelHandler.java
+++ b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/pwaas/L2TunnelHandler.java
@@ -53,13 +53,6 @@
List<L2TunnelPolicy> getL2PendingPolicies();
/**
- * Returns a copy of the pending l2 tunnels that exist in the store.
- *
- * @return The l2 tunnels.
- */
- List<L2Tunnel> getL2PendingTunnels();
-
- /**
* Helper function to handle the pw removal.
* <p>
* This method should for the mastership of the device because it is
@@ -67,10 +60,19 @@
* one instance only to program each pseudowire.
*
* @param pwToRemove the pseudo wires to remove
+ * @deprecated onos-1.12 Do not use this method.
*/
+ @Deprecated
void tearDown(Set<L2TunnelDescription> pwToRemove);
/**
+ * Returns a copy of the pending l2 tunnels that exist in the store.
+ *
+ * @return The l2 tunnels.
+ */
+ List<L2Tunnel> getL2PendingTunnels();
+
+ /**
* Pwaas pipelines.
*/
enum Pipeline {
@@ -106,27 +108,33 @@
/**
*
*/
- REMOVAL_ERROR(5, "Can not remove pseudowire from network configuration"),
+ PATH_NOT_FOUND(7, "Could not find valid path between connection points!"),
/**
*
*/
- ADDITION_ERROR(6, "Can not add pseudowire in network configuration"),
-
- /**
- *
- */
- CONFIG_NOT_FOUND(7, "Can not find configuration class for pseudowires");
+ CONFIGURATION_ERROR(8, "Conflicting pseudowire configurations!");
private final int code;
private final String description;
- protected int nextId;
+
+ private String specificError;
+ public int nextId;
Result(int code, String description) {
this.code = code;
this.description = description;
}
+ public Result appendError(String error) {
+ this.specificError = error;
+ return this;
+ }
+
+ public String getSpecificError() {
+ return specificError;
+ }
+
public String getDescription() {
return description;
}
diff --git a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/pwaas/PwaasUtil.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/pwaas/PwaasUtil.java
index d17ec10..f4a71ab 100644
--- a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/pwaas/PwaasUtil.java
+++ b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/pwaas/PwaasUtil.java
@@ -437,7 +437,7 @@
}
- public static boolean configurationValidity(List<L2TunnelDescription> pseudowires) {
+ public static L2TunnelHandler.Result configurationValidity(List<L2TunnelDescription> pseudowires) {
// structures to keep pw information
// in order to see if instantiating them will create
@@ -462,13 +462,12 @@
log.debug("Verifying pseudowire {}", pw);
verifyPseudoWire(pw, labelsUsed, vlanIds, tunIds);
}
+
+ return L2TunnelHandler.Result.SUCCESS;
} catch (Exception e) {
log.error("Caught exception while validating pseudowire : {}", e.getMessage());
- return false;
+ return L2TunnelHandler.Result.CONFIGURATION_ERROR
+ .appendError(e.getMessage());
}
-
- // return true
- return true;
}
-
}
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);