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

Isolate all containers configuration #572

Draft
wants to merge 2 commits into
base: 1.12
Choose a base branch
from

Conversation

tonicospinelli
Copy link

@tonicospinelli tonicospinelli commented Jun 7, 2021

it depends #568

tonicospinelli and others added 2 commits June 3, 2021 14:52
Many MacOS users may be facing slow responses from Shop or Admin when
running the sylius application in the container. The main reason is how
docker was build in the MacOS architecture. The PHP application has
thousands of files to be loaded in memory (vendor directory mainly).

So, to decrease the page response time, the vendor directory is not
synced into the container. It will be ignored by docker-composer.yml
using the named volume.

Also, was added a health check for the PHP-fpm container to know when it
is ready. If you want to check healthiness, just run the command
`docker-compose ps` to see the container State.

The HEALTHCHECK instruction tells Docker how to test a container
to check that it is still working. This can detect cases such as a
web server stuck in an infinite loop and unable to handle new
connections, even though the server process is still running.

The PHP and PHP-FPM configurations received some adjustments to get
better support development environment experience using containers.

I did pick up these updates from two other contributors that I
co-authored in this commit.

Co-authored-by: Kévin Dunglas <[email protected]>
Co-authored-by: arti0090 <[email protected]>
\
apk del .build-deps
COPY --from=composer:latest /usr/bin/composer /usr/bin/composer
COPY docker/sylius/conf.d/* /usr/local/etc/php/conf.d

Choose a reason for hiding this comment

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

Target directory must end with /.

COPY --from=composer:latest /usr/bin/composer /usr/bin/composer
COPY docker/sylius/conf.d/* /usr/local/etc/php/conf.d
COPY docker/sylius/php-cli.ini.tmp /usr/local/etc/php/php-cli.ini.tmp
COPY docker/sylius/php-fpm.d/* /usr/local/etc/php-fpm.d

Choose a reason for hiding this comment

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

Target directory must end with /.

```bash
$ docker-compose pull
$ docker-compose build
$ docker-compose -f docker-compose.yml -f docker/docker-compose.setup.yml run setup

Choose a reason for hiding this comment

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

The --rm switch should be added tot he run command in order to remove the created container after finishing the setup. Currently if setup is run multiple times, it will create as many containers, leaving mess behind.

If the pages not load the assets, just run the below commands:

```bash
$ docker-compose run assets yarn install

Choose a reason for hiding this comment

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

--rm switch is needed here too.

- .:/srv/sylius:rw,cached
- ./public:/srv/sylius/public:rw,delegated
- public-media:/srv/sylius/public/media:rw
- sylius-vendor:/srv/sylius/vendor

Choose a reason for hiding this comment

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

Creating a named volume makes the vendor folder empty on the host machine. It's a problem, because your IDE can't see and the installed dependencies.

- .env
depends_on:
- sylius
command: "php ./bin/console sylius:install"

Choose a reason for hiding this comment

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

After successfully running this script, the assets are still missing on a fresh install. Probably the best would be to run the Yarn assets build command as well.

COPY docker/sylius/container-entrypoint.sh /usr/local/bin/container-entrypoint
RUN chmod +x /usr/local/bin/container-entrypoint
ENTRYPOINT ["container-entrypoint"]

Choose a reason for hiding this comment

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

It would be good to change the default user to the web app user by adding USER www-data. Currently when you run docker-compose exec sylius composer install it will create all files as root.

Also to avoid file permission issues the user ID of www-data should be changed to the ID of the host user. That way files can be edited on the host as well as by php-fpm.

@elvetemedve
Copy link

In case of a PHP-FPM error the output of the sylius container should display the error, but it only shows the 500 response code.
docker-compose logs -f sylius
image

@Ferror Ferror added the Docker Docker-related issues and PRs. label May 31, 2022
@diimpp
Copy link
Member

diimpp commented Nov 23, 2023

Hi @tonicospinelli, can you please elaborate why this change is necessary and what kind issue it's solving? Is it dev/prod feature?

@tonicospinelli
Copy link
Author

@diimpp, it has been over two years since I made these changes. The main idea was to isolate vendor and node_modules from the shared folder between the host and the container. It increases the response time for local tests because those folders have thousands of files to be synced and interpreted in runtime.

After this quick change, I saw an opportunity to isolate the configurations and build a process of sylius, php-fpm, node, and other configuration files. Today, reading all the PR to remember that I made, it's evident there must be multiple PRs about my changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Docker Docker-related issues and PRs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants