Skip to content

Commit

Permalink
Add readiness probe to Server Deployment (#183)
Browse files Browse the repository at this point in the history
Fixes #153
  • Loading branch information
nstogner committed Aug 11, 2023
1 parent 09c0b48 commit 546e219
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 18 deletions.
13 changes: 10 additions & 3 deletions docs/container-contract.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ The working directory MUST be set to `/content`.
WORKDIR /content
```

### Jupyter
## Jupyter

This requirement applies to Model, Dataset, and Notebook containers.

Expand All @@ -22,7 +22,7 @@ The `notebook.sh` script MUST be located in `$PATH`. It is recommended that this

Note: This requirement is satisfied by default when using Substratus base images.

### Directory Structure
## Directory Structure

```
/content/ # Working directory.
Expand All @@ -33,7 +33,7 @@ Note: This requirement is satisfied by default when using Substratus base images
saved-model/ # Location where a previously saved model will be mounted.
```

### Parameters
## Parameters

Substratus provides params as a file (`/content/params.json`) and as environment variables to containers.

Expand All @@ -48,3 +48,10 @@ spec:
Parameters get converted to environment variables using the following scheme.

`PARAM_{upper(param_key)}={param_value}`

## Server

Substratus Server containers are expected to:

* Serve HTTP traffic on port `8080`.
* Serve a 200 OK on the root path `/` when ready to serve traffic.
5 changes: 1 addition & 4 deletions examples/facebook-opt-125m/base-server.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,6 @@ kind: Server
metadata:
name: facebook-opt-125m
spec:
build:
git:
url: https://github.com/substratusai/images
path: model-server-basaran
image: substratusai/model-server-basaran
model:
name: facebook-opt-125m
6 changes: 2 additions & 4 deletions internal/controller/model_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,10 +297,8 @@ func (r *ModelReconciler) modellerJob(ctx context.Context, model, baseModel *api
ServiceAccountName: modellerServiceAccountName,
Containers: []corev1.Container{
{
Name: containerName,
Image: model.GetImage(),
// NOTE: tini should be installed as the ENTRYPOINT the image and will be used
// to execute this script.
Name: containerName,
Image: model.GetImage(),
Command: model.Spec.Command,
Env: paramsToEnv(model.Spec.Params),
},
Expand Down
6 changes: 2 additions & 4 deletions internal/controller/notebook_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,10 +336,8 @@ func (r *NotebookReconciler) notebookPod(notebook *apiv1.Notebook, model *apiv1.
ServiceAccountName: notebookServiceAccountName,
Containers: []corev1.Container{
{
Name: containerName,
Image: notebook.GetImage(),
// NOTE: tini should be installed as the ENTRYPOINT the image and will be used
// to execute this script.
Name: containerName,
Image: notebook.GetImage(),
Command: notebook.Spec.Command,
//WorkingDir: "/home/jovyan",
Ports: []corev1.ContainerPort{
Expand Down
15 changes: 12 additions & 3 deletions internal/controller/server_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,15 +139,24 @@ func (r *ServerReconciler) serverDeployment(server *apiv1.Server, model *apiv1.M
Name: containerName,
Image: server.GetImage(),
ImagePullPolicy: "Always",
// NOTE: tini should be installed as the ENTRYPOINT the image and will be used
// to execute this script.
Command: server.Spec.Command,
Command: server.Spec.Command,
Ports: []corev1.ContainerPort{
{
Name: modelServerHTTPServePortName,
ContainerPort: 8080,
},
},
ReadinessProbe: &corev1.Probe{
ProbeHandler: corev1.ProbeHandler{
HTTPGet: &corev1.HTTPGetAction{
// TBD: Not sure if this should ever be something we configure via
// the Server API. For now, we'll just hardcode it and add it to the container
// contract.
Path: "/",
Port: intstr.FromString(modelServerHTTPServePortName),
},
},
},
},
},
},
Expand Down

0 comments on commit 546e219

Please sign in to comment.