test/lib: pep8 and pylint improvements

Signed-off-by: IWASE Yusuke <iwase.yusuke0@gmail.com>

6 files changed, 178 insertions, 100 deletions

diff --git a/test/lib/bagpipe.py b/test/lib/bagpipe.py
index c73b6472..efca1c4e 100644
--- a/test/lib/bagpipe.py
+++ b/test/lib/bagpipe.py
@@ -13,7 +13,15 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from base import *
+from __future__ import absolute_import
+
+from fabric import colors
+from fabric.api import local
+
+from lib.base import (
+    BGPContainer,
+    CmdBuffer,
+)
 
 
 class BagpipeContainer(BGPContainer):
@@ -41,7 +49,7 @@ class BagpipeContainer(BGPContainer):
         c << '[BGP]'
         if len(self.ip_addrs) > 0:
             c << 'local_address={0}'.format(self.ip_addrs[0][1].split('/')[0])
-        for peer, info in self.peers.iteritems():
+        for info in self.peers.values():
             c << 'peers={0}'.format(info['neigh_addr'].split('/')[0])
         c << 'my_as={0}'.format(self.asn)
         c << 'enable_rtc=True'
diff --git a/test/lib/base.py b/test/lib/base.py
index cbbb764f..b6e77118 100644
--- a/test/lib/base.py
+++ b/test/lib/base.py
@@ -13,18 +13,20 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+from __future__ import absolute_import
+
+import os
+import time
+import itertools
+
 from fabric.api import local, lcd
 from fabric import colors
-from fabric.utils import indent
 from fabric.state import env, output
 try:
     from docker import Client
 except ImportError:
     from docker import APIClient as Client
 import netaddr
-import os
-import time
-import itertools
 
 DEFAULT_TEST_PREFIX = ''
 DEFAULT_TEST_BASE_DIR = '/tmp/gobgp'
@@ -49,6 +51,7 @@ BGP_ATTR_TYPE_EXTENDED_COMMUNITIES = 16
 env.abort_exception = RuntimeError
 output.stderr = False
 
+
 def wait_for_completion(f, timeout=120):
     interval = 1
     count = 0
@@ -63,8 +66,8 @@ def wait_for_completion(f, timeout=120):
 
 
 def try_several_times(f, t=3, s=1):
-    e = None
-    for i in range(t):
+    e = Exception
+    for _ in range(t):
         try:
             r = f()
         except RuntimeError as e:
@@ -75,11 +78,11 @@ def try_several_times(f, t=3, s=1):
 
 
 def get_bridges():
-    return try_several_times(lambda : local("docker network ls | awk 'NR > 1{print $2}'", capture=True)).split('\n')
+    return try_several_times(lambda: local("docker network ls | awk 'NR > 1{print $2}'", capture=True)).split('\n')
 
 
 def get_containers():
-    return try_several_times(lambda : local("docker ps -a | awk 'NR > 1 {print $NF}'", capture=True)).split('\n')
+    return try_several_times(lambda: local("docker ps -a | awk 'NR > 1 {print $NF}'", capture=True)).split('\n')
 
 
 class CmdBuffer(list):
@@ -126,10 +129,10 @@ class Bridge(object):
         if with_ip:
             self.subnet = netaddr.IPNetwork(subnet)
 
-            def f():
+            def _f():
                 for host in self.subnet:
                     yield host
-            self._ip_generator = f()
+            self._ip_generator = _f()
             # throw away first network address
             self.next_ip_address()
 
@@ -145,7 +148,7 @@ class Bridge(object):
         self.self_ip = self_ip
         if self_ip:
             self.ip_addr = self.next_ip_address()
-            try_several_times(lambda :local("ip addr add {0} dev {1}".format(self.ip_addr, self.name)))
+            try_several_times(lambda: local("ip addr add {0} dev {1}".format(self.ip_addr, self.name)))
         self.ctns = []
 
     def next_ip_address(self):
@@ -153,7 +156,7 @@ class Bridge(object):
                                 self.subnet.prefixlen)
 
     def addif(self, ctn):
-        name = ctn.next_if_name()
+        _name = ctn.next_if_name()
         self.ctns.append(ctn)
         local("docker network connect {0} {1}".format(self.name, ctn.docker_name()))
         i = [x for x in Client(timeout=60, version='auto').inspect_network(self.id)['Containers'].values() if x['Name'] == ctn.docker_name()][0]
@@ -164,7 +167,7 @@ class Bridge(object):
         ctn.ip_addrs.append(('eth1', addr, self.name))
 
     def delete(self):
-        try_several_times(lambda : local("docker network rm {0}".format(self.name)))
+        try_several_times(lambda: local("docker network rm {0}".format(self.name)))
 
 
 class Container(object):
@@ -187,7 +190,7 @@ class Container(object):
         return '{0}_{1}'.format(TEST_PREFIX, self.name)
 
     def next_if_name(self):
-        name = 'eth{0}'.format(len(self.eths)+1)
+        name = 'eth{0}'.format(len(self.eths) + 1)
         self.eths.append(name)
         return name
 
@@ -197,7 +200,7 @@ class Container(object):
         for sv in self.shared_volumes:
             c << "-v {0}:{1}".format(sv[0], sv[1])
         c << "--name {0} -id {1}".format(self.docker_name(), self.image)
-        self.id = try_several_times(lambda : local(str(c), capture=True))
+        self.id = try_several_times(lambda: local(str(c), capture=True))
         self.is_running = True
         self.local("ip li set up dev lo")
         for line in self.local("ip a show dev eth0", capture=True).split('\n'):
@@ -210,12 +213,12 @@ class Container(object):
         return 0
 
     def stop(self):
-        ret = try_several_times(lambda : local("docker stop -t 0 " + self.docker_name(), capture=True))
+        ret = try_several_times(lambda: local("docker stop -t 0 " + self.docker_name(), capture=True))
         self.is_running = False
         return ret
 
     def remove(self):
-        ret = try_several_times(lambda : local("docker rm -f " + self.docker_name(), capture=True))
+        ret = try_several_times(lambda: local("docker rm -f " + self.docker_name(), capture=True))
         self.is_running = False
         return ret
 
@@ -232,7 +235,7 @@ class Container(object):
             intf_name = "eth1"
         c << "{0} {1}".format(self.docker_name(), ip_addr)
         self.ip_addrs.append((intf_name, ip_addr, bridge.name))
-        try_several_times(lambda :local(str(c)))
+        try_several_times(lambda: local(str(c)))
 
     def local(self, cmd, capture=False, stream=False, detach=False, tty=True):
         if stream:
@@ -283,7 +286,7 @@ class BGPContainer(Container):
         super(BGPContainer, self).__init__(name, ctn_image_name)
 
     def __repr__(self):
-        return str({'name':self.name, 'asn':self.asn, 'router_id':self.router_id})
+        return str({'name': self.name, 'asn': self.asn, 'router_id': self.router_id})
 
     def run(self):
         self.create_config()
@@ -368,7 +371,7 @@ class BGPContainer(Container):
                               'local-pref': local_pref,
                               'extended-community': extendedcommunity,
                               'matchs': matchs,
-                              'thens' : thens}
+                              'thens': thens}
         if self.is_running and reload_config:
             self.create_config()
             self.reload_config()
@@ -410,27 +413,27 @@ class BGPContainer(Container):
         raise Exception('implement get_neighbor() method')
 
     def get_reachablily(self, prefix, timeout=20):
