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

Generalize label selectors #341

Closed
bgrant0607 opened this issue Jul 2, 2014 · 66 comments
Closed

Generalize label selectors #341

bgrant0607 opened this issue Jul 2, 2014 · 66 comments

Comments

@bgrant0607
Copy link
Member

@bgrant0607 bgrant0607 commented Jul 2, 2014

Right now, our label selectors support conjunctions of exact matches of key-value pairs. We should support conjunctions of:

  • key exists
  • key is in (set of values)
  • key is not in (set of values)

Exact match can be expressed using "is in".

This would allow selectors such as:

service is in (foo),
environment is not in (qa),
stage is in (daily, weekly)

@meirf
Copy link
Contributor

@meirf meirf commented Jul 4, 2014

I've started working on this. I will submit a PR over the next day or so.

@timothysc
Copy link
Member

@timothysc timothysc commented Jul 8, 2014

If folks intend to use labels for constraint matching, there will likely need to be comparison operators.

@bgrant0607
Copy link
Member Author

@bgrant0607 bgrant0607 commented Jul 9, 2014

@timothysc Could you please provide a motivating example for comparisons? We could accept integer ranges, including open ranges. However, the restrictive selection semantics are careful chosen to facilitate set overlap detection, efficient indexing and reverse indexing, and human understandability.

@timothysc
Copy link
Member

@timothysc timothysc commented Jul 9, 2014

It is my understanding that Labels 'currently' only apply to pods. If labeling were extended to Kublets then users could define things such as:

rank = memory
(implies sort, meaning < operator required)

requirements = memory > 4GB
(Only land my pod on machines with > 4GB)

@bgrant0607
Copy link
Member Author

@bgrant0607 bgrant0607 commented Jul 9, 2014

Ok, thanks. As I commented in #367 , labels / constraints shouldn't be used for resource-based placement or binpacking. The scheduler should place pods based on their minimum resource requirements and resources available.

@timothysc
Copy link
Member

@timothysc timothysc commented Jul 9, 2014

You could also view it as

requirements = distance(podX) < 3

@bgrant0607
Copy link
Member Author

@bgrant0607 bgrant0607 commented Jul 9, 2014

Labels are explicit and literal.

Locality requirements need to be expressed at a higher level. Expressing them as individual hard constraints on pods is likely to lead to a greedy scheduler to paint itself into a corner.

Scheduling is a nuanced service, with workload-specific and provider-specific objectives and constraints, topological and architectural considerations, security concerns, dependencies, QoS and fairness considerations, workload interference, etc. Let's not try to shoehorn too much into the label mechanism.

@thockin
Copy link
Member

@thockin thockin commented Jul 9, 2014

I really think scheduling resources and scheduling constraints are two
different things, both useful, but not the same.

For your other example, distance to another pod, I would also not suggest
labels. Labels are relatively static and low in cardinality. Asking for
distance to another pod (for some definition of distance) may be a legit
constrainy but doesn't sound like labels to me.

I also started with the expectation that label selectors would be an
arbitrary expression, including regex matching or prefix/suffix matching.
But I did not come up with a solid use case for more than simple sets.

Tim
On Jul 8, 2014 5:45 PM, "Timothy St. Clair" notifications@github.com
wrote:

It is my understanding that Labels 'currently' only apply to pods. If
labeling were extended to Kublets then users could define things such as:

rank = memory
(implies sort, meaning < operator required)

requirements = memory > 4GB
(Only land my pod on machines with > 4GB)

Reply to this email directly or view it on GitHub
#341 (comment)
.

@timothysc
Copy link
Member

@timothysc timothysc commented Jul 9, 2014

@thockin ok I'll buy the idea of not trying to overload labels.
I'll try to hash it out on #367 then.

@bgrant0607
Copy link
Member Author

@bgrant0607 bgrant0607 commented Jul 9, 2014

Regular expressions and prefix/suffix matching are hostile to the goals I mentioned earlier. We should not support them. The escape hatch is attaching a new label using whatever arbitrary computation you wish to decide where to attach it, or to dump the data into a database and use SQL or whatever.

@timothysc
Copy link
Member

@timothysc timothysc commented Jul 9, 2014

I proposed the SQL method on NVP sets earlier today.

@thockin
Copy link
Member

@thockin thockin commented Jul 9, 2014

I am behind on email, so I promise to digest the other threads tonight :)
On Jul 8, 2014 7:22 PM, "Timothy St. Clair" notifications@github.com
wrote:

I proposed the SQL method on NVP sets earlier today.

Reply to this email directly or view it on GitHub
#341 (comment)
.

@bgrant0607
Copy link
Member Author

@bgrant0607 bgrant0607 commented Dec 3, 2014

P1 to decide how to surface this in v1beta3.

@bgrant0607 bgrant0607 self-assigned this Dec 3, 2014
@timothysc
Copy link
Member

@timothysc timothysc commented Dec 3, 2014

