Minor CLI refactoring, mostly changing names (#323)

pkg/kudoctl/cmd/get/instances.go

@@ -36,7 +36,7 @@ func NewGetInstancesCmd() *cobra.Command {
 		Long: `
 	# Get all available instances
 	kudoctl get instances`,
-		Run: instancesGetCmd,
+		Run: run,
 	}
 
 	getCmd.Flags().StringVar(&kubeConfig, "kubeconfig", "", "The file path to kubernetes configuration file; defaults to $HOME/.kube/config")
@@ -45,7 +45,7 @@ func NewGetInstancesCmd() *cobra.Command {
 	return getCmd
 }
 
-func instancesGetCmd(cmd *cobra.Command, args []string) {
+func run(cmd *cobra.Command, args []string) {
 
 	mustKubeConfig()
 

pkg/kudoctl/cmd/install.go

@@ -28,7 +28,7 @@ func newInstallCmd() *cobra.Command {
 		Long:    `Install a KUDO package from the official GitHub repo.`,
 		Example: installExample,
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return install.RunInstall(cmd, args, options)
+			return install.Run(cmd, args, options)
 		},
 		SilenceUsage: true,
 	}

pkg/kudoctl/cmd/install/install.go

@@ -30,8 +30,8 @@ var DefaultOptions = &Options{
 	Namespace: "default",
 }
 
-// RunInstall returns the errors associated with cmd env
-func RunInstall(cmd *cobra.Command, args []string, options *Options) error {
+// Run returns the errors associated with cmd env
+func Run(cmd *cobra.Command, args []string, options *Options) error {
 
 	// This makes --kubeconfig flag optional
 	if _, err := cmd.Flags().GetString("kubeconfig"); err != nil {
@@ -115,7 +115,7 @@ func installFrameworks(args []string, options *Options) error {
 		return errors.Wrap(err, "getting config failed")
 	}
 
-	kc, err := kudo.NewKudoClient(options.Namespace, options.KubeConfigPath)
+	kc, err := kudo.NewClient(options.Namespace, options.KubeConfigPath)
 	if err != nil {
 		return errors.Wrap(err, "creating kudo client")
 	}

pkg/kudoctl/cmd/plan/plan_history.go

@@ -3,9 +3,11 @@ package plan
 import (
 	"encoding/json"
 	"fmt"
-	"log"
 	"time"
 
+	"github.com/kudobuilder/kudo/pkg/kudoctl/util/check"
+	"github.com/pkg/errors"
+
 	kudov1alpha1 "github.com/kudobuilder/kudo/pkg/apis/kudo/v1alpha1"
 	"github.com/spf13/cobra"
 	"github.com/xlab/treeprint"
@@ -15,49 +17,68 @@ import (
 	"k8s.io/client-go/tools/clientcmd"
 )
 
+type historyOptions struct {
+	instance       string
+	kubeConfigPath string
+	namespace      string
+}
+
+var defaultHistoryOptions = &historyOptions{}
+
 // NewPlanHistoryCmd creates a command that shows the plan instory for an instance
 func NewPlanHistoryCmd() *cobra.Command {
+	options := defaultHistoryOptions
 	listCmd := &cobra.Command{
 		//Args: cobra.ExactArgs(1),
 		Use:   "history",
 		Short: "Lists history to a specific framework-version of an instance.",
 		Long: `
 	# View plan status
 	kudoctl plan history <frameworkVersion> --instance=<instanceName>`,
-		Run: planHistoryCmd,
+		RunE: func(cmd *cobra.Command, args []string) error {
+			return runHistory(cmd, args, options)
+		},
 	}
 
-	listCmd.Flags().StringVar(&instance, "instance", "", "The instance name.")
-	listCmd.Flags().StringVar(&kubeConfig, "kubeconfig", "", "The file path to kubernetes configuration file; defaults to $HOME/.kube/config")
-	listCmd.Flags().StringVar(&namespace, "namespace", "default", "The namespace where the operator watches for changes.")
+	listCmd.Flags().StringVar(&options.instance, "instance", "", "The instance name.")
+	listCmd.Flags().StringVar(&options.kubeConfigPath, "kubeconfig", "", "The file path to kubernetes configuration file; defaults to $HOME/.kube/config")
+	listCmd.Flags().StringVar(&options.namespace, "namespace", "default", "The namespace where the operator watches for changes.")
 
 	return listCmd
 }
 
-func planHistoryCmd(cmd *cobra.Command, args []string) {
+func runHistory(cmd *cobra.Command, args []string, options *historyOptions) error {
 
 	instanceFlag, err := cmd.Flags().GetString("instance")
 	if err != nil || instanceFlag == "" {
-		log.Fatal("Flag Error: Please set instance flag, e.g. \"--instance=<instanceName>\"")
+		return fmt.Errorf("flag Error: Please set instance flag, e.g. \"--instance=<instanceName>\"")
 	}
 
-	mustKubeConfig()
+	configPath, err := check.KubeConfigLocationOrDefault(options.kubeConfigPath)
+	if err != nil {
+		return fmt.Errorf("error when getting default kubeconfig path: %+v", err)
+	}
+	options.kubeConfigPath = configPath
+	if err := check.ValidateKubeConfigPath(options.kubeConfigPath); err != nil {
+		return errors.WithMessage(err, "could not check kubeconfig path")
+	}
 
 	_, err = cmd.Flags().GetString("kubeconfig")
 	// Todo: wrong flag
 	if err != nil || instanceFlag == "" {
-		log.Fatalf("Flag Error: %v", err)
+		return fmt.Errorf("flag Error: %v", err)
 	}
 
-	err = planHistory(args)
+	err = planHistory(args, options)
 	if err != nil {
-		log.Fatalf("Client Error: %v", err)
+		return fmt.Errorf("client Error: %v", err)
 	}
+	return nil
 }
 
-func planHistory(args []string) error {
+func planHistory(args []string, options *historyOptions) error {
 
-	config, err := clientcmd.BuildConfigFromFlags("", kubeConfig)
+	config, err := clientcmd.BuildConfigFromFlags("", options.kubeConfigPath)
 	if err != nil {
 		return err
 	}
@@ -76,14 +97,14 @@ func planHistory(args []string) error {
 
 	var labelSelector string
 	if len(args) == 0 {
-		fmt.Printf("History of all plan-executions for instance \"%s\" in namespace \"%s\":\n", instance, namespace)
-		labelSelector = "instance=" + instance
+		fmt.Printf("History of all plan-executions for instance \"%s\" in namespace \"%s\":\n", options.instance, options.namespace)
+		labelSelector = "instance=" + options.instance
 	} else {
-		fmt.Printf("History of plan-executions for instance \"%s\" in namespace \"%s\" to framework-version \"%s\":\n", instance, namespace, args[0])
-		labelSelector = "framework-version=" + args[0] + ", instance=" + instance
+		fmt.Printf("History of plan-executions for instance \"%s\" in namespace \"%s\" to framework-version \"%s\":\n", options.instance, options.namespace, args[0])
+		labelSelector = "framework-version=" + args[0] + ", instance=" + options.instance
 	}
 
-	instObj, err := dynamicClient.Resource(planExecutionsGVR).Namespace(namespace).List(metav1.ListOptions{
+	instObj, err := dynamicClient.Resource(planExecutionsGVR).Namespace(options.namespace).List(metav1.ListOptions{
 		LabelSelector: labelSelector,
 	})
 	if err != nil {
@@ -105,7 +126,7 @@ func planHistory(args []string) error {
 	tree := treeprint.New()
 
 	if len(planExecutionList.Items) == 0 {
-		fmt.Printf("No history found for \"%s\" in namespace \"%s\".\n", instance, namespace)
+		fmt.Printf("No history found for \"%s\" in namespace \"%s\".\n", options.instance, options.namespace)
 	} else {
 		for _, i := range planExecutionList.Items {
 			duration := time.Since(i.CreationTimestamp.Time)

pkg/kudoctl/cmd/plan/plan_status.go

@@ -3,9 +3,10 @@ package plan
 import (
 	"encoding/json"
 	"fmt"
-	"log"
 
 	kudov1alpha1 "github.com/kudobuilder/kudo/pkg/apis/kudo/v1alpha1"
+	"github.com/kudobuilder/kudo/pkg/kudoctl/util/check"
+	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 	"github.com/xlab/treeprint"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -14,49 +15,68 @@ import (
 	"k8s.io/client-go/tools/clientcmd"
 )
 
+type statusOptions struct {
+	instance       string
+	kubeConfigPath string
+	namespace      string
+}
+
+var defaultStatusOptions = &statusOptions{}
+
 //NewPlanStatusCmd creates a new command that shows the status of an instance by looking at its current plan
 func NewPlanStatusCmd() *cobra.Command {
+	options := defaultStatusOptions
 	statusCmd := &cobra.Command{
 		Use:   "status",
 		Short: "Shows the status of all plans to an particular instance.",
 		Long: `
 	# View plan status
 	kudoctl plan status --instance=<instanceName> --kubeconfig=<$HOME/.kube/config>`,
-		Run: planStatusCmd,
+		RunE: func(cmd *cobra.Command, args []string) error {
+			return runStatus(cmd, args, options)
+		},
 	}
 
-	statusCmd.Flags().StringVar(&instance, "instance", "", "The instance name available from 'kubectl get instances'")
-	statusCmd.Flags().StringVar(&kubeConfig, "kubeconfig", "", "The file path to kubernetes configuration file; defaults to $HOME/.kube/config")
-	statusCmd.Flags().StringVar(&namespace, "namespace", "default", "The namespace where the instance is running.")
+	statusCmd.Flags().StringVar(&options.instance, "instance", "", "The instance name available from 'kubectl get instances'")
+	statusCmd.Flags().StringVar(&options.kubeConfigPath, "kubeconfig", "", "The file path to kubernetes configuration file; defaults to $HOME/.kube/config")
+	statusCmd.Flags().StringVar(&options.namespace, "namespace", "default", "The namespace where the instance is running.")
 
 	return statusCmd
 }
 
-func planStatusCmd(cmd *cobra.Command, args []string) {
+func runStatus(cmd *cobra.Command, args []string, options *statusOptions) error {
 
 	instanceFlag, err := cmd.Flags().GetString("instance")
 	if err != nil || instanceFlag == "" {
-		log.Fatal("Flag Error: Please set instance flag, e.g. \"--instance=<instanceName>\"")
+		return fmt.Errorf("flag Error: Please set instance flag, e.g. \"--instance=<instanceName>\"")
 	}
 
-	mustKubeConfig()
+	configPath, err := check.KubeConfigLocationOrDefault(options.kubeConfigPath)
+	if err != nil {
+		return fmt.Errorf("error when getting default kubeconfig path: %+v", err)
+	}
+	options.kubeConfigPath = configPath
+	if err := check.ValidateKubeConfigPath(options.kubeConfigPath); err != nil {
+		return errors.WithMessage(err, "could not check kubeconfig path")
+	}
 
 	_, err = cmd.Flags().GetString("kubeconfig")
 	if err != nil || instanceFlag == "" {
-		log.Fatal("Flag Error: Please set kubeconfig flag, e.g. \"--kubeconfig=<$HOME/.kube/config>\"")
+		return fmt.Errorf("flag Error: Please set kubeconfig flag, e.g. \"--kubeconfig=<$HOME/.kube/config>\"")
 	}
 
-	err = planStatus()
+	err = planStatus(options)
 	if err != nil {
-		log.Fatalf("Client Error: %v", err)
+		return fmt.Errorf("client Error: %v", err)
 	}
+	return nil
 }
 
-func planStatus() error {
+func planStatus(options *statusOptions) error {
 
 	tree := treeprint.New()
 
-	config, err := clientcmd.BuildConfigFromFlags("", kubeConfig)
+	config, err := clientcmd.BuildConfigFromFlags("", options.kubeConfigPath)
 	if err != nil {
 		return err
 	}
@@ -73,7 +93,7 @@ func planStatus() error {
 		Resource: "instances",
 	}
 
-	instObj, err := dynamicClient.Resource(instancesGVR).Namespace(namespace).Get(instance, metav1.GetOptions{})
+	instObj, err := dynamicClient.Resource(instancesGVR).Namespace(options.namespace).Get(options.instance, metav1.GetOptions{})
 	if err != nil {
 		return err
 	}
@@ -99,7 +119,7 @@ func planStatus() error {
 	}
 
 	//  List all of the Virtual Services.
-	frameworkObj, err := dynamicClient.Resource(frameworkGVR).Namespace(namespace).Get(frameworkVersionNameOfInstance, metav1.GetOptions{})
+	frameworkObj, err := dynamicClient.Resource(frameworkGVR).Namespace(options.namespace).Get(frameworkVersionNameOfInstance, metav1.GetOptions{})
 	if err != nil {
 		return err
 	}
@@ -122,7 +142,7 @@ func planStatus() error {
 		Resource: "planexecutions",
 	}
 
-	activePlanObj, err := dynamicClient.Resource(planExecutionsGVR).Namespace(namespace).Get(instance.Status.ActivePlan.Name, metav1.GetOptions{})
+	activePlanObj, err := dynamicClient.Resource(planExecutionsGVR).Namespace(options.namespace).Get(instance.Status.ActivePlan.Name, metav1.GetOptions{})
 	if err != nil {
 		return err
 	}
@@ -172,7 +192,7 @@ func planStatus() error {
 		}
 	}
 
-	fmt.Printf("Plan(s) for \"%s\" in namespace \"%s\":\n", instance.Name, namespace)
+	fmt.Printf("Plan(s) for \"%s\" in namespace \"%s\":\n", instance.Name, options.namespace)
 	fmt.Println(tree.String())
 
 	return nil

pkg/kudoctl/util/kudo/kudo.go

@@ -17,8 +17,8 @@ type Client struct {
 	clientset versioned.Interface
 }
 
-// NewKudoClient creates new KUDO Client
-func NewKudoClient(namespace, kubeConfigPath string) (*Client, error) {
+// NewClient creates new KUDO Client
+func NewClient(namespace, kubeConfigPath string) (*Client, error) {
 
 	// use the current context in kubeconfig
 	config, err := clientcmd.BuildConfigFromFlags("", kubeConfigPath)

pkg/kudoctl/util/kudo/kudo_test.go

@@ -24,7 +24,7 @@ func TestNewK2oClient(t *testing.T) {
 
 	for _, tt := range tests {
 		// Just interested in errors
-		_, err := NewKudoClient("default", "")
+		_, err := NewClient("default", "")
 		if err.Error() != tt.err {
 			t.Errorf("non existing test:\nexpected: %v\n     got: %v", tt.err, err.Error())
 		}