sanity: Add max volumes attached test

This adds a test for ControllerPublishVolume to check the maximum volume attachment on a node. It also sets max attach limit in the mock driver to 2 and adds a check in the driver for the number of volume attachments before publishing a new volume.
kubernetes-csi · Mar 11, 2019 · 60afa9d · 60afa9d
1 parent eb3bdc3
commit 60afa9d
Show file tree

Hide file tree

Showing 9 changed files with 132 additions and 12 deletions.
diff --git a/cmd/csi-sanity/sanity_test.go b/cmd/csi-sanity/sanity_test.go
@@ -42,6 +42,7 @@ func init() {
 	flag.StringVar(&config.SecretsFile, prefix+"secrets", "", "CSI secrets file")
 	flag.Int64Var(&config.TestVolumeSize, prefix+"testvolumesize", sanity.DefTestVolumeSize, "Base volume size used for provisioned volumes")
 	flag.StringVar(&config.TestVolumeParametersFile, prefix+"testvolumeparameters", "", "YAML file of volume parameters for provisioned volumes")
+	flag.BoolVar(&config.TestNodeVolumeAttachLimit, prefix+"testnodevolumeattachlimit", false, "Test node volume attach limit")
 	flag.StringVar(&config.JUnitFile, prefix+"junitfile", "", "JUnit XML output file where test results will be written")
 	flag.Parse()
 }

diff --git a/cmd/mock-driver/main.go b/cmd/mock-driver/main.go
@@ -32,7 +32,7 @@ func main() {
 	var config service.Config
 	flag.BoolVar(&config.DisableAttach, "disable-attach", false, "Disables RPC_PUBLISH_UNPUBLISH_VOLUME capability.")
 	flag.StringVar(&config.DriverName, "name", service.Name, "CSI driver name.")
-	flag.Int64Var(&config.AttachLimit, "attach-limit", 0, "number of attachable volumes on a node")
+	flag.Int64Var(&config.AttachLimit, "attach-limit", 2, "number of attachable volumes on a node")
 	flag.BoolVar(&config.NodeExpansionRequired, "node-expand-required", false, "Enables NodeServiceCapability_RPC_EXPAND_VOLUME capacity.")
 	flag.BoolVar(&config.DisableControllerExpansion, "disable-controller-expansion", false, "Disables ControllerServiceCapability_RPC_EXPAND_VOLUME capability.")
 	flag.BoolVar(&config.DisableOnlineExpansion, "disable-online-expansion", false, "Disables online volume expansion capability.")

diff --git a/hack/_apitest/api_test.go b/hack/_apitest/api_test.go
@@ -9,9 +9,10 @@ import (
 
 func TestMyDriver(t *testing.T) {
 	config := &sanity.Config{
-		TargetPath:  os.TempDir() + "/csi",
-		StagingPath: os.TempDir() + "/csi",
-		Address:     "/tmp/e2e-csi-sanity.sock",
+		TargetPath:                os.TempDir() + "/csi",
+		StagingPath:               os.TempDir() + "/csi",
+		Address:                   "/tmp/e2e-csi-sanity.sock",
+		TestNodeVolumeAttachLimit: true,
 	}
 
 	sanity.Test(t, config)

diff --git a/hack/_embedded/embedded_test.go b/hack/_embedded/embedded_test.go
@@ -26,9 +26,10 @@ var _ = AfterSuite(func() {})
 var _ = Describe("MyCSIDriver", func() {
 	Context("Config A", func() {
 		config := &sanity.Config{
-			TargetPath:  os.TempDir() + "/csi",
-			StagingPath: os.TempDir() + "/csi",
-			Address:     "/tmp/e2e-csi-sanity.sock",
+			TargetPath:                os.TempDir() + "/csi",
+			StagingPath:               os.TempDir() + "/csi",
+			Address:                   "/tmp/e2e-csi-sanity.sock",
+			TestNodeVolumeAttachLimit: true,
 		}
 
 		BeforeEach(func() {})

diff --git a/hack/e2e.sh b/hack/e2e.sh
@@ -14,7 +14,7 @@ runTest()
 	CSI_ENDPOINT=$1 ./bin/mock-driver &
 	local pid=$!
 
-	./cmd/csi-sanity/csi-sanity $TESTARGS --csi.endpoint=$2; ret=$?
+	./cmd/csi-sanity/csi-sanity $TESTARGS --csi.endpoint=$2 --csi.testnodevolumeattachlimit; ret=$?
 	kill -9 $pid
 
 	if [ $ret -ne 0 ] ; then
@@ -27,7 +27,7 @@ runTestWithCreds()
 	CSI_ENDPOINT=$1 CSI_ENABLE_CREDS=true ./bin/mock-driver &
 	local pid=$!
 
-	./cmd/csi-sanity/csi-sanity $TESTARGS --csi.endpoint=$2 --csi.secrets=mock/mocksecret.yaml; ret=$?
+	./cmd/csi-sanity/csi-sanity $TESTARGS --csi.endpoint=$2 --csi.secrets=mock/mocksecret.yaml --csi.testnodevolumeattachlimit; ret=$?
 	kill -9 $pid
 
 	if [ $ret -ne 0 ] ; then

diff --git a/mock/service/controller.go b/mock/service/controller.go
@@ -166,6 +166,11 @@ func (s *service) ControllerPublishVolume(
 		}, nil
 	}
 
+	// Check attach limit before publishing only if attach limit is set.
+	if s.config.AttachLimit > 0 && s.getAttachCount(devPathKey) >= s.config.AttachLimit {
+		return nil, status.Errorf(codes.ResourceExhausted, "Cannot attach any more volumes to this node")
+	}
+
 	var roVal string
 	if req.GetReadonly() {
 		roVal = "true"

diff --git a/mock/service/service.go b/mock/service/service.go
@@ -148,3 +148,14 @@ func (s *service) newSnapshot(name, sourceVolumeId string, parameters map[string
 		},
 	}
 }
+
+// getAttachCount returns the number of attached volumes on the node.
+func (s *service) getAttachCount(devPathKey string) int64 {
+	var count int64
+	for _, v := range s.vols {
+		if device := v.VolumeContext[devPathKey]; device != "" {
+			count++
+		}
+	}
+	return count
+}
diff --git a/pkg/sanity/controller.go b/pkg/sanity/controller.go
@@ -895,6 +895,48 @@ var _ = DescribeSanity("Controller Service", func(sc *SanityContext) {
 			cl.UnregisterVolume(name)
 		})
 
+		It("should fail when publishing more volumes than the node max attach limit", func() {
+			if !sc.Config.TestNodeVolumeAttachLimit {
+				Skip("testnodevolumeattachlimit not enabled")
+			}
+
+			By("getting node info")
+			nodeInfo, err := n.NodeGetInfo(
+				context.Background(),
+				&csi.NodeGetInfoRequest{})
+			Expect(err).NotTo(HaveOccurred())
+			Expect(nodeInfo).NotTo(BeNil())
+
+			if nodeInfo.MaxVolumesPerNode <= 0 {
+				Skip("No MaxVolumesPerNode")
+			}
+
+			nid := nodeInfo.GetNodeId()
+			Expect(nid).NotTo(BeEmpty())
+
+			// Store the volume name and volume ID for later cleanup.
+			createdVols := map[string]string{}
+			By("creating volumes")
+			for i := int64(0); i < nodeInfo.MaxVolumesPerNode; i++ {
+				name := uniqueString(fmt.Sprintf("sanity-max-attach-limit-vol-%d", i))
+				volID, err := CreateAndControllerPublishVolume(sc, c, name, nid)
+				Expect(err).NotTo(HaveOccurred())
+				cl.RegisterVolume(name, VolumeInfo{VolumeID: volID, NodeID: nid})
+				createdVols[name] = volID
+			}
+
+			extraVolName := uniqueString("sanity-max-attach-limit-vol+1")
+			_, err = CreateAndControllerPublishVolume(sc, c, extraVolName, nid)
+			Expect(err).To(HaveOccurred())
+
+			By("cleaning up")
+			for volName, volID := range createdVols {
+				err = ControllerUnpublishAndDeleteVolume(sc, c, volID, nid)
+				Expect(err).NotTo(HaveOccurred())
+				cl.UnregisterVolume(volName)
+			}
+		})
+
 		It("should fail when the volume does not exist", func() {
 
 			By("calling controller publish on a non-existent volume")
@@ -1742,3 +1784,61 @@ func MakeDeleteVolumeReq(sc *SanityContext, id string) *csi.DeleteVolumeRequest
 
 	return delVolReq
 }
+
+// MakeControllerPublishVolumeReq creates and returns a ControllerPublishVolumeRequest.
+func MakeControllerPublishVolumeReq(sc *SanityContext, volID, nodeID string) *csi.ControllerPublishVolumeRequest {
+	return &csi.ControllerPublishVolumeRequest{
+		VolumeId: volID,
+		NodeId:   nodeID,
+		VolumeCapability: &csi.VolumeCapability{
+			AccessType: &csi.VolumeCapability_Mount{
+				Mount: &csi.VolumeCapability_MountVolume{},
+			},
+			AccessMode: &csi.VolumeCapability_AccessMode{
+				Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER,
+			},
+		},
+		Readonly: false,
+		Secrets:  sc.Secrets.ControllerPublishVolumeSecret,
+	}
+}
+
+// MakeControllerUnpublishVolumeReq creates and returns a ControllerUnpublishVolumeRequest.
+func MakeControllerUnpublishVolumeReq(sc *SanityContext, volID, nodeID string) *csi.ControllerUnpublishVolumeRequest {
+	return &csi.ControllerUnpublishVolumeRequest{
+		VolumeId: volID,
+		NodeId:   nodeID,
+		Secrets:  sc.Secrets.ControllerUnpublishVolumeSecret,
+	}
+}
+
+// CreateAndControllerPublishVolume creates and controller publishes a volume given a volume name and node ID.
+func CreateAndControllerPublishVolume(sc *SanityContext, c csi.ControllerClient, volName, nodeID string) (volID string, err error) {
+	vol, err := c.CreateVolume(context.Background(), MakeCreateVolumeReq(sc, volName))
+	Expect(err).NotTo(HaveOccurred())
+	Expect(vol).NotTo(BeNil())
+	Expect(vol.GetVolume()).NotTo(BeNil())
+	Expect(vol.GetVolume().GetVolumeId()).NotTo(BeEmpty())
+
+	_, err = c.ControllerPublishVolume(
+		context.Background(),
+		MakeControllerPublishVolumeReq(sc, vol.GetVolume().GetVolumeId(), nodeID),
+	)
+	return vol.GetVolume().GetVolumeId(), err
+}
+
+// ControllerUnpublishAndDeleteVolume controller unpublishes and deletes a volume, given volume ID and node ID.
+func ControllerUnpublishAndDeleteVolume(sc *SanityContext, c csi.ControllerClient, volID, nodeID string) error {
+	_, err := c.ControllerUnpublishVolume(
+		context.Background(),
+		MakeControllerUnpublishVolumeReq(sc, volID, nodeID),
+	)
+	Expect(err).NotTo(HaveOccurred())
+
+	_, err = c.DeleteVolume(
+		context.Background(),
+		MakeDeleteVolumeReq(sc, volID),
+	)
+	Expect(err).NotTo(HaveOccurred())
+	return err
+}
diff --git a/pkg/sanity/sanity.go b/pkg/sanity/sanity.go
@@ -53,9 +53,10 @@ type Config struct {
 	Address     string
 	SecretsFile string
 
-	TestVolumeSize           int64
-	TestVolumeParametersFile string
-	TestVolumeParameters     map[string]string
+	TestVolumeSize            int64
+	TestVolumeParametersFile  string
+	TestVolumeParameters      map[string]string
+	TestNodeVolumeAttachLimit bool
 
 	JUnitFile string
 }