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 <troglobit@gmail.com>

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);

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
 
 

test/case/containers/internal_link/test.py

@@ -98,4 +98,21 @@ with infamy.Test() as test:
 
         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()

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
 
 

test/case/containers/veth/test.py

@@ -108,4 +108,21 @@ with infamy.Test() as test:
         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()