Problem/Motivation

Needed for test coverage for #2616488: [meta] MySQL 5.7 / MariaDb 10.1.* support.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

catch created an issue. See original summary.

catch’s picture

mikeryan’s picture

jhedstrom’s picture

Status: Active » Needs work
FileSize
3.5 KB

Here's a start at the MySQL 5.7 container. I was following https://dev.mysql.com/doc/mysql-apt-repo-quick-guide/en/#repo-qg-apt-ins....

I may have time later today to pick this up again, but posting here in the meantime in case somebody else wants to run with it.

jhedstrom’s picture

Trying to test this out locally, I've followed https://www.drupal.org/node/2487065, but prior to that, ran docker build -t drupalci/mysql-5.7 . from within the directory added by the patch above.

The ./drupalci init command seems to work fine in terms of specifying the mysql 5.7 container, but then when I run ./drupalci run simpletest it keeps trying to use the mysql 5.5 container instead of the new one.

elachlan’s picture

elachlan’s picture

Do we need to make changes in /configsets?

jhedstrom’s picture

Do we need to make changes in /configsets?

The config sets listed in that directory don't actually appear to be used anywhere (note, they are significantly out of date from what the test bot runs). I spoke with @Mixologic and he pointed me towards this settings file for how environments are actually spun up: https://bitbucket.org/drupalorg-infrastructure/drupal.org/src/633814e57c...

When running locally, you can (in theory) change which environment is running by typing ./drupalci init.

jhedstrom’s picture

FileSize
1.13 KB
4.63 KB

Progress!

This now fails to connect to the db server, but is using the new container.

I had to manually set the DCI_DBVersion variable using

./drupalci config:set DCI_DBVersion=mysql-5.7

jhedstrom’s picture

When I run docker build I see this error too:

invoke-rc.d: policy-rc.d denied execution of start.
invoke-rc.d: policy-rc.d denied execution of reload.
jhedstrom’s picture

FileSize
560 bytes
4.63 KB

For some reason the /opt/startup.sh script isn't actually running (even though during build it claims it is). When I first ran it manually, it claimed that ldata was no longer valid, so I've updated the script to use datadir. Now running it manually just fails to connect:

vagrant@vagrant-ubuntu-trusty-64:~/drupalci_testbot$ docker exec 23bad80a60b7 /bin/bash /opt/startup.sh
rebuilding /var/lib/mysql/ibdata1
2015-11-17 19:20:30 [ERROR]   The data directory '/var/lib/mysql/' already exist and is not empty.
2015-11-17 19:20:30 [WARNING] mysql_install_db is deprecated. Please consider switching to mysqld --initialize
151117 19:20:31 mysqld_safe Logging to '/var/lib/mysql/23bad80a60b7.err'.
151117 19:20:31 mysqld_safe Starting mysqld daemon with databases from /var/lib/mysql
151117 19:20:32 mysqld_safe mysqld from pid file /var/run/mysqld/mysqld.pid ended
netcat: connect to localhost port 3306 (tcp) failed: Connection refused
jhedstrom’s picture

FileSize
1.42 KB
4.52 KB

Some of the errors above were due to now-invalid options in my.cnf. With this, the mysql process properly starts, however, the web container still cannot connect.

elachlan’s picture

If you start the container on its own, can you connect locally?

jhedstrom’s picture

If you start the container on its own, can you connect locally?

No, but with the latest patch, the error is different (access denied error instead of the more general server doesn't exist). I'm out of time for today, but may have time tomorrow to push this forward.

elachlan’s picture

Make sure you go though and update "/var/lib/mysql/ibdata1" with "/var/lib/mysql/" in startup.sh

Specifically the if statement.

joelpittet’s picture

joelpittet’s picture

Status: Needs work » Needs review
jhedstrom’s picture

Make sure you go though and update "/var/lib/mysql/ibdata1" with "/var/lib/mysql/" in startup.sh

Isn't /var/lib/mysql/ibdata1 still the file it should be checking for?

isntall’s picture

I've made a small change, couple additions to this effort and put it into a branch.

  • isntall committed 6200bc5 on 2616490-mysql-5.7-environment authored by jhedstrom
    Issue #2616490 by jhedstrom: MySQL 5.7 environment
    
  • isntall committed d5a3889 on 2616490-mysql-5.7-environment
    Issue #2616490 by jhedstrom, isntall: MySQL 5.7 environment
    
basic’s picture

Status: Needs review » Reviewed & tested by the community

#20 looks good. Lets get this image built and run a few manual tests with our staging job.

Mixologic’s picture

Component: Code » Environments
elachlan’s picture

Project: DrupalCI: Test Runner » DrupalCI: Environments
Component: Environments » Code

Moved to DrupalCI Environments.

steinmb’s picture

Component: Code » PHP Containers

Close?

Mixologic’s picture

Component: PHP Containers » Database Containers
Mixologic’s picture

No, we havent been working on this as it hasnt bubbled up the priority chain.

Since drupal is supported on mysql 5.5, the main purpose would be to see if we had introduced any regressions that do not work on 5.7.

We're also working on ways to cut down the testing time currently as introducing another database at this point gives us many, many more testing options which results in added cost.

Im going to mark this postponed on #2609560: Base DCI containers off official containers as when we implement this, it really should be based off of https://hub.docker.com/_/mysql/

Mixologic’s picture

Status: Reviewed & tested by the community » Postponed
pwolanin’s picture

@Mixologic I did see a regression on 5.7 with a view failing to work (a custom view, but not very fancy) so I think this is somewhat urgent.

joelpittet’s picture

I'll echo @pwolanin, I had a custom view that needed some grouping changed and it failed because the strictness of the GROUP BY needing it's fields in the SELECT fields as well. Default views does this but there surely could be other queries out there.

For reference what I'm blabbering about this is the setting that's on by default:
https://dev.mysql.com/doc/refman/5.7/en/sql-mode.html#sqlmode_only_full_...

mfb’s picture

What's needed to make this bubble up the priority chain? Some of the drupalci images are more-or-less unmaintained for ~2 years now which means it has less real-world usefulness as a test environment and there could be security vulns etc.

Mixologic’s picture

What's needed to make this bubble up the priority chain?

If somebody wants to submit a working mysql 5.7 container that is built using the same build process as we use currently, then we'd get that in.

it has less real-world usefulness as a test environment

Testing on the lowest, oldest acceptable version is generally desireable in a testing environment to reveal regressions with versions we call supported.

there could be security vulns etc.

We're pretty much unconcerned whatsoever if there are vulnerabilities in the containers, because the testbots already execute arbitrary code, by design. The bots themselves are ephemeral and get torn down as soon as a test is finished, or 1hr:50min passes, whichever comes first, so, there's not a lot an attacker could do with them even if they got root on them.

mfb’s picture

does same build process mean basing it on ubuntu xenial + apt-get or basing it on the 5.7 container from https://hub.docker.com/_/mysql/?

mfb’s picture

Status: Postponed » Needs review

I pushed two commits here: https://github.com/mfb/drupalci_environments/commits/2616490-mysql-5.7-e...

The first creates a new mysql-5.7 environment analogous to mysql-5.5 but based on ubuntu:xenial. The second switches this over to using the debian-based mysql:5.7 image rather than ubuntu:xenial.

I think both should work about the same since they both currently provide MySQL 5.7.20

I made only minimal necessary changes to scripts and config files. I changed the sql_mode to ONLY_FULL_GROUP_BY,TRADITIONAL because ONLY_FULL_GROUP_BY is part of the default config so it's important to test this sql mode, and TRADITIONAL includes NO_ENGINE_SUBSTITUTION so we don't need to specify it.

Mixologic’s picture

Adding a related issue for db testing.

mfb’s picture

I think both should work about the same since they both currently provide MySQL 5.7.20

The one big difference I noticed between ubuntu:xenial and mysql:5.7 is that the offiicial mysql Dockerfile defines VOLUME /var/lib/mysql

  • isntall committed 66541e0 on 2616490-mysql-5.7-environment
    Issue #2616490 by jhedstrom, isntall: MySQL 5.7 environment
    
  • isntall committed e4295ef on 2616490-mysql-5.7-environment authored by jhedstrom
    Issue #2616490 by jhedstrom: MySQL 5.7 environment
    
  • mfb committed 6ce1c78 on 2616490-mysql-5.7-environment
    Issue #2616490 by mfb: MySQL 5.7 environment
    
  • mfb committed f69fdf2 on 2616490-mysql-5.7-environment
    Issue #2616490 and #2609560 by mfb: use mysql:5.7 base image
    
Mixologic’s picture

Thanks for the bump/reminder that you got this working, sorry its been taking me so long.

I built the 5.7 container off of the branch you provided and pushed it to docker hub.

Im currently running some tests to see if things still pass with this, which look good so far.

https://dispatcher.drupalci.org/job/drupalci_test_containers/931/console

Once the tests are passing, I'll need to modify all of the jenkins jobs to expect the new container, as well as add some new environment combinations with mysql-5.7 so that they show up on drupal.org. Most likely if we add this, it will not be for every single php version, and will probably only be for php 7.0 for now.

mfb’s picture

Great! You'll want to make sure the volume setup in mysql-5.7/run-server.sh works correctly w/ the new VOLUME command. I didn't do an end-to-end test in a working drupalci system :)

mfb’s picture

Whatever happened w/ this - Any chance of running tests in MySQL 5.7 on drupal.org?

Mixologic’s picture

Sorry, this got sorta lost in a shuffle of trying to sort out what to do about adding new environments to the testing matrix. The issue we're having now is contributors submitting patches, and then somebody else will come along and mash every button available, testing every last environment, driving up testing costs. So, adding another database currently has a risk of adding several more tests to the matrix, so I've been trying to come up with a solution for how to prevent that sort of thing. But yeah, I should def push this up the priority queue and just get it out there and solve the UX/overuse problem later.

Mixologic’s picture

Status: Needs review » Fixed

Okay, well, I was delaying deploying this to save on costs. Turns out there is a Kernel test in core that is running into a bad mysql5.5 performance issue with a particularly long query, and now that we've split up the test types into their own categories, this one kernel test runs all by its lonesome for about 5 minutes while the rest of the bot is asleep.

So, adding mysql5.7 allows us to avoid that, saving 5 minutes per test, and saving money.

That being said, Im only planning on releasing this in combination with php7.1 for now, as I dont want to unnecessarily blow up the number of combinations people can choose from.

This exists on d.o. for now, but it looks like core tests have some fails on it, so you'll probably want to use it for ad-hoc tests in the meantime, until core can get this working.

Thanks for the patches mfb. Sorry for such a long delay in making this available.

mfb’s picture

Are there issue(s) already for the core fails?

wiifm’s picture

Link to the latest 8.6.x test on MySQL 5.7 https://www.drupal.org/pift-ci-job/1011706

The 2 errors being related to

1) Drupal\KernelTests\Core\Database\SchemaTest::testSchemaChangePrimaryKey with data set "simple_primary_key" (array('test_field'), array('test_field_renamed'))
Drupal\Core\Database\DatabaseExceptionWrapper: SQLSTATE[42000]: Syntax error or access violation: 1171 All parts of a PRIMARY KEY must be NOT NULL; if you need NULL in a key, use UNIQUE instead: CREATE TABLE {test_table} (
`test_field` INT DEFAULT NULL, 
`other_test_field` INT DEFAULT NULL, 
PRIMARY KEY (`test_field`)
) ENGINE = InnoDB DEFAULT CHARACTER SET utf8mb4; Array
(
)

and

3) Drupal\KernelTests\Core\Database\SchemaTest::testSchemaChangePrimaryKey with data set "composite_primary_key_different_order" (array('other_test_field', 'test_field'), array('other_test_field', 'test_field_renamed'))
Drupal\Core\Database\DatabaseExceptionWrapper: SQLSTATE[42000]: Syntax error or access violation: 1171 All parts of a PRIMARY KEY must be NOT NULL; if you need NULL in a key, use UNIQUE instead: CREATE TABLE {test_table} (
`test_field` INT DEFAULT NULL, 
`other_test_field` INT DEFAULT NULL, 
PRIMARY KEY (`other_test_field`, `test_field`)
) ENGINE = InnoDB DEFAULT CHARACTER SET utf8mb4; Array
(
)

Unsure if there is a d.o issue for these already.

mfb’s picture

Could we also run the tests on Drupal 7? Hopefully tests will pass, but would be nice to verify.

Mixologic’s picture

mfb’s picture

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

mfb’s picture

Now that MySQL 5.7 is passing on Drupal 7 it would be nice to set it up as "issue testing default" environment, or at least a "tested on commit" environment. MySQL 5.5 is officially EOL as of 2019 :'(