diff --git a/.pylintrc b/.pylintrc new file mode 100644 index 0000000..646c5ae --- /dev/null +++ b/.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 diff --git a/README.md b/README.md index e29ef36..5162bc3 100644 --- a/README.md +++ b/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 diff --git a/config/__init__.py b/config/__init__.py index 9ad5725..1e4365b 100644 --- a/config/__init__.py +++ b/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("/") - if not len(e) == 2: + elems = value.split("/") + 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 - self.logger.debug(f"Validating against --schema {fn}") + fname = self.schema + 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" - self.logger.debug(f"Validating against fallthrough default schema {fn}") + fname = "./schema.yaml" + self.logger.debug(f"Validating against fallthrough default schema {fname}") - if not os.path.isfile(fn): - self.logger.error(f"Cannot file schema file: {fn}") + if not os.path.isfile(fname): + 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: - rv, msgs = v(yaml) + for validator in self.validators: + retval, msgs = validator(yaml) if msgs: ret_msgs.extend(msgs) - if not rv: - ret_rv = False + if not retval: + 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) - if not rv: - for m in msgs: - self.logger.error(m) + retval, msgs = self.validate(yaml) + if not retval: + for msg in msgs: + self.logger.error(msg) return False self.logger.info("Configuration validated successfully") diff --git a/config/address.py b/config/address.py index 73cea84..46776d5 100644 --- a/config/address.py +++ b/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"]: - ret.append(ipaddress.ip_interface(a)) + for addr in iface["addresses"]: + 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"]: - ret.append(ipaddress.ip_interface(a)) + for addr in sub_iface["addresses"]: + 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"]: - ret.append(ipaddress.ip_interface(a)) + for addr in iface["addresses"]: + 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"]: - ret.append(ipaddress.ip_interface(a)) + for addr in iface["addresses"]: + 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: - ipi = ipaddress.ip_interface(ip) + for addr in iface_addresses: + ipi = ipaddress.ip_interface(addr) if ipi.version != my_ip_network.version: continue diff --git a/config/bondethernet.py b/config/bondethernet.py index 49829ce..26bc79b 100644 --- a/config/bondethernet.py +++ b/config/bondethernet.py @@ -12,15 +12,15 @@ # limitations under the License. # import logging -import config.interface as interface -import config.mac as mac +from config import interface +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] - except: + return ret[loadbalance] + 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] - except: + return ret[loadbalance] + 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( diff --git a/config/bridgedomain.py b/config/bridgedomain.py index e93d9ed..bed5c5c 100644 --- a/config/bridgedomain.py +++ b/config/bridgedomain.py @@ -12,10 +12,8 @@ # limitations under the License. # import logging -import config.interface as interface -import config.loopback as loopback -import config.lcp as lcp -import config.address as address +from config import interface +from config import loopback 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 - for ifname, iface in yaml["bridgedomains"].items(): + ncount = 0 + for _ifname, iface in yaml["bridgedomains"].items(): if "bvi" in iface and iface["bvi"] == bviname: - n += 1 - return n < 2 + ncount += 1 + 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") diff --git a/config/interface.py b/config/interface.py index f8cb9ce..6ecdad9 100644 --- a/config/interface.py +++ b/config/interface.py @@ -12,14 +12,14 @@ # limitations under the License. # import logging -import config.bondethernet as bondethernet -import config.bridgedomain as bridgedomain -import config.loopback as loopback -import config.vxlan_tunnel as vxlan_tunnel -import config.lcp as lcp -import config.address as address -import config.mac as mac -import config.tap as tap +from config import bondethernet +from config import bridgedomain +from config import loopback +from config import vxlan_tunnel +from config import lcp +from config import address +from config import mac +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 ( - "exact-match" in encap - and encap["exact-match"] == False - and has_lcp(yaml, ifname) - ): + if "exact-match" in encap and not encap["exact-match"] 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: - return 1500 - - parent_ifname, parent_iface = get_parent_by_name(yaml, ifname) - - try: + if iface and "mtu" in iface: 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"]: - if not address.is_allowed(yaml, ifname, iface["addresses"], a): + for addr in iface["addresses"]: + 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): diff --git a/config/lcp.py b/config/lcp.py index a903505..57d76f3 100644 --- a/config/lcp.py +++ b/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"]) diff --git a/config/loopback.py b/config/loopback.py index 44f759c..92b981c 100644 --- a/config/loopback.py +++ b/config/loopback.py @@ -12,16 +12,16 @@ # limitations under the License. # import logging -import config.lcp as lcp -import config.address as address -import config.mac as mac +from config import lcp +from config import address +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"]: - if not address.is_allowed(yaml, ifname, iface["addresses"], a): + for addr in iface["addresses"]: + 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"]): diff --git a/config/mac.py b/config/mac.py index 79316d8..b42448b 100644 --- a/config/mac.py +++ b/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 diff --git a/config/tap.py b/config/tap.py index 96775b9..9e40b30 100644 --- a/config/tap.py +++ b/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"] - if n & (n - 1) != 0: + ncount = iface["rx-ring-size"] + 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"] - if n & (n - 1) != 0: + ncount = iface["tx-ring-size"] + if ncount & (ncount - 1) != 0: msgs.append(f"tap {ifname} tx-ring-size must be a power of two") result = False diff --git a/config/test_interface.py b/config/test_interface.py index acc4bdf..791c24f 100644 --- a/config/test_interface.py +++ b/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( diff --git a/config/vxlan_tunnel.py b/config/vxlan_tunnel.py index 3e7e1bd..0811cce 100644 --- a/config/vxlan_tunnel.py +++ b/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 diff --git a/intest/hippo5.yaml b/intest/hippo5.yaml index 600dbc4..7aa03cc 100644 --- a/intest/hippo5.yaml +++ b/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" diff --git a/tests.py b/tests.py index 4195a55..2600dd8 100755 --- a/tests.py +++ b/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 - msgs = [] - - return rv, msgs + return True, [] 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: - for data in yaml.load_all(f, Loader=yaml.Loader): - if n == 0: - unittest = data - n = n + 1 - elif n == 1: + with open(self.yaml_filename, "r", encoding="utf-8") as file: + for data in yaml.load_all(file, Loader=yaml.Loader): + if ncount == 0: + test = data + ncount += 1 + elif ncount == 1: cfg = data - n = n + 1 + ncount += 1 except: pass - self.assertEqual(n, 2) - self.assertIsNotNone(unittest) + self.assertEqual(ncount, 2) + self.assertIsNotNone(test) if not cfg: return - v = Validator(schema=self.yaml_schema) - rv, msgs = v.validate(cfg) + validator = Validator(schema=self.yaml_schema) + _rv, msgs = validator.validate(cfg) - msgs_unexpected = 0 msgs_expected = [] if ( - "test" in unittest - and "errors" in unittest["test"] - and "expected" in unittest["test"]["errors"] + "test" in test + and "errors" in test["test"] + 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 - and "errors" in unittest["test"] - and "count" in unittest["test"]["errors"] + "test" in test + and "errors" in test["test"] + 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 - sys.exit(retval) + RETVAL -= 2 + sys.exit(RETVAL) diff --git a/unittest/yaml/error-bridgedomain6.yaml b/unittest/yaml/error-bridgedomain6.yaml index 3411634..85da7c3 100644 --- a/unittest/yaml/error-bridgedomain6.yaml +++ b/unittest/yaml/error-bridgedomain6.yaml @@ -15,6 +15,7 @@ interfaces: mtu: 3000 sub-interfaces: 1234: + mtu: 1500 description: "BD11 and BD12" bridgedomains: diff --git a/vpp/applier.py b/vpp/applier.py index 6d729fe..d451fea 100644 --- a/vpp/applier.py +++ b/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 diff --git a/vpp/dumper.py b/vpp/dumper.py index 6dbca4b..77a247b 100644 --- a/vpp/dumper.py +++ b/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: - fh.close() + if file is not sys.stdout: + file.close() self.logger.info(f"Wrote YAML config to {outfile}") def cache_to_config(self): diff --git a/vpp/reconciler.py b/vpp/reconciler.py index 60460db..3400220 100644 --- a/vpp/reconciler.py +++ b/vpp/reconciler.py @@ -15,13 +15,13 @@ # -*- coding: utf-8 -*- import sys import logging -import config.loopback as loopback -import config.interface as interface -import config.bondethernet as bondethernet -import config.bridgedomain as bridgedomain -import config.vxlan_tunnel as vxlan_tunnel -import config.lcp as lcp -import config.tap as tap +from config import loopback +from config import interface +from config import bondethernet +from config import bridgedomain +from config import vxlan_tunnel +from config import lcp +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]: - if not a in address_list: - cli = f"set interface ip address del {ifname} {a}" + for addr in self.vpp.cache["interface_addresses"][idx]: + if not addr in address_list: + 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}") - for a in removed_addresses: - self.vpp.cache["interface_addresses"][idx].remove(a) + self.logger.debug(f"Address OK: {ifname} {addr}") + for addr in removed_addresses: + 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) - members = [] + _config_ifname, config_iface = bridgedomain.get_by_name( + 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)) - if lb != -1 and lb != vpp_bond.lb: + loadbalance = bondethernet.lb_to_int( + 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(): - vpp_iface = self.vpp.cache["interfaces"][lcp.phy_sw_if_index] + for _idx, lcp_iface in lcps.items(): + 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) - if lb: - cli += f" load-balance {lb}" + loadbalance = bondethernet.get_lb(self.cfg, ifname) + if loadbalance: + 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: - if a in vpp_addresses: + for addr in config_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]) - if n > 0: + ncount = len(self.cli[phase]) + 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) - if fh is not sys.stdout: - fh.close() + print("\n".join(output), file=file) + if file is not sys.stdout: + file.close() self.logger.info(f"Wrote {len(output)} lines to {outfile}") diff --git a/vpp/vppapi.py b/vpp/vppapi.py index 04afadd..d2501ad 100644 --- a/vpp/vppapi.py +++ b/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() - self.logger.info(f"VPP version is {v.version}") + # pylint: disable=no-member + 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 - break - if not found: - self.logger.warning( - f"Trying to remove an LCP which is not in the config: {lcpname}" - ) - 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) + ifname = self.cache["interfaces"][lcp.host_sw_if_index].interface_name + del self.cache["lcps"][lcp.phy_sw_if_index] + return self.cache_remove_interface(ifname) + self.logger.warning( + f"Trying to remove an LCP which is not in the config: {lcpname}" + ) + return False 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() - if isinstance(r, tuple) and r[0].retval == 0: - for lcp in r[1]: + api_response = self.vpp.api.lcp_itf_pair_get() + if isinstance(api_response, tuple) and api_response[0].retval == 0: + 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() - for iface in r: + api_response = self.vpp.api.sw_interface_dump() + 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() - for iface in r: + api_response = self.vpp.api.sw_bond_interface_dump() + 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() - for bridge in r: + api_response = self.vpp.api.bridge_domain_dump() + 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() - for vxlan in r: + api_response = self.vpp.api.vxlan_tunnel_v2_dump() + 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() - for l2xc in r: + api_response = self.vpp.api.l2_xconnect_dump() + 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() - for tap in r: + api_response = self.vpp.api.sw_interface_tap_v2_dump() + 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 diff --git a/vppcfg b/vppcfg index 2f6fd0f..16bbc95 100755 --- a/vppcfg +++ b/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() - if not d.readconfig(): + dumper = Dumper() + 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: - logging.error(f"Couldn't read config from {args.config}") + except OSError as err: + logging.error(f"Couldn't read config from {args.config}: {err}") sys.exit(-1) - v = Validator(schema=args.schema) - if not v.valid_config(cfg): + validator = Validator(schema=args.schema) + 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) - if not r.vpp_readconfig(): + reconciler = Reconciler(cfg) + 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")