From e3b085939564a651f5d3b4bc5b4288a8eb0c9cd8 Mon Sep 17 00:00:00 2001 From: Phil Calcado Date: Wed, 27 Dec 2017 11:33:09 -0500 Subject: [PATCH] Makes version more resilient to failure Signed-off-by: Phil Calcado --- cli/cmd/root.go | 18 ++++++++++-------- cli/cmd/version.go | 36 +++++++++++++++++++----------------- cli/cmd/version_test.go | 17 ++++++++++++++++- cli/k8s/api.go | 5 +++-- 4 files changed, 48 insertions(+), 28 deletions(-) diff --git a/cli/cmd/root.go b/cli/cmd/root.go index bce7b76b1c486..ba78e301d2ed9 100644 --- a/cli/cmd/root.go +++ b/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) } } } diff --git a/cli/cmd/version.go b/cli/cmd/version.go index 7b41357a550a5..f09be4092bd54 100644 --- a/cli/cmd/version.go +++ b/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 } diff --git a/cli/cmd/version_test.go b/cli/cmd/version_test.go index 9adcb906955db..d141afa107beb 100644 --- a/cli/cmd/version_test.go +++ b/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) + } + }) + } diff --git a/cli/k8s/api.go b/cli/k8s/api.go index 98af783467a65..ff77abc49cc91 100644 --- a/cli/k8s/api.go +++ b/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 }