diff --git a/plugin/kubernetes/README.md b/plugin/kubernetes/README.md index 613f0e869..b20b36ffc 100644 --- a/plugin/kubernetes/README.md +++ b/plugin/kubernetes/README.md @@ -87,8 +87,9 @@ kubernetes [ZONES...] { is vulnerable to abuse if used maliciously in conjunction with wildcard SSL certs. This option is provided for backward compatibility with kube-dns. * `verified`: Return an A record if there exists a pod in same namespace with matching IP. This - option requires substantially more memory than in insecure mode, since it will maintain a watch - on all pods. + option maintains a watch on all pods in the cluster, which requires additional memory in + CoreDNS (it keeps the IP, name, namespace and labels of every pod) and adds load to the + Kubernetes API server, since every pod state change in the cluster is streamed to CoreDNS. * `endpoint_pod_names` uses the pod name of the pod targeted by the endpoint as the endpoint name in A records, e.g., diff --git a/plugin/kubernetes/controller.go b/plugin/kubernetes/controller.go index b0c4bbefc..4a4ffa5f0 100644 --- a/plugin/kubernetes/controller.go +++ b/plugin/kubernetes/controller.go @@ -702,7 +702,9 @@ func (dns *dnsControl) detectChanges(oldObj, newObj any) { dns.updateMultiClusterModified() } case *object.Pod: - dns.updateModified() + if podModified(oldObj, newObj) { + dns.updateModified() + } case *object.Endpoints: if !endpointsEquivalent(oldObj.(*object.Endpoints), newObj.(*object.Endpoints)) { dns.updateModified() @@ -755,6 +757,19 @@ func subsetsEquivalent(sa, sb object.EndpointSubset) bool { return true } +// podModified checks if an update to a pod changes anything that is visible in +// DNS. Pod records and the pod IP index only depend on the pod IP, so all +// other pod status churn (conditions, container statuses, labels) does not +// need to bump the zone serial. +func podModified(oldObj, newObj any) bool { + oldPod, okOld := oldObj.(*object.Pod) + newPod, okNew := newObj.(*object.Pod) + if !okOld || !okNew { + return true + } + return oldPod.PodIP != newPod.PodIP +} + // endpointsEquivalent checks if the update to an endpoint is something // that matters to us or if they are effectively equivalent. func endpointsEquivalent(a, b *object.Endpoints) bool { diff --git a/plugin/kubernetes/controller_test.go b/plugin/kubernetes/controller_test.go index 75dcf84de..7b2d99f22 100644 --- a/plugin/kubernetes/controller_test.go +++ b/plugin/kubernetes/controller_test.go @@ -355,3 +355,51 @@ func TestServiceModified(t *testing.T) { } } } + +func TestPodModified(t *testing.T) { + var tests = []struct { + oldPod *object.Pod + newPod *object.Pod + changed bool + }{ + { + oldPod: &object.Pod{Version: "1", PodIP: "10.240.0.1", Name: "dns-test", Namespace: "testns"}, + newPod: &object.Pod{Version: "2", PodIP: "10.240.0.1", Name: "dns-test", Namespace: "testns"}, + changed: false, + }, + { + oldPod: &object.Pod{Version: "1", PodIP: "", Name: "dns-test", Namespace: "testns"}, + newPod: &object.Pod{Version: "2", PodIP: "10.240.0.1", Name: "dns-test", Namespace: "testns"}, + changed: true, + }, + { + oldPod: &object.Pod{Version: "1", PodIP: "10.240.0.1", Name: "dns-test", Namespace: "testns"}, + newPod: &object.Pod{Version: "2", PodIP: "10.240.0.2", Name: "dns-test", Namespace: "testns"}, + changed: true, + }, + } + + for i, test := range tests { + changed := podModified(test.oldPod, test.newPod) + if test.changed != changed { + t.Errorf("Expected %v for test %v. Got %v", test.changed, i, changed) + } + } +} + +func TestDetectChangesPodUpdate(t *testing.T) { + dns := &dnsControl{} + + p1 := &object.Pod{Version: "1", PodIP: "10.240.0.1", Name: "dns-test", Namespace: "testns"} + p2 := &object.Pod{Version: "2", PodIP: "10.240.0.1", Name: "dns-test", Namespace: "testns"} + dns.detectChanges(p1, p2) + if dns.Modified(ModifiedInternal) != 0 { + t.Fatal("pod update with an unchanged IP should not update the modified timestamp") + } + + p3 := &object.Pod{Version: "3", PodIP: "10.240.0.2", Name: "dns-test", Namespace: "testns"} + dns.detectChanges(p2, p3) + if dns.Modified(ModifiedInternal) == 0 { + t.Fatal("pod update with a changed IP should update the modified timestamp") + } +}