[scheduler] Support preemption of pods using ReadWriteOncePod PVCs #114051

chrishenzie · 2022-11-21T20:38:07Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

Adds support for preemption of pods using ReadWriteOncePod PVCs. This is a required feature for beta graduation.

Which issue(s) this PR fixes:

Does this PR introduce a user-facing change?

Adds scheduler preemption support for pods using `ReadWriteOncePod` PVCs

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

Testing

This feature can be tested the following ways:

# Unit tests.
go test ./pkg/scheduler/framework/... ./pkg/scheduler/internal/...

# Integration tests.
go test -v -run TestUnschedulablePodBecomesSchedulable/scheduled_pod_uses_read-write-once-pod_pvc ./test/integration/scheduler/filters
go test -v ./test/integration/scheduler/preemption/ -run TestReadWriteOncePodPreemption

# And E2E tests.
make WHAT=test/e2e/e2e.test
_output/local/bin/linux/amd64/e2e.test --kubeconfig=${HOME}/.kube/config -ginkgo.focus='Feature:ReadWriteOncePod'

/sig storage
/sig scheduling
/cc @msau42
/cc @jsafrane
/cc @alculquicondor

k8s-ci-robot · 2022-11-21T20:38:11Z

Please note that we're already in Test Freeze for the release-1.26 branch. This means every merged PR will be automatically fast-forwarded via the periodic ci-fast-forward job to the release branch of the upcoming v1.26.0 release.

Fast forwards are scheduled to happen every 6 hours, whereas the most recent run was: Mon Nov 21 15:27:42 UTC 2022.

chrishenzie · 2022-11-21T20:52:54Z

Thinking out loud -- building the conflictingPods map in cycle state requires scanning all pods for conflicts which is O(N) where N is # pods.

We may be able to do better than this by repurposing the PVCRefCounts cache to be a mapping of PVC name to pod using the PVC, call it PVCRefs. Then when we calculate the cycle state we can build conflictingPods by searching PVCRefs with the name of the PVCs using ReadWriteOncePod. This solution would be O(M) where M is the number of ReadWriteOncePod volumes used by the PVC, which is most likely < N.

alculquicondor

I didn't review tests yet.

pkg/scheduler/framework/plugins/volumerestrictions/volume_restrictions.go

pkg/scheduler/framework/types.go

chrishenzie · 2022-11-22T23:16:46Z

I restructured the PR to go with an approach like #114051 (comment). Please see the commit descriptions for each one for more context.

At a high-level, I've changed the caches to store references to the specific pods using each PVC. In VolumeRestrictions, we query this cache with the pod-to-be-scheduled's ReadWriteOncePod PVCs to build a cache of conflicting pods.

chrishenzie · 2022-11-23T03:48:03Z

/milestone v1.27

k8s-ci-robot · 2022-11-23T03:48:05Z

@chrishenzie: You must be a member of the kubernetes/milestone-maintainers GitHub team to set the milestone. If you believe you should be able to issue the /milestone command, please contact your Milestone Maintainers Team and have them propose you as an additional delegate for this responsibility.

In response to this:

/milestone 1.27

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.

pkg/scheduler/framework/plugins/volumerestrictions/volume_restrictions.go

pkg/scheduler/framework/types.go

test/e2e/storage/testsuites/readwriteoncepod.go

chrishenzie · 2022-11-23T20:41:34Z

This doesn't appear to be working (preempting lower priority pods) in a multi-node environment, but does for single-node. Looking into this.

kerthcet

Looks like this is ready to merge now, anything left? @alculquicondor

pkg/scheduler/framework/plugins/volumerestrictions/volume_restrictions.go

pkg/scheduler/framework/plugins/volumerestrictions/volume_restrictions_test.go

PVCs using the ReadWriteOncePod access mode can only be referenced by a single pod. When a pod is scheduled that uses a ReadWriteOncePod PVC, return "Unschedulable" if the PVC is already in-use in the cluster. To support preemption, the "VolumeRestrictions" scheduler plugin computes cycle state during the PreFilter phase. This cycle state contains the number of references to the ReadWriteOncePod PVCs used by the pod-to-be-scheduled. During scheduler simulation (AddPod and RemovePod), we add and remove reference counts from the cycle state if they use any of these ReadWriteOncePod PVCs. In the Filter phase, the scheduler checks if there are any PVC reference conflicts, and returns "Unschedulable" if there is a conflict. This is a required feature for the ReadWriteOncePod beta. See for more context: https://github.com/kubernetes/enhancements/tree/master/keps/sig-storage/2485-read-write-once-pod-pv-access-mode#beta

alculquicondor · 2023-01-31T15:33:51Z

pkg/scheduler/framework/plugins/volumerestrictions/volume_restrictions_test.go

+				t.Errorf("Unexpected PreFilter status (-want, +got): %s", diff)
+			}
+			// If PreFilter fails, then Filter will not run.
+			if test.preFilterWantStatus.IsSuccess() {


FYI @kidddddddddddddddddddddd regarding #114898

alculquicondor

/approve
/hold cancel
/lgtm

k8s-ci-robot · 2023-01-31T15:35:16Z

LGTM label has been added.

Git tree hash: f308a56055752323b5ae6ae2fe129430c4da8eaa

alculquicondor · 2023-01-31T15:36:27Z

/assign @aojea
for test/

chrishenzie · 2023-02-02T22:48:23Z

PRR approved, this is ready for merge. See #114494 for e2e test changes and feature gate bump to beta.

kerthcet · 2023-02-03T05:14:27Z

/lgtm
ping @aojea for tests. We have other PR blocked by this one.

aojea · 2023-02-13T18:38:11Z

/lgtm ping @aojea for tests. We have other PR blocked by this one.

/approve

k8s-ci-robot · 2023-02-13T18:38:39Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alculquicondor, aojea, chrishenzie

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:

~~pkg/scheduler/OWNERS~~ [alculquicondor]
~~test/OWNERS~~ [aojea]

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 requested review from alculquicondor and jsafrane November 21, 2022 20:38

k8s-ci-robot added the release-note-none Denotes a PR that doesn't merit a release note. label Nov 21, 2022

k8s-ci-robot requested a review from msau42 November 21, 2022 20:38

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. 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. labels Nov 21, 2022

chrishenzie changed the title ~~Rwop preemption~~ [scheduler] Support preemption of pods using ReadWriteOncePod PVCs Nov 21, 2022

k8s-ci-robot added area/test sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Nov 21, 2022

chrishenzie force-pushed the rwop-preemption branch 3 times, most recently from c431e7b to 9d97867 Compare November 22, 2022 00:25

alculquicondor reviewed Nov 22, 2022

View reviewed changes

chrishenzie force-pushed the rwop-preemption branch from 9d97867 to 31a68ac Compare November 22, 2022 23:11

chrishenzie force-pushed the rwop-preemption branch from 31a68ac to 78ca250 Compare November 22, 2022 23:18

chrishenzie mentioned this pull request Nov 22, 2022

ReadWriteOncePod PersistentVolume Access Mode kubernetes/enhancements#2485

Closed

12 tasks

kerthcet reviewed Nov 23, 2022

View reviewed changes

chrishenzie force-pushed the rwop-preemption branch 2 times, most recently from cf95111 to bac3076 Compare November 23, 2022 20:06

chrishenzie force-pushed the rwop-preemption branch 2 times, most recently from 4b58409 to 1c80d58 Compare December 13, 2022 19:10

This was referenced Dec 14, 2022

[v1.27] Graduate the ReadWriteOncePod feature to beta #114493

Closed

Graduate ReadWriteOncePod to beta, updated e2e test #114494

Merged

chrishenzie force-pushed the rwop-preemption branch 2 times, most recently from a4a0caa to 5ff76a0 Compare December 15, 2022 00:50

kerthcet reviewed Jan 9, 2023

View reviewed changes

pkg/scheduler/framework/plugins/volumerestrictions/volume_restrictions.go Outdated Show resolved Hide resolved

kerthcet mentioned this pull request Jan 9, 2023

feature(volume_restrictions): return Skip in PreFilter #114898

Merged

alculquicondor reviewed Jan 10, 2023

View reviewed changes

pkg/scheduler/framework/plugins/volumerestrictions/volume_restrictions_test.go Outdated Show resolved Hide resolved

chrishenzie force-pushed the rwop-preemption branch from 5ff76a0 to 167da36 Compare January 30, 2023 18:58

chrishenzie force-pushed the rwop-preemption branch from 167da36 to dbc7d8d Compare January 30, 2023 19:00

chrishenzie mentioned this pull request Jan 30, 2023

Enforce ReadWriteOncePod PVC access mode during scheduling #103132

Closed

alculquicondor reviewed Jan 31, 2023

View reviewed changes

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 31, 2023

k8s-ci-robot assigned alculquicondor Jan 31, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 31, 2023

k8s-ci-robot assigned aojea Jan 31, 2023

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 13, 2023

k8s-ci-robot merged commit b8b18ec into kubernetes:master Feb 13, 2023

k8s-ci-robot added this to the v1.27 milestone Feb 13, 2023

chrishenzie deleted the rwop-preemption branch March 21, 2023 00:12

kerthcet mentioned this pull request Jul 26, 2023

Enforce ReadWriteOnce PVC access mode during scheduling #103305

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[scheduler] Support preemption of pods using ReadWriteOncePod PVCs #114051

[scheduler] Support preemption of pods using ReadWriteOncePod PVCs #114051

chrishenzie commented Nov 21, 2022 •

edited

Loading

k8s-ci-robot commented Nov 21, 2022

chrishenzie commented Nov 21, 2022

alculquicondor left a comment

chrishenzie commented Nov 22, 2022

chrishenzie commented Nov 23, 2022 •

edited

Loading

k8s-ci-robot commented Nov 23, 2022

chrishenzie commented Nov 23, 2022

kerthcet left a comment •

edited

Loading

alculquicondor Jan 31, 2023

alculquicondor left a comment

k8s-ci-robot commented Jan 31, 2023

alculquicondor commented Jan 31, 2023

chrishenzie commented Feb 2, 2023

kerthcet commented Feb 3, 2023

aojea commented Feb 13, 2023

k8s-ci-robot commented Feb 13, 2023

[scheduler] Support preemption of pods using ReadWriteOncePod PVCs #114051

[scheduler] Support preemption of pods using ReadWriteOncePod PVCs #114051

Conversation

chrishenzie commented Nov 21, 2022 • edited Loading

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Does this PR introduce a user-facing change?

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

Testing

k8s-ci-robot commented Nov 21, 2022

chrishenzie commented Nov 21, 2022

alculquicondor left a comment

Choose a reason for hiding this comment

chrishenzie commented Nov 22, 2022

chrishenzie commented Nov 23, 2022 • edited Loading

k8s-ci-robot commented Nov 23, 2022

chrishenzie commented Nov 23, 2022

kerthcet left a comment • edited Loading

Choose a reason for hiding this comment

alculquicondor Jan 31, 2023

Choose a reason for hiding this comment

alculquicondor left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Jan 31, 2023

alculquicondor commented Jan 31, 2023

chrishenzie commented Feb 2, 2023

kerthcet commented Feb 3, 2023

aojea commented Feb 13, 2023

k8s-ci-robot commented Feb 13, 2023

chrishenzie commented Nov 21, 2022 •

edited

Loading

chrishenzie commented Nov 23, 2022 •

edited

Loading

kerthcet left a comment •

edited

Loading