openconfig / reference

This repository contains reference implementations, specifications and tooling related to OpenConfig-based network management.
Apache License 2.0
155 stars 88 forks source link

gNMI update with explicit deletion #201

Closed gwizdms closed 1 week ago

gwizdms commented 8 months ago

Added clarity for delete path discussed in gnmi repo issue 145.

ccole-juniper commented 8 months ago

Which implementations currently send deletions when in sample mode?

gwizdms commented 8 months ago

@ccole-juniper the purpose of this proposed change to the wording of the specification is not to call out who currently implements what, it is to clarify wording to as to eliminate a broader spectrum of interpretations. Posting this change is also a time for the audience to weigh in on the proposed change. If there is a specific reason why you would not be in favor of supporting such change, please contribute so it can be reviewed by the group.

ccole-juniper commented 8 months ago

@ccole-juniper the purpose of this proposed change to the wording of the specification is not to call out who currently implements what, it is to clarify wording to as to eliminate a broader spectrum of interpretations. Posting this change is also a time for the audience to weigh in on the proposed change. If there is a specific reason why you would not be in favor of supporting such change, please contribute so it can be reviewed by the group.

Asking for current and prior art is the intention, and I'd still like to hear from other vendors and operators re: their particular implementation. From my experience with SNMP and similar frameworks I'm not familiar with the mixture of change tracking into the equivalent of sample mode. Previous and current implementations will help to inform intention when there's ambiguity.

ccole-juniper commented 8 months ago

When would deletions be expected to be delivered: promptly on deletion ("out of sample") or at the next sample ("in sample")? If "in sample", I would expect that the notification timestamp would need to reflect the time of deletion which can significantly precede the time of the sample.

dplore commented 8 months ago

This was reviewed in Nov 14, 2023 OC Operators meeting without objection. Added to Dec 7, 2023 OC Community meeting for broader awareness.

hellt commented 8 months ago

Thanks Darren, upon further review we noticed that rollback reset functionality is not available. Here is the review comment I left for this https://github.com/openconfig/reference/pull/196#pullrequestreview-1736596093

On Thu, Nov 16, 2023 at 5:34 PM Darren Loher @.***> wrote:

This was reviewed in Nov 14, 2023 OC Operators meeting without objection. Added to Dec 7, 2023 OC Community meeting https://docs.google.com/document/d/11sXUF2zujhbPs_bt-RFJhh2DcIbEvnOpd9GypE-u3cc/edit#heading=h.1cdzya92dfta for broader awareness.

— Reply to this email directly, view it on GitHub https://github.com/openconfig/reference/pull/201#issuecomment-1814918644, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABLKV5MNVYCMXEJ56NKJXHLYEZFBHAVCNFSM6AAAAAA7BSXPM2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJUHEYTQNRUGQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

dplore commented 8 months ago

Thanks Darren, upon further review we noticed that rollback reset functionality is not available. Here is the review comment I left for this #196 (review)

I think this comment is misplaced? This PR (#201) is about gNMI Update and Notification messages.

hellt commented 8 months ago

pardon, replied to a wrong thread indeed, @dplore

ashu-ciena commented 6 months ago

I posted on question on openconfig google group on what should be the expectation from a spec perspective for a delete notification.
Should the deletion notification be sent on the deletion of a config leaf/list-instance delete even if user subscribes to the state node of that object instance, something like this - /interfaces/interface[name=intf1]/state/counters ? AND If the subscription is on a leaf node under /interfaces/interface[name=intf1]/state/counters, then should the delete notification be sent for /interfaces/interface[name=intf1]/state/counters or the subscribed leaf XPath, knowing the fact that even other leaf nodes would cease to exist after the deletion of "intf1" ?

Assume here that interface is not limited to a physically present port on the router and can be a logical construct representing some L2/L3 service endpoint.

dplore commented 6 months ago

When a node in the tree is removed, a DELETE must be sent. This includes /interfaces/interface/state. It is ok to DELETE a parent node and expect clients to recursively remove all the children nodes. See gnmi reference 3.4.6

In practice there are scenarios where physical interfaces can be persistent or ephemeral. For example, a persistent interface could be a fixed 100GE port. A configuration for an IP address could be added or removed from this interface, but the interface will still exist as state.

An ephemeral interface could be a breakout port. Take a 400GE Ethernet which has a breakout capable transceiver inserted. When a breakout is configured, new interfaces are created, including their state containers. If the breakout deleted, the breakout physical interfaces will no longer exist and the relevant state containers should be deleted. It would be ok to delete the /interfaces/interface[name='breakout1'] container for the breakouts that were deleted.

I would expect logical interfaces to be treated the same way as the breakout port example above.

ejbrever commented 6 months ago

I had two other suggestions for notes that would be good to add here:

  1. For timing of the delete message, I think it is fine for it to be asynchronous during a SAMPLE subscription, but it likely does not matter too much as long as the delete is sent. However, I can imagine there be a preference to send it asynchronously as this might alleviate the need for a caching mechanism to store a delete until the next SAMPLE interval.
  2. In certain events (i.e. failure or disable power to component), specific leaves likely need to be deleted. As an example, if you consider an interface that measures optical power levels. If the system is unable to read optical power levels off of the hardware during an event, we have an interesting decision to make on the optical power level leaf. One option is not report anything further, however, this likely means you have a stale value that can be misinterpreted. Another option is to report a default value (i.e. -40dBm), but that would make an assumption of the value which seems problematic as well. In that case, perhaps the right thing to do is send a delete for that leaf?
gwizdms commented 5 months ago

Changed wording for required in ON-CHANGE subscription mode and optional for SAMPLE subscription mode after OpenConfig Community meeting discussions.

DanG100 commented 3 months ago

Can we clarify the behavior for TARGET_DEFINED streams?

When a client makes a Subscribe request with mode TARGET_DEFINED, it doesn't know if server is going to reply with ON_CHANGE or SAMPLE (per leaf). The client then can't know if the absence of updates means some paths were implicitly deleted or that haven't been any changes.

dplore commented 3 months ago

Can we clarify the behavior for TARGET_DEFINED streams?

When a client makes a Subscribe request with mode TARGET_DEFINED, it doesn't know if server is going to reply with ON_CHANGE or SAMPLE (per leaf). The client then can't know if the absence of updates means some paths were implicitly deleted or that haven't been any changes.

Good idea, we should attempt to address delete behavior when TARGET_DEFINED is used.

In practice, TARGET_DEFINED behavior is expected to be "do the right thing" where rapidly changing leaf values (like counters) are sampled and infrequently changing leaves (like the state copy of config leaves) are sent ON_CHANGE. Likewise one way to define this is the DELETE behavior is also "TARGET_DEFINED". We could elaborate that a target is expected to send delete for leaves which are updated only when they change and optionally send a DELETE for leaves which are updated periodically (ie: even if they don't change).

We will touch on this topic at the March 26,2024 OC Operators meeting.

dplore commented 1 month ago

No objections were recorded regarding TARGET_DEFINED and explicit deletes. @gwizdms please add some text for explicitly defining what should happen with delete in TARGET_DEFINED.

dplore commented 1 month ago

Thanks @gwizdms . This change is set to last call for June 11, 2024