Skip to content

Implement Openconfig NTP provider #517

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
adamtrizuljak-sap wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Implement Openconfig NTP provider #517

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2ab090d
Implement NTP Openconfig provider
adamtrizuljak-sap Sep 3, 2026
fd9475d
Review, cleanup
adamtrizuljak-sap Sep 10, 2026
c85ecfb
Move NTP sample into config/samples/nokia/srlinux
adamtrizuljak-sap Sep 11, 2026
6d7ef8f
Fix GNMI test data
adamtrizuljak-sap Sep 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Tiltfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,9 @@ k8s_resource(new_name='mac-entry', objects=['mac-entry:probe'], trigger_mode=TRI
k8s_resource(new_name='route-prefix', objects=['route-prefix:probe'], trigger_mode=TRIGGER_MODE_MANUAL, auto_init=False, labels=['samples'])
k8s_resource(new_name='vtep-peers', objects=['vtep-peers:probe'], trigger_mode=TRIGGER_MODE_MANUAL, auto_init=False, labels=['samples'])

k8s_yaml('./config/samples/nokia/srlinux/v1alpha1_ntp.yaml')
k8s_resource(new_name='ntp-srlinux', objects=['ntp-srlinux:ntp'], trigger_mode=TRIGGER_MODE_MANUAL, auto_init=False, labels=['srlinux'])

print('🚀 network-operator development environment')
print('👉 Edit the code inside the api/, cmd/, or internal/ directories')
print('👉 Tilt will automatically rebuild and redeploy when changes are detected')
Expand Down
19 changes: 19 additions & 0 deletions config/samples/nokia/srlinux/v1alpha1_ntp.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
apiVersion: networking.metal.ironcore.dev/v1alpha1

@nikatza nikatza Sep 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I fear these new files included in this folder as they don't really fit well here in this PR. I think the examples folder is reserved to more complex scenarios. If that is OK for you we could remove them and clarify in our weekly. What do you think?

@adamtrizuljak-sap adamtrizuljak-sap Sep 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I created this file because Openconfig requires different settings from config/samples/v1alpha1_ntp.yaml.

As I wrote here #517 (comment), my idea is to have ready-to-go samples for all providers which can be used during development for testing against a Containerlab setup or similar. If I only have config/samples/v1alpha1_ntp.yaml in the repo, I would need to overwrite it each time, discard the changes afterwards (and pay attention to not commit it) and do this for every resource that differs -> not a very streamlined and uniform development workflow.

Do you have an idea for better placement of such samples? Maybe move config/samples/ into something like config/samples/common and place this file into config/samples/openconfig?

kind: NTP
metadata:
labels:
app.kubernetes.io/name: network-operator
app.kubernetes.io/managed-by: kustomize
networking.metal.ironcore.dev/device-name: leaf1
name: ntp-srlinux
spec:
deviceRef:
name: leaf1
sourceInterfaceName: mgmt0
servers:
- address: de.pool.ntp.org
prefer: true
vrfName: mgmt
- address: pool.ntp.org
prefer: false
vrfName: mgmt
1 change: 1 addition & 0 deletions examples/openconfig-containerlab/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
clab-nav-srl
14 changes: 14 additions & 0 deletions examples/openconfig-containerlab/topology.clab.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
name: nav-srl

topology:
nodes:
leaf1:
kind: nokia_srlinux
image: ghcr.io/nokia/srlinux:26.7.1
startup-config: |-
system name host-name srl
system aaa authentication admin-user password admin
system grpc-server mgmt yang-models openconfig
ports:
- 8022:22
- 9339:57400
25 changes: 25 additions & 0 deletions internal/provider/openconfig/interface.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package openconfig
import (
"context"
"encoding/json"
"errors"
"fmt"

"github.com/ironcore-dev/network-operator/api/core/v1alpha1"
Expand Down Expand Up @@ -664,3 +665,27 @@ type SubinterfaceVlanDoubleTaggedConfig struct {
InnerVlanID uint16 `json:"inner-vlan-id,omitempty"`
OuterVlanID uint16 `json:"outer-vlan-id,omitempty"`
}

type interfaceAddrs struct {
ifName string
Address gnmiext.List[string, *IPv4Address] `json:"address"`
}

func (a *interfaceAddrs) XPath() string {
return fmt.Sprintf("openconfig-interfaces:interfaces/interface[name=%s]/subinterfaces/subinterface[index=0]/openconfig-if-ip:ipv4/addresses", a.ifName)

@nikatza nikatza Sep 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We have two hard-coded elements here: the index (0) and the AFI (IPv4). I think we should be able to fetch IPv6 or indicate the address family we want. Same goes for the index. What we could do is add these fields in interfaceAddrs (with tag json:"-") and build our xpath using those values.

If so the provider should be changed accordingly, e.g., use index 0 when the interface name matches some constraints (or just document why 0 is hard-coded). For the IPv4 and IPv6 AFI: you could make that dependent on the AFI of the NTPServers in the spec.

@adamtrizuljak-sap adamtrizuljak-sap Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's a simple lookup to get the first IP address of the interface given by NTPSpec.SourceInterfaceName, which we did to avoid adding the NTPSpec.SourceAddress field. I am not sure how to include the information (especially the interface index) without again modifying NTPSpec...

}

// interfaceIPAddr retrieves the first IPv4 address from the state of the named interface.
func (p *Provider) interfaceIPAddr(ctx context.Context, name string) (string, error) {

@nikatza nikatza Sep 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should be able to get IPv6 addresses too :)

@adamtrizuljak-sap adamtrizuljak-sap Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I could do it, but in general we don't support IPv6 in network-operator?

addrs := &interfaceAddrs{ifName: name}
if err := p.client.GetState(ctx, addrs); err != nil {
if errors.Is(err, gnmiext.ErrNil) {
return "", apistatus.NewFailedPreconditionError(fmt.Sprintf("interface %q has no IPv4 address", name))
}
return "", fmt.Errorf("failed to get IPv4 address for interface %q: %w", name, err)
}
for _, a := range addrs.Address {
return a.IP, nil
}
return "", apistatus.NewFailedPreconditionError(fmt.Sprintf("interface %q has no IPv4 address", name))
}
91 changes: 91 additions & 0 deletions internal/provider/openconfig/ntp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
// SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and IronCore contributors
// SPDX-License-Identifier: Apache-2.0

package openconfig

import (
"context"

"github.com/ironcore-dev/network-operator/api/core/v1alpha1"
"github.com/ironcore-dev/network-operator/internal/provider"
"github.com/ironcore-dev/network-operator/internal/transport/gnmiext"
)

var _ provider.NTPProvider = (*Provider)(nil)

func (p *Provider) EnsureNTP(ctx context.Context, req *provider.EnsureNTPRequest) error {
spec := req.NTP.Spec

n := &NTP{
Config: &NTPConfig{
Enabled: spec.AdminState == v1alpha1.AdminStateUp,
},
}

var sourceAddress string
if spec.SourceInterfaceName != "" {
var err error
sourceAddress, err = p.interfaceIPAddr(ctx, spec.SourceInterfaceName)
if err != nil {
return err
}
}

if len(spec.Servers) > 0 {
n.Servers = &NTPServers{}
for _, s := range spec.Servers {
n.Servers.Server.Set(&NTPServer{
Address: s.Address,
Config: &NTPServerConfig{
Address: s.Address,
Prefer: s.Prefer,
NetworkInstance: s.VrfName,
SourceAddress: sourceAddress,
},
})
}
}

return p.client.Update(ctx, n)
}

func (p *Provider) DeleteNTP(ctx context.Context) error {
return p.client.Delete(ctx, &NTP{})
}

// Compile-time assertions.
var _ gnmiext.DataElement = (*NTP)(nil)

// DNS represents the OpenConfig /system/ntp container.
type NTP struct {
Config *NTPConfig `json:"config"`
Servers *NTPServers `json:"servers"`
Comment thread
felix-kaestner marked this conversation as resolved.
}

func (*NTP) XPath() string { return "openconfig-system:system/ntp" }

// NTPConfig holds the config container for NTP.
type NTPConfig struct {
Enabled bool `json:"enabled"`
}

// NTPServers holds the servers container for NTP.
type NTPServers struct {
Server gnmiext.List[string, *NTPServer] `json:"server"`
}

// NTPServer represents a single NTP server entry.
type NTPServer struct {
Address string `json:"address"`
Config *NTPServerConfig `json:"config"`
}

func (s *NTPServer) Key() string { return s.Address }

// NTPServerConfig holds the config container for a NTP server.
type NTPServerConfig struct {
Address string `json:"address"`
Prefer bool `json:"prefer"`
NetworkInstance string `json:"network-instance,omitempty"` // Maps to VrfName
SourceAddress string `json:"source-address,omitempty"`
}
192 changes: 192 additions & 0 deletions test/gnmi/testdata/openconfig/ntp.txtar
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,192 @@
# NTP
-- ntp/ntp --
apiVersion: networking.metal.ironcore.dev/v1alpha1
kind: NTP
metadata:
name: ntp
spec:
deviceRef:
name: leaf1
sourceInterfaceName: mgmt0
servers:
- address: de.pool.ntp.org
prefer: true
vrfName: mgmt
- address: pool.ntp.org
prefer: false
vrfName: mgmt

-- state/preload --
{
"openconfig-system:system": {
"state": {
"boot-time": "1784731445707000000"
}
},
"openconfig-interfaces:interfaces": {
"interface": [
{
"name": "mgmt0",
"config": {
"enabled": true,
"name": "mgmt0",
"type": "iana-if-type:ethernetCsmacd"
},
"subinterfaces": {
"subinterface": [
{
"index": 0,
"config": {
"enabled": true,
"index": 0
},
"openconfig-if-ip:ipv4": {
"addresses": {
"address": [
{
"ip": "172.20.20.2",
"state": {
"ip": "172.20.20.2",
"origin": "DHCP",
"prefix-length": 24
}
}
]
},
"config": {
"enabled": true
}
}
}
]
}
}
]
}
}

-- state/expect --
{
"openconfig-system:system": {
"state": {
"boot-time": "1784731445707000000"
},
"ntp": {
"config": {
"enabled": true
},
"servers": {
"server": [
{
"address": "de.pool.ntp.org",
"config": {
"address": "de.pool.ntp.org",
"prefer": true,
"network-instance": "mgmt",
"source-address": "172.20.20.2"
}
},
{
"address": "pool.ntp.org",
"config": {
"address": "pool.ntp.org",
"prefer": false,
"network-instance": "mgmt",
"source-address": "172.20.20.2"
}
}
]
}
}
},
"openconfig-interfaces:interfaces": {
"interface": [
{
"name": "mgmt0",
"config": {
"enabled": true,
"name": "mgmt0",
"type": "iana-if-type:ethernetCsmacd"
},
"subinterfaces": {
"subinterface": [
{
"index": 0,
"config": {
"enabled": true,
"index": 0
},
"openconfig-if-ip:ipv4": {
"addresses": {
"address": [
{
"ip": "172.20.20.2",
"state": {
"ip": "172.20.20.2",
"origin": "DHCP",
"prefix-length": 24
}
}
]
},
"config": {
"enabled": true
}
}
}
]
}
}
]
}
}

-- state/delete --
{
"openconfig-system:system": {
"state": {
"boot-time": "1784731445707000000"
}
},
"openconfig-interfaces:interfaces": {
"interface": [
{
"name": "mgmt0",
"config": {
"enabled": true,
"name": "mgmt0",
"type": "iana-if-type:ethernetCsmacd"
},
"subinterfaces": {
"subinterface": [
{
"index": 0,
"config": {
"enabled": true,
"index": 0
},
"openconfig-if-ip:ipv4": {
"addresses": {
"address": [
{
"ip": "172.20.20.2",
"state": {
"ip": "172.20.20.2",
"origin": "DHCP",
"prefix-length": 24
}
}
]
},
"config": {
"enabled": true
}
}
}
]
}
}
]
}
}

Loading