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.

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

Make database engine version available as a parameter #170

Closed
aspark21 opened this issue Jun 23, 2021 · 5 comments
Closed

Make database engine version available as a parameter #170

aspark21 opened this issue Jun 23, 2021 · 5 comments

Comments

@aspark21
Copy link
Contributor

aspark21 commented Jun 23, 2021

It seemed the version of MySQL and MariaDB were both very old / out of support. However after digging through the tags in Docker I now realise mysql:5 -> 5.7.34 and mariadb:10 ->10.5.10 which is all relatively recent.

The version is hardcoded though, and it would be useful to be able to execute this with mysql 8 for example.
https://github.com/moodlehq/moodle-docker/blob/master/db.mysql.yml#L8

MySQL 8.0 has been supported since Moodle 3.5 so all supported versions can handle it (since only 3.9+ are now in support - https://docs.moodle.org/dev/Releases#Version_support)

Thinking about it though there's two ways to go about it:

  1. New parameter which can define a specific image of mysql version for the current engine similarly to the PHP version parameter
  2. A second mysql image as an alternative database engine e.g. db.mysql8.yml which could be used by setting MOODLE_DOCKER_DB=mysql8

Any thoughts?
May provide a PR

@aspark21 aspark21 changed the title Increase database engine versions / make tt Increase database engine versions / make the version available as a parameter Jun 23, 2021
@aspark21 aspark21 changed the title Increase database engine versions / make the version available as a parameter Make database engine version available as a parameter Jun 23, 2021
@stronk7
Copy link
Member

stronk7 commented Jun 23, 2021

Yeah, we have a similar request also for moodle-ci-runner, that runs all our CI jobs, see MDLSITE-6376.

Surely both (they are different, but "parallel" enough) could be achieved in a similar way.

A) For some reason what I had been thinking was, more or less:

  1. Create new, unique, MOODLE_DOCKER_DB_VERSION (say, 8, or 13, or 5.7.12...) variable.
  2. If not set, each database will default to current values.
  3. If set, then use it as tag for MOODLE_DOCKER_DB image (mysql, postgres...) to use.
  4. Always look for db.MOODLE_DOCKER_DB.MOODLE_DOCKER_DB_VERSION (db.mysql8.yml, db.oracle19.yml...) and use them if they exist.
  5. Fallback to un-versioned db.MOODLE_DOCKER_DB (db.mysql.yml, db.oracle.yml...) if versioned doesn't exist.

I'm aware it's not the unique alternative. Maybe we could also:

B) Do everything with existing MOODLE_DOCKER_DB, making it to accept database:tag format.
C) Make a different MOODLE_DOCKER_DB_VERSION variable for each database.
D) ...

It would be really interesting to see if we can come with an agreement and get this implemented, yes!

Ciao :-)

@aspark21
Copy link
Contributor Author

aspark21 commented Jul 2, 2021

Got a version of it working - https://github.com/aspark21/moodle-docker/blob/mysql8/db.mysql8.yml
Had to remove 2 settings deprecated in 8 and add one for auth - master...aspark21:100c005241743552d2fe2a08157557b5c5f4fb54

For that reason, we're going to need a separate image.

Just verifying it runs properly and I'll submit a MR with that first step.

Having a look at moodle-ci-runner, the version change would need to take place here:
https://github.com/moodlehq/moodle-ci-runner/blob/master/runner/master/run.sh#L283
https://github.com/moodlehq/moodle-ci-runner/blob/master/runner/master/run.sh#L299

The deprecated parameters were already removed to enable mariadb
moodlehq/moodle-ci-runner@21dfcbc

So the main thing is wether or not --default-authentication-plugin=mysql_native_password is required.
This looks to have remained the default in mariadb & was only changed in mysql8.0
The setting itself has been around for a while so shouldn't be an issue to add that setting to the various mysql.cnf files for all versions&forks to use.

@stronk7
Copy link
Member

stronk7 commented Jul 12, 2022

Just for reference and similarities... we already added support to DB tags to moodle-ci-runner @ moodlehq/moodle-ci-runner#78.

@skodak
Copy link
Contributor

skodak commented Sep 27, 2022

I have added pull #225 which allows you to specify database version

@stronk7
Copy link
Member

stronk7 commented Oct 6, 2022

Hi @aspark21 ,

closing this in favour of #225, that is going to land soon.

Ciao :)

@stronk7 stronk7 closed this as completed Oct 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants