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

Améliorations #1

Open
alexandre-dos-reis opened this issue Apr 28, 2022 · 10 comments
Open

Améliorations #1

alexandre-dos-reis opened this issue Apr 28, 2022 · 10 comments
Assignees
Labels
enhancement New feature or request

Comments

@alexandre-dos-reis
Copy link

alexandre-dos-reis commented Apr 28, 2022

A mon humble avis, le config est déjà robuste ! Voici tout de même quelques points que j'améliorais :

  • Ajouter des valeurs par défaut pour les variables d'environnement\args dans le fichier compose.yml qui pourront être écrasées d'après la doc comme ceci :
  apache:
    build:
      context: './apache/'
      args:
        - APACHE_VERSION=${APACHE_VERSION:-2.4.53}

Ensuite, j'effacerai le fichier .env car cela peut provoquer des confusions avec le fichier .env de l'application.( Y'en a-t-il un d'ailleurs ?).

  • Il y ambiguïté entre ce que vous appelez les versions et les tags des images.

Exemple: Dans le fichier .env, vous appelez la version d'Apache 2.4.53-alpine, qui est en fait un tag d'image. Cela laisse à l'utilisateur le choix du tag ce qui peut provoquer des erreurs de build si l'utilisateur décide d'utiliser une image debian et que le dockerfile apache contient des commandes apk.

Je ferai plutôt :

ARG APACHE_TAG_VERSION

FROM httpd:${APACHE_TAG_VERSION}-alpine
...
  • Je trouve ça dommage de construire une image pour la DB sachant que l'import de la DB peut déjà se faire au démarrage du conteneur avec ./mysql/sql:/docker-entrypoint-initdb.d/:rw

  • Enfin je créerai 2 fichiers compose.yml, un pour la conf Apache, l'autre pour la conf Nginx. On pourrait le lancer comme ça :

    • docker-compose up -d -f compose-apache.yml
    • Ou utiliser le système d'override pour mutualiser la config mais cela rallonge considérablement les commandes :
      • docker-compose up -d -f compose.base.yml -f compose.apache.yml ou
      • docker-compose up -d -f compose.base.yml -f compose.nginx.yml
@jcheron
Copy link
Contributor

jcheron commented Apr 28, 2022

Super issue !

  • Ok pour les valeurs par défaut
  • Ok pour l'ambiguïté entre versions et noms d'images

jcheron added a commit that referenced this issue Apr 28, 2022
@jcheron
Copy link
Contributor

jcheron commented Apr 28, 2022

Ok pour le fichier .env, que je renomme en env-example
même si il n'y a pas d'ambiguïté avec les frameworks (le .env se trouve à un autre endroit généralement)

jcheron added a commit that referenced this issue Apr 28, 2022
@jcheron
Copy link
Contributor

jcheron commented Apr 28, 2022

Pour Mysql, l'import se fait effectivement avec le volume ./mysql/sql:/docker-entrypoint-initdb.d/:rw
Il restait un vieux script, que je vire, et le Docker file Mysql par la même occasion

jcheron added a commit that referenced this issue Apr 28, 2022
@jcheron
Copy link
Contributor

jcheron commented Apr 28, 2022

Pour les 2 fichiers compose, par contre, je ne vois pas l'intérêt...

Il faut se dire que d'autres serveurs vont être ajoutés (Swoole, Workerman, RoadRunner...), et se serait malvenu de créer un compose pour chacun d'entre eux.

Et avec un seul compose, c'est plus simple qd même:
Pour lancer apache

docker-compose up -d apache

Pour NginX

docker-compose up -d nginx

@jcheron jcheron self-assigned this Apr 28, 2022
@jcheron jcheron added the enhancement New feature or request label Apr 28, 2022
@alexandre-dos-reis
Copy link
Author

alexandre-dos-reis commented May 4, 2022

SI vous avez autant de serveurs implémentés dans le fichier compose.yml, la commande docker-compose up lancerait tous ces serveurs en même temps ce qui n'est pas souhaitable.

La commande docker-compose up devrait lancer un seul serveur par défaut, typiquement NginX ou Apache.

Cela permet de découper la configuration sans savoir à se répéter et d'éviter d'avoir un fichier compose.yml beaucoup trop long.

Si je prends votre configuration existante, on aurait ces fichiers là :

  • compose.yml
  • compose.override.yml (Qui lance apache par défaut)
  • compose.nginx.yml
  • compose.swoole.yml
  • compose.dernier-serveur-a-la-mode.yml
  • etc...

compose.yml

version: "3.8"
services:

  php:
    build:
      context: './php/'
      args:
        - PHP_VERSION=${PHP_VERSION:-8.0}-fpm
    networks:
      - backend
    volumes:
      - ./projects/:/var/www/html/
      
  mysql:
    image: mysql:${MYSQL_VERSION:-8.0}
    volumes:
    - ./mysql/db:/var/lib/mysql:rw
    - ./mysql/sql:/docker-entrypoint-initdb.d/:rw
    networks:
      - backend
    environment:
      - MYSQL_ROOT_PASSWORD=${MYSQL_PASSWORD}
      
  phpmyadmin:
    image: phpmyadmin/phpmyadmin
    ports:
        - '8099:80'
    restart: always
    environment:
        PMA_HOST: mysql
    depends_on:
        - mysql
        
networks:
  frontend:
  backend:

compose.override.yml

version: "3.8"
services:

  apache:
      build:
        context: './apache/'
        args:
          - APACHE_VERSION=${APACHE_VERSION:-2.4.53}-alpine
      depends_on:
        - php
        - mysql
      networks:
        - frontend
        - backend
      ports:
        - "8080:80"
      volumes:
        - ./projects/:/var/www/html/

compose.nginx.yml

version: "3.8"
services:

  nginx:
      image: nginx:${NGINX_VERSION:-latest}
      depends_on:
        - php
        - mysql
      ports:
          - "8080:80"
      volumes:
        - ./projects/:/var/www/html/
        - ./nginx/:/etc/nginx/conf.d/
      networks:
        - frontend
        - backend

Pour lancer php, mysql, phpMyAdmin avec Apache : docker-compose up -d
Pour lancer php, mysql, phpMyAdmin avec Nginx : docker-compose -f compose.yml -f compose.nginx.yml up -d

Je mets la doc de docker sur le partage de config entre plusieurs fichiers.

@jcheron
Copy link
Contributor

jcheron commented May 4, 2022

D'un côté, le multi-compose semble plus propre, mais il n'y aura que quelques serveurs (Apache, Nginx, Swoole, Workerman, ngx_php, RoadRunner), pas 60: donc le fichier compose.yml ne sera pas si grand.

Et le multi-compose a surtout une incidence assez lourde sur l'utilisation:
C'est quand-même beaucoup plus lourd de démarrer nginx en faisant:

docker-compose -f compose.yml -f compose.nginx.yml up -d

qu'avec:

docker-compose -d nginx

Pour ce qui est de la commande sans paramètre;

docker compose up

qui démarrerait tout, l'utilisateur n'est pas sensé la lancer...
enfin ce serait quand même bien que seul Apache soit lancé dans ce cas (avec php/mysql), mais sans avoir recours à la complexité due au multiples compose

@alexandre-dos-reis
Copy link
Author

Je comprends votre point de vue mais ceci dit, si vous voulez démarrer les différents services à partir d'un fichier il faudra quand même faire :

docker-compose up -d php mysql phpMyAdmin nginx

au lieu de :

docker-compose -f compose.yml -f compose.nginx.yml up -d

Du point de vue des utilisateurs Docker, empêcher ces derniers de ne pas écrire docker-compose up -d, à mon avis, casse les conventions.

Sinon pour résoudre la chose, pourquoi ne pas faire un Makefile ? 😀

@jcheron
Copy link
Contributor

jcheron commented May 4, 2022

Je comprends votre point de vue mais ceci dit, si vous voulez démarrer les différents services à partir d'un fichier il faudra quand même faire :

docker-compose up -d php mysql phpMyAdmin nginx

non, pas avec le depends_on:

    depends_on:
      - php
      - mysql

Le démarrage d'un serveur déclenche celui de php et mysql.

Mais dès que je vais ajouter d'autres Dbs, l'utilisateur sera effectivement obligé de spécifier lesquelles:

docker-compose up -d nginx postgresql

Du point de vue des utilisateurs Docker, empêcher ces derniers de ne pas écrire docker-compose up -d, à mon avis, casse les conventions.

Laradock les a cassées avant : https://laradock.io/getting-started/#Usage

Sinon pour résoudre la chose, pourquoi ne pas faire un Makefile ? 😀

oui, mais le make poserait un problème de compatibilité multi-plateformes, les utilisateurs de Windows seraient obligés d'installer un truc en plus par ex

@alexandre-dos-reis
Copy link
Author

Il me semblait que depends_on stipulait l'ordre de démarrage des conteneurs mais ne démarrait pas les conteneurs non explicitement appelé, faut que je teste ça.

@jcheron
Copy link
Contributor

jcheron commented May 4, 2022

Je confirme, le depends_on fait les 2:

  • démarrage des services mentionnés
  • dans un certain ordre

Pour le docker-compose up -d, c'est vrai que ce serait encore mieux de pouvoir l'utiliser par défaut. D'autres ont d'ailleurs déjà eu cette même problématique, voir compose-spec/compose-spec#87

Je vais voir si c'est possible avec les profiles

jcheron added a commit that referenced this issue May 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants