Skip to content

Conversation

@giacoliva
Copy link

I encountered an issue when trying the project with Cisco XRd devices, and that patch resolves the problem.

Schema validation and parsing

In some cases, the device returns the strings "true" or "false" instead of the boolean type, and that fixes the problem
The GetOptionalInstance() is not working in some contexts, while the appropriate flag (present on the same structure) seems to be IsMandatory.

Sending appropriate gNMI update and delete requests

The current implementation doesn't take into account the presence of schema in different namespaces, needed in the Cisco environment.
So, there is these needs:

  • separate updates in different requests, one for each namespace (cannot update resources in several namespaces with the same request)
  • add the namespace prefix (in the form namespace:path) instead of using a "blank" path and only a "value"

The proposed implementation is tailored to my environment, and has some more work to do to be ready for every scenario. I leave the code so that it can be taken to understand better the needs:

  • I use an in-memory map that is not persisted, and might generate some problems in case of component crash
  • probably (pretty sure) I have implemented it in the wrong place: I used json_ietf, so I put here my code, but probably it should be moved somewhere else
  • in the delete scenario I used an approach that can understand even path differentiations in subpaths of any grade, while in the update one it can understand only the first and second elements of the path (e.g. router/static belongs to the module static-routing, while router/ospf belongs to dynamic-routing, so the respective requests should be directed to static-routing:router/static and dynamic-routing:router/ospf)

if err != nil || len(entry) == 0 {
// check if the OptionalInstance (!require-instances [https://datatracker.ietf.org/doc/html/rfc7950#section-9.9.3])
if s.schema.GetField().GetType().GetOptionalInstance() {
if !s.schema.GetField().IsMandatory {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is two different things. Mandatory is a container option, mandates, that the mandatory attributes of that container have to be present.
The OptionalInstance is a leafref based option. So the default behaviour is that the referenced attribute must exist. Say under some routing protocol you reference an interface. So that interface must be created. but if the optional flag is set, it is ok, that the interface is reference, without being config-wise created. We will create a warnign for that, just lettings the user know that the referenced instance does not exist.

b, err := strconv.ParseBool(v)
if err != nil {
return nil, err
if strings.Contains(strings.ToLower(v), "true") {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the issue here? Why contains? we can maybe change to

strconv.ParseBool(strings.ToLower(v))

But the other stuff does not make much sense to me. Because thats bassically what strings.ParseBool() does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants