Syntax updates and new tests

- syntax updates to match pep8 standards
- tests to cover apex_python_utils.py

Change-Id: Ifac06fdbb97266f1b574b20610979b6965d6dd55
Signed-off-by: Dan Radez <dradez@redhat.com>

6 files changed, 93 insertions, 107 deletions

diff --git a/lib/common-functions.sh b/lib/common-functions.sh
index 6b259ac1..079b0886 100644
--- a/lib/common-functions.sh
+++ b/lib/common-functions.sh
@@ -18,7 +18,7 @@ function find_ip {
     return 1
   fi
 
-  python3.4 -B $LIB/python/apex-python-utils.py find-ip -i $1
+  python3.4 -B $LIB/python/apex_python_utils.py find-ip -i $1
 }
 
 ##attach interface to OVS and set the network config correctly
diff --git a/lib/python/apex/deploy_env.py b/lib/python/apex/deploy_env.py
index 4c71b54f..1fe137e4 100644
--- a/lib/python/apex/deploy_env.py
+++ b/lib/python/apex/deploy_env.py
@@ -24,8 +24,9 @@ REQ_DEPLOY_SETTINGS = ['sdn_controller',
 OPT_DEPLOY_SETTINGS = ['performance']
 
 VALID_ROLES = ['Controller', 'Compute', 'ObjectStorage']
-VALID_PERF_OPTS = ['kernel','nova']
-VALID_DATAPLANES = ['ovs','ovs_dpdk','fdio']
+VALID_PERF_OPTS = ['kernel', 'nova']
+VALID_DATAPLANES = ['ovs', 'ovs_dpdk', 'fdio']
+
 
 class DeploySettings:
     """
@@ -65,9 +66,9 @@ class DeploySettings:
             if setting == 'dataplane':
                 if value not in VALID_DATAPLANES:
                     planes = ' '.join(VALID_DATAPLANES)
-                    raise DeploySettingsException("Invalid dataplane {} "
-                                                  "specified. Valid dataplanes:"
-                                                  " {}".format(value,planes))
+                    raise DeploySettingsException(
+                        "Invalid dataplane {} specified. Valid dataplanes:"
+                        " {}".format(value, planes))
 
         for req_set in REQ_DEPLOY_SETTINGS:
             if req_set not in deploy_options:
@@ -80,37 +81,43 @@ class DeploySettings:
             if not isinstance(deploy_options['performance'], dict):
                 raise DeploySettingsException("Performance deploy_option"
                                               "must be a dictionary.")
-            for role,role_perf_sets in deploy_options['performance'].items():
+            for role, role_perf_sets in deploy_options['performance'].items():
                 if role not in VALID_ROLES:
                     raise DeploySettingsException("Performance role {}"
                                                   "is not valid, choose"
                                                   "from {}".format(
-                                                  role," ".join(VALID_ROLES)
+                                                      role,
+                                                      " ".join(VALID_ROLES)
                                                   ))
 
                 for key in role_perf_sets:
                     if key not in VALID_PERF_OPTS:
-                        raise DeploySettingsException("Performance option {}"
+                        raise DeploySettingsException("Performance option {} "
                                                       "is not valid, choose"
                                                       "from {}".format(
-                                                      key," ".join(
-                                                      VALID_PERF_OPTS)))
-
+                                                          key,
+                                                          " ".join(
+                                                              VALID_PERF_OPTS)
+                                                      ))
 
     def _dump_performance(self):
         """
         Creates performance settings string for bash consumption.
 
-        Output will be in the form of a list that can be iterated over in bash,
-        with each string being the direct input to the performance setting script
-        in the form <role> <category> <key> <value> to facilitate modification of the
-        correct image.
+        Output will be in the form of a list that can be iterated over in
+        bash, with each string being the direct input to the performance
+        setting script in the form <role> <category> <key> <value> to
+        facilitate modification of the correct image.
         """
         bash_str = 'performance_options=(\n'
-        for role,settings in self.deploy_settings['deploy_options']['performance'].items():
-            for category,options in settings.items():
-                for key,value in options.items():
-                    bash_str += "\"{} {} {} {}\"\n".format(role, category, key, value)
+        deploy_options = self.deploy_settings['deploy_options']
+        for role, settings in deploy_options['performance'].items():
+            for category, options in settings.items():
+                for key, value in options.items():
+                    bash_str += "\"{} {} {} {}\"\n".format(role,
+                                                           category,
+                                                           key,
+                                                           value)
         bash_str += ')\n'
         bash_str += '\n'
         bash_str += 'performance_roles=(\n'
@@ -126,11 +133,13 @@ class DeploySettings:
         Creates deploy settings array in bash syntax.
         """
         bash_str = ''
-        for key,value in self.deploy_settings['deploy_options'].items():
+        for key, value in self.deploy_settings['deploy_options'].items():
             if not isinstance(value, bool):
-                bash_str += "deploy_options_array[{}]=\"{}\"\n".format(key, value)
+                bash_str += "deploy_options_array[{}]=\"{}\"\n".format(key,
+                                                                       value)
             else:
-                bash_str += "deploy_options_array[{}]={}\n".format(key, value)
+                bash_str += "deploy_options_array[{}]={}\n".format(key,
+                                                                   value)
         return bash_str
 
     def dump_bash(self, path=None):
diff --git a/lib/python/apex/ip_utils.py b/lib/python/apex/ip_utils.py
index f51f227a..b039e26b 100644
--- a/lib/python/apex/ip_utils.py
+++ b/lib/python/apex/ip_utils.py
@@ -34,11 +34,11 @@ def get_ip_range(start_offset=None, count=None, end_offset=None,
         space, and end_offset will be calculated from the interface IP.
 
     2 of start_offset, end_offset and count options must be provided:
-        - If start_offset and end_offset are provided, a range from start_offset
-        to end_offset will be returned.
-        - If count is provided, a range from either start_offset to (start_offset
-        +count) or (end_offset-count) to end_offset will be returned. The
-        IP range returned will be of size <count>.
+        - If start_offset and end_offset are provided, a range from
+        start_offset to end_offset will be returned.
+        - If count is provided, a range from either start_offset to
+        (start_offset+count) or (end_offset-count) to end_offset will be
+        returned. The IP range returned will be of size <count>.
     Both start_offset and end_offset must be greater than 0.
 
     Returns IP range in the format of "first_addr,second_addr" or exception
@@ -47,7 +47,7 @@ def get_ip_range(start_offset=None, count=None, end_offset=None,
     if cidr:
         if count and start_offset and not end_offset:
             start_index = start_offset
-            end_index = start_offset + count -1
+            end_index = start_offset + count - 1
         elif count and end_offset and not start_offset:
             end_index = -1 - end_offset
             start_index = -1 - end_index - count + 1
@@ -81,7 +81,7 @@ def get_ip_range(start_offset=None, count=None, end_offset=None,
         else:
             if count and start_offset and not end_offset:
                 start_ip = network[start_offset]
-                end_ip = start_ip + count -1
+                end_ip = start_ip + count - 1
             elif count and end_offset and not start_offset:
                 end_ip = interface.ip - end_offset
                 start_ip = end_ip - count + 1
@@ -135,27 +135,6 @@ def get_ip(offset, cidr=None, interface=None):
         return str(ip)
 
 
-def generate_ip_range(args):
-    """
-    Generate IP range in string format for given CIDR.
-    This function works for both IPv4 and IPv6.
-
-    args is expected to contain the following members:
-    CIDR: any valid CIDR representation.
-    start_position: starting index, default to first address in subnet (1)
-    end_position:  ending index, default to last address in subnet (-1)
-
-    Returns IP range in string format. A single IP is returned if start and
-    end IPs are identical.
-    """
-    cidr = ipaddress.ip_network(args.CIDR)
-    (start_index, end_index) = (args.start_position, args.end_position)
-    if cidr[start_index] == cidr[end_index]:
-        return str(cidr[start_index])
-    else:
-        return ','.join(sorted([str(cidr[start_index]), str(cidr[end_index])]))
-
-
 def get_interface(nic, address_family=4):
     """
     Returns interface object for a given NIC name in the system
@@ -183,7 +162,7 @@ def get_interface(nic, address_family=4):
         return ipaddress.ip_interface(match.group())
     else:
         logging.info("interface ip not found! ip address output:\n{}"
-                        .format(output))
+                     .format(output))
         return None
 
 
@@ -228,8 +207,8 @@ def _validate_ip_range(start_ip, end_ip, cidr):
     """
     ip_range = "{},{}".format(start_ip, end_ip)
     if end_ip <= start_ip:
-        logging.warning("IP range {} is invalid: end_ip should be greater than "
-                        "starting ip".format(ip_range))
+        logging.warning("IP range {} is invalid: end_ip should be greater "
+                        "than starting ip".format(ip_range))
         return False
     if start_ip not in ipaddress.ip_network(cidr):
         logging.warning('start_ip {} is not in network {}'
diff --git a/lib/python/apex/network_environment.py b/lib/python/apex/network_environment.py
index fd6f5286..fec6299d 100644
--- a/lib/python/apex/network_environment.py
+++ b/lib/python/apex/network_environment.py
@@ -9,7 +9,11 @@
 
 import yaml
 import re
-from .common import constants
+from .common.constants import ADMIN_NETWORK
+from .common.constants import PRIVATE_NETWORK
+from .common.constants import STORAGE_NETWORK
+from .common.constants import PUBLIC_NETWORK
+from .common.constants import API_NETWORK
 
 PORTS = '/ports'
 # Resources defined by <resource name>: <prefix>
@@ -70,27 +74,27 @@ class NetworkEnvironment:
         if not tht_dir:
             raise NetworkEnvException('Unable to parse THT Directory')
 
-        admin_cidr = net_settings[constants.ADMIN_NETWORK]['cidr']
+        admin_cidr = net_settings[ADMIN_NETWORK]['cidr']
         admin_prefix = str(admin_cidr.prefixlen)
         self.netenv_obj[param_def]['ControlPlaneSubnetCidr'] = admin_prefix
         self.netenv_obj[param_def]['ControlPlaneDefaultRoute'] = \
-            net_settings[constants.ADMIN_NETWORK]['provisioner_ip']
-        public_cidr = net_settings[constants.PUBLIC_NETWORK]['cidr']
+            net_settings[ADMIN_NETWORK]['provisioner_ip']
+        public_cidr = net_settings[PUBLIC_NETWORK]['cidr']
         self.netenv_obj[param_def]['ExternalNetCidr'] = str(public_cidr)
-        if net_settings[constants.PUBLIC_NETWORK]['vlan'] != 'native':
+        if net_settings[PUBLIC_NETWORK]['vlan'] != 'native':
             self.netenv_obj[param_def]['ExternalNetworkVlanID'] = \
-                    net_settings[constants.PUBLIC_NETWORK]['vlan']
-        public_range = net_settings[constants.PUBLIC_NETWORK][
-                                         'usable_ip_range'].split(',')
+                net_settings[PUBLIC_NETWORK]['vlan']
+        public_range = \
+            net_settings[PUBLIC_NETWORK]['usable_ip_range'].split(',')
         self.netenv_obj[param_def]['ExternalAllocationPools'] = \
             [{'start':
               public_range[0],
               'end': public_range[1]
               }]
         self.netenv_obj[param_def]['ExternalInterfaceDefaultRoute'] = \
-            net_settings[constants.PUBLIC_NETWORK]['gateway']
+            net_settings[PUBLIC_NETWORK]['gateway']
         self.netenv_obj[param_def]['EC2MetadataIp'] = \
-            net_settings[constants.ADMIN_NETWORK]['provisioner_ip']
+            net_settings[ADMIN_NETWORK]['provisioner_ip']
         self.netenv_obj[param_def]['DnsServers'] = net_settings['dns_servers']
 
         if public_cidr.version == 6:
@@ -103,24 +107,23 @@ class NetworkEnvironment:
                 prefix = ''
             self.netenv_obj[reg][key] = tht_dir + prefix + postfix
 
-
-        if constants.PRIVATE_NETWORK in enabled_networks:
-            priv_range = net_settings[constants.PRIVATE_NETWORK][
+        if PRIVATE_NETWORK in enabled_networks:
+            priv_range = net_settings[PRIVATE_NETWORK][
                 'usable_ip_range'].split(',')
             self.netenv_obj[param_def]['TenantAllocationPools'] = \
                 [{'start':
                   priv_range[0],
                   'end': priv_range[1]
                   }]
-            priv_cidr = net_settings[constants.PRIVATE_NETWORK]['cidr']
+            priv_cidr = net_settings[PRIVATE_NETWORK]['cidr']
             self.netenv_obj[param_def]['TenantNetCidr'] = str(priv_cidr)
             if priv_cidr.version == 6:
                 postfix = '/tenant_v6.yaml'
             else:
                 postfix = '/tenant.yaml'
-            if net_settings[constants.PRIVATE_NETWORK]['vlan'] != 'native':
+            if net_settings[PRIVATE_NETWORK]['vlan'] != 'native':
                 self.netenv_obj[param_def]['TenantNetworkVlanID'] = \
-                         net_settings[constants.PRIVATE_NETWORK]['vlan']
+                    net_settings[PRIVATE_NETWORK]['vlan']
         else:
             postfix = '/noop.yaml'
 
@@ -129,8 +132,8 @@ class NetworkEnvironment:
                 prefix = ''
             self.netenv_obj[reg][key] = tht_dir + prefix + postfix
 
-        if constants.STORAGE_NETWORK in enabled_networks:
-            storage_range = net_settings[constants.STORAGE_NETWORK][
+        if STORAGE_NETWORK in enabled_networks:
+            storage_range = net_settings[STORAGE_NETWORK][
                 'usable_ip_range'].split(',')
             self.netenv_obj[param_def]['StorageAllocationPools'] = \
                 [{'start':
@@ -138,15 +141,15 @@ class NetworkEnvironment:
                   'end':
                   storage_range[1]
                   }]
-            storage_cidr = net_settings[constants.STORAGE_NETWORK]['cidr']
+            storage_cidr = net_settings[STORAGE_NETWORK]['cidr']
             self.netenv_obj[param_def]['StorageNetCidr'] = str(storage_cidr)
             if storage_cidr.version == 6:
                 postfix = '/storage_v6.yaml'
             else:
                 postfix = '/storage.yaml'
-            if net_settings[constants.STORAGE_NETWORK]['vlan'] != 'native':
+            if net_settings[STORAGE_NETWORK]['vlan'] != 'native':
                 self.netenv_obj[param_def]['StorageNetworkVlanID'] = \
-                         net_settings[constants.STORAGE_NETWORK]['vlan']
+                    net_settings[STORAGE_NETWORK]['vlan']
         else:
             postfix = '/noop.yaml'
 
@@ -155,24 +158,22 @@ class NetworkEnvironment:
                 prefix = ''
             self.netenv_obj[reg][key] = tht_dir + prefix + postfix
 
-        if constants.API_NETWORK in enabled_networks:
-            api_range = net_settings[constants.API_NETWORK][
+        if API_NETWORK in enabled_networks:
+            api_range = net_settings[API_NETWORK][
                 'usable_ip_range'].split(',')
             self.netenv_obj[param_def]['InternalApiAllocationPools'] = \
-                [{'start':
-                      api_range[0],
-                  'end':
-                      api_range[1]
+                [{'start': api_range[0],
+                  'end': api_range[1]
                   }]
-            api_cidr = net_settings[constants.API_NETWORK]['cidr']
+            api_cidr = net_settings[API_NETWORK]['cidr']
             self.netenv_obj[param_def]['InternalApiNetCidr'] = str(api_cidr)
             if api_cidr.version == 6:
                 postfix = '/internal_api_v6.yaml'
             else:
                 postfix = '/internal_api.yaml'
-            if net_settings[constants.API_NETWORK]['vlan'] != 'native':
+            if net_settings[API_NETWORK]['vlan'] != 'native':
                 self.netenv_obj[param_def]['InternalApiNetworkVlanID'] = \
-                         net_settings[constants.API_NETWORK]['vlan']
+                    net_settings[API_NETWORK]['vlan']
         else:
             postfix = '/noop.yaml'
 
diff --git a/lib/python/apex/network_settings.py b/lib/python/apex/network_settings.py
index 475082df..50dd15c3 100644
--- a/lib/python/apex/network_settings.py
+++ b/lib/python/apex/network_settings.py
@@ -19,8 +19,8 @@ class NetworkSettings:
     This class parses APEX network settings yaml file into an object. It
     generates or detects all missing fields for deployment.
 
-    The resulting object will be used later to generate network environment file
-    as well as configuring post deployment networks.
+    The resulting object will be used later to generate network environment
+    file as well as configuring post deployment networks.
 
     Currently the parsed object is dumped into a bash global definition file
     for deploy.sh consumption. This object will later be used directly as
@@ -41,7 +41,7 @@ class NetworkSettings:
         """
         if constants.ADMIN_NETWORK not in self.settings_obj or \
             not utils.str2bool(self.settings_obj[constants.ADMIN_NETWORK].get(
-                    'enabled')):
+                'enabled')):
             raise NetworkSettingsException("You must enable admin_network "
                                            "and configure it explicitly or "
                                            "use auto-detection")
@@ -274,6 +274,3 @@ class NetworkSettingsException(Exception):
 
     def __str__(self):
             return self.value
-
-
-
diff --git a/lib/python/apex-python-utils.py b/lib/python/apex_python_utils.py
index 01e6b03b..1a3cb159 100755
--- a/lib/python/apex-python-utils.py
+++ b/lib/python/apex_python_utils.py
@@ -105,16 +105,15 @@ def build_nic_template(args):
     vlans_vals = map(lambda x: settings[x]['vlan'], net_list)
     vlans = dict(zip(net_list, vlans_vals))
 
-    print(template.render(
-              enabled_networks=args.enabled_networks,
-              role=args.role,
-              vlans=vlans,
-              external_net_type=args.ext_net_type,
-              external_net_af=args.address_family,
-              ovs_dpdk_bridge=args.ovs_dpdk_bridge))
+    print(template.render(enabled_networks=args.enabled_networks,
+                          role=args.role,
+                          vlans=vlans,
+                          external_net_type=args.ext_net_type,
+                          external_net_af=args.address_family,
+                          ovs_dpdk_bridge=args.ovs_dpdk_bridge))
 
 
-def parse_args():
+def get_parser():
     parser = argparse.ArgumentParser()
     parser.add_argument('--debug', action='store_true', default=False,
                         help="Turn on debug messages")
@@ -176,11 +175,17 @@ def parse_args():
     nic_template.set_defaults(func=build_nic_template)
 
     deploy_settings = subparsers.add_parser('parse-deploy-settings',
-                              help='Parse deploy settings file')
-    deploy_settings.add_argument('-f', '--file', default='deploy_settings.yaml',
-                              help='path to deploy settings file')
+                                            help='Parse deploy settings file')
+    deploy_settings.add_argument('-f', '--file',
+                                 default='deploy_settings.yaml',
+                                 help='path to deploy settings file')
     deploy_settings.set_defaults(func=parse_deploy_settings)
 
+    return parser
+
+
+def main():
+    parser = get_parser()
     args = parser.parse_args(sys.argv[1:])
     if args.debug:
         logging.basicConfig(level=logging.DEBUG)
@@ -191,10 +196,6 @@ def parse_args():
                             format='%(asctime)s %(levelname)s: %(message)s',
                             datefmt='%m/%d/%Y %I:%M:%S %p',
                             level=logging.DEBUG)
-    return parser, args
-
-
-def main(parser, args):
     if hasattr(args, 'func'):
         args.func(args)
     else:
@@ -202,5 +203,4 @@ def main(parser, args):
         exit(1)
 
 if __name__ == "__main__":
-    parser, args = parse_args()
-    main(parser, args)
+    main()