python: T7886: Add global interface dependency verification #4765
Conversation
|
👍 |
l0crian1
force-pushed
the
int-dep-verify
branch
from
8397bfe to
f2d28f2
Compare
|
CI integration 👍 passed! Details
|
There was a problem hiding this comment.
I like the idea to start verifying those dependencies. I'm not sure what the ultimate design for that should be, though.
I think in the long run, components should be able to register their dependencies, so that the config backend is aware of the full dependency graph.
Whether this solution is good to merge as a stop-gap measure — I'm not sure. It seems to do that job. Maybe one suggestion: I'd put dependency lists and types (warning/error) in a dict so that it's easier to maintain and use a guide for converting it to a general mechanism later.
That was my original idea, but there doesn't appear to be a general programmatic mechanism for inserting data into the config object. I thought about storing them in JSON, then the dependencies could be in a simple key of the interface name, though it wouldn't be scalable since every conf_mode script has a different way and section of the config dict for dealing with interfaces. What I made is the most scalable mechanism I could execute on, though it's also probably the most brute force way to handle this. It can also be fragile, which is why 'values' default to warnings, because of the possibility of someone doing something like I think probably the best way to handle this is to store what is a dependency in the XML tree, kind of like # Ex. interface-definitions/include/ospf/protocol-common-config.xml.i
<tagNode name="interface">
<dependency>
<type>interface</type>
<level>error</level>
</dependency>
</tagNode>
Or
<tagNode name="interface">
<dependency type="interface" level="error"/>
</tagNode>And in the config dict, it would return something like: {
"dependencies": {
"interfaces": {
"br0": {
"errors": [
"protocols ospf interface br0",
"interfaces tunnel tun0 source-interface br0"
],
"warnings": [
"firewall ipv4 forward filter rule 10 inbound-interface name br0"
]
}
},
}
}I'm okay if this doesn't get merged. If all I accomplished with this PR is to generate the discussion, then I think it was time well spent. |
Change summary
There are a large number of error generating checks in the
verify()function within config scripts that checks whether an interface exists. There is however, very few checks in the reverse when deleting the dependent interface.Example:
There are checks when configuring an interface in OSPF:
But if we delete the interface after OSPF has been configured, it won't fail:
This will lead to the entire OSPF section failing to load upon a reboot, potentially causing an outage for a customer.
This check adds a global check that can be called in
conf_modescripts that checks for dependencies of an interface before it is deleted.This check was added to a single PR (
interfaces_dummy.py) as an example. The check would need to be added to the other scripts in subsequent PR/tasks.Types of changes
Related Task(s)
https://vyos.dev/T7886
Related PR(s)
How to test / Smoketest result
Embed dependencies into a config dict within the
get_config()function: Example -intefaces_dummy.pyAdd checks within the
verify()functionConfigure an interface and a dependent config section: Example - dum5 and OSPF:
Delete, commit and observe error:
Warnings are also shown if there are no Errors. These are sections that will not fail to load if the interface is missing upon boot: Example - dum5 and BGP:
interface_dummy.pysmoketest results:Checklist: