use logging library in cmctl

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

cmd/ctl/cmd/cmd.go

@@ -18,20 +18,23 @@ package cmd
 
 import (
 	"context"
-	"flag"
 	"fmt"
 	"io"
-	"os"
 
 	"github.com/spf13/cobra"
 	"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,17 +43,14 @@ 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)
-	}
+	logf.AddFlagsNonDeprecated(logOptions, cmds.PersistentFlags())
 
 	ioStreams := genericclioptions.IOStreams{In: in, Out: out, ErrOut: err}
 	for _, registerCmd := range commands.Commands() {

cmd/ctl/go.mod

@@ -23,7 +23,7 @@ require (
 	k8s.io/apimachinery v0.27.2
 	k8s.io/cli-runtime v0.27.2
 	k8s.io/client-go v0.27.2
-	k8s.io/klog/v2 v2.90.1
+	k8s.io/component-base v0.27.2
 	k8s.io/kubectl v0.27.2
 	k8s.io/utils v0.0.0-20230505201702-9f6742963106
 	sigs.k8s.io/controller-runtime v0.15.0
@@ -153,7 +153,7 @@ require (
 	gopkg.in/yaml.v2 v2.4.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 	k8s.io/apiserver v0.27.2 // indirect
-	k8s.io/component-base v0.27.2 // indirect
+	k8s.io/klog/v2 v2.90.1 // indirect
 	k8s.io/kube-aggregator v0.27.1 // indirect
 	k8s.io/kube-openapi v0.0.0-20230501164219-8b0f38b5fd1f // indirect
 	oras.land/oras-go v1.2.2 // indirect

cmd/ctl/main.go

@@ -18,22 +18,25 @@ package main
 
 import (
 	"context"
-	"fmt"
 	"os"
 
 	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()
+
 	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)
+		logf.Log.Error(err, "error executing command")
 		util.SetExitCode(err)
 	}
 }

cmd/ctl/pkg/check/api/api.go

@@ -19,8 +19,6 @@ package api
 import (
 	"context"
 	"errors"
-	"fmt"
-	"log"
 	"runtime"
 	"time"
 
@@ -33,6 +31,7 @@ import (
 
 	"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 +47,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 +74,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
@@ -99,12 +95,10 @@ func NewCmdCheckApi(ctx context.Context, ioStreams genericclioptions.IOStreams)
 			o.Run(ctx)
 			return nil
 		},
-		SilenceUsage:  true,
-		SilenceErrors: true,
+		SilenceUsage: 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)
 
@@ -113,20 +107,20 @@ 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
+	log := logf.FromContext(ctx, "checkAPI")
 
 	start := time.Now()
 	pollErr := wait.PollUntilContextCancel(ctx, o.Interval, false, 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 !log.V(2).Enabled() && simpleError != nil {
+				log.Error(simpleError, "Not ready")
+			} else if simpleError != nil {
+				log.Error(simpleError, "Not ready", "underlyingError", err)
+			} else {
+				log.Error(err, "Not ready")
 			}
 
-			log.Printf("Not ready: %v", err)
-
 			if time.Since(start) > o.Wait {
 				return false, context.DeadlineExceeded
 			}
@@ -136,17 +130,15 @@ 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)
 
 		runtime.Goexit() // Do soft exit (handle all defers, that should set correct exit code)
 	}
 
-	log.Printf("The cert-manager API is ready")
+	log.Info("The cert-manager API is ready")
 }

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.

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 {
@@ -112,8 +112,7 @@ func NewCmdInstall(ctx context.Context, ioStreams genericclioptions.IOStreams) *
 			printReleaseSummary(ioStreams.Out, rel)
 			return nil
 		},
-		SilenceUsage:  true,
-		SilenceErrors: true,
+		SilenceUsage: true,
 	}
 
 	settings.AddFlags(cmd.Flags())
@@ -160,8 +159,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 +179,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 +212,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
 	}
 

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 {
@@ -98,8 +98,7 @@ func NewCmd(ctx context.Context, ioStreams genericclioptions.IOStreams) *cobra.C
 
 			return nil
 		},
-		SilenceUsage:  true,
-		SilenceErrors: true,
+		SilenceUsage: true,
 	}
 
 	settings.AddFlags(cmd.Flags())
@@ -126,9 +125,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 +140,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

hack/verify-upgrade.sh

@@ -83,7 +83,7 @@ $helm upgrade \
     "$HELM_CHART"
 
 # Wait for the cert-manager api to be available
-$cmctl check api --wait=2m -v
+$cmctl check api --wait=2m -v 2
 
 echo "+++ Creating some cert-manager resources.."
 
@@ -99,7 +99,7 @@ $kubectl wait --for=condition=Ready cert/test1 --timeout=180s
 make e2e-setup-certmanager
 
 # Wait for the cert-manager api to be available
-$cmctl check api --wait=2m -v
+$cmctl check api --wait=2m -v 2
 
 # Test that the existing cert-manager resources can still be retrieved
 $kubectl get issuer/selfsigned-issuer cert/test1
@@ -144,7 +144,7 @@ $kubectl wait \
 	--namespace "${NAMESPACE}"
 
 # Wait for the cert-manager api to be available
-$cmctl check api --wait=2m -v
+$cmctl check api --wait=2m -v 2
 
 # Create a cert-manager issuer and cert
 $kubectl apply -f "${REPO_ROOT}/test/fixtures/cert-manager-resources.yaml" --selector=test="first"
@@ -186,7 +186,7 @@ until $rollout_cmd; do
 done
 
 # Wait for the cert-manager api to be available
-$cmctl check api --wait=2m -v
+$cmctl check api --wait=2m -v 2
 
 # Test that the existing cert-manager resources can still be retrieved
 $kubectl get issuer/selfsigned-issuer cert/test1