diff --git a/.changelog/20007.txt b/.changelog/20007.txt new file mode 100644 index 000000000..3bc9c9188 --- /dev/null +++ b/.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 +``` diff --git a/client/allocrunner/alloc_runner.go b/client/allocrunner/alloc_runner.go index 8923bcd3f..a56643ac4 100644 --- a/client/allocrunner/alloc_runner.go +++ b/client/allocrunner/alloc_runner.go @@ -908,7 +908,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 { diff --git a/client/allocrunner/taskrunner/task_runner.go b/client/allocrunner/taskrunner/task_runner.go index a8ad6228f..3f9efb033 100644 --- a/client/allocrunner/taskrunner/task_runner.go +++ b/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 { diff --git a/client/allocrunner/taskrunner/task_runner_test.go b/client/allocrunner/taskrunner/task_runner_test.go index dfa7ee907..53beecef9 100644 --- a/client/allocrunner/taskrunner/task_runner_test.go +++ b/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) + }) + } +} diff --git a/client/structs/allochook.go b/client/structs/allochook.go index 9b4db8390..f7da67368 100644 --- a/client/structs/allochook.go +++ b/client/structs/allochook.go @@ -9,6 +9,7 @@ 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 @@ -16,8 +17,9 @@ import ( // 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 +} diff --git a/website/content/docs/job-specification/network.mdx b/website/content/docs/job-specification/network.mdx index f840dbf48..8ce2bce7c 100644 --- a/website/content/docs/job-specification/network.mdx +++ b/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` ([DNSConfig](#dns-parameters): nil) - 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` ([DNSConfig](#dns-parameters): nil) - 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