Makes version more resilient to failure

Signed-off-by: Phil Calcado <pcalcado@gmail.com>

cli/cmd/root.go

@@ -1,6 +1,7 @@
 package cmd
 
 import (
+	"fmt"
 	"os"
 
 	"github.com/runconduit/conduit/cli/k8s"
@@ -14,6 +15,8 @@ var controlPlaneNamespace string
 var apiAddr string // An empty value means "use the Kubernetes configuration"
 var kubeconfigPath string
 
+const ExitCodeForRuntimeErrors = 2 // Cobra uses `1` for usage errors
+
 var RootCmd = &cobra.Command{
 	Use:   "conduit",
 	Short: "conduit manages the Conduit service mesh",
@@ -51,19 +54,18 @@ func newApiClient(kubeApi k8s.KubernetesApi) (pb.ApiClient, error) {
 	return public.NewClient(apiConfig, transport)
 }
 
-// Exit with non-zero exit status without printing the command line usage and
-// without printing the error message.
+//runEButSuppressUsageMessageOnError exits with non-zero exit status without printing the command line usage.
 //
-// When a `RunE` command returns an error, Cobra will print the usage message
-// so the `RunE` function needs to handle any non-usage errors itself without
-// returning an error. `exitSilentlyOnError` can be used as the `Run` (not
-// `RunE`) function to help with this.
+// Cobra requires any `RunE` command to handle all non-usage errors itself. Whenever a `RunE` command returns an error, Cobra assumes
+// that it is a usage error and automatically prints the usage message. `runEButSuppressUsageMessageOnError` has the same signature as `RunE`,
+// but will not display any usage information. It will print the error message to STDERR.
 //
 // TODO: This is used by the `version` command now; it should be used by other commands too.
-func exitSilentlyOnError(f func(cmd *cobra.Command, args []string) error) func(cmd *cobra.Command, args []string) {
+func runEButSuppressUsageMessageOnError(f func(cmd *cobra.Command, args []string) error) func(cmd *cobra.Command, args []string) {
 	return func(cmd *cobra.Command, args []string) {
 		if err := f(cmd, args); err != nil {
-			os.Exit(2) // Reserve 1 for usage errors.
+			fmt.Fprintln(os.Stderr, err)
+			os.Exit(ExitCodeForRuntimeErrors)
 		}
 	}
 }

cli/cmd/version.go

@@ -3,6 +3,7 @@ package cmd
 import (
 	"context"
 	"fmt"
+	"os"
 
 	"github.com/runconduit/conduit/cli/k8s"
 	"github.com/runconduit/conduit/cli/shell"
@@ -19,16 +20,18 @@ var versionCmd = &cobra.Command{
 	Short: "Print the client and server version information",
 	Long:  "Print the client and server version information.",
 	Args:  cobra.NoArgs,
-	Run: exitSilentlyOnError(func(cmd *cobra.Command, args []string) error {
+	Run: runEButSuppressUsageMessageOnError(func(cmd *cobra.Command, args []string) error {
 
-		kubeApi, err := k8s.MakeK8sAPi(shell.MakeUnixShell(), kubeconfigPath, apiAddr)
-		if err != nil {
-			return err
-		}
+		var client pb.ApiClient
 
-		client, err := newApiClient(kubeApi)
+		kubeApi, err := k8s.MakeK8sAPi(shell.MakeUnixShell(), kubeconfigPath, apiAddr)
 		if err != nil {
-			return err
+			fmt.Fprintln(os.Stderr, err)
+		} else {
+			client, err = newApiClient(kubeApi)
+			if err != nil {
+				fmt.Fprintln(os.Stderr, err)
+			}
 		}
 
 		versions := getVersions(client)
@@ -51,18 +54,17 @@ type versions struct {
 }
 
 func getVersions(client pb.ApiClient) versions {
-	resp, err := client.Version(context.Background(), &pb.Empty{})
-	if err != nil {
-		return versions{
-			Client: controller.Version,
-			Server: DefaultVersionString,
-		}
-	}
-
 	versions := versions{
-		Server: resp.GetReleaseVersion(),
 		Client: controller.Version,
+		Server: DefaultVersionString,
+	}
+	if client != nil {
+		resp, err := client.Version(context.Background(), &pb.Empty{})
+		if err != nil {
+			fmt.Fprintln(os.Stderr, err)
+		} else {
+			versions.Server = resp.GetReleaseVersion()
+		}
 	}
-
 	return versions
 }

cli/cmd/version_test.go

@@ -26,7 +26,7 @@ func TestGetVersion(t *testing.T) {
 		}
 	})
 
-	t.Run("Returns undfined when cannot gt server version", func(t *testing.T) {
+	t.Run("Returns undefined for server version when cannot talk to server", func(t *testing.T) {
 		expectedServerVersion := "unavailable"
 		expectedClientVersion := controller.Version
 		mockClient := &mockApiClient{}
@@ -39,4 +39,19 @@ func TestGetVersion(t *testing.T) {
 				expectedClientVersion, versions.Client, expectedServerVersion, versions.Server)
 		}
 	})
+
+	t.Run("Returns at least the client version when API client cant be created", func(t *testing.T) {
+		expectedServerVersion := "unavailable"
+		expectedClientVersion := controller.Version
+		mockClient := &mockApiClient{}
+		mockClient.errorToReturn = errors.New("expected")
+
+		versions := getVersions(nil)
+
+		if versions.Client != expectedClientVersion || versions.Server != expectedServerVersion {
+			t.Fatalf("Expected client version to be [%s], was [%s]; expecting server version to be [%s], was [%s]",
+				expectedClientVersion, versions.Client, expectedServerVersion, versions.Server)
+		}
+	})
+
 }

cli/k8s/api.go

@@ -41,8 +41,9 @@ func MakeK8sAPi(shell shell.Shell, k8sConfigFilesystemPathOverride string, apiHo
 		apiHostAndPortOverride = config.Host
 	}
 
-	return &kubernetesApi{
+	api := &kubernetesApi{
 		apiSchemeHostAndPort: apiHostAndPortOverride,
 		config:               config,
-	}, nil
+	}
+	return api, nil
 }