From cdeafafbb08c87c94b1bc70ea8ce63f86813d286 Mon Sep 17 00:00:00 2001 From: hc-github-team-nomad-core <82989552+hc-github-team-nomad-core@users.noreply.github.com> Date: Tue, 20 Feb 2024 10:41:59 -0600 Subject: [PATCH] Backport of cni: add DNS set by CNI plugins to task configuration into release/1.6.x (#20013) CNI plugins may set DNS configuration, but this isn't threaded through to the task configuration so that we can write it to the `/etc/resolv.conf` file as needed. Add the `AllocNetworkStatus` to the alloc hook resources so they're accessible from the taskrunner. Any DNS entries provided by the user will override these values. Fixes: https://github.com/hashicorp/nomad/issues/11102 Co-authored-by: Tim Gross --- .changelog/20007.txt | 3 + client/allocrunner/alloc_runner.go | 4 +- client/allocrunner/taskrunner/task_runner.go | 12 ++ .../taskrunner/task_runner_test.go | 120 ++++++++++++++++++ client/structs/allochook.go | 22 +++- command/agent/consul/int_test.go | 2 + .../docs/job-specification/network.mdx | 8 +- 7 files changed, 166 insertions(+), 5 deletions(-) create mode 100644 .changelog/20007.txt diff --git a/.changelog/20007.txt b/.changelog/20007.txt new file mode 100644 index 00000000000..3bc9c9188df --- /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 2c6e1fb8555..dfbfded4617 100644 --- a/client/allocrunner/alloc_runner.go +++ b/client/allocrunner/alloc_runner.go @@ -871,7 +871,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 b23381d784e..4d690f07669 100644 --- a/client/allocrunner/taskrunner/task_runner.go +++ b/client/allocrunner/taskrunner/task_runner.go @@ -1119,6 +1119,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 5c0d2ddf090..6c190a3c2fb 100644 --- a/client/allocrunner/taskrunner/task_runner_test.go +++ b/client/allocrunner/taskrunner/task_runner_test.go @@ -34,6 +34,7 @@ import ( regMock "github.com/hashicorp/nomad/client/serviceregistration/mock" "github.com/hashicorp/nomad/client/serviceregistration/wrapper" cstate "github.com/hashicorp/nomad/client/state" + cstructs "github.com/hashicorp/nomad/client/structs" ctestutil "github.com/hashicorp/nomad/client/testutil" "github.com/hashicorp/nomad/client/vaultclient" agentconsul "github.com/hashicorp/nomad/command/agent/consul" @@ -152,6 +153,7 @@ func testTaskRunnerConfig(t *testing.T, alloc *structs.Allocation, taskName stri ShutdownDelayCancelFn: shutdownDelayCancelFn, ServiceRegWrapper: wrapperMock, Getter: getter.TestSandbox(t), + AllocHookResources: cstructs.NewAllocHookResources(), } // Set the cgroup path getter if we are in v2 mode @@ -2709,3 +2711,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) + 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 010b61a170c..38c6108c231 100644 --- a/client/structs/allochook.go +++ b/client/structs/allochook.go @@ -8,6 +8,7 @@ import ( "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 @@ -15,7 +16,8 @@ import ( // AllocRunner and then only accessed via getters and setters that hold the // lock. type AllocHookResources struct { - csiMounts map[string]*csimanager.MountInfo + csiMounts map[string]*csimanager.MountInfo + networkStatus *structs.AllocNetworkStatus mu sync.RWMutex } @@ -43,3 +45,21 @@ func (a *AllocHookResources) SetCSIMounts(m map[string]*csimanager.MountInfo) { a.csiMounts = m } + +// 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/command/agent/consul/int_test.go b/command/agent/consul/int_test.go index 991db33f609..c2a53856fc0 100644 --- a/command/agent/consul/int_test.go +++ b/command/agent/consul/int_test.go @@ -21,6 +21,7 @@ import ( regMock "github.com/hashicorp/nomad/client/serviceregistration/mock" "github.com/hashicorp/nomad/client/serviceregistration/wrapper" "github.com/hashicorp/nomad/client/state" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/client/vaultclient" "github.com/hashicorp/nomad/command/agent/consul" "github.com/hashicorp/nomad/helper/testlog" @@ -169,6 +170,7 @@ func TestConsul_Integration(t *testing.T) { DriverManager: drivermanager.TestDriverManager(t), StartConditionMetCh: closedCh, ServiceRegWrapper: wrapper.NewHandlerWrapper(logger, serviceClient, regMock.NewServiceRegistrationHandler(logger)), + AllocHookResources: cstructs.NewAllocHookResources(), } tr, err := taskrunner.NewTaskRunner(config) diff --git a/website/content/docs/job-specification/network.mdx b/website/content/docs/job-specification/network.mdx index ada5550781d..cfd6b6094fc 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