Generation and path issues

[JonasKs]

Hello!

We're currently experimenting with ygot and gNMI, on a Cisco stack, and experience a few issues. We're pretty much new to golang and the gNMI-stack, so any help is appreciated.

1. Generation fails

When trying to generate models for the native Cisco IOS XE native model, with the command

//go:generate go run $GOPATH/pkg/mod/github.com/openconfig/ygot@v0.29.0/generator/generator.go -path=yang -output_file=pkg/ocdemo/oc.go -package_name=ocdemo -generate_fakeroot -fakeroot_name=device -compress_paths=true -shorten_enum_leaf_names -typedef_enum_with_defmod -exclude_modules=ietf-interfaces yang/Cisco-IOS-XE-native.yang

we get the following error:

❯ go generate
F0710 14:28:27.287938   67709 generator.go:384] ERROR Generating GoStruct Code: enumgen.go: cannot resolve enumeration name clash between the following entries: [/Cisco-IOS-XE-native/native/enable/secret/type /Cisco-IOS-XE-native/native/enable/password/type]
exit status 1
main.go:18: running "go": exit status 1

2. Casting to JSON is not valid JSON?

When we generate models for openconfig, using the command found in the examples
(//go:generate go run $GOPATH/pkg/mod/github.com/openconfig/ygot@v0.29.0/generator/generator.go -path=yang -output_file=pkg/ocdemo/oc.go -package_name=ocdemo -generate_fakeroot -fakeroot_name=device -compress_paths=true -shorten_enum_leaf_names -typedef_enum_with_defmod -exclude_modules=ietf-interfaces yang/openconfig-interfaces.yang yang/openconfig-if-ip.yang ), and the following code:

package main

import (
	"context"
	"fmt"
	"github.com/openconfig/gnmi/proto/gnmi"
	"github.com/openconfig/gnmic/api"
	"github.com/openconfig/ygot/ygot"
	"google.golang.org/protobuf/encoding/prototext"
	oc "goyang/pkg/ocdemo"
	"log"
)

// The following generation rule uses the generator binary to create the
// pkg/ocdemo package, which generates the corresponding code for OpenConfig
// interfaces.
//
//go:generate go run $GOPATH/pkg/mod/github.com/openconfig/ygot@v0.29.0/generator/generator.go -path=yang -output_file=pkg/ocdemo/oc.go -package_name=ocdemo -generate_fakeroot -fakeroot_name=device -compress_paths=true -shorten_enum_leaf_names -typedef_enum_with_defmod -exclude_modules=ietf-interfaces yang/openconfig-interfaces.yang yang/openconfig-if-ip.yang

func main() {
	d := &oc.Device{}

	i, err := d.NewInterface("GigabitEthernet2/1/3")
	if err != nil {
		log.Fatal(err)
	}
	i.Description = ygot.String("New test description")
	fmt.Println(i)

	json, err := ygot.EmitJSON(d, &ygot.EmitJSONConfig{
		Format: ygot.RFC7951,
		Indent: "  ",
		RFC7951Config: &ygot.RFC7951JSONConfig{
			AppendModuleName: true,
		},
	})
	if err != nil {
		panic(fmt.Sprintf("JSON demo error: %v", err))
	}

	tg, err := api.NewTarget(
		api.Name("Switch"),
		api.Address("<Cisco IP>"),
		api.Username("admin"),
		api.Password("<pw>"),
		api.Insecure(true),
	)
	if err != nil {
		log.Fatal(err)
	}

	ctx, cancel := context.WithCancel(context.Background())
	defer cancel()

	err = tg.CreateGNMIClient(ctx)
	if err != nil {
		log.Fatal(err)
	}
	defer tg.Close()

	request := &gnmi.SetRequest{
		Update: []*gnmi.Update{
			{
				Path: &gnmi.Path{},
				Val: &gnmi.TypedValue{
					Value: &gnmi.TypedValue_JsonIetfVal{
						JsonIetfVal: []byte(json),
					}},
			},
		}}

	fmt.Println(prototext.Format(request))

	setResp, err := tg.Set(ctx, request)
	if err != nil {
		log.Fatal(err)
	}
	fmt.Println(prototext.Format(setResp))
}

The fmt.Println(prototext.Format(request)) produces the following output:

update: {
  path: {}
  val: {
    json_ietf_val: "
        {\n  
          \"openconfig-interfaces:interfaces\": {
          \n    \"interface\": [
          \n      {
          \n        \"config\": {
          \n          \"description\": \"New test description\",
          \n          \"name\": \"GigabitEthernet2/1/3\"
          \n        },
          \n        \"name\": \"GigabitEthernet2/1/3\"
          \n
          }\n    ]
        \n  }
        \n}"
  }
}

How ever, this errors because I need the path to be set:

2023/07/10 14:42:52 rpc error: code = InvalidArgument desc = Invalid argument.  Path with 0 elements (xpath = "/") is not supported for SET and GET requests

When I use the Cisco YANG suite, the generated response looks like this:

{
  "update": [
    {
      "path": {
        "origin": "openconfig",
        "elem": [
          {
            "name": "interfaces"
          },
          {
            "name": "interface",
            "key": {
              "name": "GigabitEthernet2/1/3"
            }
          },
          {
            "name": "config"
          },
          {
            "name": "description"
          }
        ]
      },
      "val": {
        "jsonIetfVal": "\"my description\""
      }
    }
  ]
}

Which sets the path, and then set the description as only the value. Is there any way to produce the same result using ygot?

[wenovus]

Hi Jonas,

1. Since these are non-OpenConfig models, you will need to set compress_paths=false in the generation, as path compression assumes OpenConfig-style YANG which follows certain conventions that the Cisco native models don't follow. The example generator call is at https://github.com/openconfig/ygot/blob/master/uexampleoc/update.sh

2. If the device does not support root-level configurations then by default you would need to construct the paths by hand (see util.StringToStructuredPath) and then call ygot.Marshal7951() on the generated GoStruct field corresponding to the path.

If this is tedious and you'd like to auto-complete the path and just call a function that does all of the above for you, then that is what ygnmi is for; however it doesn't yet support uncompressed YANG files yet. I'm looking to implement that by the end of this month.

Lastly if the device accepts gNMI scalar TypedValues for configuration, then one hacky way to avoid having a root-level replace value is to use TogNMINotifications and then take its Update values for inserting into the SetRequest. However this means the config is not in JSON and I'm not sure if Cisco supports that.

[JonasKs]

Thanks a lot for the quick and well written reply!

The compress_paths=false trick worked! Ended up having to create a custom very stripped out YANG model, since the generated code became so big GoLand crashed if I opened the file (vscode seems to work fine), and the program would not run. Got these errors:

Any way to generate the types by being inclusive, only generating for the groupings and containers we would like? Generating with this file worked wonders:

Custom YANG (click me)

module Cisco-IOS-XE-native {
  namespace "Cisco-IOS-XE-native";
  yang-version 1;
  prefix ios;

  import cisco-semver {
    prefix cisco-semver;
  }
  import ietf-inet-types {
    prefix inet;
  }
  import Cisco-IOS-XE-types {
    prefix ios-types;
  }
  import Cisco-IOS-XE-features {
    prefix ios-features;
  }
  import Cisco-IOS-XE-interface-common {
    prefix ios-ifc;
  }
  include Cisco-IOS-XE-parser;
  include Cisco-IOS-XE-license;
  include Cisco-IOS-XE-line;
  include Cisco-IOS-XE-logging;
  include Cisco-IOS-XE-ip;
  include Cisco-IOS-XE-ipv6;
  include Cisco-IOS-XE-interfaces;

  description
    "Custom for type generator";

  typedef monitor-event-type {
    type enumeration {
      enum "error";
      enum "detail";
      enum "major";
    }
  }

  container native {
    container default {
      description
        "Set a command to its defaults";
      container crypto {
        description
          "Encryption module";
        container ikev2 {
          description
            "Configure IKEv2 Options";
          leaf proposal {
            description
              "Define IKEV2 proposals";
            type empty;
          }
          leaf policy {
            description
              "Define IKEV2 policies";
            type empty;
          }
        }
      }
    }
    uses config-vrf-definition-grouping;
    uses config-interface-grouping;
   }
}

---

We also had some problems with the generated code not working as I thought it would, this did not work, even when using ygot.BuildEmptyTree(gi). I might not totally understand the ygot.BuildEmptyTree command - so this might not be a bug!

gi, err := d.Native.Interface.NewGigabitEthernet("2/1/3")
gi.Ip.Address.NewSecondary("192.168.0.1/24") // <-- this would fail
// panic: runtime error: invalid memory address or nil pointer dereference

There was also no NewPrimary function, only a NewSecondary, which I found weird 🤔

We worked around this by using the structs all the way:

Complete go code (click me)

package main

import (
	"context"
	"fmt"
	oc "goyang/pkg/ocdemo"
	"log"

	"github.com/openconfig/gnmi/proto/gnmi"
	"github.com/openconfig/gnmic/api"
	"github.com/openconfig/ygot/ygot"
	"google.golang.org/protobuf/encoding/prototext"
)

// The following generation rule uses the generator binary to create the types in `ocdemo.go`.
// You can generate new functions by writing `go generate` in your terminal.
//go:generate go run $GOPATH/pkg/mod/github.com/openconfig/ygot@v0.29.0/generator/generator.go -typedef_enum_with_defmod -path=yang -output_file=pkg/ocdemo/oc.go -ignore_unsupported -ignore_deviate_notsupported -package_name=ocdemo -generate_fakeroot -fakeroot_name=device -exclude_modules=ietf-interfaces -generate_simple_unions yang/custom.yang

func main() {
	// Create a new Target, which is a wrapper around a gNMI client.
	target, err := api.NewTarget(
		api.Name("Switch"),
		api.Address("<ip>:50000"),
		api.Username("admin"),
		api.Password("<pw>"),
		api.Insecure(true),
	)
	if err != nil {
		log.Fatal(err)
	}
	// ctx will be passed into the gNMI client, and kind of works like a contextmanager. We can defer cancel() to ensure we close the connection when we are done.
	ctx, cancel := context.WithCancel(context.Background())
	defer cancel() // same as __exit__() in Python, call cancel

	// Create the gNMI client, passing in the context
	err = target.CreateGNMIClient(ctx)
	if err != nil {
		log.Fatal(err)
	}
	// Add target.close to the context
	defer target.Close()

	// Since we generate the models with `-generate_fakeroot -fakeroot_name=device`, we get a top-root device we can use further down in our application.
	d := &oc.Device{}
	// This function ensures we build the device tree, so that we can say `d.Native.Interface`, without crashing, since the `Interface` field is not instanciated.
	ygot.BuildEmptyTree(d)

	// Instanciate the interface map (using `make`), ensuring the map exist and is not nil.
	d.Native.Interface.NewGigabitEthernet("2/1/3")
	// Set the interface description, and IP address. We manually assign the structs to the fields, this is because we struggled with `nil` errors when using NewSecondaryIp, and NewPrimaryIp did not exist.
	// This is a workaround, but don't really complicate anything, since autocomplete will fill out the structs and we would have to define these anyway.
	d.Native.Interface.GigabitEthernet["2/1/3"] = &oc.Cisco_IOS_XENative_Native_Interface_GigabitEthernet{
		Name:        ygot.String("2/1/3"),
		Description: ygot.String("Test description"),
		Ip: &oc.Cisco_IOS_XENative_Native_Interface_GigabitEthernet_Ip{
			Address: &oc.Cisco_IOS_XENative_Native_Interface_GigabitEthernet_Ip_Address{
				Primary: &oc.Cisco_IOS_XENative_Native_Interface_GigabitEthernet_Ip_Address_Primary{
					Address: ygot.String("192.69.69.1"),
					Mask:    ygot.String("255.255.255.0"),
				},
			},
		},
	}
	// Take anything in the `2/1/3` interface, and convert it to JSON.
	json, err := ygot.EmitJSON(d.Native.Interface.GigabitEthernet["2/1/3"], &ygot.EmitJSONConfig{
		Format: ygot.RFC7951,
		Indent: "  ",
		RFC7951Config: &ygot.RFC7951JSONConfig{
			AppendModuleName: false,
		},
	})

	// Cisco does not allow top-root configuration, which means we have to specify a `*gnmi.Path` in the `*gnmi.Update` struct.
	//											v We add Cisco-IOS-XE-native to ensure we use the Cisco YANG model and not default OpenConfig.
	path, err := ygot.StringToStructuredPath("Cisco-IOS-XE-native:native/interface/GigabitEthernet[name=2/1/3]")
	path.Origin = "rfc7951" // This is required when using native models, instead of OpenConfig models.

	// Create the request body. Insert the JSON and the path we created above.
	request := &gnmi.SetRequest{
		Update: []*gnmi.Update{
			{
				Path: path,
				Val: &gnmi.TypedValue{
					Value: &gnmi.TypedValue_JsonIetfVal{
						JsonIetfVal: []byte(json),
					}},
			},
		}}

	// Use prototext.Format to print the request body nicely.
	fmt.Println(prototext.Format(request))

	// Send the request to the target.
	setResp, err := target.Set(ctx, request)
	if err != nil {
		log.Fatal(err)
	}
	fmt.Println(prototext.Format(setResp))
}

