Dockerfile and docker-compose update for php version 7.4 #536

arti0090 · 2021-04-01T06:19:50Z

No description provided.

lchrusciel · 2021-04-01T15:17:44Z

Dockerfile

@@ -76,9 +75,9 @@ WORKDIR /srv/sylius
 ARG APP_ENV=prod

 # prevent the reinstallation of vendors at every changes in the source code
-COPY composer.json composer.lock symfony.lock ./


are you sure this should be removed?

The composer.lock file is not needed anymore (since composer v2 as i remember?). The composer is creating it when building containers and modify directory on your machine (the composer.lock is created). I might be wrong with my thinking and the composer.lock file might be needed to prevent reinstalation.
When I checked manualy I made some changes in code of application, and run docker-compose build and the composer was running from cache ( no reinstallation of vendors happened).

If somebody with docker experience is looking here please give an opinion, I am still learning docker 😄

@arti0090 Thanks for your PR!

According to the composer docs here: https://getcomposer.org/doc/01-basic-usage.md#installing-with-composer-lock it is a good practice to commit your project's composer.lock file to version control. Also it makes sense to copy this file to Docker's context during image build, because composer then don't need to resolve the latest versions of a dependency but simply grabs the locked version of a dependency in the composer.lock file. This also ensures that you have a controlled Docker image build. Else it may happen that some dependencies changes during your Docker image build process which results in another behaviour.

So with the composer.lock file you can have controlled way of updating your dependencies, without it each composer install execution may install some other versions of a dependency which may produce some error in your application.

As far as I see this Sylius-Standard repo also does not include such a composer.lock file, so you need to make sure that you have run composer install (or composer create-project...) before you build a Docker image, else the Docker image build will fail with the message that "the composer.lock file does not exist...". See also #503 which proposes to remove composer.lock file from the .gitignore file.

Just a small question here, how would you run composer install if you rely on the PHP & Composer provided by the Docker image (which, as you said, will fail to built on first run b/c there is no composer.lock)?

thanks @ilimination for your review comment, unfortunately I don't think that composer.lock file can be left. As in Sylius there are is a matrix of version supported (like php 7.4 or 8.0 and different mysql's) and leaving composer lock might interfere with it. Speaking about splitting prod and dev in two - well te bo honest I struggle with my experience at the moment to make it right 😄 .

A fix for being able to build the Docker image before composer install (without an composer.lock file) was proposed here: #559

Including composer.lock in the Sylius-Standard repo might indeed interfere with the various PHP versions, but only providing instructions to Docker to COPY your own composer.lock file once it's initialized should not interfere.

I have added the line from #559 into the PR. Credits to you @tarnawski , hope this won't be a problem to use your code here? 😄

A more proper way will be merge #559 and then rebase your changes to be aline with the master, but as is a small improvement - take it as yours 😉

arti0090 · 2021-04-07T17:09:40Z

I am looking for some suggestions for this PR:

As I find out the quay.io is now private repositories thats why I deleted it. Is this an OK thing?
I have changed some lines for nginx to resolve automatically the IP's of containers. Still in my opinion there is an issue that (in my case) I need to wait for about 10minutes for nginx to work properly (otherwise I have 502 bad gateway error). Any solutions?

Any other suggestions are really appreciated 😄

tonicospinelli · 2021-05-26T01:32:29Z

docker-compose.yml

-        - quay.io/sylius/nodejs:latest
-        - quay.io/sylius/nginx:latest
-    image: quay.io/sylius/nodejs:latest
+    image: nodejs:latest


nodejs not is available image, the correct is node

I have changed any nodejs to node in files.

tonicospinelli · 2021-05-26T05:44:25Z

@arti0090 using docker I faced this prolonged response too. It's probably around php-fpm, the Nginx wait for it. Because of that you getting a Bad Gateway.

tonicospinelli · 2021-05-26T05:46:55Z

I'm trying to help with these little changes at: arti0090#1

arti0090 · 2021-05-26T06:05:01Z

@tonicospinelli I was making some investigations about nginx 502 error. It looks like the php container was working too slow, or being precise - nginx and php were starting at the same time but php was than starting doctrine scripts and also building assets. When it finished the nginx was able to show Sylius page with no trouble.

This reverts commit 45b276f.

gdarquie · 2021-05-28T08:51:08Z

I can not wait to see this PR published, thanks @arti0090 and all reviewers ;-)

lchrusciel · 2021-05-28T17:42:18Z

docker-compose.yml

+      # Quay does not work, should be replaced in future with f.e. ghcr.io
+      # cache_from:
+      #   - quay.io/sylius/php:latest
+      #   - quay.io/sylius/nodejs:latest
+      #   - quay.io/sylius/nginx:latest


Commented config (unless left for reference for users) should be removed - as it is just a dead "code"

lchrusciel · 2021-05-28T17:45:09Z

Thank you, @arti0090! 🎉

lchrusciel · 2021-05-28T17:47:43Z

Hey folks! Thanks for this amount of interest in Sylius docker - I hope we will be able to forge a better, more sustainable docker image for Sylius with you. Changes from this PR will be release in about ~2 weeks, until then you will have to use dev-master to take advantage of it.

3kynox · 2021-05-31T11:16:00Z

Hello, could it be simply updated to PHP8, as Sylius 1.10 brings it? Any love to Kubernetes as well could be planned / provided?

tonicospinelli · 2021-06-04T00:26:21Z

@lchrusciel I add some adjusts to increase the development experience over docker. Take a look on #568 when you have time 🤗

vasilvestre · 2021-08-13T16:20:33Z

Hello, could it be simply updated to PHP8, as Sylius 1.10 brings it? Any love to Kubernetes as well could be planned/provided?

I would like PHP 8 too, maybe we could create two tags: PHP 7.4 and 8.0 ?
So people could pick the version they need

vasilvestre · 2021-08-13T16:35:45Z

docker/nginx/conf.d/default.conf

+        fastcgi_pass $backendfpm;
+        # Comment the next line and uncomment the next to enable dynamic resolution (incompatible with Kubernetes);


Aren't they inversed ?

This PR was merged into the 1.9-dev branch. Discussion ---------- I think #536 has introduced a little error https://github.com/Sylius/Sylius-Standard/pull/536/files#diff-f583d11fcce9311b02b20054d21bc42d9c3ce8337322f0f4c66a16f36825f766R12-R13 Commits ------- 0cf0521 Fix inverted line

I think Sylius/Sylius-Standard#536 has introduced a little error https://github.com/Sylius/Sylius-Standard/pull/536/files#diff-f583d11fcce9311b02b20054d21bc42d9c3ce8337322f0f4c66a16f36825f766R12-R13

This PR was merged into the 1.9-dev branch. Discussion ---------- I think Sylius/Sylius-Standard#536 has introduced a little error https://github.com/Sylius/Sylius-Standard/pull/536/files#diff-f583d11fcce9311b02b20054d21bc42d9c3ce8337322f0f4c66a16f36825f766R12-R13 Commits ------- 0cf05217653166214397ce1a61c89b7f213bc517 Fix inverted line

arti0090 requested a review from a team as a code owner April 1, 2021 06:19

lchrusciel reviewed Apr 1, 2021

View reviewed changes

arti0090 force-pushed the docker-update branch from 16cab59 to 4c6e19d Compare April 7, 2021 16:45

arti0090 changed the title ~~Dockerfile update to php version 7.4~~ Dockerfile and docker-compose update for php version 7.4 Apr 7, 2021

arti0090 force-pushed the docker-update branch from 4c6e19d to 0025742 Compare April 7, 2021 17:05

arti0090 force-pushed the docker-update branch from 0025742 to 265d83e Compare April 14, 2021 09:44

olimination mentioned this pull request May 18, 2021

[Docker] Upgrade to PHP 7.4 #437

Closed

arti0090 added 2 commits May 19, 2021 13:14

Update dockerfile and docker-compose file to work with php 7.4

49976be

restore composer.lock for better performance built

45b276f

arti0090 force-pushed the docker-update branch from 265d83e to 45b276f Compare May 19, 2021 11:44

tonicospinelli reviewed May 26, 2021

View reviewed changes

arti0090 added 3 commits May 26, 2021 08:10

Revert "restore composer.lock for better performance built"

fe56227

This reverts commit 45b276f.

nodejs to node

71d3ce0

Add fix for composer files

084405a

arti0090 force-pushed the docker-update branch from 9d49069 to 084405a Compare May 26, 2021 11:09

tonicospinelli mentioned this pull request May 27, 2021

Speed up php-fpm container on MacOS #560

Closed

lchrusciel approved these changes May 28, 2021

View reviewed changes

lchrusciel changed the base branch from master to 1.9 May 28, 2021 09:10

lchrusciel changed the base branch from 1.9 to master May 28, 2021 09:10

lchrusciel approved these changes May 28, 2021

View reviewed changes

lchrusciel added Bug Docker Docker-related issues and PRs. Maintenance labels May 28, 2021

lchrusciel merged commit ba244da into Sylius:master May 28, 2021

This was referenced May 28, 2021

Fix copy composer.lock file in Dockerfile #559

Closed

Update Dockerfile COPY failed #490

Closed

vasilvestre mentioned this pull request Aug 13, 2021

Can't build docker #596

Closed

vasilvestre reviewed Aug 13, 2021

View reviewed changes

vasilvestre mentioned this pull request Sep 14, 2021

Fix inverted line in nginx configuration #607

Merged

lchrusciel mentioned this pull request Nov 19, 2021

WSL2 - Docker can't build PHP container #454

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dockerfile and docker-compose update for php version 7.4 #536

Dockerfile and docker-compose update for php version 7.4 #536

arti0090 commented Apr 1, 2021

lchrusciel Apr 1, 2021

arti0090 Apr 7, 2021

arti0090 Apr 7, 2021

olimination May 18, 2021 •

edited

Loading

cristi-contiu May 21, 2021

arti0090 May 26, 2021

cristi-contiu May 26, 2021

cristi-contiu May 26, 2021

arti0090 May 26, 2021

tarnawski May 26, 2021

arti0090 commented Apr 7, 2021

tonicospinelli May 26, 2021

arti0090 May 26, 2021

tonicospinelli commented May 26, 2021

tonicospinelli commented May 26, 2021

arti0090 commented May 26, 2021

gdarquie commented May 28, 2021

lchrusciel May 28, 2021

lchrusciel commented May 28, 2021

lchrusciel commented May 28, 2021

3kynox commented May 31, 2021

tonicospinelli commented Jun 4, 2021

vasilvestre commented Aug 13, 2021

vasilvestre Aug 13, 2021

		fastcgi_pass $backendfpm;
		# Comment the next line and uncomment the next to enable dynamic resolution (incompatible with Kubernetes);

Dockerfile and docker-compose update for php version 7.4 #536

Dockerfile and docker-compose update for php version 7.4 #536

Conversation

arti0090 commented Apr 1, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olimination May 18, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arti0090 commented Apr 7, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tonicospinelli commented May 26, 2021

tonicospinelli commented May 26, 2021

arti0090 commented May 26, 2021

gdarquie commented May 28, 2021

Choose a reason for hiding this comment

lchrusciel commented May 28, 2021

lchrusciel commented May 28, 2021

3kynox commented May 31, 2021

tonicospinelli commented Jun 4, 2021

vasilvestre commented Aug 13, 2021

Choose a reason for hiding this comment

olimination May 18, 2021 •

edited

Loading