Add support for running tests with docker-compose #300

sahilm · 2024-06-24T10:24:28Z

TODO:

Tests with replicas
~~Docs (holding off docs till support is complete)~~

morgo · 2024-06-24T13:17:00Z

This is looking really good! When you convert the replica tests, you can delete those dbdeployer workflows if you like. I just want to keep some of both running for a while just in case we've made any mistakes where tests aren't running.

There is also a related FR for improving the replication tests if you get to it: #291

Dockerfile

morgo · 2024-06-25T03:45:47Z

LGTM, but asking @kolbe for secondary review since he understands this stuff better.

sahilm · 2024-06-25T07:13:48Z

This is looking really good! When you convert the replica tests, you can delete those dbdeployer workflows if you like. I just want to keep some of both running for a while just in case we've made any mistakes where tests aren't running.

There is also a related FR for improving the replication tests if you get to it: #291

I have added the replica stuff to this PR because there were quite a few changes and fixing Validate minimum privileges for both source and replica in CI #291 is a side-effect of how I've set things up.
Changed up the compose file structure which adds a bit of duplication but simplifies the overall configuration (I think).
I vote to keep the dbdeployer stuff as-is so that we can compare for a while as you've mentioned.
I haven't added any docs yet and I plan to add it in a separate PR.

sahilm · 2024-06-25T07:19:18Z

scripts/buildandrun.sh

+go build ./cmd/spirit
+
+if [ -n "$REPLICA_DSN" ]; then
+  ./spirit --replica-dsn "$REPLICA_DSN" --host $HOST --username $USERNAME --password $PASSWORD --database=$DATABASE --table=$TABLE


Can we consider supporting MYSQL_DSN and REPLICA_DSN environment variables as overrides to the spirit CLI?

I'd prefer not having spirit behave specially in the presence of bizarre environment variables when we have command-line options that do the job just fine.

sahilm · 2024-06-25T07:20:43Z

compose/replication/compose.yml

+        condition: service_healthy
+    environment:
+      MYSQL_DSN: msandbox:msandbox@tcp(mysql)/test
+      REPLICA_DSN: rsandbox:rsandbox@tcp(mysql_replica)/


Must leave the database name in REPLICA_DSN blank because rsandbox has no privileges to access the DB. It's only a replication client.

See also: cashapp#187

Fixes cashapp#291

sahilm · 2024-06-25T08:14:02Z

compose/compose.yml

+        condition: service_healthy
+    environment:
+      HOST: mysql:3306
+      USERNAME: msandbox


We run spirit with the msandbox user which now has the minimum privilege level needed.

sahilm · 2024-06-25T09:01:18Z

compose/bootstrap.sql

+-- using the same password.
+create user if not exists tsandbox@'%' identified with caching_sha2_password by 'msandbox';
+grant R_MIGRATOR, R_REPLICATION to tsandbox@'%' ;
+grant references, super, process on *.* to tsandbox@'%'; -- used in tests


Must create this user with these additional privilege because they're used in tests.Still should be much better than using root or the earlier msandbox which was effectively root.

kolbe · 2024-06-25T17:08:51Z

My local attempt to run some of this is failing in a pretty surprising way! Maybe just an issue with cross-platform Docker stuff?

docker compose -f docker/compose.yml -f docker/8.0.28.yml -f docker/test.yml up --abort-on-container-exit --force-recreate

mysql-1  | 2024-06-25T16:58:10.142915Z 0 [ERROR] [MY-013183] [InnoDB] Assertion failure: fil0fil.cc:7628:file->n_pending_ios > 0 thread 140737102452480
mysql-1  | InnoDB: We intentionally generate a memory trap.
mysql-1  | InnoDB: Submit a detailed bug report to http://bugs.mysql.com/.
mysql-1  | InnoDB: If you get repeated assertion failures or crashes, even
mysql-1  | InnoDB: immediately after the mysqld startup, there may be
mysql-1  | InnoDB: corruption in the InnoDB tablespace. Please refer to
mysql-1  | InnoDB: http://dev.mysql.com/doc/refman/8.0/en/forcing-innodb-recovery.html
mysql-1  | InnoDB: about forcing recovery.
mysql-1  | 16:58:10 UTC - mysqld got signal 6 ;
mysql-1  | Most likely, you have hit a bug, but this error can also be caused by malfunctioning hardware.
mysql-1  | Thread pointer: 0x0
mysql-1  | Attempting backtrace. You can use the following information to find out
mysql-1  | where mysqld died. If you see no messages after this, something went
mysql-1  | terribly wrong...
mysql-1  | stack_bottom = 0 thread_stack 0x100000
mysql-1  | /usr/sbin/mysqld(my_print_stacktrace(unsigned char const*, unsigned long)+0x3d) [0x55555767957d]
mysql-1  | /usr/sbin/mysqld(print_fatal_signal(int)+0x303) [0x5555566f6453]
mysql-1  | /usr/sbin/mysqld(my_server_abort()+0x6e) [0x5555566f656e]
mysql-1  | /usr/sbin/mysqld(my_abort()+0xa) [0x55555767370a]
mysql-1  | /usr/sbin/mysqld(ut_dbg_assertion_failed(char const*, char const*, unsigned long)+0x326) [0x55555794ace6]
mysql-1  | /usr/sbin/mysqld(+0x253d825) [0x555557a91825]
mysql-1  | /usr/sbin/mysqld(fil_aio_wait(unsigned long)+0xef) [0x555557aa034f]
mysql-1  | /usr/sbin/mysqld(+0x2390dc0) [0x5555578e4dc0]
mysql-1  | /usr/sbin/mysqld(std::thread::_State_impl<std::thread::_Invoker<std::tuple<Detached_thread, void (*)(unsigned long), unsigned long> > >::_M_run()+0xc0) [0x5555578e5030]
mysql-1  | /usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0xbbb2f) [0x7fffff2b5b2f]
mysql-1  | /lib/x86_64-linux-gnu/libpthread.so.0(+0x7fa3) [0x7fffff7b8fa3]
mysql-1  | /lib/x86_64-linux-gnu/libc.so.6(clone+0x3f) [0x7ffffef95eff]
mysql-1  | The manual page at http://dev.mysql.com/doc/mysql/en/crashing.html contains
mysql-1  | information that should help you find out what is causing the crash.
mysql-1  | ERROR 2013 (HY000): Lost connection to MySQL server at 'reading initial communication packet', system error: 104

