client: Add option to enable hairpinMode on Nomad bridge (#15961)

* Add `bridge_network_hairpin_mode` client config setting * Add node attribute: `nomad.bridge.hairpin_mode` * Changed format string to use `%q` to escape user provided data * Add test to validate template JSON for developer safety Co-authored-by: Daniel Bennett <[email protected]>
hashicorp · Feb 2, 2023 · 55df5af · 55df5af
1 parent 46f3977
commit 55df5af
Show file tree

Hide file tree

Showing 11 changed files with 184 additions and 100 deletions.
diff --git a/.changelog/15961.txt b/.changelog/15961.txt
@@ -0,0 +1,3 @@
+```release-note:improvement
+client: Add option to enable hairpinMode on Nomad bridge
+```
diff --git a/client/allocrunner/alloc_runner_test.go b/client/allocrunner/alloc_runner_test.go
@@ -524,12 +524,12 @@ func TestAllocRunner_Lifecycle_Restart(t *testing.T) {
 				return ar.RestartAll(ev)
 			},
 			expectedAfter: map[string]structs.TaskState{
-				"main":              structs.TaskState{State: "running", Restarts: 1},
-				"prestart-oneshot":  structs.TaskState{State: "dead", Restarts: 1},
-				"prestart-sidecar":  structs.TaskState{State: "running", Restarts: 1},
-				"poststart-oneshot": structs.TaskState{State: "dead", Restarts: 1},
-				"poststart-sidecar": structs.TaskState{State: "running", Restarts: 1},
-				"poststop":          structs.TaskState{State: "pending", Restarts: 0},
+				"main":              {State: "running", Restarts: 1},
+				"prestart-oneshot":  {State: "dead", Restarts: 1},
+				"prestart-sidecar":  {State: "running", Restarts: 1},
+				"poststart-oneshot": {State: "dead", Restarts: 1},
+				"poststart-sidecar": {State: "running", Restarts: 1},
+				"poststop":          {State: "pending", Restarts: 0},
 			},
 		},
 		{
@@ -538,12 +538,12 @@ func TestAllocRunner_Lifecycle_Restart(t *testing.T) {
 				return ar.RestartRunning(ev)
 			},
 			expectedAfter: map[string]structs.TaskState{
-				"main":              structs.TaskState{State: "running", Restarts: 1},
-				"prestart-oneshot":  structs.TaskState{State: "dead", Restarts: 0},
-				"prestart-sidecar":  structs.TaskState{State: "running", Restarts: 1},
-				"poststart-oneshot": structs.TaskState{State: "dead", Restarts: 0},
-				"poststart-sidecar": structs.TaskState{State: "running", Restarts: 1},
-				"poststop":          structs.TaskState{State: "pending", Restarts: 0},
+				"main":              {State: "running", Restarts: 1},
+				"prestart-oneshot":  {State: "dead", Restarts: 0},
+				"prestart-sidecar":  {State: "running", Restarts: 1},
+				"poststart-oneshot": {State: "dead", Restarts: 0},
+				"poststart-sidecar": {State: "running", Restarts: 1},
+				"poststop":          {State: "pending", Restarts: 0},
 			},
 		},
 		{
@@ -561,12 +561,12 @@ func TestAllocRunner_Lifecycle_Restart(t *testing.T) {
 				return ar.RestartAll(ev)
 			},
 			expectedAfter: map[string]structs.TaskState{
-				"main":              structs.TaskState{State: "running", Restarts: 1},
-				"prestart-oneshot":  structs.TaskState{State: "dead", Restarts: 1},
-				"prestart-sidecar":  structs.TaskState{State: "running", Restarts: 1},
-				"poststart-oneshot": structs.TaskState{State: "dead", Restarts: 1},
-				"poststart-sidecar": structs.TaskState{State: "running", Restarts: 1},
-				"poststop":          structs.TaskState{State: "pending", Restarts: 0},
+				"main":              {State: "running", Restarts: 1},
+				"prestart-oneshot":  {State: "dead", Restarts: 1},
+				"prestart-sidecar":  {State: "running", Restarts: 1},
+				"poststart-oneshot": {State: "dead", Restarts: 1},
+				"poststart-sidecar": {State: "running", Restarts: 1},
+				"poststop":          {State: "pending", Restarts: 0},
 			},
 		},
 		{
@@ -584,12 +584,12 @@ func TestAllocRunner_Lifecycle_Restart(t *testing.T) {
 				return ar.RestartRunning(ev)
 			},
 			expectedAfter: map[string]structs.TaskState{
-				"main":              structs.TaskState{State: "running", Restarts: 1},
-				"prestart-oneshot":  structs.TaskState{State: "dead", Restarts: 0},
-				"prestart-sidecar":  structs.TaskState{State: "running", Restarts: 1},
-				"poststart-oneshot": structs.TaskState{State: "dead", Restarts: 0},
-				"poststart-sidecar": structs.TaskState{State: "running", Restarts: 1},
-				"poststop":          structs.TaskState{State: "pending", Restarts: 0},
+				"main":              {State: "running", Restarts: 1},
+				"prestart-oneshot":  {State: "dead", Restarts: 0},
+				"prestart-sidecar":  {State: "running", Restarts: 1},
+				"poststart-oneshot": {State: "dead", Restarts: 0},
+				"poststart-sidecar": {State: "running", Restarts: 1},
+				"poststop":          {State: "pending", Restarts: 0},
 			},
 		},
 		{
@@ -599,12 +599,12 @@ func TestAllocRunner_Lifecycle_Restart(t *testing.T) {
 				return ar.RestartAll(ev)
 			},
 			expectedAfter: map[string]structs.TaskState{
-				"main":              structs.TaskState{State: "running", Restarts: 1},
-				"prestart-oneshot":  structs.TaskState{State: "dead", Restarts: 1},
-				"prestart-sidecar":  structs.TaskState{State: "running", Restarts: 1},
-				"poststart-oneshot": structs.TaskState{State: "dead", Restarts: 1},
-				"poststart-sidecar": structs.TaskState{State: "running", Restarts: 1},
-				"poststop":          structs.TaskState{State: "pending", Restarts: 0},
+				"main":              {State: "running", Restarts: 1},
+				"prestart-oneshot":  {State: "dead", Restarts: 1},
+				"prestart-sidecar":  {State: "running", Restarts: 1},
+				"poststart-oneshot": {State: "dead", Restarts: 1},
+				"poststart-sidecar": {State: "running", Restarts: 1},
+				"poststop":          {State: "pending", Restarts: 0},
 			},
 		},
 		{
@@ -616,12 +616,12 @@ func TestAllocRunner_Lifecycle_Restart(t *testing.T) {
 				return nil
 			},
 			expectedAfter: map[string]structs.TaskState{
-				"main":              structs.TaskState{State: "dead", Restarts: 0},
-				"prestart-oneshot":  structs.TaskState{State: "dead", Restarts: 0},
-				"prestart-sidecar":  structs.TaskState{State: "dead", Restarts: 0},
-				"poststart-oneshot": structs.TaskState{State: "dead", Restarts: 0},
-				"poststart-sidecar": structs.TaskState{State: "dead", Restarts: 0},
-				"poststop":          structs.TaskState{State: "dead", Restarts: 0},
+				"main":              {State: "dead", Restarts: 0},
+				"prestart-oneshot":  {State: "dead", Restarts: 0},
+				"prestart-sidecar":  {State: "dead", Restarts: 0},
+				"poststart-oneshot": {State: "dead", Restarts: 0},
+				"poststart-sidecar": {State: "dead", Restarts: 0},
+				"poststop":          {State: "dead", Restarts: 0},
 			},
 		},
 		{
@@ -630,12 +630,12 @@ func TestAllocRunner_Lifecycle_Restart(t *testing.T) {
 				return ar.RestartTask("main", ev)
 			},
 			expectedAfter: map[string]structs.TaskState{
-				"main":              structs.TaskState{State: "running", Restarts: 1},
-				"prestart-oneshot":  structs.TaskState{State: "dead", Restarts: 0},
-				"prestart-sidecar":  structs.TaskState{State: "running", Restarts: 0},
-				"poststart-oneshot": structs.TaskState{State: "dead", Restarts: 0},
-				"poststart-sidecar": structs.TaskState{State: "running", Restarts: 0},
-				"poststop":          structs.TaskState{State: "pending", Restarts: 0},
+				"main":              {State: "running", Restarts: 1},
+				"prestart-oneshot":  {State: "dead", Restarts: 0},
+				"prestart-sidecar":  {State: "running", Restarts: 0},
+				"poststart-oneshot": {State: "dead", Restarts: 0},
+				"poststart-sidecar": {State: "running", Restarts: 0},
+				"poststop":          {State: "pending", Restarts: 0},
 			},
 		},
 		{
@@ -645,12 +645,12 @@ func TestAllocRunner_Lifecycle_Restart(t *testing.T) {
 				return ar.RestartTask("main", ev)
 			},
 			expectedAfter: map[string]structs.TaskState{
-				"main":              structs.TaskState{State: "running", Restarts: 1},
-				"prestart-oneshot":  structs.TaskState{State: "dead", Restarts: 0},
-				"prestart-sidecar":  structs.TaskState{State: "running", Restarts: 0},
-				"poststart-oneshot": structs.TaskState{State: "dead", Restarts: 0},
-				"poststart-sidecar": structs.TaskState{State: "running", Restarts: 0},
-				"poststop":          structs.TaskState{State: "pending", Restarts: 0},
+				"main":              {State: "running", Restarts: 1},
+				"prestart-oneshot":  {State: "dead", Restarts: 0},
+				"prestart-sidecar":  {State: "running", Restarts: 0},
+				"poststart-oneshot": {State: "dead", Restarts: 0},
+				"poststart-sidecar": {State: "running", Restarts: 0},
+				"poststop":          {State: "pending", Restarts: 0},
 			},
 		},
 		{
@@ -668,12 +668,12 @@ func TestAllocRunner_Lifecycle_Restart(t *testing.T) {
 				return nil
 			},
 			expectedAfter: map[string]structs.TaskState{
-				"main":              structs.TaskState{State: "dead", Restarts: 1},
-				"prestart-oneshot":  structs.TaskState{State: "dead", Restarts: 0},
-				"prestart-sidecar":  structs.TaskState{State: "dead", Restarts: 0},
-				"poststart-oneshot": structs.TaskState{State: "dead", Restarts: 0},
-				"poststart-sidecar": structs.TaskState{State: "dead", Restarts: 0},
-				"poststop":          structs.TaskState{State: "dead", Restarts: 0},
+				"main":              {State: "dead", Restarts: 1},
+				"prestart-oneshot":  {State: "dead", Restarts: 0},
+				"prestart-sidecar":  {State: "dead", Restarts: 0},
+				"poststart-oneshot": {State: "dead", Restarts: 0},
+				"poststart-sidecar": {State: "dead", Restarts: 0},
+				"poststop":          {State: "dead", Restarts: 0},
 			},
 		},
 		{
@@ -692,12 +692,12 @@ func TestAllocRunner_Lifecycle_Restart(t *testing.T) {
 				return nil
 			},
 			expectedAfter: map[string]structs.TaskState{
-				"main":              structs.TaskState{State: "dead", Restarts: 1},
-				"prestart-oneshot":  structs.TaskState{State: "dead", Restarts: 0},
-				"prestart-sidecar":  structs.TaskState{State: "dead", Restarts: 0},
-				"poststart-oneshot": structs.TaskState{State: "dead", Restarts: 0},
-				"poststart-sidecar": structs.TaskState{State: "dead", Restarts: 0},
-				"poststop":          structs.TaskState{State: "dead", Restarts: 0},
+				"main":              {State: "dead", Restarts: 1},
+				"prestart-oneshot":  {State: "dead", Restarts: 0},
+				"prestart-sidecar":  {State: "dead", Restarts: 0},
+				"poststart-oneshot": {State: "dead", Restarts: 0},
+				"poststart-sidecar": {State: "dead", Restarts: 0},
+				"poststop":          {State: "dead", Restarts: 0},
 			},
 		},
 		{
@@ -715,12 +715,12 @@ func TestAllocRunner_Lifecycle_Restart(t *testing.T) {
 				return nil
 			},
 			expectedAfter: map[string]structs.TaskState{
-				"main":              structs.TaskState{State: "dead", Restarts: 1},
-				"prestart-oneshot":  structs.TaskState{State: "dead", Restarts: 0},
-				"prestart-sidecar":  structs.TaskState{State: "dead", Restarts: 0},
-				"poststart-oneshot": structs.TaskState{State: "dead", Restarts: 0},
-				"poststart-sidecar": structs.TaskState{State: "dead", Restarts: 0},
-				"poststop":          structs.TaskState{State: "dead", Restarts: 0},
+				"main":              {State: "dead", Restarts: 1},
+				"prestart-oneshot":  {State: "dead", Restarts: 0},
+				"prestart-sidecar":  {State: "dead", Restarts: 0},
+				"poststart-oneshot": {State: "dead", Restarts: 0},
+				"poststart-sidecar": {State: "dead", Restarts: 0},
+				"poststop":          {State: "dead", Restarts: 0},
 			},
 		},
 		{
@@ -738,12 +738,12 @@ func TestAllocRunner_Lifecycle_Restart(t *testing.T) {
 				return nil
 			},
 			expectedAfter: map[string]structs.TaskState{
-				"main":              structs.TaskState{State: "dead", Restarts: 1},
-				"prestart-oneshot":  structs.TaskState{State: "dead", Restarts: 0},
-				"prestart-sidecar":  structs.TaskState{State: "dead", Restarts: 0},
-				"poststart-oneshot": structs.TaskState{State: "dead", Restarts: 0},
-				"poststart-sidecar": structs.TaskState{State: "dead", Restarts: 0},
-				"poststop":          structs.TaskState{State: "dead", Restarts: 0},
+				"main":              {State: "dead", Restarts: 1},
+				"prestart-oneshot":  {State: "dead", Restarts: 0},
+				"prestart-sidecar":  {State: "dead", Restarts: 0},
+				"poststart-oneshot": {State: "dead", Restarts: 0},
+				"poststart-sidecar": {State: "dead", Restarts: 0},
+				"poststop":          {State: "dead", Restarts: 0},
 			},
 		},
 		{
@@ -764,12 +764,12 @@ func TestAllocRunner_Lifecycle_Restart(t *testing.T) {
 			},
 			expectedErr: "Task not running",
 			expectedAfter: map[string]structs.TaskState{
-				"main":              structs.TaskState{State: "dead", Restarts: 1},
-				"prestart-oneshot":  structs.TaskState{State: "dead", Restarts: 0},
-				"prestart-sidecar":  structs.TaskState{State: "dead", Restarts: 0},
-				"poststart-oneshot": structs.TaskState{State: "dead", Restarts: 0},
-				"poststart-sidecar": structs.TaskState{State: "dead", Restarts: 0},
-				"poststop":          structs.TaskState{State: "dead", Restarts: 0},
+				"main":              {State: "dead", Restarts: 1},
+				"prestart-oneshot":  {State: "dead", Restarts: 0},
+				"prestart-sidecar":  {State: "dead", Restarts: 0},
+				"poststart-oneshot": {State: "dead", Restarts: 0},
+				"poststart-sidecar": {State: "dead", Restarts: 0},
+				"poststop":          {State: "dead", Restarts: 0},
 			},
 		},
 		{
@@ -778,12 +778,12 @@ func TestAllocRunner_Lifecycle_Restart(t *testing.T) {
 				return ar.RestartTask("prestart-sidecar", ev)
 			},
 			expectedAfter: map[string]structs.TaskState{
-				"main":              structs.TaskState{State: "running", Restarts: 0},
-				"prestart-oneshot":  structs.TaskState{State: "dead", Restarts: 0},
-				"prestart-sidecar":  structs.TaskState{State: "running", Restarts: 1},
-				"poststart-oneshot": structs.TaskState{State: "dead", Restarts: 0},
-				"poststart-sidecar": structs.TaskState{State: "running", Restarts: 0},
-				"poststop":          structs.TaskState{State: "pending", Restarts: 0},
+				"main":              {State: "running", Restarts: 0},
+				"prestart-oneshot":  {State: "dead", Restarts: 0},
+				"prestart-sidecar":  {State: "running", Restarts: 1},
+				"poststart-oneshot": {State: "dead", Restarts: 0},
+				"poststart-sidecar": {State: "running", Restarts: 0},
+				"poststop":          {State: "pending", Restarts: 0},
 			},
 		},
 		{
@@ -792,12 +792,12 @@ func TestAllocRunner_Lifecycle_Restart(t *testing.T) {
 				return ar.RestartTask("poststart-sidecar", ev)
 			},
 			expectedAfter: map[string]structs.TaskState{
-				"main":              structs.TaskState{State: "running", Restarts: 0},
-				"prestart-oneshot":  structs.TaskState{State: "dead", Restarts: 0},
-				"prestart-sidecar":  structs.TaskState{State: "running", Restarts: 0},
-				"poststart-oneshot": structs.TaskState{State: "dead", Restarts: 0},
-				"poststart-sidecar": structs.TaskState{State: "running", Restarts: 1},
-				"poststop":          structs.TaskState{State: "pending", Restarts: 0},
+				"main":              {State: "running", Restarts: 0},
+				"prestart-oneshot":  {State: "dead", Restarts: 0},
+				"prestart-sidecar":  {State: "running", Restarts: 0},
+				"poststart-oneshot": {State: "dead", Restarts: 0},
+				"poststart-sidecar": {State: "running", Restarts: 1},
+				"poststop":          {State: "pending", Restarts: 0},
 			},
 		},
 	}

