-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Add health check endpoint #18465
Add health check endpoint #18465
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will require a new username restriction,
Is there really nothing we can use on say /api or even in .well-known?
If it has to be this name then you must add a restriction to models/user and mark this as BREAKING.
@zeripath no, the path ( do you think it is OK change to |
358c0a7
to
5d3a7d2
Compare
We already have |
adding an explicit |
Fwiw, the gitea project uses the version api endpoint for this in our monitoring |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as per tech ... use the version endpoint it does the same (return 200 & a tiny string)
It would only be a valid pull if it really do check health of the running instance (check DB, cache, storage backends, ...
yes, I tried the The version api does need authentication. So it does not meet the needs. returns I know how to build a request to pass the authentication. but this is not a when system op deploy the app, it does not and has no need to know the user account of the system or a token. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK for code
but we need documentation
@6543 some tips ? Sorry, I don't know where to add the documentation. Since it is not an API for client request, I think it is Ok not write swagger doc. |
yes swagger docs would be the wrong place :D -> https://github.com/go-gitea/gitea/tree/main/docs (just read the readme in there - it's a hugo repo) add it to https://docs.gitea.io/en-us/install-on-kubernetes/ ? |
Consider implementing a response as per https://datatracker.ietf.org/doc/html/draft-inadarei-api-health-check. I think should at minimum do a test against the database.
{
"status": "pass",
"releaseId": "1.16.0",
"description": "Gitea: Git with a cup of tea",
"checks": {
"database:ping": [
{
"status": "pass",
"time": "2022-01-17T03:36:48Z"
}
]
}
} |
LGTM if it's |
@wxiaoguang option 2 will works, but maybe a little confusing ? the diff between other micro service is, gitea need interactive installation to work for the first time. is there any automatic method to make this work in k8s without interactive installation? I mean something like a cli to allow fast installation (for example: |
Does Gitea actually need interactive installation the first time? I thought you could fill in the config and enable install lock and it would just work (run the migrations on first launch). |
|
It's not about if healthz endpoint being usable for k8s, for k8s there is helm chart that will make gitea usable without installation. Problem is that health endpoint is not usable for docker/docker swarm, at least from most used perspective install stage is most common scenario as we don't have easy way to skip it. |
I don't see much point in readyz endpoint as that would pretty much be same as |
Couldn't one |
Yes, that was my suggestion |
@lafriks @wxiaoguang would ttys3#1 be acceptable, then? |
This was needed for using /api/healthz endpoint in Docker healthchecks, otherwise, Docker would never become healthy if using healthz endpoint and users would not be able to complete the installation of Gitea.
Or maybe you just want |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small nit but otherwise looks good
# Conflicts: # routers/web/web.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, since there is no objection, let me take the remaining work:
- conflicts are resolved
- remove
CheckInstall
, useCheck
with InstallLock check instead. - set
Description
tosetting.AppName
instead of the hard-coded string.
* giteaofficial/main: Add health check endpoint (go-gitea#18465) Only check for non-finished migrating task (go-gitea#19601) Make .cs highlighting legible on dark themes. (go-gitea#19604)
* chore: add health check endpoint docs: update document about health check fix: fix up Sqlite3 ping. current ping will success even if the db file is missing fix: do not expose privacy information in output field * refactor: remove HealthChecker struct * Added `/api/healthz` to install routes. This was needed for using /api/healthz endpoint in Docker healthchecks, otherwise, Docker would never become healthy if using healthz endpoint and users would not be able to complete the installation of Gitea. * Update modules/cache/cache.go * fine tune * Remove unnecessary test code. Now there are 2 routes for installation (and maybe more in future) Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com> Co-authored-by: 6543 <6543@obermui.de> Co-authored-by: Marcos de Oliveira <marcossantos@furb.br>
Add
/api/healthz
for health check. Which does not need auth and is cloud native friendlyBy convention, most cases the path is
/healthz
or/_healthz
, I think we need this.https://kubernetes.io/docs/reference/using-api/health-checks/
https://www.nomadproject.io/docs/job-specification/service#header-stanza