fix(test): reworks k8s envtest resource cleanup

- initial implementation was fetching api resources in the loop causing concurrent map reads and writes and thus flaky tests
- continuous fetching is not necessary as before each test we have all resource definitions defined
- this change moves the loading to when we create cleaner instance, which is before each test, making it more efficient but most importantly fixing concurrent access issue

test/cluster/k8s_operations.go

@@ -5,6 +5,7 @@ import (
  "strings"
  "time"
 
+ . "github.com/onsi/gomega"
  corev1 "k8s.io/api/core/v1"
  apierrors "k8s.io/apimachinery/pkg/api/errors"
  metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -13,39 +14,42 @@ import (
  "k8s.io/client-go/kubernetes"
  "k8s.io/client-go/rest"
  "sigs.k8s.io/controller-runtime/pkg/client"
-
- . "github.com/onsi/gomega"
 )
 
 // Cleaner is a struct to perform deletion of resources,
-// enforcing removal of finalizers. Otherwise deletion of namespaces wouldn't be possible.
+// enforcing removal of finalizers. Otherwise, deletion of namespaces wouldn't be possible.
 // See: https://book.kubebuilder.io/reference/envtest.html#namespace-usage-limitation
 // Based on https://github.com/kubernetes-sigs/controller-runtime/issues/880#issuecomment-749742403
 type Cleaner struct {
  clientset *kubernetes.Clientset
  client client.Client
  timeout, interval time.Duration
+ namespacedGVKs map[string]schema.GroupVersionKind
 }
 
-func CreateCleaner(c client.Client, config *rest.Config, timeout, interval time.Duration) *Cleaner {
- k8sClient, err := kubernetes.NewForConfig(config)
+func CreateCleaner(k8sClient client.Client, config *rest.Config, timeout, interval time.Duration) *Cleaner {
+ k8sClientset, err := kubernetes.NewForConfig(config)
  if err != nil {
  panic(err)
  }
+
+ namespacedGVKs := lookupNamespacedResources(k8sClientset)
+
  return &Cleaner{
- clientset: k8sClient,
- client: c,
- timeout: timeout,
- interval: interval,
+ clientset: k8sClientset,
+ client: k8sClient,
+ namespacedGVKs: namespacedGVKs,
+ timeout: timeout,
+ interval: interval,
  }
 }
 
-func (c *Cleaner) DeleteAll(objects ...client.Object) {
- for _, obj := range objects {
-
+func (c *Cleaner) DeleteAll(objects ...client.Object) { //nolint:gocognit //reason it is what is ;)
+ for _, o := range objects {
+ obj := o
  Expect(client.IgnoreNotFound(c.client.Delete(context.Background(), obj))).Should(Succeed())
 
- if ns, ok := obj.(*corev1.Namespace); ok {
+ if namespace, ok := obj.(*corev1.Namespace); ok {
  // Normally the kube-controller-manager would handle finalization
  // and garbage collection of namespaces, but with envtest, we aren't
  // running a kube-controller-manager. Instead we're gonna approximate
@@ -55,61 +59,35 @@ func (c *Cleaner) DeleteAll(objects ...client.Object) {
  // Note that any resources within the namespace that we don't
  // successfully delete could reappear if the namespace is ever
  // recreated with the same name.
-
- // Look up all namespaced resources under the discovery API
- _, apiResources, err := c.clientset.DiscoveryClient.ServerGroupsAndResources()
- Expect(err).ShouldNot(HaveOccurred())
- namespacedGVKs := make(map[string]schema.GroupVersionKind)
- for _, apiResourceList := range apiResources {
- defaultGV, err := schema.ParseGroupVersion(apiResourceList.GroupVersion)
- Expect(err).ShouldNot(HaveOccurred())
- for _, r := range apiResourceList.APIResources {
- if !r.Namespaced || strings.Contains(r.Name, "/") {
- // skip non-namespaced and subresources
- continue
- }
- gvk := schema.GroupVersionKind{
- Group: defaultGV.Group,
- Version: defaultGV.Version,
- Kind: r.Kind,
- }
- if r.Group != "" {
- gvk.Group = r.Group
- }
- if r.Version != "" {
- gvk.Version = r.Version
- }
- namespacedGVKs[gvk.String()] = gvk
- }
- }
-
  // Delete all namespaced resources in this namespace
- for _, gvk := range namespacedGVKs {
- var u unstructured.Unstructured
+ for _, gvk := range c.namespacedGVKs {
+ u := unstructured.Unstructured{}
  u.SetGroupVersionKind(gvk)
- err := c.client.DeleteAllOf(context.Background(), &u, client.InNamespace(ns.Name))
- Expect(client.IgnoreNotFound(ignoreMethodNotAllowed(err))).ShouldNot(HaveOccurred())
+
+ deleteErr := c.client.DeleteAllOf(context.Background(), &u, client.InNamespace(namespace.Name))
+ Expect(client.IgnoreNotFound(ignoreMethodNotAllowed(deleteErr))).ShouldNot(HaveOccurred())
  }
 
  Eventually(func() error {
- key := client.ObjectKeyFromObject(ns)
+ key := client.ObjectKeyFromObject(namespace)
 
- if err := c.client.Get(context.Background(), key, ns); err != nil {
- return client.IgnoreNotFound(err)
+ if getErr := c.client.Get(context.Background(), key, namespace); getErr != nil {
+ return client.IgnoreNotFound(getErr)
  }
  // remove `kubernetes` finalizer
  const k8s = "kubernetes"
  finalizers := []corev1.FinalizerName{}
- for _, f := range ns.Spec.Finalizers {
+ for _, f := range namespace.Spec.Finalizers {
  if f != k8s {
  finalizers = append(finalizers, f)
  }
  }
- ns.Spec.Finalizers = finalizers
+ namespace.Spec.Finalizers = finalizers
 
  // We have to use the k8s.io/client-go library here to expose
  // ability to patch the /finalize subresource on the namespace
- _, err = c.clientset.CoreV1().Namespaces().Finalize(context.Background(), ns, metav1.UpdateOptions{})
+ _, err := c.clientset.CoreV1().Namespaces().Finalize(context.Background(), namespace, metav1.UpdateOptions{})
+
  return err
  }, c.timeout, c.interval).Should(Succeed())
  }
@@ -119,14 +97,59 @@ func (c *Cleaner) DeleteAll(objects ...client.Object) {
  if err := c.client.Get(context.Background(), key, obj); err != nil {
  return apierrors.ReasonForError(err)
  }
+
  return ""
  }, c.timeout, c.interval).Should(Equal(metav1.StatusReasonNotFound))
  }
 }
 
+func lookupNamespacedResources(clientset *kubernetes.Clientset) map[string]schema.GroupVersionKind {
+ namespacedGVKs := make(map[string]schema.GroupVersionKind)
+
+ // Look up all namespaced resources under the discovery API
+ _, apiResources, listErr := clientset.DiscoveryClient.ServerGroupsAndResources()
+ if listErr != nil {
+ panic(listErr)
+ }
+
+ for _, apiResourceList := range apiResources {
+ resources := apiResourceList
+
+ defaultGV, parseErr := schema.ParseGroupVersion(resources.GroupVersion)
+ Expect(parseErr).ShouldNot(HaveOccurred())
+
+ for i := range resources.APIResources {
+ resource := resources.APIResources[i]
+ if !resource.Namespaced || strings.Contains(resource.Name, "/") {
+ // skip non-namespaced and sub-resources
+ continue
+ }
+
+ gvk := schema.GroupVersionKind{
+ Group: defaultGV.Group,
+ Version: defaultGV.Version,
+ Kind: resource.Kind,
+ }
+
+ if resource.Group != "" {
+ gvk.Group = resource.Group
+ }
+
+ if resource.Version != "" {
+ gvk.Version = resource.Version
+ }
+
+ namespacedGVKs[gvk.String()] = gvk
+ }
+ }
+
+ return namespacedGVKs
+}
+
 func ignoreMethodNotAllowed(err error) error {
  if apierrors.ReasonForError(err) == metav1.StatusReasonMethodNotAllowed {
  return nil
  }
+
  return err
 }