Page MenuHomeVyOS Platform

vyos.xml.defaults for tag nodes
Open, Requires assessmentPublicBUG

Description

>>> from vyos.xml import defaults
>>> import pprint
>>> pprint.pprint(defaults(['interfaces', 'ethernet']))
{'duplex': 'auto',
 'ip': {'arp_cache_timeout': '30'},
 'mtu': '1500',
 'speed': 'auto',
 'vif': {'ip': {'arp_cache_timeout': '30'}, 'mtu': '1500'},
 'vif_s': {'mtu': '1500', 'vif_c': {'mtu': '1500'}}}

Should we also get the defaults on a tag node? as ip and mtu keys should rather be the default of the tag element.

Details

Difficulty level
Unknown (require assessment)
Version
1.3-rolling-20200630
Why the issue appeared?
Will be filled on close
Is it a breaking change?
Unspecified (possibly destroys the router)

Event Timeline

c-po created this task.

Yes, I should parse the tagNode and insert them into the default data.

Re-reading the entry, I am now unsure what you believe should be different.

pasik added a subscriber: pasik.Thu, Jul 2, 12:27 PM
c-po added a comment.Fri, Jul 3, 8:29 AM

As "ip" is an invalid key in "vif" (as its no VLAN number) it should not be part of the default dict I guess - same for vif_s

c-po added a comment.Fri, Jul 3, 8:56 AM

I use myself a "cleanup" function, imagine:

def T2665_default_dict_cleanup(dict):
    """
    Cleanup default keys for tag nodes https://phabricator.vyos.net/T2665.
    """
    import pprint
    print("==== in ====")
    pprint.pprint(dict)
    # Cleanup
    for vif in ['vif', 'vif_s']:
        if vif in dict.keys():
            for key in ['ip', 'mtu']:
                if key in dict[vif].keys():
                    del dict[vif][key]

            # cleanup VIF-S defaults
            if 'vif_c' in dict[vif].keys():
                for key in ['ip', 'mtu']:
                    if key in dict[vif]['vif_c'].keys():
                        del dict[vif]['vif_c'][key]
                # If there is no vif-c defined and we just cleaned the default
                # keys - we can clean the entire vif-c dict as it's useless
                if not dict[vif]['vif_c']:
                    del dict[vif]['vif_c']

            # If there is no real vif/vif-s defined and we just cleaned the default
            # keys - we can clean the entire vif dict as it's useless
            if not dict[vif]:
                del dict[vif]

    print("==== out ====")
    pprint.pprint(dict)
    print("========")
    return dict
==== in ====
{'duplex': 'auto',
 'eth1': {'address': '2.2.2.2/32',
          'duplex': 'auto',
          'hw_id': '00:50:56:bf:ef:aa',
          'smp_affinity': 'auto',
          'speed': 'auto',
          'vif': {'100': {'address': '100.100.100.100/24'}}},
 'ifname': 'eth1',
 'ip': {'arp_cache_timeout': '30'},
 'mtu': '1500',
 'speed': 'auto',
 'vif': {'ip': {'arp_cache_timeout': '30'}, 'mtu': '1500'},
 'vif_s': {'mtu': '1500', 'vif_c': {'mtu': '1500'}}}
==== out ====
{'duplex': 'auto',
 'eth1': {'address': '2.2.2.2/32',
          'duplex': 'auto',
          'hw_id': '00:50:56:bf:ef:aa',
          'smp_affinity': 'auto',
          'speed': 'auto',
          'vif': {'100': {'address': '100.100.100.100/24'}}},
 'ifname': 'eth1',
 'ip': {'arp_cache_timeout': '30'},
 'mtu': '1500',
 'speed': 'auto'}
========

dict['vif']['mtu'] is totally invalid - it must rather be dict['vif']['100']['mtu']

So the root seems to come from this calls

>>> pprint.pprint(defaults(['interfaces', 'ethernet']))
{'duplex': 'auto',
 'ip': {'arp_cache_timeout': '30'},
 'mtu': '1500',
 'speed': 'auto',
 'vif': {'ip': {'arp_cache_timeout': '30'}, 'mtu': '1500'},
 'vif_s': {'mtu': '1500', 'vif_c': {'mtu': '1500'}}}

We have to take a design decission here - I'd not add tagNode defaults to the default dict. We rather should later auto collect all tag nodes and blend the content into the tagNode key (e.g. vlan1000) by calling

>>> pprint.pprint(defaults(['interfaces', 'ethernet', 'vif']))
{'ip': {'arp_cache_timeout': '30'}, 'mtu': '1500'}

This loop can then be made a library function get_config_dict_with_defaults() or so Which produces a perfectly nested dict

{'duplex': 'auto',
 'address': '1.1.1.1/32',
 'ip': {'arp_cache_timeout': '30'},
 'mtu': '1500',
 'speed': 'auto',
 'vif': {'100': { 'ip': {'arp_cache_timeout': '30'}, 'mtu': '1500'}},
 'vif_s': {'200': {'mtu': '1500', 'vif_c': {'201' : {'mtu': '1500'}}}}}

Ok, I will look at how we can use the current configuration to insert the tagNode name when we generate the default configuration