fixup! Add a new sdn controller plugin

Signed-off-by: David Morel <david.morel@vates.tech>

Author: bleader

SOURCES/etc/xapi.d/plugins/sdncontroller.py

@@ -1,6 +1,5 @@
 #!/usr/bin/env python
 # -*- coding: utf-8 -*-
-
 import json
 import re
 
@@ -26,152 +25,136 @@ def log_and_raise_error(code, desc):
 # All functions starting with `parse_` are helper functions compatible with the `Parser` class.
 # Each should accept `args` as input and return either (result, None) on success,
 # or (None, error_message) on failure.
+class Parser:
+    def __init__(self, args):
+        self.args = args
+        self.values = {}
+        self.errors = []
 
+    def parse_bridge(self, args):
+        BRIDGE_REGEX = re.compile(r"\b\w+\d+\b")
+        bridge = args.get("bridge")
 
-def parse_bridge(args):
-    BRIDGE_REGEX = re.compile(r"\b\w+\d+\b")
-    bridge = args.get("bridge")
-
-    if bridge is None:
-        return None, "bridge parameter is missing"
-
-    if not BRIDGE_REGEX.match(bridge):
-        return None, "'{}' is not a valid bridge name".format(bridge)
-
-    return bridge, None
-
-
-def parse_mac(args):
-    MAC_REGEX = re.compile(r"^([0-9A-Fa-f]{2}:){5}([0-9A-Fa-f]{2})$")
-    mac_addr = args.get("mac")
-
-    if mac_addr is None:
-        return None, None # no mac means network wide rule
-
-    if not MAC_REGEX.match(mac_addr) or MAC_REGEX.match(mac_addr).group(0) != mac_addr:
-        return None, "'{}' is not a valid MAC".format(mac_addr)
-
-    return mac_addr, None
-
+        if bridge is None:
+            log_and_raise_error(E_PARSER, "bridge parameter is missing")
 
-def parse_iprange(args):
-    IPRANGE_REGEX = re.compile(
-        r"^((25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}"
-        r"(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)(/\d{1,2})?$"
-    )
-    ip_range = args.get("iprange")
+        if not BRIDGE_REGEX.match(bridge):
+            log_and_raise_error(E_PARSER, "'{}' is not a valid bridge name".format(bridge))
 
-    if ip_range is None:
-        return None, "ip range parameter is missing"
+        return bridge
 
-    if not IPRANGE_REGEX.match(ip_range):
-        return None, "'{}' is not a valid IP range".format(ip_range)
+    def parse_mac(self, args):
+        MAC_REGEX = re.compile(r"^([0-9A-Fa-f]{2}:){5}([0-9A-Fa-f]{2})$")
+        mac_addr = args.get("mac")
 
-    return ip_range, None
+        if mac_addr is None:
+            return None
 
+        if not MAC_REGEX.match(mac_addr) or MAC_REGEX.match(mac_addr).group(0) != mac_addr:
+            log_and_raise_error(E_PARSER, "'{}' is not a valid MAC".format(mac_addr))
 
-def parse_direction(args):
-    direction = args.get("direction")
+        return mac_addr
 
-    if direction is None:
-        return (None, None), "direction parameter is missing"
+    def parse_iprange(self, args):
+        IPRANGE_REGEX = re.compile(
+            r"^((25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}"
+            r"(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)(/\d{1,2})?$"
+        )
+        ip_range = args.get("iprange")
 
-    dir = direction.lower().split("/")
-    has_to = "to" in dir
-    has_from = "from" in dir
+        if ip_range is None:
+            log_and_raise_error(E_PARSER, "ip range parameter is missing")
 
-    if not (has_to or has_from):
-        return (None, None), "'{}' is not a valid direction".format(direction)
+        if not IPRANGE_REGEX.match(ip_range):
+            log_and_raise_error(E_PARSER, "'{}' is not a valid IP range".format(ip_range))
 
-    return (has_to, has_from), None
+        return ip_range
 
+    def parse_direction(self, args):
+        direction = args.get("direction")
 
-def parse_protocol(args):
-    protocol = args.get("protocol")
+        if direction is None:
+            log_and_raise_error(E_PARSER, "direction parameter is missing")
 
-    if protocol is None:
-        return None, "protocol parameter is missing"
+        dir = direction.lower().split("/")
+        has_to = "to" in dir
+        has_from = "from" in dir
 
-    protocol = protocol.lower()
+        if not (has_to or has_from):
+            log_and_raise_error(E_PARSER, "'{}' is not a valid direction".format(direction))
 
-    if protocol not in VALID_PROTOCOLS:
-        return None, "'{}' is not a supported protocol".format(protocol)
+        return (has_to, has_from)
 
-    return protocol, None
+    def parse_protocol(self, args):
+        protocol = args.get("protocol")
 
+        if protocol is None:
+            log_and_raise_error(E_PARSER, "protocol parameter is missing")
 
-def parse_port(args):
-    port = args.get("port")
-    if port is None:
-        return None, None  # Port is optional
+        protocol = protocol.lower()
 
-    try:
-        p = int(port)
-        if not (0 <= p <= 65535):
-            raise ValueError
-        return port, None
-    except ValueError:
-        return None, "'{}' is not a valid port".format(port)
+        if protocol not in VALID_PROTOCOLS:
+            log_and_raise_error(E_PARSER, "'{}' is not a supported protocol".format(protocol))
 
+        return protocol
 
-def parse_allow(args):
-    allow = args.get("allow")
-    if allow is None:
-        return None, "allow parameter is missing"
+    def parse_port(self, args):
+        port = args.get("port")
+        if port is None:
+            return None
 
-    if allow.lower() not in ["true", "false"]:
-        return None, "allow parameter should be true or false, not '{}'".format(allow)
+        try:
+            p = int(port)
+            if not (0 <= p <= 65535):
+                raise ValueError
+            return port
+        except ValueError:
+            log_and_raise_error(E_PARSER, "'{}' is not a valid port".format(port))
 
-    if allow.lower() == "true":
-        return True, None
+    def parse_allow(self, args):
+        allow = args.get("allow")
+        if allow is None:
+            log_and_raise_error(E_PARSER, "allow parameter is missing")
 
-    return False, None
+        if allow.lower() not in ["true", "false"]:
+            log_and_raise_error(E_PARSER, "allow parameter should be true or false, not '{}'".format(allow))
 
-def parse_priority(args):
-    priority = args.get("priority")
-    if priority is None:
-        return None, None  # Priority is optional
+        if allow.lower() == "true":
+            return True
 
-    try:
-        p = int(priority)
-        if not (0 <= p <= 65535):
-            raise ValueError
-        return priority, None
-    except ValueError:
-        return None, "'{}' is not a valid priority".format(priority)
+        return False
 
+    def parse_priority(self, args):
+        priority = args.get("priority")
+        if priority is None:
+            return None
 
-class Parser:
-    def __init__(self, args):
-        self.args = args
-        self.values = {}
-        self.errors = []
+        try:
+            p = int(priority)
+            if not (0 <= p <= 65535):
+                raise ValueError
+            return priority
+        except ValueError:
+            log_and_raise_error(E_PARSER, "'{}' is not a valid priority".format(priority))
 
     def read(self, key, parse_fn, dests=None):
         # parse_fn can return a single value or a tuple of values.
         # In this case we are expecting dests to match the expected
         # returned tuple
-        val, err = parse_fn(self.args)
-        if err:
-            self.errors.append(err)
-            return self
+        val = parse_fn(self.args)
 
         if dests is not None:
             if not isinstance(val, tuple):
-                self.errors.append(
-                    "Internal error: parse {}: doesn't return a tuple while dests is set".format(
-                        key
-                    )
+                log_and_raise_error(
+                    E_PARSER,
+                    "Internal error: parse {}: doesn't return a tuple while dests is set".format(key)
                 )
-                return self
 
             if len(dests) != len(val):
-                self.errors.append(
-                    "Internal error: parse {}: dests is {} while {} was expected".format(
-                        key, len(dests), len(val)
-                    )
+                log_and_raise_error(
+                    E_PARSER,
+                    "Internal error: parse {}: dests is {} while {} was expected".format(key, len(dests), len(val))
                 )
-                return self
 
             for d, v in zip(dests, val):
                 self.values[d] = v
@@ -231,17 +214,18 @@ def build_rules_string(args):
 def add_rule(_session, args):
     _LOGGER.info("Calling add rule with args {}".format(args))
 
-    parser = (
-        Parser(args)
-        .read("bridge", parse_bridge)
-        .read("mac", parse_mac)
-        .read("direction", parse_direction, dests=["has_to", "has_from"])
-        .read("protocol", parse_protocol)
-        .read("iprange", parse_iprange)
-        .read("port", parse_port)
-        .read("allow", parse_allow)
-        .read("priority", parse_priority)
-    )
+    try:
+        parser = Parser(args)
+        parser.read("bridge", parser.parse_bridge)
+        parser.read("mac", parser.parse_mac)
+        parser.read("direction", parser.parse_direction, dests=["has_to", "has_from"])
+        parser.read("protocol", parser.parse_protocol)
+        parser.read("iprange", parser.parse_iprange)
+        parser.read("port", parser.parse_port)
+        parser.read("allow", parser.parse_allow)
+        parser.read("priority", parser.parse_priority)
+    except XenAPIPlugin.Failure as e:
+        log_and_raise_error(E_PARSER, "add_rule: Failed to get parameters: {}".format(e.params[1]))
 
     if parser.errors:
         log_and_raise_error(E_PARSER, "add_rule: Failed to get parameters: {}".format(parser.errors))
@@ -274,15 +258,16 @@ def add_rule(_session, args):
 def del_rule(_session, args):
     _LOGGER.info("Calling delete rule with args {}".format(args))
 
-    parser = (
-        Parser(args)
-        .read("bridge", parse_bridge)
-        .read("mac", parse_mac)
-        .read("destination", parse_direction, dests=["has_to", "has_from"])
-        .read("protocol", parse_protocol)
-        .read("iprange", parse_iprange)
-        .read("port", parse_port)
-    )
+    try:
+        parser = Parser(args)
+        parser.read("bridge", parser.parse_bridge)
+        parser.read("mac", parser.parse_mac)
+        parser.read("destination", parser.parse_direction, dests=["has_to", "has_from"])
+        parser.read("protocol", parser.parse_protocol)
+        parser.read("iprange", parser.parse_iprange)
+        parser.read("port", parser.parse_port)
+    except XenAPIPlugin.Failure as e:
+        log_and_raise_error(E_PARSER, "del_rule: Failed to get parameters: {}".format(e.params[1]))
 
     if parser.errors:
         log_and_raise_error(E_PARSER, "del_rule: Failed to get parameters: {}".format(parser.errors))
@@ -311,10 +296,11 @@ def del_rule(_session, args):
 def dump_flows(_session, args):
     _LOGGER.info("Calling dump flows with args {}".format(args))
 
-    bridge, err = parse_bridge(args)
-
-    if err:
-        log_and_raise_error(E_PARSER, "dump flows: {}".format(err))
+    parser = Parser(args)
+    try:
+        bridge = parser.parse_bridge(args)
+    except XenAPIPlugin.Failure as e:
+        log_and_raise_error(E_PARSER, "dump_flows: Failed to get parameters: {}".format(e.params[1]))
 
     ofctl_cmd = [OVS_OFCTL_CMD, "-O", OPENFLOW_PROTOCOL, "dump-flows", bridge]
     cmd = run_command(ofctl_cmd, check=False)

tests/sdncontroller_test_cases/functions.py

@@ -9,9 +9,7 @@
         'exception': {
             'type': XenAPIPlugin.Failure,
             'code': '1',
-            'text': "add_rule: Failed to get parameters: ['bridge parameter is missing', "
-            "'direction parameter is missing', 'protocol parameter is missing', 'ip range parameter is missing', "
-            "'allow parameter is missing']"
+            'text': "add_rule: Failed to get parameters: bridge parameter is missing"
         },
         'cmd': {
             'returncode': 0,
@@ -100,8 +98,7 @@
         'exception': {
             'type': XenAPIPlugin.Failure,
             'code': '1',
-            'text': "del_rule: Failed to get parameters: ['bridge parameter is missing', "
-            "'direction parameter is missing', 'protocol parameter is missing', 'ip range parameter is missing']"
+            'text': "del_rule: Failed to get parameters: bridge parameter is missing"
         },
         'cmd': {
             'returncode': 0,
@@ -171,7 +168,7 @@
         'exception': {
             'type': XenAPIPlugin.Failure,
             'code': '1',
-            'text': "dump flows: bridge parameter is missing"
+            'text': "dump_flows: Failed to get parameters: bridge parameter is missing"
         },
         'cmd': {
             'returncode': 0,
@@ -184,7 +181,7 @@
         'exception': {
             'type': XenAPIPlugin.Failure,
             'code': '1',
-            'text': "dump flows: '' is not a valid bridge name"
+            'text': "dump_flows: Failed to get parameters: '' is not a valid bridge name"
         },
         'cmd': {
             'returncode': 0,