From e94690decb66205ae78d88ea4a1bd551031d3bd2 Mon Sep 17 00:00:00 2001 From: Nick Ethier Date: Mon, 6 Jul 2020 18:51:46 -0400 Subject: [PATCH] ar: support opting into binding host ports to default network IP (#8321) * ar: support opting into binding host ports to default network IP * fix config plumbing * plumb node address into network resource * struct: only handle network resource upgrade path once --- client/allocrunner/network_manager_linux.go | 8 ++- client/allocrunner/networking_bridge_linux.go | 4 +- client/allocrunner/networking_cni.go | 33 +++++----- client/config/config.go | 10 +++ command/agent/agent.go | 1 + command/agent/config.go | 12 ++++ nomad/structs/funcs_test.go | 33 +++++++--- nomad/structs/network.go | 62 ++++++++++++------- scheduler/rank.go | 2 +- 9 files changed, 115 insertions(+), 50 deletions(-) diff --git a/client/allocrunner/network_manager_linux.go b/client/allocrunner/network_manager_linux.go index b6702a7fe..63d63a347 100644 --- a/client/allocrunner/network_manager_linux.go +++ b/client/allocrunner/network_manager_linux.go @@ -133,12 +133,16 @@ func newNetworkConfigurator(log hclog.Logger, alloc *structs.Allocation, config } netMode := strings.ToLower(tg.Networks[0].Mode) + ignorePortMappingHostIP := config.BindWildcardDefaultHostNetwork + if len(config.HostNetworks) > 0 { + ignorePortMappingHostIP = false + } switch { case netMode == "bridge": - return newBridgeNetworkConfigurator(log, config.BridgeNetworkName, config.BridgeNetworkAllocSubnet, config.CNIPath) + return newBridgeNetworkConfigurator(log, config.BridgeNetworkName, config.BridgeNetworkAllocSubnet, config.CNIPath, ignorePortMappingHostIP) case strings.HasPrefix(netMode, "cni/"): - return newCNINetworkConfigurator(log, config.CNIPath, config.CNIInterfacePrefix, config.CNIConfigDir, netMode[4:]) + return newCNINetworkConfigurator(log, config.CNIPath, config.CNIInterfacePrefix, config.CNIConfigDir, netMode[4:], ignorePortMappingHostIP) default: return &hostNetworkConfigurator{}, nil } diff --git a/client/allocrunner/networking_bridge_linux.go b/client/allocrunner/networking_bridge_linux.go index 2bd429b8a..df4f04ea9 100644 --- a/client/allocrunner/networking_bridge_linux.go +++ b/client/allocrunner/networking_bridge_linux.go @@ -39,7 +39,7 @@ type bridgeNetworkConfigurator struct { logger hclog.Logger } -func newBridgeNetworkConfigurator(log hclog.Logger, bridgeName, ipRange, cniPath string) (*bridgeNetworkConfigurator, error) { +func newBridgeNetworkConfigurator(log hclog.Logger, bridgeName, ipRange, cniPath string, ignorePortMappingHostIP bool) (*bridgeNetworkConfigurator, error) { b := &bridgeNetworkConfigurator{ bridgeName: bridgeName, allocSubnet: ipRange, @@ -54,7 +54,7 @@ func newBridgeNetworkConfigurator(log hclog.Logger, bridgeName, ipRange, cniPath b.allocSubnet = defaultNomadAllocSubnet } - c, err := newCNINetworkConfiguratorWithConf(log, cniPath, bridgeNetworkAllocIfPrefix, buildNomadBridgeNetConfig(b.bridgeName, b.allocSubnet)) + c, err := newCNINetworkConfiguratorWithConf(log, cniPath, bridgeNetworkAllocIfPrefix, ignorePortMappingHostIP, buildNomadBridgeNetConfig(b.bridgeName, b.allocSubnet)) if err != nil { return nil, err } diff --git a/client/allocrunner/networking_cni.go b/client/allocrunner/networking_cni.go index b56069c7f..8dfea82c7 100644 --- a/client/allocrunner/networking_cni.go +++ b/client/allocrunner/networking_cni.go @@ -33,27 +33,29 @@ const ( ) type cniNetworkConfigurator struct { - cni cni.CNI - cniConf []byte + cni cni.CNI + cniConf []byte + ignorePortMappingHostIP bool rand *rand.Rand logger log.Logger } -func newCNINetworkConfigurator(logger log.Logger, cniPath, cniInterfacePrefix, cniConfDir, networkName string) (*cniNetworkConfigurator, error) { +func newCNINetworkConfigurator(logger log.Logger, cniPath, cniInterfacePrefix, cniConfDir, networkName string, ignorePortMappingHostIP bool) (*cniNetworkConfigurator, error) { cniConf, err := loadCNIConf(cniConfDir, networkName) if err != nil { return nil, fmt.Errorf("failed to load CNI config: %v", err) } - return newCNINetworkConfiguratorWithConf(logger, cniPath, cniInterfacePrefix, cniConf) + return newCNINetworkConfiguratorWithConf(logger, cniPath, cniInterfacePrefix, ignorePortMappingHostIP, cniConf) } -func newCNINetworkConfiguratorWithConf(logger log.Logger, cniPath, cniInterfacePrefix string, cniConf []byte) (*cniNetworkConfigurator, error) { +func newCNINetworkConfiguratorWithConf(logger log.Logger, cniPath, cniInterfacePrefix string, ignorePortMappingHostIP bool, cniConf []byte) (*cniNetworkConfigurator, error) { conf := &cniNetworkConfigurator{ - cniConf: cniConf, - rand: rand.New(rand.NewSource(time.Now().Unix())), - logger: logger, + cniConf: cniConf, + rand: rand.New(rand.NewSource(time.Now().Unix())), + logger: logger, + ignorePortMappingHostIP: ignorePortMappingHostIP, } if cniPath == "" { if cniPath = os.Getenv(envCNIPath); cniPath == "" { @@ -88,7 +90,7 @@ func (c *cniNetworkConfigurator) Setup(ctx context.Context, alloc *structs.Alloc var firstError error for attempt := 1; ; attempt++ { //TODO eventually returning the IP from the result would be nice to have in the alloc - if _, err := c.cni.Setup(ctx, alloc.ID, spec.Path, cni.WithCapabilityPortMap(getPortMapping(alloc))); err != nil { + if _, err := c.cni.Setup(ctx, alloc.ID, spec.Path, cni.WithCapabilityPortMap(getPortMapping(alloc, c.ignorePortMappingHostIP))); err != nil { c.logger.Warn("failed to configure network", "err", err, "attempt", attempt) switch attempt { case 1: @@ -149,7 +151,7 @@ func (c *cniNetworkConfigurator) Teardown(ctx context.Context, alloc *structs.Al return err } - return c.cni.Remove(ctx, alloc.ID, spec.Path, cni.WithCapabilityPortMap(getPortMapping(alloc))) + return c.cni.Remove(ctx, alloc.ID, spec.Path, cni.WithCapabilityPortMap(getPortMapping(alloc, c.ignorePortMappingHostIP))) } func (c *cniNetworkConfigurator) ensureCNIInitialized() error { @@ -162,7 +164,7 @@ func (c *cniNetworkConfigurator) ensureCNIInitialized() error { // getPortMapping builds a list of portMapping structs that are used as the // portmapping capability arguments for the portmap CNI plugin -func getPortMapping(alloc *structs.Allocation) []cni.PortMapping { +func getPortMapping(alloc *structs.Allocation, ignoreHostIP bool) []cni.PortMapping { ports := []cni.PortMapping{} if len(alloc.AllocatedResources.Shared.Ports) == 0 && len(alloc.AllocatedResources.Shared.Networks) > 0 { @@ -186,12 +188,15 @@ func getPortMapping(alloc *structs.Allocation) []cni.PortMapping { port.To = port.Value } for _, proto := range []string{"tcp", "udp"} { - ports = append(ports, cni.PortMapping{ + portMapping := cni.PortMapping{ HostPort: int32(port.Value), ContainerPort: int32(port.To), Protocol: proto, - HostIP: port.HostIP, - }) + } + if !ignoreHostIP { + portMapping.HostIP = port.HostIP + } + ports = append(ports, portMapping) } } } diff --git a/client/config/config.go b/client/config/config.go index 2323d9add..ab282691f 100644 --- a/client/config/config.go +++ b/client/config/config.go @@ -258,6 +258,16 @@ type Config struct { // HostNetworks is a map of the conigured host networks by name. HostNetworks map[string]*structs.ClientHostNetworkConfig + + // BindWildcardDefaultHostNetwork toggles if the default host network should accept all + // destinations (true) or only filter on the IP of the default host network (false) when + // port mapping. This allows Nomad clients with no defined host networks to accept and + // port forward traffic only matching on the destination port. An example use of this + // is when a network loadbalancer is utilizing direct server return and the destination + // address of incomming packets does not match the IP address of the host interface. + // + // This configuration is only considered if no host networks are defined. + BindWildcardDefaultHostNetwork bool } type ClientTemplateConfig struct { diff --git a/command/agent/agent.go b/command/agent/agent.go index 440d89c57..45007fa5c 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -638,6 +638,7 @@ func convertClientConfig(agentConfig *Config) (*clientconfig.Config, error) { for _, hn := range agentConfig.Client.HostNetworks { conf.HostNetworks[hn.Name] = hn } + conf.BindWildcardDefaultHostNetwork = agentConfig.Client.BindWildcardDefaultHostNetwork return conf, nil } diff --git a/command/agent/config.go b/command/agent/config.go index 830ec5a07..fae4e3a6e 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -286,8 +286,15 @@ type ClientConfig struct { // the host BridgeNetworkSubnet string `hcl:"bridge_network_subnet"` + // HostNetworks describes the different host networks available to the host + // if the host uses multiple interfaces HostNetworks []*structs.ClientHostNetworkConfig `hcl:"host_network"` + // BindWildcardDefaultHostNetwork toggles if when there are no host networks, + // should the port mapping rules match the default network address (false) or + // matching any destination address (true). Defaults to true + BindWildcardDefaultHostNetwork bool `hcl:"bind_wildcard_default_host_network"` + // ExtraKeysHCL is used by hcl to surface unexpected keys ExtraKeysHCL []string `hcl:",unusedKeys" json:"-"` } @@ -819,6 +826,7 @@ func DevConfig(mode *devModeConfig) *Config { FunctionBlacklist: []string{"plugin"}, DisableSandbox: false, } + conf.Client.BindWildcardDefaultHostNetwork = true conf.Telemetry.PrometheusMetrics = true conf.Telemetry.PublishAllocationMetrics = true conf.Telemetry.PublishNodeMetrics = true @@ -864,6 +872,7 @@ func DefaultConfig() *Config { FunctionBlacklist: []string{"plugin"}, DisableSandbox: false, }, + BindWildcardDefaultHostNetwork: true, }, Server: &ServerConfig{ Enabled: false, @@ -1539,6 +1548,9 @@ func (a *ClientConfig) Merge(b *ClientConfig) *ClientConfig { result.HostNetworks = a.HostNetworks } + if b.BindWildcardDefaultHostNetwork { + result.BindWildcardDefaultHostNetwork = true + } return &result } diff --git a/nomad/structs/funcs_test.go b/nomad/structs/funcs_test.go index 7ca682e36..504cc3a8e 100644 --- a/nomad/structs/funcs_test.go +++ b/nomad/structs/funcs_test.go @@ -291,6 +291,17 @@ func TestAllocsFit(t *testing.T) { MBits: 100, }, }, + NodeNetworks: []*NodeNetworkResource{ + { + Mode: "host", + Device: "eth0", + Addresses: []NodeNetworkAddress{ + { + Address: "10.0.0.1", + }, + }, + }, + }, }, ReservedResources: &NodeReservedResources{ Cpu: NodeReservedCpuResources{ @@ -318,18 +329,24 @@ func TestAllocsFit(t *testing.T) { Memory: AllocatedMemoryResources{ MemoryMB: 1024, }, - Networks: []*NetworkResource{ - { - Device: "eth0", - IP: "10.0.0.1", - MBits: 50, - ReservedPorts: []Port{{"main", 8000, 0, ""}}, - }, - }, }, }, Shared: AllocatedSharedResources{ DiskMB: 5000, + Networks: Networks{ + { + Mode: "host", + IP: "10.0.0.1", + ReservedPorts: []Port{{"main", 8000, 0, ""}}, + }, + }, + Ports: AllocatedPorts{ + { + Label: "main", + Value: 8000, + HostIP: "10.0.0.1", + }, + }, }, }, } diff --git a/nomad/structs/network.go b/nomad/structs/network.go index 4b8c37f15..0a0932a23 100644 --- a/nomad/structs/network.go +++ b/nomad/structs/network.go @@ -149,29 +149,32 @@ func (idx *NetworkIndex) AddAllocs(allocs []*Allocation) (collide bool) { } if alloc.AllocatedResources != nil { - // Add network resources that are at the task group level - if len(alloc.AllocatedResources.Shared.Networks) > 0 { - for _, network := range alloc.AllocatedResources.Shared.Networks { - if idx.AddReserved(network) { + // Only look at AllocatedPorts if populated, otherwise use pre 0.12 logic + // COMPAT(1.0): Remove when network resources struct is removed. + if len(alloc.AllocatedResources.Shared.Ports) > 0 { + if idx.AddReservedPorts(alloc.AllocatedResources.Shared.Ports) { + collide = true + } + } else { + // Add network resources that are at the task group level + if len(alloc.AllocatedResources.Shared.Networks) > 0 { + for _, network := range alloc.AllocatedResources.Shared.Networks { + if idx.AddReserved(network) { + collide = true + } + } + } + + for _, task := range alloc.AllocatedResources.Tasks { + if len(task.Networks) == 0 { + continue + } + n := task.Networks[0] + if idx.AddReserved(n) { collide = true } } } - - for _, task := range alloc.AllocatedResources.Tasks { - if len(task.Networks) == 0 { - continue - } - n := task.Networks[0] - if idx.AddReserved(n) { - collide = true - } - } - - // Multi-interface TODO: handle upgrade path here? - if idx.AddReservedPorts(alloc.AllocatedResources.Shared.Ports) { - collide = true - } } else { // COMPAT(0.11): Remove in 0.11 for _, task := range alloc.TaskResources { @@ -332,8 +335,7 @@ func (idx *NetworkIndex) AssignPorts(ask *NetworkResource) (AllocatedPorts, erro // Check if in use if used != nil && used.Check(uint(port.Value)) { - addrErr = fmt.Errorf("reserved port collision") - continue + return nil, fmt.Errorf("reserved port collision %s=%d", port.Label, port.Value) } allocPort = &AllocatedPortMapping{ @@ -427,7 +429,7 @@ func (idx *NetworkIndex) AssignNetwork(ask *NetworkResource) (out *NetworkResour // Check if in use if used != nil && used.Check(uint(port.Value)) { - err = fmt.Errorf("reserved port collision") + err = fmt.Errorf("reserved port collision %s=%d", port.Label, port.Value) return } } @@ -565,7 +567,7 @@ func isPortReserved(haystack []int, needle int) bool { } // COMPAT(1.0) remove when NetworkResource is no longer used for materialized client view of ports -func AllocatedPortsToNetworkResouce(ask *NetworkResource, ports AllocatedPorts) *NetworkResource { +func AllocatedPortsToNetworkResouce(ask *NetworkResource, ports AllocatedPorts, node *NodeResources) *NetworkResource { out := ask.Copy() for i, port := range ask.DynamicPorts { @@ -574,6 +576,20 @@ func AllocatedPortsToNetworkResouce(ask *NetworkResource, ports AllocatedPorts) out.DynamicPorts[i].To = p.To } } + if len(node.NodeNetworks) > 0 { + for _, nw := range node.NodeNetworks { + if nw.Mode == "host" { + out.IP = nw.Addresses[0].Address + break + } + } + } else { + for _, nw := range node.Networks { + if nw.Mode == "host" { + out.IP = nw.IP + } + } + } return out } diff --git a/scheduler/rank.go b/scheduler/rank.go index 985cb9715..1653d9cf9 100644 --- a/scheduler/rank.go +++ b/scheduler/rank.go @@ -284,7 +284,7 @@ OUTER: netIdx.AddReservedPorts(offer) // Update the network ask to the offer - nwRes := structs.AllocatedPortsToNetworkResouce(ask, offer) + nwRes := structs.AllocatedPortsToNetworkResouce(ask, offer, option.Node.NodeResources) total.Shared.Networks = []*structs.NetworkResource{nwRes} total.Shared.Ports = offer option.AllocResources = &structs.AllocatedSharedResources{