From ff541e79246dd798de73a93d450ad8421bed3e65 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Fri, 12 Apr 2024 11:01:42 +0200 Subject: [PATCH 1/2] e2e node: fix -v support Since 43539c855f59f3318de73602d091c62ee5863cda (first released in v1.30.0-alpha.2), the test/e2e/framework manages -v and -vmodule and uses them for a logger which writes to the Ginkgo output stream. This did not work for test/e2e_node, because: - logs.AddFlags(pflag.CommandLine) registers its own -v and -vmodule flags - pflag.CommandLine.AddGoFlagSet(flag.CommandLine) skips the corresponding flags in the flag.CommandLine - pflag.Parse() initializes the settings in the "logs" package even though those are not used at runtime The solution is to not use the "logs" package. --- test/e2e_node/e2e_node_suite_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/e2e_node/e2e_node_suite_test.go b/test/e2e_node/e2e_node_suite_test.go index cd559b1b97c..0bd8f9d4066 100644 --- a/test/e2e_node/e2e_node_suite_test.go +++ b/test/e2e_node/e2e_node_suite_test.go @@ -41,7 +41,6 @@ import ( utilfeature "k8s.io/apiserver/pkg/util/feature" clientset "k8s.io/client-go/kubernetes" cliflag "k8s.io/component-base/cli/flag" - "k8s.io/component-base/logs" "k8s.io/kubernetes/pkg/util/rlimit" commontest "k8s.io/kubernetes/test/e2e/common" "k8s.io/kubernetes/test/e2e/framework" @@ -123,7 +122,6 @@ func TestMain(m *testing.M) { e2econfig.CopyFlags(e2econfig.Flags, flag.CommandLine) framework.RegisterCommonFlags(flag.CommandLine) registerNodeFlags(flag.CommandLine) - logs.AddFlags(pflag.CommandLine) pflag.CommandLine.AddGoFlagSet(flag.CommandLine) // Mark the run-services-mode flag as hidden to prevent user from using it. pflag.CommandLine.MarkHidden("run-services-mode") From feb27b9907d002d2a16ef0a33d89a964c607c15f Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Mon, 15 Apr 2024 10:10:56 +0200 Subject: [PATCH 2/2] e2e framework: configure Ginkgo logger and klog consistently Even if the textlogger which writes to Ginkgo is installed as the logger in klog, klog still does some verbosity checks itself (for example, klog.V().Enabled). Therefore the framework has to keep the verbosity settings in the textlogger and in klog consistent. This is done by wrapping the Set call instead of replacing it. --- test/e2e/framework/ginkgologger.go | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/test/e2e/framework/ginkgologger.go b/test/e2e/framework/ginkgologger.go index 05f22eff387..6e2c9569970 100644 --- a/test/e2e/framework/ginkgologger.go +++ b/test/e2e/framework/ginkgologger.go @@ -49,14 +49,16 @@ var ( func init() { // ktesting and testinit already registered the -v and -vmodule - // command line flags. To configure the textlogger instead, we - // need to swap out the flag.Value for those. + // command line flags. To configure the textlogger and klog + // consistently, we need to intercept the Set call. This + // can be done by swapping out the flag.Value for the -v and + // -vmodule flags with a wrapper which calls both. var fs flag.FlagSet logConfig.AddFlags(&fs) fs.VisitAll(func(loggerFlag *flag.Flag) { klogFlag := flag.CommandLine.Lookup(loggerFlag.Name) if klogFlag != nil { - klogFlag.Value = loggerFlag.Value + klogFlag.Value = &valueChain{Value: loggerFlag.Value, parentValue: klogFlag.Value} } }) @@ -75,6 +77,21 @@ func init() { klog.SetLoggerWithOptions(ginkgoLogger, opts...) } +type valueChain struct { + flag.Value + parentValue flag.Value +} + +func (v *valueChain) Set(value string) error { + if err := v.Value.Set(value); err != nil { + return err + } + if err := v.parentValue.Set(value); err != nil { + return err + } + return nil +} + func unwind(skip int) (string, int) { location := ginkgotypes.NewCodeLocation(skip + 1) return location.FileName, location.LineNumber