Sagemaker hosting container support

[palp]

• Adds /ping endpoint resolving to model readiness check. Since Sagemaker will start sending requests when this resolves to a 200, this needs to wait for the model to be ready to avoid serving errors. In a real-world test it seems SageMaker gives containers well over the claimed 4 minutes to start (10-15?) so I think this approach will work out.
• Adds /invocations endpoint resolving to model inference. No contract changes are needed at this time, but the function is a wrapper so it could mutate the request.
• Changes container build to use ENTRYPOINT instead of CMD, as Sagemaker launches containers with the serve argument which will override CMD. The argument is harmless with ENTRYPOINT specified.

This is the bare minimum to function, I have tested a deployment and it works.

[palp]

This was throwing an error on both the default GPU base image and my custom image, as both only have python3 available, not python. I also noticed that the entrypoint below specifies python3, so either that needs to also be dynamic or this serves no real purpose.

[bolasim]

Thanks for the comment. I think @joostinyi put up a fix in a separate PR

[bolasim]

Just took a cursory look and this is excellent. I remember you mentioning you might also have to mutate the headers. Is that still required?

When you feel like it's ready, please request a review from @pankajroark

Are there any docs we could add here on how to push this new image to sagemaker? We could also do this in a followup

[palp]

Just took a cursory look and this is excellent. I remember you mentioning you might also have to mutate the headers. Is that still required?

Not required, and to be useful it would need to pass the CustomAttributes header in to the user code and return a result header from it, so I think that's an item for a future followup. One of the primary uses of this header is to allow trace context propagation, which I think might be a useful PR on its own (the standard W3C headers could be supported through the normal endpoint).

Are there any docs we could add here on how to push this new image to sagemaker? We could also do this in a followup

There's nothing special required to run it - in a nutshell:

1. Build the image: truss build-image --tag <your ECR tag>
2. Push the image: docker push <your ECR tag>
3. Create the model:

aws sagemaker create-model --model-name <your model name> --execution-role-arn <preexisting role> --primary-container "Image=<your ECR tag>"

You can then create an endpoint config and deploy an endpoint, Features like async inference and batch transform should work out of the box too (though I haven't tested yet).
It might be best to just find some good AWS documentation on deploying BYOC (bring your own container) and link to it, I can try to dig that up.

[pankajroark]

@palp This looks good to me overall. Let me kick off integration tests on this.

[pankajroark]

Running integration tests here: https://github.com/basetenlabs/truss/actions/runs/5060244484/jobs/9082853359

[palp]

I've merged in latest from main which I think will resolve the test error.

[pankajroark]

I've merged in latest from main which I think will resolve the test error.

Running integration tests again here: https://github.com/basetenlabs/truss/actions/runs/5063344484

[pankajroark]

That test failure looks like test flakiness, rerunning. We'll fix the flaky test on our side separately.

[pankajroark]

@palp Would you mind updating this file: truss/truss/test_data/server.Dockerfile according to your changes to the server.Dockerfile. This test data is using the previous format. I think if you do that, the tests should start passing.

[palp]

@palp Would you mind updating this file: truss/truss/test_data/server.Dockerfile according to your changes to the server.Dockerfile. This test data is using the previous format. I think if you do that, the tests should start passing.

Done!

[pankajroark]

There's just a space mismatch in that test. For the sake of expedience, it's ok to land, I can fix the test quickly on master in a follow-up.

     'ENV INFERENCE_SERVER_PORT 8080\n',
  -  'ENTRYPOINT ["python3",  "/app/inference_server.py"]\n',
  ?                         -
  +  'ENTRYPOINT ["python3", "/app/inference_server.py"]\n',
    ]

Or if you want to fix it, you can run the failing test locally like this:
poetry run pytest truss/tests/contexts/image_builder/test_serving_image_builder.py::test_serving_image_dockerfile_from_user_base_image

[palp]

There's just a space mismatch in that test. For the sake of expedience, it's ok to land, I can fix the test quickly on master in a follow-up.

     'ENV INFERENCE_SERVER_PORT 8080\n',
  -  'ENTRYPOINT ["python3",  "/app/inference_server.py"]\n',
  ?                         -
  +  'ENTRYPOINT ["python3", "/app/inference_server.py"]\n',
    ]

Or if you want to fix it, you can run the failing test locally like this: poetry run pytest truss/tests/contexts/image_builder/test_serving_image_builder.py::test_serving_image_dockerfile_from_user_base_image

Whoops - fixed and tested locally, thanks.