From ffe69743e02b3e128ec1d29c05392bc612b28bbc Mon Sep 17 00:00:00 2001 From: Andreas Pantelopoulos Date: Tue, 20 Mar 2018 13:58:49 -0700 Subject: [PATCH] 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 --- .../segmentrouting/SegmentRoutingManager.java | 50 ++- .../segmentrouting/SegmentRoutingService.java | 6 +- .../cli/PseudowireAddCommand.java | 18 +- .../cli/PseudowireRemoveCommand.java | 12 +- .../pwaas/DefaultL2TunnelHandler.java | 316 ++---------------- .../segmentrouting/pwaas/L2TunnelHandler.java | 38 ++- .../segmentrouting/pwaas/PwaasUtil.java | 11 +- .../segmentrouting/web/PseudowireCodec.java | 47 +++ .../web/PseudowireWebResource.java | 61 ++-- 9 files changed, 191 insertions(+), 368 deletions(-) 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 88a81cc948..79dd832cdd 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.grouphandler.NextNeighbors; 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 @@ public class SegmentRoutingManager implements SegmentRoutingService { return l2TunnelHandler.getL2Policies(); } - @Override + @Deprecated public L2TunnelHandler.Result addPseudowiresBulk(List bulkPseudowires) { // get both added and pending pseudowires @@ -580,20 +580,17 @@ public class SegmentRoutingManager implements SegmentRoutingService { Set 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 @@ public class SegmentRoutingManager implements SegmentRoutingService { // 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 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 @@ public class SegmentRoutingManager implements SegmentRoutingService { 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 c329d3da21..b6887261b6 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 @@ public interface SegmentRoutingService { 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 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 4e2c4b797a..277a1c3fda 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 @@ public class PseudowireAddCommand extends AbstractShellCommand { 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 305f0be210..362ebbbada 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 @@ public class PseudowireRemoveCommand extends AbstractShellCommand { 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 f7c64f648a..faabdfe7af 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 @@ public class DefaultL2TunnelHandler implements L2TunnelHandler { } 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 @@ public class DefaultL2TunnelHandler implements L2TunnelHandler { * @return The devices on the path with the order they * are traversed. */ - List getDevicesOnPath(List path) { + private List getDevicesOnPath(List path) { // iterate over links and get all devices in the order // we find them @@ -534,7 +534,7 @@ public class DefaultL2TunnelHandler implements L2TunnelHandler { * @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 @@ public class DefaultL2TunnelHandler implements L2TunnelHandler { // 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 @@ public class DefaultL2TunnelHandler implements L2TunnelHandler { 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 @@ public class DefaultL2TunnelHandler implements L2TunnelHandler { 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 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 @@ public class DefaultL2TunnelHandler implements L2TunnelHandler { 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 @@ public class DefaultL2TunnelHandler implements L2TunnelHandler { 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 @@ public class DefaultL2TunnelHandler implements L2TunnelHandler { 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 @@ public class DefaultL2TunnelHandler implements L2TunnelHandler { 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 @@ public class DefaultL2TunnelHandler implements L2TunnelHandler { 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 @@ public class DefaultL2TunnelHandler implements L2TunnelHandler { 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 @@ public class DefaultL2TunnelHandler implements L2TunnelHandler { 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); @@ -718,248 +722,6 @@ public class DefaultL2TunnelHandler implements L2TunnelHandler { return Result.SUCCESS; } - /** - * To deploy a number of pseudo wires. - * - * @param pwToAdd the set of pseudo wires to add - */ - public void deploy(Set 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. - *

- * 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. - *

- * Meant to call asynchronously for various events, thus this call can not block and need - * to perform asynchronous operations. - *

- * 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 fwdInitNextFuture = new CompletableFuture<>(); - CompletableFuture revInitNextFuture = new CompletableFuture<>(); - CompletableFuture fwdTermNextFuture = new CompletableFuture<>(); - CompletableFuture revTermNextFuture = new CompletableFuture<>(); - CompletableFuture fwdPwFuture = new CompletableFuture<>(); - CompletableFuture 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 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. @@ -967,7 +729,8 @@ public class DefaultL2TunnelHandler implements L2TunnelHandler { * @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 @@ public class DefaultL2TunnelHandler implements L2TunnelHandler { 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 @@ public class DefaultL2TunnelHandler implements L2TunnelHandler { Versioned 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 @@ public class DefaultL2TunnelHandler implements L2TunnelHandler { * @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 pwToRemove) { for (L2TunnelDescription currentL2Tunnel : pwToRemove) { @@ -1099,16 +863,8 @@ public class DefaultL2TunnelHandler implements L2TunnelHandler { 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 @@ public class DefaultL2TunnelHandler implements L2TunnelHandler { * @return the path */ private List 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 paths = srManager.topologyService.getPaths( srManager.topologyService.currentTopology(), srcCp.deviceId(), dstCp.deviceId()); @@ -1713,12 +1463,12 @@ public class DefaultL2TunnelHandler implements L2TunnelHandler { } 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 daabb07f6f..347d7f0e82 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 @@ -52,13 +52,6 @@ public interface L2TunnelHandler { */ List getL2PendingPolicies(); - /** - * Returns a copy of the pending l2 tunnels that exist in the store. - * - * @return The l2 tunnels. - */ - List getL2PendingTunnels(); - /** * Helper function to handle the pw removal. *

@@ -67,9 +60,18 @@ public interface L2TunnelHandler { * 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 pwToRemove); + /** + * Returns a copy of the pending l2 tunnels that exist in the store. + * + * @return The l2 tunnels. + */ + List getL2PendingTunnels(); + /** * Pwaas pipelines. */ @@ -106,27 +108,33 @@ public interface L2TunnelHandler { /** * */ - 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 d17ec10e84..f4a71abef0 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 final class PwaasUtil { } - public static boolean configurationValidity(List pseudowires) { + public static L2TunnelHandler.Result configurationValidity(List pseudowires) { // structures to keep pw information // in order to see if instantiating them will create @@ -462,13 +462,12 @@ public final class PwaasUtil { 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 fba6ff9e05..755cb68768 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.onosproject.segmentrouting.pwaas.L2Mode; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.List; + import static org.onosproject.segmentrouting.pwaas.PwaasUtil.*; /** @@ -47,6 +51,11 @@ public final class PseudowireCodec extends JsonCodec 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); @@ -72,6 +81,44 @@ public final class PseudowireCodec extends JsonCodec return result; } + /** + * 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> 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. * 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 1d8bb76967..e20d282b1c 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.JsonNode; 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 @@ public class PseudowireWebResource extends AbstractWebResource { 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 @@ public class PseudowireWebResource extends AbstractWebResource { } log.debug("Creating pseudowires {} from rest api!", pseudowires); + List> 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 @@ public class PseudowireWebResource extends AbstractWebResource { 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 @@ public class PseudowireWebResource extends AbstractWebResource { 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);