Skip to content

Fix for issue #733 - broken delete - #734

Open
kraney wants to merge 5 commits into
openconfig:mainfrom
kraney:failed-delete
Open

kraney wants to merge 5 commits into
openconfig:mainfrom
kraney:failed-delete

Conversation

@kraney

@kraney kraney commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

We added gnmi_test.go:1154-1204 directly in gnmi_test.go:

• Tested deleting interface[name=eth1]/subinterfaces/subinterface[index=0]/ipv4/addresses/address[ip=192.0.2.1]. • Verified that ygot.DiffWithAtomic with both PreferShadowPath=false and PreferShadowPath=true does emit deletes for all key leaves:
• /interfaces/interface[name=eth1]/name
• /interfaces/interface[name=eth1]/subinterfaces/subinterface[index=0]/index
• /interfaces/interface[name=eth1]/subinterfaces/subinterface[index=0]/ipv4/addresses/address[ip=192.0.2.1]/ip

Because ygot.DiffWithAtomic emits deletes for key leaves correctly, the intermittency (~3% of runs) came from concurrent execution between:

• Config updates (s.configMu, preferShadowPath=true), and • State reconciler updates (s.stateMu, preferShadowPath=false).

Because Server had two separate mutexes (configMu and stateMu), a state reconciler write and a config delete write could execute concurrently. State updates running during or right after a delete could re-populate key leaves in the shared cache after the delete notification was evaluated.

Fix Applied:

• Consolidated s.configMu and s.stateMu into a single s.mu in gnmi.go:64-67 in gnmi.go, ensuring synchronized and atomic mutations to the shared datastore schemas and gNMI cache.

When deleting paths such as /interfaces/interface[name=eth1]/subinterfaces or non-existent paths, ytypes.UnmarshalSetRequest returned InvalidArgument: no match found in .... Per gNMI specification Section 3.4.4, deleting a non-existent path is a no-op success.

Fix Applied:
• In gnmi.go:317-347, if ytypes.UnmarshalSetRequest fails with no match found, delete operations that don't match the tree are tolerated as no-ops instead of failing with InvalidArgument.

In gnmi_test.go:1154-1289:

• TestInterfaceKeyLeavesDelete: Asserts that ygot.DiffWithAtomic emits Delete for all list key leaves (interface name, subinterface index, address ip).
• TestDeleteOperations:
• delete_non-existent_path_returns_OK: Deleting a non-existent path returns OK (nil error).
• delete_intermediate_container_succeeds: Deleting intermediate container /interfaces/interface[name=eth1]/subinterfaces succeeds.
• configure_and_delete_cycles_leave_zero_residue_and_are_idempotent: 200 consecutive configure-then-delete cycles against the gNMI
server, verifying 0 residue leaves remain and repeated deletes are idempotent.

All unit tests in gnmi pass cleanly.

We added gnmi_test.go:1154-1204 directly in gnmi_test.go:

• Tested deleting interface[name=eth1]/subinterfaces/subinterface[index=0]/ipv4/addresses/address[ip=192.0.2.1].
• Verified that ygot.DiffWithAtomic with both PreferShadowPath=false and PreferShadowPath=true does emit deletes for all key leaves:
    • /interfaces/interface[name=eth1]/name
    • /interfaces/interface[name=eth1]/subinterfaces/subinterface[index=0]/index
    • /interfaces/interface[name=eth1]/subinterfaces/subinterface[index=0]/ipv4/addresses/address[ip=192.0.2.1]/ip

Because ygot.DiffWithAtomic emits deletes for key leaves correctly, the intermittency (~3% of runs) came from concurrent execution
between:

• Config updates (s.configMu, preferShadowPath=true), and
• State reconciler updates (s.stateMu, preferShadowPath=false).

Because Server had two separate mutexes (configMu and stateMu), a state reconciler write and a config delete write could execute
concurrently. State updates running during or right after a delete could re-populate key leaves in the shared cache after the delete
notification was evaluated.

Fix Applied:

• Consolidated s.configMu and s.stateMu into a single s.mu in gnmi.go:64-67 in gnmi.go, ensuring synchronized and atomic mutations to the
shared datastore schemas and gNMI cache.

When deleting paths such as /interfaces/interface[name=eth1]/subinterfaces or non-existent paths, ytypes.UnmarshalSetRequest returned
InvalidArgument: no match found in .... Per gNMI specification Section 3.4.4, deleting a non-existent path is a no-op success.

Fix Applied:
• In gnmi.go:317-347, if ytypes.UnmarshalSetRequest fails with no match found, delete operations that don't match the tree are tolerated
as no-ops instead of failing with InvalidArgument.

In gnmi_test.go:1154-1289:

• TestInterfaceKeyLeavesDelete: Asserts that ygot.DiffWithAtomic emits Delete for all list key leaves (interface name, subinterface index,
address ip).
• TestDeleteOperations:
    • delete_non-existent_path_returns_OK: Deleting a non-existent path returns OK (nil error).
    • delete_intermediate_container_succeeds: Deleting intermediate container /interfaces/interface[name=eth1]/subinterfaces succeeds.
    • configure_and_delete_cycles_leave_zero_residue_and_are_idempotent: 200 consecutive configure-then-delete cycles against the gNMI
    server, verifying 0 residue leaves remain and repeated deletes are idempotent.

All unit tests in gnmi pass cleanly.
@kraney
kraney requested a review from a team as a code owner September 23, 2026 21:25
@kraney
kraney requested a review from bstoll September 30, 2026 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant