This repository has been archived by the owner on Aug 1, 2024. It is now read-only.
feat: Added mysql 8 container in docker-compose #1097
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
farhanumar
changed the title
adzuci
reviewed
Jun 15, 2023
|
|
||
| # Ensure that the MySQL databases and users are created for all IDAs. | ||
| # (A no-op for databases and users that already exist). | ||
| echo -e "${GREEN}Ensuring MySQL 5.7 databases and users exist...${NC}" | ||
| docker-compose exec -T mysql57 bash -e -c "mysql -uroot mysql" < provision.sql | ||
| echo -e "${GREEN}Ensuring MySQL 8.0 databases and users exist...${NC}" | ||
| docker-compose exec -T mysql80 bash -e -c "mysql -uroot mysql" < provision-mysql80.sql |
There was a problem hiding this comment.
Spit balling, @jmbowman Could this provision script detect if a developer has data in mysql5.7 and not in mysql8.0 and then somehow copy the data between them? (aka magically make the shift)
Alternatively, could arch or arbi-bom help SRE understand what we did during the 5.7 migration and what parts we could replicate?
https://github.com/openedx/devstack/pulls?q=is%3Apr+mysql+5.7+is%3Aclosed+
|
I think this PR can be merged as is without impacting users of devstack and we can test out the MySQL 8 container by rebasing https://github.com/openedx/devstack/pull/1098/files on top of it and then testing locally or in a hosted devstack. |
adzuci
approved these changes
Jun 19, 2023
1 task
timmc-edx
added a commit
that referenced
this pull request
Jun 28, 2023
This issue was breaking provisioning early in the process. The images for mysql57 and mysql80 start off with users already present for some reason. The old provisioning script for mysql 5.7 (provision.sql) uses implicit user creation via the `GRANT` statement, which is no longer supported in mysql 8.0. Therefore, the `CREATE USER` statements in provision-mysql80.sql fail. The fix here is to change to `CREATE USER IF NOT EXISTS`. I've also deleted `drop-mysql-user.sql`. I suspect it was only used during testing to enable repeated tests of provisioning, but it's not referenced anywhere in the PR that adds it (#1097).
1 task
timmc-edx
added a commit
that referenced
this pull request
Jun 28, 2023
This issue was breaking provisioning early in the process. The volumes for mysql57 and mysql80 may still contain users from previous provisioning runs. The old provisioning script for mysql 5.7 (provision.sql) uses implicit user creation via the `GRANT` statement, which is no longer supported in mysql 8.0. Therefore, the `CREATE USER` statements in provision-mysql80.sql fail. The fix here is to change to `CREATE USER IF NOT EXISTS`. I've also deleted `drop-mysql-user.sql`. I suspect it was only used during testing to enable repeated tests of provisioning, but it's not referenced anywhere in the PR that adds it (#1097).
timmc-edx
reviewed
Jun 28, 2023
Comment on lines
+1
to
+2
| #docker-compose exec -T mysql80 bash -e -c "mysql -uroot mysql" < drop-mysql-user.sql | ||
| #docker exec -it edx.devstack.mysql80 mysql |
There was a problem hiding this comment.
[inform] I ended up removing this file in a later PR -- see #1112 for context. Happy to restore it if there's some use for it that I didn't understand!
nsprenkle
pushed a commit
that referenced
this pull request
Nov 21, 2023
This issue was breaking provisioning early in the process. The volumes for mysql57 and mysql80 may still contain users from previous provisioning runs. The old provisioning script for mysql 5.7 (provision.sql) uses implicit user creation via the `GRANT` statement, which is no longer supported in mysql 8.0. Therefore, the `CREATE USER` statements in provision-mysql80.sql fail. The fix here is to change to `CREATE USER IF NOT EXISTS`. I've also deleted `drop-mysql-user.sql`. I suspect it was only used during testing to enable repeated tests of provisioning, but it's not referenced anywhere in the PR that adds it (#1097).
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
MySQL 8 with credentials pointing to mysql 8
I've completed each of the following or determined they are not applicable: