Make WordPress Core

Opened 9 months ago

Last modified 5 weeks ago

#57896 new enhancement

Improve devcontainer + Codespaces support

Reported by: sergeybiryukov's profile SergeyBiryukov Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: has-patch
Focuses: Cc:

Description

Background: #57187.

Codespaces support was introduced in [55303] and adjusted in [55353].

@johnbillion raised some follow-up points in comment:23:ticket:57187:

  1. README.md needs to document the three environment choices now available to devs (Codespaces, local devcontainer, local environment).
  2. The devcontainer doesn't use the same environment for WordPress that you get when you follow the "Local development" instructions in the readme. We're going to end up with two confusingly separate but similar environments. Could and should the same environment be used to run WordPress within the devcontainer?
  3. setup.sh assumes the workspace directory name is /workspaces/wordpress-develop but the local directory name may differ. The ${localWorkspaceFolderBasename} variable needs to be passed in to that script from devcontainer.json.

Change History (20)

#1 @rmccue
3 months ago

@helen, @mdawaffe, @jeremyfelt and I all worked on Codespaces support a bit during WCUS contributor day, and there were a few key issues we noted in addition to the aforementioned ones:

  1. The WordPress version inside the container is latest stable, not the version from the repo. This is due to using the "official" WordPress Docker container, which bundles its own copy of WordPress.
  2. Opening the URL causes a redirect to https://...-443.app.github.dev/
  3. Using docker ps or docker logs doesn't show the running containers

We didn't fully debug it during contributor day due to the terrible wifi, but I picked it back up today and sorted a few of these. Namely:

  1. Mounting ../src:/var/www/html will override the WP version with the version in src/, fixing this issue
  2. Setting $_SERVER['HTTP_HOST'] = $_SERVER['HTTP_X_FORWARDED_HOST'] will fix this issue (copied from the Codespaces setup on Altis)
  3. The Codespace is incorrectly configured to use docker-in-docker. This allows you to run your own containers, but can't connect to the current ones, as the devcontainer terminal connects to the container specified in "service", i.e. app. (This is new config I'd not seen before.) Switching to use docker-outside-of-docker instead handles mounting the Docker binds correctly, and allows you to view running containers and logs properly.

I'll PR this patch soon.

@mdawaffe I think the behaviour you were seeing with docker-compose cli ... working was because it was starting a new CLI service.

I also tried switching over to use the main docker-compose we already have, but that causes errors for some reason - it's very hard to see in the build logs exactly why that happens.

Additionally, the cd in the setup.sh appears to be useless - pwd is already /workspaces/wordpress-develop anyway.

A few other things I think we should aim to improve:

  • Since we're using the wordpress container rather than our own, it's also not a devcontainer-specific container - some tools like npm are hence out-of-date. It may be worth making our own for this specific reason, even if we can't switch to using the base docker-compose.yml config.
  • The default username and password (admin/password) isn't displayed within the terminal. We could display both this and the mapped URL for ease-of-use
  • Installing Chromium is slowwww due to the vast number of dependencies. We should look to either incorporate it into a prebuild or make it an optional follow-up, imo
  • The postCreateCommand log is useless - you need to pop open the full creation log to actually see the Webpack build log
  • We should consider preloading some PHP extensions, perhaps by adding the `php` feature (although this also installs PHP, which isn't that useful)

This ticket was mentioned in PR #5115 on WordPress/wordpress-develop by rmccue.


3 months ago
#2

  • Keywords has-patch added
  • Switches from docker-in-docker to docker-outside-of-docker to fix docker CLI commands
  • Mounts /src into the container so that Codespaces *actually* runs trunk
  • Fixes the host mapping problem by using overridden host name
  • Removes unneeded, potentially-inaccurate cd call

Trac ticket: https://core.trac.wordpress.org/ticket/57896

rmccue commented on PR #5115:


3 months ago
#3

Neat, post-welcome-message is failing since I haven't PR'd here before :)

#4 follow-up: @mdawaffe
3 months ago

I think the behaviour you were seeing with docker-compose cli ... working was because it was starting a new CLI service.

Agreed. I don't remember the details or what all I tried, but I think the issue was that the initial containers were created in one configuration with respect to the devcontainer container (as children? as siblings?) and the newly created CLI container was created with another.

I also tried switching over to use the main docker-compose we already have, but that causes errors for some reason - it's very hard to see in the build logs exactly why that happens.

As best I can tell, docker-outside-of-docker isn't working with core's docker-compose.yml because of nginx:alpine, which doesn't have apt-get (see docker-outside-of-docker's install.sh, OS Support). Alpine uses apk. Switching to nginx:latest fixes that problem, though others crop up.

I'll drop a link on your PR to some changes that mostly work.

mdawaffe commented on PR #5115:


3 months ago
#5

Nice! The docker-outside-of-docker definitely looks better. I tried getting codespaces to use the base docker-compose.yml, and was partly successful:

https://github.com/rmccue/wordpress-develop/compare/improve-codespaces-somewhat...mdawaffe:wordpress-develop:improve-codespaces-somewhat

  1. Switches from nginx:alpine (no apt-get - it uses `apk`) to nginx:latest. See OS Support, source.
  2. Skips manual wp-cli install (the base docker-compose.yml includes a wp-cli image).
  3. Skips chromium install for now.
  4. Hacks around some weirdness I don't understand with the wp-cli container's volumes.
  5. Skips the wp-config.php generation and WP installation steps since npm run env:install doesn't behave correctly. Those need to be done manually with these changes for now:
    docker-compose run --rm cli config create --dbname=wordpress_develop --dbuser=root --dbpass=password --dbhost=mysql --path=/var/www/src --force
    docker-compose run --rm cli core install --title="WordPress Develop" --admin_user=admin --admin_password=password --admin_email=test@test.com --skip-email --url=http://localhost:8889
    
    (Note that npm run env:install does more than just those two things. We should get that script working in this environment and use it.)

#6 in reply to: ↑ 4 ; follow-up: @rmccue
3 months ago

Replying to mdawaffe:

Agreed. I don't remember the details or what all I tried, but I think the issue was that the initial containers were created in one configuration with respect to the devcontainer container (as children? as siblings?) and the newly created CLI container was created with another.

Yeah, the detail I'd missed is that when in dockerComposeFile mode, service indicates which container to run the terminal in - that's new behaviour since last time I tried Codespaces!

tl;dr of the behaviour: Codespaces is running docker-compose on the root VM, then your terminal/etc run inside the specified service via an extended compose file. To access the cntainers, you hence have to mount the Docker socket into that service; docker-outside-of-docker handles that, thankfully, although I only realised that after I'd finished implementing the old school hacks.

As best I can tell, docker-outside-of-docker isn't working with core's docker-compose.yml because of nginx:alpine, which doesn't have apt-get (see docker-outside-of-docker's install.sh, OS Support). Alpine uses apk. Switching to nginx:latest fixes that problem, though others crop up.

I actually did manage to get it to work in the end... -ish. Rather than using core's containers for the "main" (service) container, I'm using a container built off mcr.microsoft.com/devcontainers/base with an extended docker-compose, which means we don't need to modify the main file (but can override it if needed).

My experience with Codespaces and in-container development is that it's very useful to have a devtool-only container available for working tools. (You can also use the cli container by setting the command to sleep infinity, but it's a bit more janky, and doesn't give us an easy way to add additional tools.)

The problem I ended up with though is that MySQL is not accepting connections for root from 172.18.0.0/16; the config should be allowing % for the host so not sure what's going on there.

I'll PR this up too in a bit (my Codespace is crashy right now).

I'd recommend we commit either PR 5115 or something like it in the short term to fix the current experience while we look at using the other containers though.

This ticket was mentioned in PR #5119 on WordPress/wordpress-develop by rmccue.


3 months ago
#7

Building off #5115, this switches Codespaces to use the docker-compose.yml we include for core development.

This is currently broken as MySQL is refusing connections, even with the change to the bind-address in the command. Still working on how to fix that one.

cc @mdawaffe - note I've added an extra "devcontainer" container here, so it doesn't require changing from alpine for the nginx container. That said, still broken.

Trac ticket: https://core.trac.wordpress.org/ticket/57896

rmccue commented on PR #5119:


3 months ago
#8

Curiously, mysqladmin inside the container also does not connect:

$ docker compose exec -it mysql bash
bash-4.2# mysqladmin status
mysqladmin: connect to server at 'localhost' failed
error: 'Access denied for user 'root'@'localhost' (using password: NO)'

(Even when forcing --protocol=socket)

#9 in reply to: ↑ 6 ; follow-up: @mdawaffe
3 months ago

Replying to rmccue:

I actually did manage to get it to work in the end... -ish. Rather than using core's containers for the "main" (service) container, I'm using a container built off mcr.microsoft.com/devcontainers/base with an extended docker-compose, which means we don't need to modify the main file (but can override it if needed).

That's what I was going to try next. Thank you for beating me to it :)

The problem I ended up with though is that MySQL is not accepting connections for root from 172.18.0.0/16; the config should be allowing % for the host so not sure what's going on there.

Pretty sure this is not a bind issue. I ran into this as well: The MySQL container is not running its initialization:

