-
Notifications
You must be signed in to change notification settings - Fork 33
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
Consolidate local playbooks #73
Conversation
@jwmatthews what do you think about consolidating our local playbooks and using variables instead? |
@@ -39,7 +39,6 @@ | |||
-p DOCKERHUB_USER={{ dockerhub_user_name }} | |||
-p LAUNCH_APB_ON_BIND={{ broker_launch_apb_on_bind }} | |||
-p OUTPUT_REQUEST={{ broker_output_request }} | |||
-p RECOVERY={{ broker_recovery }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we will want to add this back now that the recovery pr has merged
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for catching that, that was unintentional
@@ -78,7 +78,7 @@ | |||
url: http://{{ ansible_service_broker_route }}/v2/bootstrap | |||
method: POST | |||
register: response | |||
until: response|success and response.json and response.json.spec_count | |||
until: response|success and response.json and response.json.spec_count is defined |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change keeps catasb from failing whenever someone doesn't have any apbs yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
Would you also update the README's to coincide, assume we need some directions to create my_vars.yml a level up, also some mention of the other .yml being shared in local directory. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was able to deploy using different Broker config options and worked great for me. Good job.
ACK.
dockerhub_org_name: example_org | ||
|
||
# Omit this unless you want to store your password in plain text to skip prompts. | ||
# dockerhub_user_password: example_password |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea to have a special note about this
No description provided.