Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kubectl top output format options #1534

Closed
edoardottt opened this issue Dec 18, 2023 · 8 comments
Closed

kubectl top output format options #1534

edoardottt opened this issue Dec 18, 2023 · 8 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@edoardottt
Copy link

edoardottt commented Dec 18, 2023

What would you like to be added:

I would love to have an output format flag -o (like other subcommands) for top subcommand.
Something like:

kubectl top pods -o json

Why is this needed:
Because it would enhance compatibility with shell scripts using json as main format.
For my specific purpose I have to digest only json output and so this means I have to convert the top output in json by myself, but since it's a simple task it would be better to have this feature included inside kubectl already.

I know this feature was already requested, but the issue was closed as "stale". Maybe after 4 years many more people/orgs are using k8s and the needs increased.

@edoardottt edoardottt added the kind/feature Categorizes issue or PR as related to a new feature. label Dec 18, 2023
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Dec 18, 2023
@eddiezane
Copy link
Member

eddiezane commented Jan 3, 2024

We would accept a PR that adds the output printers to this command. JSON is a good start but we should explore what adding the others would look like.

/triage accepted

Somewhere here.

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jan 3, 2024
@ardaguclu
Copy link
Member

In case someone wants to have a look at this issue, I'm adding a basic sketch of how changes may look like(similar changes should be applied to top_pod.go as well as unit tests covering new output options);

diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/top/top_node.go b/staging/src/k8s.io/kubectl/pkg/cmd/top/top_node.go
index b6b36e4d1d3..95136dee27d 100644
--- a/staging/src/k8s.io/kubectl/pkg/cmd/top/top_node.go
+++ b/staging/src/k8s.io/kubectl/pkg/cmd/top/top_node.go
@@ -20,6 +20,9 @@ import (
        "context"
        "errors"
 
+       "k8s.io/cli-runtime/pkg/genericclioptions"
+       "k8s.io/kubectl/pkg/scheme"
+
        "github.com/spf13/cobra"
        v1 "k8s.io/api/core/v1"
        metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -48,6 +51,7 @@ type TopNodeOptions struct {
 
        NodeClient      corev1client.CoreV1Interface
        Printer         *metricsutil.TopCmdPrinter
+       PrintFlags      *genericclioptions.PrintFlags
        DiscoveryClient discovery.DiscoveryInterface
        MetricsClient   metricsclientset.Interface
 
@@ -72,6 +76,7 @@ func NewCmdTopNode(f cmdutil.Factory, o *TopNodeOptions, streams genericiooption
        if o == nil {
                o = &TopNodeOptions{
                        IOStreams:          streams,
+                       PrintFlags:         genericclioptions.NewPrintFlags("").WithTypeSetter(scheme.Scheme),
                        UseProtocolBuffers: true,
                }
        }
@@ -95,6 +100,7 @@ func NewCmdTopNode(f cmdutil.Factory, o *TopNodeOptions, streams genericiooption
        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.ShowCapacity, "show-capacity", o.ShowCapacity, "Print node resources based on Capacity instead of Allocatable(default) of the nodes.")
+       o.PrintFlags.AddFlags(cmd)
 
        return cmd
 }
@@ -200,7 +206,15 @@ func (o TopNodeOptions) RunTopNode() error {
                }
        }
 
-       return o.Printer.PrintNodeMetrics(metrics.Items, availableResources, o.NoHeaders, o.SortBy)
+       if !o.PrintFlags.OutputFlagSpecified() {
+               return o.Printer.PrintNodeMetrics(metrics.Items, availableResources, o.NoHeaders, o.SortBy)
+       }
+       printer, err := o.PrintFlags.ToPrinter()
+       if err != nil {
+               return err
+       }
+
+       return printer.PrintObj(metrics, o.Out)
 }
 
 func getNodeMetricsFromMetricsAPI(metricsClient metricsclientset.Interface, resourceName string, selector labels.Selector) (*metricsapi.NodeMetricsList, error) {

@carlory
Copy link
Member

carlory commented Jan 8, 2024

/assign

@carlory
Copy link
Member

carlory commented Jan 8, 2024

In case someone wants to have a look at this issue, I'm adding a basic sketch of how changes may look like(similar changes should be applied to top_pod.go as well as unit tests covering new output options);

diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/top/top_node.go b/staging/src/k8s.io/kubectl/pkg/cmd/top/top_node.go
index b6b36e4d1d3..95136dee27d 100644
--- a/staging/src/k8s.io/kubectl/pkg/cmd/top/top_node.go
+++ b/staging/src/k8s.io/kubectl/pkg/cmd/top/top_node.go

Hi @ardaguclu, its implementation is equal to the following command:

kubectl get nodes.metrics.k8s.io -ojson
kubectl get pods.metrics.k8s.io -ojson

I think the implemtation of the sample code is not suitable for the purpose of the issue. I think its output should be like this (not raw json):

[
  {
    "name": "node1",
    "CPU(cores)": "479m",
    "CPU%": 0.5,
    "MEMORY(bytes)": "5118Mi"
    "MEMORY%": 0.5,
  }
]

Is it what you want? @edoardottt If so, is it still acceptable? @eddiezane

@edoardottt
Copy link
Author

edoardottt commented Jan 8, 2024

Hi @carlory, thanks for the comment.

I'm trying to use the command

kubectl get pods.metrics.k8s.io -ojson

and it works as expected.

I agree that the output you pointed out should resemble what you wrote. So something like this:

[
  {
    "name": "node1",
    "CPU(cores)": "479m",
    "CPU%": 0.5,
    "MEMORY(bytes)": "5118Mi"
    "MEMORY%": 0.5,
  }
]

since this is exactly the information the command top pods outputs normally.

But in my opinion there's an issue with the metrics system itself, since both the output of kubectl top pods and

kubectl get pods.metrics.k8s.io -ojson | jq .items[].containers

don't change over time even if the cluster is "under stress".

Can anyone help me understand why this issue is present and how to overcome this problem?

Click for more details minikube version: v1.32.0
kubectl Client Version: v1.28.5
Kustomize Version: v5.0.4-0.20230601165947-6ce0bf390ce3
kubectl Server Version: v1.28.3

output.txt

Maybe kubernetes/minikube#17889 can help.

cc @eddiezane @ardaguclu

@carlory carlory removed their assignment Jan 16, 2024
@ah8ad3
Copy link
Member

ah8ad3 commented Feb 7, 2024

@ardaguclu @mpuckett159
Hi, I've been looking at this issue implementing the genericclioptions.PrintFlags for the kubectl top nodes -o json would give us the exact result as kubectl get nodes.metric -o json. If that is the case we can implement it or maybe close this issue. Otherwise if we want to print other results like the one @carlory point out like

[
  {
    "name": "node1",
    "CPU(cores)": "479m",
    "CPU%": 0.5,
    "MEMORY(bytes)": "5118Mi"
    "MEMORY%": 0.5,
  }
]

We need clarification for this issue, i can help with resolving if you make it more clear. Thanks!

@mpuckett159
Copy link
Contributor

/close
As @ah8ad3 has identified that this is achievable through other existing subcommands and functionality there is no need to implement further features here, so we will close this issue.

@k8s-ci-robot
Copy link
Contributor

@mpuckett159: Closing this issue.

In response to this:

/close
As @ah8ad3 has identified that this is achievable through other existing subcommands and functionality there is no need to implement further features here, so we will close this issue.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

No branches or pull requests

7 participants