From 495e62180cc36806bb17b8363c2aca6a8dfeae29 Mon Sep 17 00:00:00 2001 From: Sunny Date: Fri, 15 Jun 2018 13:44:36 +0530 Subject: [PATCH 1/7] sanity: Add more ControllerPublishVolume tests Adds tests for: * should fail when the volume does not exist * should fail when the node does not exist * should fail when the volume is already published but is incompatible --- mock/service/controller.go | 33 +++++- pkg/sanity/controller.go | 206 +++++++++++++++++++++++++++++++++++++ 2 files changed, 237 insertions(+), 2 deletions(-) diff --git a/mock/service/controller.go b/mock/service/controller.go index d4a44f84..53cee9ee 100644 --- a/mock/service/controller.go +++ b/mock/service/controller.go @@ -16,6 +16,7 @@ import ( const ( MaxStorageCapacity = tib + ReadOnlyKey = "readonly" ) func (s *service) CreateVolume( @@ -135,21 +136,48 @@ func (s *service) ControllerPublishVolume( // Check to see if the volume is already published. if device := v.Attributes[devPathKey]; device != "" { + var volRo bool + var roVal string + if ro, ok := v.Attributes[ReadOnlyKey]; ok { + roVal = ro + } + + if roVal == "true" { + volRo = true + } else { + volRo = false + } + + // Check if readonly flag is compatible with the publish request. + if req.GetReadonly() != volRo { + return nil, status.Error(codes.AlreadyExists, "Volume published but has incompatible readonly flag") + } + return &csi.ControllerPublishVolumeResponse{ PublishInfo: map[string]string{ - "device": device, + "device": device, + "readonly": roVal, }, }, nil } + var roVal string + if req.GetReadonly() { + roVal = "true" + } else { + roVal = "false" + } + // Publish the volume. device := "/dev/mock" v.Attributes[devPathKey] = device + v.Attributes[ReadOnlyKey] = roVal s.vols[i] = v return &csi.ControllerPublishVolumeResponse{ PublishInfo: map[string]string{ - "device": device, + "device": device, + "readonly": roVal, }, }, nil } @@ -192,6 +220,7 @@ func (s *service) ControllerUnpublishVolume( // Unpublish the volume. delete(v.Attributes, devPathKey) + delete(v.Attributes, ReadOnlyKey) s.vols[i] = v return &csi.ControllerUnpublishVolumeResponse{}, nil diff --git a/pkg/sanity/controller.go b/pkg/sanity/controller.go index 0fb22392..d40e8795 100644 --- a/pkg/sanity/controller.go +++ b/pkg/sanity/controller.go @@ -806,6 +806,212 @@ var _ = Describe("ControllerPublishVolume [Controller Server]", func() { _, err = c.DeleteVolume(context.Background(), delReq) Expect(err).NotTo(HaveOccurred()) }) + + It("should fail when the volume does not exist", func() { + + By("calling controller publish on a non-existent volume") + + pubReq := &csi.ControllerPublishVolumeRequest{ + VolumeId: "some-vol-id", + NodeId: "some-node-id", + VolumeCapability: &csi.VolumeCapability{ + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, + }, + }, + Readonly: false, + } + + if secrets != nil { + pubReq.ControllerPublishSecrets = secrets.ControllerPublishVolumeSecret + } + + conpubvol, err := c.ControllerPublishVolume(context.Background(), pubReq) + Expect(err).To(HaveOccurred()) + Expect(conpubvol).To(BeNil()) + + serverError, ok := status.FromError(err) + Expect(ok).To(BeTrue()) + Expect(serverError.Code()).To(Equal(codes.NotFound)) + }) + + It("should fail when the node does not exist", func() { + + // Create Volume First + By("creating a single node writer volume") + name := "sanity" + req := &csi.CreateVolumeRequest{ + Name: name, + VolumeCapabilities: []*csi.VolumeCapability{ + { + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, + }, + }, + }, + } + + if secrets != nil { + req.ControllerCreateSecrets = secrets.CreateVolumeSecret + } + + vol, err := c.CreateVolume(context.Background(), req) + Expect(err).NotTo(HaveOccurred()) + Expect(vol).NotTo(BeNil()) + Expect(vol.GetVolume()).NotTo(BeNil()) + Expect(vol.GetVolume().GetId()).NotTo(BeEmpty()) + + // ControllerPublishVolume + By("calling controllerpublish on that volume") + + pubReq := &csi.ControllerPublishVolumeRequest{ + VolumeId: vol.GetVolume().GetId(), + NodeId: "some-fake-node-id", + VolumeCapability: &csi.VolumeCapability{ + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, + }, + }, + Readonly: false, + } + + if secrets != nil { + pubReq.ControllerPublishSecrets = secrets.ControllerPublishVolumeSecret + } + + conpubvol, err := c.ControllerPublishVolume(context.Background(), pubReq) + Expect(err).To(HaveOccurred()) + Expect(conpubvol).To(BeNil()) + + serverError, ok := status.FromError(err) + Expect(ok).To(BeTrue()) + Expect(serverError.Code()).To(Equal(codes.NotFound)) + + By("cleaning up deleting the volume") + + delReq := &csi.DeleteVolumeRequest{ + VolumeId: vol.GetVolume().GetId(), + } + + if secrets != nil { + delReq.ControllerDeleteSecrets = secrets.DeleteVolumeSecret + } + + _, err = c.DeleteVolume(context.Background(), delReq) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should fail when the volume is already published but is incompatible", func() { + + // Create Volume First + By("creating a single node writer volume") + name := "sanity" + req := &csi.CreateVolumeRequest{ + Name: name, + VolumeCapabilities: []*csi.VolumeCapability{ + { + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, + }, + }, + }, + } + + if secrets != nil { + req.ControllerCreateSecrets = secrets.CreateVolumeSecret + } + + vol, err := c.CreateVolume(context.Background(), req) + Expect(err).NotTo(HaveOccurred()) + Expect(vol).NotTo(BeNil()) + Expect(vol.GetVolume()).NotTo(BeNil()) + Expect(vol.GetVolume().GetId()).NotTo(BeEmpty()) + + By("getting a node id") + nid, err := n.NodeGetId( + context.Background(), + &csi.NodeGetIdRequest{}) + Expect(err).NotTo(HaveOccurred()) + Expect(nid).NotTo(BeNil()) + Expect(nid.GetNodeId()).NotTo(BeEmpty()) + + // ControllerPublishVolume + By("calling controllerpublish on that volume") + + pubReq := &csi.ControllerPublishVolumeRequest{ + VolumeId: vol.GetVolume().GetId(), + NodeId: nid.GetNodeId(), + VolumeCapability: &csi.VolumeCapability{ + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, + }, + }, + Readonly: false, + } + + if secrets != nil { + pubReq.ControllerPublishSecrets = secrets.ControllerPublishVolumeSecret + } + + conpubvol, err := c.ControllerPublishVolume(context.Background(), pubReq) + Expect(err).NotTo(HaveOccurred()) + Expect(conpubvol).NotTo(BeNil()) + + // Publish again with different attributes. + pubReq.Readonly = true + + conpubvol, err = c.ControllerPublishVolume(context.Background(), pubReq) + Expect(err).To(HaveOccurred()) + Expect(conpubvol).To(BeNil()) + + serverError, ok := status.FromError(err) + Expect(ok).To(BeTrue()) + Expect(serverError.Code()).To(Equal(codes.AlreadyExists)) + + By("cleaning up unpublishing the volume") + + unpubReq := &csi.ControllerUnpublishVolumeRequest{ + VolumeId: vol.GetVolume().GetId(), + // NodeID is optional in ControllerUnpublishVolume + NodeId: nid.GetNodeId(), + } + + if secrets != nil { + unpubReq.ControllerUnpublishSecrets = secrets.ControllerUnpublishVolumeSecret + } + + conunpubvol, err := c.ControllerUnpublishVolume(context.Background(), unpubReq) + Expect(err).NotTo(HaveOccurred()) + Expect(conunpubvol).NotTo(BeNil()) + + By("cleaning up deleting the volume") + + delReq := &csi.DeleteVolumeRequest{ + VolumeId: vol.GetVolume().GetId(), + } + + if secrets != nil { + delReq.ControllerDeleteSecrets = secrets.DeleteVolumeSecret + } + + _, err = c.DeleteVolume(context.Background(), delReq) + Expect(err).NotTo(HaveOccurred()) + }) }) var _ = Describe("ControllerUnpublishVolume [Controller Server]", func() { From 711419ba762e04c2d583e687bb4e4adbd7d4601b Mon Sep 17 00:00:00 2001 From: Sunny Date: Fri, 15 Jun 2018 14:08:12 +0530 Subject: [PATCH 2/7] Mock driver: Return probe ready status CSI 0.3.0 introduced probe ready status. This change adds the Ready value in ProbeResponse. And adds a test for the ready value of probe. --- mock/service/identity.go | 5 ++++- pkg/sanity/identity.go | 5 +++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/mock/service/identity.go b/mock/service/identity.go index c83daea5..c66d3b62 100644 --- a/mock/service/identity.go +++ b/mock/service/identity.go @@ -4,6 +4,7 @@ import ( "golang.org/x/net/context" "github.com/container-storage-interface/spec/lib/go/csi/v0" + "github.com/golang/protobuf/ptypes/wrappers" ) func (s *service) GetPluginInfo( @@ -23,7 +24,9 @@ func (s *service) Probe( req *csi.ProbeRequest) ( *csi.ProbeResponse, error) { - return &csi.ProbeResponse{}, nil + return &csi.ProbeResponse{ + Ready: &wrappers.BoolValue{Value: true}, + }, nil } func (s *service) GetPluginCapabilities( diff --git a/pkg/sanity/identity.go b/pkg/sanity/identity.go index cb5aad48..d9659b26 100644 --- a/pkg/sanity/identity.go +++ b/pkg/sanity/identity.go @@ -79,6 +79,11 @@ var _ = Describe("Probe [Identity Service]", func() { Expect(ok).To(BeTrue()) Expect(serverError.Code() == codes.FailedPrecondition || serverError.Code() == codes.OK).To(BeTrue()) + + if res.GetReady() != nil { + Expect(res.GetReady().GetValue() == true || + res.GetReady().GetValue() == false).To(BeTrue()) + } }) }) From 5b7d9a783d71f90f94bf707438598dd9023536cc Mon Sep 17 00:00:00 2001 From: Sunny Date: Sun, 17 Jun 2018 23:35:13 +0530 Subject: [PATCH 3/7] sanity: ValidateVolumeCapabilities test for no volume Adds ValidateVolumeCapabilities controller test to ensure that the validation fails when a non-existent volume ID is requested for volume capability validation. --- pkg/sanity/controller.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/pkg/sanity/controller.go b/pkg/sanity/controller.go index 0fb22392..14df8b37 100644 --- a/pkg/sanity/controller.go +++ b/pkg/sanity/controller.go @@ -645,6 +645,31 @@ var _ = Describe("ValidateVolumeCapabilities [Controller Server]", func() { _, err = c.DeleteVolume(context.Background(), delReq) Expect(err).NotTo(HaveOccurred()) }) + + It("should fail when the requested volume does not exist", func() { + + _, err := c.ValidateVolumeCapabilities( + context.Background(), + &csi.ValidateVolumeCapabilitiesRequest{ + VolumeId: "some-vol-id", + VolumeCapabilities: []*csi.VolumeCapability{ + { + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, + }, + }, + }, + }, + ) + Expect(err).To(HaveOccurred()) + + serverError, ok := status.FromError(err) + Expect(ok).To(BeTrue()) + Expect(serverError.Code()).To(Equal(codes.NotFound)) + }) }) var _ = Describe("ControllerPublishVolume [Controller Server]", func() { From 42ff0950a91ca303d6f8d11c5b37330ea6122f10 Mon Sep 17 00:00:00 2001 From: wackxu Date: Wed, 20 Jun 2018 16:21:52 +0800 Subject: [PATCH 4/7] fix ControllerGetCapabilities test --- pkg/sanity/controller.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/sanity/controller.go b/pkg/sanity/controller.go index 0fb22392..b721cf73 100644 --- a/pkg/sanity/controller.go +++ b/pkg/sanity/controller.go @@ -96,6 +96,8 @@ var _ = Describe("ControllerGetCapabilities [Controller Server]", func() { case csi.ControllerServiceCapability_RPC_PUBLISH_UNPUBLISH_VOLUME: case csi.ControllerServiceCapability_RPC_LIST_VOLUMES: case csi.ControllerServiceCapability_RPC_GET_CAPACITY: + case csi.ControllerServiceCapability_RPC_CREATE_DELETE_SNAPSHOT: + case csi.ControllerServiceCapability_RPC_LIST_SNAPSHOTS: default: Fail(fmt.Sprintf("Unknown capability: %v\n", cap.GetRpc().GetType())) } From 72455be3db9d0ccae186c856ae569b3c8ed03275 Mon Sep 17 00:00:00 2001 From: Sunny Date: Wed, 20 Jun 2018 16:30:55 +0530 Subject: [PATCH 5/7] sanity: test NodeGetInfo() & accessibility capability * Adds test for NodeGetInfo() * Adds ACCESSIBILITY_CONSTRAINT plugin capability in the known plugin capability list. * Adds isPluginCapabilitySupported() to check if the ACCESSIBILITY_CONSTRAINT plugin capability is supported. --- pkg/sanity/identity.go | 1 + pkg/sanity/node.go | 49 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/pkg/sanity/identity.go b/pkg/sanity/identity.go index cb5aad48..c4a30e0e 100644 --- a/pkg/sanity/identity.go +++ b/pkg/sanity/identity.go @@ -50,6 +50,7 @@ var _ = Describe("GetPluginCapabilities [Identity Service]", func() { for _, cap := range res.GetCapabilities() { switch cap.GetService().GetType() { case csi.PluginCapability_Service_CONTROLLER_SERVICE: + case csi.PluginCapability_Service_ACCESSIBILITY_CONSTRAINTS: default: Fail(fmt.Sprintf("Unknown capability: %v\n", cap.GetService().GetType())) } diff --git a/pkg/sanity/node.go b/pkg/sanity/node.go index d57621de..5085bc50 100644 --- a/pkg/sanity/node.go +++ b/pkg/sanity/node.go @@ -49,6 +49,26 @@ func isNodeCapabilitySupported(c csi.NodeClient, return false } +func isPluginCapabilitySupported(c csi.IdentityClient, + capType csi.PluginCapability_Service_Type, +) bool { + + caps, err := c.GetPluginCapabilities( + context.Background(), + &csi.GetPluginCapabilitiesRequest{}) + Expect(err).NotTo(HaveOccurred()) + Expect(caps).NotTo(BeNil()) + Expect(caps.GetCapabilities()).NotTo(BeNil()) + + for _, cap := range caps.GetCapabilities() { + Expect(cap.GetService()).NotTo(BeNil()) + if cap.GetService().GetType() == capType { + return true + } + } + return false +} + var _ = Describe("NodeGetCapabilities [Node Server]", func() { var ( c csi.NodeClient @@ -101,6 +121,35 @@ var _ = Describe("NodeGetId [Node Server]", func() { }) }) +var _ = Describe("NodeGetInfo [Node Server]", func() { + var ( + c csi.NodeClient + i csi.IdentityClient + accessibilityConstraintSupported bool + ) + + BeforeEach(func() { + c = csi.NewNodeClient(conn) + i = csi.NewIdentityClient(conn) + accessibilityConstraintSupported = isPluginCapabilitySupported(i, csi.PluginCapability_Service_ACCESSIBILITY_CONSTRAINTS) + }) + + It("should return approproate values", func() { + ninfo, err := c.NodeGetInfo( + context.Background(), + &csi.NodeGetInfoRequest{}) + + Expect(err).NotTo(HaveOccurred()) + Expect(ninfo).NotTo(BeNil()) + Expect(ninfo.GetNodeId()).NotTo(BeEmpty()) + Expect(ninfo.GetMaxVolumesPerNode()).NotTo(BeNumerically("<", 0)) + + if accessibilityConstraintSupported { + Expect(ninfo.GetAccessibleTopology()).NotTo(BeNil()) + } + }) +}) + var _ = Describe("NodePublishVolume [Node Server]", func() { var ( s csi.ControllerClient From a37b0c2894a858d56816bf0ce54fb00743661408 Mon Sep 17 00:00:00 2001 From: Travis Rhoden Date: Fri, 22 Jun 2018 11:52:45 -0600 Subject: [PATCH 6/7] Use custom gomock.Matcher for protobuf messages 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. --- test/co_test.go | 85 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 84 insertions(+), 1 deletion(-) diff --git a/test/co_test.go b/test/co_test.go index d4e5dfc3..a8c17e97 100644 --- a/test/co_test.go +++ b/test/co_test.go @@ -16,10 +16,13 @@ limitations under the License. package test import ( + "fmt" + "reflect" "testing" csi "github.com/container-storage-interface/spec/lib/go/csi/v0" gomock "github.com/golang/mock/gomock" + "github.com/golang/protobuf/proto" mock_driver "github.com/kubernetes-csi/csi-test/driver" mock_utils "github.com/kubernetes-csi/csi-test/utils" "golang.org/x/net/context" @@ -58,6 +61,24 @@ func TestPluginInfoResponse(t *testing.T) { } } +type pbMatcher struct { + x proto.Message +} + +func (p pbMatcher) Matches(x interface{}) bool { + y := x.(proto.Message) + return proto.Equal(p.x, y) +} + +func (p pbMatcher) String() string { + return fmt.Sprintf("pb equal to %v", p.x) +} + +func pbMatch(x interface{}) gomock.Matcher { + v := x.(proto.Message) + return &pbMatcher{v} +} + func TestGRPCGetPluginInfoReponse(t *testing.T) { // Setup mock @@ -79,7 +100,7 @@ func TestGRPCGetPluginInfoReponse(t *testing.T) { // Setup expectation // !IMPORTANT!: Must set context expected value to gomock.Any() to match any value - driver.EXPECT().GetPluginInfo(gomock.Any(), in).Return(out, nil).Times(1) + driver.EXPECT().GetPluginInfo(gomock.Any(), pbMatch(in)).Return(out, nil).Times(1) // Create a new RPC server := mock_driver.NewMockCSIDriver(&mock_driver.MockCSIDriverServers{ @@ -103,3 +124,65 @@ func TestGRPCGetPluginInfoReponse(t *testing.T) { t.Errorf("Unknown name: %s\n", name) } } + +func TestGRPCAttach(t *testing.T) { + + // Setup mock + m := gomock.NewController(&mock_utils.SafeGoroutineTester{}) + defer m.Finish() + driver := mock_driver.NewMockControllerServer(m) + + // Setup input + defaultVolumeID := "myname" + defaultNodeID := "MyNodeID" + defaultCaps := &csi.VolumeCapability{ + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_MULTI_NODE_MULTI_WRITER, + }, + } + publishVolumeInfo := map[string]string{ + "first": "foo", + "second": "bar", + "third": "baz", + } + defaultRequest := &csi.ControllerPublishVolumeRequest{ + VolumeId: defaultVolumeID, + NodeId: defaultNodeID, + VolumeCapability: defaultCaps, + Readonly: false, + } + + // Setup mock outout + out := &csi.ControllerPublishVolumeResponse{ + PublishInfo: publishVolumeInfo, + } + + // Setup expectation + // !IMPORTANT!: Must set context expected value to gomock.Any() to match any value + driver.EXPECT().ControllerPublishVolume(gomock.Any(), pbMatch(defaultRequest)).Return(out, nil).Times(1) + + // Create a new RPC + server := mock_driver.NewMockCSIDriver(&mock_driver.MockCSIDriverServers{ + Controller: driver, + }) + conn, err := server.Nexus() + if err != nil { + t.Errorf("Error: %s", err.Error()) + } + defer server.Close() + + // Make call + c := csi.NewControllerClient(conn) + r, err := c.ControllerPublishVolume(context.Background(), defaultRequest) + if err != nil { + t.Errorf("Error: %s", err.Error()) + } + + info := r.GetPublishInfo() + if !reflect.DeepEqual(info, publishVolumeInfo) { + t.Errorf("Invalid publish info: %v", info) + } +} From 54328a9c721b5fa873ffa97bbfd0e77940623e71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pab=C3=B3n?= Date: Fri, 29 Jun 2018 01:19:04 -0400 Subject: [PATCH 7/7] Update .travis.yml --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 9d636c7f..414ec5fb 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,7 @@ language: go matrix: include: - - go: 1.x + - go: 1.10.3 script: - go fmt $(go list ./... | grep -v vendor) | wc -l | grep 0 - go vet $(go list ./... | grep -v vendor)