public
public copied to clipboard
openconfig
Fix ospf lsdb router-lsa link list
Fixes #1074
Operational use case
OSPF router-lsa describes a list of link-ids, but the OC model only allows for a single link id. This PR moves the link-id and it's attributes to a new list inside of the router-lsa.
Change Scope
- Set OSPF lsdb router-lsa /network-instances/network-instance/protocols/protocol/ospfv2/areas/area/lsdb/lsa-types/lsa-type/lsas/lsa/router-lsa/state/link-id to status
obsolete - Add a links/link [link-id] list to the router-lsa
- Move types-of-service into the link list
- This change is not backwards compatible. This change proposes that since the router-lsa model is broken as is, making a breaking change here is appropriate. Comments appreciated if the existing
link-idleaf should be deprecated instead of deleted.
Also note this proposes using the yang status obsolete for the leaf. OpenConfig has not traditionally used this status.
Consider this a solicitation for comments if we should start using obsolete for 'deleted' leafs, versus removing them from the model entirely.
Tree view
--- /Users/dloher/old-tree.txt 2024-03-21 10:52:00
+++ /Users/dloher/ospf-tree.txt 2024-03-21 13:40:35
@@ -5878,18 +5878,23 @@
| | | | +--ro age? uint16
| | | +--ro router-lsa
| | | | +--ro state
- | | | | | +--ro type? identityref
- | | | | | +--ro link-id? yang:dotted-quad
- | | | | | +--ro link-data? union
- | | | | | +--ro metric? oc-ospf-types:ospf-metric
- | | | | | +--ro number-links? uint16
- | | | | | +--ro number-tos-metrics? uint16
- | | | | +--ro types-of-service
- | | | | +--ro type-of-service* [tos]
- | | | | +--ro tos -> ../state/tos
+ | | | | | +--ro metric? oc-ospf-types:ospf-metric
+ | | | | | +--ro number-links? uint16
+ | | | | +--ro links
+ | | | | +--ro link* [link-id]
+ | | | | +--ro link-id -> ../state/link-id
| | | | +--ro state
- | | | | +--ro tos? uint8
- | | | | +--ro metric? oc-ospf-types:ospf-metric
+ | | | | | +--ro link-id? yang:dotted-quad
+ | | | | | +--ro type? identityref
+ | | | | | +--ro link-data? union
+ | | | | | +--ro number-tos-metrics? uint16
+ | | | | +--ro types-of-service
+ | | | | +--ro type-of-service* [tos]
+ | | | | +--ro tos -> ../state/tos
+ | | | | +--ro state
+ | | | | +--ro tos? uint8
+ | | | | +--ro metric? oc-ospf-types:ospf-metric
| | | +--ro network-lsa
| | | | +--ro state
| | | | +--ro network-mask? uint8
No major YANG version changes in commit 923a8c1a0aeeef86d60208c6936c6ad9a1965c3c
What's the motivation for
obsoletevsdeprecated?
In this case it's because the link-id leaf is not valid. Per OSPF implementation, router-lsa must contain a list of all the directly connected links of a given router. This is nearly always going to be > 1. So the current OC model is 'broken' and I suspect has not been implemented? I guess someone might have implemented this and chose to place just one of the router-lsa links?
What's the motivation for
obsoletevsdeprecated?In this case it's because the
link-idleaf is not valid. Per OSPF implementation, router-lsa must contain a list of all the directly connected links of a given router. This is nearly always going to be > 1. So the current OC model is 'broken' and I suspect has not been implemented? I guess someone might have implemented this and chose to place just one of the router-lsa links?
Whilst I see the argument here -- it seems that the two things are the same -- i.e., this leaf is no longer in the model. If I'm a developer now implementing the model whether it's obsolete or deprecated is the same thing -- "do not implement this leaf" :-)
From a tooling perspective though, we now need to ensure that all toolchains support both variants -- which are the same "don't generate
I prefer to limit our use of YANG "twiddles" to the absolute minimum possible -- supporting them in toolchains has been painful.
Whilst I see the argument here -- it seems that the two things are the same -- i.e., this leaf is no longer in the model. If I'm a developer now implementing the model whether it's
obsoleteordeprecatedis the same thing -- "do not implement this leaf" :-)From a tooling perspective though, we now need to ensure that all toolchains support both variants -- which are the same "don't generate for this leaf" where is code, documentation etc.
I prefer to limit our use of YANG "twiddles" to the absolute minimum possible -- supporting them in toolchains has been painful.
No worries, I'll deprecate the node instead. The node seems not usable as is, but the new node which replaces it doesn't conflict, so it seems there is little harm to leaving this node present in a deprecated form until a future major OC release. I appreciate any thoughts on this approach as well.
FWIW, deprecated means supported, but scheduled for deletion in an future release. Obsolete is the same as deleting the node from the model, but leaves the information what what the node used to be intact. I proposed obsolete here as I was curious about whether we should use 'obsolete' as we do the keyword 'reserved' in proto. Of course, the same issue with field numbering doesn't exist in yang (except perhaps with enums).
Upon reviewing with OC operators, we realized that there are many breaking changes here due to moving the leafs for links into the link list and moving the types of service list underneath the new link list. Since router-lsa is currently a broken node, it seems to make sense to just delete the link-id as well and make this a breaking change.