diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/top/BUILD b/staging/src/k8s.io/kubectl/pkg/cmd/top/BUILD index 2f37ab19644..fe5eabc2e25 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/top/BUILD +++ b/staging/src/k8s.io/kubectl/pkg/cmd/top/BUILD @@ -47,7 +47,6 @@ go_test( "//staging/src/k8s.io/client-go/rest/fake:go_default_library", "//staging/src/k8s.io/client-go/testing:go_default_library", "//staging/src/k8s.io/kubectl/pkg/cmd/testing:go_default_library", - "//staging/src/k8s.io/kubectl/pkg/cmd/util:go_default_library", "//staging/src/k8s.io/kubectl/pkg/scheme:go_default_library", "//staging/src/k8s.io/metrics/pkg/apis/metrics/v1alpha1:go_default_library", "//staging/src/k8s.io/metrics/pkg/apis/metrics/v1beta1:go_default_library", diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/top/top_pod_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/top/top_pod_test.go index 05671c5e86b..d8b575cc79f 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/top/top_pod_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/top/top_pod_test.go @@ -34,7 +34,6 @@ import ( "k8s.io/client-go/rest/fake" core "k8s.io/client-go/testing" cmdtesting "k8s.io/kubectl/pkg/cmd/testing" - cmdutil "k8s.io/kubectl/pkg/cmd/util" "k8s.io/kubectl/pkg/scheme" metricsv1alpha1api "k8s.io/metrics/pkg/apis/metrics/v1alpha1" metricsv1beta1api "k8s.io/metrics/pkg/apis/metrics/v1beta1" @@ -80,15 +79,16 @@ const ( func TestTopPod(t *testing.T) { testNS := "testns" testCases := []struct { - name string - namespace string - options *TopPodOptions - args []string - expectedQuery string - expectedPods []string - namespaces []string - containers bool - listsNamespaces bool + name string + namespace string + options *TopPodOptions + args []string + expectedQuery string + expectedPods []string + expectedContainers []string + namespaces []string + containers bool + listsNamespaces bool }{ { name: "all namespaces", @@ -112,24 +112,56 @@ func TestTopPod(t *testing.T) { namespaces: []string{testNS, testNS}, }, { - name: "pod with container metrics", - options: &TopPodOptions{PrintContainers: true}, - args: []string{"pod1"}, + name: "pod with container metrics", + options: &TopPodOptions{PrintContainers: true}, + args: []string{"pod1"}, + expectedContainers: []string{ + "container1-1", + "container1-2", + }, namespaces: []string{testNS}, containers: true, }, { - name: "pod with label sort by cpu", + name: "pod sort by cpu", options: &TopPodOptions{SortBy: "cpu"}, expectedPods: []string{"pod2", "pod3", "pod1"}, namespaces: []string{testNS, testNS, testNS}, }, { - name: "pod with label sort by memory", + name: "pod sort by memory", options: &TopPodOptions{SortBy: "memory"}, expectedPods: []string{"pod2", "pod3", "pod1"}, namespaces: []string{testNS, testNS, testNS}, }, + { + name: "container sort by cpu", + options: &TopPodOptions{PrintContainers: true, SortBy: "cpu"}, + expectedContainers: []string{ + "container2-3", + "container2-2", + "container2-1", + "container3-1", + "container1-2", + "container1-1", + }, + namespaces: []string{testNS, testNS, testNS}, + containers: true, + }, + { + name: "container sort by memory", + options: &TopPodOptions{PrintContainers: true, SortBy: "memory"}, + expectedContainers: []string{ + "container2-3", + "container2-2", + "container2-1", + "container3-1", + "container1-2", + "container1-1", + }, + namespaces: []string{testNS, testNS, testNS}, + containers: true, + }, } cmdtesting.InitTestErrorHandler(t) for _, testCase := range testCases { @@ -234,23 +266,34 @@ func TestTopPod(t *testing.T) { t.Errorf("unexpected metrics for %s: \n%s", name, result) } } - if cmdutil.GetFlagString(cmd, "sort-by") == "cpu" || cmdutil.GetFlagString(cmd, "sort-by") == "memory" { - resultLines := strings.Split(result, "\n") - resultPods := make([]string, len(resultLines)-2) // don't process first (header) and last (empty) line - - for i, line := range resultLines[1 : len(resultLines)-1] { // don't process first (header) and last (empty) line - lineFirstColumn := strings.Split(line, " ")[0] - resultPods[i] = lineFirstColumn - } - + if testCase.expectedPods != nil { + resultPods := getResultColumnValues(result, 0) if !reflect.DeepEqual(testCase.expectedPods, resultPods) { - t.Errorf("kinds not matching:\n\texpectedKinds: %v\n\tgotKinds: %v\n", testCase.expectedPods, resultPods) + t.Errorf("pods not matching:\n\texpectedPods: %v\n\tresultPods: %v\n", testCase.expectedPods, resultPods) + } + } + if testCase.expectedContainers != nil { + resultContainers := getResultColumnValues(result, 1) + if !reflect.DeepEqual(testCase.expectedContainers, resultContainers) { + t.Errorf("containers not matching:\n\texpectedContainers: %v\n\tresultContainers: %v\n", testCase.expectedContainers, resultContainers) } } }) } } +func getResultColumnValues(result string, columnIndex int) []string { + resultLines := strings.Split(result, "\n") + values := make([]string, len(resultLines)-2) // don't process first (header) and last (empty) line + + for i, line := range resultLines[1 : len(resultLines)-1] { // don't process first (header) and last (empty) line + value := strings.Fields(line)[columnIndex] + values[i] = value + } + + return values +} + func TestTopPodNoResourcesFound(t *testing.T) { testNS := "testns" testCases := []struct { diff --git a/staging/src/k8s.io/kubectl/pkg/metricsutil/metrics_printer.go b/staging/src/k8s.io/kubectl/pkg/metricsutil/metrics_printer.go index a68289dc005..f99046bd982 100644 --- a/staging/src/k8s.io/kubectl/pkg/metricsutil/metrics_printer.go +++ b/staging/src/k8s.io/kubectl/pkg/metricsutil/metrics_printer.go @@ -55,7 +55,6 @@ func NewTopCmdPrinter(out io.Writer) *TopCmdPrinter { type NodeMetricsSorter struct { metrics []metricsapi.NodeMetrics sortBy string - usages []v1.ResourceList } func (n *NodeMetricsSorter) Len() int { @@ -64,37 +63,24 @@ func (n *NodeMetricsSorter) Len() int { func (n *NodeMetricsSorter) Swap(i, j int) { n.metrics[i], n.metrics[j] = n.metrics[j], n.metrics[i] - n.usages[i], n.usages[j] = n.usages[j], n.usages[i] } func (n *NodeMetricsSorter) Less(i, j int) bool { switch n.sortBy { case "cpu": - qi := n.usages[i][v1.ResourceCPU] - qj := n.usages[j][v1.ResourceCPU] - return qi.MilliValue() > qj.MilliValue() + return n.metrics[i].Usage.Cpu().MilliValue() > n.metrics[j].Usage.Cpu().MilliValue() case "memory": - qi := n.usages[i][v1.ResourceMemory] - qj := n.usages[j][v1.ResourceMemory] - return qi.Value() > qj.Value() + return n.metrics[i].Usage.Memory().Value() > n.metrics[j].Usage.Memory().Value() default: return n.metrics[i].Name < n.metrics[j].Name } } -func NewNodeMetricsSorter(metrics []metricsapi.NodeMetrics, sortBy string) (*NodeMetricsSorter, error) { - var usages = make([]v1.ResourceList, len(metrics)) - if len(sortBy) > 0 { - for i, v := range metrics { - v.Usage.DeepCopyInto(&usages[i]) - } - } - +func NewNodeMetricsSorter(metrics []metricsapi.NodeMetrics, sortBy string) *NodeMetricsSorter { return &NodeMetricsSorter{ metrics: metrics, sortBy: sortBy, - usages: usages, - }, nil + } } type PodMetricsSorter struct { @@ -116,13 +102,9 @@ func (p *PodMetricsSorter) Swap(i, j int) { func (p *PodMetricsSorter) Less(i, j int) bool { switch p.sortBy { case "cpu": - qi := p.podMetrics[i][v1.ResourceCPU] - qj := p.podMetrics[j][v1.ResourceCPU] - return qi.MilliValue() > qj.MilliValue() + return p.podMetrics[i].Cpu().MilliValue() > p.podMetrics[j].Cpu().MilliValue() case "memory": - qi := p.podMetrics[i][v1.ResourceMemory] - qj := p.podMetrics[j][v1.ResourceMemory] - return qi.Value() > qj.Value() + return p.podMetrics[i].Memory().Value() > p.podMetrics[j].Memory().Value() default: if p.withNamespace && p.metrics[i].Namespace != p.metrics[j].Namespace { return p.metrics[i].Namespace < p.metrics[j].Namespace @@ -131,11 +113,11 @@ func (p *PodMetricsSorter) Less(i, j int) bool { } } -func NewPodMetricsSorter(metrics []metricsapi.PodMetrics, printContainers bool, withNamespace bool, sortBy string) (*PodMetricsSorter, error) { +func NewPodMetricsSorter(metrics []metricsapi.PodMetrics, withNamespace bool, sortBy string) *PodMetricsSorter { var podMetrics = make([]v1.ResourceList, len(metrics)) if len(sortBy) > 0 { for i, v := range metrics { - podMetrics[i], _, _ = getPodMetrics(&v, printContainers) + podMetrics[i] = getPodMetrics(&v) } } @@ -144,7 +126,38 @@ func NewPodMetricsSorter(metrics []metricsapi.PodMetrics, printContainers bool, sortBy: sortBy, withNamespace: withNamespace, podMetrics: podMetrics, - }, nil + } +} + +type ContainerMetricsSorter struct { + metrics []metricsapi.ContainerMetrics + sortBy string +} + +func (s *ContainerMetricsSorter) Len() int { + return len(s.metrics) +} + +func (s *ContainerMetricsSorter) Swap(i, j int) { + s.metrics[i], s.metrics[j] = s.metrics[j], s.metrics[i] +} + +func (s *ContainerMetricsSorter) Less(i, j int) bool { + switch s.sortBy { + case "cpu": + return s.metrics[i].Usage.Cpu().MilliValue() > s.metrics[j].Usage.Cpu().MilliValue() + case "memory": + return s.metrics[i].Usage.Memory().Value() > s.metrics[j].Usage.Memory().Value() + default: + return s.metrics[i].Name < s.metrics[j].Name + } +} + +func NewContainerMetricsSorter(metrics []metricsapi.ContainerMetrics, sortBy string) *ContainerMetricsSorter { + return &ContainerMetricsSorter{ + metrics: metrics, + sortBy: sortBy, + } } func (printer *TopCmdPrinter) PrintNodeMetrics(metrics []metricsapi.NodeMetrics, availableResources map[string]v1.ResourceList, noHeaders bool, sortBy string) error { @@ -154,11 +167,7 @@ func (printer *TopCmdPrinter) PrintNodeMetrics(metrics []metricsapi.NodeMetrics, w := printers.GetNewTabWriter(printer.out) defer w.Flush() - n, err := NewNodeMetricsSorter(metrics, sortBy) - if err != nil { - return err - } - sort.Sort(n) + sort.Sort(NewNodeMetricsSorter(metrics, sortBy)) if !noHeaders { printColumnNames(w, NodeColumns) @@ -197,16 +206,14 @@ func (printer *TopCmdPrinter) PrintPodMetrics(metrics []metricsapi.PodMetrics, p printColumnNames(w, PodColumns) } - p, err := NewPodMetricsSorter(metrics, printContainers, withNamespace, sortBy) - if err != nil { - return err - } - sort.Sort(p) + sort.Sort(NewPodMetricsSorter(metrics, withNamespace, sortBy)) for _, m := range metrics { - err := printSinglePodMetrics(w, &m, printContainers, withNamespace) - if err != nil { - return err + if printContainers { + sort.Sort(NewContainerMetricsSorter(m.Containers, sortBy)) + printSinglePodContainerMetrics(w, &m, withNamespace) + } else { + printSinglePodMetrics(w, &m, withNamespace) } } return nil @@ -219,56 +226,46 @@ func printColumnNames(out io.Writer, names []string) { fmt.Fprint(out, "\n") } -func printSinglePodMetrics(out io.Writer, m *metricsapi.PodMetrics, printContainersOnly bool, withNamespace bool) error { - podMetrics, containers, err := getPodMetrics(m, printContainersOnly) - if err != nil { - return err +func printSinglePodMetrics(out io.Writer, m *metricsapi.PodMetrics, withNamespace bool) { + podMetrics := getPodMetrics(m) + if withNamespace { + printValue(out, m.Namespace) } - if printContainersOnly { - for contName := range containers { - if withNamespace { - printValue(out, m.Namespace) - } - printValue(out, m.Name) - printMetricsLine(out, &ResourceMetricsInfo{ - Name: contName, - Metrics: containers[contName], - Available: v1.ResourceList{}, - }) - } - } else { + printMetricsLine(out, &ResourceMetricsInfo{ + Name: m.Name, + Metrics: podMetrics, + Available: v1.ResourceList{}, + }) +} + +func printSinglePodContainerMetrics(out io.Writer, m *metricsapi.PodMetrics, withNamespace bool) { + for _, c := range m.Containers { if withNamespace { printValue(out, m.Namespace) } + printValue(out, m.Name) printMetricsLine(out, &ResourceMetricsInfo{ - Name: m.Name, - Metrics: podMetrics, + Name: c.Name, + Metrics: c.Usage, Available: v1.ResourceList{}, }) } - return nil } -func getPodMetrics(m *metricsapi.PodMetrics, printContainersOnly bool) (v1.ResourceList, map[string]v1.ResourceList, error) { - containers := make(map[string]v1.ResourceList) +func getPodMetrics(m *metricsapi.PodMetrics) v1.ResourceList { podMetrics := make(v1.ResourceList) for _, res := range MeasuredResources { podMetrics[res], _ = resource.ParseQuantity("0") } - var usage v1.ResourceList for _, c := range m.Containers { - c.Usage.DeepCopyInto(&usage) - containers[c.Name] = usage - if !printContainersOnly { - for _, res := range MeasuredResources { - quantity := podMetrics[res] - quantity.Add(usage[res]) - podMetrics[res] = quantity - } + for _, res := range MeasuredResources { + quantity := podMetrics[res] + quantity.Add(c.Usage[res]) + podMetrics[res] = quantity } } - return podMetrics, containers, nil + return podMetrics } func printMetricsLine(out io.Writer, metrics *ResourceMetricsInfo) {