From: aboch Date: Wed, 27 Mar 2013 16:57:49 +0000 (-0700) Subject: Issue in flow programming: X-Git-Tag: releasepom-0.1.0~625^2 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=f402ed42c94ac876a0ffbe8decd16ffba2b76980 Issue in flow programming: 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 --- diff --git a/opendaylight/forwardingrulesmanager/src/main/java/org/opendaylight/controller/forwardingrulesmanager/IForwardingRulesManager.java b/opendaylight/forwardingrulesmanager/src/main/java/org/opendaylight/controller/forwardingrulesmanager/IForwardingRulesManager.java index 3a1250becf..60d74295ee 100644 --- a/opendaylight/forwardingrulesmanager/src/main/java/org/opendaylight/controller/forwardingrulesmanager/IForwardingRulesManager.java +++ b/opendaylight/forwardingrulesmanager/src/main/java/org/opendaylight/controller/forwardingrulesmanager/IForwardingRulesManager.java @@ -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 getPortGroupConfigs(); diff --git a/opendaylight/forwardingrulesmanager/src/main/java/org/opendaylight/controller/forwardingrulesmanager/internal/ForwardingRulesManagerImpl.java b/opendaylight/forwardingrulesmanager/src/main/java/org/opendaylight/controller/forwardingrulesmanager/internal/ForwardingRulesManagerImpl.java index 08d34127a5..bdaae5b28c 100644 --- a/opendaylight/forwardingrulesmanager/src/main/java/org/opendaylight/controller/forwardingrulesmanager/internal/ForwardingRulesManagerImpl.java +++ b/opendaylight/forwardingrulesmanager/src/main/java/org/opendaylight/controller/forwardingrulesmanager/internal/ForwardingRulesManagerImpl.java @@ -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 " + diff --git a/opendaylight/northbound/flowprogrammer/src/main/java/org/opendaylight/controller/flowprogrammer/northbound/FlowProgrammerNorthbound.java b/opendaylight/northbound/flowprogrammer/src/main/java/org/opendaylight/controller/flowprogrammer/northbound/FlowProgrammerNorthbound.java index e4ca83a260..ed3abde0ab 100644 --- a/opendaylight/northbound/flowprogrammer/src/main/java/org/opendaylight/controller/flowprogrammer/northbound/FlowProgrammerNorthbound.java +++ b/opendaylight/northbound/flowprogrammer/src/main/java/org/opendaylight/controller/flowprogrammer/northbound/FlowProgrammerNorthbound.java @@ -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(); } diff --git a/opendaylight/web/flows/src/main/java/org/opendaylight/controller/flows/web/Flows.java b/opendaylight/web/flows/src/main/java/org/opendaylight/controller/flows/web/Flows.java index 297c99c0b2..77470224fb 100644 --- a/opendaylight/web/flows/src/main/java/org/opendaylight/controller/flows/web/Flows.java +++ b/opendaylight/web/flows/src/main/java/org/opendaylight/controller/flows/web/Flows.java @@ -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 staticFlowList = frm.getStaticFlows(); @@ -111,8 +109,7 @@ public class Flows implements IOneWeb { public Map getNodePorts() { ISwitchManager switchManager = (ISwitchManager) ServiceHelper .getInstance(ISwitchManager.class, "default", this); - if (switchManager == null) - return null; + if (switchManager == null) { return null; } Map nodes = new HashMap(); Map port; @@ -121,7 +118,7 @@ public class Flows implements IOneWeb { port = new HashMap(); // new port Set 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 entry = new HashMap(); @@ -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()); } }