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

env: Update docker volumes during wp-env start #24778

Merged
merged 2 commits into from Sep 1, 2020
Merged

Conversation

noahtallen
Copy link
Member

Description

Since WordPress 5.5 was recently released, I noticed that some of my 3rd party plugins which rely on the base WordPress image were not updating to the latest version. Even after pulling the image updates, WordPress did not update.

This adds two things to the wp-env start --update process:

  1. Remove the $hash_wordpress and $hash_tests-wordpress volumes. These cache the wordpress files, so these volumes need to be removed for the next step.
  2. Run docker-compose pull to get the latest image updates.

Step 1 is necessary because without it, the container will not see that any wordpress files have changed even after pulling and rebuilding the images.

How has this been tested?

Locally in wp-env with a 3rd party plugin which was out of date.

Screenshots

Types of changes

Enhancement

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@noahtallen noahtallen added [Type] Build Tooling Issues or PRs related to build tooling [Package] Env /packages/env labels Aug 25, 2020
@noahtallen noahtallen self-assigned this Aug 25, 2020
@github-actions
Copy link

github-actions bot commented Aug 25, 2020

Size Change: +3.33 kB (0%)

Total Size: 1.17 MB

Filename Size Change
build/annotations/index.js 3.67 kB +1 B
build/autop/index.js 2.82 kB +1 B
build/block-directory/index.js 7.99 kB +20 B (0%)
build/block-editor/index.js 126 kB +272 B (0%)
build/block-editor/style-rtl.css 10.8 kB +19 B (0%)
build/block-editor/style.css 10.8 kB +19 B (0%)
build/block-library/editor-rtl.css 8.52 kB +23 B (0%)
build/block-library/editor.css 8.52 kB +22 B (0%)
build/block-library/index.js 136 kB +2.67 kB (1%)
build/block-library/style-rtl.css 7.45 kB +28 B (0%)
build/block-library/style.css 7.46 kB +29 B (0%)
build/blocks/index.js 47.7 kB -2 B (0%)
build/compose/index.js 9.67 kB -1 B
build/core-data/index.js 12.3 kB +46 B (0%)
build/data-controls/index.js 1.29 kB +1 B
build/data/index.js 8.55 kB -2 B (0%)
build/date/index.js 5.38 kB +2 B (0%)
build/dom/index.js 4.48 kB +2 B (0%)
build/edit-navigation/index.js 11.7 kB +46 B (0%)
build/edit-post/index.js 304 kB +1 B
build/edit-site/index.js 17 kB +2 B (0%)
build/edit-widgets/index.js 11.9 kB +151 B (1%)
build/editor/index.js 45.3 kB -20 B (0%)
build/format-library/index.js 7.71 kB +3 B (0%)
build/is-shallow-equal/index.js 710 B -1 B
build/media-utils/index.js 5.32 kB -2 B (0%)
build/nux/index.js 3.4 kB -1 B
build/plugins/index.js 2.56 kB -1 B
build/shortcode/index.js 1.7 kB +1 B
build/url/index.js 4.06 kB +2 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/api-fetch/index.js 3.44 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/style-rtl.css 953 B 0 B
build/block-directory/style.css 952 B 0 B
build/block-library/theme-rtl.css 729 B 0 B
build/block-library/theme.css 730 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/components/index.js 200 kB 0 B
build/components/style-rtl.css 15.7 kB 0 B
build/components/style.css 15.7 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/edit-navigation/style-rtl.css 1.16 kB 0 B
build/edit-navigation/style.css 1.16 kB 0 B
build/edit-post/style-rtl.css 5.61 kB 0 B
build/edit-post/style.css 5.61 kB 0 B
build/edit-site/style-rtl.css 3.06 kB 0 B
build/edit-site/style.css 3.06 kB 0 B
build/edit-widgets/style-rtl.css 2.45 kB 0 B
build/edit-widgets/style.css 2.45 kB 0 B
build/editor/editor-styles-rtl.css 537 B 0 B
build/editor/editor-styles.css 539 B 0 B
build/editor/style-rtl.css 3.8 kB 0 B
build/editor/style.css 3.79 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 621 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/keyboard-shortcuts/index.js 2.52 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.12 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/primitives/index.js 1.41 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 13.9 kB 0 B
build/server-side-render/index.js 2.77 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/viewport/index.js 1.85 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

Copy link
Contributor

@Addison-Stavlo Addison-Stavlo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im not exactly sure the best way to test this, but running the wp-env start --update on this branch seems to still work as expected. The code seems to make sense in regard to removing the volumes, pulling the new ones, and adding the required commandOptions where necessary.

packages/env/lib/commands/start.js Outdated Show resolved Hide resolved
const directoryHash = path.basename( workDirectoryPath );

// Note: when the base docker image is updated, we want that operation to
// also update WordPress. Since we store wordpress/tests-wordpress files
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking

Since we store wordpress/tests-wordpress files as docker volumes, simply updating the image will not change those files

To clarify my understanding, the volumes aren't updated because we've cached them. I guess the caching makes sense because volumes are meant to be stateful, and we shouldn't be updating them regularly because data would be blown away.

Does that mean running --update should now clean all of our WordPress data?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the database is stored in a separate volume, so no data should be wiped!

I'm not entirely sure why we have volumes for the wordpress directories. I think it might be performance? @noisysocks would know more

Copy link
Contributor

@jeyip jeyip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks good to me. Would you be able to elaborate on how I could set up an environment that tests that an outdated plugin is updated?

I'd imagine I manually upload an outdated plugin, but I'm not sure how to roll back the WordPress image version as well.

+1 if everything looks good in testing

Co-authored-by: Addison Stavlo <Stavz01@gmail.com>
@noahtallen
Copy link
Member Author

Would you be able to elaborate on how I could set up an environment that tests that an outdated plugin is updated?

I'm not sure. You would have to have a dev environment for something which existed before the latest WordPress environment, that is still running WordPress 5.4, and does not specify a core source. So that excludes gutenberg (which maps its own core source), but might include a different plugin dev environment

@vindl
Copy link
Member

vindl commented Aug 27, 2020

I'd imagine I manually upload an outdated plugin, but I'm not sure how to roll back the WordPress image version as well.

Yes, for example you can find older releases of Gutenberg by browsing through history here. As for the WordPress image, based on the instructions here you can specify an exact version in .wp-env.override.json, and edit it to default before running the command?

@noahtallen
Copy link
Member Author

based on the instructions here you can specify an exact version in .wp-env.override.json, and edit it to default before running the command?

Unfortunately, this will not work. That will cause WordPress to be downloaded as a standalone source via github or a git file, but it does not affect the underlying docker image at all. So if you create an instance with an image for the first time, it will use the updated version of that image even if the wordpress version from the source is older.

@vindl
Copy link
Member

That will cause WordPress to be downloaded as a standalone source via github or a git file, but it does not affect the underlying docker image at all. So if you create an instance with an image for the first time, it will use the updated version of that image even if the wordpress version from the source is older.

What's the point of having the older source if it's not used then? 🤔

@noahtallen
Copy link
Member Author

What's the point of having the older source if it's not used then

All of the docker services need to be based on some image, and for wp-env we use the official WordPress image. That happens to include the latest version of WP, so you can use wp-env out of the box just fine without specifying a core source. Once you specify a core source, it is mounted directly as its own volume, which then takes the place of whatever the image is including by itself.

This specific PR is just solving the issue of the docker image itself being out of date and not updating to the latest version.

I think it would be more effort to try to use a different image conditionally if you have specified your own wordpress source than it is worth :) No one has really tried to make that happen, though

@jeyip
Copy link
Contributor

Im not exactly sure the best way to test this, but running the wp-env start --update on this branch seems to still work as expected. The code seems to make sense in regard to removing the volumes, pulling the new ones, and adding the required commandOptions where necessary.

I'm in the same situation as Addison. Running wp-env start --update still seems to work as expected.

To keep the conversation moving forward, it seems like the potential risks of merging this PR are limited because we're only replacing WordPress volumes. (Does that sound right @noahtallen ?) Do we see value in exploring exactly how to test a PR like this? If not, and if my understanding of the risk is correct, then I'm comfortable proceeding.

@noahtallen
Copy link
Member Author

One thing to verify would be to test that your data isn't deleted when running wp-env start --update. It shouldn't be, but it's worth a check. It will always delete the volume there so that should work to make sure there are no bad side effects.

I think this is safe to merge though 👍 It is easy to fix issues that arise, and wp-env isn't a tool for production environment.

@jeyip
Copy link
Contributor

One thing to verify would be to test that your data isn't deleted when running wp-env start --update. It shouldn't be, but it's worth a check

Thanks! I'm pretty sure I checked this when you responded here #24778 (comment), but I'll double-check now to be sure 🙂

@jeyip
Copy link
Contributor

One thing to verify would be to test that your data isn't deleted when running wp-env start --update

Confirmed that data isn't deleted. I also smoke tested the WordPress environment (edit post, edit site, frontend) and everything looks like it works as expected.

@Addison-Stavlo
Copy link
Contributor

One thing to verify would be to test that your data isn't deleted when running wp-env start --update

This was true on my end as well. 👍

@noahtallen noahtallen merged commit 13f8fda into master Sep 1, 2020
@noahtallen noahtallen deleted the env/update-images branch September 1, 2020 17:03
@github-actions github-actions bot added this to the Gutenberg 9.0 milestone Sep 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Env /packages/env [Type] Build Tooling Issues or PRs related to build tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants