-
Notifications
You must be signed in to change notification settings - Fork 656
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add atomic annotations to containers in openconfig-aft. #860
Conversation
On branch aft-atomic-508 Changes to be committed: modified: openconfig-aft-common.yang modified: openconfig-aft-ethernet.yang modified: openconfig-aft-ipv4.yang modified: openconfig-aft-ipv6.yang modified: openconfig-aft-mpls.yang modified: openconfig-aft-pf.yang Add the atomic attribute to the above aft yang containers.
…le.com> On branch aft-atomic-508 Your branch is up to date with 'origin/aft-atomic-508'. Changes to be committed: modified: openconfig-aft-ipv6.yang modified: openconfig-aft-mpls.yang modified: openconfig-aft-pf.yang Fix some yang lint errors.
On branch aft-atomic-508 Your branch is up to date with 'origin/aft-atomic-508'. Changes to be committed: modified: openconfig-aft-common.yang modified: openconfig-aft-ethernet.yang modified: openconfig-aft-ipv4.yang modified: openconfig-aft-ipv6.yang modified: openconfig-aft-mpls.yang modified: openconfig-aft-pf.yang Address reviewers comments to move the atomic attribute.
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Major YANG version changes in commit 88c5a15: |
Compatibility Report for commit 88c5a15: |
@dplore This should be ready to review. FYI the CLA check is tripping because I included Dan's prior commits and the check doesn't recognize him anymore. From internal resources sounds like there are ways to override it, but waiting until the PR is otherwise ready to merge. |
Hi @michaelore , please amend this PR or submit a fresh one so the CLA is intact. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving as non-breaking change.
This is interpreted as non-breaking as adding the atomic criteria is interpreted as not breaking for old clients. If an server implementation of this model emits this data in atomic gNMI notifications, the older client can still accept the data.
Refreshing #517 as the author is no longer available to complete the request.
#Operational use case
The expectation is that for any AFT entity is only complete when it includes IPv4/IPv6/MPLS/Ethernet, Conditional Next Hop Group (CNHG), Next Hop Group (NHG), or Next Hop (NH). AFT entities must always be updated as a whole with all attributes of the entity in a single message. This is required to preserve the integrity of the AFT table stored on a client by preventing fragmented or partial AFT entities during streaming of updates.
This change adds the atomic attribute to AFT lists for IPv4/IPv6/MPLS/Ethernet/PolicyBasedForwarding, Conditional Next Hop Group (CNHG), Next Hop Group (NHG) and Next Hop (NH).