feat: Cisco IOS-XR gNMI support via module-anchored SetRequest encoding#442
Open
steiler wants to merge 3 commits into
Open
feat: Cisco IOS-XR gNMI support via module-anchored SetRequest encoding#442steiler wants to merge 3 commits into
steiler wants to merge 3 commits into
Conversation
Adds first-class support for Cisco IOS-XR / XRd devices as gNMI targets by introducing a materialization layer that owns all NOS-specific encoding, keeping the gNMI transport driver a thin wire-execution layer. Companion proto change: sdcio/sdc-protos#120 (adds `device_profile` field) ┌─────────────────────────────────────────────────────────────────────────┐ │ BEFORE │ │ │ │ applyIntent │ │ │ │ │ ▼ │ │ TargetSource adapter ◄── api.Entry wrapped here │ │ │ │ │ ▼ │ │ Target.Set(TargetSource) │ │ │ │ │ ▼ │ │ gNMI driver ── serializes internally ──► SetRequest │ │ └─ Update{origin:"", /} │ │ └─ entire JSON blob │ │ │ │ (IOS-XR rejects: unknown native YANG keys in OpenConfig context) │ └─────────────────────────────────────────────────────────────────────────┘ ┌─────────────────────────────────────────────────────────────────────────┐ │ AFTER │ │ │ │ applyIntent │ │ │ api.Entry + replace flag │ │ ▼ │ │ materialize.BuildPlan(entry, device-profile) │ │ │ │ │ ├─[generic / proto]──────────────► GnmiSetPlan (single update) │ │ │ │ │ └─[cisco-ios-xr + json/json_ietf] │ │ │ │ │ ▼ │ │ permodule encoder │ │ group children by ModuleName │ │ │ │ │ ▼ │ │ GnmiSetPlan │ │ ├─ Update{origin:"Cisco-IOS-XR-ip-static-cfg", /router} │ │ ├─ Update{origin:"Cisco-IOS-XR-ifmgr-cfg", /interfaces} │ │ └─ ...one per module... │ │ │ │ ▼ │ │ Target.Set(SouthboundSetPlan) ◄── typed plan, no NOS logic here │ │ │ │ │ ▼ │ │ gNMI driver (transport only) ──────► single SetRequest │ │ └─ all module Updates atomic │ └─────────────────────────────────────────────────────────────────────────┘ Core additions: - New `pkg/datastore/target/materialize` package: translates `api.Entry` + device-profile into a typed `SouthboundSetPlan` before any transport call - New `pkg/datastore/target/gnmi/permodule` encoder: groups config tree children by YANG module name, emits one `Update` per module with `Path.origin` set to the full module name (e.g. `Cisco-IOS-XR-ip-static-cfg`) — all updates in a single `SetRequest` to preserve gNMI atomicity - New `pkg/datastore/target/types` discriminated plan type (`GnmiSetPlan` / `NetconfSetPlan`) replacing the `TargetSource` abstraction on the Set path - `device-profile: cisco-ios-xr` config field on SBI with closed-set validation: unknown profiles fail at load time; `cisco-ios-xr` + netconf is rejected; `cisco-ios-xr` + proto is accepted without IOS-XR shaping Transport/driver changes: - gNMI driver (`gnmi.go`) simplified to pure transport: receives a pre-built `GnmiSetPlan` and executes it, with no knowledge of device-profile or module grouping - Replace transactions encoded as per-module delete + per-module update pairs in one `SetRequest` (gNMI `replace` field not used, consistent with existing behavior) - Delete paths carry `origin` resolved from schema metadata or schema-client lookup for choice-case entries Removals / cleanup: - `TargetSource` interface and its adapters retired from the Set path (`targetsource.go`, `target_source_replace.go`, `entryoutputadapter.go` deleted) - `applyIntent` now passes raw `api.Entry` + replace flag to materialize instead of wrapping in a `TargetSource` adapter Test coverage added: - `permodule` encoder: multi-module tree assertions (update count, origin values, path elements, JSON body shape, replace delete emission) - `materialize`: `BuildPlan` assertions for both generic and IOS-XR profiles - `config`: validation rejection of unknown profiles and netconf+ios-xr - `noop` and gNMI `get` path: adapted to new plan-based interface
Update comment for DeviceProfile to clarify usage.
ops.ToJson/ToJsonIETF returns nil when no leaves are new or updated.
Without a nil guard, json.Marshal(nil) produces "null", and the
resulting gNMI SetRequest carries {path:{}, val:{jsonVal:"null"}}.
SROS rejects this with GMI #2052 ("Cannot set JSON value, because
last element is not leaf") because null is not valid JSON for a
container node.
Add the nil check that existed in the old gnmi.go Set switch
(pre-materialize refactor) so a no-op reconcile never sends a
spurious null update to the device.
Co-authored-by: Cursor <cursoragent@cursor.com>
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! 🚀 New features to boost your workflow:
|
Collaborator
Author
|
@giacoliva if I provide proper instructions to you how to test this, whould you be able to do so? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds first-class support for Cisco IOS-XR / XRd devices as gNMI targets by introducing a materialization layer that owns all NOS-specific encoding, keeping the gNMI transport driver a thin wire-execution layer.
Companion proto change: sdcio/sdc-protos#120 (adds
device_profilefield)Core additions:
pkg/datastore/target/materializepackage: translatesapi.Entry+ device-profile into a typedSouthboundSetPlanbefore any transport callpkg/datastore/target/gnmi/permoduleencoder: groups config tree children by YANG module name, emits oneUpdateper module withPath.originset to the full module name (e.g.Cisco-IOS-XR-ip-static-cfg) — all updates in a singleSetRequestto preserve gNMI atomicitypkg/datastore/target/typesdiscriminated plan type (GnmiSetPlan/NetconfSetPlan) replacing theTargetSourceabstraction on the Set pathdevice-profile: cisco-ios-xrconfig field on SBI with closed-set validation: unknown profiles fail at load time;cisco-ios-xr+ netconf is rejected;cisco-ios-xr+ proto is accepted without IOS-XR shapingTransport/driver changes:
gnmi.go) simplified to pure transport: receives a pre-builtGnmiSetPlanand executes it, with no knowledge of device-profile or module groupingSetRequest(gNMIreplacefield not used, consistent with existing behavior)originresolved from schema metadata or schema-client lookup for choice-case entriesRemovals / cleanup:
TargetSourceinterface and its adapters retired from the Set path (targetsource.go,target_source_replace.go,entryoutputadapter.godeleted)applyIntentnow passes rawapi.Entry+ replace flag to materialize instead of wrapping in aTargetSourceadapterTest coverage added:
permoduleencoder: multi-module tree assertions (update count, origin values, path elements, JSON body shape, replace delete emission)materialize:BuildPlanassertions for both generic and IOS-XR profilesconfig: validation rejection of unknown profiles and netconf+ios-xrnoopand gNMIgetpath: adapted to new plan-based interface