Lowers the default nodelocaldns denial cache TTL

[blakebarnett]

What type of PR is this?
/kind bug

What this PR does / why we need it:
Similar to --no-negcache on dnsmasq, this prevents issues which poll DNS for orchestration such as operators with StatefulSets. It can also be very confusing for users when negative caching results in a change they just made seeming to be broken until the cache expires. This assumes that 5 seconds is reasonable and will still catch repeated AAAA negative responses. We could also set the denial cache size to zero which should effectively fully disable it like dnsmasq in kube-dns but testing shows this approach seems to work well in our (albeit small) test clusters.

Which issue(s) this PR fixes:

Fixes #74092

Does this PR introduce a user-facing change?:

Reduces the cache TTL for negative responses to 5s minimum.

[blakebarnett]

/sig network

[k8s-ci-robot]

Hi @blakebarnett. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[MrHohn]

/assign @prameshj

[MrHohn]

/ok-to-test

[chrisohaver]

In case negative responses in the cluster dns domain is the primary concern here, these negative responses default to a TTL of 5 as of CoreDNS 1.3.1.

e.g.

dnstools# dig notthere.default.svc.cluster.local. A

; <<>> DiG 9.11.3 <<>> notthere.default.svc.cluster.local. A
;; global options: +cmd
;; Got answer:
;; ->>HEADER<<- opcode: QUERY, status: NXDOMAIN, id: 34026
;; flags: qr aa rd; QUERY: 1, ANSWER: 0, AUTHORITY: 1, ADDITIONAL: 1
;; WARNING: recursion requested but not available

;; OPT PSEUDOSECTION:
; EDNS: version: 0, flags:; udp: 4096
;; QUESTION SECTION:
;notthere.default.svc.cluster.local. IN	A

;; AUTHORITY SECTION:
cluster.local.		5	IN	SOA	ns.dns.cluster.local. hostmaster.cluster.local. 1549979885 7200 1800 86400 5

;; Query time: 0 msec
;; SERVER: 10.96.0.10#53(10.96.0.10)
;; WHEN: Thu Feb 14 20:15:31 UTC 2019
;; MSG SIZE  rcvd: 168

[blakebarnett]

Thanks!

I was unaware, I verified we've been testing against a build using CoreDNS v1.2.5, I'll update and test again now.

[blakebarnett]

Ah, seems the latest published image is still only v1.2.6. @prameshj is upgrading to 1.3.1 and publishing an image viable?

[chrisohaver]

CoreDNS 1.3.1 is already in the k8s.gcr.io repo: k8s.gcr.io/coredns:1.3.1.

These negative responses originate from the cluster dns, not the node local dns. The version of the node local dns should not make a difference. IIUC

[prameshj]

CoreDNS 1.3.1 is already in the k8s.gcr.io repo: k8s.gcr.io/coredns:1.3.1.

These negative responses originate from the cluster dns, not the node local dns. The version of the node local dns should not make a difference. IIUC

Yes, that's how I understand it too, clusterDNS is setting the TTL on the record. If we use coreDNS as cluster dns, this TTL on the record will be 5s. So no need to use any TTL on the cache side.
However, with kube-dns, the default TTL is 30s.

[blakebarnett]

Ah, we've not moved to CoreDNS yet, so in this case I suppose this change is still needed.

[blakebarnett]

GKE is still defaulting to kube-dns also AFAIK (at least when we built our 1.11.5 cluster)

[prameshj]

In this change, the cache size for success and denial is specified as 10000, but it will be rounded down to nearest multiple of 256 = 9984, which is the default cache size value if nothing is specified.
https://github.com/coredns/coredns/tree/master/plugin/cache

So the only change here is specifying a lower TTL for denial, that looks good to me.

[prameshj]

/lgtm

[prameshj]

/priority important-soon
/approved

[prameshj]

/lgtm
/approve

[MrHohn]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: blakebarnett, MrHohn, prameshj

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

The pull request process is described here

Needs approval from an approver in each of these files:

• cluster/addons/dns/OWNERS [MrHohn]

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