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

[docs]: Promote STS minReadySeconds to beta #30435

Merged
merged 2 commits into from Nov 29, 2021
Merged

[docs]: Promote STS minReadySeconds to beta #30435

merged 2 commits into from Nov 29, 2021

Conversation

ravisantoshgudimetla
Copy link
Contributor

@k8s-ci-robot k8s-ci-robot added this to the 1.23 milestone Nov 10, 2021
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 10, 2021
@netlify
Copy link

netlify bot commented Nov 10, 2021 •
edited

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

🔨 Explore the source changes: 9ea79fa

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

@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Nov 10, 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 Nov 10, 2021
soltysh
soltysh approved these changes Nov 10, 2021
View reviewed changes
Copy link
Contributor

@soltysh soltysh left a comment

Choose a reason for hiding this comment

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

One nit, rest lgtm

content/en/docs/concepts/workloads/controllers/statefulset.md Outdated
@@ -286,7 +286,7 @@ StatefulSet will then begin to recreate the Pods using the reverted template.

### Minimum ready seconds

{{< feature-state for_k8s_version="v1.22" state="alpha" >}}
{{< feature-state for_k8s_version="v1.23" state="beta" >}}

`.spec.minReadySeconds` is an optional field that specifies the minimum number of seconds for which a newly
Copy link
Contributor

Choose a reason for hiding this comment

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

It's worth mentioning that starting from 1.23 it's on by default

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 reword line 296. maybe bring it right under this shortcode:

Please note that this field only works if you enable the StatefulSetMinReadySeconds feature gate.

@jlbutler
Copy link
Contributor

/assign @chrisnegus

@chrisnegus
Copy link
Contributor

/lgtm

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

LGTM label has been added.

Git tree hash: 24386685c768fb544450e3563843eef44fdd6b1d

RinkiyaKeDad
RinkiyaKeDad reviewed Nov 15, 2021
View reviewed changes
Copy link
Member

@RinkiyaKeDad RinkiyaKeDad left a comment

Choose a reason for hiding this comment

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

Looks good from SIG Docs side too, +1 to Maciej's suggestion!

soltysh
soltysh approved these changes Nov 16, 2021
View reviewed changes
Copy link
Contributor

@soltysh soltysh left a comment

Choose a reason for hiding this comment

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

/lgtm

sftim
sftim previously requested changes Nov 16, 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.

Thanks for this. I recommend a more thorough tidying in connection with the promotion past alpha.

Now that this is beta, we should move this to be a level-3 heading under “Components” (see line 51), and include it in the example.

We should also clarify that the default zero value is honored by default and that you have to opt-out if you want the old behavior.

content/en/docs/concepts/workloads/controllers/statefulset.md Outdated
@@ -286,7 +286,7 @@ StatefulSet will then begin to recreate the Pods using the reverted template.

### Minimum ready seconds

{{< feature-state for_k8s_version="v1.22" state="alpha" >}}
{{< feature-state for_k8s_version="v1.23" state="beta" >}}

`.spec.minReadySeconds` is an optional field that specifies the minimum number of seconds for which a newly
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 reword line 296. maybe bring it right under this shortcode:

Please note that this field only works if you enable the StatefulSetMinReadySeconds feature gate.

@sftim
Copy link
Contributor

sftim commented Nov 16, 2021

@ravisantoshgudimetla: if you agree with my suggested changes, but don't have time yet, please /lgtm cancel and mark this PR as WIP to record your intent.

@soltysh
Copy link
Contributor

soltysh commented Nov 18, 2021

@ravisantoshgudimetla: if you agree with my suggested changes, but don't have time yet, please /lgtm cancel and mark this PR as WIP to record your intent.

@sftim he's out this week, he'll respond next when he's back

@ravisantoshgudimetla
Copy link
Contributor Author

/hold

to address @sftim's comments.

@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. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 22, 2021
@ravisantoshgudimetla
Copy link
Contributor Author

Now that this is beta, we should move this to be a level-3 heading under “Components” (see line 51), and include it in the example.

Isn't it already level3?

@sftim
Copy link
Contributor

sftim commented Nov 22, 2021

It's already level 3, but part of the section headed “Rolling Updates”

@ravisantoshgudimetla
Copy link
Contributor Author

So would level 3 under components section makes sense?

@sftim
Copy link
Contributor

sftim commented Nov 22, 2021

So would level 3 under components section makes sense?

I'm asserting that it would. The minReadySeconds field is a component of the specification for a StatefulSet.

@ravisantoshgudimetla
Copy link
Contributor Author

I'm asserting that it would. The minReadySeconds field is a component of the specification for a StatefulSet.

And I agree with you. I was asking just to make sure that we're on same page.

@sftim sftim dismissed their stale review November 25, 2021 00:17

Review was indeed stale

@sftim
Copy link
Contributor

sftim commented Nov 25, 2021

/hold cancel

This is ready for further review / further rework.

@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 Nov 25, 2021
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 29, 2021
@sftim
Copy link
Contributor

sftim commented Nov 29, 2021

/sig apps

@kubernetes/sig-apps-pr-reviews is this revised documentation technically accurate?

@k8s-ci-robot k8s-ci-robot added the sig/apps Categorizes an issue or PR as relevant to SIG Apps. label Nov 29, 2021
soltysh
soltysh approved these changes Nov 29, 2021
View reviewed changes
Copy link
Contributor

@soltysh soltysh left a comment

Choose a reason for hiding this comment

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

/lgtm

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

