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

ReadWriteOncePod access mode for PVs and PVCs #102028

Merged

Conversation

chrishenzie
Copy link
Member

@chrishenzie chrishenzie commented May 15, 2021

What type of PR is this?

/kind feature

What this PR does / why we need it:

This PR introduces the ReadWriteOncePod access mode for PersistentVolumes and PersistentVolumeClaims. This access mode restricts volume access to a single pod on a single node. Please see KEP#2485 for more context.

This access mode is enforced in two places:

First is during scheduling (see #103082). When a pod uses a PVC with ReadWriteOncePod, we check for any other consumers of this PVC in the same namespace. If any are found, consider the pod unschedulable and unresolvable since there can only be one consumer. The failing pod will have error:

0/1 nodes are available: 1 node has pod using PersistentVolumeClaim with the same name and ReadWriteOncePod access mode.

Second is during volume mounting (inside of kubelet). When a volume is mounted (or a block device is mapped), we check the actual state of the world cache to check if the provided volume is already mounted to another pod. If it is, fail the mount operation to prevent access from another pod. The failing pod will have error:

MountVolume.SetUp failed for volume "pvc-<ID>" : volume uses the ReadWriteOncePod access mode and is already in use by another pod.

Special notes for your reviewer:

I tried breaking this into individual chunks in each commit. Hopefully it will make for easier review.

Does this PR introduce a user-facing change?

Adds the ReadWriteOncePod access mode for PersistentVolumes and PersistentVolumeClaims. Restricts volume access to a single pod on a single node.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

- [KEP]: https://github.com/kubernetes/enhancements/tree/master/keps/sig-storage/2485-read-write-once-pod-pv-access-mode

/sig storage
/sig scheduling
/assign @msau42
/assign @jingxu97

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. sig/storage Categorizes an issue or PR as relevant to SIG Storage. sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. labels May 15, 2021
@k8s-ci-robot
Copy link
Contributor

@chrishenzie: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels May 15, 2021
@chrishenzie chrishenzie changed the title Read write once pod access mode ReadWriteOncePod access mode for PVs and PVCs May 15, 2021
@k8s-ci-robot k8s-ci-robot added area/dependency Issues or PRs related to dependency changes area/kubectl area/kubelet kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/node Categorizes an issue or PR as relevant to SIG Node. labels May 15, 2021
@fejta-bot
Copy link

This PR may require API review.

If so, when the changes are ready, complete the pre-review checklist and request an API review.

Status of requested reviews is tracked in the API Review project.

@alculquicondor
Copy link
Member

It looks like the KEP didn't go through a sig-scheduling review.

The question that comes to mind, regarding the k8s changes: why can't users who need this use pod anti-affinity along with ReadWriteOnce?

@ehashman ehashman added this to Triage in SIG Node PR Triage May 17, 2021
@chrishenzie
Copy link
Member Author

@alculquicondor Can we retroactively do that? Any feedback is appreciated.

Users may be able to configure pod anti-affinity rules to produce similar behavior, but I feel like that's working around a lack of feature in Kubernetes that should be implemented. The issue is there is a lack of an access mode that guarantees single writer access (on a single node) to a PersistentVolume. From my understanding, most users assume this is what ReadWriteOnce does, but by definition it doesn't. By adding a new access mode to handle this case, we provide opportunities for users to express with clearer intent how they want their PersistentVolumes to be consumed.

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 28, 2021
@ahg-g
Copy link
Member

ahg-g commented Jun 28, 2021

I'm not a hug fan of the naming (no, I don't have a better one), and I am a bit surprised scheduling wants to take this on, but OK

/approve

With the new scheduling framework, it is fairly easy to add such functionality as a plugin.

@chrishenzie chrishenzie force-pushed the read-write-once-pod-access-mode branch from c0fad4d to 99fc36e Compare June 28, 2021 22:52
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 28, 2021
@chrishenzie
Copy link
Member Author

/retest

1 similar comment
@chrishenzie
Copy link
Member Author

/retest

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 29, 2021
These options provide an extensible way of configuring how PVs are
validated
These options provide an extensible way of configuring how PVCs are
validated
So it is consistent with other methods performing the same check (one
for internal and external types)
Will be used during validation of PVs and PVCs
This will only work if the "ReadWriteOncePod" feature gate is enabled.
Additionally, this access mode will only work when used by itself. This
is because when ReadWriteOncePod is used on a PV or PVC, it renders all
other access modes useless since it is most restrictive.
@chrishenzie chrishenzie force-pushed the read-write-once-pod-access-mode branch from 99fc36e to b7d732d Compare June 29, 2021 04:26
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 29, 2021
@chrishenzie
Copy link
Member Author

@jsafrane for lgtm

@jsafrane
Copy link
Member

/lgtm
/approve
for storage side

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chrishenzie, jsafrane, thockin

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 01819dd into kubernetes:master Jun 29, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.22 milestone Jun 29, 2021
@chrishenzie chrishenzie deleted the read-write-once-pod-access-mode branch June 29, 2021 17:08
@liggitt liggitt moved this from Assigned to API review completed, 1.22 in API Reviews Aug 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-review Categorizes an issue or PR as actively needing an API review. approved Indicates a PR has been approved by an approver from all required OWNERS files. area/dependency Issues or PRs related to dependency changes area/kubectl area/kubelet cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. sig/storage Categorizes an issue or PR as relevant to SIG Storage. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
Status: API review completed, 1.22
Development

Successfully merging this pull request may close these issues.

None yet

10 participants