Add more details to volume scheduling design

[msau42]

Done:

• Added diagram showing new additions to the scheduler flow
• Restrict new behavior through StorageClass API to eliminate backwards compatibility issues and no need for deprecation.
  • This also eliminates the requirement to support scheduler bypass use cases.
• Caching overview
• Handling other volume predicates correctly

@kubernetes/sig-storage-proposals
@kubernetes/sig-scheduling-proposals

[msau42]

/assign @bsalamat @jsafrane @saad-ali

[bsalamat]

LGTM

[jsafrane]

/lgtm

[msau42]

Updated design to restrict the new behavior through the StorageClass API. This allows us to retain backwards compatibility and not need to deprecate anything. It also eliminates the requirement to support scheduler bypass use cases.

[ianchakeres]

/lgtm

[jsafrane]

Is it still WIP? What's missing? It looks good to me.

[msau42]

My remaining TODOs are:

• Caching details. Does this need to be in the design doc?
• Handling other volume predicates correctly. May need to add some extra state in the scheduler to handle the scenario where volumes can get bound in the middle of running the volume predicates.
• How to enable feature during HA master upgrade. This can probably be deferred to beta/GA.

[msau42]

For StorageClass API review
/assign @thockin

[msau42]

Added some information on how caching will be done, which should also take care of the other volume predicates issue. I'm going to defer HA upgrade details until beta/GA.

So this just needs another full pass + API review.

[msau42]

Updated new binding mode value to "WaitForFirstConsumer"

[msau42]

@jsafrane does this look ok?

[jsafrane]

/lgtm

[k8s-github-robot]

/test all [submit-queue is verifying that this PR is safe to merge]

[k8s-github-robot]

Automatic merge from submit-queue.

[liggitt]

why were these removed? these are significant downsides

[msau42]

The "scope" here was referring to the scheduler having to also handle scenarios where the user manually sets Pod.Spec.NodeName to bypass the scheduler. This was because the original plan was to have this behavior change for all volume binding.

But we changed the API so that this new behavior can be controlled explicitly through StorageClass. And because of that, we decided that we don't need to support this use case of setting Pod.Spec.NodeName. It was only considered previously for backwards compatibility.

[liggitt]

I was referring to the "expanding scheduler scope" from being just a pod scheduler to being a pod-and-volume scheduler. The downsides seem to be overlap with PV binder responsibilities, and requiring custom schedulers to also be pod-and-volume schedulers.

[liggitt]

this makes it clear that the volume binding is a distinct responsibility. combining it into the scheduler in the initial implementation and requiring all other schedulers to implement the same function is a strong indication that the data flow to accomplish this binding is not well separated

[msau42]

This was the simplest way to integrate volume binding with pod scheduling. When scheduling the pod, the scheduler needs information on what volumes are available for that pod. And when the scheduler makes a decision assuming that the pod is going to be using one of the volumes, it needs a way to tell the PV controller which volume it picked for that pod. Communicating that informaton to the PV controller without an API update could cause the PV controller to make binding decisions differently from the scheduler, and would cause lots of cascading, incorrect scheduling decisions, and lots of complex recovery/retry logic.

The unfortunate effect of this design is that it raises the bar for implementing your own scheduler ONLY if you want to support this advanced use case. If you don't need this feature, then there is no change.