kube-proxy: log iptables errors in platformCheckSupported #131534

aroradaman · 2025-04-29T17:24:21Z

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

Currently we check the existence of the default iptables chain to ensure if iptables is supported by the platform. We don't log the actual error and stdout when the check fails.

E0426 10:25:33.612382       1 server.go:136] "Error running ProxyServer" err="iptables is not available on this host"                                                                                              
E0426 10:25:33.612393       1 run.go:72] "command failed" err="iptables is not available on this host"

Logging the actual stdout/stderr will help to understand the root cause of the actual failure. It can help in covering the cases when privileges are not properly configured for the kube-proxy daemonset, removal of a deprecated flag from the user space binary and many more.

E0507 16:05:47.509891       1 server.go:136] "Error running ProxyServer" err=<
	iptables is not available on this host : error listing chain "POSTROUTING" in table "nat": exit status 4: Ignoring deprecated --wait-interval option.
	iptables v1.8.9 (nf_tables): Could not fetch rule set generation id: Permission denied (you must be root)
 >
E0507 16:05:47.509907       1 run.go:72] "command failed" err=<
	iptables is not available on this host : error listing chain "POSTROUTING" in table "nat": exit status 4: Ignoring deprecated --wait-interval option.
	iptables v1.8.9 (nf_tables): Could not fetch rule set generation id: Permission denied (you must be root)
 >

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot · 2025-04-29T17:24:29Z

This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

danwinship · 2025-04-29T18:46:40Z

pkg/util/iptables/iptables.go

 	}

-	return true
+	return true, nil


The bool is redundant at this point. Just have it return an error only.

danwinship · 2025-04-29T18:49:14Z

cmd/kube-proxy/app/server_linux.go

+		ipts, err = utiliptables.NewDualStack()
+		if err != nil {
+			logger.Error(err, "Encountered errors in setting up iptables")
+		}


I would merge this with the existing error handling: if neither ipv4 nor ipv6 is supported, wrap this error as part of the return value from this function. If only one of them is not supported, log the error message as part of the "no support for family" message below (keeping it as logger.Info since it's probably an intentional configuration rather than an error in that case)

danwinship

basically good, just some nitpicks

danwinship · 2025-04-30T13:36:21Z

pkg/util/iptables/iptables.go

 		interfaces[v1.IPv6Protocol] = iptv6
 	}

-	return interfaces
+	return interfaces, errors.NewAggregate(errs)
 }

 // NewDualStack returns a map containing an IPv4 Interface (if IPv4 iptables is supported)
 // and an IPv6 Interface (if IPv6 iptables is supported). If either family is not
 // supported, no Interface will be returned for that family.


The semantics are slightly tricky now so we need to document that:

// Note that this function will return a single iptables Interface *and* an error, if only // a single family is supported.

danwinship · 2025-04-30T13:38:28Z

pkg/util/iptables/iptables.go

 	iptv6 := newInternal(exec, ProtocolIPv6, "", "")
-	if iptv6.Present() {
+	if err := iptv6.Present(); err != nil {
+		errs = append(errs, err)


If we get an error for both IPv4 and IPv6, it's virtually guaranteed that they're going to be the same error. So I would say that rather than combining the two, we should just ignore the IPv6 error and return only the IPv4 error in that case.

So:

if both Present() calls fail, then return the error from iptv4.Present()

if one Present() call succeeds and one fails then return the error from the one that failed

(if both succeed then return no error)

This reverts commit 9cb3dfb.

danwinship · 2025-05-08T12:23:50Z

/lgtm
/assign @thockin
needs a higher-level approver for the trivial pkg/kubelet change to adapt to the new API

k8s-ci-robot · 2025-05-08T12:23:57Z

LGTM label has been added.

Git tree hash: 332322d7262e9c4c360992487b6af31f5a401440

aojea · 2025-05-09T09:51:56Z

pkg/util/iptables/iptables.go

+		// If we get an error for both IPv4 and IPv6 Present() calls, it's virtually guaranteed that
+		// they're going to be the same error. We ignore the error for IPv6 if IPv4 has already failed.
+		if err == nil {
+			err = presentErr
+		}


errors.Join is supposed to solve this problem https://pkg.go.dev/errors#Join

I was using NewAggregate(errs) from k8s.io/apimachinery/pkg/util/errors previously. If both the proxiers fails, they will fail with the same error only.

yeah, the point is, we don't want to see "iptables: command not found, ip6tables: command not found". Just "iptables: command not found" is enough.

SergeyKanzhelev · 2025-06-06T19:36:02Z

pkg/kubelet/kubelet_network_linux.go

@@ -37,7 +37,7 @@ const (
 )

 func (kl *Kubelet) initNetworkUtil() {
-	iptClients := utiliptables.NewDualStack()
+	iptClients, _ := utiliptables.NewDualStack()
 	if len(iptClients) == 0 {


can this please be changed to:

Suggested change

if len(iptClients) == 0 {

if err != nil || len(iptClients) == 0 {

And err added to the log.

Relying on return value when the err is not nil is not a good practice

Signed-off-by: Daman Arora <[email protected]>

thockin · 2025-06-07T00:23:50Z

/approve
/hold for sergey

SergeyKanzhelev

/lgtm
/approve
/unhold

k8s-ci-robot · 2025-06-07T00:25:48Z

LGTM label has been added.

Git tree hash: 0cd76d558526a36f8ba3e4f9a6abe1a1b84e2f64

k8s-ci-robot · 2025-06-07T00:25:54Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aroradaman, danwinship, SergeyKanzhelev, thockin

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~cmd/kube-proxy/OWNERS~~ [danwinship,thockin]
~~pkg/kubelet/OWNERS~~ [SergeyKanzhelev,thockin]
~~pkg/proxy/OWNERS~~ [danwinship,thockin]
~~pkg/util/iptables/OWNERS~~ [danwinship,thockin]

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

k8s-triage-robot · 2025-06-07T03:41:05Z

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

The PR does have any do-not-merge/* labels
The PR does not have the needs-ok-to-test label
The PR is mergeable (does not have a needs-rebase label)
The PR is approved (has cncf-cla: yes, lgtm, approved labels)
The PR is failing tests required for merge

You can:

Review the full test history for this PR
Prevent this bot from retesting with /lgtm cancel or /hold
Help make our tests less flaky by following our Flaky Tests Guide

/retest

k8s-ci-robot added the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Apr 29, 2025

k8s-ci-robot requested review from bowei and danwinship April 29, 2025 17:24

k8s-ci-robot added area/kube-proxy area/kubelet sig/network Categorizes an issue or PR as relevant to SIG Network. sig/node Categorizes an issue or PR as relevant to SIG Node. labels Apr 29, 2025

github-project-automation bot added this to SIG Node: code and documentation PRs Apr 29, 2025

k8s-ci-robot removed the do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Apr 29, 2025

github-project-automation bot moved this to Triage in SIG Node: code and documentation PRs Apr 29, 2025

aroradaman force-pushed the kube-proxy-ipt-init-error-2 branch 2 times, most recently from 665b9c1 to cd26436 Compare April 29, 2025 17:28

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

aroradaman force-pushed the kube-proxy-ipt-init-error-2 branch from cd26436 to 265c370 Compare April 29, 2025 18:12

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

aroradaman force-pushed the kube-proxy-ipt-init-error-2 branch from 265c370 to 771bb51 Compare April 29, 2025 18:27

danwinship reviewed Apr 29, 2025

View reviewed changes

aroradaman force-pushed the kube-proxy-ipt-init-error-2 branch from 771bb51 to 029484a Compare April 29, 2025 18:58

danwinship approved these changes Apr 30, 2025

View reviewed changes

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

Revert "kube-proxy: log errors during proxy boot"

57218ac

This reverts commit 9cb3dfb.

aroradaman force-pushed the kube-proxy-ipt-init-error-2 branch from 029484a to cbe1be3 Compare May 7, 2025 16:07

aroradaman requested a review from danwinship May 7, 2025 16:07

k8s-ci-robot added the area/ipvs label May 7, 2025

k8s-ci-robot assigned thockin and danwinship May 8, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 8, 2025

aojea reviewed May 9, 2025

View reviewed changes

SergeyKanzhelev moved this from Triage to Needs Approver in SIG Node: code and documentation PRs May 31, 2025

SergeyKanzhelev reviewed Jun 6, 2025

View reviewed changes

SergeyKanzhelev moved this from Needs Approver to Waiting on Author in SIG Node: code and documentation PRs Jun 6, 2025

kube-proxy: log ipt errors during platformCheckSupported

858b88b

Signed-off-by: Daman Arora <[email protected]>

aroradaman force-pushed the kube-proxy-ipt-init-error-2 branch from cbe1be3 to 858b88b Compare June 6, 2025 19:45

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 6, 2025

k8s-ci-robot requested a review from thockin June 6, 2025 19:45

aroradaman requested a review from SergeyKanzhelev June 6, 2025 19:45

k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 7, 2025

SergeyKanzhelev approved these changes Jun 7, 2025

View reviewed changes

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 7, 2025

k8s-ci-robot assigned SergeyKanzhelev Jun 7, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 7, 2025

k8s-ci-robot merged commit 43bb11b into kubernetes:master Jun 7, 2025
15 checks passed

k8s-ci-robot added this to the v1.34 milestone Jun 7, 2025

github-project-automation bot moved this from Waiting on Author to Done in SIG Node: code and documentation PRs Jun 7, 2025

danwinship mentioned this pull request Jun 7, 2025

Fix kubelet iptables startup, clarify semantics of utiliptables calls. #132170

Merged

	if len(iptClients) == 0 {
	if err != nil \|\| len(iptClients) == 0 {

kube-proxy: log iptables errors in platformCheckSupported #131534

kube-proxy: log iptables errors in platformCheckSupported #131534

Conversation

aroradaman commented Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

Uh oh!

k8s-ci-robot commented Apr 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danwinship left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danwinship commented May 8, 2025

Uh oh!

k8s-ci-robot commented May 8, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danwinship May 9, 2025 • 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!

Choose a reason for hiding this comment

Uh oh!

thockin commented Jun 7, 2025

Uh oh!

SergeyKanzhelev left a comment

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Jun 7, 2025

Uh oh!

k8s-ci-robot commented Jun 7, 2025

Uh oh!

k8s-triage-robot commented Jun 7, 2025

Uh oh!

Uh oh!

Uh oh!

aroradaman commented Apr 29, 2025 •

edited

Loading

danwinship May 9, 2025 •

edited

Loading