Skip to content
This repository has been archived by the owner on Jul 1, 2024. It is now read-only.

Dockerització del Mailtoticket #196

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

jguillem
Copy link

Afegeix el dockerfile el entrypoint.sh i un exemple del fitxer de configuració de fetchmail fetchmailrc.
Els canvis han estats fets a sobre de la branca feature_api_rest_identitat. Es a dir la branca amb codi adaptat a la nova APP REST d'identitat.

@alexm
Copy link
Member

alexm commented Nov 20, 2019

@jguillem crec que aquest PR no està basat en la branca master actual, oi? Diria que inclou canvis que estan en algun altre PR i que per tant no correspon posar-los aquí.

@alexm alexm self-requested a review November 20, 2019 13:26
@jguillem
Copy link
Author

jguillem commented Nov 20, 2019

@jguillem crec que aquest PR no està basat en la branca master actual, oi? Diria que inclou canvis que estan en algun altre PR i que per tant no correspon posar-los aquí.

Correcte. He fet els canvis sobre la branca feature_api_rest_identitat com està a la descripció del request. Si es possible fer un pull request sobre d'una altre branca (i si m'ho permets, perquè igual es lia massa) miro com fer-ho. Del contrari podem ignorar aquest pull request i ja ho tornaria a fer un cop feu el merge de la branca feature_api_rest_identitat al master. Com em diguis.

Copy link
Member

@alexm alexm left a comment

Choose a reason for hiding this comment

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

La idea de tenir el mailtoticket i el fetchmail junts és temptadora i per tant és quelcom a considerar, però hi ha altres aspectes d'aquesta proposta que crec que s'haurien de revisar, el més important dels quals és deslligar la creació de la imatge docker d'una branca determinada.

@@ -0,0 +1,20 @@
FROM python:2.7
RUN git clone --single-branch --branch feature_api_rest_identitat https://github.com/UPC/mailtoticket.git
Copy link
Member

Choose a reason for hiding this comment

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

Crec que és mala idea clonar el repositori durant la construcció de la imatge, sobretot si el motiu és generar la imatge amb el codi d'una branca concreta.

Copy link
Author

Choose a reason for hiding this comment

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

La meva idea era generar una imatge out-of-the-box (mínim de passos), de cara a fer les proves m'anava bé.
Per adaptar el dockerfile en un context de branca master amb fetchmail inclòs, es pot canviar el git clone per un COPY de tot.
Llavors el procés seria:
1- Git clone
2- Modificar els settings de mailtoticket i fetchmail
3- Fer el build

folder "mailtoticket"
mda "/bin/sh -c 'cat | /usr/local/bin/python /mailtoticket/mailtoticket.py'"
ssl
nokeep
Copy link
Member

Choose a reason for hiding this comment

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

Jo anomenaria l'exemple de configuració del fetchmail d'una altra manera que:

  • no comenci amb un punt perquè el fitxer no quedi ocult
  • el nom digui clarament que és un exemple, e.g. fetchmailrc.example

Copy link
Author

Choose a reason for hiding this comment

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

Tens raó, es millor així.

username "suport.unitat"
password "contrasenya"
folder "mailtoticket"
mda "/bin/sh -c 'cat | /usr/local/bin/python /mailtoticket/mailtoticket.py'"
Copy link
Member

Choose a reason for hiding this comment

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

Per què caldria posar /bin/sh -c 'cat | ...' enlloc de cridar el python directament amb el mailtoticket com a argument, tal com indica la documentació del wiki?

Copy link
Author

Choose a reason for hiding this comment

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

Per alguna raó no em funcionava amb la info de la wiki i esbrinant exemples de configuració de fetchmail, aquesta opció em va funcionar. Puc tornar a provar per si va ser una falsa impressió.

@@ -0,0 +1,9 @@
set logfile "/tmp/fetchmail.log"
Copy link
Member

Choose a reason for hiding this comment

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

En un contenidor de docker els logs han d'anar a stdout i stderr perquè el sistema de fitxers és efímer. Per tant crec que aquesta línia no hi hauria de ser.

Copy link
Author

Choose a reason for hiding this comment

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

No tinc controlat el tema del logging en contenidors. Si treient l'entrada "set logfile" ja s'envien els errors per stdout, doncs es fàcil de solucionar.

RUN apt-get update
RUN apt-get upgrade -y
RUN apt-get install -y fetchmail
RUN apt-get install -y cron
Copy link
Member

Choose a reason for hiding this comment

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

Podem debatre si el fetchmail hauria d'estar dins el mateix contenidor que el mailtoticket (jo penso que haurien d'estar en contenidors separats) però el que segur que no té sentit és tenir un contenidor amb un cron només per executar el fetchmail.

Si el contenidor s'executa a un servidor, aquest ja tindrà un cron des del que es pot executar el contenidor amb docker run. Si el contenidor està a mycontainers, allà ja hi ha serveis de tipus cron que poden executar contenidors periòdicament. A més a més, el fetchmail té un mode dimoni que també podria ser interessant de valorar.

Com més senzill sigui el contenidor més fàcil serà de mantenir, debuggar, operar i escalar, per tant crec que el cron sobra.

Copy link
Author

Choose a reason for hiding this comment

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

No et sabria dir quina configuració implicaria tenir el fetchmail i el mailtoticket en dockers separats per que la cosa funcioni. Jo apostaria per tenir els dos en el mateix docker, sigui tirant del dimoni de fetchmail o bé pensant en el temporitzador que teniu muntat en mycontainers. Tinc pendent demanar l'accés a aquest servei per a fer proves.

@@ -0,0 +1,20 @@
FROM python:2.7
Copy link
Member

Choose a reason for hiding this comment

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

En general s'acostuma a anomenar el fixer Dockerfile amb la D majúscula. Crec que seria bona idea canviar-li el nom, sobretot si volem que la gent del docker hub el trobin per fer imatges automàtiques.

Copy link
Author

Choose a reason for hiding this comment

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

Es fàcil de solucionar.

@alexm
Copy link
Member

alexm commented Nov 20, 2019

Si es possible fer un pull request sobre d'una altre branca (i si m'ho permets, perquè igual es lia massa) miro com fer-ho.

Sí que es pot però només té sentit si els canvis formen part del mateix objectiu. En aquest cas, crec que fer el PR sobre la branca feature_api_rest_identitat no seria correcte perquè barrejaria canvis amb objectius diferents.

Del contrari podem ignorar aquest pull request i ja ho tornaria a fer un cop feu el merge de la branca feature_api_rest_identitat al master. Com em diguis.

No crec que calgui esperar a fer les correccions. Us he fet alguns comentaris que m'agradaria que valoreu i comentem abans de decidir per quina proposta ens decantem per la imatge oficial de docker.

@alexm alexm self-assigned this May 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants