From b4479e53ed52366b9a9faadde209644d89d1dfc4 Mon Sep 17 00:00:00 2001 From: Tim Ramlot <42113979+inteon@users.noreply.github.com> Date: Tue, 1 Aug 2023 16:07:20 +0200 Subject: [PATCH 1/3] use logging library in cmctl Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com> --- cmd/ctl/cmd/cmd.go | 37 ++++++++++++----- cmd/ctl/go.mod | 13 +++--- cmd/ctl/go.sum | 18 ++++---- cmd/ctl/main.go | 29 ++++++++++++- cmd/ctl/pkg/check/api/api.go | 61 +++++++++++++--------------- cmd/ctl/pkg/install/helm/applycrd.go | 8 ++-- cmd/ctl/pkg/install/install.go | 11 ++--- cmd/ctl/pkg/uninstall/uninstall.go | 12 +++--- test/integration/go.mod | 2 +- 9 files changed, 119 insertions(+), 72 deletions(-) diff --git a/cmd/ctl/cmd/cmd.go b/cmd/ctl/cmd/cmd.go index 811281063..0d1fb890f 100644 --- a/cmd/ctl/cmd/cmd.go +++ b/cmd/ctl/cmd/cmd.go @@ -18,20 +18,24 @@ package cmd import ( "context" - "flag" "fmt" "io" - "os" "github.com/spf13/cobra" + "github.com/spf13/pflag" "k8s.io/cli-runtime/pkg/genericclioptions" - "k8s.io/klog/v2" + "k8s.io/component-base/logs" "github.com/cert-manager/cert-manager/cmd/ctl/pkg/build" "github.com/cert-manager/cert-manager/cmd/ctl/pkg/build/commands" + logf "github.com/cert-manager/cert-manager/pkg/logs" ) func NewCertManagerCtlCommand(ctx context.Context, in io.Reader, out, err io.Writer) *cobra.Command { + ctx = logf.NewContext(ctx, logf.Log) + + logOptions := logs.NewOptions() + cmds := &cobra.Command{ Use: build.Name(), Short: "cert-manager CLI tool to manage and configure cert-manager resources", @@ -40,16 +44,29 @@ func NewCertManagerCtlCommand(ctx context.Context, in io.Reader, out, err io.Wri CompletionOptions: cobra.CompletionOptions{ DisableDefaultCmd: true, }, + PersistentPreRunE: func(cmd *cobra.Command, args []string) error { + return logf.ValidateAndApply(logOptions) + }, + SilenceErrors: true, // Errors are already logged when calling cmd.Execute() } cmds.SetUsageTemplate(usageTemplate()) - cmds.Flags().AddGoFlagSet(flag.CommandLine) - flag.CommandLine.Parse([]string{}) - fakefs := flag.NewFlagSet("fake", flag.ExitOnError) - klog.InitFlags(fakefs) - if err := fakefs.Parse([]string{"-logtostderr=false"}); err != nil { - fmt.Fprintf(os.Stderr, "%s\n", err) - os.Exit(1) + { + var logFlags pflag.FlagSet + logf.AddFlagsNonDeprecated(logOptions, &logFlags) + + logFlags.VisitAll(func(f *pflag.Flag) { + switch f.Name { + case "v": + // "cmctl check api" already had a "v" flag that did not require any value, for + // backwards compatibility we allow the "v" logging flag to be set without a value + // and default to "2" (which will result in the same behaviour as before). + f.NoOptDefVal = "2" + cmds.PersistentFlags().AddFlag(f) + default: + cmds.PersistentFlags().AddFlag(f) + } + }) } ioStreams := genericclioptions.IOStreams{In: in, Out: out, ErrOut: err} diff --git a/cmd/ctl/go.mod b/cmd/ctl/go.mod index 83d4f5823..46d2802e7 100644 --- a/cmd/ctl/go.mod +++ b/cmd/ctl/go.mod @@ -12,7 +12,7 @@ go 1.20 // or a branch name (master). require ( - github.com/cert-manager/cert-manager v1.13.0-alpha.0 + github.com/cert-manager/cert-manager v1.13.0-alpha.0.0.20230801130528-b93ec2f8242b github.com/spf13/cobra v1.7.0 github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.8.2 @@ -23,7 +23,7 @@ require ( k8s.io/apimachinery v0.27.4 k8s.io/cli-runtime v0.27.4 k8s.io/client-go v0.27.4 - k8s.io/klog/v2 v2.100.1 + k8s.io/component-base v0.27.4 k8s.io/kubectl v0.27.4 k8s.io/utils v0.0.0-20230711102312-30195339c3c7 sigs.k8s.io/controller-runtime v0.15.0 @@ -138,6 +138,7 @@ require ( go.uber.org/atomic v1.9.0 // indirect go.uber.org/multierr v1.6.0 // indirect go.uber.org/zap v1.24.0 // indirect + golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1 // indirect golang.org/x/net v0.10.0 // indirect golang.org/x/oauth2 v0.5.0 // indirect golang.org/x/sync v0.2.0 // indirect @@ -153,13 +154,13 @@ require ( gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect k8s.io/apiserver v0.27.4 // indirect - k8s.io/component-base v0.27.4 // indirect - k8s.io/kube-aggregator v0.27.2 // indirect + k8s.io/klog/v2 v2.100.1 // indirect + k8s.io/kube-aggregator v0.27.4 // indirect k8s.io/kube-openapi v0.0.0-20230515203736-54b630e78af5 // indirect oras.land/oras-go v1.2.2 // indirect - sigs.k8s.io/gateway-api v0.7.0 // indirect + sigs.k8s.io/gateway-api v0.7.1 // indirect sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect sigs.k8s.io/kustomize/api v0.13.2 // indirect sigs.k8s.io/kustomize/kyaml v0.14.1 // indirect - sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect + sigs.k8s.io/structured-merge-diff/v4 v4.3.0 // indirect ) diff --git a/cmd/ctl/go.sum b/cmd/ctl/go.sum index 1fdebcaae..1b91ff299 100644 --- a/cmd/ctl/go.sum +++ b/cmd/ctl/go.sum @@ -92,8 +92,8 @@ github.com/bugsnag/bugsnag-go v0.0.0-20141110184014-b1d153021fcd h1:rFt+Y/IK1aEZ github.com/bugsnag/osext v0.0.0-20130617224835-0dd3f918b21b h1:otBG+dV+YK+Soembjv71DPz3uX/V/6MMlSyD9JBQ6kQ= github.com/bugsnag/panicwrap v0.0.0-20151223152923-e2c28503fcd0 h1:nvj0OLI3YqYXer/kZD8Ri1aaunCxIEsOst1BVJswV0o= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= -github.com/cert-manager/cert-manager v1.13.0-alpha.0 h1:14sNUfu3OeoBysbhCbP7lGG6QRslXbw+AkY6FMuuu/s= -github.com/cert-manager/cert-manager v1.13.0-alpha.0/go.mod h1:ql0msU88JCcQSceN+PFjEY8U+AMe13y06vO2klJk8bs= +github.com/cert-manager/cert-manager v1.13.0-alpha.0.0.20230801130528-b93ec2f8242b h1:6ZJ9iOz6oIJDm+2uwdOMpL0XrhwyvsIaoxvA1+SSHH0= +github.com/cert-manager/cert-manager v1.13.0-alpha.0.0.20230801130528-b93ec2f8242b/go.mod h1:Y6JU8MFm4nSw74Af5w2oG+KdG7A/cIvlbpZ7+g7iAI4= github.com/cespare/xxhash v1.1.0/go.mod h1:XrSqR1VqqWfGrhpAt58auRo0WTKS1nRRg3ghfAqPWnc= github.com/cespare/xxhash/v2 v2.2.0 h1:DC2CZ1Ep5Y4k3ZQ899DldepgrayRUGE6BBZ/cd9Cj44= github.com/cespare/xxhash/v2 v2.2.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= @@ -682,6 +682,8 @@ golang.org/x/exp v0.0.0-20191227195350-da58074b4299/go.mod h1:2RIsYlXP63K8oxa1u0 golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u096TMicItID8zy7Y6sNkU49FU4= golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM= golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU= +golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1 h1:k/i9J1pBpvlfR+9QsetwPyERsqu1GIbi967PQMq3Ivc= +golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1/go.mod h1:V1LtkGg67GoY2N1AnLN78QLrzxkLyJw7RJb1gzOOz9w= golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js= golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= @@ -1097,8 +1099,8 @@ k8s.io/component-base v0.27.4 h1:Wqc0jMKEDGjKXdae8hBXeskRP//vu1m6ypC+gwErj4c= k8s.io/component-base v0.27.4/go.mod h1:hoiEETnLc0ioLv6WPeDt8vD34DDeB35MfQnxCARq3kY= k8s.io/klog/v2 v2.100.1 h1:7WCHKK6K8fNhTqfBhISHQ97KrnJNFZMcQvKp7gP/tmg= k8s.io/klog/v2 v2.100.1/go.mod h1:y1WjHnz7Dj687irZUWR/WLkLc5N1YHtjLdmgWjndZn0= -k8s.io/kube-aggregator v0.27.2 h1:jfHoPip+qN/fn3OcrYs8/xMuVYvkJHKo0H0DYciqdns= -k8s.io/kube-aggregator v0.27.2/go.mod h1:mwrTt4ESjQ7A6847biwohgZWn8P/KzSFHegEScbSGY4= +k8s.io/kube-aggregator v0.27.4 h1:WdK9iiBr32G8bWfpUEFVQl70RZO2dU19ZAktUXL5JFc= +k8s.io/kube-aggregator v0.27.4/go.mod h1:+eG83gkAyh0uilQEAOgheeQW4hr+PkyV+5O1nLGsjlM= k8s.io/kube-openapi v0.0.0-20230515203736-54b630e78af5 h1:azYPdzztXxPSa8wb+hksEKayiz0o+PPisO/d+QhWnoo= k8s.io/kube-openapi v0.0.0-20230515203736-54b630e78af5/go.mod h1:kzo02I3kQ4BTtEfVLaPbjvCkX97YqGve33wzlb3fofQ= k8s.io/kubectl v0.27.4 h1:RV1TQLIbtL34+vIM+W7HaS3KfAbqvy9lWn6pWB9els4= @@ -1112,15 +1114,15 @@ rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0= rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA= sigs.k8s.io/controller-runtime v0.15.0 h1:ML+5Adt3qZnMSYxZ7gAverBLNPSMQEibtzAgp0UPojU= sigs.k8s.io/controller-runtime v0.15.0/go.mod h1:7ngYvp1MLT+9GeZ+6lH3LOlcHkp/+tzA/fmHa4iq9kk= -sigs.k8s.io/gateway-api v0.7.0 h1:/mG8yyJNBifqvuVLW5gwlI4CQs0NR/5q4BKUlf1bVdY= -sigs.k8s.io/gateway-api v0.7.0/go.mod h1:Xv0+ZMxX0lu1nSSDIIPEfbVztgNZ+3cfiYrJsa2Ooso= +sigs.k8s.io/gateway-api v0.7.1 h1:Tts2jeepVkPA5rVG/iO+S43s9n7Vp7jCDhZDQYtPigQ= +sigs.k8s.io/gateway-api v0.7.1/go.mod h1:Xv0+ZMxX0lu1nSSDIIPEfbVztgNZ+3cfiYrJsa2Ooso= sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd h1:EDPBXCAspyGV4jQlpZSudPeMmr1bNJefnuqLsRAsHZo= sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd/go.mod h1:B8JuhiUyNFVKdsE8h686QcCxMaH6HrOAZj4vswFpcB0= sigs.k8s.io/kustomize/api v0.13.2 h1:kejWfLeJhUsTGioDoFNJET5LQe/ajzXhJGYoU+pJsiA= sigs.k8s.io/kustomize/api v0.13.2/go.mod h1:DUp325VVMFVcQSq+ZxyDisA8wtldwHxLZbr1g94UHsw= sigs.k8s.io/kustomize/kyaml v0.14.1 h1:c8iibius7l24G2wVAGZn/Va2wNys03GXLjYVIcFVxKA= sigs.k8s.io/kustomize/kyaml v0.14.1/go.mod h1:AN1/IpawKilWD7V+YvQwRGUvuUOOWpjsHu6uHwonSF4= -sigs.k8s.io/structured-merge-diff/v4 v4.2.3 h1:PRbqxJClWWYMNV1dhaG4NsibJbArud9kFxnAMREiWFE= -sigs.k8s.io/structured-merge-diff/v4 v4.2.3/go.mod h1:qjx8mGObPmV2aSZepjQjbmb2ihdVs8cGKBraizNC69E= +sigs.k8s.io/structured-merge-diff/v4 v4.3.0 h1:UZbZAZfX0wV2zr7YZorDz6GXROfDFj6LvqCRm4VUVKk= +sigs.k8s.io/structured-merge-diff/v4 v4.3.0/go.mod h1:N8hJocpFajUSSeSJ9bOZ77VzejKZaXsTtZo4/u7Io08= sigs.k8s.io/yaml v1.3.0 h1:a2VclLzOGrwOHDiV8EfBGhvjHvP46CtW5j6POvhYGGo= sigs.k8s.io/yaml v1.3.0/go.mod h1:GeOyir5tyXNByN85N/dRIT9es5UQNerPYEKK56eTBm8= diff --git a/cmd/ctl/main.go b/cmd/ctl/main.go index e8048c62f..f28f57aae 100644 --- a/cmd/ctl/main.go +++ b/cmd/ctl/main.go @@ -20,20 +20,45 @@ import ( "context" "fmt" "os" + "runtime" + "strings" + + cmdutil "k8s.io/kubectl/pkg/cmd/util" ctlcmd "github.com/cert-manager/cert-manager/cmd/ctl/cmd" "github.com/cert-manager/cert-manager/internal/cmd/util" + logf "github.com/cert-manager/cert-manager/pkg/logs" ) func main() { stopCh, exit := util.SetupExitHandler(util.AlwaysErrCode) defer exit() // This function might call os.Exit, so defer last + logf.InitLogs() + defer logf.FlushLogs() + + // In cmctl, we are using cmdutil.CheckErr, which will call os.Exit(1) if it receives an error. + // Instead, we want to do a soft exit, and use SetExitCode to set the correct exit code. + // Additionally, we make sure to output the final error message to stdout, as we do not want this + // message to be mixed with other log outputs from the execution of the command. + // To do this, we need to set a custom error handler. + cmdutil.BehaviorOnFatal(func(msg string, code int) { + if len(msg) > 0 { + // add newline if needed + if !strings.HasSuffix(msg, "\n") { + msg += "\n" + } + fmt.Fprint(os.Stdout, msg) + } + + util.SetExitCodeValue(code) + runtime.Goexit() // Do soft exit (handle all defers, that should set correct exit code) + }) + ctx := util.ContextWithStopCh(context.Background(), stopCh) cmd := ctlcmd.NewCertManagerCtlCommand(ctx, os.Stdin, os.Stdout, os.Stderr) if err := cmd.Execute(); err != nil { - fmt.Fprintf(os.Stderr, "%s\n", err) - util.SetExitCode(err) + cmdutil.CheckErr(err) } } diff --git a/cmd/ctl/pkg/check/api/api.go b/cmd/ctl/pkg/check/api/api.go index 3df876a95..c44e836e6 100644 --- a/cmd/ctl/pkg/check/api/api.go +++ b/cmd/ctl/pkg/check/api/api.go @@ -20,19 +20,19 @@ import ( "context" "errors" "fmt" - "log" - "runtime" "time" "github.com/spf13/cobra" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/cli-runtime/pkg/genericclioptions" + cmdutil "k8s.io/kubectl/pkg/cmd/util" "k8s.io/kubectl/pkg/scheme" "k8s.io/kubectl/pkg/util/i18n" "k8s.io/kubectl/pkg/util/templates" "github.com/cert-manager/cert-manager/cmd/ctl/pkg/factory" cmcmdutil "github.com/cert-manager/cert-manager/internal/cmd/util" + logf "github.com/cert-manager/cert-manager/pkg/logs" "github.com/cert-manager/cert-manager/pkg/util/cmapichecker" ) @@ -48,9 +48,6 @@ type Options struct { // Time between checks when waiting Interval time.Duration - // Print details regarding encountered errors - Verbose bool - genericclioptions.IOStreams *factory.Factory } @@ -78,7 +75,7 @@ func (o *Options) Complete() error { // see: https://github.com/cert-manager/cert-manager/pull/4205#discussion_r668660271 o.APIChecker, err = cmapichecker.New(o.RESTConfig, scheme.Scheme, o.Namespace) if err != nil { - return fmt.Errorf("Error: %v", err) + return err } return nil @@ -92,19 +89,13 @@ func NewCmdCheckApi(ctx context.Context, ioStreams genericclioptions.IOStreams) Use: "api", Short: "Check if the cert-manager API is ready", Long: checkApiDesc, - RunE: func(cmd *cobra.Command, args []string) error { - if err := o.Complete(); err != nil { - return err - } - o.Run(ctx) - return nil + Run: func(cmd *cobra.Command, args []string) { + cmdutil.CheckErr(o.Complete()) + cmdutil.CheckErr(o.Run(ctx)) }, - SilenceUsage: true, - SilenceErrors: true, } cmd.Flags().DurationVar(&o.Wait, "wait", 0, "Wait until the cert-manager API is ready (default 0s = poll once)") cmd.Flags().DurationVar(&o.Interval, "interval", 5*time.Second, "Time between checks when waiting, must include unit, e.g. 1m or 10m") - cmd.Flags().BoolVarP(&o.Verbose, "verbose", "v", false, "Print detailed error messages") o.Factory = factory.New(ctx, cmd) @@ -112,21 +103,26 @@ func NewCmdCheckApi(ctx context.Context, ioStreams genericclioptions.IOStreams) } // Run executes check api command -func (o *Options) Run(ctx context.Context) { - if !o.Verbose { - log.SetFlags(0) // Disable prefixing logs with timestamps. - } - log.SetOutput(o.ErrOut) // Log all intermediate errors to stderr +func (o *Options) Run(ctx context.Context) error { + log := logf.FromContext(ctx, "checkAPI") start := time.Now() - pollErr := wait.PollUntilContextCancel(ctx, o.Interval, false, func(ctx context.Context) (bool, error) { + var lastError error + pollErr := wait.PollUntilContextCancel(ctx, o.Interval, true, func(ctx context.Context) (bool, error) { if err := o.APIChecker.Check(ctx); err != nil { - if !o.Verbose && errors.Unwrap(err) != nil { - err = errors.Unwrap(err) + simpleError := cmapichecker.TranslateToSimpleError(err) + if simpleError != nil { + if !log.V(2).Enabled() { + log.Error(simpleError, "Not ready") + } else { + log.Error(simpleError, "Not ready", "underlyingError", err) + } + lastError = simpleError + } else { + log.Error(err, "Not ready") + lastError = err } - log.Printf("Not ready: %v", err) - if time.Since(start) > o.Wait { return false, context.DeadlineExceeded } @@ -136,17 +132,18 @@ func (o *Options) Run(ctx context.Context) { return true, nil }) - log.SetOutput(o.Out) // Log conclusion to stdout - if pollErr != nil { if errors.Is(pollErr, context.DeadlineExceeded) && o.Wait > 0 { - log.Printf("Timed out after %s", o.Wait) + log.Error(pollErr, "Timed out", "after", o.Wait) + cmcmdutil.SetExitCode(pollErr) + } else { + cmcmdutil.SetExitCode(lastError) } - cmcmdutil.SetExitCode(pollErr) - - runtime.Goexit() // Do soft exit (handle all defers, that should set correct exit code) + return lastError } - log.Printf("The cert-manager API is ready") + fmt.Fprintln(o.Out, "The cert-manager API is ready") + + return nil } diff --git a/cmd/ctl/pkg/install/helm/applycrd.go b/cmd/ctl/pkg/install/helm/applycrd.go index 4f1c8721a..c9ecb89b4 100644 --- a/cmd/ctl/pkg/install/helm/applycrd.go +++ b/cmd/ctl/pkg/install/helm/applycrd.go @@ -17,17 +17,19 @@ limitations under the License. package helm import ( - "log" "time" "helm.sh/helm/v3/pkg/action" "k8s.io/cli-runtime/pkg/resource" + + logf "github.com/cert-manager/cert-manager/pkg/logs" ) // CreateCRDs creates cert manager CRDs. Before calling this function, we // made sure that the CRDs are not yet installed on the cluster. func CreateCRDs(allCRDs []*resource.Info, cfg *action.Configuration) error { - log.Printf("Creating the cert-manager CRDs") + logf.Log.Info("Creating the cert-manager CRDs") + // Create all CRDs rr, err := cfg.KubeClient.Create(allCRDs) if err != nil { @@ -42,7 +44,7 @@ func CreateCRDs(allCRDs []*resource.Info, cfg *action.Configuration) error { return err } - log.Printf("Clearing discovery cache") + logf.Log.Info("Clearing discovery cache") discoveryClient.Invalidate() // Give time for the CRD to be recognized. diff --git a/cmd/ctl/pkg/install/install.go b/cmd/ctl/pkg/install/install.go index 323f480a5..fbfc232bf 100644 --- a/cmd/ctl/pkg/install/install.go +++ b/cmd/ctl/pkg/install/install.go @@ -20,7 +20,6 @@ import ( "context" "fmt" "io" - "log" "os" "strings" "time" @@ -38,6 +37,7 @@ import ( "github.com/cert-manager/cert-manager/cmd/ctl/pkg/build" "github.com/cert-manager/cert-manager/cmd/ctl/pkg/install/helm" + logf "github.com/cert-manager/cert-manager/pkg/logs" ) type InstallOptions struct { @@ -160,8 +160,7 @@ func NewCmdInstall(ctx context.Context, ioStreams genericclioptions.IOStreams) * // This creates a Helm "release" artifact in a Secret in the target namespace, which contains // a record of all the resources installed by Helm (except the CRDs). func (o *InstallOptions) runInstall(ctx context.Context) (*release.Release, error) { - log.SetFlags(0) // Disable prefixing logs with timestamps. - log.SetOutput(o.ErrOut) // Log everything to stderr so dry-run output does not get corrupted. + log := logf.FromContext(ctx, "install") // Find chart cp, err := o.client.ChartPathOptions.LocateChart(o.ChartName, o.settings) @@ -181,7 +180,7 @@ func (o *InstallOptions) runInstall(ctx context.Context) (*release.Release, erro // Console print if chart is deprecated if chart.Metadata.Deprecated { - log.Printf("This chart is deprecated") + log.Error(fmt.Errorf("chart.Metadata.Deprecated is true"), "This chart is deprecated") } // Merge all values flags @@ -214,7 +213,9 @@ func (o *InstallOptions) runInstall(ctx context.Context) (*release.Release, erro return dryRunResult, nil } - if err := o.cfg.Init(o.settings.RESTClientGetter(), o.settings.Namespace(), os.Getenv("HELM_DRIVER"), log.Printf); err != nil { + if err := o.cfg.Init(o.settings.RESTClientGetter(), o.settings.Namespace(), os.Getenv("HELM_DRIVER"), func(format string, v ...interface{}) { + log.Info(fmt.Sprintf(format, v...)) + }); err != nil { return nil, err } diff --git a/cmd/ctl/pkg/uninstall/uninstall.go b/cmd/ctl/pkg/uninstall/uninstall.go index 6e14410f2..0e2d62976 100644 --- a/cmd/ctl/pkg/uninstall/uninstall.go +++ b/cmd/ctl/pkg/uninstall/uninstall.go @@ -20,7 +20,6 @@ import ( "context" "errors" "fmt" - "log" "os" "time" @@ -32,6 +31,7 @@ import ( "k8s.io/cli-runtime/pkg/genericclioptions" "github.com/cert-manager/cert-manager/cmd/ctl/pkg/build" + logf "github.com/cert-manager/cert-manager/pkg/logs" ) type options struct { @@ -88,7 +88,7 @@ func NewCmd(ctx context.Context, ioStreams genericclioptions.IOStreams) *cobra.C RunE: func(cmd *cobra.Command, args []string) error { res, err := run(ctx, options) if err != nil { - return fmt.Errorf("run: %v", err) + return err } if options.dryRun { @@ -126,9 +126,11 @@ func NewCmd(ctx context.Context, ioStreams genericclioptions.IOStreams) *cobra.C // run assumes cert-manager was installed as a Helm release named cert-manager. // this is not configurable to avoid uninstalling non-cert-manager releases. func run(ctx context.Context, o options) (*release.UninstallReleaseResponse, error) { - log.SetFlags(0) // disable prefixing logs with timestamps. + log := logf.FromContext(ctx, "install") - if err := o.cfg.Init(o.settings.RESTClientGetter(), o.settings.Namespace(), os.Getenv("HELM_DRIVER"), log.Printf); err != nil { + if err := o.cfg.Init(o.settings.RESTClientGetter(), o.settings.Namespace(), os.Getenv("HELM_DRIVER"), func(format string, v ...interface{}) { + log.Info(fmt.Sprintf(format, v...)) + }); err != nil { return nil, fmt.Errorf("o.cfg.Init: %v", err) } @@ -139,7 +141,7 @@ func run(ctx context.Context, o options) (*release.UninstallReleaseResponse, err res, err := o.client.Run(releaseName) if errors.Is(err, driver.ErrReleaseNotFound) { - log.Fatalf("release %v not found in namespace %v, did you use the correct namespace?", releaseName, o.settings.Namespace()) + return nil, fmt.Errorf("release %v not found in namespace %v, did you use the correct namespace?", releaseName, o.settings.Namespace()) } return res, nil diff --git a/test/integration/go.mod b/test/integration/go.mod index 442b5751c..a35ec6af8 100644 --- a/test/integration/go.mod +++ b/test/integration/go.mod @@ -9,7 +9,7 @@ replace github.com/cert-manager/cert-manager/cmd/ctl => ../../cmd/ctl/ replace github.com/cert-manager/cert-manager/webhook-binary => ../../cmd/webhook/ require ( - github.com/cert-manager/cert-manager v1.13.0-alpha.0 + github.com/cert-manager/cert-manager v1.13.0-alpha.0.0.20230801130528-b93ec2f8242b github.com/cert-manager/cert-manager/cmd/ctl v0.0.0-00010101000000-000000000000 github.com/go-logr/logr v1.2.4 github.com/miekg/dns v1.1.50 From 78b78cecca29a043ba27a75172b3acea4f5edd4b Mon Sep 17 00:00:00 2001 From: Tim Ramlot <42113979+inteon@users.noreply.github.com> Date: Tue, 1 Aug 2023 16:15:14 +0200 Subject: [PATCH 2/3] check api, only log if -v is set Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com> --- cmd/ctl/pkg/check/api/api.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/cmd/ctl/pkg/check/api/api.go b/cmd/ctl/pkg/check/api/api.go index c44e836e6..657a52297 100644 --- a/cmd/ctl/pkg/check/api/api.go +++ b/cmd/ctl/pkg/check/api/api.go @@ -112,14 +112,10 @@ func (o *Options) Run(ctx context.Context) error { if err := o.APIChecker.Check(ctx); err != nil { simpleError := cmapichecker.TranslateToSimpleError(err) if simpleError != nil { - if !log.V(2).Enabled() { - log.Error(simpleError, "Not ready") - } else { - log.Error(simpleError, "Not ready", "underlyingError", err) - } + log.V(2).Info("Not ready", "err", simpleError, "underlyingError", err) lastError = simpleError } else { - log.Error(err, "Not ready") + log.V(2).Info("Not ready", "err", err) lastError = err } @@ -134,7 +130,7 @@ func (o *Options) Run(ctx context.Context) error { if pollErr != nil { if errors.Is(pollErr, context.DeadlineExceeded) && o.Wait > 0 { - log.Error(pollErr, "Timed out", "after", o.Wait) + log.V(2).Info("Timed out", "after", o.Wait, "err", pollErr) cmcmdutil.SetExitCode(pollErr) } else { cmcmdutil.SetExitCode(lastError) From e3d6717387605d7b8936a18ea012bd636c3d676d Mon Sep 17 00:00:00 2001 From: Tim Ramlot <42113979+inteon@users.noreply.github.com> Date: Tue, 1 Aug 2023 16:17:06 +0200 Subject: [PATCH 3/3] update comment and explain why we use cmdutil.CheckErr Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com> --- cmd/ctl/main.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/ctl/main.go b/cmd/ctl/main.go index f28f57aae..4a7a0196c 100644 --- a/cmd/ctl/main.go +++ b/cmd/ctl/main.go @@ -37,7 +37,8 @@ func main() { logf.InitLogs() defer logf.FlushLogs() - // In cmctl, we are using cmdutil.CheckErr, which will call os.Exit(1) if it receives an error. + // In cmctl, we are using cmdutil.CheckErr, a kubectl utility function that creates human readable + // error messages from errors. By default, this function will call os.Exit(1) if it receives an error. // Instead, we want to do a soft exit, and use SetExitCode to set the correct exit code. // Additionally, we make sure to output the final error message to stdout, as we do not want this // message to be mixed with other log outputs from the execution of the command.