Issue in flow programming: 76/76/1
authoraboch <aboch@cisco.com>
Wed, 27 Mar 2013 16:57:49 +0000 (09:57 -0700)
committeraboch <aboch@cisco.com>
Wed, 27 Mar 2013 17:38:58 +0000 (10:38 -0700)
Add a flow, uninstall it, try to remove it. Remove seems to fail from gui behavior.
Try to install the flow. It will fail and uncaught exception in forwardingrulesmanager is printed.
Fix:
Handle invalid flow status toggle request in forwardingrulesmanager.
IForwardingrulesmanager to exposed a per node and per flow name api for toggling the flow status.
Flow.web to evaluate api return status by status code and not by status description,
because a Status with StatusCode.SUCCESS does not imply the description is "Success".

Change-Id: I33567b473828721bd6548ea222f4b7bcf83c0306
Signed-off-by: aboch <aboch@cisco.com>
opendaylight/forwardingrulesmanager/src/main/java/org/opendaylight/controller/forwardingrulesmanager/IForwardingRulesManager.java
opendaylight/forwardingrulesmanager/src/main/java/org/opendaylight/controller/forwardingrulesmanager/internal/ForwardingRulesManagerImpl.java
opendaylight/northbound/flowprogrammer/src/main/java/org/opendaylight/controller/flowprogrammer/northbound/FlowProgrammerNorthbound.java
opendaylight/web/flows/src/main/java/org/opendaylight/controller/flows/web/Flows.java

index 3a1250b..60d7429 100644 (file)
@@ -255,6 +255,17 @@ public interface IForwardingRulesManager {
      * @return the {@code Status} object indicating the result of this action
      */
     public Status toggleStaticFlowStatus(FlowConfig configObject);
+    
+    /**
+     * Toggle the installation status of the specified configured flow
+     * If the flow configuration status is active, this call will
+     * change the flow status to inactive and vice-versa
+     *
+     * @param name for the static flow
+     * @param node on which the flow is attached
+     * @return the {@code Status} object indicating the result of this action
+     */
+    public Status toggleStaticFlowStatus(String name, Node node);
 
     public Map<String, PortGroupConfig> getPortGroupConfigs();
 
index 08d3412..bdaae5b 100644 (file)
@@ -1451,8 +1451,20 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager,
         return status;
     }
 
+
+
+       @Override
+       public Status toggleStaticFlowStatus(String name, Node node) {
+               return toggleStaticFlowStatus(getStaticFlow(name, node));
+       }
+       
     @Override
     public Status toggleStaticFlowStatus(FlowConfig config) {
+       if (config == null) {
+               String msg = "Invalid request: null flow config";
+               log.warn(msg);
+               return new Status(StatusCode.BADREQUEST, msg);
+       }
         // Validity check for api3 entry point
         if (config.isInternalFlow()) {
                String msg = "Invalid operation: Controller generated flow " +
index e4ca83a..ed3abde 100644 (file)
@@ -366,9 +366,7 @@ public class FlowProgrammerNorthbound {
                     + RestMessages.NOFLOW.toString());
         }
 
-        Status status = frm.toggleStaticFlowStatus(new FlowConfig("", name,
-                node, "", "", "", "", "", "", "", "", "", "", "", "", "", "",
-                "", "", "", null));
+        Status status = frm.toggleStaticFlowStatus(staticFlow);
         if (status.isSuccess()) {
             return Response.ok().build();
         }
index 297c99c..7747022 100644 (file)
@@ -43,9 +43,9 @@ import com.google.gson.Gson;
 @RequestMapping("/")
 public class Flows implements IOneWeb {
        private static final UserLevel AUTH_LEVEL = UserLevel.CONTAINERUSER;
-    private final String WEB_NAME = "Flows";
-    private final String WEB_ID = "flows";
-    private final short WEB_ORDER = 2;
+    private static final String WEB_NAME = "Flows";
+    private static final String WEB_ID = "flows";
+    private static final short WEB_ORDER = 2;
 
     public Flows() {
         ServiceHelper.registerGlobalService(IOneWeb.class, this, null);
@@ -77,14 +77,12 @@ public class Flows implements IOneWeb {
         // fetch frm
         IForwardingRulesManager frm = (IForwardingRulesManager) ServiceHelper
                 .getInstance(IForwardingRulesManager.class, "default", this);
-        if (frm == null)
-            return null;
+        if (frm == null) { return null; }
 
         // fetch sm
         ISwitchManager switchManager = (ISwitchManager) ServiceHelper
                 .getInstance(ISwitchManager.class, "default", this);
-        if (switchManager == null)
-            return null;
+        if (switchManager == null) { return null; }
         
         // get static flow list
         List<FlowConfig> staticFlowList = frm.getStaticFlows();
@@ -111,8 +109,7 @@ public class Flows implements IOneWeb {
     public Map<String, Object> getNodePorts() {
         ISwitchManager switchManager = (ISwitchManager) ServiceHelper
                 .getInstance(ISwitchManager.class, "default", this);
-        if (switchManager == null)
-            return null;
+        if (switchManager == null) { return null; }
 
         Map<String, Object> nodes = new HashMap<String, Object>();
         Map<Short, String> port;
@@ -121,7 +118,7 @@ public class Flows implements IOneWeb {
             port = new HashMap<Short, String>(); // new port
             Set<NodeConnector> nodeConnectorSet = node.getNodeConnectors();
 
-            if (nodeConnectorSet != null)
+            if (nodeConnectorSet != null) {
                 for (NodeConnector nodeConnector : nodeConnectorSet) {
                     String nodeConnectorName = ((Name) switchManager
                             .getNodeConnectorProp(nodeConnector,
@@ -130,6 +127,7 @@ public class Flows implements IOneWeb {
                              nodeConnectorName + "("
                              + nodeConnector.getNodeConnectorIDString() + ")");
                 }
+            }
             
             // add ports
             Map<String, Object> entry = new HashMap<String, Object>();
@@ -183,7 +181,7 @@ public class Flows implements IOneWeb {
     @ResponseBody
     public String actionFlow(@RequestParam(required = true) String action,
             @RequestParam(required = false) String body, @RequestParam(required = true) String nodeId) {
-       if (!authorize(UserLevel.NETWORKADMIN)) {
+       if (!isUserAuthorized(UserLevel.NETWORKADMIN)) {
                return "Operation not authorized";
        }
        
@@ -195,19 +193,19 @@ public class Flows implements IOneWeb {
         FlowConfig flow = gson.fromJson(body, FlowConfig.class);
         Node node = Node.fromString(nodeId);
         flow.setNode(node);
-        Status result = null;
+        Status result = new Status(StatusCode.BADREQUEST, "Invalid request");
         if (action.equals("add")) {
             result = frm.addStaticFlow(flow, false);
         }
 
-        return result.getDescription();
+        return (result.isSuccess())? StatusCode.SUCCESS.toString(): result.getDescription();
     }
     
     @RequestMapping(value = "/flow/{nodeId}/{name}", method = RequestMethod.POST)
     @ResponseBody
     public String removeFlow(@PathVariable("nodeId") String nodeId, @PathVariable("name") String name,
                @RequestParam(required = true) String action) {
-       if (!authorize(UserLevel.NETWORKADMIN)) { return "Operation not authorized"; }
+       if (!isUserAuthorized(UserLevel.NETWORKADMIN)) { return "Operation not authorized"; }
        
        IForwardingRulesManager frm = (IForwardingRulesManager) ServiceHelper
                 .getInstance(IForwardingRulesManager.class, "default", this);
@@ -215,39 +213,32 @@ public class Flows implements IOneWeb {
         
         Status result = null;
         Node node = Node.fromString(nodeId);
-        if (node == null) {
-            return null;
-        }
+        if (node == null) { return null; }
         if (action.equals("remove")) {
                result = frm.removeStaticFlow(name, node);
         } else if (action.equals("toggle")) {
-               FlowConfig config = frm.getStaticFlow(name, node);
-               result = frm.toggleStaticFlowStatus(config);
+               result = frm.toggleStaticFlowStatus(name, node);
         } else {
                result = new Status(StatusCode.BADREQUEST, "Unknown action");
         }
         
-        return result.getDescription();
+        return (result.isSuccess())? StatusCode.SUCCESS.toString(): result.getDescription();
     }
     
     /**
-     * Is the operation permitted for the given level
+     * Returns whether the current user's level is same or above
+     * the required authorization level. 
      * 
-     * @param level
+     * @param requiredLevel the authorization level required
      */
-    private boolean authorize(UserLevel level) {
+    private boolean isUserAuthorized(UserLevel requiredLevel) {
        IUserManager userManager = (IUserManager) ServiceHelper
                 .getGlobalInstance(IUserManager.class, this);
-        if (userManager == null) {
-               return false;
-        }
+        if (userManager == null) { return false; }
         
         String username = SecurityContextHolder.getContext().getAuthentication().getName();
         UserLevel userLevel = userManager.getUserLevel(username);
-        if (userLevel.toNumber() <= level.toNumber()) {
-               return true;
-        }
-        return false;
+        return (userLevel.ordinal() <= requiredLevel.ordinal());
     }
 
 }

©2013 OpenDaylight, A Linux Foundation Collaborative Project. All Rights Reserved.
OpenDaylight is a registered trademark of The OpenDaylight Project, Inc.
Linux Foundation and OpenDaylight are registered trademarks of the Linux Foundation.
Linux is a registered trademark of Linus Torvalds.