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 kubectl-config-cleanup plugin manifest #129

Merged
merged 6 commits into from Apr 12, 2019

Conversation

Projects
None yet
3 participants
@dbkegley
Copy link
Contributor

commented Apr 5, 2019


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=[...])
@k8s-ci-robot

This comment has been minimized.

Copy link

commented Apr 5, 2019

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.


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.

Show resolved Hide resolved plugins/config-cleanup.yaml Outdated
Show resolved Hide resolved plugins/config-cleanup.yaml Outdated
Show resolved Hide resolved plugins/config-cleanup.yaml Outdated
@ahmetb

This comment has been minimized.

Copy link
Contributor

commented Apr 8, 2019

I think you also need to sign the Linux Foundation CLA as the bot has instructed above. Let me know if you already did.

@ahmetb

This comment has been minimized.

Copy link
Contributor

commented Apr 8, 2019

I just ran this plugin and it cleared up all my contexts:

kubectl config-cleanup
apiVersion: v1
clusters: []
contexts: []
current-context: ""
kind: Config
preferences: {}
users: []

The whole cmd took 0.1 secs. I don't think it's working correctly. I have 4 GKE clusters in my context. All of them are actually working fine.

@dbkegley

This comment has been minimized.

Copy link
Contributor Author

commented Apr 8, 2019

I think you also need to sign the Linux Foundation CLA as the bot has instructed above. Let me know if you already did.

Still waiting on approval for the CLA, I'll update once it's approved.

I don't think it's working correctly. I have 4 GKE clusters in my context. All of them are actually working fine.

I just tested on GKE and am seeing the same behavior, we can close this PR for now. I'll have to make some additional changes before this plugin is ready for general use. Thank you for the initial review. I'll reopen once the CLA is approved and I've tested the plugin more thoroughly with each of the auth-provider plugins.

@dbkegley dbkegley closed this Apr 8, 2019

@ahmetb

This comment has been minimized.

Copy link
Contributor

commented Apr 8, 2019

It would be good to keep it open so that we don't lose context on the review history.

@dbkegley dbkegley reopened this Apr 9, 2019

@ahmetb

This comment has been minimized.

Copy link
Contributor

commented Apr 10, 2019

/hold
until CLA is signed, and until it works with auth providers like GKE.

@dbkegley

This comment has been minimized.

Copy link
Contributor Author

commented Apr 11, 2019

The newest version of the plugin should work with all auth provider plugins, please let me know if you run into any issues

I'll double check about the cla tomorrow

@ahmetb

This comment has been minimized.

Copy link
Contributor

commented Apr 11, 2019

@dbkegley just tried, it has removed unreachable context, but I still see a bunch of dangling (unused) users and clusters entries, is this expected/by design?

@ahmetb

This comment has been minimized.

Copy link
Contributor

commented Apr 11, 2019

My bad, just saw --users and --clusters. But when I used it, I got error:

$ kubectl config-cleanup --users --clusters > kubeconfig

E0411 16:31:00.594794   59322 round_trippers.go:174] CancelRequest not implemented by *gcp.conditionalTransport

but kubeconfig was written to the file successfully.

EDIT: and the process exits with code=0 (success)

@dbkegley

This comment has been minimized.

Copy link
Contributor Author

commented Apr 12, 2019

I've seen that one before as well, I believe it's related to this issue kubernetes/kubernetes#73791

I've seen it from both the exec plugin and the gcp auth plugins

@ahmetb

This comment has been minimized.

Copy link
Contributor

commented Apr 12, 2019

Got it. The printed error is not a blocker. Next, please look at signing the CLA.
/hold cancel

@dbkegley

This comment has been minimized.

Copy link
Contributor Author

commented Apr 12, 2019

I signed it

@ahmetb

This comment has been minimized.

Copy link
Contributor

commented Apr 12, 2019

/lgtm
/approve

@k8s-ci-robot

This comment has been minimized.

Copy link

commented Apr 12, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ahmetb, dbkegley

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 merged commit b7c1d9e into kubernetes-sigs:master Apr 12, 2019

2 of 3 checks passed

tide Not mergeable. Needs approved, lgtm labels.
Details
cla/linuxfoundation dbkegley authorized
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@dbkegley dbkegley deleted the b23llc:config-cleanup branch Apr 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.