From a953940e7820d96012fb17b44bde59ce2e70012d Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 17 Mar 2022 16:39:47 +0100 Subject: [PATCH] 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 {