feat(scheduler): Match models requirements for servers not replicas & improve status handling

[agrski]

Why

Motivation

Currently, when scheduling a model fails, e.g. due to more replicas being requested than are available on an otherwise compatible server, error/status messages are not very informative. As a result, the Kubernetes status is also not as informative as it could be.

Prior to this PR, scheduling a model used a debugTrail that would accumulate a list of scheduling failures. In itself, this is not very useful for anyone wanting to understand, concisely, why a model failed to be scheduled. In fact, this might see messages about various servers they know aren't compatible and wouldn't expect to be used.

This PR improves the situation for both error/status messages and logs more generally. Furthermore, it changes the requirements filtering from applying to server replicas to applying to servers. All the replicas of a single server share their requirements, so it is more efficient to filter for this earlier rather than later.

What

Changes

• Change replica requirements filter to server requirements filter.
• Use summary messages when failing to schedule.
• Log intermediate scheduling information instead of accumulating it for statuses.

Testing

Running in a simple kind cluster (as provided via the Ansible playbooks), I built and loaded a new scheduler corresponding to the changes on this branch:

make -C scheduler/ docker-build-scheduler
make -C scheduler/ kind-image-install-scheduler
kubectl -n seldon rollout restart sts seldon-scheduler

I then created a simple iris model with five replicas; this is more than the default MLServer installation supports with its single replica:

kubectl delete model -n seldon iris
cat samples/models/iris-v1.yaml | sed '/^spec:$/a\  replicas: 5' | kubectl -n seldon apply -f -
kubectl get model -n seldon iris -o json | jq '.status.conditions[] | select(.type == "ModelReady")'

{
  "lastTransitionTime": "2023-08-23T15:54:27Z",
  "message": "ModelFailed",
  "reason": "rpc error: code = FailedPrecondition desc = Failed to schedule model iris as no matching server had enough suitable replicas",
  "status": "False",
  "type": "ModelReady"
}

Relevant scheduler logs:

time="2023-08-23T15:54:27Z" level=debug msg="Schedule model" Name=SimpleScheduler func=scheduleToServer model=iris
time="2023-08-23T15:54:27Z" level=debug msg="Filtering servers for model" Name=SimpleScheduler func=filterServer model=iris num_servers=2
time="2023-08-23T15:54:27Z" level=debug msg="Accepting server for model" Name=SimpleScheduler func=filterServer model=iris server=mlserver
time="2023-08-23T15:54:27Z" level=debug msg="Rejecting server for model" Name=SimpleScheduler filter=ServerRequirementsFilter func=filterServer model=iris reason="model requirements [sklearn] se
rver capabilities [triton dali fil onnx openvino python pytorch tensorflow tensorrt]" server=triton
time="2023-08-23T15:54:27Z" level=debug msg="Identified candidate servers for model" Name=SimpleScheduler candidate_servers="[mlserver]" desired_replicas=5 func=scheduleToServer model=iris
time="2023-08-23T15:54:27Z" level=debug msg="Checking compatibility with candidate server" Name=SimpleScheduler func=scheduleToServer model=iris server=mlserver
time="2023-08-23T15:54:27Z" level=debug msg="Filtering server replicas for model" Name=SimpleScheduler func=filterReplicas model=iris server=mlserver
time="2023-08-23T15:54:27Z" level=debug msg="Accepting server replica for model" Name=SimpleScheduler func=filterReplicas model=iris replica=0 server=mlserver
time="2023-08-23T15:54:27Z" level=debug msg="Skipping server due to insufficient suitable replicas" Name=SimpleScheduler available_replicas=1 desired_replicas=5 func=scheduleToServer model=iris 
server=mlserver
time="2023-08-23T15:54:27Z" level=debug msg="Failed to schedule model as no matching server had enough suitable replicas" Name=SimpleScheduler func=scheduleToServer model=iris

Creating a model with a single replica succeeds:

kubectl delete model -n seldon iris
kubectl -n seldon apply -f samples/models/iris-v1.yaml
kubectl get model -n seldon iris -o json | jq '.status.conditions[] | select(.type == "ModelReady")'

{
  "lastTransitionTime": "2023-08-25T13:20:39Z",
  "message": "ModelAvailable",
  "status": "True",
  "type": "ModelReady"
}

[jesse-c]

Use summary messages when failing to schedule.

For my understanding, this means that instead of the big debug audit trail, there's the new, deliberate summary message [1]?

[1] https://github.com/SeldonIO/seldon-core/pull/5107/files#diff-b6060c1a371e5e0f7f24614386ef51aace951716ec8c2581d682aaf234354ad4R211

[jesse-c]

From my current understanding, it looks like a good change to make it clear and concise what the end result is.

[agrski]

ℹ️ 🔧 This test is currently failing because it breaks a key assumption of the source code: that server replicas have identical capabilities. This is a reasonable assumption -- guaranteed in k8s in fact, because of how a Server resource is defined -- and sensible outside k8s. If server replicas have different capabilities and memory/overcommit capacities, then they should really be different server resources.

I realise that this is hard to enforce outside k8s for the time being, but I'd argue anyone not adhering to the above assumptions is implementing anti-patterns that make server administration difficult.

In a follow-up PR to set capabilities per server, there could be validation logic that ensures this assumption is met, and otherwise rejects the agent attempting to register with the scheduler.

[agrski]

The same applies to scheduler_test.go and its failing NotEnoughReplicas test. This test case is also forcing a failure by setting non-matching capabilities for a replica, even though this breaks an assumption. Really these should either mock the underlying calls to inject failures, or should use cases where server replicas are otherwise incompatible, such as a lack of available memory.

[jesse-c]

That does make sense. I think having validation (in a follow-up) is key. Are there documentation changes we could make too?

[agrski]

Documentation changes are a good suggestion. While I think it's best to enforce any assumptions, e.g. by rejecting the registration of a new agent if it makes claims that diverge from other replicas in the server it implements, clarifying expectations in docs is no bad thing. For the sake of expediency and this PR not growing any more, I'll make any docs changes in a follow-up PR.