Skip to content

Exclude existing "pluggable" content in case it's been updated #507

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

Merged
merged 1 commit into from
Jun 18, 2020

Conversation

tianon
Copy link
Member

@tianon tianon commented Jun 17, 2020

If any directories like "wp-content/plugins/akismet" already exist, they're likely from a previous install where only "wp-content" was persisted and are likely to be already updated, so we should leave them alone.

Closes #506

If any directories like "wp-content/plugins/akismet" already exist, they're likely from a previous install where only "wp-content" was persisted and are likely to be already updated, so we should leave them alone.
@yosifkit yosifkit merged commit 5a7623d into docker-library:master Jun 18, 2020
@yosifkit yosifkit deleted the exclude-extra-content branch June 18, 2020 17:00
docker-library-bot added a commit to docker-library-bot/official-images that referenced this pull request Jun 18, 2020
Changes:

- docker-library/wordpress@5a7623d: Merge pull request docker-library/wordpress#507 from infosiftr/exclude-extra-content
- docker-library/wordpress@5a0d85b: Exclude existing "pluggable" content in case it's been updated
@temama
Copy link

temama commented Jul 3, 2020

Could this be done optionally, please? This fix blocks my workaround for undesired VOLUME in Dockerfile.

Of course, ideally, I'm dreaming of a separate container/tag w/o VOLUME, but I understand it may require lots of additional work during new version preparations.

Thanks

@tianon
Copy link
Member Author

tianon commented Jul 4, 2020 via email

@temama
Copy link

temama commented Jul 4, 2020

@tianon I'm trying to achieve the below workflow (we are still working on it, so it's not in prod yet):

  • Our FE dev prepares themes, plugins, etc. locally, pushes them to GitLab, it builds a container with our changes copied to /usr/src/wordpress/...
  • We are using the same container at testing & prod envs but with different databases & "uploads" dirs

So I was expecting docker-entrypoint.sh is to copy everything from /usr/src/wordpress/ to /var/www/html (including "pluggable" content) on each container start. Like it's doing now when "/var/www/html" is empty

Also, as I said earlier, the best case for us is to have only "uploads" directory as a volume

@tianon
Copy link
Member Author

tianon commented Jul 6, 2020

That should work fine -- what I'd suggest is having a persistent volume / bind mount for the uploads directory, and either letting /var/www/html stay anonymous or using a tmpfs to force it to cull on redeploy (depending on what tool you're using to deploy - docker-compose vs docker stack deploy vs kubernetes etc):

  wp:
    image: our-wordpress-image
    tmpfs:
      - /var/www/html
    volumes:
      - uploads:/var/www/html/wp-content/uploads

@temama
Copy link

temama commented Jul 6, 2020

@tianon thanks a lot for quick action and solution. It looks pretty nice :)

@tianon
Copy link
Member Author

tianon commented Jul 6, 2020

Another solution you might find interesting would be to change Apache's DocumentRoot to be /usr/src/wordpress directly and adding your volume at /usr/src/wordpress/wp-content/uploads. Then you could even run the container with --read-only (to enforce that wp-content/uploads is the only writable directory) and there would be no copying necessary.

@y0hnn
Copy link

y0hnn commented Jul 9, 2020

Hello,
I confirm it would be great to have it optionally. It should have been tagged as a breaking change for this image. Our deployment was broken for 3 weeks, and we've found why with this PR.

We are building a Wordpress plugin containing ACF schema, from another application. Since this PR, the plugin is not updated anymore as we are doing :

FROM wordpress:5.4-fpm
COPY --from=builder /usr/src/app/wordpress/wp-content/plugins/acfschema /usr/src/wordpress/wp-content/plugins/acfschema

The plugin is never updated in our image at redeploy...

@tianon
Copy link
Member Author

tianon commented Jul 9, 2020

I'm sorry your deployments broke; can you provide a minimal set of deployment steps to reproduce your failure? If we can understand what you're trying to accomplish and how you're trying to accomplish it currently, we're more likely to be able to help.

@y0hnn
Copy link

y0hnn commented Aug 5, 2020

Hello @tianon,
Sorry for my late answer!
We are using a tool that generates ACF schemas based on JS files (it's https://github.com/smooth-code/smooth.js for more details).
So, with NodeJS, I will build a plugin containing new files. Once this plugin is built, I want it of course to be included in my Wordpress app. In my Dockerfile, I do :

FROM wordpress:5.4-fpm
COPY --from=builder /usr/src/app/wordpress/wp-content/plugins/acfschema /usr/src/wordpress/wp-content/plugins/acfschema

Now, as the plugin is existing of course, it's not updated anymore in the volume.

I've tried your solution based on tmpfs to not save a volume. This work, but it adds a new problem : if I want to use this volume for the CLI image. For example :

docker run -it --rm --volumes-from wordpress_1 --network container:wordpress_1 wordpress:cli search-replace localhost example.com --all-tables

It will break. (and it is normal as the volume is just tmpfs volume).

This change is definitively a breaking change. I imagine there is a lot of other users struggling to know why they deployment broke recently. It would be very good to trigger this behavior only with a context variable. For example, checking if a variable like OVERWRITE_EXISTING_WP_CONTENT is set to true before excluding these files. What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Question Akismet plugin - strange behavior
4 participants