Move BGP community and BGP extended community match-set-options back to bgp-conditions #982
Conversation
|
No major YANG version changes in commit 6104d63 |
|
Added to Oct 31 OC Operators review Added to Nov 2, 2023 OC community meeting agenda for feedback: |
|
Last call for this PR, to be merged on Dec 21, 2023 |
There was a problem hiding this comment.
- I think we shall consider this as BRAKING change. Yes, old path are not removed (yet) but deprecated. Hence not required to be supported. I can imagine compliant new implementation that supports only new style, hence fail to accept old-style policy config.
- Under this proposal it is possible (using deprecated) leaf write policy with dual-inversion, or even ALL (match-community-set/config/match-set-options) && INVERT (in deprecated but still legal community-set/config/match-set-options). Which creates quite fragile situation. Perhaps smart server can detect it and return Error. But still not nice possibility.
The concern 2) would be solved if this change would be declared as breaking. - This change introduce ambiguity how community+policy could be configured. It is cleare that old is deprecated, but old style is still there and would be accepted.
|
Thanks for the suggestion! I have bumped the version to |
|
Deprecating the old paths and adding new is part of the definition of non-breaking changes. The major version increment is not required. The only function of a 'deprecating' a leaf is notification that it will be deleted in some future, major (breaking change) release. The leaf is still required to be supported by a network operating system (NOS). To facilitate versioning in OC/Yang, a NOS must:
v3.0.0 of OC is a new major release of OC. In this scenario some deprecated leaves were deleted, creating a breaking change and need for a major version increment. The major version increment notifies consumers that it contains breaking changes in OC and code changes are to be expected to use this OC release. In an operational environment, a configuration generation system will need to support v2.2.0 of OC and v3.0.0 of OC. It will also need to know which NOS support which version in order to generate compatible OC. In OC we are commiting to making major releases infrequently, such as once per year. See https://www.openconfig.net/docs/guides/releases/ |
There was a problem hiding this comment.
Xuquma,
I do have one more concern.
Why this match os not under /conditions/bg-conditions/ as all other that are based on BGP only attributes? You introduced inconsistency this way (while fixing the other inconsistency)
I belive grouping match conditions per-protocol would greatly improve/simplife validation or policy attachment. E.g. if policy w/ bgp-condition are attached to say ISIS, server can easly spot it and generate error.
|
Thanks Darren for explanation.
Or only one of above? Which one? |
Yes, a server/network operating system release (brand new implementation or pre-existing with upgrade) is expected to support a given release of OC, including deprecated leafs. In practice, all NOS have Yang deviations/augments as well as deviations in behavior, some of which are captured in featureprofiles. In practice, a client / network management system must accommodate the deviations for each NOS in a given OC release, plus deal with multiple OC releases, as any operational network will always have some variation of deployed NOS releases and therefore OC releases. |
@xuqma I think I agree with @rszarecki here. The refactored |
xuqma
force-pushed
the
master
branch
2 times, most recently
from
3bce9f8 to
c9be1f6
Compare
Good catch! I put the wrong paths in the PR description though the actual model change moved With Darren's help, I was able to generate the model diff using pyang from this PR and confirmed that the new paths are part of I also changed the version back to Let me know if you need further info. Thanks! |
|
I'm good. |
…to policy-definitions/statements/.../bgp-conditions
|
/cpbrun |
…to policy-definitions/statements/.../bgp-conditions (openconfig#982)
Change Scope
Move
match-set-optionsof BGP community and BGP extended community back topolicy-definitions/statements/.../bgp-conditionsfor consistency across sets. This fixes Inconsistencies in sets - openconfig-bgp-policy.yang #518.The original paths will be deprecated, including:
/routing-policy/defined-sets/bgp-defined-sets/community-sets/community-set/config/match-set-options/routing-policy/defined-sets/bgp-defined-sets/community-sets/ext-community-set/config/match-set-options/routing-policy/policy-definitions/policy-definition/statements/statement/conditions/bgp-conditions/config/community-set/routing-policy/policy-definitions/policy-definition/statements/statement/conditions/bgp-conditions/config/ext-community-setThe new paths are:
/routing-policy/policy-definitions/policy-definition/statements/statement/conditions/bgp-conditions/match-community-set/config/match-set-options/routing-policy/policy-definitions/policy-definition/statements/statement/conditions/bgp-conditions/match-ext-community-set/config/match-set-options/routing-policy/policy-definitions/policy-definition/statements/statement/conditions/bgp-conditions/match-community-set/config/community-set/routing-policy/policy-definitions/policy-definition/statements/statement/conditions/bgp-conditions/match-ext-community-set/config/ext-community-setThis change is backwards compatible, as we mark the old paths as deprecated.
Platform Implementations