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

Implementation of the Docker environment #833

Closed
wants to merge 2 commits into from
Closed

Implementation of the Docker environment #833

wants to merge 2 commits into from

Conversation

ajardin
Copy link

@ajardin ajardin commented Jul 16, 2018

Here is a first implementation of what has been requested in #802.

I made the pull request earlier in order to receive your feedbacks about the design itself. The documentation still has to be updated. Basically, I took the decision to create something very simple (SQLite without HTTPS) and that can be used to test Symfony by directly editing the project.

The installation process is currently composed of three steps:

And that's all! The browser will be automatically open once the application is ready.

@javiereguiluz, could you have a look at this and tell me what you think?

RUN \
curl -sS https://getcomposer.org/installer | php && \
mv composer.phar /usr/local/bin/composer && \
composer global require "hirak/prestissimo:dev-master" --no-suggest --optimize-autoloader --classmap-authoritative
Copy link
Member

Choose a reason for hiding this comment

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

not needed IMO. Flex already optimizes the downloading of packages to do it in parallel (and it might now play well with hirak/prestissimo if they both try to do it)

Copy link
Author

@ajardin ajardin Jul 16, 2018

Choose a reason for hiding this comment

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

You were right! It's no longer needed with the next version of Composer. I've tested with 1.7.0-RC when I saw the announcement and the time spent to install dependencies is exactly the same with or without hirak/prestissimo.

Makefile Outdated
##

composer: ## Install Composer dependencies from the "php" container
$(DOCKER_COMPOSE) exec php sh -c "composer install --optimize-autoloader"
Copy link
Contributor

@ker0x ker0x Jul 17, 2018

Choose a reason for hiding this comment

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

I don't know if it's a good idea to add --optimize-autoloader as an option since it's not recommended to enable it in a dev environment https://getcomposer.org/doc/articles/autoloader-optimization.md

Copy link
Author

Choose a reason for hiding this comment

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

From my understanding, there is no real issue with --optimize-autoloader as its purpose is to generate a classmap file to avoid filesystem checks. A thing quite interesting in term of performances while you are using OPcache.