-            version = netaddr.IPNetwork(prefix).version
-            addr = prefix.split('/')[0]
-            if version == 4:
-                ping_cmd = 'ping'
-            elif version == 6:
-                ping_cmd = 'ping6'
-            else:
-                raise Exception('unsupported route family: {0}'.format(version))
-            cmd = '/bin/bash -c "/bin/{0} -c 1 -w 1 {1} | xargs echo"'.format(ping_cmd, addr)
-            interval = 1
-            count = 0
-            while True:
-                res = self.local(cmd, capture=True)
-                print colors.yellow(res)
-                if '1 packets received' in res and '0% packet loss':
-                    break
-                time.sleep(interval)
-                count += interval
-                if count >= timeout:
-                    raise Exception('timeout')
-            return True
+        version = netaddr.IPNetwork(prefix).version
+        addr = prefix.split('/')[0]
+        if version == 4:
+            ping_cmd = 'ping'
+        elif version == 6:
+            ping_cmd = 'ping6'
+        else:
+            raise Exception('unsupported route family: {0}'.format(version))
+        cmd = '/bin/bash -c "/bin/{0} -c 1 -w 1 {1} | xargs echo"'.format(ping_cmd, addr)
+        interval = 1
+        count = 0
+        while True:
+            res = self.local(cmd, capture=True)
+            print colors.yellow(res)
+            if '1 packets received' in res and '0% packet loss':
+                break
+            time.sleep(interval)
+            count += interval
+            if count >= timeout:
+                raise Exception('timeout')
+        return True
 
     def wait_for(self, expected_state, peer, timeout=120):
         interval = 1
diff --git a/test/lib/bird.py b/test/lib/bird.py
index 598276bc..c728d986 100644
--- a/test/lib/bird.py
+++ b/test/lib/bird.py
@@ -13,7 +13,20 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from base import *
+from __future__ import absolute_import
+
+import time
+
+from fabric import colors
+from fabric.api import local
+from fabric.utils import indent
+
+from lib.base import (
+    BGPContainer,
+    CmdBuffer,
+    try_several_times,
+)
+
 
 class BirdContainer(BGPContainer):
 
diff --git a/test/lib/exabgp.py b/test/lib/exabgp.py
index e3934c79..511a7890 100644
--- a/test/lib/exabgp.py
+++ b/test/lib/exabgp.py
@@ -13,8 +13,19 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from base import *
-from itertools import chain
+from __future__ import absolute_import
+
+import time
+
+from fabric import colors
+from fabric.api import local
+
+from lib.base import (
+    BGPContainer,
+    CmdBuffer,
+    try_several_times,
+)
+
 
 class ExaBGPContainer(BGPContainer):
 
@@ -46,7 +57,7 @@ class ExaBGPContainer(BGPContainer):
         c << 'cp {0}/etc/exabgp/exabgp.env {1}'.format(remotepath, self.SHARED_VOLUME)
         c << 'sed -i -e \'s/all = false/all = true/g\' {0}/exabgp.env'.format(self.SHARED_VOLUME)
         c << 'cp -r {0}/exabgp {1}'.format(self.SHARED_VOLUME,
-                                           remotepath[:-1*len('exabgp')])
+                                           remotepath[:-1 * len('exabgp')])
         c << 'cp {0}/exabgp.env {1}/etc/exabgp/'.format(self.SHARED_VOLUME, remotepath)
         cmd = 'echo "{0:s}" > {1}/update.sh'.format(c, self.config_dir)
         local(cmd, capture=True)
diff --git a/test/lib/gobgp.py b/test/lib/gobgp.py
index 8c0d9cdd..468bd0ae 100644
--- a/test/lib/gobgp.py
+++ b/test/lib/gobgp.py
@@ -13,22 +13,39 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from base import *
+from __future__ import absolute_import
+
 import json
-import toml
-import yaml
 from itertools import chain
 from threading import Thread
-import socket
 import subprocess
 import os
 
+from fabric import colors
+from fabric.api import local
+from fabric.utils import indent
+import netaddr
+import toml
+import yaml
+
+from lib.base import (
+    BGPContainer,
+    CmdBuffer,
+    BGP_ATTR_TYPE_AS_PATH,
+    BGP_ATTR_TYPE_NEXT_HOP,
+    BGP_ATTR_TYPE_MULTI_EXIT_DISC,
+    BGP_ATTR_TYPE_LOCAL_PREF,
+    BGP_ATTR_TYPE_MP_REACH_NLRI,
+)
+
+
 def extract_path_attribute(path, typ):
     for a in path['attrs']:
         if a['type'] == typ:
             return a
     return None
 