(You can tell since the user.myd file is 0 bytes.)

It magically fixed itself when I committed my changes to my own fork rather than having uncommitted changes in yours. (Correlated not causal, I suspect.)

rmccue commented on PR #5119:


3 months ago
#10

Looks like my attempts to fix the problem was hidden by cached data on the volume per @mdawaffe on https://core.trac.wordpress.org/ticket/57896#comment:9

Changing the volume's name to force invalidation fixes this, or you can do some voodoo to delete the volume.

Anyway, that's fixed, and now there's a problem with the installer, which is that it waits forever on the container to be up - localhost:8889 isn't accessible from the container for some reason, but works fine via the browser, so there's some sort of networking issue there.

In the process, I updated the installer to insert the host hack from #5115 if it detects it's running in Codespaces, as well as permitting LOCAL_URL to be passed instead of just LOCAL_PORT.

rmccue commented on PR #5119:


3 months ago
#11

Fixed the networking issue by moving the devcontainer from the bridge network to the host network. You won't be able to do mysql -h mysql, but you can do curl localhost:8889 as you'd expect, so I think that's fine.

I also had to remove chromium as it's not available on the devcontainer distro's package manager (Ubuntu), and per https://core.trac.wordpress.org/ticket/57896#comment:1 it's very slow anyway.

#12 in reply to: ↑ 9 ; follow-up: @rmccue
3 months ago

Replying to mdawaffe:

Pretty sure this is not a bind issue. I ran into this as well: The MySQL container is not running its initialization:

I think in the end it was both a bind issue and data sticking around inside the volume, which I managed to flush out.

https://github.com/WordPress/wordpress-develop/pull/5119 should now be working as expected - I'd still say we stick with https://github.com/WordPress/wordpress-develop/pull/5115 as an immediate fix until 5119 has some more testing on it, but I'm not super fussed.

mdawaffe commented on PR #5119:


3 months ago
#13

The CLI container is broken for me in this PR: ls /var/www shows an empty directory.

This is because the CLI container is not created during the Codespace creation (well - one such container is created, but it immediately exits/stops). Instead, CLI containers are spun up as necessary for each CLI command. Since these containers are being created within the devcontainer container, and since we're "proxying" docker and docker.sock from the "real" host, we need to specify volume sources with the *host's* absolute directories, not the devcontainer container's directories (relative or absolute).

This fixes it for me: https://gist.github.com/mdawaffe/f46496261af01a408f93e2caabae512c

There may be a cleaner way to do that without changing the root docker-compose.yml, but then all CLI commands would have to be run as:

docker-compose -f docker-compose.yml -f .devcontainer/docker-compose.codespaces.yml run --rm cli …

Which would require a bunch of changes to tools/local-env/scripts/docker.js and tools/local-env/scripts/install.js.

This is also the reason that any npm run env:cli call yields:

WARN[0000] Found orphan containers ([wordpress-develop_devcontainer_1]) for this project. If you removed or renamed this service in your compose file, you can run this command with the --remove-orphans flag to clean it up.

The root docker-compose.yml doesn't know anything about the devcontainer service, so it thinks that container is orphaned. Perhaps those changes to the various local-env scripts are warranted. (Though env:stop shouldn't turn off the devcontainer container :)).

#14 in reply to: ↑ 12 @mdawaffe
3 months ago

Replying to rmccue:

I'd still say we stick with https://github.com/WordPress/wordpress-develop/pull/5115 as an immediate fix until 5119 has some more testing on it, but I'm not super fussed.

I like 5119 (though there's still a little wonkiness) more than 5115 because the non-core WordPress docker image makes me sad :) but I am likewise unfussed.

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


3 months ago

mdawaffe commented on PR #5119:


3 months ago
#16

There may be a cleaner way to do that without changing the root docker-compose.yml

Subjectively "cleaner" method here: https://gist.github.com/mdawaffe/1bf45cafaa6bd7115cbde93101fb8207

All(? I didn't try everything) npm run commands work (including stop, start, etc.) except npm run env:install (the most useful one :)) since LOCAL_URL is not set. npm run env:install still works within setup.sh, since that file sets the variable. I guess we could copy that code from setup.sh to install.js?

I'm not sure how important it is that npm run env:stop etc. work, but it feels nice: the codespace environment behaves more like the local dev environment.

rmccue commented on PR #5119:


3 months ago
#17

The CLI container is broken for me in this PR: ls /var/www shows an empty directory.

That is... strange. I'm not seeing that:

@rmccue ➜ /workspaces/wordpress-develop (unify-docker-compose-codespaces) $ docker-compose run --entrypoint /bin/bash cli
WARN[0000] Found orphan containers ([wordpress-develop_devcontainer_1]) for this project. If you removed or renamed this service in your compose file, you can run this command with the --remove-orphans flag to clean it up. 
root@13a841c7de49:/var/www# ls -al
total 1324
drwxrwxrwx+   10 wp_php root      4096 Aug 30 16:47 .
drwxr-xr-x     1 root   root      4096 Nov 15  2022 ..
drwxrwxrwx+    2 wp_php root      4096 Aug 24 18:30 .cache
drwxrwxrwx+    3 wp_php root      4096 Aug 30 16:50 .devcontainer
-rw-rw-rw-     1 wp_php root       454 Aug 24 18:30 .editorconfig
-rw-rw-rw-     1 wp_php root      1844 Aug 30 01:17 .env
-rw-rw-rw-     1 wp_php root       268 Aug 24 18:30 .eslintignore

I *am* seeing the orphan containers warning you note though due to the multiple file issue; worth noting that the full command Codespaces runs also includes a bunch of other files too to set various other mount points, so not sure if we need to account for that.

The issue in theory makes sense though given the switch from docker-in-docker to docker-outside-of-docker.

Subjectively "cleaner" method here: https://gist.github.com/mdawaffe/1bf45cafaa6bd7115cbde93101fb8207

It might make sense to have a $LOCAL_ROOT_DIR used in the main docker-compose to avoid needing to override this in our Codespaces file? I can see these getting out of sync pretty quickly, and seems like it might be useful in the generic case anyway.

All(? I didn't try everything) npm run commands work (including stop, start, etc.) except npm run env:install (the most useful one :)) since LOCAL_URL is not set. npm run env:install still works within setup.sh, since that file sets the variable. I guess we could copy that code from setup.sh to install.js?

There's probably no real reason not to just set LOCAL_URL in our devcontainer.json if we can - I think all the vars are already in scope anyway.

mdawaffe commented on PR #5119:


3 months ago
#18

I wonder if we're making things too complicated for ourselves.

How about we combine the existing setup (Docker-in-Docker) with your setup (separate devcontainer container)? We'd get the benefits of both: no host v. container path weirdness with Docker-outside-of-Docker so no need to make any changes to docker-compose.yml, and a simpler devcontainer configuration.

I tested that here:
https://github.com/WordPress/wordpress-develop/compare/trunk...mdawaffe:wordpress-develop:codespaces-did-experiment

The diff is pretty simple, and so far everything works the same as in local development: npm run env:start, docker ps, npm run env:install, npm run env:cli, npm run env:restart, npm run test:php, etc.

#19 @tnolte
6 weeks ago

Just chiming in here that, taking the current Dev Container setup as a loose example, I was able to setup a fully working Codespaces, and local Dev Container, setup where a specific version of WordPress is installed. My setup is for plugin development, and uses a Composer-based install of WordPress, however, the working solution should work properly with the wordpress-develop repo using it as the WordPress install. There were some required adjustments needed to both the docker-compose.yml file as well as the wp-config.php file being used to drive the site especially when running behind the Codespaces proxy.

My working bits are here:

I didn't take the time to read through all of the comments here in detail to account for any desired changes to the setup. I can give a crack at this and open up a PR with something that will hopefully work.

One additional nice benefit is that the local Dev Container can be used with Visual Studio Code but also other IDE by using the DevPod tool which has full support for Dev Containers. I have been able to successfully use PhpStorm for example with starting up the Dev Container via DevPod and then just using an SSH terminal to have the container in the PhpStorm terminal. JetBrains support for Dev Containers is in beta and is pretty lacking but this may be more integrated in the future.

Last edited 6 weeks ago by tnolte (previous) (diff)

@tnolte commented on PR #5119:


5 weeks ago
#20

Just noting that this setup is not a universally compatible Dev Container setup. This assumes that running the Dev Container is only every being done in Codespaces. This is an incorrect use of Dev Containers. Dev Containers can also be used locally only and this setup should allow for this. I have personally been completely dropping the use of wp-env because of it's lack of flexibility and it's inherit requirement that I have a NodeJS version installed on my local machine. The whole point of a Dev Container is to have the entire development environment self contained, which includes NodeJS. You can see what the changes I made here. https://github.com/forumone/wp-cfm/tree/develop/.devcontainer

Is wp-env really a sustainable development environment tool going forward? It is heavy always spinning up Dev & Testing environments. It's inflexible for any sort of plugin that integrates with anything outside of WordPress where you want to include other Docker services for the purposes of integration tests. A Dev Container setup easily provides both an online and local consistent development environment that is open to expansion and changes needed by any plugins.

Note: See TracTickets for help on using tickets.