ci: Check health of services after running integration tests

[ethanhs]

Fixes #1762

[chadwhitacre]

Some suggested improvements.

[chadwhitacre]

Whoa! Is it ... working? 🤔

self-hosted-1-snuba-replacer-1                             "./docker_entrypoint…"   snuba-replacer                             restarting          

https://github.com/getsentry/self-hosted/actions/runs/3877996309/jobs/6613667226#step:4:6875

Is the replacer supposed to restart?

[chadwhitacre]

Seeing snuba-replacer restarting again but not consistently. What fails on that second? 🤔

[ethanhs]

@chadwhitacre

Seeing snuba-replacer restarting again but not consistently. What fails on that second? 🤔

My guess is that it restarted but switched to a running state after we queried docker ps, so the second docker ps (the one we print) doesn't show the issue.

[BYK]

Not sure if this helps but Compose V2 has a --wait option that waits until all the containers are healthy: https://docs.docker.com/engine/reference/commandline/compose_up/

[chadwhitacre]

My guess is that it restarted but switched to a running state after we queried docker ps, so the second docker ps (the one we print) doesn't show the issue.

Seems plausible. Should we capture the output of the first call so we can output exactly what we're testing against? Related: I did try to find a $dc ps --format that would reduce the columns of output for easier reading (we only really care about name and state/status), but afaict docker-compose ps --format does not support templates the way docker ps does (of course). Maybe dump that json and work with it twice?

[ethanhs]

Maybe dump that json and work with it twice?

Yeah I'm leaning towards that right now as well.

[ethanhs]

Of course, I'm still not sure why snuba-replacer is restarting...

For using --wait, I think since geoipupdate always exit we can't use that?

[ethanhs]

OK, now printing the JSON of the failing/exited services.

[chadwhitacre]

I'm still not sure why snuba-replacer is restarting

Aye, there's the rub!

[ethanhs]

Found it! Batching was removed from replacer, added a fix for that.

[chadwhitacre]

So does that mean self-hosted has been broken for three weeks? We should add some tests to catch this sort of thing. Oh wait ... ;-)

[chadwhitacre]

I pushed 668ae2a1f867509556eee72f097e96dd6ec1cfce in order to evaluate the failure output formatting.

[chadwhitacre]

Here's what I'm seeing:

{
  "Service": "snuba-replacer",
  "State": "restarting"
}

How about using the @tsv filter?

https://stackoverflow.com/questions/39139107/how-to-format-a-json-string-as-a-table-using-jq

[chadwhitacre]

(I force-pushed 668ae2a1f867509556eee72f097e96dd6ec1cfce off again.)

[chadwhitacre]

Could we use the @tsv filter for better readability?

[chadwhitacre]

Heh, oops! With @tsv there is always output (at least the headers) so it always fails.

https://github.com/getsentry/self-hosted/actions/runs/3895765109/jobs/6651476439#step:4:6707

[chadwhitacre]

Sob and actually broken run won't blend.

jq: error (at <stdin>:1): object ({"Service":...) cannot be tsv-formatted, only array

https://github.com/getsentry/self-hosted/actions/runs/3895778334/jobs/6651506996#step:4:2661

[chadwhitacre]

Can you push a commit to show the error case?

[chadwhitacre]

b9f949c is expected to pass CI (right?).

[ethanhs]

b9f949c is expected to pass CI (right?).

Yes

[ethanhs]

I will push something that breaks but I wanted to make sure the expected success case works first :)

[ethanhs]

64d51a2 adds back the snuba breakage to test things

[ethanhs]

Sample run with TSV: https://github.com/getsentry/self-hosted/actions/runs/3896522967/jobs/6653150931

[hubertdeng123]

Can you add this comment to the PR description? For future reference so it's easier to see why it was deleted. Otherwise lgtm