initialize logging after flag parsing

It wasn't documented that InitLogs already uses the log flush frequency, so
some commands have called it before parsing (for example, kubectl in the
original code for logs.go). The flag never had an effect in such commands.

Fixing this turned into a major refactoring of how commands set up flags and
run their Cobra command:

- component-base/logs: implicitely registering flags during package init is an
  anti-pattern that makes it impossible to use the package in commands which
  want full control over their command line. Logging flags must be added
  explicitly now, something that the new cli.Run does automatically.

- component-base/logs: AddFlags would have crashed in kubectl-convert if it
  had been called because it relied on the global pflag.CommandLine. This
  has been fixed and kubectl-convert now has the same --log-flush-frequency
  flag as other commands.

- component-base/logs/testinit: an exception are tests where flag.CommandLine has
  to be used. This new package can be imported to add flags to that
  once per test program.

- Normalization of the klog command line flags was inconsistent. Some commands
  unintentionally didn't normalize to the recommended format with hyphens. This
  gets fixed for sample programs, but not for production programs because
  it would be a breaking change.

This refactoring has the following user-visible effects:

- The validation error for `go run ./cmd/kube-apiserver --logging-format=json
  --add-dir-header` now references `add-dir-header` instead of `add_dir_header`.

- `staging/src/k8s.io/cloud-provider/sample` uses flags with hyphen instead of
  underscore.

- `--log-flush-frequency` is not listed anymore in the --logging-format flag's
  `non-default formats don't honor these flags` usage text because it will also
  work for non-default formats once it is needed.

- `cmd/kubelet`: the description of `--logging-format` uses hyphens instead of
  underscores for the flags, which now matches what the command is using.

- `staging/src/k8s.io/component-base/logs/example/cmd`: added logging flags.

- `apiextensions-apiserver` no longer prints a useless stack trace for `main`
  when command line parsing raises an error.
This commit is contained in:
Patrick Ohly
2021-09-16 18:18:35 +02:00
parent c9c4357c03
commit 21d1bcd6b8
42 changed files with 270 additions and 386 deletions

View File

@@ -27,7 +27,6 @@ import (
// libs that provide registration functions
"k8s.io/component-base/logs"
"k8s.io/component-base/version/verflag"
"k8s.io/klog/v2"
// ensure libs have a chance to globally register their flags
_ "k8s.io/kubernetes/pkg/credentialprovider/azure"
@@ -38,7 +37,6 @@ import (
// against the global flagsets from "flag" and "github.com/spf13/pflag".
// We do this in order to prevent unwanted flags from leaking into the Kubelet's flagset.
func AddGlobalFlags(fs *pflag.FlagSet) {
addKlogFlags(fs)
addCadvisorFlags(fs)
addCredentialProviderFlags(fs)
verflag.AddFlags(fs)
@@ -88,10 +86,3 @@ func addCredentialProviderFlags(fs *pflag.FlagSet) {
fs.AddFlagSet(local)
}
// addKlogFlags adds flags from k8s.io/klog
func addKlogFlags(fs *pflag.FlagSet) {
local := flag.NewFlagSet(os.Args[0], flag.ExitOnError)
klog.InitFlags(local)
fs.AddGoFlagSet(local)
}

View File

@@ -383,6 +383,7 @@ func (f *KubeletFlags) AddFlags(mainfs *pflag.FlagSet) {
// AddKubeletConfigFlags adds flags for a specific kubeletconfig.KubeletConfiguration to the specified FlagSet
func AddKubeletConfigFlags(mainfs *pflag.FlagSet, c *kubeletconfig.KubeletConfiguration) {
fs := pflag.NewFlagSet("", pflag.ExitOnError)
fs.SetNormalizeFunc(mainfs.GetNormalizeFunc())
defer func() {
// All KubeletConfiguration flags are now deprecated, and any new flags that point to
// KubeletConfiguration fields are deprecated-on-creation. When removing flags at the end

View File

@@ -259,6 +259,9 @@ HTTP server: The kubelet can also listen for HTTP and respond to a simple API
}
}
// Config and flags parsed, now we can initialize logging.
logs.InitLogs()
// construct a KubeletServer from kubeletFlags and kubeletConfig
kubeletServer := &options.KubeletServer{
KubeletFlags: *kubeletFlags,

View File

@@ -26,6 +26,9 @@ import (
"os"
"time"
"github.com/spf13/cobra"
cliflag "k8s.io/component-base/cli/flag"
"k8s.io/component-base/logs"
_ "k8s.io/component-base/logs/json/register" // for JSON log format registration
_ "k8s.io/component-base/metrics/prometheus/restclient"
@@ -34,13 +37,23 @@ import (
)
func main() {
command := app.NewKubeletCommand()
// kubelet uses a config file and does its own special
// parsing of flags and that config file. It initializes
// logging after it is done with that. Therefore it does
// not use cli.Run like other, simpler commands.
code := run(command)
os.Exit(code)
}
func run(command *cobra.Command) int {
defer logs.FlushLogs()
rand.Seed(time.Now().UnixNano())
command := app.NewKubeletCommand()
logs.InitLogs()
defer logs.FlushLogs()
command.SetGlobalNormalizationFunc(cliflag.WordSepNormalizeFunc)
if err := command.Execute(); err != nil {
os.Exit(1)
return 1
}
return 0
}