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

allow pvc by default #8298

Merged
merged 1 commit into from
Mar 30, 2016
Merged

allow pvc by default #8298

merged 1 commit into from
Mar 30, 2016

Conversation

pweil-
Copy link
Contributor

@pweil- pweil- commented Mar 30, 2016

Fixes: #8297

@liggitt @smarterclayton - will work on an e2e test that uses pvcs

@smarterclayton
Copy link
Contributor

@pweil- storage team really needs to own that - can you sync with @bchilds
and @markturansky?

On Wed, Mar 30, 2016 at 11:38 AM, Paul Weil [email protected]
wrote:

Fixes: #8297 #8297

@liggitt https://github.com/liggitt @smarterclayton
https://github.com/smarterclayton - will work on an e2e test that uses

pvcs

You can view, comment on, or merge this pull request online at:

#8298
Commit Summary

  • allow pvc by default

File Changes

Patch Links:


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#8298

@liggitt
Copy link
Contributor

liggitt commented Mar 30, 2016

unit test to ensure all sccs include the fstypes we've determined should always be allowed by default, then LGTM

@pweil-
Copy link
Contributor Author

pweil- commented Mar 30, 2016

test added to check bootstrapped constraints for []kapi.FSType{kapi.FSTypeEmptyDir, kapi.FSTypeSecret, kapi.FSTypeDownwardAPI, kapi.FSTypeConfigMap, kapi.FSTypePersistentVolumeClaim} or FSTypeAll

@@ -35,6 +38,12 @@ func TestBootstrappedConstraints(t *testing.T) {
if !reflect.DeepEqual(u, constraint.Users) {
t.Errorf("unexpected user access for %s. Found %v, wanted %v", constraint.Name, constraint.Users, u)
}

for _, expectedVolume := range expectedVolumes {
Copy link
Contributor

Choose a reason for hiding this comment

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

or has "*", right?

Copy link
Contributor

Choose a reason for hiding this comment

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

nm, missed the check below

@liggitt
Copy link
Contributor

liggitt commented Mar 30, 2016

LGTM

@pweil-
Copy link
Contributor Author

pweil- commented Mar 30, 2016

[merge]

@openshift-bot
Copy link
Contributor

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

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 020adab

@openshift-bot
Copy link
Contributor

[Test]ing while waiting on the merge queue

@smarterclayton
Copy link
Contributor

[test]

On Wed, Mar 30, 2016 at 1:30 PM, OpenShift Bot [email protected]
wrote:

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


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#8298 (comment)

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 020adab

@openshift-bot
Copy link
Contributor

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

@openshift-bot openshift-bot merged commit d92dd0d into openshift:master Mar 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants