Broker bind output #25

rthallisey · 2017-05-24T18:43:53Z

Rework of the broker bind output. We're going to have the broker exec into the container to gather bind creds.
Ref: openshift/ansible-service-broker#124

cfchase · 2017-05-25T19:48:16Z

postgresql-apb/roles/postgresql-apb-openshift/tasks/main.yml

- debug:
-    msg: "<BIND_CREDENTIALS>{{ encoded_bind_credentials.stdout }}</BIND_CREDENTIALS>"
+- copy:
+   content="<BIND_CREDENTIALS>{{ encoded_bind_credentials.stdout }}</BIND_CREDENTIALS>"


Can we use colons instead of equals?

- copy: content: "<BIND_CREDENTIALS>{{ encoded_bind_credentials.stdout }}</BIND_CREDENTIALS>" dest: /etc/ansible-service-broker/bind-creds

instead of:

- copy: content="<BIND_CREDENTIALS>{{ encoded_bind_credentials.stdout }}</BIND_CREDENTIALS>" dest=/etc/ansible-service-broker/bind-creds

Now that bind credentials are in their own file instead of scraping a log, do we still need to use <BIND_CREDENTIALS> tags? Can the broker just read it without having to look for and omit the tags?

Ya I can make that work.

I'm going to fix <BIND_CRENTIALS> in another patch #34

jmrodri

More of a question, I'm a little confused how all this fits together. maybe because it's 1:13am and I'm not thinking clearly. :)

jmrodri · 2017-05-26T05:11:02Z

apb-base/Dockerfile

             /etc/ansible /opt/ansible \
             ${BASE_DIR} ${BASE_DIR}/etc \
             ${BASE_DIR}/.kube ${BASE_DIR}/.ansible/tmp && \
             useradd -u ${USER_UID} -r -g 0 -M -d ${BASE_DIR} -b ${BASE_DIR} -s /sbin/nologin -c "apb user" ${USER_NAME} && \
             chown -R ${USER_NAME}:0 /opt/{ansible,apb} && \
+             chown -R ${USER_NAME}:0 /etc/ansible-service-broker && \


I actually find it weird to see ansible-service-broker in the base apb image. Maybe it's not a bad thing, but I find it weird to see it leaking in. Especially in /etc/ which I think of a config directory

I'm using this location to hold the bind credentials for the apb. I can change it to be /etc/apb if that makes it clearer.

Agree with wanting to remove 'ansible-service-broker' and go with a different name, also feels weird to be writing into '/etc' for this, I would think we'd want a dir under /var

I figured '/etc' because it's storing credentials. But I could also see '/var/lib/apb' since it's data relative to the apb.

I never addressed this. I think we want this to be apb.

cfchase

Switch to : from = and test.

rthallisey · 2017-06-01T16:23:38Z

@cfchase tested : for =. I'll remove the <BIND_CRENTIALS> tag later.

jmrodri · 2017-06-05T02:45:17Z

@rthallisey I think I understand what's going on. We used to just print out the credentials to stdout. The container could conceivably go away before the broker saw the credentials. This patch seems to keep the container running until broker can get the creds. And instead of using stdout, it uses a file on the container.

jwmatthews · 2017-06-23T20:32:02Z

Before we merge this I want to confirm the workflow with a few others on AOS to be sure we are on right track.

Tracking this in card:
https://trello.com/c/PZJfBeph

jwmatthews · 2017-06-27T19:06:57Z

Verified this approach is good to go.

We want to get this in when we can.

- Script for the broker to use when it execs into the container - Script that will keep the apb running until the broker scoops up the bind credentials

… playbooks

This patch was written when only bind was returning credentials. Reconfigure this to support both provision and bind.

eriknelson

NACK pending changes from the broker PR:

openshift/ansible-service-broker#124 (review)

Only execute a one time command and exit so we keep the retry logic in one place.

eriknelson · 2017-07-13T20:32:34Z

Merging, openshift/ansible-service-broker#124 just got merged to broker master. This PR is required for any images that will be run by that broker moving forwards.

This was referenced May 24, 2017

Broker bind output rework openshift/ansible-service-broker#124

Merged

Added instructions for bind ansibleplaybookbundle/ansible-playbook-bundle#39

Merged

cfchase reviewed May 25, 2017

View reviewed changes

jmrodri requested changes May 26, 2017

View reviewed changes

jmrodri approved these changes May 30, 2017

View reviewed changes

cfchase requested changes May 31, 2017

View reviewed changes

rthallisey force-pushed the broker-bind-output branch from f56c0a6 to 19692aa Compare June 1, 2017 16:21

cfchase approved these changes Jun 1, 2017

View reviewed changes

rthallisey force-pushed the broker-bind-output branch from 19692aa to 2bf8385 Compare June 2, 2017 19:50

jmrodri approved these changes Jun 5, 2017

View reviewed changes

rthallisey force-pushed the broker-bind-output branch from 2bf8385 to f795bd6 Compare June 6, 2017 14:31

jwmatthews changed the title ~~Broker bind output~~ [DO NOT MERGE] Broker bind output Jun 23, 2017

jwmatthews changed the title ~~[DO NOT MERGE] Broker bind output~~ Broker bind output Jun 27, 2017

rthallisey force-pushed the broker-bind-output branch 3 times, most recently from bb4e706 to c3b7642 Compare June 29, 2017 21:21

rthallisey force-pushed the broker-bind-output branch 2 times, most recently from 796ca99 to 39ef43b Compare July 11, 2017 18:52

Ryan Hallisey added 6 commits July 12, 2017 07:33

Rework the Broker Bind Output

4509fe3

- Script for the broker to use when it execs into the container - Script that will keep the apb running until the broker scoops up the bind credentials

Update the existing APBs with bind to use the new bind method

b59c831

Post rebase additions

e6a7dd3

Change ansible-service-broker to apb

4640f87

Change bind creds directory from ansible-service-broker to apb in the…

c2258f6

… playbooks

Fix logic to support provision and bind

544feb5

This patch was written when only bind was returning credentials. Reconfigure this to support both provision and bind.

rthallisey force-pushed the broker-bind-output branch from 39ef43b to 67d8f2b Compare July 12, 2017 11:36

eriknelson requested changes Jul 12, 2017

View reviewed changes

Add a timeout to bind-init

2fac6eb

rthallisey force-pushed the broker-bind-output branch from 67d8f2b to 2fac6eb Compare July 13, 2017 15:12

rthallisey closed this Jul 13, 2017

rthallisey reopened this Jul 13, 2017

Move all retry logic to the broker

adea5dd

Only execute a one time command and exit so we keep the retry logic in one place.

eriknelson approved these changes Jul 13, 2017

View reviewed changes

eriknelson merged commit e480a05 into fusor:master Jul 13, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Broker bind output #25

Broker bind output #25

rthallisey commented May 24, 2017 •

edited

cfchase May 25, 2017 •

edited

cfchase May 25, 2017

rthallisey May 25, 2017

rthallisey Jun 1, 2017

jmrodri left a comment

jmrodri May 26, 2017

rthallisey May 26, 2017

jwmatthews Jun 7, 2017

rthallisey Jun 7, 2017

rthallisey Jun 28, 2017

cfchase left a comment

rthallisey commented Jun 1, 2017

jmrodri commented Jun 5, 2017

jwmatthews commented Jun 23, 2017

jwmatthews commented Jun 27, 2017

eriknelson left a comment

eriknelson commented Jul 13, 2017

Broker bind output #25

Broker bind output #25

Conversation

rthallisey commented May 24, 2017 • edited

cfchase May 25, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmrodri left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cfchase left a comment

Choose a reason for hiding this comment

rthallisey commented Jun 1, 2017

jmrodri commented Jun 5, 2017

jwmatthews commented Jun 23, 2017

jwmatthews commented Jun 27, 2017

eriknelson left a comment

Choose a reason for hiding this comment

eriknelson commented Jul 13, 2017

rthallisey commented May 24, 2017 •

edited

cfchase May 25, 2017 •

edited