Fix wp-env docker-compose config with explicit users and passwords #29770
Conversation
youknowriad
added
[Tool] Env
|
Size Change: 0 B Total Size: 1.4 MB ℹ️ View Unchanged
|
| }, | ||
| volumes: [ 'mysql:/var/lib/mysql' ], |
There was a problem hiding this comment.
The volume for the mysql container should be retained. Removing this means any restart of the environment loses any existing data.
The clean command exists to wipe databases on the dev environment when needed.
There was a problem hiding this comment.
I think restarting the environment doesn't mean losing data, destroying the environment means that and for a dev environment, I think that's totally acceptable.
There was a problem hiding this comment.
Regardless, I restored volumes to see if it solves the permission issues
| @@ -176,26 +176,41 @@ module.exports = function buildDockerComposeConfig( config ) { | |||
| image: 'mariadb', | |||
| ports: [ '3306' ], | |||
| environment: { | |||
| MYSQL_ALLOW_EMPTY_PASSWORD: 'yes', | |||
| MYSQL_ROOT_PASSWORD: 'password', | |||
| MYSQL_DATABASE: 'wordpress', | |||
There was a problem hiding this comment.
Setting the database here, makes sure the container creates it (which was done previously in the wordpress image)
There was a problem hiding this comment.
This is also why we need a separate container for the database of the tests env.
|
Have folks tested pointing at the previous docker images to pinpoint where things changed? |
|
I think it changed in "latest" so probably not tagged yet. I think ideally, I'm going to end my day soon (and be AFK tomorrow) so I'm passing on the batton on this one. |
|
Don't know if this is useful information, just wanted to let you know that I tried the fix as it is now and it doesn't work for me. This is what I get now: I'm using just a plain directory with Some system info: OS: Linux |
|
I tested this locally with a .wp-env.json file and that did seem to address the error. I'm not sure of the most appropriate way to install this from a particular branch but the mechanism I used was just Is there a better way? |
|
@pbking By 'address the error' do you mean that it worked when you used a particular Also, if it helps, I just run |
|
Sure thing. I also realized that |
This was my .wp-env.json file I ran from this location. With the above patch linked via This is after It was likely specifying the version of CORE to use that got it to work in the first place. |
|
Interesting, I still get the same old error. Though tests seem to be getting the error that you mention. EDIT: Apparently MariaDB will not create a database if a volume already exists... Access denied happens because there is no database. After running So now I'm getting the same thing as @pbking, without |
Experimenting here: #29798 |
Found what I believe is the relevant PR: docker-library/official-images#9772 |
|
Looking at the
Not quite sure what that means for us, but I guess it's mostly relevant for these bits: gutenberg/packages/env/lib/build-docker-compose-config.js Lines 62 to 99 in e6d3f29 |
Ah, I might be mistaken. The gutenberg/packages/env/lib/wordpress.js Lines 86 to 88 in e6d3f29 |
So it seems like these lines: gutenberg/packages/env/lib/wordpress.js Lines 79 to 90 in e6d3f29 are used to add config settings like these to the respective gutenberg/packages/env/lib/config/config.js Lines 78 to 87 in e6d3f29 It seems like the more 'idiomatic' way that the WP Docker image provides to pass extra config settings is the Maybe we can use that, and drop the logic that attempts to set these via WP CLI. |
Could definitely look into this. We'd have to drop support for setting
IIRC |
I've wondered if it would make sense to rely on a single Docker image, instead of different ones for every environment. For example, we have separate phpunit and CLI instances -- it seems like we could bundle CLI/phpunit into the other services if we used a single Docker image. |
Yes absolutely! The |
|
Might be worth considering that Docker is meant to be used 1 process per container. If it is desired to have a particular PHP environment shared between images, that can be accomplished by creating a base image which is then extended for every process. Might not be the best idea to bundle everything into a single image. An article that talks a bit about this: https://developers.redhat.com/blog/2016/02/24/10-things-to-avoid-in-docker-containers/ |
|
Closing in favor of #29800. |
closes #29752
There has been some update to the WordPress default docker image with these two updates:
This PR fixes that by assigning explicit values for these, the issue though is that all pre-existing wp-env usage is broken including third-party and old branches.