Merge pull request #73677 from jarrpa/csi-cluster-registrar

e2e: Use cluster-driver-registrar for CSIDriverRegistry tests

test/e2e/storage/BUILD

@@ -64,7 +64,6 @@ go_library(
         "//staging/src/k8s.io/client-go/kubernetes:go_default_library",
         "//staging/src/k8s.io/client-go/kubernetes/typed/core/v1:go_default_library",
         "//staging/src/k8s.io/cloud-provider/volume/helpers:go_default_library",
-        "//staging/src/k8s.io/csi-api/pkg/apis/csi/v1alpha1:go_default_library",
         "//staging/src/k8s.io/csi-api/pkg/client/clientset/versioned:go_default_library",
         "//test/e2e/framework:go_default_library",
         "//test/e2e/framework/metrics:go_default_library",

test/e2e/storage/csi_volumes.go

@@ -22,6 +22,7 @@ import (
 	"fmt"
 	"regexp"
 	"strings"
+	"time"
 
 	"k8s.io/api/core/v1"
 	storagev1 "k8s.io/api/storage/v1"
@@ -29,7 +30,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
 	clientset "k8s.io/client-go/kubernetes"
-	csiv1alpha1 "k8s.io/csi-api/pkg/apis/csi/v1alpha1"
 	csiclient "k8s.io/csi-api/pkg/client/clientset/versioned"
 	"k8s.io/kubernetes/test/e2e/framework"
 	"k8s.io/kubernetes/test/e2e/framework/podlogs"
@@ -196,57 +196,49 @@ var _ = utils.SIGDescribe("CSI Volumes", func() {
 	})
 
 	// The CSIDriverRegistry feature gate is needed for this test in Kubernetes 1.12.
-	Context("CSI attach test using HostPath driver [Feature:CSIDriverRegistry]", func() {
+
+	Context("CSI attach test using mock driver [Feature:CSIDriverRegistry]", func() {
 		var (
 			driver testsuites.TestDriver
 		)
 
-		BeforeEach(func() {
-			config := testsuites.TestConfig{
-				Framework: f,
-				Prefix:    "csi-attach",
-			}
-			driver = drivers.InitHostPathCSIDriver(config)
-			driver.CreateDriver()
-		})
-
-		AfterEach(func() {
-			driver.CleanupDriver()
-		})
-
 		tests := []struct {
 			name             string
 			driverAttachable bool
 			driverExists     bool
-			expectVolumeAttachment bool
 		}{
 			{
-				name:                   "non-attachable volume does not need VolumeAttachment",
+				name:             "should not require VolumeAttach for drivers without attachment",
 				driverAttachable: false,
 				driverExists:     true,
-				expectVolumeAttachment: false,
 			},
 			{
-				name:                   "attachable volume needs VolumeAttachment",
+				name:             "should require VolumeAttach for drivers with attachment",
 				driverAttachable: true,
 				driverExists:     true,
-				expectVolumeAttachment: true,
 			},
 			{
-				name:                   "volume with no CSI driver needs VolumeAttachment",
+				name:             "should preserve attachment policy when no CSIDriver present",
+				driverAttachable: true,
 				driverExists:     false,
-				expectVolumeAttachment: true,
 			},
 		}
 
 		for _, t := range tests {
 			test := t
 			It(test.name, func() {
-				if test.driverExists {
+				By("Deploying mock CSI driver")
-					csiDriver := createCSIDriver(csics, testsuites.GetUniqueDriverName(driver), test.driverAttachable, nil)
+				config := testsuites.TestConfig{
-					if csiDriver != nil {
+					Framework: f,
-						defer csics.CsiV1alpha1().CSIDrivers().Delete(csiDriver.Name, nil)
+					Prefix:    "csi-attach",
 				}
+
+				driver = drivers.InitMockCSIDriver(config, test.driverExists, test.driverAttachable, nil)
+				driver.CreateDriver()
+
+				if test.driverExists {
+					defer destroyCSIDriver(csics, driver)
+					defer driver.CleanupDriver()
 				}
 
 				By("Creating pod")
@@ -270,7 +262,8 @@ var _ = utils.SIGDescribe("CSI Volumes", func() {
 					defer cs.StorageV1().StorageClasses().Delete(class.Name, nil)
 				}
 				if claim != nil {
-					defer cs.CoreV1().PersistentVolumeClaims(ns.Name).Delete(claim.Name, nil)
+					// Fully delete PV before deleting CSI driver
+					defer deleteVolume(cs, claim)
 				}
 				if pod != nil {
 					// Fully delete (=unmount) the pod before deleting CSI driver
@@ -284,28 +277,27 @@ var _ = utils.SIGDescribe("CSI Volumes", func() {
 				framework.ExpectNoError(err, "Failed to start pod: %v", err)
 
 				By("Checking if VolumeAttachment was created for the pod")
-				// Check that VolumeAttachment does not exist
 				handle := getVolumeHandle(cs, claim)
 				attachmentHash := sha256.Sum256([]byte(fmt.Sprintf("%s%s%s", handle, scTest.Provisioner, nodeName)))
 				attachmentName := fmt.Sprintf("csi-%x", attachmentHash)
 				_, err = cs.StorageV1beta1().VolumeAttachments().Get(attachmentName, metav1.GetOptions{})
 				if err != nil {
 					if errors.IsNotFound(err) {
-						if test.expectVolumeAttachment {
+						if test.driverAttachable {
 							framework.ExpectNoError(err, "Expected VolumeAttachment but none was found")
 						}
 					} else {
 						framework.ExpectNoError(err, "Failed to find VolumeAttachment")
 					}
 				}
-				if !test.expectVolumeAttachment {
+				if !test.driverAttachable {
 					Expect(err).To(HaveOccurred(), "Unexpected VolumeAttachment found")
 				}
 			})
 		}
 	})
 
-	Context("CSI workload information [Feature:CSIDriverRegistry]", func() {
+	Context("CSI workload information using mock driver [Feature:CSIDriverRegistry]", func() {
 		var (
 			driver         testsuites.TestDriver
 			podInfoV1      = "v1"
@@ -313,19 +305,6 @@ var _ = utils.SIGDescribe("CSI Volumes", func() {
 			podInfoEmpty   = ""
 		)
 
-		BeforeEach(func() {
-			config := testsuites.TestConfig{
-				Framework: f,
-				Prefix:    "csi-workload",
-			}
-			driver = drivers.InitMockCSIDriver(config)
-			driver.CreateDriver()
-		})
-
-		AfterEach(func() {
-			driver.CleanupDriver()
-		})
-
 		tests := []struct {
 			name                  string
 			podInfoOnMountVersion *string
@@ -365,11 +344,18 @@ var _ = utils.SIGDescribe("CSI Volumes", func() {
 		for _, t := range tests {
 			test := t
 			It(test.name, func() {
-				if test.driverExists {
+				By("Deploying mock CSI driver")
-					csiDriver := createCSIDriver(csics, testsuites.GetUniqueDriverName(driver), true, test.podInfoOnMountVersion)
+				config := testsuites.TestConfig{
-					if csiDriver != nil {
+					Framework: f,
-						defer csics.CsiV1alpha1().CSIDrivers().Delete(csiDriver.Name, nil)
+					Prefix:    "csi-workload",
 				}
+
+				driver = drivers.InitMockCSIDriver(config, test.driverExists, true, test.podInfoOnMountVersion)
+				driver.CreateDriver()
+
+				if test.driverExists {
+					defer destroyCSIDriver(csics, driver)
+					defer driver.CleanupDriver()
 				}
 
 				By("Creating pod")
@@ -397,7 +383,8 @@ var _ = utils.SIGDescribe("CSI Volumes", func() {
 					defer cs.StorageV1().StorageClasses().Delete(class.Name, nil)
 				}
 				if claim != nil {
-					defer cs.CoreV1().PersistentVolumeClaims(ns.Name).Delete(claim.Name, nil)
+					// Fully delete PV before deleting CSI driver
+					defer deleteVolume(cs, claim)
 				}
 				if pod != nil {
 					// Fully delete (=unmount) the pod before deleting CSI driver
@@ -472,20 +459,15 @@ func testTopologyNegative(cs clientset.Interface, suffix, namespace string, dela
 	}
 }
 
-func createCSIDriver(csics csiclient.Interface, name string, attachable bool, podInfoOnMountVersion *string) *csiv1alpha1.CSIDriver {
+func destroyCSIDriver(csics csiclient.Interface, driver testsuites.TestDriver) {
-	By("Creating CSIDriver instance")
+	driverName := testsuites.GetUniqueDriverName(driver)
-	driver := &csiv1alpha1.CSIDriver{
+	driverGet, err := csics.CsiV1alpha1().CSIDrivers().Get(driverName, metav1.GetOptions{})
-		ObjectMeta: metav1.ObjectMeta{
+	if err == nil {
-			Name: name,
+		framework.Logf("deleting %s.%s: %s", driverGet.TypeMeta.APIVersion, driverGet.TypeMeta.Kind, driverGet.ObjectMeta.Name)
-		},
+		// Uncomment the following line to get full dump of CSIDriver object
-		Spec: csiv1alpha1.CSIDriverSpec{
+		// framework.Logf("%s", framework.PrettyPrint(driverGet))
-			AttachRequired:        &attachable,
+		csics.CsiV1alpha1().CSIDrivers().Delete(driverName, nil)
-			PodInfoOnMountVersion: podInfoOnMountVersion,
-		},
 	}
-	driver, err := csics.CsiV1alpha1().CSIDrivers().Create(driver)
-	framework.ExpectNoError(err, "Failed to create CSIDriver: %v", err)
-	return driver
 }
 
 func getVolumeHandle(cs clientset.Interface, claim *v1.PersistentVolumeClaim) string {
@@ -508,6 +490,15 @@ func getVolumeHandle(cs clientset.Interface, claim *v1.PersistentVolumeClaim) st
 	return pv.Spec.CSI.VolumeHandle
 }
 
+func deleteVolume(cs clientset.Interface, claim *v1.PersistentVolumeClaim) {
+	// re-get the claim to the latest state with bound volume
+	claim, err := cs.CoreV1().PersistentVolumeClaims(claim.Namespace).Get(claim.Name, metav1.GetOptions{})
+	if err == nil {
+		cs.CoreV1().PersistentVolumeClaims(claim.Namespace).Delete(claim.Name, nil)
+		framework.WaitForPersistentVolumeDeleted(cs, claim.Spec.VolumeName, 2*time.Second, 2*time.Minute)
+	}
+}
+
 func startPausePod(cs clientset.Interface, t testsuites.StorageClassTest, node testsuites.NodeSelection, ns string) (*storagev1.StorageClass, *v1.PersistentVolumeClaim, *v1.Pod) {
 	class := newStorageClass(t, ns, "")
 	class, err := cs.StorageV1().StorageClasses().Create(class)

test/e2e/storage/drivers/csi.go

@@ -168,13 +168,37 @@ func (h *hostpathCSIDriver) CleanupDriver() {
 type mockCSIDriver struct {
 	cleanup        func()
 	driverInfo     testsuites.DriverInfo
+	manifests      []string
+	podInfoVersion *string
 }
 
 var _ testsuites.TestDriver = &mockCSIDriver{}
 var _ testsuites.DynamicPVTestDriver = &mockCSIDriver{}
 
 // InitMockCSIDriver returns a mockCSIDriver that implements TestDriver interface
-func InitMockCSIDriver(config testsuites.TestConfig) testsuites.TestDriver {
+func InitMockCSIDriver(config testsuites.TestConfig, registerDriver, driverAttachable bool, podInfoVersion *string) testsuites.TestDriver {
+	driverManifests := []string{
+		"test/e2e/testing-manifests/storage-csi/cluster-driver-registrar/rbac.yaml",
+		"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/testing-manifests/storage-csi/mock/csi-mock-rbac.yaml",
+		"test/e2e/testing-manifests/storage-csi/mock/csi-storageclass.yaml",
+		"test/e2e/testing-manifests/storage-csi/mock/csi-mock-driver.yaml",
+	}
+
+	config.ServerConfig = &framework.VolumeTestConfig{}
+
+	if registerDriver {
+		driverManifests = append(driverManifests, "test/e2e/testing-manifests/storage-csi/mock/csi-mock-cluster-driver-registrar.yaml")
+	}
+
+	if driverAttachable {
+		driverManifests = append(driverManifests, "test/e2e/testing-manifests/storage-csi/mock/csi-mock-driver-attacher.yaml")
+	} else {
+		config.ServerConfig.ServerArgs = append(config.ServerConfig.ServerArgs, "--disable-attach")
+	}
+
 	return &mockCSIDriver{
 		driverInfo: testsuites.DriverInfo{
 			Name:        "csi-mock",
@@ -190,6 +214,8 @@ func InitMockCSIDriver(config testsuites.TestConfig) testsuites.TestDriver {
 			},
 			Config: config,
 		},
+		manifests:      driverManifests,
+		podInfoVersion: podInfoVersion,
 	}
 }
 
@@ -223,26 +249,28 @@ func (m *mockCSIDriver) CreateDriver() {
 	node := nodes.Items[rand.Intn(len(nodes.Items))]
 	m.driverInfo.Config.ClientNodeName = node.Name
 
+	containerArgs := []string{"--name=csi-mock-" + f.UniqueName}
+
+	if m.driverInfo.Config.ServerConfig != nil && m.driverInfo.Config.ServerConfig.ServerArgs != nil {
+		containerArgs = append(containerArgs, m.driverInfo.Config.ServerConfig.ServerArgs...)
+	}
+
 	// TODO (?): the storage.csi.image.version and storage.csi.image.registry
 	// settings are ignored for this test. We could patch the image definitions.
 	o := utils.PatchCSIOptions{
 		OldDriverName:                 "csi-mock",
 		NewDriverName:                 "csi-mock-" + f.UniqueName,
 		DriverContainerName:           "mock",
-		DriverContainerArguments: []string{"--name=csi-mock-" + f.UniqueName},
+		DriverContainerArguments:      containerArgs,
 		ProvisionerContainerName:      "csi-provisioner",
+		ClusterRegistrarContainerName: "csi-cluster-driver-registrar",
 		NodeName:                      m.driverInfo.Config.ClientNodeName,
+		PodInfoVersion:                m.podInfoVersion,
 	}
 	cleanup, err := f.CreateFromManifests(func(item interface{}) error {
 		return utils.PatchCSIDeployment(f, o, item)
 	},
-		"test/e2e/testing-manifests/storage-csi/driver-registrar/rbac.yaml",
+		m.manifests...)
-		"test/e2e/testing-manifests/storage-csi/external-attacher/rbac.yaml",
-		"test/e2e/testing-manifests/storage-csi/external-provisioner/rbac.yaml",
-		"test/e2e/testing-manifests/storage-csi/mock/csi-mock-rbac.yaml",
-		"test/e2e/testing-manifests/storage-csi/mock/csi-storageclass.yaml",
-		"test/e2e/testing-manifests/storage-csi/mock/csi-mock-driver.yaml",
-	)
 	m.cleanup = cleanup
 	if err != nil {
 		framework.Failf("deploying csi mock driver: %v", err)

test/e2e/storage/utils/deployment.go

@@ -97,6 +97,10 @@ func PatchCSIDeployment(f *framework.Framework, o PatchCSIOptions, object interf
 				// Driver name is expected to be the same
 				// as the snapshotter here.
 				container.Args = append(container.Args, "--snapshotter="+o.NewDriverName)
+			case o.ClusterRegistrarContainerName:
+				if o.PodInfoVersion != nil {
+					container.Args = append(container.Args, "--pod-info-mount-version="+*o.PodInfoVersion)
+				}
 			}
 		}
 	}
@@ -153,6 +157,12 @@ type PatchCSIOptions struct {
 	// If non-empty, --snapshotter with new name will be appended
 	// to the argument list.
 	SnapshotterContainerName string
+	// The name of the container which has the cluster-driver-registrar
+	// binary.
+	ClusterRegistrarContainerName string
 	// If non-empty, all pods are forced to run on this node.
 	NodeName string
+	// If not nil, the argument to pass to the cluster-driver-registrar's
+	// pod-info-mount-version argument.
+	PodInfoVersion *string
 }

test/e2e/testing-manifests/storage-csi/cluster-driver-registrar/README.md

@@ -0,0 +1 @@
+The original file is (or will be) https://github.com/kubernetes-csi/cluster-driver-registrar/blob/master/deploy/kubernetes/rbac.yaml

test/e2e/testing-manifests/storage-csi/cluster-driver-registrar/rbac.yaml

@@ -0,0 +1,38 @@
+# This YAML file contains all RBAC objects that are necessary to run
+# cluster-driver-registrar.
+#
+# In production, each CSI driver deployment has to be customized:
+# - to avoid conflicts, use non-default namespace and different names
+#   for non-namespaced entities like the ClusterRole
+
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: csi-cluster-driver-registrar
+  # replace with non-default namespace name
+  namespace: default
+
+---
+kind: ClusterRole
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: cluster-driver-registrar-runner
+rules:
+  - apiGroups: ["csi.storage.k8s.io"]
+    resources: ["csidrivers"]
+    verbs: ["create", "delete"]
+
+---
+kind: ClusterRoleBinding
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: csi-cluster-driver-registrar-role
+subjects:
+  - kind: ServiceAccount
+    name: csi-cluster-driver-registrar
+    # replace with non-default namespace name
+    namespace: default
+roleRef:
+  kind: ClusterRole
+  name: cluster-driver-registrar-runner
+  apiGroup: rbac.authorization.k8s.io

test/e2e/testing-manifests/storage-csi/mock/csi-mock-cluster-driver-registrar.yaml

@@ -0,0 +1,33 @@
+kind: StatefulSet
+apiVersion: apps/v1
+metadata:
+  name: csi-mockplugin-cluster-driver-registrar
+spec:
+  selector:
+    matchLabels:
+      app: csi-mockplugin-cluster-driver-registrar
+  replicas: 1
+  template:
+    metadata:
+      labels:
+        app: csi-mockplugin-cluster-driver-registrar
+    spec:
+      serviceAccountName: csi-mock
+      containers:
+        - name: csi-cluster-driver-registrar
+          image: quay.io/k8scsi/csi-cluster-driver-registrar:canary
+          args:
+            - --v=5
+            - --csi-address=$(ADDRESS)
+          env:
+            - name: ADDRESS
+              value: /csi/csi.sock
+          imagePullPolicy: Always
+          volumeMounts:
+          - mountPath: /csi
+            name: socket-dir
+      volumes:
+        - hostPath:
+            path: /var/lib/kubelet/plugins/csi-mock
+            type: DirectoryOrCreate
+          name: socket-dir

test/e2e/testing-manifests/storage-csi/mock/csi-mock-driver-attacher.yaml

@@ -0,0 +1,34 @@
+kind: StatefulSet
+apiVersion: apps/v1
+metadata:
+  name: csi-mockplugin-attacher
+spec:
+  selector:
+    matchLabels:
+      app: csi-mockplugin-attacher
+  replicas: 1
+  template:
+    metadata:
+      labels:
+        app: csi-mockplugin-attacher
+    spec:
+      serviceAccountName: csi-mock
+      containers:
+        - name: csi-attacher
+          image: quay.io/k8scsi/csi-attacher:v1.0.1
+          args:
+            - --v=5
+            - --csi-address=$(ADDRESS)
+          env:
+            - name: ADDRESS
+              value: /csi/csi.sock
+          imagePullPolicy: Always
+          volumeMounts:
+          - mountPath: /csi
+            name: socket-dir
+      volumes:
+        - hostPath:
+            path: /var/lib/kubelet/plugins/csi-mock
+            type: DirectoryOrCreate
+          name: socket-dir
+

test/e2e/testing-manifests/storage-csi/mock/csi-mock-driver.yaml

@@ -16,18 +16,6 @@ spec:
       containers:
 
 
-        - name: csi-attacher
-          image: quay.io/k8scsi/csi-attacher:v1.0.1
-          args:
-            - --v=5
-            - --csi-address=$(ADDRESS)
-          env:
-            - name: ADDRESS
-              value: /csi/csi.sock
-          imagePullPolicy: Always
-          volumeMounts:
-          - mountPath: /csi
-            name: socket-dir
         - name: csi-provisioner
           image: quay.io/k8scsi/csi-provisioner:v1.0.1
           args:

test/e2e/testing-manifests/storage-csi/mock/csi-mock-rbac.yaml

@@ -31,6 +31,20 @@ roleRef:
   name: external-provisioner-runner
   apiGroup: rbac.authorization.k8s.io
 
+---
+kind: ClusterRoleBinding
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: csi-controller-cluster-driver-registrar-role
+subjects:
+  - kind: ServiceAccount
+    name: csi-mock
+    namespace: default
+roleRef:
+  kind: ClusterRole
+  name: cluster-driver-registrar-runner
+  apiGroup: rbac.authorization.k8s.io
+
 ---
 # priviledged Pod Security Policy, previously defined via PrivilegedTestPSPClusterRoleBinding()
 kind: ClusterRoleBinding