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

Automatize deployment script #2

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

Conversation

pciruzzi
Copy link

@pciruzzi pciruzzi commented Jul 1, 2018

Comparto el script que hicimos para facilitar el deployment automatizandolo un poco. Está bastante comentado en el código qué es lo que hace.

No incluimos la parte del redis con Terraform, pero si querés también la podemos subir!

⚠️👁 Ojo que el bucket de S3 se llama tp-arquitecturas en este caso. No sé si querés que lo cambie de vuelta a cómo estaba (Al igual que la VPC y demás)

Copy link
Owner

@toblich toblich left a comment

Choose a reason for hiding this comment

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

Buenísimo, gracias por la colaboración!

Algunos pedidos nomás para emprolijar detallecitos y para que sea lo más reusable posible (y que las configs default funquen con mi cuenta, que soy el que va a estar probando esto devuelta en unos meses 😆)

variables.tf Outdated
@@ -24,17 +24,17 @@ variable "private_key_location" {
}

variable "vpc_id" {
default = "vpc-7563400d"
default = "vpc-d7cec3ac"
Copy link
Owner

Choose a reason for hiding this comment

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

Por favor, rollbackear los cambios de este archivo

start.sh Outdated
echo "##### Zipping node source code #####"
./zip
echo "##### Uploading node source code to S3 bucket #####"
aws s3 cp src.zip s3://tp-arquitecturas/src.zip --profile terraform
Copy link
Owner

Choose a reason for hiding this comment

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

Por favor, hacer que la url del código se lea de alguna configuración (archivo local, variable de entorno o argumento)

python.tf Outdated
@@ -13,6 +13,11 @@ resource "aws_instance" "python" {
command = "echo ${aws_instance.python.public_ip} > python/ip"
}

# Use the python app IP in node code
provisioner "local-exec" {
command = "sed -E \"s/(remoteBaseUri:)[^,]*,/\\1 'http:\\/\\/$(cat python/ip):3000',/\" node/config.js > node/temp.js && mv node/temp.js node/config.js"
Copy link
Owner

Choose a reason for hiding this comment

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

Sugerencia: Mejorar esto para que use el parámetro -i de sed en vez de crear un archivo temporal.

(tengo que hacer lo mismo en el paso en que configuro datadog dentro del user data yo, je)


while IFS='' read -r ip <&3 || [[ -n "$ip" ]]; do
$DIR/update $ip
done 3< ips
Copy link
Owner

Choose a reason for hiding this comment

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

Por favor, agregar un salto de línea al final para que GitHub no ponga símbolos feos en el diff 😅

node/update Outdated
$DIR/remote $IP "\
echo 'Downloading updated src' && \
curl https://s3.amazonaws.com/tp-arqui-node-app-src/src.zip > src.zip && \
curl https://s3.amazonaws.com/tp-arquitecturas/src.zip > src.zip && \
Copy link
Owner

Choose a reason for hiding this comment

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

Por favor, hacer que la url se lea de alguna configuración (por ejemplo, un archivo local que se llame source-location o similar)

Copy link
Author

Choose a reason for hiding this comment

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

Ahí estoy haciendo que lo lea de una variable de entorno. No sé si es posible hacer que esa misma se use en el variables.tf también, tenés idea?

Copy link
Owner

Choose a reason for hiding this comment

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

Sí, deberías poder hacer que el variables.tf lea del archivo usando una de las funciones de interpolación (file, como está hecho en esta línea). Nunca probé usarlo en el default del variables.tf, pero imagino que está soportado, no veo motivo por el que no lo estaría jajaja.

start.sh Outdated
./update "$ip"
done 3< ips

cd ..
Copy link
Owner

Choose a reason for hiding this comment

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

Por favor, agregar salto de línea al final del archivo (para evitar el siguiente símbolo feo en el diff de github nomás jajaja)
image

@@ -15,4 +15,5 @@ else
COMMAND="${@:2}"
fi

ssh -i $DIR/../key.pem ec2-user@$IP -- $COMMAND
# The -o StrictHostKeyChecking=no is to avoid asking for permission to add the $IP to the list of known hosts
ssh -o StrictHostKeyChecking=no -i $DIR/../key.pem ec2-user@$IP -- $COMMAND
Copy link
Owner

Choose a reason for hiding this comment

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

Genial jajaja 👍

node.tf Outdated
@@ -35,7 +35,7 @@ resource "aws_elb" "node_asg_elb" {
}

provisioner "local-exec" {
command = "echo ${aws_elb.node_asg_elb.dns_name} >> node/elb_dns"
command = "echo ${aws_elb.node_asg_elb.dns_name} > node/elb_dns"
Copy link
Owner

Choose a reason for hiding this comment

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

Este cambio ya está hecho en master, qué raro que siga apareciendo en el diff... Podrías hacer un rebase de la branch porfa (o desde una nueva branch, para no modificar lo que hicieorn uds para su tp)?

README.md Outdated

> **IMPORTANTE:** Es necesario tener instalado el [`aws-cli`](https://docs.aws.amazon.com/es_es/cli/latest/userguide/cli-chap-welcome.html) y [configurado](https://docs.aws.amazon.com/es_es/cli/latest/userguide/cli-config-files.html) con las credenciales correspondientes, donde además [se utiliza un perfil](https://docs.aws.amazon.com/es_es/cli/latest/userguide/cli-multiple-profiles.html) llamado `terraform`. Además, en el mismo se utiliza el binario de `terraform`, asumiendo que el mismo se encuentra en `~`. Por último, tal como se explicó antes, se asume que existe un bucket de S3 llamado `tp-arquitecturas`, al cual tiene acceso dicho usuario.

### Explicación
Copy link
Owner

Choose a reason for hiding this comment

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

Me encanta la documentación!

Lo único, qué opinás de mover la explicación detallada de cómo funciona el script al script mismo? O sea, comentarios arriba de cada línea explicando que hacen por ejemplo (en español está bien por mí, aunque la mayor parte del código esté en inglés), para no tener el código casi duplicado como queda así, que si alguien modifica cualquier cosa del script tiene que venir también a cambiar el README. El TL;DR sí me parece bien que esté acá para que el script tenga visibilidad.

@toblich
Copy link
Owner

toblich commented Jul 2, 2018

Lo de Redis me parece mejor que no esté en el repo, quiero que cada uno tenga que meterse en el terraform y AWS a aprender cómo es.
De hecho, estoy pensando que va a ser muy fácil para los que hagan el tp el próximo cuatri encontrar los forks de los que lo hicieron este cuatri, así que capaz que en vez de un Redis les pido algo distinto, para que les sea menos fácil copiarse si alguno se quiere tirar a chanta 🤷‍♂️

@pciruzzi
Copy link
Author

pciruzzi commented Jul 2, 2018

Me parece bien lo de redis. Si preferís, puedo hacerlo privado mi repo.

También ya modifiqué las cosas que me comentaste antes! Ahora veo lo del sed -i...

python.tf Outdated
@@ -13,6 +13,11 @@ resource "aws_instance" "python" {
command = "echo ${aws_instance.python.public_ip} > python/ip"
}

# Use the python app IP in node code
provisioner "local-exec" {
command = "sed -Ei '' \"s/(remoteBaseUri:)[^,]*,/\\1 'http:\\/\\/$(cat python/ip):3000',/\" node/config.js"
Copy link
Author

@pciruzzi pciruzzi Jul 2, 2018

Choose a reason for hiding this comment

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

El '' está por si el comando se ejecuta desde MacOS (En Linux no es necesario, y creo que de todas formas funciona igualmente así en Linux). Ver esta respuesta de SO.

Edit: Creo que habría que usar:

sed -Ei.bak \"s/(remoteBaseUri:)[^,]*,/\\1 'http:\\/\\/$(cat python/ip):3000',/\" node/config.js

Para que sea compatible tanto con Linux como con MacOS no?

Copy link
Owner

Choose a reason for hiding this comment

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

Mirá vos, curioso. Buen catch 👍

@toblich
Copy link
Owner

toblich commented Jul 2, 2018

Por lo de redis o hacer privado el repo, no te preocupes, sea público o privado igual hay otros forks y sigue siendo hiper fácil que circule jajaj. Veremos cuántas ganas tengo de inventar algo nuevo el cuatri que viene o si me dedico al trabajo profesional como debería en vez de cambiar esto que ya está bastante hecho 😅

@@ -1,11 +1,15 @@
locals {
Copy link
Author

Choose a reason for hiding this comment

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

Al final no pude interpolar dentro del variables.tf. Esta fue la forma que encontré de hacerlo posible 😄

Copy link
Owner

@toblich toblich left a comment

Choose a reason for hiding this comment

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

Bueno, te estoy haciendo laburar un montón jajajaja

Me gusta como está quedando todo 👍 , solo te pido una cosita respecto a que los scripts funquen sin necesitar que la terminal esté en un directorio en particular

node/update Outdated
$DIR/remote $IP "\
echo 'Downloading updated src' && \
curl https://s3.amazonaws.com/tp-arqui-node-app-src/src.zip > src.zip && \
curl https://s3.amazonaws.com/$(cat ../source_location)/src.zip > src.zip && \
Copy link
Owner

Choose a reason for hiding this comment

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

Cambiá porfa el ../source_location a algo tipo $DIR/../source_location para que no dependa de dónde se ubica la terminal, así se puede correr parado desde donde uno quiera (esa variable se setea como dice en la segunda respuesta de este post.

start.sh Outdated
cd python && ./start

# Since now, all the commands are executed within the node folder
cd ../node
Copy link
Owner

Choose a reason for hiding this comment

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

Por favor, seteá una variable $DIR como en los otros scripts y usala, para que se pueda correr desde cualquier lado

start.sh Outdated
# Create infrastructure. This also changes the python IP in the node app so that they can communicate
# The -auto-approve flag makes it not to ask for plan approval. Use at your own risk!
echo "##### Applying terraform infrastructure #####"
~/terraform apply -auto-approve
Copy link
Owner

Choose a reason for hiding this comment

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

Preferiría sacar el ~/ y suponer que terraform está agregado al path ya, con lo que esta línea quedaría directamente

terraform apply -auto-approve

start.sh Outdated
# applying the infrastructure, that is the one that starts that process) but the python one no.
# So, it's important to start it in the appropiate server:
echo "##### Starting python app #####"
cd python && ./start
Copy link
Owner

Choose a reason for hiding this comment

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

Esto se puede cambiar por algo tipo

$DIR/python/start

y así funciona independendientemente de dónde esté parada la terminal al ejecutar el script (ver otros scripts sobre cómo setearla).

start.sh Outdated
# The node code that is actually running has a wrong python server URL, so we must zip it again with the
# correct IP that has been changed when applying the python.tf infrastructure
echo "##### Zipping node source code #####"
./zip
Copy link
Owner

Choose a reason for hiding this comment

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

Usar un path absoluto con $DIR

start.sh Outdated
./zip
# And we must upload the src.zip file to the S3 bucket (Which name is in TP_ARQUI_S3_BUCKET env var) by using the aws-cli.
echo "##### Uploading node source code to S3 bucket #####"
aws s3 cp src.zip "s3://$(cat ../source_location)/src.zip" --profile terraform
Copy link
Owner

Choose a reason for hiding this comment

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

Usar paths absolutos con $DIR

start.sh Outdated
# The "3" indicates the file descriptor to run that loop, so that it doesn't intercede with stdin
while IFS='' read -r ip <&3 || [[ -n "$ip" ]]; do
echo "##### Updating node source code of instance with IP $ip #####"
./update "$ip"
Copy link
Owner

Choose a reason for hiding this comment

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

Usar paths absolutos con $DIR

Copy link
Author

@pciruzzi pciruzzi left a comment

Choose a reason for hiding this comment

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

Jajaj no pasa nada! Ahí pushie esos cambios y te dejé un comment

# Create infrastructure. This also changes the python IP in the node app so that they can communicate
# The -auto-approve flag makes it not to ask for plan approval. Use at your own risk!
echo "##### Applying terraform infrastructure #####"
terraform apply -auto-approve $DIR
Copy link
Author

Choose a reason for hiding this comment

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

Esto supuestamente corre los archivos de configuración que encuentra en $DIR. Eso parecería funcionar, pero por alguna razón, al correrlo desde algún lado que no es la carpeta raíz del proyecto, no me encuentra los providers:

➜  Documents terraform apply tp-arq-aws
Plugin reinitialization required. Please run "terraform init".
Reason: Could not satisfy plugin requirements.

Plugins are external binaries that Terraform uses to access and manipulate
resources. The configuration provided requires plugins which can't be located,
don't satisfy the version constraints, or are otherwise incompatible.

2 error(s) occurred:

* provider.aws: no suitable version installed
  version requirements: "(any version)"
  versions installed: none
* provider.template: no suitable version installed
  version requirements: "(any version)"
  versions installed: none

Terraform automatically discovers provider requirements from your
configuration, including providers used in child modules. To see the
requirements and constraints from each module, run "terraform providers".

Error: error satisfying plugin requirements

O sea, ahora está todo con absolute paths salvo que el terraform apply no estaría funcionando desde otro lado que no sea el root del proyecto

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.

2 participants