IPVS: kubelet, kube-proxy: unmark packets before masquerading … #92584

aojea · 2020-06-28T15:49:04Z

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespace from that line:

/kind api-change
/kind bug
/kind cleanup
/kind deprecation
/kind design
/kind documentation

/kind failing-test

/kind feature
/kind flake

What this PR does / why we need it:

The sig-network tests started to fail after the double nat fix.
The problem is that the IPVS proxier uses a POSTORUTING for the hairpin traffic that needs to happen before the
kubernetes specific postrouting rules. Prepending this ipvs only masquerade rule solves the issue.

Which issue(s) this PR fixes:

Fixes #92553

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.:

@aojea catching up on this. So we're reverting the double-nat fix for IPVS only since it used ipset and was not impacted?

aojea · 2020-06-28T15:50:04Z

/test pull-kubernetes-e2e-gci-gce-ipvs

aojea · 2020-06-28T16:49:06Z

/assign @andrewsykim @uablrek

This solves all the test failures reported in #92553

I0628 16:24:49.389] FAIL! -- 757 Passed | 1 Failed | 0 Pending | 4355 Skipped

and gives us back to the original situation reported in, with the flexvolume test failure #91253

I0628 16:24:49.388] [Fail] [sig-storage] Flexvolumes [It] should be mountable when non-attachable 
I0628 16:24:49.388] test/e2e/framework/volume/fixtures.go:502

@danwinship added this fix to solve the double natting problem, I tried to backtrace the problem but seems that IPVS uses ipset too and I wasn't able to find out the problem , so reverting the change only in IPVS will solve the issue

aojea · 2020-06-28T16:52:07Z

/priority urgent-critical
This breaks kube-proxy in IPVS mode

/test pull-kubernetes-e2e-kind-ipv6
/test pull-kubernetes-e2e-gci-gce-ipvs

k8s-ci-robot · 2020-06-28T16:52:08Z

@aojea: The label(s) priority/urgent-critical cannot be applied, because the repository doesn't have them

In response to this:

/priority urgent-critical
This breaks kube-proxy in IPVS mode

/test pull-kubernetes-e2e-kind-ipv6
/test pull-kubernetes-e2e-gci-gce-ipvs

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.

andrewsykim · 2020-06-28T18:41:35Z

@aojea catching up on this. So we're reverting the double-nat fix for IPVS only since it used ipset and was not impacted?

aojea · 2020-06-28T19:13:51Z

@aojea catching up on this. So we're reverting the double-nat fix for IPVS only since it used ipset and was not impacted?

I don't know if it was impacted, sorry, didn't go so far understanding the ipset - iptables relationship, however, with the fix in IPVS is broken per the test results.

I think that we should follow up and verify if the double-nat fix is needed for IPVS, but have it working until then.

aojea · 2020-06-28T21:16:36Z

/retest
it works 😄

pull-kubernetes-e2e-gci-gce-ipvs — Job succeeded.

aojea · 2020-06-28T22:30:32Z

/retest

Lion-Wei · 2020-06-29T02:20:44Z

test/e2e/storage/flexvolume.go

@@ -182,7 +182,7 @@ var _ = utils.SIGDescribe("Flexvolumes", func() {
 		suffix = ns.Name
 	})

-	ginkgo.It("should be mountable when non-attachable", func() {
+	ginkgo.It("should be mountable when non-attachable [Feature:Flexvolumes]", func() {


I know this is for storage feature, but still feel inappropriate to directly add this label.

This test is failing constantly, is breaking this job.

It seems a leftover from #91329, the linked issue says that these tests should have the feature tag to avoid breaking the e2e suite
#78737 (comment)

I've added it as a separate commit so the git history is clean and can be reverted independently.

cc @msau42 is this okay to do here?

The switch the distroless should not have impacted the "non-attachable" test case. This test case is passing fine in the normal k8s job: https://k8s-testgrid.appspot.com/sig-storage-kubernetes#gce&include-filter-by-regex=Flexvolume

So it would be interesting to figure out why it is consistently failing in this job.

sorry about that, my mistake

aojea · 2020-06-29T08:10:06Z

/retest

Lion-Wei · 2020-06-29T08:56:12Z

@aojea I think part of the problem is because RETURN rule in KUBE-POSTROUTING break the loop back logic.

Chain KUBE-POSTROUTING (1 references)
target     prot opt source               destination
RETURN     all  --  0.0.0.0/0            0.0.0.0/0            mark match ! 0x4000/0x4000
MARK       all  --  0.0.0.0/0            0.0.0.0/0            MARK xor 0x4000
MASQUERADE  all  --  0.0.0.0/0            0.0.0.0/0            /* kubernetes service traffic requiring SNAT */
MASQUERADE  all  --  0.0.0.0/0            0.0.0.0/0            /* Kubernetes endpoints dst ip:port, source ip for solving hairpin purpose */ match-set KUBE-LOOP-BACK dst,dst,src

However, I think straightly revert the double-nat fix is not a good idea, maybe move KUBE-LOOP-BACK up could be an option.

aojea · 2020-06-29T09:44:29Z

However, I think straightly revert the double-nat fix is not a good idea, maybe move KUBE-LOOP-BACK up could be an option.

agree that the best solution is to solve it, but right now IPVS mode is broken and I don't know if the double-nat issue with the random-fully flag is affecting IPVS, but if it does affect, it's only in some specific scenarios, we can see that all the test succeed now.

Honestly, I don't have more time to investigate 😅 , seems this logic was added here #54219 , my suggestion is to merge this, fix the job and IPVS so we can have a good test signal, and open a new Issue to solve/understand the double-nat issue in IPVS.

Lion-Wei · 2020-06-29T11:01:28Z

However, I think straightly revert the double-nat fix is not a good idea, maybe move KUBE-LOOP-BACK up could be an option.

agree that the best solution is to solve it, but right now IPVS mode is broken and I don't know if the double-nat issue with the random-fully flag is affecting IPVS, but if it does affect, it's only in some specific scenarios, we can see that all the test succeed now.

Honestly, I don't have more time to investigate 😅 , seems this logic was added here #54219 , my suggestion is to merge this, fix the job and IPVS so we can have a good test signal, and open a new Issue to solve/understand the double-nat issue in IPVS.

Agree.

danwinship · 2020-06-29T13:45:31Z

There were definitely people using IPVS in various places in the sprawling tangle of bug reports related to the random-fully/checksum problem.

I think part of the problem is because RETURN rule in KUBE-POSTROUTING break the loop back logic.

maybe move KUBE-LOOP-BACK up could be an option.

er, yeah, my fix assumed that kubelet and kube-proxy were creating identical KUBE-POSTROUTING rules.

This will hopefully eventually get fixed as part of #82125 but yes, it seems like for now just moving the KUBE-LOOP-BACK rule should fix this

k8s-ci-robot · 2020-07-01T07:18:06Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andrewsykim, aojea

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~~ [andrewsykim]

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

aojea · 2020-07-01T07:18:16Z

/hold cancel
flexvolume commit removed

aojea · 2020-07-01T07:19:12Z

@Lion-Wei do you mind lgtm again?

aojea · 2020-07-01T08:35:13Z

/retest
the ipvs job failure is because the flexvolume test only

Lion-Wei · 2020-07-01T08:40:14Z

@aojea Not at all.
/lgtm

aojea · 2020-07-01T10:20:09Z

/retest
🤔

andrewsykim · 2020-07-01T13:21:05Z

I thought the flex volume tests were failing because quay.io was down but that seemed to be fixed now #91242 ?

andrewsykim · 2020-07-01T13:22:00Z

Maybe we just need to update to use the staging CI image for quay? @BenTheElder might know?

andrewsykim

/hold cancel

Removing the hold though since only the flex volume tests are failing and this is a known issue

fejta-bot · 2020-07-01T15:35:28Z

/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 or /hold comment for consistent failures.

msau42 · 2020-07-01T16:28:21Z

I don't think root cause of flex volume test failing in this job is known. It's passing in normal k8s ci, so I suspect it is related to the configuration of this job.

This line from the test log is potentially suspicious and indicates that the test is trying to use IPs.

Jul  1 15:50:26.384: INFO: Getting external IP address for e2e-bc0cae53a3-2e794-minion-group-mxnp

fejta-bot · 2020-07-01T18:44:28Z

/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 or /hold comment for consistent failures.

fejta-bot · 2020-07-01T22:13:41Z

/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 or /hold comment for consistent failures.

fejta-bot · 2020-07-02T02:04:32Z

/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 or /hold comment for consistent failures.

k8s-ci-robot · 2020-07-02T02:54:30Z

@aojea: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-kubernetes-e2e-gci-gce-ipvs	`c40081b`	link	`/test pull-kubernetes-e2e-gci-gce-ipvs`

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.

fejta-bot · 2020-07-02T05:55:33Z

/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 or /hold comment for consistent failures.

…upstream-release-1.18 Automated cherry pick of #92584: kube-proxy ipvs masquerade hairpin traffic

…upstream-release-1.17 Automated cherry pick of #92584: kube-proxy ipvs masquerade hairpin traffic

…upstream-release-1.16 Automated cherry pick of #92584: kube-proxy ipvs masquerade hairpin traffic

k8s-ci-robot removed the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Jun 28, 2020

k8s-ci-robot requested review from cmluciano and nicksardo June 28, 2020 15:50

k8s-ci-robot assigned andrewsykim and uablrek Jun 28, 2020

k8s-ci-robot added area/test sig/storage Categorizes an issue or PR as relevant to SIG Storage. sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Jun 28, 2020

Lion-Wei reviewed Jun 29, 2020

View reviewed changes

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 1, 2020

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

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 1, 2020

andrewsykim reviewed Jul 1, 2020

View reviewed changes

This was referenced Jul 1, 2020

FlexVolume tests are failing #91253

Closed

[failing test] "Flexvolumes should be mountable when non-attachable" fails when using ubuntu node image #92708

Closed

k8s-ci-robot merged commit 4d0ce2e into kubernetes:master Jul 2, 2020

k8s-ci-robot added this to the v1.19 milestone Jul 2, 2020

k8s-ci-robot added a commit that referenced this pull request Jul 9, 2020

Merge pull request #92746 from aojea/automated-cherry-pick-of-#92584-…

299f14e

…upstream-release-1.18 Automated cherry pick of #92584: kube-proxy ipvs masquerade hairpin traffic

k8s-ci-robot added a commit that referenced this pull request Jul 9, 2020

Merge pull request #92747 from aojea/automated-cherry-pick-of-#92584-…

740f9b0

…upstream-release-1.17 Automated cherry pick of #92584: kube-proxy ipvs masquerade hairpin traffic

k8s-ci-robot added a commit that referenced this pull request Jul 10, 2020

Merge pull request #92748 from aojea/automated-cherry-pick-of-#92584-…

61b06f9

…upstream-release-1.16 Automated cherry pick of #92584: kube-proxy ipvs masquerade hairpin traffic

couloum mentioned this pull request Nov 26, 2020

Additional 1s latency in host -> service IP -> pod with iptables #96868

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IPVS: kubelet, kube-proxy: unmark packets before masquerading … #92584

IPVS: kubelet, kube-proxy: unmark packets before masquerading … #92584

aojea commented Jun 28, 2020 •

edited

Loading

aojea commented Jun 28, 2020

aojea commented Jun 28, 2020

aojea commented Jun 28, 2020

k8s-ci-robot commented Jun 28, 2020

andrewsykim commented Jun 28, 2020

aojea commented Jun 28, 2020

aojea commented Jun 28, 2020

aojea commented Jun 28, 2020

Lion-Wei Jun 29, 2020

aojea Jun 29, 2020

andrewsykim Jun 30, 2020

msau42 Jul 1, 2020

aojea Jul 1, 2020

aojea commented Jun 29, 2020

Lion-Wei commented Jun 29, 2020

aojea commented Jun 29, 2020

Lion-Wei commented Jun 29, 2020

danwinship commented Jun 29, 2020

k8s-ci-robot commented Jul 1, 2020

aojea commented Jul 1, 2020

aojea commented Jul 1, 2020

aojea commented Jul 1, 2020

Lion-Wei commented Jul 1, 2020

aojea commented Jul 1, 2020

andrewsykim commented Jul 1, 2020

andrewsykim commented Jul 1, 2020

andrewsykim left a comment

fejta-bot commented Jul 1, 2020

msau42 commented Jul 1, 2020

fejta-bot commented Jul 1, 2020

fejta-bot commented Jul 1, 2020

fejta-bot commented Jul 2, 2020

k8s-ci-robot commented Jul 2, 2020

fejta-bot commented Jul 2, 2020

IPVS: kubelet, kube-proxy: unmark packets before masquerading … #92584

IPVS: kubelet, kube-proxy: unmark packets before masquerading … #92584

Conversation

aojea commented Jun 28, 2020 • edited Loading

aojea commented Jun 28, 2020

aojea commented Jun 28, 2020

aojea commented Jun 28, 2020

k8s-ci-robot commented Jun 28, 2020

andrewsykim commented Jun 28, 2020

aojea commented Jun 28, 2020

aojea commented Jun 28, 2020

aojea commented Jun 28, 2020

Lion-Wei Jun 29, 2020

Choose a reason for hiding this comment

aojea Jun 29, 2020

Choose a reason for hiding this comment

andrewsykim Jun 30, 2020

Choose a reason for hiding this comment

msau42 Jul 1, 2020

Choose a reason for hiding this comment

aojea Jul 1, 2020

Choose a reason for hiding this comment

aojea commented Jun 29, 2020

Lion-Wei commented Jun 29, 2020

aojea commented Jun 29, 2020

Lion-Wei commented Jun 29, 2020

danwinship commented Jun 29, 2020

k8s-ci-robot commented Jul 1, 2020

aojea commented Jul 1, 2020

aojea commented Jul 1, 2020

aojea commented Jul 1, 2020

Lion-Wei commented Jul 1, 2020

aojea commented Jul 1, 2020

andrewsykim commented Jul 1, 2020

andrewsykim commented Jul 1, 2020

andrewsykim left a comment

Choose a reason for hiding this comment

fejta-bot commented Jul 1, 2020

msau42 commented Jul 1, 2020

fejta-bot commented Jul 1, 2020

fejta-bot commented Jul 1, 2020

fejta-bot commented Jul 2, 2020

k8s-ci-robot commented Jul 2, 2020

fejta-bot commented Jul 2, 2020

aojea commented Jun 28, 2020 •

edited

Loading