and this worked wonders! We were able to configure using the StringToStructuredPath, and set multiple values under the interface at once.

Thank you so much for your reply, we're looking forward to testing out ygnmi as soon as you're done with it! 😊 It looks awesome. Is there a YANG-community (Discord or similar?) I can join? 😊

[wenovus]

You can experiment with structs_split_files_count to see if that helps with the size of the generated code files:

-output_dir=gostructs
-structs_split_files_count=10

BuildEmptyTree only instantiates containers, but for list elements you still need to create them:

gi.GetOrCreateIp(???).GetOrCreateAddress(???).NewSecondary("192.168.0.1/24")

I'm guessing Primary is a container, which only has GetOrCreate() as a way of instantiating. New requires the list element to be non-existent and so NewPrimary would need to have the semantics. However it is not clear why this semantics would be necessary esp. since ygot currently doesn't support presence containers (apart from an optional struct tag generation).

I'm not aware of what YANG communities are out there, but if you find out then let me know!

[JonasKs]

You can experiment with structs_split_files_count to see if that helps with the size of the generated code files:

Thanks, I'll test that out! I will still have the problem where my code can not run (as described in the picture), though? (I'll double check when I get to work in a few hours.)

EDIT: Splitting files saved GoLand indeed - thanks! It did not fix the errors described in the screenshot above.

BuildEmptyTree only instantiates containers, but for list elements you still need to create them:

That makes sense, thanks for clarifying. 😊

I think this issue can be closed, unless you want it to stay open due to the generated code from the Native-model seem to crash? I can also create a new issue for that? 😊

Thanks again! This has been really helpful😊

[JonasKs]

With your clarification of BuildEmptyTree I assume this should work:

	d := &oc.Device{}
	// This function ensures we build the device tree, so that we can say `d.Native.Interface`, without crashing, since the `Interface` field is not instanciated.
	ygot.BuildEmptyTree(d)

	gi, err := d.Native.Interface.NewGigabitEthernet("1/1/1")
	if err != nil {
		log.Fatal(err)
	}
	ygot.BuildEmptyTree(gi)
	gi.Description = ygot.String("My description")
	gi.Name = ygot.String("1/1/1")
	gi.Ip.Address.Primary.Address = ygot.String("192.168.69.50")
	gi.Ip.Address.Primary.Mask = ygot.String("192.168.69.50")
	first_validation_error := gi.Validate()
	if first_validation_error != nil {
		fmt.Println("Validation error for 1/1/1:\n")
		fmt.Println(first_validation_error)
	} else {
		fmt.Println("No validation errors for 1/1/1")
	}

