cmd: switch to already vendored cobra

This allows us to use the already vendored cobra cli package, and drop gopkg.in/alecthomas/kingpin.v2. This also splits cli into 3 different command groups: targets, destroy and version. cobra allows us to add new subcommands and local flags without putting everything into single switch in main.go
wking · Oct 8, 2018 · dc118f2 · dc118f2
1 parent 9611a0e
commit dc118f2
Show file tree

Hide file tree

Showing 4 changed files with 184 additions and 101 deletions.
diff --git a/cmd/openshift-install/destroy.go b/cmd/openshift-install/destroy.go
@@ -0,0 +1,31 @@
+package main
+
+import (
+	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
+	"github.com/spf13/cobra"
+
+	"github.com/openshift/installer/pkg/destroy"
+	_ "github.com/openshift/installer/pkg/destroy/libvirt"
+)
+
+func newDestroyCmd() *cobra.Command {
+	return &cobra.Command{
+		Use:   "destroy-cluster",
+		Short: "Destroy an OpenShift cluster",
+		Long:  "",
+		RunE:  runDestroyCmd,
+	}
+}
+
+func runDestroyCmd(cmd *cobra.Command, args []string) error {
+	destroyer, err := destroy.New(logrus.StandardLogger(), rootOpts.dir)
+	if err != nil {
+		return errors.Wrap(err, "Failed while preparing to destroy cluster")
+	}
+	if err := destroyer.Run(); err != nil {
+		return errors.Wrap(err, "Failed to destroy cluster")
+
+	}
+	return nil
+}
diff --git a/cmd/openshift-install/main.go b/cmd/openshift-install/main.go
@@ -1,121 +1,57 @@
 package main
 
 import (
-	"fmt"
-	"os"
-	"os/exec"
-	"strings"
-
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
-	"gopkg.in/alecthomas/kingpin.v2"
-
-	"github.com/openshift/installer/pkg/asset"
-	"github.com/openshift/installer/pkg/asset/cluster"
-	"github.com/openshift/installer/pkg/asset/ignition/bootstrap"
-	"github.com/openshift/installer/pkg/asset/ignition/machine"
-	"github.com/openshift/installer/pkg/asset/installconfig"
-	"github.com/openshift/installer/pkg/asset/kubeconfig"
-	"github.com/openshift/installer/pkg/asset/manifests"
-	"github.com/openshift/installer/pkg/asset/metadata"
-	"github.com/openshift/installer/pkg/destroy"
-	_ "github.com/openshift/installer/pkg/destroy/libvirt"
-	"github.com/openshift/installer/pkg/terraform"
+	"github.com/spf13/cobra"
 )
 
 var (
-	installConfigCommand   = kingpin.Command("install-config", "Generate the Install Config asset")
-	ignitionConfigsCommand = kingpin.Command("ignition-configs", "Generate the Ignition Config assets")
-	manifestsCommand       = kingpin.Command("manifests", "Generate the Kubernetes manifests")
-	clusterCommand         = kingpin.Command("cluster", "Create an OpenShift cluster")
-	versionCommand         = kingpin.Command("version", "Print version information and exit")
-
-	destroyCommand = kingpin.Command("destroy-cluster", "Destroy an OpenShift cluster")
-
-	dirFlag  = kingpin.Flag("dir", "assets directory").Default(".").String()
-	logLevel = kingpin.Flag("log-level", "log level (e.g. \"debug\")").Default("info").Enum("debug", "info", "warn", "error")
-
-	version = "was not built correctly" // set in hack/build.sh
+	rootOpts struct {
+		dir      string
+		logLevel string
+	}
 )
 
 func main() {
-	command := kingpin.Parse()
-
-	logrus.SetFormatter(&logrus.TextFormatter{
-		DisableTimestamp:       true,
-		DisableLevelTruncation: true,
-	})
-	if level, err := logrus.ParseLevel(*logLevel); err == nil {
-		logrus.SetLevel(level)
-	} else {
-		// By definition we should never enter this condition since kingpin should be guarding against incorrect values.
-		logrus.Panicf("Invalid log-level: %v", err)
+	rootCmd := newRootCmd()
+	subCmds := []*cobra.Command{
+		newInstallConfigCmd(), newIgnitionConfigsCmd(), newManifestsCmd(), newClusterCmd(),
+		newDestroyCmd(),
+		newVersionCmd(),
+	}
+	for _, subCmd := range subCmds {
+		rootCmd.AddCommand(subCmd)
 	}
 
-	if command == versionCommand.FullCommand() {
-		fmt.Printf("%s %s\n", os.Args[0], version)
-		terraformVersion, err := terraform.Version()
-		if err != nil {
-			exitError, ok := err.(*exec.ExitError)
-			if ok && len(exitError.Stderr) > 0 {
-				logrus.Error(strings.Trim(string(exitError.Stderr), "\n"))
-			}
-			logrus.Fatalf("Failed to calculate Terraform version: %v", err)
-		}
-		fmt.Println(terraformVersion)
-		return
+	if err := rootCmd.Execute(); err != nil {
+		cause := errors.Cause(err)
+		logrus.Fatalf("Error executing openshift-intall: %v", cause)
 	}
+}
 
-	var targetAssets []asset.WritableAsset
-	switch command {
-	case installConfigCommand.FullCommand():
-		targetAssets = []asset.WritableAsset{&installconfig.InstallConfig{}}
-	case ignitionConfigsCommand.FullCommand():
-		targetAssets = []asset.WritableAsset{
-			&bootstrap.Bootstrap{},
-			&machine.Master{},
-			&machine.Worker{},
-		}
-	case manifestsCommand.FullCommand():
-		targetAssets = []asset.WritableAsset{
-			&manifests.Manifests{},
-			&manifests.Tectonic{},
-		}
-	case clusterCommand.FullCommand():
-		targetAssets = []asset.WritableAsset{
-			&cluster.TerraformVariables{},
-			&kubeconfig.Admin{},
-			&cluster.Cluster{},
-			&metadata.Metadata{},
-		}
+func newRootCmd() *cobra.Command {
+	cmd := &cobra.Command{
+		Use:               "openshift-install",
+		Short:             "Creates OpenShift clusters",
+		Long:              "",
+		PersistentPreRunE: runRootCmd,
 	}
+	cmd.PersistentFlags().StringVar(&rootOpts.dir, "dir", ".", "assets directory")
+	cmd.PersistentFlags().StringVar(&rootOpts.logLevel, "log-level", "info", "log level (e.g. \"debug | info | warn | error\")")
+	return cmd
+}
 
-	switch command {
-	case installConfigCommand.FullCommand(),
-		ignitionConfigsCommand.FullCommand(),
-		manifestsCommand.FullCommand(),
-		clusterCommand.FullCommand():
-		assetStore := &asset.StoreImpl{}
-		for _, a := range targetAssets {
-			err := assetStore.Fetch(a)
-			if err != nil {
-				if exitError, ok := errors.Cause(err).(*exec.ExitError); ok && len(exitError.Stderr) > 0 {
-					logrus.Error(strings.Trim(string(exitError.Stderr), "\n"))
-				}
-				logrus.Fatalf("Failed to generate %s: %v", a.Name(), err)
-			}
+func runRootCmd(cmd *cobra.Command, args []string) error {
+	logrus.SetFormatter(&logrus.TextFormatter{
+		DisableTimestamp:       true,
+		DisableLevelTruncation: true,
+	})
+	level, err := logrus.ParseLevel(rootOpts.logLevel)
+	if err != nil {
+		return errors.Wrap(err, "invalid log-level")
 
-			if err := asset.PersistToFile(a, *dirFlag); err != nil {
-				logrus.Fatalf("Failed to write asset (%s) to disk: %v", a.Name(), err)
-			}
-		}
-	case destroyCommand.FullCommand():
-		destroyer, err := destroy.New(logrus.StandardLogger(), *dirFlag)
-		if err != nil {
-			logrus.Fatalf("Failed while preparing to destroy cluster: %v", err)
-		}
-		if err := destroyer.Run(); err != nil {
-			logrus.Fatalf("Failed to destroy cluster: %v", err)
-		}
 	}
+	logrus.SetLevel(level)
+	return nil
 }
diff --git a/cmd/openshift-install/targets.go b/cmd/openshift-install/targets.go
@@ -0,0 +1,75 @@
+package main
+
+import (
+	"os/exec"
+	"strings"
+
+	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
+	"github.com/spf13/cobra"
+
+	"github.com/openshift/installer/pkg/asset"
+	"github.com/openshift/installer/pkg/asset/cluster"
+	"github.com/openshift/installer/pkg/asset/ignition/bootstrap"
+	"github.com/openshift/installer/pkg/asset/ignition/machine"
+	"github.com/openshift/installer/pkg/asset/installconfig"
+	"github.com/openshift/installer/pkg/asset/kubeconfig"
+	"github.com/openshift/installer/pkg/asset/manifests"
+	"github.com/openshift/installer/pkg/asset/metadata"
+)
+
+func newInstallConfigCmd() *cobra.Command {
+	return &cobra.Command{
+		Use:   "install-config",
+		Short: "Generates the Install Config asset",
+		Long:  "",
+		RunE:  runTargetCmd(&installconfig.InstallConfig{}),
+	}
+}
+
+func newIgnitionConfigsCmd() *cobra.Command {
+	return &cobra.Command{
+		Use:   "ignition-configs",
+		Short: "Generates the Ignition Config asset",
+		Long:  "",
+		RunE:  runTargetCmd(&bootstrap.Bootstrap{}, &machine.Master{}, &machine.Worker{}),
+	}
+}
+
+func newManifestsCmd() *cobra.Command {
+	return &cobra.Command{
+		Use:   "manifests",
+		Short: "Generates the Kubernetes manifests",
+		Long:  "",
+		RunE:  runTargetCmd(&manifests.Manifests{}, &manifests.Tectonic{}),
+	}
+}
+
+func newClusterCmd() *cobra.Command {
+	return &cobra.Command{
+		Use:   "cluster",
+		Short: "Create an OpenShift cluster",
+		Long:  "",
+		RunE:  runTargetCmd(&cluster.TerraformVariables{}, &kubeconfig.Admin{}, &cluster.Cluster{}, &metadata.Metadata{}),
+	}
+}
+
+func runTargetCmd(targets ...asset.WritableAsset) func(cmd *cobra.Command, args []string) error {
+	return func(cmd *cobra.Command, args []string) error {
+		assetStore := &asset.StoreImpl{}
+		for _, a := range targets {
+			err := assetStore.Fetch(a)
+			if err != nil {
+				if exitError, ok := errors.Cause(err).(*exec.ExitError); ok && len(exitError.Stderr) > 0 {
+					logrus.Error(strings.Trim(string(exitError.Stderr), "\n"))
+				}
+				return errors.Wrapf(err, "failed to generate %s", a.Name())
+			}
+
+			if err := asset.PersistToFile(a, rootOpts.dir); err != nil {
+				return errors.Wrapf(err, "failed to write asset (%s) to disk", a.Name())
+			}
+		}
+		return nil
+	}
+}
diff --git a/cmd/openshift-install/version.go b/cmd/openshift-install/version.go
@@ -0,0 +1,41 @@
+package main
+
+import (
+	"fmt"
+	"os"
+	"os/exec"
+	"strings"
+
+	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
+	"github.com/spf13/cobra"
+
+	"github.com/openshift/installer/pkg/terraform"
+)
+
+var (
+	version = "was not built correctly" // set in hack/build.sh
+)
+
+func newVersionCmd() *cobra.Command {
+	return &cobra.Command{
+		Use:   "version",
+		Short: "Print version information",
+		Long:  "",
+		RunE:  runVersionCmd,
+	}
+}
+
+func runVersionCmd(cmd *cobra.Command, args []string) error {
+	fmt.Printf("%s %s\n", os.Args[0], version)
+	terraformVersion, err := terraform.Version()
+	if err != nil {
+		exitError, ok := err.(*exec.ExitError)
+		if ok && len(exitError.Stderr) > 0 {
+			logrus.Error(strings.Trim(string(exitError.Stderr), "\n"))
+		}
+		return errors.Wrap(err, "Failed to calculate Terraform version")
+	}
+	fmt.Println(terraformVersion)
+	return nil
+}