fix: add DNS resolution fallback in CheckProtocol() and export resolved env vars in start-db.sh by cleman95 · Pull Request #7342 · kubeovn/kube-ovn

cleman95 · 2026-08-28T18:15:10Z

Description

In HCP mode, ovn-central intentionally supplies a headless-Service hostname as POD_IP (e.g. ovn-central-0.ovn-central.kube-system.svc), while the leader checker needs the address family to pick the EndpointSlice address type (#7341).

Per review feedback, this PR keeps the generic CheckProtocol validator literal-only and introduces an explicit, testable resolver used only by the HCP leader-checker path:

util.resolveProtocol (injectable lookup) + util.ResolveProtocol (wrapper over net.DefaultResolver) in pkg/util/net.go
Dual-stack behavior is defined explicitly: when a hostname resolves to both A and AAAA records, IPv4 is preferred — deterministic, since resolver ordering is not and callers only distinguish IPv4/IPv6
The leader checker falls back to IPv4 with a warning when resolution fails, preserving its previous defaulting behavior
CheckProtocol is unchanged: no ambient DNS lookups from IP/CIDR validation paths
start-db.sh: export the shell-resolved POD_IP/NODE_IPS/DB_CLUSTER_ADDR after normalize_raft_addrs

Changes

pkg/util/net.go: add resolveProtocol (injectable lookup) + ResolveProtocol; CheckProtocol untouched
pkg/ovn_leader_checker/ovn.go: classify POD_IP via ResolveProtocol, warn + default to IPv4 on failure
pkg/util/net_test.go: TestResolveProtocol with fake lookup covering literal v4/v6, hostname→v4/v6, hostname→both families (asserts IPv4 determinism), lookup failure, empty result, empty input
dist/images/start-db.sh: export resolved env values

Review notes

The container capabilities change initially bundled here was split into a separate PR (to be linked).

Test Plan

go test ./pkg/util/ -run "TestCheckProtocol|TestResolveProtocol" -v — all pass, including the previously failing invalid-input cases (not-an-ip, 1.2.3)
go test ./pkg/ovn_leader_checker/ — all pass

Fixes #7341

zhangzujian

Request changes. Issue #7341 is valid: HCP ovn-central intentionally supplies a headless-Service hostname as POD_IP, while the leader checker needs the address family. The two DNS-related changes are directionally relevant, but the current implementation introduces a broad validator regression and lacks deterministic hostname coverage. The capability change is unrelated to this issue and should not be bundled without covering both chart variants.

zhangzujian · 2026-09-02T11:17:15Z

 	if ip == nil {
-		klog.Errorf("failed to parse address %q", address)
-		return ""
+		addrs, err := net.LookupIP(address)


[P1] Please keep the generic CheckProtocol validator literal-only, or move hostname resolution behind an explicit resolver used by the HCP leader-checker path. CheckProtocol is called throughout IP/CIDR validation, so this fallback makes arbitrary invalid strings trigger blocking ambient DNS lookups and changes them into valid protocols when they happen to resolve. In the PR worktree, go test ./pkg/util already fails because the existing invalid-input cases not-an-ip and 1.2.3 resolve to IPv4. Please add controlled regression tests for hostname success/failure without making all validation dependent on cluster DNS.

Done. CheckProtocol is back to literal-only, and hostname resolution now lives in an explicit resolver:

util.resolveProtocol(ctx, lookup, address) injectable lookup function so regression tests are deterministic and independent of cluster DNS

util.ResolveProtocol exported wrapper over net.DefaultResolver

The leader checker is the only caller. Regression tests added in TestResolveProtocol cover hostname success/failure; TestCheckProtocol keeps its invalid-input cases (not-an-ip, 1.2.3) as literal-only.

zhangzujian · 2026-09-02T11:17:15Z

+			klog.Errorf("failed to parse or resolve address %q: %v", address, err)
+			return ""
+		}
+		ip = addrs[0]


[P1] addrs[0] is not a deterministic protocol choice for a hostname with both A and AAAA records. Resolver ordering can vary, so the same HCP pod may be classified as IPv4 or IPv6 across environments. Please define the dual-stack behavior explicitly (or use an address-family-aware lookup) and add a regression test covering a hostname that returns both families.

Done. Dual-stack behavior is now defined explicitly: when a hostname resolves to both A and AAAA records, IPv4 is preferred. The choice must be deterministic because resolver ordering is not, and callers (EndpointSlice address type) only distinguish IPv4 from IPv6. Both orderings (v6-first and v4-first resolver output) are asserted in TestResolveProtocol/hostnameResolvesBothFamilies*.

zhangzujian · 2026-09-02T11:17:15Z

              {{- if .Values.central.hcp.enabled }}
              add:
                - CHOWN
+                - DAC_OVERRIDE


[P2] This capability change is unrelated to #7341's DNS/leader-checker fix and changes the HCP container privilege model. It also updates only kube-ovn-v2; the equivalent v1 HCP template still runs the same chown -R with only CHOWN while dropping all other capabilities. Please split this into a separate PR, or update both supported charts with focused rendering tests and explain why it belongs here.

Agreed this change was unrelated to #7341 and only touched one chart. It is removed from this PR and will be submitted separately, covering both charts (kube-ovn and kube-ovn-v2)

…lver In HCP mode ovn-central intentionally supplies a headless-Service hostname (e.g. ovn-central-0.ovn-central.kube-system.svc) as POD_IP, while the leader checker needs the address family to pick the EndpointSlice address type. Keep the generic CheckProtocol validator literal-only: it is called throughout IP/CIDR validation and a DNS fallback there would make arbitrary invalid strings trigger blocking cluster DNS lookups and turn them into valid protocols when they happen to resolve. Instead, add an explicit resolver: - util.resolveProtocol classifies a literal IP or hostname into ProtocolIPv4/ProtocolIPv6, with an injectable lookup function so regression tests are deterministic and independent of cluster DNS. - util.ResolveProtocol is the exported wrapper over net.DefaultResolver. Dual-stack behavior is defined explicitly: when a hostname resolves to both address families, IPv4 is preferred. The choice must be deterministic because resolver ordering is not, and callers only distinguish IPv4 from IPv6. The leader checker uses ResolveProtocol for POD_IP and falls back to IPv4 with a warning when resolution fails, preserving its previous defaulting behavior. POD_IPS (real pod IPs from the downward API) is unaffected. Fixes kubeovn#7341 Signed-off-by: Clement Phu <[email protected]>

normalize_raft_addrs resolves the HCP raft addresses from hostnames to IPs in the shell, but the resolved values were only visible to child processes spawned by start-db.sh. Export POD_IP, NODE_IPS and DB_CLUSTER_ADDR after normalization so every consumer gets the resolved values. Signed-off-by: Clement Phu <[email protected]>

cleman95 · 2026-09-07T09:23:15Z

The capabilities change is now in a standalone PR covering both charts: #7386.

dosubot Bot added size:XS This PR changes 0-9 lines, ignoring generated files. bug Something isn't working labels Aug 28, 2026

cleman95 force-pushed the fix-checkprotocol-dns-clean branch 2 times, most recently from 8e24c48 to 8695344 Compare August 28, 2026 18:36

cleman95 mentioned this pull request Aug 31, 2026

Split Helm chart into control-plane and data-plane + fix upgrade hook for dataPlaneOnly mode #7339

Open

zhangzujian requested changes Sep 2, 2026

View reviewed changes

cleman95 added 2 commits September 7, 2026 11:19

cleman95 force-pushed the fix-checkprotocol-dns-clean branch from 533e532 to 9ceee60 Compare September 7, 2026 09:19

cleman95 mentioned this pull request Sep 7, 2026

fix(chart): grant volume-init DAC capabilities in HCP mode #7386

Open

Navigation Menu

Uh oh!

fix: add DNS resolution fallback in CheckProtocol() and export resolved env vars in start-db.sh - #7342

fix: add DNS resolution fallback in CheckProtocol() and export resolved env vars in start-db.sh#7342
cleman95 wants to merge 2 commits into
kubeovn:masterfrom
cleman95:fix-checkprotocol-dns-clean

cleman95 commented Aug 28, 2026 •

edited

Loading

Uh oh!

zhangzujian left a comment

Uh oh!

zhangzujian Sep 2, 2026

Uh oh!

cleman95 Sep 7, 2026 •

edited

Loading

Uh oh!

zhangzujian Sep 2, 2026

Uh oh!

cleman95 Sep 7, 2026

Uh oh!

zhangzujian Sep 2, 2026

Uh oh!

cleman95 Sep 7, 2026 •

edited

Loading

Uh oh!

cleman95 commented Sep 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

cleman95 commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Review notes

Test Plan

Uh oh!

zhangzujian left a comment

Choose a reason for hiding this comment

Uh oh!

zhangzujian Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

cleman95 Sep 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhangzujian Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

cleman95 Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

zhangzujian Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

cleman95 Sep 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cleman95 commented Sep 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cleman95 commented Aug 28, 2026 •

edited

Loading

cleman95 Sep 7, 2026 •

edited

Loading

cleman95 Sep 7, 2026 •

edited

Loading