diff --git a/client/allocrunner/network_manager_linux.go b/client/allocrunner/network_manager_linux.go
@@ -184,7 +184,7 @@ func newNetworkConfigurator(log hclog.Logger, alloc *structs.Allocation, config
 
 	switch {
 	case netMode == "bridge":
-		c, err := newBridgeNetworkConfigurator(log, config.BridgeNetworkName, config.BridgeNetworkAllocSubnet, config.CNIPath, ignorePortMappingHostIP)
+		c, err := newBridgeNetworkConfigurator(log, config.BridgeNetworkName, config.BridgeNetworkAllocSubnet, config.BridgeNetworkHairpinMode, config.CNIPath, ignorePortMappingHostIP)
 		if err != nil {
 			return nil, err
 		}

diff --git a/client/allocrunner/networking_bridge_linux.go b/client/allocrunner/networking_bridge_linux.go
@@ -3,6 +3,7 @@ package allocrunner
 import (
 	"context"
 	"fmt"
+	"text/template"
 
 	"github.com/coreos/go-iptables/iptables"
 	hclog "github.com/hashicorp/go-hclog"
@@ -28,21 +29,25 @@ const (
 	cniAdminChainName = "NOMAD-ADMIN"
 )
 
+var nomadBridgeTmpl = template.Must(template.New("cniConf").Parse(nomadCNIConfigTemplate))
+
 // bridgeNetworkConfigurator is a NetworkConfigurator which adds the alloc to a
 // shared bridge, configures masquerading for egress traffic and port mapping
 // for ingress
 type bridgeNetworkConfigurator struct {
 	cni         *cniNetworkConfigurator
 	allocSubnet string
 	bridgeName  string
+	hairpinMode bool
 
 	logger hclog.Logger
 }
 
-func newBridgeNetworkConfigurator(log hclog.Logger, bridgeName, ipRange, cniPath string, ignorePortMappingHostIP bool) (*bridgeNetworkConfigurator, error) {
+func newBridgeNetworkConfigurator(log hclog.Logger, bridgeName, ipRange string, hairpinMode bool, cniPath string, ignorePortMappingHostIP bool) (*bridgeNetworkConfigurator, error) {
 	b := &bridgeNetworkConfigurator{
 		bridgeName:  bridgeName,
 		allocSubnet: ipRange,
+		hairpinMode: hairpinMode,
 		logger:      log,
 	}
 
@@ -54,7 +59,7 @@ func newBridgeNetworkConfigurator(log hclog.Logger, bridgeName, ipRange, cniPath
 		b.allocSubnet = defaultNomadAllocSubnet
 	}
 
-	c, err := newCNINetworkConfiguratorWithConf(log, cniPath, bridgeNetworkAllocIfPrefix, ignorePortMappingHostIP, buildNomadBridgeNetConfig(b.bridgeName, b.allocSubnet))
+	c, err := newCNINetworkConfiguratorWithConf(log, cniPath, bridgeNetworkAllocIfPrefix, ignorePortMappingHostIP, buildNomadBridgeNetConfig(*b))
 	if err != nil {
 		return nil, err
 	}
@@ -134,8 +139,12 @@ func (b *bridgeNetworkConfigurator) Teardown(ctx context.Context, alloc *structs
 	return b.cni.Teardown(ctx, alloc, spec)
 }
 
-func buildNomadBridgeNetConfig(bridgeName, subnet string) []byte {
-	return []byte(fmt.Sprintf(nomadCNIConfigTemplate, bridgeName, subnet, cniAdminChainName))
+func buildNomadBridgeNetConfig(b bridgeNetworkConfigurator) []byte {
+	return []byte(fmt.Sprintf(nomadCNIConfigTemplate,
+		b.bridgeName,
+		b.hairpinMode,
+		b.allocSubnet,
+		cniAdminChainName))
 }
 
 const nomadCNIConfigTemplate = `{
@@ -147,16 +156,17 @@ const nomadCNIConfigTemplate = `{
 		},
 		{
 			"type": "bridge",
-			"bridge": "%s",
+			"bridge": %q,
 			"ipMasq": true,
 			"isGateway": true,
 			"forceAddress": true,
+			"hairpinMode": %v,
 			"ipam": {
 				"type": "host-local",
 				"ranges": [
 					[
 						{
-							"subnet": "%s"
+							"subnet": %q
 						}
 					]
 				],
@@ -168,7 +178,7 @@ const nomadCNIConfigTemplate = `{
 		{
 			"type": "firewall",
 			"backend": "iptables",
-			"iptablesAdminChainName": "%s"
+			"iptablesAdminChainName": %q
 		},
 		{
 			"type": "portmap",