Merge pull request #112120 from pohly/klog-flag-removal

logs: remove deprecated klog flags

hack/jenkins/benchmark-dockerized.sh

@@ -66,7 +66,8 @@ cd "${GOPATH}/src/k8s.io/kubernetes"
 ./hack/install-etcd.sh
 
 # Run the benchmark tests and pretty-print the results into a separate file.
-make test-integration WHAT="$*" KUBE_TEST_ARGS="-run='XXX' -bench=${TEST_PREFIX:-.} -benchtime=${BENCHTIME:-1s} -benchmem  -alsologtostderr=false -logtostderr=false -data-items-dir=${ARTIFACTS}" \
+# Log output of the tests go to stderr.
+make test-integration WHAT="$*" KUBE_TEST_ARGS="-run='XXX' -bench=${TEST_PREFIX:-.} -benchtime=${BENCHTIME:-1s} -benchmem  -data-items-dir=${ARTIFACTS}" \
   | (go run test/integration/benchmark/extractlog/main.go) \
   | tee \
    >(prettybench -no-passthrough > "${ARTIFACTS}/BenchmarkResults.txt") \

hack/make-rules/test-e2e-node.sh

@@ -162,7 +162,7 @@ if [ "${remote}" = true ] && [ "${remote_mode}" = gce ] ; then
   echo "Kubelet Config File: ${kubelet_config_file}"
 
   # Invoke the runner
-  go run test/e2e_node/runner/remote/run_remote.go  --logtostderr --vmodule=*=4 --ssh-env="gce" \
+  go run test/e2e_node/runner/remote/run_remote.go  --vmodule=*=4 --ssh-env="gce" \
     --zone="${zone}" --project="${project}" --gubernator="${gubernator}" \
     --hosts="${hosts}" --images="${images}" --cleanup="${cleanup}" \
     --results-dir="${artifacts}" --ginkgo-flags="${ginkgoflags}" --runtime-config="${runtime_config}" \
@@ -189,7 +189,7 @@ elif [ "${remote}" = true ] && [ "${remote_mode}" = ssh ] ; then
   test_args='--kubelet-flags="--cluster-domain='${KUBE_DNS_DOMAIN:-cluster.local}'" '${test_args}
 
   # Invoke the runner
-  go run test/e2e_node/runner/remote/run_remote.go  --mode="ssh" --logtostderr --vmodule=*=4 \
+  go run test/e2e_node/runner/remote/run_remote.go  --mode="ssh" --vmodule=*=4 \
     --hosts="${hosts}" --results-dir="${artifacts}" --ginkgo-flags="${ginkgoflags}" \
     --test_args="${test_args}" --system-spec-name="${system_spec_name}" \
     --runtime-config="${runtime_config}" \
@@ -222,7 +222,7 @@ else
   go run test/e2e_node/runner/local/run_local.go \
     --system-spec-name="${system_spec_name}" --extra-envs="${extra_envs}" \
     --ginkgo-flags="${ginkgoflags}" \
-    --test-flags="--alsologtostderr --v 4 --report-dir=${artifacts} --node-name $(hostname) ${test_args}" \
+    --test-flags="--v 4 --report-dir=${artifacts} --node-name $(hostname) ${test_args}" \
     --runtime-config="${runtime_config}" \
     --kubelet-config-file="${kubelet_config_file}" \
     --build-dependencies=true 2>&1 | tee -i "${artifacts}/build-log.txt"

hack/make-rules/test-integration.sh