LGTM label has been added.

Git tree hash: 92eede88316b7645bde6d0ca9adf8ef213f53873

@soltysh
Copy link
Contributor

soltysh commented Nov 29, 2021

@kubernetes/sig-apps-pr-reviews is this revised documentation technically accurate?

Yes, it is :)

@sftim
Copy link
Contributor

sftim commented Nov 29, 2021

LGTM for SIG Docs too

@jlbutler
Copy link
Contributor

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jlbutler, soltysh

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 Nov 29, 2021
@k8s-ci-robot k8s-ci-robot merged commit 3aa8d6b into kubernetes:dev-1.23 Nov 29, 2021
@soltysh soltysh mentioned this pull request Aug 2, 2022
Closed
12 tasks
gmbnomis added a commit to gmbnomis/charts that referenced this pull request Mar 9, 2023
@gmbnomis
[bitnami/redis] minReadySeconds feature only requires k8s >=1.23
e933a6a 
The `minReadySeconds` Helm values have a kubernetes version constraint
`>=1.25` since MR bitnami#13783. While `minReadySeconds` for `StatefulSet` is stable
since 1.25, it has been in beta state since 1.23 (see
kubernetes/website#30435), which means that
it is available on k8s >=1.23 by default unless specifically disabled.

Change the version constraint to `>=1.23` to make the feature available
on older k8s clusters.

Signed-off-by: Simon Baatz <gmbnomis@gmail.com>
gmbnomis added a commit to gmbnomis/charts that referenced this pull request Mar 9, 2023
@gmbnomis
[bitnami/redis] minReadySeconds feature only requires k8s >=1.23
9211358 
The `minReadySeconds` Helm values have a kubernetes version constraint
`>=1.25` since MR bitnami#13783. While `minReadySeconds` for `StatefulSet` is stable
since 1.25, it has been in beta state since 1.23 (see
kubernetes/website#30435), which means that
it is available on k8s >=1.23 by default unless specifically disabled.

Change the version constraint to `>=1.23-0` to make the feature available
on older k8s clusters. (The `-0` includes pre-releases and follows
the general pattern used in the Bitnami Helm charts)

Signed-off-by: Simon Baatz <gmbnomis@gmail.com>
@gmbnomis gmbnomis mentioned this pull request Mar 9, 2023
Merged
3 tasks
jotamartos pushed a commit to bitnami/charts that referenced this pull request Mar 10, 2023
@gmbnomis
[bitnami/redis] minReadySeconds feature only requires k8s >=1.23 (#15417
a748281 
)

The `minReadySeconds` Helm values have a kubernetes version constraint
`>=1.25` since MR #13783. While `minReadySeconds` for `StatefulSet` is stable
since 1.25, it has been in beta state since 1.23 (see
kubernetes/website#30435), which means that
it is available on k8s >=1.23 by default unless specifically disabled.

Change the version constraint to `>=1.23-0` to make the feature available
on older k8s clusters. (The `-0` includes pre-releases and follows
the general pattern used in the Bitnami Helm charts)

Signed-off-by: Simon Baatz <gmbnomis@gmail.com>
uncycler pushed a commit to uncycler/charts-1 that referenced this pull request Mar 14, 2023
@gmbnomis @uncycler
[bitnami/redis] minReadySeconds feature only requires k8s >=1.23 (bit…
d9b8f20 
…nami#15417)

The `minReadySeconds` Helm values have a kubernetes version constraint
`>=1.25` since MR bitnami#13783. While `minReadySeconds` for `StatefulSet` is stable
since 1.25, it has been in beta state since 1.23 (see
kubernetes/website#30435), which means that
it is available on k8s >=1.23 by default unless specifically disabled.

Change the version constraint to `>=1.23-0` to make the feature available
on older k8s clusters. (The `-0` includes pre-releases and follows
the general pattern used in the Bitnami Helm charts)

Signed-off-by: Simon Baatz <gmbnomis@gmail.com>
Signed-off-by: Frédéric Marchand <frederic.marchand@kudelskisecurity.com>
aoterolorenzo pushed a commit to bitnami/charts that referenced this pull request Mar 23, 2023
@gmbnomis @aoterolorenzo
[bitnami/redis] minReadySeconds feature only requires k8s >=1.23 (#15417
a12bca7 
)

The `minReadySeconds` Helm values have a kubernetes version constraint
`>=1.25` since MR #13783. While `minReadySeconds` for `StatefulSet` is stable
since 1.25, it has been in beta state since 1.23 (see
kubernetes/website#30435), which means that
it is available on k8s >=1.23 by default unless specifically disabled.

Change the version constraint to `>=1.23-0` to make the feature available
on older k8s clusters. (The `-0` includes pre-releases and follows
the general pattern used in the Bitnami Helm charts)

Signed-off-by: Simon Baatz <gmbnomis@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sftim sftim sftim left review comments

@RinkiyaKeDad RinkiyaKeDad RinkiyaKeDad left review comments

@soltysh soltysh soltysh approved these changes

@celestehorgan celestehorgan Awaiting requested review from celestehorgan

@smarterclayton smarterclayton Awaiting requested review from smarterclayton

Assignees

@soltysh soltysh

@chrisnegus chrisnegus

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/apps Categorizes an issue or PR as relevant to SIG Apps. 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.
Projects
None yet
Milestone
1.23
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@ravisantoshgudimetla @jlbutler @chrisnegus @k8s-ci-robot @sftim @soltysh @RinkiyaKeDad