Use external target for DNSPolicy test by lwr20 · Pull Request #35 · projectcalico/tiger-bench

lwr20 · 2026-01-30T21:25:23Z

Change dnsperf test to use external domain instead of internal fqdns
Fix tcpdump pods - they need to run as root to watch traffic
Add more validation to configuration switching

Copilot

Pull request overview

Updates the DNSPerf benchmark to use a configurable external target domain (instead of in-cluster pod/service FQDNs) when generating and exercising DNSPolicy tests.

Changes:

Add TargetDomain to DNSPerf config and thread it through DNSPolicy generation and DNSPerf test execution.
Improve cluster/DNS policy setup and cleanup by adding Calico NetworkPolicy deletion and additional Felix-mode verification via calico-node logs.
Add startup validation to ensure the test node pool has the required node count/labels.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
pkg/utils/k8s_utils.go	Adds Calico NetworkPolicy deletion helper and a helper to fetch calico-node pod logs.
pkg/dnsperf/dnsperf_test.go	Updates unit test to match new `MakeDNSPolicy(..., targetDomain)` signature and domain behavior.
pkg/dnsperf/dnsperf.go	Switches DNSPerf target selection to external `TargetDomain`, updates curl target formatting, and adjusts goroutine argument capture.
pkg/config/config.go	Replaces `TargetType` with `TargetDomain` and adds additional dnsperf validation logic.
pkg/cluster/cluster.go	Enhances Felix DNS policy mode patching by reading dataplane from Installation and adding post-update verification/log inspection.
cmd/benchmark.go	Initializes controller-runtime logger, validates test-node prerequisites, updates DNSPolicy creation call, and expands cleanup.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

pkg/cluster/cluster.go

pkg/utils/k8s_utils.go

pkg/config/config.go

pkg/dnsperf/dnsperf.go

pkg/cluster/cluster.go

Copilot

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

pkg/dnsperf/dnsperf.go

cmd/benchmark.go

Copilot

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 8 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

cmd/benchmark.go

pkg/dnsperf/dnsperf.go

pkg/utils/k8s_utils.go

pkg/config/config.go

pkg/dnsperf/README.md

pkg/config/config.go

Copilot

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

pkg/cluster/cluster.go

pkg/dnsperf/dnsperf.go

pkg/dnsperf/README.md

Copilot

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

pkg/dnsperf/dnsperf.go

Copilot

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

pkg/dnsperf/README.md

Copilot · 2026-02-03T16:41:38Z

pkg/dnsperf/dnsperf.go

+			go func(tcpdumpPod corev1.Pod, testPod corev1.Pod) {
+				if e := runTCPDump(testctx, clients, &tcpdumpPod, testPod, targetPort, testConfig.Duration+60); e != nil {
+					log.WithError(e).Error("failed to run tcpdump")


runTCPDump is started with testctx, which is cancelled after testConfig.Duration. That means the tcpdump exec will be interrupted at the test duration regardless of the timeout argument (Duration+60), so you won’t actually capture the extra tail time. Consider using a separate context with a longer timeout (or the parent ctx) for tcpdump so the timeout parameter is meaningful.

Its just a failsafe.

Copilot · 2026-02-03T16:41:38Z

pkg/dnsperf/dnsperf.go

 	cmdfrag := `curl -m 8 -w '{"time_lookup": %{time_namelookup}, "time_connect": %{time_connect}}\n' -s -o /dev/null`
-	cmd := fmt.Sprintf("%s %s:8080", cmdfrag, target)
+	cmd := fmt.Sprintf("%s %s", cmdfrag, target)
 	stdout, _, err := utils.ExecCommandInPod(ctx, srcPod, cmd, 10)


runDNSPerfTest builds a /bin/sh -c command by string-concatenating the full URL. If TargetURL contains shell metacharacters (e.g. &, ;, #, spaces), curl will receive a mangled URL or unintended extra shell commands may run. Quote/escape the URL argument (or avoid the shell and exec curl with args) to make external URLs robust.

Copilot · 2026-02-03T16:41:39Z

pkg/dnsperf/dnsperf.go

+			if token == "IP" && i+3 < len(tokens) {
+				srcAddr = tokens[i+1]
+				if tokens[i+2] == ">" {
+					// Remove trailing colon from destination
+					dstAddr = strings.TrimSuffix(tokens[i+3], ":")


processTCPDumpOutput only recognizes the "IP" token when extracting src/dst, so IPv6 tcpdump lines (which typically use the "IP6" token) will be skipped and duplicate SYN/SYN-ACK counts will be wrong on IPv6 clusters. Please handle both "IP" and "IP6" (or parse more defensively).

We won't be testing with IPv6 for now.

Copilot · 2026-02-03T16:41:39Z

pkg/utils/k8s_utils.go

+		scanner := bufio.NewScanner(logs)
+		for scanner.Scan() {
+			line := scanner.Text()


bufio.Scanner has a default token limit (~64K). If a calico-node log line exceeds that, scanning will error and this helper will fail (potentially causing Inline-mode validation to fail spuriously). Consider increasing the scanner buffer size (or using bufio.Reader) before scanning.

Should be fine.

Copilot · 2026-02-03T16:41:39Z

pkg/cluster/cluster.go

+	// If we're using Inline, check felix logs to verify mode change - if Inline mode is not possible, Felix will fall back to DelayDeniedPacket mode and log this.
+	// The log will be: "Failed to load BPF DNS parser program. Maybe the kernel is old. Falling back to DelayDeniedPacket"
+	if dnsPolicyMode == "Inline" {


The Inline-mode fallback check (wait for TigeraStatus + sleep + scan calico-node logs) runs whenever dnsPolicyMode == "Inline", but the log message being searched for is specifically about loading the BPF DNS parser program. This adds ~70s of delay even on non-BPF dataplanes where that fallback isn’t applicable. Consider gating this log check on the BPF dataplane only.

Suggested change

// If we're using Inline, check felix logs to verify mode change - if Inline mode is not possible, Felix will fall back to DelayDeniedPacket mode and log this.

// The log will be: "Failed to load BPF DNS parser program. Maybe the kernel is old. Falling back to DelayDeniedPacket"

if dnsPolicyMode == "Inline" {

// If we're using Inline on the BPF dataplane, check felix logs to verify mode change - if Inline mode is not possible, Felix will fall back to DelayDeniedPacket mode and log this.

// The log will be: "Failed to load BPF DNS parser program. Maybe the kernel is old. Falling back to DelayDeniedPacket"

if dnsPolicyMode == "Inline" && dataplane != nil && *dataplane == operatorv1.LinuxDataplaneBPF {

The change is just wrong, this applies for Inline mode always.

Use external target for DNSPolicy test

2bf2f47

lwr20 requested a review from Copilot January 30, 2026 21:25

Copilot started reviewing on behalf of lwr20 January 30, 2026 21:25 View session

Copilot AI reviewed Jan 30, 2026

View reviewed changes

lwr20 requested a review from Copilot January 30, 2026 22:06

Copilot started reviewing on behalf of lwr20 January 30, 2026 22:06 View session

lwr20 force-pushed the lwr-dnsperf-ext branch from 3374e8a to 95a2b43 Compare January 30, 2026 22:08

Copilot AI reviewed Jan 30, 2026

View reviewed changes

pkg/dnsperf/dnsperf.go Outdated Show resolved Hide resolved

cmd/benchmark.go Outdated Show resolved Hide resolved

review markups

53c12fe

lwr20 force-pushed the lwr-dnsperf-ext branch from 95a2b43 to 53c12fe Compare January 30, 2026 22:22

lwr20 marked this pull request as ready for review January 30, 2026 22:34

This comment was marked as outdated.

Sign in to view

lwr20 requested a review from Copilot February 1, 2026 23:31

Copilot started reviewing on behalf of lwr20 February 1, 2026 23:31 View session

Merge branch 'main' into lwr-dnsperf-ext

0feb83b

Copilot AI reviewed Feb 1, 2026

View reviewed changes

Improve tcpdump reliability

10ea8a2

lwr20 force-pushed the lwr-dnsperf-ext branch from 7b9750a to 24e9527 Compare February 2, 2026 16:31

Review markups

ab6a07a

lwr20 force-pushed the lwr-dnsperf-ext branch from 24e9527 to ab6a07a Compare February 2, 2026 16:40

lwr20 requested a review from Copilot February 2, 2026 16:45

Copilot started reviewing on behalf of lwr20 February 2, 2026 16:45 View session

Copilot AI reviewed Feb 2, 2026

View reviewed changes

Specify target URL, not domain

0cd5521

lwr20 force-pushed the lwr-dnsperf-ext branch from 36f3b00 to 0cd5521 Compare February 3, 2026 15:00

Review markups

0259d33

lwr20 requested a review from Copilot February 3, 2026 15:35

Copilot started reviewing on behalf of lwr20 February 3, 2026 15:35 View session

Copilot AI reviewed Feb 3, 2026

View reviewed changes

pkg/dnsperf/dnsperf.go Outdated Show resolved Hide resolved

pkg/dnsperf/dnsperf.go Outdated Show resolved Hide resolved

lwr20 requested a review from Copilot February 3, 2026 16:29

Copilot started reviewing on behalf of lwr20 February 3, 2026 16:29 View session

Copilot AI reviewed Feb 3, 2026

View reviewed changes

More review markups

1e034aa

lwr20 force-pushed the lwr-dnsperf-ext branch from 58948ff to 1e034aa Compare February 3, 2026 18:34

lwr20 merged commit 81cab28 into projectcalico:main Feb 6, 2026
6 checks passed

Conversation

lwr20 commented Jan 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

lwr20 Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

lwr20 Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

lwr20 Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

lwr20 commented Jan 30, 2026 •

edited

Loading

lwr20 Feb 3, 2026 •

edited

Loading