Skip to content

Commit

Permalink
backport of commit 45b2c34
Browse files Browse the repository at this point in the history
  • Loading branch information
tgross authored Feb 20, 2024
1 parent f878a79 commit 955ec8d
Show file tree
Hide file tree
Showing 6 changed files with 163 additions and 6 deletions.
3 changes: 3 additions & 0 deletions .changelog/20007.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
cni: Fixed a bug where DNS set by CNI plugins was not provided to task drivers
```
4 changes: 3 additions & 1 deletion client/allocrunner/alloc_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
12 changes: 12 additions & 0 deletions client/allocrunner/taskrunner/task_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
118 changes: 118 additions & 0 deletions client/allocrunner/taskrunner/task_runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}
24 changes: 22 additions & 2 deletions client/structs/allochook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
8 changes: 5 additions & 3 deletions website/content/docs/job-specification/network.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 955ec8d

Please sign in to comment.