geobretagne / mdchecker

This is a simple quality assurance tool performing unit tests on a CSW-enabled catalog.
GNU General Public License v3.0
2 stars 2 forks source link

Amélioration image docker #20

Open bchartier opened 8 years ago

bchartier commented 8 years ago

Quelques pistes d'amélioration de l'image docker :

  1. [x] Pouvoir sauvegarder la base de données. Est-ce qu'il faut déclarer un volume pour cela ou est-ce qu'une autre stratégie est plus pertinente ?
  2. [ ] @fphg voulait pouvoir stocker les données dans la base PostgreSQL de geOrchestra. Pour que de faire un volume il vaudrait sans doute mieux rendre paramétrable la connexion à la base de données -> paramètres du dockerfile. Il faudrait alors changer la manière de lancer create_db.py.
  3. [ ] Il faudrait sans doute pouvoir définir le contenu de app.json au lancement du container.
  4. [x] Réduire le nombre de layers de l'image docker en fusionnant les dernières commandes RUN
  5. [ ] Optimiser la taille de l'image docker en utilisant une image de base plus adaptée (je suis parti d'une image python pour une question de simplicité/facilité.
fphg commented 8 years ago

Précision : je pense qu'il faut pouvoir requêter entre mdchecker et ogcstatistics. Ca n'implique pas forcément la même base.

fvanderbiest commented 8 years ago

Dans la branche docker-compose, avec fda1e3 j'ai d'abord supprimé supervisord, qui ne colle pas bien au paradigme docker ("un process par container").

De fait, j'ai dû ajouter une composition (fichier docker-compose.yml) avec un nginx "standard", tiré du hub.

Effet de bord : on a tout de suite des logs sympas en sortie de docker-compose up. Avec supervisord, c'était la galère pour obtenir ces logs ...

Pour la sauvegarde de la base, j'ai mis un volume nommé (mdcheckerdb) qui est monté sur /data dans le container applicatif. La base est initialisée par l'entrypoint, ce dernier passant la main à uwsgi ensuite.

Enfin, j'ai supprimé le besoin d'avoir un fichier conf/server.cfg, en utilisant plutot des variables d'environnement à la place (que l'on retrouve dans le docker-compose.yml).

En faisant tout ça, je ne suis pas certain de ne pas avoir cassé complètement le fonctionnement sans docker. Du coup, c'est encore expérimental... car j'aimerais être le moins intrusif possible.

Bref, affaire à suivre, n'hésitez pas à essayer la branche et à commenter.

bchartier commented 8 years ago

@fphg voulait pouvoir stocker les données dans la base PostgreSQL de geOrchestra. Pour que de faire un volume il vaudrait sans doute mieux rendre paramétrable la connexion à la base de données -> paramètres du dockerfile. Il faudrait alors changer la manière de lancer create_db.py.

Précision : je pense qu'il faut pouvoir requêter entre mdchecker et ogcstatistics. Ca n'implique pas forcément la même base.

Pas forcément en effet, mais ce serait bien de pouvoir dire à mdchecker (lorsqu'on lance l'appli web) d'exploiter une base existante plutôt que de créer une base sqllite. On est toujours d'accord sur ce point ?

bchartier commented 8 years ago

De fait, j'ai dû ajouter une composition (fichier docker-compose.yml) avec un nginx "standard", tiré du hub.

Très bien, du coup ce sera plus facile à intégrer à une architecture Rancher, j'imagine.

Effet de bord : on a tout de suite des logs sympas en sortie de docker-compose up. Avec supervisord, c'était la galère pour obtenir ces logs ...

Super. J'avoue que j'avais mis supervisord sans réfléchir plus que cela : je suis reparti d'un exemple.

Pour la sauvegarde de la base, j'ai mis un volume nommé (mdcheckerdb) qui est monté sur /data dans le container applicatif. La base est initialisée par l'entrypoint, ce dernier passant la main à uwsgi ensuite.

Est-ce qu'on peut supprimer le répertoire app/mdchecker/db alors ?

fvanderbiest commented 8 years ago

Est-ce qu'on peut supprimer le répertoire app/mdchecker/db alors ?

Bah, faut voir si ça ne casse pas le fonctionnement sans docker.

bchartier commented 8 years ago

Est-ce qu'on peut supprimer le répertoire app/mdchecker/db alors ?

Bah, faut voir si ça ne casse pas le fonctionnement sans docker.

J'ai apporté des modifications à cette branche pour que l'appli puisse fonctionner sans docker : cfd31ba J'ai corrigé une erreur dans runserver.py.

La base de données est maintenant créée par défaut sur /tmp (à la racine du système de fichiers quand je le lance en local sur mon ordinateur) : cf. https://github.com/geobretagne/mdchecker/blob/docker-compose/app/mdchecker/__init__.py#L14. Est-ce qu'on ne pourrait pas créer la base par défaut dans l'arborescence de l'appli ?

fvanderbiest commented 8 years ago

Est-ce qu'on ne pourrait pas créer la base par défaut dans l'arborescence de l'appli ?

Oh oui, probablement, mais je n'aime pas trop mélanger code et données personnellement...

fphg commented 8 years ago

@bchartier pas de contre indications à ce que la base aille dans pg:georchestra, bien au contraire. Loïc a formulé une demande : authentification vers le CSW. Pas simple du tout à faire, mais cela me fait penser au secproxy : si mdchecker reçoit les headers, il connaît l'utilisateur et peut stocker cette information pour présenter ensuite "mes tests"

Effectivement le requêtage d'ogcstatistics apporterait potentiellement des choses à mdchecker : les stats de fréquentation. Mais encore une fois il peut s'agir d'une passerelle entre les deux applis, sans passer par la base.

bchartier commented 8 years ago

Est-ce qu'on ne pourrait pas créer la base par défaut dans l'arborescence de l'appli ?

Oh oui, probablement, mais je n'aime pas trop mélanger code et données personnellement...

Effectivement, ça a des inconvénients. Aujourd'hui on peut configurer le chemin de la base simplement en modifiant la variable d'environnement MDCHECKER_DB_PATH. C'est suffisant. Il faudra le documenter dans la procédure d'install.

bchartier commented 8 years ago

Loïc a formulé une demande : authentification vers le CSW. Pas simple du tout à faire, mais cela me fait penser au secproxy : si mdchecker reçoit les headers, il connaît l'utilisateur et peut stocker cette information pour présenter ensuite "mes tests"

Loïc a formulé la demande dans un ticket ou il faut qu'on crée le ticket ?

bchartier commented 8 years ago

Effectivement le requêtage d'ogcstatistics apporterait potentiellement des choses à mdchecker : les stats de fréquentation. Mais encore une fois il peut s'agir d'une passerelle entre les deux applis, sans passer par la base.

Je ne connais pas ogcstatistics. C'est la même chose que analytics dans geOrchestra ? Du coup, j'ai du mal à voir l'image globale de ce que l'on pourrait faire. On déplace la discussion dans un ticket spécifique ?

bchartier commented 8 years ago

@fphg et @fvanderbiest : je propose que vous fassiez un dernier test de la branche docker-compose avant de la basculer dans master.

Mes tests ont été positifs (en déploiement classique et avec docker-compose). J'ai corrigé ce matin une ligne en commentaire qui trainait (signalée par @fvanderbiest) et j'ai ajouté un try/except sur les erreurs de création de la base. J'ai également fait un test de création de base avec la variable d'environnement MDCHECKER_DB_PATH. Ça fonctionne très bien de mon point de vue.

bchartier commented 8 years ago

@fvanderbiest : par curiosité, pourquoi as-tu renommé app en application dans la branche docker-compose ?

fvanderbiest commented 8 years ago

par curiosité, pourquoi as-tu renommé app en application dans la branche docker-compose ?

"uWSGI looks for a callable named application" http://stackoverflow.com/questions/26000572/flask-with-uwsgi-python-application-not-found#comment40727723_26000960

fvanderbiest commented 8 years ago

D'après http://uwsgi-docs.readthedocs.io/en/latest/WSGIquickstart.html, on pourrait probablement s'en sortir plus élégamment avec --callable app dans https://github.com/geobretagne/mdchecker/blob/docker-compose/Dockerfile#L20

bchartier commented 8 years ago

par curiosité, pourquoi as-tu renommé app en application dans la branche docker-compose ?

"uWSGI looks for a callable named application" http://stackoverflow.com/questions/26000572/flask-with-uwsgi-python-application-not-found#comment40727723_26000960

ok. merci pour l'info

fvanderbiest commented 8 years ago

on pourrait probablement s'en sortir plus élégamment avec --callable app

Validé avec d874c4a :-)

fvanderbiest commented 8 years ago

Je créée une PR ciblant master du coup: https://github.com/geobretagne/mdchecker/pull/21

bchartier commented 8 years ago

on pourrait probablement s'en sortir plus élégamment avec --callable app

Validé avec d874c4a :-)

C'est ce que j'avais utilisé pour la première version du dockerfile

bchartier commented 8 years ago

Je créée une PR ciblant master du coup: #21

Super. J'ai appliqué le PR sur master.