From fafe98b4f003be83586df426dd79e8d8d07a6709 Mon Sep 17 00:00:00 2001 From: hongming Date: Wed, 10 May 2023 14:01:46 +0800 Subject: [PATCH] refactor: remove usless options (#5671) refactor: remove useless options --- cmd/controller-manager/app/options/options.go | 32 ++-- cmd/controller-manager/app/server.go | 9 -- cmd/ks-apiserver/apiserver.go | 11 +- cmd/ks-apiserver/app/options/options.go | 71 ++------- cmd/ks-apiserver/app/server.go | 2 +- pkg/apiserver/apiserver.go | 3 - pkg/simple/client/cache/cache.go | 8 +- pkg/simple/client/cache/inmemory_cache.go | 7 +- pkg/simple/client/cache/options.go | 2 +- pkg/simple/client/s3/s3.go | 8 + vendor/k8s.io/component-base/cli/OWNERS | 12 ++ vendor/k8s.io/component-base/cli/run.go | 148 ++++++++++++++++++ vendor/modules.txt | 1 + 13 files changed, 208 insertions(+), 106 deletions(-) create mode 100644 vendor/k8s.io/component-base/cli/OWNERS create mode 100644 vendor/k8s.io/component-base/cli/run.go diff --git a/cmd/controller-manager/app/options/options.go b/cmd/controller-manager/app/options/options.go index 8298d4eb1..da22a09e3 100644 --- a/cmd/controller-manager/app/options/options.go +++ b/cmd/controller-manager/app/options/options.go @@ -40,14 +40,12 @@ import ( "kubesphere.io/kubesphere/pkg/simple/client/multicluster" "kubesphere.io/kubesphere/pkg/simple/client/network" "kubesphere.io/kubesphere/pkg/simple/client/openpitrix" - "kubesphere.io/kubesphere/pkg/simple/client/s3" "kubesphere.io/kubesphere/pkg/simple/client/servicemesh" ) type KubeSphereControllerManagerOptions struct { KubernetesOptions *k8s.KubernetesOptions DevopsOptions *jenkins.Options - S3Options *s3.Options AuthenticationOptions *authentication.Options LdapOptions *ldapclient.Options OpenPitrixOptions *openpitrix.Options @@ -88,7 +86,6 @@ func NewKubeSphereControllerManagerOptions() *KubeSphereControllerManagerOptions s := &KubeSphereControllerManagerOptions{ KubernetesOptions: k8s.NewKubernetesOptions(), DevopsOptions: jenkins.NewDevopsOptions(), - S3Options: s3.NewS3Options(), LdapOptions: ldapclient.NewOptions(), OpenPitrixOptions: openpitrix.NewOptions(), NetworkOptions: network.NewNetworkOptions(), @@ -116,7 +113,6 @@ func (s *KubeSphereControllerManagerOptions) Flags(allControllerNameSelectors [] s.KubernetesOptions.AddFlags(fss.FlagSet("kubernetes"), s.KubernetesOptions) s.DevopsOptions.AddFlags(fss.FlagSet("devops"), s.DevopsOptions) - s.S3Options.AddFlags(fss.FlagSet("s3"), s.S3Options) s.AuthenticationOptions.AddFlags(fss.FlagSet("authentication"), s.AuthenticationOptions) s.LdapOptions.AddFlags(fss.FlagSet("ldap"), s.LdapOptions) s.OpenPitrixOptions.AddFlags(fss.FlagSet("openpitrix"), s.OpenPitrixOptions) @@ -161,20 +157,19 @@ func (s *KubeSphereControllerManagerOptions) Flags(allControllerNameSelectors [] } // Validate Options and Genetic Options -func (o *KubeSphereControllerManagerOptions) Validate(allControllerNameSelectors []string) []error { +func (s *KubeSphereControllerManagerOptions) Validate(allControllerNameSelectors []string) []error { var errs []error - errs = append(errs, o.DevopsOptions.Validate()...) - errs = append(errs, o.KubernetesOptions.Validate()...) - errs = append(errs, o.S3Options.Validate()...) - errs = append(errs, o.OpenPitrixOptions.Validate()...) - errs = append(errs, o.NetworkOptions.Validate()...) - errs = append(errs, o.LdapOptions.Validate()...) - errs = append(errs, o.MultiClusterOptions.Validate()...) - errs = append(errs, o.AlertingOptions.Validate()...) + errs = append(errs, s.DevopsOptions.Validate()...) + errs = append(errs, s.KubernetesOptions.Validate()...) + errs = append(errs, s.OpenPitrixOptions.Validate()...) + errs = append(errs, s.NetworkOptions.Validate()...) + errs = append(errs, s.LdapOptions.Validate()...) + errs = append(errs, s.MultiClusterOptions.Validate()...) + errs = append(errs, s.AlertingOptions.Validate()...) // genetic option: application-selector - if len(o.ApplicationSelector) != 0 { - _, err := labels.Parse(o.ApplicationSelector) + if len(s.ApplicationSelector) != 0 { + _, err := labels.Parse(s.ApplicationSelector) if err != nil { errs = append(errs, err) } @@ -182,7 +177,7 @@ func (o *KubeSphereControllerManagerOptions) Validate(allControllerNameSelectors // genetic option: controllers, check all selectors are valid allControllersNameSet := sets.New(allControllerNameSelectors...) - for _, selector := range o.ControllerGates { + for _, selector := range s.ControllerGates { if selector == "*" { continue } @@ -196,9 +191,9 @@ func (o *KubeSphereControllerManagerOptions) Validate(allControllerNameSelectors } // IsControllerEnabled check if a specified controller enabled or not. -func (o *KubeSphereControllerManagerOptions) IsControllerEnabled(name string) bool { +func (s *KubeSphereControllerManagerOptions) IsControllerEnabled(name string) bool { hasStar := false - for _, ctrl := range o.ControllerGates { + for _, ctrl := range s.ControllerGates { if ctrl == name { return true } @@ -234,7 +229,6 @@ func (s *KubeSphereControllerManagerOptions) bindLeaderElectionFlags(l *leaderel func (s *KubeSphereControllerManagerOptions) MergeConfig(cfg *controllerconfig.Config) { s.KubernetesOptions = cfg.KubernetesOptions s.DevopsOptions = cfg.DevopsOptions - s.S3Options = cfg.S3Options s.AuthenticationOptions = cfg.AuthenticationOptions s.LdapOptions = cfg.LdapOptions s.OpenPitrixOptions = cfg.OpenPitrixOptions diff --git a/cmd/controller-manager/app/server.go b/cmd/controller-manager/app/server.go index 4804d1c56..c513fc457 100644 --- a/cmd/controller-manager/app/server.go +++ b/cmd/controller-manager/app/server.go @@ -44,7 +44,6 @@ import ( "kubesphere.io/kubesphere/pkg/controller/user" "kubesphere.io/kubesphere/pkg/informers" "kubesphere.io/kubesphere/pkg/simple/client/k8s" - "kubesphere.io/kubesphere/pkg/simple/client/s3" "kubesphere.io/kubesphere/pkg/utils/metrics" "kubesphere.io/kubesphere/pkg/utils/term" "kubesphere.io/kubesphere/pkg/version" @@ -58,7 +57,6 @@ func NewControllerManagerCommand() *cobra.Command { s = &options.KubeSphereControllerManagerOptions{ KubernetesOptions: conf.KubernetesOptions, DevopsOptions: conf.DevopsOptions, - S3Options: conf.S3Options, AuthenticationOptions: conf.AuthenticationOptions, LdapOptions: conf.LdapOptions, OpenPitrixOptions: conf.OpenPitrixOptions, @@ -170,13 +168,6 @@ func run(s *options.KubeSphereControllerManagerOptions, ctx context.Context) err return err } - if s.S3Options != nil && len(s.S3Options.Endpoint) != 0 { - _, err = s3.NewS3Client(s.S3Options) - if err != nil { - return fmt.Errorf("failed to connect to s3, please check s3 service status, error: %v", err) - } - } - informerFactory := informers.NewInformerFactories( kubernetesClient.Kubernetes(), kubernetesClient.KubeSphere(), diff --git a/cmd/ks-apiserver/apiserver.go b/cmd/ks-apiserver/apiserver.go index 65c5e4528..7a2cd8c22 100644 --- a/cmd/ks-apiserver/apiserver.go +++ b/cmd/ks-apiserver/apiserver.go @@ -17,16 +17,15 @@ limitations under the License. package main import ( - "log" + "os" + + "k8s.io/component-base/cli" "kubesphere.io/kubesphere/cmd/ks-apiserver/app" ) func main() { - cmd := app.NewAPIServerCommand() - - if err := cmd.Execute(); err != nil { - log.Fatalln(err) - } + code := cli.Run(cmd) + os.Exit(code) } diff --git a/cmd/ks-apiserver/app/options/options.go b/cmd/ks-apiserver/app/options/options.go index ce883872a..38c0666b6 100644 --- a/cmd/ks-apiserver/app/options/options.go +++ b/cmd/ks-apiserver/app/options/options.go @@ -24,6 +24,8 @@ import ( "strings" "sync" + "kubesphere.io/kubesphere/pkg/simple/client/cache" + "sigs.k8s.io/controller-runtime/pkg/client/apiutil" openpitrixv1 "kubesphere.io/kubesphere/pkg/kapis/openpitrix/v1" @@ -44,16 +46,12 @@ import ( genericoptions "kubesphere.io/kubesphere/pkg/server/options" "kubesphere.io/kubesphere/pkg/simple/client/alerting" auditingclient "kubesphere.io/kubesphere/pkg/simple/client/auditing/elasticsearch" - "kubesphere.io/kubesphere/pkg/simple/client/cache" - "kubesphere.io/kubesphere/pkg/simple/client/devops/jenkins" eventsclient "kubesphere.io/kubesphere/pkg/simple/client/events/elasticsearch" "kubesphere.io/kubesphere/pkg/simple/client/k8s" esclient "kubesphere.io/kubesphere/pkg/simple/client/logging/elasticsearch" "kubesphere.io/kubesphere/pkg/simple/client/monitoring/metricsserver" "kubesphere.io/kubesphere/pkg/simple/client/monitoring/prometheus" - "kubesphere.io/kubesphere/pkg/simple/client/s3" - fakes3 "kubesphere.io/kubesphere/pkg/simple/client/s3/fake" "kubesphere.io/kubesphere/pkg/simple/client/sonarqube" ) @@ -63,9 +61,6 @@ type ServerRunOptions struct { *apiserverconfig.Config schemeOnce sync.Once DebugMode bool - - // Enable gops or not. - GOPSEnabled bool } func NewServerRunOptions() *ServerRunOptions { @@ -81,8 +76,6 @@ func NewServerRunOptions() *ServerRunOptions { func (s *ServerRunOptions) Flags() (fss cliflag.NamedFlagSets) { fs := fss.FlagSet("generic") fs.BoolVar(&s.DebugMode, "debug", false, "Don't enable this if you don't know what it means.") - fs.BoolVar(&s.GOPSEnabled, "gops", false, "Whether to enable gops or not. When enabled this option, "+ - "ks-apiserver will listen on a random port on 127.0.0.1, then you can use the gops tool to list and diagnose the ks-apiserver currently running.") s.GenericServerRunOptions.AddFlags(fs, s.GenericServerRunOptions) s.KubernetesOptions.AddFlags(fss.FlagSet("kubernetes"), s.KubernetesOptions) s.AuthenticationOptions.AddFlags(fss.FlagSet("authentication"), s.AuthenticationOptions) @@ -111,8 +104,6 @@ func (s *ServerRunOptions) Flags() (fss cliflag.NamedFlagSets) { return fss } -const fakeInterface string = "FAKE" - // NewAPIServer creates an APIServer instance using given options func (s *ServerRunOptions) NewAPIServer(stopCh <-chan struct{}) (*apiserver.APIServer, error) { apiServer := &apiserver.APIServer{ @@ -132,41 +123,23 @@ func (s *ServerRunOptions) NewAPIServer(stopCh <-chan struct{}) (*apiserver.APIS if s.MonitoringOptions == nil || len(s.MonitoringOptions.Endpoint) == 0 { return nil, fmt.Errorf("moinitoring service address in configuration MUST not be empty, please check configmap/kubesphere-config in kubesphere-system namespace") } else { - monitoringClient, err := prometheus.NewPrometheus(s.MonitoringOptions) - if err != nil { + if apiServer.MonitoringClient, err = prometheus.NewPrometheus(s.MonitoringOptions); err != nil { return nil, fmt.Errorf("failed to connect to prometheus, please check prometheus status, error: %v", err) } - apiServer.MonitoringClient = monitoringClient } apiServer.MetricsClient = metricsserver.NewMetricsClient(kubernetesClient.Kubernetes(), s.KubernetesOptions) if s.LoggingOptions.Host != "" { - loggingClient, err := esclient.NewClient(s.LoggingOptions) - if err != nil { + if apiServer.LoggingClient, err = esclient.NewClient(s.LoggingOptions); err != nil { return nil, fmt.Errorf("failed to connect to elasticsearch, please check elasticsearch status, error: %v", err) } - apiServer.LoggingClient = loggingClient - } - - if s.S3Options.Endpoint != "" { - if s.S3Options.Endpoint == fakeInterface && s.DebugMode { - apiServer.S3Client = fakes3.NewFakeS3() - } else { - s3Client, err := s3.NewS3Client(s.S3Options) - if err != nil { - return nil, fmt.Errorf("failed to connect to s3, please check s3 service status, error: %v", err) - } - apiServer.S3Client = s3Client - } } if s.DevopsOptions.Host != "" { - devopsClient, err := jenkins.NewDevopsClient(s.DevopsOptions) - if err != nil { + if apiServer.DevopsClient, err = jenkins.NewDevopsClient(s.DevopsOptions); err != nil { return nil, fmt.Errorf("failed to connect to jenkins, please check jenkins status, error: %v", err) } - apiServer.DevopsClient = devopsClient } if s.SonarQubeOptions.Host != "" { @@ -177,52 +150,30 @@ func (s *ServerRunOptions) NewAPIServer(stopCh <-chan struct{}) (*apiserver.APIS apiServer.SonarClient = sonarqube.NewSonar(sonarClient.SonarQube()) } - // If debug mode is on or CacheOptions is nil, will create a fake cache. - if s.CacheOptions.Type != "" { - if s.DebugMode { - s.CacheOptions.Type = cache.DefaultCacheType - } - cacheClient, err := cache.New(s.CacheOptions, stopCh) - if err != nil { - return nil, fmt.Errorf("failed to create cache, error: %v", err) - } - apiServer.CacheClient = cacheClient - } else { - s.CacheOptions = &cache.Options{Type: cache.DefaultCacheType} - // fake cache has no error to return - cacheClient, _ := cache.New(s.CacheOptions, stopCh) - apiServer.CacheClient = cacheClient - klog.Warning("ks-apiserver starts without cache provided, it will use in memory cache. " + - "This may cause inconsistencies when running ks-apiserver with multiple replicas, and memory leak risk") + if apiServer.CacheClient, err = cache.New(s.CacheOptions, stopCh); err != nil { + return nil, fmt.Errorf("failed to create cache, error: %v", err) } if s.EventsOptions.Host != "" { - eventsClient, err := eventsclient.NewClient(s.EventsOptions) - if err != nil { + if apiServer.EventsClient, err = eventsclient.NewClient(s.EventsOptions); err != nil { return nil, fmt.Errorf("failed to connect to elasticsearch, please check elasticsearch status, error: %v", err) } - apiServer.EventsClient = eventsClient } if s.AuditingOptions.Host != "" { - auditingClient, err := auditingclient.NewClient(s.AuditingOptions) - if err != nil { + if apiServer.AuditingClient, err = auditingclient.NewClient(s.AuditingOptions); err != nil { return nil, fmt.Errorf("failed to connect to elasticsearch, please check elasticsearch status, error: %v", err) } - apiServer.AuditingClient = auditingClient } if s.AlertingOptions != nil && (s.AlertingOptions.PrometheusEndpoint != "" || s.AlertingOptions.ThanosRulerEndpoint != "") { - alertingClient, err := alerting.NewRuleClient(s.AlertingOptions) - if err != nil { + if apiServer.AlertingClient, err = alerting.NewRuleClient(s.AlertingOptions); err != nil { return nil, fmt.Errorf("failed to init alerting client: %v", err) } - apiServer.AlertingClient = alertingClient } if s.Config.MultiClusterOptions.Enable { - cc := clusterclient.NewClusterClient(informerFactory.KubeSphereSharedInformerFactory().Cluster().V1alpha1().Clusters()) - apiServer.ClusterClient = cc + apiServer.ClusterClient = clusterclient.NewClusterClient(informerFactory.KubeSphereSharedInformerFactory().Cluster().V1alpha1().Clusters()) } apiServer.OpenpitrixClient = openpitrixv1.NewOpenpitrixClient(informerFactory, apiServer.KubernetesClient.KubeSphere(), s.OpenPitrixOptions, apiServer.ClusterClient) diff --git a/cmd/ks-apiserver/app/server.go b/cmd/ks-apiserver/app/server.go index 12c85d9c4..037bf91d1 100644 --- a/cmd/ks-apiserver/app/server.go +++ b/cmd/ks-apiserver/app/server.go @@ -56,7 +56,7 @@ cluster's shared state through which all other components interact.`, return utilerrors.NewAggregate(errs) } - if s.GOPSEnabled { + if s.DebugMode { // Add agent to report additional information such as the current stack trace, Go version, memory stats, etc. // Bind to a random port on address 127.0.0.1. if err := agent.Listen(agent.Options{}); err != nil { diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index 1e4175e88..bbc1efd47 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -107,7 +107,6 @@ import ( "kubesphere.io/kubesphere/pkg/simple/client/k8s" "kubesphere.io/kubesphere/pkg/simple/client/logging" "kubesphere.io/kubesphere/pkg/simple/client/monitoring" - "kubesphere.io/kubesphere/pkg/simple/client/s3" "kubesphere.io/kubesphere/pkg/simple/client/sonarqube" "kubesphere.io/kubesphere/pkg/utils/clusterclient" "kubesphere.io/kubesphere/pkg/utils/iputil" @@ -146,8 +145,6 @@ type APIServer struct { DevopsClient devops.Interface - S3Client s3.Interface - SonarClient sonarqube.SonarInterface EventsClient events.Client diff --git a/pkg/simple/client/cache/cache.go b/pkg/simple/client/cache/cache.go index 0d8b11f7e..2acbc7089 100644 --- a/pkg/simple/client/cache/cache.go +++ b/pkg/simple/client/cache/cache.go @@ -39,13 +39,13 @@ type Interface interface { // Set sets the value and living duration of the given key, zero duration means never expire Set(key string, value string, duration time.Duration) error - // Del deletes the given key, no error returned if the key doesn't exists + // Del deletes the given key, no error returned if the key doesn't exist Del(keys ...string) error // Exists checks the existence of a give key Exists(keys ...string) (bool, error) - // Expires updates object's expiration time, return err if key doesn't exist + // Expire updates object's expiration time, return err if key doesn't exist Expire(key string, duration time.Duration) error } @@ -60,6 +60,10 @@ func New(option *Options, stopCh <-chan struct{}) (Interface, error) { return nil, err } + if option.Type == TypeInMemoryCache { + klog.Warning("In-memory cache will be used, this may cause data inconsistencies when running with multiple replicas.") + } + cache, err := cacheFactories[option.Type].Create(option.Options, stopCh) if err != nil { klog.Errorf("failed to create cache, error: %v", err) diff --git a/pkg/simple/client/cache/inmemory_cache.go b/pkg/simple/client/cache/inmemory_cache.go index 899b9275f..c784034e6 100644 --- a/pkg/simple/client/cache/inmemory_cache.go +++ b/pkg/simple/client/cache/inmemory_cache.go @@ -32,9 +32,7 @@ import ( var ErrNoSuchKey = errors.New("no such key") const ( - typeInMemoryCache = "InMemoryCache" - DefaultCacheType = typeInMemoryCache - + TypeInMemoryCache = "InMemoryCache" defaultCleanupPeriod = 2 * time.Hour ) @@ -176,12 +174,11 @@ type inMemoryCacheFactory struct { } func (sf *inMemoryCacheFactory) Type() string { - return typeInMemoryCache + return TypeInMemoryCache } func (sf *inMemoryCacheFactory) Create(options options.DynamicOptions, stopCh <-chan struct{}) (Interface, error) { var sOptions InMemoryCacheOptions - decoder, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ DecodeHook: mapstructure.StringToTimeDurationHookFunc(), WeaklyTypedInput: true, diff --git a/pkg/simple/client/cache/options.go b/pkg/simple/client/cache/options.go index eb84f034a..c17c5cc44 100644 --- a/pkg/simple/client/cache/options.go +++ b/pkg/simple/client/cache/options.go @@ -31,7 +31,7 @@ type Options struct { // because redis is not required for some components func NewCacheOptions() *Options { return &Options{ - Type: "", + Type: TypeInMemoryCache, Options: map[string]interface{}{}, } } diff --git a/pkg/simple/client/s3/s3.go b/pkg/simple/client/s3/s3.go index b8ee5c023..ca973fce4 100644 --- a/pkg/simple/client/s3/s3.go +++ b/pkg/simple/client/s3/s3.go @@ -22,6 +22,8 @@ import ( "math" "time" + fakes3 "kubesphere.io/kubesphere/pkg/simple/client/s3/fake" + "code.cloudfoundry.org/bytefmt" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/credentials" @@ -44,6 +46,8 @@ const ( MinConcurrency = 5 // MaxConcurrency is the maximum concurrency to limit the goroutines. MaxConcurrency = 128 + + fakeS3Host = "FAKE" ) // calculateConcurrency calculates the concurrency for better performance, @@ -120,6 +124,10 @@ func (s *Client) Delete(key string) error { } func NewS3Client(options *Options) (Interface, error) { + if options.Endpoint == fakeS3Host { + return fakes3.NewFakeS3(), nil + } + cred := credentials.NewStaticCredentials(options.AccessKeyID, options.SecretAccessKey, options.SessionToken) config := aws.Config{ diff --git a/vendor/k8s.io/component-base/cli/OWNERS b/vendor/k8s.io/component-base/cli/OWNERS new file mode 100644 index 000000000..02f7a6a00 --- /dev/null +++ b/vendor/k8s.io/component-base/cli/OWNERS @@ -0,0 +1,12 @@ +# See the OWNERS docs at https://go.k8s.io/owners + +# Currently assigned cli to sig-cli since: +# (a) its literally named "cli" +# (b) flags are the bread-and-butter of cli tools. + +approvers: + - sig-cli-maintainers +reviewers: + - sig-cli-reviewers +labels: + - sig/cli diff --git a/vendor/k8s.io/component-base/cli/run.go b/vendor/k8s.io/component-base/cli/run.go new file mode 100644 index 000000000..c72b303e2 --- /dev/null +++ b/vendor/k8s.io/component-base/cli/run.go @@ -0,0 +1,148 @@ +/* +Copyright 2021 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 cli + +import ( + "fmt" + "math/rand" + "os" + "time" + + "github.com/spf13/cobra" + + cliflag "k8s.io/component-base/cli/flag" + "k8s.io/component-base/logs" + "k8s.io/klog/v2" +) + +// Run provides the common boilerplate code around executing a cobra command. +// For example, it ensures that logging is set up properly. Logging +// flags get added to the command line if not added already. Flags get normalized +// so that help texts show them with hyphens. Underscores are accepted +// as alternative for the command parameters. +// +// Run tries to be smart about how to print errors that are returned by the +// command: before logging is known to be set up, it prints them as plain text +// to stderr. This covers command line flag parse errors and unknown commands. +// Afterwards it logs them. This covers runtime errors. +// +// Commands like kubectl where logging is not normally part of the runtime output +// should use RunNoErrOutput instead and deal with the returned error themselves. +func Run(cmd *cobra.Command) int { + if logsInitialized, err := run(cmd); err != nil { + // If the error is about flag parsing, then printing that error + // with the decoration that klog would add ("E0923 + // 23:02:03.219216 4168816 run.go:61] unknown shorthand flag") + // is less readable. Using klog.Fatal is even worse because it + // dumps a stack trace that isn't about the error. + // + // But if it is some other error encountered at runtime, then + // we want to log it as error, at least in most commands because + // their output is a log event stream. + // + // We can distinguish these two cases depending on whether + // we got to logs.InitLogs() above. + // + // This heuristic might be problematic for command line + // tools like kubectl where the output is carefully controlled + // and not a log by default. They should use RunNoErrOutput + // instead. + // + // The usage of klog is problematic also because we don't know + // whether the command has managed to configure it. This cannot + // be checked right now, but may become possible when the early + // logging proposal from + // https://github.com/kubernetes/enhancements/pull/3078 + // ("contextual logging") is implemented. + if !logsInitialized { + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + } else { + klog.ErrorS(err, "command failed") + } + return 1 + } + return 0 +} + +// RunNoErrOutput is a version of Run which returns the cobra command error +// instead of printing it. +func RunNoErrOutput(cmd *cobra.Command) error { + _, err := run(cmd) + return err +} + +func run(cmd *cobra.Command) (logsInitialized bool, err error) { + rand.Seed(time.Now().UnixNano()) + defer logs.FlushLogs() + + cmd.SetGlobalNormalizationFunc(cliflag.WordSepNormalizeFunc) + + // When error printing is enabled for the Cobra command, a flag parse + // error gets printed first, then optionally the often long usage + // text. This is very unreadable in a console because the last few + // lines that will be visible on screen don't include the error. + // + // The recommendation from #sig-cli was to print the usage text, then + // the error. We implement this consistently for all commands here. + // However, we don't want to print the usage text when command + // execution fails for other reasons than parsing. We detect this via + // the FlagParseError callback. + // + // Some commands, like kubectl, already deal with this themselves. + // We don't change the behavior for those. + if !cmd.SilenceUsage { + cmd.SilenceUsage = true + cmd.SetFlagErrorFunc(func(c *cobra.Command, err error) error { + // Re-enable usage printing. + c.SilenceUsage = false + return err + }) + } + + // In all cases error printing is done below. + cmd.SilenceErrors = true + + // This is idempotent. + logs.AddFlags(cmd.PersistentFlags()) + + // Inject logs.InitLogs after command line parsing into one of the + // PersistentPre* functions. + switch { + case cmd.PersistentPreRun != nil: + pre := cmd.PersistentPreRun + cmd.PersistentPreRun = func(cmd *cobra.Command, args []string) { + logs.InitLogs() + logsInitialized = true + pre(cmd, args) + } + case cmd.PersistentPreRunE != nil: + pre := cmd.PersistentPreRunE + cmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { + logs.InitLogs() + logsInitialized = true + return pre(cmd, args) + } + default: + cmd.PersistentPreRun = func(cmd *cobra.Command, args []string) { + logs.InitLogs() + logsInitialized = true + } + } + + err = cmd.Execute() + return +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 68e6e0817..ecbc0b136 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -2256,6 +2256,7 @@ k8s.io/code-generator/pkg/namer k8s.io/code-generator/pkg/util # k8s.io/component-base v0.26.1 => k8s.io/component-base v0.26.1 ## explicit; go 1.19 +k8s.io/component-base/cli k8s.io/component-base/cli/flag k8s.io/component-base/config k8s.io/component-base/config/v1alpha1