Add /health/ping and /health/status endpoint to Python REST Microservices

[kparaju]

This PR will add two endpoints to the python REST flask app.

/health/ping: quick check to make sure the flask app is alive
/health/status: more through check to make sure that the model is healthy

(I'm open to changing the url of these endpoints, these seemed to fit well)

There are two ways we are planning on using these endpoints:

1. After we docker image is built, we will load it up and use /health/ping and /health/status to verify that the final artifact is good.
2. Use it in k8s health checks to make sure that the model is up and predicting (using /health/status) and restart the pod if anything funky happens.
3. In the future, it can also be added to the orchestrator so that clients can know the status and potentially use the Circuit Breaker pattern.

Tested by doing:

seldon-core-microservice MyModel REST --service-type MODEL --persistence 0 --workers=2

and

(base) ➜  ~ curl localhost:5000/health/ping
pong%
(base) ➜  ~ curl localhost:5000/health/status
{"data":{"names":[],"tensor":{"shape":[1],"values":[3]}},"meta":{}}
(base) ➜  ~

[seldondev]

Hi @kparaju. Thanks for your PR.

I'm waiting for a SeldonIO or seldonio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[axsaucedo]

/ok-to-test

[axsaucedo]

This is great @kparaju! Would be great to add something to the docs. I guess we can to this ourselves (unless you want to add something).

By the way, the linter seems to be failing - you can make sure it works by running make -C python setup_linter run_linter_check (details for this in https://github.com/SeldonIO/seldon-core/blob/master/CONTRIBUTING.md)

[kparaju]

@axsaucedo I added some docs as well as ran the linter -- sorry about that.

[axsaucedo]

/test integration

[axsaucedo]

This looks solid! All tests pass so happy to land, however there's still the GRPC route missing, as currently the implementation is done in Python but not in GRPC. You can see this in this line:

seldon-core/python/seldon_core/wrapper.py

Line 170 in 3483b11

prediction_pb2_grpc.add_SeldonServicer_to_server(seldon_model, server)

[ukclivecox]

This looks solid! All tests pass so happy to land, however there's still the GRPC route missing, as currently the implementation is done in Python but not in GRPC. You can see this in this line:

seldon-core/python/seldon_core/wrapper.py

Line 170 in 3483b11

prediction_pb2_grpc.add_SeldonServicer_to_server(seldon_model, server)

@axsaucedo I don't think we would change the gRPC as that would require updating the prediction protos - but maybe you didn'tmean this. Better would be the have a lightweight Flask running on a separate port that has just the health endpoints exposed on http?

[axsaucedo]

This looks solid! All tests pass so happy to land, however there's still the GRPC route missing, as currently the implementation is done in Python but not in GRPC. You can see this in this line:

seldon-core/python/seldon_core/wrapper.py

Line 170 in 3483b11

prediction_pb2_grpc.add_SeldonServicer_to_server(seldon_model, server)

@axsaucedo I don't think we would change the gRPC as that would require updating the prediction protos - but maybe you didn'tmean this. Better would be the have a lightweight Flask running on a separate port that has just the health endpoints exposed on http?

Oh yeah you are right actually. Even if the model is a GRPC one, we'd still want to have health endpoints! Ok happy with that then, will resolve request for review

[axsaucedo]

/hold

[axsaucedo]

Oh my bad, resolving the comment automatically approved it. @cliveseldon I've added a [slash] hold, to approve just do [slash] hold remove

[axsaucedo]

/lgtm

[seldondev]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: axsaucedo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [axsaucedo]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[seldondev]

New changes are detected. LGTM label has been removed.

[kparaju]

@cliveseldon @axsaucedo updated with documentation about how to use these problems in your SeldonDeployment YAML.

[ukclivecox]

/hold cancel

[axsaucedo]

/test integration

[seldondev]

@kparaju: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
integration	51ff940	link	/test integration

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.