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

Revise istioctl global flags displaying in options, and organize flag sets #40982

Closed
wants to merge 2 commits into from

Conversation

hanxiaop
Copy link
Member

@hanxiaop hanxiaop commented Sep 14, 2022

Please provide a description of this PR:
Originally #37938 was resolved in #38236. However it broke all --help commands, see #38236 (review). This PR addresses the problem, and organizes all the flags of a command in a new Options section like kubectl. For example:

istioctl admin log -h
Retrieve or update logging levels of istiod components.

Aliases:
log, l

Examples:
  # Retrieve information about istiod logging levels.
  istioctl admin log

  # Retrieve information about istiod logging levels on a specific control plane pod.
  istioctl admin l istiod-5c868d8bdd-pmvgg

  # Update levels of the specified loggers.
  istioctl admin log --level ads:debug,authorization:debug

  # Reset levels of all the loggers to default value (info).
  istioctl admin log -r

Options:
    --ctrlz_port=9876:
        ControlZ port

    --level='':
        Comma-separated list of output logging level for scopes in format
        <scope>:<level>[,<scope>:<level>,...]Possible values for <level>: none, error, warn, info, debug

    -o, --output='short':
        Output format: one of json|short

    -r, --reset=false:
        Reset levels to default value. (info)

    -l, --selector='app=istiod':
        label selector

    --stack-trace-level='':
        Comma-separated list of stack trace level  for scopes in format
        <scope>:<stack-trace-level>[,<scope>:<stack-trace-level>,...] Possible values for <stack-trace-level>: none,
        error, warn, info, debug

Usage:
  istioctl admin log [<pod-name>] [--level <scope>:<level>][--stack-trace-level
<scope>:<level>]|[-r|--reset]|[--output|-o short|yaml] [options]

Use "istioctl options" for a list of global command-line options (applies to all commands).

@hanxiaop hanxiaop requested review from a team as code owners September 14, 2022 09:46
@istio-testing istio-testing added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Sep 14, 2022
@hanxiaop
Copy link
Member Author

/retest-required

Copy link
Member

@GregHanson GregHanson left a comment

Choose a reason for hiding this comment

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

release note looks good

@istio-testing
Copy link
Collaborator

@hanxiaop: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
integ-operator-controller_istio 14390bf link true /test integ-operator-controller

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. I understand the commands that are listed here.

@howardjohn
Copy link
Member

Why is this better? it just seems needlessly verbose?

@hanxiaop
Copy link
Member Author

Why is this better? it just seems needlessly verbose?

@howardjohn Some of the global flags are displayed in the help func but some of them are in the istioctl options command. I think it is better to organize them into the same place to avoid of confusion? Here are some comments: #38236 (review), #37938 (comment). I'm not really sure about why we marked those log flags as hidden flags. What do you think?
The changes of persistent flags are only due to unfriendly displaying of flags of subcommands, as a result I revised them all to get better user experience.

@istio-testing
Copy link
Collaborator

@hanxiaop: PR needs rebase.

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.

@istio-testing istio-testing added the needs-rebase Indicates a PR needs to be rebased before being merged label Oct 11, 2022
@istio-policy-bot istio-policy-bot added the lifecycle/stale Indicates a PR or issue hasn't been manipulated by an Istio team member for a while label Oct 21, 2022
@istio-policy-bot
Copy link

🚧 This issue or pull request has been closed due to not having had activity from an Istio team member since 2022-09-20. If you feel this issue or pull request deserves attention, please reopen the issue. Please see this wiki page for more information. Thank you for your contributions.

Created by the issue and PR lifecycle manager.

@istio-policy-bot istio-policy-bot added the lifecycle/automatically-closed Indicates a PR or issue that has been closed automatically. label Nov 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/user experience lifecycle/automatically-closed Indicates a PR or issue that has been closed automatically. lifecycle/stale Indicates a PR or issue hasn't been manipulated by an Istio team member for a while needs-rebase Indicates a PR needs to be rebased before being merged size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants