From 9977e5fb0cce02401c0525f9db82ccc504c576fb Mon Sep 17 00:00:00 2001 From: Phil Calcado Date: Wed, 20 Dec 2017 17:26:19 +1100 Subject: [PATCH 1/7] Add func to rsolve kubectl-like names to canonical names Signed-off-by: Phil Calcado --- cli/k8s/kubectl.go | 30 +++++++++++++++++++++++------- cli/k8s/kubectl_test.go | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 7 deletions(-) diff --git a/cli/k8s/kubectl.go b/cli/k8s/kubectl.go index 1b64cabe82032..9067a62253611 100644 --- a/cli/k8s/kubectl.go +++ b/cli/k8s/kubectl.go @@ -1,13 +1,14 @@ package k8s import ( - "fmt" "errors" - "strings" + "fmt" + "net/url" "strconv" + "strings" "time" + "github.com/runconduit/conduit/cli/shell" - "net/url" ) type Kubectl interface { @@ -23,6 +24,8 @@ type kubectl struct { } const ( + KubernetesDeployments = "deployments" + KubernetesPods = "pods" kubectlDefaultProxyPort = 8001 kubectlDefaultTimeout = 10 * time.Second portWhenProxyNotRunning = -1 @@ -30,7 +33,7 @@ const ( magicCharacterThatIndicatesProxyIsRunning = '\n' ) -var minimunKubectlVersionExpected = [3]int{1, 8, 0} +var minimumKubectlVersionExpected = [3]int{1, 8, 0} func (kctl *kubectl) ProxyPort() int { return kctl.proxyPort @@ -96,7 +99,7 @@ func (kctl *kubectl) UrlFor(namespace string, extraPathStartingWithSlash string) return nil, errors.New("proxy needs to be started before generating URLs") } - schemeHostAndPort := fmt.Sprintf("%s://%s:%d",kctl.ProxyScheme(),kctl.ProxyHost(), kctl.ProxyPort()) + schemeHostAndPort := fmt.Sprintf("%s://%s:%d", kctl.ProxyScheme(), kctl.ProxyHost(), kctl.ProxyPort()) return generateKubernetesApiBaseUrlFor(schemeHostAndPort, namespace, extraPathStartingWithSlash) } @@ -117,6 +120,19 @@ func isCompatibleVersion(minimalRequirementVersion [3]int, actualVersion [3]int) return false } +//CanonicalKubernetesNameFromFriendlyName returns a canonical name from common shorthands used in command line tools. +// This works based on https://github.com/kubernetes/kubernetes/blob/63ffb1995b292be0a1e9ebde6216b83fc79dd988/pkg/kubectl/kubectl.go#L39 +func CanonicalKubernetesNameFromFriendlyName(friendlyName string) (string, error) { + switch friendlyName { + case "deploy", "deployment", "deployments": + return KubernetesDeployments, nil + case "po", "pod", "pods": + return KubernetesPods, nil + } + + return "", fmt.Errorf("cannot find Kubernetes canonical name from friendly name [%s]", friendlyName) +} + func MakeKubectl(shell shell.Shell) (Kubectl, error) { kubectl := &kubectl{ @@ -130,11 +146,11 @@ func MakeKubectl(shell shell.Shell) (Kubectl, error) { return nil, err } - if !isCompatibleVersion(minimunKubectlVersionExpected, actualVersion) { + if !isCompatibleVersion(minimumKubectlVersionExpected, actualVersion) { return nil, fmt.Errorf( "Kubectl is on version [%d.%d.%d], but version [%d.%d.%d] or more recent is required", actualVersion[0], actualVersion[1], actualVersion[2], - minimunKubectlVersionExpected[0], minimunKubectlVersionExpected[1], minimunKubectlVersionExpected[2]) + minimumKubectlVersionExpected[0], minimumKubectlVersionExpected[1], minimumKubectlVersionExpected[2]) } return kubectl, nil diff --git a/cli/k8s/kubectl_test.go b/cli/k8s/kubectl_test.go index 840ad24b7743c..68a286be9904d 100644 --- a/cli/k8s/kubectl_test.go +++ b/cli/k8s/kubectl_test.go @@ -281,3 +281,38 @@ func TestMakeKubectl(t *testing.T) { } }) } + +func TestCanonicalKubernetesNameFromFriendlyName(t *testing.T) { + t.Run("Returns canonical name for all known variants", func(t *testing.T) { + expectations := map[string]string{ + "po": KubernetesPods, + "pod": KubernetesPods, + "deployment": KubernetesDeployments, + "deployments": KubernetesDeployments, + } + + for input, expectedName := range expectations { + actualName, err := CanonicalKubernetesNameFromFriendlyName(input) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + if actualName != expectedName { + t.Fatalf("Expected friendly name [%s] to resolve to [%s], but got [%s]", input, expectedName, actualName) + } + } + }) + + t.Run("Returns error if inout isn't a supported name", func(t *testing.T) { + unsupportedNames := []string{ + "pdo", "dop", "paths", "path", "", "mesh", + } + + for _, n := range unsupportedNames { + out, err := CanonicalKubernetesNameFromFriendlyName(n) + if err == nil { + t.Fatalf("Expecting error when resolving [%s], but it did resolkve to [%s]", n, out) + } + } + }) +} From 837ac6ad3d03a810728c5c9a853a51170e739539 Mon Sep 17 00:00:00 2001 From: Phil Calcado Date: Wed, 20 Dec 2017 17:35:56 +1100 Subject: [PATCH 2/7] Refactor API instantiation Signed-off-by: Phil Calcado --- cli/cmd/get.go | 10 +++++++++- cli/cmd/root.go | 8 +------- cli/cmd/stat.go | 10 +++++++++- cli/cmd/tap.go | 10 +++++++++- cli/cmd/version.go | 10 +++++++++- 5 files changed, 37 insertions(+), 11 deletions(-) diff --git a/cli/cmd/get.go b/cli/cmd/get.go index d567f263605ad..35dc7af473a23 100644 --- a/cli/cmd/get.go +++ b/cli/cmd/get.go @@ -5,6 +5,9 @@ import ( "errors" "fmt" + "github.com/runconduit/conduit/cli/k8s" + "github.com/runconduit/conduit/cli/shell" + pb "github.com/runconduit/conduit/controller/gen/public" "github.com/spf13/cobra" ) @@ -22,7 +25,12 @@ Valid resource types include: resourceType := args[0] switch resourceType { case "pod", "pods", "po": - client, err := newApiClient() + kubeApi, err := k8s.MakeK8sAPi(shell.MakeUnixShell(), kubeconfigPath, apiAddr) + if err != nil { + return err + } + + client, err := newApiClient(kubeApi) if err != nil { return err } diff --git a/cli/cmd/root.go b/cli/cmd/root.go index 4b6492b7dbc22..bce7b76b1c486 100644 --- a/cli/cmd/root.go +++ b/cli/cmd/root.go @@ -4,7 +4,6 @@ import ( "os" "github.com/runconduit/conduit/cli/k8s" - "github.com/runconduit/conduit/cli/shell" "github.com/runconduit/conduit/controller/api/public" pb "github.com/runconduit/conduit/controller/gen/public" "github.com/spf13/cobra" @@ -34,12 +33,7 @@ func addControlPlaneNetworkingArgs(cmd *cobra.Command) { cmd.PersistentFlags().StringVar(&apiAddr, "api-addr", "", "Override kubeconfig and communicate directly with the control plane at host:port (mostly for testing)") } -func newApiClient() (pb.ApiClient, error) { - kubeApi, err := k8s.MakeK8sAPi(shell.MakeUnixShell(), kubeconfigPath, apiAddr) - if err != nil { - return nil, err - } - +func newApiClient(kubeApi k8s.KubernetesApi) (pb.ApiClient, error) { url, err := kubeApi.UrlFor(controlPlaneNamespace, "/services/http:api:http/proxy/") if err != nil { return nil, err diff --git a/cli/cmd/stat.go b/cli/cmd/stat.go index 7f37d98f0e364..b26ca5ca0cd4c 100644 --- a/cli/cmd/stat.go +++ b/cli/cmd/stat.go @@ -9,6 +9,9 @@ import ( "strings" "text/tabwriter" + "github.com/runconduit/conduit/cli/k8s" + "github.com/runconduit/conduit/cli/shell" + "github.com/runconduit/conduit/controller/api/util" pb "github.com/runconduit/conduit/controller/gen/public" "github.com/spf13/cobra" @@ -67,7 +70,12 @@ The optional [TARGET] option can be either a name for a deployment or pod resour } func makeStatsRequest(aggType pb.AggregationType) error { - client, err := newApiClient() + kubeApi, err := k8s.MakeK8sAPi(shell.MakeUnixShell(), kubeconfigPath, apiAddr) + if err != nil { + return err + } + + client, err := newApiClient(kubeApi) if err != nil { return fmt.Errorf("error creating api client while making stats request: %v", err) } diff --git a/cli/cmd/tap.go b/cli/cmd/tap.go index f0b4acfafe50a..2cac9f01c976f 100644 --- a/cli/cmd/tap.go +++ b/cli/cmd/tap.go @@ -8,6 +8,9 @@ import ( "os" "strings" + "github.com/runconduit/conduit/cli/k8s" + "github.com/runconduit/conduit/cli/shell" + common "github.com/runconduit/conduit/controller/gen/common" pb "github.com/runconduit/conduit/controller/gen/public" "github.com/runconduit/conduit/controller/util" @@ -68,7 +71,12 @@ Valid targets include: return errors.New("invalid target type") } - client, err := newApiClient() + kubeApi, err := k8s.MakeK8sAPi(shell.MakeUnixShell(), kubeconfigPath, apiAddr) + if err != nil { + return err + } + + client, err := newApiClient(kubeApi) if err != nil { return err } diff --git a/cli/cmd/version.go b/cli/cmd/version.go index fa8f65eb3a371..81810f2f302d8 100644 --- a/cli/cmd/version.go +++ b/cli/cmd/version.go @@ -4,6 +4,9 @@ import ( "context" "fmt" + "github.com/runconduit/conduit/cli/k8s" + "github.com/runconduit/conduit/cli/shell" + "github.com/runconduit/conduit/controller" pb "github.com/runconduit/conduit/controller/gen/public" "github.com/spf13/cobra" @@ -33,7 +36,12 @@ func init() { } func getVersion() (string, error) { - client, err := newApiClient() + kubeApi, err := k8s.MakeK8sAPi(shell.MakeUnixShell(), kubeconfigPath, apiAddr) + if err != nil { + return "", err + } + + client, err := newApiClient(kubeApi) if err != nil { return "", err } From ed97b0722949f04544b67938db5877048a55ec89 Mon Sep 17 00:00:00 2001 From: Phil Calcado Date: Wed, 20 Dec 2017 18:26:11 +1100 Subject: [PATCH 3/7] Make version command testable Signed-off-by: Phil Calcado --- cli/cmd/version.go | 45 ++++++++++++++++++---------- cli/cmd/version_test.go | 65 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 15 deletions(-) create mode 100644 cli/cmd/version_test.go diff --git a/cli/cmd/version.go b/cli/cmd/version.go index 81810f2f302d8..7b41357a550a5 100644 --- a/cli/cmd/version.go +++ b/cli/cmd/version.go @@ -12,19 +12,29 @@ import ( "github.com/spf13/cobra" ) +const DefaultVersionString = "unavailable" + var versionCmd = &cobra.Command{ Use: "version", 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 { - fmt.Println("Client version: " + controller.Version) - serverVersion, err := getVersion() + kubeApi, err := k8s.MakeK8sAPi(shell.MakeUnixShell(), kubeconfigPath, apiAddr) + if err != nil { + return err + } + + client, err := newApiClient(kubeApi) if err != nil { - serverVersion = "unavailable" + return err } - fmt.Println("Server version: " + serverVersion) + + versions := getVersions(client) + + fmt.Printf("Client version: %s\n", versions.Client) + fmt.Printf("Server version: %s\n", versions.Server) return err }), @@ -35,19 +45,24 @@ func init() { addControlPlaneNetworkingArgs(versionCmd) } -func getVersion() (string, error) { - kubeApi, err := k8s.MakeK8sAPi(shell.MakeUnixShell(), kubeconfigPath, apiAddr) - if err != nil { - return "", err - } +type versions struct { + Server string + Client string +} - client, err := newApiClient(kubeApi) - if err != nil { - return "", err - } +func getVersions(client pb.ApiClient) versions { resp, err := client.Version(context.Background(), &pb.Empty{}) if err != nil { - return "", err + return versions{ + Client: controller.Version, + Server: DefaultVersionString, + } } - return resp.GetReleaseVersion(), nil + + versions := versions{ + Server: resp.GetReleaseVersion(), + Client: controller.Version, + } + + return versions } diff --git a/cli/cmd/version_test.go b/cli/cmd/version_test.go new file mode 100644 index 0000000000000..e00865b4f5581 --- /dev/null +++ b/cli/cmd/version_test.go @@ -0,0 +1,65 @@ +package cmd + +import ( + "context" + "errors" + "testing" + + "github.com/runconduit/conduit/controller" + + pb "github.com/runconduit/conduit/controller/gen/public" + "google.golang.org/grpc" +) + +type mockClient struct { + errorToReturn error + versionInfoToReturn *pb.VersionInfo +} + +func (c *mockClient) Stat(ctx context.Context, in *pb.MetricRequest, opts ...grpc.CallOption) (*pb.MetricResponse, error) { + return nil, c.errorToReturn +} + +func (c *mockClient) Version(ctx context.Context, in *pb.Empty, opts ...grpc.CallOption) (*pb.VersionInfo, error) { + return c.versionInfoToReturn, c.errorToReturn +} + +func (c *mockClient) ListPods(ctx context.Context, in *pb.Empty, opts ...grpc.CallOption) (*pb.ListPodsResponse, error) { + return nil, c.errorToReturn +} + +func (c *mockClient) Tap(ctx context.Context, in *pb.TapRequest, opts ...grpc.CallOption) (pb.Api_TapClient, error) { + return nil, c.errorToReturn +} + +func TestGetVersion(t *testing.T) { + t.Run("Returns existing versions from server and client", func(t *testing.T) { + expectedServerVersion := "1.2.3" + expectedClientVersion := controller.Version + mockClient := &mockClient{} + mockClient.versionInfoToReturn = &pb.VersionInfo{ + ReleaseVersion: expectedServerVersion, + } + + versions := getVersions(mockClient) + + 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]", + versions.Client, expectedClientVersion, versions.Server, expectedServerVersion) + } + }) + + t.Run("Returns undfined when cannot gt server version", func(t *testing.T) { + expectedServerVersion := "unavailable" + expectedClientVersion := controller.Version + mockClient := &mockClient{} + mockClient.errorToReturn = errors.New("expected") + + versions := getVersions(mockClient) + + 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) + } + }) +} From c38f78f3aea1e20327916c5ad35c98c9c04987f0 Mon Sep 17 00:00:00 2001 From: Phil Calcado Date: Wed, 20 Dec 2017 19:13:33 +1100 Subject: [PATCH 4/7] Make get command testable Signed-off-by: Phil Calcado --- cli/cmd/get.go | 72 ++++++++++++++++++++++++----------------- cli/cmd/get_test.go | 69 +++++++++++++++++++++++++++++++++++++++ cli/cmd/test_helper.go | 30 +++++++++++++++++ cli/cmd/version_test.go | 27 ++-------------- 4 files changed, 144 insertions(+), 54 deletions(-) create mode 100644 cli/cmd/get_test.go create mode 100644 cli/cmd/test_helper.go diff --git a/cli/cmd/get.go b/cli/cmd/get.go index 35dc7af473a23..046ef426cb5a5 100644 --- a/cli/cmd/get.go +++ b/cli/cmd/get.go @@ -20,37 +20,37 @@ var getCmd = &cobra.Command{ Valid resource types include: * pods (aka pod, po)`, RunE: func(cmd *cobra.Command, args []string) error { - switch len(args) { - case 1: - resourceType := args[0] - switch resourceType { - case "pod", "pods", "po": - kubeApi, err := k8s.MakeK8sAPi(shell.MakeUnixShell(), kubeconfigPath, apiAddr) - if err != nil { - return err - } - - client, err := newApiClient(kubeApi) - if err != nil { - return err - } - resp, err := client.ListPods(context.Background(), &pb.Empty{}) - if err != nil { - return err - } - - for _, pod := range resp.GetPods() { - fmt.Println(pod.Name) - } - - default: - return errors.New("invalid resource type") - } - - return nil - default: + if len(args) != 1 { return errors.New("please specify a resource type") } + + friendlyName := args[0] + resourceType, err := k8s.CanonicalKubernetesNameFromFriendlyName(friendlyName) + + if err != nil || resourceType != k8s.KubernetesPods { + return fmt.Errorf("invalid resource type [%s]", friendlyName) + } + + kubeApi, err := k8s.MakeK8sAPi(shell.MakeUnixShell(), kubeconfigPath, apiAddr) + if err != nil { + return err + } + + client, err := newApiClient(kubeApi) + if err != nil { + return err + } + + podNames, err := getPods(client) + if err != nil { + return err + } + + for _, podName := range podNames { + fmt.Println(podName) + } + + return nil }, } @@ -58,3 +58,17 @@ func init() { RootCmd.AddCommand(getCmd) addControlPlaneNetworkingArgs(getCmd) } + +func getPods(apiClient pb.ApiClient) ([]string, error) { + resp, err := apiClient.ListPods(context.Background(), &pb.Empty{}) + if err != nil { + return nil, err + } + + names := make([]string, 0) + for _, pod := range resp.GetPods() { + names = append(names, pod.Name) + } + + return names, nil +} diff --git a/cli/cmd/get_test.go b/cli/cmd/get_test.go new file mode 100644 index 0000000000000..e41ec58ac0335 --- /dev/null +++ b/cli/cmd/get_test.go @@ -0,0 +1,69 @@ +package cmd + +import ( + "errors" + "testing" + + pb "github.com/runconduit/conduit/controller/gen/public" +) + +func TestGetPods(t *testing.T) { + t.Run("Returns names of existing pods if everything went ok", func(t *testing.T) { + mockClient := &mockApiClient{} + + pods := []*pb.Pod{ + {Name: "pod-a"}, + {Name: "pod-b"}, + {Name: "pod-c"}, + } + + expectedPodNames := []string{ + "pod-a", + "pod-b", + "pod-c", + } + response := &pb.ListPodsResponse{ + Pods: pods, + } + + mockClient.listPodsResponseToReturn = response + actualPodNames, err := getPods(mockClient) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + for i, actualName := range actualPodNames { + expectedName := expectedPodNames[i] + if expectedName != actualName { + t.Fatalf("Expected %dth element on %v to be [%s], but was [%s]", i, actualPodNames, expectedName, actualName) + } + } + }) + + t.Run("Returns empty list if no [ods found", func(t *testing.T) { + mockClient := &mockApiClient{} + + mockClient.listPodsResponseToReturn = &pb.ListPodsResponse{ + Pods: []*pb.Pod{}, + } + + actualPodNames, err := getPods(mockClient) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + if len(actualPodNames) != 0 { + t.Fatalf("Expecting no pod names, got %v", actualPodNames) + } + }) + + t.Run("Returns error if cant find pods in API", func(t *testing.T) { + mockClient := &mockApiClient{} + mockClient.errorToReturn = errors.New("expected") + + _, err := getPods(mockClient) + if err == nil { + t.Fatalf("Expecting error, got noting") + } + }) +} diff --git a/cli/cmd/test_helper.go b/cli/cmd/test_helper.go new file mode 100644 index 0000000000000..63c30f485ad06 --- /dev/null +++ b/cli/cmd/test_helper.go @@ -0,0 +1,30 @@ +package cmd + +import ( + "context" + + pb "github.com/runconduit/conduit/controller/gen/public" + "google.golang.org/grpc" +) + +type mockApiClient struct { + errorToReturn error + versionInfoToReturn *pb.VersionInfo + listPodsResponseToReturn *pb.ListPodsResponse +} + +func (c *mockApiClient) Stat(ctx context.Context, in *pb.MetricRequest, opts ...grpc.CallOption) (*pb.MetricResponse, error) { + return nil, c.errorToReturn +} + +func (c *mockApiClient) Version(ctx context.Context, in *pb.Empty, opts ...grpc.CallOption) (*pb.VersionInfo, error) { + return c.versionInfoToReturn, c.errorToReturn +} + +func (c *mockApiClient) ListPods(ctx context.Context, in *pb.Empty, opts ...grpc.CallOption) (*pb.ListPodsResponse, error) { + return c.listPodsResponseToReturn, c.errorToReturn +} + +func (c *mockApiClient) Tap(ctx context.Context, in *pb.TapRequest, opts ...grpc.CallOption) (pb.Api_TapClient, error) { + return nil, c.errorToReturn +} diff --git a/cli/cmd/version_test.go b/cli/cmd/version_test.go index e00865b4f5581..9adcb906955db 100644 --- a/cli/cmd/version_test.go +++ b/cli/cmd/version_test.go @@ -1,42 +1,19 @@ package cmd import ( - "context" "errors" "testing" "github.com/runconduit/conduit/controller" pb "github.com/runconduit/conduit/controller/gen/public" - "google.golang.org/grpc" ) -type mockClient struct { - errorToReturn error - versionInfoToReturn *pb.VersionInfo -} - -func (c *mockClient) Stat(ctx context.Context, in *pb.MetricRequest, opts ...grpc.CallOption) (*pb.MetricResponse, error) { - return nil, c.errorToReturn -} - -func (c *mockClient) Version(ctx context.Context, in *pb.Empty, opts ...grpc.CallOption) (*pb.VersionInfo, error) { - return c.versionInfoToReturn, c.errorToReturn -} - -func (c *mockClient) ListPods(ctx context.Context, in *pb.Empty, opts ...grpc.CallOption) (*pb.ListPodsResponse, error) { - return nil, c.errorToReturn -} - -func (c *mockClient) Tap(ctx context.Context, in *pb.TapRequest, opts ...grpc.CallOption) (pb.Api_TapClient, error) { - return nil, c.errorToReturn -} - func TestGetVersion(t *testing.T) { t.Run("Returns existing versions from server and client", func(t *testing.T) { expectedServerVersion := "1.2.3" expectedClientVersion := controller.Version - mockClient := &mockClient{} + mockClient := &mockApiClient{} mockClient.versionInfoToReturn = &pb.VersionInfo{ ReleaseVersion: expectedServerVersion, } @@ -52,7 +29,7 @@ func TestGetVersion(t *testing.T) { t.Run("Returns undfined when cannot gt server version", func(t *testing.T) { expectedServerVersion := "unavailable" expectedClientVersion := controller.Version - mockClient := &mockClient{} + mockClient := &mockApiClient{} mockClient.errorToReturn = errors.New("expected") versions := getVersions(mockClient) From 0dca1a42897fc726c74d18587bb25325b947e1c6 Mon Sep 17 00:00:00 2001 From: Phil Calcado Date: Thu, 21 Dec 2017 12:16:29 +1100 Subject: [PATCH 5/7] Add tests for api utils Signed-off-by: Phil Calcado --- controller/api/util/api_utils.go | 17 ++- controller/api/util/api_utils_test.go | 161 ++++++++++++++++++++++++++ 2 files changed, 172 insertions(+), 6 deletions(-) create mode 100644 controller/api/util/api_utils_test.go diff --git a/controller/api/util/api_utils.go b/controller/api/util/api_utils.go index 31bf444c28709..7e8bc1afca805 100644 --- a/controller/api/util/api_utils.go +++ b/controller/api/util/api_utils.go @@ -2,6 +2,7 @@ package util import ( "errors" + "fmt" pb "github.com/runconduit/conduit/controller/gen/public" ) @@ -10,8 +11,8 @@ import ( Shared utilities for interacting with the controller public api */ -func GetWindow(timeWindow string) (pb.TimeWindow, error) { - switch timeWindow { +func GetWindow(timeWindowFriendlyName string) (pb.TimeWindow, error) { + switch timeWindowFriendlyName { case "10s": return pb.TimeWindow_TEN_SEC, nil case "1m": @@ -20,8 +21,9 @@ func GetWindow(timeWindow string) (pb.TimeWindow, error) { return pb.TimeWindow_TEN_MIN, nil case "1h": return pb.TimeWindow_ONE_HOUR, nil + default: + return pb.TimeWindow_ONE_MIN, errors.New("invalid time-window " + timeWindowFriendlyName) } - return pb.TimeWindow_ONE_MIN, errors.New("invalid time-window " + timeWindow) } func GetWindowString(timeWindow pb.TimeWindow) (string, error) { @@ -34,8 +36,9 @@ func GetWindowString(timeWindow pb.TimeWindow) (string, error) { return "10m", nil case pb.TimeWindow_ONE_HOUR: return "1h", nil + default: + return "", fmt.Errorf("invalid time-window %v", timeWindow) } - return "", errors.New("invalid time-window " + timeWindow.String()) } func GetMetricName(metricName string) (pb.MetricName, error) { @@ -46,8 +49,9 @@ func GetMetricName(metricName string) (pb.MetricName, error) { return pb.MetricName_LATENCY, nil case "successRate": return pb.MetricName_SUCCESS_RATE, nil + default: + return pb.MetricName_REQUEST_RATE, errors.New("invalid metric name " + metricName) } - return pb.MetricName_REQUEST_RATE, errors.New("invalid metric name " + metricName) } func GetAggregationType(aggregationType string) (pb.AggregationType, error) { @@ -64,6 +68,7 @@ func GetAggregationType(aggregationType string) (pb.AggregationType, error) { return pb.AggregationType_MESH, nil case "path": return pb.AggregationType_PATH, nil + default: + return pb.AggregationType_TARGET_POD, errors.New("invalid aggregation type " + aggregationType) } - return pb.AggregationType_TARGET_POD, errors.New("invalid aggregation type " + aggregationType) } diff --git a/controller/api/util/api_utils_test.go b/controller/api/util/api_utils_test.go new file mode 100644 index 0000000000000..602f11b890aa9 --- /dev/null +++ b/controller/api/util/api_utils_test.go @@ -0,0 +1,161 @@ +package util + +import ( + "testing" + + pb "github.com/runconduit/conduit/controller/gen/public" +) + +func TestGetWindow(t *testing.T) { + t.Run("Returns valid windows", func(t *testing.T) { + expectations := map[string]pb.TimeWindow{ + "10s": pb.TimeWindow_TEN_SEC, + "1m": pb.TimeWindow_ONE_MIN, + "10m": pb.TimeWindow_TEN_MIN, + "1h": pb.TimeWindow_ONE_HOUR, + } + + for windowFriendlyName, expectedTimeWindow := range expectations { + actualTimeWindow, err := GetWindow(windowFriendlyName) + if err != nil { + t.Fatalf("Unexpected error when resolving time window friendly name [%s]: %v", + windowFriendlyName, err) + } + + if actualTimeWindow != expectedTimeWindow { + t.Fatalf("Expected resolving friendly name [%s] to return timw window [%v], but got [%v]", + windowFriendlyName, expectedTimeWindow, actualTimeWindow) + } + } + }) + + t.Run("Returns error and default value if unknown friendly name for TimeWindow", func(t *testing.T) { + invalidNames := []string{ + "10seconds", "10sec", "9s", + "10minutes", "10min", "9m", + "1minute", "1min", "0s", "2s", + "1hour", "0h", "2h", + "10", ""} + defaultTimeWindow := pb.TimeWindow_ONE_MIN + + for _, invalidName := range invalidNames { + window, err := GetWindow(invalidName) + if err == nil { + t.Fatalf("Expected invalid friendly name [%s] to generate error, but got no error and result [%v]", + invalidName, window) + } + + if window != defaultTimeWindow { + t.Fatalf("Expected invalid friendly name resolution to return default window [%v], but got [%v]", + defaultTimeWindow, window) + } + } + }) +} + +func TestGetWindowString(t *testing.T) { + t.Run("Returns names for valid windows", func(t *testing.T) { + expectations := map[pb.TimeWindow]string{ + pb.TimeWindow_TEN_SEC: "10s", + pb.TimeWindow_ONE_MIN: "1m", + pb.TimeWindow_TEN_MIN: "10m", + pb.TimeWindow_ONE_HOUR: "1h", + } + + for window, expectedName := range expectations { + actualName, err := GetWindowString(window) + if err != nil { + t.Fatalf("Unexpected error when resolving name for window [%v]: %v", window, err) + } + + if actualName != expectedName { + t.Fatalf("Expected window [%v] to resolve to name [%s], but got [%s]", window, expectedName, actualName) + } + } + }) +} + +func TestGetMetricName(t *testing.T) { + t.Run("Returns valid metrics from name", func(t *testing.T) { + expectations := map[string]pb.MetricName{ + "requests": pb.MetricName_REQUEST_RATE, + "latency": pb.MetricName_LATENCY, + "successRate": pb.MetricName_SUCCESS_RATE, + } + + for metricFriendlyName, expectedMetricName := range expectations { + actualMetricName, err := GetMetricName(metricFriendlyName) + if err != nil { + t.Fatalf("Unexpected error when resolving metric friendly name [%s]: %v", + metricFriendlyName, err) + } + + if actualMetricName != expectedMetricName { + t.Fatalf("Expected resolving metric friendly name [%s] to return metric [%v], but got [%v]", + metricFriendlyName, expectedMetricName, actualMetricName) + } + } + }) + + t.Run("Returns error and default value if unknown friendly name for TimeWindow", func(t *testing.T) { + invalidNames := []string{"failureRate", ""} + defaultMetricName := pb.MetricName_REQUEST_RATE + + for _, invalidName := range invalidNames { + window, err := GetMetricName(invalidName) + if err == nil { + t.Fatalf("Expected invalid friendly name [%s] to generate error, but got no error and result [%v]", + invalidName, window) + } + + if window != defaultMetricName { + t.Fatalf("Expected invalid friendly name resolution to return default name [%v], but got [%v]", + defaultMetricName, window) + } + } + }) +} + +func TestGetAggregationType(t *testing.T) { + t.Run("Returns valid metrics from name", func(t *testing.T) { + expectations := map[string]pb.AggregationType{ + "target_pod": pb.AggregationType_TARGET_POD, + "target_deploy": pb.AggregationType_TARGET_DEPLOY, + "source_pod": pb.AggregationType_SOURCE_POD, + "source_deploy": pb.AggregationType_SOURCE_DEPLOY, + "mesh": pb.AggregationType_MESH, + "path": pb.AggregationType_PATH, + } + + for aggregationFriendlyName, expectedAggregation := range expectations { + actualAggregation, err := GetAggregationType(aggregationFriendlyName) + if err != nil { + t.Fatalf("Unexpected error when resolving friendly name [%s]: %v", + aggregationFriendlyName, err) + } + + if actualAggregation != expectedAggregation { + t.Fatalf("Expected resolving friendly name [%s] to return [%v], but got [%v]", + aggregationFriendlyName, expectedAggregation, actualAggregation) + } + } + }) + + t.Run("Returns error and default value if unknown friendly name for TimeWindow", func(t *testing.T) { + invalidNames := []string{"pod_target", "pod", "service", "target_service", "target_mesh", ""} + defaultAggregation := pb.AggregationType_TARGET_POD + + for _, invalidName := range invalidNames { + aggregation, err := GetAggregationType(invalidName) + if err == nil { + t.Fatalf("Expected invalid friendly name [%s] to generate error, but got no error and result [%v]", + invalidName, aggregation) + } + + if aggregation != defaultAggregation { + t.Fatalf("Expected invalid friendly name resolution to return default [%v], but got [%v]", + defaultAggregation, aggregation) + } + } + }) +} From 1b6b280285db0c5cbc0f12cab81cb27813be4b50 Mon Sep 17 00:00:00 2001 From: Phil Calcado Date: Fri, 22 Dec 2017 10:32:56 +1100 Subject: [PATCH 6/7] Make stat command testable Signed-off-by: Phil Calcado --- cli/cmd/get.go | 8 +- cli/cmd/stat.go | 117 +++++++----- cli/cmd/stat_test.go | 207 +++++++++++++++++++++- cli/cmd/test_helper.go | 3 +- cli/cmd/testdata/stat_busy_output.golden | 11 ++ cli/cmd/testdata/stat_empty_output.golden | 1 + cli/cmd/testdata/stat_one_output.golden | 2 + 7 files changed, 296 insertions(+), 53 deletions(-) create mode 100644 cli/cmd/testdata/stat_busy_output.golden create mode 100644 cli/cmd/testdata/stat_empty_output.golden create mode 100644 cli/cmd/testdata/stat_one_output.golden diff --git a/cli/cmd/get.go b/cli/cmd/get.go index 046ef426cb5a5..96ad3a27cf9b0 100644 --- a/cli/cmd/get.go +++ b/cli/cmd/get.go @@ -20,15 +20,19 @@ var getCmd = &cobra.Command{ Valid resource types include: * pods (aka pod, po)`, RunE: func(cmd *cobra.Command, args []string) error { - if len(args) != 1 { + if len(args) < 1 { return errors.New("please specify a resource type") } + if len(args) > 1 { + return errors.New("please specify only one resource type") + } + friendlyName := args[0] resourceType, err := k8s.CanonicalKubernetesNameFromFriendlyName(friendlyName) if err != nil || resourceType != k8s.KubernetesPods { - return fmt.Errorf("invalid resource type [%s]", friendlyName) + return fmt.Errorf("invalid resource type %s, only %s are allowed as resource types", friendlyName, k8s.KubernetesPods) } kubeApi, err := k8s.MakeK8sAPi(shell.MakeUnixShell(), kubeconfigPath, apiAddr) diff --git a/cli/cmd/stat.go b/cli/cmd/stat.go index b26ca5ca0cd4c..165813773ddb4 100644 --- a/cli/cmd/stat.go +++ b/cli/cmd/stat.go @@ -17,14 +17,7 @@ import ( "github.com/spf13/cobra" ) -const padding = 3 - -type row struct { - requestRate float64 - successRate float64 - latencyP50 int64 - latencyP99 int64 -} +const ConduitPaths = "paths" var target string var timeWindow string @@ -43,75 +36,100 @@ Valid resource types include: The optional [TARGET] option can be either a name for a deployment or pod resource`, RunE: func(cmd *cobra.Command, args []string) error { - var resourceType string + var friendlyName string + switch len(args) { case 1: - resourceType = args[0] + friendlyName = args[0] case 2: - resourceType = args[0] + friendlyName = args[0] target = args[1] default: return errors.New("please specify a resource type: pods, deployments or paths") } - switch resourceType { - case "pods", "pod", "po": - return makeStatsRequest(pb.AggregationType_TARGET_POD) - case "deployments", "deployment", "deploy": - return makeStatsRequest(pb.AggregationType_TARGET_DEPLOY) - case "paths", "path", "pa": - return makeStatsRequest(pb.AggregationType_PATH) - default: - return errors.New("invalid resource type") + validatedResourceType, err := k8s.CanonicalKubernetesNameFromFriendlyName(friendlyName) + if err != nil { + switch friendlyName { + case "paths", "path", "pa": + validatedResourceType = ConduitPaths + default: + return fmt.Errorf("invalid resource type %s, only %v are allowed as resource types", friendlyName, []string{k8s.KubernetesPods, k8s.KubernetesDeployments, ConduitPaths}) + } + } + kubeApi, err := k8s.MakeK8sAPi(shell.MakeUnixShell(), kubeconfigPath, apiAddr) + if err != nil { + return err + } + + client, err := newApiClient(kubeApi) + if err != nil { + return fmt.Errorf("error creating api client while making stats request: %v", err) + } + + output, err := requestStatsFromApi(client, validatedResourceType) + if err != nil { + return err } - return nil + _, err = fmt.Print(output) + + return err }, } -func makeStatsRequest(aggType pb.AggregationType) error { - kubeApi, err := k8s.MakeK8sAPi(shell.MakeUnixShell(), kubeconfigPath, apiAddr) - if err != nil { - return err - } +func init() { + RootCmd.AddCommand(statCmd) + addControlPlaneNetworkingArgs(statCmd) + statCmd.PersistentFlags().StringVarP(&timeWindow, "time-window", "t", "1m", "Stat window. One of: '10s', '1m', '10m', '1h', '6h', '24h'.") + statCmd.PersistentFlags().BoolVarP(&watch, "watch", "w", false, "After listing/getting the requested object, watch for changes.") + statCmd.PersistentFlags().BoolVar(&watchOnly, "watch-only", false, "Watch for changes to the requested object(s), without listing/getting first.") +} - client, err := newApiClient(kubeApi) - if err != nil { - return fmt.Errorf("error creating api client while making stats request: %v", err) - } +var resourceTypeToAggregationType = map[string]pb.AggregationType{ + k8s.KubernetesPods: pb.AggregationType_TARGET_POD, + k8s.KubernetesDeployments: pb.AggregationType_TARGET_DEPLOY, + ConduitPaths: pb.AggregationType_PATH, +} + +func requestStatsFromApi(client pb.ApiClient, resourceType string) (string, error) { + aggType := resourceTypeToAggregationType[resourceType] req, err := buildMetricRequest(aggType) if err != nil { - return fmt.Errorf("error creating metrics request while making stats request: %v", err) + return "", fmt.Errorf("error creating metrics request while making stats request: %v", err) } resp, err := client.Stat(context.Background(), req) if err != nil { - return fmt.Errorf("error calling stat with request: %v", err) + return "", fmt.Errorf("error calling stat with request: %v", err) } + return renderStats(resp) +} + +func renderStats(resp *pb.MetricResponse) (string, error) { var buffer bytes.Buffer w := tabwriter.NewWriter(&buffer, 0, 0, padding, ' ', tabwriter.AlignRight) - displayStats(resp, w) + writeStatsToBuffer(resp, w) w.Flush() // strip left padding on the first column out := string(buffer.Bytes()[padding:]) out = strings.Replace(out, "\n"+strings.Repeat(" ", padding), "\n", -1) - _, err = fmt.Print(out) - return err + return out, nil } -func sortStatsKeys(stats map[string]*row) []string { - var sortedKeys []string - for key, _ := range stats { - sortedKeys = append(sortedKeys, key) - } - sort.Strings(sortedKeys) - return sortedKeys +const padding = 3 + +type row struct { + requestRate float64 + successRate float64 + latencyP50 int64 + latencyP99 int64 } -func displayStats(resp *pb.MetricResponse, w *tabwriter.Writer) { +func writeStatsToBuffer(resp *pb.MetricResponse, w *tabwriter.Writer) { nameHeader := "NAME" maxNameLength := len(nameHeader) @@ -208,10 +226,11 @@ func buildMetricRequest(aggregationType pb.AggregationType) (*pb.MetricRequest, }, nil } -func init() { - RootCmd.AddCommand(statCmd) - addControlPlaneNetworkingArgs(statCmd) - statCmd.PersistentFlags().StringVarP(&timeWindow, "time-window", "t", "1m", "Stat window. One of: '10s', '1m', '10m', '1h', '6h', '24h'.") - statCmd.PersistentFlags().BoolVarP(&watch, "watch", "w", false, "After listing/getting the requested object, watch for changes.") - statCmd.PersistentFlags().BoolVar(&watchOnly, "watch-only", false, "Watch for changes to the requested object(s), without listing/getting first.") +func sortStatsKeys(stats map[string]*row) []string { + var sortedKeys []string + for key, _ := range stats { + sortedKeys = append(sortedKeys, key) + } + sort.Strings(sortedKeys) + return sortedKeys } diff --git a/cli/cmd/stat_test.go b/cli/cmd/stat_test.go index 76a3130145ac7..0c5643ec5f811 100644 --- a/cli/cmd/stat_test.go +++ b/cli/cmd/stat_test.go @@ -1,11 +1,157 @@ package cmd import ( + "errors" + "fmt" + "io/ioutil" + "math/rand" + "strings" "testing" + "github.com/runconduit/conduit/cli/k8s" + + pb "github.com/runconduit/conduit/controller/gen/public" "github.com/stretchr/testify/assert" ) +func TestRequestStatsFromApi(t *testing.T) { + t.Run("Returns string output containing the data returned by the API", func(t *testing.T) { + mockClient := &mockApiClient{} + + podName := "pod-1" + metricDatapoints := []*pb.MetricDatapoint{ + { + Value: &pb.MetricValue{ + Value: &pb.MetricValue_Counter{ + Counter: 666, + }, + }, + }, + } + series := []*pb.MetricSeries{ + { + Name: pb.MetricName_SUCCESS_RATE, + Metadata: &pb.MetricMetadata{ + TargetPod: podName, + }, + Datapoints: metricDatapoints, + }, + } + mockClient.metricResponseToReturn = &pb.MetricResponse{ + Metrics: series, + } + + stats, err := requestStatsFromApi(mockClient, k8s.KubernetesPods) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + if !strings.Contains(stats, podName) { + t.Fatalf("Expected response to contain [%s], but was [%s]", podName, stats) + } + }) + + t.Run("Returns error if API call failed", func(t *testing.T) { + mockClient := &mockApiClient{} + mockClient.errorToReturn = errors.New("Expected") + output, err := requestStatsFromApi(mockClient, k8s.KubernetesPods) + + if err == nil { + t.Fatalf("Expected error, got nothing but the output [%s]", output) + } + }) +} + +func TestRenderStats(t *testing.T) { + t.Run("Prints stats correctly for example with one entry", func(t *testing.T) { + allSeries := make([]*pb.MetricSeries, 0) + seriesForPodX := generateMetricSeriesFor(fmt.Sprintf("deployment-%d", 66), int64(10)) + allSeries = append(allSeries, seriesForPodX...) + + //shuffles + for i := range allSeries { + j := rand.Intn(i + 1) + allSeries[i], allSeries[j] = allSeries[j], allSeries[i] + } + + response := &pb.MetricResponse{ + Metrics: allSeries, + } + + renderedStats, err := renderStats(response) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + goldenFileBytes, err := ioutil.ReadFile("testdata/stat_one_output.golden") + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + expectedContent := string(goldenFileBytes) + + if expectedContent != renderedStats { + t.Fatalf("Expected function to render:\n%s\bbut got:\n%s", expectedContent, renderedStats) + } + }) + + t.Run("Prints stats correctly for busy example", func(t *testing.T) { + allSeries := make([]*pb.MetricSeries, 0) + for i := 0; i < 10; i++ { + seriesForPodX := generateMetricSeriesFor(fmt.Sprintf("pod-%d", i), int64(i)) + allSeries = append(allSeries, seriesForPodX...) + } + + //shuffles + for i := range allSeries { + j := rand.Intn(i + 1) + allSeries[i], allSeries[j] = allSeries[j], allSeries[i] + } + + response := &pb.MetricResponse{ + Metrics: allSeries, + } + + renderedStats, err := renderStats(response) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + goldenFileBytes, err := ioutil.ReadFile("testdata/stat_busy_output.golden") + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + expectedContent := string(goldenFileBytes) + + if expectedContent != renderedStats { + t.Fatalf("Expected function to render:\n%s\bbut got:\n%s", expectedContent, renderedStats) + } + }) + + t.Run("Prints stats correctly for empty example", func(t *testing.T) { + response := &pb.MetricResponse{ + Metrics: make([]*pb.MetricSeries, 0), + } + + renderedStats, err := renderStats(response) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + goldenFileBytes, err := ioutil.ReadFile("testdata/stat_empty_output.golden") + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + expectedContent := string(goldenFileBytes) + + if expectedContent != renderedStats { + t.Fatalf("Expected function to render:\n%s\bbut got:\n%s", expectedContent, renderedStats) + } + }) +} + func TestSortStatsKeys(t *testing.T) { t.Run("Sorts the keys alphabetically", func(t *testing.T) { unsorted := map[string]*row{ @@ -22,8 +168,67 @@ func TestSortStatsKeys(t *testing.T) { expected := []string{"kube-system/heapster-v1.4.3", "kube-system/kubernetes-dashboard", "kube-system/l7-default-backend", "other/grafana", "test/backend4", "test/hello10", "test/world-deploy1", "test/world-deploy2"} - sorted := sortStatsKeys(unsorted) assert.Equal(t, expected, sorted, "Not Sorted!") }) } + +func generateMetricSeriesFor(podName string, seed int64) []*pb.MetricSeries { + metricDatapoints := []*pb.MetricDatapoint{ + { + Value: &pb.MetricValue{ + Value: &pb.MetricValue_Gauge{ + Gauge: float64(seed) / 10, + }, + }, + }, + } + latencyHistogram := []*pb.MetricDatapoint{ + { + Value: &pb.MetricValue{ + Value: &pb.MetricValue_Histogram{ + Histogram: &pb.Histogram{ + Values: []*pb.HistogramValue{ + { + Label: pb.HistogramLabel_P50, + Value: 1 + seed, + }, + { + Label: pb.HistogramLabel_P99, + Value: 9 + seed, + }, + { + Label: pb.HistogramLabel_P95, + Value: 5 + seed, + }, + }, + }, + }, + }, + }, + } + series := []*pb.MetricSeries{ + { + Name: pb.MetricName_REQUEST_RATE, + Metadata: &pb.MetricMetadata{ + TargetPod: podName, + }, + Datapoints: metricDatapoints, + }, + { + Name: pb.MetricName_SUCCESS_RATE, + Metadata: &pb.MetricMetadata{ + TargetPod: podName, + }, + Datapoints: metricDatapoints, + }, + { + Name: pb.MetricName_LATENCY, + Metadata: &pb.MetricMetadata{ + TargetPod: podName, + }, + Datapoints: latencyHistogram, + }, + } + return series +} diff --git a/cli/cmd/test_helper.go b/cli/cmd/test_helper.go index 63c30f485ad06..8febdfedb9a65 100644 --- a/cli/cmd/test_helper.go +++ b/cli/cmd/test_helper.go @@ -11,10 +11,11 @@ type mockApiClient struct { errorToReturn error versionInfoToReturn *pb.VersionInfo listPodsResponseToReturn *pb.ListPodsResponse + metricResponseToReturn *pb.MetricResponse } func (c *mockApiClient) Stat(ctx context.Context, in *pb.MetricRequest, opts ...grpc.CallOption) (*pb.MetricResponse, error) { - return nil, c.errorToReturn + return c.metricResponseToReturn, c.errorToReturn } func (c *mockApiClient) Version(ctx context.Context, in *pb.Empty, opts ...grpc.CallOption) (*pb.VersionInfo, error) { diff --git a/cli/cmd/testdata/stat_busy_output.golden b/cli/cmd/testdata/stat_busy_output.golden new file mode 100644 index 0000000000000..acf4fcc6d85b8 --- /dev/null +++ b/cli/cmd/testdata/stat_busy_output.golden @@ -0,0 +1,11 @@ +NAME REQUEST_RATE SUCCESS_RATE P50_LATENCY P99_LATENCY +pod-0 0.0rps 0.00% 1ms 9ms +pod-1 0.1rps 10.00% 2ms 10ms +pod-2 0.2rps 20.00% 3ms 11ms +pod-3 0.3rps 30.00% 4ms 12ms +pod-4 0.4rps 40.00% 5ms 13ms +pod-5 0.5rps 50.00% 6ms 14ms +pod-6 0.6rps 60.00% 7ms 15ms +pod-7 0.7rps 70.00% 8ms 16ms +pod-8 0.8rps 80.00% 9ms 17ms +pod-9 0.9rps 90.00% 10ms 18ms diff --git a/cli/cmd/testdata/stat_empty_output.golden b/cli/cmd/testdata/stat_empty_output.golden new file mode 100644 index 0000000000000..9b0a983508a04 --- /dev/null +++ b/cli/cmd/testdata/stat_empty_output.golden @@ -0,0 +1 @@ +NAME REQUEST_RATE SUCCESS_RATE P50_LATENCY P99_LATENCY diff --git a/cli/cmd/testdata/stat_one_output.golden b/cli/cmd/testdata/stat_one_output.golden new file mode 100644 index 0000000000000..05aba31613843 --- /dev/null +++ b/cli/cmd/testdata/stat_one_output.golden @@ -0,0 +1,2 @@ +NAME REQUEST_RATE SUCCESS_RATE P50_LATENCY P99_LATENCY +deployment-66 1.0rps 100.00% 11ms 19ms From da97dc60bfe7222e5bbf243b4f236bacddbc9325 Mon Sep 17 00:00:00 2001 From: Phil Calcado Date: Fri, 22 Dec 2017 13:36:13 +1100 Subject: [PATCH 7/7] =?UTF-8?q?Make=20tap=20command=20testabl=C3=AB?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Phil Calcado --- cli/cmd/stat.go | 12 +- cli/cmd/tap.go | 143 ++++++++++------- cli/cmd/tap_test.go | 186 ++++++++++++++++++++++- cli/cmd/test_helper.go | 27 +++- cli/cmd/testdata/tap_busy_output.golden | 2 + cli/cmd/testdata/tap_empty_output.golden | 0 6 files changed, 306 insertions(+), 64 deletions(-) create mode 100644 cli/cmd/testdata/tap_busy_output.golden create mode 100644 cli/cmd/testdata/tap_empty_output.golden diff --git a/cli/cmd/stat.go b/cli/cmd/stat.go index 165813773ddb4..36681e1117519 100644 --- a/cli/cmd/stat.go +++ b/cli/cmd/stat.go @@ -36,25 +36,25 @@ Valid resource types include: The optional [TARGET] option can be either a name for a deployment or pod resource`, RunE: func(cmd *cobra.Command, args []string) error { - var friendlyName string + var friendlyNameForResourceType string switch len(args) { case 1: - friendlyName = args[0] + friendlyNameForResourceType = args[0] case 2: - friendlyName = args[0] + friendlyNameForResourceType = args[0] target = args[1] default: return errors.New("please specify a resource type: pods, deployments or paths") } - validatedResourceType, err := k8s.CanonicalKubernetesNameFromFriendlyName(friendlyName) + validatedResourceType, err := k8s.CanonicalKubernetesNameFromFriendlyName(friendlyNameForResourceType) if err != nil { - switch friendlyName { + switch friendlyNameForResourceType { case "paths", "path", "pa": validatedResourceType = ConduitPaths default: - return fmt.Errorf("invalid resource type %s, only %v are allowed as resource types", friendlyName, []string{k8s.KubernetesPods, k8s.KubernetesDeployments, ConduitPaths}) + return fmt.Errorf("invalid resource type %s, only %v are allowed as resource types", friendlyNameForResourceType, []string{k8s.KubernetesPods, k8s.KubernetesDeployments, ConduitPaths}) } } kubeApi, err := k8s.MakeK8sAPi(shell.MakeUnixShell(), kubeconfigPath, apiAddr) diff --git a/cli/cmd/tap.go b/cli/cmd/tap.go index 2cac9f01c976f..69821103f1856 100644 --- a/cli/cmd/tap.go +++ b/cli/cmd/tap.go @@ -1,12 +1,14 @@ package cmd import ( + "bytes" "context" "errors" "fmt" "io" "os" "strings" + "text/tabwriter" "github.com/runconduit/conduit/cli/k8s" "github.com/runconduit/conduit/cli/shell" @@ -39,57 +41,46 @@ Valid targets include: * Pods (default/hello-world-h4fb2) * Deployments (default/hello-world)`, RunE: func(cmd *cobra.Command, args []string) error { - switch len(args) { - case 2: - resourceType := strings.ToLower(args[0]) - - // We don't validate inputs because they are validated on the server. - req := &pb.TapRequest{ - MaxRps: maxRps, - ToPort: toPort, - ToIP: toIP, - FromPort: fromPort, - FromIP: fromIP, - Scheme: scheme, - Method: method, - Authority: authority, - Path: path, - } - - switch resourceType { - case "deploy", "deployment", "deployments": - req.Target = &pb.TapRequest_Deployment{ - Deployment: args[1], - } - - case "po", "pod", "pods": - req.Target = &pb.TapRequest_Pod{ - Pod: args[1], - } - - default: - return errors.New("invalid target type") - } - - kubeApi, err := k8s.MakeK8sAPi(shell.MakeUnixShell(), kubeconfigPath, apiAddr) - if err != nil { - return err - } - - client, err := newApiClient(kubeApi) - if err != nil { - return err - } - rsp, err := client.Tap(context.Background(), req) - if err != nil { - return err - } - print(rsp) - - return nil - default: + if len(args) != 2 { return errors.New("please specify a target") } + + // We don't validate inputs because they are validated on the server. + partialReq := &pb.TapRequest{ + MaxRps: maxRps, + ToPort: toPort, + ToIP: toIP, + FromPort: fromPort, + FromIP: fromIP, + Scheme: scheme, + Method: method, + Authority: authority, + Path: path, + } + + friendlyNameForResourceType := strings.ToLower(args[0]) + validatedResourceType, err := k8s.CanonicalKubernetesNameFromFriendlyName(friendlyNameForResourceType) + if err != nil { + return fmt.Errorf("unsupported resourceType [%s]", friendlyNameForResourceType) + } + + kubeApi, err := k8s.MakeK8sAPi(shell.MakeUnixShell(), kubeconfigPath, apiAddr) + if err != nil { + return err + } + + client, err := newApiClient(kubeApi) + if err != nil { + return err + } + + output, err := requestTapFromApi(client, args[1], validatedResourceType, partialReq) + if err != nil { + return err + } + _, err = fmt.Print(output) + + return err }, } @@ -107,7 +98,46 @@ func init() { tapCmd.PersistentFlags().StringVar(&path, "path", "", "Display requests with paths that start with this prefix") } -func print(rsp pb.Api_TapClient) { +func requestTapFromApi(client pb.ApiClient, targetName string, resourceType string, req *pb.TapRequest) (string, error) { + switch resourceType { + case k8s.KubernetesDeployments: + req.Target = &pb.TapRequest_Deployment{ + Deployment: targetName, + } + + case k8s.KubernetesPods: + req.Target = &pb.TapRequest_Pod{ + Pod: targetName, + } + default: + return "", fmt.Errorf("unsupported resourceType [%s]", resourceType) + } + + rsp, err := client.Tap(context.Background(), req) + if err != nil { + return "", err + } + + return renderTap(rsp) +} + +func renderTap(rsp pb.Api_TapClient) (string, error) { + var buffer bytes.Buffer + w := tabwriter.NewWriter(&buffer, 0, 0, 0, ' ', tabwriter.AlignRight) + err := writeTapEvenToBuffer(rsp, w) + if err != nil { + return "", err + } + w.Flush() + + // strip left padding on the first column + out := string(buffer.Bytes()) + + return out, nil + +} + +func writeTapEvenToBuffer(rsp pb.Api_TapClient, w *tabwriter.Writer) error { for { event, err := rsp.Recv() if err == io.EOF { @@ -117,17 +147,24 @@ func print(rsp pb.Api_TapClient) { fmt.Fprintln(os.Stderr, err) break } - fmt.Println(eventToString(event)) + _, err = fmt.Fprintln(w, renderTapEvent(event)) + if err != nil { + return err + } } + + return nil } -func eventToString(event *common.TapEvent) string { +func renderTapEvent(event *common.TapEvent) string { flow := fmt.Sprintf("src=%s dst=%s", util.AddressToString(event.GetSource()), util.AddressToString(event.GetTarget()), ) - switch ev := event.GetHttp().Event.(type) { + http := event.GetHttp() + http_event := http.Event + switch ev := http_event.(type) { case *common.TapEvent_Http_RequestInit_: return fmt.Sprintf("req id=%d:%d %s :method=%s :authority=%s :path=%s", ev.RequestInit.Id.Base, diff --git a/cli/cmd/tap_test.go b/cli/cmd/tap_test.go index 80ac88c616843..b22d101476942 100644 --- a/cli/cmd/tap_test.go +++ b/cli/cmd/tap_test.go @@ -1,15 +1,170 @@ package cmd import ( + "errors" + "io/ioutil" "net/http" "testing" + "github.com/runconduit/conduit/cli/k8s" + pb "github.com/runconduit/conduit/controller/gen/public" + "github.com/golang/protobuf/ptypes/duration" + google_protobuf "github.com/golang/protobuf/ptypes/duration" common "github.com/runconduit/conduit/controller/gen/common" "github.com/runconduit/conduit/controller/util" "google.golang.org/grpc/codes" ) +func TestRequestTapFromApi(t *testing.T) { + t.Run("Should render busy response if everything went well", func(t *testing.T) { + authority := "localhost" + targetName := "pod-666" + resourceType := k8s.KubernetesPods + scheme := "https" + method := "GET" + path := "/some/path" + sourceIp := "234.234.234.234" + targetIp := "123.123.123.123" + mockApiClient := &mockApiClient{} + + event1 := createEvent(&common.TapEvent_Http{ + Event: &common.TapEvent_Http_RequestInit_{ + RequestInit: &common.TapEvent_Http_RequestInit{ + Id: &common.TapEvent_Http_StreamId{ + Base: 1, + }, + Authority: authority, + Path: path, + }, + }, + }) + event2 := createEvent(&common.TapEvent_Http{ + Event: &common.TapEvent_Http_ResponseEnd_{ + ResponseEnd: &common.TapEvent_Http_ResponseEnd{ + Id: &common.TapEvent_Http_StreamId{ + Base: 1, + }, + GrpcStatus: 666, + SinceRequestInit: &google_protobuf.Duration{ + Seconds: 10, + }, + SinceResponseInit: &google_protobuf.Duration{ + Seconds: 100, + }, + ResponseBytes: 1337, + }, + }, + }) + mockApiClient.api_TapClientToReturn = &mockApi_TapClient{ + tapEventsToReturn: []common.TapEvent{event1, event2}, + } + + partialReq := &pb.TapRequest{ + MaxRps: 0, + ToPort: 8080, + ToIP: targetIp, + FromPort: 90, + FromIP: sourceIp, + Scheme: scheme, + Method: method, + Authority: authority, + Path: path, + } + + output, err := requestTapFromApi(mockApiClient, targetName, resourceType, partialReq) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + goldenFileBytes, err := ioutil.ReadFile("testdata/tap_busy_output.golden") + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + expectedContent := string(goldenFileBytes) + + if expectedContent != output { + t.Fatalf("Expected function to render:\n%s\bbut got:\n%s", expectedContent, output) + } + }) + + t.Run("Should render empty response if no events returned", func(t *testing.T) { + authority := "localhost" + targetName := "pod-666" + resourceType := k8s.KubernetesPods + scheme := "https" + method := "GET" + path := "/some/path" + sourceIp := "234.234.234.234" + targetIp := "123.123.123.123" + mockApiClient := &mockApiClient{} + + mockApiClient.api_TapClientToReturn = &mockApi_TapClient{ + tapEventsToReturn: []common.TapEvent{}, + } + + partialReq := &pb.TapRequest{ + MaxRps: 0, + ToPort: 8080, + ToIP: targetIp, + FromPort: 90, + FromIP: sourceIp, + Scheme: scheme, + Method: method, + Authority: authority, + Path: path, + } + + output, err := requestTapFromApi(mockApiClient, targetName, resourceType, partialReq) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + goldenFileBytes, err := ioutil.ReadFile("testdata/tap_empty_output.golden") + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + expectedContent := string(goldenFileBytes) + + if expectedContent != output { + t.Fatalf("Expected function to render:\n%s\bbut got:\n%s", expectedContent, output) + } + }) + + t.Run("Should return error if stream returned error", func(t *testing.T) { + t.SkipNow() + authority := "localhost" + targetName := "pod-666" + resourceType := k8s.KubernetesPods + scheme := "https" + method := "GET" + path := "/some/path" + sourceIp := "234.234.234.234" + targetIp := "123.123.123.123" + mockApiClient := &mockApiClient{} + mockApiClient.api_TapClientToReturn = &mockApi_TapClient{ + errorsToReturn: []error{errors.New("expected")}, + } + + partialReq := &pb.TapRequest{ + MaxRps: 0, + ToPort: 8080, + ToIP: targetIp, + FromPort: 90, + FromIP: sourceIp, + Scheme: scheme, + Method: method, + Authority: authority, + Path: path, + } + + output, err := requestTapFromApi(mockApiClient, targetName, resourceType, partialReq) + if err == nil { + t.Fatalf("Expecting error, got nothing but outpus [%s]", output) + } + }) +} + func TestEventToString(t *testing.T) { toTapEvent := func(httpEvent *common.TapEvent_Http) *common.TapEvent { streamId := &common.TapEvent_Http_StreamId{ @@ -60,7 +215,7 @@ func TestEventToString(t *testing.T) { }) expectedOutput := "req id=7:8 src=1.2.3.4:5555 dst=2.3.4.5:6666 :method=POST :authority=hello.default:7777 :path=/hello.v1.HelloService/Hello" - output := eventToString(event) + output := renderTapEvent(event) if output != expectedOutput { t.Fatalf("Expecting command output to be [%s], got [%s]", expectedOutput, output) } @@ -77,7 +232,7 @@ func TestEventToString(t *testing.T) { }) expectedOutput := "rsp id=7:8 src=1.2.3.4:5555 dst=2.3.4.5:6666 :status=200 latency=999µs" - output := eventToString(event) + output := renderTapEvent(event) if output != expectedOutput { t.Fatalf("Expecting command output to be [%s], got [%s]", expectedOutput, output) } @@ -96,7 +251,7 @@ func TestEventToString(t *testing.T) { }) expectedOutput := "end id=7:8 src=1.2.3.4:5555 dst=2.3.4.5:6666 grpc-status=OK duration=888µs response-length=111B" - output := eventToString(event) + output := renderTapEvent(event) if output != expectedOutput { t.Fatalf("Expecting command output to be [%s], got [%s]", expectedOutput, output) } @@ -106,9 +261,32 @@ func TestEventToString(t *testing.T) { event := toTapEvent(&common.TapEvent_Http{}) expectedOutput := "unknown src=1.2.3.4:5555 dst=2.3.4.5:6666" - output := eventToString(event) + output := renderTapEvent(event) if output != expectedOutput { t.Fatalf("Expecting command output to be [%s], got [%s]", expectedOutput, output) } }) } + +func createEvent(event_http *common.TapEvent_Http) common.TapEvent { + event := common.TapEvent{ + Source: &common.TcpAddress{ + Ip: &common.IPAddress{ + Ip: &common.IPAddress_Ipv4{ + Ipv4: uint32(1), + }, + }, + }, + Target: &common.TcpAddress{ + Ip: &common.IPAddress{ + Ip: &common.IPAddress_Ipv4{ + Ipv4: uint32(9), + }, + }, + }, + Event: &common.TapEvent_Http_{ + Http: event_http, + }, + } + return event +} diff --git a/cli/cmd/test_helper.go b/cli/cmd/test_helper.go index 8febdfedb9a65..7303958a60f1c 100644 --- a/cli/cmd/test_helper.go +++ b/cli/cmd/test_helper.go @@ -2,7 +2,9 @@ package cmd import ( "context" + "io" + common "github.com/runconduit/conduit/controller/gen/common" pb "github.com/runconduit/conduit/controller/gen/public" "google.golang.org/grpc" ) @@ -12,6 +14,7 @@ type mockApiClient struct { versionInfoToReturn *pb.VersionInfo listPodsResponseToReturn *pb.ListPodsResponse metricResponseToReturn *pb.MetricResponse + api_TapClientToReturn pb.Api_TapClient } func (c *mockApiClient) Stat(ctx context.Context, in *pb.MetricRequest, opts ...grpc.CallOption) (*pb.MetricResponse, error) { @@ -27,5 +30,27 @@ func (c *mockApiClient) ListPods(ctx context.Context, in *pb.Empty, opts ...grpc } func (c *mockApiClient) Tap(ctx context.Context, in *pb.TapRequest, opts ...grpc.CallOption) (pb.Api_TapClient, error) { - return nil, c.errorToReturn + return c.api_TapClientToReturn, c.errorToReturn +} + +type mockApi_TapClient struct { + tapEventsToReturn []common.TapEvent + errorsToReturn []error + grpc.ClientStream +} + +func (a *mockApi_TapClient) Recv() (*common.TapEvent, error) { + var eventPopped common.TapEvent + var errorPopped error + if len(a.tapEventsToReturn) == 0 && len(a.errorsToReturn) == 0 { + return nil, io.EOF + } + if len(a.tapEventsToReturn) != 0 { + eventPopped, a.tapEventsToReturn = a.tapEventsToReturn[0], a.tapEventsToReturn[1:] + } + if len(a.errorsToReturn) != 0 { + errorPopped, a.errorsToReturn = a.errorsToReturn[0], a.errorsToReturn[1:] + } + + return &eventPopped, errorPopped } diff --git a/cli/cmd/testdata/tap_busy_output.golden b/cli/cmd/testdata/tap_busy_output.golden new file mode 100644 index 0000000000000..f3d7fe3575836 --- /dev/null +++ b/cli/cmd/testdata/tap_busy_output.golden @@ -0,0 +1,2 @@ +req id=1:0 src=0.0.0.1:0 dst=0.0.0.9:0 :method=GET :authority=localhost :path=/some/path +end id=1:0 src=0.0.0.1:0 dst=0.0.0.9:0 grpc-status=Code(666) duration=0µs response-length=1337B diff --git a/cli/cmd/testdata/tap_empty_output.golden b/cli/cmd/testdata/tap_empty_output.golden new file mode 100644 index 0000000000000..e69de29bb2d1d