Sort kubectl top output when --sort-by and --containers are used together

- Changed kubectl top to sort first at the pod level and then at the container level when --sort-by and --containers are used together.
- Refactored printSinglePodMetrics into two separate functions instead of passing in bool to change behavior.
- Refactored MetricsSorters to simplify code.
- Added unit tests to test container sorting.
- Fixed pod sorting unit tests which were not working because it was checking for --sort-by command line flag which was never true.

Kubernetes-commit: 04266b37ded103ddb84a192ec816499904fce1d1
This commit is contained in:
brianpursley 2020-08-06 11:47:09 -04:00 committed by Kubernetes Publisher
parent 00e703ea95
commit f9c04a0266
2 changed files with 137 additions and 97 deletions

View File

@ -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 {

View File

@ -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) {