Changes to beforeserve to pick up storage array labels and parameters if provided#486
Conversation
and CreateVolume (as necessary) to pick up StorageArrays Labels and Parameters if provided within the new powermax secret.
donatwork
left a comment
There was a problem hiding this comment.
Please add unit tests to cover all of the cases, especially where no labels nor parameters are present.
getStorageArrays() func created and moved the code out of beforeServe.
donatwork
left a comment
There was a problem hiding this comment.
Please add unit tests.
| } | ||
|
|
||
| // StorageArrayConfig represents the configuration of a storage array in the config file | ||
| type StorageArrayConfig struct { |
There was a problem hiding this comment.
I know it may seem redundant but adding the arrayID is a good way to double check the structure. Data structures should be independent of the container used to store it. Without the arrayID it cannot stand on its own.
| } | ||
|
|
||
| opts.StorageArrays = make(map[string]StorageArrayConfig) | ||
| GetStorageArrays(secretParams, &opts) |
There was a problem hiding this comment.
Not that you need to change this but given that GetStorageArrays is a function and not a method you should not pass in the entire structure if GetStorageArrays is only modifying one part of the structure. Passing in the address of StorageArrays is safer and you pass in only what the function needs. So you could have coded as:
| GetStorageArrays(secretParams, &opts) | |
| opts.StorageArrays = GetStorageArrays(secretParams) |
In this way you simplify the code and reduce the amount of data that the function can modify.
There was a problem hiding this comment.
Don's comment makes sense. Please see if you update it to
opts.StorageArrays = GetStorageArrays(secretParams)
There was a problem hiding this comment.
will update in next PR.
Merging this branch will not change overall coverage
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. Changed unit test files
|
|
@anathoodell Please add a GitHub issue. |
|
| PodmonPort string // to indicates the port to be used for exposing podmon API health | ||
| PodmonPollingFreq string // indicates the polling frequency to check array connectivity | ||
| TLSCertDir string | ||
| StorageArrays map[string]StorageArrayConfig |
There was a problem hiding this comment.
It is not a full detail of storage arrays. Can we name it differently?
| } | ||
|
|
||
| opts.StorageArrays = make(map[string]StorageArrayConfig) | ||
| GetStorageArrays(secretParams, &opts) |
There was a problem hiding this comment.
Don's comment makes sense. Please see if you update it to
opts.StorageArrays = GetStorageArrays(secretParams)
Description
Changes to beforeServe in the driver container to allow for NodeGetInfo and CreateVolume (as necessary) to pick up StorageArrays Labels and Parameters if provided within the new powermax secret.
GitHub Issues
Checklist:
How Has This Been Tested?
pmax on k8s cluster.