From 3b2afc93dcd3c386c875b9a61ee67fb231b26667 Mon Sep 17 00:00:00 2001 From: David Ward Date: Fri, 24 Dec 2021 07:53:44 -0500 Subject: [PATCH 1/2] host-device: Bring interfaces up after moving into container If an interface is not configured with IPAM (because it functions at layer 2), it will not be brought up otherwise. Signed-off-by: David Ward --- plugins/main/host-device/host-device.go | 20 +++++++++++----- plugins/main/host-device/host-device_test.go | 24 +++++++++++++------- 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/plugins/main/host-device/host-device.go b/plugins/main/host-device/host-device.go index eda1412df..6d51204d4 100644 --- a/plugins/main/host-device/host-device.go +++ b/plugins/main/host-device/host-device.go @@ -230,6 +230,10 @@ func moveLinkIn(hostDev netlink.Link, containerNs ns.NetNS, ifName string) (netl if err := netlink.LinkSetName(contDev, ifName); err != nil { return fmt.Errorf("failed to rename device %q to %q: %v", hostDev.Attrs().Name, ifName, err) } + // Bring container device up + if err = netlink.LinkSetUp(contDev); err != nil { + return fmt.Errorf("failed to set %q up: %v", ifName, err) + } // Retrieve link again to get up-to-date name and attributes contDev, err = netlink.LinkByName(ifName) if err != nil { @@ -261,18 +265,22 @@ func moveLinkOut(containerNs ns.NetNS, ifName string) error { return fmt.Errorf("failed to set %q down: %v", ifName, err) } - // Rename device to it's original name - if err = netlink.LinkSetName(dev, dev.Attrs().Alias); err != nil { - return fmt.Errorf("failed to restore %q to original name %q: %v", ifName, dev.Attrs().Alias, err) - } defer func() { + // If moving the device to the host namespace fails, set its name back to ifName so that this + // function can be retried. Also bring the device back up, unless it was already down before. if err != nil { - // if moving device to host namespace fails, we should revert device name - // to ifName to make sure that device can be found in retries _ = netlink.LinkSetName(dev, ifName) + if dev.Attrs().Flags & net.FlagUp == net.FlagUp { + _ = netlink.LinkSetUp(dev) + } } }() + // Rename the device to its original name from the host namespace + if err = netlink.LinkSetName(dev, dev.Attrs().Alias); err != nil { + return fmt.Errorf("failed to restore %q to original name %q: %v", ifName, dev.Attrs().Alias, err) + } + if err = netlink.LinkSetNsFd(dev, int(defaultNs.Fd())); err != nil { return fmt.Errorf("failed to move %q to host netns: %v", dev.Attrs().Alias, err) } diff --git a/plugins/main/host-device/host-device_test.go b/plugins/main/host-device/host-device_test.go index 057a8560f..4bee50ef1 100644 --- a/plugins/main/host-device/host-device_test.go +++ b/plugins/main/host-device/host-device_test.go @@ -381,12 +381,13 @@ var _ = Describe("base functionality", func() { t := newTesterByVersion(ver) t.expectInterfaces(resI, cniName, origLink.Attrs().HardwareAddr.String(), targetNS.Path()) - // assert that dummy0 is now in the target namespace + // assert that dummy0 is now in the target namespace and is up _ = targetNS.Do(func(ns.NetNS) error { defer GinkgoRecover() link, err := netlink.LinkByName(cniName) Expect(err).NotTo(HaveOccurred()) Expect(link.Attrs().HardwareAddr).To(Equal(origLink.Attrs().HardwareAddr)) + Expect(link.Attrs().Flags & net.FlagUp).To(Equal(net.FlagUp)) return nil }) @@ -461,12 +462,13 @@ var _ = Describe("base functionality", func() { t := newTesterByVersion(ver) t.expectInterfaces(resI, cniName, origLink.Attrs().HardwareAddr.String(), targetNS.Path()) - // assert that dummy0 is now in the target namespace + // assert that dummy0 is now in the target namespace and is up _ = targetNS.Do(func(ns.NetNS) error { defer GinkgoRecover() link, err := netlink.LinkByName(cniName) Expect(err).NotTo(HaveOccurred()) Expect(link.Attrs().HardwareAddr).To(Equal(origLink.Attrs().HardwareAddr)) + Expect(link.Attrs().Flags & net.FlagUp).To(Equal(net.FlagUp)) return nil }) @@ -504,12 +506,13 @@ var _ = Describe("base functionality", func() { return nil }) - // assert container interface "eth0" still exists in target namespace + // assert container interface "eth0" still exists in target namespace and is up _ = targetNS.Do(func(ns.NetNS) error { defer GinkgoRecover() link, err := netlink.LinkByName(cniName) Expect(err).NotTo(HaveOccurred()) Expect(link.Attrs().HardwareAddr).To(Equal(origLink.Attrs().HardwareAddr)) + Expect(link.Attrs().Flags & net.FlagUp).To(Equal(net.FlagUp)) return nil }) @@ -654,12 +657,13 @@ var _ = Describe("base functionality", func() { t := newTesterByVersion(ver) t.expectInterfaces(resI, cniName, origLink.Attrs().HardwareAddr.String(), targetNS.Path()) - // assert that dummy0 is now in the target namespace + // assert that dummy0 is now in the target namespace and is up _ = targetNS.Do(func(ns.NetNS) error { defer GinkgoRecover() link, err := netlink.LinkByName(cniName) Expect(err).NotTo(HaveOccurred()) Expect(link.Attrs().HardwareAddr).To(Equal(origLink.Attrs().HardwareAddr)) + Expect(link.Attrs().Flags & net.FlagUp).To(Equal(net.FlagUp)) //get the IP address of the interface in the target namespace addrs, err := netlink.AddrList(link, netlink.FAMILY_V4) @@ -757,12 +761,13 @@ var _ = Describe("base functionality", func() { t := newTesterByVersion(ver) t.expectInterfaces(resI, cniName, origLink.Attrs().HardwareAddr.String(), targetNS.Path()) - // assert that dummy0 is now in the target namespace + // assert that dummy0 is now in the target namespace and is up _ = targetNS.Do(func(ns.NetNS) error { defer GinkgoRecover() link, err := netlink.LinkByName(cniName) Expect(err).NotTo(HaveOccurred()) Expect(link.Attrs().HardwareAddr).To(Equal(origLink.Attrs().HardwareAddr)) + Expect(link.Attrs().Flags & net.FlagUp).To(Equal(net.FlagUp)) return nil }) @@ -949,12 +954,13 @@ var _ = Describe("base functionality", func() { t := newTesterByVersion(ver) t.expectInterfaces(resI, cniName, origLink.Attrs().HardwareAddr.String(), targetNS.Path()) - // assert that dummy0 is now in the target namespace + // assert that dummy0 is now in the target namespace and is up _ = targetNS.Do(func(ns.NetNS) error { defer GinkgoRecover() link, err := netlink.LinkByName(cniName) Expect(err).NotTo(HaveOccurred()) Expect(link.Attrs().HardwareAddr).To(Equal(origLink.Attrs().HardwareAddr)) + Expect(link.Attrs().Flags & net.FlagUp).To(Equal(net.FlagUp)) //get the IP address of the interface in the target namespace addrs, err := netlink.AddrList(link, netlink.FAMILY_V4) @@ -1063,12 +1069,13 @@ var _ = Describe("base functionality", func() { t := newTesterByVersion(ver) t.expectInterfaces(resI, cniName, origLink.Attrs().HardwareAddr.String(), targetNS.Path()) - // assert that dummy0 is now in the target namespace + // assert that dummy0 is now in the target namespace and is up _ = targetNS.Do(func(ns.NetNS) error { defer GinkgoRecover() link, err := netlink.LinkByName(cniName) Expect(err).NotTo(HaveOccurred()) Expect(link.Attrs().HardwareAddr).To(Equal(origLink.Attrs().HardwareAddr)) + Expect(link.Attrs().Flags & net.FlagUp).To(Equal(net.FlagUp)) return nil }) @@ -1106,12 +1113,13 @@ var _ = Describe("base functionality", func() { return nil }) - // assert container interface "eth0" still exists in target namespace + // assert container interface "eth0" still exists in target namespace and is up err = targetNS.Do(func(ns.NetNS) error { defer GinkgoRecover() link, err := netlink.LinkByName(cniName) Expect(err).NotTo(HaveOccurred()) Expect(link.Attrs().HardwareAddr).To(Equal(origLink.Attrs().HardwareAddr)) + Expect(link.Attrs().Flags & net.FlagUp).To(Equal(net.FlagUp)) return nil }) Expect(err).NotTo(HaveOccurred()) From 90e8e1faf929f5566e60ad738ef9ef6e3101cb6b Mon Sep 17 00:00:00 2001 From: Casey Callendrello Date: Wed, 26 Jan 2022 17:57:13 +0100 Subject: [PATCH 2/2] Fix host-device gofmt Signed-off-by: Casey Callendrello --- plugins/main/host-device/host-device.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/main/host-device/host-device.go b/plugins/main/host-device/host-device.go index 6d51204d4..06cdf8986 100644 --- a/plugins/main/host-device/host-device.go +++ b/plugins/main/host-device/host-device.go @@ -270,7 +270,7 @@ func moveLinkOut(containerNs ns.NetNS, ifName string) error { // function can be retried. Also bring the device back up, unless it was already down before. if err != nil { _ = netlink.LinkSetName(dev, ifName) - if dev.Attrs().Flags & net.FlagUp == net.FlagUp { + if dev.Attrs().Flags&net.FlagUp == net.FlagUp { _ = netlink.LinkSetUp(dev) } }