Service Internal Traffic Policy by maplain · Pull Request #96600 · kubernetes/kubernetes

maplain · 2020-11-16T08:05:01Z

What type of PR is this?
/kind feature

What this PR does / why we need it:
This PR implements the Service Internal Traffic Policy KEP

Which issue(s) this PR fixes:
Fixes kubernetes/enhancements#2086

Special notes for your reviewer:

Does this PR introduce a user-facing change?:
Yes

One new field "InternalTrafficPolicy" in Service is added.
It specifies if the cluster internal traffic should be routed to all endpoints or node-local endpoints only.
"Cluster" routes internal traffic to a Service to all endpoints.
"Local" routes traffic to node-local endpoints only, and traffic is dropped if no node-local endpoints are ready.
The default value is "Cluster".

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

TBD

k8s-ci-robot · 2020-11-16T08:05:10Z

Hi @maplain. 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/test-infra repository.

andrewsykim · 2020-11-16T14:32:50Z

/triage accepted
/milestone v1.21
/assign

Thanks for starting this @maplain!

andrewsykim · 2020-11-30T19:08:07Z

I know this PR is a WIP, but some comments for what is still required:

We need a ServiceInternalTrafficPolicy feature gate in https://github.com/kubernetes/kubernetes/blob/master/pkg/features/kube_features.go that is alpha status and off by default.
We need to drop the new field in the Service REST code when the feature gate is off. See spec.allocateLoadBalancerNodePorts as one example: https://github.com/kubernetes/kubernetes/blob/master/pkg/registry/core/service/strategy.go#L185-L190
We need to set the default value to Cluster if the feature gate is enabeld, see SetDefaults_Service: https://github.com/kubernetes/kubernetes/blob/master/pkg/apis/core/v1/defaults.go#L97
kube-proxy changes

thockin

I have nothing left to pick on.

/approve
/lgtm

k8s-ci-robot · 2021-03-05T23:27:23Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: maplain, thockin

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~api/OWNERS~~ [thockin]
~~pkg/api/OWNERS~~ [thockin]
~~pkg/apis/core/OWNERS~~ [thockin]
~~pkg/features/OWNERS~~ [thockin]
~~pkg/proxy/OWNERS~~ [thockin]
~~pkg/registry/OWNERS~~ [thockin]
~~staging/src/k8s.io/api/OWNERS~~ [thockin]
~~staging/src/k8s.io/client-go/OWNERS~~ [thockin]

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

maplain · 2021-03-06T00:31:38Z

/test pull-kubernetes-unit
/test pull-kubernetes-verify

maplain · 2021-03-06T00:46:41Z

Rebase and re-generate static files:

    1. `git clean -fxd`
    2. `make update`

maplain · 2021-03-06T17:55:55Z

weird. it keeps saying /home/prow/go/src/k8s.io/kubernetes/api/openapi-spec is out of date. Please run hack/update-openapi-spec.sh, but I've tried a few times, and nothing changes..

Rebase again

maplain · 2021-03-06T20:02:15Z

/test pull-kubernetes-verify

thockin · 2021-03-08T00:11:17Z

Needs rebase

1. Add API definitions; 2. Add feature gate and drops the field when feature gate is not on; 3. Set default values for the field; 4. Add API Validation 5. add kube-proxy iptables and ipvs implementations 6. add tests

Fields: 1. rename onlyNodeLocalEndpoints to nodeLocalExternal; 2. rename onlyNodeLocalEndpointsForInternal to nodeLocalInternal; Methods: 1. rename OnlyNodeLocalEndpoints to NodeLocalExternal; 2. rename OnlyNodeLocalEndpointsForInternal to NodeLocalInternal;

JornShen · 2021-03-08T02:27:34Z

for failed check pull-kubernetes-verify, need to run hack/update-openapi-spec.sh

maplain · 2021-03-08T02:49:14Z

@JornShen I've tried to run it a few times, but it doesn't update anything locally.. I actually run FORCE_ALL=true make update every time before I push to make sure it is in sync.

let me try it one more time

maplain · 2021-03-08T02:53:57Z

➜  kubernetes git:(internal-traffic-policy) hack/update-openapi-spec.sh
+++ [0307 18:48:44] Building go targets for darwin/amd64:
    ./vendor/k8s.io/code-generator/cmd/prerelease-lifecycle-gen
Generating prerelease lifecycle code for       26 targets
+++ [0307 18:48:51] Building go targets for darwin/amd64:
    ./vendor/k8s.io/code-generator/cmd/deepcopy-gen
Generating deepcopy code for      226 targets
+++ [0307 18:49:03] Building go targets for darwin/amd64:
    ./vendor/k8s.io/code-generator/cmd/defaulter-gen
Generating defaulter code for       88 targets
+++ [0307 18:49:14] Building go targets for darwin/amd64:
    ./vendor/k8s.io/code-generator/cmd/conversion-gen
Generating conversion code for      123 targets
+++ [0307 18:49:37] Building go targets for darwin/amd64:
    ./vendor/k8s.io/kube-openapi/cmd/openapi-gen
Generating openapi code for KUBE
Generating openapi code for AGGREGATOR
Generating openapi code for APIEXTENSIONS
^A^KGenerating openapi code for CODEGEN
Generating openapi code for SAMPLEAPISERVER
+++ [0307 18:49:52] Building go targets for darwin/amd64:
    ./vendor/github.com/go-bindata/go-bindata/go-bindata
+++ [0307 18:49:54] Building go targets for darwin/amd64:
    cmd/kube-apiserver
etcd --advertise-client-urls http://127.0.0.1:2379 --data-dir /var/folders/l5/66tl8_29065g0jrk7t92qdjm0000gn/T/tmp.vlrfbPAq --listen-client-urls http://127.0.0.1:2379 --log-level=debug > "/dev/null" 2>/dev/null
Waiting for etcd to come up.
+++ [0307 18:51:25] On try 3, etcd: : {"health":"true"}
{"header":{"cluster_id":"14841639068965178418","member_id":"10276657743932975437","revision":"2","raft_term":"2"}}+++ [0307 18:51:25] Starting kube-apiserver
+++ [0307 18:51:34] On try 9, apiserver: : ok
+++ [0307 18:51:34] Updating
    hack/../api/openapi-spec
+++ [0307 18:51:35] SUCCESS
+++ [0307 18:52:35] Clean up complete


➜  kubernetes git:(internal-traffic-policy) gst
On branch internal-traffic-policy
Your branch is up to date with 'my/internal-traffic-policy'.

nothing to commit, working tree clean

@JornShen is it possible that this is related to the go version?

go version go1.16 darwin/amd64

maplain · 2021-03-08T02:54:35Z

/test pull-kubernetes-e2e-gce-alpha-features

maplain · 2021-03-08T02:55:01Z

/test pull-kubernetes-verify

JornShen · 2021-03-08T09:00:38Z

@maplain using linux robot will be ok.

JornShen@c1a7b78

I have upload it to my repo. You can cherry-pick it, maybe.

1. `git clean -fxd` 2. `make update`

andrewsykim · 2021-03-08T16:43:23Z

Go version seems right, I think we're on 1.16

thockin · 2021-03-08T17:44:52Z

/lgtm

maplain · 2021-03-08T17:57:38Z

Thanks @thockin for approval, and @andrewsykim and @JornShen for your help!

I manually copied the file attached by Shen and it passed the test.

Though after copying the file and running hack/update-openapi-spec.sh locally again, I do see changes this time, which is clearly wrong:

diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json
index 39807afb663..ec45c84199a 100644
--- a/api/openapi-spec/swagger.json
+++ b/api/openapi-spec/swagger.json
@@ -11795,6 +11795,552 @@
       },
       "type": "object"
     },
+    "io.k8s.api.flowcontrol.v1alpha1.FlowDistinguisherMethod": {
+      "description": "FlowDistinguisherMethod specifies the method of a flow distinguisher.",
+      "properties": {
+        "type": {
+          "description": "`type` is the type of flow distinguisher method The supported types are \"ByUser\" and \"ByNamespace\". Required.",
+          "type": "string"
+        }
+      },
+      "required": [
+        "type"
+      ],
+      "type": "object"
+    },
+    "io.k8s.api.flowcontrol.v1alpha1.FlowSchema": {
+      "description": "FlowSchema defines the schema of a group of flows. Note that a flow is made up of a set of inbound API requests with similar attributes and is identified by a pair of strings: the name of the FlowSchema and a \"flow distinguisher\".",

k8s-ci-robot added the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Nov 16, 2020

k8s-ci-robot requested review from brendandburns and deads2k November 16, 2020 08:05

k8s-ci-robot added sig/apps Categorizes an issue or PR as relevant to SIG Apps. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Nov 16, 2020

k8s-ci-robot assigned andrewsykim Nov 16, 2020

k8s-ci-robot added the triage/accepted Indicates an issue or PR is ready to be actively worked on. label Nov 16, 2020

k8s-ci-robot added this to the v1.21 milestone Nov 16, 2020

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

andrewsykim reviewed Nov 30, 2020

View reviewed changes

Comment thread pkg/apis/core/types.go Outdated

andrewsykim reviewed Nov 30, 2020

View reviewed changes

Comment thread pkg/apis/core/types.go Outdated

maplain force-pushed the internal-traffic-policy branch from 3e1ec29 to ad6088e Compare December 2, 2020 18:27

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Dec 2, 2020

andrewsykim reviewed Dec 2, 2020

View reviewed changes

Comment thread staging/src/k8s.io/api/core/v1/types.go Outdated

Comment thread pkg/apis/core/types.go Outdated

Comment thread pkg/registry/core/service/strategy.go Outdated

maplain force-pushed the internal-traffic-policy branch from ad6088e to c085297 Compare December 4, 2020 03:34

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 4, 2020

maplain force-pushed the internal-traffic-policy branch from c085297 to 36febbd Compare December 5, 2020 08:38

thockin reviewed Mar 5, 2021

View reviewed changes

Comment thread pkg/apis/core/types.go Outdated

thockin reviewed Mar 5, 2021

View reviewed changes

Comment thread staging/src/k8s.io/api/core/v1/types.go Outdated

maplain force-pushed the internal-traffic-policy branch from 2bc331d to ca6c9ae Compare March 5, 2021 23:23

thockin reviewed Mar 5, 2021

View reviewed changes

k8s-ci-robot assigned thockin Mar 5, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 5, 2021

Fangyuan Li added 2 commits March 7, 2021 16:52

Implements Service Internal Traffic Policy

7ed2f1d

1. Add API definitions; 2. Add feature gate and drops the field when feature gate is not on; 3. Set default values for the field; 4. Add API Validation 5. add kube-proxy iptables and ipvs implementations 6. add tests

Update generated files

62804c1

1. `git clean -fxd` 2. `make update`

maplain mentioned this pull request Mar 9, 2021

REQUEST: New membership for maplain kubernetes/org#2552

Closed

6 tasks

bootc mentioned this pull request Mar 21, 2021

Support spec.externalIPs and NodePort services kakao/network-node-manager#11

Closed

reylejano mentioned this pull request Mar 22, 2021

Add documentation for Service InternalTrafficPolicy kubernetes/website#27088

Merged

andrewsykim mentioned this pull request Apr 2, 2021

Implement Service Internal Traffic Policy antrea-io/antrea#2023

Closed

rikatz mentioned this pull request Apr 6, 2021

Update libcalico-go to latest version projectcalico/felix#2754

Merged

andrewsykim mentioned this pull request Jul 5, 2021

Promote the ServiceInternalTrafficPolicy field to Beta and on by default #103462

Merged

Conversation

maplain commented Nov 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Nov 16, 2020

Uh oh!

andrewsykim commented Nov 16, 2020

Uh oh!

Uh oh!

Uh oh!

andrewsykim commented Nov 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thockin left a comment

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Mar 5, 2021

Uh oh!

maplain commented Mar 6, 2021

Uh oh!

maplain commented Mar 6, 2021

Uh oh!

maplain commented Mar 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maplain commented Mar 6, 2021

Uh oh!

thockin commented Mar 8, 2021

Uh oh!

JornShen commented Mar 8, 2021

Uh oh!

maplain commented Mar 8, 2021

Uh oh!

maplain commented Mar 8, 2021

Uh oh!

maplain commented Mar 8, 2021

Uh oh!

maplain commented Mar 8, 2021

Uh oh!

JornShen commented Mar 8, 2021

Uh oh!

andrewsykim commented Mar 8, 2021

Uh oh!

thockin commented Mar 8, 2021

Uh oh!

maplain commented Mar 8, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

maplain commented Nov 16, 2020 •

edited

Loading

andrewsykim commented Nov 30, 2020 •

edited

Loading

maplain commented Mar 6, 2021 •

edited

Loading