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

docker build #2992

Draft
wants to merge 1 commit into
base: alpha
Choose a base branch
from
Draft

docker build #2992

wants to merge 1 commit into from

Conversation

pifou25
Copy link
Contributor

@pifou25 pifou25 commented Dec 7, 2024

Description

ajout du fichier ".dockerignore" : comme pour .git les fichiers listés ici ne sont pas copiés par l'instruction COPY . du Dockerfile
copier "install.sh" dans /root car on l'utilise pendant la séquence d'initialisation "init.sh"
Ajout du "healthcheck": permet de valider la bonne initialisation du container, on peut ainsi le tester dans le workflow
Ne pas re-télécharger le source jeedom durant l'initialisation (init.sh) puisqu'on l'a déjà fait durant le build (Dockerfile).
Fix la fonction "trap" pour terminer sans tuer les process du container

Suggested changelog entry

Fix et Optimisation de l'image Docker + Healthcheck

Related issues/external references

Fixes #

Types of changes

  • Bug fix (non-breaking change which fixes)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
    • This change is only breaking for integrators, not for external standards or end-users.
  • Documentation improvement

PR checklist

@pifou25
Copy link
Contributor Author

pifou25 commented Dec 7, 2024

il y a une erreur à l'installation du package chromium, je ne sais pas l'expliquer... Il faut attendre et relancer le job :/ une autre PR a la même erreur.

0.730 The following information may help to resolve the situation:
0.730 
0.730 The following packages have unmet dependencies:
0.780  chromium : Depends: libc++1-19 (>= 1:19.1.4) but it is not installable
0.780             Depends: libc++abi1-19 (>= 1:19.1.4) but it is not installable
0.780             Depends: libunwind-19 (>= 1:19.1.4) but it is not installable
0.780             Depends: chromium-common (= 131.0.6778.108-1~deb12u1) but it is not going to be installed
0.780             Recommends: chromium-sandbox but it is not going to be installed
0.784 E: Unable to correct problems, you have held broken packages.
------
Dockerfile:38
--------------------
  37 |     RUN apt update -y 
  38 | >>> RUN apt -o Dpkg::Options::="--force-confdef" -y install software-properties-common \
  39 | >>>   ntp ca-certificates unzip curl sudo cron locate tar telnet wget logrotate dos2unix ntpdate htop \
  40 | >>>   iotop vim iftop smbclient git python3 python3-pip libexpat1 ssl-cert \
  41 | >>>   apt-transport-https xvfb cutycapt xauth at mariadb-client espeak net-tools nmap ffmpeg usbutils \
  42 | >>>   gettext libcurl3-gnutls chromium librsync-dev ssl-cert iputils-ping \
  43 | >>>   apache2 apache2-utils libexpat1 ssl-cert \
  44 | >>>   php libapache2-mod-php php-json php-mysql php-curl php-gd php-imap php-xml php-opcache php-soap php-xmlrpc \
  45 | >>>   php-common php-dev php-zip php-ssh2 php-mbstring php-ldap php-yaml php-snmp && apt -y remove brltty
  46 |     
--------------------

/root/install.sh -s 6 -v ${VERSION} -w ${WEBSERVER_HOME}

# do not re-install jeedom
if [ ! -f ${WEBSERVER_HOME}/core/config/common.config.sample.php ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Je suis pas sur que ca soit bon, a chaque mise à jour du core ce fichier va revenir et donc au redemarrage du docker ca va reinstaller jeedom.... Il faut se baser sur le fichier common.config.php

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alors déjà, il y a la négation, donc si le fichier est présent, c'est "normal" mais s'il est absent on va re-installer les sources.
Et, il ne devrait jamais être absent, puisque ce fichier est copié, jamais supprimé.
Mais en vrai je voulais juste supprimer cette étape, on n'a pas besoin le le vérifier, on a forcément déjà les sources arrivé ici. Le "init.sh" c'est le runtime, le démarrage du container, mais le code a été copié durant le build de l'image. C'est pour ça que j'ai mis une condition que je suppose toujours fausse, sur un fichier en principe toujours présent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Voici comment je l'interprète:
docker run -p 80:80 -v /tmp/jeedom:/var/www/html --rm --name jeedom_server jeedom
Avec l'option -v je map le rep de l'host sur celui du container. Du coup ça permet de persister Jeedom sur l'host, et ça marche lors de l'arrêt / relance du container, on ne perd rien. Mais au 1er lancement, si mon rep /tmp/jeedom est vide sur l'host, il est vide aussi dans le container, malgré que je pensais y avoir copié les sources (lors du build). Dans ce cas uniquement, on doit recopier les sources.
C'est ce que je ne comprends pas dans la doc, normalement si le rep est vide on devrait y retrouver les données du container:
https://docs.docker.com/engine/storage/volumes/#mounting-a-volume-over-existing-data

ajout du fichier ".dockerignore" : comme pour .git les fichiers listés ici ne sont pas copiés par l'instruction COPY . du Dockerfile
copier "install.sh" dans /root car on l'utilise pendant la séquence d'initialisation "init.sh"
Ajout du "healthcheck": permet de valider la bonne initialisation du container, on peut ainsi le tester dans le workflow
Ne pas re-télécharger le source jeedom durant l'initialisation (init.sh) puisqu'on l'a déjà fait durant le build (Dockerfile).
Fix la fonction "trap" à l'arrêt du conteneur
@pifou25
Copy link
Contributor Author

pifou25 commented Dec 15, 2024

Hello, c'est bon tout est passé au vert :)
Par contre je me rends compte que c'est peut être pas simple, je mélange plusieurs fix / améliorations... Si besoin et pour plus de clarté je peux faire plusieurs PR, pour séparer les sujets et si y'a des questions n'hésitez pas ?
idem pour la #2866 je vois qu'elle est en conflit - depuis 6 mois c'est normal - je vais la reprendre et éventuellement la diviser en plusieurs.

@Hotfirenet
Copy link
Contributor

Salut @pifou25,

effectivement si on peut faire plusieurs PR ce serait pas mal
Cdtl

@pifou25
Copy link
Contributor Author

pifou25 commented Dec 22, 2024

Salut @pifou25,

effectivement si on peut faire plusieurs PR ce serait pas mal Cdtl

ok, c'est en cours je me garde quand même celle-ci comme réréfence pour le moment :)

@pifou25 pifou25 marked this pull request as draft December 22, 2024 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants