• Kubernetes Submit Queue's avatar
    Merge pull request #50028 from julia-stripe/fix-incorrect-scheduler-bind-call · 898b1b33
    Kubernetes Submit Queue authored
    Automatic merge from submit-queue
    
    Fix incorrect call to 'bind' in scheduler
    
    I previously submitted https://github.com/kubernetes/kubernetes/pull/49661 -- I'm not sure if that PR is too big or what, but this is an attempt at a smaller PR that makes progress on the same issue and is easier to review.
    
    **What this PR does / why we need it**:
    
    In this refactor (https://github.com/kubernetes/kubernetes/commit/ecb962e6585#diff-67f2b61521299ca8d8687b0933bbfb19R223) the scheduler code was refactored into separate `bind` and `assume` functions. When that happened, `bind` was called with `pod` as an argument. The argument to `bind` should be the assumed pod, not the original pod. Evidence that `assumedPod` is the correct argument bind and not `pod`: https://github.com/kubernetes/kubernetes/blob/80f26fa8a89ef5863cb19c71a620bb389d025166/plugin/pkg/scheduler/scheduler.go#L229-L234. (and it says `assumed` in the function signature for `bind`, even though it's not called with the assumed pod as an argument).
    
    This is an issue (and causes #49314, where pods that fail to bind to a node get stuck indefinitely) in the following scenario:
    
    1. The pod fails to bind to the node
    2. `bind` calls `ForgetPod` with the `pod` argument
    3. since `ForgetPod` is expecting the assumed pod as an argument (because that's what's in the scheduler cache), it fails with an error like `scheduler cache ForgetPod failed: pod test-677550-rc-edit-namespace/nginx-jvn09 state was assumed on a different node`
    4. The pod gets lost forever because of some incomplete error handling (which I haven't addressed here in the interest of making a simpler PR)
    
    In this PR I've fixed the call to `bind` and modified the tests to make sure that `ForgetPod` gets called with the correct argument (the assumed pod) when binding fails.
    
    **Which issue this PR fixes**: fixes #49314
    
    **Special notes for your reviewer**:
    
    **Release note**:
    
    ```release-note
    ```
    898b1b33
Name
Last commit
Last update
..
algorithm Loading commit data...
algorithmprovider Loading commit data...
api Loading commit data...
core Loading commit data...
factory Loading commit data...
metrics Loading commit data...
schedulercache Loading commit data...
testing Loading commit data...
util Loading commit data...
BUILD Loading commit data...
OWNERS Loading commit data...
scheduler.go Loading commit data...
scheduler_test.go Loading commit data...
testutil.go Loading commit data...