From aec2b4ce525213870962cbfecf8eb5e0448ba342 Mon Sep 17 00:00:00 2001 From: Alexander Khaustov Date: Mon, 6 May 2019 10:12:12 +0300 Subject: [PATCH 1/3] CreateVolumeRequest respects the topology requirements of the node --- pkg/sanity/controller.go | 60 +++++++++++++++++++++++++--------------- pkg/sanity/node.go | 30 ++++++++++++-------- 2 files changed, 57 insertions(+), 33 deletions(-) diff --git a/pkg/sanity/controller.go b/pkg/sanity/controller.go index d2ac2241..14a272ec 100644 --- a/pkg/sanity/controller.go +++ b/pkg/sanity/controller.go @@ -1118,6 +1118,21 @@ var _ = DescribeSanity("Controller Service", func(sc *SanityContext) { It("should return appropriate values (no optional values added)", func() { + By("getting node information") + ni, err := n.NodeGetInfo( + context.Background(), + &csi.NodeGetInfoRequest{}) + Expect(err).NotTo(HaveOccurred()) + Expect(ni).NotTo(BeNil()) + Expect(ni.GetNodeId()).NotTo(BeEmpty()) + + var accReqs *csi.TopologyRequirement + if ni.AccessibleTopology != nil { + accReqs = &csi.TopologyRequirement{ + Requisite: []*csi.Topology{ni.AccessibleTopology}, + } + } + // Create Volume First By("creating a single node writer volume") name := UniqueString("sanity-controller-publish") @@ -1138,6 +1153,7 @@ var _ = DescribeSanity("Controller Service", func(sc *SanityContext) { }, Secrets: sc.Secrets.CreateVolumeSecret, Parameters: sc.Config.TestVolumeParameters, + AccessibilityRequirements: accReqs, }, ) Expect(err).NotTo(HaveOccurred()) @@ -1146,14 +1162,6 @@ var _ = DescribeSanity("Controller Service", func(sc *SanityContext) { Expect(vol.GetVolume().GetVolumeId()).NotTo(BeEmpty()) cl.RegisterVolume(name, VolumeInfo{VolumeID: vol.GetVolume().GetVolumeId()}) - By("getting a node id") - nid, err := n.NodeGetInfo( - context.Background(), - &csi.NodeGetInfoRequest{}) - Expect(err).NotTo(HaveOccurred()) - Expect(nid).NotTo(BeNil()) - Expect(nid.GetNodeId()).NotTo(BeEmpty()) - // ControllerPublishVolume By("calling controllerpublish on that volume") @@ -1161,7 +1169,7 @@ var _ = DescribeSanity("Controller Service", func(sc *SanityContext) { context.Background(), &csi.ControllerPublishVolumeRequest{ VolumeId: vol.GetVolume().GetVolumeId(), - NodeId: nid.GetNodeId(), + NodeId: ni.GetNodeId(), VolumeCapability: &csi.VolumeCapability{ AccessType: &csi.VolumeCapability_Mount{ Mount: &csi.VolumeCapability_MountVolume{}, @@ -1175,7 +1183,7 @@ var _ = DescribeSanity("Controller Service", func(sc *SanityContext) { }, ) Expect(err).NotTo(HaveOccurred()) - cl.RegisterVolume(name, VolumeInfo{VolumeID: vol.GetVolume().GetVolumeId(), NodeID: nid.GetNodeId()}) + cl.RegisterVolume(name, VolumeInfo{VolumeID: vol.GetVolume().GetVolumeId(), NodeID: ni.GetNodeId()}) Expect(conpubvol).NotTo(BeNil()) By("cleaning up unpublishing the volume") @@ -1185,7 +1193,7 @@ var _ = DescribeSanity("Controller Service", func(sc *SanityContext) { &csi.ControllerUnpublishVolumeRequest{ VolumeId: vol.GetVolume().GetVolumeId(), // NodeID is optional in ControllerUnpublishVolume - NodeId: nid.GetNodeId(), + NodeId: ni.GetNodeId(), Secrets: sc.Secrets.ControllerUnpublishVolumeSecret, }, ) @@ -1477,6 +1485,21 @@ var _ = DescribeSanity("Controller Service", func(sc *SanityContext) { By("creating a single node writer volume") name := UniqueString("sanity-controller-unpublish") + By("getting node information") + ni, err := n.NodeGetInfo( + context.Background(), + &csi.NodeGetInfoRequest{}) + Expect(err).NotTo(HaveOccurred()) + Expect(ni).NotTo(BeNil()) + Expect(ni.GetNodeId()).NotTo(BeEmpty()) + + var accReqs *csi.TopologyRequirement + if ni.AccessibleTopology != nil { + accReqs = &csi.TopologyRequirement{ + Requisite: []*csi.Topology{ni.AccessibleTopology}, + } + } + vol, err := c.CreateVolume( context.Background(), &csi.CreateVolumeRequest{ @@ -1493,6 +1516,7 @@ var _ = DescribeSanity("Controller Service", func(sc *SanityContext) { }, Secrets: sc.Secrets.CreateVolumeSecret, Parameters: sc.Config.TestVolumeParameters, + AccessibilityRequirements: accReqs, }, ) Expect(err).NotTo(HaveOccurred()) @@ -1501,14 +1525,6 @@ var _ = DescribeSanity("Controller Service", func(sc *SanityContext) { Expect(vol.GetVolume().GetVolumeId()).NotTo(BeEmpty()) cl.RegisterVolume(name, VolumeInfo{VolumeID: vol.GetVolume().GetVolumeId()}) - By("getting a node id") - nid, err := n.NodeGetInfo( - context.Background(), - &csi.NodeGetInfoRequest{}) - Expect(err).NotTo(HaveOccurred()) - Expect(nid).NotTo(BeNil()) - Expect(nid.GetNodeId()).NotTo(BeEmpty()) - // ControllerPublishVolume By("calling controllerpublish on that volume") @@ -1516,7 +1532,7 @@ var _ = DescribeSanity("Controller Service", func(sc *SanityContext) { context.Background(), &csi.ControllerPublishVolumeRequest{ VolumeId: vol.GetVolume().GetVolumeId(), - NodeId: nid.GetNodeId(), + NodeId: ni.GetNodeId(), VolumeCapability: &csi.VolumeCapability{ AccessType: &csi.VolumeCapability_Mount{ Mount: &csi.VolumeCapability_MountVolume{}, @@ -1530,7 +1546,7 @@ var _ = DescribeSanity("Controller Service", func(sc *SanityContext) { }, ) Expect(err).NotTo(HaveOccurred()) - cl.RegisterVolume(name, VolumeInfo{VolumeID: vol.GetVolume().GetVolumeId(), NodeID: nid.GetNodeId()}) + cl.RegisterVolume(name, VolumeInfo{VolumeID: vol.GetVolume().GetVolumeId(), NodeID: ni.GetNodeId()}) Expect(conpubvol).NotTo(BeNil()) // ControllerUnpublishVolume @@ -1541,7 +1557,7 @@ var _ = DescribeSanity("Controller Service", func(sc *SanityContext) { &csi.ControllerUnpublishVolumeRequest{ VolumeId: vol.GetVolume().GetVolumeId(), // NodeID is optional in ControllerUnpublishVolume - NodeId: nid.GetNodeId(), + NodeId: ni.GetNodeId(), Secrets: sc.Secrets.ControllerUnpublishVolumeSecret, }, ) diff --git a/pkg/sanity/node.go b/pkg/sanity/node.go index f4ceddcd..b852fbde 100644 --- a/pkg/sanity/node.go +++ b/pkg/sanity/node.go @@ -625,6 +625,21 @@ var _ = DescribeSanity("Node Service", func(sc *SanityContext) { It("should work", func() { name := UniqueString("sanity-node-full") + By("getting node information") + ni, err := c.NodeGetInfo( + context.Background(), + &csi.NodeGetInfoRequest{}) + Expect(err).NotTo(HaveOccurred()) + Expect(ni).NotTo(BeNil()) + Expect(ni.GetNodeId()).NotTo(BeEmpty()) + + var accReqs *csi.TopologyRequirement + if ni.AccessibleTopology != nil { + accReqs = &csi.TopologyRequirement{ + Requisite: []*csi.Topology{ni.AccessibleTopology}, + } + } + // Create Volume First By("creating a single node writer volume") vol, err := s.CreateVolume( @@ -643,6 +658,7 @@ var _ = DescribeSanity("Node Service", func(sc *SanityContext) { }, Secrets: sc.Secrets.CreateVolumeSecret, Parameters: sc.Config.TestVolumeParameters, + AccessibilityRequirements: accReqs, }, ) Expect(err).NotTo(HaveOccurred()) @@ -651,14 +667,6 @@ var _ = DescribeSanity("Node Service", func(sc *SanityContext) { Expect(vol.GetVolume().GetVolumeId()).NotTo(BeEmpty()) cl.RegisterVolume(name, VolumeInfo{VolumeID: vol.GetVolume().GetVolumeId()}) - By("getting a node id") - nid, err := c.NodeGetInfo( - context.Background(), - &csi.NodeGetInfoRequest{}) - Expect(err).NotTo(HaveOccurred()) - Expect(nid).NotTo(BeNil()) - Expect(nid.GetNodeId()).NotTo(BeEmpty()) - var conpubvol *csi.ControllerPublishVolumeResponse if controllerPublishSupported { By("controller publishing volume") @@ -667,7 +675,7 @@ var _ = DescribeSanity("Node Service", func(sc *SanityContext) { context.Background(), &csi.ControllerPublishVolumeRequest{ VolumeId: vol.GetVolume().GetVolumeId(), - NodeId: nid.GetNodeId(), + NodeId: ni.GetNodeId(), VolumeCapability: &csi.VolumeCapability{ AccessType: &csi.VolumeCapability_Mount{ Mount: &csi.VolumeCapability_MountVolume{}, @@ -682,7 +690,7 @@ var _ = DescribeSanity("Node Service", func(sc *SanityContext) { }, ) Expect(err).NotTo(HaveOccurred()) - cl.RegisterVolume(name, VolumeInfo{VolumeID: vol.GetVolume().GetVolumeId(), NodeID: nid.GetNodeId()}) + cl.RegisterVolume(name, VolumeInfo{VolumeID: vol.GetVolume().GetVolumeId(), NodeID: ni.GetNodeId()}) Expect(conpubvol).NotTo(BeNil()) } // NodeStageVolume @@ -782,7 +790,7 @@ var _ = DescribeSanity("Node Service", func(sc *SanityContext) { context.Background(), &csi.ControllerUnpublishVolumeRequest{ VolumeId: vol.GetVolume().GetVolumeId(), - NodeId: nid.GetNodeId(), + NodeId: ni.GetNodeId(), Secrets: sc.Secrets.ControllerUnpublishVolumeSecret, }, ) From c75f362fdccde9c0879495dab11be282f7d43f35 Mon Sep 17 00:00:00 2001 From: Alexander Khaustov Date: Mon, 6 May 2019 14:26:29 +0300 Subject: [PATCH 2/3] fix formatting --- pkg/sanity/controller.go | 8 ++++---- pkg/sanity/node.go | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/sanity/controller.go b/pkg/sanity/controller.go index 14a272ec..a3870fcd 100644 --- a/pkg/sanity/controller.go +++ b/pkg/sanity/controller.go @@ -1151,8 +1151,8 @@ var _ = DescribeSanity("Controller Service", func(sc *SanityContext) { }, }, }, - Secrets: sc.Secrets.CreateVolumeSecret, - Parameters: sc.Config.TestVolumeParameters, + Secrets: sc.Secrets.CreateVolumeSecret, + Parameters: sc.Config.TestVolumeParameters, AccessibilityRequirements: accReqs, }, ) @@ -1514,8 +1514,8 @@ var _ = DescribeSanity("Controller Service", func(sc *SanityContext) { }, }, }, - Secrets: sc.Secrets.CreateVolumeSecret, - Parameters: sc.Config.TestVolumeParameters, + Secrets: sc.Secrets.CreateVolumeSecret, + Parameters: sc.Config.TestVolumeParameters, AccessibilityRequirements: accReqs, }, ) diff --git a/pkg/sanity/node.go b/pkg/sanity/node.go index b852fbde..0c1097ca 100644 --- a/pkg/sanity/node.go +++ b/pkg/sanity/node.go @@ -656,8 +656,8 @@ var _ = DescribeSanity("Node Service", func(sc *SanityContext) { }, }, }, - Secrets: sc.Secrets.CreateVolumeSecret, - Parameters: sc.Config.TestVolumeParameters, + Secrets: sc.Secrets.CreateVolumeSecret, + Parameters: sc.Config.TestVolumeParameters, AccessibilityRequirements: accReqs, }, ) From 119b8c5fc9382a8b401ac40644e7bd6703fb49cd Mon Sep 17 00:00:00 2001 From: Alexander Khaustov Date: Tue, 14 May 2019 10:08:12 +0300 Subject: [PATCH 3/3] comment on test assumptions --- pkg/sanity/controller.go | 6 ++++++ pkg/sanity/node.go | 3 +++ 2 files changed, 9 insertions(+) diff --git a/pkg/sanity/controller.go b/pkg/sanity/controller.go index a3870fcd..be411d54 100644 --- a/pkg/sanity/controller.go +++ b/pkg/sanity/controller.go @@ -1116,6 +1116,8 @@ var _ = DescribeSanity("Controller Service", func(sc *SanityContext) { Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) }) + // CSI spec poses no specific requirements for the cluster/storage setups that a SP MUST support. To perform + // meaningful checks the following test assumes that topology-aware provisioning on a single node setup is supported It("should return appropriate values (no optional values added)", func() { By("getting node information") @@ -1128,6 +1130,7 @@ var _ = DescribeSanity("Controller Service", func(sc *SanityContext) { var accReqs *csi.TopologyRequirement if ni.AccessibleTopology != nil { + // Topology requirements are honored if provided by the driver accReqs = &csi.TopologyRequirement{ Requisite: []*csi.Topology{ni.AccessibleTopology}, } @@ -1479,6 +1482,8 @@ var _ = DescribeSanity("Controller Service", func(sc *SanityContext) { Expect(serverError.Code()).To(Equal(codes.InvalidArgument)) }) + // CSI spec poses no specific requirements for the cluster/storage setups that a SP MUST support. To perform + // meaningful checks the following test assumes that topology-aware provisioning on a single node setup is supported It("should return appropriate values (no optional values added)", func() { // Create Volume First @@ -1495,6 +1500,7 @@ var _ = DescribeSanity("Controller Service", func(sc *SanityContext) { var accReqs *csi.TopologyRequirement if ni.AccessibleTopology != nil { + // Topology requirements are honored if provided by the driver accReqs = &csi.TopologyRequirement{ Requisite: []*csi.Topology{ni.AccessibleTopology}, } diff --git a/pkg/sanity/node.go b/pkg/sanity/node.go index 0c1097ca..c0d69d8b 100644 --- a/pkg/sanity/node.go +++ b/pkg/sanity/node.go @@ -622,6 +622,8 @@ var _ = DescribeSanity("Node Service", func(sc *SanityContext) { }) + // CSI spec poses no specific requirements for the cluster/storage setups that a SP MUST support. To perform + // meaningful checks the following test assumes that topology-aware provisioning on a single node setup is supported It("should work", func() { name := UniqueString("sanity-node-full") @@ -635,6 +637,7 @@ var _ = DescribeSanity("Node Service", func(sc *SanityContext) { var accReqs *csi.TopologyRequirement if ni.AccessibleTopology != nil { + // Topology requirements are honored if provided by the driver accReqs = &csi.TopologyRequirement{ Requisite: []*csi.Topology{ni.AccessibleTopology}, }