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

migrate the K8s networking model section #28617

Merged
merged 1 commit into from
Jan 5, 2022
Merged

migrate the K8s networking model section #28617

merged 1 commit into from
Jan 5, 2022

Conversation

neha-viswanathan
Copy link
Contributor

Fixes #25832

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 25, 2021
@k8s-ci-robot k8s-ci-robot requested review from tengqm and thockin June 25, 2021 05:32
@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/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 25, 2021
@netlify
Copy link

netlify bot commented Jun 25, 2021 •
edited
Loading

✔️ Deploy Preview for kubernetes-io-main-staging ready!

🔨 Explore the source changes: 48237c4

🔍 Inspect the deploy log: https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/61d4fa45cdc93c0008f1d68a

😎 Browse the preview: https://deploy-preview-28617--kubernetes-io-main-staging.netlify.app

@neha-viswanathan
Copy link
Contributor Author

/assign @irvifa

sftim
sftim reviewed Jul 29, 2021
View reviewed changes
Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

@shannonxtreme would you be willing to take a look at this pull request? I'd be keep to understand how that fits in with some changes you're considering in this section.

@sftim
Copy link
Contributor

sftim commented Jul 29, 2021

LGTM

@shannonxtreme
Copy link
Contributor

@shannonxtreme would you be willing to take a look at this pull request? I'd be keep to understand how that fits in with some changes you're considering in this section.

Not sure how I feel about having the TOC on that page when there is text on it. Sidebar nav should be enough (similar to https://kubernetes.io/docs/concepts/workloads/)

This can be a separate PR though:

I think it would benefit from a diagram as well to visualize the networking model. Similar to the Components/Cluster Architecture ones where the overview has a basic diagram that drills deeper into each component in the relevant section.

This medium post has some nice diagrams that are broken into each component.

@sftim
Copy link
Contributor

sftim commented Jul 30, 2021

One thing I'll point out: the sidebar is visible to desktop clients and on big tablets. On a mobile or portrait-oriented device, you might not see it.

@sftim
Copy link
Contributor

sftim commented Jul 30, 2021

👍 to suppressing the automatic TOC (use no_list: true in the front matter).

/lgtm

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

LGTM label has been added.

Git tree hash: 3ce99ca52b5be6910f9fce76068bcb66a78ac591

@kbhawkey
Copy link
Contributor

👋 @neha-viswanathan . Thanks for your contribution.
Because the _index page contains a somewhat lengthy TOC (12 links), I don't know if it makes sense to move the model content to that page. Could this content move to a new page or another page? What do you think?

Page preview:
https://deploy-preview-28617--kubernetes-io-main-staging.netlify.app/docs/concepts/services-networking/#the-kubernetes-network-model

@neha-viswanathan neha-viswanathan force-pushed the 25832-migrate-network-model branch from c8cc8af to c55328a Compare October 1, 2021 18:16
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 1, 2021
@neha-viswanathan
Copy link
Contributor Author

neha-viswanathan commented Oct 1, 2021 •
edited
Loading

I don't know if it makes sense to move the model content to that page. Could this content move to a new page or another page?

Hi @kbhawkey 🙋🏽‍♀️ I followed the suggestion from @sftim on using no_list: true so the TOC doesn't appear on the page anymore.

https://deploy-preview-28617--kubernetes-io-main-staging.netlify.app/docs/concepts/services-networking/

@sftim
Copy link
Contributor

sftim commented Oct 2, 2021 •
edited
Loading

I just realized: Setting no_list: true makes sense only when we provide equivalent links within the page text. This is because not all ways of viewing the docs show the left-hand sidebar.

@shannonxtreme
Copy link
Contributor

shannonxtreme commented Nov 8, 2021 •
edited
Loading

Agreeing with Tim. Can we include organic links to topics in this section, similar to https://kubernetes.io/docs/concepts/containers/? LGTM from me after that

@kbhawkey
Copy link
Contributor

kbhawkey commented Nov 9, 2021

Hi @neha-viswanathan . The updates look good.
The commits include a couple of extra directory changes (submodules).
Can you remove the api-ref-generator and docsy submodules?
Thanks!

@kbhawkey
Copy link
Contributor

kbhawkey commented Nov 9, 2021

/label tide/merge-method-squash

@k8s-ci-robot k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Nov 9, 2021
@neha-viswanathan neha-viswanathan force-pushed the 25832-migrate-network-model branch 3 times, most recently from 390a2fd to 944fb8a Compare November 9, 2021 03:27
@neha-viswanathan
Copy link
Contributor Author

@shannonxtreme @kbhawkey ready for review again :)

@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 5, 2022
@neha-viswanathan neha-viswanathan force-pushed the 25832-migrate-network-model branch from 1ec5348 to 2e91a14 Compare January 5, 2022 01:51
@neha-viswanathan neha-viswanathan force-pushed the 25832-migrate-network-model branch from 2e91a14 to 48237c4 Compare January 5, 2022 01:54
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 5, 2022
@sftim
Copy link
Contributor

sftim commented Jan 5, 2022

/remove-language es
/remove-language fr
/remove-language id
/remove-language it
/remove-language ja
/remove-language ko
/remove-language pt
/remove-language ru
/remove-language zh

@k8s-ci-robot k8s-ci-robot removed language/es Issues or PRs related to Spanish language language/fr Issues or PRs related to French language language/id Issues or PRs related to Indonesian language language/it Issues or PRs related to Italian language language/ja Issues or PRs related to Japanese language language/ko Issues or PRs related to Korean language language/pt Issues or PRs related to Portuguese language language/ru Issues or PRs related to Russian language language/zh Issues or PRs related to Chinese language labels Jan 5, 2022
@sftim
Copy link
Contributor

sftim commented Jan 5, 2022

Taking LGTM from #28617 (comment)

/lgtm
/approve

Thanks folks!

@sftim
Copy link
Contributor

sftim commented Jan 5, 2022

/remove-area blog

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed area/blog Issues or PRs related to the Kubernetes Blog subproject labels Jan 5, 2022
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 7af6076629fb62991073e421cf9cc8e97b421ac5

@sftim
Copy link
Contributor

sftim commented Jan 5, 2022

/remove-area release-eng

@k8s-ci-robot k8s-ci-robot removed the area/release-eng Issues or PRs related to the Release Engineering subproject label Jan 5, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sftim, 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 Jan 5, 2022
@sftim
Copy link
Contributor

sftim commented Jan 5, 2022

/sig network
/remove-sig release

@k8s-ci-robot k8s-ci-robot added sig/network Categorizes an issue or PR as relevant to SIG Network. and removed sig/release Categorizes an issue or PR as relevant to SIG Release. labels Jan 5, 2022
@k8s-ci-robot k8s-ci-robot merged commit 24ee252 into kubernetes:main Jan 5, 2022
@ianychoi ianychoi mentioned this pull request Mar 5, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tengqm tengqm Awaiting requested review from tengqm

@thockin thockin Awaiting requested review from thockin

@sftim sftim Awaiting requested review from sftim

@shannonxtreme shannonxtreme Awaiting requested review from shannonxtreme

@mehabhalodiya mehabhalodiya Awaiting requested review from mehabhalodiya

Assignees

@thockin thockin

@irvifa irvifa

@sftim sftim

@shannonxtreme shannonxtreme

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. refactor Indicates a PR with large refactoring changes e.g. removes files or moves content 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. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Migrate “The Kubernetes network model” into “Services, Load Balancing, and Networking” section
9 participants
@neha-viswanathan @sftim @shannonxtreme @k8s-ci-robot @kbhawkey @reylejano @thockin @mehabhalodiya @irvifa