Fixes NodePort in ipv6 with proxy-mode=ipvs #71035

uablrek · 2018-11-14T12:33:31Z

Use ipv6 addresses for NodePort with proxy-mode=ipvs in a ipv6-only cluster.

What type of PR is this?

/kind bug

What this PR does / why we need it:

This affects ipv6-only cluster with proxy-mode=ipvs only.

In an ipv6-only cluster the address scanning function used for setting up NodePort rules in ipvs ipvs return ipv4 addresses.

This PR ensures that ipv6 addresses are returned by the scanning function in an ipv6-only cluster. This makes NodePort work in an ipv6-only cluster with proxy-mode=ipvs.

Which issue(s) this PR fixes

Fixes #68437

Special notes for your reviewer:

I added a ipv6 flag in the netlinkHandle struct rather than return both ipv4 and ipv6 addresses. The main reason for this is that IHO it reduces the risk for breaking the existing ipv4 function.

I am unsure how this will affect future dual-stack work, but the again I guess nobody knows and maybe it even simplifies.

Only manual testing that NodePort works in ipv6-only/ipvs is performed.

Does this PR introduce a user-facing change?:

NONE

Use ipv6 addresses for NodePort with proxy-mode=ipvs in a ipv6-only cluster.

k8s-ci-robot · 2018-11-14T12:33:32Z

@uablrek: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

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.

k8s-ci-robot · 2018-11-14T12:33:38Z

Hi @uablrek. 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.

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.

uablrek · 2018-11-14T12:37:55Z

/sig network

Lion-Wei · 2018-11-19T08:22:08Z

/ok-to-test

uablrek · 2018-11-19T09:09:46Z

/retest

Lion-Wei · 2018-11-19T09:21:28Z

pkg/proxy/ipvs/proxier.go

@@ -382,14 +382,14 @@ func NewProxier(ipt utiliptables.Interface,
 		healthzServer:         healthzServer,
 		ipvs:                  ipvs,
 		ipvsScheduler:         scheduler,
-		ipGetter:              &realIPGetter{nl: NewNetLinkHandle()},
+		ipGetter:              &realIPGetter{nl: NewNetLinkHandle(nodeIP.To4() == nil)},


You can use the var isIPv6 instead of nodeIP.To4() == nil

Lion-Wei · 2018-11-19T09:22:15Z

pkg/proxy/ipvs/proxier.go

 		iptablesData:          bytes.NewBuffer(nil),
 		filterChainsData:      bytes.NewBuffer(nil),
 		natChains:             bytes.NewBuffer(nil),
 		natRules:              bytes.NewBuffer(nil),
 		filterChains:          bytes.NewBuffer(nil),
 		filterRules:           bytes.NewBuffer(nil),
-		netlinkHandle:         NewNetLinkHandle(),
+		netlinkHandle:         NewNetLinkHandle(nodeIP.To4() == nil),


Same as above

uablrek · 2018-11-19T09:32:16Z

@Lion-Wei God point. I will update

uablrek · 2018-11-19T09:34:07Z

I will also rename the parameter to NewNetLinkHandle ipv6 -> isIPv6 to have consitent naming

uablrek · 2018-11-19T10:45:36Z

/test pull-kubernetes-verify

Lion-Wei · 2018-11-21T03:43:05Z

/release-note-none
/lgtm
/assign @m1093782566

Lion-Wei · 2018-11-21T03:44:19Z

xref: #71202

m1093782566 · 2018-11-21T04:11:47Z

/approve

k8s-ci-robot · 2018-11-21T04:11:54Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: m1093782566, uablrek

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:

~~pkg/proxy/ipvs/OWNERS~~ [m1093782566]

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

m1093782566 · 2018-11-29T01:54:39Z

I think this bug fix should be in v1.13.

/milestone v1.13

m1093782566 · 2018-11-29T04:14:35Z

Adding label
/priority critical-urgent

as it may break an IPV6 cluster.

AishSundar · 2018-11-29T14:39:02Z

/hold

AishSundar · 2018-11-29T14:40:52Z

@uablrek @m1093782566 was this PR meant for 1.13? we no longer accept merges into 1.13 branch and only extremely critical urgent fixes can to be CP'ed if needed.

uablrek · 2018-11-29T14:49:50Z

I think at the time of creation the version was still "1.13...something" so it should work if the patch can be applied. Note that this is a problem only for ipv6-only in combination with proxy-mode=ipvs. The most common is still proxy-mode=iptables which seem to work. So maybe this can slide to 1.14 but if it is possible to document that ipv6+proxy-mode=ipvs is not supported <1.14 it may save some time for new ipv6 users.

nikopen · 2018-11-29T17:50:37Z

It's a pretty rare cluster conf combination for people to use directly 1.13.0 in production..

@uablrek This can be cherry picked back to 1.13.1 after it merges on master right?

It can be in the release notes that the patch will go in on 1.13.1 @marpaia

/milestone v1.14

uablrek · 2018-11-29T17:55:57Z

@nikopen Not much have happened in the code as far as I know, and the base was 1.13... so I am pretty sure it can be cherry-picked.

AishSundar · 2018-11-29T18:47:04Z

Thanks all

/hold cancel

m1093782566 · 2018-12-10T06:57:32Z

/release-note-none

Putting this PR into merge pool...

m1093782566 · 2018-12-10T06:58:55Z

/milestone clear

fejta-bot · 2018-12-10T11:22:03Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

fejta-bot · 2018-12-10T15:13:06Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

Fixes NodePort in ipv6 with proxy-mode=ipvs. kubernetes#68437

e2409a0

Use ipv6 addresses for NodePort with proxy-mode=ipvs in a ipv6-only cluster.

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. sig/network Categorizes an issue or PR as relevant to SIG Network. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Nov 14, 2018

k8s-ci-robot requested review from Lion-Wei and thockin November 14, 2018 12:34

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Nov 19, 2018

Add the extra parameter for non-linux code

1be71b8

Lion-Wei reviewed Nov 19, 2018

View reviewed changes

Updates after review

2e5a985

k8s-ci-robot assigned m1093782566 and Lion-Wei Nov 21, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 21, 2018

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 21, 2018

k8s-ci-robot added this to the v1.13 milestone Nov 29, 2018

k8s-ci-robot added priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Nov 29, 2018

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 29, 2018

k8s-ci-robot modified the milestones: v1.13, v1.14 Nov 29, 2018

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

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Dec 10, 2018

k8s-ci-robot removed this from the v1.14 milestone Dec 10, 2018

k8s-ci-robot merged commit 0914272 into kubernetes:master Dec 10, 2018

This was referenced Mar 11, 2020

REQUEST: New membership for @uablrek kubernetes/org#1701

Closed

REQUEST: New membership for @uablrek kubernetes/org#1702

Closed

uablrek deleted the issue-68437 branch April 29, 2023 15:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes NodePort in ipv6 with proxy-mode=ipvs #71035

Fixes NodePort in ipv6 with proxy-mode=ipvs #71035

uablrek commented Nov 14, 2018 •

edited

k8s-ci-robot commented Nov 14, 2018

k8s-ci-robot commented Nov 14, 2018

uablrek commented Nov 14, 2018

Lion-Wei commented Nov 19, 2018

uablrek commented Nov 19, 2018

Lion-Wei Nov 19, 2018

Lion-Wei Nov 19, 2018

uablrek commented Nov 19, 2018

uablrek commented Nov 19, 2018

uablrek commented Nov 19, 2018

Lion-Wei commented Nov 21, 2018 •

edited

Lion-Wei commented Nov 21, 2018

m1093782566 commented Nov 21, 2018

k8s-ci-robot commented Nov 21, 2018

m1093782566 commented Nov 29, 2018

m1093782566 commented Nov 29, 2018

AishSundar commented Nov 29, 2018

AishSundar commented Nov 29, 2018

uablrek commented Nov 29, 2018

nikopen commented Nov 29, 2018

uablrek commented Nov 29, 2018

AishSundar commented Nov 29, 2018

m1093782566 commented Dec 10, 2018 •

edited

m1093782566 commented Dec 10, 2018

fejta-bot commented Dec 10, 2018

fejta-bot commented Dec 10, 2018

Fixes NodePort in ipv6 with proxy-mode=ipvs #71035

Fixes NodePort in ipv6 with proxy-mode=ipvs #71035

Conversation

uablrek commented Nov 14, 2018 • edited

k8s-ci-robot commented Nov 14, 2018

k8s-ci-robot commented Nov 14, 2018

uablrek commented Nov 14, 2018

Lion-Wei commented Nov 19, 2018

uablrek commented Nov 19, 2018

Lion-Wei Nov 19, 2018

Choose a reason for hiding this comment

Lion-Wei Nov 19, 2018

Choose a reason for hiding this comment

uablrek commented Nov 19, 2018

uablrek commented Nov 19, 2018

uablrek commented Nov 19, 2018

Lion-Wei commented Nov 21, 2018 • edited

Lion-Wei commented Nov 21, 2018

m1093782566 commented Nov 21, 2018

k8s-ci-robot commented Nov 21, 2018

m1093782566 commented Nov 29, 2018

m1093782566 commented Nov 29, 2018

AishSundar commented Nov 29, 2018

AishSundar commented Nov 29, 2018

uablrek commented Nov 29, 2018

nikopen commented Nov 29, 2018

uablrek commented Nov 29, 2018

AishSundar commented Nov 29, 2018

m1093782566 commented Dec 10, 2018 • edited

m1093782566 commented Dec 10, 2018

fejta-bot commented Dec 10, 2018

fejta-bot commented Dec 10, 2018

uablrek commented Nov 14, 2018 •

edited

Lion-Wei commented Nov 21, 2018 •

edited

m1093782566 commented Dec 10, 2018 •

edited