But this prints these validation errors:

Validation error for 1/1/1:

/device/native/interface/GigabitEthernet/backup/interface: multiple cases [Port-channel-subinterface ATM-ACRsubinterface LISP-subinterface Serial-subinterface ATM-subinterface] selected for choice interface-choice
/device/native/interface/GigabitEthernet/backup/interface: interface-choice/
encap-choice/
multiple cases [slip isl ppp frame-relay-settings raw-tcp raw-udp scada dot1Q frame-relay] selected for choice encap-choice
/device/native/interface/GigabitEthernet/ip/access-group: /device/native/interface/GigabitEthernet/ip/access-group/in: multiple cases [apply-common apply-intf] selected for choice apply-type
/device/native/interface/GigabitEthernet/ip/access-group: /device/native/interface/GigabitEthernet/ip/access-group/out: apply-type/
/device/native/interface/GigabitEthernet/ip/access-group: /device/native/interface/GigabitEthernet/ip/access-group/out: multiple cases [apply-common apply-intf] selected for choice apply-type
/device/native/interface/GigabitEthernet/ip/access-group: /device/native/interface/GigabitEthernet/ip/access-group/in: apply-type/
/device/native/interface/GigabitEthernet/ip/address-choice/address/address: address-choice/
/device/native/interface/GigabitEthernet/ip/address-choice/address/address: multiple cases [dhcp-case fixed-case] selected for choice address-choice
/device/native/interface/GigabitEthernet/peer/default: /device/native/interface/GigabitEthernet/peer/default/ip: /device/native/interface/GigabitEthernet/peer/default/ip/address: address-choice/
/device/native/interface/GigabitEthernet/peer/default: /device/native/interface/GigabitEthernet/peer/default/ip: /device/native/interface/GigabitEthernet/peer/default/ip/address: multiple cases [dhcp-pool pool] selected for choice address-choice

Meanwhile, instantiating this manually like this:

	// Instanciate the interface map (using `make`), ensuring the map exist and is not nil.
	d.Native.Interface.NewGigabitEthernet("2/1/3")
	// Set the interface description, and IP address. We manually assign the structs to the fields, this is because we struggled with `nil` errors when using NewSecondaryIp, and NewPrimaryIp did not exist.
	// This is a workaround, but don't really complicate anything, since autocomplete will fill out the structs and we would have to define these anyway.
	d.Native.Interface.GigabitEthernet["2/1/3"] = &oc.Cisco_IOS_XENative_Native_Interface_GigabitEthernet{
		Name:        ygot.String("2/1/3"),
		Description: ygot.String("Test description"),
		Ip: &oc.Cisco_IOS_XENative_Native_Interface_GigabitEthernet_Ip{
			Address: &oc.Cisco_IOS_XENative_Native_Interface_GigabitEthernet_Ip_Address{
				Primary: &oc.Cisco_IOS_XENative_Native_Interface_GigabitEthernet_Ip_Address_Primary{
					Address: ygot.String("192.69.69.1"),
					Mask:    ygot.String("255.255.255.0"),
				},
			},
		},
	}
	second_validation_error := d.Native.Interface.GigabitEthernet["2/1/3"].Validate()
	if second_validation_error != nil {
		fmt.Println("Validation error for 2/1/3:\n")
		fmt.Println(second_validation_error)
	} else {
		fmt.Println("No validation errors for 2/1/3")
	}

prints zero validation errors:

No validation errors for 2/1/3

I believe the validation errors happens because the Choice is not respected, so when calling BuildEmptyTree it initializes all choices, instead of ignoring them.
If we look at the YANG model, only one can be defined:

[JonasKs]

Sorry for my deleted comment, it was based on some misunderstandings on my part.
Here's a rewritten one:

We're getting further with our gNMI implementation, thanks for all the nice functionality such as DeepCopy and Diff 😊

It seems like the current behavior of unique is done through creating a map[string]<type>. So for this YANG model:

grouping vrf-af-route-target-grouping {
    container import-route-target {
      list without-stitching {
        description
          "Export Target-VPN community";
        key "asn-ip";
        leaf asn-ip {
          type ios-types:asn-ip-type;
        }
      }
    }

Which creates this struct (I've shortned names a bit)

// Cisco_IOS_XENative_Native_Vrf_Definition_AddressFamily_Ipv4_RouteTarget_ImportRouteTarget represents the /Cisco-IOS-XE-native/native/vrf/definition/address-family/ipv4/route-target/import-route-target YANG schema element.
type ImportRouteTarget struct {
	WithoutStitching    map[string]*WithoutStitching    `path:"without-stitching" module:"Cisco-IOS-XE-native"`
}

// Cisco_IOS_XENative_Native_Vrf_Definition_AddressFamily_Ipv4_RouteTarget_ImportRouteTarget_WithoutStitching represents the /Cisco-IOS-XE-native/native/vrf/definition/address-family/ipv4/route-target/import-route-target/without-stitching YANG schema element.
type WithoutStitching struct {
	AsnIp     *string   `path:"asn-ip" module:"Cisco-IOS-XE-native"`
}

When we ygot.Diff two GoStructs, where current has 1 ExportRouteTarget, and desired has 0, it seems to add one element called "asn-ip" at the end, which should not be there when deleting (according to the device).

See example below:

delete:  {
  path:  {
    elem:  {
      name:  "address-family"
    }
    elem:  {
      name:  "ipv4"
    }
    elem:  {
      name:  "route-target"
    }
    elem:  {
      name:  "export-route-target"
    }
    elem:  {
      name:  "without-stitching"
      key:  {
        key:  "asn-ip"
        value:  "49586:30420"
      }
    }
    elem:  {
      name:  "asn-ip" <-------------------- This shouldn't be here?
    }
  }
}

Workaround for me has been to delete the last element:

for _, x := range diff.Delete {
	x.Elem = x.Elem[:len(x.Elem)-1]
}

The delete-request now works.

NOTE: The element can be present when doing update-requests! 🤔

---

Second issue I have is that I need send updates with the JsonIetfVal, not stringVal. So for every update, I have to do this:

for _, x := range diff.Update {
	x.Val = &oc_gnmi.TypedValue{
		Value: &oc_gnmi.TypedValue_JsonIetfVal{
			JsonIetfVal: []byte(`"` + x.Val.GetStringVal() + `"`),
		}}
	}

Any suggested workarounds for these two issues? 😊

[robshakir]

On the first case -- the diff is correct, in YANG a list has a key that is also present in the list element itself, so it's like writing something like this in go:

type AContainer struct {
   AList map[string]*ListElement
}

type ListElement string {
  Key string
}

where the value of the string key in the map is also the value of the Key. The device may error with this because it might think you're trying to delete the key whilst not deleting the list element itself. However, in some cases, it's materially interesting to know that there was a diff here (in ygot generated structs, it's possible that you can change the key field value without changing the list key's value...). We could entertain a PR that adds a DiffOpt to avoid reporting list keys.

w.r.t wanting JsonIetfVal, both Diff and TogNMINotifications are written to use the individual leaf types, rather than the JSON types. Again we could add an option to produce the JsonIetfVal since we already know how to marshal to such a type. PRs very welcome :-)

[JonasKs]

Thanks for the reply😊 That makes sense.

I just went on a 2 week vacation, so I won’t be able to raise a PR right now, but I’d love to try when I get back home.

I see you've replied in #581 as well - perfect 😊