Is there any spec around this?

@sdminonne
Copy link
Contributor

@sdminonne sdminonne commented Dec 1, 2015

@davidopp, @soltysh I've submitted #18011 to rename PodSelector to LabelSelector, let me know if you're ok with that. I'm ok to close it if this is not the plan.

@davidopp
Copy link
Member

@davidopp davidopp commented Dec 1, 2015

I'm not sure we should rename to LabelSelector, because NodeSelector will also be a form of label selector.

@sdminonne
Copy link
Contributor

@sdminonne sdminonne commented Dec 1, 2015

I agree but I couldn't find a better name even if GeneralizedSelector looked better (finally everyone inside the project call in that way). If you've some ideas I can rename it.

@soltysh
Copy link
Contributor

@soltysh soltysh commented Dec 1, 2015

My 2 cents, if PodSelector is the generic one and it should be used as the primary, then LabelSelector @sdminonne proposes, will do the trick. From there you can always create a specialized selector (such as NodeSelector) which will cover the subset of LabelSelector with application to specific resource.

@soltysh
Copy link
Contributor

@soltysh soltysh commented Dec 1, 2015

On the side note, NodeSelector is just a field in PodSpec, whereas LabelSelector will be a type. Unless I'm missing something here.

@soltysh
Copy link
Contributor

@soltysh soltysh commented Dec 1, 2015

I think I found the NodeSlector you were talking about in predicates.go, still I think above is true.

@derekwaynecarr
Copy link
Member

@derekwaynecarr derekwaynecarr commented Jan 14, 2016

@smarterclayton @bgrant0607 @erictune - I am looking for some guidance on how to handle existing resources, so input appreciated.

I want to add an optional label and an optional field selector to LimitRange and ResourceQuota so an administrator can define constraints by varying dimensions. One dimension could be based on pod.spec.restartPolicy so pods derived from a Job could get different min/max/defaults defined than long running pods derived from a ReplicationController, same applies to ResourceQuota where I would want to quota differently based on anticipated time of footprint in cluster. The other dimension would be more open-ended based on end-user specified labels so things with database=mysql get quota differently than things with web=ruby for example.

Should I use the new label selector syntax as seen in apis/extensions/types.go or do I stick with the existing v1 syntax for existing resource types? I suspect I have to stick with the syntax as defined in v1/types.go but wanted confirmation before diving too deep.

@bgrant0607
Copy link
Member Author

@bgrant0607 bgrant0607 commented Jan 14, 2016

@derekwaynecarr Use the new syntax. Copy/move LabelSelector to v1.

@derekwaynecarr
Copy link
Member

@derekwaynecarr derekwaynecarr commented Jan 14, 2016

@bgrant0607 - thanks for the quick reply.

@bgrant0607
Copy link
Member Author

@bgrant0607 bgrant0607 commented Feb 12, 2016

This is covered by more specific issues, so closing.

@bgrant0607 bgrant0607 closed this Feb 12, 2016
vishh added a commit to vishh/kubernetes that referenced this issue Apr 6, 2016
Report error while fetching network stats.
keontang pushed a commit to keontang/kubernetes that referenced this issue May 14, 2016
support bring up baremetal cluster with CA signed cert.
keontang pushed a commit to keontang/kubernetes that referenced this issue Jul 1, 2016
support bring up baremetal cluster with CA signed cert.
harryge00 pushed a commit to harryge00/kubernetes that referenced this issue Aug 11, 2016
support bring up baremetal cluster with CA signed cert.
mqliang pushed a commit to mqliang/kubernetes that referenced this issue Dec 8, 2016
support bring up baremetal cluster with CA signed cert.
metadave pushed a commit to metadave/kubernetes that referenced this issue Feb 22, 2017
…to 0.10 (kubernetes#341)

* Update Jenkins version to current LTS (2.19.4)

* Update Jenkins plugins: Kubernetes and 2 more

Kubernetes 0.10
Credentials Binding 1.10
Git 3.0.1

* Update Jenkins config to 0.10 plugin format

Kubernetes Plugin 0.10 allows Pod Templates to have more then one image.
If the Pod Template doesn't have a container named `jnlp`, it'll include
one, so I renamed the `default` container to `jnlp`.

* Bump chart version to 0.1.7 and ImageTag to 0.6.0
mqliang pushed a commit to mqliang/kubernetes that referenced this issue Mar 3, 2017
support bring up baremetal cluster with CA signed cert.
wenjiaswe pushed a commit to wenjiaswe/kubernetes that referenced this issue Mar 19, 2019
…or_pods_fix

ClusterLoader - Fixing RC deletion handling
seans3 pushed a commit to seans3/kubernetes that referenced this issue Apr 10, 2019
wking pushed a commit to wking/kubernetes that referenced this issue Jul 21, 2020
Add util functions to read/write manifest file and update sub commands
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
You can’t perform that action at this time.