From 4000ec53b7ba08ef8d09dcd9a8be1419759bae86 Mon Sep 17 00:00:00 2001 From: Sunny Date: Sun, 27 Jan 2019 17:47:48 +0530 Subject: [PATCH 1/4] Make mount and staging dir creation flexible This change adds an option to attach callback functions to CSI sanity for mount target directory creation. This could be used to customize the target and staging mount path creation. Also, adds `targetPath` and `stagingPath` fields to `SanityContext` to store the actual target and staging paths, derived from the sanity config. Adds an e2e test section to test the above setup. --- hack/_apitest2/api_test.go | 63 ++++++++++++++++++++++++++++++++++++++ hack/e2e.sh | 17 ++++++++++ pkg/sanity/node.go | 34 +++++++++----------- pkg/sanity/sanity.go | 61 +++++++++++++++++++++++++++--------- 4 files changed, 141 insertions(+), 34 deletions(-) create mode 100644 hack/_apitest2/api_test.go diff --git a/hack/_apitest2/api_test.go b/hack/_apitest2/api_test.go new file mode 100644 index 00000000..40acc3f4 --- /dev/null +++ b/hack/_apitest2/api_test.go @@ -0,0 +1,63 @@ +package apitest2 + +import ( + "fmt" + "os" + "path" + "testing" + + "github.com/kubernetes-csi/csi-test/pkg/sanity" +) + +// TestMyDriverWithCustomTargetPaths verifies that CreateTargetDir and +// CreateStagingDir are called a specific number of times. +func TestMyDriverWithCustomTargetPaths(t *testing.T) { + var createTargetDirCalls, createStagingDirCalls int + + wantCreateTargetCalls := 3 + wantCreateStagingCalls := 3 + + // tmpPath could be a CO specific directory under which all the target dirs + // are created. For k8s, it could be /var/lib/kubelet/pods under which the + // mount directories could be created. + tmpPath := path.Join(os.TempDir(), "csi") + config := &sanity.Config{ + TargetPath: "foo/target/mount", + StagingPath: "foo/staging/mount", + Address: "/tmp/e2e-csi-sanity.sock", + CreateTargetDir: func(targetPath string) (string, error) { + createTargetDirCalls++ + targetPath = path.Join(tmpPath, targetPath) + return targetPath, createTargetDir(targetPath) + }, + CreateStagingDir: func(targetPath string) (string, error) { + createStagingDirCalls++ + targetPath = path.Join(tmpPath, targetPath) + return targetPath, createTargetDir(targetPath) + }, + } + + sanity.Test(t, config) + + if createTargetDirCalls != wantCreateTargetCalls { + t.Errorf("unexpected number of CreateTargetDir calls:\n(WNT) %d\n(GOT) %d", wantCreateTargetCalls, createTargetDirCalls) + } + + if createStagingDirCalls != wantCreateStagingCalls { + t.Errorf("unexpected number of CreateStagingDir calls:\n(WNT) %d\n(GOT) %d", wantCreateStagingCalls, createStagingDirCalls) + } +} + +func createTargetDir(targetPath string) error { + fileInfo, err := os.Stat(targetPath) + if err != nil && os.IsNotExist(err) { + return os.MkdirAll(targetPath, 0755) + } else if err != nil { + return err + } + if !fileInfo.IsDir() { + return fmt.Errorf("Target location %s is not a directory", targetPath) + } + + return nil +} diff --git a/hack/e2e.sh b/hack/e2e.sh index 419405c4..83f4770d 100755 --- a/hack/e2e.sh +++ b/hack/e2e.sh @@ -69,6 +69,20 @@ runTestAPI() fi } +runTestAPIWithCustomTargetPaths() +{ + CSI_ENDPOINT=$1 ./bin/mock-driver & + local pid=$! + + # Running a specific test to verify that the custom target paths are called + # a deterministic number of times. + GOCACHE=off go test -v ./hack/_apitest2/api_test.go -ginkgo.focus="NodePublishVolume"; ret=$? + + if [ $ret -ne 0 ] ; then + exit $ret + fi +} + make cd cmd/csi-sanity @@ -88,4 +102,7 @@ runTestWithDifferentAddresses "${UDS_NODE}" "${UDS_CONTROLLER}" rm -f $UDS_NODE rm -f $UDS_CONTROLLER +runTestAPIWithCustomTargetPaths "${UDS}" +rm -rf $UDS + exit 0 diff --git a/pkg/sanity/node.go b/pkg/sanity/node.go index bd706c51..62339525 100644 --- a/pkg/sanity/node.go +++ b/pkg/sanity/node.go @@ -86,10 +86,6 @@ var _ = DescribeSanity("Node Service", func(sc *SanityContext) { s, csi.ControllerServiceCapability_RPC_PUBLISH_UNPUBLISH_VOLUME) nodeStageSupported = isNodeCapabilitySupported(c, csi.NodeServiceCapability_RPC_STAGE_UNSTAGE_VOLUME) - if nodeStageSupported { - err := createMountTargetLocation(sc.Config.StagingPath) - Expect(err).NotTo(HaveOccurred()) - } nodeVolumeStatsSupported = isNodeCapabilitySupported(c, csi.NodeServiceCapability_RPC_GET_VOLUME_STATS) cl = &Cleanup{ Context: sc, @@ -190,7 +186,7 @@ var _ = DescribeSanity("Node Service", func(sc *SanityContext) { context.Background(), &csi.NodePublishVolumeRequest{ VolumeId: "id", - TargetPath: sc.Config.TargetPath, + TargetPath: sc.targetPath, Secrets: sc.Secrets.NodePublishVolumeSecret, }, ) @@ -247,7 +243,7 @@ var _ = DescribeSanity("Node Service", func(sc *SanityContext) { _, err := c.NodeStageVolume( context.Background(), &csi.NodeStageVolumeRequest{ - StagingTargetPath: sc.Config.StagingPath, + StagingTargetPath: sc.stagingPath, VolumeCapability: &csi.VolumeCapability{ AccessType: &csi.VolumeCapability_Mount{ Mount: &csi.VolumeCapability_MountVolume{}, @@ -329,7 +325,7 @@ var _ = DescribeSanity("Node Service", func(sc *SanityContext) { context.Background(), &csi.NodeStageVolumeRequest{ VolumeId: vol.GetVolume().GetVolumeId(), - StagingTargetPath: sc.Config.StagingPath, + StagingTargetPath: sc.stagingPath, PublishContext: map[string]string{ "device": device, }, @@ -368,7 +364,7 @@ var _ = DescribeSanity("Node Service", func(sc *SanityContext) { _, err := c.NodeUnstageVolume( context.Background(), &csi.NodeUnstageVolumeRequest{ - StagingTargetPath: sc.Config.StagingPath, + StagingTargetPath: sc.stagingPath, }) Expect(err).To(HaveOccurred()) @@ -519,7 +515,7 @@ var _ = DescribeSanity("Node Service", func(sc *SanityContext) { Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, }, }, - StagingTargetPath: sc.Config.StagingPath, + StagingTargetPath: sc.stagingPath, VolumeContext: vol.GetVolume().GetVolumeContext(), PublishContext: conpubvol.GetPublishContext(), Secrets: sc.Secrets.NodeStageVolumeSecret, @@ -532,13 +528,13 @@ var _ = DescribeSanity("Node Service", func(sc *SanityContext) { By("publishing the volume on a node") var stagingPath string if nodeStageSupported { - stagingPath = sc.Config.StagingPath + stagingPath = sc.stagingPath } nodepubvol, err := c.NodePublishVolume( context.Background(), &csi.NodePublishVolumeRequest{ VolumeId: vol.GetVolume().GetVolumeId(), - TargetPath: sc.Config.TargetPath, + TargetPath: sc.targetPath, StagingTargetPath: stagingPath, VolumeCapability: &csi.VolumeCapability{ AccessType: &csi.VolumeCapability_Mount{ @@ -577,7 +573,7 @@ var _ = DescribeSanity("Node Service", func(sc *SanityContext) { context.Background(), &csi.NodeUnpublishVolumeRequest{ VolumeId: vol.GetVolume().GetVolumeId(), - TargetPath: sc.Config.TargetPath, + TargetPath: sc.targetPath, }) Expect(err).NotTo(HaveOccurred()) Expect(nodeunpubvol).NotTo(BeNil()) @@ -588,7 +584,7 @@ var _ = DescribeSanity("Node Service", func(sc *SanityContext) { context.Background(), &csi.NodeUnstageVolumeRequest{ VolumeId: vol.GetVolume().GetVolumeId(), - StagingTargetPath: sc.Config.StagingPath, + StagingTargetPath: sc.stagingPath, }, ) Expect(err).NotTo(HaveOccurred()) @@ -703,7 +699,7 @@ var _ = DescribeSanity("Node Service", func(sc *SanityContext) { Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, }, }, - StagingTargetPath: sc.Config.StagingPath, + StagingTargetPath: sc.stagingPath, VolumeContext: vol.GetVolume().GetVolumeContext(), PublishContext: conpubvol.GetPublishContext(), Secrets: sc.Secrets.NodeStageVolumeSecret, @@ -716,13 +712,13 @@ var _ = DescribeSanity("Node Service", func(sc *SanityContext) { By("publishing the volume on a node") var stagingPath string if nodeStageSupported { - stagingPath = sc.Config.StagingPath + stagingPath = sc.stagingPath } nodepubvol, err := c.NodePublishVolume( context.Background(), &csi.NodePublishVolumeRequest{ VolumeId: vol.GetVolume().GetVolumeId(), - TargetPath: sc.Config.TargetPath, + TargetPath: sc.targetPath, StagingTargetPath: stagingPath, VolumeCapability: &csi.VolumeCapability{ AccessType: &csi.VolumeCapability_Mount{ @@ -747,7 +743,7 @@ var _ = DescribeSanity("Node Service", func(sc *SanityContext) { context.Background(), &csi.NodeGetVolumeStatsRequest{ VolumeId: vol.GetVolume().GetVolumeId(), - VolumePath: sc.Config.TargetPath, + VolumePath: sc.targetPath, }, ) Expect(err).ToNot(HaveOccurred()) @@ -760,7 +756,7 @@ var _ = DescribeSanity("Node Service", func(sc *SanityContext) { context.Background(), &csi.NodeUnpublishVolumeRequest{ VolumeId: vol.GetVolume().GetVolumeId(), - TargetPath: sc.Config.TargetPath, + TargetPath: sc.targetPath, }) Expect(err).NotTo(HaveOccurred()) Expect(nodeunpubvol).NotTo(BeNil()) @@ -771,7 +767,7 @@ var _ = DescribeSanity("Node Service", func(sc *SanityContext) { context.Background(), &csi.NodeUnstageVolumeRequest{ VolumeId: vol.GetVolume().GetVolumeId(), - StagingTargetPath: sc.Config.StagingPath, + StagingTargetPath: sc.stagingPath, }, ) Expect(err).NotTo(HaveOccurred()) diff --git a/pkg/sanity/sanity.go b/pkg/sanity/sanity.go index f6ff707d..ad075356 100644 --- a/pkg/sanity/sanity.go +++ b/pkg/sanity/sanity.go @@ -60,6 +60,13 @@ type Config struct { TestNodeVolumeAttachLimit bool JUnitFile string + + // Callback functions to customize the creation of target and staging + // directories. Returns the new paths for mount and staging. + // If not defined, directories are created in the default way at TargetPath + // and StagingPath. + CreateTargetDir func(path string) (string, error) + CreateStagingDir func(path string) (string, error) } // SanityContext holds the variables that each test can depend on. It @@ -72,6 +79,10 @@ type SanityContext struct { connAddress string controllerConnAddress string + + // Target and staging paths derived from the sanity config. + targetPath string + stagingPath string } // Test will test the CSI driver at the specified address by @@ -153,12 +164,15 @@ func (sc *SanityContext) setup() { } By("creating mount and staging directories") - err = createMountTargetLocation(sc.Config.TargetPath) - Expect(err).NotTo(HaveOccurred()) - if len(sc.Config.StagingPath) > 0 { - err = createMountTargetLocation(sc.Config.StagingPath) - Expect(err).NotTo(HaveOccurred()) - } + // If callback function for creating target dir is specified, use it. + targetPath, err := createMountTargetLocation(sc.Config.TargetPath, sc.Config.CreateTargetDir) + Expect(err).NotTo(HaveOccurred(), "failed to create target directory %s", targetPath) + sc.targetPath = targetPath + + // If callback function for creating staging dir is specified, use it. + stagingPath, err := createMountTargetLocation(sc.Config.StagingPath, sc.Config.CreateStagingDir) + Expect(err).NotTo(HaveOccurred(), "failed to create staging directory %s", stagingPath) + sc.stagingPath = stagingPath } func (sc *SanityContext) teardown() { @@ -174,18 +188,35 @@ func (sc *SanityContext) teardown() { // (https://github.com/kubernetes-csi/csi-test/pull/98). } -func createMountTargetLocation(targetPath string) error { - fileInfo, err := os.Stat(targetPath) - if err != nil && os.IsNotExist(err) { - return os.MkdirAll(targetPath, 0755) - } else if err != nil { - return err +func createMountTargetLocation(targetPath string, customCreateDir func(string) (string, error)) (string, error) { + + // Return the target path if empty. + if len(targetPath) < 0 { + return targetPath, nil } - if !fileInfo.IsDir() { - return fmt.Errorf("Target location %s is not a directory", targetPath) + + var newTargetPath string + + if customCreateDir != nil { + newpath, err := customCreateDir(targetPath) + if err != nil { + return "", err + } + newTargetPath = newpath + } else { + fileInfo, err := os.Stat(targetPath) + if err != nil && os.IsNotExist(err) { + return "", os.MkdirAll(targetPath, 0755) + } else if err != nil { + return "", err + } + if !fileInfo.IsDir() { + return "", fmt.Errorf("Target location %s is not a directory", targetPath) + } + newTargetPath = targetPath } - return nil + return newTargetPath, nil } func loadSecrets(path string) (*CSISecrets, error) { From cbc42e8ea88ae43c762498b9745e85d6f61d24c2 Mon Sep 17 00:00:00 2001 From: Sunny Date: Sun, 3 Mar 2019 17:22:12 +0530 Subject: [PATCH 2/4] Add target path checks and rm paths at teardown This adds target path exists checks in the mock driver in NodeStageVolume and NodePublishVolume to ensure that the paths actually exist. Also adds path cleanup in the teardown. This ensures that every test creates and deletes their target directories. Without this, the test setup function for Controller only tests created the target directory and the same was being used by subsequent tests. --- mock/service/node.go | 22 ++++++++++++++++++++++ pkg/sanity/sanity.go | 19 ++++++++++++++----- 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/mock/service/node.go b/mock/service/node.go index 7157404e..51eebf7f 100644 --- a/mock/service/node.go +++ b/mock/service/node.go @@ -1,6 +1,7 @@ package service import ( + "os" "path" "strconv" @@ -40,6 +41,10 @@ func (s *service) NodeStageVolume( return nil, status.Error(codes.InvalidArgument, "Volume Capability cannot be empty") } + if err := checkTargetExists(req.StagingTargetPath); err != nil { + return nil, status.Error(codes.Internal, err.Error()) + } + s.volsRWL.Lock() defer s.volsRWL.Unlock() @@ -131,6 +136,10 @@ func (s *service) NodePublishVolume( return nil, status.Error(codes.InvalidArgument, "Volume Capability cannot be empty") } + if err := checkTargetExists(req.TargetPath); err != nil { + return nil, status.Error(codes.Internal, err.Error()) + } + s.volsRWL.Lock() defer s.volsRWL.Unlock() @@ -157,6 +166,9 @@ func (s *service) NodePublishVolume( // Publish the volume. if req.GetStagingTargetPath() != "" { + if err := checkTargetExists(req.GetStagingTargetPath()); err != nil { + return nil, status.Error(codes.Internal, err.Error()) + } v.VolumeContext[nodeMntPathKey] = req.GetStagingTargetPath() } else { v.VolumeContext[nodeMntPathKey] = device @@ -336,3 +348,13 @@ func (s *service) NodeGetVolumeStats(ctx context.Context, }, }, nil } + +// checkTargetExists checks if a given path exists and returns error if the path +// does not exists. +func checkTargetExists(targetPath string) error { + _, err := os.Stat(targetPath) + if err != nil && os.IsNotExist(err) { + return status.Errorf(codes.Internal, "target path %s does not exists", targetPath) + } + return nil +} diff --git a/pkg/sanity/sanity.go b/pkg/sanity/sanity.go index ad075356..db71e950 100644 --- a/pkg/sanity/sanity.go +++ b/pkg/sanity/sanity.go @@ -176,6 +176,10 @@ func (sc *SanityContext) setup() { } func (sc *SanityContext) teardown() { + // Delete the created paths if any. + os.RemoveAll(sc.targetPath) + os.RemoveAll(sc.stagingPath) + // We intentionally do not close the connection to the CSI // driver here because the large amount of connection attempts // caused test failures @@ -191,7 +195,7 @@ func (sc *SanityContext) teardown() { func createMountTargetLocation(targetPath string, customCreateDir func(string) (string, error)) (string, error) { // Return the target path if empty. - if len(targetPath) < 0 { + if targetPath == "" { return targetPath, nil } @@ -205,11 +209,16 @@ func createMountTargetLocation(targetPath string, customCreateDir func(string) ( newTargetPath = newpath } else { fileInfo, err := os.Stat(targetPath) - if err != nil && os.IsNotExist(err) { - return "", os.MkdirAll(targetPath, 0755) - } else if err != nil { - return "", err + if err != nil { + if !os.IsNotExist(err) { + return "", err + } + if err := os.MkdirAll(targetPath, 0755); err != nil { + return "", err + } + return targetPath, nil } + if !fileInfo.IsDir() { return "", fmt.Errorf("Target location %s is not a directory", targetPath) } From a9b0016bd16c0f9e4c1fdb23e95220fe1cbc3869 Mon Sep 17 00:00:00 2001 From: Sunny Date: Sun, 3 Mar 2019 20:31:36 +0530 Subject: [PATCH 3/4] Add command line support for custom target paths This adds three new flags to support custom target path creation: - `createmountpathcmd`: Command to create a custom mount path. - `createstagingpathcmd`: Command to create a custom staging path. - `createpathcmdtimeout`: Timeout for the custom path creation commands. Also adds a new test section in e2e tests to test this custom command support. --- cmd/csi-sanity/sanity_test.go | 3 +++ hack/e2e.sh | 33 +++++++++++++++++++++++++++++ pkg/sanity/sanity.go | 39 +++++++++++++++++++++++++++++++---- 3 files changed, 71 insertions(+), 4 deletions(-) diff --git a/cmd/csi-sanity/sanity_test.go b/cmd/csi-sanity/sanity_test.go index 6c8f5e2e..e65d31b3 100644 --- a/cmd/csi-sanity/sanity_test.go +++ b/cmd/csi-sanity/sanity_test.go @@ -40,6 +40,9 @@ func init() { flag.BoolVar(&version, prefix+"version", false, "Version of this program") flag.StringVar(&config.TargetPath, prefix+"mountdir", os.TempDir()+"/csi", "Mount point for NodePublish") flag.StringVar(&config.StagingPath, prefix+"stagingdir", os.TempDir()+"/csi", "Mount point for NodeStage if staging is supported") + flag.StringVar(&config.CreateTargetPathCmd, prefix+"createmountpathcmd", "", "Command to run for target path creation") + flag.StringVar(&config.CreateStagingPathCmd, prefix+"createstagingpathcmd", "", "Command to run for staging path creation") + flag.IntVar(&config.CreatePathCmdTimeout, prefix+"createpathcmdtimeout", 10, "Timeout for the commands to create target and staging paths, in seconds") 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") diff --git a/hack/e2e.sh b/hack/e2e.sh index 83f4770d..f1258841 100755 --- a/hack/e2e.sh +++ b/hack/e2e.sh @@ -83,6 +83,36 @@ runTestAPIWithCustomTargetPaths() fi } +runTestWithCustomTargetPaths() +{ + CSI_ENDPOINT=$1 ./bin/mock-driver & + local pid=$! + + # Create a script for custom target path creation. + echo '#!/bin/bash +targetpath="/tmp/csi/$@" +mkdir -p $targetpath +echo $targetpath +' > custompath.bash + chmod +x custompath.bash + local scriptpath="$PWD/custompath.bash" + + ./cmd/csi-sanity/csi-sanity $TESTARGS \ + --csi.endpoint=$2 \ + --csi.mountdir="foo/target/mount" \ + --csi.stagingdir="foo/staging/mount" \ + --csi.createmountpathcmd=$scriptpath \ + --csi.createstagingpathcmd=$scriptpath; ret=$? + kill -9 $pid + + # Delete the script. + rm $scriptpath + + if [ $ret -ne 0 ] ; then + exit $ret + fi +} + make cd cmd/csi-sanity @@ -105,4 +135,7 @@ rm -f $UDS_CONTROLLER runTestAPIWithCustomTargetPaths "${UDS}" rm -rf $UDS +runTestWithCustomTargetPaths "${UDS}" "${UDS}" +rm -rf $UDS + exit 0 diff --git a/pkg/sanity/sanity.go b/pkg/sanity/sanity.go index db71e950..3cf9501a 100644 --- a/pkg/sanity/sanity.go +++ b/pkg/sanity/sanity.go @@ -17,11 +17,15 @@ limitations under the License. package sanity import ( + "context" "crypto/rand" "fmt" "io/ioutil" "os" + "os/exec" + "strings" "testing" + "time" "github.com/kubernetes-csi/csi-test/utils" yaml "gopkg.in/yaml.v2" @@ -67,6 +71,14 @@ type Config struct { // and StagingPath. CreateTargetDir func(path string) (string, error) CreateStagingDir func(path string) (string, error) + + // Commands to be executed for customized creation of the target and staging + // paths. This command must be available on the host where sanity runs. The + // stdout of the commands are the paths for mount and staging. + CreateTargetPathCmd string + CreateStagingPathCmd string + // Timeout for the executed commands for path creation. + CreatePathCmdTimeout int } // SanityContext holds the variables that each test can depend on. It @@ -164,13 +176,14 @@ func (sc *SanityContext) setup() { } By("creating mount and staging directories") + // If callback function for creating target dir is specified, use it. - targetPath, err := createMountTargetLocation(sc.Config.TargetPath, sc.Config.CreateTargetDir) + targetPath, err := createMountTargetLocation(sc.Config.TargetPath, sc.Config.CreateTargetPathCmd, sc.Config.CreateTargetDir, sc.Config.CreatePathCmdTimeout) Expect(err).NotTo(HaveOccurred(), "failed to create target directory %s", targetPath) sc.targetPath = targetPath // If callback function for creating staging dir is specified, use it. - stagingPath, err := createMountTargetLocation(sc.Config.StagingPath, sc.Config.CreateStagingDir) + stagingPath, err := createMountTargetLocation(sc.Config.StagingPath, sc.Config.CreateStagingPathCmd, sc.Config.CreateStagingDir, sc.Config.CreatePathCmdTimeout) Expect(err).NotTo(HaveOccurred(), "failed to create staging directory %s", stagingPath) sc.stagingPath = stagingPath } @@ -192,7 +205,10 @@ func (sc *SanityContext) teardown() { // (https://github.com/kubernetes-csi/csi-test/pull/98). } -func createMountTargetLocation(targetPath string, customCreateDir func(string) (string, error)) (string, error) { +// createMountTargetLocation takes a target path parameter and creates the +// target path using a custom command, custom function or falls back to the +// default using mkdir and returns the new target path. +func createMountTargetLocation(targetPath string, createPathCmd string, customCreateDir func(string) (string, error), timeout int) (string, error) { // Return the target path if empty. if targetPath == "" { @@ -201,13 +217,28 @@ func createMountTargetLocation(targetPath string, customCreateDir func(string) ( var newTargetPath string - if customCreateDir != nil { + if createPathCmd != "" { + // Create the target path using the create path command. + ctx, cancel := context.WithTimeout(context.Background(), time.Duration(timeout)*time.Second) + defer cancel() + + cmd := exec.CommandContext(ctx, createPathCmd, targetPath) + cmd.Stderr = os.Stderr + out, err := cmd.Output() + if err != nil { + return "", fmt.Errorf("target path creation command %s failed: %v", createPathCmd, err) + } + // Set the command's stdout as the new target path. + newTargetPath = strings.TrimSpace(string(out)) + } else if customCreateDir != nil { + // Create the target path using the custom create dir function. newpath, err := customCreateDir(targetPath) if err != nil { return "", err } newTargetPath = newpath } else { + // Create the target path using mkdir. fileInfo, err := os.Stat(targetPath) if err != nil { if !os.IsNotExist(err) { From 506d12811cbbd23cf4bda1e50d3a99ecd903dde3 Mon Sep 17 00:00:00 2001 From: Sunny Date: Sun, 17 Mar 2019 16:29:24 +0530 Subject: [PATCH 4/4] Add support for custom target path removal This adds three new flags to support custom target path removal: - `removemountpathcmd`: Command to remove a custom mount path. - `removestagingpathcmd`: Command to remove a custom staging path. - `removepathcmdtimeout`: Timeout for the custom path removal commands. --- cmd/csi-sanity/sanity_test.go | 3 +++ hack/_apitest2/api_test.go | 21 ++++++++++++++- hack/e2e.sh | 21 ++++++++++----- pkg/sanity/sanity.go | 48 ++++++++++++++++++++++++++++++++--- 4 files changed, 83 insertions(+), 10 deletions(-) diff --git a/cmd/csi-sanity/sanity_test.go b/cmd/csi-sanity/sanity_test.go index e65d31b3..032b791a 100644 --- a/cmd/csi-sanity/sanity_test.go +++ b/cmd/csi-sanity/sanity_test.go @@ -43,6 +43,9 @@ func init() { flag.StringVar(&config.CreateTargetPathCmd, prefix+"createmountpathcmd", "", "Command to run for target path creation") flag.StringVar(&config.CreateStagingPathCmd, prefix+"createstagingpathcmd", "", "Command to run for staging path creation") flag.IntVar(&config.CreatePathCmdTimeout, prefix+"createpathcmdtimeout", 10, "Timeout for the commands to create target and staging paths, in seconds") + flag.StringVar(&config.RemoveTargetPathCmd, prefix+"removemountpathcmd", "", "Command to run for target path removal") + flag.StringVar(&config.RemoveStagingPathCmd, prefix+"removestagingpathcmd", "", "Command to run for staging path removal") + flag.IntVar(&config.RemovePathCmdTimeout, prefix+"removepathcmdtimeout", 10, "Timeout for the commands to remove target and staging paths, in seconds") 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") diff --git a/hack/_apitest2/api_test.go b/hack/_apitest2/api_test.go index 40acc3f4..28da8560 100644 --- a/hack/_apitest2/api_test.go +++ b/hack/_apitest2/api_test.go @@ -12,10 +12,13 @@ import ( // TestMyDriverWithCustomTargetPaths verifies that CreateTargetDir and // CreateStagingDir are called a specific number of times. func TestMyDriverWithCustomTargetPaths(t *testing.T) { - var createTargetDirCalls, createStagingDirCalls int + var createTargetDirCalls, createStagingDirCalls, + removeTargetDirCalls, removeStagingDirCalls int wantCreateTargetCalls := 3 wantCreateStagingCalls := 3 + wantRemoveTargetCalls := 3 + wantRemoveStagingCalls := 3 // tmpPath could be a CO specific directory under which all the target dirs // are created. For k8s, it could be /var/lib/kubelet/pods under which the @@ -35,6 +38,14 @@ func TestMyDriverWithCustomTargetPaths(t *testing.T) { targetPath = path.Join(tmpPath, targetPath) return targetPath, createTargetDir(targetPath) }, + RemoveTargetPath: func(targetPath string) error { + removeTargetDirCalls++ + return os.RemoveAll(targetPath) + }, + RemoveStagingPath: func(targetPath string) error { + removeStagingDirCalls++ + return os.RemoveAll(targetPath) + }, } sanity.Test(t, config) @@ -46,6 +57,14 @@ func TestMyDriverWithCustomTargetPaths(t *testing.T) { if createStagingDirCalls != wantCreateStagingCalls { t.Errorf("unexpected number of CreateStagingDir calls:\n(WNT) %d\n(GOT) %d", wantCreateStagingCalls, createStagingDirCalls) } + + if removeTargetDirCalls != wantRemoveTargetCalls { + t.Errorf("unexpected number of RemoveTargetDir calls:\n(WNT) %d\n(GOT) %d", wantRemoveTargetCalls, removeTargetDirCalls) + } + + if removeStagingDirCalls != wantRemoveStagingCalls { + t.Errorf("unexpected number of RemoveStagingDir calls:\n(WNT) %d\n(GOT) %d", wantRemoveStagingCalls, removeStagingDirCalls) + } } func createTargetDir(targetPath string) error { diff --git a/hack/e2e.sh b/hack/e2e.sh index f1258841..18460b46 100755 --- a/hack/e2e.sh +++ b/hack/e2e.sh @@ -93,20 +93,29 @@ runTestWithCustomTargetPaths() targetpath="/tmp/csi/$@" mkdir -p $targetpath echo $targetpath -' > custompath.bash - chmod +x custompath.bash - local scriptpath="$PWD/custompath.bash" +' > custompathcreation.bash + + # Create a script for custom target path removal. + echo '#!/bin/bash +rm -rf $@ +' > custompathremoval.bash + + chmod +x custompathcreation.bash custompathremoval.bash + local creationscriptpath="$PWD/custompathcreation.bash" + local removalscriptpath="$PWD/custompathremoval.bash" ./cmd/csi-sanity/csi-sanity $TESTARGS \ --csi.endpoint=$2 \ --csi.mountdir="foo/target/mount" \ --csi.stagingdir="foo/staging/mount" \ - --csi.createmountpathcmd=$scriptpath \ - --csi.createstagingpathcmd=$scriptpath; ret=$? + --csi.createmountpathcmd=$creationscriptpath \ + --csi.createstagingpathcmd=$creationscriptpath \ + --csi.removemountpathcmd=$removalscriptpath \ + --csi.removestagingpathcmd=$removalscriptpath; ret=$? kill -9 $pid # Delete the script. - rm $scriptpath + rm $creationscriptpath $removalscriptpath if [ $ret -ne 0 ] ; then exit $ret diff --git a/pkg/sanity/sanity.go b/pkg/sanity/sanity.go index 3cf9501a..68e782b3 100644 --- a/pkg/sanity/sanity.go +++ b/pkg/sanity/sanity.go @@ -68,10 +68,17 @@ type Config struct { // Callback functions to customize the creation of target and staging // directories. Returns the new paths for mount and staging. // If not defined, directories are created in the default way at TargetPath - // and StagingPath. + // and StagingPath on the host. CreateTargetDir func(path string) (string, error) CreateStagingDir func(path string) (string, error) + // Callback functions to customize the removal of the target and staging + // directories. + // If not defined, directories are removed in the default way at TargetPath + // and StagingPath on the host. + RemoveTargetPath func(path string) error + RemoveStagingPath func(path string) error + // Commands to be executed for customized creation of the target and staging // paths. This command must be available on the host where sanity runs. The // stdout of the commands are the paths for mount and staging. @@ -79,6 +86,13 @@ type Config struct { CreateStagingPathCmd string // Timeout for the executed commands for path creation. CreatePathCmdTimeout int + + // Commands to be executed for customized removal of the target and staging + // paths. Thie command must be available on the host where sanity runs. + RemoveTargetPathCmd string + RemoveStagingPathCmd string + // Timeout for the executed commands for path removal. + RemovePathCmdTimeout int } // SanityContext holds the variables that each test can depend on. It @@ -190,8 +204,8 @@ func (sc *SanityContext) setup() { func (sc *SanityContext) teardown() { // Delete the created paths if any. - os.RemoveAll(sc.targetPath) - os.RemoveAll(sc.stagingPath) + removeMountTargetLocation(sc.targetPath, sc.Config.RemoveTargetPathCmd, sc.Config.RemoveTargetPath, sc.Config.RemovePathCmdTimeout) + removeMountTargetLocation(sc.stagingPath, sc.Config.RemoveStagingPathCmd, sc.Config.RemoveStagingPath, sc.Config.RemovePathCmdTimeout) // We intentionally do not close the connection to the CSI // driver here because the large amount of connection attempts @@ -259,6 +273,34 @@ func createMountTargetLocation(targetPath string, createPathCmd string, customCr return newTargetPath, nil } +// removeMountTargetLocation takes a target path parameter and removes the path +// using a custom command, custom function or falls back to the default removal +// by deleting the path on the host. +func removeMountTargetLocation(targetPath string, removePathCmd string, customRemovePath func(string) error, timeout int) error { + if targetPath == "" { + return nil + } + + if removePathCmd != "" { + ctx, cancel := context.WithTimeout(context.Background(), time.Duration(timeout)*time.Second) + defer cancel() + + cmd := exec.CommandContext(ctx, removePathCmd, targetPath) + cmd.Stderr = os.Stderr + _, err := cmd.Output() + if err != nil { + return fmt.Errorf("target path removal command %s failed: %v", removePathCmd, err) + } + } else if customRemovePath != nil { + if err := customRemovePath(targetPath); err != nil { + return err + } + } else { + return os.RemoveAll(targetPath) + } + return nil +} + func loadSecrets(path string) (*CSISecrets, error) { var creds CSISecrets