internal: more flexible and isolated logging (#14)

joelanford · web-flow · commit 5ac9c8dcf5d9 · 2020-08-10T08:22:11.000-04:00
The `internal/pkg/log` is moved to be an internal implementation detail
of the `cmd` package;

Also action structs in package `internal/pkg/actions` that log now have
configurable Logf fields and no longer log by default when used as a
library.
diff --git a/internal/cmd/catalog_add.go b/internal/cmd/catalog_add.go
@@ -8,12 +8,13 @@ import (
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 
+	"github.com/operator-framework/kubectl-operator/internal/cmd/internal/log"
 	"github.com/operator-framework/kubectl-operator/internal/pkg/action"
-	"github.com/operator-framework/kubectl-operator/internal/pkg/log"
 )
 
 func newCatalogAddCmd(cfg *action.Configuration) *cobra.Command {
 	a := action.NewCatalogAdd(cfg)
+	a.Logf = log.Printf
 
 	cmd := &cobra.Command{
 		Use:   "add <name> <index_image>",
diff --git a/internal/cmd/catalog_list.go b/internal/cmd/catalog_list.go
@@ -10,8 +10,8 @@ import (
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/util/duration"
 
+	"github.com/operator-framework/kubectl-operator/internal/cmd/internal/log"
 	"github.com/operator-framework/kubectl-operator/internal/pkg/action"
-	"github.com/operator-framework/kubectl-operator/internal/pkg/log"
 )
 
 func newCatalogListCmd(cfg *action.Configuration) *cobra.Command {
diff --git a/internal/cmd/catalog_remove.go b/internal/cmd/catalog_remove.go
@@ -3,8 +3,8 @@ package cmd
 import (
 	"github.com/spf13/cobra"
 
+	"github.com/operator-framework/kubectl-operator/internal/cmd/internal/log"
 	"github.com/operator-framework/kubectl-operator/internal/pkg/action"
-	"github.com/operator-framework/kubectl-operator/internal/pkg/log"
 )
 
 func newCatalogRemoveCmd(cfg *action.Configuration) *cobra.Command {
diff --git a/internal/cmd/internal/log/log.go b/internal/cmd/internal/log/log.go
diff --git a/internal/cmd/operator_install.go b/internal/cmd/operator_install.go
@@ -5,12 +5,14 @@ import (
 
 	"github.com/spf13/cobra"
 
+	"github.com/operator-framework/kubectl-operator/internal/cmd/internal/log"
 	"github.com/operator-framework/kubectl-operator/internal/pkg/action"
-	"github.com/operator-framework/kubectl-operator/internal/pkg/log"
 )
 
 func newOperatorInstallCmd(cfg *action.Configuration) *cobra.Command {
 	i := action.NewOperatorInstall(cfg)
+	i.Logf = log.Printf
+
 	cmd := &cobra.Command{
 		Use:   "install <operator>",
 		Short: "Install an operator",
diff --git a/internal/cmd/operator_list.go b/internal/cmd/operator_list.go
@@ -12,8 +12,8 @@ import (
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/util/duration"
 
+	"github.com/operator-framework/kubectl-operator/internal/cmd/internal/log"
 	"github.com/operator-framework/kubectl-operator/internal/pkg/action"
-	"github.com/operator-framework/kubectl-operator/internal/pkg/log"
 )
 
 func newOperatorListCmd(cfg *action.Configuration) *cobra.Command {
diff --git a/internal/cmd/operator_list_available.go b/internal/cmd/operator_list_available.go
@@ -12,8 +12,8 @@ import (
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/util/duration"
 
+	"github.com/operator-framework/kubectl-operator/internal/cmd/internal/log"
 	"github.com/operator-framework/kubectl-operator/internal/pkg/action"
-	"github.com/operator-framework/kubectl-operator/internal/pkg/log"
 )
 
 func newOperatorListAvailableCmd(cfg *action.Configuration) *cobra.Command {
diff --git a/internal/cmd/operator_uninstall.go b/internal/cmd/operator_uninstall.go
@@ -3,12 +3,14 @@ package cmd
 import (
 	"github.com/spf13/cobra"
 
+	"github.com/operator-framework/kubectl-operator/internal/cmd/internal/log"
 	"github.com/operator-framework/kubectl-operator/internal/pkg/action"
-	"github.com/operator-framework/kubectl-operator/internal/pkg/log"
 )
 
 func newOperatorUninstallCmd(cfg *action.Configuration) *cobra.Command {
 	u := action.NewOperatorUninstall(cfg)
+	u.Logf = log.Printf
+
 	cmd := &cobra.Command{
 		Use:   "uninstall <operator>",
 		Short: "Uninstall an operator",
diff --git a/internal/cmd/operator_upgrade.go b/internal/cmd/operator_upgrade.go
@@ -5,8 +5,8 @@ import (
 
 	"github.com/spf13/cobra"
 
+	"github.com/operator-framework/kubectl-operator/internal/cmd/internal/log"
 	"github.com/operator-framework/kubectl-operator/internal/pkg/action"
-	"github.com/operator-framework/kubectl-operator/internal/pkg/log"
 )
 
 func newOperatorUpgradeCmd(cfg *action.Configuration) *cobra.Command {
diff --git a/internal/cmd/root.go b/internal/cmd/root.go
@@ -3,10 +3,16 @@ package cmd
 import (
 	"github.com/spf13/cobra"
 
+	"github.com/operator-framework/kubectl-operator/internal/cmd/internal/log"
 	"github.com/operator-framework/kubectl-operator/internal/pkg/action"
 )
 
-func New() *cobra.Command {
+func Execute() {
+	if err := newCmd().Execute(); err != nil {
+		log.Fatal(err)
+	}
+}
+func newCmd() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "operator",
 		Short: "Manage operators in a cluster from the command line",
diff --git a/internal/pkg/action/catalog_add.go b/internal/pkg/action/catalog_add.go
@@ -22,7 +22,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	"github.com/operator-framework/kubectl-operator/internal/pkg/catalog"
-	"github.com/operator-framework/kubectl-operator/internal/pkg/log"
 )
 
 const grpcPort = "50051"
@@ -39,6 +38,7 @@ type CatalogAdd struct {
 	AddTimeout        time.Duration
 	CleanupTimeout    time.Duration
 
+	Logf            func(string, ...interface{})
 	RegistryOptions []containerdregistry.RegistryOption
 
 	registry *containerdregistry.Registry
@@ -47,6 +47,7 @@ type CatalogAdd struct {
 func NewCatalogAdd(cfg *Configuration) *CatalogAdd {
 	return &CatalogAdd{
 		config: cfg,
+		Logf:   func(string, ...interface{}) {},
 	}
 }
 
@@ -70,7 +71,7 @@ func (a *CatalogAdd) Run(ctx context.Context) (*v1alpha1.CatalogSource, error) {
 
 	defer func() {
 		if err := a.registry.Destroy(); err != nil {
-			log.Printf("registry cleanup: %v", err)
+			a.Logf("registry cleanup: %v", err)
 		}
 	}()
 
@@ -277,6 +278,6 @@ func (a *CatalogAdd) cleanup(cs *v1alpha1.CatalogSource) {
 	ctx, cancel := context.WithTimeout(context.Background(), a.CleanupTimeout)
 	defer cancel()
 	if err := a.config.Client.Delete(ctx, cs); err != nil && !apierrors.IsNotFound(err) {
-		log.Printf("delete catalogsource %q: %v", cs.Name, err)
+		a.Logf("delete catalogsource %q: %v", cs.Name, err)
 	}
 }
diff --git a/internal/pkg/action/operator_install.go b/internal/pkg/action/operator_install.go
@@ -16,7 +16,6 @@ import (
 	"k8s.io/apimachinery/pkg/util/wait"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
-	"github.com/operator-framework/kubectl-operator/internal/pkg/log"
 	"github.com/operator-framework/kubectl-operator/internal/pkg/operator"
 	"github.com/operator-framework/kubectl-operator/internal/pkg/subscription"
 )
@@ -33,11 +32,14 @@ type OperatorInstall struct {
 	InstallTimeout      time.Duration
 	CleanupTimeout      time.Duration
 	CreateOperatorGroup bool
+
+	Logf func(string, ...interface{})
 }
 
 func NewOperatorInstall(cfg *Configuration) *OperatorInstall {
 	return &OperatorInstall{
 		config: cfg,
+		Logf:   func(string, ...interface{}) {},
 	}
 }
 
@@ -83,7 +85,7 @@ func (i *OperatorInstall) Run(ctx context.Context) (*v1alpha1.ClusterServiceVers
 	if err != nil {
 		return nil, err
 	}
-	log.Printf("subscription %q created", sub.Name)
+	i.Logf("subscription %q created", sub.Name)
 
 	ip, err := i.getInstallPlan(ctx, sub)
 	if err != nil {
@@ -186,7 +188,7 @@ func (i *OperatorInstall) ensureOperatorGroup(ctx context.Context, pm *operators
 			if og, err = i.createOperatorGroup(ctx, targetNamespaces); err != nil {
 				return nil, fmt.Errorf("create operator group: %v", err)
 			}
-			log.Printf("operatorgroup %q created", og.Name)
+			i.Logf("operatorgroup %q created", og.Name)
 		} else {
 			return nil, fmt.Errorf("namespace %q has no existing operator group; use --create-operator-group to create one automatically", i.config.Namespace)
 		}
diff --git a/internal/pkg/action/operator_uninstall.go b/internal/pkg/action/operator_uninstall.go
@@ -17,8 +17,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/yaml"
-
-	"github.com/operator-framework/kubectl-operator/internal/pkg/log"
 )
 
 type OperatorUninstall struct {
@@ -28,11 +26,14 @@ type OperatorUninstall struct {
 	DeleteOperatorGroup bool
 	DeleteCRDs          bool
 	DeleteAll           bool
+
+	Logf func(string, ...interface{})
 }
 
 func NewOperatorUninstall(cfg *Configuration) *OperatorUninstall {
 	return &OperatorUninstall{
 		config: cfg,
+		Logf:   func(string, ...interface{}) {},
 	}
 }
 
@@ -81,7 +82,7 @@ func (u *OperatorUninstall) Run(ctx context.Context) error {
 	if err := u.config.Client.Delete(ctx, sub); err != nil {
 		return fmt.Errorf("delete subscription %q: %v", sub.Name, err)
 	}
-	log.Printf("subscription %q deleted", sub.Name)
+	u.Logf("subscription %q deleted", sub.Name)
 
 	if u.DeleteCRDs {
 		if err := u.deleteCRDs(ctx, crds); err != nil {
@@ -112,7 +113,7 @@ func (u *OperatorUninstall) Run(ctx context.Context) error {
 				if err := u.config.Client.Delete(ctx, &og); err != nil {
 					return fmt.Errorf("delete operatorgroup %q: %v", og.Name, err)
 				}
-				log.Printf("operatorgroup %q deleted", og.Name)
+				u.Logf("operatorgroup %q deleted", og.Name)
 			}
 		}
 	}
@@ -134,7 +135,7 @@ func (u *OperatorUninstall) deleteObjects(ctx context.Context, waitForDelete boo
 		if err := u.config.Client.Delete(ctx, obj); err != nil && !apierrors.IsNotFound(err) {
 			return fmt.Errorf("delete %s %q: %v", lowerKind, obj.GetName(), err)
 		} else if err == nil {
-			log.Printf("%s %q deleted", lowerKind, obj.GetName())
+			u.Logf("%s %q deleted", lowerKind, obj.GetName())
 		}
 		if waitForDelete {
 			key, err := client.ObjectKeyFromObject(obj)
diff --git a/main.go b/main.go
@@ -2,11 +2,8 @@ package main
 
 import (
 	"github.com/operator-framework/kubectl-operator/internal/cmd"
-	"github.com/operator-framework/kubectl-operator/internal/pkg/log"
 )
 
 func main() {
-	if err := cmd.New().Execute(); err != nil {
-		log.Fatal(err)
-	}
+	cmd.Execute()
 }

Original file line number	Diff line number	Diff line change
`@@ -10,8 +10,8 @@ import (`
`10`	`10`	`v1 "k8s.io/api/core/v1"`
`11`	`11`	`"k8s.io/apimachinery/pkg/util/duration"`
`12`	`12`
	`13`	`+ "github.com/operator-framework/kubectl-operator/internal/cmd/internal/log"`
`13`	`14`	`"github.com/operator-framework/kubectl-operator/internal/pkg/action"`
`14`		`- "github.com/operator-framework/kubectl-operator/internal/pkg/log"`
`15`	`15`	`)`
`16`	`16`
`17`	`17`	`func newCatalogListCmd(cfg action.Configuration) cobra.Command {`
Original file line number	Diff line number	Diff line change
`@@ -3,8 +3,8 @@ package cmd`
`3`	`3`	`import (`
`4`	`4`	`"github.com/spf13/cobra"`
`5`	`5`
	`6`	`+ "github.com/operator-framework/kubectl-operator/internal/cmd/internal/log"`
`6`	`7`	`"github.com/operator-framework/kubectl-operator/internal/pkg/action"`
`7`		`- "github.com/operator-framework/kubectl-operator/internal/pkg/log"`
`8`	`8`	`)`
`9`	`9`
`10`	`10`	`func newCatalogRemoveCmd(cfg action.Configuration) cobra.Command {`
Original file line number	Diff line number	Diff line change
`@@ -12,8 +12,8 @@ import (`
`12`	`12`	`v1 "k8s.io/api/core/v1"`
`13`	`13`	`"k8s.io/apimachinery/pkg/util/duration"`
`14`	`14`
	`15`	`+ "github.com/operator-framework/kubectl-operator/internal/cmd/internal/log"`
`15`	`16`	`"github.com/operator-framework/kubectl-operator/internal/pkg/action"`
`16`		`- "github.com/operator-framework/kubectl-operator/internal/pkg/log"`
`17`	`17`	`)`
`18`	`18`
`19`	`19`	`func newOperatorListCmd(cfg action.Configuration) cobra.Command {`
Original file line number	Diff line number	Diff line change
`@@ -5,8 +5,8 @@ import (`
`5`	`5`
`6`	`6`	`"github.com/spf13/cobra"`
`7`	`7`
	`8`	`+ "github.com/operator-framework/kubectl-operator/internal/cmd/internal/log"`
`8`	`9`	`"github.com/operator-framework/kubectl-operator/internal/pkg/action"`
`9`		`- "github.com/operator-framework/kubectl-operator/internal/pkg/log"`
`10`	`10`	`)`
`11`	`11`
`12`	`12`	`func newOperatorUpgradeCmd(cfg action.Configuration) cobra.Command {`