diff --git a/.changelog/20189.txt b/.changelog/20189.txt new file mode 100644 index 000000000..eac272c0b --- /dev/null +++ b/.changelog/20189.txt @@ -0,0 +1,3 @@ +```release-note:bug +cni: Fixed a regression where default DNS set by `dockerd` or other task drivers was not respected +``` diff --git a/client/allocrunner/networking_cni.go b/client/allocrunner/networking_cni.go index 654e7349d..3641aebcb 100644 --- a/client/allocrunner/networking_cni.go +++ b/client/allocrunner/networking_cni.go @@ -184,12 +184,15 @@ func (c *cniNetworkConfigurator) cniToAllocNet(res *cni.Result) (*structs.AllocN } - // Use the first DNS results. + // Use the first DNS results, if non-empty if len(res.DNS) > 0 { - netStatus.DNS = &structs.DNSConfig{ - Servers: res.DNS[0].Nameservers, - Searches: res.DNS[0].Search, - Options: res.DNS[0].Options, + cniDNS := res.DNS[0] + if len(cniDNS.Nameservers) > 0 { + netStatus.DNS = &structs.DNSConfig{ + Servers: cniDNS.Nameservers, + Searches: cniDNS.Search, + Options: cniDNS.Options, + } } } diff --git a/client/allocrunner/networking_cni_test.go b/client/allocrunner/networking_cni_test.go index 5c25725b2..b773a9486 100644 --- a/client/allocrunner/networking_cni_test.go +++ b/client/allocrunner/networking_cni_test.go @@ -11,10 +11,11 @@ import ( "testing" "github.com/containerd/go-cni" + "github.com/containernetworking/cni/pkg/types" "github.com/hashicorp/nomad/ci" "github.com/hashicorp/nomad/helper/testlog" + "github.com/shoenig/test" "github.com/shoenig/test/must" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -148,9 +149,8 @@ func TestCNI_cniToAllocNet_NoInterfaces(t *testing.T) { func TestCNI_cniToAllocNet_Fallback(t *testing.T) { ci.Parallel(t) - // Calico's CNI plugin v3.12.3 has been observed to return the - // following: cniResult := &cni.Result{ + // Calico's CNI plugin v3.12.3 has been observed to return the following: Interfaces: map[string]*cni.Config{ "cali39179aa3-74": {}, "eth0": { @@ -161,6 +161,8 @@ func TestCNI_cniToAllocNet_Fallback(t *testing.T) { }, }, }, + // cni.Result will return a single empty struct, not an empty slice + DNS: []types.DNS{{}}, } // Only need a logger @@ -168,11 +170,11 @@ func TestCNI_cniToAllocNet_Fallback(t *testing.T) { logger: testlog.HCLogger(t), } allocNet, err := c.cniToAllocNet(cniResult) - require.NoError(t, err) - require.NotNil(t, allocNet) - assert.Equal(t, "192.168.135.232", allocNet.Address) - assert.Equal(t, "eth0", allocNet.InterfaceName) - assert.Nil(t, allocNet.DNS) + must.NoError(t, err) + must.NotNil(t, allocNet) + test.Eq(t, "192.168.135.232", allocNet.Address) + test.Eq(t, "eth0", allocNet.InterfaceName) + test.Nil(t, allocNet.DNS) } // TestCNI_cniToAllocNet_Invalid asserts an error is returned if a CNI plugin diff --git a/client/allocrunner/taskrunner/task_runner_test.go b/client/allocrunner/taskrunner/task_runner_test.go index e5fe60dc2..e4b6e678f 100644 --- a/client/allocrunner/taskrunner/task_runner_test.go +++ b/client/allocrunner/taskrunner/task_runner_test.go @@ -2935,41 +2935,39 @@ func TestTaskRunner_IdentityHook_Disabled(t *testing.T) { 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"} + alloc := mock.Alloc() + task := alloc.Job.TaskGroups[0].Tasks[0] + task.Driver = "mock_driver" + task.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"} + groupNetworks := []*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}}, + }} + + groupNetworksWithoutDNS := []*structs.NetworkResource{{ + Device: "eth0", + IP: "192.168.0.100", + ReservedPorts: []structs.Port{{Label: "admin", Value: 5000}}, + DynamicPorts: []structs.Port{{Label: "http", Value: 9876}}, + }} testCases := []struct { - name string - alloc *structs.Allocation - task *structs.Task - fromCNI *structs.DNSConfig - expect *drivers.DNSConfig + name string + networks []*structs.NetworkResource + fromCNI *structs.DNSConfig + expect *drivers.DNSConfig }{ { - name: "task with group networking overrides CNI", - alloc: alloc1, - task: task1, + name: "task with group networking overrides CNI", + networks: groupNetworks, fromCNI: &structs.DNSConfig{ Servers: []string{"10.37.105.17"}, Searches: []string{"node.consul"}, @@ -2982,9 +2980,7 @@ func TestTaskRunner_AllocNetworkStatus(t *testing.T) { }, }, { - name: "task with CNI alone", - alloc: alloc2, - task: task1, + name: "task with CNI alone", fromCNI: &structs.DNSConfig{ Servers: []string{"10.37.105.17"}, Searches: []string{"node.consul"}, @@ -2997,10 +2993,9 @@ func TestTaskRunner_AllocNetworkStatus(t *testing.T) { }, }, { - name: "task with group networking alone", - alloc: alloc1, - task: task1, - fromCNI: nil, + name: "task with group networking alone wth DNS", + networks: groupNetworks, + fromCNI: nil, expect: &drivers.DNSConfig{ Servers: []string{"1.1.1.1", "8.8.8.8"}, Searches: []string{"test.local"}, @@ -3008,9 +3003,13 @@ func TestTaskRunner_AllocNetworkStatus(t *testing.T) { }, }, { - name: "task without group networking", - alloc: alloc2, - task: task2, + name: "task with group networking and no CNI dns", + networks: groupNetworksWithoutDNS, + fromCNI: &structs.DNSConfig{}, + expect: &drivers.DNSConfig{}, + }, + { + name: "task without group networking or CNI", fromCNI: nil, expect: nil, }, @@ -3019,7 +3018,10 @@ func TestTaskRunner_AllocNetworkStatus(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - conf, cleanup := testTaskRunnerConfig(t, tc.alloc, tc.task.Name, nil) + testAlloc := alloc.Copy() + testAlloc.AllocatedResources.Shared.Networks = tc.networks + + conf, cleanup := testTaskRunnerConfig(t, testAlloc, task.Name, nil) t.Cleanup(cleanup) // note this will never actually be set if we don't have group/CNI