Merge pull request #99346 from mauriciopoppe/fix/storage-cloning-e2e

Avoid creation of the same storageclass in e2e tests

test/e2e/storage/regional_pd.go

@@ -160,6 +160,9 @@ func testVolumeProvisioning(c clientset.Interface, t *framework.TimeoutContext,
 			StorageClassName: &(test.Class.Name),
 			VolumeMode:       &test.VolumeMode,
 		}, ns)
+		_, clearStorageClass := testsuites.SetupStorageClass(test.Client, test.Class)
+		defer clearStorageClass()
+
 		test.TestDynamicProvisioning()
 	}
 }
@@ -388,6 +391,9 @@ func testRegionalAllowedTopologies(c clientset.Interface, ns string) {
 		VolumeMode:       &test.VolumeMode,
 	}, ns)
 
+	_, clearStorageClass := testsuites.SetupStorageClass(test.Client, test.Class)
+	defer clearStorageClass()
+
 	pv := test.TestDynamicProvisioning()
 	checkZonesFromLabelAndAffinity(pv, sets.NewString(zones...), true)
 }

test/e2e/storage/testsuites/provisioning.go

@@ -191,6 +191,9 @@ func (p *provisioningTestSuite) DefineTests(driver storageframework.TestDriver,
 		l.testCase.PvCheck = func(claim *v1.PersistentVolumeClaim) {
 			PVWriteReadSingleNodeCheck(l.cs, f.Timeouts, claim, l.config.ClientNodeSelection)
 		}
+		_, clearProvisionedStorageClass := SetupStorageClass(l.testCase.Client, l.testCase.Class)
+		defer clearProvisionedStorageClass()
+
 		l.testCase.TestDynamicProvisioning()
 	})
 
@@ -289,11 +292,8 @@ func (p *provisioningTestSuite) DefineTests(driver storageframework.TestDriver,
 				myTestConfig := testConfig
 				myTestConfig.Prefix = fmt.Sprintf("%s-%d", myTestConfig.Prefix, i)
 
-				// Each go routine must have its own testCase copy to store their claim
+				t := *l.testCase
-				myTestCase := *l.testCase
+				t.PvCheck = func(claim *v1.PersistentVolumeClaim) {
-				myTestCase.Claim = myTestCase.Claim.DeepCopy()
-				myTestCase.Class = nil // Do not create/delete the storage class in TestDynamicProvisioning, it already exists.
-				myTestCase.PvCheck = func(claim *v1.PersistentVolumeClaim) {
 					ginkgo.By(fmt.Sprintf("checking whether the created volume %d has the pre-populated data", i))
 					tests := []e2evolume.Test{
 						{
@@ -305,38 +305,70 @@ func (p *provisioningTestSuite) DefineTests(driver storageframework.TestDriver,
 					}
 					e2evolume.TestVolumeClientSlow(f, myTestConfig, nil, "", tests)
 				}
-				myTestCase.TestDynamicProvisioning()
+				t.TestDynamicProvisioning()
 			}(i)
 		}
 		wg.Wait()
 	})
 }
 
+// SetupStorageClass ensures that a StorageClass from a spec exists, if the StorageClass already exists
+// then it's returned as it is, if it doesn't exist then it's created first
+// and then returned, if the spec is nil then we return the `default` StorageClass
+func SetupStorageClass(
+	client clientset.Interface,
+	class *storagev1.StorageClass,
+) (*storagev1.StorageClass, func()) {
+	gomega.Expect(client).NotTo(gomega.BeNil(), "SetupStorageClass.client is required")
+
+	var err error
+	var computedStorageClass *storagev1.StorageClass
+	var clearComputedStorageClass = func() {}
+	if class != nil {
+		computedStorageClass, err = client.StorageV1().StorageClasses().Get(context.TODO(), class.Name, metav1.GetOptions{})
+		if err == nil {
+			// skip storageclass creation if it already exists
+			ginkgo.By("Storage class " + computedStorageClass.Name + " is already created, skipping creation.")
+		} else {
+			ginkgo.By("Creating a StorageClass " + class.Name)
+			_, err = client.StorageV1().StorageClasses().Create(context.TODO(), class, metav1.CreateOptions{})
+			framework.ExpectNoError(err)
+			computedStorageClass, err = client.StorageV1().StorageClasses().Get(context.TODO(), class.Name, metav1.GetOptions{})
+			framework.ExpectNoError(err)
+			clearComputedStorageClass = func() {
+				framework.Logf("deleting storage class %s", computedStorageClass.Name)
+				framework.ExpectNoError(client.StorageV1().StorageClasses().Delete(context.TODO(), computedStorageClass.Name, metav1.DeleteOptions{}))
+			}
+		}
+	} else {
+		// StorageClass is nil, so the default one will be used
+		scName, err := e2epv.GetDefaultStorageClassName(client)
+		framework.ExpectNoError(err)
+		ginkgo.By("Wanted storage class is nil, fetching default StorageClass=" + scName)
+		computedStorageClass, err = client.StorageV1().StorageClasses().Get(context.TODO(), scName, metav1.GetOptions{})
+		framework.ExpectNoError(err)
+	}
+
+	return computedStorageClass, clearComputedStorageClass
+}
+
 // TestDynamicProvisioning tests dynamic provisioning with specified StorageClassTest
+// it's assumed that the StorageClass `t.Class` is already provisioned,
+// see #ProvisionStorageClass
 func (t StorageClassTest) TestDynamicProvisioning() *v1.PersistentVolume {
+	var err error
+
 	client := t.Client
 	gomega.Expect(client).NotTo(gomega.BeNil(), "StorageClassTest.Client is required")
 	claim := t.Claim
 	gomega.Expect(claim).NotTo(gomega.BeNil(), "StorageClassTest.Claim is required")
+	gomega.Expect(claim.GenerateName).NotTo(gomega.BeEmpty(), "StorageClassTest.Claim.GenerateName must not be empty")
 	class := t.Class
+	gomega.Expect(class).NotTo(gomega.BeNil(), "StorageClassTest.Class is required")
+	class, err = client.StorageV1().StorageClasses().Get(context.TODO(), class.Name, metav1.GetOptions{})
+	framework.ExpectNoError(err, "StorageClass.Class "+class.Name+" couldn't be fetched from the cluster")
 
-	var err error
+	ginkgo.By(fmt.Sprintf("creating claim=%+v", claim))
-	if class != nil {
-		framework.ExpectEqual(*claim.Spec.StorageClassName, class.Name)
-		ginkgo.By("creating a StorageClass " + class.Name)
-		_, err = client.StorageV1().StorageClasses().Create(context.TODO(), class, metav1.CreateOptions{})
-		// The "should provision storage with snapshot data source" test already has created the class.
-		// TODO: make class creation optional and remove the IsAlreadyExists exception
-		framework.ExpectEqual(err == nil || apierrors.IsAlreadyExists(err), true)
-		class, err = client.StorageV1().StorageClasses().Get(context.TODO(), class.Name, metav1.GetOptions{})
-		framework.ExpectNoError(err)
-		defer func() {
-			framework.Logf("deleting storage class %s", class.Name)
-			framework.ExpectNoError(client.StorageV1().StorageClasses().Delete(context.TODO(), class.Name, metav1.DeleteOptions{}))
-		}()
-	}
-
-	ginkgo.By("creating a claim")
 	claim, err = client.CoreV1().PersistentVolumeClaims(claim.Namespace).Create(context.TODO(), claim, metav1.CreateOptions{})
 	framework.ExpectNoError(err)
 	defer func() {
@@ -348,21 +380,22 @@ func (t StorageClassTest) TestDynamicProvisioning() *v1.PersistentVolume {
 		}
 	}()
 
-	if class == nil {
+	// ensure that the claim refers to the provisioned StorageClass
-		// StorageClass is nil, so the default one will be used
+	framework.ExpectEqual(*claim.Spec.StorageClassName, class.Name)
-		scName, err := e2epv.GetDefaultStorageClassName(client)
+
-		framework.ExpectNoError(err)
+	// if late binding is configured, create and delete a pod to provision the volume
-		defaultSC, err := client.StorageV1().StorageClasses().Get(context.TODO(), scName, metav1.GetOptions{})
+	if *class.VolumeBindingMode == storagev1.VolumeBindingWaitForFirstConsumer {
-		framework.ExpectNoError(err)
+		ginkgo.By(fmt.Sprintf("creating a pod referring to the class=%+v claim=%+v", class, claim))
-		// If late binding is configured, create and delete a pod to provision the volume
+		var podConfig *e2epod.Config = &e2epod.Config{
-		if *defaultSC.VolumeBindingMode == storagev1.VolumeBindingWaitForFirstConsumer {
+			NS:   claim.Namespace,
-			ginkgo.By("creating a pod referring to the claim")
+			PVCs: []*v1.PersistentVolumeClaim{claim},
-			var pod *v1.Pod
-			pod, err := e2epod.CreatePod(t.Client, claim.Namespace, nil /* nodeSelector */, []*v1.PersistentVolumeClaim{claim}, true /* isPrivileged */, "" /* command */)
-			// Delete pod now, otherwise PV can't be deleted below
-			framework.ExpectNoError(err)
-			e2epod.DeletePodOrFail(t.Client, pod.Namespace, pod.Name)
 		}
+
+		var pod *v1.Pod
+		pod, err := e2epod.CreateSecPod(client, podConfig, framework.PodStartTimeout)
+		// Delete pod now, otherwise PV can't be deleted below
+		framework.ExpectNoError(err)
+		e2epod.DeletePodOrFail(client, pod.Namespace, pod.Name)
 	}
 
 	// Run the checker
@@ -795,12 +828,7 @@ func prepareSnapshotDataSourceForProvisioning(
 	mode v1.PersistentVolumeMode,
 	injectContent string,
 ) (*v1.TypedLocalObjectReference, func()) {
-	var err error
+	_, clearComputedStorageClass := SetupStorageClass(client, class)
-	if class != nil {
-		ginkgo.By("[Initialize dataSource]creating a StorageClass " + class.Name)
-		_, err = client.StorageV1().StorageClasses().Create(context.TODO(), class, metav1.CreateOptions{})
-		framework.ExpectNoError(err)
-	}
 
 	ginkgo.By("[Initialize dataSource]creating a initClaim")
 	updatedClaim, err := client.CoreV1().PersistentVolumeClaims(initClaim.Namespace).Create(context.TODO(), initClaim, metav1.CreateOptions{})
@@ -837,11 +865,8 @@ func prepareSnapshotDataSourceForProvisioning(
 		err = snapshotResource.CleanupResource(f.Timeouts)
 		framework.ExpectNoError(err)
 
-		ginkgo.By("deleting StorageClass " + class.Name)
+		clearComputedStorageClass()
-		err = client.StorageV1().StorageClasses().Delete(context.TODO(), class.GetName(), metav1.DeleteOptions{})
+
-		if err != nil && !apierrors.IsNotFound(err) {
-			framework.Failf("Error deleting storage class %q. Error: %v", class.GetName(), err)
-		}
 	}
 
 	return dataSourceRef, cleanupFunc
@@ -856,12 +881,7 @@ func preparePVCDataSourceForProvisioning(
 	mode v1.PersistentVolumeMode,
 	injectContent string,
 ) (*v1.TypedLocalObjectReference, func()) {
-	var err error
+	_, clearComputedStorageClass := SetupStorageClass(client, class)
-	if class != nil {
-		ginkgo.By("[Initialize dataSource]creating a StorageClass " + class.Name)
-		class, err = client.StorageV1().StorageClasses().Create(context.TODO(), class, metav1.CreateOptions{})
-		framework.ExpectNoError(err)
-	}
 
 	ginkgo.By("[Initialize dataSource]creating a source PVC")
 	sourcePVC, err := client.CoreV1().PersistentVolumeClaims(source.Namespace).Create(context.TODO(), source, metav1.CreateOptions{})
@@ -888,13 +908,8 @@ func preparePVCDataSourceForProvisioning(
 		if err != nil && !apierrors.IsNotFound(err) {
 			framework.Failf("Error deleting source PVC %q. Error: %v", sourcePVC.Name, err)
 		}
-		if class != nil {
+
-			framework.Logf("deleting class %q", class.Name)
+		clearComputedStorageClass()
-			err := client.StorageV1().StorageClasses().Delete(context.TODO(), class.Name, metav1.DeleteOptions{})
-			if err != nil && !apierrors.IsNotFound(err) {
-				framework.Failf("Error deleting storage class %q. Error: %v", class.Name, err)
-			}
-		}
 	}
 
 	return dataSourceRef, cleanupFunc

test/e2e/storage/volume_provisioning.go

@@ -344,7 +344,7 @@ var _ = utils.SIGDescribe("Dynamic Provisioning", func() {
 
 			var betaTest *testsuites.StorageClassTest
 			for i, t := range tests {
-				// Beware of clojure, use local variables instead of those from
+				// Beware of closure, use local variables instead of those from
 				// outer scope
 				test := t
 
@@ -365,26 +365,37 @@ var _ = utils.SIGDescribe("Dynamic Provisioning", func() {
 					StorageClassName: &test.Class.Name,
 					VolumeMode:       &test.VolumeMode,
 				}, ns)
+
+				// overwrite StorageClass spec with provisioned StorageClass
+				class, clearStorageClass := testsuites.SetupStorageClass(test.Client, test.Class)
+				test.Class = class
+				defer clearStorageClass()
+
 				test.TestDynamicProvisioning()
 			}
 
 			// Run the last test with storage.k8s.io/v1beta1 on pvc
 			if betaTest != nil {
 				ginkgo.By("Testing " + betaTest.Name + " with beta volume provisioning")
-				class := newBetaStorageClass(*betaTest, "beta")
+				betaClass := newBetaStorageClass(*betaTest, "beta")
-				// we need to create the class manually, testDynamicProvisioning does not accept beta class
+				// create beta class manually
-				class, err := c.StorageV1beta1().StorageClasses().Create(context.TODO(), class, metav1.CreateOptions{})
+				betaClass, err := c.StorageV1beta1().StorageClasses().Create(context.TODO(), betaClass, metav1.CreateOptions{})
+				framework.ExpectNoError(err)
+				defer deleteStorageClass(c, betaClass.Name)
+
+				// fetch V1beta1 StorageClass as V1 object for the test
+				class, err := c.StorageV1().StorageClasses().Get(context.TODO(), betaClass.Name, metav1.GetOptions{})
 				framework.ExpectNoError(err)
-				defer deleteStorageClass(c, class.Name)
 
 				betaTest.Client = c
-				betaTest.Class = nil
+				betaTest.Class = class
 				betaTest.Claim = e2epv.MakePersistentVolumeClaim(e2epv.PersistentVolumeClaimConfig{
 					ClaimSize:        betaTest.ClaimSize,
 					StorageClassName: &class.Name,
 					VolumeMode:       &betaTest.VolumeMode,
 				}, ns)
 				betaTest.Claim.Spec.StorageClassName = &(class.Name)
+
 				(*betaTest).TestDynamicProvisioning()
 			}
 		})
@@ -419,6 +430,9 @@ var _ = utils.SIGDescribe("Dynamic Provisioning", func() {
 				StorageClassName: &test.Class.Name,
 				VolumeMode:       &test.VolumeMode,
 			}, ns)
+			_, clearStorageClass := testsuites.SetupStorageClass(test.Client, test.Class)
+			defer clearStorageClass()
+
 			pv := test.TestDynamicProvisioning()
 
 			ginkgo.By(fmt.Sprintf("waiting for the provisioned PV %q to enter phase %s", pv.Name, v1.VolumeReleased))
@@ -663,7 +677,13 @@ var _ = utils.SIGDescribe("Dynamic Provisioning", func() {
 				VolumeMode:       &test.VolumeMode,
 			}, ns)
 
+			// rewrite the storageClass with the computed storageClass
+			storageClass, clearStorageClass := testsuites.SetupStorageClass(test.Client, test.Class)
+			defer clearStorageClass()
+			test.Class = storageClass
+
 			ginkgo.By("creating a claim with a external provisioning annotation")
+
 			test.TestDynamicProvisioning()
 		})
 	})
@@ -685,6 +705,11 @@ var _ = utils.SIGDescribe("Dynamic Provisioning", func() {
 				ClaimSize:  test.ClaimSize,
 				VolumeMode: &test.VolumeMode,
 			}, ns)
+			// NOTE: this test assumes that there's a default storageclass
+			storageClass, clearStorageClass := testsuites.SetupStorageClass(test.Client, nil)
+			test.Class = storageClass
+			defer clearStorageClass()
+
 			test.TestDynamicProvisioning()
 		})
 
@@ -791,6 +816,8 @@ var _ = utils.SIGDescribe("Dynamic Provisioning", func() {
 				VolumeMode:       &test.VolumeMode,
 			}, ns)
 
+			_, clearStorageClass := testsuites.SetupStorageClass(test.Client, test.Class)
+			defer clearStorageClass()
 			test.TestDynamicProvisioning()
 		})
 	})