If a class is not present in the classmap, the autoload will still request the filesystem to load it. You can find more details a little bit further in the documentation you linked (https://getcomposer.org/doc/articles/autoloader-optimization.md#trade-offs).

@ajardin
Copy link
Author

ajardin commented Jul 18, 2018

The postgres service has been added to the stack.

@javiereguiluz
Copy link
Member

I've tested this and i's great! I tested most of the make commands and they all worked great.

A minor comment: the first-time installation was too long because I think it compiled PHP. If this is true, could we instead install a minimal PHP binary for a modern PHP 7.2 version instead of building it from source? Thanks!

@frastel
Copy link
Contributor

frastel commented Jul 31, 2018

@javiereguiluz It doesn't compile. Docker has to build all required images before the application could be started. For the very first run this build takes quite some time as no layer exists in the cache yet and all needed PHP packages have to be downloaded (think of apt-get update && apt-get install php-cli when you want to provision a standard debian/ubuntu server). The rebuilds however could re-use layers and thus the installation of the required packages is ultra fast.

@javiereguiluz
Copy link
Member

I'm afraid my experience was different than this. I have literally thousands and thousands of compiling lines like this:

 cc -I. -I/usr/src/php/ext/zip -DPHP_ATOM_INC -I/usr/src/php/ext/zip/include -I/usr/src/php/ext/zip/main -I/usr/src/php/ext/zip -I/usr/local/include/php -I/usr/local/include/php/main -I/usr/local/include/php/TSRM -I/usr/local/include/php/Zend -I/usr/local/include/php/ext -I/usr/local/include/php/ext/date/lib -I/usr/src/php/ext/zip/lib -fstack-protector-strong -fpic -fpie -O2 -DHAVE_CONFIG_H -fstack-protector-strong -fpic -fpie -O2 -c /usr/src/php/ext/zip/lib/zip_unchange_data.c  -fPIC -DPIC -o lib/.libs/zip_unchange_data.o

Is there a way to have PHP extensions precompiled? Also, can we check if we are installing just the minimum number of extensions needed to run this app? Thanks!

Copy link

@maxhelias maxhelias left a comment

Choose a reason for hiding this comment

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

A minor comment: would a h2-proxy service not be relevant?

@ajardin
Copy link
Author

ajardin commented Jul 31, 2018

Hi @javiereguiluz, and thanks for your feedbacks!

As @frastel nicely explained, what you see is the "image build process" performed by Docker when required cache layers are not available your local machine. This is completely normal to have thousands of lines during that step:

The php service is built on top of an Alpine image which is the smallest PHP image available on the Docker Store (~80MB). And with the addition of Symfony/Composer requirements, the result is an image of ~180MB.

Here is the list of PHP compiled modules.

[PHP Modules]
Core
ctype
curl
date
dom
fileinfo
filter
ftp
hash
iconv
intl
json
libxml
mbstring
mysqlnd
openssl
pcre
PDO
pdo_pgsql
pdo_sqlite
Phar
posix
readline
Reflection
session
SimpleXML
sodium
SPL
sqlite3
standard
tokenizer
xml
xmlreader
xmlwriter
Zend OPcache
zip
zlib

[Zend Modules]
Zend OPcache

Finally, I made some additional adjustments to reduce the installation time (from ~4:20m to ~2:35m) by removing the assets compilation (already present in the repository) and moving to the preview version of Composer (to take advantage of the latest improvements).

@maxhelias : I don't think a h2 proxy is required here, but I let @javiereguiluz answer. 😉

.env.dist Outdated
@@ -10,9 +14,9 @@ APP_SECRET=67d829bf61dc5f87a73fd814e2c9f629

###> doctrine/doctrine-bundle ###
# Format described at http://docs.doctrine-project.org/projects/doctrine-dbal/en/latest/reference/configuration.html#connecting-using-a-url
# For a sqlite database, use: "sqlite:///%kernel.project_dir%/var/data.db"
# For a sqlite database, use: "sqlite:///%kernel.project_dir%/var/data.db?charset=utf8mb4"
Copy link
Member

Choose a reason for hiding this comment

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

utf8mb4 does not make sense for sqlite AFAIK. That's a MySQL thing

RUN \
curl -sS https://getcomposer.org/installer | php && \
mv composer.phar /usr/local/bin/composer && \
/usr/local/bin/composer self-update --preview
Copy link
Member

Choose a reason for hiding this comment

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

all that can be simplify: curl -sS https://getcomposer.org/installer | php -- --install-dir=/usr/local/bin --filename=composer --preview (and this will avoid downloading the phar twice)

@mykbas
Copy link

mykbas commented Aug 31, 2018

Ubuntu: tried to run phpunit - inside php container everything OK, running make phpunit outside gives an error for App\Tests\Controller\BlogControllerTest::testNewComment

Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'Hi, Symfony!'
+'
+            
+            
+                John Doe commented on
+                                Aug 31, 2018, 11:14 AM
+            
+            
+                Eros diam egestas libero eu vulputate risus. Sunt torquises imitari velox mirabilis medicinaes. Pellentesque et sapien pulvinar consectetur. Curabitur aliquam euismod dolor non ornare. Era brevis ratione est. Sed varius a risus eget aliquam. Ut suscipit posuere justo at vulputate. Morbi tempus commodo mattis. Ubi est barbatus nix.
+            
+        '

@ajardin
Copy link
Author

ajardin commented Sep 20, 2018

Do you have additional questions or concerns? If it's good enough for you, I'll start to write a small documentation.

@ajardin ajardin closed this Dec 16, 2018
@javiereguiluz
Copy link
Member

I'm sorry this PR stalled. I was waiting for more approvals from Docker experienced users. May I ask why did you close it? Note that we're still super interested in having Docker enabled for this repo. Thanks!

@ajardin
Copy link
Author

ajardin commented Dec 17, 2018

I closed it for several reasons. 😉

  • Because I thought you'll prefer to focus on SymfonyCloud for the local environment since the pull request hasn't received any feedback since September.
  • I'm not satisfied with this implementation as the Docker environment will be tightly coupled with the application. I personally prefer an approach where the environment is retrieved like a dependency.

Long story short, that's not a renouncement as I'm still looking for the best solution.

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

Successfully merging this pull request may close these issues.

7 participants