Study and respond to PyLint

Add a reasonably tolerant .pylintrc and fix most pylint errors and
warnings.

------------------------------------------------------------------
Your code has been rated at 9.78/10

.pylintrc

@@ -0,0 +1,20 @@
+[MASTER]
+init-hook="from pylint.config import find_pylintrc; import os, sys; sys.path.append(os.path.dirname(find_pylintrc()))"
+ignore-patterns=test_.*
+
+[MESSAGES CONTROL]
+
+# Pointless whinging
+#   R0201 = Method could be a function
+#   W0212 = Accessing protected attribute of client class
+#   W0613 = Unused argument
+#   C0301 = Line too long
+#   R0914 = Too many local variables
+#   C0111 = Missing docstring
+#   W1203 = Use lazy % formatting in logging functions
+
+# Disable the message(s) with the given id(s).
+disable=R0201,W0212,W0613,C0301,R0914,C0111,W1203
+
+[FORMAT]
+max-line-length=148

README.md

@@ -20,6 +20,7 @@ sudo pip3 install netaddr
 sudo pip3 install ipaddress
 sudo pip3 install pyinstaller
 sudo pip3 install black
+sudo pip3 install pylint
 
 ## Ensure all unittests pass.
 ./tests.py -d -t unittest/yaml/*.yaml

config/__init__.py

@@ -29,6 +29,8 @@ try:
 except ImportError:
     print("ERROR: install yamale manually: sudo pip install yamale")
     sys.exit(-2)
+from yamale.validators import DefaultValidators, Validator
+
 from config.loopback import validate_loopbacks
 from config.bondethernet import validate_bondethernets
 from config.interface import validate_interfaces
@@ -36,8 +38,6 @@ from config.bridgedomain import validate_bridgedomains
 from config.vxlan_tunnel import validate_vxlan_tunnels
 from config.tap import validate_taps
 
-from yamale.validators import DefaultValidators, Validator
-
 
 class IPInterfaceWithPrefixLength(Validator):
     """Custom IPAddress config - takes IP/prefixlen as input:
@@ -50,22 +50,22 @@ class IPInterfaceWithPrefixLength(Validator):
 
     def _is_valid(self, value):
         try:
-            network = ipaddress.ip_interface(value)
+            _network = ipaddress.ip_interface(value)
         except:
             return False
         if not isinstance(value, str):
             return False
         if not "/" in value:
             return False
-        e = value.split("/")
+        elems = value.split("/")
-        if not len(e) == 2:
+        if not len(elems) == 2:
             return False
-        if not e[1].isnumeric():
+        if not elems[1].isnumeric():
             return False
         return True
 
 
-class Validator(object):
+class Validator:
     def __init__(self, schema):
         self.logger = logging.getLogger("vppcfg.config")
         self.logger.addHandler(logging.NullHandler())
@@ -81,52 +81,52 @@ class Validator(object):
         ]
 
     def validate(self, yaml):
-        ret_rv = True
+        ret_retval = True
         ret_msgs = []
         if not yaml:
-            return ret_rv, ret_msgs
+            return ret_retval, ret_msgs
 
         validators = DefaultValidators.copy()
         validators[IPInterfaceWithPrefixLength.tag] = IPInterfaceWithPrefixLength
         if self.schema:
-            fn = self.schema
+            fname = self.schema
-            self.logger.debug(f"Validating against --schema {fn}")
+            self.logger.debug(f"Validating against --schema {fname}")
         elif hasattr(sys, "_MEIPASS"):
             ## See vppcfg.spec data_files that includes schema.yaml into the bundle
             self.logger.debug("Validating against built-in schema")
-            fn = os.path.join(sys._MEIPASS, "schema.yaml")
+            fname = os.path.join(sys._MEIPASS, "schema.yaml")
         else:
-            fn = "./schema.yaml"
+            fname = "./schema.yaml"
-            self.logger.debug(f"Validating against fallthrough default schema {fn}")
+            self.logger.debug(f"Validating against fallthrough default schema {fname}")
 
-        if not os.path.isfile(fn):
+        if not os.path.isfile(fname):
-            self.logger.error(f"Cannot file schema file: {fn}")
+            self.logger.error(f"Cannot file schema file: {fname}")
             return False, ret_msgs
 
         try:
-            schema = yamale.make_schema(fn, validators=validators)
+            schema = yamale.make_schema(fname, validators=validators)
             data = yamale.make_data(content=str(yaml))
             yamale.validate(schema, data)
             self.logger.debug("Schema correctly validated by yamale")
         except ValueError as e:
-            ret_rv = False
+            ret_retval = False
             for result in e.results:
                 for error in result.errors:
                     ret_msgs.extend([f"yamale: {error}"])
-            return ret_rv, ret_msgs
+            return ret_retval, ret_msgs
 
         self.logger.debug("Validating Semantics...")
 
-        for v in self.validators:
+        for validator in self.validators:
-            rv, msgs = v(yaml)
+            retval, msgs = validator(yaml)
             if msgs:
                 ret_msgs.extend(msgs)
-            if not rv:
+            if not retval:
-                ret_rv = False
+                ret_retval = False
 
-        if ret_rv:
+        if ret_retval:
             self.logger.debug("Semantics correctly validated")
-        return ret_rv, ret_msgs
+        return ret_retval, ret_msgs
 
     def valid_config(self, yaml):
         """Validate the given YAML configuration in 'yaml' against syntax
@@ -135,10 +135,10 @@ class Validator(object):
         Returns True if the configuration is valid, False otherwise.
         """
 
-        rv, msgs = self.validate(yaml)
+        retval, msgs = self.validate(yaml)
-        if not rv:
+        if not retval:
-            for m in msgs:
+            for msg in msgs:
-                self.logger.error(m)
+                self.logger.error(msg)
             return False
 
         self.logger.info("Configuration validated successfully")

config/address.py

@@ -11,8 +11,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 #
-import logging
-import config.interface as interface
 import ipaddress
 
 
@@ -27,8 +25,8 @@ def get_all_addresses_except_ifname(yaml, except_ifname):
                 continue
 
             if "addresses" in iface:
-                for a in iface["addresses"]:
+                for addr in iface["addresses"]:
-                    ret.append(ipaddress.ip_interface(a))
+                    ret.append(ipaddress.ip_interface(addr))
             if "sub-interfaces" in iface:
                 for subid, sub_iface in iface["sub-interfaces"].items():
                     sub_ifname = f"{ifname}.{int(subid)}"
@@ -36,24 +34,24 @@ def get_all_addresses_except_ifname(yaml, except_ifname):
                         continue
 
                     if "addresses" in sub_iface:
-                        for a in sub_iface["addresses"]:
+                        for addr in sub_iface["addresses"]:
-                            ret.append(ipaddress.ip_interface(a))
+                            ret.append(ipaddress.ip_interface(addr))
     if "loopbacks" in yaml:
         for ifname, iface in yaml["loopbacks"].items():
             if ifname == except_ifname:
                 continue
 
             if "addresses" in iface:
-                for a in iface["addresses"]:
+                for addr in iface["addresses"]:
-                    ret.append(ipaddress.ip_interface(a))
+                    ret.append(ipaddress.ip_interface(addr))
     if "bridgedomains" in yaml:
         for ifname, iface in yaml["bridgedomains"].items():
             if ifname == except_ifname:
                 continue
 
             if "addresses" in iface:
-                for a in iface["addresses"]:
+                for addr in iface["addresses"]:
-                    ret.append(ipaddress.ip_interface(a))
+                    ret.append(ipaddress.ip_interface(addr))
 
     return ret
 
@@ -99,8 +97,8 @@ def is_allowed(yaml, ifname, iface_addresses, ip_interface):
         if my_ip_network.subnet_of(ipaddress.ip_network(ipi, strict=False)):
             return False
 
-    for ip in iface_addresses:
+    for addr in iface_addresses:
-        ipi = ipaddress.ip_interface(ip)
+        ipi = ipaddress.ip_interface(addr)
         if ipi.version != my_ip_network.version:
             continue
 

config/bondethernet.py

@@ -12,15 +12,15 @@
 # limitations under the License.
 #
 import logging
-import config.interface as interface
+from config import interface
-import config.mac as mac
+from config import mac
 
 
 def get_bondethernets(yaml):
     """Return a list of all bondethernets."""
     ret = []
     if "bondethernets" in yaml:
-        for ifname, iface in yaml["bondethernets"].items():
+        for ifname, _iface in yaml["bondethernets"].items():
             ret.append(ifname)
     return ret
 
@@ -30,7 +30,7 @@ def get_by_name(yaml, ifname):
     try:
         if ifname in yaml["bondethernets"]:
             return ifname, yaml["bondethernets"][ifname]
-    except:
+    except KeyError:
         pass
     return None, None
 
@@ -38,7 +38,7 @@ def get_by_name(yaml, ifname):
 def is_bondethernet(yaml, ifname):
     """Returns True if the interface name is an existing BondEthernet."""
     ifname, iface = get_by_name(yaml, ifname)
-    return not iface == None
+    return iface is not None
 
 
 def is_bond_member(yaml, ifname):
@@ -46,7 +46,7 @@ def is_bond_member(yaml, ifname):
     if not "bondethernets" in yaml:
         return False
 
-    for bond, iface in yaml["bondethernets"].items():
+    for _bond, iface in yaml["bondethernets"].items():
         if not "interfaces" in iface:
             continue
         if ifname in iface["interfaces"]:
@@ -78,7 +78,7 @@ def mode_to_int(mode):
     ret = {"round-robin": 1, "active-backup": 2, "xor": 3, "broadcast": 4, "lacp": 5}
     try:
         return ret[mode]
-    except:
+    except KeyError:
         pass
     return -1
 
@@ -92,7 +92,7 @@ def int_to_mode(mode):
     ret = {1: "round-robin", 2: "active-backup", 3: "xor", 4: "broadcast", 5: "lacp"}
     try:
         return ret[mode]
-    except:
+    except KeyError:
         pass
     return ""
 
@@ -109,7 +109,7 @@ def get_lb(yaml, ifname):
     if not iface:
         return None
     mode = get_mode(yaml, ifname)
-    if not mode in ["xor", "lacp"]:
+    if mode not in ["xor", "lacp"]:
         return None
 
     if not "load-balance" in iface:
@@ -117,7 +117,7 @@ def get_lb(yaml, ifname):
     return iface["load-balance"]
 
 
-def lb_to_int(lb):
+def lb_to_int(loadbalance):
     """Returns the integer representation in VPP of a given load-balance strategy,
     or -1 if 'lb' is not a valid string.
 
@@ -133,13 +133,13 @@ def lb_to_int(lb):
         "active-backup": 5,
     }
     try:
-        return ret[lb]
+        return ret[loadbalance]
-    except:
+    except KeyError:
         pass
     return -1
 
 
-def int_to_lb(lb):
+def int_to_lb(loadbalance):
     """Returns the string representation in VPP of a given load-balance strategy,
     or "" if 'lb' is not a valid int.
 
@@ -155,8 +155,8 @@ def int_to_lb(lb):
         5: "active-backup",
     }
     try:
-        return ret[lb]
+        return ret[loadbalance]
-    except:
+    except KeyError:
         pass
     return ""
 
@@ -186,7 +186,7 @@ def validate_bondethernets(yaml):
             )
             result = False
         if (
-            not get_mode(yaml, bond_ifname) in ["xor", "lacp"]
+            get_mode(yaml, bond_ifname) not in ["xor", "lacp"]
             and "load-balance" in iface
         ):
             msgs.append(

config/bridgedomain.py

@@ -12,10 +12,8 @@
 # limitations under the License.
 #
 import logging
-import config.interface as interface
+from config import interface
-import config.loopback as loopback
+from config import loopback
-import config.lcp as lcp
-import config.address as address
 
 
 def get_bridgedomains(yaml):
@@ -23,7 +21,7 @@ def get_bridgedomains(yaml):
     ret = []
     if not "bridgedomains" in yaml:
         return ret
-    for ifname, iface in yaml["bridgedomains"].items():
+    for ifname, _iface in yaml["bridgedomains"].items():
         ret.append(ifname)
     return ret
 
@@ -33,7 +31,7 @@ def get_by_name(yaml, ifname):
     try:
         if ifname in yaml["bridgedomains"]:
             return ifname, yaml["bridgedomains"][ifname]
-    except:
+    except KeyError:
         pass
     return None, None
 
@@ -41,7 +39,7 @@ def get_by_name(yaml, ifname):
 def is_bridgedomain(yaml, ifname):
     """Returns True if the name (bd*) is an existing bridgedomain."""
     ifname, iface = get_by_name(yaml, ifname)
-    return not iface == None
+    return iface is not None
 
 
 def get_bridge_interfaces(yaml):
@@ -51,7 +49,7 @@ def get_bridge_interfaces(yaml):
     if not "bridgedomains" in yaml:
         return ret
 
-    for ifname, iface in yaml["bridgedomains"].items():
+    for _ifname, iface in yaml["bridgedomains"].items():
         if "interfaces" in iface:
             ret.extend(iface["interfaces"])
 
@@ -75,11 +73,11 @@ def bvi_unique(yaml, bviname):
     """Returns True if the BVI identified by bviname is unique among all BridgeDomains."""
     if not "bridgedomains" in yaml:
         return True
-    n = 0
+    ncount = 0
-    for ifname, iface in yaml["bridgedomains"].items():
+    for _ifname, iface in yaml["bridgedomains"].items():
         if "bvi" in iface and iface["bvi"] == bviname:
-            n += 1
+            ncount += 1
-    return n < 2
+    return ncount < 2
 
 
 def get_settings(yaml, ifname):
@@ -141,7 +139,6 @@ def validate_bridgedomains(yaml):
             result = False
 
         if "bvi" in iface:
-            bviname = iface["bvi"]
             bvi_ifname, bvi_iface = loopback.get_by_name(yaml, iface["bvi"])
             if not bvi_unique(yaml, bvi_ifname):
                 msgs.append(f"bridgedomain {ifname} BVI {bvi_ifname} is not unique")

config/interface.py

@@ -12,14 +12,14 @@
 # limitations under the License.
 #
 import logging
-import config.bondethernet as bondethernet
+from config import bondethernet
-import config.bridgedomain as bridgedomain
+from config import bridgedomain
-import config.loopback as loopback
+from config import loopback
-import config.vxlan_tunnel as vxlan_tunnel
+from config import vxlan_tunnel
-import config.lcp as lcp
+from config import lcp
-import config.address as address
+from config import address
-import config.mac as mac
+from config import mac
-import config.tap as tap
+from config import tap
 
 
 def get_qinx_parent_by_name(yaml, ifname):
@@ -28,7 +28,7 @@ def get_qinx_parent_by_name(yaml, ifname):
 
     if not is_qinx(yaml, ifname):
         return None, None
-    qinx_ifname, qinx_iface = get_by_name(yaml, ifname)
+    _qinx_ifname, qinx_iface = get_by_name(yaml, ifname)
     if not qinx_iface:
         return None, None
 
@@ -54,12 +54,17 @@ def get_qinx_parent_by_name(yaml, ifname):
 
 def get_parent_by_name(yaml, ifname):
     """Returns the sub-interface's parent, or None,None if the sub-int doesn't exist."""
+    if not ifname:
+        return None, None
+
     try:
         parent_ifname, subid = ifname.split(".")
         subid = int(subid)
         iface = yaml["interfaces"][parent_ifname]
         return parent_ifname, iface
-    except:
+    except KeyError:
+        pass
+    except ValueError:
         pass
     return None, None
 
@@ -88,20 +93,22 @@ def get_by_name(yaml, ifname):
             subid = int(subid)
             iface = yaml["interfaces"][phy_ifname]["sub-interfaces"][subid]
             return ifname, iface
-        except:
+        except ValueError:
+            return None, None
+        except KeyError:
             return None, None
 
     try:
         iface = yaml["interfaces"][ifname]
         return ifname, iface
-    except:
+    except KeyError:
         pass
     return None, None
 
 
 def is_sub(yaml, ifname):
     """Returns True if this interface is a sub-interface"""
-    parent_ifname, parent_iface = get_parent_by_name(yaml, ifname)
+    _parent_ifname, parent_iface = get_parent_by_name(yaml, ifname)
     return isinstance(parent_iface, dict)
 
 
@@ -153,11 +160,11 @@ def get_l2xc_target_interfaces(yaml):
     """Returns a list of all interfaces that are the target of an L2 CrossConnect"""
     ret = []
     if "interfaces" in yaml:
-        for ifname, iface in yaml["interfaces"].items():
+        for _ifname, iface in yaml["interfaces"].items():
             if "l2xc" in iface:
                 ret.append(iface["l2xc"])
             if "sub-interfaces" in iface:
-                for subid, sub_iface in iface["sub-interfaces"].items():
+                for _subid, sub_iface in iface["sub-interfaces"].items():
                     if "l2xc" in sub_iface:
                         ret.append(sub_iface["l2xc"])
 
@@ -200,11 +207,7 @@ def valid_encapsulation(yaml, ifname):
         return False
     if "inner-dot1q" in encap and not ("dot1ad" in encap or "dot1q" in encap):
         return False
-    if (
+    if "exact-match" in encap and not encap["exact-match"] and has_lcp(yaml, ifname):
-        "exact-match" in encap
-        and encap["exact-match"] == False
-        and has_lcp(yaml, ifname)
-    ):
         return False
 
     return True
@@ -227,10 +230,10 @@ def get_encapsulation(yaml, ifname):
     if not iface:
         return None
 
-    parent_ifname, parent_iface = get_parent_by_name(yaml, ifname)
+    _parent_ifname, parent_iface = get_parent_by_name(yaml, ifname)
     if not iface or not parent_iface:
         return None
-    parent_ifname, subid = ifname.split(".")
+    _parent_ifname, subid = ifname.split(".")
 
     dot1q = 0
     dot1ad = 0
@@ -265,7 +268,7 @@ def get_phys(yaml):
     ret = []
     if not "interfaces" in yaml:
         return ret
-    for ifname, iface in yaml["interfaces"].items():
+    for ifname, _iface in yaml["interfaces"].items():
         if is_phy(yaml, ifname):
             ret.append(ifname)
     return ret
@@ -275,7 +278,7 @@ def is_phy(yaml, ifname):
     """Returns True if the ifname is the name of a physical network interface."""
 
     ifname, iface = get_by_name(yaml, ifname)
-    if iface == None:
+    if iface is None:
         return False
     if is_sub(yaml, ifname):
         return False
@@ -300,7 +303,7 @@ def get_interfaces(yaml):
         ret.append(ifname)
         if not "sub-interfaces" in iface:
             continue
-        for subid, sub_iface in iface["sub-interfaces"].items():
+        for subid, _sub_iface in iface["sub-interfaces"].items():
             ret.append(f"{ifname}.{int(subid)}")
     return ret
 
@@ -351,7 +354,7 @@ def unique_encapsulation(yaml, sub_ifname):
         return False
 
     ncount = 0
-    for subid, sibling_iface in parent_iface["sub-interfaces"].items():
+    for subid, _sibling_iface in parent_iface["sub-interfaces"].items():
         sibling_ifname = f"{parent_ifname}.{int(subid)}"
         sibling_encap = get_encapsulation(yaml, sibling_ifname)
         if sub_encap == sibling_encap and new_ifname != sibling_ifname:
@@ -391,16 +394,12 @@ def get_mtu(yaml, ifname):
     """Returns MTU of the interface. If it's not set, return the parent's MTU, and
     return 1500 if no MTU was set on the sub-int or the parent."""
     ifname, iface = get_by_name(yaml, ifname)
-    if not iface:
+    if iface and "mtu" in iface:
-        return 1500
-
-    parent_ifname, parent_iface = get_parent_by_name(yaml, ifname)
-
-    try:
         return iface["mtu"]
+
+    _parent_ifname, parent_iface = get_parent_by_name(yaml, ifname)
+    if parent_iface and "mtu" in parent_iface:
         return parent_iface["mtu"]
-    except:
-        pass
     return 1500
 
 
@@ -451,7 +450,7 @@ def validate_interfaces(yaml):
         iface_address = has_address(yaml, ifname)
 
         if ifname.startswith("tap"):
-            tap_ifname, tap_iface = tap.get_by_name(yaml, ifname)
+            _tap_ifname, tap_iface = tap.get_by_name(yaml, ifname)
             if not tap_iface:
                 msgs.append(f"interface {ifname} is a TAP but does not exist in taps")
                 result = False
@@ -489,10 +488,10 @@ def validate_interfaces(yaml):
             result = False
 
         if "addresses" in iface:
-            for a in iface["addresses"]:
+            for addr in iface["addresses"]:
-                if not address.is_allowed(yaml, ifname, iface["addresses"], a):
+                if not address.is_allowed(yaml, ifname, iface["addresses"], addr):
                     msgs.append(
-                        f"interface {ifname} IP address {a} conflicts with another"
+                        f"interface {ifname} IP address {addr} conflicts with another"
                     )
                     result = False
 
@@ -624,12 +623,12 @@ def validate_interfaces(yaml):
                             f"sub-interface {sub_ifname} is in L2 mode but has an address"
                         )
                         result = False
-                    for a in sub_iface["addresses"]:
+                    for addr in sub_iface["addresses"]:
                         if not address.is_allowed(
-                            yaml, sub_ifname, sub_iface["addresses"], a
+                            yaml, sub_ifname, sub_iface["addresses"], addr
                         ):
                             msgs.append(
-                                f"sub-interface {sub_ifname} IP address {a} conflicts with another"
+                                f"sub-interface {sub_ifname} IP address {addr} conflicts with another"
                             )
                             result = False
                 if not valid_encapsulation(yaml, sub_ifname):

config/lcp.py

@@ -11,7 +11,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 #
-import logging
 
 
 def get_lcps(yaml, interfaces=True, loopbacks=True, bridgedomains=True):
@@ -20,20 +19,20 @@ def get_lcps(yaml, interfaces=True, loopbacks=True, bridgedomains=True):
 
     ret = []
     if interfaces and "interfaces" in yaml:
-        for ifname, iface in yaml["interfaces"].items():
+        for _ifname, iface in yaml["interfaces"].items():
             if "lcp" in iface:
                 ret.append(iface["lcp"])
             if "sub-interfaces" in iface:
-                for subid, sub_iface in iface["sub-interfaces"].items():
+                for _subid, sub_iface in iface["sub-interfaces"].items():
                     if "lcp" in sub_iface:
                         ret.append(sub_iface["lcp"])
 
     if loopbacks and "loopbacks" in yaml:
-        for ifname, iface in yaml["loopbacks"].items():
+        for _ifname, iface in yaml["loopbacks"].items():
             if "lcp" in iface:
                 ret.append(iface["lcp"])
     if bridgedomains and "bridgedomains" in yaml:
-        for ifname, iface in yaml["bridgedomains"].items():
+        for _ifname, iface in yaml["bridgedomains"].items():
             if "lcp" in iface:
                 ret.append(iface["lcp"])
 

config/loopback.py

@@ -12,16 +12,16 @@
 # limitations under the License.
 #
 import logging
-import config.lcp as lcp
+from config import lcp
-import config.address as address
+from config import address
-import config.mac as mac
+from config import mac
 
 
 def get_loopbacks(yaml):
     """Return a list of all loopbacks."""
     ret = []
     if "loopbacks" in yaml:
-        for ifname, iface in yaml["loopbacks"].items():
+        for ifname, _iface in yaml["loopbacks"].items():
             ret.append(ifname)
     return ret
 
@@ -41,7 +41,7 @@ def get_by_name(yaml, ifname):
     try:
         if ifname in yaml["loopbacks"]:
             return ifname, yaml["loopbacks"][ifname]
-    except:
+    except KeyError:
         pass
     return None, None
 
@@ -49,7 +49,7 @@ def get_by_name(yaml, ifname):
 def is_loopback(yaml, ifname):
     """Returns True if the interface name is an existing loopback."""
     ifname, iface = get_by_name(yaml, ifname)
-    return not iface == None
+    return iface is not None
 
 
 def validate_loopbacks(yaml):
@@ -75,10 +75,10 @@ def validate_loopbacks(yaml):
             )
             result = False
         if "addresses" in iface:
-            for a in iface["addresses"]:
+            for addr in iface["addresses"]:
-                if not address.is_allowed(yaml, ifname, iface["addresses"], a):
+                if not address.is_allowed(yaml, ifname, iface["addresses"], addr):
                     msgs.append(
-                        f"loopback {ifname} IP address {a} conflicts with another"
+                        f"loopback {ifname} IP address {addr} conflicts with another"
                     )
                     result = False
         if "mac" in iface and mac.is_multicast(iface["mac"]):

config/mac.py

@@ -11,7 +11,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 #
-import logging
 import netaddr
 
 
@@ -19,7 +18,7 @@ def is_valid(mac):
     """Return True if the string given in `mac` is a valid (6-byte) MAC address,
     as defined by netaddr.EUI"""
     try:
-        addr = netaddr.EUI(mac)
+        _addr = netaddr.EUI(mac)
     except:
         return False
     return True

config/tap.py

@@ -12,14 +12,14 @@
 # limitations under the License.
 #
 import logging
-import config.mac as mac
+from config import mac
 
 
 def get_taps(yaml):
     """Return a list of all taps."""
     ret = []
     if "taps" in yaml:
-        for ifname, iface in yaml["taps"].items():
+        for ifname, _iface in yaml["taps"].items():
             ret.append(ifname)
     return ret
 
@@ -29,7 +29,7 @@ def get_by_name(yaml, ifname):
     try:
         if ifname in yaml["taps"]:
             return ifname, yaml["taps"][ifname]
-    except:
+    except KeyError:
         pass
     return None, None
 
@@ -40,7 +40,7 @@ def is_tap(yaml, ifname):
     a TAP belonging to a Linux Control Plane (LCP) will return False.
     """
     ifname, iface = get_by_name(yaml, ifname)
-    return not iface == None
+    return iface is not None
 
 
 def is_host_name_unique(yaml, hostname):
@@ -48,7 +48,7 @@ def is_host_name_unique(yaml, hostname):
     if not "taps" in yaml:
         return True
     host_names = []
-    for tap_ifname, tap_iface in yaml["taps"].items():
+    for _tap_ifname, tap_iface in yaml["taps"].items():
         host_names.append(tap_iface["host"]["name"])
     return host_names.count(hostname) < 2
 
@@ -78,14 +78,14 @@ def validate_taps(yaml):
             result = False
 
         if "rx-ring-size" in iface:
-            n = iface["rx-ring-size"]
+            ncount = iface["rx-ring-size"]
-            if n & (n - 1) != 0:
+            if ncount & (ncount - 1) != 0:
                 msgs.append(f"tap {ifname} rx-ring-size must be a power of two")
                 result = False
 
         if "tx-ring-size" in iface:
-            n = iface["tx-ring-size"]
+            ncount = iface["tx-ring-size"]
-            if n & (n - 1) != 0:
+            if ncount & (ncount - 1) != 0:
                 msgs.append(f"tap {ifname} tx-ring-size must be a power of two")
                 result = False
 

config/test_interface.py

@@ -50,7 +50,7 @@ class TestInterfaceMethods(unittest.TestCase):
     def test_mtu(self):
         self.assertEqual(interface.get_mtu(self.cfg, "GigabitEthernet1/0/1"), 9216)
         self.assertEqual(interface.get_mtu(self.cfg, "GigabitEthernet1/0/1.200"), 9000)
-        self.assertEqual(interface.get_mtu(self.cfg, "GigabitEthernet1/0/1.201"), 1500)
+        self.assertEqual(interface.get_mtu(self.cfg, "GigabitEthernet1/0/1.201"), 9216)
 
     def test_encapsulation(self):
         self.assertTrue(

config/vxlan_tunnel.py

@@ -12,7 +12,6 @@
 # limitations under the License.
 #
 import logging
-import config.interface as interface
 import ipaddress
 
 
@@ -21,7 +20,7 @@ def get_by_name(yaml, ifname):
     try:
         if ifname in yaml["vxlan_tunnels"]:
             return ifname, yaml["vxlan_tunnels"][ifname]
-    except:
+    except KeyError:
         pass
     return None, None
 
@@ -29,7 +28,7 @@ def get_by_name(yaml, ifname):
 def is_vxlan_tunnel(yaml, ifname):
     """Returns True if the interface name is an existing VXLAN Tunnel."""
     ifname, iface = get_by_name(yaml, ifname)
-    return not iface == None
+    return iface is not None
 
 
 def vni_unique(yaml, vni):
@@ -38,7 +37,7 @@ def vni_unique(yaml, vni):
         return True
 
     ncount = 0
-    for ifname, iface in yaml["vxlan_tunnels"].items():
+    for _ifname, iface in yaml["vxlan_tunnels"].items():
         if iface["vni"] == vni:
             ncount = ncount + 1
 
@@ -51,7 +50,7 @@ def get_vxlan_tunnels(yaml):
     if not "vxlan_tunnels" in yaml:
         return ret
 
-    for ifname, iface in yaml["vxlan_tunnels"].items():
+    for ifname, _iface in yaml["vxlan_tunnels"].items():
         ret.append(ifname)
     return ret
 

intest/hippo5.yaml

@@ -6,6 +6,7 @@ interfaces:
         mtu: 2000
         l2xc: vxlan_tunnel0
       101:
+        mtu: 1500
         l2xc: vxlan_tunnel1
   GigabitEthernet3/0/1:
     lcp: "e3-0-1"

tests.py

@@ -15,11 +15,11 @@
 # -*- coding: utf-8 -*-
 
 import sys
-import yaml
-from config import Validator
 import glob
 import re
 import unittest
+import yaml
+from config import Validator
 
 try:
     import argparse
@@ -28,73 +28,71 @@ except ImportError:
     sys.exit(-2)
 
 
-def example_validator(yaml):
+def example_validator(_yaml):
     """A simple example validator that takes the YAML configuration file as an input,
     and returns a tuple of rv (return value, True is success), and a list of string
     messages to the validation framework."""
-    rv = True
+    return True, []
-    msgs = []
-
-    return rv, msgs
 
 
 class YAMLTest(unittest.TestCase):
     def __init__(self, testName, yaml_filename, yaml_schema):
         # calling the super class init varies for different python versions.  This works for 2.7
-        super(YAMLTest, self).__init__(testName)
+        super().__init__(testName)
         self.yaml_filename = yaml_filename
         self.yaml_schema = yaml_schema
 
     def test_yaml(self):
-        unittest = None
+        test = None
         cfg = None
-        n = 0
+        ncount = 0
         try:
-            with open(self.yaml_filename, "r") as f:
+            with open(self.yaml_filename, "r", encoding="utf-8") as file:
-                for data in yaml.load_all(f, Loader=yaml.Loader):
+                for data in yaml.load_all(file, Loader=yaml.Loader):
-                    if n == 0:
+                    if ncount == 0:
-                        unittest = data
+                        test = data
-                        n = n + 1
+                        ncount += 1
-                    elif n == 1:
+                    elif ncount == 1:
                         cfg = data
-                        n = n + 1
+                        ncount += 1
         except:
             pass
-        self.assertEqual(n, 2)
+        self.assertEqual(ncount, 2)
-        self.assertIsNotNone(unittest)
+        self.assertIsNotNone(test)
         if not cfg:
             return
 
-        v = Validator(schema=self.yaml_schema)
+        validator = Validator(schema=self.yaml_schema)
-        rv, msgs = v.validate(cfg)
+        _rv, msgs = validator.validate(cfg)
 
-        msgs_unexpected = 0
         msgs_expected = []
         if (
-            "test" in unittest
+            "test" in test
-            and "errors" in unittest["test"]
+            and "errors" in test["test"]
-            and "expected" in unittest["test"]["errors"]
+            and "expected" in test["test"]["errors"]
         ):
-            msgs_expected = unittest["test"]["errors"]["expected"]
+            msgs_expected = test["test"]["errors"]["expected"]
 
         fail = False
-        for m in msgs:
+        for msg in msgs:
             this_msg_expected = False
             for expected in msgs_expected:
-                if re.match(expected, m):
+                if re.match(expected, msg):
                     this_msg_expected = True
                     break
             if not this_msg_expected:
-                print(f"{self.yaml_filename}: Unexpected message: {m}", file=sys.stderr)
+                print(
+                    f"{self.yaml_filename}: Unexpected message: {msg}", file=sys.stderr
+                )
                 fail = True
 
         count = 0
         if (
-            "test" in unittest
+            "test" in test
-            and "errors" in unittest["test"]
+            and "errors" in test["test"]
-            and "count" in unittest["test"]["errors"]
+            and "count" in test["test"]["errors"]
         ):
-            count = unittest["test"]["errors"]["count"]
+            count = test["test"]["errors"]["count"]
 
         if len(msgs) != count:
             print(
@@ -143,11 +141,11 @@ if __name__ == "__main__":
 
     args = parser.parse_args()
     if args.debug:
-        verbosity = 2
+        VERBOSITY = 2
     elif args.quiet:
-        verbosity = 0
+        VERBOSITY = 0
     else:
-        verbosity = 1
+        VERBOSITY = 1
     yaml_suite = unittest.TestSuite()
     for pattern in args.test:
         for fn in glob.glob(pattern):
@@ -155,21 +153,21 @@ if __name__ == "__main__":
                 YAMLTest("test_yaml", yaml_filename=fn, yaml_schema=args.schema)
             )
     yaml_ok = (
-        unittest.TextTestRunner(verbosity=verbosity, buffer=True)
+        unittest.TextTestRunner(verbosity=VERBOSITY, buffer=True)
         .run(yaml_suite)
         .wasSuccessful()
     )
 
     tests = unittest.TestLoader().discover(start_dir=".", pattern="test_*.py")
     unit_ok = (
-        unittest.TextTestRunner(verbosity=verbosity, buffer=True)
+        unittest.TextTestRunner(verbosity=VERBOSITY, buffer=True)
         .run(tests)
         .wasSuccessful()
     )
 
-    retval = 0
+    RETVAL = 0
     if not yaml_ok:
-        retval = retval - 1
+        RETVAL -= 1
     if not unit_ok:
-        retval = retval - 2
+        RETVAL -= 2
-    sys.exit(retval)
+    sys.exit(RETVAL)

unittest/yaml/error-bridgedomain6.yaml

@@ -15,6 +15,7 @@ interfaces:
     mtu: 3000
     sub-interfaces:
       1234:
+        mtu: 1500
         description: "BD11 and BD12"
 
 bridgedomains:

vpp/applier.py

@@ -11,6 +11,8 @@ class Applier(VPPApi):
     and will ensure that the local cache is consistent after creations and
     modifications."""
 
+    # pylint: disable=unnecessary-pass
+
     def __init__(self, address="/run/vpp/api.sock", clientname="vppcfg"):
         VPPApi.__init__(self, address, clientname)
         self.logger.info("VPP Applier: changing the dataplane is enabled")
@@ -47,9 +49,6 @@ class Applier(VPPApi):
         """'config' is the YAML configuration for the vxlan_tunnels: entry"""
         pass
 
-    def delete_subinterface(self, ifname):
-        pass
-
     def set_interface_link_mtu(self, ifname, link_mtu):
         pass
 

vpp/dumper.py

@@ -3,10 +3,10 @@ The functions in this file interact with the VPP API to retrieve certain
 interface metadata and write it to a YAML file.
 """
 
-from vpp.vppapi import VPPApi
 import sys
 import yaml
-import config.bondethernet as bondethernet
+from vpp.vppapi import VPPApi
+from config import bondethernet
 
 
 class Dumper(VPPApi):
@@ -15,17 +15,17 @@ class Dumper(VPPApi):
 
     def write(self, outfile):
         if outfile and outfile == "-":
-            fh = sys.stdout
+            file = sys.stdout
             outfile = "(stdout)"
         else:
-            fh = open(outfile, "w")
+            file = open(outfile, "w", encoding="utf-8")
 
         config = self.cache_to_config()
 
-        print(yaml.dump(config), file=fh)
+        print(yaml.dump(config), file=file)
 
-        if fh is not sys.stdout:
+        if file is not sys.stdout:
-            fh.close()
+            file.close()
         self.logger.info(f"Wrote YAML config to {outfile}")
 
     def cache_to_config(self):

vpp/reconciler.py

@@ -15,13 +15,13 @@
 # -*- coding: utf-8 -*-
 import sys
 import logging
-import config.loopback as loopback
+from config import loopback
-import config.interface as interface
+from config import interface
-import config.bondethernet as bondethernet
+from config import bondethernet
-import config.bridgedomain as bridgedomain
+from config import bridgedomain
-import config.vxlan_tunnel as vxlan_tunnel
+from config import vxlan_tunnel
-import config.lcp as lcp
+from config import lcp
-import config.tap as tap
+from config import tap
 from vpp.vppapi import VPPApi
 
 
@@ -117,26 +117,26 @@ class Reconciler:
         """
         idx = self.vpp.cache["interface_names"][ifname].sw_if_index
         removed_addresses = []
-        for a in self.vpp.cache["interface_addresses"][idx]:
+        for addr in self.vpp.cache["interface_addresses"][idx]:
-            if not a in address_list:
+            if not addr in address_list:
-                cli = f"set interface ip address del {ifname} {a}"
+                cli = f"set interface ip address del {ifname} {addr}"
                 self.cli["prune"].append(cli)
-                removed_addresses.append(a)
+                removed_addresses.append(addr)
             else:
-                self.logger.debug(f"Address OK: {ifname} {a}")
+                self.logger.debug(f"Address OK: {ifname} {addr}")
-        for a in removed_addresses:
+        for addr in removed_addresses:
-            self.vpp.cache["interface_addresses"][idx].remove(a)
+            self.vpp.cache["interface_addresses"][idx].remove(addr)
 
     def prune_loopbacks(self):
         """Remove loopbacks from VPP, if they do not occur in the config."""
         removed_interfaces = []
         for numtags in [2, 1, 0]:
-            for idx, vpp_iface in self.vpp.cache["interfaces"].items():
+            for _idx, vpp_iface in self.vpp.cache["interfaces"].items():
                 if vpp_iface.interface_dev_type != "Loopback":
                     continue
                 if vpp_iface.sub_number_of_tags != numtags:
                     continue
-                config_ifname, config_iface = loopback.get_by_name(
+                _config_ifname, config_iface = loopback.get_by_name(
                     self.cfg, vpp_iface.interface_name
                 )
                 if not config_iface:
@@ -166,8 +166,9 @@ class Reconciler:
         found in to-be removed bridge-domains, they are returned to L3 mode, and tag-rewrites removed."""
         for idx, bridge in self.vpp.cache["bridgedomains"].items():
             bridgename = f"bd{int(idx)}"
-            config_ifname, config_iface = bridgedomain.get_by_name(self.cfg, bridgename)
+            _config_ifname, config_iface = bridgedomain.get_by_name(
-            members = []
+                self.cfg, bridgename
+            )
             if not config_iface:
                 for member in bridge.sw_if_details:
                     if member.sw_if_index == bridge.bvi_sw_if_index:
@@ -222,7 +223,7 @@ class Reconciler:
         but are crossconnected to a different interface name, also remove them. Interfaces are put
         back into L3 mode, and their tag-rewrites removed."""
         removed_l2xcs = []
-        for idx, l2xc in self.vpp.cache["l2xcs"].items():
+        for _idx, l2xc in self.vpp.cache["l2xcs"].items():
             vpp_rx_ifname = self.vpp.cache["interfaces"][
                 l2xc.rx_sw_if_index
             ].interface_name
@@ -276,7 +277,7 @@ class Reconciler:
             return True
         vpp_vxlan = self.vpp.cache["vxlan_tunnels"][vpp_iface.sw_if_index]
 
-        config_ifname, config_iface = vxlan_tunnel.get_by_name(self.cfg, ifname)
+        _config_ifname, config_iface = vxlan_tunnel.get_by_name(self.cfg, ifname)
         if not config_iface:
             return True
 
@@ -299,7 +300,7 @@ class Reconciler:
         vpp_iface = self.vpp.cache["interface_names"][ifname]
         vpp_tap = self.vpp.cache["taps"][vpp_iface.sw_if_index]
 
-        config_ifname, config_iface = tap.get_by_name(self.cfg, ifname)
+        _config_ifname, config_iface = tap.get_by_name(self.cfg, ifname)
         if not config_iface:
             return True
 
@@ -352,10 +353,12 @@ class Reconciler:
 
         vpp_bond = self.vpp.cache["bondethernets"][vpp_iface.sw_if_index]
         mode = bondethernet.mode_to_int(bondethernet.get_mode(self.cfg, config_ifname))
-        if mode != -1 and mode != vpp_bond.mode:
+        if mode not in (-1, vpp_bond.mode):
             return True
-        lb = bondethernet.lb_to_int(bondethernet.get_lb(self.cfg, config_ifname))
+        loadbalance = bondethernet.lb_to_int(
-        if lb != -1 and lb != vpp_bond.lb:
+            bondethernet.get_lb(self.cfg, config_ifname)
+        )
+        if loadbalance not in (-1, vpp_bond.lb):
             return True
 
         return False
@@ -365,7 +368,7 @@ class Reconciler:
         TAPs which are a part of Linux Control Plane, are left alone, to be handled
         by prune_lcps() later."""
         removed_taps = []
-        for idx, vpp_tap in self.vpp.cache["taps"].items():
+        for _idx, vpp_tap in self.vpp.cache["taps"].items():
             vpp_iface = self.vpp.cache["interfaces"][vpp_tap.sw_if_index]
             vpp_ifname = vpp_iface.interface_name
             if self.vpp.tap_is_lcp(vpp_ifname):
@@ -388,7 +391,9 @@ class Reconciler:
         removed_bondethernet_members = []
         for idx, bond in self.vpp.cache["bondethernets"].items():
             vpp_ifname = bond.interface_name
-            config_ifname, config_iface = bondethernet.get_by_name(self.cfg, vpp_ifname)
+            _config_ifname, config_iface = bondethernet.get_by_name(
+                self.cfg, vpp_ifname
+            )
 
             if self.__bond_has_diff(vpp_ifname):
                 self.prune_addresses(vpp_ifname, [])
@@ -478,7 +483,7 @@ class Reconciler:
                     continue
 
                 prune = False
-                config_ifname, config_iface = interface.get_by_name(
+                _config_ifname, config_iface = interface.get_by_name(
                     self.cfg, vpp_ifname
                 )
                 if not config_iface:
@@ -489,7 +494,7 @@ class Reconciler:
                 ):
                     (
                         config_parent_ifname,
-                        config_parent_iface,
+                        _config_parent_iface,
                     ) = interface.get_parent_by_name(self.cfg, vpp_ifname)
                     if self.__bond_has_diff(config_parent_ifname):
                         prune = True
@@ -521,7 +526,7 @@ class Reconciler:
         """Set default MTU and remove IPs for PHYs that are not in the config."""
         for vpp_ifname in self.vpp.get_phys():
             vpp_iface = self.vpp.cache["interface_names"][vpp_ifname]
-            config_ifname, config_iface = interface.get_by_name(self.cfg, vpp_ifname)
+            _config_ifname, config_iface = interface.get_by_name(self.cfg, vpp_ifname)
             if not config_iface:
                 ## Interfaces were sent DOWN in the prune_admin_state() step previously
                 self.prune_addresses(vpp_ifname, [])
@@ -589,25 +594,25 @@ class Reconciler:
 
         removed_lcps = []
         for numtags in [2, 1, 0]:
-            for idx, lcp in lcps.items():
+            for _idx, lcp_iface in lcps.items():
-                vpp_iface = self.vpp.cache["interfaces"][lcp.phy_sw_if_index]
+                vpp_iface = self.vpp.cache["interfaces"][lcp_iface.phy_sw_if_index]
                 if vpp_iface.sub_number_of_tags != numtags:
                     continue
                 if vpp_iface.interface_dev_type == "Loopback":
                     config_ifname, config_iface = loopback.get_by_lcp_name(
-                        self.cfg, lcp.host_if_name
+                        self.cfg, lcp_iface.host_if_name
                     )
                 else:
                     config_ifname, config_iface = interface.get_by_lcp_name(
-                        self.cfg, lcp.host_if_name
+                        self.cfg, lcp_iface.host_if_name
                     )
                 if not config_iface:
                     ## Interface doesn't exist in the config
-                    removed_lcps.append(lcp)
+                    removed_lcps.append(lcp_iface)
                     continue
                 if not "lcp" in config_iface:
                     ## Interface doesn't have an LCP
-                    removed_lcps.append(lcp)
+                    removed_lcps.append(lcp_iface)
                     continue
                 if vpp_iface.sub_number_of_tags == 2:
                     vpp_parent_idx = self.__parent_iface_by_encap(
@@ -623,15 +628,15 @@ class Reconciler:
                     ) = interface.get_by_lcp_name(self.cfg, parent_lcp.host_if_name)
                     if not config_parent_iface:
                         ## QinX's parent doesn't exist in the config
-                        removed_lcps.append(lcp)
+                        removed_lcps.append(lcp_iface)
                         continue
                     if not "lcp" in config_parent_iface:
                         ## QinX's parent doesn't have an LCP
-                        removed_lcps.append(lcp)
+                        removed_lcps.append(lcp_iface)
                         continue
                     if parent_lcp.host_if_name != config_parent_iface["lcp"]:
                         ## QinX's parent LCP name mismatch
-                        removed_lcps.append(lcp)
+                        removed_lcps.append(lcp_iface)
                         continue
                     config_parent_encap = interface.get_encapsulation(
                         self.cfg, config_parent_ifname
@@ -639,7 +644,7 @@ class Reconciler:
                     vpp_parent_encap = self.__get_encapsulation(vpp_parent_iface)
                     if config_parent_encap != vpp_parent_encap:
                         ## QinX's parent encapsulation mismatch
-                        removed_lcps.append(lcp)
+                        removed_lcps.append(lcp_iface)
                         continue
 
                 if vpp_iface.sub_number_of_tags > 0:
@@ -647,7 +652,7 @@ class Reconciler:
                     vpp_encap = self.__get_encapsulation(vpp_iface)
                     if config_encap != vpp_encap:
                         ## Encapsulation mismatch
-                        removed_lcps.append(lcp)
+                        removed_lcps.append(lcp_iface)
                         continue
 
                 if vpp_iface.interface_dev_type == "Loopback":
@@ -657,37 +662,37 @@ class Reconciler:
                     bond_iface = self.vpp.cache["interfaces"][vpp_iface.sup_sw_if_index]
                     if self.__bond_has_diff(bond_iface.interface_name):
                         ## If BondEthernet changed, it has to be re-created, so all LCPs must be removed.
-                        removed_lcps.append(lcp)
+                        removed_lcps.append(lcp_iface)
                         continue
 
                 phy_lcp = lcps[vpp_iface.sup_sw_if_index]
-                config_phy_ifname, config_phy_iface = interface.get_by_lcp_name(
+                _config_phy_ifname, config_phy_iface = interface.get_by_lcp_name(
                     self.cfg, phy_lcp.host_if_name
                 )
                 if not config_phy_iface:
                     ## Phy doesn't exist in the config
-                    removed_lcps.append(lcp)
+                    removed_lcps.append(lcp_iface)
                     continue
                 if not "lcp" in config_phy_iface:
                     ## Phy doesn't have an LCP
-                    removed_lcps.append(lcp)
+                    removed_lcps.append(lcp_iface)
                     continue
                 if phy_lcp.host_if_name != config_phy_iface["lcp"]:
                     ## Phy LCP name mismatch
-                    removed_lcps.append(lcp)
+                    removed_lcps.append(lcp_iface)
                     continue
 
                 self.logger.debug(
-                    f"LCP OK: {lcp.host_if_name} -> (vpp={vpp_iface.interface_name}, config={config_ifname})"
+                    f"LCP OK: {lcp_iface.host_if_name} -> (vpp={vpp_iface.interface_name}, config={config_ifname})"
                 )
 
-        for lcp in removed_lcps:
+        for lcp_iface in removed_lcps:
             vpp_ifname = self.vpp.cache["interfaces"][
-                lcp.phy_sw_if_index
+                lcp_iface.phy_sw_if_index
             ].interface_name
             cli = f"lcp delete {vpp_ifname}"
             self.cli["prune"].append(cli)
-            self.vpp.cache_remove_lcp(lcp.host_if_name)
+            self.vpp.cache_remove_lcp(lcp_iface.host_if_name)
         return True
 
     def prune_admin_state(self):
@@ -762,9 +767,9 @@ class Reconciler:
             instance = int(ifname[12:])
             mode = bondethernet.get_mode(self.cfg, ifname)
             cli = f"create bond id {int(instance)} mode {mode}"
-            lb = bondethernet.get_lb(self.cfg, ifname)
+            loadbalance = bondethernet.get_lb(self.cfg, ifname)
-            if lb:
+            if loadbalance:
-                cli += f" load-balance {lb}"
+                cli += f" load-balance {loadbalance}"
             if "mac" in iface:
                 cli += f" hw-addr {iface['mac']}"
             self.cli["create"].append(cli)
@@ -790,7 +795,7 @@ class Reconciler:
                 if not do_qinx == interface.is_qinx(self.cfg, ifname):
                     continue
 
-                ifname, iface = interface.get_by_name(self.cfg, ifname)
+                ifname, _iface = interface.get_by_name(self.cfg, ifname)
                 if ifname in self.vpp.cache["interface_names"]:
                     continue
 
@@ -802,7 +807,7 @@ class Reconciler:
                     encapstr = f"dot1q {int(encap['dot1q'])}"
                 if do_qinx:
                     encapstr += f" inner-dot1q {int(encap['inner-dot1q'])}"
-                if encap["exact-match"] == True:
+                if encap["exact-match"]:
                     encapstr += " exact-match"
                 parent, subid = ifname.split(".")
                 cli = f"create sub {parent} {int(int(subid))} {encapstr}"
@@ -834,7 +839,7 @@ class Reconciler:
 
     def create_bridgedomains(self):
         for ifname in bridgedomain.get_bridgedomains(self.cfg):
-            ifname, iface = bridgedomain.get_by_name(self.cfg, ifname)
+            ifname, _iface = bridgedomain.get_by_name(self.cfg, ifname)
             instance = int(ifname[2:])
             settings = bridgedomain.get_settings(self.cfg, ifname)
             if instance in self.vpp.cache["bridgedomains"]:
@@ -1076,7 +1081,7 @@ class Reconciler:
             if not "interfaces" in config_bridge_iface:
                 continue
             for member_ifname in config_bridge_iface["interfaces"]:
-                member_ifname, member_iface = interface.get_by_name(
+                member_ifname, _member_iface = interface.get_by_name(
                     self.cfg, member_ifname
                 )
                 if not member_ifname in bridge_members:
@@ -1094,7 +1099,7 @@ class Reconciler:
     def sync_l2xcs(self):
         for ifname in interface.get_l2xc_interfaces(self.cfg):
             config_rx_ifname, config_rx_iface = interface.get_by_name(self.cfg, ifname)
-            config_tx_ifname, config_tx_iface = interface.get_by_name(
+            config_tx_ifname, _config_tx_iface = interface.get_by_name(
                 self.cfg, config_rx_iface["l2xc"]
             )
             vpp_rx_iface = None
@@ -1171,13 +1176,13 @@ class Reconciler:
         return True
 
     def sync_link_mtu_direction(self, shrink=True):
-        for idx, vpp_iface in self.vpp.cache["interfaces"].items():
+        for _idx, vpp_iface in self.vpp.cache["interfaces"].items():
             if vpp_iface.sub_number_of_tags != 0:
                 continue
             if vpp_iface.interface_dev_type in ["local", "Loopback", "VXLAN", "virtio"]:
                 continue
 
-            config_ifname, config_iface = interface.get_by_name(
+            _config_ifname, config_iface = interface.get_by_name(
                 self.cfg, vpp_iface.interface_name
             )
             if not config_iface:
@@ -1266,10 +1271,10 @@ class Reconciler:
                         str(x)
                         for x in self.vpp.cache["interface_addresses"][sw_if_index]
                     ]
-            for a in config_addresses:
+            for addr in config_addresses:
-                if a in vpp_addresses:
+                if addr in vpp_addresses:
                     continue
-                cli = f"set interface ip address {vpp_ifname} {a}"
+                cli = f"set interface ip address {vpp_ifname} {addr}"
                 self.cli["sync"].append(cli)
         return True
 
@@ -1278,10 +1283,10 @@ class Reconciler:
             self.cfg
         ):
             if ifname.startswith("loop"):
-                vpp_ifname, config_iface = loopback.get_by_name(self.cfg, ifname)
+                vpp_ifname, _config_iface = loopback.get_by_name(self.cfg, ifname)
                 config_admin_state = 1
             else:
-                vpp_ifname, config_iface = interface.get_by_name(self.cfg, ifname)
+                vpp_ifname, _config_iface = interface.get_by_name(self.cfg, ifname)
                 config_admin_state = interface.get_admin_state(self.cfg, ifname)
 
             vpp_admin_state = 0
@@ -1298,39 +1303,39 @@ class Reconciler:
             self.cli["sync"].append(cli)
         return True
 
-    def write(self, outfile, ok=False):
+    def write(self, outfile, emit_ok=False):
         """Emit the CLI contents to stdout (if outfile=='-') or a named file otherwise.
-        If the 'ok' flag is False, emit a warning at the top and bottom of the file.
+        If the 'emit_ok' flag is False, emit a warning at the top and bottom of the file.
         """
         # Assemble the intended output into a list
         output = []
-        if not ok:
+        if not emit_ok:
             output.append(
                 "comment { vppcfg: Planning failed, be careful with this output! }"
             )
 
         for phase in ["prune", "create", "sync"]:
-            n = len(self.cli[phase])
+            ncount = len(self.cli[phase])
-            if n > 0:
+            if ncount > 0:
                 output.append(
-                    f"comment {{ vppcfg {phase}: {n} CLI statement(s) follow }}"
+                    f"comment {{ vppcfg {phase}: {ncount} CLI statement(s) follow }}"
                 )
                 output.extend(self.cli[phase])
 
-        if not ok:
+        if not emit_ok:
             output.append(
                 "comment { vppcfg: Planning failed, be careful with this output! }"
             )
 
         # Emit the output list to stdout or a file
         if outfile and outfile == "-":
-            fh = sys.stdout
+            file = sys.stdout
             outfile = "(stdout)"
         else:
-            fh = open(outfile, "w")
+            file = open(outfile, "w", encoding="utf-8")
         if len(output) > 0:
-            print("\n".join(output), file=fh)
+            print("\n".join(output), file=file)
-        if fh is not sys.stdout:
+        if file is not sys.stdout:
-            fh.close()
+            file.close()
 
         self.logger.info(f"Wrote {len(output)} lines to {outfile}")

vpp/vppapi.py

@@ -4,10 +4,11 @@ interface metadata. Its base class will never change state. See the
 derived classes VPPApiDumper() and VPPApiApplier()
 """
 
-from vpp_papi import VPPApiClient
 import os
 import fnmatch
 import logging
+import socket
+from vpp_papi import VPPApiClient
 
 
 class VPPApi:
@@ -32,7 +33,7 @@ class VPPApi:
 
         # construct a list of all the json api files
         jsonfiles = []
-        for root, dirnames, filenames in os.walk(vpp_json_dir):
+        for root, _dirnames, filenames in os.walk(vpp_json_dir):
             for filename in fnmatch.filter(filenames, "*.api.json"):
                 jsonfiles.append(os.path.join(root, filename))
 
@@ -47,8 +48,9 @@ class VPPApi:
         except:
             return False
 
-        v = self.vpp.api.show_version()
+        # pylint: disable=no-member
-        self.logger.info(f"VPP version is {v.version}")
+        api_response = self.vpp.api.show_version()
+        self.logger.info(f"VPP version is {api_response.version}")
 
         self.connected = True
         return True
@@ -78,22 +80,15 @@ class VPPApi:
 
     def cache_remove_lcp(self, lcpname):
         """Removes the LCP and TAP interface, identified by lcpname, from the config."""
-        found = False
+        for _idx, lcp in self.cache["lcps"].items():
-        for idx, lcp in self.cache["lcps"].items():
             if lcp.host_if_name == lcpname:
-                found = True
+                ifname = self.cache["interfaces"][lcp.host_sw_if_index].interface_name
-                break
+                del self.cache["lcps"][lcp.phy_sw_if_index]
-        if not found:
+                return self.cache_remove_interface(ifname)
-            self.logger.warning(
+        self.logger.warning(
-                f"Trying to remove an LCP which is not in the config: {lcpname}"
+            f"Trying to remove an LCP which is not in the config: {lcpname}"
-            )
+        )
-            return False
+        return False
-
-        ifname = self.cache["interfaces"][lcp.host_sw_if_index].interface_name
-        del self.cache["lcps"][lcp.phy_sw_if_index]
-
-        # Remove the TAP interface and its dependencies
-        return self.cache_remove_interface(ifname)
 
     def cache_remove_bondethernet_member(self, ifname):
         """Removes the bonderthernet member interface, identified by name, from the config."""
@@ -159,6 +154,7 @@ class VPPApi:
         return True
 
     def readconfig(self):
+        # pylint: disable=no-member
         if not self.connected and not self.connect():
             self.logger.error("Could not connect to VPP")
             return False
@@ -169,9 +165,9 @@ class VPPApi:
         self.lcp_enabled = False
         try:
             self.logger.debug("Retrieving LCPs")
-            r = self.vpp.api.lcp_itf_pair_get()
+            api_response = self.vpp.api.lcp_itf_pair_get()
-            if isinstance(r, tuple) and r[0].retval == 0:
+            if isinstance(api_response, tuple) and api_response[0].retval == 0:
-                for lcp in r[1]:
+                for lcp in api_response[1]:
                     if lcp.phy_sw_if_index > 65535 or lcp.host_sw_if_index > 65535:
                         ## Work around endianness bug: https://gerrit.fd.io/r/c/vpp/+/35479
                         ## TODO(pim) - remove this when 22.06 ships
@@ -193,8 +189,8 @@ class VPPApi:
             )
 
         self.logger.debug("Retrieving interfaces")
-        r = self.vpp.api.sw_interface_dump()
+        api_response = self.vpp.api.sw_interface_dump()
-        for iface in r:
+        for iface in api_response:
             self.cache["interfaces"][iface.sw_if_index] = iface
             self.cache["interface_names"][iface.interface_name] = iface
             self.cache["interface_addresses"][iface.sw_if_index] = []
@@ -202,22 +198,22 @@ class VPPApi:
             ipr = self.vpp.api.ip_address_dump(
                 sw_if_index=iface.sw_if_index, is_ipv6=False
             )
-            for ip in ipr:
+            for addr in ipr:
                 self.cache["interface_addresses"][iface.sw_if_index].append(
-                    str(ip.prefix)
+                    str(addr.prefix)
                 )
             self.logger.debug(f"Retrieving IPv6 addresses for {iface.interface_name}")
             ipr = self.vpp.api.ip_address_dump(
                 sw_if_index=iface.sw_if_index, is_ipv6=True
             )
-            for ip in ipr:
+            for addr in ipr:
                 self.cache["interface_addresses"][iface.sw_if_index].append(
-                    str(ip.prefix)
+                    str(addr.prefix)
                 )
 
         self.logger.debug("Retrieving bondethernets")
-        r = self.vpp.api.sw_bond_interface_dump()
+        api_response = self.vpp.api.sw_bond_interface_dump()
-        for iface in r:
+        for iface in api_response:
             self.cache["bondethernets"][iface.sw_if_index] = iface
             self.cache["bondethernet_members"][iface.sw_if_index] = []
             for member in self.vpp.api.sw_member_interface_dump(
@@ -228,23 +224,23 @@ class VPPApi:
                 )
 
         self.logger.debug("Retrieving bridgedomains")
-        r = self.vpp.api.bridge_domain_dump()
+        api_response = self.vpp.api.bridge_domain_dump()
-        for bridge in r:
+        for bridge in api_response:
             self.cache["bridgedomains"][bridge.bd_id] = bridge
 
         self.logger.debug("Retrieving vxlan_tunnels")
-        r = self.vpp.api.vxlan_tunnel_v2_dump()
+        api_response = self.vpp.api.vxlan_tunnel_v2_dump()
-        for vxlan in r:
+        for vxlan in api_response:
             self.cache["vxlan_tunnels"][vxlan.sw_if_index] = vxlan
 
         self.logger.debug("Retrieving L2 Cross Connects")
-        r = self.vpp.api.l2_xconnect_dump()
+        api_response = self.vpp.api.l2_xconnect_dump()
-        for l2xc in r:
+        for l2xc in api_response:
             self.cache["l2xcs"][l2xc.rx_sw_if_index] = l2xc
 
         self.logger.debug("Retrieving TAPs")
-        r = self.vpp.api.sw_interface_tap_v2_dump()
+        api_response = self.vpp.api.sw_interface_tap_v2_dump()
-        for tap in r:
+        for tap in api_response:
             self.cache["taps"][tap.sw_if_index] = tap
 
         self.cache_read = True
@@ -322,7 +318,7 @@ class VPPApi:
         return vxlan_tunnels
 
     def get_lcp_by_interface(self, sw_if_index):
-        for idx, lcp in self.cache["lcps"].items():
+        for _idx, lcp in self.cache["lcps"].items():
             if lcp.phy_sw_if_index == sw_if_index:
                 return lcp
         return None
@@ -337,7 +333,7 @@ class VPPApi:
         if not vpp_iface.interface_dev_type == "virtio":
             return False
 
-        for idx, lcp in self.cache["lcps"].items():
+        for _idx, lcp in self.cache["lcps"].items():
             if vpp_iface.sw_if_index == lcp.host_sw_if_index:
                 return True
         return False

vppcfg

@@ -13,10 +13,11 @@
 # limitations under the License.
 #
 # -*- coding: utf-8 -*-
-
+"""vppcfg is a utility to configure a running VPP Dataplane using YAML
+config files. See http://github.com/pimvanpelt/vppcfg/README.md for details. """
 import sys
-import yaml
 import logging
+import yaml
 from config import Validator
 from vpp.reconciler import Reconciler
 from vpp.dumper import Dumper
@@ -149,64 +150,64 @@ def main():
     )
 
     if args.command == "dump":
-        d = Dumper()
+        dumper = Dumper()
-        if not d.readconfig():
+        if not dumper.readconfig():
             logging.error("Could not retrieve config from VPP")
             sys.exit(-7)
-        d.write(args.outfile)
+        dumper.write(args.outfile)
         sys.exit(0)
 
     try:
-        with open(args.config, "r") as f:
+        with open(args.config, "r", encoding="utf-8") as file:
             logging.info(f"Loading configfile {args.config}")
-            cfg = yaml.load(f, Loader=yaml.FullLoader)
+            cfg = yaml.load(file, Loader=yaml.FullLoader)
             logging.debug(f"Config: {cfg}")
-    except:
+    except OSError as err:
-        logging.error(f"Couldn't read config from {args.config}")
+        logging.error(f"Couldn't read config from {args.config}: {err}")
         sys.exit(-1)
 
-    v = Validator(schema=args.schema)
+    validator = Validator(schema=args.schema)
-    if not v.valid_config(cfg):
+    if not validator.valid_config(cfg):
         logging.error("Configuration is not valid, bailing")
         sys.exit(-2)
     logging.info("Configuration is valid")
     if args.command == "check":
         sys.exit(0)
 
-    r = Reconciler(cfg)
+    reconciler = Reconciler(cfg)
-    if not r.vpp_readconfig():
+    if not reconciler.vpp_readconfig():
         sys.exit(-3)
 
-    if not r.phys_exist_in_vpp():
+    if not reconciler.phys_exist_in_vpp():
         logging.error("Not all PHYs in the config exist in VPP")
         sys.exit(-4)
 
-    if not r.phys_exist_in_config():
+    if not reconciler.phys_exist_in_config():
         logging.error("Not all PHYs in VPP exist in the config")
         sys.exit(-5)
 
-    if not r.lcps_exist_with_lcp_enabled():
+    if not reconciler.lcps_exist_with_lcp_enabled():
         logging.error(
             "Linux Control Plane is needed, but linux-cp API is not available"
         )
         sys.exit(-6)
 
     failed = False
-    if not r.prune():
+    if not reconciler.prune():
         if not args.force:
             logging.error("Planning prune failure")
             sys.exit(-10)
         failed = True
         logging.warning("Planning prune failure, continuing due to --force")
 
-    if not r.create():
+    if not reconciler.create():
         if not args.force:
             logging.error("Planning create failure")
             sys.exit(-20)
         failed = True
         logging.warning("Planning create failure, continuing due to --force")
 
-    if not r.sync():
+    if not reconciler.sync():
         if not args.force:
             logging.error("Planning sync failure")
             sys.exit(-30)
@@ -214,7 +215,7 @@ def main():
         logging.warning("Planning sync failure, continuing due to --force")
 
     if args.command == "plan":
-        r.write(args.outfile, ok=not failed)
+        reconciler.write(args.outfile, emit_ok=not failed)
 
     if failed:
         logging.error("Planning failed")