cli: give up and suppress klog output by default (#6288)

i don't think these logs have ever caught a real problem,
and are often confusing / inscrutable. they're really meant
for people writing k8s controllers, and i don't think they
make sense to show to tilt users.

Signed-off-by: Nick Santos <nick.santos@docker.com>

internal/cli/klog.go

@@ -6,32 +6,13 @@ import (
 	"io"
 	"log"
 	"os"
-	"regexp"
-
-	"github.com/tilt-dev/tilt/internal/filteredwriter"
 
 	"k8s.io/klog/v2"
 )
 
 // https://kubernetes.io/docs/reference/kubectl/cheatsheet/#kubectl-output-verbosity-and-debugging
 var klogLevel = 0
 
-// everything on google indicates this warning is useless and should be ignored
-// https://github.com/kubernetes/kubernetes/issues/22024
-var isResourceVersionTooOldRegexp = regexp.MustCompile("reflector.go.*watch of.*ended with: too old resource version")
-
-// isGroupVersionEmptyRegexp matches errors that are logged deep within K8s when
-// populating the cache of resource types for a group-version if the group-version
-// has no resource types registered. This is an uncommon but valid scenario that
-// most commonly occurs with prometheus-adapter and the `external.metrics.k8s.io/v1beta1`
-// group if no external metrics are actually registered (but the group will still
-// exist). The K8s code returns an error instead of treating it as empty, which gets
-// logged at an error level so will show up in Tilt logs and leads to confusion,
-// particularly on `tilt down` where they show up mixed in the CLI output.
-// https://github.com/kubernetes/kubernetes/blob/a4e5239bdc3d85f1f90c7811b03dc153d5121ffe/staging/src/k8s.io/client-go/discovery/cached/memory/memcache.go#L212-L221
-// https://github.com/kubernetes/kubernetes/issues/92480
-var isGroupVersionEmptyRegexp = regexp.MustCompile("couldn't get resource list for.*Got empty response")
-
 // HACK(nick): The Kubernetes libs we use sometimes use klog to log things to
 // os.Stderr. There are no API hooks to configure this. And printing to Stderr
 // scrambles the HUD tty.
@@ -41,13 +22,7 @@ var isGroupVersionEmptyRegexp = regexp.MustCompile("couldn't get resource list f
 func initKlog(w io.Writer) {
 	var tmpFlagSet = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
 	klog.InitFlags(tmpFlagSet)
-	if klogLevel == 0 {
-		w = filteredwriter.New(w, filterMux(
-			isResourceVersionTooOldRegexp.MatchString,
-			isGroupVersionEmptyRegexp.MatchString,
-		))
-	}
-	klog.SetOutput(w)
+	maybeSetKlogOutput(w)
 
 	flags := []string{
 		"--stderrthreshold=FATAL",
@@ -64,14 +39,33 @@ func initKlog(w io.Writer) {
 	}
 }
 
-// filterMux combines multiple filter functions, returning true if any match.
-func filterMux(filterFuncs ...func(s string) bool) func(s string) bool {
-	return func(s string) bool {
-		for _, fn := range filterFuncs {
-			if fn(s) {
-				return true
-			}
-		}
-		return false
+// We've historically had a lot of problems with bad klog output. For example:
+//
+// everything on google indicates this warning is useless and should be ignored
+// https://github.com/kubernetes/kubernetes/issues/22024
+//
+// errors that are logged deep within K8s when populating the cache of resource
+// types for a group-version if the group-version has no resource types
+// registered. This is an uncommon but valid scenario that most commonly occurs
+// with prometheus-adapter and the `external.metrics.k8s.io/v1beta1` group if no
+// external metrics are actually registered (but the group will still
+// exist). The K8s code returns an error instead of treating it as empty, which
+// gets logged at an error level so will show up in Tilt logs and leads to
+// confusion, particularly on `tilt down` where they show up mixed in the CLI
+// output.
+// https://github.com/kubernetes/kubernetes/blob/a4e5239bdc3d85f1f90c7811b03dc153d5121ffe/staging/src/k8s.io/client-go/discovery/cached/memory/memcache.go#L212-L221
+// https://github.com/kubernetes/kubernetes/issues/92480
+//
+// informer errors when CRDs are removed
+// https://github.com/kubernetes/kubernetes/issues/79610
+//
+// We're not convinced that controller-runtime logs EVER make sense to show to
+// tilt users, so let's filter them out by default. Users can turn them on
+// with --klog if they want.
+func maybeSetKlogOutput(w io.Writer) {
+	if klogLevel == 0 {
+		klog.SetOutput(io.Discard)
+	} else {
+		klog.SetOutput(w)
 	}
 }

internal/cli/klog_test.go

@@ -18,8 +18,8 @@ func TestResourceVersionTooOldWarningsSilenced(t *testing.T) {
 
 	PrintWatchEndedWarning()
 	klog.Flush()
-	assert.Contains(t, out.String(), "klog_test.go")
-	assert.Contains(t, out.String(), "watch ended")
+	assert.Contains(t, out.String(), "")
+	assert.Contains(t, out.String(), "")
 }
 
 func TestResourceVersionTooOldWarningsPrinted(t *testing.T) {

internal/cli/up.go

@@ -13,7 +13,6 @@ import (
 
 	"github.com/mattn/go-isatty"
 	"github.com/spf13/cobra"
-	"k8s.io/klog/v2"
 
 	"github.com/tilt-dev/tilt/internal/analytics"
 	engineanalytics "github.com/tilt-dev/tilt/internal/engine/analytics"
@@ -177,7 +176,7 @@ func (c *upCmd) run(ctx context.Context, args []string) error {
 func redirectLogs(ctx context.Context, l logger.Logger) context.Context {
 	ctx = logger.WithLogger(ctx, l)
 	log.SetOutput(l.Writer(logger.InfoLvl))
-	klog.SetOutput(l.Writer(logger.InfoLvl))
+	maybeSetKlogOutput(l.Writer(logger.InfoLvl))
 	return ctx
 }