@@ -78,7 +78,7 @@ runTests() {
   make -C "${KUBE_ROOT}" test \
       WHAT="${WHAT:-$(kube::test::find_integration_test_dirs | paste -sd' ' -)}" \
       GOFLAGS="${GOFLAGS:-}" \
-      KUBE_TEST_ARGS="--alsologtostderr=true ${SHORT:--short=true} --vmodule=${KUBE_TEST_VMODULE} ${KUBE_TEST_ARGS:-}" \
+      KUBE_TEST_ARGS="${SHORT:--short=true} --vmodule=${KUBE_TEST_VMODULE} ${KUBE_TEST_ARGS:-}" \
       KUBE_TIMEOUT="${KUBE_TIMEOUT}" \
       KUBE_RACE=""
 

hack/update-openapi-spec.sh

@@ -84,7 +84,6 @@ kube::log::status "Starting kube-apiserver"
   --service-account-lookup="${SERVICE_ACCOUNT_LOOKUP}" \
   --service-account-issuer="https://kubernetes.default.svc" \
   --service-account-signing-key-file="${SERVICE_ACCOUNT_KEY}" \
-  --logtostderr \
   --v=2 \
   --service-cluster-ip-range="10.0.0.0/24" >"${API_LOGFILE}" 2>&1 &
 APISERVER_PID=$!

staging/src/k8s.io/component-base/cli/globalflag/globalflags_test.go

@@ -62,7 +62,7 @@ func TestAddGlobalFlags(t *testing.T) {
 	}{
 		{
 			// Happy case
-			expectedFlag:  []string{"add-dir-header", "alsologtostderr", "help", "log-backtrace-at", "log-dir", "log-file", "log-file-max-size", "log-flush-frequency", "logtostderr", "one-output", "skip-headers", "skip-log-headers", "stderrthreshold", "v", "vmodule"},
+			expectedFlag:  []string{"help", "log-flush-frequency", "v", "vmodule"},
 			matchExpected: false,
 		},
 		{

staging/src/k8s.io/component-base/logs/api/v1/options.go

@@ -20,7 +20,6 @@ import (
 	"flag"
 	"fmt"
 	"math"
-	"sort"
 	"strings"
 	"time"
 
@@ -32,6 +31,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	cliflag "k8s.io/component-base/cli/flag"
 	"k8s.io/component-base/featuregate"
+	"k8s.io/component-base/logs/klogflags"
 )
 
 const (
@@ -183,12 +183,8 @@ func apply(c *LoggingConfiguration, featureGate featuregate.FeatureGate) error {
 
 // AddFlags adds command line flags for the configuration.
 func AddFlags(c *LoggingConfiguration, fs *pflag.FlagSet) {
-	// The help text is generated assuming that flags will eventually use
-	// hyphens, even if currently no normalization function is set for the
-	// flag set yet.
-	unsupportedFlags := strings.Join(unsupportedLoggingFlagNames(cliflag.WordSepNormalizeFunc), ", ")
 	formats := logRegistry.list()
-	fs.StringVar(&c.Format, "logging-format", c.Format, fmt.Sprintf("Sets the log format. Permitted formats: %s.\nNon-default formats don't honor these flags: %s.\nNon-default choices are currently alpha and subject to change without warning.", formats, unsupportedFlags))
+	fs.StringVar(&c.Format, "logging-format", c.Format, fmt.Sprintf("Sets the log format. Permitted formats: %s.", formats))
 	// No new log formats should be added after generation is of flag options
 	logRegistry.freeze()
 
@@ -236,14 +232,13 @@ var loggingFlags pflag.FlagSet
 
 func init() {
 	var fs flag.FlagSet
-	klog.InitFlags(&fs)
+	klogflags.Init(&fs)
 	loggingFlags.AddGoFlagSet(&fs)
 }
 
 // List of logs (k8s.io/klog + k8s.io/component-base/logs) flags supported by all logging formats
 var supportedLogsFlags = map[string]struct{}{
 	"v": {},
-	// TODO: support vmodule after 1.19 Alpha
 }
 
 // unsupportedLoggingFlags lists unsupported logging flags. The normalize
@@ -268,15 +263,3 @@ func unsupportedLoggingFlags(normalizeFunc func(f *pflag.FlagSet, name string) p
 	})
 	return allFlags
 }
-
-// unsupportedLoggingFlagNames lists unsupported logging flags by name, with
-// optional normalization and sorted.
-func unsupportedLoggingFlagNames(normalizeFunc func(f *pflag.FlagSet, name string) pflag.NormalizedName) []string {
-	unsupportedFlags := unsupportedLoggingFlags(normalizeFunc)
-	names := make([]string, 0, len(unsupportedFlags))
-	for _, f := range unsupportedFlags {
-		names = append(names, "--"+f.Name)
-	}
-	sort.Strings(names)
-	return names
-}

staging/src/k8s.io/component-base/logs/api/v1/options_test.go

@@ -39,9 +39,7 @@ func TestFlags(t *testing.T) {
 	fs.SetOutput(&output)
 	fs.PrintDefaults()
 	want := `      --log-flush-frequency duration   Maximum number of seconds between log flushes (default 5s)
-      --logging-format string          Sets the log format. Permitted formats: "text".
-                                       Non-default formats don't honor these flags: --add-dir-header, --alsologtostderr, --log-backtrace-at, --log-dir, --log-file, --log-file-max-size, --logtostderr, --one-output, --skip-headers, --skip-log-headers, --stderrthreshold, --vmodule.
-                                       Non-default choices are currently alpha and subject to change without warning. (default "text")
+      --logging-format string          Sets the log format. Permitted formats: "text". (default "text")
   -v, --v Level                        number for the log level verbosity
       --vmodule pattern=N,...          comma-separated list of pattern=N settings for file-filtered logging (only works for text log format)
 `

staging/src/k8s.io/component-base/logs/klogflags/klogflags.go

@@ -0,0 +1,41 @@
+/*
+Copyright 2022 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package klogflags
+
+import (
+	"flag"
+
+	"k8s.io/klog/v2"
+)
+
+// Init is a replacement for klog.InitFlags which only adds those flags
+// that are still supported for Kubernetes components (i.e. -v and -vmodule).
+// See
+// https://github.com/kubernetes/enhancements/tree/master/keps/sig-instrumentation/2845-deprecate-klog-specific-flags-in-k8s-components.
+func Init(fs *flag.FlagSet) {
+	var allFlags flag.FlagSet
+	klog.InitFlags(&allFlags)
+	if fs == nil {
+		fs = flag.CommandLine
+	}
+	allFlags.VisitAll(func(f *flag.Flag) {
+		switch f.Name {
+		case "v", "vmodule":
+			fs.Var(f.Value, f.Name, f.Usage)
+		}
+	})
+}

staging/src/k8s.io/component-base/logs/logs.go

@@ -27,16 +27,11 @@ import (
 
 	"github.com/spf13/pflag"
 	logsapi "k8s.io/component-base/logs/api/v1"
+	"k8s.io/component-base/logs/klogflags"
 	"k8s.io/klog/v2"
 )
 
-const deprecated = "will be removed in a future release, see https://github.com/kubernetes/enhancements/tree/master/keps/sig-instrumentation/2845-deprecate-klog-specific-flags-in-k8s-components"
-
-// TODO (https://github.com/kubernetes/kubernetes/issues/105310): once klog
-// flags are removed, stop warning about "Non-default formats don't honor these
-// flags" in config.go and instead add this remark here.
-//
-// const vmoduleUsage = " (only works for the default text log format)"
+const vmoduleUsage = " (only works for the default text log format)"
 
 var (
 	packageFlags = flag.NewFlagSet("logging", flag.ContinueOnError)
@@ -47,7 +42,7 @@ var (
 )
 
 func init() {
-	klog.InitFlags(packageFlags)
+	klogflags.Init(packageFlags)
 	packageFlags.DurationVar(&logFlushFreq, logsapi.LogFlushFreqFlagName, logsapi.LogFlushFreqDefault, "Maximum number of seconds between log flushes")
 }
 
@@ -81,42 +76,29 @@ var NewOptions = logsapi.NewLoggingConfiguration
 //
 // May be called more than once.
 func AddFlags(fs *pflag.FlagSet, opts ...Option) {
-	// Determine whether the flags are already present by looking up one
-	// which always should exist.
-	if fs.Lookup("logtostderr") != nil {
-		return
-	}
-
 	o := addFlagsOptions{}
 	for _, opt := range opts {
 		opt(&o)
 	}
 
-	// Add flags with pflag deprecation remark for some klog flags.
+	// Add all supported flags.
 	packageFlags.VisitAll(func(f *flag.Flag) {
 		pf := pflag.PFlagFromGoFlag(f)
 		switch f.Name {
-		case "v":
-			// unchanged, potentially skip it
-			if o.skipLoggingConfigurationFlags {
-				return
-			}
-		case logsapi.LogFlushFreqFlagName:
+		case "v", logsapi.LogFlushFreqFlagName:
 			// unchanged, potentially skip it
 			if o.skipLoggingConfigurationFlags {
 				return
 			}
 		case "vmodule":
-			// TODO: see above
-			// pf.Usage += vmoduleUsage
 			if o.skipLoggingConfigurationFlags {
 				return
 			}
-		default:
-			// deprecated, but not hidden
-			pf.Deprecated = deprecated
+			pf.Usage += vmoduleUsage
+		}
+		if fs.Lookup(pf.Name) == nil {
+			fs.AddFlag(pf)
 		}
-		fs.AddFlag(pf)
 	})
 }
 
@@ -137,24 +119,16 @@ func AddGoFlags(fs *flag.FlagSet, opts ...Option) {
 	packageFlags.VisitAll(func(f *flag.Flag) {
 		usage := f.Usage
 		switch f.Name {
-		case "v":
-			// unchanged
-			if o.skipLoggingConfigurationFlags {
-				return
-			}
-		case logsapi.LogFlushFreqFlagName:
+		case "v", logsapi.LogFlushFreqFlagName:
 			// unchanged
 			if o.skipLoggingConfigurationFlags {
 				return
 			}
 		case "vmodule":
-			// TODO: see above
-			// usage += vmoduleUsage
 			if o.skipLoggingConfigurationFlags {
 				return
 			}
-		default:
-			usage += " (DEPRECATED: " + deprecated + ")"
+			usage += vmoduleUsage
 		}
 		fs.Var(f.Value, f.Name, usage)
 	})

test/e2e_node/conformance/run_test.sh

@@ -201,7 +201,6 @@ start_kubelet --kubeconfig "${KUBELET_KUBECONFIG}" \
   --system-cgroups=/system \
   --cgroup-root=/ \
   --v=$log_level \
-  --logtostderr
 
 wait_kubelet
 

test/e2e_node/jenkins/conformance/conformance-jenkins.sh

@@ -33,7 +33,7 @@ TIMEOUT=${TIMEOUT:-"45m"}
 mkdir -p "${ARTIFACTS}"
 
 go run test/e2e_node/runner/remote/run_remote.go  --test-suite=conformance \
-  --logtostderr --vmodule=*=4 --ssh-env="gce" --ssh-user="$GCE_USER" \
+  --vmodule=*=4 --ssh-env="gce" --ssh-user="$GCE_USER" \
   --zone="$GCE_ZONE" --project="$GCE_PROJECT" --hosts="$GCE_HOSTS" \
   --images="$GCE_IMAGES" --image-project="$GCE_IMAGE_PROJECT" \
   --image-config-file="$GCE_IMAGE_CONFIG_PATH" --cleanup="$CLEANUP" \

test/e2e_node/remote/node_conformance.go

@@ -181,7 +181,7 @@ func launchKubelet(host, workspace, results, testArgs, bearerToken string) error
 		return fmt.Errorf("failed to create kubelet pod manifest path %q: error - %v output - %q",
 			podManifestPath, err, output)
 	}
-	startKubeletCmd := fmt.Sprintf("./%s --run-kubelet-mode --logtostderr --node-name=%s"+
+	startKubeletCmd := fmt.Sprintf("./%s --run-kubelet-mode --node-name=%s"+
 		" --bearer-token=%s"+
 		" --report-dir=%s %s --kubelet-flags=--pod-manifest-path=%s > %s 2>&1",
 		conformanceTestBinary, host, bearerToken, results, testArgs, podManifestPath, filepath.Join(results, kubeletLauncherLog))

test/e2e_node/remote/node_e2e.go

@@ -200,7 +200,7 @@ func (n *NodeE2ERemote) RunTest(host, workspace, results, imageDesc, junitFilePr
 	klog.V(2).Infof("Starting tests on %q", host)
 	cmd := getSSHCommand(" && ",
 		fmt.Sprintf("cd %s", workspace),
-		fmt.Sprintf("timeout -k 30s %fs ./ginkgo %s ./e2e_node.test -- --system-spec-name=%s --system-spec-file=%s --extra-envs=%s --runtime-config=%s --logtostderr --v 4 --node-name=%s --report-dir=%s --report-prefix=%s --image-description=\"%s\" %s",
+		fmt.Sprintf("timeout -k 30s %fs ./ginkgo %s ./e2e_node.test -- --system-spec-name=%s --system-spec-file=%s --extra-envs=%s --runtime-config=%s --v 4 --node-name=%s --report-dir=%s --report-prefix=%s --image-description=\"%s\" %s",
 			timeout.Seconds(), ginkgoArgs, systemSpecName, systemSpecFile, extraEnvs, runtimeConfig, host, results, junitFilePrefix, imageDesc, testArgs),
 	)
 	return SSH(host, "sh", "-c", cmd)

test/e2e_node/runner/remote/run_remote.go

@@ -15,9 +15,9 @@ limitations under the License.
 */
 
 // To run the node e2e tests remotely against one or more hosts on gce:
-// $ go run run_remote.go --logtostderr --v 2 --ssh-env gce --hosts <comma separated hosts>
+// $ go run run_remote.go --v 2 --ssh-env gce --hosts <comma separated hosts>
 // To run the node e2e tests remotely against one or more images on gce and provision them:
-// $ go run run_remote.go --logtostderr --v 2 --project <project> --zone <zone> --ssh-env gce --images <comma separated images>
+// $ go run run_remote.go --v 2 --project <project> --zone <zone> --ssh-env gce --images <comma separated images>
 package main
 
 import (

test/e2e_node/services/kubelet.go

@@ -252,7 +252,7 @@ func (e *E2EServices) startKubelet(featureGates map[string]bool) (*server, error
 	cmdArgs = append(cmdArgs,
 		"--kubeconfig", kubeconfigPath,
 		"--root-dir", KubeletRootDirectory,
-		"--v", LogVerbosityLevel, "--logtostderr",
+		"--v", LogVerbosityLevel,
 	)
 
 	// Apply test framework feature gates by default. This could also be overridden

test/integration/scheduler_perf/README.md

@@ -32,7 +32,7 @@ How To Run
 
 ```shell
 # In Kubernetes root path
-make test-integration WHAT=./test/integration/scheduler_perf ETCD_LOGLEVEL=warn KUBE_TEST_VMODULE="''" KUBE_TEST_ARGS="-alsologtostderr=false -logtostderr=false -run=^$$ -benchtime=1ns -bench=BenchmarkPerfScheduling"
+make test-integration WHAT=./test/integration/scheduler_perf ETCD_LOGLEVEL=warn KUBE_TEST_VMODULE="''" KUBE_TEST_ARGS="-run=^$$ -benchtime=1ns -bench=BenchmarkPerfScheduling"
 ```
 
 The benchmark suite runs all the tests specified under config/performance-config.yaml.
@@ -47,14 +47,14 @@ Otherwise, the golang benchmark framework will try to run a test more than once
 
 ```shell
 # In Kubernetes root path
-make test-integration WHAT=./test/integration/scheduler_perf ETCD_LOGLEVEL=warn KUBE_TEST_VMODULE="''" KUBE_TEST_ARGS="-alsologtostderr=false -logtostderr=false -run=^$$ -benchtime=1ns -bench=BenchmarkPerfScheduling/SchedulingBasic/5000Nodes/5000InitPods/1000PodsToSchedule"
+make test-integration WHAT=./test/integration/scheduler_perf ETCD_LOGLEVEL=warn KUBE_TEST_VMODULE="''" KUBE_TEST_ARGS="-run=^$$ -benchtime=1ns -bench=BenchmarkPerfScheduling/SchedulingBasic/5000Nodes/5000InitPods/1000PodsToSchedule"
 ```
 
 To produce a cpu profile:
 
 ```shell
 # In Kubernetes root path
-make test-integration WHAT=./test/integration/scheduler_perf KUBE_TIMEOUT="-timeout=3600s" ETCD_LOGLEVEL=warn KUBE_TEST_VMODULE="''" KUBE_TEST_ARGS="-alsologtostderr=false -logtostderr=false -run=^$$ -benchtime=1ns -bench=BenchmarkPerfScheduling -cpuprofile ~/cpu-profile.out"
+make test-integration WHAT=./test/integration/scheduler_perf KUBE_TIMEOUT="-timeout=3600s" ETCD_LOGLEVEL=warn KUBE_TEST_VMODULE="''" KUBE_TEST_ARGS="-run=^$$ -benchtime=1ns -bench=BenchmarkPerfScheduling -cpuprofile ~/cpu-profile.out"
 ```
 
 ### How to configure benchmark tests

vendor/modules.txt

@@ -2014,6 +2014,7 @@ k8s.io/component-base/logs
 k8s.io/component-base/logs/api/v1
 k8s.io/component-base/logs/json
 k8s.io/component-base/logs/json/register
+k8s.io/component-base/logs/klogflags
 k8s.io/component-base/logs/logreduction
 k8s.io/component-base/logs/testinit
 k8s.io/component-base/metrics