Backport of cni: add DNS set by CNI plugins to task configuration int…

…o release/1.7.x (#20014)

Co-authored-by: Tim Gross <tgross@hashicorp.com>

.changelog/20007.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+cni: Fixed a bug where DNS set by CNI plugins was not provided to task drivers
+```

client/allocrunner/alloc_runner.go

@@ -901,7 +901,9 @@ func (ar *allocRunner) SetClientStatus(clientStatus string) {
 func (ar *allocRunner) SetNetworkStatus(s *structs.AllocNetworkStatus) {
 	ar.stateLock.Lock()
 	defer ar.stateLock.Unlock()
-	ar.state.NetworkStatus = s.Copy()
+	ans := s.Copy()
+	ar.state.NetworkStatus = ans
+	ar.hookResources.SetAllocNetworkStatus(ans)
 }
 
 func (ar *allocRunner) NetworkStatus() *structs.AllocNetworkStatus {

client/allocrunner/taskrunner/task_runner.go

@@ -1125,6 +1125,18 @@ func (tr *TaskRunner) buildTaskConfig() *drivers.TaskConfig {
 	defer tr.networkIsolationLock.Unlock()
 
 	var dns *drivers.DNSConfig
+
+	// set DNS from any CNI plugins
+	netStatus := tr.allocHookResources.GetAllocNetworkStatus()
+	if netStatus != nil && netStatus.DNS != nil {
+		dns = &drivers.DNSConfig{
+			Servers:  netStatus.DNS.Servers,
+			Searches: netStatus.DNS.Searches,
+			Options:  netStatus.DNS.Options,
+		}
+	}
+
+	// override DNS if set by job submitter
 	if alloc.AllocatedResources != nil && len(alloc.AllocatedResources.Shared.Networks) > 0 {
 		allocDNS := alloc.AllocatedResources.Shared.Networks[0].DNS
 		if allocDNS != nil {

client/allocrunner/taskrunner/task_runner_test.go

@@ -2931,3 +2931,121 @@ func TestTaskRunner_IdentityHook_Disabled(t *testing.T) {
 	taskEnv := tr.envBuilder.Build()
 	must.MapNotContainsKey(t, taskEnv.EnvMap, "NOMAD_TOKEN")
 }
+
+func TestTaskRunner_AllocNetworkStatus(t *testing.T) {
+	ci.Parallel(t)
+
+	// Mock task with group network
+	alloc1 := mock.Alloc()
+	task1 := alloc1.Job.TaskGroups[0].Tasks[0]
+	alloc1.AllocatedResources.Shared.Networks = []*structs.NetworkResource{
+		{
+			Device: "eth0",
+			IP:     "192.168.0.100",
+			DNS: &structs.DNSConfig{
+				Servers:  []string{"1.1.1.1", "8.8.8.8"},
+				Searches: []string{"test.local"},
+				Options:  []string{"ndots:1"},
+			},
+			ReservedPorts: []structs.Port{{Label: "admin", Value: 5000}},
+			DynamicPorts:  []structs.Port{{Label: "http", Value: 9876}},
+		}}
+	task1.Driver = "mock_driver"
+	task1.Config = map[string]interface{}{"run_for": "2s"}
+
+	// Mock task with task networking only
+	alloc2 := mock.Alloc()
+	task2 := alloc2.Job.TaskGroups[0].Tasks[0]
+	task2.Driver = "mock_driver"
+	task2.Config = map[string]interface{}{"run_for": "2s"}
+
+	testCases := []struct {
+		name    string
+		alloc   *structs.Allocation
+		task    *structs.Task
+		fromCNI *structs.DNSConfig
+		expect  *drivers.DNSConfig
+	}{
+		{
+			name:  "task with group networking overrides CNI",
+			alloc: alloc1,
+			task:  task1,
+			fromCNI: &structs.DNSConfig{
+				Servers:  []string{"10.37.105.17"},
+				Searches: []string{"node.consul"},
+				Options:  []string{"ndots:2", "edns0"},
+			},
+			expect: &drivers.DNSConfig{
+				Servers:  []string{"1.1.1.1", "8.8.8.8"},
+				Searches: []string{"test.local"},
+				Options:  []string{"ndots:1"},
+			},
+		},
+		{
+			name:  "task with CNI alone",
+			alloc: alloc2,
+			task:  task1,
+			fromCNI: &structs.DNSConfig{
+				Servers:  []string{"10.37.105.17"},
+				Searches: []string{"node.consul"},
+				Options:  []string{"ndots:2", "edns0"},
+			},
+			expect: &drivers.DNSConfig{
+				Servers:  []string{"10.37.105.17"},
+				Searches: []string{"node.consul"},
+				Options:  []string{"ndots:2", "edns0"},
+			},
+		},
+		{
+			name:    "task with group networking alone",
+			alloc:   alloc1,
+			task:    task1,
+			fromCNI: nil,
+			expect: &drivers.DNSConfig{
+				Servers:  []string{"1.1.1.1", "8.8.8.8"},
+				Searches: []string{"test.local"},
+				Options:  []string{"ndots:1"},
+			},
+		},
+		{
+			name:    "task without group networking",
+			alloc:   alloc2,
+			task:    task2,
+			fromCNI: nil,
+			expect:  nil,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+
+			conf, cleanup := testTaskRunnerConfig(t, tc.alloc, tc.task.Name, nil)
+			t.Cleanup(cleanup)
+
+			// note this will never actually be set if we don't have group/CNI
+			// networking, but it's a good validation no-group/CNI code path
+			conf.AllocHookResources.SetAllocNetworkStatus(&structs.AllocNetworkStatus{
+				InterfaceName: "",
+				Address:       "",
+				DNS:           tc.fromCNI,
+			})
+
+			tr, err := NewTaskRunner(conf)
+			must.NoError(t, err)
+
+			// Run the task runner.
+			go tr.Run()
+			t.Cleanup(func() {
+				tr.Kill(context.Background(), structs.NewTaskEvent("cleanup"))
+			})
+
+			// Wait for task to complete.
+			testWaitForTaskToStart(t, tr)
+
+			tr.stateLock.RLock()
+			t.Cleanup(tr.stateLock.RUnlock)
+
+			must.Eq(t, tc.expect, tr.localState.TaskHandle.Config.DNS)
+		})
+	}
+}

client/structs/allochook.go

@@ -9,15 +9,17 @@ import (
 	consulapi "github.com/hashicorp/consul/api"
 	"github.com/hashicorp/nomad/client/pluginmanager/csimanager"
 	"github.com/hashicorp/nomad/helper"
+	"github.com/hashicorp/nomad/nomad/structs"
 )
 
 // AllocHookResources contains data that is provided by AllocRunner Hooks for
 // consumption by TaskRunners. This should be instantiated once in the
 // AllocRunner and then only accessed via getters and setters that hold the
 // lock.
 type AllocHookResources struct {
-	csiMounts    map[string]*csimanager.MountInfo
-	consulTokens map[string]map[string]*consulapi.ACLToken // Consul cluster -> service identity -> token
+	csiMounts     map[string]*csimanager.MountInfo
+	consulTokens  map[string]map[string]*consulapi.ACLToken // Consul cluster -> service identity -> token
+	networkStatus *structs.AllocNetworkStatus
 
 	mu sync.RWMutex
 }
@@ -67,3 +69,21 @@ func (a *AllocHookResources) SetConsulTokens(m map[string]map[string]*consulapi.
 		a.consulTokens[k] = v
 	}
 }
+
+// GetAllocNetworkStatus returns a copy of the AllocNetworkStatus previously
+// written the group's network_hook
+func (a *AllocHookResources) GetAllocNetworkStatus() *structs.AllocNetworkStatus {
+	a.mu.RLock()
+	defer a.mu.RUnlock()
+
+	return a.networkStatus.Copy()
+}
+
+// SetAllocNetworkStatus stores the AllocNetworkStatus for later use by the
+// taskrunner's buildTaskConfig() method
+func (a *AllocHookResources) SetAllocNetworkStatus(ans *structs.AllocNetworkStatus) {
+	a.mu.Lock()
+	defer a.mu.Unlock()
+
+	a.networkStatus = ans
+}

website/content/docs/job-specification/network.mdx

@@ -84,9 +84,11 @@ All other operating systems use the `host` networking mode.
   [mode](#mode) is set to [`bridge`](#bridge). This parameter supports
   [interpolation](/nomad/docs/runtime/interpolation).
 
-- `dns` <code>([DNSConfig](#dns-parameters): nil)</code> - Sets the DNS configuration
-  for the allocations. By default all DNS configuration is inherited from the client host.
-  DNS configuration is only supported on Linux clients at this time.
+- `dns` <code>([DNSConfig](#dns-parameters): nil)</code> - Sets the DNS
+  configuration for the allocations. By default all task drivers will inherit
+  DNS configuration from the client host. DNS configuration is only supported on
+  Linux clients at this time. Note that if you are using a `mode="cni/*`, these
+  values will override any DNS configuration the CNI plugins return.
 
 ### `port` Parameters