Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add documentation for Service InternalTrafficPolicy #27088

Merged
merged 1 commit into from
Mar 31, 2021
Merged

Add documentation for Service InternalTrafficPolicy #27088

merged 1 commit into from
Mar 31, 2021

Conversation

maplain
Copy link

@maplain maplain commented Mar 17, 2021

This adds documentation for the new Service Internal Traffic Policy feature.

@k8s-ci-robot k8s-ci-robot added this to the 1.21 milestone Mar 17, 2021
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 17, 2021
@k8sio-netlify-preview-bot
Copy link
Collaborator

k8sio-netlify-preview-bot commented Mar 17, 2021 •
edited
Loading

Deploy preview for kubernetes-io-vnext-staging processing.

Building with commit 66cb8f9

https://app.netlify.com/sites/kubernetes-io-vnext-staging/deploys/6064f690b050d2000783821f

@maplain
Copy link
Author

maplain commented Mar 17, 2021

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 17, 2021
@k8s-ci-robot k8s-ci-robot added language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 17, 2021
@reylejano
Copy link
Member

/assign @ChandaniM123
/sig network

@k8s-ci-robot k8s-ci-robot added the sig/network Categorizes an issue or PR as relevant to SIG Network. label Mar 17, 2021
@reylejano
Copy link
Member

Hi @maplain , I see that kubernetes/kubernetes#96600 has merged. Are there any other code level PRs that are open and holding this PR.
Are there additional documentation changes required for the new Service Internal Traffic Policy feature.

Here are upcoming doc related dates for the 1.21 release:

  • Docs Ready for Review deadline is March 24
  • Docs Ready to Merge deadline is March 31

@reylejano
Copy link
Member

@maplain kubernetes/kubernetes#96600 has merged, is this PR ready to merge?

@maplain
Copy link
Author

maplain commented Mar 29, 2021

Hi @reylejano I'll update this PR with docs today. sorry for the delay.

@maplain maplain force-pushed the service-internal-traffic-policy branch from 0286dbd to 71cddfd Compare March 29, 2021 23:49
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 29, 2021
@maplain
Copy link
Author

maplain commented Mar 29, 2021

added a new doc page for this feature.

cc @andrewsykim @reylejano

@reylejano
Copy link
Member

@andrewsykim or @kubernetes/sig-network-pr-reviews , please provide a technical review on this PR by tomorrow to get this into the 1.21 release

@sftim
Copy link
Contributor

sftim commented Mar 30, 2021

As this is documenting an alpha feature, it's OK to have a few documentation rough edges.

We do want:

  • factual accuracy, no obvious incorrect statements etc
  • a clear indication that the feature is alpha (DONE!)
  • enough detail that someone can decide whether or not that feature is relevant; if they find it's on in their cluster, should they worry, etc.
  • accurate feature gate & related details

I think that's the minimum. Obviously going beyond that is better and welcome.

We will consider PRs against dev-1.21 even after the initial docs merge, and because this is website is continuously delivered, we'll also consider the same equivalent commits post-release if you rebase against the primary branch.

TL;DR: So we don't need this PR to be completely finished feature docs, so long as the minimum is done well.

@maplain
Copy link
Author

maplain commented Mar 30, 2021

cc @robscott @thockin

@maplain maplain force-pushed the service-internal-traffic-policy branch from 71cddfd to e923c19 Compare March 31, 2021 06:10
@maplain
Copy link
Author

maplain commented Mar 31, 2021

/unhold

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 31, 2021
@maplain
Copy link
Author

maplain commented Mar 31, 2021

cc @andrewsykim for a technical review

@kbhawkey
Copy link
Contributor

Page preview:
https://deploy-preview-27088--kubernetes-io-vnext-staging.netlify.app/docs/concepts/services-networking/service-traffic-policy/

@kbhawkey
Copy link
Contributor

Hi @maplain .
Changes look okay. Would you check the links in the "What's next" section?
Thanks!
/lgtm

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

LGTM label has been added.

Git tree hash: 3559c4f0ea7e55f51a33b1f96a81d72709c1b028

@reylejano
Copy link
Member

Hi @maplain .
Changes look okay. Would you check the links in the "What's next" section?
Thanks!
/lgtm

The Topology Aware Hints link in What's Next is not live yet and is in PR #27032

@maplain
Copy link
Author

maplain commented Mar 31, 2021

Hi @maplain .
Changes look okay. Would you check the links in the "What's next" section?
Thanks!
/lgtm

Exactly like @reylejano said, that page is added here: https://github.com/kubernetes/website/pull/27032/files#diff-67b4e9d0b2af43e92aebb4f605cc31ce54ff8d33652d946302528c31cba470ed

@k8s-ci-robot
Copy link
Contributor

@maplain: you cannot LGTM your own PR.

In response to this:

Hi @maplain .
Changes look okay. Would you check the links in the "What's next" section?
Thanks!
/lgtm

Exactly like @reylejano said, that page is added here: https://github.com/kubernetes/website/pull/27032/files#diff-67b4e9d0b2af43e92aebb4f605cc31ce54ff8d33652d946302528c31cba470ed

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.

@maplain maplain force-pushed the service-internal-traffic-policy branch from e923c19 to 7655e4b Compare March 31, 2021 17:40
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 31, 2021
thockin
thockin reviewed Mar 31, 2021
View reviewed changes
Copy link
Member

@thockin thockin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small stuff

/assign

content/en/docs/concepts/services-networking/service-traffic-policy.md Outdated Show resolved Hide resolved
content/en/docs/concepts/services-networking/service-traffic-policy.md Outdated Show resolved Hide resolved
content/en/docs/concepts/services-networking/service-traffic-policy.md Outdated Show resolved Hide resolved
content/en/docs/concepts/services-networking/service-traffic-policy.md Show resolved Hide resolved
@maplain maplain force-pushed the service-internal-traffic-policy branch from 7655e4b to 30dd130 Compare March 31, 2021 21:16
@thockin
Copy link
Member

thockin commented Mar 31, 2021

/approve - just one typo found :)

@maplain maplain force-pushed the service-internal-traffic-policy branch from 30dd130 to 66cb8f9 Compare March 31, 2021 22:24
@thockin
Copy link
Member

thockin commented Mar 31, 2021

/lgtm

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

LGTM label has been added.

Git tree hash: dfb400a20f3ab17db0b0960863dd49ccfe11d288

@reylejano
Copy link
Member

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

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

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:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 31, 2021
@k8s-ci-robot k8s-ci-robot merged commit 8cadfc4 into kubernetes:dev-1.21 Mar 31, 2021
@maplain maplain deleted the service-internal-traffic-policy branch April 1, 2021 04:29
@sftim sftim mentioned this pull request Apr 3, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jimangel jimangel jimangel left review comments

@thockin thockin thockin left review comments

@celestehorgan celestehorgan celestehorgan left review comments

@sftim sftim sftim left review comments

@daminisatya daminisatya Awaiting requested review from daminisatya

Assignees

@kbhawkey kbhawkey

@thockin thockin

@ChandaniM123 ChandaniM123

@reylejano reylejano

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/docs Categorizes an issue or PR as relevant to SIG Docs. sig/network Categorizes an issue or PR as relevant to SIG Network. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
1.21
Development

Successfully merging this pull request may close these issues.
10 participants
@maplain @k8sio-netlify-preview-bot @reylejano @sftim @kbhawkey @k8s-ci-robot @thockin @jimangel @celestehorgan @ChandaniM123