diff --git a/pkg/client/client.go b/pkg/client/client.go index e6c075eb00..451f7b2a1b 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -124,19 +124,15 @@ func newClient(config *rest.Config, options Options) (*client, error) { config.UserAgent = rest.DefaultKubernetesUserAgent() } - if !options.WarningHandler.SuppressWarnings { - // surface warnings - logger := log.Log.WithName("KubeAPIWarningLogger") - // Set a WarningHandler, the default WarningHandler - // is log.KubeAPIWarningLogger with deduplication enabled. - // See log.KubeAPIWarningLoggerOptions for considerations - // regarding deduplication. - config.WarningHandler = log.NewKubeAPIWarningLogger( - logger, - log.KubeAPIWarningLoggerOptions{ - Deduplicate: !options.WarningHandler.AllowDuplicateLogs, - }, - ) + // 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{} } // Use the rest HTTP client for the provided config if unset diff --git a/pkg/client/client_suite_test.go b/pkg/client/client_suite_test.go index f3942502d3..89cab3f7ed 100644 --- a/pkg/client/client_suite_test.go +++ b/pkg/client/client_suite_test.go @@ -17,6 +17,8 @@ limitations under the License. package client_test import ( + "bytes" + "io" "testing" . "github.com/onsi/ginkgo/v2" @@ -24,6 +26,7 @@ import ( "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" + "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/examples/crd/pkg" "sigs.k8s.io/controller-runtime/pkg/envtest" @@ -36,12 +39,24 @@ func TestClient(t *testing.T) { RunSpecs(t, "Client Suite") } -var testenv *envtest.Environment -var cfg *rest.Config -var clientset *kubernetes.Clientset +var ( + testenv *envtest.Environment + cfg *rest.Config + clientset *kubernetes.Clientset + + // Used by tests to inspect controller and client log messages. + log bytes.Buffer +) var _ = BeforeSuite(func() { - logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) + // Forwards logs to ginkgo output, and allows tests to inspect logs. + mw := io.MultiWriter(&log, GinkgoWriter) + + // Use prefixes to help us tell the source of the log message. + // controller-runtime uses logf + logf.SetLogger(zap.New(zap.WriteTo(mw), zap.UseDevMode(true)).WithName("logf")) + // client-go logs uses klog + klog.SetLogger(zap.New(zap.WriteTo(mw), zap.UseDevMode(true)).WithName("klog")) testenv = &envtest.Environment{CRDDirectoryPaths: []string{"./testdata"}} diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 51dd66512e..2af2a6af11 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -17,11 +17,13 @@ limitations under the License. package client_test import ( + "bufio" "context" "encoding/json" "errors" "fmt" "reflect" + "strings" "sync/atomic" "time" @@ -226,6 +228,90 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC Expect(client.IgnoreNotFound(err)).NotTo(HaveOccurred()) }) + Describe("WarningHandler", func() { + It("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{}}}, + }) + Expect(err).NotTo(HaveOccurred()) + Expect(cl).NotTo(BeNil()) + + tns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "ws-disabled"}} + 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()) + + scanner := bufio.NewScanner(&log) + for scanner.Scan() { + line := scanner.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()) + + 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 + } + } + deleteNamespace(ctx, tns) + }) + }) + Describe("New", func() { It("should return a new Client", func() { cl, err := client.New(cfg, client.Options{})