Revert "Make external driver storage class name generation contain a more random suffix in case of double generation in the same framework context (twice in the same test)"

This reverts commit c50e7fd because
it included API changes that shouldn't have been in that PR and
fixing the storage class conflict inside the framework is probably the
wrong place.

Author: pohly

test/e2e/framework/BUILD

@@ -92,7 +92,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)
 	}