use logging library in cmctl

Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>

cmd/acmesolver/main.go

@@ -37,6 +37,6 @@ func main() {
 
 	if err := cmd.Execute(); err != nil {
 		logf.Log.Error(err, "error executing command")
-		util.SetExitCode(err)
+		util.SetExitCodeFromError(err)
 	}
 }

cmd/cainjector/main.go

@@ -43,6 +43,6 @@ func main() {
 
 	if err := cmd.Execute(); err != nil {
 		logf.Log.Error(err, "error executing command")
-		util.SetExitCode(err)
+		util.SetExitCodeFromError(err)
 	}
 }

cmd/controller/main.go

@@ -37,6 +37,6 @@ func main() {
 
 	if err := cmd.Execute(); err != nil {
 		logf.Log.Error(err, "error executing command")
-		util.SetExitCode(err)
+		util.SetExitCodeFromError(err)
 	}
 }

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}

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.20230727155451-9de9809ac5ea
 	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
 )

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.20230727155451-9de9809ac5ea h1:/hM1BkaxswEPRv0Hz2NLwbMUpo8HvxT7D1rT2/3Jnkg=
+github.com/cert-manager/cert-manager v1.13.0-alpha.0.0.20230727155451-9de9809ac5ea/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=

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.SetExitCode(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)
 	}
 }

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,41 +89,40 @@ 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)
 
 	return cmd
 }
 
 // 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.SetExitCodeFromError(pollErr)
+		} else {
+			cmcmdutil.SetExitCodeFromError(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
 }

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.