From ba22b1aad81b6251a295515d1df54ba729ebef37 Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Mon, 21 Mar 2022 01:18:03 +0000 Subject: [PATCH] Refactor for *_get_by_name() They now all return a list [ifname, iface]. If no interface was found they return None,None. If one was found, they return the (string) name and the dictionary with interface contents. --- tests.py | 13 ++++-- validator/bondethernet.py | 8 ++-- validator/bridgedomain.py | 9 ++-- validator/interface.py | 85 ++++++++++++++++++++----------------- validator/loopback.py | 2 +- validator/test_interface.py | 20 +++++++-- validator/test_lcp.py | 6 +-- validator/vxlan_tunnel.py | 2 +- 8 files changed, 84 insertions(+), 61 deletions(-) diff --git a/tests.py b/tests.py index 4fca629..65b54d5 100755 --- a/tests.py +++ b/tests.py @@ -51,8 +51,8 @@ class YAMLTest(unittest.TestCase): n = n + 1 except: pass - assert n == 2, "%s: Too many documents" % self.yaml_filename - assert unittest, "%s: Couldn't extract unittest metadata" % self.yaml_filename + self.assertEqual(n,2) + self.assertIsNotNone(unittest) if not cfg: return @@ -61,13 +61,16 @@ class YAMLTest(unittest.TestCase): count = 0 if 'test' in unittest and 'errors' in unittest['test'] and 'count' in unittest['test']['errors']: count = unittest['test']['errors']['count'] - assert len(msgs) == count, "%s: Expected %d error messages, got %d" % (self.yaml_filename, count, len(msgs)) + if len(msgs) != count: + print(msgs, file=sys.stderr) + self.assertEqual(len(msgs), count) msgs_unexpected = 0 msgs_expected = [] if 'test' in unittest and 'errors' in unittest['test'] and 'expected' in unittest['test']['errors']: msgs_expected = unittest['test']['errors']['expected'] + fail = False for m in msgs: this_msg_expected = False for expected in msgs_expected: @@ -75,7 +78,9 @@ class YAMLTest(unittest.TestCase): this_msg_expected = True break if not this_msg_expected: - assert this_msg_expected, "%s: Unexpected message: %s" % (self.yaml_filename, m) + print("%s: Unexpected message: %s" % (self.yaml_filename, m), file=sys.stderr) + fail = True + self.assertFalse(fail) return diff --git a/validator/bondethernet.py b/validator/bondethernet.py index d78f536..464e252 100644 --- a/validator/bondethernet.py +++ b/validator/bondethernet.py @@ -20,13 +20,13 @@ class NullHandler(logging.Handler): def get_by_name(yaml, ifname): - """ Return the BondEthernet by name, if it exists. Return None otherwise. """ + """ Return the BondEthernet by name, if it exists. Return None,None otherwise. """ try: if ifname in yaml['bondethernets']: - return yaml['bondethernets'][ifname] + return ifname, yaml['bondethernets'][ifname] except: pass - return None + return None, None def is_bond_member(yaml, ifname): @@ -54,7 +54,7 @@ def validate_bondethernets(yaml): for ifname, iface in yaml['bondethernets'].items(): logger.debug("bondethernet %s: %s" % (ifname, iface)) for member in iface['interfaces']: - if not interface.get_by_name(yaml, member): + if (None, None) == interface.get_by_name(yaml, member): msgs.append("bondethernet %s member %s does not exist" % (ifname, member)) result = False diff --git a/validator/bridgedomain.py b/validator/bridgedomain.py index 4f393c0..4907c45 100644 --- a/validator/bridgedomain.py +++ b/validator/bridgedomain.py @@ -22,13 +22,13 @@ class NullHandler(logging.Handler): def get_by_name(yaml, ifname): - """ Return the BridgeDomain by name, if it exists. Return None otherwise. """ + """ Return the BridgeDomain by name, if it exists. Return None,None otherwise. """ try: if ifname in yaml['bridgedomains']: - return yaml['bridgedomains'][ifname] + return ifname, yaml['bridgedomains'][ifname] except: pass - return None + return None, None def get_bridge_interfaces(yaml): @@ -86,8 +86,7 @@ def validate_bridgedomains(yaml): if 'interfaces' in iface: for member in iface['interfaces']: - member_iface = interface.get_by_name(yaml, member) - if not member_iface: + if (None, None) == interface.get_by_name(yaml, member): msgs.append("bridgedomain %s member %s does not exist" % (ifname, member)) result = False continue diff --git a/validator/interface.py b/validator/interface.py index 3942986..07a251b 100644 --- a/validator/interface.py +++ b/validator/interface.py @@ -22,18 +22,18 @@ class NullHandler(logging.Handler): pass def get_qinx_parent_by_name(yaml, ifname): - """ Returns the sub-interface which matches a QinAD or QinQ outer tag, or None + """ Returns the sub-interface which matches a QinAD or QinQ outer tag, or None,None if that sub-interface doesn't exist. """ if not is_qinx(yaml, ifname): - return None - qinx_iface = get_by_name(yaml, ifname) + return None, None + qinx_ifname, qinx_iface = get_by_name(yaml, ifname) if not qinx_iface: - return None + return None,None qinx_encap = get_encapsulation(yaml, ifname) if not qinx_encap: - return None + return None,None for ifname, iface in yaml['interfaces'].items(): for subid, sub_iface in iface['sub-interfaces'].items(): @@ -42,48 +42,48 @@ def get_qinx_parent_by_name(yaml, ifname): if not sub_encap: continue if qinx_encap['dot1q'] > 0 and sub_encap['dot1q'] == qinx_encap['dot1q']: - return sub_iface + return sub_ifname, sub_iface if qinx_encap['dot1ad'] > 0 and sub_encap['dot1ad'] == qinx_encap['dot1ad']: - return sub_iface - return None + return sub_ifname, sub_iface + return None,None def get_parent_by_name(yaml, ifname): - """ Returns the sub-interface's parent, or None if the sub-int doesn't exist. """ + """ Returns the sub-interface's parent, or None,None if the sub-int doesn't exist. """ if not '.' in ifname: - return None + return None, None try: - ifname, subid = ifname.split('.') + parent_ifname, subid = ifname.split('.') subid = int(subid) - iface = yaml['interfaces'][ifname] - return iface + iface = yaml['interfaces'][parent_ifname] + return parent_ifname, iface except: pass - return None + return None,None def get_by_name(yaml, ifname): - """ Returns the interface or sub-interface by a given name, or None if it does not exist """ + """ Returns the interface or sub-interface by a given name, or None,None if it does not exist """ if '.' in ifname: try: - ifname, subid = ifname.split('.') + phy_ifname, subid = ifname.split('.') subid = int(subid) - iface = yaml['interfaces'][ifname]['sub-interfaces'][subid] - return iface + iface = yaml['interfaces'][phy_ifname]['sub-interfaces'][subid] + return ifname, iface except: - return None + return None, None try: iface = yaml['interfaces'][ifname] - return iface + return ifname, iface except: pass - return None + return None, None def is_sub(yaml, ifname): """ Returns True if this interface is a sub-interface """ - parent_iface = get_parent_by_name(yaml, ifname) + parent_ifname, parent_iface = get_parent_by_name(yaml, ifname) return isinstance(parent_iface, dict) @@ -102,7 +102,7 @@ def has_sub(yaml, ifname): def has_address(yaml, ifname): """ Returns True if this interface or sub-interface has one or more addresses""" - iface = get_by_name(yaml, ifname) + ifname, iface = get_by_name(yaml, ifname) if not iface: return False return 'addresses' in iface @@ -161,7 +161,7 @@ def is_l2xc_target_interface_unique(yaml, ifname): def has_lcp(yaml, ifname): """ Returns True if this interface or sub-interface has an LCP """ - iface = get_by_name(yaml, ifname) + ifname, iface = get_by_name(yaml, ifname) if not iface: return False return 'lcp' in iface @@ -170,7 +170,9 @@ def has_lcp(yaml, ifname): def valid_encapsulation(yaml, ifname): """ Returns True if the sub interface has a valid encapsulation, or none at all """ - iface = get_by_name(yaml, ifname) + ifname, iface = get_by_name(yaml, ifname) + if not iface: + return True if not 'encapsulation' in iface: return True @@ -198,8 +200,8 @@ def get_encapsulation(yaml, ifname): if not valid_encapsulation(yaml, ifname): return None - iface = get_by_name(yaml, ifname) - parent_iface = get_parent_by_name(yaml, ifname) + ifname, iface = get_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('.') @@ -277,13 +279,12 @@ def is_qinx(yaml, ifname): def unique_encapsulation(yaml, sub_ifname): """ Ensures that for the sub_ifname specified, there exist no other sub-ints on the parent with the same encapsulation. """ - iface = get_by_name(yaml, sub_ifname) - parent_iface = get_parent_by_name(yaml, sub_ifname) + new_ifname, iface = get_by_name(yaml, sub_ifname) + parent_ifname, parent_iface = get_parent_by_name(yaml, new_ifname) if not iface or not parent_iface: return False - parent_ifname, subid = sub_ifname.split('.') - sub_encap = get_encapsulation(yaml, sub_ifname) + sub_encap = get_encapsulation(yaml, new_ifname) if not sub_encap: return False @@ -291,7 +292,7 @@ def unique_encapsulation(yaml, sub_ifname): for subid, sibling_iface in parent_iface['sub-interfaces'].items(): sibling_ifname = "%s.%d" % (parent_ifname, subid) sibling_encap = get_encapsulation(yaml, sibling_ifname) - if sub_encap == sibling_encap and sub_ifname != sibling_ifname: + if sub_encap == sibling_encap and new_ifname != sibling_ifname: ## print("%s overlaps with %s" % (sub_encap, sibling_encap)) ncount = ncount + 1 @@ -317,8 +318,11 @@ def get_lcp(yaml, ifname): enabled, synthesize it based on its parent, using smart QinQ syntax. Return None if no LCP can be found. """ - iface = get_by_name(yaml, ifname) - parent_iface = get_parent_by_name(yaml, ifname) + ifname, iface = get_by_name(yaml, ifname) + if not iface: + return None + + parent_ifname, parent_iface = get_parent_by_name(yaml, ifname) if 'lcp' in iface: return iface['lcp'] if is_l2(yaml, ifname): @@ -357,8 +361,11 @@ def get_lcp(yaml, ifname): 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.""" - iface = get_by_name(yaml, ifname) - parent_iface = get_parent_by_name(yaml, ifname) + ifname, iface = get_by_name(yaml, ifname) + if not iface: + return 1500 + + parent_ifname, parent_iface = get_parent_by_name(yaml, ifname) try: return iface['mtu'] @@ -379,7 +386,7 @@ def validate_interfaces(yaml): for ifname, iface in yaml['interfaces'].items(): logger.debug("interface %s" % iface) - if ifname.startswith("BondEthernet") and not bondethernet.get_by_name(yaml, ifname): + if ifname.startswith("BondEthernet") and (None,None) == bondethernet.get_by_name(yaml, ifname): msgs.append("interface %s does not exist in bondethernets" % ifname) result = False @@ -413,7 +420,7 @@ def validate_interfaces(yaml): if iface_address: msgs.append("interface %s has l2xc so it cannot have an address" % (ifname)) result = False - if not get_by_name(yaml, iface['l2xc']): + if (None,None) == get_by_name(yaml, iface['l2xc']): msgs.append("interface %s l2xc target %s does not exist" % (ifname, iface['l2xc'])) result = False if iface['l2xc'] == ifname: @@ -482,7 +489,7 @@ def validate_interfaces(yaml): if has_address(yaml, sub_ifname): msgs.append("sub-interface %s has l2xc so it cannot have an address" % (sub_ifname)) result = False - if not get_by_name(yaml, sub_iface['l2xc']): + if (None, None) == get_by_name(yaml, sub_iface['l2xc']): msgs.append("sub-interface %s l2xc target %s does not exist" % (sub_ifname, sub_iface['l2xc'])) result = False if sub_iface['l2xc'] == sub_ifname: diff --git a/validator/loopback.py b/validator/loopback.py index b6bfee1..32330eb 100644 --- a/validator/loopback.py +++ b/validator/loopback.py @@ -23,7 +23,7 @@ def get_by_name(yaml, ifname): """ Return the loopback by name, if it exists. Return None otherwise. """ try: if ifname in yaml['loopbacks']: - return yaml['loopbacks'][ifname] + return ifname, yaml['loopbacks'][ifname] except: pass return None diff --git a/validator/test_interface.py b/validator/test_interface.py index ca972c1..f3a82ba 100644 --- a/validator/test_interface.py +++ b/validator/test_interface.py @@ -118,7 +118,19 @@ class TestInterfaceMethods(unittest.TestCase): def test_qinx_parent(self): self.assertIsNotNone(interface.get_qinx_parent_by_name(self.cfg, "GigabitEthernet1/0/1.202")) self.assertIsNotNone(interface.get_qinx_parent_by_name(self.cfg, "GigabitEthernet1/0/1.203")) - self.assertIsNone(interface.get_qinx_parent_by_name(self.cfg, "GigabitEthernet1/0/1")) - self.assertIsNone(interface.get_qinx_parent_by_name(self.cfg, "GigabitEthernet1/0/1.100")) - self.assertIsNone(interface.get_qinx_parent_by_name(self.cfg, "GigabitEthernet1/0/1.200")) - self.assertIsNone(interface.get_qinx_parent_by_name(self.cfg, "GigabitEthernet1/0/1.201")) + + ifname, iface = interface.get_qinx_parent_by_name(self.cfg, "GigabitEthernet1/0/1") + self.assertIsNone(iface) + self.assertIsNone(ifname) + + ifname, iface = interface.get_qinx_parent_by_name(self.cfg, "GigabitEthernet1/0/1.100") + self.assertIsNone(iface) + self.assertIsNone(ifname) + + ifname, iface = interface.get_qinx_parent_by_name(self.cfg, "GigabitEthernet1/0/1.200") + self.assertIsNone(iface) + self.assertIsNone(ifname) + + ifname, iface = interface.get_qinx_parent_by_name(self.cfg, "GigabitEthernet1/0/1.201") + self.assertIsNone(iface) + self.assertIsNone(ifname) diff --git a/validator/test_lcp.py b/validator/test_lcp.py index 3f0798d..8a0fae6 100644 --- a/validator/test_lcp.py +++ b/validator/test_lcp.py @@ -16,9 +16,9 @@ class TestLCPMethods(unittest.TestCase): ## self.assertFalse(lcp.is_unique(self.cfg, "e1.1000")) def test_qinx(self): - qinq_iface = interface.get_by_name(self.cfg, "GigabitEthernet1/0/1.201") - mid_iface = interface.get_qinx_parent_by_name(self.cfg, "GigabitEthernet1/0/1.201") - parent_iface = interface.get_parent_by_name(self.cfg, "GigabitEthernet1/0/1.201") + qint_ifname, qinq_iface = interface.get_by_name(self.cfg, "GigabitEthernet1/0/1.201") + mid_ifname, mid_iface = interface.get_qinx_parent_by_name(self.cfg, "GigabitEthernet1/0/1.201") + parent_ifname, parent_iface = interface.get_parent_by_name(self.cfg, "GigabitEthernet1/0/1.201") # TODO(pim) - complete once get_*_by_name() returns a dict # print("qinq", qinq_iface) diff --git a/validator/vxlan_tunnel.py b/validator/vxlan_tunnel.py index 3840c48..48df5b7 100644 --- a/validator/vxlan_tunnel.py +++ b/validator/vxlan_tunnel.py @@ -24,7 +24,7 @@ def get_by_name(yaml, ifname): """ Return the VXLAN by name, if it exists. Return None otherwise. """ try: if ifname in yaml['vxlan_tunnels']: - return yaml['vxlan_tunnels'][ifname] + return ifname, yaml['vxlan_tunnels'][ifname] except: pass return None