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

KEP: Add kubectl plugin manager (krew) #2340

Closed
wants to merge 2 commits into from
Closed

Conversation

lbb
Copy link

@lbb lbb commented Jul 3, 2018

No description provided.

@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 3, 2018
@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. labels Jul 3, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

Assign the PR to them by writing /assign @idvoretskyi 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 cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 3, 2018

The kubectl plugin system allows users to extend kubectl by placing extension
executables in a plugin directory. This allows new subcommands and subcommand
trees to be added in the kubectl CLI interface under `kubectl plugin <NAME>`.
Copy link

Choose a reason for hiding this comment

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

let's really see if we can remove the "plugin" word from the cmd line - that's a pretty big UX issue

Copy link

Choose a reason for hiding this comment

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

save the "plugin" word for managing plugins

Copy link
Author

Choose a reason for hiding this comment

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

@idealhack
Copy link
Member

/ok-to-test

@k8s-ci-robot k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jul 4, 2018
Copy link
Contributor

@soltysh soltysh left a comment

Choose a reason for hiding this comment

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

/hold
As part of 1.12 release plugins are undergoing significant refactoring, due to this factor I'd prefer to hold this proposal until after that initial effort is implemented. Additionally, from the several discussion we've had during past several months I haven't heard of anyone bringing the topic of managing plugins to the table for sig-cli. Moreover, each time we discussed plugins we clearly stated we don't want to manage them, at least not yet.
Before proceeding this further I'd like to see this topic being presented during one of sig-cli meetings and most importantly take into account planned changes to the plugins mechanism.

#### Capabilities

Note: This proposal has already been implemented in
`github.com/GoogleContainerTools/krew` as a proof of concept.
Copy link
Contributor

Choose a reason for hiding this comment

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

Does not exist.

Copy link
Author

Choose a reason for hiding this comment

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

The repository is now publicly accessible 😄
Same URL: https://github.com/GoogleContainerTools/krew

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 4, 2018
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 20, 2018
@k8s-ci-robot
Copy link
Contributor

@lbb: 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.

@soltysh
Copy link
Contributor

soltysh commented Aug 24, 2018

@lbb now that we can plugins up and ready can you update the current proposal and this can land?

@errordeveloper
Copy link
Member

errordeveloper commented Aug 31, 2018

@soltysh are you referring to https://github.com/kubernetes/community/blob/master/keps/sig-cli/0024-kubectl-plugins.md? I'm finding it difficult to track the status of that KEP.

@ahmetb
Copy link
Member

ahmetb commented Aug 31, 2018

I'm tempted to hold off on this KEP until there is a kubectl release with the new plugin model shipped:

  1. The plugin model change breaks krew and renders it unusable. We need to refactor krew and its plugin manifest to install plugins to somewhere in PATH.
  2. We don't have enough plugins to understand if the manifest, installation system and the upgrade model is sufficient and satisfies users and packaging across multiple platforms (esp. Windows). We are working on getting more plugin devs to adopt the krew packaging format.

After we feel confident, we should come back to make progress on the KEP.

@juanvallejo
Copy link
Contributor

@errordeveloper

are you referring to https://github.com/kubernetes/community/blob/master/keps/sig-cli/0024-kubectl-plugins.md? I'm finding it difficult to track the status of that KEP.

The implementation for that KEP will be available as part of the 1.12 release

@soltysh
Copy link
Contributor

soltysh commented Sep 25, 2018

Now that we have the plugin implementation ready as part of 1.12 release, this can continue.
/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 25, 2018
@ahmetb
Copy link
Member

ahmetb commented Sep 25, 2018

Small status update: we're still working on porting krew to the new plugin model introduced in 1.12. There are quite a lot of blocking changes, but we're close to releasing a krew v0.2 that's testable.

@justaugustus
Copy link
Member

/kind kep

@thockin thockin changed the title Add kubectl plugin manager (krew) KEP KEP: Add kubectl plugin manager (krew) Oct 18, 2018
editor: lbb
creation-date: 2018-07-03
last-updated: yyyy-mm-dd
status: provisional
Copy link
Contributor

Choose a reason for hiding this comment

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

Given the discussion on the issue it sounds like this KEP is in the implementable status. See https://github.com/kubernetes/community/blob/master/keps/0001-kubernetes-enhancement-proposal-process.md#kep-workflow

### Scenarios (standalone only)

* User wants to install krew. They can run one `curl` command to install the
plugin manager, the plugin manager should then be able to function properly.
Copy link
Contributor

Choose a reason for hiding this comment

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

Will the install scenario be the case or will it be apart of kubectl as earlier stated? Should this define the process for the proof of concept or what's going to be part of the toolchain?

@@ -0,0 +1,237 @@
---
kep-number: 14
title: Krew
Copy link
Contributor

Choose a reason for hiding this comment

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

The name confused me when I first looked at this. I understand that krew is the name of the proof of concept. Should the title be more expressive of the purpose?

plugin manager to discover and manage plugins.
* Developer wants to update their plugin and make those changes public.
They create a new PR against the index repository updating their plugin meta
description file.
Copy link
Contributor

Choose a reason for hiding this comment

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

How do developers pin or otherwise deal with versions of plugins?

@mattfarina
Copy link
Contributor

@soltysh Where is this at in terms of its lifecycle? How far is it from being merged where updates can be via other PRs? Is krew going to be moved under k8s control?

@justaugustus
Copy link
Member

REMINDER: KEPs are moving to k/enhancements on November 30. Please attempt to merge this KEP before then to signal consensus.
For more details on this change, review this thread.

Any questions regarding this move should be directed to that thread and not asked on GitHub.

@justaugustus
Copy link
Member

KEPs have moved to k/enhancements.
This PR will be closed and any additional changes to this KEP should be submitted to k/enhancements.
For more details on this change, review this thread.

Any questions regarding this move should be directed to that thread and not asked on GitHub.
/close

@k8s-ci-robot
Copy link
Contributor

@justaugustus: Closed this PR.

In response to this:

KEPs have moved to k/enhancements.
This PR will be closed and any additional changes to this KEP should be submitted to k/enhancements.
For more details on this change, review this thread.

Any questions regarding this move should be directed to that thread and not asked on GitHub.
/close

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.

@dhiller
Copy link
Contributor

dhiller commented Jun 14, 2019

@lbb I understand this PR should have been moved to k/enhancements. When will this happen?

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. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.