This change is 

/cc @bsalamat

I'm going to add some test for it.

why not wait scheduledPodPopulator.hasSynced ?

why not wait scheduledPodPopulator.hasSynced ?

reasonable, let me check the detail of Informer.

5908108#diff-77bdee21550bc698bfa654de11c64324L455

I think it will never return because the corresponding controller is not started.

5908108#diff-77bdee21550bc698bfa654de11c64324L455
I think it will never return because the corresponding controller is not started.

Thanks for the comments, addressed.

Tested in local cluster by following steps:

1. Start cluster with ./hack/local-cluster-up.sh (8 cpu, 2G mem)
2. Submit 10 pods (1 cpu, 300m mem): 7 pod will be running and 3 pods will be pending (dns consumed 260m cpu)
3. Keep restarting "hyperkube scheduler":
   • without fix, it may report OutOfCpu
   • with fix, not status changed (7 running, 3 pending)

The reason is that we did not wait for cache to be synced by before scheduling, so the running pods may not be added into cache when scheduling. We did not need to wait for unassigned tasks, scheduler will get it one by one to dispatch.

/cc @bsalamat @wojtek-t

/lgtm

Thanks for the fix, Klaus!

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: bsalamat, k82cn
We suggest the following additional approver: wojtek-t

Assign the PR to them by writing /assign @wojtek-t in a comment when ready.

@k82cn please add a release note on this one.

@ncdc any comments? It looks good to me, re: prevents a startup race, but does this fix need to propagate to the kcm?

please add a release note on this one.

Done

re: prevents a startup race, but does this fix need to propagate to the kcm?

I can do some check today and append comments here.

@ncdc any comments? It looks good to me, re: prevents a startup race, but does this fix need to propagate to the kcm?

@ncdc / @timothysc , just check the kcm: each controller call cache.WaitForCacheSync in Run() (cronjob sync it by List).

@k82cn Scheduler seems use the sharedInformerFactory for other resources like nodes,configMap,pv, except for the pod, here is the code for pods: scheduledPodIndexer, c.scheduledPodPopulator = cache.NewIndexerInformer, it's a not registered informer of the sharedInformerFactory, so I think your code sharedInformerFactory,WaitForCacheSync will not sync the scheduled pod and notScheduledPod.
Also, your code fix part of the problem, you synced the other resources except pods, I am not clear why the scheduledPodIndexer and scheduledPodPopulator not using the sharedInformer, maybe we have other concerns ? Correct me if I am wrong.
/cc @timothysc @davidopp

scheduledPodIndexer, c.scheduledPodPopulator = cache.NewIndexerInformer, it's a not registered informer of the sharedInformerFactory,

Good point! I'll add logic to handle it. We did not add it into informer factory because the ListerWatcher is customized (un-assigned, non-terminated).

Automatic merge from submit-queue (batch tested with PRs 45453, 45307, 44987)