StorageObjectInUseProtection (was Persistent Volume Protection)

[pospispa]

Feature Description

• One-line feature description (can be used as a release note): Prevent deletion of Storage Object that is In active Use (was Prevent deletion of Persistent Volume that is bound to a Persistent Volume Claim)
• Primary contact (assignee): Pavel Pospisil (@pospispa)
• Responsible SIGs: @kubernetes/sig-storage-feature-requests
• Design proposal link (community repo):
• Reviewer(s) - (for LGTM) recommend having 2+ reviewers (at least one from code-area OWNERS file) agreed to review. Reviewers from multiple companies preferred: @jsafrane @msau42
• Approver (likely from SIG/area to which feature belongs): @jsafrane
• Feature target (which target equals to which milestone):
  • Alpha release target (x.y): 1.10 skipped
  • Beta release target (x.y): 1.10
  • Stable release target (x.y): 1.11

[pospispa]

/assign

[pospispa]

This feature aims to fix the bug: kubernetes/kubernetes#33355

[saad-ali]

This did not make it in to 1.9, punting to next release

[NickrenREN]

/assign

[idvoretskyi]

@saad-ali @pospispa @kubernetes/sig-storage-feature-requests still on track for 1.10?

[pospispa]

@NickrenREN has taken over development of this feature. @NickrenREN will you, please, comment on the status of the development of this feature.

[NickrenREN]

Yeah, on track for 1.10. I am writing the proposal . will send it out tomorrow.

[jsafrane]

added 1.10 milestone

[Bradamant3]

@NickrenREN do we need additional doc for this feature? If so, please submit PR as soon as possible. Either way, please update the 1.10 feature tracking spreadsheet
Thanks!

[msau42]

The PR is here, but I think needs to be updated to use the new name of the admission controller:
https://github.com/kubernetes/website/pull/7165/files

[NickrenREN]

@msau42 admission controller name is being updated here: https://github.com/kubernetes/website/pull/7576/files
@Bradamant3 i am writing PV protection description, will send it out soon

[NickrenREN]

PR: kubernetes/website#7620

[saad-ali]

Correct me if I am wrong but this feature skipped alpha and went directly to beta because it piggy backed on the work done for #498.

Changing labels to reflect that.

[AishSundar]

@pospispa I am the CI Signal lead for 1.11 and also work on Conformance testing program for K8s. I see this feature is going to Stable in 1.11.
I assume you already have e2e tests for it. I am following up to see if and which of those tests should we promote to the conformance suite in 1.11
As part of the process to increase conformance coverage, outlined by Conformance WG and Sig-Arch, we expect features going into stable/GA to have representation in Conformance suite. Your update on the same will help us evaluate this feature better.

[pospispa]

@AishSundar there're 2 E2E tests for the Storage Object In Use Protection feature:

• https://github.com/kubernetes/kubernetes/blob/master/test/e2e/storage/pv_protection.go
• https://github.com/kubernetes/kubernetes/blob/master/test/e2e/storage/pvc_protection.go

IMHO, both E2E tests should be added to Conformance suite.

Please, what am I supposed to do now?

[AishSundar]

@pospispa if you/owning SIG determine that these tests should be part of Conformance (for base profile) then the next step is to send a PR to promote the test to Conformance suite. Here's a sample PR

• Please use "Promote xxx e2e test to Conformance" as template of your PR title
• Tag your PR with "/area conformance" label
• Send your PR to Sig-Architecture for review by adding "@kubernetes/sig-architecture-pr-reviews"
• CC your Sig, Sig-Architecture and me in your PR

Lastly Code freeze for v1.11 is 6/5, so we should ideally aim for the PR to be merged by then.

cc @jagosan @mithrav as FYI
cc @jberkus @tpepper we should look to make sure this PR makes it in before code freeze.

[AishSundar]

cc @timothysc as FYI

[msau42]

Are PVCs and dynamic provisioning currently part of conformance?

[AishSundar]

@msau42 I dont see them in current Conformance suite - https://k8s-testgrid.appspot.com/conformance-all#GCE,%20master%20(dev)

[AishSundar]

@pospispa I had a chance to talk to 1.11 RT about this PR. Considering we will be entering CodeFreeze next Tuesday, 6/5, we should ideally aim for the promotion PR to be reviewed and merge by EOD Thursday 5/31 for this to make into 1.11. We will then have time for a couple of test runs before code freeze and branch cut. Please let me know if you think this can be wrapped by Thursday. Thanks.

