Skip to content

Commit

Permalink
Merge pull request #182 from pohly/publish-dir
Browse files Browse the repository at this point in the history
strict staging and target directory
  • Loading branch information
k8s-ci-robot authored Mar 25, 2019
2 parents edc1a4f + 267dbbc commit 692e5bd
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 32 deletions.
4 changes: 2 additions & 2 deletions cmd/csi-sanity/sanity_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ func init() {
flag.StringVar(&config.Address, prefix+"endpoint", "", "CSI endpoint")
flag.StringVar(&config.ControllerAddress, prefix+"controllerendpoint", "", "CSI controller endpoint")
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.TargetPath, prefix+"mountdir", os.TempDir()+"/csi-mount", "Mount point for NodePublish")
flag.StringVar(&config.StagingPath, prefix+"stagingdir", os.TempDir()+"/csi-staging", "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")
Expand Down
4 changes: 2 additions & 2 deletions hack/_apitest/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import (

func TestMyDriver(t *testing.T) {
config := &sanity.Config{
TargetPath: os.TempDir() + "/csi",
StagingPath: os.TempDir() + "/csi",
TargetPath: os.TempDir() + "/csi-target",
StagingPath: os.TempDir() + "/csi-staging",
Address: "/tmp/e2e-csi-sanity.sock",
TestNodeVolumeAttachLimit: true,
}
Expand Down
4 changes: 2 additions & 2 deletions hack/_embedded/embedded_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ var _ = AfterSuite(func() {})
var _ = Describe("MyCSIDriver", func() {
Context("Config A", func() {
config := &sanity.Config{
TargetPath: os.TempDir() + "/csi",
StagingPath: os.TempDir() + "/csi",
TargetPath: os.TempDir() + "/csi-target",
StagingPath: os.TempDir() + "/csi-staging",
Address: "/tmp/e2e-csi-sanity.sock",
TestNodeVolumeAttachLimit: true,
}
Expand Down
22 changes: 19 additions & 3 deletions mock/service/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func (s *service) NodePublishVolume(
return nil, status.Error(codes.InvalidArgument, "Volume Capability cannot be empty")
}

if err := checkTargetExists(req.TargetPath); err != nil {
if err := checkTargetNotExists(req.TargetPath); err != nil {
return nil, status.Error(codes.Internal, err.Error())
}

Expand Down Expand Up @@ -353,8 +353,24 @@ func (s *service) NodeGetVolumeStats(ctx context.Context,
// does not exists.
func checkTargetExists(targetPath string) error {
_, err := os.Stat(targetPath)
if err != nil && os.IsNotExist(err) {
if err == nil {
return nil
}
if os.IsNotExist(err) {
return status.Errorf(codes.Internal, "target path %s does not exists", targetPath)
}
return nil
return status.Errorf(codes.Internal, "stat target path %s: %s", targetPath, err)
}

// checkTargetNotExists checks if a given path does not exist and returns error if the path
// does exist.
func checkTargetNotExists(targetPath string) error {
_, err := os.Stat(targetPath)
if err == nil {
return status.Errorf(codes.Internal, "target path %s does exist", targetPath)
}
if os.IsNotExist(err) {
return nil
}
return status.Errorf(codes.Internal, "stat target path %s: %s", targetPath, err)
}
12 changes: 6 additions & 6 deletions pkg/sanity/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ var _ = DescribeSanity("Node Service", func(sc *SanityContext) {
context.Background(),
&csi.NodePublishVolumeRequest{
VolumeId: "id",
TargetPath: sc.targetPath,
TargetPath: sc.targetPath + "/target",
Secrets: sc.Secrets.NodePublishVolumeSecret,
},
)
Expand Down Expand Up @@ -534,7 +534,7 @@ var _ = DescribeSanity("Node Service", func(sc *SanityContext) {
context.Background(),
&csi.NodePublishVolumeRequest{
VolumeId: vol.GetVolume().GetVolumeId(),
TargetPath: sc.targetPath,
TargetPath: sc.targetPath + "/target",
StagingTargetPath: stagingPath,
VolumeCapability: &csi.VolumeCapability{
AccessType: &csi.VolumeCapability_Mount{
Expand Down Expand Up @@ -573,7 +573,7 @@ var _ = DescribeSanity("Node Service", func(sc *SanityContext) {
context.Background(),
&csi.NodeUnpublishVolumeRequest{
VolumeId: vol.GetVolume().GetVolumeId(),
TargetPath: sc.targetPath,
TargetPath: sc.targetPath + "/target",
})
Expect(err).NotTo(HaveOccurred())
Expect(nodeunpubvol).NotTo(BeNil())
Expand Down Expand Up @@ -718,7 +718,7 @@ var _ = DescribeSanity("Node Service", func(sc *SanityContext) {
context.Background(),
&csi.NodePublishVolumeRequest{
VolumeId: vol.GetVolume().GetVolumeId(),
TargetPath: sc.targetPath,
TargetPath: sc.targetPath + "/target",
StagingTargetPath: stagingPath,
VolumeCapability: &csi.VolumeCapability{
AccessType: &csi.VolumeCapability_Mount{
Expand All @@ -743,7 +743,7 @@ var _ = DescribeSanity("Node Service", func(sc *SanityContext) {
context.Background(),
&csi.NodeGetVolumeStatsRequest{
VolumeId: vol.GetVolume().GetVolumeId(),
VolumePath: sc.targetPath,
VolumePath: sc.targetPath + "/target",
},
)
Expect(err).ToNot(HaveOccurred())
Expand All @@ -756,7 +756,7 @@ var _ = DescribeSanity("Node Service", func(sc *SanityContext) {
context.Background(),
&csi.NodeUnpublishVolumeRequest{
VolumeId: vol.GetVolume().GetVolumeId(),
TargetPath: sc.targetPath,
TargetPath: sc.targetPath + "/target",
})
Expect(err).NotTo(HaveOccurred())
Expect(nodeunpubvol).NotTo(BeNil())
Expand Down
49 changes: 32 additions & 17 deletions pkg/sanity/sanity.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,14 @@ type CSISecrets struct {
// Config provides the configuration for the sanity tests. It
// needs to be initialized by the user of the sanity package.
type Config struct {
TargetPath string
StagingPath string
// TargetPath is the *parent* directory for NodePublishVolumeRequest.target_path.
// It gets created and removed by csi-sanity.
TargetPath string

// StagingPath is the NodeStageVolumeRequest.staging_target_path.
// It gets created and removed by csi-sanity.
StagingPath string

Address string
ControllerAddress string
SecretsFile string
Expand All @@ -70,13 +76,30 @@ type Config struct {
// directories. Returns the new paths for mount and staging.
// If not defined, directories are created in the default way at TargetPath
// and StagingPath on the host.
//
// Both functions can replace the suggested path. What the test then uses
// is the path returned by them.
//
// Note that target and staging directory have different
// semantics in the CSI spec: for NodeStateVolume,
// CreateTargetDir must create the directory and return the
// full path to it. For NodePublishVolume, CreateStagingDir
// must create the *parent* directory of `path` (or some other
// directory) and return the full path for an entry inside
// that created directory.
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.
//
// Both functions are passed the actual paths as used during the test.
//
// Note that RemoveTargetPath only needs to remove the *parent* of the
// given path. The CSI driver should have removed the entry at that path
// already.
RemoveTargetPath func(path string) error
RemoveStagingPath func(path string) error

Expand Down Expand Up @@ -255,20 +278,11 @@ func createMountTargetLocation(targetPath string, createPathCmd string, customCr
}
newTargetPath = newpath
} else {
// Create the target path using mkdir.
fileInfo, err := os.Stat(targetPath)
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)
// Create the target path. Only the directory itself
// and not its parents get created, and it is an error
// if the directory already exists.
if err := os.Mkdir(targetPath, 0755); err != nil {
return "", err
}
newTargetPath = targetPath
}
Expand Down Expand Up @@ -299,7 +313,8 @@ func removeMountTargetLocation(targetPath string, removePathCmd string, customRe
return err
}
} else {
return os.RemoveAll(targetPath)
// It's an error if the directory is not empty by now.
return os.Remove(targetPath)
}
return nil
}
Expand Down

0 comments on commit 692e5bd

Please sign in to comment.