cmd/catalog: Migrate to using cobra for CLI flag management

cmd/catalog/main.go

@@ -2,14 +2,13 @@ package main
 
 import (
 	"context"
-	"flag"
 	"fmt"
 	"net/http"
 	"os"
 	"time"
 
 	configv1client "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1"
-	log "github.com/sirupsen/logrus"
+	"github.com/sirupsen/logrus"
 	utilclock "k8s.io/apimachinery/pkg/util/clock"
 	k8sscheme "k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/tools/clientcmd"
@@ -20,9 +19,7 @@ import (
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorclient"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorstatus"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/server"
-	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/signals"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/metrics"
-	olmversion "github.com/operator-framework/operator-lifecycle-manager/pkg/version"
 )
 
 const (
@@ -36,80 +33,33 @@ const (
 )
 
 // config flags defined globally so that they appear on the test binary as well
-var (
-	kubeConfigPath = flag.String(
-		"kubeconfig", os.Getenv("KUBECONFIG"), "absolute path to the kubeconfig file")
-
-	wakeupInterval = flag.Duration(
-		"interval", defaultWakeupInterval, "wakeup interval")
-
-	catalogNamespace = flag.String(
-		"namespace", defaultCatalogNamespace, "namespace where catalog will run and install catalog resources")
-
-	configmapServerImage = flag.String(
-		"configmapServerImage", defaultConfigMapServerImage, "the image to use for serving the operator registry api for a configmap")
-
-	opmImage = flag.String(
-		"opmImage", defaultOPMImage, "the image to use for unpacking bundle content with opm")
-
-	utilImage = flag.String(
-		"util-image", defaultUtilImage, "an image containing custom olm utilities")
-
-	writeStatusName = flag.String(
-		"writeStatusName", defaultOperatorName, "ClusterOperator name in which to write status, set to \"\" to disable.")
-
-	debug = flag.Bool(
-		"debug", false, "use debug log level")
-
-	version = flag.Bool("version", false, "displays olm version")
-
-	tlsKeyPath = flag.String(
-		"tls-key", "", "Path to use for private key (requires tls-cert)")
-
-	tlsCertPath = flag.String(
-		"tls-cert", "", "Path to use for certificate key (requires tls-key)")
-
-	_ = flag.Bool("profiling", false, "deprecated")
-
-	clientCAPath = flag.String("client-ca", "", "path to watch for client ca bundle")
-
-	installPlanTimeout  = flag.Duration("install-plan-retry-timeout", 1*time.Minute, "time since first attempt at which plan execution errors are considered fatal")
-	bundleUnpackTimeout = flag.Duration("bundle-unpack-timeout", 10*time.Minute, "The time limit for bundle unpacking, after which InstallPlan execution is considered to have failed. 0 is considered as having no timeout.")
-)
 
 func init() {
 	metrics.RegisterCatalog()
 }
 
 func main() {
-	// Get exit signal context
-	ctx, cancel := context.WithCancel(signals.Context())
-	defer cancel()
-
-	// Parse the command-line flags.
-	flag.Parse()
-
-	// Check if version flag was set
-	if *version {
-		fmt.Print(olmversion.String())
-		os.Exit(0)
-	}
-
-	logger := log.New()
-	if *debug {
-		logger.SetLevel(log.DebugLevel)
+	cmd := newRootCmd()
+	if err := cmd.Execute(); err != nil {
+		fmt.Fprintln(os.Stderr, err)
+		os.Exit(1)
 	}
-	logger.Infof("log level %s", logger.Level)
+}
 
+func (o *options) run(ctx context.Context, logger *logrus.Logger) error {
 	// If the catalogNamespaceEnvVarName environment variable is set, then  update the value of catalogNamespace.
 	if catalogNamespaceEnvVarValue := os.Getenv(catalogNamespaceEnvVarName); catalogNamespaceEnvVarValue != "" {
 		logger.Infof("%s environment variable is set. Updating Global Catalog Namespace to %s", catalogNamespaceEnvVarName, catalogNamespaceEnvVarValue)
-		*catalogNamespace = catalogNamespaceEnvVarValue
+		o.catalogNamespace = catalogNamespaceEnvVarValue
 	}
 
-	listenAndServe, err := server.GetListenAndServeFunc(server.WithLogger(logger), server.WithTLS(tlsCertPath, tlsKeyPath, clientCAPath), server.WithDebug(*debug))
+	listenAndServe, err := server.GetListenAndServeFunc(
+		server.WithLogger(logger),
+		server.WithTLS(&o.tlsCertPath, &o.tlsKeyPath, &o.clientCAPath),
+		server.WithDebug(o.debug),
+	)
 	if err != nil {
-		logger.Fatalf("Error setting up health/metric/pprof service: %v", err)
+		return fmt.Errorf("error setting up health/metric/pprof service: %v", err)
 	}
 
 	go func() {
@@ -119,29 +69,49 @@ func main() {
 	}()
 
 	// create a config client for operator status
-	config, err := clientcmd.BuildConfigFromFlags("", *kubeConfigPath)
+	config, err := clientcmd.BuildConfigFromFlags("", o.kubeconfig)
 	if err != nil {
-		log.Fatalf("error configuring client: %s", err.Error())
+		return fmt.Errorf("error configuring client: %s", err.Error())
 	}
 	configClient, err := configv1client.NewForConfig(config)
 	if err != nil {
-		log.Fatalf("error configuring client: %s", err.Error())
+		return fmt.Errorf("error configuring client: %s", err.Error())
 	}
-	opClient := operatorclient.NewClientFromConfig(*kubeConfigPath, logger)
-	crClient, err := client.NewClient(*kubeConfigPath)
+	opClient := operatorclient.NewClientFromConfig(o.kubeconfig, logger)
+	crClient, err := client.NewClient(o.kubeconfig)
 	if err != nil {
-		log.Fatalf("error configuring client: %s", err.Error())
+		return fmt.Errorf("error configuring client: %s", err.Error())
 	}
 
+	// TODO(tflannag): Use options pattern for catalog operator
 	// Create a new instance of the operator.
-	op, err := catalog.NewOperator(ctx, *kubeConfigPath, utilclock.RealClock{}, logger, *wakeupInterval, *configmapServerImage, *opmImage, *utilImage, *catalogNamespace, k8sscheme.Scheme, *installPlanTimeout, *bundleUnpackTimeout)
+	op, err := catalog.NewOperator(
+		ctx,
+		o.kubeconfig,
+		utilclock.RealClock{},
+		logger,
+		o.wakeupInterval,
+		o.configMapServerImage,
+		o.opmImage,
+		o.utilImage,
+		o.catalogNamespace,
+		k8sscheme.Scheme,
+		o.installPlanTimeout,
+		o.bundleUnpackTimeout,
+	)
 	if err != nil {
-		log.Fatalf("error configuring catalog operator: %s", err.Error())
+		return fmt.Errorf("error configuring catalog operator: %s", err.Error())
 	}
 
-	opCatalogTemplate, err := catalogtemplate.NewOperator(ctx, *kubeConfigPath, logger, *wakeupInterval, *catalogNamespace)
+	opCatalogTemplate, err := catalogtemplate.NewOperator(
+		ctx,
+		o.kubeconfig,
+		logger,
+		o.wakeupInterval,
+		o.catalogNamespace,
+	)
 	if err != nil {
-		log.Fatalf("error configuring catalog template operator: %s", err.Error())
+		return fmt.Errorf("error configuring catalog template operator: %s", err.Error())
 	}
 
 	op.Run(ctx)
@@ -150,9 +120,11 @@ func main() {
 	opCatalogTemplate.Run(ctx)
 	<-opCatalogTemplate.Ready()
 
-	if *writeStatusName != "" {
-		operatorstatus.MonitorClusterStatus(*writeStatusName, op.AtLevel(), op.Done(), opClient, configClient, crClient)
+	if o.writeStatusName != "" {
+		operatorstatus.MonitorClusterStatus(o.writeStatusName, op.AtLevel(), op.Done(), opClient, configClient, crClient)
 	}
 
 	<-op.Done()
+
+	return nil
 }

cmd/catalog/start.go

@@ -0,0 +1,84 @@
+package main
+
+import (
+	"context"
+	"fmt"
+	"os"
+	"time"
+
+	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/signals"
+	olmversion "github.com/operator-framework/operator-lifecycle-manager/pkg/version"
+	"github.com/sirupsen/logrus"
+	"github.com/spf13/cobra"
+)
+
+type options struct {
+	kubeconfig           string
+	catalogNamespace     string
+	configMapServerImage string
+	opmImage             string
+	utilImage            string
+	writeStatusName      string
+	debug                bool
+	version              bool
+	profiling            bool
+	tlsKeyPath           string
+	tlsCertPath          string
+	clientCAPath         string
+
+	installPlanTimeout  time.Duration
+	bundleUnpackTimeout time.Duration
+	wakeupInterval      time.Duration
+}
+
+func newRootCmd() *cobra.Command {
+	o := options{}
+
+	cmd := &cobra.Command{
+		Use:          "Start",
+		Short:        "Starts the Catalog Operator",
+		SilenceUsage: true,
+		RunE: func(cmd *cobra.Command, args []string) error {
+			if o.version {
+				fmt.Print(olmversion.String())
+				return nil
+			}
+
+			logger := logrus.New()
+			if o.debug {
+				logger.SetLevel(logrus.DebugLevel)
+			}
+			logger.Infof("log level %s", logger.Level)
+
+			ctx, cancel := context.WithCancel(signals.Context())
+			defer cancel()
+
+			if err := o.run(ctx, logger); err != nil {
+				return err
+			}
+			return nil
+		},
+	}
+
+	cmd.Flags().StringVar(&o.kubeconfig, "kubeconfig", os.Getenv("KUBECONFIG"), "absolute path to the kubeconfig file")
+	cmd.Flags().StringVar(&o.catalogNamespace, "namespace", defaultCatalogNamespace, "namespace where catalog will run and install catalog resources")
+	cmd.Flags().StringVar(&o.configMapServerImage, "configmapServerImage", defaultConfigMapServerImage, "the image to use for serving the operator registry api for a configmap")
+	cmd.Flags().StringVar(&o.opmImage, "opmImage", defaultOPMImage, "the image to use for unpacking bundle content with opm")
+	cmd.Flags().StringVar(&o.utilImage, "util-image", defaultUtilImage, "an image containing custom olm utilities")
+	cmd.Flags().StringVar(&o.writeStatusName, "writeStatusName", defaultOperatorName, "ClusterOperator name in which to write status, set to \"\" to disable.")
+
+	cmd.Flags().BoolVar(&o.debug, "debug", false, "use debug log level")
+	cmd.Flags().BoolVar(&o.version, "version", false, "displays the olm version")
+	cmd.Flags().BoolVar(&o.profiling, "profiling", false, "deprecated")
+	cmd.Flags().MarkDeprecated("profiling", "profiling is now enabled by default")
+
+	cmd.Flags().StringVar(&o.tlsKeyPath, "tls-key", "", "path to use for private key (requires tls-cert)")
+	cmd.Flags().StringVar(&o.tlsCertPath, "tls-cert", "", "path to use for certificate key (requires tls-key)")
+	cmd.Flags().StringVar(&o.clientCAPath, "client-ca", "", "path to watch for client ca bundle")
+
+	cmd.Flags().DurationVar(&o.wakeupInterval, "interval", defaultWakeupInterval, "wakeup interval")
+	cmd.Flags().DurationVar(&o.bundleUnpackTimeout, "bundle-unpack-timeout", 10*time.Minute, "The time limit for bundle unpacking, after which InstallPlan execution is considered to have failed. 0 is considered as having no timeout.")
+	cmd.Flags().DurationVar(&o.installPlanTimeout, "install-plan-retry-timeout", 1*time.Minute, "time since first attempt at which plan execution errors are considered fatal")
+
+	return cmd
+}

deploy/chart/templates/0000_50_olm_08-catalog-operator.deployment.yaml

@@ -19,7 +19,7 @@ spec:
     spec:
       serviceAccountName: olm-operator-serviceaccount
       {{- if or .Values.catalog.tlsSecret .Values.catalog.clientCASecret }}
-      volumes: 
+      volumes:
       {{- end }}
       {{- if .Values.catalog.tlsSecret }}
       - name: srv-cert
@@ -49,21 +49,21 @@ spec:
           command:
           - /bin/catalog
           args:
-          - '-namespace'
+          - '--namespace'
           - {{ .Values.catalog_namespace }}
           {{- if .Values.debug }}
-          - '-debug'
+          - '--debug'
           {{- end }}
           {{- if .Values.catalog.commandArgs }}
           - {{ .Values.catalog.commandArgs }}
           {{- end }}
           {{- if .Values.catalog.opmImageArgs }}
           - {{ .Values.catalog.opmImageArgs }}
           {{- end }}
-          - -util-image
+          - --util-image
           -  {{ .Values.catalog.image.ref }}
           {{- if .Values.writeStatusNameCatalog }}
-          - -writeStatusName
+          - --writeStatusName
           - {{ .Values.writeStatusNameCatalog }}
           {{- end }}
           {{- if .Values.catalog.tlsSecret }}

deploy/chart/values.yaml

@@ -26,7 +26,7 @@ olm:
 
 catalog:
   replicaCount: 1
-  commandArgs: -configmapServerImage=quay.io/operator-framework/configmap-operator-registry:latest
+  commandArgs: --configmapServerImage=quay.io/operator-framework/configmap-operator-registry:latest
   image:
     ref: quay.io/operator-framework/olm:master
     pullPolicy: Always