From 62c5c6345e1fea1bdf7d24e0b28cdb1498f2fc48 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Wed, 8 May 2019 17:28:13 +0200 Subject: [PATCH] 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 c50e7fd30155d05fdde4174169945a43e26cf867 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. --- test/e2e/framework/BUILD | 1 - test/e2e/framework/create.go | 52 +++++++++++++++--------------------- 2 files changed, 21 insertions(+), 32 deletions(-) diff --git a/test/e2e/framework/BUILD b/test/e2e/framework/BUILD index fe67cc58134..6d317ac5f7b 100644 --- a/test/e2e/framework/BUILD +++ b/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", diff --git a/test/e2e/framework/create.go b/test/e2e/framework/create.go index c250bc1b3dc..bd22e0f646c 100644 --- a/test/e2e/framework/create.go +++ b/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) }