Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support health check #18

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

fyuan1316
Copy link

[description]
add health check for deployment

close #17

@zhekazuev
Copy link
Contributor

@fyuan1316
Hi,

Maybe better to use exec or tcpProbe startupProbe instead of httpGet?

startupProbe:
  tcpSocket:
    port: 8080
  initialDelaySeconds: 60
  periodSeconds: 5

As I understand image has healthcheck for livenessProbe(source):

livenessProbe:
  initialDelaySeconds: 10
  periodSeconds: 10
  timeoutSeconds: 10
  httpGet:
    scheme: HTTP
    port: 8080
    path: /readyz

And readinessProbe - as you did:

readinessProbe:
  initialDelaySeconds: 10
  periodSeconds: 5
  timeoutSeconds: 10
  httpGet:
    scheme: HTTP
    port: 8080
    path: /v1/models

What do you think?

And how to check if the file has already been built?
Maybe check the process or file via exec startupProbe?
I'm not very good at using models.

@fyuan1316
Copy link
Author

Hi, @zhekazuev thanks for reply.

As I understand image has healthcheck for livenessProbe(source):

Inspired by your comment, I looked a little deeper into the code.
https://github.com/go-skynet/LocalAI/blob/master/api/api.go#L168-#L171

Based on the code, I think we can change readinessProbe path to "/readyz" and livenessProbe to "/healthz". This way we can be consistent with the original intent of the code design.

Maybe better to use exec or tcpProbe startupProbe instead of httpGet?

Regarding startupProbe, at first I noticed that the pod status was running but when I accessed the /v1/models service it didn't return properly. So my original expectation was that the startupProbe would indicate that the model was ready for service.
If we use a tcpSocket here, I think startupProbe should behave the same as now, i.e. not wait for the model to be ready for service. Do I understand correctly?

And how to check if the file has already been built?
Maybe check the process or file via exec startupProbe?
I'm not very good at using models.

I think it might be a good suggestion to see how we can better check that the model is built out or serviced properly. Because for me the current behaviour does seem a bit disconcerting for first time users.

@mudler
Copy link
Member

mudler commented Sep 22, 2023

@fyuan1316 that makes sense indeed - in LocalAI those endpoints are used for docker compose to wait and check when the service is healthy. any update on this PR?

@cryptk
Copy link

cryptk commented Mar 19, 2024

Regarding startupProbe, at first I noticed that the pod status was running but when I accessed the /v1/models service it didn't return properly. So my original expectation was that the startupProbe would indicate that the model was ready for service.

This isn't what startupProbes do. A startup probe merely delays the time that a liveness/readiness probe start running and is used for services that can take a while (and an unpredictable amount of time) to be ready for health checking to begin. LocalAI starts quite quicky, so startupProbes are not needed here.

Either way, the work here should be closed as superseded by #45 which has liveness/readiness probes as well as quite a few other enhancements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[enhance] The deployment of local-ai should check if the model is ready.
4 participants