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

[IV-21-22] Objetivo 5 #35

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

[IV-21-22] Objetivo 5 #35

wants to merge 14 commits into from

Conversation

LuisArostegui
Copy link
Owner

@LuisArostegui LuisArostegui commented Dec 28, 2021

  • ¿Se han establecido criterios previos de elección de la imagen base, así
    como los criterios de búsqueda que han conducido a las imágenes que se están
    considerando? Si.
  • ¿Se han seguido las buenas prácticas en las órdenes que construyen
    la imagen? ¿Especialmente las relativas a usuarios no privilegiados y demás? Si.
  • ¿La imagen incluye sólo y exclusivamente la infraestructura
    necesaria para pasar los tests? Por ejemplo, si se ha elegido la imagen
    "oficial" (lo que hay que hacer sólo en casos muy justificados) ¿se ha
    comprobado que lo que se instale sea sólo y exclusivamente lo necesario para
    nuestro proyecto? Si. En nuestro caso las dependencias son Task y las que vienen en el fichero go.mod. Se instala solamente eso.
  • ¿Se han documentado y enlazado los commits a las imágenes que se
    han testeado?
  • ¿Está establecido correctamente como WORKDIR el que se va a usar en los
    tests que se van a lanzar? Si.
  • ¿Se usa algún directorio adecuado para copiar contenidos del repositorio
    en vez del directorio de trabajo? Se opta por hacer la copia en el directorio /app. Evitando copiar todo en /app/test.
  • ¿Se han establecido claramente los criterios de búsqueda y de aceptación de
    la imagen base? Si.
  • ¿Tienes claro cuales son las condiciones en las que tiene que actualizarse
    la imagen en DockerHub y has configurado el workflow de acuerdo con ello? En
    particular, ¿sabes qué condiciones tienen que darse para que se actualice y qué
    condiciones no deben darse? Si.

Observación para este objetivo

Se recuerda al estudiante que este objetivo no consiste en “poner las tres o cuatro imágenes oficiales del lenguaje que esté usando y quedarme con Alpine”. Para superarlo, tiene realmente que demostrar que ha seguido las mejores prácticas en la búsqueda y elección de un contenedor base que sea el más adecuado para testear el repositorio.

Se termina optando por Alpine, pero se analizan, basandonos en varios requisitos, varias imagenes hasta obtener la mejor elección posible para nuestro proyecto.

Otros cambios

Se ha desarrollado #28 para avanzar con la lógica de negocio del proyecto.
Se cambia de framework de tests. Se documenta y se informa en #36

@JJ
Copy link

JJ commented Dec 29, 2021

⛹ Revisores → @Olasergiolas @joaquingv12 @amerigal

@LuisArostegui LuisArostegui linked an issue Dec 29, 2021 that may be closed by this pull request
Copy link

@amerigal amerigal left a comment

Choose a reason for hiding this comment

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

En general lo veo bastante completo, bien trabajado el uso de buenas prácticas y la documentación, pero creo que sería conveniente completar la justificación de la elección de la imagen base para el contenedor.

Por otro lado, pienso que sería apropiado que el milestone reflejara de manera más explícita el trabajo realizado en este objetivo.

También incluyo una observación sobre la automatización de la subida del contenedor.

Un último comentario menor es que los enlaces del comentario de este PR en la lista de comprobación no se están renderizando bien porque hay un espacio entre los corchetes y los paréntesis.

Comment on lines 6 to 12
on:
push:
branches:
- main
pull_request:
branches:
- main
Copy link

Choose a reason for hiding this comment

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

De esta manera, se subirá la imagen a DockerHub en todos los push que se hagan a main o desde PR, lo que puede resultar innecesario en la mayoría de las ocasiones. Pienso que sería conveniente limitar este workflow a aquellas situaciones en que se modifiquen archivos que hagan necesaria la actualización de la imagen.

Copy link
Owner Author

Choose a reason for hiding this comment

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

He añadido un path para indicar que cuando se modifica go.mod o los ficheros .go se lance la publicación de la imagen. En la documentación he redactado todo de porque he realizado estos cambios.

Copy link

Choose a reason for hiding this comment

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

Pienso que efectivamente poner la condición de path es una buena manera de abordar esta cuestión. La inclusión del go.mod la veo muy apropiada pues efectivamente en la imagen se instalan las dependencias y es importante reconstruirla cuando cambien las dependencias, lo que se refleja en el go.mod.

Sin embargo, diría que la imagen construida no depende directamente de la lógica de negocio mientras no hayan cambiado las dependencias. El código de la aplicación se incluye más tarde una vez construido el contenedor montándolo en el mismo. No termino de ver entonces la necesidad de incluir ahí los ficheros .go, ni que en el caso de los pull requests siempre haya que reconstruirla.

Por otro lado, piensa que también sería necesaria la reconstrucción de la imagen en el caso de cambiar la estructura que se ha definido en el Dockerfile.

docs/docker.md Outdated
task: Failed to run task "docker": exit status 1
```

Otro error a nuestro catalogo. Al buscar el error parece que tenía que incluir en el Dockerfile una operación como `RUN apk add g++` ya que el error parece que viene de un paquete de Testify que necesita funciones escritas en C. Pero aun incluyendo tenía el mismo error una y otra vez.
Copy link

Choose a reason for hiding this comment

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

Se me ocurre que hubiera sido más apropiado apk add gcc, o mejor apk add build-base tal y como recomiendan aquí. Desconozco si eso hubiera sido suficiente. En cualquier caso, me parece positivo el cambio a una dependencia que presenta menos complicaciones y no necesita de instalaciones adicionales.

Copy link
Owner Author

Choose a reason for hiding this comment

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

La verdad que llegué a probar apk add gcc y build-essentials. En concreto build-base no lo probé. Sin embargo, como bien dices con este framework no tengo dependencias indirectas y no me ha surgido ningún problema.

docs/docker.md Outdated
Comment on lines 39 to 47
### Imagen de Golang

Docker tiene muchas imagenes de golang, cada una de ellas diseñada para un caso de uso específico:

* `golang:<version>`, es la imagen por defecto. Si no estamos seguros de cuáles son nuestras necesidades, probablemente esta es la mejor opción. Además puede incluir etiquetas como pueden ser *bullseye*, *buster* o *stretch*. Estas etiquetas son los nombres de código de la suite para las versiones de Debian e indican en que versión se basa la imagen.
* `golang:<version>-alpine`, esta imagen se basa en el proyecto Alpine Linux. Las imagenes Alpine Linux son mucho más livianas que la mayoría de imágenes base de distribución (~5 MB). Esta variante es experimental y no es oficialmente compatible con el [proyecto Go](https://github.com/golang/go/issues/19938). La principal advertencia a tener en cuenta es que utiliza **musl libc** en lugar de **glibc**, puede llegar a provocar un comportamiento inesperador en nuestra aplicación. En [este artículo](https://news.ycombinator.com/item?id=10782897) se conversa acerca de los problemas que puede traer este tipos de imagenes.
* `golang:<version>-windowsservercore`, esta imagen se basa en Windows Server Core.

En nuestro caso tenemos que debatir se seleccionar la imagen basada en Debian o en Alpine. Como hemos comentado la principal diferencia entre estas es el tamaño y Alpine viene con la desventaja de que la imagen es una variante experimental pero esto para nuestro proyecto en un principio no acarrea ningún problema y se va a optar por la imagen Alpine por su menor tamaño respecto a Debian. En concreo la versión de la imagan va a ser la 1.17, hay una versión 1.18 a dia de hoy, 28/12/2021, pero es una versión beta.
Copy link

Choose a reason for hiding this comment

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

Entiendo que una de las claves de este objetivo es la justificación de la elección de la imagen base. En este sentido, creo que sería conveniente indicar primero cuáles son los requisitos en los que basar la elección además de incluir alguna justificación referente a si se han considerado más opciones como partir de una imagen con solo el sistema operativo e instalarle el lenguaje o sobre por qué se ha elegido la versión 1.17.

@LuisArostegui
Copy link
Owner Author

En general lo veo bastante completo, bien trabajado el uso de buenas prácticas y la documentación, pero creo que sería conveniente completar la justificación de la elección de la imagen base para el contenedor.

Por otro lado, pienso que sería apropiado que el milestone reflejara de manera más explícita el trabajo realizado en este objetivo.

También incluyo una observación sobre la automatización de la subida del contenedor.

Un último comentario menor es que los enlaces del comentario de este PR en la lista de comprobación no se están renderizando bien porque hay un espacio entre los corchetes y los paréntesis.

Gracias por todos los comentarios. He actualizado todo lo que me has comentado. Modifique también el milestone para indicar el trabajo que se realiza en el objetivo.

@amerigal
Copy link

amerigal commented Jan 3, 2022

Gracias por todos los comentarios. He actualizado todo lo que me has comentado. Modifique también el milestone para indicar el trabajo que se realiza en el objetivo.

No hay de qué! Diría que ahora está más clara y mejor argumentada la decisión de la imagen base ⭐
Te he comentado sobre una cuestión que creo que sería necesaria perfilar del despliegue de la imagen y que también influiría en la documentación y descripción del milestone, pero efectivamente la descripción del milestone ya está más completa con el trabajo de este objetivo.

@LuisArostegui
Copy link
Owner Author

@JJ, listo para revisión. Pese a que dos compañeros no han podido hacer la revisión pienso que con las correciones realizadas por @amerigal está bastante correcto el objetivo. Disculpe las molestias si no lo está bajo su punto de vista.

@LuisArostegui
Copy link
Owner Author

He actualizado la configuración para la publicación de la imagen mientras estaba se estaba trabajando en el objetivo 6 para avanzar, de ahi que no se ejecute correctamente los tests en Circle CI, ya que no hay configuración en esta rama, pero si se ejecuta correctamente la publicación de la imagen que es lo relativo a este objetivo.

Copy link

@amerigal amerigal left a comment

Choose a reason for hiding this comment

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

👍

@lentes4k
Copy link

lentes4k commented Jan 7, 2022

Vengo del hito 6 (es como venir del futuro... jajajaja) a comentarte lo de seguir las buenas prácticas y eliminar los comentarios inline (puedes dejar aclaraciones o cosas que sean 100% necesarias) lo demás debería de verse reflejado en la documentación

Los comentarios se añaden en la documentacion. Aparte de añade seccion para comparar tamaño de imagenes tomando distitnas imagenes base
@LuisArostegui
Copy link
Owner Author

Vengo del hito 6 (es como venir del futuro... jajajaja) a comentarte lo de seguir las buenas prácticas y eliminar los comentarios inline (puedes dejar aclaraciones o cosas que sean 100% necesarias) lo demás debería de verse reflejado en la documentación

Perfecto, lo acabo de corregir. He dejado sin comentarios el Dockerfile y el fichero dockerhub.yml. Todos los comentarios que tenía los he puesto en la documentación. Aparte he agregado una pequeña sección comparando el tamaño de las distintas imagenes base para comparar las imagenes de Debian con la de Alpine que creo que puede ser interesante hacer un estudio más profundo de todas las imagenes que podemos usar. Muchas gracias por el comentario. Ahora te agradezco el comentario del Objetivo 6 jajajajajajajaja

@LuisArostegui
Copy link
Owner Author

Una duda que tengo... Tengo claro que al hacer push y al modificarse el fichero Dockerfile o go.mod se va a lanzar la publicación de una nueva imagen del proyecto, es lo que indiqué en el fichero dockerhub.yml pero claro al actualizar el PR y como se ha modificado el fichero Dockerfile también se ha ejecutado la publicación de la imagen... Es del todo correcto? Debería de dejar solamente que se generé una nueva imagen cuando se actualice el PR y quitar la sección de cuando se hace push? Quité la parte de que si hago push a la rama main se generé una nueva imagen ya que nunca vamos a hacer un push al main porque en la asignatura se avanza haciendo PR...

Gracias de antemano a todos.

@JJ
Copy link

JJ commented Jan 7, 2022

Una duda que tengo... Tengo claro que al hacer push y al modificarse el fichero Dockerfile o go.mod se va a lanzar la publicación de una nueva imagen del proyecto, es lo que indiqué en el fichero dockerhub.yml pero claro al actualizar el PR y como se ha modificado el fichero Dockerfile también se ha ejecutado la publicación de la imagen... Es del todo correcto? Debería de dejar solamente que se generé una nueva imagen cuando se actualice el PR y quitar la sección de cuando se hace push? Quité la parte de que si hago push a la rama main se generé una nueva imagen ya que nunca vamos a hacer un push al main porque en la asignatura se avanza haciendo PR...

Gracias de antemano a todos.

La imagen actualizada debería funcionar también en esta rama. En todo caso, tienes un error en Circle, tendrás que solucionarlo...

@LuisArostegui
Copy link
Owner Author

es porque he estado trabajando en el objetivo 6 mientras estaba con el 5 arreglando algunas cosas. Una solución sería añadir la configuración de Circle CI en esta rama para que no no aparezca el error, pero toda la documentación la mantendría en el objetivo 6 y ya usted lo corrige todo de ahi. Gracias.

@JJ
Copy link

JJ commented Jan 7, 2022

es porque he estado trabajando en el objetivo 6 mientras estaba con el 5 arreglando algunas cosas. Una solución sería añadir la configuración de Circle CI en esta rama para que no no aparezca el error, pero toda la documentación la mantendría en el objetivo 6 y ya usted lo corrige todo de ahi. Gracias.

Lo que sea, pero no puede haber tests que no pasen.

@LuisArostegui LuisArostegui linked an issue Jan 7, 2022 that may be closed by this pull request
@LuisArostegui
Copy link
Owner Author

Corregido @JJ . Se comenta todo en #41 .

@JJ
Copy link

JJ commented Jan 8, 2022

  • ¿Se han seguido las buenas prácticas en las órdenes que construyen
    la imagen? ¿Especialmente las relativas a usuarios no privilegiados y demás? Sí, se puede comprobar aquí.

    • ¿La imagen incluye sólo y exclusivamente la infraestructura
      necesaria para pasar los tests? Sí, se puede comprobar aquí.

    • ¿Se han documentado y enlazado los commits a las imágenes que se
      han testeado? Sí, se puede comprobar aquí.

    • ¿Está establecido correctamente como WORKDIR el que se va a usar en los
      tests que se van a lanzar? Si, tanto en el Dockerfile como en la documentación se puede comprobar.

    • ¿Se han establecido claramente los criterios de búsqueda y de aceptación de
      la imagen base? Sí, se puede comprobar aquí.

    • ¿Tienes claro cuales son las condiciones en las que tiene que actualizarse
      la imagen en DockerHub y has configurado el workflow de acuerdo con ello? Si, se indica en el fichero dockerhub.yml. Y se indica en la documentación un enlace donde se ha obtenido la información.

Como te he comentado, sólo se están creando cuando cambia main. Ni siquiera tendría que funcionar en esta rama que has creado.

@JJ
Copy link

JJ commented Jan 8, 2022

  • ¿Se han seguido las buenas prácticas en las órdenes que construyen
    la imagen? ¿Especialmente las relativas a usuarios no privilegiados y demás? Sí, se puede comprobar aquí.

    • ¿La imagen incluye sólo y exclusivamente la infraestructura
      necesaria para pasar los tests? Sí, se puede comprobar aquí.

Es un documento, no un commit donde se hayan hecho pruebas sobre una imagen.

Copy link

@JJ JJ left a comment

Choose a reason for hiding this comment

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

En todos los objetivos de esta asignatura lo principal es que se entiendan (y se expliquen) los criterios para elegir una u otra herramienta, hasta el punto que se han dedicado varias clases exclusivamente a eso. No lo haces aquí y por eso no superas el objetivo, lo siento.

pull_request:
branches:
- main
paths:
Copy link

Choose a reason for hiding this comment

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

Si usas el contenedor para CI, ¿qué pasa si estás haciendo cambios en una rama?

Copy link
Owner Author

Choose a reason for hiding this comment

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

No llego a comprender porque no es correcto. Es decir, entiendo que cuando hago push y se modifica el fichero Dockerfile o go.mod se va a lanzar los jobs pero creo que es correcto la parte del pull_request el problema con como lo tengo montando sería que solo funciona si hago yo ese PR, porque si lo hace otra persona no va a tener mis credenciales para poder ejecutar la parte de jobs... no? Si estoy haciendo cambios en una rama y quiero crear una imagen cuando actualice esa rama pondría algo como... on: push: branches: - '*' y ya cada vez que haga push sea cual sea la rama, se ejecutará la publicación de la imagen, correcto?

Copy link

Choose a reason for hiding this comment

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

¿Cómo se está modificando el contenedor si estás haciendo push a una rama aquí? ¿Entiendes eso? Siempre vas a desarrollar en una rama...

Copy link
Owner Author

Choose a reason for hiding this comment

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

Vale, lo he modificado. Se publicará una imagen si se hace push y se modifica el fichero Dockerfile y go.mod. La parte de pull_request y branches ha sido eliminada. Asi entiendo que se crea la imagen correctamente este en la rama que este.

jobs:
test-app:
docker:
- image: luisarostegui/mywallet:latest
Copy link

Choose a reason for hiding this comment

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

¿Qué pasa si cambian las dependencias en una rama? Esto petará.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Vale, tendría que publicar una imagen cuando se modifique sea cual sea la rama de esta manera al realizar los tests no presentará problemas. Todo reside en que me falta añadir esa sección a mi fichero para publicar una imagen en Docker Hub

Dockerfile Outdated

WORKDIR $TEST_DIR

COPY go.mod ./
Copy link

Choose a reason for hiding this comment

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

Este directorio no debes usarlo para nada, es sólo para montar el externo...

Copy link
Owner Author

Choose a reason for hiding this comment

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

Vale, es decir, debería de hacer la copia de mi fichero de dependecias en la ruta /app (por ejemplo) y cuando tenga todo listo para ejecutar los tests hacer WORKDIR app/test, correcto?

Copy link

Choose a reason for hiding this comment

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

Lo que debes es entender que el directorio de trabajo es eso, un directorio de trabajo para que se monte el repo, no para ninguna otra cosa.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Corregido.

docs/CI.md Outdated
@@ -0,0 +1,38 @@
# Integración Continua - OBJETIVO 6
Copy link

Choose a reason for hiding this comment

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

Podrías haber intentado copiar a esta rama sólo los ficheros necesarios para el test, no todos ellos. Esto es tremendamente confuso.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Se eliminará.

docs/docker.md Outdated
@@ -0,0 +1,210 @@
# Contenedor de pruebas

En anteriores commits había información sobre Docker y algunas de sus alternativas pero como el objetivo indica que se use Docker se ha eliminado este apartado y evitar relleno.
Copy link

Choose a reason for hiding this comment

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

y si hubieras eliminado también esto...

Copy link
Owner Author

Choose a reason for hiding this comment

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

Se eliminará.

docs/docker.md Outdated

### Cuestiones a tener en cuenta en la elección de una imagen

Nos vamos a encontrar, en general, con dos tipos de imagen distintas, una basada en debian y otra en Alpine. Las diferencias entre estas dos son las siguientes:
Copy link

Choose a reason for hiding this comment

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

O en Ubuntu. O en Debian de diferentes tipos. ¿Qué significa "nos vamos a encontrar"?

docs/docker.md Outdated
Nos vamos a encontrar, en general, con dos tipos de imagen distintas, una basada en debian y otra en Alpine. Las diferencias entre estas dos son las siguientes:

* El tamaño entre una y otra. Alpine tiene un tamaño menor que Debian. Con esto conseguimos que operaciones como construir, pull y push sean más rápidas.
* Consumir menos memoria por el propio sistema operativo en comparación con Debian. Alpine se considera seguro y rápido.
Copy link

Choose a reason for hiding this comment

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

¿Y Debian es lento e inseguro? ¿Podéis intentar establecer unos criterios previos a la elección?

docs/docker.md Outdated

Los requisitos que se buscan son:

* El tamaño de la imagen debe de ser lo más ligera posible para agilizar el trabajo.
Copy link

Choose a reason for hiding this comment

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

¿Por qué se busca esto? ¿Qué tiene que ver con tu proyecto?

docs/docker.md Outdated

### Imagen de Golang

Docker tiene muchas imagenes de golang, cada una de ellas diseñada para un caso de uso específico:
Copy link

Choose a reason for hiding this comment

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

¿No hay más imágenes aparte de la oficial? ¿Has hecho una búsqueda en Docker Hub?

docs/docker.md Outdated
* `golang:<version>-alpine`, esta imagen se basa en el proyecto Alpine Linux. Las imagenes Alpine Linux son mucho más livianas que la mayoría de imágenes base de distribución (~5 MB). Esta variante es experimental y no es oficialmente compatible con el [proyecto Go](https://github.com/golang/go/issues/19938). La principal advertencia a tener en cuenta es que utiliza **musl libc** en lugar de **glibc**, puede llegar a provocar un comportamiento inesperador en nuestra aplicación. En [este artículo](https://news.ycombinator.com/item?id=10782897) se conversa acerca de los problemas que puede traer este tipos de imagenes.
* `golang:<version>-windowsservercore`, esta imagen se basa en Windows Server Core.

En nuestro caso tenemos que debatir se seleccionar la imagen basada en Debian o en Alpine. Como hemos comentado la principal diferencia entre estas es el tamaño y Alpine viene con la desventaja de que la imagen es una variante experimental pero esto para nuestro proyecto en un principio no acarrea ningún problema y se va a optar por la imagen Alpine por su menor tamaño respecto a Debian. En concreto la versión de la imagen va a ser la 1.17, hay una versión 1.18 a dia de hoy, 28/12/2021, pero es una versión beta, la version 1.17 es la última más estable actualmente.
Copy link

Choose a reason for hiding this comment

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

¿Por qué "tienes" que hacer eso?

@LuisArostegui
Copy link
Owner Author

En todos los objetivos de esta asignatura lo principal es que se entiendan (y se expliquen) los criterios para elegir una u otra herramienta, hasta el punto que se han dedicado varias clases exclusivamente a eso. No lo haces aquí y por eso no superas el objetivo, lo siento.

OK, lo entiendo. Estoy haciendo cambios en la documentación para entregar correctamente los siguientes objetivos. Aun asi le dejo unas dudas por aqui. No me vendría mal alguna tutoría (ya sea en presencial o preguntandole por Telegram) acerca de la documentación, pensaba que mi workflow era mejorable pero en ningún caso la documentación. Voy a investigar más y a repasar conceptos. Gracias.

@JJ
Copy link

JJ commented Jan 8, 2022 via email

@LuisArostegui
Copy link
Owner Author

LuisArostegui commented Jan 8, 2022

El sáb, 8 ene 2022 a las 20:45, Luis Arostegui Ruiz (< @.***>) escribió:
En todos los objetivos de esta asignatura lo principal es que se entiendan (y se expliquen) los criterios para elegir una u otra herramienta, hasta el punto que se han dedicado varias clases exclusivamente a eso. No lo haces aquí y por eso no superas el objetivo, lo siento. OK, lo entiendo. Estoy haciendo cambios en la documentación para entregar correctamente los siguientes objetivos. Aun asi le dejo unas dudas por aqui. No me vendría mal alguna tutoría (ya sea en presencial o preguntandole por Telegram) acerca de la documentación, pensaba que mi workflow era mejorable pero en ningún caso la documentación. Voy a investigar más y a repasar conceptos. Gracias.
Pon las dudas que tengas por aquí o, mejor, por el grupo de Telegram.

Se han corregido bastantes cosas. Tanto documentación como la configuración de Docker y la publicación de la imagen. Avanzaré en el resto de objetivos. El 6 está a pique de revisión en cuanto miré un par de cosas más. Gracias por las respuestas a las dudas.

LuisArostegui and others added 2 commits January 26, 2022 19:54
Elimina DOCKER_USER, mismo nombre que github pero en minusculas
@JJ
Copy link

JJ commented Jan 27, 2022

⛹ Revisores → @amerigal @modejota @Parka015

@XileonXL
Copy link

XileonXL commented Feb 2, 2022

Buenas, no he sido asignado a este objetivo pero paso por aquí para checkearlo y ayudar en lo que pueda 😄. Te dejo lo que he podido encontrar a mejorar aquí debajo.

Antes de nada, felicitarte por el curro que te has pegado (sobre todo en la parte de testeo de las imágenes).

@XileonXL
Copy link

XileonXL commented Feb 2, 2022

  • golang:<version>-alpine, esta imagen se basa en el proyecto Alpine Linux. Las imagenes Alpine Linux son mucho más livianas que la mayoría de imágenes base de distribución (~5 MB). Esta variante es experimental y no es oficialmente compatible con el proyecto Go. La principal advertencia a tener en cuenta es que utiliza musl libc en lugar de glibc, puede llegar a provocar un comportamiento inesperador en nuestra aplicación. En este artículo se conversa acerca de los problemas que puede traer este tipos de imagenes.

No me deja referenciarla directamente desde los "files changed" porque imagino que se ha mergeado desde el "futuro" como decía el compañero más arriba.

Creo que es conveniente añadir o por lo menos dejar constancia de si esa diferencia de librerías puede afectar a tu proyecto o no. Solo mencionas que puede llegar a provocar un comportamiento inesperado. ¿Lo sabes con certeza o es algo que intuyes? Sería mejor concretar.

@gomares
Copy link

gomares commented Feb 5, 2022

Hola buenas! Me he leído tu objetivo y ya que no tienes muchas revisiones pues al menos voy a dejarte la mía Luis 😃 .

docs/docker.md Outdated
@@ -6,9 +6,12 @@ Para tener una forma de hacer que la aplicación sea portable y esté lista para
* Debe de ser una imagen ligera, siempre que se pueda, es decir, tener las funcionalidades necesarias de Go para cumplir con la correcta construcción y ejecución de nuestro proyecto. Basicamente, esto sirve para acelerar la construcción, la implementación y también reducir costos con el almacenamiento y la salida de la red si está utilizando algún proveedor de la nube.
Copy link

Choose a reason for hiding this comment

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

Te falta una tilde en Básicamente

docs/docker.md Outdated

## Imagen de Golang

Teniendo en cuenta los requisitos nombrados podremos lograr un tamaño mínimo de imagen de Docker utilizando imagenes base que se centran en el minimalismo, como Alpine Linux. Dentro de Docker Hub nos vamos a centrar en la imagen oficial de Golang suministrada por Dockerhub, ya que hoy dia es la que más actualizaciones recibe y con mayor frecuencia. Tenemos otras opciones de *Verified Publisher* que son entidades comerciales que publican imagenes muy confiables y estan mantenidas por ellos, como Circle CI o portainer, en el caso de Circle CI las actualizaciones se reciben cada 3/4 semanas y en el caso de portainer la última actualización se recibió hace 4 años. Por tanto, teniendo en cuenta los requisitos nombrados vamos a centramos en las imagenes oficiales de Dockerhub.
Copy link

Choose a reason for hiding this comment

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

Aquí tienes errores ortográficos en "imagenes base","dia","estan mantenidas" y un error gramatical en "vamos a centramos en".

docs/docker.md Outdated
@@ -17,41 +20,127 @@ Las variantes que nos encontramos son:
* `golang:<version>-windowsservercore`, esta imagen se basa en Windows Server Core.


### Versiones de Go
Dentro de las posibles imagenes tenemos que saber elegir la versión de Go para ejecutar nuestro proyecto, las distintas versiones las podemos encontrar [aquí](https://go.dev/doc/devel/release). Tenemos que usar una versión que permita obtener los resultados esperados en nuestra aplicación, que tenga soporte y actualizaciones frecuentemente. Tanto [aquí](https://endoflife.date/go) como la [página oficial](https://go.dev/doc/devel/release), podemos ver que las versiones 1.16 y 1.17 son actualmente tienen soporte, por tanto las versiones 1.15 y anteriores quedan descartadas, según los requisitos nombrados. Vamos a tener en cuenta, por ahora, ambas versiones y realizaremos pruebas con ambas para obtener una conclusión final. Cabe mencionar que la versión 1.17 recibe actualizaciones más a menudo que la versión 1.16.
Copy link

Choose a reason for hiding this comment

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

Aquí el mismo errorcillo de poner imagenes sin tilde.


## Análisis de construcción

### Tiempos de construcción
Copy link

Choose a reason for hiding this comment

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

Muy buen trabajo en esta sección, se nota que lo has trabajado este objetivo.

docs/docker.md Outdated
sys 0m0,046s
```

Los resultados son prácticamente iguales, variando a nivel de milesimas.
Copy link

Choose a reason for hiding this comment

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

Tenías en caché las partes necesarias para construir el container ya? Recuerdo que a mí alpine al borrar todo me tardaba más, pero claro, era Python y no Go.

Copy link
Owner Author

Choose a reason for hiding this comment

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

En el documento dejo la instrucción con la que hago el análisis de tiempo de contrucción, en este caso indico el flag --no-cache. Si es verdad que la primera vez que quise hacer el análisis obtuve un tiempo pero que muy inferior con la imagen de Alpine con la versión 1.17 ya que fue la que use la primera vez que entregué el objetivo, pero investigué de como poder hacer la prueba sin necesidad de cargar de la cache nada para que sean resultados "justos" lo que obtuviese.

Es verdad que viendo correcciones a distintas personas que usan Python con Alpine he visto que había bastantes más problemas en cuanto a tiempo de compilación o incluso de tamaño, siendo en algunos casos más ligera la versión Slim que la de Alpine.

docs/docker.md Outdated
Encontramos una diferencia bastante notoria relativa al peso de las imagenes, la imagen Alpine es bastante más ligera que el resto.
Encontramos una diferencia bastante notoria relativa al peso de las imagenes, la imagen Alpine es bastante más ligera que el resto. La más ligera es la de la versión 1.16 de Go, pero en tan solo 14MB. Teniendo en cuenta nuestros requisitos, la imagen de Alpine es una clara candidata para nuestro proyecto.

Aun no se puede tomar una decisión final, analizando el rendimiento de las imagenes encontré [esta página](https://nickjanetakis.com/blog/benchmarking-debian-vs-alpine-as-a-base-docker-image) donde se realiza un benchmarking entre imagenes Debian y Alpine. La conclusión es que son dos imagenes con las que se obtienen resultados muy similares y que a no ser que se encuentren errores significativos con Alpine por su tamaño y velocidad es más recomendable.
Copy link

Choose a reason for hiding this comment

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

Te faltan tildes en aun y en imagenes.

@LuisArostegui
Copy link
Owner Author

  • golang:<version>-alpine, esta imagen se basa en el proyecto Alpine Linux. Las imagenes Alpine Linux son mucho más livianas que la mayoría de imágenes base de distribución (~5 MB). Esta variante es experimental y no es oficialmente compatible con el proyecto Go. La principal advertencia a tener en cuenta es que utiliza musl libc en lugar de glibc, puede llegar a provocar un comportamiento inesperador en nuestra aplicación. En este artículo se conversa acerca de los problemas que puede traer este tipos de imagenes.

No me deja referenciarla directamente desde los "files changed" porque imagino que se ha mergeado desde el "futuro" como decía el compañero más arriba.

Creo que es conveniente añadir o por lo menos dejar constancia de si esa diferencia de librerías puede afectar a tu proyecto o no. Solo mencionas que puede llegar a provocar un comportamiento inesperado. ¿Lo sabes con certeza o es algo que intuyes? Sería mejor concretar.

Muchas gracias por el comentario. He añadido información sobre los errores que puede acarrear ambas librerías y justificando porque no obtendremos errores si usamos una u otra librería. Espero que asi quede más claro. Gracias 😄 .

@LuisArostegui
Copy link
Owner Author

Hola buenas! Me he leído tu objetivo y ya que no tienes muchas revisiones pues al menos voy a dejarte la mía Luis 😃 .

Muchas gracias por los comentarios. Ya he puesto todas las tildes jajajajajajaja. Te dejo un comentario acerca de la caché cuando construyo cada imagen.

@LuisArostegui
Copy link
Owner Author

@JJ, listo para revisión.

Copy link

@JJ JJ left a comment

Choose a reason for hiding this comment

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

👍 Aunque gran parte de las comparaciones realmente no tienen sentido, por lo menos te has molestado en hacer comparciones, especialmente de tiempo de ejecución.


### Tiempos de construcción

En esta sección se generan los contenedores usando la herramienta `time`. Se ejecuta la orden `time docker build --no-cache . -f ./Dockerfile -t go_<bullseye|alpine>_1.1X`. Los resultados son los siguientes:
Copy link

Choose a reason for hiding this comment

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

Las comparaciones de tiempo de construcción no sirven para nada.

go_alpine_1.16 latest 7f201f405681 337MB
go_bullseye_1.16 latest dafa5fb683e3 954MB
go_bullseye_1.17 latest d2c15c730270 976MB
go_alpine_1.17 latest 8c6c1dda8a53 351MB
Copy link

Choose a reason for hiding this comment

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

En general, debéis usar siempre la última versión del sistema operativo, que es la que corrige las vulnerabilidades. Comparar versiones por tamaño no tiene sentido, porque son cosas diferentes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants