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.

Sign up for GitHub

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

fix docker compose tests #5479

Merged
merged 4 commits into from
Dec 16, 2023
Merged

fix docker compose tests #5479

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2865303
wait for cassandra healthy state on ci and local docker compose
3vilhamster Dec 15, 2023
d75b2ec
set docker network name and remove extra generation in all tests
3vilhamster Dec 15, 2023
07e6515
use depends_on healthy on CI and remove unused containers on lint
3vilhamster Dec 15, 2023
9641a99
increase timeout
3vilhamster Dec 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .buildkite/pipeline-master.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ steps:
limit: 1
plugins:
- docker-compose#v3.0.0:
run: unit-test
run: coverage-report
config: docker/buildkite/docker-compose.yml

- label: ":golang: integration test with cassandra"
Expand Down
38 changes: 27 additions & 11 deletions .buildkite/pipeline-pull-request.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ steps:
queue: "workers"
docker: "*"
commands:
- "sleep 30"
- "make .just-build" # ensure that we are not rebuilding binaries and not regenerating code
- "CASSANDRA_HOST=cassandra make install-schema && make cover_profile" # make install-schema is needed for a server startup test. See main_test.go
artifact_paths:
- ".build/coverage/*.out"
Expand All @@ -21,7 +21,7 @@ steps:
run: unit-test
config: docker/buildkite/docker-compose.yml

- label: ":golangci-lint: validate code is clean"
- label: ":lint: validate code is clean"
agents:
queue: "workers"
docker: "*"
Expand All @@ -32,14 +32,16 @@ steps:
limit: 1
plugins:
- docker-compose#v3.0.0:
run: unit-test
run: coverage-report
config: docker/buildkite/docker-compose.yml

- label: ":golang: integration test with cassandra"
agents:
queue: "workers"
docker: "*"
command: "make cover_integration_profile"
commands:
- "make .just-build" # ensure that we are not rebuilding binaries and not regenerating code
- "make cover_integration_profile"
artifact_paths:
- ".build/coverage/*.out"
retry:
Expand All @@ -54,7 +56,9 @@ steps:
agents:
queue: "workers"
docker: "*"
command: "make cover_integration_profile"
command:
- "make .just-build" # ensure that we are not rebuilding binaries and not regenerating code
- "make cover_integration_profile"
artifact_paths:
- ".build/coverage/*.out"
retry:
Expand All @@ -68,7 +72,9 @@ steps:
agents:
queue: "workers"
docker: "*"
command: "make cover_integration_profile"
commands:
- "make .just-build" # ensure that we are not rebuilding binaries and not regenerating code
- "make cover_integration_profile"
artifact_paths:
- ".build/coverage/*.out"
retry:
Expand All @@ -82,7 +88,9 @@ steps:
agents:
queue: "workers"
docker: "*"
command: "make cover_ndc_profile"
commands:
- "make .just-build" # ensure that we are not rebuilding binaries and not regenerating code
- "make cover_ndc_profile"
artifact_paths:
- ".build/coverage/*.out"
retry:
Expand All @@ -97,7 +105,9 @@ steps:
agents:
queue: "workers"
docker: "*"
command: "make cover_integration_profile"
commands:
- "make .just-build" # ensure that we are not rebuilding binaries and not regenerating code
- "make cover_integration_profile"
artifact_paths:
- ".build/coverage/*.out"
retry:
Expand All @@ -112,7 +122,9 @@ steps:
agents:
queue: "workers"
docker: "*"
command: "make cover_ndc_profile"
commands:
- "make .just-build" # ensure that we are not rebuilding binaries and not regenerating code
- "make cover_ndc_profile"
artifact_paths:
- ".build/coverage/*.out"
retry:
Expand All @@ -127,7 +139,9 @@ steps:
agents:
queue: "workers"
docker: "*"
command: "make cover_integration_profile"
commands:
- "make .just-build" # ensure that we are not rebuilding binaries and not regenerating code
- "make cover_integration_profile"
artifact_paths:
- ".build/coverage/*.out"
retry:
Expand All @@ -142,7 +156,9 @@ steps:
agents:
queue: "workers"
docker: "*"
command: "make cover_ndc_profile"
commands:
- "make .just-build" # ensure that we are not rebuilding binaries and not regenerating code
- "make cover_ndc_profile"
artifact_paths:
- ".build/coverage/*.out"
retry:
Expand Down
4 changes: 3 additions & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ test_e2e_xdc: bins
go test $(TEST_ARG) -coverprofile=$@ "$$dir" $(TEST_TAG) | tee -a test.log; \
done;

cover_profile: bins
Copy link
Member

@Groxx Groxx Dec 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea, fair. I suppose this is really only intended for CI, most people should be using make cover instead?

cover_profile:
$Q mkdir -p $(BUILD)
$Q mkdir -p $(COVER_ROOT)
$Q echo "mode: atomic" > $(UNIT_COVER_FILE)
Expand Down Expand Up @@ -599,12 +599,14 @@ cover_ci: $(COVER_ROOT)/cover.out $(BIN)/goveralls
$(BIN)/goveralls -coverprofile=$(COVER_ROOT)/cover.out -service=buildkite || echo Coveralls failed;

install-schema: cadence-cassandra-tool
$Q echo installing schema
3vilhamster marked this conversation as resolved.
Show resolved Hide resolved
./cadence-cassandra-tool create -k cadence --rf 1
./cadence-cassandra-tool -k cadence setup-schema -v 0.0
./cadence-cassandra-tool -k cadence update-schema -d ./schema/cassandra/cadence/versioned
./cadence-cassandra-tool create -k cadence_visibility --rf 1
./cadence-cassandra-tool -k cadence_visibility setup-schema -v 0.0
./cadence-cassandra-tool -k cadence_visibility update-schema -d ./schema/cassandra/visibility/versioned
$Q echo installed schema

install-schema-mysql: cadence-sql-tool
./cadence-sql-tool --user root --pw cadence create --db cadence
Expand Down
64 changes: 50 additions & 14 deletions docker/buildkite/docker-compose-local.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ services:
services-network:
aliases:
- cassandra
healthcheck:
test: ["CMD", "cqlsh", "-u cassandra", "-p cassandra" ,"-e describe keyspaces"]
interval: 15s
timeout: 30s
retries: 10

mysql:
image: mysql:8.0
Expand Down Expand Up @@ -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"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :\

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

environment:
- "CASSANDRA_HOST=cassandra"
- "CASSANDRA=1"
- "MYSQL=1"
- "POSTGRES=1"
Expand All @@ -98,12 +104,18 @@ services:
- "POSTGRES_USER=cadence"
- "POSTGRES_PASSWORD=cadence"
depends_on:
- cassandra
- mysql
- postgres
- mongo
cassandra:
condition: service_healthy
mysql:
condition: service_started
postgres:
condition: service_started
mongo:
condition: service_started
volumes:
- ../../:/cadence
- /cadence/.build/ # ensure we don't mount the build directory
- /cadence/.bin/ # ensure we don't mount the bin directory
networks:
services-network:
aliases:
Expand All @@ -115,17 +127,23 @@ services:
dockerfile: ./docker/buildkite/Dockerfile
command: make cover_integration_profile
environment:
- "CASSANDRA_HOST=cassandra"
- "CASSANDRA=1"
- "CASSANDRA_SEEDS=cassandra"
- "ES_SEEDS=elasticsearch"
- "KAFKA_SEEDS=kafka"
- "TEST_TAG=esintegration"
depends_on:
- cassandra
- elasticsearch
- kafka
cassandra:
condition: service_healthy
elasticsearch:
condition: service_started
kafka:
condition: service_started
volumes:
- ../../:/cadence
- /cadence/.build/ # ensure we don't mount the build directory
- /cadence/.bin/ # ensure we don't mount the bin directory
Comment on lines +145 to +146
Copy link
Member

@Groxx Groxx Dec 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Member

@Groxx Groxx Dec 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

networks:
services-network:
aliases:
Expand All @@ -149,6 +167,8 @@ services:
- kafka
volumes:
- ../../:/cadence
- /cadence/.build/ # ensure we don't mount the build directory
- /cadence/.bin/ # ensure we don't mount the bin directory
networks:
services-network:
aliases:
Expand All @@ -173,6 +193,8 @@ services:
- kafka
volumes:
- ../../:/cadence
- /cadence/.build/ # ensure we don't mount the build directory
- /cadence/.bin/ # ensure we don't mount the bin directory
networks:
services-network:
aliases:
Expand All @@ -184,17 +206,23 @@ services:
dockerfile: ./docker/buildkite/Dockerfile
command: make cover_integration_profile EVENTSV2=true
environment:
- "CASSANDRA_HOST=cassandra"
- "CASSANDRA=1"
- "CASSANDRA_SEEDS=cassandra"
- "ES_SEEDS=elasticsearch"
- "KAFKA_SEEDS=kafka"
- "TEST_TAG=esintegration"
depends_on:
- cassandra
- elasticsearch
- kafka
cassandra:
condition: service_healthy
elasticsearch:
condition: service_started
kafka:
condition: service_started
volumes:
- ../../:/cadence
- /cadence/.build/ # ensure we don't mount the build directory
- /cadence/.bin/ # ensure we don't mount the bin directory
networks:
services-network:
aliases:
Expand All @@ -206,17 +234,23 @@ services:
dockerfile: ./docker/buildkite/Dockerfile
command: make cover_ndc_profile
environment:
- "CASSANDRA_HOST=cassandra"
- "CASSANDRA=1"
- "CASSANDRA_SEEDS=cassandra"
- "ES_SEEDS=elasticsearch"
- "KAFKA_SEEDS=kafka"
- "TEST_TAG=esintegration"
depends_on:
- cassandra
- elasticsearch
- kafka
cassandra:
condition: service_healthy
elasticsearch:
condition: service_started
kafka:
condition: service_started
volumes:
- ../../:/cadence
- /cadence/.build/ # ensure we don't mount the build directory
- /cadence/.bin/ # ensure we don't mount the bin directory
networks:
services-network:
aliases:
Expand All @@ -240,6 +274,8 @@ services:
- kafka
volumes:
- ../../:/cadence
- /cadence/.build/ # ensure we don't mount the build directory
- /cadence/.bin/ # ensure we don't mount the bin directory
networks:
services-network:
aliases:
Expand Down
36 changes: 26 additions & 10 deletions docker/buildkite/docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ services:
services-network:
aliases:
- cassandra
healthcheck:
test: ["CMD", "cqlsh", "-u cassandra", "-p cassandra" ,"-e describe keyspaces"]
interval: 15s
timeout: 30s
retries: 10

mysql:
image: mysql:8.0
Expand Down Expand Up @@ -73,6 +78,7 @@ services:
context: ../../
dockerfile: ./docker/buildkite/Dockerfile
environment:
- "CASSANDRA_HOST=cassandra"
Groxx marked this conversation as resolved.
Show resolved Hide resolved
- "CASSANDRA=1"
- "MYSQL=1"
- "POSTGRES=1"
Expand All @@ -86,10 +92,14 @@ services:
- BUILDKITE_BUILD_ID
- BUILDKITE_BUILD_NUMBER
depends_on:
- cassandra
- mysql
- postgres
- mongo
cassandra:
condition: service_healthy
mysql:
condition: service_started
postgres:
condition: service_started
mongo:
condition: service_started
volumes:
- ../../:/cadence
- /usr/bin/buildkite-agent:/usr/bin/buildkite-agent
Expand All @@ -113,9 +123,12 @@ services:
- BUILDKITE_BUILD_ID
- BUILDKITE_BUILD_NUMBER
depends_on:
- cassandra
- elasticsearch
- kafka
cassandra:
condition: service_healthy
elasticsearch:
condition: service_started
kafka:
condition: service_started
volumes:
- ../../:/cadence
- /usr/bin/buildkite-agent:/usr/bin/buildkite-agent
Expand Down Expand Up @@ -195,9 +208,12 @@ services:
- BUILDKITE_BUILD_ID
- BUILDKITE_BUILD_NUMBER
depends_on:
- cassandra
- elasticsearch
- kafka
cassandra:
condition: service_healthy
elasticsearch:
condition: service_started
kafka:
condition: service_started
volumes:
- ../../:/cadence
- /usr/bin/buildkite-agent:/usr/bin/buildkite-agent
Expand Down