Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(#23388): Add a preferred address family option for network-interface #23389

Merged
merged 7 commits into from
Jul 12, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions client/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,9 @@ type Config struct {
// Network interface to be used in network fingerprinting
NetworkInterface string

// Preferred address family to be used in network fingerprinting
PreferredAddressFamily structs.NodeNetworkAF

// Network speed is the default speed of network interfaces if they can not
// be determined dynamically.
NetworkSpeed int
Expand Down
90 changes: 88 additions & 2 deletions client/fingerprint/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package fingerprint
import (
"fmt"
"net"
"sort"
"strings"

log "github.com/hashicorp/go-hclog"
Expand Down Expand Up @@ -100,7 +101,7 @@ func (f *NetworkFingerprint) Fingerprint(req *FingerprintRequest, resp *Fingerpr

// Create the network resources from the interface
disallowLinkLocal := cfg.ReadBoolDefault(networkDisallowLinkLocalOption, networkDisallowLinkLocalDefault)
nwResources, err := f.createNetworkResources(mbits, intf, disallowLinkLocal)
nwResources, err := f.createNetworkResources(mbits, intf, disallowLinkLocal, cfg.PreferredAddressFamily)
if err != nil {
return err
}
Expand Down Expand Up @@ -169,6 +170,7 @@ func (f *NetworkFingerprint) createNodeNetworkResources(ifaces []net.Interface,
} else {
family = structs.NodeNetworkAF_IPv6
}

for _, alias := range deriveAddressAliases(iface, ip, conf) {
newAddr := structs.NodeNetworkAddress{
Address: ip.String(),
Expand All @@ -190,6 +192,9 @@ func (f *NetworkFingerprint) createNodeNetworkResources(ifaces []net.Interface,
}
}

sortNodeNetworkAddresses(networkAddrs, conf.PreferredAddressFamily)
sortNodeNetworkAddresses(linkLocalAddrs, conf.PreferredAddressFamily)

if len(networkAddrs) == 0 && len(linkLocalAddrs) > 0 {
if disallowLinkLocal {
f.logger.Debug("ignoring detected link-local address on interface", "interface", iface.Name)
Expand All @@ -204,6 +209,9 @@ func (f *NetworkFingerprint) createNodeNetworkResources(ifaces []net.Interface,
nets = append(nets, newNetwork)
}
}

sortNodeNetworkResources(nets, conf.PreferredAddressFamily)

return nets, nil
}

Expand Down Expand Up @@ -257,7 +265,7 @@ func deriveAddressAliases(iface net.Interface, addr net.IP, config *config.Confi
}

// createNetworkResources creates network resources for every IP
func (f *NetworkFingerprint) createNetworkResources(throughput int, intf *net.Interface, disallowLinkLocal bool) ([]*structs.NetworkResource, error) {
func (f *NetworkFingerprint) createNetworkResources(throughput int, intf *net.Interface, disallowLinkLocal bool, preferredAF structs.NodeNetworkAF) ([]*structs.NetworkResource, error) {
// Find the interface with the name
addrs, err := f.interfaceDetector.Addrs(intf)
if err != nil {
Expand Down Expand Up @@ -288,6 +296,7 @@ func (f *NetworkFingerprint) createNetworkResources(throughput int, intf *net.In
if ip.To4() != nil {
newNetwork.CIDR = newNetwork.IP + "/32"
} else {
//continue // force IPv4, ignore IPv6
guifran001 marked this conversation as resolved.
Show resolved Hide resolved
newNetwork.CIDR = newNetwork.IP + "/128"
}

Expand All @@ -301,6 +310,9 @@ func (f *NetworkFingerprint) createNetworkResources(throughput int, intf *net.In
nwResources = append(nwResources, newNetwork)
}

sortNetworkResources(nwResources, preferredAF)
sortNetworkResources(linkLocals, preferredAF)

if len(nwResources) == 0 && len(linkLocals) != 0 {
if disallowLinkLocal {
f.logger.Debug("ignoring detected link-local address on interface", "interface", intf.Name)
Expand Down Expand Up @@ -335,3 +347,77 @@ func (f *NetworkFingerprint) findInterface(deviceName string) (*net.Interface, e

return f.interfaceDetector.InterfaceByName(deviceName)
}

// Define a type for the comparison function
type LessFunc[T any] func(a, b T) bool

// Generic sort function
func sortResources[T any](res []T, less LessFunc[T]) {
sort.Slice(res, func(i, j int) bool {
return less(res[i], res[j])
})
}

// Less functions for each resource type and address family
func lessNetworkResourceIPv4(a, b *structs.NetworkResource) bool {
return net.ParseIP(a.IP).To4() != nil && net.ParseIP(b.IP).To4() == nil
}

func lessNetworkResourceIPv6(a, b *structs.NetworkResource) bool {
return net.ParseIP(a.IP).To4() == nil && net.ParseIP(b.IP).To4() != nil
}

func lessNodeNetworkResourceIPv4(a, b *structs.NodeNetworkResource) bool {
if len(a.Addresses) == 0 && len(b.Addresses) == 0 {
return false
} else if len(a.Addresses) == 0 {
return false
} else if len(b.Addresses) == 0 {
return true
} else if a.Addresses[0].Family == structs.NodeNetworkAF_IPv4 && b.Addresses[0].Family == structs.NodeNetworkAF_IPv6 {
return true
}
return false
}

func lessNodeNetworkResourceIPv6(a, b *structs.NodeNetworkResource) bool {
if len(a.Addresses) == 0 {
return false
} else if len(b.Addresses) == 0 {
return true
}
return a.Addresses[0].Family == structs.NodeNetworkAF_IPv6 && b.Addresses[0].Family == structs.NodeNetworkAF_IPv4
}

func lessNodeNetworkAddressIPv4(a, b structs.NodeNetworkAddress) bool {
return a.Family == structs.NodeNetworkAF_IPv4 && b.Family == structs.NodeNetworkAF_IPv6
}

func lessNodeNetworkAddressIPv6(a, b structs.NodeNetworkAddress) bool {
return a.Family == structs.NodeNetworkAF_IPv6 && b.Family == structs.NodeNetworkAF_IPv4
}

// Sorting functions for different resource types and address families
func sortNetworkResources(res []*structs.NetworkResource, preferredAF structs.NodeNetworkAF) {
if preferredAF == structs.NodeNetworkAF_IPv4 {
sortResources(res, lessNetworkResourceIPv4)
} else if preferredAF == structs.NodeNetworkAF_IPv6 {
sortResources(res, lessNetworkResourceIPv6)
}
}

func sortNodeNetworkResources(res []*structs.NodeNetworkResource, preferredAF structs.NodeNetworkAF) {
if preferredAF == structs.NodeNetworkAF_IPv4 {
sortResources(res, lessNodeNetworkResourceIPv4)
} else if preferredAF == structs.NodeNetworkAF_IPv6 {
sortResources(res, lessNodeNetworkResourceIPv6)
}
}

func sortNodeNetworkAddresses(res []structs.NodeNetworkAddress, preferredAF structs.NodeNetworkAF) {
if preferredAF == structs.NodeNetworkAF_IPv4 {
sortResources(res, lessNodeNetworkAddressIPv4)
} else if preferredAF == structs.NodeNetworkAF_IPv6 {
sortResources(res, lessNodeNetworkAddressIPv6)
}
}
112 changes: 72 additions & 40 deletions client/fingerprint/network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"net"
"os"
"sort"
"strings"
"testing"

"github.com/hashicorp/nomad/ci"
Expand Down Expand Up @@ -73,6 +74,13 @@ var (
}
)

const (
loIPv4 = "127.0.0.1"
loCIDRv4 = loIPv4 + "/8"
loIPv6 = "2001:DB8::"
loCIDRv6 = loIPv6 + "/48"
)

// A fake network detector which returns no devices
type NetworkInterfaceDetectorNoDevices struct {
}
Expand Down Expand Up @@ -107,8 +115,8 @@ func (n *NetworkInterfaceDetectorOnlyLo) InterfaceByName(name string) (*net.Inte

func (n *NetworkInterfaceDetectorOnlyLo) Addrs(intf *net.Interface) ([]net.Addr, error) {
if intf.Name == "lo" {
_, ipnet1, _ := net.ParseCIDR("127.0.0.1/8")
_, ipnet2, _ := net.ParseCIDR("2001:DB8::/48")
_, ipnet1, _ := net.ParseCIDR(loCIDRv6)
_, ipnet2, _ := net.ParseCIDR(loCIDRv4)
return []net.Addr{ipnet1, ipnet2}, nil
}

Expand Down Expand Up @@ -149,8 +157,8 @@ func (n *NetworkInterfaceDetectorMultipleInterfaces) InterfaceByName(name string

func (n *NetworkInterfaceDetectorMultipleInterfaces) Addrs(intf *net.Interface) ([]net.Addr, error) {
if intf.Name == "lo" {
_, ipnet1, _ := net.ParseCIDR("127.0.0.1/8")
_, ipnet2, _ := net.ParseCIDR("2001:DB8::/48")
_, ipnet1, _ := net.ParseCIDR(loCIDRv6)
_, ipnet2, _ := net.ParseCIDR(loCIDRv4)
return []net.Addr{ipnet1, ipnet2}, nil
}

Expand Down Expand Up @@ -269,54 +277,78 @@ func TestNetworkFingerprint_default_device_absent(t *testing.T) {

func TestNetworkFingerPrint_default_device(t *testing.T) {
ci.Parallel(t)
gulducat marked this conversation as resolved.
Show resolved Hide resolved

f := &NetworkFingerprint{logger: testlog.HCLogger(t), interfaceDetector: &NetworkInterfaceDetectorOnlyLo{}}
node := &structs.Node{
Attributes: make(map[string]string),
}
cfg := &config.Config{NetworkSpeed: 100, NetworkInterface: "lo"}

request := &FingerprintRequest{Config: cfg, Node: node}
var response FingerprintResponse
err := f.Fingerprint(request, &response)
if err != nil {
t.Fatalf("err: %v", err)
testCases := []struct {
name string
config *config.Config
expectedCIDR string
}{
{
name: "Loopback IPv6",
config: &config.Config{NetworkSpeed: 100, NetworkInterface: "lo", PreferredAddressFamily: "ipv6"},
gulducat marked this conversation as resolved.
Show resolved Hide resolved
expectedCIDR: loCIDRv6,
},
{
name: "Loopback IPv4",
config: &config.Config{NetworkSpeed: 100, NetworkInterface: "lo", PreferredAddressFamily: "ipv4"},
expectedCIDR: loCIDRv4,
},
{
name: "Loopback No preferred address family",
config: &config.Config{NetworkSpeed: 100, NetworkInterface: "lo"},
expectedCIDR: loCIDRv6,
},
}

if !response.Detected {
t.Fatalf("expected response to be applicable")
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
request := &FingerprintRequest{Config: tc.config, Node: node}
var response FingerprintResponse
err := f.Fingerprint(request, &response)
if err != nil {
t.Fatalf("err: %v", err)
}

attributes := response.Attributes
if len(attributes) == 0 {
t.Fatalf("should apply")
}
if !response.Detected {
t.Fatalf("expected response to be applicable")
}

assertNodeAttributeContains(t, attributes, "unique.network.ip-address")
attributes := response.Attributes
if len(attributes) == 0 {
t.Fatalf("should apply")
}

ip := attributes["unique.network.ip-address"]
match := net.ParseIP(ip)
if match == nil {
t.Fatalf("Bad IP match: %s", ip)
}
assertNodeAttributeContains(t, attributes, "unique.network.ip-address")

if len(response.NodeResources.Networks) == 0 {
t.Fatal("Expected to find Network Resources")
}
ip := attributes["unique.network.ip-address"]
match := net.ParseIP(ip)
if match == nil {
t.Fatalf("Bad IP match: %s", ip)
}

// Test at least the first Network Resource
net := response.NodeResources.Networks[0]
if net.IP == "" {
t.Fatal("Expected Network Resource to not be empty")
}
if net.CIDR == "" {
t.Fatal("Expected Network Resource to have a CIDR")
}
if net.Device == "" {
t.Fatal("Expected Network Resource to have a Device Name")
}
if net.MBits == 0 {
t.Fatal("Expected Network Resource to have a non-zero bandwidth")
if len(response.NodeResources.Networks) == 0 {
t.Fatal("Expected to find Network Resources")
}

// Test at least the first Network Resource
net := response.NodeResources.Networks[0]
if net.IP == "" {
t.Fatal("Expected Network Resource to not be empty")
}
if strings.ToUpper(net.CIDR) == strings.ToUpper(tc.expectedCIDR) {
t.Errorf("Expected CIDR %s; got %s", tc.expectedCIDR, net.IP)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't this be an error if the CIDRs are not equal? also the error output shows net.IP which confused me for a sec.

Suggested change
if strings.ToUpper(net.CIDR) == strings.ToUpper(tc.expectedCIDR) {
t.Errorf("Expected CIDR %s; got %s", tc.expectedCIDR, net.IP)
if strings.ToUpper(net.CIDR) != strings.ToUpper(tc.expectedCIDR) {
t.Errorf("Expected CIDR %s; got %s", tc.expectedCIDR, net.CIDR)

that causes the test to fail:

=== RUN   TestNetworkFingerPrint_default_device/Loopback_IPv6
    network_test.go:343: Expected CIDR 2001:DB8::/48; got 2001:db8::/128

=== RUN   TestNetworkFingerPrint_default_device/Loopback_IPv4
    network_test.go:343: Expected CIDR 127.0.0.1/8; got 127.0.0.0/32

=== RUN   TestNetworkFingerPrint_default_device/Loopback_No_preferred_address_family
    network_test.go:343: Expected CIDR 2001:DB8::/48; got 2001:db8::/128

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!
I'm a bit confused about how CIDR 127.0.0.1/8 result in IP address 127.0.0.0 instead of 127.0.0.1

I'm going to compare the IP instead of the CIDR. In the end, we want to make sure they are on the expected family.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like 127.0.0.0 is coming from the test NetworkInterfaceDetectorOnlyLo's usage of net.ParseCIDR

It returns the IP address and the network implied by the IP and prefix length. For example, ParseCIDR("192.0.2.1/24") returns the IP address 192.0.2.1 and the network 192.0.2.0/24.

the mockish test interface implementation is only using the IPNet and discarding IP.

I agree using the IP for this test is fine, since it does confirm the by-family sorting behavior we're expecting to see. 👍

}
if net.Device == "" {
t.Fatal("Expected Network Resource to have a Device Name")
}
if net.MBits == 0 {
t.Fatal("Expected Network Resource to have a non-zero bandwidth")
}
})
}
}

Expand Down
9 changes: 9 additions & 0 deletions client/taskenv/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -950,7 +950,16 @@ func buildNetworkEnv(envMap map[string]string, nets structs.Networks, driverNet
func buildPortEnv(envMap map[string]string, p structs.Port, ip string, driverNet *drivers.DriverNetwork) {
// Host IP, port, and address
portStr := strconv.Itoa(p.Value)

var ipFamilyPrefix string
if strings.Contains(ip, ":") {
ipFamilyPrefix = "NOMAD_IPv6_"
} else {
ipFamilyPrefix = "NOMAD_IPv4_"
}

envMap[IpPrefix+p.Label] = ip
envMap[ipFamilyPrefix+p.Label] = ip
envMap[HostPortPrefix+p.Label] = portStr
envMap[AddrPrefix+p.Label] = net.JoinHostPort(ip, portStr)

Expand Down
2 changes: 2 additions & 0 deletions client/taskenv/env_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,9 +219,11 @@ func TestEnvironment_AsList(t *testing.T) {
"NOMAD_ADDR_http=127.0.0.1:80",
"NOMAD_PORT_http=80",
"NOMAD_IP_http=127.0.0.1",
"NOMAD_IPv4_http=127.0.0.1",
"NOMAD_ADDR_https=127.0.0.1:8080",
"NOMAD_PORT_https=443",
"NOMAD_IP_https=127.0.0.1",
"NOMAD_IPv4_https=127.0.0.1",
"NOMAD_HOST_PORT_http=80",
"NOMAD_HOST_PORT_https=8080",
"NOMAD_TASK_NAME=web",
Expand Down
3 changes: 3 additions & 0 deletions command/agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -737,6 +737,9 @@ func convertClientConfig(agentConfig *Config) (*clientconfig.Config, error) {
if agentConfig.Client.NetworkInterface != "" {
conf.NetworkInterface = agentConfig.Client.NetworkInterface
}

conf.PreferredAddressFamily = agentConfig.Client.PreferredAddressFamily

conf.ChrootEnv = agentConfig.Client.ChrootEnv
conf.Options = agentConfig.Client.Options
if agentConfig.Client.NetworkSpeed != 0 {
Expand Down
14 changes: 14 additions & 0 deletions command/agent/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ func (c *Command) readConfig() *Config {
flags.StringVar(&servers, "servers", "", "")
flags.Var((*flaghelper.StringFlag)(&meta), "meta", "")
flags.StringVar(&cmdConfig.Client.NetworkInterface, "network-interface", "", "")
flags.StringVar((*string)(&cmdConfig.Client.PreferredAddressFamily), "preferred-address-family", "", "ipv4 or ipv6")
flags.IntVar(&cmdConfig.Client.NetworkSpeed, "network-speed", 0, "")

// General options
Expand Down Expand Up @@ -485,6 +486,14 @@ func (c *Command) IsValidConfig(config, cmdConfig *Config) bool {
return false
}

if err := config.Client.PreferredAddressFamily.Validate(); err != nil {
c.Ui.Error(fmt.Sprintf("Invalid preferred-address-family value: %s (valid values: %s, %s)",
config.Client.PreferredAddressFamily,
structs.NodeNetworkAF_IPv4, structs.NodeNetworkAF_IPv6),
)
return false
}

if !config.DevMode {
// Ensure that we have the directories we need to run.
if config.Server.Enabled && config.DataDir == "" {
Expand Down Expand Up @@ -1549,6 +1558,11 @@ Client Options:

-network-interface
Forces the network fingerprinter to use the specified network interface.

-preferred-address-family
Specify which IP family to prefer when selecting an IP address of the
network interface. Valid values are "ipv4" and "ipv6". When not specified,
the agent will not sort the addresses and use the first one.

-network-speed
The default speed for network interfaces in MBits if the link speed can not
Expand Down
Loading