Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport of cni: add DNS set by CNI plugins to task configuration into release/1.7.x #20014

Merged
merged 1 commit into from
Feb 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading