openconfig/reference

Does gNMI Set allow for a Replace on a subtree (aka. non-root) node?

wenovus opened this issue · 9 comments

Hi, I did not find in the proto definition, nor in this section (https://github.com/openconfig/reference/blob/master/rpc/gnmi/gnmi-specification.md#344-modes-of-update-replace-versus-update), whether this is allowed.

So, is it right that as long as a single replace operation is provided, that any other path in the entire data tree that's not specified as a replace or update in the same SetRequest will be erased?

If you have:

  • replace /a and replace /b where each subtree is replaced, then all children of the subtrees /a and /b that are not specified in their respective payload (Update.val field of the message with the specified Update.path) would be removed. A subtree /c would be unaffected since none of the paths would have subsumed this path.
  • replace / and replace /b, all existing data is erased, and rather replaced with the contents of the combined Update.val of the two Update messages.
  • replace / and update /a, all existing data is erased, and replaced with the contents of the combined Update.val of the two Update messages.
  • replace /b and replace / the data is replaced with the contents of only the / update, since the order is significant as per section 3.4 of the spec.

+1 If this isn't clear in the spec, we should add it. Set.replace isn't limited to / and it's not intuitive to me that paths outside the subtree specified in the replace operation should be modified / erased.

I suspect this could be made clearer by ensuring that we clarify that the rules in 3.4.4 apply to the paths that are specified in an Update in replace not to the tree outside it. I think we implicitly say this since the operation is scoped to the path, but we could be more explicit.

I think the reason I'm confused is because I'm assuming that the Update messages might only ever contain leaf values (say the user wanted to use ygot's TogNMINotifications, which serializes a GoStruct into leaf notifications), so I incorrectly guessed that the only way replace did anything different than update was that everything else got erased.

Therefore, it sounds to me that in order to replace on a subtree, the val field for Update must contain the data for a container? A leaf update is indistinguishable from a leaf replace? If so, what ygot operation is currently recommended for doing a replace after populating a non-root GoStruct? I'm guessing EncodeTypedValue with JSON_IETF as the encoding type? Alternatively, it seems functionally equivalent to do a delete on the non-root node first if we choose to serialize the GoStruct into leaf notifications (assuming no conflicts with other operations exist).

For Subscribe, Update messages (generally) only contain leaf values. For Set, the path can be any path in the tree.

You're right, if the path in the Update corresponds to a subtree, not a leaf, then the val must contain data that corresponds to a serialised-format of that container (e.g., JSON). A leaf update is indistinguishable from a replace since the difference here is in nodes that are part of the subtree that is referenced by the path but not included in the data supplied in the val, so as soon as there is a leaf, and the subtree can only be 1 node, then we must change that node's value, and hence the difference is moot.

For ygot operations for doing a replace: I don't quite follow the question here. For replacing a non-leaf node, I'd expect that you serialise using EmitJSON (or similiar) and then put this into a SetRequest with the path to the node, and then JSON value as the payload.

I think TogNMINotifications is a bit of a red-herring here -- this is specifically intended to give you serialised values that correspond to what is expected in Subscribe. IMHO, we should avoid sending Set operations that include <leaf-path, leaf> values (although they are of course possible), in favour of sending <node-path, json> since this gives us the most understandable change. One method of doing this is to always serialise the root to /, and then determine whether the user wants to update or replace during the Set transaction.

Thanks, that answers my question.

I do think the spec should be clarified with respect to this. In section 3.4.4 it says:

A replace operation is issued where e and f are set, and all other elements are omitted. In this scenario, b MUST be reverted to its default setting of True and the configuration of c MUST be deleted from the tree, and returned to its original un-configured setting.

It reads to me that a Set request consisting of replace /e and replace /f would revert /b, which was surprising to read but I'm glad to hear in this thread is not true.

+1 If this isn't clear in the spec, we should add it. Set.replace isn't limited to / and it's not intuitive to me that paths outside the subtree specified in the replace operation should be modified / erased.

The spec isn't updated yet to clarify this point. If there are three paths /a/b/c, /a/b/d/e and /a/b/d/f, will Set.replace( path: /a/b/d/f val: 'v')

  1. Replace f with the value 'v' (which is equivalent to Set.update of the same)
  2. Remove e
  3. Remove b/c

I think only 1. should be true.
If the request is Set.replace( path: /a/b/d val: {'f': 'v'}), then only 1 and 2 will be true. The alternate subtrees of each of the intermediate nodes in the path shouldn't be affected i.e. b/c in the above example.
If the request is Set.update( path: /a/b/d val: {'f': 'v'}), then only 1 will be true. That's the distinction between the two request types. If node 'f' doesn't exist, it should be created in either requests.

The section https://github.com/openconfig/reference/blob/master/rpc/gnmi/gnmi-specification.md#344-modes-of-update-replace-versus-update of the spec doesn't clarify on this. If you agree to the above, the example used in the section needs to be corrected.

hellt commented

I think the ambiguity in 3.4.4 comes from the fact that this paragraph

A replace operation is issued where e and f are set, and all other elements are omitted. In this scenario, b MUST be reverted to its default setting of True and the configuration of c MUST be deleted from the tree, and returned to its original un-configured setting.

doesn't say which path this Set.Replace is issued on. Therefore it sources confusion.
And I also like the idea of providing more examples (like the ones by @charanjith-anet) in the spec covering the most common cases.