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

[Antctl] Add an output formatter to better display multi-line string responses. #3589

Merged
merged 1 commit into from
Jul 6, 2022

Conversation

Atish-iaf
Copy link
Contributor

@Atish-iaf Atish-iaf commented Apr 6, 2022

Current output formatters (table, json, and yaml) of Antctl are
not good at displaying multi-line string responses.

Add a new output formatter raw whose output is similar to
fmt.Print(responseString) to better display multi-line string responses.

Fixes #3426

Signed-off-by: Kumar Atish atish.iaf@gmail.com

@Atish-iaf Atish-iaf force-pushed the antctl_multi_line_string branch from 82dcd99 to ea0bae8 Compare April 6, 2022 11:08
@codecov-commenter
Copy link

codecov-commenter commented Apr 6, 2022

Codecov Report

Merging #3589 (848940a) into main (af9ee6a) will decrease coverage by 13.58%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #3589       +/-   ##
===========================================
- Coverage   61.75%   48.17%   -13.59%     
===========================================
  Files         294      265       -29     
  Lines       43631    39664     -3967     
===========================================
- Hits        26946    19109     -7837     
- Misses      14445    18713     +4268     
+ Partials     2240     1842      -398     
Flag Coverage Δ
e2e-tests 41.27% <ø> (?)
integration-tests 35.22% <ø> (?)
kind-e2e-tests 32.40% <ø> (-12.41%) ⬇️
unit-tests ?
Impacted Files Coverage Δ
pkg/controller/networkpolicy/endpoint_querier.go 4.58% <0.00%> (-88.08%) ⬇️
pkg/controller/ipam/validate.go 0.00% <0.00%> (-79.79%) ⬇️
pkg/cni/client.go 0.00% <0.00%> (-77.78%) ⬇️
pkg/controller/ipam/antrea_ipam_controller.go 0.00% <0.00%> (-76.42%) ⬇️
pkg/controller/externalippool/validate.go 0.00% <0.00%> (-75.87%) ⬇️
pkg/apiserver/handlers/featuregates/handler.go 1.63% <0.00%> (-73.78%) ⬇️
.../registry/networkpolicy/clustergroupmember/rest.go 11.11% <0.00%> (-73.62%) ⬇️
pkg/controller/networkpolicy/clustergroup.go 3.50% <0.00%> (-73.57%) ⬇️
pkg/controller/networkpolicy/crd_utils.go 19.13% <0.00%> (-73.46%) ⬇️
.../certificatesigningrequest/approving_controller.go 0.00% <0.00%> (-73.41%) ⬇️
... and 167 more

jsonFormatter formatterType = "json"
yamlFormatter formatterType = "yaml"
tableFormatter formatterType = "table"
rawStringFormatter formatterType = "rawString"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be more user-friendly to name it raw or rawstring

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to raw

@Atish-iaf Atish-iaf force-pushed the antctl_multi_line_string branch from ea0bae8 to 7f8cce2 Compare April 6, 2022 12:56
@Atish-iaf Atish-iaf marked this pull request as ready for review April 6, 2022 12:59
Copy link
Contributor

@jianjuns jianjuns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have an example of such multi-line output, with a comparison of "raw" and other format?

@Atish-iaf
Copy link
Contributor Author

Do we have an example of such multi-line output, with a comparison of "raw" and other format?

Currently we don't have any antctl command whose output include multiple-line string responses.
You can find example in the issue #3426 . Also i changed output of an antctl command for testing purpose.

root@minikube:/# antctl trace-packet
|
  Hello
  World
   Project Antrea
root@minikube:/# antctl trace-packet -o yaml
|
  Hello
  World
   Project Antrea
root@minikube:/# antctl trace-packet -o json
"Hello\nWorld\n Project Antrea\n"
root@minikube:/# antctl trace-packet -o raw
Hello
World
 Project Antrea

@jianjuns
Copy link
Contributor

jianjuns commented Apr 6, 2022

Thanks for the examples. The proposal looks good to me.

xliuxu
xliuxu previously approved these changes Apr 28, 2022
Copy link
Contributor

@xliuxu xliuxu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

dreamtalen
dreamtalen previously approved these changes May 10, 2022
Copy link
Contributor

@dreamtalen dreamtalen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks atish working on this.
Could maintainers help merge this PR? This will help me develop another patch described in issue #3426. Thanks!

@luolanzone
Copy link
Contributor

Hi @tnqn @jianjuns Could you help to check this PR and move forward? thanks.

jianjuns
jianjuns previously approved these changes Jul 5, 2022
@jianjuns
Copy link
Contributor

jianjuns commented Jul 6, 2022

/test-all

Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. help message needs update, otherwise people don't know the format is available:
# ./bin/antctl-linux get controllerinfo -h
Print Antrea controller's basic information including version, deployment, NetworkPolicy controller, ControllerConditions, etc.

Usage:
  antctl-linux get controllerinfo [flags]

Aliases:
  controllerinfo, controllerinfos, ci

Examples:
  Get the controllerinfo
  $ antctl-linux get controllerinfo


Flags:
  -h, --help            help for controllerinfo
  -o, --output string   output format: json|table|yaml (default "table")
  1. The commit message seems to talk about something unrelated to this change:

Added a new output formatter called "raw" whose output is similar to fmt.Print(responseString) which directly lets user redirect the output into a file and apply it afterwards.

What does "directly lets user redirect the output into a file and apply it afterwards" mean?

  1. Commit message body should be wrapped at 72 chars, otherwise git log doesn't look neat:
commit 66f06522e756d37f70cf2b1d0f18d5ff944e6f27 (HEAD -> main)
Author: Kumar Atish <atish.iaf@gmail.com>
Date:   Wed Apr 6 16:27:24 2022 +0530

    Add an output format to better display multi-line string responses.

    Current output formatters (table, json, and yaml) of Antctl are not good at displaying multi-line string responses.
    Added a new output formatter called "raw" whose output is similar to fmt.Print(responseString) which directly lets user redirect the output into a file and apply it afterwards.

    Fixes #3426

    Signed-off-by: Kumar Atish <atish.iaf@gmail.com>

commit af9ee6ada8a597fc136298b73e02f3da163ed20d (origin/main, origin/HEAD)
Author: Quan Tian <qtian@vmware.com>
Date:   Wed Jul 6 22:35:52 2022 +0800

    Fix egress e2e test (#3953)

    The verification was wrong, which lead to false positive.

    Besides, DeleteSNATRule should only call iptables or ip6tables based on
    the protocol of SNAT IP, otherwise it would fail and lead to retries.

    Signed-off-by: Quan Tian <qtian@vmware.com>

commit 6e8aaac2d4fa8d679b72745783ad1b263d3e3a0e
Author: Lan <luola@vmware.com>
Date:   Wed Jul 6 22:33:00 2022 +0800

    Fix integration data on codecov (#3955)

    1. The crd path is changed, so fix the path in MC integration test.
    2. Change the codecov caller parameters

    Signed-off-by: Lan Luo <luola@vmware.com>

Current output formatters (table, json, and yaml) of Antctl are
not good at displaying multi-line string responses.

Add a new output formatter `raw` whose output is similar to
fmt.Print(responseString) to better display multi-line string responses.

Fixes antrea-io#3426

Signed-off-by: Kumar Atish <atish.iaf@gmail.com>
@Atish-iaf Atish-iaf dismissed stale reviews from jianjuns, dreamtalen, and xliuxu via 848940a July 6, 2022 16:56
@Atish-iaf Atish-iaf force-pushed the antctl_multi_line_string branch from 7f8cce2 to 848940a Compare July 6, 2022 16:56
@Atish-iaf Atish-iaf changed the title [Antctl] Add an output format to better display multi-line string responses. [Antctl] Add an output formatter to better display multi-line string responses. Jul 6, 2022
@jianjuns
Copy link
Contributor

jianjuns commented Jul 6, 2022

/test-all

@Atish-iaf Atish-iaf requested a review from tnqn July 6, 2022 17:01
@jianjuns jianjuns merged commit acddced into antrea-io:main Jul 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Antctl] Add an output format to better display multi-line string responses
7 participants