diff --git a/pkg/client/client.go b/pkg/client/client.go index 451f7b2a1b..fe9862b814 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -50,28 +50,10 @@ type Options struct { // Cache, if provided, is used to read objects from the cache. Cache *CacheOptions - // WarningHandler is used to configure the warning handler responsible for - // surfacing and handling warnings messages sent by the API server. - WarningHandler WarningHandlerOptions - // DryRun instructs the client to only perform dry run requests. DryRun *bool } -// WarningHandlerOptions are options for configuring a -// warning handler for the client which is responsible -// for surfacing API Server warnings. -type WarningHandlerOptions struct { - // SuppressWarnings decides if the warnings from the - // API server are suppressed or surfaced in the client. - SuppressWarnings bool - // AllowDuplicateLogs does not deduplicate the to-be - // logged surfaced warnings messages. See - // log.WarningHandlerOptions for considerations - // regarding deduplication - AllowDuplicateLogs bool -} - // CacheOptions are options for creating a cache-backed client. type CacheOptions struct { // Reader is a cache-backed reader that will be used to read objects from the cache. @@ -91,6 +73,12 @@ 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. +// See [sigs.k8s.io/controller-runtime/pkg/log.KubeAPIWarningLogger] for +// an example. +// // 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 +112,14 @@ 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{ + Deduplicate: true, + }, + ) } // 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..59ddf13664 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,15 +231,19 @@ 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{} - cl, err := client.New(cfg, client.Options{ - WarningHandler: client.WarningHandlerOptions{SuppressWarnings: false}, Cache: &client.CacheOptions{Reader: cache, DisableFor: []client.Object{&corev1.Namespace{}}}, - }) + + 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: "ws-disabled"}} + 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()) @@ -257,9 +263,9 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) - scanner := bufio.NewScanner(&log) - for scanner.Scan() { - line := scanner.Text() + scannerTestLog := bufio.NewScanner(&testLog) + for scannerTestLog.Scan() { + line := scannerTestLog.Text() if strings.Contains( line, "unknown field \"status\"", @@ -267,35 +273,7 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC return } } - defer Fail("expected to find one API server warning in the client log") - }) - - It("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{}}}, - }) - Expect(err).NotTo(HaveOccurred()) - Expect(cl).NotTo(BeNil()) - - tns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "ws-enabled"}} - tns, err = clientset.CoreV1().Namespaces().Create(ctx, tns, metav1.CreateOptions{}) - Expect(err).NotTo(HaveOccurred()) - Expect(tns).NotTo(BeNil()) - - 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()) + defer Fail("expected to find one API server warning logged the config.WarningHandler") scanner := bufio.NewScanner(&log) for scanner.Scan() { @@ -308,7 +286,6 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC break } } - deleteNamespace(ctx, tns) }) }) diff --git a/pkg/client/example_test.go b/pkg/client/example_test.go index 89cfa69cbd..2f8f975831 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,26 @@ func ExampleNew() { } } +func ExampleNew_suppress_warnings() { + cfg := config.GetConfigOrDie() + // Use a rest.WarningHandler that discards warning messages. + 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.