-
Notifications
You must be signed in to change notification settings - Fork 382
Conversation
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 agree with adding the Task section, good addition.
docs/tasks/index.md
Outdated
## [Install a Broker](./install_broker.md) | ||
|
||
How to add a broker to your catalog. This will make the services that broker | ||
available to users on your cluster. Adding it as a Cluster Service Broker |
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.
grammar... "This will make the services that broker available to users..."
"that a broker offers" ?
docs/tasks/index.md
Outdated
|
||
How to add a broker to your catalog. This will make the services that broker | ||
available to users on your cluster. Adding it as a Cluster Service Broker | ||
(CSB) will make it available cluster-wide. Adding it as a Service Broker, |
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.
you are making up new abbreviates here - - CSB and NSB. Not certain we want to do that. ??
You might try something like "A Service Broker can be registered with Service Catalog as either a Cluster Service Broker which makes its resources available cluster wide, to all users in all namespaces, or as a Service Broker which only exposes the resources within the specified namespace."
ACTUALLY.... how about just saying this task covers how to add a service broker. Leave the details about CSB & NSB to the actual task page?
docs/tasks/index.md
Outdated
|
||
## [Install a Broker](./install_broker.md) | ||
|
||
How to add a broker to your catalog. This will make the services that broker |
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 that first sentence should be a complete sentence that is grammatically correct. Maybe "You must install a broker into the service catalog to make the services it offers available."
docs/tasks/index.md
Outdated
|
||
## [Sync a Broker](./sync_broker.md) | ||
|
||
How to re-sync a broker that is already added to your catalog. When a broker's |
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.
Again I'd make the first sentence grammatically correct. "This task shows how to ..." or something. And I'd avoid the sync abbreviation vs spelling it out.
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.
my issue with spelling it out vs just saying 'sync' is we've already used sync in places - the command is called sync, the help in svcat calls it sync, etc.
docs/tasks/index.md
Outdated
## [Sync a Broker](./sync_broker.md) | ||
|
||
How to re-sync a broker that is already added to your catalog. When a broker's | ||
catalog changes, it may need to be resynced with Service Catalog. |
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.
resynchronized
docs/tasks/index.md
Outdated
also called a Namespaced Service Broker (NSB) will make it available | ||
only in a single namespace. | ||
|
||
## [Sync a Broker](./sync_broker.md) |
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.
Instead of Sync can we spell out Synchronize?
docs/tasks/install_broker.md
Outdated
done by creating either a ClusterServiceBroker or ServiceBroker object, and | ||
then allowing Service Catalog to sync with the broker. This is typically | ||
referred to as "registering" the broker. | ||
|
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 this is the right place to elaborate on the differences between Cluster Vs NS. "A Service Broker can be registered with Service Catalog as either a Cluster Service Broker which makes its resources available cluster wide, to all users in all namespaces, or as a Service Broker which only exposes the resources within the specified namespace."
docs/tasks/sync_broker.md
Outdated
layout: docwithnav | ||
--- | ||
|
||
A broker's catalog may occasionaly change the services it offers. A broker |
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.
nits about the wording here: a broker doesn't resync - - svcat resyncs.
like everywhere else, spell out sync. Good details though Jonathan.
docs/tasks/sync_broker.md
Outdated
A broker's catalog may occasionaly change the services it offers. A broker | ||
may resync automatically or may need to be resynced manually. By default, | ||
brokers are resynced automatically based on an interval that is globally | ||
set in Service Catalog. If a broker must be resynced immeadiately or |
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.
immediately
docs/tasks/sync_broker.md
Outdated
layout: docwithnav | ||
--- | ||
|
||
A broker's catalog may occasionaly change the services it offers. A broker |
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.
occasionally
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.
Therefore, Service Catalog must resync with the broker to get the updated services
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.
add the name of the global flag in the helm chart
`spec.relistBehavior` has been set to manual, then it can be resynced | ||
manually by incrementing `spec.relistRequests`. This can be done using svcat: | ||
```console | ||
$ svcat sync broker foobar --scope cluster |
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.
add command line example of updated catalog: 2 -> 3 classes
docs/tasks/install_broker.md
Outdated
$ svcat register foobarbroker --url http://foobarbroker.com --scope cluster | ||
``` | ||
|
||
Most actual brokers will require authenticaiton of some kind. To add this, use the |
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.
authentication
docs/tasks/sync_broker.md
Outdated
may resync automatically or may need to be resynced manually. By default, | ||
brokers are resynced automatically based on an interval that is globally | ||
set in Service Catalog. If a broker must be resynced immeadiately or | ||
`spec.relistBehavior` has been set to manual, then it can be resynced |
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.
use .spec...
of the broker
Looked at these with jonathan, his comments are my comments. |
- how to install a broker - how to sync a broker - refactor concepts section
/retest |
@MHBauer could you take a look at this? |
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.
browsed, the spelling seems good now.
/lgtm
Nice additions @jberkhahn |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jboyd01 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
- how to install a broker - how to sync a broker - refactor concepts section
This PR is a
Fixes #1795