Implement Openconfig NTP provider

[adamtrizuljak-sap]

Work in progress

• Add OpenConfig provider implementation for the NTP resource (internal/provider/openconfig/ntp.go) and a gnmi testdata file (test/gnmi/testdata/openconfig/ntp.txt).
• Add optional field NTPSpec.SourceAddress and validation logic (see below)
• Add a simple Containerlab setup with one SRLinux node + the associated network-operator Device and NTP resources
• Lint required renaming SrcIpItems, SrcIp to SrcIPItems,SrcIP in internal/provider/cisco/nxos/ntp.go

Applying examples/openconfig-containerlab/kubernetes/01-devices/v1alpha1-leaf1-clab-ntp.yaml produces the following config in SRLinux:

--{ + running }--[  ]--
A:admin@srl# info system ntp | as json
{
  "admin-state": "enable",
  "server": [
    {
      "address": "de.pool.ntp.org",
      "prefer": true,
      "network-instance": "mgmt",
      "source-address": "192.168.65.254"
    },
    {
      "address": "pool.ntp.org",
      "network-instance": "mgmt",
      "source-address": "192.168.65.254"
    }
  ]
}

Cisco vs Openconfig compatibility issue (solved)

NTPSpec defines the SourceInterfaceName field https://github.com/ironcore-dev/network-operator/blob/main/api/core/v1alpha1/ntp_types.go#L36. Openconfig has something similar source-address https://openconfig.net/projects/models/schemadocs/yangdoc/openconfig-system.html#system-ntp-servers-server-config-source-address

A way to handle it would be to look up the interface given by SourceInterfaceName , get its IPv4 address and put it into source-address. Does it make sense? Do you usually do such operations, or do you keep the provider code clean from reading any additional k8s resources?

Other way would be to just ignore SourceInterfaceName and not pass it into the Openconfig struct, or even throw a apistatus.FieldViolation saying that it's unsupported

Solution

Discussed with @nikatza and @rgildein

• Mark NTPSpec.SourceInterfaceName as optional
• Add optional field NTPSpec.SourceAddress
  • Maps to https://pubhub.devnetcloud.com/media/dme-docs-10-4-3/docs/System/datetime:NtpSrcIp/ in Cisco
  • Maps to https://openconfig.net/projects/models/schemadocs/yangdoc/openconfig-system.html#system-ntp-servers-server-config-source-address in Openconfig
• In Cisco controller, ensure that at least one of the fields is defined
• In Openconfig controller, throw an unsupported error if SourceInterfaceName is defined

Since both fields are optional, the change will be backwards-compatible with the previous NTPSpec and the generated custom resources.

[github-actions]

Merging this branch will decrease overall coverage

Impacted Packages	Coverage Δ	🤖
github.com/ironcore-dev/network-operator/api/core/v1alpha1	2.66% (ø)
github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos	9.82% (-0.02%)	👎
github.com/ironcore-dev/network-operator/internal/provider/openconfig	7.69% (-0.28%)	👎

---

Coverage by file

Changed files (no unit tests)

Changed File	Coverage Δ	Total	Covered	Missed	🤖
github.com/ironcore-dev/network-operator/api/core/v1alpha1/ntp_types.go	12.50% (ø)	8	1	7
github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos/ntp.go	50.00% (ø)	4	2	2
github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos/provider.go	0.37% (-0.00%)	2167 (+6)	8	2159 (+6)	👎
github.com/ironcore-dev/network-operator/internal/provider/openconfig/ntp.go	0.00% (ø)	21 (+21)	0	21 (+21)

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

[felix-kaestner]

Why do we need all these files in examples/openconfig-containerlab?

If you just use the following containerlab topology:

name: srlinux
topology:
  nodes:
    srl:
      kind: nokia_srlinux
      image: ghcr.io/nokia/srlinux:26.3.1
      startup-config: |-
        system grpc-server mgmt yang-models native
        system aaa authentication admin-user password admin
      ports:
        - 8022:22
        - 9339:57400

The default device sample in https://github.com/ironcore-dev/network-operator/blob/main/config/samples/v1alpha1_device.yaml connecting to host.docker.internal on port 9339 with admin:admin will work out of the box. This is already setup and integrated with the Tiltfile, so everything is already there.

[adamtrizuljak-sap]

That's great! I missed the part where Tiltfile overrides the device IP. It's working nicely for me.

Therefore, under examples/openconfig-containerlab I can keep just the topology.clab.yml and the NTP resource file. I think the NTP resource must remain separate because Cisco needs vrfName: management and Nokia needs mgmt. Unless we want to do some Tiltfile hacks as with the device IP...

[felix-kaestner]

Could we just look up the source address by using the source interface name inside of the provider implementation and then use it as source address without the need to alter the API?

[adamtrizuljak-sap]

We discussed this with @nikatza. In the current architecture the providers don't make requests to the K8s API to fetch more information, they purely translate the EnsureXXRequest to the vendor-specific configuration format. We didn't want to violate this principle.

Other option would be to look up the Interface in the NTP controller if SourceInterfaceName and pass it into the EnsureNTPRequest.

[adamtrizuljak-sap]

One catch in looking up the interface: When we use sourceInterfaceName: mgmt0 (or another vendor-specific management interface name), we don't create an Interface resource for the management interface. Therefore there is no Interface resource to look up and get the IP address. Maybe the only way would be to read the address of the interface directly from the device using the GRPC client? Are you already doing this in some other controllers?

[nikatza]

Thanks a lot for pushing the OpenConfig provider, really nice :)

I think that what Felix here implies is that we shouldn't change the API to accommodate vendor-specific behavior. Instead, the EnsureNTP method in the openconfig provider could fetch the configuration for the referenced interface and use the assigned IP address to configure NTP. Essentially something along the lines of this function, or calling directly GetConfig on the appropiate structure that contains the addressing. This means that in .spec.SourceInterfaceName we don't give the name of a k8s Interface resource, but the name of the interface in the device itself (e.g., in Cisco eth1/1), which the provider can use.

One note with regard to API changes: even if the project is still in early stages we have already agreed on how to handle API changes in these docs. It's a bit tedious, but I think we can avoid this here and happily use the existing API :)