Added --sum flag to kubectl top pod #105100

lauchokyip · 2021-09-17T13:26:36Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

 $ ./kubectl top pods --sum -A
NAMESPACE        NAME                                          CPU(cores)   MEMORY(bytes)   
cert-manager     cert-manager-7998c69865-m5pjn                 11m          14Mi            
cert-manager     cert-manager-cainjector-7b744d56fb-jd66q      12m          19Mi            
cert-manager     cert-manager-webhook-97f8b47bc-4smlq          14m          8Mi             
default          personal-website-deployment-b6699dcfb-bl6cp   0m           2Mi             
default          personal-website-deployment-b6699dcfb-rnv7t   0m           2Mi             
default          personal-website-deployment-b6699dcfb-z5lm2   0m           2Mi             
gallery          gallery-deployment-8db9454f9-6rlkk            1m           8Mi             
kube-system      coredns-7448499f4d-5p4ht                      9m           9Mi             
kube-system      local-path-provisioner-5ff76fc89d-8qct7       4m           5Mi             
kube-system      metrics-server-6995b8fd7d-hczx7               16m          12Mi            
kube-system      nginx-ingress-controller-699bf59c5-j6dfw      6m           74Mi            
metallb-system   metallb-controller-df647b67b-q84kg            1m           3Mi             
metallb-system   metallb-speaker-6sk9z                         2m           6Mi             
metallb-system   metallb-speaker-qxbrs                         3m           4Mi             
web-analytics    mysql-bb9597d6f-5czf9                         2m           64Mi            
web-analytics    web-analytics-deployment-75d65dd4f8-5zrwq     1m           31Mi            
web-analytics    web-analytics-deployment-75d65dd4f8-fv2s2     1m           29Mi            
web-analytics    web-analytics-deployment-75d65dd4f8-xs4l5     1m           32Mi            
                                                               ________     ________        
                                                               74m          333Mi

Does this PR introduce a user-facing change?

Added sum feature to `kubectl top pod`

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

lauchokyip · 2021-09-17T13:28:36Z

staging/src/k8s.io/kubectl/pkg/metricsutil/metrics_printer.go

@@ -52,114 +52,6 @@ func NewTopCmdPrinter(out io.Writer) *TopCmdPrinter {
 	return &TopCmdPrinter{out: out}
 }

