From 47b613eded334a2dc27ecc117aa54724da07aac4 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Wed, 3 Jan 2024 11:41:26 +0100 Subject: [PATCH] e2e framework: support creating TContext This makes it possible to call helper packages which expect a TContext from E2E tests. The implementation uses GinkgoT as TB and supports registering cleanup callbacks which expect a context. These callbacks then run with a context that comes from ginkgo.DeferCleanup, just as if they had called that directly. --- test/e2e/framework/framework.go | 104 +++++++++++++++++- .../unittests/cleanup/cleanup_test.go | 28 ++++- 2 files changed, 128 insertions(+), 4 deletions(-) diff --git a/test/e2e/framework/framework.go b/test/e2e/framework/framework.go index 74df75a6346..1df56e26a4f 100644 --- a/test/e2e/framework/framework.go +++ b/test/e2e/framework/framework.go @@ -31,12 +31,12 @@ import ( "strings" "time" - "k8s.io/apimachinery/pkg/runtime" - v1 "k8s.io/api/core/v1" + apiextensions "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/wait" v1svc "k8s.io/client-go/applyconfigurations/core/v1" @@ -48,8 +48,10 @@ import ( "k8s.io/client-go/rest" "k8s.io/client-go/restmapper" scaleclient "k8s.io/client-go/scale" + "k8s.io/kubernetes/test/utils/ktesting" admissionapi "k8s.io/pod-security-admission/api" + "github.com/go-logr/logr" "github.com/onsi/ginkgo/v2" ) @@ -95,7 +97,16 @@ var ( // The default pod security profile is "restricted". // Each of the labels can be overridden by using more specific NamespacePodSecurity* attributes of this // struct. +// +// A framework instance implements ktesting.TB and thus can be passed as +// first parameter to [github.com/stretchr/testify/assert]. There are +// just two caveats: +// - Error and Errorf abort the currently running test. +// - The implementation is only usable while a test runs, +// not while defining tests. type Framework struct { + ktesting.TB + BaseName string // Set together with creating the ClientSet and the namespace. @@ -104,6 +115,7 @@ type Framework struct { UniqueName string clientConfig *rest.Config + restMapper *restmapper.DeferredDiscoveryRESTMapper ClientSet clientset.Interface KubemarkExternalClusterClientSet clientset.Interface @@ -139,6 +151,86 @@ type Framework struct { DumpAllNamespaceInfo DumpAllNamespaceInfoAction } +// CleanupCtx implements [ktesting.ContextTB.CleanupCtx]. It's identical to +// ginkgo.DeferCleanup. +func (f *Framework) CleanupCtx(cb func(context.Context)) { + ginkgo.GinkgoHelper() + ginkgo.DeferCleanup(cb) +} + +// Log implements TB.Log. It overrides the implementation from Ginkgo to ensure consistent output. +func (f *Framework) Log(args ...any) { + log(1, strings.TrimSuffix(fmt.Sprintln(args...), "\n")) +} + +// Logf implements TB.Logf. It overrides the implementation from Ginkgo to ensure consistent output. +func (f *Framework) Logf(format string, args ...any) { + log(1, fmt.Sprintf(format, args...)) +} + +var _ ktesting.ContextTB = &Framework{} + +// TContext combines the framework and the context in a [ktesting.TContext]. +// +// Legacy code which accepts separate [context.Context] and [clientset.Interface] +// parameters can call [ContextTODO] with those two parameters to +// create a [ktesting.TContext] which also has all clients, but this is +// an interim solution. It's cleaner to rewrite that code to accept +// a [ktesting.TContext]. +func (f *Framework) TContext(ctx context.Context) ktesting.TContext { + tCtx := ktesting.InitCtx(ctx, f) + tCtx = tCtx.WithClients(f.clientConfig, f.restMapper, f.ClientSet, f.DynamicClient, apiextensions.NewForConfigOrDie(f.clientConfig)) + tCtx = tCtx.WithNamespace(f.Namespace.Name) + tCtx = ensureLogger(tCtx) + return tCtx +} + +// ContextTODO can be used as interim solution in functions which currently +// accept a [context.Context] and [clientset.Interface] to create +// a [ktesting.TContext] for other code which already uses that instead +// of different parameters. If the client is [Framework.ClientSet], +// the result is identical to the one from [Framework.TContext]. +// In particular, dynamic client and rest config are available. +// +// Long-term code relying on ContextTODO should get rewritten. +func ContextTODO(ctx context.Context, client clientset.Interface) ktesting.TContext { + // If the client was taken from a framework instance, then + // we can cast it here to prepare a complete context. + if fc, ok := client.(fClient); ok { + return fc.f.TContext(ctx) + } + f := NewDefaultFramework("tcontext") + tCtx := ktesting.InitCtx(ctx, f) + tCtx = tCtx.WithClients(nil, nil, client, nil, nil) + tCtx = ensureLogger(tCtx) + return tCtx +} + +// ensureLogger installs the Ginkgo logger in the context if there isn't one +// already. +// +// This isn't needed for code which uses klog.FromContext because that uses the +// klog logger as default, which is the Gingko logger. But there might also be +// code which uses go-logr directly: then providing the logger through the +// context is useful. +func ensureLogger(tCtx ktesting.TContext) ktesting.TContext { + _, ok := logr.FromContext(tCtx) + if ok == nil { + return tCtx + } + + return tCtx.WithLogger(ginkgoLogger) +} + +// fClient is stored in framework.ClientSet and used by ContextTODO. +// +// TODO: remove this and ContextTODO once it is no longer needed, i.e. +// when all code accepts a TContext. +type fClient struct { + clientset.Interface + f *Framework +} + // DumpAllNamespaceInfoAction is called after each failed test for namespaces // created for the test. type DumpAllNamespaceInfoAction func(ctx context.Context, f *Framework, namespace string) @@ -188,6 +280,7 @@ func NewDefaultFramework(baseName string) *Framework { // NewFramework creates a test framework. func NewFramework(baseName string, options Options, client clientset.Interface) *Framework { f := &Framework{ + TB: ginkgo.GinkgoT(), BaseName: baseName, Options: options, ClientSet: client, @@ -230,8 +323,12 @@ func (f *Framework) BeforeEach(ctx context.Context) { config.ContentType = TestContext.KubeAPIContentType } f.clientConfig = rest.CopyConfig(config) - f.ClientSet, err = clientset.NewForConfig(config) + clientSet, err := clientset.NewForConfig(config) ExpectNoError(err) + f.ClientSet = fClient{ + Interface: clientSet, + f: f, + } f.DynamicClient, err = dynamic.NewForConfig(config) ExpectNoError(err) @@ -250,6 +347,7 @@ func (f *Framework) BeforeEach(ctx context.Context) { cachedDiscoClient := cacheddiscovery.NewMemCacheClient(discoClient) restMapper := restmapper.NewDeferredDiscoveryRESTMapper(cachedDiscoClient) restMapper.Reset() + f.restMapper = restMapper resolver := scaleclient.NewDiscoveryScaleKindResolver(cachedDiscoClient) f.ScalesGetter = scaleclient.New(restClient, restMapper, dynamic.LegacyAPIPathResolverFunc, resolver) diff --git a/test/e2e/framework/internal/unittests/cleanup/cleanup_test.go b/test/e2e/framework/internal/unittests/cleanup/cleanup_test.go index a0a2059b6bb..7d29dbc07c4 100644 --- a/test/e2e/framework/internal/unittests/cleanup/cleanup_test.go +++ b/test/e2e/framework/internal/unittests/cleanup/cleanup_test.go @@ -33,10 +33,10 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/klog/v2" - "k8s.io/klog/v2/ktesting" "k8s.io/kubernetes/test/e2e/framework" "k8s.io/kubernetes/test/e2e/framework/internal/output" testapiserver "k8s.io/kubernetes/test/utils/apiserver" + "k8s.io/kubernetes/test/utils/ktesting" ) // The line number of the following code is checked in TestFailureOutput below. @@ -102,6 +102,28 @@ var _ = ginkgo.Describe("e2e", func() { } ginkgo.DeferCleanup(framework.IgnoreNotFound(fail), "failure") // Without a failure the output would not be shown in JUnit. + tCtx := f.TContext(ctx) + tCtx.Log("log", "hello", "world") + tCtx.Logger().Info("info hello world") + var discardLogger klog.Logger + tCtx = tCtx.WithLogger(discardLogger) + oldCtx := tCtx + tCtx.CleanupCtx(func(tCtx ktesting.TContext) { + if tCtx.Logger() != discardLogger { + tCtx.Errorf("expected discard logger in context, got %+v", tCtx.Logger()) + } + _, ok := tCtx.Value("GINKGO_SPEC_CONTEXT").(ginkgo.SpecContext) + if !ok { + tCtx.Errorf("expected Ginkgo context, got %+v", tCtx.Context) + } + if oldCtx.Err() == nil { + tCtx.Error("Ginkgo.It context should be canceled but isn't") + } + if tCtx.Err() != nil { + tCtx.Errorf("Ginkgo.DeferCleanup context should not be canceled but is: %v", tCtx.Err()) + } + }) + // More test cases can be added here without affeccting line numbering // of existing tests. }) @@ -134,6 +156,8 @@ STEP: Building a namespace api object, basename test-namespace - framework.go:xx cleanup_test.go:76] before #2 < Exit [BeforeEach] e2e - cleanup_test.go:75