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

Add debug plugin #135

Closed
wants to merge 3 commits into from
Closed

Add debug plugin #135

wants to merge 3 commits into from

Conversation

aylei
Copy link

@aylei aylei commented Apr 13, 2019

Hi there, thanks for reviewing my PR! Please don't hesitate to point out areas where I need to change.

Plugin Home Page

Signed-off-by: Aylei rayingecho@gmail.com


Checklist for plugin developers:

  • Read the Plugin Naming Guide (for new plugins)
  • Verify the installation from URL or a local archive works (kubectl krew install --manifest=[...] --archive=[...])

Signed-off-by: Aylei <rayingecho@gmail.com>
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 13, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: aylei
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: juanvallejo

If they are not already assigned, you can assign the PR to them by writing /assign @juanvallejo in a comment when ready.

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:

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

@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Apr 13, 2019
Signed-off-by: Aylei <rayingecho@gmail.com>
@ahmetb
Copy link
Member

ahmetb commented Apr 13, 2019

This name is too generic. Please choose something more unique and descriptive. Also there’s been a proposal to create an official debug command, which might be still going on: kubernetes/kubernetes#45922

@aylei
Copy link
Author

aylei commented Apr 15, 2019

Thank you for your advice!
Actually, the project homepage x-ref the related KEP(Troubleshooting Running Pods), and the purpose of this plugin is help the users to do the similar stuff before this KEP completes and help the users who still using the old version of kubernetes after the KEP completes.
How about kubectl-debug-pod? The form kubectl debug pod will be shadowed when debug command is added officially, and kubectl debug-pod is fairly descriptive I guess.

@ahmetb
Copy link
Member

ahmetb commented Apr 15, 2019

I agree debug-container or debug-pod is better (debug-pod would be my preference). Sadly first-class kubectl commands can claim vauge/broad terminology like "debug" but I think we should be more careful.

Does this cause a problem with your documentation etc. Are you planning to go make the name change there as well?

@aylei
Copy link
Author

aylei commented Apr 16, 2019

Does this cause a problem with your documentation etc.

Not quite, I will add documents to explain the krew part about the slightly different name and it's a not a burden for me.

Are you planning to go make the name change there as well?

I prefer not, changing name can be troublesome for me and existing adopters.

Signed-off-by: Aylei <rayingecho@gmail.com>
@aylei
Copy link
Author

aylei commented Apr 16, 2019

I realize that I have to update some codes to support both kubectl debug-pod and kubectl debug command suggestions, so I will close this PR now and open another PR in the next release of my project.

@aylei aylei closed this Apr 16, 2019
@ahmetb
Copy link
Member

ahmetb commented Apr 30, 2019

Thanks! Expecting a new PR soon, happy to discuss what would be a more appropriate name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants