From 862b1654273da2fc55e4b4e9a678ea05a776f7c9 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Wed, 31 May 2017 10:38:39 -0700 Subject: [PATCH 1/3] Proxy: comments on --masquerade-all flag --- cmd/kube-proxy/app/server.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/kube-proxy/app/server.go b/cmd/kube-proxy/app/server.go index 7c50f18fb7..00042a5640 100644 --- a/cmd/kube-proxy/app/server.go +++ b/cmd/kube-proxy/app/server.go @@ -140,7 +140,7 @@ func AddFlags(options *Options, fs *pflag.FlagSet) { fs.DurationVar(&options.config.IPTables.SyncPeriod.Duration, "iptables-sync-period", options.config.IPTables.SyncPeriod.Duration, "The maximum interval of how often iptables rules are refreshed (e.g. '5s', '1m', '2h22m'). Must be greater than 0.") fs.DurationVar(&options.config.IPTables.MinSyncPeriod.Duration, "iptables-min-sync-period", options.config.IPTables.MinSyncPeriod.Duration, "The minimum interval of how often the iptables rules can be refreshed as endpoints and services change (e.g. '5s', '1m', '2h22m').") fs.DurationVar(&options.config.ConfigSyncPeriod.Duration, "config-sync-period", options.config.ConfigSyncPeriod.Duration, "How often configuration from the apiserver is refreshed. Must be greater than 0.") - fs.BoolVar(&options.config.IPTables.MasqueradeAll, "masquerade-all", options.config.IPTables.MasqueradeAll, "If using the pure iptables proxy, SNAT everything") + fs.BoolVar(&options.config.IPTables.MasqueradeAll, "masquerade-all", options.config.IPTables.MasqueradeAll, "If using the pure iptables proxy, SNAT everything (this not commonly needed)") fs.StringVar(&options.config.ClusterCIDR, "cluster-cidr", options.config.ClusterCIDR, "The CIDR range of pods in the cluster. It is used to bridge traffic coming from outside of the cluster. If not provided, no off-cluster bridging will be performed.") fs.StringVar(&options.config.ClientConnection.ContentType, "kube-api-content-type", options.config.ClientConnection.ContentType, "Content type of requests sent to apiserver.") fs.Float32Var(&options.config.ClientConnection.QPS, "kube-api-qps", options.config.ClientConnection.QPS, "QPS to use while talking with kubernetes apiserver") From ce8309780f9e11a1ee2f175f272092c410ba537d Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Wed, 31 May 2017 10:39:12 -0700 Subject: [PATCH 2/3] Proxy: comments around ClusterCIDR use --- pkg/proxy/iptables/proxier.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/pkg/proxy/iptables/proxier.go b/pkg/proxy/iptables/proxier.go index 6f2f71e5fe..dd4a0e180d 100644 --- a/pkg/proxy/iptables/proxier.go +++ b/pkg/proxy/iptables/proxier.go @@ -1175,8 +1175,12 @@ func (proxier *Proxier) syncProxyRules() { ) if proxier.masqueradeAll { writeLine(proxier.natRules, append(args, "-j", string(KubeMarkMasqChain))...) - } - if len(proxier.clusterCIDR) > 0 { + } else if len(proxier.clusterCIDR) > 0 { + // This masquerades off-cluster traffic to a service VIP. The idea + // is that you can establish a static route for your Service range, + // routing to any node, and that node will bridge into the Service + // for you. Since that might bounce off-node, we masquerade here. + // If/when we support "Local" policy for VIPs, we should update this. writeLine(proxier.natRules, append(args, "! -s", proxier.clusterCIDR, "-j", string(KubeMarkMasqChain))...) } writeLine(proxier.natRules, append(args, "-j", string(svcChain))...) @@ -1480,7 +1484,7 @@ func (proxier *Proxier) syncProxyRules() { localEndpointChains = append(localEndpointChains, endpointChains[i]) } } - // First rule in the chain redirects all pod -> external vip traffic to the + // First rule in the chain redirects all pod -> external VIP traffic to the // Service's ClusterIP instead. This happens whether or not we have local // endpoints; only if clusterCIDR is specified if len(proxier.clusterCIDR) > 0 { From fc34a9d6ba4679298abc2d371b86eb32354f0c7b Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Wed, 31 May 2017 11:35:24 -0700 Subject: [PATCH 3/3] 'Global' -> 'Cluster' for traffic policy --- api/openapi-spec/swagger.json | 2 +- api/swagger-spec/v1.json | 2 +- docs/api-reference/v1/definitions.html | 4 ++-- federation/apis/openapi-spec/swagger.json | 2 +- federation/apis/swagger-spec/v1.json | 2 +- .../docs/api-reference/v1/definitions.html | 4 ++-- pkg/api/service/util_test.go | 18 +++++++++--------- pkg/api/testing/fuzzer.go | 2 +- pkg/api/types.go | 15 +++++++++------ pkg/api/v1/defaults.go | 2 +- pkg/api/v1/defaults_test.go | 8 ++++---- pkg/api/v1/generated.proto | 9 ++++++--- pkg/api/v1/service/util_test.go | 18 +++++++++--------- pkg/api/v1/types.go | 15 +++++++++------ pkg/api/v1/types_swagger_doc_generated.go | 2 +- pkg/api/validation/validation.go | 4 ++-- pkg/api/validation/validation_test.go | 2 +- pkg/registry/core/service/rest_test.go | 2 +- staging/src/k8s.io/client-go/pkg/api/types.go | 15 +++++++++------ .../k8s.io/client-go/pkg/api/v1/defaults.go | 2 +- .../client-go/pkg/api/v1/generated.proto | 9 ++++++--- .../src/k8s.io/client-go/pkg/api/v1/types.go | 15 +++++++++------ .../pkg/api/v1/types_swagger_doc_generated.go | 2 +- test/e2e/service.go | 2 +- 24 files changed, 88 insertions(+), 70 deletions(-) diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index 0dff8e7808..c99052d5bc 100644 --- a/api/openapi-spec/swagger.json +++ b/api/openapi-spec/swagger.json @@ -49749,7 +49749,7 @@ "type": "string" }, "externalTrafficPolicy": { - "description": "externalTrafficPolicy denotes if this Service desires to route external traffic to local endpoints only. This preserves Source IP and avoids a second hop for LoadBalancer and Nodeport type services.", + "description": "externalTrafficPolicy denotes if this Service desires to route external traffic to node-local or cluster-wide endpoints. \"Local\" preserves the client source IP and avoids a second hop for LoadBalancer and Nodeport type services, but risks potentially imbalanced traffic spreading. \"Cluster\" obscures the client source IP and may cause a second hop to another node, but should have good overall load-spreading.", "type": "string" }, "healthCheckNodePort": { diff --git a/api/swagger-spec/v1.json b/api/swagger-spec/v1.json index dbf74004c2..4c657b9871 100644 --- a/api/swagger-spec/v1.json +++ b/api/swagger-spec/v1.json @@ -21715,7 +21715,7 @@ }, "externalTrafficPolicy": { "type": "string", - "description": "externalTrafficPolicy denotes if this Service desires to route external traffic to local endpoints only. This preserves Source IP and avoids a second hop for LoadBalancer and Nodeport type services." + "description": "externalTrafficPolicy denotes if this Service desires to route external traffic to node-local or cluster-wide endpoints. \"Local\" preserves the client source IP and avoids a second hop for LoadBalancer and Nodeport type services, but risks potentially imbalanced traffic spreading. \"Cluster\" obscures the client source IP and may cause a second hop to another node, but should have good overall load-spreading." }, "healthCheckNodePort": { "type": "integer", diff --git a/docs/api-reference/v1/definitions.html b/docs/api-reference/v1/definitions.html index e81bd174e4..3cddae11c2 100755 --- a/docs/api-reference/v1/definitions.html +++ b/docs/api-reference/v1/definitions.html @@ -9861,7 +9861,7 @@ Examples:

externalTrafficPolicy

-

externalTrafficPolicy denotes if this Service desires to route external traffic to local endpoints only. This preserves Source IP and avoids a second hop for LoadBalancer and Nodeport type services.

+

externalTrafficPolicy denotes if this Service desires to route external traffic to node-local or cluster-wide endpoints. "Local" preserves the client source IP and avoids a second hop for LoadBalancer and Nodeport type services, but risks potentially imbalanced traffic spreading. "Cluster" obscures the client source IP and may cause a second hop to another node, but should have good overall load-spreading.

false

string

@@ -10105,7 +10105,7 @@ Examples:
diff --git a/federation/apis/openapi-spec/swagger.json b/federation/apis/openapi-spec/swagger.json index 5deda45be2..d36e6574b6 100644 --- a/federation/apis/openapi-spec/swagger.json +++ b/federation/apis/openapi-spec/swagger.json @@ -12589,7 +12589,7 @@ "type": "string" }, "externalTrafficPolicy": { - "description": "externalTrafficPolicy denotes if this Service desires to route external traffic to local endpoints only. This preserves Source IP and avoids a second hop for LoadBalancer and Nodeport type services.", + "description": "externalTrafficPolicy denotes if this Service desires to route external traffic to node-local or cluster-wide endpoints. \"Local\" preserves the client source IP and avoids a second hop for LoadBalancer and Nodeport type services, but risks potentially imbalanced traffic spreading. \"Cluster\" obscures the client source IP and may cause a second hop to another node, but should have good overall load-spreading.", "type": "string" }, "healthCheckNodePort": { diff --git a/federation/apis/swagger-spec/v1.json b/federation/apis/swagger-spec/v1.json index d441bfc94d..f1444562a7 100644 --- a/federation/apis/swagger-spec/v1.json +++ b/federation/apis/swagger-spec/v1.json @@ -5118,7 +5118,7 @@ }, "externalTrafficPolicy": { "type": "string", - "description": "externalTrafficPolicy denotes if this Service desires to route external traffic to local endpoints only. This preserves Source IP and avoids a second hop for LoadBalancer and Nodeport type services." + "description": "externalTrafficPolicy denotes if this Service desires to route external traffic to node-local or cluster-wide endpoints. \"Local\" preserves the client source IP and avoids a second hop for LoadBalancer and Nodeport type services, but risks potentially imbalanced traffic spreading. \"Cluster\" obscures the client source IP and may cause a second hop to another node, but should have good overall load-spreading." }, "healthCheckNodePort": { "type": "integer", diff --git a/federation/docs/api-reference/v1/definitions.html b/federation/docs/api-reference/v1/definitions.html index 7e4bd446f5..f3c50871e7 100755 --- a/federation/docs/api-reference/v1/definitions.html +++ b/federation/docs/api-reference/v1/definitions.html @@ -2207,7 +2207,7 @@ When an object is created, the system will populate this list with the current s

externalTrafficPolicy

-

externalTrafficPolicy denotes if this Service desires to route external traffic to local endpoints only. This preserves Source IP and avoids a second hop for LoadBalancer and Nodeport type services.

+

externalTrafficPolicy denotes if this Service desires to route external traffic to node-local or cluster-wide endpoints. "Local" preserves the client source IP and avoids a second hop for LoadBalancer and Nodeport type services, but risks potentially imbalanced traffic spreading. "Cluster" obscures the client source IP and may cause a second hop to another node, but should have good overall load-spreading.

false

string

@@ -2331,7 +2331,7 @@ Examples:
diff --git a/pkg/api/service/util_test.go b/pkg/api/service/util_test.go index 82ae6a813f..6c87173ed2 100644 --- a/pkg/api/service/util_test.go +++ b/pkg/api/service/util_test.go @@ -157,7 +157,7 @@ func TestRequestsOnlyLocalTraffic(t *testing.T) { checkRequestsOnlyLocalTraffic(false, &api.Service{ Spec: api.ServiceSpec{ Type: api.ServiceTypeNodePort, - ExternalTrafficPolicy: api.ServiceExternalTrafficPolicyTypeGlobal, + ExternalTrafficPolicy: api.ServiceExternalTrafficPolicyTypeCluster, }, }) checkRequestsOnlyLocalTraffic(true, &api.Service{ @@ -169,7 +169,7 @@ func TestRequestsOnlyLocalTraffic(t *testing.T) { checkRequestsOnlyLocalTraffic(false, &api.Service{ Spec: api.ServiceSpec{ Type: api.ServiceTypeLoadBalancer, - ExternalTrafficPolicy: api.ServiceExternalTrafficPolicyTypeGlobal, + ExternalTrafficPolicy: api.ServiceExternalTrafficPolicyTypeCluster, }, }) checkRequestsOnlyLocalTraffic(true, &api.Service{ @@ -197,7 +197,7 @@ func TestNeedsHealthCheck(t *testing.T) { checkNeedsHealthCheck(false, &api.Service{ Spec: api.ServiceSpec{ Type: api.ServiceTypeNodePort, - ExternalTrafficPolicy: api.ServiceExternalTrafficPolicyTypeGlobal, + ExternalTrafficPolicy: api.ServiceExternalTrafficPolicyTypeCluster, }, }) checkNeedsHealthCheck(false, &api.Service{ @@ -209,7 +209,7 @@ func TestNeedsHealthCheck(t *testing.T) { checkNeedsHealthCheck(false, &api.Service{ Spec: api.ServiceSpec{ Type: api.ServiceTypeLoadBalancer, - ExternalTrafficPolicy: api.ServiceExternalTrafficPolicyTypeGlobal, + ExternalTrafficPolicy: api.ServiceExternalTrafficPolicyTypeCluster, }, }) checkNeedsHealthCheck(true, &api.Service{ @@ -268,13 +268,13 @@ func TestGetServiceHealthCheckNodePort(t *testing.T) { checkGetServiceHealthCheckNodePort(0, &api.Service{ Spec: api.ServiceSpec{ Type: api.ServiceTypeNodePort, - ExternalTrafficPolicy: api.ServiceExternalTrafficPolicyTypeGlobal, + ExternalTrafficPolicy: api.ServiceExternalTrafficPolicyTypeCluster, }, }) checkGetServiceHealthCheckNodePort(0, &api.Service{ Spec: api.ServiceSpec{ Type: api.ServiceTypeLoadBalancer, - ExternalTrafficPolicy: api.ServiceExternalTrafficPolicyTypeGlobal, + ExternalTrafficPolicy: api.ServiceExternalTrafficPolicyTypeCluster, }, }) checkGetServiceHealthCheckNodePort(34567, &api.Service{ @@ -306,7 +306,7 @@ func TestClearExternalTrafficPolicy(t *testing.T) { &api.Service{ Spec: api.ServiceSpec{ Type: api.ServiceTypeClusterIP, - ExternalTrafficPolicy: api.ServiceExternalTrafficPolicyTypeGlobal, + ExternalTrafficPolicy: api.ServiceExternalTrafficPolicyTypeCluster, }, }, }, @@ -346,7 +346,7 @@ func TestSetServiceHealthCheckNodePort(t *testing.T) { &api.Service{ Spec: api.ServiceSpec{ Type: api.ServiceTypeClusterIP, - ExternalTrafficPolicy: api.ServiceExternalTrafficPolicyTypeGlobal, + ExternalTrafficPolicy: api.ServiceExternalTrafficPolicyTypeCluster, }, }, 30012, @@ -356,7 +356,7 @@ func TestSetServiceHealthCheckNodePort(t *testing.T) { &api.Service{ Spec: api.ServiceSpec{ Type: api.ServiceTypeClusterIP, - ExternalTrafficPolicy: api.ServiceExternalTrafficPolicyTypeGlobal, + ExternalTrafficPolicy: api.ServiceExternalTrafficPolicyTypeCluster, }, }, 0, diff --git a/pkg/api/testing/fuzzer.go b/pkg/api/testing/fuzzer.go index e3884e42e8..8a6795df7e 100644 --- a/pkg/api/testing/fuzzer.go +++ b/pkg/api/testing/fuzzer.go @@ -322,7 +322,7 @@ func coreFuncs(t apitesting.TestingCommon) []interface{} { *p = types[c.Rand.Intn(len(types))] }, func(p *api.ServiceExternalTrafficPolicyType, c fuzz.Continue) { - types := []api.ServiceExternalTrafficPolicyType{api.ServiceExternalTrafficPolicyTypeGlobal, api.ServiceExternalTrafficPolicyTypeLocal} + types := []api.ServiceExternalTrafficPolicyType{api.ServiceExternalTrafficPolicyTypeCluster, api.ServiceExternalTrafficPolicyTypeLocal} *p = types[c.Rand.Intn(len(types))] }, func(ct *api.Container, c fuzz.Continue) { diff --git a/pkg/api/types.go b/pkg/api/types.go index 6d8943c36a..6349c0b111 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -2504,10 +2504,10 @@ const ( type ServiceExternalTrafficPolicyType string const ( - // ServiceExternalTrafficPolicyTypeLocal specifies local endpoints behavior. + // ServiceExternalTrafficPolicyTypeLocal specifies node-local endpoints behavior. ServiceExternalTrafficPolicyTypeLocal ServiceExternalTrafficPolicyType = "Local" - // ServiceExternalTrafficPolicyTypeGlobal specifies global (legacy) behavior. - ServiceExternalTrafficPolicyTypeGlobal ServiceExternalTrafficPolicyType = "Global" + // ServiceExternalTrafficPolicyTypeCluster specifies cluster-wide (legacy) behavior. + ServiceExternalTrafficPolicyTypeCluster ServiceExternalTrafficPolicyType = "Cluster" ) // ServiceStatus represents the current status of a service @@ -2610,9 +2610,12 @@ type ServiceSpec struct { // +optional LoadBalancerSourceRanges []string - // externalTrafficPolicy denotes if this Service desires to route external traffic to - // local endpoints only. This preserves Source IP and avoids a second hop for - // LoadBalancer and Nodeport type services. + // externalTrafficPolicy denotes if this Service desires to route external + // traffic to node-local or cluster-wide endpoints. "Local" preserves the + // client source IP and avoids a second hop for LoadBalancer and Nodeport + // type services, but risks potentially imbalanced traffic spreading. + // "Cluster" obscures the client source IP and may cause a second hop to + // another node, but should have good overall load-spreading. // +optional ExternalTrafficPolicy ServiceExternalTrafficPolicyType diff --git a/pkg/api/v1/defaults.go b/pkg/api/v1/defaults.go index 84e95cc334..d6cf9fc287 100644 --- a/pkg/api/v1/defaults.go +++ b/pkg/api/v1/defaults.go @@ -120,7 +120,7 @@ func SetDefaults_Service(obj *Service) { } else if (obj.Spec.Type == ServiceTypeNodePort || obj.Spec.Type == ServiceTypeLoadBalancer) && obj.Spec.ExternalTrafficPolicy == "" { - obj.Spec.ExternalTrafficPolicy = ServiceExternalTrafficPolicyTypeGlobal + obj.Spec.ExternalTrafficPolicy = ServiceExternalTrafficPolicyTypeCluster } } func SetDefaults_Pod(obj *Pod) { diff --git a/pkg/api/v1/defaults_test.go b/pkg/api/v1/defaults_test.go index c63bfda9bb..fbd355448c 100644 --- a/pkg/api/v1/defaults_test.go +++ b/pkg/api/v1/defaults_test.go @@ -885,15 +885,15 @@ func TestSetDefaulServiceExternalTraffic(t *testing.T) { in = &v1.Service{Spec: v1.ServiceSpec{Type: v1.ServiceTypeNodePort}} obj = roundTrip(t, runtime.Object(in)) out = obj.(*v1.Service) - if out.Spec.ExternalTrafficPolicy != v1.ServiceExternalTrafficPolicyTypeGlobal { - t.Errorf("Expected ExternalTrafficPolicy to be %v, got %v", v1.ServiceExternalTrafficPolicyTypeGlobal, out.Spec.ExternalTrafficPolicy) + if out.Spec.ExternalTrafficPolicy != v1.ServiceExternalTrafficPolicyTypeCluster { + t.Errorf("Expected ExternalTrafficPolicy to be %v, got %v", v1.ServiceExternalTrafficPolicyTypeCluster, out.Spec.ExternalTrafficPolicy) } in = &v1.Service{Spec: v1.ServiceSpec{Type: v1.ServiceTypeLoadBalancer}} obj = roundTrip(t, runtime.Object(in)) out = obj.(*v1.Service) - if out.Spec.ExternalTrafficPolicy != v1.ServiceExternalTrafficPolicyTypeGlobal { - t.Errorf("Expected ExternalTrafficPolicy to be %v, got %v", v1.ServiceExternalTrafficPolicyTypeGlobal, out.Spec.ExternalTrafficPolicy) + if out.Spec.ExternalTrafficPolicy != v1.ServiceExternalTrafficPolicyTypeCluster { + t.Errorf("Expected ExternalTrafficPolicy to be %v, got %v", v1.ServiceExternalTrafficPolicyTypeCluster, out.Spec.ExternalTrafficPolicy) } in = &v1.Service{ diff --git a/pkg/api/v1/generated.proto b/pkg/api/v1/generated.proto index e76c7c4b63..59c76960c3 100644 --- a/pkg/api/v1/generated.proto +++ b/pkg/api/v1/generated.proto @@ -3734,9 +3734,12 @@ message ServiceSpec { // +optional optional string externalName = 10; - // externalTrafficPolicy denotes if this Service desires to route external traffic to - // local endpoints only. This preserves Source IP and avoids a second hop for - // LoadBalancer and Nodeport type services. + // externalTrafficPolicy denotes if this Service desires to route external + // traffic to node-local or cluster-wide endpoints. "Local" preserves the + // client source IP and avoids a second hop for LoadBalancer and Nodeport + // type services, but risks potentially imbalanced traffic spreading. + // "Cluster" obscures the client source IP and may cause a second hop to + // another node, but should have good overall load-spreading. // +optional optional string externalTrafficPolicy = 11; diff --git a/pkg/api/v1/service/util_test.go b/pkg/api/v1/service/util_test.go index ade8bc81d2..9dcb64512e 100644 --- a/pkg/api/v1/service/util_test.go +++ b/pkg/api/v1/service/util_test.go @@ -157,7 +157,7 @@ func TestRequestsOnlyLocalTraffic(t *testing.T) { checkRequestsOnlyLocalTraffic(false, &v1.Service{ Spec: v1.ServiceSpec{ Type: v1.ServiceTypeNodePort, - ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeGlobal, + ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeCluster, }, }) checkRequestsOnlyLocalTraffic(true, &v1.Service{ @@ -169,7 +169,7 @@ func TestRequestsOnlyLocalTraffic(t *testing.T) { checkRequestsOnlyLocalTraffic(false, &v1.Service{ Spec: v1.ServiceSpec{ Type: v1.ServiceTypeLoadBalancer, - ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeGlobal, + ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeCluster, }, }) checkRequestsOnlyLocalTraffic(true, &v1.Service{ @@ -197,7 +197,7 @@ func TestNeedsHealthCheck(t *testing.T) { checkNeedsHealthCheck(false, &v1.Service{ Spec: v1.ServiceSpec{ Type: v1.ServiceTypeNodePort, - ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeGlobal, + ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeCluster, }, }) checkNeedsHealthCheck(false, &v1.Service{ @@ -209,7 +209,7 @@ func TestNeedsHealthCheck(t *testing.T) { checkNeedsHealthCheck(false, &v1.Service{ Spec: v1.ServiceSpec{ Type: v1.ServiceTypeLoadBalancer, - ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeGlobal, + ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeCluster, }, }) checkNeedsHealthCheck(true, &v1.Service{ @@ -268,13 +268,13 @@ func TestGetServiceHealthCheckNodePort(t *testing.T) { checkGetServiceHealthCheckNodePort(0, &v1.Service{ Spec: v1.ServiceSpec{ Type: v1.ServiceTypeNodePort, - ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeGlobal, + ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeCluster, }, }) checkGetServiceHealthCheckNodePort(0, &v1.Service{ Spec: v1.ServiceSpec{ Type: v1.ServiceTypeLoadBalancer, - ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeGlobal, + ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeCluster, }, }) checkGetServiceHealthCheckNodePort(34567, &v1.Service{ @@ -306,7 +306,7 @@ func TestClearExternalTrafficPolicy(t *testing.T) { &v1.Service{ Spec: v1.ServiceSpec{ Type: v1.ServiceTypeClusterIP, - ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeGlobal, + ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeCluster, }, }, }, @@ -346,7 +346,7 @@ func TestSetServiceHealthCheckNodePort(t *testing.T) { &v1.Service{ Spec: v1.ServiceSpec{ Type: v1.ServiceTypeClusterIP, - ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeGlobal, + ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeCluster, }, }, 30012, @@ -356,7 +356,7 @@ func TestSetServiceHealthCheckNodePort(t *testing.T) { &v1.Service{ Spec: v1.ServiceSpec{ Type: v1.ServiceTypeClusterIP, - ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeGlobal, + ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeCluster, }, }, 0, diff --git a/pkg/api/v1/types.go b/pkg/api/v1/types.go index 93caed774e..3e5d772dfd 100644 --- a/pkg/api/v1/types.go +++ b/pkg/api/v1/types.go @@ -2842,10 +2842,10 @@ const ( type ServiceExternalTrafficPolicyType string const ( - // ServiceExternalTrafficPolicyTypeLocal specifies local endpoints behavior. + // ServiceExternalTrafficPolicyTypeLocal specifies node-local endpoints behavior. ServiceExternalTrafficPolicyTypeLocal ServiceExternalTrafficPolicyType = "Local" - // ServiceExternalTrafficPolicyTypeGlobal specifies global (legacy) behavior. - ServiceExternalTrafficPolicyTypeGlobal ServiceExternalTrafficPolicyType = "Global" + // ServiceExternalTrafficPolicyTypeCluster specifies node-global (legacy) behavior. + ServiceExternalTrafficPolicyTypeCluster ServiceExternalTrafficPolicyType = "Cluster" ) // ServiceStatus represents the current status of a service. @@ -2961,9 +2961,12 @@ type ServiceSpec struct { // +optional ExternalName string `json:"externalName,omitempty" protobuf:"bytes,10,opt,name=externalName"` - // externalTrafficPolicy denotes if this Service desires to route external traffic to - // local endpoints only. This preserves Source IP and avoids a second hop for - // LoadBalancer and Nodeport type services. + // externalTrafficPolicy denotes if this Service desires to route external + // traffic to node-local or cluster-wide endpoints. "Local" preserves the + // client source IP and avoids a second hop for LoadBalancer and Nodeport + // type services, but risks potentially imbalanced traffic spreading. + // "Cluster" obscures the client source IP and may cause a second hop to + // another node, but should have good overall load-spreading. // +optional ExternalTrafficPolicy ServiceExternalTrafficPolicyType `json:"externalTrafficPolicy,omitempty" protobuf:"bytes,11,opt,name=externalTrafficPolicy"` diff --git a/pkg/api/v1/types_swagger_doc_generated.go b/pkg/api/v1/types_swagger_doc_generated.go index 7b2a450245..b5ab0eb659 100644 --- a/pkg/api/v1/types_swagger_doc_generated.go +++ b/pkg/api/v1/types_swagger_doc_generated.go @@ -1858,7 +1858,7 @@ var map_ServiceSpec = map[string]string{ "loadBalancerIP": "Only applies to Service Type: LoadBalancer LoadBalancer will get created with the IP specified in this field. This feature depends on whether the underlying cloud-provider supports specifying the loadBalancerIP when a load balancer is created. This field will be ignored if the cloud-provider does not support the feature.", "loadBalancerSourceRanges": "If specified and supported by the platform, this will restrict traffic through the cloud-provider load-balancer will be restricted to the specified client IPs. This field will be ignored if the cloud-provider does not support the feature.\" More info: https://kubernetes.io/docs/tasks/access-application-cluster/configure-cloud-provider-firewall/", "externalName": "externalName is the external reference that kubedns or equivalent will return as a CNAME record for this service. No proxying will be involved. Must be a valid DNS name and requires Type to be ExternalName.", - "externalTrafficPolicy": "externalTrafficPolicy denotes if this Service desires to route external traffic to local endpoints only. This preserves Source IP and avoids a second hop for LoadBalancer and Nodeport type services.", + "externalTrafficPolicy": "externalTrafficPolicy denotes if this Service desires to route external traffic to node-local or cluster-wide endpoints. \"Local\" preserves the client source IP and avoids a second hop for LoadBalancer and Nodeport type services, but risks potentially imbalanced traffic spreading. \"Cluster\" obscures the client source IP and may cause a second hop to another node, but should have good overall load-spreading.", "healthCheckNodePort": "healthCheckNodePort specifies the healthcheck nodePort for the service. If not specified, HealthCheckNodePort is created by the service api backend with the allocated nodePort. Will use user-specified nodePort value if specified by the client. Only effects when Type is set to LoadBalancer and ExternalTrafficPolicy is set to Local.", } diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index f35516b29e..2d165895e9 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -2903,10 +2903,10 @@ func validateServiceExternalTrafficFieldsValue(service *api.Service) field.Error // Check first class fields. if service.Spec.ExternalTrafficPolicy != "" && - service.Spec.ExternalTrafficPolicy != api.ServiceExternalTrafficPolicyTypeGlobal && + service.Spec.ExternalTrafficPolicy != api.ServiceExternalTrafficPolicyTypeCluster && service.Spec.ExternalTrafficPolicy != api.ServiceExternalTrafficPolicyTypeLocal { allErrs = append(allErrs, field.Invalid(field.NewPath("spec").Child("externalTrafficPolicy"), service.Spec.ExternalTrafficPolicy, - fmt.Sprintf("ExternalTrafficPolicy must be empty, %v or %v", api.ServiceExternalTrafficPolicyTypeGlobal, api.ServiceExternalTrafficPolicyTypeLocal))) + fmt.Sprintf("ExternalTrafficPolicy must be empty, %v or %v", api.ServiceExternalTrafficPolicyTypeCluster, api.ServiceExternalTrafficPolicyTypeLocal))) } if service.Spec.HealthCheckNodePort < 0 { allErrs = append(allErrs, field.Invalid(field.NewPath("spec").Child("healthCheckNodePort"), service.Spec.HealthCheckNodePort, diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 9b03fd1534..0bb013028a 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -6380,7 +6380,7 @@ func TestValidateServiceExternalTrafficFieldsCombination(t *testing.T) { name: "cannot set healthCheckNodePort field on loadBalancer service with externalTrafficPolicy!=Local", tweakSvc: func(s *api.Service) { s.Spec.Type = api.ServiceTypeLoadBalancer - s.Spec.ExternalTrafficPolicy = api.ServiceExternalTrafficPolicyTypeGlobal + s.Spec.ExternalTrafficPolicy = api.ServiceExternalTrafficPolicyTypeCluster s.Spec.HealthCheckNodePort = 34567 }, numErrs: 1, diff --git a/pkg/registry/core/service/rest_test.go b/pkg/registry/core/service/rest_test.go index ebd863c373..3509fd4b7b 100644 --- a/pkg/registry/core/service/rest_test.go +++ b/pkg/registry/core/service/rest_test.go @@ -1189,7 +1189,7 @@ func TestServiceRegistryExternalTrafficGlobal(t *testing.T) { Protocol: api.ProtocolTCP, TargetPort: intstr.FromInt(6502), }}, - ExternalTrafficPolicy: api.ServiceExternalTrafficPolicyTypeGlobal, + ExternalTrafficPolicy: api.ServiceExternalTrafficPolicyTypeCluster, }, } created_svc, err := storage.Create(ctx, svc) diff --git a/staging/src/k8s.io/client-go/pkg/api/types.go b/staging/src/k8s.io/client-go/pkg/api/types.go index 6d8943c36a..6349c0b111 100644 --- a/staging/src/k8s.io/client-go/pkg/api/types.go +++ b/staging/src/k8s.io/client-go/pkg/api/types.go @@ -2504,10 +2504,10 @@ const ( type ServiceExternalTrafficPolicyType string const ( - // ServiceExternalTrafficPolicyTypeLocal specifies local endpoints behavior. + // ServiceExternalTrafficPolicyTypeLocal specifies node-local endpoints behavior. ServiceExternalTrafficPolicyTypeLocal ServiceExternalTrafficPolicyType = "Local" - // ServiceExternalTrafficPolicyTypeGlobal specifies global (legacy) behavior. - ServiceExternalTrafficPolicyTypeGlobal ServiceExternalTrafficPolicyType = "Global" + // ServiceExternalTrafficPolicyTypeCluster specifies cluster-wide (legacy) behavior. + ServiceExternalTrafficPolicyTypeCluster ServiceExternalTrafficPolicyType = "Cluster" ) // ServiceStatus represents the current status of a service @@ -2610,9 +2610,12 @@ type ServiceSpec struct { // +optional LoadBalancerSourceRanges []string - // externalTrafficPolicy denotes if this Service desires to route external traffic to - // local endpoints only. This preserves Source IP and avoids a second hop for - // LoadBalancer and Nodeport type services. + // externalTrafficPolicy denotes if this Service desires to route external + // traffic to node-local or cluster-wide endpoints. "Local" preserves the + // client source IP and avoids a second hop for LoadBalancer and Nodeport + // type services, but risks potentially imbalanced traffic spreading. + // "Cluster" obscures the client source IP and may cause a second hop to + // another node, but should have good overall load-spreading. // +optional ExternalTrafficPolicy ServiceExternalTrafficPolicyType diff --git a/staging/src/k8s.io/client-go/pkg/api/v1/defaults.go b/staging/src/k8s.io/client-go/pkg/api/v1/defaults.go index 19c6f6f56c..a791ba7b5c 100644 --- a/staging/src/k8s.io/client-go/pkg/api/v1/defaults.go +++ b/staging/src/k8s.io/client-go/pkg/api/v1/defaults.go @@ -120,7 +120,7 @@ func SetDefaults_Service(obj *Service) { } else if (obj.Spec.Type == ServiceTypeNodePort || obj.Spec.Type == ServiceTypeLoadBalancer) && obj.Spec.ExternalTrafficPolicy == "" { - obj.Spec.ExternalTrafficPolicy = ServiceExternalTrafficPolicyTypeGlobal + obj.Spec.ExternalTrafficPolicy = ServiceExternalTrafficPolicyTypeCluster } } func SetDefaults_Pod(obj *Pod) { diff --git a/staging/src/k8s.io/client-go/pkg/api/v1/generated.proto b/staging/src/k8s.io/client-go/pkg/api/v1/generated.proto index a84f642d16..7f1b196f61 100644 --- a/staging/src/k8s.io/client-go/pkg/api/v1/generated.proto +++ b/staging/src/k8s.io/client-go/pkg/api/v1/generated.proto @@ -3734,9 +3734,12 @@ message ServiceSpec { // +optional optional string externalName = 10; - // externalTrafficPolicy denotes if this Service desires to route external traffic to - // local endpoints only. This preserves Source IP and avoids a second hop for - // LoadBalancer and Nodeport type services. + // externalTrafficPolicy denotes if this Service desires to route external + // traffic to node-local or cluster-wide endpoints. "Local" preserves the + // client source IP and avoids a second hop for LoadBalancer and Nodeport + // type services, but risks potentially imbalanced traffic spreading. + // "Cluster" obscures the client source IP and may cause a second hop to + // another node, but should have good overall load-spreading. // +optional optional string externalTrafficPolicy = 11; diff --git a/staging/src/k8s.io/client-go/pkg/api/v1/types.go b/staging/src/k8s.io/client-go/pkg/api/v1/types.go index 93caed774e..3e5d772dfd 100644 --- a/staging/src/k8s.io/client-go/pkg/api/v1/types.go +++ b/staging/src/k8s.io/client-go/pkg/api/v1/types.go @@ -2842,10 +2842,10 @@ const ( type ServiceExternalTrafficPolicyType string const ( - // ServiceExternalTrafficPolicyTypeLocal specifies local endpoints behavior. + // ServiceExternalTrafficPolicyTypeLocal specifies node-local endpoints behavior. ServiceExternalTrafficPolicyTypeLocal ServiceExternalTrafficPolicyType = "Local" - // ServiceExternalTrafficPolicyTypeGlobal specifies global (legacy) behavior. - ServiceExternalTrafficPolicyTypeGlobal ServiceExternalTrafficPolicyType = "Global" + // ServiceExternalTrafficPolicyTypeCluster specifies node-global (legacy) behavior. + ServiceExternalTrafficPolicyTypeCluster ServiceExternalTrafficPolicyType = "Cluster" ) // ServiceStatus represents the current status of a service. @@ -2961,9 +2961,12 @@ type ServiceSpec struct { // +optional ExternalName string `json:"externalName,omitempty" protobuf:"bytes,10,opt,name=externalName"` - // externalTrafficPolicy denotes if this Service desires to route external traffic to - // local endpoints only. This preserves Source IP and avoids a second hop for - // LoadBalancer and Nodeport type services. + // externalTrafficPolicy denotes if this Service desires to route external + // traffic to node-local or cluster-wide endpoints. "Local" preserves the + // client source IP and avoids a second hop for LoadBalancer and Nodeport + // type services, but risks potentially imbalanced traffic spreading. + // "Cluster" obscures the client source IP and may cause a second hop to + // another node, but should have good overall load-spreading. // +optional ExternalTrafficPolicy ServiceExternalTrafficPolicyType `json:"externalTrafficPolicy,omitempty" protobuf:"bytes,11,opt,name=externalTrafficPolicy"` diff --git a/staging/src/k8s.io/client-go/pkg/api/v1/types_swagger_doc_generated.go b/staging/src/k8s.io/client-go/pkg/api/v1/types_swagger_doc_generated.go index 7b2a450245..b5ab0eb659 100644 --- a/staging/src/k8s.io/client-go/pkg/api/v1/types_swagger_doc_generated.go +++ b/staging/src/k8s.io/client-go/pkg/api/v1/types_swagger_doc_generated.go @@ -1858,7 +1858,7 @@ var map_ServiceSpec = map[string]string{ "loadBalancerIP": "Only applies to Service Type: LoadBalancer LoadBalancer will get created with the IP specified in this field. This feature depends on whether the underlying cloud-provider supports specifying the loadBalancerIP when a load balancer is created. This field will be ignored if the cloud-provider does not support the feature.", "loadBalancerSourceRanges": "If specified and supported by the platform, this will restrict traffic through the cloud-provider load-balancer will be restricted to the specified client IPs. This field will be ignored if the cloud-provider does not support the feature.\" More info: https://kubernetes.io/docs/tasks/access-application-cluster/configure-cloud-provider-firewall/", "externalName": "externalName is the external reference that kubedns or equivalent will return as a CNAME record for this service. No proxying will be involved. Must be a valid DNS name and requires Type to be ExternalName.", - "externalTrafficPolicy": "externalTrafficPolicy denotes if this Service desires to route external traffic to local endpoints only. This preserves Source IP and avoids a second hop for LoadBalancer and Nodeport type services.", + "externalTrafficPolicy": "externalTrafficPolicy denotes if this Service desires to route external traffic to node-local or cluster-wide endpoints. \"Local\" preserves the client source IP and avoids a second hop for LoadBalancer and Nodeport type services, but risks potentially imbalanced traffic spreading. \"Cluster\" obscures the client source IP and may cause a second hop to another node, but should have good overall load-spreading.", "healthCheckNodePort": "healthCheckNodePort specifies the healthcheck nodePort for the service. If not specified, HealthCheckNodePort is created by the service api backend with the allocated nodePort. Will use user-specified nodePort value if specified by the client. Only effects when Type is set to LoadBalancer and ExternalTrafficPolicy is set to Local.", } diff --git a/test/e2e/service.go b/test/e2e/service.go index 03282748bc..d78dbc6c51 100644 --- a/test/e2e/service.go +++ b/test/e2e/service.go @@ -1456,7 +1456,7 @@ var _ = framework.KubeDescribe("ESIPP [Slow]", func() { By("turning ESIPP off") svc = jig.UpdateServiceOrFail(svc.Namespace, svc.Name, func(svc *v1.Service) { - svc.Spec.ExternalTrafficPolicy = v1.ServiceExternalTrafficPolicyTypeGlobal + svc.Spec.ExternalTrafficPolicy = v1.ServiceExternalTrafficPolicyTypeCluster }) if service.GetServiceHealthCheckNodePort(svc) > 0 { framework.Failf("Service HealthCheck NodePort still present")