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

Implement docker #931

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .env
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ 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 MySQL database, use: "mysql://db_user:db_password@127.0.0.1:3306/db_name"
# If you want to use Docker MySQL, use: "mysql://symfony-demo:symfony-demo@db:3306/symfony-demo"
# Configure your db driver and server_version in config/packages/doctrine.yaml
DATABASE_URL=sqlite:///%kernel.project_dir%/data/database.sqlite
###< doctrine/doctrine-bundle ###
Expand Down
4 changes: 4 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
/public/build/fonts/glyphicons-*
/public/build/images/glyphicons-*

###> docker ###
docker-compose.override.yml
###< docker ###

###> symfony/framework-bundle ###
/.env.local
/.env.*.local
Expand Down
1 change: 1 addition & 0 deletions .php_cs.dist
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ COMMENT;
$finder = PhpCsFixer\Finder::create()
->in(__DIR__)
->exclude('config')
->exclude('src/Migrations')
->exclude('var')
->exclude('public/bundles')
->exclude('public/build')
Expand Down
181 changes: 181 additions & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,181 @@
ARG NODE_VERSION=11.6.0
ARG COMPOSER_VERSION=1.8.0
ARG PHP_VERSION=7.2.13
ARG ICU_VERSION=63.1
ARG APCU_VERSION=5.1.16
ARG XDEBUG_VERSION=2.6.1


#####################################
## APP ##
#####################################
FROM php:${PHP_VERSION}-fpm as app

ARG ICU_VERSION
ARG APCU_VERSION

# Used for the ICU compilation
ENV PHP_CPPFLAGS="${PHP_CPPFLAGS} -std=c++11"
ENV APP_VERSION=0.0.0
Copy link
Contributor

Choose a reason for hiding this comment

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

If we don't need this envs at runtime, we can set their values in the RUN directive:

RUN export PHP_CPPFLAGS="${PHP_CPPFLAGS} -std=c++11" ENV APP_VERSION=0.0.0 \
    set -ex; \
    # Install required system packages
    apt-get update; \
    apt-get install -qy --no-install-recommends \
    ...

See https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#env

Copy link
Author

Choose a reason for hiding this comment

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

I agree, it's interesting :-) I do it ;-)


WORKDIR /app

# Install paquet requirements
RUN set -ex; \
# Install required system packages
apt-get update; \
apt-get install -qy --no-install-recommends \
zlib1g-dev \
git \
; \
# Compile ICU (required by intl php extension)
curl -L -o /tmp/icu.tar.gz http://download.icu-project.org/files/icu4c/${ICU_VERSION}/icu4c-$(echo ${ICU_VERSION} | sed s/\\./_/g)-src.tgz; \
tar -zxf /tmp/icu.tar.gz -C /tmp; \
cd /tmp/icu/source; \
./configure --prefix=/usr/local; \
make clean; \
make; \
make install; \
#Install the PHP extensions
Copy link

Choose a reason for hiding this comment

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

I'd split it as separate RUN step, any mistake in app-specific set of docker-php-ext requires full rebuild of this later which takes ages.

Copy link
Author

Choose a reason for hiding this comment

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

I've add all of it in a single layout to follow Dockerfile best practices that recommend to do the less number of layer as possible. Then, when you want to add something in your image, you can split it to test before, then regroup it after ?

I think, it's not totaly usefull to split this part, because we just need the split sometimes when you want to change something, but otherwise we never change anything.

That's my opinion. But I understand what you say, because when I edit this part I split it for try, because this part (with ICU) is very long.

Wait coments, but if more people prefer, we can split after the ICU install.

docker-php-ext-configure intl --with-icu-dir=/usr/local; \
docker-php-ext-install -j "$(nproc)" \
intl \
pdo \
# pdo_mysql \ Uncomment it to use MySQL, and remove the pdo_sqlite (see: docker-compose.yml, docker-compose.override.yml.dist)
zip \
bcmath \
; \
pecl install \
apcu-${APCU_VERSION} \
; \
docker-php-ext-enable \
opcache \
apcu \
; \
docker-php-source delete; \
# Clean aptitude cache and tmp directory
apt-get clean && rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/*;

## set recommended PHP.ini settings
RUN { \
echo 'date.timezone = Europe/Paris'; \
echo 'short_open_tag = off'; \
echo 'expose_php = off'; \
echo 'error_log = /proc/self/fd/2'; \
echo 'memory_limit = 128m'; \
echo 'post_max_size = 110m'; \
echo 'upload_max_filesize = 100m'; \
echo 'opcache.enable = 1'; \
echo 'opcache.enable_cli = 1'; \
echo 'opcache.memory_consumption = 256'; \
echo 'opcache.interned_strings_buffer = 16'; \
echo 'opcache.max_accelerated_files = 20011'; \
echo 'opcache.fast_shutdown = 1'; \
echo 'realpath_cache_size = 4096K'; \
echo 'realpath_cache_ttl = 600'; \
} > /usr/local/etc/php/php.ini

RUN { \
echo 'date.timezone = Europe/Paris'; \
echo 'short_open_tag = off'; \
echo 'memory_limit = 8192M'; \
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it's better to just use: memory_limit = -1?

Copy link
Author

Choose a reason for hiding this comment

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

I agree, I've just copied some files used on a personnal project, and I've forget to removed some parts...

Choose a reason for hiding this comment

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

By the way, date.timezone can be set to UTC so everyone will have same dates on the app, WDYT?

Copy link
Author

@mpiot mpiot Mar 1, 2019

Choose a reason for hiding this comment

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

@Pierstoval Because I'm in France I've configure it like that, but you're totaly right.

Thanks a lot :-)

} > /usr/local/etc/php/php-cli.ini

CMD ["php-fpm"]


#####################################
## APP DEV ##
#####################################
FROM app as app-dev

ARG NODE_VERSION
ARG COMPOSER_VERSION
ARG XDEBUG_VERSION

ENV COMPOSER_ALLOW_SUPERUSER=1
ENV APP_ENV=dev

# Install paquet requirements
RUN set -ex; \
# Install required system packages
apt-get update; \
apt-get install -qy --no-install-recommends \
unzip \
; \
# Clean aptitude cache and tmp directory
apt-get clean && rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/*;

# Install Node
RUN set -ex; \
curl -L -o /tmp/nodejs.tar.gz https://nodejs.org/dist/v${NODE_VERSION}/node-v${NODE_VERSION}-linux-x64.tar.gz; \
tar xfvz /tmp/nodejs.tar.gz -C /usr/local --strip-components=1; \
rm -f /tmp/nodejs.tar.gz; \
npm install yarn -g

# Install Composer
RUN set -ex; \
EXPECTED_SIGNATURE="$(curl -L https://getcomposer.org/download/${COMPOSER_VERSION}/composer.phar.sha256sum)"; \
curl -L -o composer.phar https://getcomposer.org/download/${COMPOSER_VERSION}/composer.phar; \
ACTUAL_SIGNATURE="$(sha256sum composer.phar)"; \
if [ "$EXPECTED_SIGNATURE" != "$ACTUAL_SIGNATURE" ]; then >&2 echo 'ERROR: Invalid installer signature' && rm /usr/local/bin/composer && exit 1 ; fi; \
chmod +x composer.phar && mv composer.phar /usr/local/bin/composer; \
RESULT=$?; \
exit $RESULT;

# Edit OPCache configuration
RUN set -ex; \
{ \
echo 'opcache.validate_timestamps = 1'; \
echo 'opcache.revalidate_freq = 0'; \
} >> /usr/local/etc/php/php.ini

# Install Xdebug
RUN set -ex; \
if [ "${XDEBUG_VERSION}" != 0 ]; \
then \
pecl install xdebug-${XDEBUG_VERSION}; \
docker-php-ext-enable xdebug; \
{ \
echo 'xdebug.remote_enable = on'; \
echo 'xdebug.remote_connect_back = on'; \
} >> /usr/local/etc/php/php.ini \
; fi


#####################################
## PROD ASSETS BUILDER ##
#####################################
FROM node:${NODE_VERSION} as assets-builder

COPY . /app
WORKDIR /app

RUN yarn install && yarn build && rm -R node_modules

#####################################
## PROD VENDOR BUILDER ##
#####################################
FROM composer:${COMPOSER_VERSION} as vendor-builder
Copy link

Choose a reason for hiding this comment

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

Here I'd inherit from app step, because if application relies on some extension installed before (in app step) (example bcmath) here this dependency will be missing. Proposal:

FROM app as vendor-builder

ARG COMPOSER_VERSION

# Install Composer
RUN set -ex; \
    EXPECTED_SIGNATURE="$(curl -L https://getcomposer.org/download/${COMPOSER_VERSION}/composer.phar.sha256sum)"; \
    curl -L -o composer.phar https://getcomposer.org/download/${COMPOSER_VERSION}/composer.phar; \
    ACTUAL_SIGNATURE="$(sha256sum composer.phar)"; \
    if [ "$EXPECTED_SIGNATURE" != "$ACTUAL_SIGNATURE" ]; then >&2 echo 'ERROR: Invalid installer signature' && rm /usr/local/bin/composer && exit 1 ; fi; \
    chmod +x composer.phar && mv composer.phar /usr/local/bin/composer; \
    RESULT=$?; \
    exit $RESULT;

Copy link
Author

Choose a reason for hiding this comment

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

Previously, I was doing like it, but I've change to avoid have composer in the final image.

At the moment on all my projects, I've never had this problem (but, sure, I've not test all dependencies). The goal of this part is to only install vendors and avoid to have composer in the final image.

Copy link

@athlan athlan Aug 2, 2019

Choose a reason for hiding this comment

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

That's good point 👍 . Build mechanisms should't be on final image.

However, final image won't have this dependency, because app-prod inherits from app, not vendor-builder. You wisely used only copy of /app files from vendor-builder while composer is located under /usr/local/bin/composer.

(hope it will be finally merged - good contribution I think)

Copy link
Author

@mpiot mpiot Aug 2, 2019

Choose a reason for hiding this comment

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

Yes but if we install composer on app then, you have composer on app-prod and finaly on the image.

Do you have a case were it fails ? At the moment on all projects it work fine like it (use the composer image to install vendors).

For the last point, I'm not sure, this PR is open since a long time... :(

Copy link

@athlan athlan Aug 2, 2019

Choose a reason for hiding this comment

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

Yes yes, that's why my proposal is to have it only on vendor-builder (not app):

FROM app as vendor-builder

I do have a case for amqp not-native library, which requires bcmath. Otherwise I'd need to install amqp extension, so, both cases would require that chage.


COPY --from=assets-builder /app /app
WORKDIR /app

RUN APP_ENV=prod composer install -o -n --no-ansi --no-dev


#####################################
## APP PROD ##
#####################################
FROM app as app-prod

ENV APP_ENV=prod

COPY --from=vendor-builder /app /app
WORKDIR /app

# Edit OPCache configuration
RUN set -ex; \
{ \
echo 'opcache.validate_timestamps = 0'; \
} >> /usr/local/etc/php/php.ini
165 changes: 165 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
DOCKER_COMPOSE?=docker-compose
EXEC?=$(DOCKER_COMPOSE) exec app
CONSOLE=bin/console
PHPCSFIXER?=$(EXEC) php -d memory_limit=1024m vendor/bin/php-cs-fixer

.DEFAULT_GOAL := help
.PHONY: help start stop restart install uninstall reset clear-cache tty clear clean
.PHONY: db-diff db-migrate db-rollback db-reset db-validate wait-for-db
.PHONY: watch assets assets-build
.PHONY: tests lint lint-symfony lint-yaml lint-twig lint-twig php-cs php-cs-fix security-check test-schema test-all
.PHONY: build up perm
.PHONY: docker-compose.override.yml

help:
@grep -E '(^[a-zA-Z_-]+:.*?##.*$$)|(^##)' $(MAKEFILE_LIST) | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[32m%-30s\033[0m %s\n", $$1, $$2}' | sed -e 's/\[32m##/[33m/'

##
## Project setup
##---------------------------------------------------------------------------

start: ## Start docker containers
$(DOCKER_COMPOSE) start

stop: ## Stop docker containers
$(DOCKER_COMPOSE) stop

restart: ## Restart docker containers
$(DOCKER_COMPOSE) restart

install: docker-compose.override.yml build up vendor perm ## Create and start docker containers

uninstall: stop ## Remove docker containers
$(DOCKER_COMPOSE) rm -vf

reset: uninstall install ## Remove and re-create docker containers

clear-cache: perm
$(EXEC) $(CONSOLE) cache:clear --no-warmup
$(EXEC) $(CONSOLE) cache:warmup

tty: ## Run app container in interactive mode
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest to use sh instead o tty here.

Copy link
Author

Choose a reason for hiding this comment

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

This part open a terminal in the container, then I've named it tty:
https://en.wikipedia.org/wiki/Computer_terminal#Text_terminals

I think sh is not really correct because there is a difference between /bin/sh and /bin/bash

Copy link
Contributor

Choose a reason for hiding this comment

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

So, maybe a bash would be more appropriate name? BTW, I think you forgot to add a -it flags to allocate tty.

$(EXEC) /bin/bash

clear: perm ## Remove all the cache, the logs, the sessions and the built assets
$(EXEC) rm -rf var/cache/*
$(EXEC) $(CONSOLE) redis:flushall -n
Copy link
Contributor

Choose a reason for hiding this comment

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

Redis?

Copy link
Author

Choose a reason for hiding this comment

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

Idem...

rm -rf var/log/*
rm -rf public/build
rm -f var/.php_cs.cache

clean: clear ## Clear and remove dependencies
rm -rf vendor node_modules


##
## Database
##---------------------------------------------------------------------------

wait-for-db:
$(EXEC) php -r "set_time_limit(60);for(;;){if(@fsockopen('db',3306)){break;}echo \"Waiting for MySQL\n\";sleep(1);}"

db-diff: vendor wait-for-db ## Generate a migration by comparing your current database to your mapping information
Copy link
Contributor

Choose a reason for hiding this comment

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

So, If we don't use the MySql all these command will wait 60 seconds?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, in fact... I can remove all the db part, or just remove the waiting part and the db-reset part.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is actually quite nice oneliner. Wait until services are ready is very useful, so I would like to see something like this in the makefile, even if commented out as we use sqlite here.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, but it's difficult to add it in the Makefile whithout MySQL... Basically I've add all the Makefile db part in the case the user would like to use MySQL, but then we've removed all MySQL trace...

$(EXEC) $(CONSOLE) doctrine:migration:diff

db-migrate: vendor wait-for-db ## Migrate database schema to the latest available version
$(EXEC) $(CONSOLE) doctrine:migration:migrate -n

db-rollback: vendor wait-for-db ## Rollback the latest executed migration
$(EXEC) $(CONSOLE) doctrine:migration:migrate prev -n

db-reset: vendor wait-for-db ## Reset the database
$(EXEC) $(CONSOLE) doctrine:database:drop --force --if-exists
$(EXEC) $(CONSOLE) doctrine:database:create --if-not-exists
$(EXEC) $(CONSOLE) doctrine:migrations:migrate -n

db-fixtures: vendor wait-for-db ## Apply doctrine fixtures
$(EXEC) $(CONSOLE) doctrine:fixtures:load -n

db-validate: vendor wait-for-db ## Check the ORM mapping
$(EXEC) $(CONSOLE) doctrine:schema:validate


##
## Assets
##---------------------------------------------------------------------------

watch: node_modules ## Watch the assets and build their development version on change
$(EXEC) yarn watch

assets: node_modules ## Build the development version of the assets
$(EXEC) yarn dev

assets-build: node_modules ## Build the production version of the assets
$(EXEC) yarn build

##
## Tests
##---------------------------------------------------------------------------

tests: ## Run all the PHP tests
$(EXEC) bin/phpunit

lint: lint-symfony php-cs ## Run lint on Twig, YAML, PHP and Javascript files

lint-symfony: lint-yaml lint-twig lint-xliff ## Lint Symfony (Twig and YAML) files

lint-yaml: ## Lint YAML files
$(EXEC) $(CONSOLE) lint:yaml config

lint-twig: ## Lint Twig files
$(EXEC) $(CONSOLE) lint:twig templates

lint-xliff: ## Lint Translation files
$(EXEC) $(CONSOLE) lint:xliff translations

php-cs: vendor ## Lint PHP code
$(PHPCSFIXER) fix --diff --dry-run --no-interaction -v

php-cs-fix: vendor ## Lint and fix PHP code to follow the convention
$(PHPCSFIXER) fix

security-check: vendor ## Check for vulnerable dependencies
$(EXEC) vendor/bin/security-checker security:check

test-schema: vendor ## Test the doctrine Schema
$(EXEC) $(CONSOLE) doctrine:schema:validate --skip-sync -vvv --no-interaction

test-all: lint test-schema security-check tests ## Lint all, check vulnerable dependencies, run PHP tests

Copy link
Contributor

@mstrom mstrom Sep 6, 2019

Choose a reason for hiding this comment

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

What do you think do we need to add command to update dependencies?

Suggested change
deps-update:
## Update the project dependencies
$(EXEC) composer update -n
$(EXEC) yarn upgrade

##


# Internal rules

build:
$(DOCKER_COMPOSE) pull --ignore-pull-failures
$(DOCKER_COMPOSE) build --force-rm

up:
$(DOCKER_COMPOSE) up -d --remove-orphans

perm:
$(EXEC) chmod -R 777 node_modules vendor
$(EXEC) chown -R www-data:root node_modules vendor

docker-compose.override.yml:
ifneq ($(wildcard docker-compose.override.yml),docker-compose.override.yml)
@echo docker-compose.override.yml do not exists, copy docker-compose.override.yml.dist to create it, and fill it.
exit 1
endif


# Rules from files

vendor: composer.lock
$(EXEC) composer install -n

composer.lock: composer.json
@echo compose.lock is not up to date.

node_modules: yarn.lock
$(EXEC) yarn install

yarn.lock: package.json
@echo yarn.lock is not up to date.
11 changes: 11 additions & 0 deletions docker-compose.override.yml.dist
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
version: '3.4'

services:
nginx:
ports:
- 127.0.0.1:8080:80

# Uncomment it, if you want to use MySQL (see: Dockerfile, docker-compose.yml)
# db:
# ports:
# - 127.0.0.1:3306:3306
Loading