CSPL-588: This PR improves code coverage for clustermaster specific code

[gaurav-splunk]

No description provided.

[sgontla]

Any reason we are removing this error check? This was introduced to handle the case when somebody accidentally deletes the secret object consisting the S4 keys.

[gaurav-splunk]

I knew this would come up :). If you look at the code for AreRemoteVolumeKeysChanged, there is no case in that function where we are returning true and also returning err as well.

[sgontla]

Good point Gaurav. That was a miss i guess. In that case, we need to handle it differently such that we still return for that case.

[smohan-splunk]

Something like explicitly checking the return value of AreRemoteVolumeKeysChanged and when it is false, checking if it has thrown as err?
Am I right in saying ApplyClusterMaster shouldn't proceed if smartstore was configured but the secret object is not accessible anymore?

[gaurav-splunk]

yes, you are right! we should error out in that case. So in short, we need to take care of case where it returns false with error. Let me fix it and push a change again.

[sgontla]

I think we are in the flow of removing the mock flag, and fixing those dependencies? In my earlier changes, I removed the mock dependencies, and that even created some holes in the coverage, so I raised a couple of stories to fix them formally.

[gaurav-splunk]

As discussed this thing touches MC code, hence I left it as is as it might require some more code changes on MC side.

[akondur]

Looks good!

[gaurav-splunk]

This should take care of above comment.