From f897c861194f63069b5c8c121f7c8ad6fea0865d Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Wed, 19 Oct 2022 11:24:53 +0200 Subject: [PATCH] e2e framework: support ignoring "not found" errors during DeferCleanup The wrapper can be used in combination with ginkgo.DeferCleanup to ignore harmless "not found" errors during delete operations. Original code suggested by Onsi Fakhouri. --- test/e2e/framework/ginkgowrapper.go | 26 ++++++++ .../unittests/cleanup/cleanup_test.go | 61 ++++++++++++------- 2 files changed, 65 insertions(+), 22 deletions(-) diff --git a/test/e2e/framework/ginkgowrapper.go b/test/e2e/framework/ginkgowrapper.go index d8117934831..aa3d6c0a475 100644 --- a/test/e2e/framework/ginkgowrapper.go +++ b/test/e2e/framework/ginkgowrapper.go @@ -18,10 +18,36 @@ package framework import ( "path" + "reflect" "github.com/onsi/ginkgo/v2/types" + + apierrors "k8s.io/apimachinery/pkg/api/errors" ) +var errInterface = reflect.TypeOf((*error)(nil)).Elem() + +// IgnoreNotFound can be used to wrap an arbitrary function in a call to +// [ginkgo.DeferCleanup]. When the wrapped function returns an error that +// `apierrors.IsNotFound` considers as "not found", the error is ignored +// instead of failing the test during cleanup. This is useful for cleanup code +// that just needs to ensure that some object does not exist anymore. +func IgnoreNotFound(in any) any { + inType := reflect.TypeOf(in) + inValue := reflect.ValueOf(in) + return reflect.MakeFunc(inType, func(args []reflect.Value) []reflect.Value { + out := inValue.Call(args) + if len(out) > 0 { + lastValue := out[len(out)-1] + last := lastValue.Interface() + if last != nil && lastValue.Type().Implements(errInterface) && apierrors.IsNotFound(last.(error)) { + out[len(out)-1] = reflect.Zero(errInterface) + } + } + return out + }).Interface() +} + // AnnotatedLocation can be used to provide more informative source code // locations by passing the result as additional parameter to a // BeforeEach/AfterEach/DeferCleanup/It/etc. diff --git a/test/e2e/framework/internal/unittests/cleanup/cleanup_test.go b/test/e2e/framework/internal/unittests/cleanup/cleanup_test.go index c47fd14a568..36ed99f12c1 100644 --- a/test/e2e/framework/internal/unittests/cleanup/cleanup_test.go +++ b/test/e2e/framework/internal/unittests/cleanup/cleanup_test.go @@ -24,11 +24,13 @@ package cleanup import ( "context" "flag" + "fmt" "regexp" "testing" "github.com/onsi/ginkgo/v2" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/klog/v2" "k8s.io/klog/v2/ktesting" "k8s.io/kubernetes/test/e2e/framework" @@ -45,10 +47,18 @@ import ( // // // -// -// // This must be line #50. +func init() { + framework.NewFrameworkExtensions = append(framework.NewFrameworkExtensions, + // This callback runs directly after NewDefaultFramework is done. + func(f *framework.Framework) { + ginkgo.BeforeEach(func() { framework.Logf("extension before") }) + ginkgo.AfterEach(func() { framework.Logf("extension after") }) + }, + ) +} + var _ = ginkgo.Describe("e2e", func() { ginkgo.BeforeEach(func() { framework.Logf("before") @@ -85,22 +95,21 @@ var _ = ginkgo.Describe("e2e", func() { ginkgo.DeferCleanup(func() { framework.Logf("cleanup first") }) + + ginkgo.DeferCleanup(framework.IgnoreNotFound(f.ClientSet.CoreV1().PersistentVolumes().Delete), "simple", metav1.DeleteOptions{}) + fail := func(ctx context.Context, name string) error { + return fmt.Errorf("fake error for %q", name) + } + ginkgo.DeferCleanup(framework.IgnoreNotFound(fail), "failure") + + // More test cases can be added here without affeccting line numbering + // of existing tests. }) }) -func init() { - framework.NewFrameworkExtensions = append(framework.NewFrameworkExtensions, - // This callback runs directly after NewDefaultFramework is done. - func(f *framework.Framework) { - ginkgo.BeforeEach(func() { framework.Logf("extension before") }) - ginkgo.AfterEach(func() { framework.Logf("extension after") }) - }, - ) -} - const ( ginkgoOutput = `[BeforeEach] e2e - cleanup_test.go:53 + cleanup_test.go:63 INFO: before [BeforeEach] e2e set up framework | framework.go:xxx @@ -109,30 +118,34 @@ INFO: >>> kubeConfig: yyy/kube.config STEP: Building a namespace api object, basename test-namespace INFO: Skipping waiting for service account [BeforeEach] e2e - cleanup_test.go:95 + cleanup_test.go:56 INFO: extension before [BeforeEach] e2e - cleanup_test.go:61 + cleanup_test.go:71 INFO: before #1 [BeforeEach] e2e - cleanup_test.go:65 + cleanup_test.go:75 INFO: before #2 [It] works - cleanup_test.go:80 + cleanup_test.go:90 [AfterEach] e2e - cleanup_test.go:96 + cleanup_test.go:57 INFO: extension after [AfterEach] e2e - cleanup_test.go:69 + cleanup_test.go:79 INFO: after #1 [AfterEach] e2e - cleanup_test.go:76 + cleanup_test.go:86 INFO: after #2 [DeferCleanup (Each)] e2e - cleanup_test.go:85 + cleanup_test.go:103 +[DeferCleanup (Each)] e2e + cleanup_test.go:99 +[DeferCleanup (Each)] e2e + cleanup_test.go:95 INFO: cleanup first [DeferCleanup (Each)] e2e - cleanup_test.go:82 + cleanup_test.go:92 INFO: cleanup last [DeferCleanup (Each)] e2e dump namespaces | framework.go:xxx @@ -187,6 +200,10 @@ func TestCleanup(t *testing.T) { Name: "e2e works", NormalizeOutput: normalizeOutput, Output: ginkgoOutput, + // It would be nice to get the cleanup failure into the + // output, but that depends on Ginkgo enhancements: + // https://github.com/onsi/ginkgo/issues/1041#issuecomment-1274611444 + Failure: `DeferCleanup callback returned error: fake error for "failure"`, }, }