sanity: clean up configuration handling

NewTestConfig now must be used to initialize the struct with defaults. This makes it easier to introduce new fields where the empty value isn't a suitable default. To ensure that callers adapt to the new semantic, the struct gets renamed. This also allows removing replicated default values all over the source code. SanityContext gets renamed to TestContext for consistency. Its new Finalize method should be used to clean up. To allow that, GinkgoTest returns the context pointer.
kubernetes-csi · Oct 29, 2019 · d5badbb · d5badbb
1 parent ec1441c
commit d5badbb
Show file tree

Hide file tree

Showing 10 changed files with 193 additions and 145 deletions.
diff --git a/cmd/csi-sanity/sanity_test.go b/cmd/csi-sanity/sanity_test.go
@@ -30,38 +30,58 @@ const (
 
 var (
 	VERSION = "(dev)"
-	version bool
-	config  sanity.Config
+	config  = sanity.NewTestConfig()
 )
 
-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", "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")
-	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.Int64Var(&config.TestVolumeExpandSize, prefix+"testvolumeexpandsize", 0, "Target size for expanded 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()
+func stringVar(p *string, name string, usage string) {
+	flag.StringVar(p, prefix+name, *p, usage)
 }
 
-func TestSanity(t *testing.T) {
-	if version {
+func boolVar(p *bool, name string, usage string) {
+	flag.BoolVar(p, prefix+name, *p, usage)
+}
+
+func intVar(p *int, name string, usage string) {
+	flag.IntVar(p, prefix+name, *p, usage)
+}
+
+func int64Var(p *int64, name string, usage string) {
+	flag.Int64Var(p, prefix+name, *p, usage)
+}
+
+func TestMain(m *testing.M) {
+	version := flag.Bool("version", false, "print version of this program")
+
+	// Support overriding the default configuration via flags.
+	stringVar(&config.Address, "endpoint", "CSI endpoint")
+	stringVar(&config.ControllerAddress, "controllerendpoint", "CSI controller endpoint")
+	stringVar(&config.TargetPath, "mountdir", "Mount point for NodePublish")
+	stringVar(&config.StagingPath, "stagingdir", "Mount point for NodeStage if staging is supported")
+	stringVar(&config.CreateTargetPathCmd, "createmountpathcmd", "Command to run for target path creation")
+	stringVar(&config.CreateStagingPathCmd, "createstagingpathcmd", "Command to run for staging path creation")
+	intVar(&config.CreatePathCmdTimeout, "createpathcmdtimeout", "Timeout for the commands to create target and staging paths, in seconds")
+	stringVar(&config.RemoveTargetPathCmd, "removemountpathcmd", "Command to run for target path removal")
+	stringVar(&config.RemoveStagingPathCmd, "removestagingpathcmd", "Command to run for staging path removal")
+	intVar(&config.RemovePathCmdTimeout, "removepathcmdtimeout", "Timeout for the commands to remove target and staging paths, in seconds")
+	stringVar(&config.SecretsFile, "secrets", "CSI secrets file")
+	int64Var(&config.TestVolumeSize, "testvolumesize", "Base volume size used for provisioned volumes")
+	int64Var(&config.TestVolumeExpandSize, "testvolumeexpandsize", "Target size for expanded volumes")
+	stringVar(&config.TestVolumeParametersFile, "testvolumeparameters", "YAML file of volume parameters for provisioned volumes")
+	boolVar(&config.TestNodeVolumeAttachLimit, "testnodevolumeattachlimit", "Test node volume attach limit")
+	stringVar(&config.JUnitFile, "junitfile", "JUnit XML output file where test results will be written")
+
+	flag.Parse()
+	if *version {
 		fmt.Printf("Version = %s\n", VERSION)
-		return
+		os.Exit(0)
 	}
-	if len(config.Address) == 0 {
-		t.Fatalf("--%sendpoint must be provided with an CSI endpoint", prefix)
+	if config.Address == "" {
+		fmt.Printf("--%sendpoint must be provided with an CSI endpoint\n", prefix)
+		os.Exit(1)
 	}
-	sanity.Test(t, &config)
+	os.Exit(m.Run())
+}
+
+func TestSanity(t *testing.T) {
+	sanity.Test(t, config)
 }
diff --git a/hack/_apitest/api_test.go b/hack/_apitest/api_test.go
@@ -1,19 +1,15 @@
 package apitest
 
 import (
-	"os"
 	"testing"
 
 	"github.com/kubernetes-csi/csi-test/pkg/sanity"
 )
 
 func TestMyDriver(t *testing.T) {
-	config := &sanity.Config{
-		TargetPath:                os.TempDir() + "/csi-target",
-		StagingPath:               os.TempDir() + "/csi-staging",
-		Address:                   "/tmp/e2e-csi-sanity.sock",
-		TestNodeVolumeAttachLimit: true,
-	}
+	config := sanity.NewTestConfig()
+	config.Address = "/tmp/e2e-csi-sanity.sock"
+	config.TestNodeVolumeAttachLimit = true
 
 	sanity.Test(t, config)
 }
diff --git a/hack/_apitest2/api_test.go b/hack/_apitest2/api_test.go
@@ -24,28 +24,27 @@ func TestMyDriverWithCustomTargetPaths(t *testing.T) {
 	// 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)
-		},
-		RemoveTargetPath: func(targetPath string) error {
-			removeTargetDirCalls++
-			return os.RemoveAll(targetPath)
-		},
-		RemoveStagingPath: func(targetPath string) error {
-			removeStagingDirCalls++
-			return os.RemoveAll(targetPath)
-		},
+	config := sanity.NewTestConfig()
+	config.TargetPath = "foo/target/mount"
+	config.StagingPath = "foo/staging/mount"
+	config.Address = "/tmp/e2e-csi-sanity.sock"
+	config.CreateTargetDir = func(targetPath string) (string, error) {
+		createTargetDirCalls++
+		targetPath = path.Join(tmpPath, targetPath)
+		return targetPath, createTargetDir(targetPath)
+	}
+	config.CreateStagingDir = func(targetPath string) (string, error) {
+		createStagingDirCalls++
+		targetPath = path.Join(tmpPath, targetPath)
+		return targetPath, createTargetDir(targetPath)
+	}
+	config.RemoveTargetPath = func(targetPath string) error {
+		removeTargetDirCalls++
+		return os.RemoveAll(targetPath)
+	}
+	config.RemoveStagingPath = func(targetPath string) error {
+		removeStagingDirCalls++
+		return os.RemoveAll(targetPath)
 	}
 
 	sanity.Test(t, config)

diff --git a/hack/_embedded/embedded_test.go b/hack/_embedded/embedded_test.go
@@ -1,14 +1,15 @@
 package embedded
 
 import (
-	"os"
 	"testing"
 
 	"github.com/kubernetes-csi/csi-test/pkg/sanity"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 )
 
+var context *sanity.TestContext
+
 func TestMyDriverGinkgo(t *testing.T) {
 	RegisterFailHandler(Fail)
 	RunSpecs(t, "CSI Sanity Test Suite")
@@ -21,24 +22,24 @@ func TestMyDriverGinkgo(t *testing.T) {
 // in hack/e2e.sh if a PR adds back such functions in the sanity test
 // code.
 var _ = BeforeSuite(func() {})
-var _ = AfterSuite(func() {})
+var _ = AfterSuite(func() {
+	if context != nil {
+		context.Finalize()
+	}
+})
 
 var _ = Describe("MyCSIDriver", func() {
 	Context("Config A", func() {
-		config := &sanity.Config{
-			TargetPath:                os.TempDir() + "/csi-target",
-			StagingPath:               os.TempDir() + "/csi-staging",
-			Address:                   "/tmp/e2e-csi-sanity.sock",
-			TestNodeVolumeAttachLimit: true,
-			IDGen:                     &sanity.DefaultIDGenerator{},
-		}
+		config := sanity.NewTestConfig()
+		config.Address = "/tmp/e2e-csi-sanity.sock"
+		config.TestNodeVolumeAttachLimit = true
 
 		BeforeEach(func() {})
 
 		AfterEach(func() {})
 
 		Describe("CSI Driver Test Suite", func() {
-			sanity.GinkgoTest(config)
+			context = sanity.GinkgoTest(&config)
 		})
 	})
 })
diff --git a/pkg/sanity/cleanup.go b/pkg/sanity/cleanup.go
@@ -39,7 +39,7 @@ type VolumeInfo struct {
 // Cleanup keeps track of resources, in particular volumes, which need
 // to be freed when testing is done. All methods can be called concurrently.
 type Cleanup struct {
-	Context                    *SanityContext
+	Context                    *TestContext
 	ControllerClient           csi.ControllerClient
 	NodeClient                 csi.NodeClient
 	ControllerPublishSupported bool

diff --git a/pkg/sanity/controller.go b/pkg/sanity/controller.go
@@ -31,11 +31,6 @@ import (
 )
 
 const (
-	// DefTestVolumeSize defines the base size of dynamically
-	// provisioned volumes. 10GB by default, can be overridden by
-	// setting Config.TestVolumeSize.
-	DefTestVolumeSize int64 = 10 * 1024 * 1024 * 1024
-
 	// DefTestVolumeExpand defines the size increment for volume
 	// expansion. It can be overriden by setting an
 	// Config.TestVolumeExpandSize, which will be taken as absolute
@@ -45,14 +40,11 @@ const (
 	MaxNameLength int = 128
 )
 
-func TestVolumeSize(sc *SanityContext) int64 {
-	if sc.Config.TestVolumeSize > 0 {
-		return sc.Config.TestVolumeSize
-	}
-	return DefTestVolumeSize
+func TestVolumeSize(sc *TestContext) int64 {
+	return sc.Config.TestVolumeSize
 }
 
-func TestVolumeExpandSize(sc *SanityContext) int64 {
+func TestVolumeExpandSize(sc *TestContext) int64 {
 	if sc.Config.TestVolumeExpandSize > 0 {
 		return sc.Config.TestVolumeExpandSize
 	}
@@ -92,7 +84,7 @@ func isControllerCapabilitySupported(
 	return false
 }
 
-var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *SanityContext) {
+var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestContext) {
 	var (
 		c csi.ControllerClient
 		n csi.NodeClient
@@ -1617,7 +1609,7 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *Sanity
 	})
 })
 
-var _ = DescribeSanity("ListSnapshots [Controller Server]", func(sc *SanityContext) {
+var _ = DescribeSanity("ListSnapshots [Controller Server]", func(sc *TestContext) {
 	var (
 		c csi.ControllerClient
 	)
@@ -1870,7 +1862,7 @@ var _ = DescribeSanity("ListSnapshots [Controller Server]", func(sc *SanityConte
 
 })
 
-var _ = DescribeSanity("DeleteSnapshot [Controller Server]", func(sc *SanityContext) {
+var _ = DescribeSanity("DeleteSnapshot [Controller Server]", func(sc *TestContext) {
 	var (
 		c csi.ControllerClient
 	)
@@ -1933,7 +1925,7 @@ var _ = DescribeSanity("DeleteSnapshot [Controller Server]", func(sc *SanityCont
 	})
 })
 
-var _ = DescribeSanity("CreateSnapshot [Controller Server]", func(sc *SanityContext) {
+var _ = DescribeSanity("CreateSnapshot [Controller Server]", func(sc *TestContext) {
 	var (
 		c csi.ControllerClient
 	)
@@ -2082,7 +2074,7 @@ var _ = DescribeSanity("CreateSnapshot [Controller Server]", func(sc *SanityCont
 	})
 })
 
-var _ = DescribeSanity("ExpandVolume [Controller Server]", func(sc *SanityContext) {
+var _ = DescribeSanity("ExpandVolume [Controller Server]", func(sc *TestContext) {
 	var (
 		c  csi.ControllerClient
 		cl *Cleanup
@@ -2188,7 +2180,7 @@ var _ = DescribeSanity("ExpandVolume [Controller Server]", func(sc *SanityContex
 	})
 })
 
-func MakeCreateVolumeReq(sc *SanityContext, name string) *csi.CreateVolumeRequest {
+func MakeCreateVolumeReq(sc *TestContext, name string) *csi.CreateVolumeRequest {
 	size1 := TestVolumeSize(sc)
 
 	req := &csi.CreateVolumeRequest{
@@ -2217,7 +2209,7 @@ func MakeCreateVolumeReq(sc *SanityContext, name string) *csi.CreateVolumeReques
 	return req
 }
 
-func MakeCreateSnapshotReq(sc *SanityContext, name, sourceVolumeId string, parameters map[string]string) *csi.CreateSnapshotRequest {
+func MakeCreateSnapshotReq(sc *TestContext, name, sourceVolumeId string, parameters map[string]string) *csi.CreateSnapshotRequest {
 	req := &csi.CreateSnapshotRequest{
 		Name:           name,
 		SourceVolumeId: sourceVolumeId,
@@ -2231,7 +2223,7 @@ func MakeCreateSnapshotReq(sc *SanityContext, name, sourceVolumeId string, param
 	return req
 }
 
-func MakeDeleteSnapshotReq(sc *SanityContext, id string) *csi.DeleteSnapshotRequest {
+func MakeDeleteSnapshotReq(sc *TestContext, id string) *csi.DeleteSnapshotRequest {
 	delSnapReq := &csi.DeleteSnapshotRequest{
 		SnapshotId: id,
 	}
@@ -2243,7 +2235,7 @@ func MakeDeleteSnapshotReq(sc *SanityContext, id string) *csi.DeleteSnapshotRequ
 	return delSnapReq
 }
 
-func MakeDeleteVolumeReq(sc *SanityContext, id string) *csi.DeleteVolumeRequest {
+func MakeDeleteVolumeReq(sc *TestContext, id string) *csi.DeleteVolumeRequest {
 	delVolReq := &csi.DeleteVolumeRequest{
 		VolumeId: id,
 	}
@@ -2256,7 +2248,7 @@ func MakeDeleteVolumeReq(sc *SanityContext, id string) *csi.DeleteVolumeRequest
 }
 
 // MakeControllerPublishVolumeReq creates and returns a ControllerPublishVolumeRequest.
-func MakeControllerPublishVolumeReq(sc *SanityContext, volID, nodeID string) *csi.ControllerPublishVolumeRequest {
+func MakeControllerPublishVolumeReq(sc *TestContext, volID, nodeID string) *csi.ControllerPublishVolumeRequest {
 	return &csi.ControllerPublishVolumeRequest{
 		VolumeId: volID,
 		NodeId:   nodeID,
@@ -2274,7 +2266,7 @@ func MakeControllerPublishVolumeReq(sc *SanityContext, volID, nodeID string) *cs
 }
 
 // MakeControllerUnpublishVolumeReq creates and returns a ControllerUnpublishVolumeRequest.
-func MakeControllerUnpublishVolumeReq(sc *SanityContext, volID, nodeID string) *csi.ControllerUnpublishVolumeRequest {
+func MakeControllerUnpublishVolumeReq(sc *TestContext, volID, nodeID string) *csi.ControllerUnpublishVolumeRequest {
 	return &csi.ControllerUnpublishVolumeRequest{
 		VolumeId: volID,
 		NodeId:   nodeID,
@@ -2283,7 +2275,7 @@ func MakeControllerUnpublishVolumeReq(sc *SanityContext, volID, nodeID string) *
 }
 
 // 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) {
+func CreateAndControllerPublishVolume(sc *TestContext, 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())
@@ -2298,7 +2290,7 @@ func CreateAndControllerPublishVolume(sc *SanityContext, c csi.ControllerClient,
 }
 
 // ControllerUnpublishAndDeleteVolume controller unpublishes and deletes a volume, given volume ID and node ID.
-func ControllerUnpublishAndDeleteVolume(sc *SanityContext, c csi.ControllerClient, volID, nodeID string) error {
+func ControllerUnpublishAndDeleteVolume(sc *TestContext, c csi.ControllerClient, volID, nodeID string) error {
 	_, err := c.ControllerUnpublishVolume(
 		context.Background(),
 		MakeControllerUnpublishVolumeReq(sc, volID, nodeID),

diff --git a/pkg/sanity/identity.go b/pkg/sanity/identity.go
@@ -30,7 +30,7 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-var _ = DescribeSanity("Identity Service", func(sc *SanityContext) {
+var _ = DescribeSanity("Identity Service", func(sc *TestContext) {
 	var (
 		c csi.IdentityClient
 	)

diff --git a/pkg/sanity/node.go b/pkg/sanity/node.go
@@ -66,7 +66,7 @@ func isPluginCapabilitySupported(c csi.IdentityClient,
 	return false
 }
 
-var _ = DescribeSanity("Node Service", func(sc *SanityContext) {
+var _ = DescribeSanity("Node Service", func(sc *TestContext) {
 	var (
 		cl *Cleanup
 		c  csi.NodeClient