openconfig / gnmic

gNMIc is a gNMI CLI client and collector
https://gnmic.openconfig.net
Apache License 2.0
190 stars 57 forks source link

Support of mixed schema requests in Set #46

Closed marcushines closed 6 months ago

marcushines commented 1 year ago

It is not super easy to figure out how to do a mixed schema set request in the examples.

Since the origin:cli portion is byte blob i was thinking it might be nice to have a option to say something like

./gnmic set -a 192.168.16.50:6030 --skip-verify -u admin -p admin --replace / --replace-file /tmp/setrequest.json

so in this you would need to properly escape the "cli" config into json string to put into the request file it would be nice to have the OC portion be this file but have a seperate file for the CLI coinfig

./gnmic set -a 192.168.16.50:6030 --skip-verify -u admin -p admin --replace / --replace-file /tmp/setrequest.json --replace-cli-origin "cli" --replace-cli-file /tmp/vendor-config-file.txt

karimra commented 1 year ago

in this case the content of /tmp/vendor-config.txt would be multiple cli commands that need to end up each in an ascii_val ?

Instead of introducing new flags, I wonder if it would be possible to do something like:

gnmic --replace-path / --replace-file /tmp/setrequest.json \
      --replace-path cli:/ --replace-file /tmp/vendor-config.txt

The content of the first file will be set as a JSON or JSON_IETF value, while the content of the second one is set as ascii_val(s) because the origin is cli. WDYT ?

marcushines commented 1 year ago

The whole file is a single ascii value

hellt commented 1 year ago

@karimra i think the whole contents of a file is a single ascii_val

marcushines commented 1 year ago

let me try your suggestion and see what that looks like from a proto perspective

also is there debug option to like dry run this as i would like to see the actual proto message sent to device?

karimra commented 1 year ago

There is a --print-request flag but it's not a dry run, but it can be added. Currently, if the flag --replace-file </path/to/file> is set, a JSON or JSON_IETF value is used. My suggestion is to add some logic to use an ASCII TypedValue if origin is cli.

marcushines commented 1 year ago

i would say adding a flag is clearer to the user the behavior will be different

otherwise you have to have an explicit list of what origins are magic (currently we are suggesting vendors need to be clear on origin meanings) so origin:cisco-netconf origin:cisco-cli and so on

karimra commented 1 year ago

I find --replace-cli-origin "cli" a bit redundant, the CLI origin is supposed to always be equal to cli. Maybe add just 2 flags --replace-cli <single cli cmd> and --replace-cli-file </path/to/file.cli> ?

BTW can CLI origin be set in a path part of an update (rather than a replace ?) didn't find any mention of that here: https://github.com/openconfig/reference/blob/master/rpc/gnmi/mixed-schema.md Even if not allowed by the server, worth adding to the client to be able to validate the server behavior ?

marcushines commented 1 year ago

updates replaces can both have origin cli

karimra commented 1 year ago

The reason gnmic --update-path cli:/ --update-file </path/to/file> didn't work is that the file is assumed to be either JSON or YAML. I removed that restriction in #55 , the command will have to include -e ascii

$ gnmic XXXX set --update-path cli:/ --update-file test.cli --format prototext -e ascii --print-request
Set Request:
update: {
  path: {
    origin: "cli"
  }
  val: {
    ascii_val: "interface ethernet-1/1 admin-state disable\ninterface ethernet-1/2 admin-state disable\ninterface ethernet-1/3 admin-state disable"
  }
}

Set Response:
timestamp: 1678087211664203768

The same PR adds the flags --replace-cli and --replace-cli-file (and the corresponding update flags). the first flag allows to set a single command with cli origin, the second flag setting commands defined in a file.

$ gnmic XXXX set --update-cli "interface ethernet-1/1 admin-state enable" --format prototext --print-request
Set Request:
update: {
  path: {
    origin: "cli"
  }
  val: {
    ascii_val: "interface ethernet-1/1 admin-state enable"
  }
}

Set Response:
timestamp: 1678088488154517826
$ gnmic XXXX set --update-cli-file test.cli --format prototext --print-request 
Set Request:
update: {
  path: {
    origin: "cli"
  }
  val: {
    ascii_val: "interface ethernet-1/1 admin-state enable\ninterface ethernet-1/2 admin-state enable\ninterface ethernet-1/3 admin-state enable\n"
  }
}

Set Response:
timestamp: 1678088842632957881
github-actions[bot] commented 7 months ago

This issue is stale because it has been open for 12 months with no activity.

github-actions[bot] commented 6 months ago

This issue was closed because it has been inactive for 30 days since being marked as stale.