PowerStore CSI driver Support for the multiple interface for iSCSI discovery + Increased UT Coverage#434
Conversation
| break | ||
| } | ||
| log.Debugf("Portal %s is not rechable from the node", address.Portal) | ||
| log.Debugf("Portal is not rechable from the node") |
There was a problem hiding this comment.
Any reason to not print the portal in the log?
There was a problem hiding this comment.
ipAddress will not have any value populated at this line.
There was a problem hiding this comment.
@karthikk92 Could you print 'address.Portal' here.? It will be helpful in debugging.
donatwork
left a comment
There was a problem hiding this comment.
Please add some more to the description regarding what was changed. A summary of changes perhaps?
| Controller: controllerService, | ||
| Identity: identityService, | ||
| Node: nodeService, | ||
| Interceptors: interList, |
There was a problem hiding this comment.
interList is not a good name, inter could be expanded into many things; interceptor, interface for example.
| } | ||
| s.SetArrays(arrays) | ||
| IPToArray = matcher | ||
| updateIPToArray(matcher) |
There was a problem hiding this comment.
You are not really updating, you are setting so should start the function with set*.
| for _, address := range infoList { | ||
| // first check if this portal is reachable from this machine or not | ||
| if ReachableEndPoint(address.Portal) { | ||
| ipAddressList := strings.Split(address.Portal, ":") |
There was a problem hiding this comment.
We may not yet support IPv6 but what's the impact here? is using colons the right choice of separator?
| for _, address := range infoList { | ||
| // first check if this portal is reachable from this machine or not | ||
| if ReachableEndPoint(address.Portal) { | ||
| ipAddressList := strings.Split(address.Portal, ":") |
There was a problem hiding this comment.
Same here for use of colons. You could encapsulate this in a common utility function.
30e2882
| break | ||
| } | ||
| log.Debugf("Portal %s is not rechable from the node", address.Portal) | ||
| log.Debugf("Portal is not rechable from the node") |
There was a problem hiding this comment.
@karthikk92 Could you print 'address.Portal' here.? It will be helpful in debugging.
Description
Bug Fixes:
Refactoring and Unit Tests:
Concurrency and Safety:
GitHub Issues
List the GitHub issues impacted by this PR:
Checklist:
How Has This Been Tested?