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

Jump to bottom

Expose the pod name/namespace to APB #546

Merged
merged 1 commit into from Nov 13, 2017
Merged

Expose the pod name/namespace to APB #546

merged 1 commit into from Nov 13, 2017

Conversation

djzager
Copy link
Member

@djzager djzager commented Nov 10, 2017

Use kubernetes downward API to expose the pod name and namespace to the
APB. This will give us more flexibility w.r.t. managing APB execution,
secrets, and annotations.

@djzager
Expose the pod name/namespace to APB
1ca7b62 
Use kubernetes downward API to expose the pod name and namespace to the
APB. This will give us more flexibility w.r.t. managing APB execution,
secrets, and annotations.
@openshift-ci-robot openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Nov 10, 2017
@djzager
Copy link
Member Author

djzager commented Nov 10, 2017

I know that @maleck13 was working on the proposal in #537 where APBs could annotate themselves. Also, if we wish to move away from execing into containers to get APB credentials then the APB will need to know the pod name/namespace information.

@shawn-hurley I remember you suggesting using volumes, I can make that change, I just wanted to get eyeballs on this change.

@maleck13
Copy link
Contributor

This looks pretty reasonable to me. Wondering what advantages we would gain from using a volume rather than an envvar?

@shawn-hurley
Copy link
Contributor

@djzager I am starting to agree with @maleck13. I think that the env variables will be easy to interact with.

@jmrodri jmrodri self-requested a review November 10, 2017 16:03
jmrodri
jmrodri approved these changes Nov 10, 2017
View reviewed changes
Copy link
Contributor

@jmrodri jmrodri left a comment

Choose a reason for hiding this comment

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

Man kube apis are heinously ugly. My eyes hurt.

dymurray
dymurray approved these changes Nov 13, 2017
View reviewed changes
Copy link
Member

@dymurray dymurray left a comment

Choose a reason for hiding this comment

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

LGTM. This could also be used for handling deprovision as we expand the capability to do multiple provisions in the same namespace.

@djzager djzager merged commit 8dda327 into openshift:master Nov 13, 2017
jianzhangbjz pushed a commit to jianzhangbjz/ansible-service-broker that referenced this pull request May 17, 2018
@djzager
Expose the pod name/namespace to APB (openshift#546)
6a19cc0 
Use kubernetes downward API to expose the pod name and namespace to the
APB. This will give us more flexibility w.r.t. managing APB execution,
secrets, and annotations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jmrodri jmrodri jmrodri approved these changes

@eriknelson eriknelson eriknelson approved these changes

@dymurray dymurray dymurray approved these changes

Assignees
No one assigned
Labels
size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@djzager @maleck13 @shawn-hurley @jmrodri @eriknelson @dymurray @openshift-ci-robot