Make kube-proxy check if IPv6 is really supported before assuming dual-stack #99127
Conversation
k8s-ci-robot
added
release-note
k8s-ci-robot
added
the
approved
danwinship
force-pushed
the
non-dual-stack-proxy
branch
from
2b710ad
to
1c9c233
Compare
k8s-ci-robot
added
area/kubelet
sig/node
|
/test pull-kubernetes-gci-gce-ipvs |
|
@aojea: The specified target(s) for
Use
In response to this:
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/test-infra repository. |
|
/test pull-kubernetes-e2e-gci-gce-ipvs |
This is much better 👍 ... the service IP family detection from the other PR can be done as a follow up, and we can get rid of the bindAddress hack |
| } | ||
| } else { | ||
| ipt[0] = utiliptables.New(execer, utiliptables.ProtocolIPv4) | ||
| ipt[1] = iptInterface |
There was a problem hiding this comment.
do you think we'll see the day that this fail because IPv4 is disabled in the host? 🙃
There was a problem hiding this comment.
I doubt any of this code will still be around then. The kernel does not currently support disabling IPv4.
danwinship
force-pushed
the
non-dual-stack-proxy
branch
from
1c9c233
to
29d7c66
Compare
|
/test pull-kubernetes-e2e-gci-gce-ipvs |
|
I presume there's no way to do unit tests for this? |
|
danwinship
force-pushed
the
non-dual-stack-proxy
branch
from
29d7c66
to
95c6a48
Compare
great error reporting there but that's a general utiliptables problem. repushed without the debug commit. Should be ready to go now |
It's possible... we'd have to refactor kube-proxy startup to make this subpart of it independently testable... but it would be kind of vacuous. "If we hack it so iptables returns exactly the specific error we're testing for, then kube-proxy will fall back to single-stack". It doesn't really prove that the code actually does the right thing in any real environment. |
|
/triage accepted |
k8s-ci-robot
added
triage/accepted
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/retest |
|
ping @dcbw for final approval |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: danwinship, dcbw 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:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
|
@danwinship: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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/test-infra repository. I understand the commands that are listed here. |
|
CI system issues... /retest |
What type of PR is this?
/kind bug
What this PR does / why we need it:
Now that the dual stack feature gate is enabled by default, kube-proxy defaults to trying to run dual stack (regardless of the cluster configuration), but some nodes have no IPv6 support, resulting in lots of spurious errors.
Which issue(s) this PR fixes:
Fixes #99031
Special notes for your reviewer:
@aojea I wrote this before noticing #99066... but this version is much smaller and more back-port-able. Maybe we want to merge pieces of both PRs together
Does this PR introduce a user-facing change?
-->
/priority important-soon
/sig network