Read storage-cli config JSON from capi (AzureRM) - #4581
Conversation
Stop building storage-cli JSON inside CC; instead consume files rendered by capi job templates. StorageCliClient now selects the JSON file by `resource_type`.
05de380 to
dbf843c
Compare
dbf843c to
8c8c630
Compare
e763ccc to
4d069a4
Compare
4d069a4 to
4c7d0cc
Compare
jochenehret
left a comment
There was a problem hiding this comment.
Looks good, just some minor findings.
| raise BlobstoreError.new("Unknown resource_type: #{resource_type}") | ||
| end | ||
|
|
||
| path = VCAP::CloudController::Config.config.get(key) |
There was a problem hiding this comment.
What if config.get(key) returns nil? Maybe include key in the error message?
There was a problem hiding this comment.
If resource_type does not match any of the specified cases line 50-54, the else branch will raise a BlobstoreError before key is assigned, so execution will not reach line 59 with a nil value for key.
| json | ||
| end | ||
|
|
||
| def config_path_for!(resource_type) |
There was a problem hiding this comment.
Why an exclamation mark in the method name? It doesn't modify the passed object. Same for the other methods in this class.
There was a problem hiding this comment.
In cloud controller validation/check functions that raise an error end with an exclamation mark vs validation/check function that raise not an error do not end with an exclamation mark..
…nt is not building the config any more
5c884e0 to
2596e92
Compare
Changes in cloud_controller_ng:
- Read storage-cli config JSON from capi (AzureRM)
PR: cloudfoundry/cloud_controller_ng#4581
Author: Katharina Przybill <30441792+kathap@users.noreply.github.com>
* Read storage-cli config JSON from capi (AzureRM) Stop building storage-cli JSON inside CC; instead consume files rendered by capi job templates. StorageCliClient now selects the JSON file by `resource_type`.
Stop building storage-cli JSON inside CC; instead consume files rendered by capi job templates. StorageCliClient now selects the JSON file by
resource_type.Thanks for contributing to cloud_controller_ng. To speed up the process of reviewing your pull request please provide us with:
A short explanation of the proposed change:
Adopt storage_cli_client to read json configs rendered by capi. Detect correct json file by resource_type.
An explanation of the use cases your change solves
Switch responsibility of JSON generation to capi-release so CC only reads well-formed per-resource configs. This removes duplication of storage cli config file creation.
Links to any other associated PRs
Azure storage-cli integration capi-release#580
I have reviewed the contributing guide
I have viewed, signed, and submitted the Contributor License Agreement
I have made this pull request to the
mainbranchI have run all the unit tests using
bundle exec rakeI have run CF Acceptance Tests