Merge pull request #77605 from pohly/multivolume-storage-class-fix

test/e2e/framework/BUILD

@@ -90,7 +90,6 @@ go_library(
         "//staging/src/k8s.io/apimachinery/pkg/util/yaml:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/watch:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/authentication/serviceaccount:go_default_library",
-        "//staging/src/k8s.io/apiserver/pkg/storage/names:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
         "//staging/src/k8s.io/client-go/discovery:go_default_library",
         "//staging/src/k8s.io/client-go/discovery/cached/memory:go_default_library",

test/e2e/framework/create.go

@@ -31,7 +31,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
-	"k8s.io/apiserver/pkg/storage/names"
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/kubernetes/test/e2e/framework/testfiles"
@@ -101,9 +100,9 @@ func visitManifests(cb func([]byte) error, files ...string) error {
 	return nil
 }
 
-// PatchItems modifies the given items in place such that each
-// test gets its own instances, to avoid conflicts between different tests and
-// between tests and normal deployments.
+// PatchItems modifies the given items in place such that each test
+// gets its own instances, to avoid conflicts between different tests
+// and between tests and normal deployments.
 //
 // This is done by:
 // - creating namespaced items inside the test's namespace
@@ -288,27 +287,18 @@ var factories = map[What]ItemFactory{
 	{"StorageClass"}:       &storageClassFactory{},
 }
 
-// uniquifyName makes the name of some item unique per namespace by appending the
-// generated unique name of the test namespace.
-func (f *Framework) uniquifyName(item *string) {
+// PatchName makes the name of some item unique by appending the
+// generated unique name.
+func (f *Framework) PatchName(item *string) {
 	if *item != "" {
 		*item = *item + "-" + f.UniqueName
 	}
 }
 
-// randomizeStorageClassName makes the name of the storage class unique per call
-// by appending the generated unique name of the test namespace and a random 5
-// character string
-func (f *Framework) randomizeStorageClassName(item *string) {
-	if *item != "" {
-		*item = names.SimpleNameGenerator.GenerateName(*item + "-" + f.UniqueName + "-")
-	}
-}
-
-// patchNamespace moves the item into the test's namespace.  Not
+// PatchNamespace moves the item into the test's namespace.  Not
 // all items can be namespaced. For those, the name also needs to be
 // patched.
-func (f *Framework) patchNamespace(item *string) {
+func (f *Framework) PatchNamespace(item *string) {
 	if f.Namespace != nil {
 		*item = f.Namespace.GetName()
 	}
@@ -317,31 +307,31 @@ func (f *Framework) patchNamespace(item *string) {
 func (f *Framework) patchItemRecursively(item interface{}) error {
 	switch item := item.(type) {
 	case *rbac.Subject:
-		f.patchNamespace(&item.Namespace)
+		f.PatchNamespace(&item.Namespace)
 	case *rbac.RoleRef:
 		// TODO: avoid hard-coding this special name. Perhaps add a Framework.PredefinedRoles
 		// which contains all role names that are defined cluster-wide before the test starts?
 		// All those names are excempt from renaming. That list could be populated by querying
 		// and get extended by tests.
 		if item.Name != "e2e-test-privileged-psp" {
-			f.uniquifyName(&item.Name)
+			f.PatchName(&item.Name)
 		}
 	case *rbac.ClusterRole:
-		f.uniquifyName(&item.Name)
+		f.PatchName(&item.Name)
 	case *rbac.Role:
-		f.patchNamespace(&item.Namespace)
+		f.PatchNamespace(&item.Namespace)
 		// Roles are namespaced, but because for RoleRef above we don't
 		// know whether the referenced role is a ClusterRole or Role
 		// and therefore always renames, we have to do the same here.
-		f.uniquifyName(&item.Name)
+		f.PatchName(&item.Name)
 	case *storage.StorageClass:
-		f.randomizeStorageClassName(&item.Name)
+		f.PatchName(&item.Name)
 	case *v1.ServiceAccount:
-		f.patchNamespace(&item.ObjectMeta.Namespace)
+		f.PatchNamespace(&item.ObjectMeta.Namespace)
 	case *v1.Secret:
-		f.patchNamespace(&item.ObjectMeta.Namespace)
+		f.PatchNamespace(&item.ObjectMeta.Namespace)
 	case *rbac.ClusterRoleBinding:
-		f.uniquifyName(&item.Name)
+		f.PatchName(&item.Name)
 		for i := range item.Subjects {
 			if err := f.patchItemRecursively(&item.Subjects[i]); err != nil {
 				return errors.Wrapf(err, "%T", f)
@@ -351,7 +341,7 @@ func (f *Framework) patchItemRecursively(item interface{}) error {
 			return errors.Wrapf(err, "%T", f)
 		}
 	case *rbac.RoleBinding:
-		f.patchNamespace(&item.Namespace)
+		f.PatchNamespace(&item.Namespace)
 		for i := range item.Subjects {
 			if err := f.patchItemRecursively(&item.Subjects[i]); err != nil {
 				return errors.Wrapf(err, "%T", f)
@@ -361,11 +351,11 @@ func (f *Framework) patchItemRecursively(item interface{}) error {
 			return errors.Wrapf(err, "%T", f)
 		}
 	case *v1.Service:
-		f.patchNamespace(&item.ObjectMeta.Namespace)
+		f.PatchNamespace(&item.ObjectMeta.Namespace)
 	case *apps.StatefulSet:
-		f.patchNamespace(&item.ObjectMeta.Namespace)
+		f.PatchNamespace(&item.ObjectMeta.Namespace)
 	case *apps.DaemonSet:
-		f.patchNamespace(&item.ObjectMeta.Namespace)
+		f.PatchNamespace(&item.ObjectMeta.Namespace)
 	default:
 		return errors.Errorf("missing support for patching item of type %T", item)
 	}

test/e2e/storage/external/BUILD

@@ -11,6 +11,7 @@ go_library(
         "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
+        "//staging/src/k8s.io/apiserver/pkg/storage/names:go_default_library",
         "//staging/src/k8s.io/client-go/kubernetes/scheme:go_default_library",
         "//test/e2e/framework:go_default_library",
         "//test/e2e/storage/testpatterns:go_default_library",

test/e2e/storage/external/external.go

@@ -27,6 +27,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/sets"
+	"k8s.io/apiserver/pkg/storage/names"
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/kubernetes/test/e2e/framework"
 	"k8s.io/kubernetes/test/e2e/storage/testpatterns"
@@ -242,6 +243,9 @@ func (d *driverDefinition) GetDynamicProvisionStorageClass(config *testsuites.Pe
 
 	sc, ok := items[0].(*storagev1.StorageClass)
 	gomega.Expect(ok).To(gomega.BeTrue(), "storage class from %s", d.StorageClass.FromFile)
+	// Ensure that we can load more than once as required for
+	// GetDynamicProvisionStorageClass by adding a random suffix.
+	sc.Name = names.SimpleNameGenerator.GenerateName(sc.Name + "-")
 	if fsType != "" {
 		if sc.Parameters == nil {
 			sc.Parameters = map[string]string{}

test/e2e/storage/testsuites/testdriver.go

@@ -88,6 +88,9 @@ type PreprovisionedPVTestDriver interface {
 type DynamicPVTestDriver interface {
 	TestDriver
 	// GetDynamicProvisionStorageClass returns a StorageClass dynamic provision Persistent Volume.
+	// The StorageClass must be created in the current test's namespace and have
+	// a unique name inside that namespace because GetDynamicProvisionStorageClass might
+	// be called more than once per test.
 	// It will set fsType to the StorageClass, if TestDriver supports it.
 	// It will return nil, if the TestDriver doesn't support it.
 	GetDynamicProvisionStorageClass(config *PerTestConfig, fsType string) *storagev1.StorageClass