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

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<DefaultL2TunnelDescription> bulkPseudowires) {
 
         // get both added and pending pseudowires
@@ -580,22 +580,19 @@ public class SegmentRoutingManager implements SegmentRoutingService {
 
         Set<L2TunnelDescription> newPseudowires = new HashSet(bulkPseudowires);
 
-        // check global validity for all the new pseudowires, if it fails
+        L2TunnelHandler.Result retRes = L2TunnelHandler.Result.SUCCESS;
-        // do not add any of them
+        L2TunnelHandler.Result res;
-        log.debug("Verifying set of pseudowires {}", pseudowires);
+        for (DefaultL2TunnelDescription pw : bulkPseudowires) {
-        boolean res = configurationValidity(pseudowires);
+            res = addPseudowire(pw);
-        if (res) {
+            if (res != L2TunnelHandler.Result.SUCCESS) {
-            log.debug("Pseudowire configuration is valid, deploying pseudowires!");
+                log.error("Pseudowire with id {} can not be instantiated !", res);
-            l2TunnelHandler.deploy(newPseudowires);
+                retRes = res;
-
-            return L2TunnelHandler.Result.SUCCESS;
-        } else {
-            log.error("Bulk pseudowires {} can not be added, error in global configuration!",
-                     newPseudowires);
-            return L2TunnelHandler.Result.ADDITION_ERROR;
             }
         }
 
+        return retRes;
+    }
+
     @Override
     public L2TunnelHandler.Result addPseudowire(L2TunnelDescription l2TunnelDescription) {
 
@@ -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);
+        L2TunnelHandler.Result res = configurationValidity(newPseudowires);
-        if (res) {
+        if (res == L2TunnelHandler.Result.SUCCESS) {
-            // deploy a set with ONLY the new pseudowire
+            log.debug("Pseudowire with {} deployment started, check log for any errors in this process!",
-            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!",
                      l2TunnelDescription.l2Tunnel().tunnelId());
-            return L2TunnelHandler.Result.SUCCESS;
+            return l2TunnelHandler.deployPseudowire(l2TunnelDescription, false);
         } else {
-            log.error("Pseudowire with {} can not be added!", l2TunnelDescription.l2Tunnel().tunnelId());
+            log.error("Pseudowire with {} can not be added, error in global validity!",
-            return L2TunnelHandler.Result.ADDITION_ERROR;
+                      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;

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<DefaultL2TunnelDescription> l2TunnelDescriptions);
 
     /**

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:
+            case WRONG_PARAMETERS:
-                print("Pseudowire could not be added!");
+                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;
         }
-
     }
 }

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:
+            case WRONG_PARAMETERS:
-                error("Error in deletion, pseudowire not found!");
+                error("Pseudowire could not be removed , wrong parameters: \n\t %s\n",
+                      res.getSpecificError());
                 break;
-            case CONFIG_NOT_FOUND:
+            case INTERNAL_ERROR:
-                error("Could not fetch pseudowire class configuration!");
+                error("Pseudowire could not be removed, internal error : \n\t %s\n",
+                      res.getSpecificError());
+                break;
+            case SUCCESS:
                 break;
             default:
                 break;

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<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 @@ 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.");
+            log.error("Device for pseudowire connection points does not exist in the configuration");
-            return Result.INTERNAL_ERROR;
+            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);
+            log.error("Deploying process : No path between the connection points for pseudowire {}", l2TunnelId);
-            return WRONG_PARAMETERS;
+            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!");
+            log.info("Deploying process : Error in installing intermediate rules for tagged transport");
-            return Result.ADDITION_ERROR;
+            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<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.
@@ -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<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 @@ 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<L2TunnelDescription> 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) {
+            if (result != Result.SUCCESS) {
-                case WRONG_PARAMETERS:
+                log.error("Could not remove pseudowire {}!", tunnelId);
-                    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);
             }
         }
     }
@@ -1602,12 +1358,6 @@ public class DefaultL2TunnelHandler implements L2TunnelHandler {
      * @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 @@ 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) {

apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/pwaas/L2TunnelHandler.java

@@ -52,13 +52,6 @@ public interface L2TunnelHandler {
      */
     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>
@@ -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<L2TunnelDescription> pwToRemove);
 
+    /**
+     * Returns a copy of the pending l2 tunnels that exist in the store.
+     *
+     * @return The l2 tunnels.
+     */
+    List<L2Tunnel> 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"),
+        CONFIGURATION_ERROR(8, "Conflicting pseudowire configurations!");
-
-        /**
-         *
-         */
-        CONFIG_NOT_FOUND(7, "Can not find configuration class for pseudowires");
 
         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;
         }

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<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 @@ 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;
     }
-
 }

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

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:
+            case WRONG_PARAMETERS:
-                log.error("Pseudowire {} could not be added, error in configuration," +
+            case CONFIGURATION_ERROR:
-                                  " please check logs for more details!",
+            case PATH_NOT_FOUND:
-                          pseudowire.l2Tunnel().tunnelId());
+            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<Pair<DefaultL2TunnelDescription, String>> failed = new ArrayList<>();
 
-        L2TunnelHandler.Result res = srService.addPseudowiresBulk(pseudowires);
+        for (DefaultL2TunnelDescription pw : pseudowires) {
-        switch (res) {
+            L2TunnelHandler.Result res = srService.addPseudowire(pw);
-            case ADDITION_ERROR:
+            if (!(res == L2TunnelHandler.Result.SUCCESS)) {
-                log.error("Bulk of pseudowires {} could not be added, error in configuration," +
+                log.trace("Could not create pseudowire {} : {}", pw.l2Tunnel().tunnelId(), res.getSpecificError());
-                                  " please check logs for more details!",
+                failed.add(new Pair<>(pw, res.getSpecificError()));
-                          pseudowires);
+            }
-                return Response.serverError().status(Response.Status.INTERNAL_SERVER_ERROR).build();
+        }
 
-            case SUCCESS:
+        if (failed.size() == 0) {
-                log.debug("Bulk of pseudowires {} succesfully deployed!", pseudowires);
+            // all pseudowires were instantiated
-                return Response.ok().build();
-            default:
             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:
+            case WRONG_PARAMETERS:
-                log.error("Pseudowire {} could not be removed, error in configuration," +
+            case INTERNAL_ERROR:
-                                  " please check logs for more details!",
+                log.error("Pseudowire {} could not be removed : {}",
-                          pseudowireId);
+                          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:
+                case WRONG_PARAMETERS:
-                    log.error("Pseudowire {} could not be removed, error in configuration," +
+                case INTERNAL_ERROR:
-                                      " please check logs for more details!",
+                    log.error("Pseudowire {} could not be removed, internal error : {}",
-                              pseudowireId);
+                              pseudowireId, res.getSpecificError());
                     break;
                 case SUCCESS:
                     log.debug("Pseudowire {} was removed succesfully!", pseudowireId);