Repository navigation
Conversation
connectSBI() sets Datastore.sbi asynchronously after New() returns, while ConnectionState(), Stop(), and applyIntent() read it concurrently from RPC handlers, causing a data race detected by -race.
GetContainer().GetKeys() returns the schema's own slice, which is shared and read concurrently by other import tasks. Sorting it in place raced with those readers/sorters running in other worker-pool goroutines. Sort a cloned copy instead.
must-statements declared directly on a YANG list were evaluated both at the list's key-level node (whose children are keyed instances, not the instance's own siblings) and at each resolved instance. At the key-level node, xpath expressions referencing sibling leaves degrade to an empty node-set and spuriously fail, producing errors with no key predicate in the path. Mirror validateMandatory's key-level descent (now extracted into a shared descendKeyLevels helper) so must-statements on a list are only ever evaluated once, against a resolved instance.
Bumps github.com/sdcio/yang-parser to ee8fd4a, which fixes two related bugs behind count(leaflist[predicate]) must-statements (e.g. SONiC's "count(adv_speeds[text()='all']) = 0 or count(adv_speeds) = 1"): DatumSlice comparisons collapsing to Bool instead of composing with count(), and a path-stack leak between a predicated and a bare reference to the same leaf-list in one expression. That fix only takes effect when a leaf-list Entry's GetValue() reports as a DatumSlice. yangParserEntryAdapter.GetValue() returned an empty NodesetDatum instead for a leaf-list with no value at all, which is the common case (e.g. adv_speeds/adv_interface_types are normally unset on every SONiC port) -- causing the same crash to persist for that specific shape. Report an empty DatumSliceDatum for that case instead, matching the populated-leaf-list branch. Adds a count-leaflist leaf-list + must-statement to the test schema (ygot bindings regenerated) and a regression test covering unset, single-value, and multi-value leaf-list states.
…ebug logging in transaction_rpc - NewOnceSync/NewStreamSync now take a targetName parameter and set gapi.Target(targetName) on their SubscribeRequest, matching gnmi.go's existing t.cfg.GnmiOptions.TargetName usage for Get/Set requests (already committed). gnmi.go's call sites already passed this argument; without this commit, HEAD fails to build: 'too many arguments in call to NewOnceSync/NewStreamSync'. - transaction_rpc.go: add two V(VDebug)-gated log lines (expanded intent updates; intent updates inserted into tree) added while investigating the sonic-spine1 SRv6 locator/must-statement bugs (see .scratch/sonic-spine1-srv6-locator-bug/handoff.md, now fully resolved end-to-end). Kept as low-risk diagnostic instrumentation.
…ual Datastore literal
commit 8984fef added Datastore.sbiMutex (*sync.RWMutex) to guard sbi,
but this test builds a Datastore{} literal directly and set sbi without
ever setting sbiMutex, leaving it nil. applyIntent()'s d.getSBI() call
dereferences the nil *sync.RWMutex, panicking. No other test constructs
Datastore with sbi set, so this went unnoticed until now.
… as real schema members The self-wrap unwrap added in 95b8408 ran unconditionally for any top-level container whose single JSON key matched its own qualified name, with no device-profile check (converter.go is generic infrastructure shared by every vendor's GET/Subscribe response parsing, not just SONiC's). That's a plausible false-positive for any non-SONiC target whose single populated key legitimately happens to share the container's own qualified name as a real schema-defined child, silently misinterpreting a legitimate field as a self-wrap and dropping a path segment instead of processing it normally. Guard the unwrap with getItem: a container never lists itself as one of its own schema children, so a true self-reference is guaranteed to fail that lookup, while any real, resolvable child (the common case) is left untouched and processed via the normal field/child path below. This keeps the fix scoped to the exact translib quirk it targets without adding a device-profile/vendor check to this generic function. Added TestExpandUpdateContainerDoesNotUnwrapLegitimateChildSharingTheContainersName covering the false-positive case. This is a defensive scoping fix for a suspected (not yet CI-confirmed) cause of the 02-Crud.12-Srl-Create-Delete integration test regression seen after rebasing onto ciscoiosxrd2 (SRL's ConfigSet never reaching Ready, while 11-Sros-Create-Delete passes unaffected).
…igateSdcpbPath commit 4275945 moved must-statement evaluation from the list's key-level node to the resolved instance (key-value) node so that must-statements referencing the instance's own sibling fields evaluate correctly. However, must-statements that navigate `../field` (to the YANG parent) broke: from a key-value node (schema==nil), `..` landed on the key-level (the list's schema node), and `type` was then looked up among the key-level's children — which are other key-value instances, not the parent container's fields. The resulting empty node-set made XPath `emptyNodeset != 'module:identity'` evaluate to FALSE per RFC 7950 §7.5.3, causing every affected must-statement to spuriously fail. The SDCIO tree contains an extra level of indirection for list instances: parent-container ← YANG parent of the list list KEY-LEVEL ← carries the list schema; children are key-value nodes key-value-1 ← schema==nil; children are the instance's data fields From a key-value node, one `..` step reaches the key-level, not the YANG parent that YANG/XPath authors expect. Fix NavigateSdcpbPath to detect this case (current node has no schema) and skip to the YANG parent directly by calling GetFirstAncestorWithSchema to land on the key-level, then one more GetParent() to reach the actual YANG-level containing node. The consecutive `../.. ` path (used internally) is unchanged: it still calls GetFirstAncestorWithSchema on the starting node, and each `..` leg in the recursive descent then benefits from the same key-value skip.
Complete subscribe target naming from legacy 57f1c7d on current main: wire t.cfg.GnmiOptions.TargetName at gnmi.go call sites and tests.
Map every commit on ciscoiosxrd2 and sonic-device-profile to base, main, SONiC, or Cisco buckets; record sdc-protos#120 pin and cherry-pick notes.
Update INDEX frontier and ticket 02 checklist/delivery notes; record branch name and inventory SHA mapping. Not merged to main yet.
This was referenced Sep 24, 2026
steiler
added a commit
that referenced
this pull request
Sep 24, 2026
Resolve converter_test.go import conflict: keep both encoding/json and fmt.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! 🚀 New features to boost your workflow:
|
faebr
approved these changes
Oct 29, 2026
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
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.
Summary
Nine generic fixes from the #504 legacy inventory (
#480→mainbucket). No device-profile materialization, protos pin, or NOS encoders.Fixes (what / why / how)
c368539— What: Serialize access to the datastoresbitarget field. Why:-raceflagged concurrent read/write during intent apply. How:sbiMutexaround updates and reads ofsbi.b3aecc4— What: InitializesbiMutexinTestTransactionSet_PreviouslyApplied's hand-builtDatastore. Why: Test struct must match production locking afterc368539. How: Set mutex on the literal used in that test.e16aaa7— What: Stop mutating the shared schema key slice during import. Why: In-place sort corrupted keys for other importers on the same schema node. How: Clone the key slice before sort inimportConfigTask.4275945— What: Skip listmustXPath evaluation on the key-level list node. Why: Musts belong on the list entry, not the key placeholder—false failures/wrong context. How: Key-level validation path invalidation_keylevel/ must wiring.4ea1b4f— What: Supportcount()in must expressions when a leaf-list is unset. Why: Prior yang-parser path crashed instead of treating unset as empty. How: Bump yang-parser + adapter in tree validation tests (sdcio_model_count.yang).1417054— What: Correct parent resolution inNavigateSdcpbPathfrom key-value nodes. Why: Wrong parent broke XPath/must navigation (SONiC integration hit this first). How: Adjust parent walk innavigatesdcpbpath.go.95b8408(main only) — What: Richer errors when a must-statement fails (path + must text). Why: Bare xpath errors were hard to debug in lab/CI. How:validation_entry_mustwraps evaluation errors with entry path and must string. (NilGnmiSetPlan/ materialize → #506.)cbb179d— What: Narrow RFC7951 GET self-wrap unwrap. Why: Broad unwrap broke values whose keys are real schema children (e.g. SRL-style payloads). How: Unwrap only when the key does not resolve as a schema member (converter.go).57f1c7d+a06a69e— What: Pass configured target name into gNMI Once/Stream subscribe options (and sync constructor call sites on currentmain); gate noisy debug logging in transaction RPC. Why: SubscribeTargetmust match datastore identity; debug spam in normal runs. How:targetNamethrough gnmi Once/Stream +transaction_rpcdebug guard.Out of scope
sdc-protos#120, materialize /GnmiSetPlan, profile enablement, encoders → #506 / #507 / #508.Test plan
go test ./...Stack
Merge to
main, then rebase device-profile stacks; NOS PRs link here until merged.