[msau42]

I spoke with @saad-ali briefly about adding this to the conformance suite. We're not sure dynamic provisioning in general can be supported in conformance because it requires vendor-specific volume plugins to work.

[AishSundar]

@bgrant0607 for some guidance on adding this scenario to Conformance suite.

[AishSundar]

cc @bgrant0607

[bgrant0607]

There are multiple issues:

1. Dynamic provisioning is higher-level automation, such as cluster autoscaling, and is optional.
2. All NAS volume sources are optional, provider-specific, and non-portable.
3. We don't have any PV, PVC, or StorageClass conformance tests currently.

We need to finalize the notion of profiles before we can add optional and/or non-portable functionality to the suite.

The way I'd like to craft the profile(s) is based on the abstract functionality provided, such as by the default/standard StorageClass(es), rather than by provider-specific functionality/types. For example, "PVCs with shared-read volumes" could be something we add to a conformance profile.

It would be helpful for the Storage SIG to draft a proposal for what functionality could be expected to be reasonably portable and widely supported that we could shape into a profile or add to a profile (e.g., a cloud-provider profile).

cc @kubernetes/sig-architecture-api-reviews @kubernetes/sig-storage-api-reviews

[AishSundar]

Thanks @bgrant0607.

@msau42 @pospispa looks like this warrants a more involved discussion and might not be in scope for 1.11 timeframe(Code freeze is 6/5). Correct me if you think otherwise.

I will schedule a followup meeting with @msau42 @saad-ali @pospispa @WilliamDenniss to explore profiles in context of Sig Storage.

[AishSundar]

@pospispa I am trying to add you to a meeting, but am unable to find your contact in Slack. Please provide me with an email id to add to the sync.

[pospispa]

@msau42 @pospispa looks like this warrants a more involved discussion and might not be in scope for 1.11 timeframe(Code freeze is 6/5). Correct me if you think otherwise.

My understanding of the comment is same. As it requires several prerequisites described in comment IMHO, it's too late for the 1.11 timeframe.

@AishSundar my email is pospispa@gmail.com and I live in Central European timezone.

[bgrant0607]

I'm also not so keen to add a mechanism that's only applicable to storage resources.

What mechanism is this intending to use? Finalizers?

[pospispa]

@bgrant0607 the StorageObjectInUseProtection feature prevents PV removal from the system in case it is bound to a PVC. Or the feature prevents PVC removal in case it's actively used by a pod. The StorageObjectInUseProtection feature uses finalizers to prevent the removal.

Current StorageObjectInUseProtection E2E tests use a StorageClass to create PV <-> PVC pairs because of ease of use. However, usage of the StorageClass is not necessary. The E2E tests also create a pod.

@bgrant0607 do you currently need any additional information about the StorageObjectInUseProtection feature?

[bgrant0607]

@pospispa Prevents how?

[msau42]

It prevents removal by adding a finalizer to the PV if it is bound to a PVC, and/or to the PVC if it's in use by a Pod

[pospispa]

I'd like to add that there's a K8s server API feature that prevents removal of objects whose list of finalizers is not empty. In case such an object is deleted by a user the object with non-empty finalizers list is not removed but it's deletion timestamp is set. As soon as the object's finalizers list becomes empty the object is removed from the system.

[bgrant0607]

Ok, thanks.

I see that here:
https://github.com/kubernetes/kubernetes/blob/master/plugin/pkg/admission/storage/storageobjectinuseprotection/admission.go#L113

[kacole2]

@pospispa This feature was worked on in the previous milestone, so we'd like to check in and see if there are any plans for this to graduate stages in Kubernetes 1.12. This still has the 1.11 milestone as well so we need to update it accordingly.

Can you please update the feature milestone tracking in the original post as well?
If there are any updates, please explicitly ping @justaugustus, @kacole2, @robertsandoval, @rajendar38 to note that it is ready to be included in the Features Tracking Spreadsheet for Kubernetes 1.12.

---

Please note that the Features Freeze is July 31st, after which any incomplete Feature issues will require an Exception request to be accepted into the milestone.

In addition, please be aware of the following relevant deadlines:

• Docs deadline (open placeholder PRs): 8/21
• Test case freeze: 8/28

Please make sure all PRs for features have relevant release notes included as well.

Happy shipping!

[msau42]

This feature went GA in 1.11. I think this can be closed?

[justaugustus]

Good catch, @msau42!
/close