From a301bbf727b5d87bc8ff3c053c7280877ff5239b Mon Sep 17 00:00:00 2001 From: AndrewChubatiuk Date: Mon, 1 Mar 2021 12:37:52 +0200 Subject: [PATCH 1/5] added multiple host network aliases for the same interface --- client/fingerprint/network.go | 18 +++++++++++------- nomad/mock/mock.go | 2 +- nomad/structs/network.go | 4 +++- nomad/structs/structs.go | 10 ++++++---- scheduler/system_sched_test.go | 2 +- 5 files changed, 22 insertions(+), 14 deletions(-) diff --git a/client/fingerprint/network.go b/client/fingerprint/network.go index 1c77a7657b39..1f03a734edee 100644 --- a/client/fingerprint/network.go +++ b/client/fingerprint/network.go @@ -171,10 +171,10 @@ func (f *NetworkFingerprint) createNodeNetworkResources(ifaces []net.Interface, newAddr := structs.NodeNetworkAddress{ Address: ip.String(), Family: family, - Alias: deriveAddressAlias(iface, ip, conf), + Aliases: deriveAddressAliases(iface, ip, conf), } - if newAddr.Alias != "" { + if len(newAddr.Aliases) > 0 { if ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() { linkLocalAddrs = append(linkLocalAddrs, newAddr) } else { @@ -200,7 +200,7 @@ func (f *NetworkFingerprint) createNodeNetworkResources(ifaces []net.Interface, return nets, nil } -func deriveAddressAlias(iface net.Interface, addr net.IP, config *config.Config) string { +func deriveAddressAliases(iface net.Interface, addr net.IP, config *config.Config) (aliases []string) { for name, conf := range config.HostNetworks { var cidrMatch, ifaceMatch bool if conf.CIDR != "" { @@ -231,22 +231,26 @@ func deriveAddressAlias(iface net.Interface, addr net.IP, config *config.Config) ifaceMatch = true } if cidrMatch && ifaceMatch { - return name + aliases = append(aliases, name) } } + if len(aliases) > 0 { + return + } + if config.NetworkInterface != "" { if config.NetworkInterface == iface.Name { - return "default" + return []string{"default"} } } else if ri, err := sockaddr.NewRouteInfo(); err == nil { defaultIface, err := ri.GetDefaultInterfaceName() if err == nil && iface.Name == defaultIface { - return "default" + return []string{"default"} } } - return "" + return } // createNetworkResources creates network resources for every IP diff --git a/nomad/mock/mock.go b/nomad/mock/mock.go index bf80e154e8fb..d14e8e272033 100644 --- a/nomad/mock/mock.go +++ b/nomad/mock/mock.go @@ -80,7 +80,7 @@ func Node() *structs.Node { Speed: 1000, Addresses: []structs.NodeNetworkAddress{ { - Alias: "default", + Aliases: []string{"default"}, Address: "192.168.0.100", Family: structs.NodeNetworkAF_IPv4, }, diff --git a/nomad/structs/network.go b/nomad/structs/network.go index 88a629abf8b4..7a62e77ef0e5 100644 --- a/nomad/structs/network.go +++ b/nomad/structs/network.go @@ -117,7 +117,9 @@ func (idx *NetworkIndex) SetNode(node *Node) (collide bool) { // is it possible to get duplicates here? for _, n := range nodeNetworks { for _, a := range n.Addresses { - idx.AvailAddresses[a.Alias] = append(idx.AvailAddresses[a.Alias], a) + for _, alias := range a.Aliases { + idx.AvailAddresses[alias] = append(idx.AvailAddresses[alias], a) + } if idx.AddReservedPortsForIP(a.ReservedPorts, a.Address) { collide = true } diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 6f1b110401a6..6a398d2adfe4 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -1963,7 +1963,7 @@ func (n *Node) Canonicalize() { if nr.IP != "" { nnr.Addresses = []NodeNetworkAddress{ { - Alias: "default", + Aliases: []string{"default"}, Address: nr.IP, }, } @@ -2442,8 +2442,10 @@ func (n *NodeNetworkResource) Equals(o *NodeNetworkResource) bool { func (n *NodeNetworkResource) HasAlias(alias string) bool { for _, addr := range n.Addresses { - if addr.Alias == alias { - return true + for _, a := range addr.Aliases { + if a == alias { + return true + } } } return false @@ -2458,7 +2460,7 @@ const ( type NodeNetworkAddress struct { Family NodeNetworkAF - Alias string + Aliases []string Address string ReservedPorts string Gateway string // default route for this address diff --git a/scheduler/system_sched_test.go b/scheduler/system_sched_test.go index c4cce45ffcc9..142f71c41f56 100644 --- a/scheduler/system_sched_test.go +++ b/scheduler/system_sched_test.go @@ -1897,7 +1897,7 @@ func TestSystemSched_Preemption(t *testing.T) { Addresses: []structs.NodeNetworkAddress{ { Family: structs.NodeNetworkAF_IPv4, - Alias: "default", + Aliases: []string{"default"}, Address: "192.168.0.100", }, }, From 034dd3a32770e10fe4e3b18922c9428b6c49d083 Mon Sep 17 00:00:00 2001 From: AndrewChubatiuk Date: Wed, 24 Mar 2021 23:24:53 +0200 Subject: [PATCH 2/5] create multiple NodeNetworkAddress instead of aliases --- client/fingerprint/network.go | 22 ++++++++++++---------- nomad/mock/mock.go | 2 +- nomad/structs/network.go | 4 +--- nomad/structs/structs.go | 10 ++++------ 4 files changed, 18 insertions(+), 20 deletions(-) diff --git a/client/fingerprint/network.go b/client/fingerprint/network.go index 1f03a734edee..b36a54826840 100644 --- a/client/fingerprint/network.go +++ b/client/fingerprint/network.go @@ -168,17 +168,19 @@ func (f *NetworkFingerprint) createNodeNetworkResources(ifaces []net.Interface, } else { family = structs.NodeNetworkAF_IPv6 } - newAddr := structs.NodeNetworkAddress{ - Address: ip.String(), - Family: family, - Aliases: deriveAddressAliases(iface, ip, conf), - } + for _, alias := range deriveAddressAliases(iface, ip, conf) { + newAddr := structs.NodeNetworkAddress{ + Address: ip.String(), + Family: family, + Alias: alias, + } - if len(newAddr.Aliases) > 0 { - if ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() { - linkLocalAddrs = append(linkLocalAddrs, newAddr) - } else { - networkAddrs = append(networkAddrs, newAddr) + if newAddr.Alias != "" { + if ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() { + linkLocalAddrs = append(linkLocalAddrs, newAddr) + } else { + networkAddrs = append(networkAddrs, newAddr) + } } } } diff --git a/nomad/mock/mock.go b/nomad/mock/mock.go index d14e8e272033..bf80e154e8fb 100644 --- a/nomad/mock/mock.go +++ b/nomad/mock/mock.go @@ -80,7 +80,7 @@ func Node() *structs.Node { Speed: 1000, Addresses: []structs.NodeNetworkAddress{ { - Aliases: []string{"default"}, + Alias: "default", Address: "192.168.0.100", Family: structs.NodeNetworkAF_IPv4, }, diff --git a/nomad/structs/network.go b/nomad/structs/network.go index 7a62e77ef0e5..88a629abf8b4 100644 --- a/nomad/structs/network.go +++ b/nomad/structs/network.go @@ -117,9 +117,7 @@ func (idx *NetworkIndex) SetNode(node *Node) (collide bool) { // is it possible to get duplicates here? for _, n := range nodeNetworks { for _, a := range n.Addresses { - for _, alias := range a.Aliases { - idx.AvailAddresses[alias] = append(idx.AvailAddresses[alias], a) - } + idx.AvailAddresses[a.Alias] = append(idx.AvailAddresses[a.Alias], a) if idx.AddReservedPortsForIP(a.ReservedPorts, a.Address) { collide = true } diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 6a398d2adfe4..22d10c5001ee 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -1963,7 +1963,7 @@ func (n *Node) Canonicalize() { if nr.IP != "" { nnr.Addresses = []NodeNetworkAddress{ { - Aliases: []string{"default"}, + Alias: "default", Address: nr.IP, }, } @@ -2442,10 +2442,8 @@ func (n *NodeNetworkResource) Equals(o *NodeNetworkResource) bool { func (n *NodeNetworkResource) HasAlias(alias string) bool { for _, addr := range n.Addresses { - for _, a := range addr.Aliases { - if a == alias { - return true - } + for addr.Alias == alias { + return true } } return false @@ -2460,7 +2458,7 @@ const ( type NodeNetworkAddress struct { Family NodeNetworkAF - Aliases []string + Alias string Address string ReservedPorts string Gateway string // default route for this address From 8cb8308d6299f3fef84f1902702bdc4c2b865d93 Mon Sep 17 00:00:00 2001 From: AndrewChubatiuk Date: Wed, 24 Mar 2021 23:31:25 +0200 Subject: [PATCH 3/5] minor fix --- nomad/structs/structs.go | 2 +- scheduler/system_sched_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 22d10c5001ee..6f1b110401a6 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -2442,7 +2442,7 @@ func (n *NodeNetworkResource) Equals(o *NodeNetworkResource) bool { func (n *NodeNetworkResource) HasAlias(alias string) bool { for _, addr := range n.Addresses { - for addr.Alias == alias { + if addr.Alias == alias { return true } } diff --git a/scheduler/system_sched_test.go b/scheduler/system_sched_test.go index 142f71c41f56..c4cce45ffcc9 100644 --- a/scheduler/system_sched_test.go +++ b/scheduler/system_sched_test.go @@ -1897,7 +1897,7 @@ func TestSystemSched_Preemption(t *testing.T) { Addresses: []structs.NodeNetworkAddress{ { Family: structs.NodeNetworkAF_IPv4, - Aliases: []string{"default"}, + Alias: "default", Address: "192.168.0.100", }, }, From 83b48268169b20da4a6c0caeafdb9a983a295e13 Mon Sep 17 00:00:00 2001 From: AndrewChubatiuk Date: Thu, 25 Mar 2021 00:41:46 +0200 Subject: [PATCH 4/5] added multiple aliases test --- client/fingerprint/network_test.go | 51 ++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/client/fingerprint/network_test.go b/client/fingerprint/network_test.go index 5b55b87a6904..70388db75a68 100644 --- a/client/fingerprint/network_test.go +++ b/client/fingerprint/network_test.go @@ -437,3 +437,54 @@ func TestNetworkFingerPrint_LinkLocal_Disallowed(t *testing.T) { t.Fatalf("should not apply attributes") } } + +func TestNetworkFingerPrint_MultipleAliases(t *testing.T) { + f := &NetworkFingerprint{logger: testlog.HCLogger(t), interfaceDetector: &NetworkInterfaceDetectorMultipleInterfaces{}} + node := &structs.Node{ + Attributes: make(map[string]string), + } + cfg := &config.Config{ + NetworkSpeed: 100, + NetworkInterface: "eth3", + HostNetworks: map[string]*structs.ClientHostNetworkConfig{ + "alias1": { + Name: "alias1", + Interface: "eth3", + CIDR: "169.254.155.20/32", + }, + "alias2": { + Name: "alias2", + Interface: "eth3", + CIDR: "169.254.155.20/32", + }, + "alias3": { + Name: "alias3", + Interface: "eth0", + CIDR: "100.64.0.11/10", + }, + }, + } + + request := &FingerprintRequest{Config: cfg, Node: node} + var response FingerprintResponse + err := f.Fingerprint(request, &response) + if err != nil { + t.Fatalf("err: %v", err) + } + + aliases := []string{} + + for _, network := range response.NodeResources.NodeNetworks { + for _, address := range network.Addresses { + aliases = append(aliases, address.Alias) + } + } + + if len(aliases) != len(cfg.HostNetworks) { + actualAliases := []string{} + for alias, _ := range cfg.HostNetworks { + actualAliases = append(actualAliases, alias) + } + t.Fatalf("number host networks is not valid len(%v) != len(%v)", aliases, actualAliases) + } +} From dd838ffff3cd98ab38aae169caef3f0621a76fb8 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Tue, 13 Apr 2021 09:07:04 -0400 Subject: [PATCH 5/5] testing style cleanup --- client/fingerprint/network_test.go | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/client/fingerprint/network_test.go b/client/fingerprint/network_test.go index 70388db75a68..4b0fce93dcc0 100644 --- a/client/fingerprint/network_test.go +++ b/client/fingerprint/network_test.go @@ -4,11 +4,13 @@ import ( "fmt" "net" "os" + "sort" "testing" "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/helper/testlog" "github.com/hashicorp/nomad/nomad/structs" + "github.com/stretchr/testify/require" ) // Set skipOnlineTestEnvVar to a non-empty value to skip network tests. Useful @@ -468,23 +470,19 @@ func TestNetworkFingerPrint_MultipleAliases(t *testing.T) { request := &FingerprintRequest{Config: cfg, Node: node} var response FingerprintResponse err := f.Fingerprint(request, &response) - if err != nil { - t.Fatalf("err: %v", err) - } + require.NoError(t, err) aliases := []string{} - for _, network := range response.NodeResources.NodeNetworks { for _, address := range network.Addresses { aliases = append(aliases, address.Alias) } } - - if len(aliases) != len(cfg.HostNetworks) { - actualAliases := []string{} - for alias, _ := range cfg.HostNetworks { - actualAliases = append(actualAliases, alias) - } - t.Fatalf("number host networks is not valid len(%v) != len(%v)", aliases, actualAliases) + expected := []string{} + for alias, _ := range cfg.HostNetworks { + expected = append(expected, alias) } + sort.Strings(expected) + sort.Strings(aliases) + require.Equal(t, expected, aliases, "host networks should match aliases") }