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