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

StatefulSet AutoDelete blog post #30596

Merged
merged 1 commit into from
Dec 16, 2021
Merged

Conversation

mattcary
Copy link
Contributor

Blog post for statefulset autodelete.

KEP 1847

Implementation PR #99728

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 22, 2021
@k8s-ci-robot k8s-ci-robot added area/blog Issues or PRs related to the Kubernetes Blog subproject language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Nov 22, 2021
@mattcary mattcary force-pushed the blogpost branch 2 times, most recently from 5a814c2 to 0a7e28a Compare November 22, 2021 17:57
@netlify
Copy link

netlify bot commented Nov 22, 2021

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

🔨 Explore the source changes: 6d4ff36

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

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

@netlify
Copy link

netlify bot commented Nov 22, 2021

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

🔨 Explore the source changes: 1e7731f

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

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

@mattcary mattcary force-pushed the blogpost branch 3 times, most recently from 99a6279 to 9da7226 Compare November 22, 2021 20:43
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.

Nice!

One nit though. Please change all the site links to relative links
If you have a hyperlink that starts https://kubernetes.io/docs/, that should be /docs/

@mattcary
Copy link
Contributor Author

Nice!

One nit though. Please change all the site links to relative links If you have a hyperlink that starts https://kubernetes.io/docs/, that should be /docs/

thx, done.

@mattcary
Copy link
Contributor Author

I checked this with a local server and it seems to look good.

@sftim
Copy link
Contributor

sftim commented Nov 28, 2021

From a docs (and blog team) point of view, this informally LGTM.

@jimangel
Copy link
Member

jimangel commented Dec 5, 2021

/milestone v1.23
/cc @jlbutler

@k8s-ci-robot k8s-ci-robot requested a review from jlbutler December 5, 2021 23:55
@k8s-ci-robot
Copy link
Contributor

@jimangel: The provided milestone is not valid for this repository. Milestones in this repository: [1.18-ja.1, 1.18-ja.2, 1.23, dev-1.22-ko.2, dev-1.22-ko.3]

Use /milestone clear to clear the milestone.

In response to this:

/milestone v1.23
/cc @jlbutler

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/test-infra repository.

@jimangel
Copy link
Member

jimangel commented Dec 5, 2021

Take two:
/milestone 1.23

@sftim
Copy link
Contributor

sftim commented Dec 8, 2021

@karenhchu is the date (2021-12-16) right for this one?

@karenhchu
Copy link
Contributor

@karenhchu is the date (2021-12-16) right for this one?

It's correct. The path also needs to be updated: (content/en/blog/_posts/2021-12-16-StatefulSet-PVC-Auto-Deletion.md)

sftim
sftim previously requested changes Dec 8, 2021
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.

@mattcary could you update the path:
content/en/blog/_posts/2021-12-15-StatefulSet-PVC-Auto-Deletion.md
to match the new publication date?

Another option is that we merge this and then do a fixup PR; if you've got the cycles for it, I'd prefer to have it land with the right path.

PS I've mentioned some nits, but those are optional to fix. Only work on those if you do have time.

Comment on lines 33 to 39
* **whenDeleted and whenScaled are both Retain.** This matches the existing behavior for StatefulSets, where no PVCs are deleted. This is also the default retention policy. It’s appropriate to use when data on StatefulSet volumes may be irreplaceable and should only be deleted manually.

* **whenDeleted is Delete and whenScaled is Retain.** In this case, PVCs are deleted only when the entire StatefulSet is deleted. If the StatefulSet is scaled down, PVCs are not touched, meaning they are available to be reattached if a scale-up occurs with any data from the previous replica. This might be used for a temporary StatefulSet, such as in a CI instance or ETL pipeline, where the data on the StatefulSet is needed only during the lifetime of the StatefulSet lifetime, but while the task is running the data is not easily reconstructible. Any retained state is needed for any replicas that scale down and then up.

* **whenDeleted and whenScaled are both Delete.** PVCs are deleted immediately when their replica is no longer needed. Note this does not include when a Pod is deleted and a new version rescheduled, for example when a node is drained and Pods need to migrate elsewhere. The PVC is deleted only when the replica is no longer needed as signified by a scale-down or StatefulSet deletion. This use case is for when data does not need to live beyond the life of its replica. Perhaps the data is easily reconstructable and the cost savings of deleting unused PVCs is more important than quick scale-up, or perhaps that when a new replica is created, any data from a previous replica is not usable and must be reconstructed anyway.

* **whenDeleted is Retain and whenScaled is Delete.** This is similar to the previous case, when there is little benefit to keeping PVCs for fast reuse during scale-up. An example of a situation where you might use this is an Elasticsearch cluster.. Typically you would scale that workload up and down to match demand, whilst ensuring a minimum number of replicas (for example: 3). When scaling down, data is migrated away from removed replicas and there is no benefit to retaining those PVCs. However, it can be useful to bring the entire Elasticsearch cluster down temporarily for maintenance. If you need to take the Elasticsearch system offline, you can do this by temporarily deleting the StatefulSet, and then bringing the Elasticsearch cluster back by recreating the StatefulSet. The PVCs holding the Elasticsearch data will still exist and the new replicas will automatically use them.
Copy link
Contributor

