From 24fa7439dffdccc0e4071d09dbc47e8325b4eae2 Mon Sep 17 00:00:00 2001
From: Tim Gross <tgross@hashicorp.com>
Date: Mon, 16 Dec 2024 09:36:35 -0500
Subject: [PATCH] cni: use tmpfs location for ipam plugin (#24650)

When a Nomad host reboots, the network namespace files in the tmpfs in
`/var/run` are wiped out. So when we restore allocations after a host reboot, we
need to be able to restore both the network namespace and the network
configuration. But because the netns is newly created and we need to run the CNI
plugins again, this create potential conflicts with the IPAM plugin which has
written state to persistent disk at `/var/lib/cni`. These IPs aren't the ones
advertised to Consul, so there's no particular reason to keep them around after
a host reboot because all virtual interfaces need to be recreated too.

Reconfigure the CNI bridge configuration to use `/var/run/cni` as its state
directory. We already expect this location to be created by CNI because the
netns files are hard-coded to be created there too in `libcni`.

Note this does not fix the problem described for Docker in #24292 because that
appears to be related to the netns itself being restored unexpectedly from
Docker's state.

Ref: https://github.com/hashicorp/nomad/issues/24292#issuecomment-2537078584
Ref: https://www.cni.dev/plugins/current/ipam/host-local/#files
---
 .changelog/24650.txt                                      | 3 +++
 client/allocrunner/cni/bridge.go                          | 7 ++++---
 client/allocrunner/cni/plugins.go                         | 7 ++++---
 client/allocrunner/test_fixtures/bad_input.conflist.json  | 3 ++-
 client/allocrunner/test_fixtures/consul-cni.conflist.json | 3 ++-
 client/allocrunner/test_fixtures/empty.conflist.json      | 3 ++-
 client/allocrunner/test_fixtures/hairpin.conflist.json    | 3 ++-
 client/allocrunner/test_fixtures/ipv6.conflist.json       | 3 ++-
 8 files changed, 21 insertions(+), 11 deletions(-)
 create mode 100644 .changelog/24650.txt

diff --git a/.changelog/24650.txt b/.changelog/24650.txt
new file mode 100644
index 00000000000..5efb4f2330e
--- /dev/null
+++ b/.changelog/24650.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+networking: use a tmpfs location for the state of CNI IPAM plugin used by bridge mode, to fix a bug where allocations would fail to restore after host reboot
+```
diff --git a/client/allocrunner/cni/bridge.go b/client/allocrunner/cni/bridge.go
index 91b438b8e59..5889c2f510d 100644
--- a/client/allocrunner/cni/bridge.go
+++ b/client/allocrunner/cni/bridge.go
@@ -58,9 +58,10 @@ func NewNomadBridgeConflist(conf NomadBridgeConfig) Conflist {
 			ForceAddress: true,
 			HairpinMode:  conf.HairpinMode,
 			Ipam: IPAM{
-				Type:   "host-local",
-				Ranges: ipRanges,
-				Routes: ipRoutes,
+				Type:    "host-local",
+				Ranges:  ipRanges,
+				Routes:  ipRoutes,
+				DataDir: "/var/run/cni",
 			},
 		},
 		Firewall{
diff --git a/client/allocrunner/cni/plugins.go b/client/allocrunner/cni/plugins.go
index 091208ca3ba..f31ff9dd605 100644
--- a/client/allocrunner/cni/plugins.go
+++ b/client/allocrunner/cni/plugins.go
@@ -20,9 +20,10 @@ type Bridge struct {
 	Ipam         IPAM   `json:"ipam"`
 }
 type IPAM struct {
-	Type   string    `json:"type"`
-	Ranges [][]Range `json:"ranges"`
-	Routes []Route   `json:"routes"`
+	Type    string    `json:"type"`
+	Ranges  [][]Range `json:"ranges"`
+	Routes  []Route   `json:"routes"`
+	DataDir string    `json:"dataDir"`
 }
 type Range struct {
 	Subnet string `json:"subnet"`
diff --git a/client/allocrunner/test_fixtures/bad_input.conflist.json b/client/allocrunner/test_fixtures/bad_input.conflist.json
index f9c9be906ce..9b2bb2fc990 100644
--- a/client/allocrunner/test_fixtures/bad_input.conflist.json
+++ b/client/allocrunner/test_fixtures/bad_input.conflist.json
@@ -25,7 +25,8 @@
 					{
 						"dst": "0.0.0.0/0"
 					}
-				]
+				],
+				"dataDir": "/var/run/cni"
 			}
 		},
 		{
diff --git a/client/allocrunner/test_fixtures/consul-cni.conflist.json b/client/allocrunner/test_fixtures/consul-cni.conflist.json
index 3fe7b270edb..95b7a9c0058 100644
--- a/client/allocrunner/test_fixtures/consul-cni.conflist.json
+++ b/client/allocrunner/test_fixtures/consul-cni.conflist.json
@@ -25,7 +25,8 @@
 					{
 						"dst": "0.0.0.0/0"
 					}
-				]
+				],
+				"dataDir": "/var/run/cni"
 			}
 		},
 		{
diff --git a/client/allocrunner/test_fixtures/empty.conflist.json b/client/allocrunner/test_fixtures/empty.conflist.json
index 484feb44674..7ac34cdf404 100644
--- a/client/allocrunner/test_fixtures/empty.conflist.json
+++ b/client/allocrunner/test_fixtures/empty.conflist.json
@@ -25,7 +25,8 @@
 					{
 						"dst": "0.0.0.0/0"
 					}
-				]
+				],
+				"dataDir": "/var/run/cni"
 			}
 		},
 		{
diff --git a/client/allocrunner/test_fixtures/hairpin.conflist.json b/client/allocrunner/test_fixtures/hairpin.conflist.json
index 1e584c6c115..ba6f276472c 100644
--- a/client/allocrunner/test_fixtures/hairpin.conflist.json
+++ b/client/allocrunner/test_fixtures/hairpin.conflist.json
@@ -25,7 +25,8 @@
 					{
 						"dst": "0.0.0.0/0"
 					}
-				]
+				],
+				"dataDir": "/var/run/cni"
 			}
 		},
 		{
diff --git a/client/allocrunner/test_fixtures/ipv6.conflist.json b/client/allocrunner/test_fixtures/ipv6.conflist.json
index e71afdbe5a7..eb15ada4b12 100644
--- a/client/allocrunner/test_fixtures/ipv6.conflist.json
+++ b/client/allocrunner/test_fixtures/ipv6.conflist.json
@@ -33,7 +33,8 @@
 					{
 						"dst": "::/0"
 					}
-				]
+				],
+				"dataDir": "/var/run/cni"
 			}
 		},
 		{