Update ESIPP documentation to reflect new fields #4093

MrHohn · 2017-06-15T01:31:43Z

Documentation for kubernetes/enhancements#27.

External Source IP Preservation (ESIPP) is promoted from Beta to GA in 1.7.

/assign @chenopis
cc @bowei @freehan

This change is

chenopis

A few minor edit suggestions.

chenopis · 2017-06-21T15:51:08Z

docs/tasks/access-application-cluster/create-external-load-balancer.md

+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.
+* `Service.Spec.HealthCheckNodePort` - It specifies the healthcheck nodePort for the
+service. If not specified, HealthCheckNodePort is created by the service api backend


capitalize API

chenopis · 2017-06-21T15:51:29Z

docs/tasks/access-application-cluster/create-external-load-balancer.md

+may cause a second hop to another node, but should have good overall load-spreading.
+* `Service.Spec.HealthCheckNodePort` - It 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


Add "It" and "the": "It will use the user-specified..."

chenopis · 2017-06-21T15:57:00Z

docs/tasks/access-application-cluster/create-external-load-balancer.md

+* `Service.Spec.HealthCheckNodePort` - It 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


I'm not sure what the meaning of this sentence is. I think you mean:

It is only has an effect when Type is set to "LoadBalancer" and ExternalTrafficPolicy is set to "Local".

Thanks, this is much better, applied.

chenopis · 2017-06-21T15:59:27Z

docs/tasks/access-application-cluster/create-external-load-balancer.md

+* `service.beta.kubernetes.io/healthcheck-nodeport` annotation <-> `Service.Spec.HealthCheckNodePort` field
+
+`service.beta.kubernetes.io/external-traffic` annotation has a different set of values
+comparing to `Service.Spec.ExternalTrafficPolicy` field, the values match as follows:


suggested edit: "...compared to the `Service.Spec.ExternalTrafficPolicy` field. The values match as follows:"

Thanks, applied.

MrHohn · 2017-06-21T21:48:33Z

Thanks! Re-pushed commit.

chenopis · 2017-06-21T21:55:55Z

Ok, we just need a tech lgtm from @bowei or @freehan.

MrHohn · 2017-06-21T21:56:39Z

/assign @bowei @freehan

chenopis · 2017-06-26T16:24:21Z

@bowei @freehan This PR still requires a tech review.

FYI, all feedback must be addressed and LGTMs given by EOD Tue, June 27th so that this can be merged for the 1.7 release on June 28th.

ahmetb · 2017-06-26T20:16:05Z

docs/tasks/access-application-cluster/create-external-load-balancer.md


-This feature can be activated by adding the beta annotation below to the metadata section of the Service Configuration file.
+* `Service.Spec.ExternalTrafficPolicy` - It denotes if this Service desires to route


Suggested change: service.spec.externalTrafficPolicy

Specify fields as they appear in YAML/JSON manifests, not in the API:

Makes sense, change applied.

ahmetb · 2017-06-26T20:16:20Z

docs/tasks/access-application-cluster/create-external-load-balancer.md

+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.
+* `Service.Spec.HealthCheckNodePort` - It specifies the healthcheck nodePort for the


service.spec.healthCheckNodePort

Change applied.

ahmetb · 2017-06-26T20:17:13Z

docs/tasks/access-application-cluster/create-external-load-balancer.md

+v1.7. Please consider using API fields instead.
+
+* `service.beta.kubernetes.io/external-traffic` annotation <-> `Service.Spec.ExternalTrafficPolicy` field
+* `service.beta.kubernetes.io/healthcheck-nodeport` annotation <-> `Service.Spec.HealthCheckNodePort` field


Same as above, use field names as they appear in the YAML manifests (same below)

Changes applied.

ahmetb · 2017-06-26T20:18:31Z

docs/tasks/access-application-cluster/create-external-load-balancer.md

      }
    }
 ```

+Below you could find the deprecated beta annotations. We may stop supporting them after


Do not use "we". Suggested rewrite:

Below you could find the deprecated annotations used to enable this feature prior to its stable version. Newer Kubernetes versions may stop supporting these after v1.7. Please update existing applications to
use the fields directly.

Thanks, this is much better, applied change.

bowei · 2017-06-27T00:00:20Z

docs/tasks/access-application-cluster/create-external-load-balancer.md


-This feature can be activated by adding the beta annotation below to the metadata section of the Service Configuration file.
+* `service.spec.externalTrafficPolicy` - It denotes if this Service desires to route


The starting word "It" seems strange, remove?

I think you need to add what values the field can take (this is a boolean, right?)

Yeah it could be either "Local" or "Cluster". Added a sentence to make it clear.

bowei · 2017-06-27T00:00:27Z

docs/tasks/access-application-cluster/create-external-load-balancer.md

+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.
+* `service.spec.healthCheckNodePort` - It specifies the healthcheck nodePort for the


The starting word "It" seems strange, remove?

You need to add the values that this can take.

Updated sentence.

bowei · 2017-06-27T00:00:56Z

docs/tasks/access-application-cluster/create-external-load-balancer.md

      }
    }
 ```

+Below you could find the deprecated annotations used to enable this feature


Make this a subsection

bowei · 2017-06-27T00:03:08Z

docs/tasks/access-application-cluster/create-external-load-balancer.md

@@ -97,20 +99,34 @@ which should produce output like this:

 The IP address is listed next to `LoadBalancer Ingress`.

-## Annotation to modify the LoadBalancer behavior for preservation of Source IP
-In 1.5, a Beta feature has been added that changes the behavior of the external LoadBalancer feature.
+## Modify the LoadBalancer behavior for preservation of Source IP


I would make this more direct:

## Preserving the client source IP

Sounds great, applied.

bowei · 2017-06-27T00:03:50Z

docs/tasks/access-application-cluster/create-external-load-balancer.md

-In 1.5, a Beta feature has been added that changes the behavior of the external LoadBalancer feature.
+## Modify the LoadBalancer behavior for preservation of Source IP
+
+Due to the implementation of this feature, the source IP for sessions as seen in the target


delete "for sessions as"

bowei · 2017-06-27T00:07:29Z

docs/tasks/access-application-cluster/create-external-load-balancer.md

+
+Due to the implementation of this feature, the source IP for sessions as seen in the target
+container will *not be the original source IP* of the client. This is the default behavior as
+of Kubernetes v1.7. However, starting in v1.5, an optional beta feature has been added that


Replace starting with "This is the default..."

To enable preservation of the client IP, the following field can be configured in the service spec:

you can delete the version stuff as you already cover it below in the beta etc discussion.

Makes sense, modified sentences.

bowei · 2017-06-27T05:18:06Z

docs/tasks/access-application-cluster/create-external-load-balancer.md

+load-spreading. "Local" preserves the client source IP and avoids a second hop
+for LoadBalancer and NodePort type services, but risks potentially imbalanced
+traffic spreading.
+* `service.spec.healthCheckNodePort` - specifies the healthcheck nodePort (a 32


16-bit, probably easier just to say (numeric port number)

Thanks, applied.

bowei · 2017-06-27T05:21:49Z

docs/tasks/access-application-cluster/create-external-load-balancer.md

      }
    }
 ```

+### Deprecated Beta annotations


### Feature availability | 1.7+ | supports the full ... | | 1.5 - 1.6 | Beta ... | | <1.5 | Unsupported |

Added availability table.

k8s-ci-robot assigned chenopis Jun 15, 2017

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 15, 2017

chenopis added this to the 1.7 milestone Jun 15, 2017

chenopis added the kind/feature label Jun 16, 2017

chenopis requested review from bowei and freehan June 20, 2017 17:57

chenopis added Needs Docs Review labels Jun 20, 2017

chenopis suggested changes Jun 21, 2017

View reviewed changes

chenopis added Docs Review: Open Issues and removed Needs Docs Review labels Jun 21, 2017

MrHohn force-pushed the release-1.7-esipp-ga branch from 3c9db99 to 9cbed0c Compare June 21, 2017 21:47

chenopis added Docs LGTM and removed Docs Review: Open Issues labels Jun 21, 2017

chenopis approved these changes Jun 21, 2017

View reviewed changes

k8s-ci-robot assigned bowei and freehan Jun 21, 2017

ahmetb suggested changes Jun 26, 2017

View reviewed changes

chenopis added Tech Review: Open Issues and removed Needs Tech Review labels Jun 26, 2017

MrHohn force-pushed the release-1.7-esipp-ga branch from 9cbed0c to 2be3c76 Compare June 26, 2017 23:54

freehan added Tech Review LGTM and removed Tech Review: Open Issues labels Jun 26, 2017

bowei reviewed Jun 27, 2017

View reviewed changes

MrHohn force-pushed the release-1.7-esipp-ga branch from e26f786 to 7267270 Compare June 27, 2017 04:05

bowei reviewed Jun 27, 2017

View reviewed changes

Update ESIPP docs for new added API fields

48caf20

MrHohn force-pushed the release-1.7-esipp-ga branch from 7267270 to 48caf20 Compare June 27, 2017 17:03

chenopis merged commit d8f5b4f into kubernetes:release-1.7 Jun 27, 2017

MrHohn deleted the release-1.7-esipp-ga branch October 5, 2017 00:57


		This feature can be activated by adding the beta annotation below to the metadata section of the Service Configuration file.
		* `Service.Spec.ExternalTrafficPolicy` - It denotes if this Service desires to route

Update ESIPP documentation to reflect new fields #4093

Update ESIPP documentation to reflect new fields #4093

Conversation

MrHohn commented Jun 15, 2017 • edited by k8s-reviewable Loading

chenopis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MrHohn commented Jun 21, 2017

chenopis commented Jun 21, 2017

MrHohn commented Jun 21, 2017

chenopis commented Jun 26, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MrHohn commented Jun 15, 2017 •

edited by k8s-reviewable

Loading