This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

/sig storage

In a similar vein, we have noticed the same issue in K8s v1.27 when the kubernetes.io/hostname label doesn't match the node name. We see nodeinfo not found for <hostname>, despite (I think) the nodeinfo util supposed to be called using a node name.

/sig scheduling

This is because the non-existent node 1270-prod-worker is obtained from the cache, which has been fixed by #119779

before 119779, get node will return error:

after 119779 it's fixed

since version 1.30, it can run normally

@sanposhiho PTAL, need to cherry-pick for the previous version?

Looks like my PR #119779 accidentally fixed this issue. 😅

380c7f2 is the trigger of this issue, introduced in v1.27; volumebinding plugin just returns node names in PreFilterResult based on nodeAffinity in PV, and thus PreFilterResult could contain node names actually doesn't exist.

@kubernetes/sig-scheduling-leads

One Line Summary: If nodeaffinity in PV has non-existing node name in In with kubernetes.io/hostname, the scheduling always fails.
And, this bug is already coincidentally fixed by #119779.

Questions:

• In the first place, is it a bug worth cherry-picking?
• If Yes, should we cherry-pick run all PreFilter when the preemption will happen later in the same scheduling cycle #119779, which actually isn't the PR motivated for this issue? Or should we go another way

If it's a regression, we should consider cherry-pick.

🤔
#119779 looks big, but the actual code changes are about 10 lines (the rest are comments and tests), so I think it would be ok to cherry-pick.

Can you explain why your PR fixes the problem?

Can you explain why your PR fixes the problem?

Before my PR, we take NodeInfo of nodes in preFilterResult via sched.nodeInfoSnapshot.NodeInfos().Get(n) (ref). Consequently, we could get not found errors from it when preFilterResult has non-existing node name(s).

And, after my PR, we are not using sched.nodeInfoSnapshot.NodeInfos().Get(n) anymore.

The pitfall is that when PreFilterResult was introduced, scheduler directly fetch the nodeNames returned by PreFilter() in its node snapshot, so technically speaking, when a plugin (in-tree or out-of-tree) return non-existent/illegal nodes, the pod scheduling flow will abort immediately.

And the PR #109877 happened to stomp on this pitfall.

@Huang-Wei do you mean that you would like to see a smaller version of #119779 for cherry-pick? Note that it is only about 10 lines of actual change. The rest is comments and tests.

Since this is already released code, I would suggest cherry-picking as-is.

Also, worth testing E2E whether the issue reproduces in v1.30.

@dsgnr do you think you can give it a try?

@Huang-Wei do you mean that you would like to see a smaller version of #119779 for cherry-pick? Note that it is only about 10 lines of actual change. The rest is comments and tests.

yup, i'm in favor of picking the minimum fix, along with a minimum test #124539:

		for _, n := range allNodes {
			if !preRes.NodeNames.Has(n.Node().Name) {
				// We consider Nodes that are filtered out by PreFilterResult as rejected via UnschedulableAndUnresolvable.
				// We have to record them in NodeToStatusMap so that they won't be considered as candidates in the preemption.
				diagnosis.NodeToStatusMap[n.Node().Name] = framework.NewStatus(framework.UnschedulableAndUnresolvable, "node is filtered out by the prefilter result")
				continue
			}
			nodes = append(nodes, n)

b/c in addition to the "fix" part, #119779 actually introduced a slight behavior change. @sanposhiho do you think the above ^^ minimum fix + #124539 makes sense?

Also, worth testing E2E whether the issue reproduces in v1.30.

#124539 can guard it.

@Huang-Wei do you mean that you would like to see a smaller version of #119779 for cherry-pick? Note that it is only about 10 lines of actual change. The rest is comments and tests.

yup, i'm in favor of picking the minimum fix, along with a minimum test #124539:

		for _, n := range allNodes {
			if !preRes.NodeNames.Has(n.Node().Name) {
				// We consider Nodes that are filtered out by PreFilterResult as rejected via UnschedulableAndUnresolvable.
				// We have to record them in NodeToStatusMap so that they won't be considered as candidates in the preemption.
				diagnosis.NodeToStatusMap[n.Node().Name] = framework.NewStatus(framework.UnschedulableAndUnresolvable, "node is filtered out by the prefilter result")
				continue
			}
			nodes = append(nodes, n)

b/c in addition to the "fix" part, #119779 actually introduced a slight behavior change. @sanposhiho do you think the above ^^ minimum fix + #124539 makes sense?

@Huang-Wei @alculquicondor @sanposhiho Could you let me try it?，picking the minimum fix and add e2e test

A simplified version of the sample is used for testing,manifest.yaml:

apiVersion: v1
kind: PersistentVolume
metadata:
  name: data
spec:
  capacity:
    storage: 10Gi
  accessModes:
    - ReadWriteMany
  hostPath:
    path: /tmp/data
  claimRef:
    namespace: default
    name: data
  nodeAffinity:
    required:
      nodeSelectorTerms:
        - matchExpressions:
          - key: kubernetes.io/hostname
            operator: In
            values:
            - 1280-test-worker
            - 1280-prod-worker

---
apiVersion: v1
kind: PersistentVolumeClaim
metadata:
  name: data
  namespace: default
spec:
  accessModes:
  - ReadWriteMany
  resources:
    requests:
      storage: 5Gi
  volumeName: data
---
apiVersion: v1
kind: Pod
metadata:
  name: pause
  labels:
    app: pause
spec:
  containers:
  - name: pause
    image: registry.k8s.io/pause:3.6
    volumeMounts:
    - mountPath: /tmp
      name: data
  volumes:
  - name: data
    persistentVolumeClaim:
      claimName: data

also get this failedScheduling error, kubectl describe pod pause:

@sanposhiho do you think the above ^^ minimum fix + #124539 makes sense?

@Huang-Wei That small fix should work.
I don't have a strong opinion on whether we should have a minimum fix vs cherry-pick as it is.
Defer to you both @alculquicondor @Huang-Wei on that point.

Let's move ahead with the small fix, since we all agree on it.

/reopen

Cherry-pick PR(s) aren't done

@sanposhiho: Reopened this issue.

FYI on a related crash #124930

Are there any news on the backporting the solution to older versions (v1.27+)?

This is already backported and released. But the fix for the crash reported in #124930 was not released yet.

Let's close this issue for now, as it is completed.

/close

@alculquicondor: Closing this issue.

The issue here stems from the observation by Mike Sarfaty, above that volume-binding's prefilter assumes that it can equate the matchConstraint hostname labels with node names. This doesn't have to be the case. See #125336 here. Prefiltering will always fail in this situation. AFAICT this issue persists with the current head of the master branch.