search

ContainerSolutions / dcos-ansible-packet

Ansible playbook for installing DC/OS on Packet cloud
4 stars 3 forks source link

Review codebase #9

Closed frankscholten closed 7 years ago

frankscholten commented 7 years ago

@thijsschnitger Can you have a quick look at this code?

frankscholten commented 7 years ago

I already created issues for a few improvements. See https://github.com/ContainerSolutions/dcos-ansible/projects/1

thijsschnitger commented 7 years ago

where does this ip address come from? https://github.com/ContainerSolutions/dcos-ansible/blob/master/playbook.yml#L57

thijsschnitger commented 7 years ago

https://github.com/ContainerSolutions/dcos-ansible/blob/master/setup_ip_whitelist.sh

thijsschnitger commented 7 years ago

https://github.com/ContainerSolutions/dcos-ansible/blob/master/roles/bootstrap/tasks/main.yml#L15 store the url in a var https://github.com/ContainerSolutions/dcos-ansible/blob/master/roles/bootstrap/tasks/main.yml#L22 same here

https://github.com/ContainerSolutions/dcos-ansible/blob/master/roles/bootstrap/tasks/main.yml#L43 https://github.com/ContainerSolutions/dcos-ansible/blob/master/roles/bootstrap/tasks/main.yml#L47 add name to task, this is clearer when debugging.

thijsschnitger commented 7 years ago

https://github.com/ContainerSolutions/dcos-ansible/blob/master/roles/bootstrap/templates/config.yaml.j2#L10-L11 use variables instead of fixed ip's

frankscholten commented 7 years ago

Thanks for reviewing!