sahilm · 2024-06-25T21:50:59Z

My local attempt to run some of this is failing in a pretty surprising way! Maybe just an issue with cross-platform Docker stuff?

@kolbe 8.0.28 doesn't work on arm64. Oracle doesn't provide an arm64 build. All other versions should work fine on Mac laptops.

sahilm · 2024-06-25T22:24:31Z

@morgo @kolbe Do you need any changes? If not may I have approval to merge this.

kolbe

LGTM

sahilm force-pushed the sahilm/docker-compose-mysql-for-testing branch 2 times, most recently from 2259494 to b0adc32 Compare June 24, 2024 12:08

morgo reviewed Jun 24, 2024

View reviewed changes

Dockerfile Outdated Show resolved Hide resolved

sahilm requested a review from morgo June 25, 2024 00:00

morgo requested a review from kolbe June 25, 2024 03:45

sahilm force-pushed the sahilm/docker-compose-mysql-for-testing branch 3 times, most recently from 268511c to 39d29ea Compare June 25, 2024 07:11

sahilm commented Jun 25, 2024

View reviewed changes

sahilm added 2 commits June 25, 2024 17:28

Add support for running tests with docker-compose

a080ca8

See also: cashapp#187

Use the minimum privileges needed

44e3dcb

Fixes cashapp#291

sahilm force-pushed the sahilm/docker-compose-mysql-for-testing branch from 39d29ea to 44e3dcb Compare June 25, 2024 07:28

Tests expect root privileges

40e996e

sahilm commented Jun 25, 2024

View reviewed changes

sahilm added 2 commits June 25, 2024 18:41

Use a new test user in tests to reduce privileges

3a19ef9

Tests use these additional privileges

9e339fd

sahilm commented Jun 25, 2024

View reviewed changes

sahilm added 2 commits June 26, 2024 00:03

oops fix script args

9cfc4ad

use consistent lower-case in sql

e5fff3e

Merge branch 'main' into sahilm/docker-compose-mysql-for-testing

d8a3f52

kolbe approved these changes Jun 25, 2024

View reviewed changes

sahilm merged commit ff783a7 into cashapp:main Jun 25, 2024
11 checks passed

sahilm deleted the sahilm/docker-compose-mysql-for-testing branch June 25, 2024 23:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for running tests with docker-compose #300

Add support for running tests with docker-compose #300

sahilm commented Jun 24, 2024 •

edited

Loading

morgo commented Jun 24, 2024

morgo commented Jun 25, 2024

sahilm commented Jun 25, 2024 •

edited

Loading

sahilm Jun 25, 2024

kolbe Jun 25, 2024

sahilm Jun 25, 2024

sahilm Jun 25, 2024

sahilm Jun 25, 2024

kolbe commented Jun 25, 2024

sahilm commented Jun 25, 2024

sahilm commented Jun 25, 2024

kolbe left a comment

Add support for running tests with docker-compose #300

Add support for running tests with docker-compose #300

Conversation

sahilm commented Jun 24, 2024 • edited Loading

morgo commented Jun 24, 2024

morgo commented Jun 25, 2024

sahilm commented Jun 25, 2024 • edited Loading

sahilm Jun 25, 2024

Choose a reason for hiding this comment

kolbe Jun 25, 2024

Choose a reason for hiding this comment

sahilm Jun 25, 2024

Choose a reason for hiding this comment

sahilm Jun 25, 2024

Choose a reason for hiding this comment

sahilm Jun 25, 2024

Choose a reason for hiding this comment

kolbe commented Jun 25, 2024

sahilm commented Jun 25, 2024

sahilm commented Jun 25, 2024

kolbe left a comment

Choose a reason for hiding this comment

sahilm commented Jun 24, 2024 •

edited

Loading

sahilm commented Jun 25, 2024 •

edited

Loading