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

add project annotation selectors to cluster quota #9757

Merged
merged 1 commit into from
Jul 13, 2016

Conversation

deads2k
Copy link
Contributor

@deads2k deads2k commented Jul 8, 2016

This adds an annotation selector (map[string]string) to the ClusterResourceQuota so that we can select on project requestor username: oc create clusterquota for-deads --project-annotation-selector openshift.io/requester=deads --hard secrets=10

This test will be unreliable until quota enforcement merges with #9740

@openshift/api-review The API change is breaking, but no one is using it today
@jwforres @spadgett Breaking API change to ClusterResourceQuota.Selector to allow the selection of projects based on annotation selectors.
@Kargakis --project-label-selector and --project-annotation-selector for the create?

@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 11, 2016
// many more less active projects). These projects will contend on object creation through
// this resource.
Selector *unversioned.LabelSelector
Selector ClusterResourceQuotaSelector
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@smarterclayton We agreed in principle in person. You good with the representation?

@deads2k deads2k force-pushed the cq-10-annotation-selector branch from 01bf6ce to ea51474 Compare July 11, 2016 19:24
@deads2k deads2k changed the title [DO NOT MERGE] add project annotation selectors to cluster quota add project annotation selectors to cluster quota Jul 11, 2016
@deads2k
Copy link
Contributor Author

deads2k commented Jul 11, 2016

[test]

// LabelSelector is used to select projects by label.
LabelSelector *unversioned.LabelSelector

// AnnotationSelector is used to select projects by annotation.
Copy link
Contributor

Choose a reason for hiding this comment

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

Use namespaces consistently in the godoc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use namespaces consistently in the godoc.

switched to project in the docs

@smarterclayton
Copy link
Contributor

Yeah it's fine.

@deads2k
Copy link
Contributor Author

deads2k commented Jul 11, 2016

@Kargakis this updates every layer to handle simple annotation selectors in addition to label selectors so we can restrict by user.

@openshift-bot openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 11, 2016
}
}

annotationSelector, err := unversioned.ParseToLabelSelector(cmdutil.GetFlagString(cmd, "project-annotation-selector"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Will an empty string create an empty selector?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will an empty string create an empty selector?

It will create one with zero MatchLabels in it, which is treated as empty. Empty is allowed as long as the label selector is provided.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looked closer at validation. ok

@0xmichalis
Copy link
Contributor

Swagger spec needs update

@deads2k deads2k force-pushed the cq-10-annotation-selector branch from 05bdb14 to 0af555d Compare July 12, 2016 12:06
@0xmichalis
Copy link
Contributor

lgtm

@deads2k deads2k force-pushed the cq-10-annotation-selector branch from 0af555d to 19aa546 Compare July 12, 2016 12:10
@deads2k
Copy link
Contributor Author

deads2k commented Jul 12, 2016

[merge]

@deads2k
Copy link
Contributor Author

deads2k commented Jul 12, 2016

[test]

@deads2k deads2k force-pushed the cq-10-annotation-selector branch from 19aa546 to 3a44a24 Compare July 12, 2016 13:44
@deads2k
Copy link
Contributor Author

deads2k commented Jul 12, 2016

re[test]

@deads2k deads2k force-pushed the cq-10-annotation-selector branch from 3a44a24 to 3635d26 Compare July 12, 2016 16:57
@deads2k
Copy link
Contributor Author

deads2k commented Jul 13, 2016

deployment flakes re[merge] re[test]

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 3635d26

@openshift-bot
Copy link
Contributor

openshift-bot commented Jul 13, 2016

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

@deads2k
Copy link
Contributor Author

deads2k commented Jul 13, 2016

deployments re[test]

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 3635d26

@openshift-bot
Copy link
Contributor

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

@openshift-bot openshift-bot merged commit a5adcd2 into openshift:master Jul 13, 2016
@deads2k deads2k deleted the cq-10-annotation-selector branch September 6, 2016 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants