Fix flaky AdmissionWebhook e2e-crd tests

Several of the tests("It") in the e2e suite reuse the CRD.
However they each try to setup and tear down the CRD independently.
Since these tests can be running in parallel, causing intermittant
failures.
Added a new framework utility for creating CRDs per test.
Then making the relevant tests use the utility to prevent name
collision/race.
Fixed bazel build.
Factored in fixes for @caesarxuchao
Making suggested change for @janetkuo

test/e2e/apimachinery/webhook.go

@@ -27,8 +27,6 @@ import (
 	extensions "k8s.io/api/extensions/v1beta1"
 	rbacv1beta1 "k8s.io/api/rbac/v1beta1"
 	apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
-	crdclientset "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
-	"k8s.io/apiextensions-apiserver/test/integration/testserver"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -67,10 +65,6 @@ const (
 	hangingPodName          = "hanging-pod"
 	disallowedConfigMapName = "disallowed-configmap"
 	allowedConfigMapName    = "allowed-configmap"
-	crdName                 = "e2e-test-webhook-crd"
-	crdKind                 = "E2e-test-webhook-crd"
-	crdAPIGroup             = "webhook-crd-test.k8s.io"
-	crdAPIVersion           = "v1"
 	failNamespaceLabelKey   = "fail-closed-webhook"
 	failNamespaceLabelValue = "yes"
 	failNamespaceName       = "fail-closed-namesapce"
@@ -107,6 +101,7 @@ var _ = SIGDescribe("AdmissionWebhook", func() {
 		// the development 1.9 cycle.
 		deployWebhookAndService(f, imageutils.GetE2EImage(imageutils.AdmissionWebhook), context)
 	})
+
 	AfterEach(func() {
 		cleanWebhookTest(client, namespaceName)
 	})
@@ -118,11 +113,14 @@ var _ = SIGDescribe("AdmissionWebhook", func() {
 	})
 
 	It("Should be able to deny custom resource creation", func() {
-		crdCleanup, dynamicClient := createCRD(f)
+		testcrd, err := framework.CreateTestCRD(f)
-		defer crdCleanup()
+		if err != nil {
-		webhookCleanup := registerWebhookForCRD(f, context)
+			return
+		}
+		defer testcrd.CleanUp()
+		webhookCleanup := registerWebhookForCRD(f, context, testcrd)
 		defer webhookCleanup()
-		testCRDWebhook(f, dynamicClient)
+		testCRDWebhook(f, testcrd.Crd, testcrd.DynamicClient)
 	})
 
 	It("Should unconditionally reject operations on fail closed webhook", func() {
@@ -144,11 +142,14 @@ var _ = SIGDescribe("AdmissionWebhook", func() {
 	})
 
 	It("Should mutate crd", func() {
-		crdCleanup, dynamicClient := createCRD(f)
+		testcrd, err := framework.CreateTestCRD(f)
-		defer crdCleanup()
+		if err != nil {
-		webhookCleanup := registerMutatingWebhookForCRD(f, context)
+			return
+		}
+		defer testcrd.CleanUp()
+		webhookCleanup := registerMutatingWebhookForCRD(f, context, testcrd)
 		defer webhookCleanup()
-		testMutatingCRDWebhook(f, dynamicClient)
+		testMutatingCRDWebhook(f, testcrd.Crd, testcrd.DynamicClient)
 	})
 
 	// TODO: add more e2e tests for mutating webhooks
@@ -814,57 +815,7 @@ func cleanWebhookTest(client clientset.Interface, namespaceName string) {
 	_ = client.RbacV1beta1().RoleBindings("kube-system").Delete(roleBindingName, nil)
 }
 
-// newCRDForAdmissionWebhookTest generates a CRD
+func registerWebhookForCRD(f *framework.Framework, context *certContext, testcrd *framework.TestCrd) func() {
-func newCRDForAdmissionWebhookTest() *apiextensionsv1beta1.CustomResourceDefinition {
-	return &apiextensionsv1beta1.CustomResourceDefinition{
-		ObjectMeta: metav1.ObjectMeta{Name: crdName + "s." + crdAPIGroup},
-		Spec: apiextensionsv1beta1.CustomResourceDefinitionSpec{
-			Group:   crdAPIGroup,
-			Version: crdAPIVersion,
-			Names: apiextensionsv1beta1.CustomResourceDefinitionNames{
-				Plural:   crdName + "s",
-				Singular: crdName,
-				Kind:     crdKind,
-				ListKind: crdName + "List",
-			},
-			Scope: apiextensionsv1beta1.NamespaceScoped,
-		},
-	}
-}
-
-func createCRD(f *framework.Framework) (func(), dynamic.ResourceInterface) {
-	config, err := framework.LoadConfig()
-	if err != nil {
-		framework.Failf("failed to load config: %v", err)
-	}
-
-	apiExtensionClient, err := crdclientset.NewForConfig(config)
-	if err != nil {
-		framework.Failf("failed to initialize apiExtensionClient: %v", err)
-	}
-
-	crd := newCRDForAdmissionWebhookTest()
-
-	//create CRD and waits for the resource to be recognized and available.
-	dynamicClient, err := testserver.CreateNewCustomResourceDefinitionWatchUnsafe(crd, apiExtensionClient, f.ClientPool)
-	if err != nil {
-		framework.Failf("failed to create CustomResourceDefinition: %v", err)
-	}
-
-	resourceClient := dynamicClient.Resource(&metav1.APIResource{
-		Name:       crd.Spec.Names.Plural,
-		Namespaced: true,
-	}, f.Namespace.Name)
-
-	return func() {
-		err = testserver.DeleteCustomResourceDefinition(crd, apiExtensionClient)
-		if err != nil {
-			framework.Failf("failed to delete CustomResourceDefinition: %v", err)
-		}
-	}, resourceClient
-}
-
-func registerWebhookForCRD(f *framework.Framework, context *certContext) func() {
 	client := f.ClientSet
 	By("Registering the crd webhook via the AdmissionRegistration API")
 
@@ -880,9 +831,9 @@ func registerWebhookForCRD(f *framework.Framework, context *certContext) func()
 				Rules: []v1beta1.RuleWithOperations{{
 					Operations: []v1beta1.OperationType{v1beta1.Create},
 					Rule: v1beta1.Rule{
-						APIGroups:   []string{crdAPIGroup},
+						APIGroups:   []string{testcrd.ApiGroup},
-						APIVersions: []string{crdAPIVersion},
+						APIVersions: []string{testcrd.ApiVersion},
-						Resources:   []string{crdName + "s"},
+						Resources:   []string{testcrd.GetPluralName()},
 					},
 				}},
 				ClientConfig: v1beta1.WebhookClientConfig{
@@ -905,7 +856,7 @@ func registerWebhookForCRD(f *framework.Framework, context *certContext) func()
 	}
 }
 
-func registerMutatingWebhookForCRD(f *framework.Framework, context *certContext) func() {
+func registerMutatingWebhookForCRD(f *framework.Framework, context *certContext, testcrd *framework.TestCrd) func() {
 	client := f.ClientSet
 	By("Registering the mutating webhook for crd via the AdmissionRegistration API")
 
@@ -921,9 +872,9 @@ func registerMutatingWebhookForCRD(f *framework.Framework, context *certContext)
 				Rules: []v1beta1.RuleWithOperations{{
 					Operations: []v1beta1.OperationType{v1beta1.Create},
 					Rule: v1beta1.Rule{
-						APIGroups:   []string{crdAPIGroup},
+						APIGroups:   []string{testcrd.ApiGroup},
-						APIVersions: []string{crdAPIVersion},
+						APIVersions: []string{testcrd.ApiVersion},
-						Resources:   []string{crdName + "s"},
+						Resources:   []string{testcrd.GetPluralName()},
 					},
 				}},
 				ClientConfig: v1beta1.WebhookClientConfig{
@@ -940,9 +891,9 @@ func registerMutatingWebhookForCRD(f *framework.Framework, context *certContext)
 				Rules: []v1beta1.RuleWithOperations{{
 					Operations: []v1beta1.OperationType{v1beta1.Create},
 					Rule: v1beta1.Rule{
-						APIGroups:   []string{crdAPIGroup},
+						APIGroups:   []string{testcrd.ApiGroup},
-						APIVersions: []string{crdAPIVersion},
+						APIVersions: []string{testcrd.ApiVersion},
-						Resources:   []string{crdName + "s"},
+						Resources:   []string{testcrd.GetPluralName()},
 					},
 				}},
 				ClientConfig: v1beta1.WebhookClientConfig{
@@ -964,9 +915,8 @@ func registerMutatingWebhookForCRD(f *framework.Framework, context *certContext)
 	return func() { client.AdmissionregistrationV1beta1().MutatingWebhookConfigurations().Delete(configName, nil) }
 }
 
-func testCRDWebhook(f *framework.Framework, crdClient dynamic.ResourceInterface) {
+func testCRDWebhook(f *framework.Framework, crd *apiextensionsv1beta1.CustomResourceDefinition, crdClient dynamic.ResourceInterface) {
 	By("Creating a custom resource that should be denied by the webhook")
-	crd := newCRDForAdmissionWebhookTest()
 	crInstance := &unstructured.Unstructured{
 		Object: map[string]interface{}{
 			"kind":       crd.Spec.Names.Kind,
@@ -988,9 +938,8 @@ func testCRDWebhook(f *framework.Framework, crdClient dynamic.ResourceInterface)
 	}
 }
 
-func testMutatingCRDWebhook(f *framework.Framework, crdClient dynamic.ResourceInterface) {
+func testMutatingCRDWebhook(f *framework.Framework, crd *apiextensionsv1beta1.CustomResourceDefinition, crdClient dynamic.ResourceInterface) {
 	By("Creating a custom resource that should be mutated by the webhook")
-	crd := newCRDForAdmissionWebhookTest()
 	cr := &unstructured.Unstructured{
 		Object: map[string]interface{}{
 			"kind":       crd.Spec.Names.Kind,

test/e2e/framework/BUILD

@@ -10,6 +10,7 @@ go_library(
     srcs = [
         "authorizer_util.go",
         "cleanup.go",
+        "crd_util.go",
         "deployment_util.go",
         "exec_util.go",
         "firewall_util.go",
@@ -110,6 +111,9 @@ go_library(
         "//vendor/k8s.io/api/extensions/v1beta1:go_default_library",
         "//vendor/k8s.io/api/policy/v1beta1:go_default_library",
         "//vendor/k8s.io/api/rbac/v1beta1:go_default_library",
+        "//vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1:go_default_library",
+        "//vendor/k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset:go_default_library",
+        "//vendor/k8s.io/apiextensions-apiserver/test/integration/testserver:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/api/meta:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library",

test/e2e/framework/crd_util.go

@@ -0,0 +1,126 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package framework
+
+import (
+	"fmt"
+
+	apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
+	crdclientset "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
+	"k8s.io/apiextensions-apiserver/test/integration/testserver"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/dynamic"
+)
+
+// CleanCrdFn declares the clean up function needed to remove the CRD
+type CleanCrdFn func() error
+
+// TestCrd holds all the pieces needed to test with the CRD
+type TestCrd struct {
+	Name               string
+	Kind               string
+	ApiGroup           string
+	ApiVersion         string
+	ApiExtensionClient *crdclientset.Clientset
+	Crd                *apiextensionsv1beta1.CustomResourceDefinition
+	DynamicClient      dynamic.ResourceInterface
+	CleanUp            CleanCrdFn
+}
+
+// CreateTestCRD creates a new CRD specifically for the calling test.
+func CreateTestCRD(f *Framework) (*TestCrd, error) {
+	suffix := randomSuffix()
+	name := fmt.Sprintf("e2e-test-%s-%s-crd", f.BaseName, suffix)
+	kind := fmt.Sprintf("E2e-test-%s-%s-crd", f.BaseName, suffix)
+	group := fmt.Sprintf("%s-crd-test.k8s.io", f.BaseName)
+	apiVersion := "v1"
+	testcrd := &TestCrd{
+		Name:       name,
+		Kind:       kind,
+		ApiGroup:   group,
+		ApiVersion: apiVersion,
+	}
+
+	// Creating a custom resource definition for use by assorted tests.
+	config, err := LoadConfig()
+	if err != nil {
+		Failf("failed to load config: %v", err)
+		return nil, err
+	}
+	apiExtensionClient, err := crdclientset.NewForConfig(config)
+	if err != nil {
+		Failf("failed to initialize apiExtensionClient: %v", err)
+		return nil, err
+	}
+	crd := newCRDForTest(testcrd)
+
+	//create CRD and waits for the resource to be recognized and available.
+	dynamicClient, err := testserver.CreateNewCustomResourceDefinitionWatchUnsafe(crd, apiExtensionClient, f.ClientPool)
+	if err != nil {
+		Failf("failed to create CustomResourceDefinition: %v", err)
+		return nil, err
+	}
+	resourceClient := dynamicClient.Resource(&metav1.APIResource{
+		Name:       crd.Spec.Names.Plural,
+		Namespaced: true,
+	}, f.Namespace.Name)
+
+	testcrd.ApiExtensionClient = apiExtensionClient
+	testcrd.Crd = crd
+	testcrd.DynamicClient = resourceClient
+	testcrd.CleanUp = func() error {
+		err := testserver.DeleteCustomResourceDefinition(crd, apiExtensionClient)
+		if err != nil {
+			Failf("failed to delete CustomResourceDefinition(%s): %v", name, err)
+		}
+		return err
+	}
+	return testcrd, nil
+}
+
+// newCRDForTest generates a CRD definition for the test
+func newCRDForTest(testcrd *TestCrd) *apiextensionsv1beta1.CustomResourceDefinition {
+	return &apiextensionsv1beta1.CustomResourceDefinition{
+		ObjectMeta: metav1.ObjectMeta{Name: testcrd.GetMetaName()},
+		Spec: apiextensionsv1beta1.CustomResourceDefinitionSpec{
+			Group:   testcrd.ApiGroup,
+			Version: testcrd.ApiVersion,
+			Names: apiextensionsv1beta1.CustomResourceDefinitionNames{
+				Plural:   testcrd.GetPluralName(),
+				Singular: testcrd.Name,
+				Kind:     testcrd.Kind,
+				ListKind: testcrd.GetListName(),
+			},
+			Scope: apiextensionsv1beta1.NamespaceScoped,
+		},
+	}
+}
+
+// GetMetaName returns the metaname for the CRD.
+func (c *TestCrd) GetMetaName() string {
+	return c.Name + "s." + c.ApiGroup
+}
+
+// GetPluralName returns the plural form of the CRD name
+func (c *TestCrd) GetPluralName() string {
+	return c.Name + "s"
+}
+
+// GetListName returns the name for the CRD list resources
+func (c *TestCrd) GetListName() string {
+	return c.Name + "List"
+}