plugin/kubernetes: skip zone serial bump on DNS neutral pod updates (#8338)

* plugin/kubernetes: skip zone serial bump on DNS neutral pod updates

In pods verified mode every pod update event bumped the zone modified
timestamp, even when the pod IP did not change. Pod records only depend
on the pod IP, so routine status churn (conditions, container statuses,
labels) caused spurious SOA serial changes and needless zone transfer
activity, even though pod records are not part of transfers at all.

Only bump the modified timestamp when the pod IP changes, mirroring how
service and endpoint updates are already filtered. Also update the pods
verified documentation to describe the actual overhead: modest memory
for a stripped down pod object, plus watch load on the API server.

Ref #8043

Signed-off-by: Karan V <karanvknarayanan@gmail.com>

* plugin/kubernetes: keep pods verified cost description neutral

Avoid characterizing the memory overhead as modest until benchmark
data quantifies it. State only what the code does: the watch requires
additional memory in CoreDNS and adds load to the API server.

Signed-off-by: Karan V <karanvknarayanan@gmail.com>

---------

Signed-off-by: Karan V <karanvknarayanan@gmail.com>

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
+     option maintains a watch on all pods in the cluster, which requires additional memory in
-     on all pods.
+     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.,

plugin/kubernetes/controller.go

@@ -702,7 +702,9 @@ func (dns *dnsControl) detectChanges(oldObj, newObj any) {
 			dns.updateMultiClusterModified()
 		}
 	case *object.Pod:
+		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 {

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")
+	}
+}