address reviewer comments

test/e2e/storage/drivers/csi.go

@@ -61,7 +61,7 @@ type hostpathCSIDriver struct {
 	manifests  []string
 }
 
-func initHostPathCSIDriver(name string, config testsuites.TestConfig, manifests ...string) testsuites.TestDriver {
+func initHostPathCSIDriver(name string, config testsuites.TestConfig, capabilities map[testsuites.Capability]bool, manifests ...string) testsuites.TestDriver {
 	return &hostpathCSIDriver{
 		driverInfo: testsuites.DriverInfo{
 			Name:        name,
@@ -70,12 +70,8 @@ func initHostPathCSIDriver(name string, config testsuites.TestConfig, manifests
 			SupportedFsType: sets.NewString(
 				"", // Default fsType
 			),
-			Capabilities: map[testsuites.Capability]bool{
-				testsuites.CapPersistence: true,
-				testsuites.CapDataSource:  true,
-			},
-
-			Config: config,
+			Capabilities: capabilities,
+			Config:       config,
 		},
 		manifests: manifests,
 	}
@@ -88,6 +84,7 @@ var _ testsuites.SnapshottableTestDriver = &hostpathCSIDriver{}
 // InitHostPathCSIDriver returns hostpathCSIDriver that implements TestDriver interface
 func InitHostPathCSIDriver(config testsuites.TestConfig) testsuites.TestDriver {
 	return initHostPathCSIDriver("csi-hostpath", config,
+		map[testsuites.Capability]bool{testsuites.CapPersistence: true, testsuites.CapDataSource: true},
 		"test/e2e/testing-manifests/storage-csi/driver-registrar/rbac.yaml",
 		"test/e2e/testing-manifests/storage-csi/external-attacher/rbac.yaml",
 		"test/e2e/testing-manifests/storage-csi/external-provisioner/rbac.yaml",
@@ -262,6 +259,7 @@ func (m *mockCSIDriver) CleanupDriver() {
 // InitHostPathV0CSIDriver returns a variant of hostpathCSIDriver with different manifests.
 func InitHostPathV0CSIDriver(config testsuites.TestConfig) testsuites.TestDriver {
 	return initHostPathCSIDriver("csi-hostpath-v0", config,
+		map[testsuites.Capability]bool{testsuites.CapPersistence: true},
 		"test/e2e/testing-manifests/storage-csi/driver-registrar/rbac.yaml",
 		"test/e2e/testing-manifests/storage-csi/external-attacher/rbac.yaml",
 		"test/e2e/testing-manifests/storage-csi/external-provisioner/rbac.yaml",

test/e2e/storage/testsuites/provisioning.go

@@ -162,8 +162,8 @@ func (p *provisioningTestResource) setupResource(driver TestDriver, pattern test
 			p.pvc = getClaim(p.claimSize, driver.GetDriverInfo().Config.Framework.Namespace.Name)
 			p.pvc.Spec.StorageClassName = &p.sc.Name
 			framework.Logf("In creating storage class object and pvc object for driver - sc: %v, pvc: %v", p.sc, p.pvc)
-			if dDriver, ok := driver.(SnapshottableTestDriver); ok {
-				p.vsc = dDriver.GetSnapshotClass()
+			if sDriver, ok := driver.(SnapshottableTestDriver); ok {
+				p.vsc = sDriver.GetSnapshotClass()
 			}
 		}
 	default:
@@ -213,14 +213,10 @@ func testProvisioning(input *provisioningTestInput) {
 		if !input.dInfo.Capabilities[CapDataSource] {
 			framework.Skipf("Driver %q does not support populate data from snapshot - skipping", input.dInfo.Name)
 		}
-		if input.dInfo.Name == "csi-hostpath-v0" {
-			framework.Skipf("Driver %q does not support populate data from snapshot - skipping", input.dInfo.Name)
-		}
 
 		input.testCase.SkipWriteReadCheck = true
 		dataSource, cleanupFunc := prepareDataSourceForProvisioning(input.testCase, input.cs, input.dc, input.pvc, input.sc, input.vsc)
 		defer cleanupFunc()
-		framework.Logf("volume snapshot dataSource %v", dataSource)
 
 		input.pvc.Spec.DataSource = dataSource
 		TestDynamicProvisioning(input.testCase, input.cs, input.pvc, input.sc)
@@ -233,6 +229,8 @@ func TestDynamicProvisioning(t StorageClassTest, client clientset.Interface, cla
 	if class != nil {
 		By("creating a StorageClass " + class.Name)
 		_, err = client.StorageV1().StorageClasses().Create(class)
+		// The "should provision storage with snapshot data source" test already has created the class.
+		// TODO: make class creation optional and remove the IsAlreadyExists exception
 		Expect(err == nil || apierrs.IsAlreadyExists(err)).To(Equal(true))
 		class, err = client.StorageV1().StorageClasses().Get(class.Name, metav1.GetOptions{})
 		Expect(err).NotTo(HaveOccurred())
@@ -297,10 +295,7 @@ func TestDynamicProvisioning(t StorageClassTest, client clientset.Interface, cla
 		Expect(err).NotTo(HaveOccurred())
 	}
 
-	By(fmt.Sprintf("xxxxxxx dataSource %v", claim.Spec.DataSource))
 	if claim.Spec.DataSource != nil {
-		framework.Logf("check datasource %q/%q", claim.Namespace, claim.Name)
-
 		By("checking the created volume whether has the pre-populated data")
 		command := fmt.Sprintf("grep '%s' /mnt/test/initialData", claim.Namespace)
 		runInPodWithVolume(client, claim.Namespace, claim.Name, t.NodeName, command, t.NodeSelector, t.ExpectUnschedulable)

test/e2e/storage/testsuites/snapshottable.go

@@ -162,8 +162,8 @@ func (s *snapshottableTestResource) setupResource(driver TestDriver, pattern tes
 			s.pvc.Spec.StorageClassName = &s.sc.Name
 			framework.Logf("In creating storage class object and pvc object for driver - sc: %v, pvc: %v", s.sc, s.pvc)
 
-			if dDriver, ok := driver.(SnapshottableTestDriver); ok {
-				s.vsc = dDriver.GetSnapshotClass()
+			if sDriver, ok := driver.(SnapshottableTestDriver); ok {
+				s.vsc = sDriver.GetSnapshotClass()
 			}
 		}
 
@@ -189,7 +189,7 @@ type snapshottableTestInput struct {
 func testSnapshot(input *snapshottableTestInput) {
 	It("should create snapshot with defaults", func() {
 		if input.dInfo.Name == "csi-hostpath-v0" {
-			framework.Skipf("Driver %q does not support create snapshot - skipping", input.dInfo.Name)
+			framework.Skipf("skip test when using driver csi-hostpath-v0 - skipping")
 		}
 		TestCreateSnapshot(input.testCase, input.cs, input.dc, input.pvc, input.sc, input.vsc)
 	})

test/e2e/testing-manifests/storage-csi/external-snapshotter/README.md

@@ -1 +1 @@
-The original file is (or will be) https://github.com/kubernetes-csi/external-snapshotter/blob/master/deploy/kubernetes/rbac.yaml
+The original file is https://github.com/kubernetes-csi/external-snapshotter/blob/master/deploy/kubernetes/rbac.yaml

test/e2e/testing-manifests/storage-csi/external-snapshotter/rbac.yaml

@@ -62,35 +62,4 @@ roleRef:
   kind: ClusterRole
   # change the name also here if the ClusterRole gets renamed
   name: external-snapshotter-runner
-  apiGroup: rbac.authorization.k8s.io
-
----
-# napshotter must be able to work with config map in currenSt namespace
-# if (and only if) leadership election is enabled
-kind: Role
-apiVersion: rbac.authorization.k8s.io/v1
-metadata:
-  # replace with non-default namespace name
-  namespace: default
-  name: external-snapshotter-cfg
-rules:
-- apiGroups: [""]
-  resources: ["configmaps"]
-  verbs: ["get", "watch", "list", "delete", "update", "create"]
-
----
-kind: RoleBinding
-apiVersion: rbac.authorization.k8s.io/v1
-metadata:
-  name: csi-snapshotter-role-cfg
-  # replace with non-default namespace name
-  namespace: default
-subjects:
-- kind: ServiceAccount
-  name: csi-snapshotter
-  # replace with non-default namespace name
-  namespace: default
-roleRef:
-  kind: Role
-  name: external-snapshotter-cfg
-  apiGroup: rbac.authorization.k8s.io
+  apiGroup: rbac.authorization.k8s.io