[v0.14.x] Allow dnsmasq to be backed by a local copy of CoreDNS #1891

kfr2 · 2020-07-30T14:11:17Z

This commit allows the user to specify that dnsmasq should be backed by a pod-local copy of CoreDNS rather than relying on the global CoreDNS service. If enabled, the dnsmasq-node DaemonSet will be configured to use a local copy of CoreDNS for its resolution while setting the global CoreDNS service as a fallback. This is handy in situations where the number of DNS requests within a cluster grows large and causes resolution issues as dnsmasq reaches out to the global CoreDNS service.

Additionally, several values passed to dnsmasq are now configurable, including its --cache-size and --dns-forward-max.

See this postmortem for an investigation into this situation which was instrumental in understanding issues we were facing. Many thanks to @dominicgunn for providing the manifests which I codified into this commit.

These features can be enabled and tuned by setting the following values within cluster.yaml:

kubeDns:
  dnsmasq:
    coreDNSLocal:
      # When enabled, this will run a copy of CoreDNS within each DNS-masq pod and
      # configure the utility to use it for resolution.
      enabled: true

      # Defines the resource requests/limits for the coredns-local container.
      # cpu and/or memory constraints can be removed by setting the appropriate value(s)
      # to an empty string.
      resources:
        requests:
          cpu: 50m
          memory: 100Mi
        limits:
          cpu: 50m
          memory: 100Mi

    # The size of dnsmasq's cache.
    cacheSize: 50000

    # The maximum number of concurrent DNS queries.
    dnsForwardMax: 500

    # This option gives a default value for time-to-live (in seconds) which dnsmasq
    # uses to cache negative replies even in the absence of an SOA record.
    negTTL: 60

k8s-ci-robot · 2020-07-30T14:11:31Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign dominicgunn
You can assign the PR to them by writing /assign @dominicgunn in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

dominicgunn

Small merge conflict needs fixing.

dominicgunn · 2020-07-30T14:41:38Z

builtin/files/cluster.yaml.tmpl

@@ -1405,6 +1410,13 @@ kubeProxy:
 # It is enabled by default.
 #cloudFormationStreaming: true

+<<<<<<< Updated upstream


Can we fix this merge conflict?

dominicgunn · 2020-07-30T14:41:46Z

builtin/files/userdata/cloud-config-controller

+            application: coredns
+        data:
+          Corefile: |
+            cluster.local:9254 {{ .PodCIDR }}:9254 {{ .ServiceCIDR }}:9254 {


dominicgunn · 2020-07-30T14:43:40Z

builtin/files/userdata/cloud-config-controller

+          labels:
+            application: coredns
+        data:
+          Corefile: |


Re-reviewing this Corefile against Zalandos, I see that they actually have their custom (additional) DNS configuration above the cluster.local... block. I wonder if that performs better?

This commit introduces a new configuration option for cluster.yaml, `kubeDns.coreDNSLocal`. If this option and `kubeDns.nodeLocalResolver` are set to true, the dnsmasq-node DaemonSet will be configured to use a local copy of CoreDNS for its resolution while setting the global CoreDNS service as a fallback. This is handy in situations where the number of DNS requests within a cluster grows large and causes resolution issues as dnsmasq reaches out to the global CoreDNS service. See [0] for an investigation into this situation which was instrumental in understanding issues we were facing. Many thanks to dominicgunn for providing the manifests which I codified into this commit. 0: https://github.com/zalando-incubator/kubernetes-on-aws/blob/dev/docs/postmortems/jan-2019-dns-outage.md

With this commit, the user can override the following dnsmasq defaults: * --cache-size (50000) * --dns-forward-max (500) * --neg-ttl (60)

The dnsmasq-node's coredns-local's container limits can now be configured (or removed) by specifying the following: ``` dnsmasq: coreDNSLocal: limits: cpu: 100m memory: "" # disabled ```

dominicgunn · 2020-07-31T13:43:30Z

builtin/files/userdata/cloud-config-controller

@@ -1058,6 +1058,9 @@ write_files:
        "${mfdir}/kube-dns-de.yaml"
      {{- end }}
      {{ if .KubeDns.NodeLocalResolver -}}
+      {{ if .KubeDns.dnsmasq.CoreDNSLocal.Enabled -}}
+      deploy "${mfdir}/dnsmasq-node-coredns-local.yaml"


I know that kube-aws doesn't do a fantastic job of clean up, but seems like this one would be easy. Could we throw in a

{{- else }} remove "${mfdir}/canal.yaml" {{- end }}}

Sorry, remove "${mfdir}/dnsmasq-node-coredns-local.yaml"

👍 Will do.

dominicgunn · 2020-07-31T13:44:32Z

pkg/api/types.go

@@ -209,6 +209,23 @@ type IPVSMode struct {
 	MinSyncPeriod string `yaml:"minSyncPeriod"`
 }

+type CoreDNSLocalLimits struct {


Could we use ComputeResources instead?

Yep -- great idea!

coredns-local's resources can now be configured using: ``` kubeDns: dnsmasq: coreDNSLocal: resources: requests: cpu: "200m" memory: "1000Mi" limits: cpu: "400m" memory: "2000Mi" ```

dominicgunn · 2020-07-31T14:41:13Z

/lgtm

k8s-ci-robot added do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jul 30, 2020

k8s-ci-robot requested review from dominicgunn and mumoshu July 30, 2020 14:11

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 30, 2020

kfr2 changed the title ~~Allow dnsmasq to be backed by a local copy of CoreDNS~~ [v0.14.x] Allow dnsmasq to be backed by a local copy of CoreDNS Jul 30, 2020

kfr2 force-pushed the coredns-local-v0.14.x branch from 1adcbea to ad76f04 Compare July 30, 2020 14:42

k8s-ci-robot removed the do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. label Jul 30, 2020

dominicgunn suggested changes Jul 30, 2020

View reviewed changes

kfr2 force-pushed the coredns-local-v0.14.x branch from ad76f04 to 1ea50d8 Compare July 30, 2020 15:03

dominicgunn approved these changes Jul 30, 2020

View reviewed changes

kfr2 force-pushed the coredns-local-v0.14.x branch 3 times, most recently from 2406955 to a9e301c Compare July 30, 2020 18:06

kfr2 added 3 commits July 30, 2020 14:29

Inject AdditionalZoneCoreDNSConfig above other zone entries

7c80b39

Store dnsmasq settings under a new key

a7ded63

kfr2 force-pushed the coredns-local-v0.14.x branch from a9e301c to 7145636 Compare July 30, 2020 18:35

kfr2 added 2 commits July 30, 2020 15:00

Allow customization of values passed as dnsmasq flags

8d75bdd

With this commit, the user can override the following dnsmasq defaults: * --cache-size (50000) * --dns-forward-max (500) * --neg-ttl (60)

Apply CoreDNS-local manifest if needed

e4a412b

kfr2 force-pushed the coredns-local-v0.14.x branch from 7145636 to e4a412b Compare July 30, 2020 19:02

Make CoreDNS-local resource limits configurable

bbd9f97

The dnsmasq-node's coredns-local's container limits can now be configured (or removed) by specifying the following: ``` dnsmasq: coreDNSLocal: limits: cpu: 100m memory: "" # disabled ```

dominicgunn reviewed Jul 31, 2020

View reviewed changes

Make coredns-local's requests configurable as well

a2e0cf0

coredns-local's resources can now be configured using: ``` kubeDns: dnsmasq: coreDNSLocal: resources: requests: cpu: "200m" memory: "1000Mi" limits: cpu: "400m" memory: "2000Mi" ```

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 31, 2020

Remove coredns-local resources if the feature is not enabled

2d28eca

dominicgunn approved these changes Jul 31, 2020

View reviewed changes

k8s-ci-robot assigned dominicgunn Jul 31, 2020

k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 31, 2020

dominicgunn approved these changes Aug 1, 2020

View reviewed changes

dominicgunn merged commit d14e2fb into kubernetes-retired:v0.14.x Aug 1, 2020

This was referenced Aug 4, 2020

[v0.16.x] Allow dnsmasq to be backed by a local copy of CoreDNS #1895

Merged

[v0.17.x] Allow dnsmasq to be backed by a local copy of CoreDNS #1896

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[v0.14.x] Allow dnsmasq to be backed by a local copy of CoreDNS #1891

[v0.14.x] Allow dnsmasq to be backed by a local copy of CoreDNS #1891

Uh oh!

kfr2 commented Jul 30, 2020 •

edited

Loading

Uh oh!

k8s-ci-robot commented Jul 30, 2020

Uh oh!

dominicgunn left a comment

Uh oh!

dominicgunn Jul 30, 2020

Uh oh!

dominicgunn Jul 30, 2020

Uh oh!

dominicgunn Jul 30, 2020

Uh oh!

dominicgunn Jul 31, 2020

Uh oh!

dominicgunn Jul 31, 2020

Uh oh!

kfr2 Jul 31, 2020

Uh oh!

dominicgunn Jul 31, 2020

Uh oh!

kfr2 Jul 31, 2020

Uh oh!

dominicgunn commented Jul 31, 2020

Uh oh!

Uh oh!

[v0.14.x] Allow dnsmasq to be backed by a local copy of CoreDNS #1891

[v0.14.x] Allow dnsmasq to be backed by a local copy of CoreDNS #1891

Uh oh!

Conversation

kfr2 commented Jul 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Jul 30, 2020

Uh oh!

dominicgunn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dominicgunn commented Jul 31, 2020

Uh oh!

Uh oh!

kfr2 commented Jul 30, 2020 •

edited

Loading