Skip to content
This repository has been archived by the owner on Dec 7, 2018. It is now read-only.

AEROGEAR-1970 Run ansible scripts inside AWS EC2 instance #39

Merged
merged 5 commits into from
Jan 31, 2018

Conversation

vchepeli
Copy link
Contributor

@vchepeli vchepeli commented Jan 30, 2018

Motivation

AEROGEAR-1970

Description

AEROGEAR-1970

Progress

  • Finished task
  • [] TODO

Additional Notes

@vchepeli vchepeli force-pushed the t_AEROGEAR-1970 branch 4 times, most recently from 6237cbc to ff53cb8 Compare January 30, 2018 10:00
@maleck13
Copy link
Contributor

@AdamSaleh Looking

Copy link
Contributor

@maleck13 maleck13 left a comment

Choose a reason for hiding this comment

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

changes look good, just one question. I want to run the installer locally too just to ensure everything works as expected still for local setup

}

stage("Run Ansible scripts") {
def metadata_endpoint = "http://169.254.169.254/latest/meta-data"
Copy link
Contributor

Choose a reason for hiding this comment

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

does this have to be hardcoded? do we want a follow up issue to remove it or will it never change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes this should be hardcoded. Thats static variable

@maleck13
Copy link
Contributor

@aidenkeating ok to take a quick look at this with your knowledge of the installer

Copy link
Contributor

@aidenkeating aidenkeating left a comment

Choose a reason for hiding this comment

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

Looks good

@david-martin
Copy link
Contributor

@vchepeli Technically, this is good to merge.
However, I would like to see some example in the readme/docs for the command & args to use to provision to a remote instance.

@vchepeli
Copy link
Contributor Author

@david-martin they are in Jenkinsfile. Arguments should be same, there is only AWS specific public hostname and routing suffix

@david-martin
Copy link
Contributor

Thanks for the clarification @vchepeli.
Is this intended just for automation/testing, or is it intended for someone evaluating mobile in a remote openshift cluster?

readonly TAG="${5}"
readonly WILDCARD_DNS="${6}"
readonly ANSIBLE_SERVICE_BROKER_NAMESPACE="${7}"
readonly DOCKERHUB_USER=${1:?"[ERROR]You must provide a dockerhub username."}
Copy link
Contributor

Choose a reason for hiding this comment

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

@vchepeli I'm hitting an issue with local provsion with this change.

My dockerhub password has a symbol ) in it, which doesn't get parsed OK.

"stderr_lines": ["/bin/sh: -c: line 0: syntax error near unexpected token `)'"

Can we add back quotes around this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added already. @david-martin can you try it again please?

@AdamSaleh
Copy link
Contributor

@david-martin this is mostly for automation/testing.

@@ -58,7 +58,7 @@
mode: u+x

- name: Execute OpenShift Ansible Broker (OAB) provision script
shell: bash -e /tmp/provision-ansible-service-broker.sh "{{ dockerhub_username }}" "{{ dockerhub_password }}" "{{ dockerhub_org }}" "{{ launch_apb_on_bind }}" "{{dockerhub_tag}}" "{{ wildcard_dns_host }}" "{{ ansible_service_broker_namespace }}"
shell: "bash -e /tmp/provision-ansible-service-broker.sh {{ dockerhub_username }} {{ dockerhub_password }} {{ dockerhub_org }} {{ launch_apb_on_bind }} {{dockerhub_tag}} {{ cluster_public_ip }} {{ wildcard_dns_host }} {{ ansible_service_broker_namespace }}"
Copy link
Contributor

Choose a reason for hiding this comment

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

@vchepeli looks like it needs to be escaped here too. Not sure why the escaping was removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure why the escaping was removed

There was no comment why it escapes every symbol and I removed it

@david-martin david-martin merged commit a5f0dfb into master Jan 31, 2018
@david-martin david-martin deleted the t_AEROGEAR-1970 branch January 31, 2018 13:23
@david-martin
Copy link
Contributor

Thanks @vchepeli 👍

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.

None yet

6 participants