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

networking: option to enable ipv6 on bridge network #23882

Merged
merged 3 commits into from
Sep 4, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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 .changelog/23882.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
networking: IPv6 can now be enabled on the Nomad bridge network mode
```
5 changes: 5 additions & 0 deletions client/allocrunner/cni/bridge.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ type NomadBridgeConfig struct {
BridgeName string
AdminChainName string
IPv4Subnet string
IPv6Subnet string
HairpinMode bool
ConsulCNI bool
}
Expand All @@ -40,6 +41,10 @@ func NewNomadBridgeConflist(conf NomadBridgeConfig) Conflist {
ipRoutes := []Route{
{Dst: "0.0.0.0/0"},
}
if conf.IPv6Subnet != "" {
ipRanges = append(ipRanges, []Range{{Subnet: conf.IPv6Subnet}})
ipRoutes = append(ipRoutes, Route{Dst: "::/0"})
Comment on lines +45 to +46
Copy link
Member Author

Choose a reason for hiding this comment

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

these are the meat and potatoes; everything else revolves around this.

}

plugins := []any{
Generic{
Expand Down
5 changes: 4 additions & 1 deletion client/allocrunner/network_manager_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,10 @@ func newNetworkConfigurator(log hclog.Logger, alloc *structs.Allocation, config

switch {
case netMode == "bridge":
c, err := newBridgeNetworkConfigurator(log, alloc, config.BridgeNetworkName, config.BridgeNetworkAllocSubnet, config.BridgeNetworkHairpinMode, config.CNIPath, ignorePortMappingHostIP, config.Node)
c, err := newBridgeNetworkConfigurator(log, alloc,
config.BridgeNetworkName, config.BridgeNetworkAllocSubnet, config.BridgeNetworkAllocSubnetIPv6, config.CNIPath,
config.BridgeNetworkHairpinMode, ignorePortMappingHostIP,
config.Node)
if err != nil {
return nil, err
}
Expand Down
41 changes: 27 additions & 14 deletions client/allocrunner/networking_bridge_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,31 +31,33 @@ const (
// shared bridge, configures masquerading for egress traffic and port mapping
// for ingress
type bridgeNetworkConfigurator struct {
cni *cniNetworkConfigurator
allocSubnet string
bridgeName string
hairpinMode bool
cni *cniNetworkConfigurator
allocSubnetIPv6 string
allocSubnetIPv4 string
bridgeName string
hairpinMode bool

newIPTables func(structs.NodeNetworkAF) (IPTablesChain, error)

logger hclog.Logger
}

func newBridgeNetworkConfigurator(log hclog.Logger, alloc *structs.Allocation, bridgeName, ipRange string, hairpinMode bool, cniPath string, ignorePortMappingHostIP bool, node *structs.Node) (*bridgeNetworkConfigurator, error) {
func newBridgeNetworkConfigurator(log hclog.Logger, alloc *structs.Allocation, bridgeName, ipv4Range, ipv6Range, cniPath string, hairpinMode, ignorePortMappingHostIP bool, node *structs.Node) (*bridgeNetworkConfigurator, error) {
b := &bridgeNetworkConfigurator{
bridgeName: bridgeName,
allocSubnet: ipRange,
hairpinMode: hairpinMode,
newIPTables: newIPTablesChain,
logger: log,
bridgeName: bridgeName,
hairpinMode: hairpinMode,
allocSubnetIPv4: ipv4Range,
allocSubnetIPv6: ipv6Range,
newIPTables: newIPTablesChain,
logger: log,
}

if b.bridgeName == "" {
b.bridgeName = defaultNomadBridgeName
}

if b.allocSubnet == "" {
b.allocSubnet = defaultNomadAllocSubnet
if b.allocSubnetIPv4 == "" {
b.allocSubnetIPv4 = defaultNomadAllocSubnet
}

var netCfg []byte
Expand Down Expand Up @@ -95,12 +97,22 @@ func newBridgeNetworkConfigurator(log hclog.Logger, alloc *structs.Allocation, b
// ensureForwardingRules ensures that a forwarding rule is added to iptables
// to allow traffic inbound to the bridge network
func (b *bridgeNetworkConfigurator) ensureForwardingRules() error {
if b.allocSubnetIPv6 != "" {
ip6t, err := b.newIPTables(structs.NodeNetworkAF_IPv6)
if err != nil {
return err
}
if err = ensureChainRule(ip6t, b.bridgeName, b.allocSubnetIPv6); err != nil {
return err
}
}

ipt, err := b.newIPTables(structs.NodeNetworkAF_IPv4)
if err != nil {
return err
}

if err = ensureChainRule(ipt, b.bridgeName, b.allocSubnet); err != nil {
if err = ensureChainRule(ipt, b.bridgeName, b.allocSubnetIPv4); err != nil {
return err
}

Expand All @@ -125,7 +137,8 @@ func buildNomadBridgeNetConfig(b bridgeNetworkConfigurator, withConsulCNI bool)
conf := cni.NewNomadBridgeConflist(cni.NomadBridgeConfig{
BridgeName: b.bridgeName,
AdminChainName: cniAdminChainName,
IPv4Subnet: b.allocSubnet,
IPv4Subnet: b.allocSubnetIPv4,
IPv6Subnet: b.allocSubnetIPv6,
HairpinMode: b.hairpinMode,
ConsulCNI: withConsulCNI,
})
Expand Down
147 changes: 138 additions & 9 deletions client/allocrunner/networking_bridge_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,17 @@ package allocrunner

import (
"encoding/json"
"errors"
"fmt"
"os"
"path/filepath"
"testing"

"github.com/coreos/go-iptables/iptables"
"github.com/hashicorp/go-hclog"
"github.com/hashicorp/nomad/ci"
"github.com/hashicorp/nomad/nomad/mock"
"github.com/hashicorp/nomad/nomad/structs"
"github.com/shoenig/test/must"
)

Expand All @@ -25,29 +31,37 @@ func Test_buildNomadBridgeNetConfig(t *testing.T) {
b: &bridgeNetworkConfigurator{},
},

{
name: "ipv6",
b: &bridgeNetworkConfigurator{
bridgeName: defaultNomadBridgeName,
allocSubnetIPv6: "3fff:cab0:0d13::/120",
allocSubnetIPv4: defaultNomadAllocSubnet,
},
},
{
name: "hairpin",
b: &bridgeNetworkConfigurator{
bridgeName: defaultNomadBridgeName,
allocSubnet: defaultNomadAllocSubnet,
hairpinMode: true,
bridgeName: defaultNomadBridgeName,
allocSubnetIPv4: defaultNomadAllocSubnet,
hairpinMode: true,
},
},
{
name: "bad_input",
b: &bridgeNetworkConfigurator{
bridgeName: `bad"`,
allocSubnet: defaultNomadAllocSubnet,
hairpinMode: true,
bridgeName: `bad"`,
allocSubnetIPv4: defaultNomadAllocSubnet,
hairpinMode: true,
},
},
{
name: "consul-cni",
withConsulCNI: true,
b: &bridgeNetworkConfigurator{
bridgeName: defaultNomadBridgeName,
allocSubnet: defaultNomadAllocSubnet,
hairpinMode: true,
bridgeName: defaultNomadBridgeName,
allocSubnetIPv4: defaultNomadAllocSubnet,
hairpinMode: true,
},
},
}
Expand All @@ -67,3 +81,118 @@ func Test_buildNomadBridgeNetConfig(t *testing.T) {
})
}
}

func TestBridgeNetworkConfigurator_newIPTables_default(t *testing.T) {
t.Parallel()

b, err := newBridgeNetworkConfigurator(hclog.Default(),
mock.MinAlloc(),
"", "", "", "",
false, false,
mock.Node())
must.NoError(t, err)

for family, expect := range map[structs.NodeNetworkAF]iptables.Protocol{
"ipv6": iptables.ProtocolIPv6,
"ipv4": iptables.ProtocolIPv4,
"other": iptables.ProtocolIPv4,
} {
t.Run(string(family), func(t *testing.T) {
mgr, err := b.newIPTables(family)
must.NoError(t, err)
ipt := mgr.(*iptables.IPTables)
must.Eq(t, expect, ipt.Proto(), must.Sprint("unexpected ip family"))
})
}
}

func TestBridgeNetworkConfigurator_ensureForwardingRules(t *testing.T) {
t.Parallel()

newMockIPTables := func(b *bridgeNetworkConfigurator) (*mockIPTablesChain, *mockIPTablesChain) {
ipt := &mockIPTablesChain{}
ip6t := &mockIPTablesChain{}
b.newIPTables = func(fam structs.NodeNetworkAF) (IPTablesChain, error) {
switch fam {
case "ipv6":
return ip6t, nil
case "ipv4":
return ipt, nil
}
return nil, fmt.Errorf("unknown fam %q in newMockIPTables", fam)
}
return ipt, ip6t
}

cases := []struct {
name string
bridgeName, ip4, ip6 string
expectIP4Rules []string
expectIP6Rules []string
ip4Err, ip6Err error
}{
{
name: "defaults",
expectIP4Rules: []string{"-o", defaultNomadBridgeName, "-d", defaultNomadAllocSubnet, "-j", "ACCEPT"},
},
{
name: "configured",
bridgeName: "golden-gate",
ip4: "a.b.c.d/z",
ip6: "aa:bb:cc:dd/z",
expectIP4Rules: []string{"-o", "golden-gate", "-d", "a.b.c.d/z", "-j", "ACCEPT"},
expectIP6Rules: []string{"-o", "golden-gate", "-d", "aa:bb:cc:dd/z", "-j", "ACCEPT"},
},
{
name: "ip4error",
ip4Err: errors.New("test ip4error"),
},
{
name: "ip6error",
ip6: "aa:bb:cc:dd/z",
ip6Err: errors.New("test ip6error"),
},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
b, err := newBridgeNetworkConfigurator(hclog.Default(),
mock.MinAlloc(),
tc.bridgeName, tc.ip4, tc.ip6, "",
false, false,
mock.Node())
must.NoError(t, err)

ipt, ip6t := newMockIPTables(b)
ipt.newChainErr = tc.ip4Err
ip6t.newChainErr = tc.ip6Err

// method under test
err = b.ensureForwardingRules()

if tc.ip6Err != nil {
must.ErrorIs(t, err, tc.ip6Err)
return
}
if tc.ip4Err != nil {
must.ErrorIs(t, err, tc.ip4Err)
return
}
must.NoError(t, err)

must.Eq(t, ipt.chain, cniAdminChainName)
must.Eq(t, ipt.table, "filter")
must.Eq(t, ipt.rules, tc.expectIP4Rules)

if tc.expectIP6Rules != nil {
must.Eq(t, ip6t.chain, cniAdminChainName)
must.Eq(t, ip6t.table, "filter")
must.Eq(t, ip6t.rules, tc.expectIP6Rules)
} else {
must.Eq(t, "", ip6t.chain, must.Sprint("expect empty ip6tables chain"))
must.Eq(t, "", ip6t.table, must.Sprint("expect empty ip6tables table"))
must.Len(t, 0, ip6t.rules, must.Sprint("expect empty ip6tables rules"))
}
})
}
}
17 changes: 15 additions & 2 deletions client/allocrunner/networking_cni.go
Original file line number Diff line number Diff line change
Expand Up @@ -513,8 +513,20 @@ func (c *cniNetworkConfigurator) Teardown(ctx context.Context, alloc *structs.Al
portMap := getPortMapping(alloc, c.ignorePortMappingHostIP)

if err := c.cni.Remove(ctx, alloc.ID, spec.Path, cni.WithCapabilityPortMap(portMap.ports)); err != nil {
c.logger.Warn("error from cni.Remove; attempting manual iptables cleanup", "err", err)

// best effort cleanup ipv6
ipt, iptErr := c.newIPTables(structs.NodeNetworkAF_IPv6)
if iptErr != nil {
c.logger.Debug("failed to detect ip6tables: %v", iptErr)
} else {
if err := c.forceCleanup(ipt, alloc.ID); err != nil {
c.logger.Warn("ip6tables: %v", err)
}
}

// create a real handle to iptables
ipt, iptErr := c.newIPTables(structs.NodeNetworkAF_IPv4)
ipt, iptErr = c.newIPTables(structs.NodeNetworkAF_IPv4)
if iptErr != nil {
return fmt.Errorf("failed to detect iptables: %w", iptErr)
}
Expand Down Expand Up @@ -560,7 +572,8 @@ func (c *cniNetworkConfigurator) forceCleanup(ipt IPTablesCleanup, allocID strin

// no rule found for our allocation, just give up
if ruleToPurge == "" {
return fmt.Errorf("failed to find postrouting rule for alloc %s", allocID)
c.logger.Info("iptables cleanup: did not find postrouting rule for alloc", "alloc_id", allocID)
return nil
}

// re-create the rule we need to delete, as tokens
Expand Down
2 changes: 1 addition & 1 deletion client/allocrunner/networking_cni_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ func TestCNI_forceCleanup(t *testing.T) {
},
}
err := c.forceCleanup(ipt, "2dd71cac-2b1e-ff08-167c-735f7f9f4964")
must.EqError(t, err, "failed to find postrouting rule for alloc 2dd71cac-2b1e-ff08-167c-735f7f9f4964")
must.NoError(t, err, must.Sprint("absent rule should not error"))
})

t.Run("list error", func(t *testing.T) {
Expand Down
52 changes: 52 additions & 0 deletions client/allocrunner/test_fixtures/ipv6.conflist.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
{
"cniVersion": "0.4.0",
"name": "nomad",
"plugins": [
{
"type": "loopback"
},
{
"type": "bridge",
"bridge": "nomad",
"ipMasq": true,
"isGateway": true,
"forceAddress": true,
"hairpinMode": false,
"ipam": {
"type": "host-local",
"ranges": [
[
{
"subnet": "172.26.64.0/20"
}
],
[
{
"subnet": "3fff:cab0:0d13::/120"
}
]
],
"routes": [
{
"dst": "0.0.0.0/0"
},
{
"dst": "::/0"
}
]
}
},
{
"type": "firewall",
"backend": "iptables",
"iptablesAdminChainName": "NOMAD-ADMIN"
},
{
"type": "portmap",
"capabilities": {
"portMappings": true
},
"snat": true
}
]
}
Loading