From 4df80f5472dd153a42ec5268cce4c8fd2efdb870 Mon Sep 17 00:00:00 2001 From: Thomas Lefebvre Date: Fri, 28 Oct 2022 08:26:29 +0000 Subject: [PATCH] Support more than one IP per interface and IPv6 for results returned by CNI Note that the TestNetworkStaticValidationFails_IPConfiguration test was removed since IPv6 support got added. Signed-off-by: Thomas Lefebvre --- cni/vmconf/vmconf.go | 61 +++++++++++------------ cni/vmconf/vmconf_test.go | 12 +++-- examples/cmd/snapshotting/example_demo.go | 4 +- machine.go | 9 ++-- machine_test.go | 4 +- network.go | 32 ++++++------ network_test.go | 55 ++++++++++---------- 7 files changed, 87 insertions(+), 90 deletions(-) diff --git a/cni/vmconf/vmconf.go b/cni/vmconf/vmconf.go index f0f7e2ab..b1e0f254 100644 --- a/cni/vmconf/vmconf.go +++ b/cni/vmconf/vmconf.go @@ -12,19 +12,19 @@ // permissions and limitations under the License. /* - Package vmconf defines an interface for converting particular CNI invocation - results to networking configuration usable by a VM. It expects the CNI result - to have the following properties: - * The results should contain an interface for a tap device, which will be used - as the VM's tap device. - * The results should contain an interface with the same name as the tap device - but with sandbox ID set to the containerID provided during CNI invocation. - This should be a "pseudo-interface", not one that has actually been created. - It represents the configuration that should be applied to the VM internally. - The CNI "containerID" is, in this case, used more as a "vmID" to represent - the VM's internal network interface. - * If the CNI results specify an IP associated with this interface, that IP - should be used to statically configure the VM's internal network interface. +Package vmconf defines an interface for converting particular CNI invocation +results to networking configuration usable by a VM. It expects the CNI result +to have the following properties: + - The results should contain an interface for a tap device, which will be used + as the VM's tap device. + - The results should contain an interface with the same name as the tap device + but with sandbox ID set to the containerID provided during CNI invocation. + This should be a "pseudo-interface", not one that has actually been created. + It represents the configuration that should be applied to the VM internally. + The CNI "containerID" is, in this case, used more as a "vmID" to represent + the VM's internal network interface. + - If the CNI results specify an IP associated with this interface, that IP + should be used to statically configure the VM's internal network interface. */ package vmconf @@ -62,7 +62,7 @@ type StaticNetworkConf struct { VMMTU int // VMIPConfig is the ip configuration that callers should configure their VM's internal // primary interface to use. - VMIPConfig *current.IPConfig + VMIPConfig []*current.IPConfig // VMRoutes are the routes that callers should configure their VM's internal route table // to have VMRoutes []*types.Route @@ -88,32 +88,32 @@ type StaticNetworkConf struct { // // Due to the limitation of "ip=", not all configuration specified in StaticNetworkConf can be // applied automatically. In particular: -// * The MacAddr and MTU cannot be applied -// * The only routes created will match what's specified in VMIPConfig; VMRoutes will be ignored. -// * Only up to two namesevers can be supplied. If VMNameservers is has more than 2 entries, only -// the first two in the slice will be applied in the VM. -// * VMDomain, VMSearchDomains and VMResolverOptions will be ignored -// * Nameserver settings are also only set in /proc/net/pnp. Most applications will thus require -// /etc/resolv.conf to be a symlink to /proc/net/pnp in order to resolve names as expected. +// - The MacAddr and MTU cannot be applied +// - The only routes created will match what's specified in VMIPConfig; VMRoutes will be ignored. +// - Only up to two namesevers can be supplied. If VMNameservers is has more than 2 entries, only +// the first two in the slice will be applied in the VM. +// - VMDomain, VMSearchDomains and VMResolverOptions will be ignored +// - Nameserver settings are also only set in /proc/net/pnp. Most applications will thus require +// /etc/resolv.conf to be a symlink to /proc/net/pnp in order to resolve names as expected. func (c StaticNetworkConf) IPBootParam() string { // See "ip=" section of kernel linked above for details on each field listed below. // client-ip is really just the ip that will be assigned to the primary interface - clientIP := c.VMIPConfig.Address.IP.String() + clientIP := c.VMIPConfig[0].Address.IP.String() // don't set nfs server IP const serverIP = "" // default gateway for the network; used to generate a corresponding route table entry - defaultGateway := c.VMIPConfig.Gateway.String() + defaultGateway := c.VMIPConfig[0].Gateway.String() // subnet mask used to generate a corresponding route table entry for the primary interface // (must be provided in dotted decimal notation) subnetMask := fmt.Sprintf("%d.%d.%d.%d", - c.VMIPConfig.Address.Mask[0], - c.VMIPConfig.Address.Mask[1], - c.VMIPConfig.Address.Mask[2], - c.VMIPConfig.Address.Mask[3], + c.VMIPConfig[0].Address.Mask[0], + c.VMIPConfig[0].Address.Mask[1], + c.VMIPConfig[0].Address.Mask[2], + c.VMIPConfig[0].Address.Mask[3], ) // the "hostname" field actually just configures a hostname value for DHCP requests, thus no need to set it @@ -168,11 +168,6 @@ func StaticNetworkConfFrom(result types.Result, containerID string) (*StaticNetw // find the IP associated with the VM iface vmIPs := internal.InterfaceIPs(currentResult, vmIface.Name, vmIface.Sandbox) - if len(vmIPs) != 1 { - return nil, fmt.Errorf("expected to find 1 IP for vm interface %q, but instead found %+v", - vmIface.Name, vmIPs) - } - vmIP := vmIPs[0] netNS, err := ns.GetNS(tapIface.Sandbox) if err != nil { @@ -189,7 +184,7 @@ func StaticNetworkConfFrom(result types.Result, containerID string) (*StaticNetw NetNSPath: tapIface.Sandbox, VMMacAddr: vmIface.Mac, VMMTU: tapMTU, - VMIPConfig: vmIP, + VMIPConfig: vmIPs, VMRoutes: currentResult.Routes, VMNameservers: currentResult.DNS.Nameservers, VMDomain: currentResult.DNS.Domain, diff --git a/cni/vmconf/vmconf_test.go b/cni/vmconf/vmconf_test.go index b9ef03d8..282b0e8b 100644 --- a/cni/vmconf/vmconf_test.go +++ b/cni/vmconf/vmconf_test.go @@ -68,12 +68,14 @@ func TestIPBootParams(t *testing.T) { VMMacAddr: "00:11:22:33:44:55", VMIfName: "eth0", VMMTU: 1337, - VMIPConfig: ¤t.IPConfig{ - Address: net.IPNet{ - IP: net.IPv4(10, 0, 0, 2), - Mask: net.IPv4Mask(255, 255, 255, 0), + VMIPConfig: []*current.IPConfig{ + ¤t.IPConfig{ + Address: net.IPNet{ + IP: net.IPv4(10, 0, 0, 2), + Mask: net.IPv4Mask(255, 255, 255, 0), + }, + Gateway: net.IPv4(10, 0, 0, 1), }, - Gateway: net.IPv4(10, 0, 0, 1), }, VMRoutes: []*types.Route{{ Dst: net.IPNet{ diff --git a/examples/cmd/snapshotting/example_demo.go b/examples/cmd/snapshotting/example_demo.go index 976bf140..67a429ad 100644 --- a/examples/cmd/snapshotting/example_demo.go +++ b/examples/cmd/snapshotting/example_demo.go @@ -134,7 +134,7 @@ func connectToVM(m *sdk.Machine, sshKeyPath string) (*ssh.Client, error) { return nil, errors.New("No network interfaces") } - ip := m.Cfg.NetworkInterfaces[0].StaticConfiguration.IPConfiguration.IPAddr.IP // IP of VM + ip := m.Cfg.NetworkInterfaces[0].StaticConfiguration.IPConfiguration[0].IPAddr.IP // IP of VM return ssh.Dial("tcp", fmt.Sprintf("%s:22", ip), config) } @@ -194,7 +194,7 @@ func createSnapshotSSH(ctx context.Context, socketPath, memPath, snapPath string } }() - vmIP := m.Cfg.NetworkInterfaces[0].StaticConfiguration.IPConfiguration.IPAddr.IP.String() + vmIP := m.Cfg.NetworkInterfaces[0].StaticConfiguration.IPConfiguration[0].IPAddr.IP.String() fmt.Printf("IP of VM: %v\n", vmIP) sshKeyPath := filepath.Join(dir, "root-drive-ssh-key") diff --git a/machine.go b/machine.go index 56fa3e25..2122c148 100644 --- a/machine.go +++ b/machine.go @@ -503,8 +503,11 @@ func (m *Machine) setupKernelArgs(ctx context.Context) error { // If any network interfaces have a static IP configured, we need to set the "ip=" boot param. // Validation that we are not overriding an existing "ip=" setting happens in the network validation if staticIPInterface := m.Cfg.NetworkInterfaces.staticIPInterface(); staticIPInterface != nil { - ipBootParam := staticIPInterface.StaticConfiguration.IPConfiguration.ipBootParam() - kernelArgs["ip"] = &ipBootParam + // Only generate the ip= boot param if there is a single ip on the interface + if len(staticIPInterface.StaticConfiguration.IPConfiguration) == 1 { + ipBootParam := staticIPInterface.StaticConfiguration.IPConfiguration[0].ipBootParam() + kernelArgs["ip"] = &ipBootParam + } } m.Cfg.KernelArgs = kernelArgs.String() @@ -649,7 +652,7 @@ func (m *Machine) startVMM(ctx context.Context) error { return nil } -//StopVMM stops the current VMM. +// StopVMM stops the current VMM. func (m *Machine) StopVMM() error { return m.stopVMM() } diff --git a/machine_test.go b/machine_test.go index 34615fa4..262d936a 100644 --- a/machine_test.go +++ b/machine_test.go @@ -1998,7 +1998,7 @@ func connectToVM(m *Machine, sshKeyPath string) (*ssh.Client, error) { return nil, errors.New("No network interfaces") } - ip := m.Cfg.NetworkInterfaces.staticIPInterface().StaticConfiguration.IPConfiguration.IPAddr.IP + ip := m.Cfg.NetworkInterfaces.staticIPInterface().StaticConfiguration.IPConfiguration[0].IPAddr.IP return ssh.Dial("tcp", fmt.Sprintf("%s:22", ip), config) } @@ -2193,7 +2193,7 @@ func TestLoadSnapshot(t *testing.T) { require.NoError(t, err) defer client.Close() - ipToRestore = m.Cfg.NetworkInterfaces.staticIPInterface().StaticConfiguration.IPConfiguration.IPAddr.IP.String() + ipToRestore = m.Cfg.NetworkInterfaces.staticIPInterface().StaticConfiguration.IPConfiguration[0].IPAddr.IP.String() session, err := client.NewSession() require.NoError(t, err) diff --git a/network.go b/network.go index 35352996..e6b41856 100644 --- a/network.go +++ b/network.go @@ -141,19 +141,18 @@ func (networkInterfaces NetworkInterfaces) setupNetwork( MacAddress: vmNetConf.VMMacAddr, } - if vmNetConf.VMIPConfig != nil { + for _, vmIPCfg := range vmNetConf.VMIPConfig { if len(vmNetConf.VMNameservers) > 2 { logger.Warnf("more than 2 nameservers provided from CNI result, only the first 2 %+v will be applied", vmNetConf.VMNameservers[:2]) vmNetConf.VMNameservers = vmNetConf.VMNameservers[:2] } - - cniNetworkInterface.StaticConfiguration.IPConfiguration = &IPConfiguration{ - IPAddr: vmNetConf.VMIPConfig.Address, - Gateway: vmNetConf.VMIPConfig.Gateway, + cniNetworkInterface.StaticConfiguration.IPConfiguration = append(cniNetworkInterface.StaticConfiguration.IPConfiguration, &IPConfiguration{ + IPAddr: vmIPCfg.Address, + Gateway: vmIPCfg.Gateway, Nameservers: vmNetConf.VMNameservers, IfName: cniNetworkInterface.CNIConfiguration.VMIfName, - } + }) } } @@ -484,7 +483,7 @@ type StaticNetworkConfiguration struct { // IPConfiguration (optional) allows a static IP, gateway and up to 2 DNS nameservers // to be automatically configured within the VM upon startup. - IPConfiguration *IPConfiguration + IPConfiguration []*IPConfiguration } func (staticConf StaticNetworkConfiguration) validate() error { @@ -492,8 +491,8 @@ func (staticConf StaticNetworkConfiguration) validate() error { return fmt.Errorf("HostDevName must be provided if StaticNetworkConfiguration is provided: %+v", staticConf) } - if staticConf.IPConfiguration != nil { - err := staticConf.IPConfiguration.validate() + for _, ipCfg := range staticConf.IPConfiguration { + err := ipCfg.validate() if err != nil { return err } @@ -522,10 +521,10 @@ type IPConfiguration struct { } func (ipConf IPConfiguration) validate() error { - // Make sure only ipv4 is being provided (for now). + // Make sure it is a valid ip. for _, ip := range []net.IP{ipConf.IPAddr.IP, ipConf.Gateway} { - if ip.To4() == nil { - return fmt.Errorf("invalid ip, only ipv4 addresses are supported: %+v", ip) + if ip.To4() == nil && ip.To16() == nil { + return fmt.Errorf("invalid ip: %+v", ip) } } @@ -538,11 +537,14 @@ func (ipConf IPConfiguration) validate() error { func (conf IPConfiguration) ipBootParam() string { // the vmconf package already has a function for doing this, just re-use it + vmConf := vmconf.StaticNetworkConf{ VMNameservers: conf.Nameservers, - VMIPConfig: ¤t.IPConfig{ - Address: conf.IPAddr, - Gateway: conf.Gateway, + VMIPConfig: []*current.IPConfig{ + { + Address: conf.IPAddr, + Gateway: conf.Gateway, + }, }, VMIfName: conf.IfName, } diff --git a/network_test.go b/network_test.go index 346b5cf3..cef36930 100644 --- a/network_test.go +++ b/network_test.go @@ -44,23 +44,27 @@ var ( kernelArgsWithIP = parseKernelArgs("foo=bar this=phony ip=whatevz") // These RFC 5737 IPs are reserved for documentation, they are not usable - validIPConfiguration = &IPConfiguration{ - IPAddr: net.IPNet{ - IP: net.IPv4(198, 51, 100, 2), - Mask: net.IPv4Mask(255, 255, 255, 0), + validIPConfiguration = []*IPConfiguration{ + &IPConfiguration{ + IPAddr: net.IPNet{ + IP: net.IPv4(198, 51, 100, 2), + Mask: net.IPv4Mask(255, 255, 255, 0), + }, + Gateway: net.IPv4(198, 51, 100, 1), + Nameservers: []string{"192.0.2.1", "192.0.2.2"}, }, - Gateway: net.IPv4(198, 51, 100, 1), - Nameservers: []string{"192.0.2.1", "192.0.2.2"}, } // IPv6 is currently invalid // These RFC 3849 IPs are reserved for documentation, they are not usable - invalidIPConfiguration = &IPConfiguration{ - IPAddr: net.IPNet{ - IP: net.ParseIP("2001:db8:a0b:12f0::2"), - Mask: net.CIDRMask(24, 128), + invalidIPConfiguration = []*IPConfiguration{ + &IPConfiguration{ + IPAddr: net.IPNet{ + IP: net.ParseIP("2001:db8:a0b:12f0::2"), + Mask: net.CIDRMask(24, 128), + }, + Gateway: net.ParseIP("2001:db8:a0b:12f0::1"), }, - Gateway: net.ParseIP("2001:db8:a0b:12f0::1"), } validStaticNetworkInterface = NetworkInterface{ @@ -99,13 +103,15 @@ func TestNetworkStaticValidationFails_TooManyNameservers(t *testing.T) { staticNetworkConfig := StaticNetworkConfiguration{ MacAddress: mockMacAddrString, HostDevName: tapName, - IPConfiguration: &IPConfiguration{ - IPAddr: net.IPNet{ - IP: net.IPv4(198, 51, 100, 2), - Mask: net.IPv4Mask(255, 255, 255, 0), + IPConfiguration: []*IPConfiguration{ + &IPConfiguration{ + IPAddr: net.IPNet{ + IP: net.IPv4(198, 51, 100, 2), + Mask: net.IPv4Mask(255, 255, 255, 0), + }, + Gateway: net.IPv4(198, 51, 100, 1), + Nameservers: []string{"192.0.2.1", "192.0.2.2", "192.0.2.3"}, }, - Gateway: net.IPv4(198, 51, 100, 1), - Nameservers: []string{"192.0.2.1", "192.0.2.2", "192.0.2.3"}, }, } @@ -113,17 +119,6 @@ func TestNetworkStaticValidationFails_TooManyNameservers(t *testing.T) { assert.Error(t, err, "network config with more than two nameservers did not result in validation error") } -func TestNetworkStaticValidationFails_IPConfiguration(t *testing.T) { - staticNetworkConfig := StaticNetworkConfiguration{ - MacAddress: mockMacAddrString, - HostDevName: tapName, - IPConfiguration: invalidIPConfiguration, - } - - err := staticNetworkConfig.validate() - assert.Error(t, err, "invalid network config hostdevname did not result in validation error") -} - func TestNetworkCNIValidation(t *testing.T) { err := validCNIInterface.CNIConfiguration.validate() assert.NoError(t, err, "valid cni network config unexpectedly returned validation error") @@ -448,12 +443,12 @@ func startCNIMachine(t *testing.T, ctx context.Context, m *Machine) string { assert.NotEmpty(t, staticConfig.HostDevName, "static config should have host dev name set") - ipConfig := staticConfig.IPConfiguration + ipConfig := staticConfig.IPConfiguration[0] require.NotNil(t, ipConfig, "cni configuration should have updated network interface ip configuration") require.Equal(t, m.Cfg.NetworkInterfaces[0].CNIConfiguration.VMIfName, - staticConfig.IPConfiguration.IfName, + staticConfig.IPConfiguration[0].IfName, "interface name should be propagated to static conf") return ipConfig.IPAddr.IP.String()