diff --git a/plugins/meta/vrf/main.go b/plugins/meta/vrf/main.go index fc1f61203..93d536820 100644 --- a/plugins/meta/vrf/main.go +++ b/plugins/meta/vrf/main.go @@ -99,10 +99,9 @@ func cmdDel(args *skel.CmdArgs) error { } err = ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error { vrf, err := findVRF(conf.VRFName) - if _, ok := err.(netlink.LinkNotFoundError); ok { + if linkNotFound(err) { return nil } - if err != nil { return err } @@ -119,8 +118,11 @@ func cmdDel(args *skel.CmdArgs) error { // Meaning, we are deleting the last interface assigned to the VRF if len(interfaces) == 0 { - err = netlink.LinkDel(vrf) - if err != nil { + // CNI DEL must be idempotent: a concurrent netns teardown can remove + // the VRF between findVRF and here, so LinkDel returns ENODEV. Treat + // "already gone" as success (LinkDel returns a bare errno, not the + // typed netlink.LinkNotFoundError). + if err = netlink.LinkDel(vrf); err != nil && !linkNotFound(err) { return err } } diff --git a/plugins/meta/vrf/vrf.go b/plugins/meta/vrf/vrf.go index 5e45f5988..793580e6d 100644 --- a/plugins/meta/vrf/vrf.go +++ b/plugins/meta/vrf/vrf.go @@ -15,9 +15,11 @@ package main import ( + "errors" "fmt" "math" "net" + "syscall" "time" "github.com/vishvananda/netlink" @@ -214,16 +216,32 @@ func findFreeRoutingTableID(links []netlink.Link) (uint32, error) { func resetMaster(interfaceName string) error { intf, err := netlinksafe.LinkByName(interfaceName) - if err != nil { - return fmt.Errorf("resetMaster: could not get link by name %s", interfaceName) + if linkNotFound(err) { + // Interface already gone (e.g. concurrent netns teardown). DEL is + // best-effort, so there is nothing left to reset. + return nil } - err = netlink.LinkSetNoMaster(intf) if err != nil { - return fmt.Errorf("resetMaster: could reset master to %s", interfaceName) + return fmt.Errorf("resetMaster: could not get link by name %s: %w", interfaceName, err) + } + if err := netlink.LinkSetNoMaster(intf); err != nil { + return fmt.Errorf("resetMaster: could not reset master of %s: %w", interfaceName, err) } return nil } +// linkNotFound reports whether err indicates the link is already gone. netlink +// returns a typed LinkNotFoundError from LinkByName, but a bare errno (ENODEV) +// from LinkDel, so both must be matched. Per the CNI spec, DEL is best-effort +// and must not fail when a resource has already been removed. +func linkNotFound(err error) bool { + if err == nil { + return false + } + var lnf netlink.LinkNotFoundError + return errors.As(err, &lnf) || errors.Is(err, syscall.ENODEV) || errors.Is(err, syscall.ENOENT) +} + // getGlobalAddresses returns the global addresses of the given interface func getGlobalAddresses(link netlink.Link, family int) ([]netlink.Addr, error) { addresses, err := netlinksafe.AddrList(link, family) diff --git a/plugins/meta/vrf/vrf_test.go b/plugins/meta/vrf/vrf_test.go index 4e1212297..f771fcfc8 100644 --- a/plugins/meta/vrf/vrf_test.go +++ b/plugins/meta/vrf/vrf_test.go @@ -821,6 +821,54 @@ var _ = Describe("vrf plugin", func() { }) }) + It("returns success on DEL when the enslaved interface is already gone (idempotent)", func() { + conf0 := configFor("test", IF0Name, VRF0Name, "10.0.0.2/24") + + By("Adding the interface to the VRF", func() { + err := originalNS.Do(func(ns.NetNS) error { + defer GinkgoRecover() + args := &skel.CmdArgs{ + ContainerID: "dummy", + Netns: targetNS.Path(), + IfName: IF0Name, + StdinData: conf0, + } + _, _, err := testutils.CmdAddWithArgs(args, func() error { + return cmdAdd(args) + }) + Expect(err).NotTo(HaveOccurred()) + return nil + }) + Expect(err).NotTo(HaveOccurred()) + }) + + By("Removing the enslaved interface out-of-band (simulating a teardown race)", func() { + err := targetNS.Do(func(ns.NetNS) error { + defer GinkgoRecover() + link, err := netlinksafe.LinkByName(IF0Name) + Expect(err).NotTo(HaveOccurred()) + return netlink.LinkDel(link) + }) + Expect(err).NotTo(HaveOccurred()) + }) + + By("DEL succeeding even though the interface is gone", func() { + err := originalNS.Do(func(ns.NetNS) error { + defer GinkgoRecover() + args := &skel.CmdArgs{ + ContainerID: "dummy", + Netns: targetNS.Path(), + IfName: IF0Name, + StdinData: conf0, + } + return testutils.CmdDelWithArgs(args, func() error { + return cmdDel(args) + }) + }) + Expect(err).NotTo(HaveOccurred()) + }) + }) + It("configures and deconfigures VRF with CNI 0.4.0 ADD/DEL", func() { conf := []byte(fmt.Sprintf(`{ "name": "test",