Add dual-stack support for node-cache by DockToFuture · Pull Request #657 · kubernetes/dns

DockToFuture · 2024-11-20T13:49:38Z

Add dual-stack support for node-cache

k8s-ci-robot · 2024-11-20T13:49:47Z

Welcome @DockToFuture!

It looks like this is your first PR to kubernetes/dns 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/dns has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2024-11-20T13:49:48Z

Hi @DockToFuture. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

k8s-triage-robot · 2025-02-18T13:53:10Z

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Mark this PR as fresh with /remove-lifecycle stale
Close this PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

DockToFuture · 2025-02-18T14:18:41Z

/remove-lifecycle stale

DamianSawicki · 2025-03-18T11:23:16Z

Implementation-wise, it would be ideal to have a test for this new feature.

Project-wise, let's get an agreement on #642 before merging this. I've just reopened it so that the discussion can continue.

ScheererJ · 2025-03-19T08:56:19Z

@DamianSawicki How is #642, which is about graceful shutdown/readiness, related to supporting dual-stack, i.e. IPv4 and IPv6? As of now node-local-dns only supports exclusively either IPv4 or IPv6, but not both at the same time. To me, these two features seem to be rather orthogonal to each other.

Please advise.

DamianSawicki · 2025-03-19T10:58:22Z

@DamianSawicki How is #642, which is about graceful shutdown/readiness, related to supporting dual-stack, i.e. IPv4 and IPv6? As of now node-local-dns only supports exclusively either IPv4 or IPv6, but not both at the same time. To me, these two features seem to be rather orthogonal to each other.

Please advise.

I'm terribly sorry, I somehow confused PR #669, which is related to Issue #642, with the present PR. Please disregard my comment.

ScheererJ · 2025-03-21T15:40:26Z

No problem at all. Thanks for clarifying.

k8s-triage-robot · 2025-06-24T19:07:49Z

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Mark this PR as fresh with /remove-lifecycle stale
Close this PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot · 2025-07-24T19:19:49Z

The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Mark this PR as fresh with /remove-lifecycle rotten
Close this PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

DamianSawicki

#655 is closed, should this PR remain open?

If so, it would be good guard this change with a feature flag and add some tests.

DamianSawicki · 2025-07-25T09:33:57Z

cmd/node-cache/app/cache_app.go

-			{utiliptables.Table("raw"), utiliptables.ChainOutput, []string{"-p", "tcp", "-s", localIP,
-				"--sport", c.params.HealthPort, "-j", "NOTRACK", "-m", "comment", "--comment", iptablesCommentSkipConntrack}},
-		}...)
+		if utilnet.IsIPv6(net.ParseIP(localIP)) {


If the loops for v4 and v6 should do the same, I'd suggest something like

const ( ipv4 int = iota ipv6 ) func isIPv6ToIndex(isIPv6 bool) int { if isIPv6 { return ipv6 } return ipv4 } ... i := isIPv6ToIndex(utilnet.IsIPv6(net.ParseIP(localIP))) c.iptablesRules[i] = = append(c.iptablesRules[i], ...)

DamianSawicki · 2025-07-25T09:34:18Z

cmd/node-cache/app/cache_app.go

+
 	}
-	c.iptables = newIPTables(c.isIPv6())
+	c.iptablesV4 = newIPTables(iptables.ProtocolIPv4)


Is this a system call? To save resources, it would probably be better to do something like

if len(c.iptablesRules[ipv4]) > 0 { c.iptables[ipv4] = newIPTables(iptables.ProtocolIPv4) } if len(c.iptablesRules[ipv6]) > 0 { c.iptables[ipv6] = newIPTables(iptables.ProtocolIPv6) }

DamianSawicki · 2025-07-25T09:34:30Z

cmd/node-cache/app/cache_app.go

 	}
 	if c.params.SetupIptables {
-		for _, rule := range c.iptablesRules {
+		for _, rule := range c.iptablesRulesV4 {


The treatment for ipv4 and ipv6 should be identical, right? If so, I'd suggest collapsing

iptablesV4 utiliptables.Interface iptablesV6 utiliptables.Interface

to

iptables [2]utiliptables.Interface

and similarly

iptablesRules [2][]iptablesRule

This way, we can write something like

for i := range(2) { for _, rule := range c.iptablesRules[i] { do something with c.iptables[i] instead of c.iptablesV4 or c.iptablesV6 } }

Actually, instead of i := range 2 we could even write _, i := range []int{ipv4, ipv6}.

DamianSawicki · 2025-07-25T09:37:06Z

cmd/node-cache/app/cache_app.go

 	if c.params.SetupIptables {
-		for _, rule := range c.iptablesRules {
-			exists, err := c.iptables.EnsureRule(utiliptables.Prepend, rule.table, rule.chain, rule.args...)
+		for _, rule := range c.iptablesRulesV4 {


Same remark about looping through {0,1} here.

DamianSawicki · 2025-07-25T09:37:18Z

cmd/node-cache/main.go

 		params.LocalIPs = append(params.LocalIPs, newIP)
 	}

-	// validate all the IPs have the same IP family


Are we sure the condition validated here is not assumed anywhere else? This seems to require testing. The deleted comment above func (c *CacheApp) isIPv6() bool says

// LocalIPs are guaranteed to have the same family

DockToFuture · 2025-07-25T10:16:58Z

@DamianSawicki is there some interest in dual-stack support for node-cache, as there were no reactions for a long time? Then I will adapt the PR.

DamianSawicki · 2025-07-25T10:52:16Z

@DamianSawicki is there some interest in dual-stack support for node-cache, as there were no reactions for a long time? Then I will adapt the PR.

Thanks for a quick response @DockToFuture! From comments above and in gardener/gardener#10891 (review), it looked that @ScheererJ was interested.

@Michcioperz @marqc, is dual-stack in node-cache something of interest to you?

DockToFuture · 2025-08-05T14:00:09Z

Any opinion on that @Michcioperz @marqc?

DamianSawicki · 2025-08-19T12:24:47Z

It's a holiday season, so let's give it some more time.

/remove-lifecycle rotten

k8s-ci-robot · 2025-10-20T11:29:09Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: DockToFuture
Once this PR has been reviewed and has the lgtm label, please assign damiansawicki for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

DockToFuture · 2025-10-20T12:55:33Z

Hello @DamianSawicki, I've updated the pull request and addressed the feedback you provided.

I've seen when I check out master and execute the tests, that the test are failing with:

(⎈|kind-gardener-extensions:default)➜  app git:(master) go test
# k8s.io/dns/cmd/node-cache/app [k8s.io/dns/cmd/node-cache/app.test]
./cache_app.go:59:21: undefined: utiliptables.Table
./cache_app.go:60:21: undefined: utiliptables.Chain
./cache_app.go:66:29: undefined: utiliptables.Interface
./cache_app.go:117:18: undefined: utiliptables.Table
./cache_app.go:117:45: undefined: utiliptables.ChainPrerouting
./cache_app.go:119:18: undefined: utiliptables.Table
./cache_app.go:119:45: undefined: utiliptables.ChainPrerouting
./cache_app.go:123:18: undefined: utiliptables.TableFilter
./cache_app.go:123:44: undefined: utiliptables.ChainInput
./cache_app.go:152:44: undefined: utiliptables.Interface
./cache_app.go:123:44: too many errors
FAIL    k8s.io/dns/cmd/node-cache/app [build failed]

This needs to be fixed upstream.

BR Sebastian

DockToFuture · 2025-12-08T16:01:53Z

Can we get this now in @DamianSawicki @Michcioperz @marqc?

axel7born · 2025-12-10T10:09:50Z

I see an additional motivation for merging this PR:

Without dual-stack support in node-local DNS cache, some DNS queries will bypass the cache and hit CoreDNS directly due to resolver behavior differences.

Example with nslookup:

nslookup sets the Recursion Desired (RD) flag in DNS queries
CoreDNS resolves the query successfully but doesn't set the Recursion Available (RA) bit in responses
nslookup interprets the missing RA bit as an error and fails over to the next nameserver
The failover queries the IPv6 address of the kube-dns service
Since node-local DNS cache currently only intercepts IPv4, these IPv6 queries bypass the cache entirely and reach CoreDNS directly

This defeats the purpose of the node-local DNS cache for dual-stack clusters and negates its performance and reliability benefits for tools like nslookup and potentially other resolvers with similar behavior.

DockToFuture · 2026-01-27T11:24:15Z

PTAL @DamianSawicki @Michcioperz @marqc

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 20, 2024

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Nov 20, 2024

k8s-ci-robot requested a review from kl52752 November 20, 2024 13:49

k8s-ci-robot requested a review from MrHohn November 20, 2024 13:49

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 20, 2024

DockToFuture mentioned this pull request Nov 20, 2024

Node local dns for dual-stack #655

Closed

ScheererJ mentioned this pull request Nov 22, 2024

Add support for node-local-dns in dual-stack cluster. gardener/gardener#10891

Merged

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 18, 2025

k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 18, 2025

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 24, 2025

k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jul 24, 2025

DamianSawicki reviewed Jul 25, 2025

View reviewed changes

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. labels Aug 19, 2025

add dual-stack support for node-cache

70561f0

DockToFuture force-pushed the support/dual-stack branch from 2646038 to 70561f0 Compare October 20, 2025 11:28

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 20, 2025

address review feedback

12f43ea

Conversation

DockToFuture commented Nov 20, 2024

Uh oh!

k8s-ci-robot commented Nov 20, 2024

Uh oh!

k8s-ci-robot commented Nov 20, 2024

Uh oh!

k8s-triage-robot commented Feb 18, 2025

Uh oh!

DockToFuture commented Feb 18, 2025

Uh oh!

DamianSawicki commented Mar 18, 2025

Uh oh!

ScheererJ commented Mar 19, 2025

Uh oh!

DamianSawicki commented Mar 19, 2025

Uh oh!

ScheererJ commented Mar 21, 2025

Uh oh!

k8s-triage-robot commented Jun 24, 2025

Uh oh!

k8s-triage-robot commented Jul 24, 2025

Uh oh!

DamianSawicki left a comment

Choose a reason for hiding this comment

Uh oh!

DamianSawicki Jul 25, 2025

Choose a reason for hiding this comment

Uh oh!

DamianSawicki Jul 25, 2025

Choose a reason for hiding this comment

Uh oh!

DamianSawicki Jul 25, 2025

Choose a reason for hiding this comment

Uh oh!

DamianSawicki Jul 25, 2025

Choose a reason for hiding this comment

Uh oh!

DamianSawicki Jul 25, 2025

Choose a reason for hiding this comment

Uh oh!

DockToFuture commented Jul 25, 2025

Uh oh!

DamianSawicki commented Jul 25, 2025

Uh oh!

DockToFuture commented Aug 5, 2025

Uh oh!

DamianSawicki commented Aug 19, 2025

Uh oh!

k8s-ci-robot commented Oct 20, 2025

Uh oh!

DockToFuture commented Oct 20, 2025

Uh oh!

DockToFuture commented Dec 8, 2025

Uh oh!

axel7born commented Dec 10, 2025

Uh oh!

DockToFuture commented Jan 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants