Skip to content

fix: flaky localdns exporter e2e validation#8454

Merged
djsly merged 2 commits into
mainfrom
jingwenwu/localdns-exporter-e2e-fix
Jun 30, 2026
Merged

fix: flaky localdns exporter e2e validation#8454
djsly merged 2 commits into
mainfrom
jingwenwu/localdns-exporter-e2e-fix

Conversation

@jingwenw15

@jingwenw15 jingwenw15 commented May 5, 2026

Copy link
Copy Markdown
Member

Summary

  • Replace echo "$VAR" | grep/awk/sed pipes with grep/awk/sed <<< "$VAR" here-strings throughout the exporter validation script
  • Capture ss -tln output once into a variable (SS_LISTEN_OUTPUT) instead of calling ss multiple times
  • Replace echo "$METRICS" | grep ... | head -10 with awk one-liners that exit after 10 matches

Why: With set -euo pipefail enabled, echo "$LARGE_VAR" | grep -q ... can fail spuriously. If grep finds its match and exits early, echo receives SIGPIPE and returns a non-zero exit status, which pipefail treats as a command failure. Here-strings (<<<) avoid the pipe entirely — the shell passes the variable directly to the command's stdin via a temporary file, so there is no writer process to receive SIGPIPE.

This was the root cause of intermittent test failures in the exporter e2e where metrics were present but the validation still failed.

Copilot AI review requested due to automatic review settings May 5, 2026 17:44
@jingwenw15 jingwenw15 changed the title Fix flaky localdns exporter e2e validation Fix: flaky localdns exporter e2e validation May 5, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Fixes flakiness in the LocalDNS exporter E2E validation script by avoiding pipefail-related false negatives when grepping large metrics payloads.

Changes:

  • Cache ss -tln output and reuse it for port checks and listen address detection.
  • Replace echo "$VAR" | grep ... patterns with here-strings (grep ... <<< "$VAR") to avoid SIGPIPE/pipefail failures.
  • Minor refactors to avoid pipelines (e.g., head, grep | sed) when inspecting $METRICS.

Comment thread e2e/localdns/validate-localdns-exporter-metrics.sh
@jingwenw15 jingwenw15 changed the title Fix: flaky localdns exporter e2e validation fix: flaky localdns exporter e2e validation May 5, 2026
@yewmsft

yewmsft commented May 20, 2026

Copy link
Copy Markdown
Member

The here-string conversion is a sound fix for the pipefail/SIGPIPE rationale in the inline comment — grep <<<"$VAR" keeps grep's stdin a regular file descriptor instead of a pipe that grep can close early on first match. Caching the ss -tln output once per check is a nice bonus that also removes a class of TOCTOU between the TCP/UDP probes.

Two small suggestions:

  1. Some of the swaps (e.g. head -n 1sed -n '1p') are pure style and don't change pipefail semantics — fewer churn lines = easier bisect if this validator regresses again. Worth keeping the diff scoped to the actual fix.

  2. Add a brief assertion (even just a shell snippet in a comment, or a tiny shellspec case if there's a harness nearby) that exercises the cached-ss path with a large fake payload, so a future well-meaning refactor that reverts to echo … | grep gets caught instead of going flaky again under load.

@yewmsft

yewmsft commented May 29, 2026

Copy link
Copy Markdown
Member

stop re-review

@jingwenw15 jingwenw15 requested a review from runzhen as a code owner June 16, 2026 19:58
@jingwenw15 jingwenw15 force-pushed the jingwenwu/localdns-exporter-e2e-fix branch from a01b4a8 to 61f3245 Compare June 30, 2026 20:05
@jingwenw15 jingwenw15 requested a review from xuexu6666 as a code owner June 30, 2026 20:05
@jingwenw15 jingwenw15 force-pushed the jingwenwu/localdns-exporter-e2e-fix branch from 61f3245 to 563b6ee Compare June 30, 2026 20:17
@djsly djsly merged commit 3e1ad43 into main Jun 30, 2026
18 of 21 checks passed
@djsly djsly deleted the jingwenwu/localdns-exporter-e2e-fix branch June 30, 2026 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants