From 20350fa52c10a09dbb1910b7f1b52803b491d18e Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Fri, 26 Jun 2026 16:00:36 +0200 Subject: [PATCH 1/3] test/infamy: new method, delete_xpaths() Signed-off-by: Joachim Wiberg --- test/infamy/netconf.py | 44 ++++++++++++++++++++++++++++++++++++++++ test/infamy/restconf.py | 20 ++++++++++++++++++ test/infamy/transport.py | 4 ++++ 3 files changed, 68 insertions(+) diff --git a/test/infamy/netconf.py b/test/infamy/netconf.py index dbcf2282a..690f79b63 100644 --- a/test/infamy/netconf.py +++ b/test/infamy/netconf.py @@ -455,3 +455,47 @@ def delete_xpath(self, xpath): # Apply the configuration change return self.put_config(lyd.print_mem("xml", with_siblings=True, pretty=False)) + + def delete_xpaths(self, xpaths): + """Delete several xpaths in a single transaction. + + Needed when the targets reference one another, e.g. both ends of a + VETH pair (mutually mandatory leafrefs), and so cannot be removed + one at a time. + """ + pattern = r"^/(?P[^:]+):(?P[^/]+)" + + # Group xpaths by their top-level module/container so each is + # removed from a single fetched config tree (one diff per module). + groups = {} + order = [] + for xpath in xpaths: + coverage.track_xpath(xpath) + match = re.search(pattern, xpath) + if not match: + raise ValueError(f"Failed parsing xpath:{xpath}") + modpath = f"/{match.group('module')}:{match.group('path')}" + if modpath not in groups: + groups[modpath] = (match.group('module'), []) + order.append(modpath) + groups[modpath][1].append(xpath) + + edit = "" + for modpath in order: + module, paths = groups[modpath] + old = self.get_config_dict(modpath) + new = copy.deepcopy(old) + for xpath in paths: + if not libyang.xpath_del(new, xpath): + raise ValueError(f"Failed to delete specified xpath: {xpath}") + + mod = self.ly.get_module(module) + oldd = mod.parse_data_dict(old, no_state=True, validate=False) + newd = mod.parse_data_dict(new, no_state=True, validate=False) + + lyd = oldd.diff(newd) + if lyd is None: + raise ValueError(f"Failed generating diff for {modpath}") + edit += lyd.print_mem("xml", with_siblings=True, pretty=False) + + return self.put_config(edit) diff --git a/test/infamy/restconf.py b/test/infamy/restconf.py index 3cb0a5bed..ead930024 100644 --- a/test/infamy/restconf.py +++ b/test/infamy/restconf.py @@ -502,3 +502,23 @@ def delete_xpath(self, xpath): response.raise_for_status() return True + + def delete_xpaths(self, xpaths): + """Delete several xpaths in a single transaction. + + Needed when the targets reference one another, e.g. both ends of a + VETH pair (mutually mandatory leafrefs), and so cannot be removed + one at a time. Stage the deletions in the candidate datastore, then + copy it to running so they commit and validate together. + """ + self.copy("running", "candidate") + for xpath in xpaths: + coverage.track_xpath(xpath) + path = f"/ds/ietf-datastores:candidate{xpath_to_uri(xpath)}" + url = f"{self.restconf_url}{path}" + response = requests_workaround_delete(url, headers=self.headers, + auth=self.auth, verify=False) + response.raise_for_status() + self.copy("candidate", "running") + + return True diff --git a/test/infamy/transport.py b/test/infamy/transport.py index 2f6cc1848..17a9ab322 100644 --- a/test/infamy/transport.py +++ b/test/infamy/transport.py @@ -34,6 +34,10 @@ def get_dict(self, xpath=None): def delete_xpath(self, xpath): pass + @abstractmethod + def delete_xpaths(self, xpaths): + pass + @abstractmethod def copy(self, source, target): pass From 2ae1a70bacd278b317ff12b94c65d0d67bccfb55 Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Fri, 26 Jun 2026 16:01:05 +0200 Subject: [PATCH 2/3] test: simplify interfaces/veth_delete Signed-off-by: Joachim Wiberg --- test/case/interfaces/veth_delete/test.adoc | 5 ++--- test/case/interfaces/veth_delete/test.py | 26 ++++++++-------------- 2 files changed, 11 insertions(+), 20 deletions(-) diff --git a/test/case/interfaces/veth_delete/test.adoc b/test/case/interfaces/veth_delete/test.adoc index 372faa71a..aed8c5243 100644 --- a/test/case/interfaces/veth_delete/test.adoc +++ b/test/case/interfaces/veth_delete/test.adoc @@ -23,8 +23,7 @@ image::topology.svg[Verify that VETH Pairs Can Be Deleted topology, align=center . Verify interfaces 'veth0a' and 'veth0b' exist . Set IP address on target:data1 (dummy op) . Set IP address on target:data2 (dummy op) -. Reset configuration -. Verify target:data1 and target:data2 still exist -. Verify VETH pair have been removed +. Delete VETH pair +. Verify VETH pair has been removed diff --git a/test/case/interfaces/veth_delete/test.py b/test/case/interfaces/veth_delete/test.py index 67091d984..8bc1bd65b 100755 --- a/test/case/interfaces/veth_delete/test.py +++ b/test/case/interfaces/veth_delete/test.py @@ -86,23 +86,15 @@ } }}) - # TODO: need target.del_config_dict() or similar for VETH _pairs_, - # because both interfaces must be removed at the same time. - # with test.step("Delete VETH pair"): - # xpath = f"/ietf-interfaces:interfaces/interface[name='{veth0a}']" - # target.delete_xpath(xpath) - # XXX: temporary workaround - with test.step("Reset configuration"): - # Calls target.test_reset() to apply safe-config - target = env.attach("target", "mgmt") - - with test.step("Verify target:data1 and target:data2 still exist"): - assert iface.exist(target, data1), \ - f"Interface {data1} missing!" - assert iface.exist(target, data2), \ - f"Interface {data2} missing!" - - with test.step("Verify VETH pair have been removed"): + with test.step("Delete VETH pair"): + # Both ends are mutually mandatory leafrefs, so they must be + # removed in a single transaction. + target.delete_xpaths([ + f"/ietf-interfaces:interfaces/interface[name='{veth0a}']", + f"/ietf-interfaces:interfaces/interface[name='{veth0b}']", + ]) + + with test.step("Verify VETH pair has been removed"): assert not iface.exist(target, veth0a), \ f"Interface <{veth0a}> still exists!" assert not iface.exist(target, veth0b), \ From ddc57ed6ae6acb9afc6aabb4a430fdaf9db655d1 Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Fri, 26 Jun 2026 12:43:19 +0200 Subject: [PATCH 3/3] confd: fix veth teardown regression When a veth pair has one end in a container and the other in the host namespace, removing it failed: the container end's teardown deleted the whole pair, so the host end's own delete then failed ("Cannot find device") and aborted the teardown, leaving the interface behind. The host-namespace delete added for the both-ends-in-container case was emitted for every container veth end. Restrict it to the primary end, which for a host/container pair is the host end that already deletes the pair itself. Extend the container veth tests to remove the pair and verify it is gone, covering both the single-end (regression) and both-ends teardown paths. Fixes: #1546 Signed-off-by: Joachim Wiberg --- src/confd/src/cni.c | 14 ++++++++------ test/case/containers/internal_link/test.adoc | 1 + test/case/containers/internal_link/test.py | 17 +++++++++++++++++ test/case/containers/veth/test.adoc | 1 + test/case/containers/veth/test.py | 17 +++++++++++++++++ 5 files changed, 44 insertions(+), 6 deletions(-) diff --git a/src/confd/src/cni.c b/src/confd/src/cni.c index 9f7980aa4..a7ebfb703 100644 --- a/src/confd/src/cni.c +++ b/src/confd/src/cni.c @@ -415,13 +415,15 @@ int cni_netdag_gen_iface(struct dagger *net, const char *ifname, fprintf(fp, "container -a -f delete network %s >/dev/null\n", ifname); - /* If this end belongs to a veth pair, the kernel keeps the pair - * alive after CNI host-device returns the interface to the host - * namespace. Remove it here, once the container is gone, so the - * pair does not linger and block a later re-creation. Tolerant: - * the peer's teardown may already have removed it. + /* When both ends of a veth pair are in containers, neither end + * is torn down from the host namespace, so the kernel keeps the + * pair alive after CNI host-device returns the interfaces here. + * Have the primary end delete it, once its container is gone. + * When the peer is a host interface it is the primary and deletes + * the pair itself (veth_gen_del); we must not race it here. + * Tolerant: the pair may already be gone. */ - if (lydx_get_child(dif, "veth")) + if (lydx_get_child(dif, "veth") && veth_is_primary(dif)) fprintf(fp, "ip link del dev %s 2>/dev/null || true\n", ifname); fclose(fp); diff --git a/test/case/containers/internal_link/test.adoc b/test/case/containers/internal_link/test.adoc index f52c311a3..d1ce703e8 100644 --- a/test/case/containers/internal_link/test.adoc +++ b/test/case/containers/internal_link/test.adoc @@ -29,5 +29,6 @@ image::topology.svg[VETH Pair Between Two Containers topology, align=center, sca . Create VETH pair with both ends assigned to containers . Verify both containers have started . Verify {LEFT} reaches {RIGHT} over the internal VETH pair +. Remove containers and VETH pair, verify clean teardown diff --git a/test/case/containers/internal_link/test.py b/test/case/containers/internal_link/test.py index 26c342ebf..24ea7e1f1 100755 --- a/test/case/containers/internal_link/test.py +++ b/test/case/containers/internal_link/test.py @@ -98,4 +98,21 @@ def reachable(): until(reachable, attempts=30) + with test.step("Remove containers and VETH pair, verify clean teardown"): + # Removing a pair with both ends in containers must tear it down + # cleanly, leaving nothing behind in the host namespace. + target.delete_xpaths([ + f"/infix-containers:containers/container[name='{LEFT}']", + f"/infix-containers:containers/container[name='{RIGHT}']", + f"/ietf-interfaces:interfaces/interface[name='{IFACE_LEFT}']", + f"/ietf-interfaces:interfaces/interface[name='{IFACE_RIGHT}']", + ]) + + def removed(): + ifaces = target.get_data("/ietf-interfaces:interfaces")["interfaces"]["interface"] + names = [iface["name"] for iface in ifaces] + return IFACE_LEFT not in names and IFACE_RIGHT not in names + + until(removed, attempts=30) + test.succeed() diff --git a/test/case/containers/veth/test.adoc b/test/case/containers/veth/test.adoc index 7adb242e5..8ddfdddff 100644 --- a/test/case/containers/veth/test.adoc +++ b/test/case/containers/veth/test.adoc @@ -28,5 +28,6 @@ image::topology.svg[Container with VETH Pair topology, align=center, scaledwidth . Verify container 'web-br0-veth' has started . Verify basic DUT connectivity, host:data can ping DUT 10.0.0.2 . Verify container 'web-br0-veth' is reachable on http://10.0.0.2:91 +. Remove container and VETH pair, verify clean teardown diff --git a/test/case/containers/veth/test.py b/test/case/containers/veth/test.py index 8d50ec9fc..cf85076ef 100755 --- a/test/case/containers/veth/test.py +++ b/test/case/containers/veth/test.py @@ -108,4 +108,21 @@ with test.step("Verify container 'web-br0-veth' is reachable on http://10.0.0.2:91"): until(lambda: MESG in ns.call(lambda: curl(URL)), attempts=10) + with test.step("Remove container and VETH pair, verify clean teardown"): + # Regression test for #941/#1546: removing a veth pair with one end + # in a container must tear the pair down cleanly, leaving nothing + # behind in the host namespace. + target.delete_xpaths([ + f"/infix-containers:containers/container[name='{NAME}']", + f"/ietf-interfaces:interfaces/interface[name='{NAME}']", + "/ietf-interfaces:interfaces/interface[name='veth0b']", + ]) + + def removed(): + ifaces = target.get_data("/ietf-interfaces:interfaces")["interfaces"]["interface"] + names = [iface["name"] for iface in ifaces] + return NAME not in names and "veth0b" not in names + + until(removed, attempts=30) + test.succeed()