Skip to content

Comments

Add OAB proxy config docs#8024

Merged
adellape merged 1 commit intoopenshift:masterfrom
adellape:oab_proxy
Mar 27, 2018
Merged

Add OAB proxy config docs#8024
adellape merged 1 commit intoopenshift:masterfrom
adellape:oab_proxy

Conversation

@adellape
Copy link
Contributor

@adellape adellape commented Mar 6, 2018

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Mar 6, 2018
@adellape
Copy link
Contributor Author

adellape commented Mar 6, 2018

@adellape
Copy link
Contributor Author

adellape commented Mar 6, 2018

@eriknelson PTAL, based on openshift/ansible-service-broker#634.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@eriknelson This bit "explicitly if recognizes them" seems like a typo, but I wasn't sure what it should be. Maybe "explicitly if it recognizes them"?

@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 6, 2018
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The original content had dh here, though in the OCP doc we've only mentioned the dockerhub type. Are either acceptable?

Choose a reason for hiding this comment

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

There are two fields in a registry configuration, the type. In this case, only dockerhub is acceptable. The name defaults to dh, but this can be an arbitrary value. dh, or another relatively short name is preferred, as it's used as part of the ServiceClass label, which can become quite long.

@adellape
Copy link
Contributor Author

adellape commented Mar 6, 2018

Alternate preview from a different branch where I'm trying out splitting oab_broker_configuration.adoc into a subdir w/ separate topics:

http://file.rdu.redhat.com/~adellape/030618/oab_auth/install_config/oab/oab_proxy.html

Choose a reason for hiding this comment

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

Copy link

@eriknelson eriknelson 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 and correct to me, although I think there may be a few things missing from this document that I think should be added (and I probably owe).

  1. WRT: Configuring the broker for a proxy, the upstream documentation is written from a developer's perspective which are the steps required to manually configure a broker.

I'm assuming most consumers of these docs will be using openshift-ansible, which we have patched so the broker deployment inherits the cluster-wide proxy settings.

We should split the section into something like: Configuring the broker with a proxy using openshift-ansible, and then Configuring the broker manually.

  1. Another default behavior for openshift-ansible is that it will generate the NO_PROXY list, and include the clusterNetworkCIDR and the serviceNetworkCIDR. It's important these are added if configuring manually. I will follow up and confirm the advised NO_PROXY list today.

@adellape
Copy link
Contributor Author

adellape commented Mar 9, 2018

@adellape adellape added the peer-review-needed Signifies that the peer review team needs to review this PR label Mar 9, 2018
Copy link
Contributor

Choose a reason for hiding this comment

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

s/vars/variables

@ahardin-rh ahardin-rh added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Mar 9, 2018
@eriknelson
Copy link

@adellape New docs look good, that's the correct NO_PROXY 👍, ACK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-3.9 peer-review-done Signifies that the peer review team has reviewed this PR size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants