Page MenuHomeVyOS Platform

vyos.xml.defaults for tag nodes
Closed, ResolvedPublicBUG

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)
Issue type
Internal change (not visible to end users)

Event Timeline

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.

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

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

c-po triaged this task as High priority.Dec 26 2020, 3:52 PM
c-po moved this task from Need Triage to Backlog on the VyOS 1.3 Equuleus board.
erkin set Issue type to Internal change (not visible to end users).Aug 29 2021, 1:59 PM
erkin removed a subscriber: Active contributors.
jestabro moved this task from Need Triage to Finished on the VyOS 1.4 Sagitta board.