-
Notifications
You must be signed in to change notification settings - Fork 14.5k
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 for minReadySeconds in StatefulSet #27683
Add docs for minReadySeconds in StatefulSet #27683
Conversation
Deploy preview for kubernetes-io-master-staging ready! Built with commit ee7bd56 https://deploy-preview-27683--kubernetes-io-master-staging.netlify.app |
Hi @ravisantoshgudimetla . |
Yes @PI-Victor, updated the branch. Thank you. |
/assign @chrisnegus |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ee7bd56
to
8eafd71
Compare
👷 Deploy Preview for kubernetes-io-vnext-staging processing. 🔨 Explore the source changes: 2dff666 🔍 Inspect the deploy log: https://app.netlify.com/sites/kubernetes-io-vnext-staging/deploys/60f9757d7dcd870007d59f9c |
8eafd71
to
8d6019d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
LGTM label has been added. Git tree hash: 4e07138c98b4e1a2cdd45e6c645bd90f01a8d91a
|
@@ -228,6 +228,13 @@ and Ready or completely terminated prior to launching or terminating another | |||
Pod. This option only affects the behavior for scaling operations. Updates are not | |||
affected. | |||
|
|||
#### Min Ready Seconds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest we spell this as "Minimum ready seconds".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. PTAL
8d6019d
to
0159ef9
Compare
61b7599
to
80b3683
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
LGTM label has been added. Git tree hash: 50e90d95813a363b9c3fae38e1df69889601c7a4
|
/milestone 1.22 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we also need to update https://kubernetes.io/docs/reference/command-line-tools-reference/feature-gates/ to mention StatefulSetMinReadySeconds
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this behind the StatefulSetMinReadySeconds
feature gate? If so, let's also mention that in the StatefulSet concept page, and update https://kubernetes.io/docs/reference/command-line-tools-reference/feature-gates/
I'm 1.22 release docs shadow for this PR. It looks like the changes for this feature to the dev-1.22 feature-gates page have already been merged. This PR seems to cover the StatefulSet concept page you asked about. @sftim is there more that you think needs to be done or is this ready to be merged? |
based on Chris' comment i'd like to merge this, anything to else needs to be added? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this feedback is right - not sure. If I'm wrong please comment and then dismiss this review.
#### Minimum Ready Seconds | ||
|
||
{{< feature-state for_k8s_version="v1.22" state="alpha" >}} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's explain that this field only works if you enable the StatefulSetMinReadySeconds
feature gate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I added it here. PTAL.
80b3683
to
9cc7f3c
Compare
This defaults to 0 (the Pod will be considered available as soon as it is ready). To learn more about when | ||
a Pod is considered ready, see [Container Probes](/docs/concepts/workloads/pods/pod-lifecycle/#container-probes). | ||
|
||
Please note that this field only works if you enable the `StatefulSetMinReadySeconds` featuregate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please note that this field only works if you enable the `StatefulSetMinReadySeconds` featuregate | |
Please note that this field only works if you enable the `StatefulSetMinReadySeconds` feature gate. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. PTAL
9cc7f3c
to
2dff666
Compare
All feedbacks addressed. Good to go. |
LGTM label has been added. Git tree hash: e54afbee4344820f21013487217732c57aaf0bbe
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: soltysh, tengqm 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 |
Add docs from minReadySeconds.
xref: kubernetes/enhancements#2607