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.

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

Add docs/dev/sig-network with the service proxy doc from kpng #124872

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

danwinship
Copy link
Contributor

What this PR does / why we need it:

kubernetes/community#7421 stalled, and just got bounced away from lifecycle/rotten again, so let's try to jump start it.

This copies the service proxying doc from kpng into a new docs/dev/sig-network directory. It doesn't currently try to set up anything to deal with publishing it anywhere. The doc itself is mostly unmodified from the existing kpng version, with the largest change being to update it for the loadbalancer IPMode feature, which is new since the last updates to the kpng doc. (Diff from kpng version)

Although this doesn't yet solve the problem of making this doc more accessible to potential readers, it at least moves it to a better location, and maybe it will encourage other people to think about similar things? (And I have a partial doc on implementing NetworkPolicy to add to it, and I suspect I'll be writing up a doc on implementing and e2e-testing cloud load balancers too...)

Which issue(s) this PR fixes:

It is a start at addressing kubernetes/community#7421.

Does this PR introduce a user-facing change?

NONE

/kind documentation
/sig network

/assign @thockin

@k8s-ci-robot k8s-ci-robot added the release-note-none Denotes a PR that doesn't merit a release note. label May 14, 2024
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. kind/documentation Categorizes issue or PR as related to documentation. sig/network Categorizes an issue or PR as relevant to SIG Network. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels May 14, 2024
@k8s-ci-robot
Copy link
Contributor

This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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-sigs/prow repository.

@thockin
Copy link
Member

thockin commented May 14, 2024

Thanks!

/lgtm
/approve

/hold for a few days to see if anyone balks

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels May 14, 2024
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: e150c644a89efcafb291ed4882ae11cad73b6d0e

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: danwinship, 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 May 14, 2024
@aojea
Copy link
Member

aojea commented May 15, 2024

The problem I see of adding docs here is that then you are tied to the releases and its rules, code freeze, test freeze ... if the problem is discovery an alternative is just create a docs/README.md that redirect to the community ...

In any case, having the docs in both places at the same time is fragmented and still more confusing

@danwinship
Copy link
Contributor Author

Read the linked issue; part of the goal of putting them here was to make it easier to modify the docs at the same time as the APIs. And neither code freeze nor test freeze applies to docs anyway, right? (If the existing descriptions are unclear about that, it's probably because there currently isn't much in the way of docs in k/k.)

@aojea
Copy link
Member

aojea commented May 15, 2024

Read the linked issue; part of the goal of putting them here was to make it easier to modify the docs at the same time as the APIs. And neither code freeze nor test freeze applies to docs anyway, right? (If the existing descriptions are unclear about that, it's probably because there currently isn't much in the way of docs in k/k.)

I don't feel we'll have the discipline to update the docs and the APIs, but I can be wrong and I will be super happy of being wrong ... still, or all the sigs do the same and move the docs in tree, or having it fragmented with sigs in one places and others in other place looks worse situation than before ...

@thockin
Copy link
Member

thockin commented May 15, 2024

This is the conversation I wanted to have. Today, it's far too easy to forget to update docs having them in-tree means I can do it in one PR. Obviously, IMO, these won't be the only or even final docs with translation etc.

I want things to be easier to find, and close to the places they are used. This doc is really aimed at implementers. If we had a dedicated kube-proxy repo, maybe it would go there. That's an option - staging/src/k8s.io/kube-proxy/docs ?

I'd like API conventions to be similarly scoped.

@danwinship
Copy link
Contributor Author

If we had a dedicated kube-proxy repo

We do, we just don't currently use it for anything except the versioned config APIs. (Though there's that vague plan to eventually move pkg/proxy there, once the APIs are in better shape.)

I guess the discussion in kubernetes/community#7421 mentioned that we could potentially be pulling content from multiple repos into the developer site, so this wouldn't block that.

OTOH I want to merge a NetworkPolicy implementation notes doc too, and that wouldn't really fit in k8s.io/kube-proxy (or, really, anywhere except k/k or sigs.k8s.io/network-policy-api).

(I also want to merge a cloud load balancer implementation notes doc, and that could fit in k8s.io/kube-proxy, though maybe it would be better in k8s.io/cloud-provider? I guess that's one down side of not using k/k/docs/devel; it makes it harder to figure out where any individual doc is.)

@thockin
Copy link
Member

thockin commented May 15, 2024

Those docs sound great!

Today the answer for such docs would be either https://git.k8s.io/community/contributors/devel/sig-network or https://git.k8s.io/community/sig-network - neither is terrible, but really "community" is not the repo wherein I would expect to find technical docs. IMO, these are not "glossy", end-user facing docs that need fancy formatting. They are (IMO) text docs (or simple markdown), which are aimed at implementors. I'm not AGAINST fancy formatting, but I don't want to waste time on it.

So, to me it makes sense to stick them in k/k/docs/..., but I would be fine with something like k/dev-docs/..., and if pressed I could just concede one of the above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. kind/documentation Categorizes issue or PR as related to documentation. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. release-note-none Denotes a PR that doesn't merit a release note. sig/network Categorizes an issue or PR as relevant to SIG Network. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants