From bce79ebcc4fd16b7e93743f6de3f7ca9f0cbd1a8 Mon Sep 17 00:00:00 2001 From: Risha Mars Date: Wed, 9 May 2018 16:22:11 -0700 Subject: [PATCH 01/14] Refactor resource query into its own function --- controller/api/public/stat_summary.go | 53 ++++++++++++++++++++------- 1 file changed, 40 insertions(+), 13 deletions(-) diff --git a/controller/api/public/stat_summary.go b/controller/api/public/stat_summary.go index c5e25633a4600..3570ac30f19af 100644 --- a/controller/api/public/stat_summary.go +++ b/controller/api/public/stat_summary.go @@ -60,6 +60,25 @@ func (s *grpcServer) StatSummary(ctx context.Context, req *pb.StatSummaryRequest }, nil } + res, err := s.resourceQuery(ctx, req) + if err != nil { + return nil, util.GRPCError(err) + } + + rsp := pb.StatSummaryResponse{ + Response: &pb.StatSummaryResponse_Ok_{ // https://github.com/golang/protobuf/issues/205 + Ok: &pb.StatSummaryResponse_Ok{ + StatTables: []*pb.StatTable{ + res, + }, + }, + }, + } + + return &rsp, nil +} + +func (s *grpcServer) resourceQuery(ctx context.Context, req *pb.StatSummaryRequest) (*pb.StatTable, error) { objects, err := s.lister.GetObjects(req.Selector.Resource.Namespace, req.Selector.Resource.Type, req.Selector.Resource.Name) if err != nil { return nil, util.GRPCError(err) @@ -102,7 +121,7 @@ func (s *grpcServer) objectQuery( req *pb.StatSummaryRequest, objects map[string]metav1.Object, meshCount map[string]*podCount, -) (*pb.StatSummaryResponse, error) { +) (*pb.StatTable, error) { rows := make([]*pb.StatTable_PodGroup_Row, 0) requestMetrics, err := s.getRequests(ctx, req, req.TimeWindow) @@ -149,22 +168,30 @@ func (s *grpcServer) objectQuery( rows = append(rows, &row) } - rsp := pb.StatSummaryResponse{ - Response: &pb.StatSummaryResponse_Ok_{ // https://github.com/golang/protobuf/issues/205 - Ok: &pb.StatSummaryResponse_Ok{ - StatTables: []*pb.StatTable{ - &pb.StatTable{ - Table: &pb.StatTable_PodGroup_{ - PodGroup: &pb.StatTable_PodGroup{ - Rows: rows, - }, - }, - }, - }, + rsp := pb.StatTable{ + Table: &pb.StatTable_PodGroup_{ + PodGroup: &pb.StatTable_PodGroup{ + Rows: rows, }, }, } + // rsp := pb.StatSummaryResponse{ + // Response: &pb.StatSummaryResponse_Ok_{ // https://github.com/golang/protobuf/issues/205 + // Ok: &pb.StatSummaryResponse_Ok{ + // StatTables: []*pb.StatTable{ + // &pb.StatTable{ + // Table: &pb.StatTable_PodGroup_{ + // PodGroup: &pb.StatTable_PodGroup{ + // Rows: rows, + // }, + // }, + // }, + // }, + // }, + // }, + // } + return &rsp, nil } From 0af4c588b51459bce3196d03d84c59e9feb1a929 Mon Sep 17 00:00:00 2001 From: Risha Mars Date: Wed, 9 May 2018 16:45:11 -0700 Subject: [PATCH 02/14] Handle requests for all resources --- controller/api/public/stat_summary.go | 82 +++++++++++++++++---------- controller/api/util/api_utils.go | 10 +++- 2 files changed, 60 insertions(+), 32 deletions(-) diff --git a/controller/api/public/stat_summary.go b/controller/api/public/stat_summary.go index 3570ac30f19af..1cb50556afd32 100644 --- a/controller/api/public/stat_summary.go +++ b/controller/api/public/stat_summary.go @@ -26,6 +26,11 @@ type promResult struct { err error } +type resourceResult struct { + res *pb.StatTable + err error +} + const ( reqQuery = "sum(increase(response_total%s[%s])) by (%s, classification)" latencyQuantileQuery = "histogram_quantile(%s, sum(irate(response_latency_ms_bucket%s[%s])) by (le, %s))" @@ -41,6 +46,9 @@ const ( var promTypes = []promType{promRequests, promLatencyP50, promLatencyP95, promLatencyP99} +// resources to query when Resource.Type is "all" +var resourceTypes = []string{k8s.Pods, k8s.Deployments, k8s.ReplicationControllers, k8s.Services} + type podCount struct { inMesh uint64 total uint64 @@ -60,17 +68,49 @@ func (s *grpcServer) StatSummary(ctx context.Context, req *pb.StatSummaryRequest }, nil } - res, err := s.resourceQuery(ctx, req) - if err != nil { - return nil, util.GRPCError(err) + statTables := make([]*pb.StatTable, 0) + + if req.Selector.Resource.Type == "all" { + // request stats for all resourceTypes, in parallel + resultChan := make(chan resourceResult) + + for _, resource := range resourceTypes { + req := &pb.StatSummaryRequest{ + Selector: &pb.ResourceSelection{ + Resource: &pb.Resource{ + Type: resource, + Namespace: req.Selector.Resource.Namespace, + }, + LabelSelector: req.Selector.LabelSelector, + }, + TimeWindow: req.TimeWindow, + } + + go func() { + resultChan <- s.resourceQuery(ctx, req) + }() + } + + for i := 0; i < len(resourceTypes); i++ { + result := <-resultChan + if result.err != nil { + return nil, util.GRPCError(result.err) + } + statTables = append(statTables, result.res) + } + } else { + // get stats for a single resourceType + result := s.resourceQuery(ctx, req) + if result.err != nil { + return nil, util.GRPCError(result.err) + } + statTables = append(statTables, result.res) } rsp := pb.StatSummaryResponse{ Response: &pb.StatSummaryResponse_Ok_{ // https://github.com/golang/protobuf/issues/205 Ok: &pb.StatSummaryResponse_Ok{ - StatTables: []*pb.StatTable{ - res, - }, + StatTables: statTables, }, }, } @@ -78,10 +118,10 @@ func (s *grpcServer) StatSummary(ctx context.Context, req *pb.StatSummaryRequest return &rsp, nil } -func (s *grpcServer) resourceQuery(ctx context.Context, req *pb.StatSummaryRequest) (*pb.StatTable, error) { +func (s *grpcServer) resourceQuery(ctx context.Context, req *pb.StatSummaryRequest) resourceResult { objects, err := s.lister.GetObjects(req.Selector.Resource.Namespace, req.Selector.Resource.Type, req.Selector.Resource.Name) if err != nil { - return nil, util.GRPCError(err) + return resourceResult{res: nil, err: err} } // TODO: make these one struct: @@ -92,28 +132,28 @@ func (s *grpcServer) resourceQuery(ctx context.Context, req *pb.StatSummaryReque for _, object := range objects { key, err := cache.MetaNamespaceKeyFunc(object) if err != nil { - return nil, util.GRPCError(err) + return resourceResult{res: nil, err: err} } metaObj, err := meta.Accessor(object) if err != nil { - return nil, util.GRPCError(err) + return resourceResult{res: nil, err: err} } objectMap[key] = metaObj meshCount, err := s.getMeshedPodCount(object) if err != nil { - return nil, util.GRPCError(err) + return resourceResult{res: nil, err: err} } meshCountMap[key] = meshCount } res, err := s.objectQuery(ctx, req, objectMap, meshCountMap) if err != nil { - return nil, util.GRPCError(err) + return resourceResult{res: nil, err: err} } - return res, nil + return resourceResult{res: res, err: nil} } func (s *grpcServer) objectQuery( @@ -176,22 +216,6 @@ func (s *grpcServer) objectQuery( }, } - // rsp := pb.StatSummaryResponse{ - // Response: &pb.StatSummaryResponse_Ok_{ // https://github.com/golang/protobuf/issues/205 - // Ok: &pb.StatSummaryResponse_Ok{ - // StatTables: []*pb.StatTable{ - // &pb.StatTable{ - // Table: &pb.StatTable_PodGroup_{ - // PodGroup: &pb.StatTable_PodGroup{ - // Rows: rows, - // }, - // }, - // }, - // }, - // }, - // }, - // } - return &rsp, nil } diff --git a/controller/api/util/api_utils.go b/controller/api/util/api_utils.go index 1f19a1f4f0833..c98fda9be974e 100644 --- a/controller/api/util/api_utils.go +++ b/controller/api/util/api_utils.go @@ -98,9 +98,13 @@ func BuildStatSummaryRequest(p StatSummaryRequestParams) (*pb.StatSummaryRequest window = p.TimeWindow } - resourceType, err := k8s.CanonicalKubernetesNameFromFriendlyName(p.ResourceType) - if err != nil { - return nil, err + resourceType := p.ResourceType + var err error + if resourceType != "all" { + resourceType, err = k8s.CanonicalKubernetesNameFromFriendlyName(p.ResourceType) + if err != nil { + return nil, err + } } statRequest := &pb.StatSummaryRequest{ From fb3631438a749b82e105a65b0ce4a312e0847a92 Mon Sep 17 00:00:00 2001 From: Risha Mars Date: Wed, 9 May 2018 17:17:06 -0700 Subject: [PATCH 03/14] Modify CLI to also query for all --- controller/api/public/stat_summary.go | 2 +- controller/api/util/api_utils.go | 10 ++++++++-- pkg/k8s/k8s.go | 1 + 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/controller/api/public/stat_summary.go b/controller/api/public/stat_summary.go index 1cb50556afd32..754d5adda016b 100644 --- a/controller/api/public/stat_summary.go +++ b/controller/api/public/stat_summary.go @@ -70,7 +70,7 @@ func (s *grpcServer) StatSummary(ctx context.Context, req *pb.StatSummaryRequest statTables := make([]*pb.StatTable, 0) - if req.Selector.Resource.Type == "all" { + if req.Selector.Resource.Type == k8s.All { // request stats for all resourceTypes, in parallel resultChan := make(chan resourceResult) diff --git a/controller/api/util/api_utils.go b/controller/api/util/api_utils.go index c98fda9be974e..14957550470e5 100644 --- a/controller/api/util/api_utils.go +++ b/controller/api/util/api_utils.go @@ -100,7 +100,7 @@ func BuildStatSummaryRequest(p StatSummaryRequestParams) (*pb.StatSummaryRequest resourceType := p.ResourceType var err error - if resourceType != "all" { + if resourceType != k8s.All { resourceType, err = k8s.CanonicalKubernetesNameFromFriendlyName(p.ResourceType) if err != nil { return nil, err @@ -194,7 +194,13 @@ func BuildResource(namespace string, args ...string) (pb.Resource, error) { } func buildResource(namespace string, resType string, name string) (pb.Resource, error) { - canonicalType, err := k8s.CanonicalKubernetesNameFromFriendlyName(resType) + canonicalType := resType + var err error + + if canonicalType != k8s.All { + canonicalType, err = k8s.CanonicalKubernetesNameFromFriendlyName(resType) + } + if err != nil { return pb.Resource{}, err } diff --git a/pkg/k8s/k8s.go b/pkg/k8s/k8s.go index f259cbf630c3a..866805f259674 100644 --- a/pkg/k8s/k8s.go +++ b/pkg/k8s/k8s.go @@ -14,6 +14,7 @@ const ( Pods = "pods" ReplicationControllers = "replicationcontrollers" Services = "services" + All = "all" ) // ResourceTypesToProxyLabels maps Kubernetes resource type names to keys From 9caff27e0b112199b7e03f6b80ff4a892ba14b34 Mon Sep 17 00:00:00 2001 From: Risha Mars Date: Wed, 9 May 2018 17:50:15 -0700 Subject: [PATCH 04/14] Modify CLI stat display to display stat tables for multiple resource types --- cli/cmd/stat.go | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/cli/cmd/stat.go b/cli/cmd/stat.go index 88707dd148ba2..0502ee36ece2c 100644 --- a/cli/cmd/stat.go +++ b/cli/cmd/stat.go @@ -149,20 +149,29 @@ type row struct { *rowStats } +var ( + nameHeader = "NAME" + maxNameLength = len(nameHeader) + namespaceHeader = "NAMESPACE" + maxNamespaceLength = len(namespaceHeader) +) + func writeStatsToBuffer(resp *pb.StatSummaryResponse, w *tabwriter.Writer) { - nameHeader := "NAME" - maxNameLength := len(nameHeader) - namespaceHeader := "NAMESPACE" - maxNamespaceLength := len(namespaceHeader) - stats := make(map[string]*row) + statTables := make(map[string]map[string]*row) for _, statTable := range resp.GetOk().StatTables { table := statTable.GetPodGroup() + for _, r := range table.Rows { name := r.Resource.Name namespace := r.Resource.Namespace key := fmt.Sprintf("%s/%s", namespace, name) + resourceKey := r.Resource.Type + + if _, ok := statTables[resourceKey]; !ok { + statTables[resourceKey] = make(map[string]*row) + } if len(name) > maxNameLength { maxNameLength = len(name) @@ -172,12 +181,12 @@ func writeStatsToBuffer(resp *pb.StatSummaryResponse, w *tabwriter.Writer) { maxNamespaceLength = len(namespace) } - stats[key] = &row{ + statTables[resourceKey][key] = &row{ meshed: fmt.Sprintf("%d/%d", r.MeshedPodCount, r.RunningPodCount), } if r.Stats != nil { - stats[key].rowStats = &rowStats{ + statTables[resourceKey][key].rowStats = &rowStats{ requestRate: getRequestRate(*r), successRate: getSuccessRate(*r), latencyP50: r.Stats.LatencyMsP50, @@ -188,6 +197,14 @@ func writeStatsToBuffer(resp *pb.StatSummaryResponse, w *tabwriter.Writer) { } } + fmt.Fprint(w, "\n\n") + for resourceType, stats := range statTables { + fmt.Fprintf(w, "\n%s\n\n", resourceType) + printStatTable(stats, w) + } +} + +func printStatTable(stats map[string]*row, w *tabwriter.Writer) { if len(stats) == 0 { fmt.Fprintln(os.Stderr, "No traffic found.") os.Exit(0) From 6714a9a1d0de3b805dfeb17180620ddc71e03fff Mon Sep 17 00:00:00 2001 From: Risha Mars Date: Thu, 10 May 2018 10:56:59 -0700 Subject: [PATCH 05/14] Add All to k8s.CanonicalKubernetesNameFromFriendlyName --- controller/api/public/stat_summary.go | 10 ++++++++++ controller/api/util/api_utils.go | 17 ++++------------- pkg/k8s/k8s.go | 2 ++ 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/controller/api/public/stat_summary.go b/controller/api/public/stat_summary.go index 754d5adda016b..55187cd397f6e 100644 --- a/controller/api/public/stat_summary.go +++ b/controller/api/public/stat_summary.go @@ -66,6 +66,16 @@ func (s *grpcServer) StatSummary(ctx context.Context, req *pb.StatSummaryRequest }, }, }, nil + + switch req.Outbound.(type) { + case *pb.StatSummaryRequest_ToResource: + if req.Outbound.(*pb.StatSummaryRequest_ToResource).ToResource.Type == k8s.All { + return nil, status.Errorf(codes.InvalidArgument, "resource type 'all' is not supported as a filter") + } + case *pb.StatSummaryRequest_FromResource: + if req.Outbound.(*pb.StatSummaryRequest_FromResource).FromResource.Type == k8s.All { + return nil, status.Errorf(codes.InvalidArgument, "resource type 'all' is not supported as a filter") + } } statTables := make([]*pb.StatTable, 0) diff --git a/controller/api/util/api_utils.go b/controller/api/util/api_utils.go index 14957550470e5..5d4fcdc7658a4 100644 --- a/controller/api/util/api_utils.go +++ b/controller/api/util/api_utils.go @@ -98,13 +98,9 @@ func BuildStatSummaryRequest(p StatSummaryRequestParams) (*pb.StatSummaryRequest window = p.TimeWindow } - resourceType := p.ResourceType - var err error - if resourceType != k8s.All { - resourceType, err = k8s.CanonicalKubernetesNameFromFriendlyName(p.ResourceType) - if err != nil { - return nil, err - } + resourceType, err := k8s.CanonicalKubernetesNameFromFriendlyName(p.ResourceType) + if err != nil { + return nil, err } statRequest := &pb.StatSummaryRequest{ @@ -194,12 +190,7 @@ func BuildResource(namespace string, args ...string) (pb.Resource, error) { } func buildResource(namespace string, resType string, name string) (pb.Resource, error) { - canonicalType := resType - var err error - - if canonicalType != k8s.All { - canonicalType, err = k8s.CanonicalKubernetesNameFromFriendlyName(resType) - } + canonicalType, err := k8s.CanonicalKubernetesNameFromFriendlyName(resType) if err != nil { return pb.Resource{}, err diff --git a/pkg/k8s/k8s.go b/pkg/k8s/k8s.go index 866805f259674..cab84f7d8d859 100644 --- a/pkg/k8s/k8s.go +++ b/pkg/k8s/k8s.go @@ -80,6 +80,8 @@ func CanonicalKubernetesNameFromFriendlyName(friendlyName string) (string, error return ReplicationControllers, nil case "svc", "service", "services": return Services, nil + case "all": + return All, nil } return "", fmt.Errorf("cannot find Kubernetes canonical name from friendly name [%s]", friendlyName) From 4c04863c6b0e3e2cef516562c89f4dd70427d44a Mon Sep 17 00:00:00 2001 From: Risha Mars Date: Thu, 10 May 2018 11:04:24 -0700 Subject: [PATCH 06/14] Remove special casing for single resource type --- controller/api/public/stat_summary.go | 52 +++++++++++++-------------- 1 file changed, 25 insertions(+), 27 deletions(-) diff --git a/controller/api/public/stat_summary.go b/controller/api/public/stat_summary.go index 55187cd397f6e..2c14dd904db3e 100644 --- a/controller/api/public/stat_summary.go +++ b/controller/api/public/stat_summary.go @@ -80,37 +80,35 @@ func (s *grpcServer) StatSummary(ctx context.Context, req *pb.StatSummaryRequest statTables := make([]*pb.StatTable, 0) + var resourcesToQuery []string if req.Selector.Resource.Type == k8s.All { - // request stats for all resourceTypes, in parallel - resultChan := make(chan resourceResult) - - for _, resource := range resourceTypes { - req := &pb.StatSummaryRequest{ - Selector: &pb.ResourceSelection{ - Resource: &pb.Resource{ - Type: resource, - Namespace: req.Selector.Resource.Namespace, - }, - LabelSelector: req.Selector.LabelSelector, - }, - TimeWindow: req.TimeWindow, - } + resourcesToQuery = resourceTypes + } else { + resourcesToQuery = []string{req.Selector.Resource.Type} + } - go func() { - resultChan <- s.resourceQuery(ctx, req) - }() - } + // request stats for the resourcesToQuery, in parallel + resultChan := make(chan resourceResult) - for i := 0; i < len(resourceTypes); i++ { - result := <-resultChan - if result.err != nil { - return nil, util.GRPCError(result.err) - } - statTables = append(statTables, result.res) + for _, resource := range resourcesToQuery { + statReq := &pb.StatSummaryRequest{ + Selector: &pb.ResourceSelection{ + Resource: &pb.Resource{ + Type: resource, + Namespace: req.Selector.Resource.Namespace, + }, + LabelSelector: req.Selector.LabelSelector, + }, + TimeWindow: req.TimeWindow, } - } else { - // get stats for a single resourceType - result := s.resourceQuery(ctx, req) + + go func() { + resultChan <- s.resourceQuery(ctx, statReq) + }() + } + + for i := 0; i < len(resourcesToQuery); i++ { + result := <-resultChan if result.err != nil { return nil, util.GRPCError(result.err) } From 488076c0e25f878213bdc7d44681b51ff6613190 Mon Sep 17 00:00:00 2001 From: Risha Mars Date: Thu, 10 May 2018 13:06:48 -0700 Subject: [PATCH 07/14] Make output of conduit stat all more like kubectl --- cli/cmd/stat.go | 44 ++++++++++++++++++++++++++++++++------------ pkg/k8s/k8s.go | 19 +++++++++++++++++++ 2 files changed, 51 insertions(+), 12 deletions(-) diff --git a/cli/cmd/stat.go b/cli/cmd/stat.go index 0502ee36ece2c..d7e8cc71ccfe3 100644 --- a/cli/cmd/stat.go +++ b/cli/cmd/stat.go @@ -12,6 +12,7 @@ import ( "github.com/runconduit/conduit/controller/api/util" pb "github.com/runconduit/conduit/controller/gen/public" + "github.com/runconduit/conduit/pkg/k8s" log "github.com/sirupsen/logrus" "github.com/spf13/cobra" "k8s.io/api/core/v1" @@ -118,13 +119,13 @@ func requestStatsFromAPI(client pb.ApiClient, req *pb.StatSummaryRequest) (strin return "", fmt.Errorf("StatSummary API response error: %v", e.Error) } - return renderStats(resp), nil + return renderStats(resp, req.Selector.Resource.Type), nil } -func renderStats(resp *pb.StatSummaryResponse) string { +func renderStats(resp *pb.StatSummaryResponse, resourceType string) string { var buffer bytes.Buffer w := tabwriter.NewWriter(&buffer, 0, 0, padding, ' ', tabwriter.AlignRight) - writeStatsToBuffer(resp, w) + writeStatsToBuffer(resp, resourceType, w) w.Flush() // strip left padding on the first column @@ -151,12 +152,12 @@ type row struct { var ( nameHeader = "NAME" - maxNameLength = len(nameHeader) namespaceHeader = "NAMESPACE" + maxNameLength = len(nameHeader) maxNamespaceLength = len(namespaceHeader) ) -func writeStatsToBuffer(resp *pb.StatSummaryResponse, w *tabwriter.Writer) { +func writeStatsToBuffer(resp *pb.StatSummaryResponse, reqResourceType string, w *tabwriter.Writer) { statTables := make(map[string]map[string]*row) @@ -165,6 +166,11 @@ func writeStatsToBuffer(resp *pb.StatSummaryResponse, w *tabwriter.Writer) { for _, r := range table.Rows { name := r.Resource.Name + nameWithPrefix := name + if reqResourceType == k8s.All { + nameWithPrefix = getNamePrefix(r.Resource.Type) + nameWithPrefix + } + namespace := r.Resource.Namespace key := fmt.Sprintf("%s/%s", namespace, name) resourceKey := r.Resource.Type @@ -173,8 +179,8 @@ func writeStatsToBuffer(resp *pb.StatSummaryResponse, w *tabwriter.Writer) { statTables[resourceKey] = make(map[string]*row) } - if len(name) > maxNameLength { - maxNameLength = len(name) + if len(nameWithPrefix) > maxNameLength { + maxNameLength = len(nameWithPrefix) } if len(namespace) > maxNamespaceLength { @@ -197,14 +203,18 @@ func writeStatsToBuffer(resp *pb.StatSummaryResponse, w *tabwriter.Writer) { } } - fmt.Fprint(w, "\n\n") for resourceType, stats := range statTables { - fmt.Fprintf(w, "\n%s\n\n", resourceType) - printStatTable(stats, w) + + if reqResourceType == k8s.All { + printStatTable(stats, resourceType, w) + } else { + printStatTable(stats, "", w) + } + fmt.Fprint(w, "\n") } } -func printStatTable(stats map[string]*row, w *tabwriter.Writer) { +func printStatTable(stats map[string]*row, resourceType string, w *tabwriter.Writer) { if len(stats) == 0 { fmt.Fprintln(os.Stderr, "No traffic found.") os.Exit(0) @@ -227,11 +237,13 @@ func printStatTable(stats map[string]*row, w *tabwriter.Writer) { fmt.Fprintln(w, strings.Join(headers, "\t")) + namePrefix := getNamePrefix(resourceType) + sortedKeys := sortStatsKeys(stats) for _, key := range sortedKeys { parts := strings.Split(key, "/") namespace := parts[0] - name := parts[1] + name := namePrefix + parts[1] values := make([]interface{}, 0) templateString := "%s\t%s\t%.2f%%\t%.1frps\t%dms\t%dms\t%dms\t\n" templateStringEmpty := "%s\t%s\t-\t-\t-\t-\t-\t\n" @@ -263,6 +275,14 @@ func printStatTable(stats map[string]*row, w *tabwriter.Writer) { } } +func getNamePrefix(resourceType string) string { + if resourceType == "" { + return "" + } else { + return k8s.ShortNameFromCanonicalKubernetesName(resourceType) + "/" + } +} + func buildStatSummaryRequest( timeWindow string, allNamespaces bool, resource []string, namespace string, diff --git a/pkg/k8s/k8s.go b/pkg/k8s/k8s.go index cab84f7d8d859..3a0640534e78b 100644 --- a/pkg/k8s/k8s.go +++ b/pkg/k8s/k8s.go @@ -86,3 +86,22 @@ func CanonicalKubernetesNameFromFriendlyName(friendlyName string) (string, error return "", fmt.Errorf("cannot find Kubernetes canonical name from friendly name [%s]", friendlyName) } + +// Return a the shortest name for a k8s canonical name. +// Essentially the reverse of CanonicalKubernetesNameFromFriendlyName +func ShortNameFromCanonicalKubernetesName(canonicalName string) string { + switch canonicalName { + case Deployments: + return "deploy" + case Namespaces: + return "ns" + case Pods: + return "po" + case ReplicationControllers: + return "rc" + case Services: + return "svc" + default: + return "" + } +} From 6d57f003924fa0ea6bd449263fc7c430d209241b Mon Sep 17 00:00:00 2001 From: Risha Mars Date: Thu, 10 May 2018 13:10:10 -0700 Subject: [PATCH 08/14] move maxNameLength and maxNamespace length to not be global variables --- cli/cmd/stat.go | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/cli/cmd/stat.go b/cli/cmd/stat.go index d7e8cc71ccfe3..0fbdf29bd6fcb 100644 --- a/cli/cmd/stat.go +++ b/cli/cmd/stat.go @@ -151,14 +151,13 @@ type row struct { } var ( - nameHeader = "NAME" - namespaceHeader = "NAMESPACE" - maxNameLength = len(nameHeader) - maxNamespaceLength = len(namespaceHeader) + nameHeader = "NAME" + namespaceHeader = "NAMESPACE" ) func writeStatsToBuffer(resp *pb.StatSummaryResponse, reqResourceType string, w *tabwriter.Writer) { - + maxNameLength := len(nameHeader) + maxNamespaceLength := len(namespaceHeader) statTables := make(map[string]map[string]*row) for _, statTable := range resp.GetOk().StatTables { @@ -206,15 +205,15 @@ func writeStatsToBuffer(resp *pb.StatSummaryResponse, reqResourceType string, w for resourceType, stats := range statTables { if reqResourceType == k8s.All { - printStatTable(stats, resourceType, w) + printStatTable(stats, resourceType, w, maxNameLength, maxNamespaceLength) } else { - printStatTable(stats, "", w) + printStatTable(stats, "", w, maxNameLength, maxNamespaceLength) } fmt.Fprint(w, "\n") } } -func printStatTable(stats map[string]*row, resourceType string, w *tabwriter.Writer) { +func printStatTable(stats map[string]*row, resourceType string, w *tabwriter.Writer, maxNameLength int, maxNamespaceLength int) { if len(stats) == 0 { fmt.Fprintln(os.Stderr, "No traffic found.") os.Exit(0) From bd6b2a7275a677040da0510e56ef0ecf54ac728a Mon Sep 17 00:00:00 2001 From: Risha Mars Date: Thu, 10 May 2018 16:48:26 -0700 Subject: [PATCH 09/14] Add a test for StatSummary with resourceType all --- controller/api/public/stat_summary_test.go | 231 ++++++++++++++++++--- 1 file changed, 198 insertions(+), 33 deletions(-) diff --git a/controller/api/public/stat_summary_test.go b/controller/api/public/stat_summary_test.go index 59a46e98471c7..654dd5c3481d7 100644 --- a/controller/api/public/stat_summary_test.go +++ b/controller/api/public/stat_summary_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "reflect" + "sort" "testing" "github.com/prometheus/common/model" @@ -24,6 +25,68 @@ type statSumExpected struct { res pb.StatSummaryResponse } +func testStatSummary(t *testing.T, expectations []statSumExpected) { + for _, exp := range expectations { + k8sObjs := []runtime.Object{} + for _, res := range exp.k8sRes { + decode := scheme.Codecs.UniversalDeserializer().Decode + obj, _, err := decode([]byte(res), nil, nil) + if err != nil { + t.Fatalf("could not decode yml: %s", err) + } + k8sObjs = append(k8sObjs, obj) + } + + clientSet := fake.NewSimpleClientset(k8sObjs...) + lister := k8s.NewLister(clientSet) + + fakeGrpcServer := newGrpcServer( + &MockProm{Res: exp.promRes}, + tap.NewTapClient(nil), + lister, + "conduit", + []string{}, + ) + err := lister.Sync() + if err != nil { + t.Fatalf("timed out wait for caches to sync") + } + + rsp, err := fakeGrpcServer.StatSummary(context.TODO(), &exp.req) + if err != exp.err { + t.Fatalf("Expected error: %s, Got: %s", exp.err, err) + } + + unsortedStatTables := rsp.GetOk().StatTables + sort.Sort(byStatResult(unsortedStatTables)) + + if !reflect.DeepEqual(exp.res.GetOk().StatTables, unsortedStatTables) { + t.Fatalf("Expected: %+v, Got: %+v", &exp.res, rsp) + } + } +} + +type byStatResult []*pb.StatTable + +func (s byStatResult) Len() int { + return len(s) +} + +func (s byStatResult) Swap(i, j int) { + s[i], s[j] = s[j], s[i] +} + +func (s byStatResult) Less(i, j int) bool { + if len(s[i].GetPodGroup().Rows) == 0 { + return true + } + if len(s[j].GetPodGroup().Rows) == 0 { + return false + } + + return s[i].GetPodGroup().Rows[0].Resource.Type < s[j].GetPodGroup().Rows[0].Resource.Type +} + func TestStatSummary(t *testing.T) { t.Run("Successfully performs a query based on resource type", func(t *testing.T) { expectations := []statSumExpected{ @@ -136,41 +199,143 @@ status: }, } - for _, exp := range expectations { - k8sObjs := []runtime.Object{} - for _, res := range exp.k8sRes { - decode := scheme.Codecs.UniversalDeserializer().Decode - obj, _, err := decode([]byte(res), nil, nil) - if err != nil { - t.Fatalf("could not decode yml: %s", err) - } - k8sObjs = append(k8sObjs, obj) - } - - clientSet := fake.NewSimpleClientset(k8sObjs...) - lister := k8s.NewLister(clientSet) - - fakeGrpcServer := newGrpcServer( - &MockProm{Res: exp.promRes}, - tap.NewTapClient(nil), - lister, - "conduit", - []string{}, - ) - err := lister.Sync() - if err != nil { - t.Fatalf("timed out wait for caches to sync") - } - - rsp, err := fakeGrpcServer.StatSummary(context.TODO(), &exp.req) - if err != exp.err { - t.Fatalf("Expected error: %s, Got: %s", exp.err, err) - } + testStatSummary(t, expectations) + }) - if !reflect.DeepEqual(exp.res, *rsp) { - t.Fatalf("Expected: %+v, Got: %+v", &exp.res, rsp) - } + t.Run("Successfully queries for resource type 'all'", func(t *testing.T) { + expectations := []statSumExpected{ + statSumExpected{ + err: nil, + k8sRes: []string{` +apiVersion: apps/v1beta2 +kind: Deployment +metadata: + name: emoji-deploy + namespace: emojivoto +spec: + selector: + matchLabels: + app: emoji-svc + strategy: {} + template: + spec: + containers: + - image: buoyantio/emojivoto-emoji-svc:v3 +`, ` +apiVersion: v1 +kind: Pod +metadata: + name: emojivoto-pod-1 + namespace: not-right-emojivoto-namespace + labels: + app: emoji-svc + annotations: + conduit.io/proxy-version: testinjectversion +status: + phase: Running +`, ` +apiVersion: v1 +kind: Pod +metadata: + name: emojivoto-pod-2 + namespace: emojivoto + labels: + app: emoji-svc + annotations: + conduit.io/proxy-version: testinjectversion +status: + phase: Running +`, + }, + promRes: model.Vector{ + &model.Sample{ + Metric: model.Metric{ + "deployment": "emoji-deploy", + "namespace": "emojivoto", + "classification": "success", + }, + Value: 123, + Timestamp: 456, + }, + }, + req: pb.StatSummaryRequest{ + Selector: &pb.ResourceSelection{ + Resource: &pb.Resource{ + Namespace: "emojivoto", + Type: pkgK8s.All, + }, + }, + TimeWindow: "1m", + }, + res: pb.StatSummaryResponse{ + Response: &pb.StatSummaryResponse_Ok_{ // https://github.com/golang/protobuf/issues/205 + Ok: &pb.StatSummaryResponse_Ok{ + StatTables: []*pb.StatTable{ + &pb.StatTable{ + Table: &pb.StatTable_PodGroup_{ + PodGroup: &pb.StatTable_PodGroup{ + Rows: []*pb.StatTable_PodGroup_Row{}, + }, + }, + }, + &pb.StatTable{ + Table: &pb.StatTable_PodGroup_{ + PodGroup: &pb.StatTable_PodGroup{ + Rows: []*pb.StatTable_PodGroup_Row{}, + }, + }, + }, + &pb.StatTable{ + Table: &pb.StatTable_PodGroup_{ + PodGroup: &pb.StatTable_PodGroup{ + Rows: []*pb.StatTable_PodGroup_Row{ + &pb.StatTable_PodGroup_Row{ + Resource: &pb.Resource{ + Namespace: "emojivoto", + Type: "deployments", + Name: "emoji-deploy", + }, + Stats: &pb.BasicStats{ + SuccessCount: 123, + FailureCount: 0, + LatencyMsP50: 123, + LatencyMsP95: 123, + LatencyMsP99: 123, + }, + TimeWindow: "1m", + MeshedPodCount: 1, + RunningPodCount: 1, + }, + }, + }, + }, + }, + &pb.StatTable{ + Table: &pb.StatTable_PodGroup_{ + PodGroup: &pb.StatTable_PodGroup{ + Rows: []*pb.StatTable_PodGroup_Row{ + &pb.StatTable_PodGroup_Row{ + Resource: &pb.Resource{ + Namespace: "emojivoto", + Type: "pods", + Name: "emojivoto-pod-2", + }, + TimeWindow: "1m", + MeshedPodCount: 1, + RunningPodCount: 1, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, } + + testStatSummary(t, expectations) }) t.Run("Given an invalid resource type, returns error", func(t *testing.T) { From b135d38071900730e839fd54a3d260d85a86a779 Mon Sep 17 00:00:00 2001 From: Risha Mars Date: Thu, 10 May 2018 16:56:55 -0700 Subject: [PATCH 10/14] Add Service to test for all resources --- controller/api/public/stat_summary_test.go | 35 +++++++++++++++++----- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/controller/api/public/stat_summary_test.go b/controller/api/public/stat_summary_test.go index 654dd5c3481d7..e717665a9b63c 100644 --- a/controller/api/public/stat_summary_test.go +++ b/controller/api/public/stat_summary_test.go @@ -223,6 +223,16 @@ spec: - image: buoyantio/emojivoto-emoji-svc:v3 `, ` apiVersion: v1 +kind: Service +metadata: + name: emoji-svc + namespace: emojivoto +spec: + clusterIP: None + selector: + app: emoji-svc +`, ` +apiVersion: v1 kind: Pod metadata: name: emojivoto-pod-1 @@ -278,13 +288,6 @@ status: }, }, }, - &pb.StatTable{ - Table: &pb.StatTable_PodGroup_{ - PodGroup: &pb.StatTable_PodGroup{ - Rows: []*pb.StatTable_PodGroup_Row{}, - }, - }, - }, &pb.StatTable{ Table: &pb.StatTable_PodGroup_{ PodGroup: &pb.StatTable_PodGroup{ @@ -328,6 +331,24 @@ status: }, }, }, + &pb.StatTable{ + Table: &pb.StatTable_PodGroup_{ + PodGroup: &pb.StatTable_PodGroup{ + Rows: []*pb.StatTable_PodGroup_Row{ + &pb.StatTable_PodGroup_Row{ + Resource: &pb.Resource{ + Namespace: "emojivoto", + Type: "services", + Name: "emoji-svc", + }, + TimeWindow: "1m", + MeshedPodCount: 1, + RunningPodCount: 1, + }, + }, + }, + }, + }, }, }, }, From 9a7ad0c0d524afc5b82fa3d9462cdc7c768f5c21 Mon Sep 17 00:00:00 2001 From: Risha Mars Date: Fri, 11 May 2018 10:29:04 -0700 Subject: [PATCH 11/14] Move len(stat) check, fix trailing newline --- cli/cmd/stat.go | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/cli/cmd/stat.go b/cli/cmd/stat.go index 0fbdf29bd6fcb..2cf28b9fcb11b 100644 --- a/cli/cmd/stat.go +++ b/cli/cmd/stat.go @@ -202,23 +202,28 @@ func writeStatsToBuffer(resp *pb.StatSummaryResponse, reqResourceType string, w } } + if len(statTables) == 0 { + fmt.Fprintln(os.Stderr, "No traffic found.") + os.Exit(0) + } + + lastDisplayedStat := true // don't print a newline after the final stat for resourceType, stats := range statTables { + if !lastDisplayedStat { + fmt.Fprint(w, "\n") + } + lastDisplayedStat = false if reqResourceType == k8s.All { printStatTable(stats, resourceType, w, maxNameLength, maxNamespaceLength) } else { printStatTable(stats, "", w, maxNameLength, maxNamespaceLength) } - fmt.Fprint(w, "\n") + } } func printStatTable(stats map[string]*row, resourceType string, w *tabwriter.Writer, maxNameLength int, maxNamespaceLength int) { - if len(stats) == 0 { - fmt.Fprintln(os.Stderr, "No traffic found.") - os.Exit(0) - } - headers := make([]string, 0) if allNamespaces { headers = append(headers, From 2dd7938e8562cbab9b6d7205d3341736a356b0da Mon Sep 17 00:00:00 2001 From: Risha Mars Date: Fri, 11 May 2018 11:10:09 -0700 Subject: [PATCH 12/14] Merge master, resolve conflicts --- controller/api/public/stat_summary.go | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/controller/api/public/stat_summary.go b/controller/api/public/stat_summary.go index 2c14dd904db3e..18689c8193494 100644 --- a/controller/api/public/stat_summary.go +++ b/controller/api/public/stat_summary.go @@ -58,23 +58,17 @@ type podCount struct { func (s *grpcServer) StatSummary(ctx context.Context, req *pb.StatSummaryRequest) (*pb.StatSummaryResponse, error) { // special case to check for services as outbound only if isInvalidServiceRequest(req) { - return &pb.StatSummaryResponse{ - Response: &pb.StatSummaryResponse_Error{ - Error: &pb.ResourceError{ - Resource: req.Selector.Resource, - Error: "service only supported as a target on 'from' queries, or as a destination on 'to' queries", - }, - }, - }, nil + return statSummaryError(req, "service only supported as a target on 'from' queries, or as a destination on 'to' queries"), nil + } switch req.Outbound.(type) { case *pb.StatSummaryRequest_ToResource: if req.Outbound.(*pb.StatSummaryRequest_ToResource).ToResource.Type == k8s.All { - return nil, status.Errorf(codes.InvalidArgument, "resource type 'all' is not supported as a filter") + return statSummaryError(req, "resource type 'all' is not supported as a filter"), nil } case *pb.StatSummaryRequest_FromResource: if req.Outbound.(*pb.StatSummaryRequest_FromResource).FromResource.Type == k8s.All { - return nil, status.Errorf(codes.InvalidArgument, "resource type 'all' is not supported as a filter") + return statSummaryError(req, "resource type 'all' is not supported as a filter"), nil } } @@ -126,6 +120,17 @@ func (s *grpcServer) StatSummary(ctx context.Context, req *pb.StatSummaryRequest return &rsp, nil } +func statSummaryError(req *pb.StatSummaryRequest, message string) *pb.StatSummaryResponse { + return &pb.StatSummaryResponse{ + Response: &pb.StatSummaryResponse_Error{ + Error: &pb.ResourceError{ + Resource: req.Selector.Resource, + Error: message, + }, + }, + } +} + func (s *grpcServer) resourceQuery(ctx context.Context, req *pb.StatSummaryRequest) resourceResult { objects, err := s.lister.GetObjects(req.Selector.Resource.Namespace, req.Selector.Resource.Type, req.Selector.Resource.Name) if err != nil { From 4eca3cc0e2c2d2e88e15054fae247351ac301a75 Mon Sep 17 00:00:00 2001 From: Risha Mars Date: Fri, 11 May 2018 12:50:18 -0700 Subject: [PATCH 13/14] Add 'all' to CLI help text --- cli/cmd/stat.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cli/cmd/stat.go b/cli/cmd/stat.go index 2cf28b9fcb11b..c72dce52e8381 100644 --- a/cli/cmd/stat.go +++ b/cli/cmd/stat.go @@ -39,6 +39,7 @@ var statCmd = &cobra.Command{ * deploy/my-deploy * deploy my-deploy * ns/my-ns + * all Valid resource types include: @@ -47,6 +48,7 @@ Valid resource types include: * pods * replicationcontrollers * services (only supported if a "--from" is also specified, or as a "--to") + * all (all resource types, not supported in --from or --to) This command will hide resources that have completed, such as pods that are in the Succeeded or Failed phases. If no resource name is specified, displays stats about all resources of the specified RESOURCETYPE`, From 5a842b13322d0ee12af92f558e9f4c578facb440 Mon Sep 17 00:00:00 2001 From: Risha Mars Date: Fri, 11 May 2018 13:13:29 -0700 Subject: [PATCH 14/14] whitespace cleanups --- cli/cmd/stat.go | 2 -- controller/api/util/api_utils.go | 1 - 2 files changed, 3 deletions(-) diff --git a/cli/cmd/stat.go b/cli/cmd/stat.go index c72dce52e8381..3917e71071094 100644 --- a/cli/cmd/stat.go +++ b/cli/cmd/stat.go @@ -212,7 +212,6 @@ func writeStatsToBuffer(resp *pb.StatSummaryResponse, reqResourceType string, w lastDisplayedStat := true // don't print a newline after the final stat for resourceType, stats := range statTables { if !lastDisplayedStat { - fmt.Fprint(w, "\n") } lastDisplayedStat = false @@ -221,7 +220,6 @@ func writeStatsToBuffer(resp *pb.StatSummaryResponse, reqResourceType string, w } else { printStatTable(stats, "", w, maxNameLength, maxNamespaceLength) } - } } diff --git a/controller/api/util/api_utils.go b/controller/api/util/api_utils.go index 5d4fcdc7658a4..1f19a1f4f0833 100644 --- a/controller/api/util/api_utils.go +++ b/controller/api/util/api_utils.go @@ -191,7 +191,6 @@ func BuildResource(namespace string, args ...string) (pb.Resource, error) { func buildResource(namespace string, resType string, name string) (pb.Resource, error) { canonicalType, err := k8s.CanonicalKubernetesNameFromFriendlyName(resType) - if err != nil { return pb.Resource{}, err }