Merge pull request #8590 from concourse/dns-proxy-refactor

refactor dns package and test

integration/internal/cmdtest/cmd.go

@@ -3,6 +3,7 @@ package cmdtest
 import (
 	"bytes"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -69,7 +70,8 @@ func (cmd Cmd) ExpectExit(code int) Cmd {
 func (cmd Cmd) Run(t *testing.T, args ...string) {
 	err := cmd.Try(args...)
 	if err != nil {
-		if exitErr, ok := err.(*exec.ExitError); ok {
+		var exitErr *exec.ExitError
+		if errors.As(err, &exitErr) {
 			if exitErr.ExitCode() != cmd.ExpectExitCode {
 				t.Fatalf("ExitCode %d != %d", exitErr.ExitCode(), cmd.ExpectExitCode)
 			}

integration/worker/dns_test.go

@@ -0,0 +1,49 @@
+package main
+
+import (
+	"testing"
+
+	"github.com/concourse/concourse/integration/internal/dctest"
+	"github.com/concourse/concourse/integration/internal/flytest"
+	"github.com/stretchr/testify/require"
+)
+
+func TestDNSProxyEnabled(t *testing.T) {
+	t.Parallel()
+
+	dc := dctest.Init(t, "../docker-compose.yml", "overrides/enable_dns_proxy.yml")
+
+	t.Run("deploy with DNS proxy enabled", func(t *testing.T) {
+		dc.Run(t, "up", "-d")
+	})
+
+	fly := flytest.Init(t, dc)
+	fly.ExpectExit(0).Run(t, "execute", "-c", "tasks/resolve_web_dns.yml")
+}
+
+func TestDNSProxyDisabled(t *testing.T) {
+	t.Parallel()
+
+	dc := dctest.Init(t, "../docker-compose.yml", "overrides/disable_dns_proxy.yml")
+
+	t.Run("deploy with DNS proxy disabled", func(t *testing.T) {
+		dc.Run(t, "up", "-d")
+	})
+
+	fly := flytest.Init(t, dc)
+	fly.ExpectExit(1).Run(t, "execute", "-c", "tasks/resolve_web_dns.yml")
+}
+
+func TestExtraDNSServersAreAdded(t *testing.T) {
+	t.Parallel()
+
+	dc := dctest.Init(t, "../docker-compose.yml", "overrides/add_extra_dns_servers.yml")
+
+	t.Run("deploy with extra DNS servers", func(t *testing.T) {
+		dc.Run(t, "up", "-d")
+	})
+
+	fly := flytest.Init(t, dc)
+	output := fly.WithEnv("FILE=/etc/resolv.conf").Output(t, "execute", "-c", "tasks/cat_file.yml")
+	require.Contains(t, output, "nameserver 1.1.1.1")
+}

integration/worker/overrides/add_extra_dns_servers.yml

@@ -0,0 +1,7 @@
+version: '3'
+
+services:
+  worker:
+    environment:
+      CONCOURSE_CONTAINERD_DNS_SERVER: "1.1.1.1"
+      CONCOURSE_RUNTIME: containerd

integration/worker/overrides/disable_dns_proxy.yml

@@ -0,0 +1,7 @@
+version: '3'
+
+services:
+  worker:
+    environment:
+      CONCOURSE_CONTAINERD_DNS_PROXY_ENABLE: "false"
+      CONCOURSE_RUNTIME: containerd

integration/worker/overrides/enable_dns_proxy.yml

@@ -0,0 +1,7 @@
+version: '3'
+
+services:
+  worker:
+    environment:
+      CONCOURSE_CONTAINERD_DNS_PROXY_ENABLE: "true"
+      CONCOURSE_RUNTIME: containerd

integration/worker/tasks/cat_file.yml

@@ -0,0 +1,15 @@
+platform: linux
+
+image_resource:
+  type: mock
+  source: {mirror_self: true}
+
+params:
+  FILE:
+
+run:
+  path: sh
+  args:
+  - -c
+  - |
+    cat $FILE

integration/worker/tasks/resolve_web_dns.yml

@@ -0,0 +1,12 @@
+platform: linux
+
+image_resource:
+  type: mock
+  source: {mirror_self: true}
+
+run:
+  path: sh
+  args:
+  - -c
+  - |
+    nslookup web

topgun/k8s/dns_proxy_test.go

@@ -1,10 +1,7 @@
 package k8s_test
 
 import (
-	"log"
-
 	. "github.com/onsi/ginkgo"
-	. "github.com/onsi/ginkgo/extensions/table"
 	. "github.com/onsi/gomega"
 )
 
@@ -13,7 +10,7 @@ var _ = Describe("DNS Resolution", func() {
 	var atc Endpoint
 
 	BeforeEach(func() {
-		setReleaseNameAndNamespace("dp")
+		setReleaseNameAndNamespace("dns")
 	})
 
 	AfterEach(func() {
@@ -25,107 +22,30 @@ var _ = Describe("DNS Resolution", func() {
 		return releaseName + "-web." + namespace + ".svc.cluster.local:8080/api/v1/info"
 	}
 
-	var shortAddress = func() string {
-		return releaseName + "-web:8080/api/v1/info"
-	}
-
-	type Case struct {
-		// args
-		enableDnsProxy  string
-		dnsServer       string
-		addressFunction func() string
-
-		// expectations
-		shouldWork bool
-	}
-
 	const containerdRuntime = "containerd"
 	const guardianRuntime = "guardian"
 
-	var setupDeployment = func(runtime string, dnsProxyEnable, dnsServer string) {
-		args := []string{
-			`--set=worker.replicas=1`,
-			`--set-string=concourse.worker.runtime=` + runtime,
-		}
-		switch {
-		case runtime == containerdRuntime:
-			args = append(args, `--set-string=concourse.worker.containerd.dnsProxyEnable=`+dnsProxyEnable)
-			if dnsServer != "" {
-				args = append(args,
-					`--set=worker.env[0].name=CONCOURSE_CONTAINERD_DNS_SERVER`,
-					`--set=worker.env[0].value=`+dnsServer)
+	DNSShouldWork := func(runtime string) {
+		It("can reach local k8s services", func() {
+			args := []string{
+				`--set=worker.replicas=1`,
+				`--set-string=concourse.worker.runtime=` + runtime,
 			}
-		case runtime == guardianRuntime:
-			args = append(args, `--set-string=concourse.worker.garden.dnsProxyEnable=`+dnsProxyEnable)
-			if dnsServer != "" {
-				args = append(args,
-					`--set=worker.env[0].name=CONCOURSE_GARDEN_DNS_SERVER`,
-					`--set=worker.env[0].value=`+dnsServer)
-			}
-		default:
-			log.Fatalf("Invalid runtime type %s. Test aborted.", runtime)
-			return
-		}
-
-		deployConcourseChart(releaseName, args...)
-		atc = waitAndLogin(namespace, releaseName+"-web")
-	}
-
-	expectedDnsProxyBehaviour := func(runtime string) {
-		DescribeTable("different proxy settings",
-			func(c Case) {
-				setupDeployment(runtime, c.enableDnsProxy, c.dnsServer)
-
-				sess := fly.Start("execute", "-c", "tasks/dns-proxy-task.yml", "-v", "url="+c.addressFunction())
-				<-sess.Exited
+			deployConcourseChart(releaseName, args...)
+			atc = waitAndLogin(namespace, releaseName+"-web")
 
-				if !c.shouldWork {
-					Expect(sess.ExitCode()).ToNot(BeZero())
-					return
-				}
+			sess := fly.Start("execute", "-c", "tasks/dns-proxy-task.yml", "-v", "url="+fullAddress())
+			<-sess.Exited
 
-				Expect(sess.ExitCode()).To(BeZero())
-			},
-			Entry("Proxy Enabled, with full service name", Case{
-				enableDnsProxy:  "true",
-				addressFunction: fullAddress,
-				shouldWork:      true,
-			}),
-			Entry("Proxy Enabled, with short service name", Case{
-				enableDnsProxy:  "true",
-				addressFunction: shortAddress,
-				shouldWork:      false,
-			}),
-			Entry("Proxy Disabled, with full service name", Case{
-				enableDnsProxy:  "false",
-				addressFunction: fullAddress,
-				shouldWork:      true,
-			}),
-			Entry("Proxy Disabled, with short service name", Case{
-				enableDnsProxy:  "false",
-				addressFunction: shortAddress,
-				shouldWork:      true,
-			}),
-			Entry("Adding extra dns server, with Proxy Disabled and full address", Case{
-				enableDnsProxy:  "false",
-				dnsServer:       "8.8.8.8",
-				addressFunction: fullAddress,
-				shouldWork:      false,
-			}),
-			Entry("Adding extra dns server, with Proxy Enabled and full address", Case{
-				enableDnsProxy:  "true",
-				dnsServer:       "8.8.8.8",
-				addressFunction: fullAddress,
-				shouldWork:      false,
-			}),
-		)
+			Expect(sess.ExitCode()).To(BeZero())
+		})
 	}
 
 	Context("with gdn backend", func() {
-		expectedDnsProxyBehaviour(guardianRuntime)
+		DNSShouldWork(guardianRuntime)
 	})
 
 	Context("with containerd backend", func() {
-		expectedDnsProxyBehaviour(containerdRuntime)
+		DNSShouldWork(containerdRuntime)
 	})
 })

worker/workercmd/dns.go → worker/network/dns.go

@@ -1,16 +1,12 @@
-package workercmd
+package network
 
 import (
 	"fmt"
 
 	"github.com/miekg/dns"
 )
 
-type DNSConfig struct {
-	Enable bool `long:"enable" description:"Enable proxy DNS server."`
-}
-
-func (config DNSConfig) Server() (*dns.Server, error) {
+func DNSServer() (*dns.Server, error) {
 	resolvConf, err := dns.ClientConfigFromFile("/etc/resolv.conf")
 	if err != nil {
 		return nil, err

worker/workercmd/worker_linux.go

@@ -14,6 +14,7 @@ import (
 	"code.cloudfoundry.org/lager"
 	"github.com/concourse/concourse/atc"
 	concourseCmd "github.com/concourse/concourse/cmd"
+	"github.com/concourse/concourse/worker/network"
 	"github.com/concourse/flag"
 	"github.com/jessevdk/go-flags"
 	"github.com/tedsuo/ifrit"
@@ -49,6 +50,10 @@ type ContainerdRuntime struct {
 	NetworkPool        string    `long:"network-pool" default:"10.80.0.0/16" description:"Network range to use for dynamically allocated container subnets."`
 }
 
+type DNSConfig struct {
+	Enable bool `long:"enable" description:"Enable proxy DNS server."`
+}
+
 const containerdRuntime = "containerd"
 const guardianRuntime = "guardian"
 const houdiniRuntime = "houdini"
@@ -141,7 +146,7 @@ func (cmd *WorkerCommand) checkRoot() error {
 }
 
 func (cmd *WorkerCommand) dnsProxyRunner(logger lager.Logger) (ifrit.Runner, error) {
-	server, err := cmd.Guardian.DNS.Server()
+	server, err := network.DNSServer()
 	if err != nil {
 		return nil, err
 	}