Choose a reason for hiding this comment

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

Within the bold bits, I'd be tempted to put whenDeleted and whenScale in backticks (like I did there). I think it'd actually be easier to read like that.

Also, a nit (I think it might actually be my typo from a previous suggestion):

this is an Elasticsearch cluster.. Typically

two periods

Copy link
Contributor

Choose a reason for hiding this comment

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

This will also be easier on localization teams if you wrap these at 80 to 100 character length lines, rather than one paragraph per line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

both done.

I also put Retain and Delete in backticks which is consistent what I've done in the text above.

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 8, 2021
@sftim sftim dismissed their stale review December 8, 2021 01:49

Feedback addressed

@sftim
Copy link
Contributor

sftim commented Dec 8, 2021

This could use a technical review from SIG Storage a relevant SIG (I hope: just a formality).
For SIG Docs: /lgtm

@mattcary
Copy link
Contributor Author

mattcary commented Dec 8, 2021

/assign @smarterclayton

@mattcary
Copy link
Contributor Author

mattcary commented Dec 8, 2021

This could use a technical review from SIG Storage a relevant SIG (I hope: just a formality). For SIG Docs: /lgtm

We had sig-apps review the documentation, @smarterclayton from that sig was kind enough to do that and hopefully his graciousness will continue :)

@jlbutler jlbutler removed this from the 1.23 milestone Dec 13, 2021
StatefulSet spec template for cases when they should be deleted automatically when the StatefulSet
is deleted or pods in the StatefulSet are scaled down.

## What problem does this solve?

Choose a reason for hiding this comment

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

should there be a blank line after this title?

Copy link
Contributor

Choose a reason for hiding this comment

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

retaining those PVCs. However, it can be useful to bring the entire Elasticsearch cluster down
temporarily for maintenance. If you need to take the Elasticsearch system offline, you can do
this by temporarily deleting the StatefulSet, and then bringing the Elasticsearch cluster back
by recreating the StatefulSet. The PVCs holding the Elasticsearch data will still exist and the

Choose a reason for hiding this comment

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

"If you need to take the Elasticsearch system offline, you can temporarily delete the StatefulSet, then bring the Elasticsearch cluster back by recreating the StatefulSet." is more concise.

Maybe you're not looking for English editorial, though ☺

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like that better... but given the deadline I think I'll do a post-PR update (see Tim's comment below)

replica. This might be used for a temporary StatefulSet, such as in a CI instance or ETL
pipeline, where the data on the StatefulSet is needed only during the lifetime of the
StatefulSet lifetime, but while the task is running the data is not easily reconstructible. Any
retained state is needed for any replicas that scale down and then up.

Choose a reason for hiding this comment

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

If I Retain an openebs-hostpath PVC on scale-down, how does K8s know to scale-up a new Pod on the same Node as the PVC? My understanding of the creation-time scheduling is that the PVC is created on the Node selected for the Pod, but I'm not clear on how the affinity happens for a PVC that exists before the Pod.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd have to check that... but since Retain is currently existing behavior, a concise answer is "whatever happens right now" :-)

@sftim
Copy link
Contributor

sftim commented Dec 16, 2021

/approve

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 16, 2021
@sftim
Copy link
Contributor

sftim commented Dec 16, 2021

/lgtm

I can't see anything wrong with this article, it's due to publish today, and nobody has raised a serious objection.

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

LGTM label has been added.

Git tree hash: 805c1cfa0f3ab0c94df5cab90efbde51904e3c40

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

deleted.

The first situation is when the StatefulSet resource is deleted (which implies that all replicas are
also deleted). This is controlled by the `whenDeleted` policy. The second situation, controlled by
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
also deleted). This is controlled by the `whenDeleted` policy. The second situation, controlled by
also deleted). This is controlled by the `whenDeleted` policy field. The second situation, controlled by

The first situation is when the StatefulSet resource is deleted (which implies that all replicas are
also deleted). This is controlled by the `whenDeleted` policy. The second situation, controlled by
`whenScaled` is when the StatefulSet is scaled down, which removes some but not all of the replicas
in a StatefulSet. In both cases the policy can either be `Retain`, where the corresponding PVCs are
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
in a StatefulSet. In both cases the policy can either be `Retain`, where the corresponding PVCs are
in a StatefulSet. In both cases the policy value can either be `Retain`, where the corresponding PVCs are

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sftim, 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 merged commit e2e7843 into kubernetes:main Dec 16, 2021
@sftim
Copy link
Contributor

sftim commented Dec 16, 2021

@mattcary if you want to do post-publication tweaks based on #30596 (review) (in a new PR) then please do; if you're happy with it as it is, that's fine also.

@mattcary
Copy link
Contributor Author

@mattcary if you want to do post-publication tweaks based on #30596 (review) (in a new PR) then please do; if you're happy with it as it is, that's fine also.

Thanks Tim. I don't disagree with any of the suggestions, but given bandwidth & review cycles I think I'm happy with the blog post as-is. I appreciate your help with this!

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. area/blog Issues or PRs related to the Kubernetes Blog subproject 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. 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.

9 participants