From 7e131a0076de6bc69ed0ebc54c5903604ed940fa Mon Sep 17 00:00:00 2001 From: Or Mergi Date: Mon, 8 Jan 2024 19:17:01 +0200 Subject: [PATCH] bridge: Enable disabling bridge interface The new `disableContainerInterface` parameter is added to the bridge plugin to enable setting the container interface state down. When the parameter is enabled, the container interface (veth peer that is placed at the container ns) remain down (i.e: disabled). The bridge and host peer interfaces state are not affected by the parameter. Since IPAM logic involve various configurations including waiting for addresses to be realized and setting the interface state UP, the new parameter cannot work with IPAM. In case both IPAM and DisableContainerInterface parameters are set, the bridge plugin will raise an error. Signed-off-by: Or Mergi --- plugins/main/bridge/bridge.go | 67 ++++++++++++++++------------ plugins/main/bridge/bridge_test.go | 71 +++++++++++++++++++++++++++--- 2 files changed, 104 insertions(+), 34 deletions(-) diff --git a/plugins/main/bridge/bridge.go b/plugins/main/bridge/bridge.go index a24d2d4a..ad5f1963 100644 --- a/plugins/main/bridge/bridge.go +++ b/plugins/main/bridge/bridge.go @@ -47,19 +47,20 @@ const defaultBrName = "cni0" type NetConf struct { types.NetConf - BrName string `json:"bridge"` - IsGW bool `json:"isGateway"` - IsDefaultGW bool `json:"isDefaultGateway"` - ForceAddress bool `json:"forceAddress"` - IPMasq bool `json:"ipMasq"` - MTU int `json:"mtu"` - HairpinMode bool `json:"hairpinMode"` - PromiscMode bool `json:"promiscMode"` - Vlan int `json:"vlan"` - VlanTrunk []*VlanTrunk `json:"vlanTrunk,omitempty"` - PreserveDefaultVlan bool `json:"preserveDefaultVlan"` - MacSpoofChk bool `json:"macspoofchk,omitempty"` - EnableDad bool `json:"enabledad,omitempty"` + BrName string `json:"bridge"` + IsGW bool `json:"isGateway"` + IsDefaultGW bool `json:"isDefaultGateway"` + ForceAddress bool `json:"forceAddress"` + IPMasq bool `json:"ipMasq"` + MTU int `json:"mtu"` + HairpinMode bool `json:"hairpinMode"` + PromiscMode bool `json:"promiscMode"` + Vlan int `json:"vlan"` + VlanTrunk []*VlanTrunk `json:"vlanTrunk,omitempty"` + PreserveDefaultVlan bool `json:"preserveDefaultVlan"` + MacSpoofChk bool `json:"macspoofchk,omitempty"` + EnableDad bool `json:"enabledad,omitempty"` + DisableContainerInterface bool `json:"disableContainerInterface,omitempty"` Args struct { Cni BridgeArgs `json:"cni,omitempty"` @@ -530,6 +531,10 @@ func cmdAdd(args *skel.CmdArgs) error { isLayer3 := n.IPAM.Type != "" + if isLayer3 && n.DisableContainerInterface { + return fmt.Errorf("cannot use IPAM when DisableContainerInterface flag is set") + } + if n.IsDefaultGW { n.IsGW = true } @@ -676,12 +681,13 @@ func cmdAdd(args *skel.CmdArgs) error { } } } - } else { + } else if !n.DisableContainerInterface { if err := netns.Do(func(_ ns.NetNS) error { link, err := netlink.LinkByName(args.IfName) if err != nil { return fmt.Errorf("failed to retrieve link: %v", err) } + // If layer 2 we still need to set the container veth to up if err = netlink.LinkSetUp(link); err != nil { return fmt.Errorf("failed to set %q up: %v", args.IfName, err) @@ -692,23 +698,28 @@ func cmdAdd(args *skel.CmdArgs) error { } } - var hostVeth netlink.Link + hostVeth, err := netlink.LinkByName(hostInterface.Name) + if err != nil { + return err + } - // check bridge port state - retries := []int{0, 50, 500, 1000, 1000} - for idx, sleep := range retries { - time.Sleep(time.Duration(sleep) * time.Millisecond) + if !n.DisableContainerInterface { + // check bridge port state + retries := []int{0, 50, 500, 1000, 1000} + for idx, sleep := range retries { + time.Sleep(time.Duration(sleep) * time.Millisecond) - hostVeth, err = netlink.LinkByName(hostInterface.Name) - if err != nil { - return err - } - if hostVeth.Attrs().OperState == netlink.OperUp { - break - } + hostVeth, err = netlink.LinkByName(hostInterface.Name) + if err != nil { + return err + } + if hostVeth.Attrs().OperState == netlink.OperUp { + break + } - if idx == len(retries)-1 { - return fmt.Errorf("bridge port in error state: %s", hostVeth.Attrs().OperState) + if idx == len(retries)-1 { + return fmt.Errorf("bridge port in error state: %s", hostVeth.Attrs().OperState) + } } } diff --git a/plugins/main/bridge/bridge_test.go b/plugins/main/bridge/bridge_test.go index 74b82ce3..42c70ae2 100644 --- a/plugins/main/bridge/bridge_test.go +++ b/plugins/main/bridge/bridge_test.go @@ -78,10 +78,12 @@ type testCase struct { removeDefaultVlan bool ipMasq bool macspoofchk bool - AddErr020 string - DelErr020 string - AddErr010 string - DelErr010 string + disableContIface bool + + AddErr020 string + DelErr020 string + AddErr010 string + DelErr010 string envArgs string // CNI_ARGS runtimeConfig struct { @@ -154,6 +156,9 @@ const ( netDefault = `, "isDefaultGateway": true` + disableContainerInterface = `, + "disableContainerInterface": true` + ipamStartStr = `, "ipam": { "type": "host-local"` @@ -248,6 +253,10 @@ func (tc testCase) netConfJSON(dataDir string) string { conf += fmt.Sprintf(macspoofchkFormat, tc.macspoofchk) } + if tc.disableContIface { + conf += disableContainerInterface + } + if !tc.isLayer2 { conf += netDefault if tc.subnet != "" || tc.ranges != nil { @@ -677,14 +686,16 @@ func (tester *testerV10x) cmdAddTest(tc testCase, dataDir string) (types.Result, Expect(err).NotTo(HaveOccurred()) Expect(link.Attrs().Name).To(Equal(IFNAME)) Expect(link).To(BeAssignableToTypeOf(&netlink.Veth{})) + assertContainerInterfaceLinkState(&tc, link) expCIDRsV4, expCIDRsV6 := tc.expectedCIDRs() addrs, err := netlink.AddrList(link, netlink.FAMILY_V4) Expect(err).NotTo(HaveOccurred()) Expect(addrs).To(HaveLen(len(expCIDRsV4))) addrs, err = netlink.AddrList(link, netlink.FAMILY_V6) - Expect(addrs).To(HaveLen(len(expCIDRsV6) + 1)) // add one for the link-local Expect(err).NotTo(HaveOccurred()) + assertIPv6Addresses(&tc, addrs, expCIDRsV6) + // Ignore link local address which may or may not be // ready when we read addresses. var foundAddrs int @@ -728,6 +739,15 @@ func (tester *testerV10x) cmdAddTest(tc testCase, dataDir string) (types.Result, return result, nil } +func assertContainerInterfaceLinkState(tc *testCase, link netlink.Link) { + linkState := int(link.Attrs().OperState) + if tc.disableContIface { + Expect(linkState).ToNot(Equal(netlink.OperUp)) + } else { + Expect(linkState).To(Equal(netlink.OperUp)) + } +} + func (tester *testerV10x) cmdCheckTest(tc testCase, conf *Net, _ string) { // Generate network config and command arguments tester.args = tc.createCheckCmdArgs(tester.targetNS, conf) @@ -1008,8 +1028,9 @@ func (tester *testerV04x) cmdAddTest(tc testCase, dataDir string) (types.Result, Expect(err).NotTo(HaveOccurred()) Expect(addrs).To(HaveLen(len(expCIDRsV4))) addrs, err = netlink.AddrList(link, netlink.FAMILY_V6) - Expect(addrs).To(HaveLen(len(expCIDRsV6) + 1)) // add one for the link-local Expect(err).NotTo(HaveOccurred()) + assertIPv6Addresses(&tc, addrs, expCIDRsV6) + // Ignore link local address which may or may not be // ready when we read addresses. var foundAddrs int @@ -1053,6 +1074,14 @@ func (tester *testerV04x) cmdAddTest(tc testCase, dataDir string) (types.Result, return result, nil } +func assertIPv6Addresses(tc *testCase, addrs []netlink.Addr, expCIDRsV6 []*net.IPNet) { + if tc.disableContIface { + Expect(addrs).To(BeEmpty()) + } else { + Expect(addrs).To(HaveLen(len(expCIDRsV6) + 1)) // add one for the link-local + } +} + func (tester *testerV04x) cmdCheckTest(tc testCase, conf *Net, _ string) { // Generate network config and command arguments tester.args = tc.createCheckCmdArgs(tester.targetNS, conf) @@ -2461,6 +2490,36 @@ var _ = Describe("bridge Operations", func() { return nil })).To(Succeed()) }) + + It(fmt.Sprintf("[%s] should fail when both IPAM and DisableContainerInterface are set", ver), func() { + Expect(originalNS.Do(func(ns.NetNS) error { + defer GinkgoRecover() + tc := testCase{ + cniVersion: ver, + subnet: "10.1.2.0/24", + disableContIface: true, + } + args := tc.createCmdArgs(targetNS, dataDir) + Expect(cmdAdd(args)).To(HaveOccurred()) + + return nil + })).To(Succeed()) + }) + + It(fmt.Sprintf("[%s] should set the container veth peer state down", ver), func() { + Expect(originalNS.Do(func(ns.NetNS) error { + defer GinkgoRecover() + tc := testCase{ + cniVersion: ver, + disableContIface: true, + isLayer2: true, + AddErr020: "cannot convert: no valid IP addresses", + AddErr010: "cannot convert: no valid IP addresses", + } + cmdAddDelTest(originalNS, targetNS, tc, dataDir) + return nil + })).To(Succeed()) + }) } It("check vlan id when loading net conf", func() {