diff --git a/pkg/client/client.go b/pkg/client/client.go index 451f7b2a1b..1b1949d9c7 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -52,6 +52,11 @@ type Options struct { // WarningHandler is used to configure the warning handler responsible for // surfacing and handling warnings messages sent by the API server. + // + // Deprecated: This is deprecated and will be removed in a future release. + // Deprecation and future removal does not change the default behavior, which is to + // de-duplicate and surface warnings. For custom behavior, pass config.WarningHandler + // to New(). WarningHandler WarningHandlerOptions // DryRun instructs the client to only perform dry run requests. @@ -61,6 +66,8 @@ type Options struct { // WarningHandlerOptions are options for configuring a // warning handler for the client which is responsible // for surfacing API Server warnings. +// +// Deprecated: This is deprecated and will be removed in a future release. type WarningHandlerOptions struct { // SuppressWarnings decides if the warnings from the // API server are suppressed or surfaced in the client. @@ -91,6 +98,11 @@ type NewClientFunc func(config *rest.Config, options Options) (Client, error) // New returns a new Client using the provided config and Options. // +// By default, the client surfaces warnings returned by the server. To +// suppress warnings, set config.WarningHandler = rest.NoWarnings{}. To +// define custom behavior, implement the rest.WarningHandler interface. +// For consistent log formatting, use pkg/log.Log in the implementation. +// // The client's read behavior is determined by Options.Cache. // If either Options.Cache or Options.Cache.Reader is nil, // the client reads directly from the API server. @@ -124,15 +136,19 @@ func newClient(config *rest.Config, options Options) (*client, error) { config.UserAgent = rest.DefaultKubernetesUserAgent() } - // By default, we de-duplicate and surface warnings. - config.WarningHandler = log.NewKubeAPIWarningLogger( - log.Log.WithName("KubeAPIWarningLogger"), - log.KubeAPIWarningLoggerOptions{ - Deduplicate: !options.WarningHandler.AllowDuplicateLogs, - }, - ) - if options.WarningHandler.SuppressWarnings { - config.WarningHandler = rest.NoWarnings{} + if config.WarningHandler == nil { + // By default, we de-duplicate and surface warnings. + config.WarningHandler = log.NewKubeAPIWarningLogger( + log.Log.WithName("KubeAPIWarningLogger"), + log.KubeAPIWarningLoggerOptions{ + // When we remove WarningHandlerOptions, the below will always be set to true. + Deduplicate: !options.WarningHandler.AllowDuplicateLogs, + }, + ) + // When we remove WarningHandlerOptions, we will remove the below condition. + if options.WarningHandler.SuppressWarnings { + config.WarningHandler = rest.NoWarnings{} + } } // Use the rest HTTP client for the provided config if unset diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 2af2a6af11..27d67b046f 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -18,6 +18,7 @@ package client_test import ( "bufio" + "bytes" "context" "encoding/json" "errors" @@ -43,6 +44,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" kscheme "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/rest" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/examples/crd/pkg" @@ -229,7 +231,64 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC }) Describe("WarningHandler", func() { - It("should log warnings when warning suppression is disabled", func() { + It("should log warnings with config.WarningHandler, if one is defined", func() { + cache := &fakeReader{} + + testCfg := rest.CopyConfig(cfg) + + var testLog bytes.Buffer + testCfg.WarningHandler = rest.NewWarningWriter(&testLog, rest.WarningWriterOptions{}) + + cl, err := client.New(testCfg, client.Options{Cache: &client.CacheOptions{Reader: cache, DisableFor: []client.Object{&corev1.Namespace{}}}}) + Expect(err).NotTo(HaveOccurred()) + Expect(cl).NotTo(BeNil()) + + tns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "wh-defined"}} + tns, err = clientset.CoreV1().Namespaces().Create(ctx, tns, metav1.CreateOptions{}) + Expect(err).NotTo(HaveOccurred()) + Expect(tns).NotTo(BeNil()) + defer deleteNamespace(ctx, tns) + + toCreate := &pkg.ChaosPod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example", + Namespace: tns.Name, + }, + // The ChaosPod CRD does not define Status, so the field is unknown to the API server, + // but field validation is not strict by default, so the API server returns a warning, + // and we need a warning to check whether suppression works. + Status: pkg.ChaosPodStatus{}, + } + err = cl.Create(ctx, toCreate) + Expect(err).NotTo(HaveOccurred()) + Expect(cl).NotTo(BeNil()) + + scannerTestLog := bufio.NewScanner(&testLog) + for scannerTestLog.Scan() { + line := scannerTestLog.Text() + if strings.Contains( + line, + "unknown field \"status\"", + ) { + return + } + } + defer Fail("expected to find one API server warning logged the config.WarningHandler") + + scanner := bufio.NewScanner(&log) + for scanner.Scan() { + line := scanner.Text() + if strings.Contains( + line, + "unknown field \"status\"", + ) { + defer Fail("expected to find zero API server warnings in the client log") + break + } + } + }) + + It("(deprecated behavior) should log warnings when warning suppression is disabled", func() { cache := &fakeReader{} cl, err := client.New(cfg, client.Options{ WarningHandler: client.WarningHandlerOptions{SuppressWarnings: false}, Cache: &client.CacheOptions{Reader: cache, DisableFor: []client.Object{&corev1.Namespace{}}}, @@ -270,7 +329,7 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC defer Fail("expected to find one API server warning in the client log") }) - It("should not log warnings when warning suppression is enabled", func() { + It("(deprecated behavior) should not log warnings when warning suppression is enabled", func() { cache := &fakeReader{} cl, err := client.New(cfg, client.Options{ WarningHandler: client.WarningHandlerOptions{SuppressWarnings: true}, Cache: &client.CacheOptions{Reader: cache, DisableFor: []client.Object{&corev1.Namespace{}}}, diff --git a/pkg/client/example_test.go b/pkg/client/example_test.go index 89cfa69cbd..8978bcef7f 100644 --- a/pkg/client/example_test.go +++ b/pkg/client/example_test.go @@ -29,6 +29,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" corev1ac "k8s.io/client-go/applyconfigurations/core/v1" + "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/config" @@ -56,6 +57,24 @@ func ExampleNew() { } } +func ExampleNew_suppress_warnings() { + cfg := config.GetConfigOrDie() + cfg.WarningHandler = rest.NoWarnings{} + cl, err := client.New(cfg, client.Options{}) + if err != nil { + fmt.Println("failed to create client") + os.Exit(1) + } + + podList := &corev1.PodList{} + + err = cl.List(context.Background(), podList, client.InNamespace("default")) + if err != nil { + fmt.Printf("failed to list pods in namespace default: %v\n", err) + os.Exit(1) + } +} + // This example shows how to use the client with typed and unstructured objects to retrieve an object. func ExampleClient_get() { // Using a typed object.