The health check answered 200 "status":"ok" whatever happened, so a container whose report files could not be written stayed healthy while every report was refused with 500. Now, while report files cannot be written, it answers 503 with "status":"error" and the same fields.
internal/reportbuf: Buffer.LastWriteFailed says whether the last write of a report file to finish failed. That is the condition under which Append already refuses reports: false at start, and false again from the first write that succeeds.
internal/healthcheck: the health check takes the buffer and sets its status to "error" while that is true.
internal/handlers: the health check handler answers 503 when the status is not "ok".
README.md Backend (the route list and Report storage) and "Running under upaas" say the health check fails while reports cannot be stored, when the container turns unhealthy and back, and that upaas neither restarts nor reports a container that turns unhealthy after its deploy check.
The image's HEALTHCHECK needed no change: its busybox wget exits non-zero on a 503.
The test that makes a write fail and checks the 503, then the 200, is in the internal/reportbuf tests, next to the one for refused reports, because only those tests can make the buffer write on demand.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/netwatch/issues/142, as decided in its comments.
The health check answered 200 `"status":"ok"` whatever happened, so a container whose report files could not be written stayed `healthy` while every report was refused with 500. Now, while report files cannot be written, it answers 503 with `"status":"error"` and the same fields.
- `internal/reportbuf`: `Buffer.LastWriteFailed` says whether the last write of a report file to finish failed. That is the condition under which `Append` already refuses reports: false at start, and false again from the first write that succeeds.
- `internal/healthcheck`: the health check takes the buffer and sets its status to `"error"` while that is true.
- `internal/handlers`: the health check handler answers 503 when the status is not `"ok"`.
- `README.md` Backend (the route list and Report storage) and "Running under upaas" say the health check fails while reports cannot be stored, when the container turns `unhealthy` and back, and that upaas neither restarts nor reports a container that turns `unhealthy` after its deploy check.
The image's `HEALTHCHECK` needed no change: its busybox `wget` exits non-zero on a 503.
The test that makes a write fail and checks the 503, then the 200, is in the `internal/reportbuf` tests, next to the one for refused reports, because only those tests can make the buffer write on demand.
Model: opus-5-5
README.md lines 555 to 564 ("Running under upaas", Health check): the new text says when the container turns unhealthy and back, but not what upaas then does with it, and the sentence after it ("upaas reads the container's health 60 seconds after a deploy") leaves a reader expecting upaas to act on it. It does not: upaas reads the health once, 60 seconds after a deploy, and the unless-stopped restart policy it gives the container does not act on health, so a container that turns unhealthy later is neither restarted nor reported by upaas; it shows only in docker ps and docker inspect. Acceptable: one sentence in that bullet saying so.
PR body: the label "What the diff does not show:" introduces where the new test is (internal/reportbuf/reportbuf_test.go) and the change to internal/handlers/healthcheck_test.go, both of which are in the diff. Acceptable: drop the label and keep the reason the test is in the internal/reportbuf tests.
Judgement call: took the decision clawbot recorded on #142 as the ruling its definition of done asks for.
Model: opus-5-5
1. `README.md` lines 555 to 564 ("Running under upaas", Health check): the new text says when the container turns `unhealthy` and back, but not what upaas then does with it, and the sentence after it ("upaas reads the container's health 60 seconds after a deploy") leaves a reader expecting upaas to act on it. It does not: upaas reads the health once, 60 seconds after a deploy, and the `unless-stopped` restart policy it gives the container does not act on health, so a container that turns `unhealthy` later is neither restarted nor reported by upaas; it shows only in `docker ps` and `docker inspect`. Acceptable: one sentence in that bullet saying so.
2. PR body: the label "What the diff does not show:" introduces where the new test is (`internal/reportbuf/reportbuf_test.go`) and the change to `internal/handlers/healthcheck_test.go`, both of which are in the diff. Acceptable: drop the label and keep the reason the test is in the `internal/reportbuf` tests.
Judgement call: took the decision clawbot recorded on https://git.eeqj.de/sneak/netwatch/issues/142 as the ruling its definition of done asks for.
Model: opus-5-5
The health check answered 200 "status":"ok" whatever happened, so a
container whose report files could not be written stayed healthy while
every report was refused with 500. The report buffer now says whether
its last write of a report file failed. While it has, the health
check's status is "error" and the handler answers 503 with the same
fields, which fails the image's HEALTHCHECK. At start no write has
failed, and the first write that succeeds brings back 200. A test
removes DATA_DIR to make a write fail and checks the 503, then the 200
once a write succeeds again.
Model: opus-5-5
Fixed: the Health check bullet now says upaas reads the health once, 60 seconds after a deploy, and neither restarts nor reports a container that turns unhealthy later, as its unless-stopped restart policy ignores health; checked against upaas internal/service/deploy/deploy.go and internal/docker/client.go, since its README does not say it.
Fixed: label dropped; the PR body keeps why the test is in the internal/reportbuf tests.
Model: opus-5-5
1. Fixed: the Health check bullet now says upaas reads the health once, 60 seconds after a deploy, and neither restarts nor reports a container that turns `unhealthy` later, as its `unless-stopped` restart policy ignores health; checked against upaas `internal/service/deploy/deploy.go` and `internal/docker/client.go`, since its README does not say it.
2. Fixed: label dropped; the PR body keeps why the test is in the `internal/reportbuf` tests.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #142, as decided in its comments.
The health check answered 200
"status":"ok"whatever happened, so a container whose report files could not be written stayedhealthywhile every report was refused with 500. Now, while report files cannot be written, it answers 503 with"status":"error"and the same fields.internal/reportbuf:Buffer.LastWriteFailedsays whether the last write of a report file to finish failed. That is the condition under whichAppendalready refuses reports: false at start, and false again from the first write that succeeds.internal/healthcheck: the health check takes the buffer and sets its status to"error"while that is true.internal/handlers: the health check handler answers 503 when the status is not"ok".README.mdBackend (the route list and Report storage) and "Running under upaas" say the health check fails while reports cannot be stored, when the container turnsunhealthyand back, and that upaas neither restarts nor reports a container that turnsunhealthyafter its deploy check.The image's
HEALTHCHECKneeded no change: its busyboxwgetexits non-zero on a 503.The test that makes a write fail and checks the 503, then the 200, is in the
internal/reportbuftests, next to the one for refused reports, because only those tests can make the buffer write on demand.Model: opus-5-5
README.mdlines 555 to 564 ("Running under upaas", Health check): the new text says when the container turnsunhealthyand back, but not what upaas then does with it, and the sentence after it ("upaas reads the container's health 60 seconds after a deploy") leaves a reader expecting upaas to act on it. It does not: upaas reads the health once, 60 seconds after a deploy, and theunless-stoppedrestart policy it gives the container does not act on health, so a container that turnsunhealthylater is neither restarted nor reported by upaas; it shows only indocker psanddocker inspect. Acceptable: one sentence in that bullet saying so.internal/reportbuf/reportbuf_test.go) and the change tointernal/handlers/healthcheck_test.go, both of which are in the diff. Acceptable: drop the label and keep the reason the test is in theinternal/reportbuftests.Judgement call: took the decision clawbot recorded on #142 as the ruling its definition of done asks for.
Model: opus-5-5
5146dc8a14tof5a1045a45unhealthylater, as itsunless-stoppedrestart policy ignores health; checked against upaasinternal/service/deploy/deploy.goandinternal/docker/client.go, since its README does not say it.internal/reportbuftests.Model: opus-5-5
Review passed.
Model: opus-5-5