+
 class GoBGPContainer(BGPContainer):
 
     SHARED_VOLUME = '/root/shared_volume'
@@ -45,6 +62,7 @@ class GoBGPContainer(BGPContainer):
         self.prefix_set = None
         self.neighbor_set = None
         self.bgp_set = None
+        self.statements = None
         self.default_policy = None
         self.zebra = zebra
         self.zapi_version = zapi_version
@@ -81,26 +99,29 @@ class GoBGPContainer(BGPContainer):
         self._start_gobgp()
         return self.WAIT_FOR_BOOT
 
-    def _get_as_path(self, path):
-        asps = (p['as_paths'] for p in path['attrs'] if
-                p['type'] == BGP_ATTR_TYPE_AS_PATH and 'as_paths' in p
-                and p['as_paths'] != None)
+    @staticmethod
+    def _get_as_path(path):
+        asps = (p['as_paths'] for p in path['attrs']
+                if p['type'] == BGP_ATTR_TYPE_AS_PATH and 'as_paths' in p and p['as_paths'] is not None)
         asps = chain.from_iterable(asps)
         asns = (asp['asns'] for asp in asps)
         return list(chain.from_iterable(asns))
 
-    def _get_nexthop(self, path):
+    @staticmethod
+    def _get_nexthop(path):
         for p in path['attrs']:
             if p['type'] == BGP_ATTR_TYPE_NEXT_HOP or p['type'] == BGP_ATTR_TYPE_MP_REACH_NLRI:
                 return p['nexthop']
 
-    def _get_local_pref(self, path):
+    @staticmethod
+    def _get_local_pref(path):
         for p in path['attrs']:
             if p['type'] == BGP_ATTR_TYPE_LOCAL_PREF:
                 return p['value']
         return None
 
-    def _get_med(self, path):
+    @staticmethod
+    def _get_med(path):
         for p in path['attrs']:
             if p['type'] == BGP_ATTR_TYPE_MULTI_EXIT_DISC:
                 return p['metric']
@@ -222,7 +243,7 @@ class GoBGPContainer(BGPContainer):
         self.statements = []
 
     def set_prefix_set(self, ps):
-        if type(ps) is not list:
+        if not isinstance(ps, list):
             ps = [ps]
         self.prefix_set = ps
 
@@ -232,7 +253,7 @@ class GoBGPContainer(BGPContainer):
         self.prefix_set.append(ps)
 
     def set_neighbor_set(self, ns):
-        if type(ns) is not list:
+        if not isinstance(ns, list):
             ns = [ns]
         self.neighbor_set = ns
 
@@ -250,13 +271,20 @@ class GoBGPContainer(BGPContainer):
             self._create_config_zebra()
 
     def _create_config_bgp(self):
-        config = {'global': {'config': {'as': self.asn, 'router-id': self.router_id},
-                'route-selection-options':{
+        config = {
+            'global': {
+                'config': {
+                    'as': self.asn,
+                    'router-id': self.router_id,
+                },
+                'route-selection-options': {
                     'config': {
                         'external-compare-router-id': True,
                     },
                 },
-        }}
+            },
+            'neighbors': [],
+        }
 
         if self.zebra and self.zapi_version == 2:
             config['global']['use-multiple-paths'] = {'config': {'enabled': True}}
@@ -265,9 +293,9 @@ class GoBGPContainer(BGPContainer):
             afi_safi_list = []
             version = netaddr.IPNetwork(info['neigh_addr']).version
             if version == 4:
-                afi_safi_list.append({'config':{'afi-safi-name': 'ipv4-unicast'}})
+                afi_safi_list.append({'config': {'afi-safi-name': 'ipv4-unicast'}})
             elif version == 6:
-                afi_safi_list.append({'config':{'afi-safi-name': 'ipv6-unicast'}})
+                afi_safi_list.append({'config': {'afi-safi-name': 'ipv6-unicast'}})
             else:
                 Exception('invalid ip address version. {0}'.format(version))
 
@@ -283,18 +311,23 @@ class GoBGPContainer(BGPContainer):
                 afi_safi_list.append({'config': {'afi-safi-name': 'ipv6-flowspec'}})
                 afi_safi_list.append({'config': {'afi-safi-name': 'l3vpn-ipv6-flowspec'}})
 
-            n = {'config':
-                 {'neighbor-address': info['neigh_addr'].split('/')[0],
-                  'peer-as': peer.asn,
-                  'auth-password': info['passwd'],
-                  'vrf': info['vrf'],
-                  },
-                 'afi-safis': afi_safi_list,
-                 'timers': {'config': {
-                     'connect-retry': 10,
-                  }},
-                 'transport': {'config': {}},
-                 }
+            n = {
+                'config': {
+                    'neighbor-address': info['neigh_addr'].split('/')[0],
+                    'peer-as': peer.asn,
+                    'auth-password': info['passwd'],
+                    'vrf': info['vrf'],
+                },
+                'afi-safis': afi_safi_list,
+                'timers': {
+                    'config': {
+                        'connect-retry': 10,
+                    },
+                },
+                'transport': {
+                    'config': {},
+                },
+            }
 
             if ':' in info['local_addr']:
                 n['transport']['config']['local-address'] = info['local_addr'].split('/')[0]
@@ -310,7 +343,7 @@ class GoBGPContainer(BGPContainer):
 
             if info['prefix_limit']:
                 for v in afi_safi_list:
-                    v['prefix-limit'] = {'config': {'max-prefixes': info['prefix_limit'], 'shutdown-threshold-pct': 80 }}
+                    v['prefix-limit'] = {'config': {'max-prefixes': info['prefix_limit'], 'shutdown-threshold-pct': 80}}
 
             if info['graceful_restart'] is not None:
                 n['graceful-restart'] = {'config': {'enabled': True, 'restart-time': 20}}
@@ -324,11 +357,11 @@ class GoBGPContainer(BGPContainer):
                         afi_safi['long-lived-graceful-restart'] = {'config': {'enabled': True, 'restart-time': 30}}
 
             if info['is_rr_client']:
-                clusterId = self.router_id
+                cluster_id = self.router_id
                 if 'cluster_id' in info and info['cluster_id'] is not None:
-                    clusterId = info['cluster_id']
-                n['route-reflector'] = {'config' : {'route-reflector-client': True,
-                                                   'route-reflector-cluster-id': clusterId}}
+                    cluster_id = info['cluster_id']
+                n['route-reflector'] = {'config': {'route-reflector-client': True,
+                                                   'route-reflector-cluster-id': cluster_id}}
 
             if len(info.get('default-policy', [])) + len(info.get('policies', [])) > 0:
                 n['apply-policy'] = {'config': {}}
@@ -336,7 +369,7 @@ class GoBGPContainer(BGPContainer):
             for typ, p in info.get('policies', {}).iteritems():
                 n['apply-policy']['config']['{0}-policy-list'.format(typ)] = [p['name']]
 
-            def f(v):
+            def _f(v):
                 if v == 'reject':
                     return 'reject-route'
                 elif v == 'accept':
@@ -344,10 +377,7 @@ class GoBGPContainer(BGPContainer):
                 raise Exception('invalid default policy type {0}'.format(v))
 
             for typ, d in info.get('default-policy', {}).iteritems():
-                n['apply-policy']['config']['default-{0}-policy'.format(typ)] = f(d)
-
-            if 'neighbors' not in config:
-                config['neighbors'] = []
+                n['apply-policy']['config']['default-{0}-policy'.format(typ)] = _f(d)
 
             config['neighbors'].append(n)
 
@@ -372,9 +402,9 @@ class GoBGPContainer(BGPContainer):
             config['policy-definitions'] = policy_list
 
         if self.zebra:
-            config['zebra'] = {'config':{'enabled': True,
-                                         'redistribute-route-type-list':['connect'],
-                                         'version': self.zapi_version}}
+            config['zebra'] = {'config': {'enabled': True,
+                                          'redistribute-route-type-list': ['connect'],
+                                          'version': self.zapi_version}}
 
         with open('{0}/gobgpd.conf'.format(self.config_dir), 'w') as f:
             print colors.yellow('[{0}\'s new config]'.format(self.name))
@@ -405,8 +435,7 @@ class GoBGPContainer(BGPContainer):
             f.writelines(str(c))
 
     def reload_config(self):
-        daemon = []
-        daemon.append('gobgpd')
+        daemon = ['gobgpd']
         if self.zebra:
             daemon.append('zebra')
         for d in daemon:
@@ -428,7 +457,7 @@ class GoBGPContainer(BGPContainer):
                 cmd = 'gobgp global '\
                       'rib add match {0} then {1} -a {2}'.format(' '.join(v['matchs']), ' '.join(v['thens']), v['rf'])
             else:
-                raise Exception('unsupported route faily: {0}'.format(rf))
+                raise Exception('unsupported route faily: {0}'.format(v['rf']))
             self.local(cmd)
 
 
@@ -449,6 +478,7 @@ class RawGoBGPContainer(GoBGPContainer):
         super(RawGoBGPContainer, self).__init__(name, asn, router_id,
                                                 ctn_image_name, log_level,
                                                 zebra, config_format)
+
     def create_config(self):
         with open('{0}/gobgpd.conf'.format(self.config_dir), 'w') as f:
             print colors.yellow('[{0}\'s new config]'.format(self.name))
diff --git a/test/lib/quagga.py b/test/lib/quagga.py
index 1f0d4d3b..ce3f9e84 100644
--- a/test/lib/quagga.py
+++ b/test/lib/quagga.py
@@ -13,8 +13,21 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from base import *
-from nsenter import Namespace
+from __future__ import absolute_import
+
+from fabric import colors
+from fabric.utils import indent
+import netaddr
+
+from lib.base import (
+    BGPContainer,
+    CmdBuffer,
+    BGP_FSM_IDLE,
+    BGP_FSM_ACTIVE,
+    BGP_FSM_ESTABLISHED,
+    BGP_ATTR_TYPE_MULTI_EXIT_DISC,
+    BGP_ATTR_TYPE_LOCAL_PREF,
+)
 
 
 class QuaggaBGPContainer(BGPContainer):
@@ -44,6 +57,7 @@ class QuaggaBGPContainer(BGPContainer):
         read_next = False
 
         for line in out.split('\n'):
+            ibgp = False
             if line[:2] == '*>':
                 line = line[2:]
                 ibgp = False
@@ -121,10 +135,10 @@ class QuaggaBGPContainer(BGPContainer):
 
         idx1 = info[0].index('BGP neighbor is ')
         idx2 = info[0].index(',')
-        n_addr = info[0][idx1+len('BGP neighbor is '):idx2]
+        n_addr = info[0][idx1 + len('BGP neighbor is '):idx2]
         if n_addr == neigh_addr:
             idx1 = info[2].index('= ')
-            state = info[2][idx1+len('= '):]
+            state = info[2][idx1 + len('= '):]
             if state.startswith('Idle'):
                 return BGP_FSM_IDLE
             elif state.startswith('Active'):
@@ -228,7 +242,7 @@ class QuaggaBGPContainer(BGPContainer):
             f.writelines(str(c))
 
     def vtysh(self, cmd, config=True):
-        if type(cmd) is not list:
+        if not isinstance(cmd, list):
             cmd = [cmd]
         cmd = ' '.join("-c '{0}'".format(c) for c in cmd)
         if config:
@@ -237,8 +251,7 @@ class QuaggaBGPContainer(BGPContainer):
             return self.local("vtysh -d bgpd {0}".format(cmd), capture=True)
 
     def reload_config(self):
-        daemon = []
-        daemon.append('bgpd')
+        daemon = ['bgpd']
         if self.zebra:
             daemon.append('zebra')
         for d in daemon: