Skip to content

Commit ac7db73

Browse files
committed
change: fix update_config_cache crash when 'after' entry is missing
When sysrepo sends a batch of changes where a ChangeDeleted removes a list entry and a subsequent ChangeCreated references that same entry in its 'after' parameter (to specify insertion order), xpath_set() raises a ValueError because the referenced entry no longer exists in the cache. This happens in practice when list entries are rapidly created and deleted (e.g. coredump files appearing during repeated service crashes). The ValueError propagates up and kills the sysrepo subscription callback, which in the case of the alarm manager means no further alarms are processed for the rest of the session. Fix this by catching the ValueError and retrying without the 'after' parameter, which appends the new entry at the end of the list. The entry is still created, only the insertion order is lost, which is an acceptable trade-off compared to crashing the entire callback. The fix is done in sysrepo-python (the caller that knows the context) rather than in libyang-python (which is correct to be strict about invalid 'after' references). Add a dedicated unit test for update_config_cache covering both the normal case (after entry exists) and the fallback case (after entry was deleted). Acked-by: Jean-Sébastien Bevilacqua <jean-sebastien.bevilacqua@6wind.com>
1 parent c5fbdaf commit ac7db73

2 files changed

Lines changed: 62 additions & 1 deletion

File tree

sysrepo/change.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,12 @@ def update_config_cache(conf: Dict, changes: List[Change]) -> None:
206206
"""
207207
for c in changes:
208208
if isinstance(c, ChangeCreated):
209-
libyang.xpath_set(conf, c.xpath, c.value, after=c.after)
209+
try:
210+
libyang.xpath_set(conf, c.xpath, c.value, after=c.after)
211+
except ValueError:
212+
# The 'after' entry may have been deleted by a previous change
213+
# in the same batch. Fall back to appending at the end.
214+
libyang.xpath_set(conf, c.xpath, c.value)
210215
elif isinstance(c, ChangeModified):
211216
libyang.xpath_set(conf, c.xpath, c.value)
212217
elif isinstance(c, ChangeMoved):

tests/test_update_config_cache.py

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
# Copyright (c) 2026 6WIND S.A.
2+
# SPDX-License-Identifier: BSD-3-Clause
3+
4+
import unittest
5+
6+
from sysrepo.change import ChangeCreated, ChangeDeleted, update_config_cache
7+
8+
9+
class UpdateConfigCacheTest(unittest.TestCase):
10+
def test_created_after_missing_entry(self):
11+
"""
12+
When a ChangeCreated references an 'after' entry that was deleted by a
13+
previous ChangeDeleted in the same batch, the entry should still be
14+
created (appended at the end) instead of raising ValueError.
15+
"""
16+
conf = {
17+
"coredump": [
18+
{"name": "core.A"},
19+
{"name": "core.B"},
20+
],
21+
}
22+
changes = [
23+
ChangeDeleted("/coredump[name='core.A']", {"name": "core.A"}),
24+
ChangeCreated(
25+
"/coredump[name='core.C']",
26+
{"name": "core.C"},
27+
after="[name='core.A']",
28+
),
29+
]
30+
update_config_cache(conf, changes)
31+
# core.A deleted, core.C added (appended since 'after' ref is gone)
32+
self.assertEqual(
33+
conf["coredump"],
34+
[{"name": "core.B"}, {"name": "core.C"}],
35+
)
36+
37+
def test_created_after_existing_entry(self):
38+
"""Normal case: 'after' entry exists, insertion at correct position."""
39+
conf = {
40+
"coredump": [
41+
{"name": "core.A"},
42+
{"name": "core.B"},
43+
],
44+
}
45+
changes = [
46+
ChangeCreated(
47+
"/coredump[name='core.C']",
48+
{"name": "core.C"},
49+
after="[name='core.A']",
50+
),
51+
]
52+
update_config_cache(conf, changes)
53+
self.assertEqual(
54+
conf["coredump"],
55+
[{"name": "core.A"}, {"name": "core.C"}, {"name": "core.B"}],
56+
)

0 commit comments

Comments
 (0)