Enable PersistentVolumeLabel admission plugin #9822

ncdc · 2016-07-13T17:12:44Z

Enable PersistentVolumeLabel admission plugin. This labels AWS and GCE volumes with their zone, so the scheduler can limit the nodes for a pod to only those in the same zone as the peristent volumes being used by the pod.

Fixes bug 1356010

@smarterclayton @deads2k @liggitt

liggitt · 2016-07-13T17:13:49Z

pkg/cmd/server/origin/master_config.go

@@ -318,6 +318,7 @@ var (
 		"SCCExecRestrictions",
 		quotaadmission.PluginName,
 		"ClusterResourceQuota",
+		"PersistentVolumeLabel",


quota needs to go last

Seems like a comment is in order for that so the next poor soul doesn't do this too

Seems like a comment is in order for that so the next poor soul doesn't do this too

Probably test worthy. There's already some tests for consistency.

deads2k · 2016-07-13T17:20:43Z

Seems like this is something that deserves a test. Is there one you can enable from upstream in our conformance suite?

smarterclayton · 2016-07-13T17:21:44Z

I think those rely on cloud providers. Until Steve turns on the "deploy to AWS" stuff we're probably blocked. Can we fake it in an integration test?

ncdc · 2016-07-13T17:24:48Z

Agreed that this is cloud provider specific. However there is an upstream unit test: https://github.com/kubernetes/kubernetes/blob/master/plugin/pkg/admission/persistentvolume/label/admission_test.go

deads2k · 2016-07-13T17:46:44Z

pkg/cmd/server/origin/master_config.go

@@ -317,6 +317,9 @@ var (
 		"LimitPodHardAntiAffinityTopology",
 		"SCCExecRestrictions",
 		quotaadmission.PluginName,
+		"PersistentVolumeLabel",


before both quota plugins.

deads2k · 2016-07-13T17:47:01Z

Agreed that this is cloud provider specific.

Ok, if it can't be tested automatically, is there somewhere we have to make a mark to be sure that test checks it during a regression suite?

ncdc · 2016-07-13T17:51:05Z

[test]

Enable PersistentVolumeLabel admission plugin. This labels AWS and GCE volumes with their zone, so the scheduler can limit the nodes for a pod to only those in the same zone as the peristent volumes being used by the pod.

ncdc · 2016-07-13T18:33:13Z

Updated PTAL

openshift-bot · 2016-07-13T19:15:34Z

Evaluated for origin test up to b66d87f

deads2k · 2016-07-13T19:16:58Z

lgtm [merge]

openshift-bot · 2016-07-13T20:40:29Z

continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/6221/)

openshift-bot · 2016-07-13T22:45:33Z

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/6279/) (Image: devenv-rhel7_4589)

ncdc · 2016-07-14T14:33:15Z

[merge]

openshift-bot · 2016-07-14T14:35:31Z

Evaluated for origin merge up to b66d87f

smarterclayton · 2016-09-09T01:54:30Z

Ok, this merged but is disabled in the admission_sync_test.go. Why didn't it fail?

ncdc · 2016-09-09T13:31:45Z

It didn't fail because the plugin is now both registered & used. Previously it was registered but not used (and the test case marks it as not used). If you change from registered/not-used/marked-not-used to registered/used/marked-not-used, the test will pass.

The test only tries to catch

we aren't trying to specify plugins in the kube admission chain that aren't registered
if new kube admission plugins show up, we either add them to origin.KubeAdmissionPlugins, or we add them to the list of not-used plugins in the test case

I'll submit a PR to remove PersistentVolumeLabel from the list of not-used plugins.

ncdc · 2016-09-09T13:36:40Z

#10862

smarterclayton · 2016-09-09T15:33:03Z

I'm doing it in the rebase - don't worry.

On Sep 9, 2016, at 9:31 AM, Andy Goldstein [email protected] wrote:

It didn't fail because the plugin is now both registered & used. Previously
it was registered but not used (and the test case marks it as not used). If
you change from registered/not-used/marked-not-used to
registered/used/marked-not-used, the test will pass.

The test only tries to catch

we aren't trying to specify plugins in the kube admission chain that
aren't registered
if new kube admission plugins show up, we either add them to
origin.KubeAdmissionPlugins, or we add them to the list of not-used plugins
in the test case

I'll submit a PR to remove PersistentVolumeLabel from the list of not-used
plugins.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#9822 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABG_p3qGB7diRgKnbvBHmYlS865zeDUWks5qoV_DgaJpZM4JLpNH
.

liggitt reviewed Jul 13, 2016
View reviewed changes

ncdc force-pushed the enable-pv-label-admission branch from 738dc33 to d8df3e3 Compare July 13, 2016 17:16

ncdc mentioned this pull request Jul 13, 2016

Need to document multi AZ volume scheduling openshift/openshift-docs#2497

Closed

deads2k reviewed Jul 13, 2016
View reviewed changes

Enable PersistentVolumeLabel admission plugin

b66d87f

Enable PersistentVolumeLabel admission plugin. This labels AWS and GCE volumes with their zone, so the scheduler can limit the nodes for a pod to only those in the same zone as the peristent volumes being used by the pod.

ncdc force-pushed the enable-pv-label-admission branch from d8df3e3 to b66d87f Compare July 13, 2016 18:32

openshift-bot merged commit ff15d54 into openshift:master Jul 14, 2016

ncdc mentioned this pull request Sep 9, 2016

Remove PersistentVolumeLabel from unused list #10862

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable PersistentVolumeLabel admission plugin #9822

Enable PersistentVolumeLabel admission plugin #9822

ncdc commented Jul 13, 2016 •

edited

Loading

liggitt Jul 13, 2016

ncdc Jul 13, 2016

deads2k Jul 13, 2016

deads2k commented Jul 13, 2016

smarterclayton commented Jul 13, 2016

ncdc commented Jul 13, 2016

deads2k Jul 13, 2016

ncdc Jul 13, 2016

deads2k commented Jul 13, 2016

ncdc commented Jul 13, 2016

ncdc commented Jul 13, 2016

openshift-bot commented Jul 13, 2016

deads2k commented Jul 13, 2016

openshift-bot commented Jul 13, 2016

openshift-bot commented Jul 13, 2016 •

edited

Loading

ncdc commented Jul 14, 2016

openshift-bot commented Jul 14, 2016

smarterclayton commented Sep 9, 2016

ncdc commented Sep 9, 2016

ncdc commented Sep 9, 2016

smarterclayton commented Sep 9, 2016

Enable PersistentVolumeLabel admission plugin #9822

Enable PersistentVolumeLabel admission plugin #9822

Conversation

ncdc commented Jul 13, 2016 • edited Loading

liggitt Jul 13, 2016

Choose a reason for hiding this comment

ncdc Jul 13, 2016

Choose a reason for hiding this comment

deads2k Jul 13, 2016

Choose a reason for hiding this comment

deads2k commented Jul 13, 2016

smarterclayton commented Jul 13, 2016

ncdc commented Jul 13, 2016

deads2k Jul 13, 2016

Choose a reason for hiding this comment

ncdc Jul 13, 2016

Choose a reason for hiding this comment

deads2k commented Jul 13, 2016

ncdc commented Jul 13, 2016

ncdc commented Jul 13, 2016

openshift-bot commented Jul 13, 2016

deads2k commented Jul 13, 2016

openshift-bot commented Jul 13, 2016

openshift-bot commented Jul 13, 2016 • edited Loading

ncdc commented Jul 14, 2016

openshift-bot commented Jul 14, 2016

smarterclayton commented Sep 9, 2016

ncdc commented Sep 9, 2016

ncdc commented Sep 9, 2016

smarterclayton commented Sep 9, 2016

ncdc commented Jul 13, 2016 •

edited

Loading

openshift-bot commented Jul 13, 2016 •

edited

Loading