Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

⚠️ Remove options.WarningHandler #2903

Merged
merged 1 commit into from
Aug 6, 2024
Merged

⚠️ Remove options.WarningHandler #2903

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 14 additions & 27 deletions pkg/client/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand Down
53 changes: 15 additions & 38 deletions pkg/client/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package client_test

import (
"bufio"
"bytes"
"context"
"encoding/json"
"errors"
Expand All @@ -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"
Expand Down Expand Up @@ -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())
Expand All @@ -257,45 +263,17 @@ 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\"",
) {
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() {
Expand All @@ -308,7 +286,6 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC
break
}
}
deleteNamespace(ctx, tns)
})
})

Expand Down
21 changes: 21 additions & 0 deletions pkg/client/example_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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.
Expand Down
Loading