-type NodeMetricsSorter struct {


Move sorter out of metrics_printer to metrics_sorter because it clutters the printer file

lauchokyip · 2021-09-17T13:30:00Z

staging/src/k8s.io/kubectl/pkg/metricsutil/metrics_printer.go

 	if len(metrics) == 0 {
 		return nil
 	}
 	w := printers.GetNewTabWriter(printer.out)
 	defer w.Flush()
 	if !noHeaders {
 		if withNamespace {
-			printValue(w, NamespaceColumn)
+			PodColumns = append([]string{NamespaceColumn}, PodColumns...)


Used append instead so we can know how many columns to skip

// skipping len(PodCoumns)-2 columns for i := 0; i < len(PodColumns)-2; i++ { printValue(out, "") }

lauchokyip · 2021-09-17T13:30:27Z

/assign @eddiezane

lauchokyip · 2021-09-17T13:32:12Z

/triage accepted
/priority backlog

lauchokyip · 2021-09-20T20:24:08Z

/retest

eddiezane · 2021-09-29T21:42:09Z

staging/src/k8s.io/kubectl/pkg/metricsutil/metrics_printer.go

 	if len(metrics) == 0 {
 		return nil
 	}
 	w := printers.GetNewTabWriter(printer.out)
 	defer w.Flush()
 	if !noHeaders {
 		if withNamespace {
-			printValue(w, NamespaceColumn)
+			PodColumns = append([]string{NamespaceColumn}, PodColumns...)


How can we do this without mutating the globally scoped variable here?

we can keep track of how many PodColums will be outputted using a local variable I think. Will try that approach

eddiezane

Looking good!

I am assuming this a quirk of how we round somewhere?

NAME                            CPU(cores)   MEMORY(bytes)
hass-597bc475f5-knvr2           50m          276Mi
mosquitto-564d776dcd-2vdbw      1m           2Mi
zwavejs2mqtt-76c5fbb78c-6mr26   2m           74Mi
                                ________     ________
                                52m          353Mi

eddiezane · 2021-11-16T18:25:30Z

staging/src/k8s.io/kubectl/pkg/cmd/top/top_pod.go

@@ -115,6 +116,7 @@ func NewCmdTopPod(f cmdutil.Factory, o *TopPodOptions, streams genericclioptions
 	cmd.Flags().BoolVarP(&o.AllNamespaces, "all-namespaces", "A", o.AllNamespaces, "If present, list the requested object(s) across all namespaces. Namespace in current context is ignored even if specified with --namespace.")
 	cmd.Flags().BoolVar(&o.NoHeaders, "no-headers", o.NoHeaders, "If present, print output without headers.")
 	cmd.Flags().BoolVar(&o.UseProtocolBuffers, "use-protocol-buffers", o.UseProtocolBuffers, "Enables using protocol-buffers to access Metrics API.")
+	cmd.Flags().BoolVar(&o.Sum, "sum", o.Sum, "If present, print the sum of the resource usages")


Suggested change

cmd.Flags().BoolVar(&o.Sum, "sum", o.Sum, "If present, print the sum of the resource usages")

cmd.Flags().BoolVar(&o.Sum, "sum", o.Sum, "Print the sum of the resource usages")

eddiezane · 2021-11-16T18:25:49Z

staging/src/k8s.io/kubectl/pkg/metricsutil/metrics_printer.go

@@ -21,7 +21,7 @@ import (
 	"io"
 	"sort"

-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"


Suggested change

v1 "k8s.io/api/core/v1"

corev1 "k8s.io/api/core/v1"

eddiezane · 2021-11-16T18:31:21Z

staging/src/k8s.io/kubectl/pkg/metricsutil/metrics_resource_adder.go

+func (adder *ResourceAdder) AddPodMetrics(m *metricsapi.PodMetrics) {
+	for _, c := range m.Containers {
+		for _, res := range adder.resources {
+			total_val := adder.total[res]


nit: camelCase

eddiezane · 2021-11-16T18:34:34Z

staging/src/k8s.io/kubectl/pkg/metricsutil/metrics_resource_adder.go

+	}
+}
+
+// AddSinglePodMetrics will add each pod metrics to the total


Suggested change

// AddSinglePodMetrics will add each pod metrics to the total

// AddPodMetrics will add each pod metric to the total

eddiezane · 2021-11-16T19:00:52Z

staging/src/k8s.io/kubectl/pkg/metricsutil/metrics_printer.go

@@ -310,3 +216,21 @@ func printSingleResourceUsage(out io.Writer, resourceType v1.ResourceName, quant
 		fmt.Fprintf(out, "%v", quantity.Value())
 	}
 }
+
+func printPodResourcesSum(out io.Writer, total v1.ResourceList, withNamespace, printContainers bool, columnWidth int) {


withNamespace and printContainers are unused.

eddiezane · 2021-11-16T19:16:08Z

staging/src/k8s.io/kubectl/pkg/metricsutil/metrics_resource_adder_test.go

+func getResourceQuantity(t *testing.T, quantityStr string) resource.Quantity {
+	t.Helper()
+	var err error
+	quantity, _ := resource.ParseQuantity("0")


For future sanity let's check this error.

eddiezane · 2021-11-16T19:27:00Z

staging/src/k8s.io/kubectl/pkg/metricsutil/metrics_sorter.go

@@ -0,0 +1,130 @@
+/*


Did you make any changes aside from moving this file out?

No I didn't

lauchokyip · 2021-11-29T03:03:47Z

Looking good!

I am assuming this a quirk of how we round somewhere?

NAME                            CPU(cores)   MEMORY(bytes)
hass-597bc475f5-knvr2           50m          276Mi
mosquitto-564d776dcd-2vdbw      1m           2Mi
zwavejs2mqtt-76c5fbb78c-6mr26   2m           74Mi
                                ________     ________
                                52m          353Mi

@eddiezane yup, I believe we are just getting the estimate value over here

dims · 2022-01-05T13:32:38Z

/assign @KnVerey

eddiezane

/lgtm
/approve

k8s-ci-robot · 2022-03-30T00:00:54Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: eddiezane, lauchokyip

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~staging/src/k8s.io/kubectl/OWNERS~~ [eddiezane]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

eddiezane · 2022-03-30T02:32:29Z

Flake is #109133

/retest

k8s-ci-robot requested review from dixudx and KnVerey September 17, 2021 13:27

k8s-ci-robot added area/kubectl sig/cli Categorizes an issue or PR as relevant to SIG CLI. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Sep 17, 2021

lauchokyip commented Sep 17, 2021

View reviewed changes

k8s-ci-robot assigned eddiezane Sep 17, 2021

pacoxu added this to Needs review in SIG CLI Sep 27, 2021

eddiezane reviewed Sep 29, 2021

View reviewed changes

lauchokyip force-pushed the addTopSum branch from 9d438be to 75df48d Compare October 20, 2021 13:13

lauchokyip requested a review from eddiezane October 20, 2021 13:13

eddiezane reviewed Nov 16, 2021

View reviewed changes

Added --sum flag to kubectl top pod

71acf56

lauchokyip force-pushed the addTopSum branch from 75df48d to 71acf56 Compare November 29, 2021 03:41

lauchokyip requested a review from eddiezane November 29, 2021 03:41

k8s-ci-robot assigned KnVerey Jan 5, 2022

eddiezane approved these changes Mar 30, 2022

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 30, 2022

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 30, 2022

k8s-ci-robot merged commit 0401cc2 into kubernetes:master May 4, 2022

SIG CLI automation moved this from Needs review to Done May 4, 2022

k8s-ci-robot added this to the v1.25 milestone May 4, 2022

github-actions bot mentioned this pull request May 11, 2022

Week Ending May 8, 2022 dev-obs/actus#434

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added --sum flag to kubectl top pod #105100

Added --sum flag to kubectl top pod #105100

lauchokyip commented Sep 17, 2021 •

edited

lauchokyip Sep 17, 2021

lauchokyip Sep 17, 2021

lauchokyip commented Sep 17, 2021

lauchokyip commented Sep 17, 2021

lauchokyip commented Sep 20, 2021

eddiezane Sep 29, 2021

lauchokyip Oct 8, 2021

eddiezane left a comment

eddiezane Nov 16, 2021

lauchokyip Nov 29, 2021

eddiezane Nov 16, 2021

eddiezane Nov 16, 2021

lauchokyip Nov 29, 2021

eddiezane Nov 16, 2021

lauchokyip Nov 29, 2021

eddiezane Nov 16, 2021

lauchokyip Nov 29, 2021

eddiezane Nov 16, 2021

eddiezane Nov 16, 2021

lauchokyip Nov 29, 2021

lauchokyip commented Nov 29, 2021

dims commented Jan 5, 2022

eddiezane left a comment

k8s-ci-robot commented Mar 30, 2022

eddiezane commented Mar 30, 2022

	cmd.Flags().BoolVar(&o.Sum, "sum", o.Sum, "If present, print the sum of the resource usages")
	cmd.Flags().BoolVar(&o.Sum, "sum", o.Sum, "Print the sum of the resource usages")

	// AddSinglePodMetrics will add each pod metrics to the total
	// AddPodMetrics will add each pod metric to the total

Added --sum flag to kubectl top pod #105100

Added --sum flag to kubectl top pod #105100

Conversation

lauchokyip commented Sep 17, 2021 • edited

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lauchokyip commented Sep 17, 2021

lauchokyip commented Sep 17, 2021

lauchokyip commented Sep 20, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eddiezane left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lauchokyip commented Nov 29, 2021

dims commented Jan 5, 2022

eddiezane left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Mar 30, 2022

eddiezane commented Mar 30, 2022

lauchokyip commented Sep 17, 2021 •

edited