Added all the 1.10 labels.

[MILESTONENOTIFIER] Milestone Issue: Up-to-date for process

@liggitt

/milestone clear

not 1.10 blocking

Does this mean that our documentation is not accurate and should be updated?
Documentation aside, many users may rely on the node affinity/anti-affinity or node selector features for scheduling. Does this issue imply that none of those features are reliable?

Does this mean that our documentation is not accurate and should be updated?

Nowhere in that documentation does it state the value of that label matches the node name.

Documentation aside, many users may rely on the node affinity/anti-affinity or node selector features for scheduling. Does this issue imply that none of those features are reliable?

If they are trying to match the hostname label to the node name object, they will be disappointed in many environments.

They can set affinity based on labels they set themselves (as the documentation gives them examples to do), or they can set anti-affinity on the kubernetes.io/hostname label (requesting that pods be scheduled to nodes with different values for that label, which has no bearing on whether the label values match the node names)

Nowhere in that documentation does it state the value of that label matches the node name.

I think the document should be updated and explicitly state that the value is not necessarily the node name. I would guess that zone, etc. are not reliable either. If so, we should state that as well.

If they are trying to match the hostname label to the node name object, they will be disappointed in many environments.

This is unfortunate. Something that surprises us, will definitely surprise our user. It causes bad UX. We should update the doc.

Thanks very much, learned 👍

If so, maybe we can introduce a new label, named kubernetes.io/nodename; the label will be updated by kubelet to .spec.NodeName.

maybe we can introduce a new label, named kubernetes.io/nodename; the label will be updated by kubelet to .spec.NodeName.

that seems like a mistake. It can't be trusted (since the kubelet sets it), and there is potential for overlap (two node objects having the same label)

It can't be trusted (since the kubelet sets it)

All node info are set by kubelet, we can trust it.

and there is potential for overlap (two node objects having the same label)

hostname is get by os.Hostname() if no override; nodename is got from cloud provider if any. Yes, the maybe the same value; but that gives user more options for scheduler policy, e.g. it's more easy to get nodeName instead of hostname for 'some' cloud provider, so they can use nodeSelector/NodeAffinity by node name.

All node info are set by kubelet, we can trust it

That is not accurate, and self-reported node info is actively being restricted/tightened in 1.11 (see kubernetes/community#911)

Currently, the DaemonSet controller directly assigns pods to nodes. With the Node authorization mode and NodeRestriction admission plugin in place (as they are in many default deployments), that means there is no way for a node other than the assigned node to get access to the secrets/configmaps/volumes referenced by a DaemonSet pod intended for another node.

If you move to using the scheduler to assign daemonset pods to nodes, the attributes/predicates used by the scheduler must not regress that guarantee.

Using a label set by a node to determine the identity of a node opens us up to node A claiming it is node B and getting pods intended for node B scheduled to it.

The bigger question is why the scheduler is being asked to pick a node when there is only one correct answer. The hoops we’re having to jump through to use affinity rules to try to pin to a particular node are a symptom of coupling priority enforcement to the node-selection/scheduling step.

The bigger question is why the scheduler is being asked to pick a node when there is only one correct answer. The hoops we’re having to jump through to use affinity rules to try to pin to a particular node are a symptom of coupling priority enforcement to the node-selection/scheduling step.

There was a long long debate about this (originally we decided not to do it (#42002) but then decided to do it (#59194). There are docs and whatnot mentioned in the latter issue that explain the rationale. In my mind the high-level reason is that DaemonSet is not simply "one instance of this pod on every node" but rather "one instance of this pod on every node that meets the constraints, possibly after preemption." Even without preemption, you start duplicating a lot of predicate logic into the DaemonSet controller, and then with preemption that duplicated logic grows. Having the default scheduler schedule the pods also makes the behavior of DaemonSet scheduling more obvious, e.g. if you want DaemonSets to schedule on nodes that have certain conditions or problems, you represent that with an explicit toleration rather than just baking such rules into the DaemonSet controller logic. [When DaemonSet was originally being considered, I argued we should not even have a separate API object and controller, we should just have a special replicas=-1 value in ReplicaSet that gives you the per-node behavior, but that's another story.]

So I think we need to find a way to make this work. Unfortunately I forgot how NodeName works. You can set it in the PodSpec (as the API server does when the scheduler requests a binding), but how do you know the NodeName of a node? It does not appear to be in the NodeSpec or NodeStatus, as far as I can tell, and we already established that it is not a label on the Node.

Like a new RequestedNodeName field in the PodSpec?

more like a nodeName term in pod affinity (which feels a little silly to me, since affinity to a single node should be equivalent to assignment to that node, but that's what making the daemonset assignment work via the node assignment phase of scheduling requires)

What's the objection to using a label, as long as the master sets it?

That's the objection. It's overloading unstructured labels as an API because they're convenient.

There's is the motivation for this feature.

An API seems heavy: not only a new API field, but also predicates (or others) enhancement for it. And it can not be re-used, e.g. both node selector, NodeAffinity can use labels.

That is not accurate, and self-reported node info is actively being restricted/tightened in 1.11 (see kubernetes/community#911)

Oh, yes, some taints are also updated by NodeController. IMO, some node info must be updated by kubelet instead of other component, e.g. hostname, nodename, conditions.

An API seems heavy

If we require guaranteed assignment to a particular node (which we do for daemonset, and others may as well), then the current label-selector affinity isn't sufficient (unless we change the meaning of labels for this one resource, which seems like a bad idea).

Label selectors inherently allow multiple matching objects. Expressing "the node named foo" as a label selector is misleading at best, and does not function correctly at worst.

some taints are also updated by NodeController

that's fine, the issue is with self-reported labels/taints from nodes

some node info must be updated by kubelet instead of other component, e.g. hostname, nodename, conditions.

• hostname is questionable (would be better to use information from the cloud provider... TLS serving cert bootstrapping is blocked because the addresses reported by the kubelet cannot be trusted)
• the kubelet doesn't get to choose or update its node name, it just gets permissions to create/update status for a node with a particular name, based on its credentials
• conditions are fine... specific non-security-related conditions get translated into taints by the NodeController.

This is tangentially related: #61042

If we require guaranteed assignment to a particular node ...

predicates will guarantee that. If there's only one candidate for nodeSelector/NodeAffinity, kube-scheduler will bind pod to it (if also passing other predicates). This's a valid case for nodeSelector/NodeAffinity :)

Also thinking the new field solution recently, it's just a different way of description to me: .RequestedNodeName vs. NodeAffinity{ hostname in target}; the other logic are similar, so prefer to use NodeAffinity + NodeName label :)

For such kind of labels, it should be: 1. reserved for k8s, 2. only updated by special components

the kubelet doesn't get to choose or update its node name, it just gets permissions to create/update status for a node with a particular name, based on its credentials

Who will create Node object in api server?

predicates will guarantee that

A label selector predicate doesn't select a single node, it selects a set of nodes. Hoping multiple nodes don't match that label isn't a guarantee.

For such kind of labels, it should be: 1. reserved for k8s, 2. only updated by special components

That approach:

• duplicates the node name (immutable) into a label (mutable and uncontrolled)
• does not use the existing selection mechanism that can select by name (field selector on metadata.name)
• requires enforcing this label's value be unique or DaemonSets break
• requires preventing any component from setting a different value for that label or DaemonSets break

Who will create Node object in api server?

The kubelet currently does, but is only authorized to create the node whose name matches its credential

Labels are not names and vice versa. The real gap here seems to be that we are trying to elicit a scheduling side effect (preemption) from a monolithic scheduler by emulating a scheduling decision. An affinity predicate seems much more appropriate than abusing labels.

We have repeatedly that labels cannot map arbitrary data - a predicate can, so it’s not clear to me why you don’t add one?

By predicate I meant “affinity rule”

Could be “nodeFieldTerms” or similar

(As I’m thinking about this more)

Resources are a constraint to the scheduler but we don’t model them as labels, same for tolerations, same for host ports and pvcs. From an API perspective:

1. this is a pretty specific use case
2. We shouldn’t abuse labels to represent things that aren’t labels
3. Labels are arbitrary topology
4. Node affinity defines general purpose constraints on topology
5. We don’t take other fields and turn them into labels

I see no reason why we can’t add a node affinity constraint for node name or other fields in the future that are of limited scope but valuable use. Schedulers aren’t required to honor constraints, so a scheduler could decide it doesn’t like your tolerations as much as it could decide to ignore your node affinity.

node affinity constraint seems better than a new field .RequestedNodeName in PodSpec :).

@bsalamat , does this make sense to you?

/cc @davidopp

I agree with @liggitt and @smarterclayton's points.
Similar to what @smarterclayton suggested we can add a new field to NodeSelectorTerm to match against node name. I would not make this field matchable against any field in the Node object though. More specifically, I would avoid adding a selector that requires reflections to find the field in the Node object. It would be an overkill and a performance killer with very limited use-cases.

We don’t actually use reflection on the others, we just use a general purpose api design. Ie downward api and others are consistent with that. However, having a specific oneof style matches for nodeName works just as well. I might advise to consider letting it be an array for future extension, but that’s only if we envision a use case for it.

We don’t actually use reflection on the others, we just use a general purpose api design. Ie downward api and others are consistent with that.

Could you elaborate on this? I thought 'metadata.name' which is provided in front of 'key' in your comment was just a string that could match any field of the Node object.

Such kind of map is enough for us:

fieldKey := map[string] func(*v1.Node)string {
 "spec.nodeName": func(pod *v1.Node) string {return node.Spec.NodeName},
}

Unless you anticipate other fields on pod being relevant just having a
specific term for nodeName or nodeNames is probably simpler.

Yes. If we want to follow the same switch statement model, the selector can match only the hardcoded fields as opposed to a reflection model that could match any field, including those that are added in the future. Since we do not want to use reflection, I agree that we should use nodeNames for now.

Sounds reasonable to me.

/close

All related PRs were merged :)