fix docker compose tests #5479
fix docker compose tests #5479
Conversation
3vilhamster
requested review from
Shaddoll,
neil-xie,
davidporter-id-au,
Groxx,
shijiesheng,
agautam478,
jakobht,
sankari165,
dkrotx,
taylanisikdemir and
demirkayaender
as code owners
docker/buildkite/docker-compose.yml
Outdated
| healthcheck: | ||
| test: ["CMD", "cqlsh", "-u cassandra", "-p cassandra" ,"-e describe keyspaces"] | ||
| interval: 15s | ||
| timeout: 10s |
There was a problem hiding this comment.
let's increase timeout to 30s to avoid flakiness in case cassandra bootstrap doesn't settle quickly when there's not enough cpu/memory
| - /cadence/.build/ # ensure we don't mount the build directory | ||
| - /cadence/.bin/ # ensure we don't mount the bin directory |
There was a problem hiding this comment.
does this.... hide the out-of-docker .build and .bin folders? any other consequences, e.g. reusing them between runs since volumes are generally intended to be persistent?
hiding seems useful and if that's all this does it's nice and simple, though we probably also want to hide the binaries to be a bit more complete (it'll influence build, and arch may be different)
There was a problem hiding this comment.
ah, TIL "anonymous volumes". so it sounds like this does reuse them between runs.
might be useful? it'll save re-downloading stuff, which is likely good. assuming the .build / .bin dirty-detection is good enough anyway, otherwise people will also have to know to delete these volumes...
if we really want this all to run quickly on local, we can also mount the GOPATH/pkg/mod folder, since that's platform-agnostic and accounts for the vast majority of the downloads. I'm not sure how to make use of that in buildkite tho.
There was a problem hiding this comment.
It not only rebuild stuff, it reruns generate, goimports, copyright. So no point on that.
And it is broken on local machine, unless you are running on linux x86.
I want to introduce GOPATH/pkg/mod cache in a separate pr. Maybe as a separate problem we can introduce volumes. Though, all our steps are running independently, so I doubt they are reusing volumes.
| @@ -86,8 +91,9 @@ services: | |||
| build: | |||
| context: ../../ | |||
| dockerfile: ./docker/buildkite/Dockerfile | |||
| command: make cover_profile | |||
| command: sh -c "make .just-build && make install-schema && make cover_profile" | |||
There was a problem hiding this comment.
poking at this locally, I kinda think we don't want make install-schema here. it fails if the schema already exists, which means you can't re-run unit tests without tossing and restarting the cassandra container :\
There was a problem hiding this comment.
This is not the tool that you use to run unit tests. This is a "close to CI" way to simulate what is running on buildkite, so I wanted it to behave as CI run. If it will be needed, we can separate it to an extra step, but it was useful to debug the problem
There was a problem hiding this comment.
I suppose in that scenario, people would have to drop cassandra each time? and that's reasonable because it matches CI?
which may be reasonable, this is in a buildkite folder after all. it's not a totally general thing. just checking / understanding.
| @@ -548,7 +548,7 @@ test_e2e_xdc: bins | |||
| go test $(TEST_ARG) -coverprofile=$@ "$$dir" $(TEST_TAG) | tee -a test.log; \ | |||
| done; | |||
|
|
|||
| cover_profile: bins | |||
There was a problem hiding this comment.
bins may be excessive, but this is broadly useful to ensure it builds before running all tests. otherwise it can get like 90% of the way through before failing on a syntax error or partial-refactor or something.
There was a problem hiding this comment.
We have a separate job that generates bins and verifies copyright and etc. If something is broken, I expect tests to fail. In this case bins generation/verification takes a lot of time.
There was a problem hiding this comment.
yea, fair. I suppose this is really only intended for CI, most people should be using make cover instead?
What changed?
Added health checks for CI runners with cassandra using healthcheck and depends_on: service_healthy.
Also, removed some unnecessary rebuilds/regenerations that were performed in the tests.
Also, removed extra steps from unit test executions and made it possible to run docker-compose like on CI locally.
Why?
Fixing test failures on CI
How did you test it?
Run locally
Potential risks
Failing builds on CI
Release notes
Documentation Changes