ipspace / netlab

Making virtual networking labs suck less
https://netlab.tools
Other
439 stars 66 forks source link

Call module pre_validate hook based on attributes present in topology, not module list #514

Closed ipspace closed 1 year ago

ipspace commented 2 years ago

Unfortunately we got into a complicated mess where modules use data structures belonging to other modules (evpn => vxlan => vlan, evpn => vrf).

Even worse, EVPN module does not require VRF module, so there's no way to detect that something is missing.

The "only" challenge identified so far is the lack of data structure normalization. For example, the VRF module cleans up the vrfs dictionary in pre_transform hook, so the EVPN module can safely use those structures, but if the topology does not use the VRF module, those structures could be anything, and the EVPN module dies a horrible death (that's why we have to check the None values in EVPN module).

To solve this conundrum, we should:

The change would go into 1.4.

Any thoughts on this one @jbemmel?

ipspace commented 2 years ago

Test case:

#
# Test EVPN with vrfs but without vrf module
#
# Should report an error but not crash
#

module: [ bgp, evpn ]
bgp.as: 65000

vrfs:
  red:

nodes:
  r1:
    device: eos

links:
- r1:
  vrf: red
jbemmel commented 2 years ago

I'm thinking we have these hooks per entity class, currently:

with hooks:

However, there are also these additional entities / constructs:

[ unfinished thought, draft saved ]

ipspace commented 2 years ago

FYI, the hooks are not based on entity classes but on the steps of the data transformation, which are roughly:

See https://netsim-tools.readthedocs.io/en/dev/dev/transform.html for details.

ipspace commented 1 year ago

As a collateral "damage" of thorough attribute checks we can no longer get into a position where a module would use 'vrfs' global or node attribute without VRF module being enabled -- global and node attributes are checked before pre_transform module calls.

Just make sure you don't touch other modules' data in init or pre_default callbacks ;)