From 817fe423b800e78765415e8eaff1129cef74aaa1 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 17 Mar 2022 14:28:06 +0100 Subject: [PATCH 1/5] libnetwork/cni: allow no ipam plugin Parse no ipam plugin and display it as ipam driver none. Also set the ipam driver field for unsupported plugins. Signed-off-by: Paul Holzinger --- libnetwork/cni/cni_conversion.go | 126 +++++++++++++++---------------- libnetwork/cni/config_test.go | 23 +++++- libnetwork/types/const.go | 4 +- 3 files changed, 87 insertions(+), 66 deletions(-) diff --git a/libnetwork/cni/cni_conversion.go b/libnetwork/cni/cni_conversion.go index 8c4eeff9d..0efc40fd1 100644 --- a/libnetwork/cni/cni_conversion.go +++ b/libnetwork/cni/cni_conversion.go @@ -128,76 +128,76 @@ func findPluginByName(plugins []*libcni.NetworkConfig, name string) bool { // convertIPAMConfToNetwork converts A cni IPAMConfig to libpod network subnets. // It returns an array of subnets and an extra bool if dhcp is configured. func convertIPAMConfToNetwork(network *types.Network, ipam *ipamConfig, confPath string) error { - if ipam.PluginType == types.DHCPIPAMDriver { + switch ipam.PluginType { + case "": + network.IPAMOptions[types.Driver] = types.NoneIPAMDriver + case types.DHCPIPAMDriver: network.IPAMOptions[types.Driver] = types.DHCPIPAMDriver - return nil - } - - if ipam.PluginType != types.HostLocalIPAMDriver { - // This is not an error. While we only support certain ipam drivers, we - // cannot make it fail for unsupported ones. CNI is still able to use them, - // just our translation logic cannot convert this into a Network. - // For the same reason this is not warning, it would just be annoying for - // everyone using a unknown ipam driver. - logrus.Infof("unsupported ipam plugin %q in %s", ipam.PluginType, confPath) - return nil - } - - network.IPAMOptions[types.Driver] = types.HostLocalIPAMDriver - for _, r := range ipam.Ranges { - for _, ipam := range r { - s := types.Subnet{} - - // Do not use types.ParseCIDR() because we want the ip to be - // the network address and not a random ip in the sub. - _, sub, err := net.ParseCIDR(ipam.Subnet) - if err != nil { - return err - } - s.Subnet = types.IPNet{IPNet: *sub} - - // gateway - var gateway net.IP - if ipam.Gateway != "" { - gateway = net.ParseIP(ipam.Gateway) - if gateway == nil { - return errors.Errorf("failed to parse gateway ip %s", ipam.Gateway) - } - // convert to 4 byte if ipv4 - util.NormalizeIP(&gateway) - } else if !network.Internal { - // only add a gateway address if the network is not internal - gateway, err = util.FirstIPInSubnet(sub) + case types.HostLocalIPAMDriver: + network.IPAMOptions[types.Driver] = types.HostLocalIPAMDriver + for _, r := range ipam.Ranges { + for _, ipam := range r { + s := types.Subnet{} + + // Do not use types.ParseCIDR() because we want the ip to be + // the network address and not a random ip in the sub. + _, sub, err := net.ParseCIDR(ipam.Subnet) if err != nil { - return errors.Errorf("failed to get first ip in subnet %s", sub.String()) + return err } - } - s.Gateway = gateway - - var rangeStart net.IP - var rangeEnd net.IP - if ipam.RangeStart != "" { - rangeStart = net.ParseIP(ipam.RangeStart) - if rangeStart == nil { - return errors.Errorf("failed to parse range start ip %s", ipam.RangeStart) + s.Subnet = types.IPNet{IPNet: *sub} + + // gateway + var gateway net.IP + if ipam.Gateway != "" { + gateway = net.ParseIP(ipam.Gateway) + if gateway == nil { + return errors.Errorf("failed to parse gateway ip %s", ipam.Gateway) + } + // convert to 4 byte if ipv4 + util.NormalizeIP(&gateway) + } else if !network.Internal { + // only add a gateway address if the network is not internal + gateway, err = util.FirstIPInSubnet(sub) + if err != nil { + return errors.Errorf("failed to get first ip in subnet %s", sub.String()) + } } - } - if ipam.RangeEnd != "" { - rangeEnd = net.ParseIP(ipam.RangeEnd) - if rangeEnd == nil { - return errors.Errorf("failed to parse range end ip %s", ipam.RangeEnd) + s.Gateway = gateway + + var rangeStart net.IP + var rangeEnd net.IP + if ipam.RangeStart != "" { + rangeStart = net.ParseIP(ipam.RangeStart) + if rangeStart == nil { + return errors.Errorf("failed to parse range start ip %s", ipam.RangeStart) + } } + if ipam.RangeEnd != "" { + rangeEnd = net.ParseIP(ipam.RangeEnd) + if rangeEnd == nil { + return errors.Errorf("failed to parse range end ip %s", ipam.RangeEnd) + } + } + if rangeStart != nil || rangeEnd != nil { + s.LeaseRange = &types.LeaseRange{} + s.LeaseRange.StartIP = rangeStart + s.LeaseRange.EndIP = rangeEnd + } + if util.IsIPv6(s.Subnet.IP) { + network.IPv6Enabled = true + } + network.Subnets = append(network.Subnets, s) } - if rangeStart != nil || rangeEnd != nil { - s.LeaseRange = &types.LeaseRange{} - s.LeaseRange.StartIP = rangeStart - s.LeaseRange.EndIP = rangeEnd - } - if util.IsIPv6(s.Subnet.IP) { - network.IPv6Enabled = true - } - network.Subnets = append(network.Subnets, s) } + default: + // This is not an error. While we only support certain ipam drivers, we + // cannot make it fail for unsupported ones. CNI is still able to use them, + // just our translation logic cannot convert this into a Network. + // For the same reason this is not warning, it would just be annoying for + // everyone using a unknown ipam driver. + logrus.Infof("unsupported ipam plugin %q in %s", ipam.PluginType, confPath) + network.IPAMOptions[types.Driver] = ipam.PluginType } return nil } diff --git a/libnetwork/cni/config_test.go b/libnetwork/cni/config_test.go index 99ef32027..25f965ae8 100644 --- a/libnetwork/cni/config_test.go +++ b/libnetwork/cni/config_test.go @@ -1114,8 +1114,6 @@ var _ = Describe("Config", func() { // check for the unsupported ipam plugin message logString := logBuffer.String() Expect(logString).ToNot(BeEmpty()) - Expect(logString).To(ContainSubstring("unsupported ipam plugin \\\"\\\" in %s", cniConfDir+"/ipam-none.conflist")) - Expect(logString).To(ContainSubstring("unsupported ipam plugin \\\"\\\" in %s", cniConfDir+"/ipam-empty.conflist")) Expect(logString).To(ContainSubstring("unsupported ipam plugin \\\"static\\\" in %s", cniConfDir+"/ipam-static.conflist")) }) @@ -1258,6 +1256,27 @@ var _ = Describe("Config", func() { Expect(network.ID).To(HaveLen(64)) Expect(network.Driver).To(Equal("bridge")) Expect(network.Subnets).To(HaveLen(0)) + Expect(network.IPAMOptions).To(HaveKeyWithValue("driver", "static")) + }) + + It("ipam none network", func() { + network, err := libpodNet.NetworkInspect("ipam-none") + Expect(err).To(BeNil()) + Expect(network.Name).To(Equal("ipam-none")) + Expect(network.ID).To(HaveLen(64)) + Expect(network.Driver).To(Equal("bridge")) + Expect(network.Subnets).To(HaveLen(0)) + Expect(network.IPAMOptions).To(HaveKeyWithValue("driver", "none")) + }) + + It("ipam empty network", func() { + network, err := libpodNet.NetworkInspect("ipam-empty") + Expect(err).To(BeNil()) + Expect(network.Name).To(Equal("ipam-empty")) + Expect(network.ID).To(HaveLen(64)) + Expect(network.Driver).To(Equal("bridge")) + Expect(network.Subnets).To(HaveLen(0)) + Expect(network.IPAMOptions).To(HaveKeyWithValue("driver", "none")) }) It("network list with filters (name)", func() { diff --git a/libnetwork/types/const.go b/libnetwork/types/const.go index 5690a6058..a1e4d71ed 100644 --- a/libnetwork/types/const.go +++ b/libnetwork/types/const.go @@ -12,10 +12,12 @@ const ( // IPAM drivers Driver = "driver" - // HostLocalIPAMDriver store the ip + // HostLocalIPAMDriver store the ip locally in a db HostLocalIPAMDriver = "host-local" // DHCPIPAMDriver get subnet and ip from dhcp server DHCPIPAMDriver = "dhcp" + // NoneIPAMDriver do not provide ipam management + NoneIPAMDriver = "none" // DefaultSubnet is the name that will be used for the default CNI network. DefaultNetworkName = "podman" From a953940e7820d96012fb17b44bde59ce2e70012d Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 17 Mar 2022 16:39:47 +0100 Subject: [PATCH 2/5] libnetwork/cni: allow network create with no ipam driver Network create now uses the ipam driver. This allows the user to configure the ipam driver manually instead of choosing a fixed default. If the ipam driver is `none` no cni ipam plugin will be added to teh config. This means that the interfaces are created but no extra ip addresses are assigned. Signed-off-by: Paul Holzinger --- libnetwork/cni/cni_conversion.go | 25 +++++--- libnetwork/cni/cni_types.go | 14 +++-- libnetwork/cni/config.go | 42 ++++++++++++-- libnetwork/cni/config_test.go | 92 ++++++++++++++++++++++++++++++ libnetwork/internal/util/bridge.go | 4 +- 5 files changed, 157 insertions(+), 20 deletions(-) diff --git a/libnetwork/cni/cni_conversion.go b/libnetwork/cni/cni_conversion.go index 0efc40fd1..448c1d099 100644 --- a/libnetwork/cni/cni_conversion.go +++ b/libnetwork/cni/cni_conversion.go @@ -225,10 +225,13 @@ func (n *cniNetwork) createCNIConfigListFromNetwork(network *types.Network, writ var ( routes []ipamRoute ipamRanges [][]ipamLocalHostRangeConf - ipamConf ipamConfig + ipamConf *ipamConfig err error ) - if len(network.Subnets) > 0 { + + ipamDriver := network.IPAMOptions[types.Driver] + switch ipamDriver { + case types.HostLocalIPAMDriver: defIpv4Route := false defIpv6Route := false for _, subnet := range network.Subnets { @@ -257,9 +260,15 @@ func (n *cniNetwork) createCNIConfigListFromNetwork(network *types.Network, writ routes = append(routes, route) } } - ipamConf = newIPAMHostLocalConf(routes, ipamRanges) - } else { - ipamConf = ipamConfig{PluginType: "dhcp"} + conf := newIPAMHostLocalConf(routes, ipamRanges) + ipamConf = &conf + case types.DHCPIPAMDriver: + ipamConf = &ipamConfig{PluginType: "dhcp"} + + case types.NoneIPAMDriver: + // do nothing + default: + return nil, "", errors.Errorf("unsupported ipam driver %q", ipamDriver) } vlan := 0 @@ -314,7 +323,7 @@ func (n *cniNetwork) createCNIConfigListFromNetwork(network *types.Network, writ switch network.Driver { case types.BridgeNetworkDriver: - bridge := newHostLocalBridge(network.NetworkInterface, isGateway, ipMasq, mtu, vlan, &ipamConf) + bridge := newHostLocalBridge(network.NetworkInterface, isGateway, ipMasq, mtu, vlan, ipamConf) plugins = append(plugins, bridge, newPortMapPlugin(), newFirewallPlugin(), newTuningPlugin()) // if we find the dnsname plugin we add configuration for it if hasDNSNamePlugin(n.cniPluginDirs) && network.DNSEnabled { @@ -323,10 +332,10 @@ func (n *cniNetwork) createCNIConfigListFromNetwork(network *types.Network, writ } case types.MacVLANNetworkDriver: - plugins = append(plugins, newVLANPlugin(types.MacVLANNetworkDriver, network.NetworkInterface, vlanPluginMode, mtu, &ipamConf)) + plugins = append(plugins, newVLANPlugin(types.MacVLANNetworkDriver, network.NetworkInterface, vlanPluginMode, mtu, ipamConf)) case types.IPVLANNetworkDriver: - plugins = append(plugins, newVLANPlugin(types.IPVLANNetworkDriver, network.NetworkInterface, vlanPluginMode, mtu, &ipamConf)) + plugins = append(plugins, newVLANPlugin(types.IPVLANNetworkDriver, network.NetworkInterface, vlanPluginMode, mtu, ipamConf)) default: return nil, "", errors.Errorf("driver %q is not supported by cni", network.Driver) diff --git a/libnetwork/cni/cni_types.go b/libnetwork/cni/cni_types.go index 9ee159886..25cc173a6 100644 --- a/libnetwork/cni/cni_types.go +++ b/libnetwork/cni/cni_types.go @@ -145,11 +145,13 @@ func newHostLocalBridge(name string, isGateWay, ipMasq bool, mtu, vlan int, ipam MTU: mtu, HairpinMode: true, Vlan: vlan, - IPAM: *ipamConf, } - // if we use host-local set the ips cap to ensure we can set static ips via runtime config - if ipamConf.PluginType == types.HostLocalIPAMDriver { - bridge.Capabilities = caps + if ipamConf != nil { + bridge.IPAM = *ipamConf + // if we use host-local set the ips cap to ensure we can set static ips via runtime config + if ipamConf.PluginType == types.HostLocalIPAMDriver { + bridge.Capabilities = caps + } } return &bridge } @@ -259,7 +261,9 @@ func hasDNSNamePlugin(paths []string) bool { func newVLANPlugin(pluginType, device, mode string, mtu int, ipam *ipamConfig) VLANConfig { m := VLANConfig{ PluginType: pluginType, - IPAM: *ipam, + } + if ipam != nil { + m.IPAM = *ipam } if mtu > 0 { m.MTU = mtu diff --git a/libnetwork/cni/config.go b/libnetwork/cni/config.go index 8b300a03b..7b9fd0ded 100644 --- a/libnetwork/cni/config.go +++ b/libnetwork/cni/config.go @@ -53,6 +53,11 @@ func (n *cniNetwork) networkCreate(newNetwork *types.Network, defaultNet bool) ( return nil, err } + err = validateIPAMDriver(newNetwork) + if err != nil { + return nil, err + } + // Only get the used networks for validation if we do not create the default network. // The default network should not be validated against used subnets, we have to ensure // that this network can always be created even when a subnet is already used on the host. @@ -197,13 +202,38 @@ func createIPMACVLAN(network *types.Network) error { return errors.Errorf("parent interface %s does not exist", network.NetworkInterface) } } - if len(network.Subnets) == 0 { - network.IPAMOptions[types.Driver] = types.DHCPIPAMDriver - if network.Internal { - return errors.New("internal is not supported with macvlan and dhcp ipam driver") + + switch network.IPAMOptions[types.Driver] { + // set default + case "": + if len(network.Subnets) == 0 { + // if no subnets and no driver choose dhcp + network.IPAMOptions[types.Driver] = types.DHCPIPAMDriver + } else { + network.IPAMOptions[types.Driver] = types.HostLocalIPAMDriver + } + case types.HostLocalIPAMDriver: + if len(network.Subnets) == 0 { + return errors.New("host-local ipam driver set but no subnets are given") + } + } + + if network.IPAMOptions[types.Driver] == types.DHCPIPAMDriver && network.Internal { + return errors.New("internal is not supported with macvlan and dhcp ipam driver") + } + return nil +} + +func validateIPAMDriver(n *types.Network) error { + ipamDriver := n.IPAMOptions[types.Driver] + switch ipamDriver { + case "", types.HostLocalIPAMDriver: + case types.DHCPIPAMDriver, types.NoneIPAMDriver: + if len(n.Subnets) > 0 { + return errors.Errorf("%s ipam driver is set but subnets are given", ipamDriver) } - } else { - network.IPAMOptions[types.Driver] = types.HostLocalIPAMDriver + default: + return errors.Errorf("unsupported ipam driver %q", ipamDriver) } return nil } diff --git a/libnetwork/cni/config_test.go b/libnetwork/cni/config_test.go index 25f965ae8..91e927ce8 100644 --- a/libnetwork/cni/config_test.go +++ b/libnetwork/cni/config_test.go @@ -167,6 +167,86 @@ var _ = Describe("Config", func() { Expect(network1.Internal).To(BeFalse()) }) + It("create bridge config with none ipam driver", func() { + network := types.Network{ + Driver: "bridge", + IPAMOptions: map[string]string{ + "driver": "none", + }, + } + network1, err := libpodNet.NetworkCreate(network) + Expect(err).To(BeNil()) + Expect(network1.Driver).To(Equal("bridge")) + Expect(network1.IPAMOptions).ToNot(BeEmpty()) + Expect(network1.IPAMOptions).To(HaveKeyWithValue("driver", "none")) + Expect(network1.Subnets).To(HaveLen(0)) + + // reload configs from disk + libpodNet, err = getNetworkInterface(cniConfDir) + Expect(err).To(BeNil()) + + network2, err := libpodNet.NetworkInspect(network1.Name) + Expect(err).To(BeNil()) + Expect(network2).To(Equal(network1)) + }) + + It("create bridge config with none ipam driver and subnets", func() { + subnet := "10.1.0.0/24" + n, _ := types.ParseCIDR(subnet) + network := types.Network{ + Driver: "bridge", + IPAMOptions: map[string]string{ + "driver": "none", + }, + Subnets: []types.Subnet{ + {Subnet: n}, + }, + } + _, err := libpodNet.NetworkCreate(network) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("none ipam driver is set but subnets are given")) + }) + + It("create bridge config with dhcp ipam driver", func() { + network := types.Network{ + Driver: "bridge", + IPAMOptions: map[string]string{ + "driver": "dhcp", + }, + } + network1, err := libpodNet.NetworkCreate(network) + Expect(err).To(BeNil()) + Expect(network1.Driver).To(Equal("bridge")) + Expect(network1.IPAMOptions).ToNot(BeEmpty()) + Expect(network1.IPAMOptions).To(HaveKeyWithValue("driver", "dhcp")) + Expect(network1.Subnets).To(HaveLen(0)) + + // reload configs from disk + libpodNet, err = getNetworkInterface(cniConfDir) + Expect(err).To(BeNil()) + + network2, err := libpodNet.NetworkInspect(network1.Name) + Expect(err).To(BeNil()) + Expect(network2).To(Equal(network1)) + }) + + It("create bridge config with none hdcp driver and subnets", func() { + subnet := "10.1.0.0/24" + n, _ := types.ParseCIDR(subnet) + network := types.Network{ + Driver: "bridge", + IPAMOptions: map[string]string{ + "driver": "dhcp", + }, + Subnets: []types.Subnet{ + {Subnet: n}, + }, + } + _, err := libpodNet.NetworkCreate(network) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("dhcp ipam driver is set but subnets are given")) + }) + It("create bridge with same name should fail", func() { network := types.Network{ Driver: "bridge", @@ -1070,6 +1150,18 @@ var _ = Describe("Config", func() { Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("subnet 10.10.0.0/24 is already used on the host or by another config")) }) + + It("create network with invalid ipam driver", func() { + network := types.Network{ + Driver: "bridge", + IPAMOptions: map[string]string{ + "driver": "blah", + }, + } + _, err := libpodNet.NetworkCreate(network) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("unsupported ipam driver \"blah\"")) + }) }) Context("network load valid existing ones", func() { diff --git a/libnetwork/internal/util/bridge.go b/libnetwork/internal/util/bridge.go index 5a4752e2b..bfa72808d 100644 --- a/libnetwork/internal/util/bridge.go +++ b/libnetwork/internal/util/bridge.go @@ -27,7 +27,9 @@ func CreateBridge(n NetUtil, network *types.Network, usedNetworks []*net.IPNet, } } - if network.IPAMOptions[types.Driver] != types.DHCPIPAMDriver { + ipamDriver := network.IPAMOptions[types.Driver] + // also do this when the driver is unset + if ipamDriver == "" || ipamDriver == types.HostLocalIPAMDriver { if len(network.Subnets) == 0 { freeSubnet, err := GetFreeIPv4NetworkSubnet(usedNetworks, subnetPools) if err != nil { From 06a6c0035e38c506b59ed0da34b15cb257943f52 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 17 Mar 2022 17:25:17 +0100 Subject: [PATCH 3/5] libnetwork/cni: fix CNIResultToStatus conversion logic When we read the cni result we should loop over the interfaces and then the ips. If we only loop over ips we will miss interfaces that have no ips assigned. We also only care about interfaces created in the netns. This is required for ipam driver none case, see the test case. Signed-off-by: Paul Holzinger --- libnetwork/cni/run.go | 45 ++++++++++---------- libnetwork/cni/run_test.go | 85 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 21 deletions(-) diff --git a/libnetwork/cni/run.go b/libnetwork/cni/run.go index 8bea87893..c7fa86ed0 100644 --- a/libnetwork/cni/run.go +++ b/libnetwork/cni/run.go @@ -125,35 +125,38 @@ func CNIResultToStatus(res cnitypes.Result) (types.StatusBlock, error) { result.DNSSearchDomains = cniResult.DNS.Search interfaces := make(map[string]types.NetInterface) - for _, ip := range cniResult.IPs { - if ip.Interface == nil { - // we do no expect ips without an interface + for intIndex, netInterface := range cniResult.Interfaces { + // we are only interested about interfaces in the container namespace + if netInterface.Sandbox == "" { continue } - if len(cniResult.Interfaces) <= *ip.Interface { - return result, errors.Errorf("invalid cni result, interface index %d out of range", *ip.Interface) + + mac, err := net.ParseMAC(netInterface.Mac) + if err != nil { + return result, err } - cniInt := cniResult.Interfaces[*ip.Interface] - netInt, ok := interfaces[cniInt.Name] - if ok { - netInt.Subnets = append(netInt.Subnets, types.NetAddress{ - IPNet: types.IPNet{IPNet: ip.Address}, - Gateway: ip.Gateway, - }) - interfaces[cniInt.Name] = netInt - } else { - mac, err := net.ParseMAC(cniInt.Mac) - if err != nil { - return result, err + subnets := make([]types.NetAddress, 0, len(cniResult.IPs)) + for _, ip := range cniResult.IPs { + if ip.Interface == nil { + // we do no expect ips without an interface + continue } - interfaces[cniInt.Name] = types.NetInterface{ - MacAddress: types.HardwareAddr(mac), - Subnets: []types.NetAddress{{ + if len(cniResult.Interfaces) <= *ip.Interface { + return result, errors.Errorf("invalid cni result, interface index %d out of range", *ip.Interface) + } + + // when we have a ip for this interface add it to the subnets + if *ip.Interface == intIndex { + subnets = append(subnets, types.NetAddress{ IPNet: types.IPNet{IPNet: ip.Address}, Gateway: ip.Gateway, - }}, + }) } } + interfaces[netInterface.Name] = types.NetInterface{ + MacAddress: types.HardwareAddr(mac), + Subnets: subnets, + } } result.Interfaces = interfaces return result, nil diff --git a/libnetwork/cni/run_test.go b/libnetwork/cni/run_test.go index d4425681b..e3ab435e4 100644 --- a/libnetwork/cni/run_test.go +++ b/libnetwork/cni/run_test.go @@ -830,6 +830,91 @@ var _ = Describe("run CNI", func() { Expect(err).To(BeNil()) }) }) + + It("setup ipam driver none network", func() { + runTest(func() { + network := types.Network{ + IPAMOptions: map[string]string{ + types.Driver: types.NoneIPAMDriver, + }, + } + network1, err := libpodNet.NetworkCreate(network) + Expect(err).To(BeNil()) + + intName1 := "eth0" + netName1 := network1.Name + + setupOpts := types.SetupOptions{ + NetworkOptions: types.NetworkOptions{ + ContainerID: stringid.GenerateNonCryptoID(), + Networks: map[string]types.PerNetworkOptions{ + netName1: { + InterfaceName: intName1, + }, + }, + }, + } + + res, err := libpodNet.Setup(netNSContainer.Path(), setupOpts) + Expect(err).To(BeNil()) + Expect(res).To(HaveLen(1)) + + Expect(res).To(HaveKey(netName1)) + Expect(res[netName1].Interfaces).To(HaveKey(intName1)) + Expect(res[netName1].Interfaces[intName1].Subnets).To(HaveLen(0)) + macInt1 := res[netName1].Interfaces[intName1].MacAddress + Expect(macInt1).To(HaveLen(6)) + + // check in the container namespace if the settings are applied + err = netNSContainer.Do(func(_ ns.NetNS) error { + defer GinkgoRecover() + i, err := net.InterfaceByName(intName1) + Expect(err).To(BeNil()) + Expect(i.Name).To(Equal(intName1)) + Expect(i.HardwareAddr).To(Equal(net.HardwareAddr(macInt1))) + addrs, err := i.Addrs() + Expect(err).To(BeNil()) + // we still have the ipv6 link local address + Expect(addrs).To(HaveLen(1)) + addr, ok := addrs[0].(*net.IPNet) + Expect(ok).To(BeTrue(), "cast address to ipnet") + // make sure we are link local + Expect(addr.IP.IsLinkLocalUnicast()).To(BeTrue(), "ip is link local address") + + // check loopback adapter + i, err = net.InterfaceByName("lo") + Expect(err).To(BeNil()) + Expect(i.Name).To(Equal("lo")) + Expect(i.Flags & net.FlagLoopback).To(Equal(net.FlagLoopback)) + Expect(i.Flags&net.FlagUp).To(Equal(net.FlagUp), "Loopback adapter should be up") + return nil + }) + Expect(err).To(BeNil()) + + err = libpodNet.Teardown(netNSContainer.Path(), types.TeardownOptions(setupOpts)) + Expect(err).To(BeNil()) + logString := logBuffer.String() + Expect(logString).To(BeEmpty()) + + // check in the container namespace that the interface is removed + err = netNSContainer.Do(func(_ ns.NetNS) error { + defer GinkgoRecover() + _, err := net.InterfaceByName(intName1) + Expect(err).To(HaveOccurred()) + + // check that only the loopback adapter is left + ints, err := net.Interfaces() + Expect(err).To(BeNil()) + Expect(ints).To(HaveLen(1)) + Expect(ints[0].Name).To(Equal("lo")) + Expect(ints[0].Flags & net.FlagLoopback).To(Equal(net.FlagLoopback)) + Expect(ints[0].Flags&net.FlagUp).To(Equal(net.FlagUp), "Loopback adapter should be up") + + return nil + }) + Expect(err).To(BeNil()) + }) + }) }) Context("network setup test with networks from disk", func() { From ab3ec4bf37b5b3780b3fc716e957d31ee9e7a90c Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 17 Mar 2022 18:16:02 +0100 Subject: [PATCH 4/5] libnetwork/netavark: allow network create with no ipam driver Network create now uses the ipam driver. This allows the user to configure the ipam driver manually instead of choosing a fixed default. If the ipam driver is `none` no ips will be assigned to this container. This means that only the interfaces are created. This will require a patch in netavark since it rejects the config when no static ips are provided. Ref containers/podman#13521 Signed-off-by: Paul Holzinger --- libnetwork/netavark/config.go | 36 +++++++++++-- libnetwork/netavark/config_test.go | 65 ++++++++++++++++++++++- libnetwork/netavark/ipam_test.go | 4 +- libnetwork/netavark/run_test.go | 83 ++++++++++++++++++++++++++++++ 4 files changed, 182 insertions(+), 6 deletions(-) diff --git a/libnetwork/netavark/config.go b/libnetwork/netavark/config.go index 99b4e0308..d07232350 100644 --- a/libnetwork/netavark/config.go +++ b/libnetwork/netavark/config.go @@ -67,6 +67,11 @@ func (n *netavarkNetwork) networkCreate(newNetwork *types.Network, defaultNet bo return nil, err } + err = validateIPAMDriver(newNetwork) + if err != nil { + return nil, err + } + // Only get the used networks for validation if we do not create the default network. // The default network should not be validated against used subnets, we have to ensure // that this network can always be created even when a subnet is already used on the host. @@ -153,10 +158,19 @@ func createMacvlan(network *types.Network) error { return errors.Errorf("parent interface %s does not exist", network.NetworkInterface) } } - if len(network.Subnets) == 0 { - return errors.Errorf("macvlan driver needs at least one subnet specified, DHCP is not supported with netavark") + + // we already validated the drivers before so we just have to set the default here + switch network.IPAMOptions[types.Driver] { + case "": + if len(network.Subnets) == 0 { + return errors.Errorf("macvlan driver needs at least one subnet specified, DHCP is not yet supported with netavark") + } + network.IPAMOptions[types.Driver] = types.HostLocalIPAMDriver + case types.HostLocalIPAMDriver: + if len(network.Subnets) == 0 { + return errors.Errorf("macvlan driver needs at least one subnet specified, when the host-local ipam driver is set") + } } - network.IPAMOptions[types.Driver] = types.HostLocalIPAMDriver // validate the given options, we do not need them but just check to make sure they are valid for key, value := range network.Options { @@ -246,3 +260,19 @@ func (n *netavarkNetwork) NetworkInspect(nameOrID string) (types.Network, error) } return *network, nil } + +func validateIPAMDriver(n *types.Network) error { + ipamDriver := n.IPAMOptions[types.Driver] + switch ipamDriver { + case "", types.HostLocalIPAMDriver: + case types.NoneIPAMDriver: + if len(n.Subnets) > 0 { + return errors.New("none ipam driver is set but subnets are given") + } + case types.DHCPIPAMDriver: + return errors.New("dhcp ipam driver is not yet supported with netavark") + default: + return errors.Errorf("unsupported ipam driver %q", ipamDriver) + } + return nil +} diff --git a/libnetwork/netavark/config_test.go b/libnetwork/netavark/config_test.go index 5a66749c1..3b5e2b802 100644 --- a/libnetwork/netavark/config_test.go +++ b/libnetwork/netavark/config_test.go @@ -794,7 +794,7 @@ var _ = Describe("Config", func() { network := types.Network{Driver: "macvlan"} _, err := libpodNet.NetworkCreate(network) Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(Equal("macvlan driver needs at least one subnet specified, DHCP is not supported with netavark")) + Expect(err.Error()).To(Equal("macvlan driver needs at least one subnet specified, DHCP is not yet supported with netavark")) }) It("create macvlan config with internal", func() { @@ -957,6 +957,69 @@ var _ = Describe("Config", func() { Expect(network1.Options).To(HaveKeyWithValue("mtu", "9000")) }) + It("create bridge config with none ipam driver", func() { + network := types.Network{ + Driver: "bridge", + IPAMOptions: map[string]string{ + "driver": "none", + }, + } + network1, err := libpodNet.NetworkCreate(network) + Expect(err).To(BeNil()) + Expect(network1.Driver).To(Equal("bridge")) + Expect(network1.IPAMOptions).ToNot(BeEmpty()) + Expect(network1.IPAMOptions).To(HaveKeyWithValue("driver", "none")) + Expect(network1.Subnets).To(HaveLen(0)) + + // reload configs from disk + libpodNet, err = getNetworkInterface(networkConfDir) + Expect(err).To(BeNil()) + + network2, err := libpodNet.NetworkInspect(network1.Name) + Expect(err).To(BeNil()) + EqualNetwork(network2, network1) + }) + + It("create bridge config with none ipam driver and subnets", func() { + subnet := "10.1.0.0/24" + n, _ := types.ParseCIDR(subnet) + network := types.Network{ + Driver: "bridge", + IPAMOptions: map[string]string{ + "driver": "none", + }, + Subnets: []types.Subnet{ + {Subnet: n}, + }, + } + _, err := libpodNet.NetworkCreate(network) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("none ipam driver is set but subnets are given")) + }) + + It("create macvlan config with none ipam driver", func() { + network := types.Network{ + Driver: "macvlan", + IPAMOptions: map[string]string{ + "driver": "none", + }, + } + network1, err := libpodNet.NetworkCreate(network) + Expect(err).To(BeNil()) + Expect(network1.Driver).To(Equal("macvlan")) + Expect(network1.IPAMOptions).ToNot(BeEmpty()) + Expect(network1.IPAMOptions).To(HaveKeyWithValue("driver", "none")) + Expect(network1.Subnets).To(HaveLen(0)) + + // reload configs from disk + libpodNet, err = getNetworkInterface(networkConfDir) + Expect(err).To(BeNil()) + + network2, err := libpodNet.NetworkInspect(network1.Name) + Expect(err).To(BeNil()) + EqualNetwork(network2, network1) + }) + }) Context("network load valid existing ones", func() { diff --git a/libnetwork/netavark/ipam_test.go b/libnetwork/netavark/ipam_test.go index 00b80ac30..ba7b37c2b 100644 --- a/libnetwork/netavark/ipam_test.go +++ b/libnetwork/netavark/ipam_test.go @@ -399,10 +399,10 @@ var _ = Describe("IPAM", func() { } }) - It("ipam with dhcp driver should not set ips", func() { + It("ipam with none driver should not set ips", func() { network, err := networkInterface.NetworkCreate(types.Network{ IPAMOptions: map[string]string{ - "driver": types.DHCPIPAMDriver, + "driver": types.NoneIPAMDriver, }, }) Expect(err).ToNot(HaveOccurred()) diff --git a/libnetwork/netavark/run_test.go b/libnetwork/netavark/run_test.go index 0060bc3fd..cfe7f62c8 100644 --- a/libnetwork/netavark/run_test.go +++ b/libnetwork/netavark/run_test.go @@ -700,6 +700,89 @@ var _ = Describe("run netavark", func() { Expect(err.Error()).To(ContainSubstring("interface eth0 already exists on container namespace")) }) }) + + It("setup ipam driver none network", func() { + runTest(func() { + network := types.Network{ + IPAMOptions: map[string]string{ + types.Driver: types.NoneIPAMDriver, + }, + } + network1, err := libpodNet.NetworkCreate(network) + Expect(err).To(BeNil()) + + intName1 := "eth0" + netName1 := network1.Name + + setupOpts := types.SetupOptions{ + NetworkOptions: types.NetworkOptions{ + ContainerID: stringid.GenerateNonCryptoID(), + Networks: map[string]types.PerNetworkOptions{ + netName1: { + InterfaceName: intName1, + }, + }, + }, + } + + res, err := libpodNet.Setup(netNSContainer.Path(), setupOpts) + Expect(err).To(BeNil()) + Expect(res).To(HaveLen(1)) + + Expect(res).To(HaveKey(netName1)) + Expect(res[netName1].Interfaces).To(HaveKey(intName1)) + Expect(res[netName1].Interfaces[intName1].Subnets).To(HaveLen(0)) + macInt1 := res[netName1].Interfaces[intName1].MacAddress + Expect(macInt1).To(HaveLen(6)) + + // check in the container namespace if the settings are applied + err = netNSContainer.Do(func(_ ns.NetNS) error { + defer GinkgoRecover() + i, err := net.InterfaceByName(intName1) + Expect(err).To(BeNil()) + Expect(i.Name).To(Equal(intName1)) + Expect(i.HardwareAddr).To(Equal(net.HardwareAddr(macInt1))) + addrs, err := i.Addrs() + Expect(err).To(BeNil()) + // we still have the ipv6 link local address + Expect(addrs).To(HaveLen(1)) + addr, ok := addrs[0].(*net.IPNet) + Expect(ok).To(BeTrue(), "cast address to ipnet") + // make sure we are link local + Expect(addr.IP.IsLinkLocalUnicast()).To(BeTrue(), "ip is link local address") + + // check loopback adapter + i, err = net.InterfaceByName("lo") + Expect(err).To(BeNil()) + Expect(i.Name).To(Equal("lo")) + Expect(i.Flags & net.FlagLoopback).To(Equal(net.FlagLoopback)) + Expect(i.Flags&net.FlagUp).To(Equal(net.FlagUp), "Loopback adapter should be up") + return nil + }) + Expect(err).To(BeNil()) + + err = libpodNet.Teardown(netNSContainer.Path(), types.TeardownOptions(setupOpts)) + Expect(err).To(BeNil()) + + // check in the container namespace that the interface is removed + err = netNSContainer.Do(func(_ ns.NetNS) error { + defer GinkgoRecover() + _, err := net.InterfaceByName(intName1) + Expect(err).To(HaveOccurred()) + + // check that only the loopback adapter is left + ints, err := net.Interfaces() + Expect(err).To(BeNil()) + Expect(ints).To(HaveLen(1)) + Expect(ints[0].Name).To(Equal("lo")) + Expect(ints[0].Flags & net.FlagLoopback).To(Equal(net.FlagLoopback)) + Expect(ints[0].Flags&net.FlagUp).To(Equal(net.FlagUp), "Loopback adapter should be up") + + return nil + }) + Expect(err).To(BeNil()) + }) + }) }) func runNetListener(wg *sync.WaitGroup, protocol, ip string, port int, expectedData string) { From 5d02f0eb84bf4f0fe5fa9d2ac8123c74ec709226 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Fri, 18 Mar 2022 16:43:25 +0100 Subject: [PATCH 5/5] libnetwork/cni: split option paring into separate function The gocyclo linter is complaining that the cyclomatic complexity is to high for `(*cniNetwork).createCNIConfigListFromNetwork()`. Split out option parsing to a new funtion should reduce the complexity. Signed-off-by: Paul Holzinger --- libnetwork/cni/cni_conversion.go | 89 ++++++++++++++++++-------------- 1 file changed, 51 insertions(+), 38 deletions(-) diff --git a/libnetwork/cni/cni_conversion.go b/libnetwork/cni/cni_conversion.go index 448c1d099..36ac468de 100644 --- a/libnetwork/cni/cni_conversion.go +++ b/libnetwork/cni/cni_conversion.go @@ -271,41 +271,9 @@ func (n *cniNetwork) createCNIConfigListFromNetwork(network *types.Network, writ return nil, "", errors.Errorf("unsupported ipam driver %q", ipamDriver) } - vlan := 0 - mtu := 0 - vlanPluginMode := "" - for k, v := range network.Options { - switch k { - case "mtu": - mtu, err = internalutil.ParseMTU(v) - if err != nil { - return nil, "", err - } - - case "vlan": - vlan, err = internalutil.ParseVlan(v) - if err != nil { - return nil, "", err - } - - case "mode": - switch network.Driver { - case types.MacVLANNetworkDriver: - if !pkgutil.StringInSlice(v, types.ValidMacVLANModes) { - return nil, "", errors.Errorf("unknown macvlan mode %q", v) - } - case types.IPVLANNetworkDriver: - if !pkgutil.StringInSlice(v, types.ValidIPVLANModes) { - return nil, "", errors.Errorf("unknown ipvlan mode %q", v) - } - default: - return nil, "", errors.Errorf("cannot set option \"mode\" with driver %q", network.Driver) - } - vlanPluginMode = v - - default: - return nil, "", errors.Errorf("unsupported network option %s", k) - } + opts, err := parseOptions(network.Options, network.Driver) + if err != nil { + return nil, "", err } isGateway := true @@ -323,7 +291,7 @@ func (n *cniNetwork) createCNIConfigListFromNetwork(network *types.Network, writ switch network.Driver { case types.BridgeNetworkDriver: - bridge := newHostLocalBridge(network.NetworkInterface, isGateway, ipMasq, mtu, vlan, ipamConf) + bridge := newHostLocalBridge(network.NetworkInterface, isGateway, ipMasq, opts.mtu, opts.vlan, ipamConf) plugins = append(plugins, bridge, newPortMapPlugin(), newFirewallPlugin(), newTuningPlugin()) // if we find the dnsname plugin we add configuration for it if hasDNSNamePlugin(n.cniPluginDirs) && network.DNSEnabled { @@ -332,10 +300,10 @@ func (n *cniNetwork) createCNIConfigListFromNetwork(network *types.Network, writ } case types.MacVLANNetworkDriver: - plugins = append(plugins, newVLANPlugin(types.MacVLANNetworkDriver, network.NetworkInterface, vlanPluginMode, mtu, ipamConf)) + plugins = append(plugins, newVLANPlugin(types.MacVLANNetworkDriver, network.NetworkInterface, opts.vlanPluginMode, opts.mtu, ipamConf)) case types.IPVLANNetworkDriver: - plugins = append(plugins, newVLANPlugin(types.IPVLANNetworkDriver, network.NetworkInterface, vlanPluginMode, mtu, ipamConf)) + plugins = append(plugins, newVLANPlugin(types.IPVLANNetworkDriver, network.NetworkInterface, opts.vlanPluginMode, opts.mtu, ipamConf)) default: return nil, "", errors.Errorf("driver %q is not supported by cni", network.Driver) @@ -411,3 +379,48 @@ func removeMachinePlugin(conf *libcni.NetworkConfigList) *libcni.NetworkConfigLi conf.Plugins = plugins return conf } + +type options struct { + vlan int + mtu int + vlanPluginMode string +} + +func parseOptions(networkOptions map[string]string, networkDriver string) (*options, error) { + opt := &options{} + var err error + for k, v := range networkOptions { + switch k { + case "mtu": + opt.mtu, err = internalutil.ParseMTU(v) + if err != nil { + return nil, err + } + + case "vlan": + opt.vlan, err = internalutil.ParseVlan(v) + if err != nil { + return nil, err + } + + case "mode": + switch networkDriver { + case types.MacVLANNetworkDriver: + if !pkgutil.StringInSlice(v, types.ValidMacVLANModes) { + return nil, errors.Errorf("unknown macvlan mode %q", v) + } + case types.IPVLANNetworkDriver: + if !pkgutil.StringInSlice(v, types.ValidIPVLANModes) { + return nil, errors.Errorf("unknown ipvlan mode %q", v) + } + default: + return nil, errors.Errorf("cannot set option \"mode\" with driver %q", networkDriver) + } + opt.vlanPluginMode = v + + default: + return nil, errors.Errorf("unsupported network option %s", k) + } + } + return opt, nil +}