Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add test for ControllerPublishVolume #84

Merged
merged 2 commits into from
Jul 2, 2018

Conversation

codenrhoden
Copy link
Contributor

I'm opening this PR to demonstrate an error I am seeing with csi-test master. When updating the CSI external attacher/detacher, I always get a test failure for ControllerPublishVolume. In fact it hangs - took a while to track it down to this.

The reflect.DeepEqual that gomock performs to check for object equivalence is failing for the ControllerPublishVolume input. Oddly, it is only that call that fails - all the other tests pass. For simplicity, I added a test here in csi-test that demonstrates it.

When the test fails, you will see the following:

Unexpected call to *driver.MockControllerServer.ControllerPublishVolume([context.Background.WithCancel.WithCancel.WithValue(peer.peerKey{}, &peer.Peer{Addr:(*net.TCPAddr)(0xc4201b0c60), AuthInfo:credentials.AuthInfo(nil)}).WithValue(metadata.mdIncomingKey{}, metadata.MD{":authority":[]string{"127.0.0.1:51284"}, "content-type":[]string{"application/grpc"}, "user-agent":[]string{"grpc-go/1.12.2"}}).WithValue(grpc.streamKey{}, <stream: 0xc42032a000, /csi.v0.Controller/ControllerPublishVolume>) volume_id:"myname" node_id:"MyNodeID" volume_capability:<mount:<> access_mode:<mode:MULTI_NODE_MULTI_WRITER > > ]) at /Users/trhoden/go/src/github.com/kubernetes-csi/csi-test/driver/driver.mock.go:522 because:
Expected call at /Users/trhoden/go/src/github.com/kubernetes-csi/csi-test/driver/driver.mock.go:529 doesn't match the argument at index 1.
Got: volume_id:"myname" node_id:"MyNodeID" volume_capability:<mount:<> access_mode:<mode:MULTI_NODE_MULTI_WRITER > >
Want: is equal to volume_id:"myname" node_id:"MyNodeID" volume_capability:<mount:<> access_mode:<mode:MULTI_NODE_MULTI_WRITER > >
panic: MOCK TEST FATAL FAILURE

Note that the Expected and Got lines show the object is correct, but it is not being picked up that way. I've tried using different versions of gomock, grpc, regenerating the mock objects, etc., but haven't gotten around this one yet. Help is appreciated!

The default gomock.Eq matcher uses reflect.DeepEqual to test for
equality, but as of protobuf 1.1.0, this will not work. Instead,
proto.Equal() needs to be used. Thus, implement a custom
protobuf matcher to test for message equality.

Add additional test for ControllerPublishVolume, which was the RPC seen
to trigger the protobuf comparison failure.
@codenrhoden
Copy link
Contributor Author

I identified the underlying issue to be that since the switch to protobuf 1.1.1, one cannot use reflect.DeepEqual to test for equivalence of proto.Message. So, I added a custom matcher that instead uses proto.Equal, and things look good!

See https://groups.google.com/forum/#!topic/protobuf/N-elvFu4dFM for details on the protobuf change. This should be ready to merge now.

@lpabon
Copy link
Member

lpabon commented Jun 29, 2018

@codenrhoden Hi, thanks for the PR. What do you suggest. Would you suggest adding this as a library to ci-test?

@lpabon lpabon merged commit 539883b into kubernetes-csi:master Jul 2, 2018
suneeth51 pushed a commit to suneeth51/csi-test that referenced this pull request Sep 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants