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

EndpointSlice 1.22 Docs Update (KEP 752) #28745

Merged
merged 1 commit into from
Jul 27, 2021

Conversation

swetharepakula
Copy link
Member

@swetharepakula swetharepakula commented Jul 2, 2021

@k8s-ci-robot k8s-ci-robot added this to the 1.22 milestone Jul 2, 2021
@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jul 2, 2021
@netlify
Copy link

netlify bot commented Jul 2, 2021

👷 Deploy Preview for kubernetes-io-vnext-staging processing.

🔨 Explore the source changes: 1168b46

🔍 Inspect the deploy log: https://app.netlify.com/sites/kubernetes-io-vnext-staging/deploys/61005c5b66a0a6000809b8ec

@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jul 2, 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. labels Jul 2, 2021
@PI-Victor
Copy link
Member

thank you for opening this, please remember to hold until the enhancement is no longer labelled 'At Risk'.

/sig network
/assign @ritpanjw

@k8s-ci-robot k8s-ci-robot added the sig/network Categorizes an issue or PR as relevant to SIG Network. label Jul 2, 2021
@sftim
Copy link
Contributor

sftim commented Jul 8, 2021

kubernetes/kubernetes#103596 suggests this PR might also need to cover EndpointSliceTerminatingCondition.

@swetharepakula
Copy link
Member Author

The enhancement is no longer at risk. All relevant PRs were merged yesterday.

This PR is to cover the changes related to kubernetes/enhancements#752

kubernetes/kubernetes#103596 is for a different KEP. I believe EndpointSliceTerminatingCondition is for kubernetes/enhancements#1672 and will be covered by a different docs PR

@ritpanjw
Copy link
Contributor

ritpanjw commented Jul 16, 2021

Hello @swetharepakula 👋 please take a look at Documenting for a release - PR Ready for Review to get your PR ready for review before Tuesday 20th of July, 11:59 PM PDT. In comments, you mentioned it is WIP, hence sending reminder. Thank you!

@PI-Victor
Copy link
Member

Hello @swetharepakula 👋 please take a look at Documenting for a release - PR Ready for Review to get your PR ready for review before Tuesday 20th of July, 11:59 PM PDT. In comments, you mentioned it is WIP, hence sending reminder. Thank you!

Hi @swetharepakula friendly reminder that today is the deadline for PRs ready for review, please make sure this PR is ready for review by the end of the day, thank you!

@k8s-ci-robot k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 20, 2021
@swetharepakula swetharepakula changed the title EndpointSlice 1.22 Docs Update EndpointSlice 1.22 Docs Update (KEP 752) Jul 20, 2021
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 20, 2021
@swetharepakula swetharepakula marked this pull request as ready for review July 20, 2021 20:45
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 20, 2021
@swetharepakula
Copy link
Member Author

Thanks for the reminder @ritpanjw & @PI-Victor! The PR is now ready for review

@swetharepakula
Copy link
Member Author

/cc @robscott @aojea

@k8s-ci-robot k8s-ci-robot requested a review from aojea July 20, 2021 20:47
@aojea
Copy link
Member

aojea commented Jul 20, 2021

LGTM

@robscott
Copy link
Member

Usually, the way we'd help readers is by advising them to check they're viewing the docs for their version of Kubernetes.
We can backport advice to older docs revisions if that makes sense to do so.

Thanks for the quick response @sftim! That approach makes sense to me. I think the docs for 1.21 are already sufficient here.

cluster annotates that Endpoints with `endpoints.kubernetes.io/over-capacity: warning`.
This annotation indicates that the affected Endpoints object is over capacity.
If an Endpoints resource has more than 1000 endpoints then a Kubernetes v1.22 (or later)
cluster annotates that Endpoints with `endpoints.kubernetes.io/over-capacity: truncated`.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Can you be more specific? What annotates the Endpoints resource?

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a small addition stating that the endpoint controller truncates the resource. Please let me know if you think we should expand more. I am not sure how detailed I should be here.

Copy link
Contributor

Choose a reason for hiding this comment

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

“the control plane has truncated” would be fine.

@@ -188,9 +188,10 @@ selectors and uses DNS names instead. For more information, see the
[ExternalName](#externalname) section later in this document.

### Over Capacity Endpoints
If an Endpoints resource has more than 1000 endpoints then a Kubernetes v1.21 (or later)
Copy link
Contributor

Choose a reason for hiding this comment

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

We should maybe edit the v1.21 docs (either now or post release) to remove the “or later” part from the v1.21 docs. WDYT @swetharepakula ?

Copy link
Member Author

@swetharepakula swetharepakula Jul 22, 2021

Choose a reason for hiding this comment

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

Oh that is a good point! I can send in a followup pr for that. Which branch would that pr need to be against?

Copy link
Contributor

Choose a reason for hiding this comment

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

At the moment, that is the main branch: commits there are continuously published to https://kubernetes.io/

Copy link
Member Author

@swetharepakula swetharepakula Jul 26, 2021

Choose a reason for hiding this comment

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

PR to update existing 1.21 docs: #29122

@sftim
Copy link
Contributor

sftim commented Jul 23, 2021

Next step is a tech review from SIG Network
@kubernetes/sig-network-pr-reviews please check this one out - it should be straightforward to verify.

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.

/lgtm

cluster annotates that Endpoints with `endpoints.kubernetes.io/over-capacity: warning`.
This annotation indicates that the affected Endpoints object is over capacity.
If an Endpoints resource has more than 1000 endpoints then a Kubernetes v1.22 (or later)
cluster annotates that Endpoints with `endpoints.kubernetes.io/over-capacity: truncated`.
Copy link
Contributor

Choose a reason for hiding this comment

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

“the control plane has truncated” would be fine.

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

LGTM label has been added.

Git tree hash: 6bccddd1d29f71c3b65153eb6bf13c04253d06e3

@sftim
Copy link
Contributor

sftim commented Jul 26, 2021

/approve
/hold
Release docs folks, feel free to unhold

@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 Jul 26, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sftim

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 Jul 26, 2021
@PI-Victor
Copy link
Member

PI-Victor commented Jul 27, 2021

@swetharepakula can you please rebase, a fix to the feature gates table was merged and caused this to conflict.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 27, 2021
 * Graduate proxying gates
 * Endpoints Controller truncates endpoints to 1000 and sets
   over-capacity annotation to `truncated`
@k8s-ci-robot k8s-ci-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jul 27, 2021
@swetharepakula
Copy link
Member Author

PR has been rebased.

Is there anything left for this PR? Or do we still need the hold?

@PI-Victor
Copy link
Member

a tech LGTM from @kubernetes/sig-network-pr-reviews is all that is needed, when this is done we can remove the hold.

@sftim
Copy link
Contributor

sftim commented Jul 27, 2021

We have the tech review I think: #28745 (comment)
/hold cancel

@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 Jul 27, 2021
@sftim
Copy link
Contributor

sftim commented Jul 27, 2021

/lgtm

@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: b344b66b4a425a01e768b284bef3ff202c42039a

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 27, 2021
@k8s-ci-robot k8s-ci-robot merged commit 8518607 into kubernetes:dev-1.22 Jul 27, 2021
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. 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/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants