kubectl binary plugins #37499

fabianofranz · 2016-11-25T21:16:17Z

What this PR does / why we need it:

Introduces the ability to extend kubectl by adding third-party plugins that will be exposed through kubectl.

Plugins are executable commands written in any language. To be included as a plugin, a binary or script file has to

be located under one of the supported plugin path locations:
1.1 ~/.kubectl/plugins dir
1.2. one or more directory set in the KUBECTL_PLUGINS_PATH env var
1.3. the kubectl/plugins dir under one or more directory set in the XDG_DATA_DIRS env var, which defaults to /usr/local/share:/usr/share
in any of the plugin path above, have a subfolder with the plugin file(s)
in the subfolder, contain at least a plugin.yaml file that describes the plugin

Example:

$ cat ~/.kube/plugins/myplugin/plugin.yaml
name: "myplugin"
shortDesc: "My plugin's short description"
command: "echo Hello plugins!"

$ kubectl myplugin
Hello plugins!

In case the plugin declares tunnel: true, the plugin engine will pass the KUBECTL_PLUGIN_API_HOST env var when calling the plugin binary. Plugins can then access the Kube REST API in "http://$KUBECTL_PLUGIN_API_HOST/api" using the same context currently in use by kubectl.

Test plugins are provided in pkg/kubectl/plugins/examples. Just copy (or symlink) the files to ~/.kube/plugins to test.

Which issue this PR fixes:

Related to the discussions in the proposal document: #30086 and kubernetes/community#122.

Release note:

Introduces the ability to extend kubectl by adding third-party plugins. Developer preview, please refer to the documentation for instructions about how to use it.

This change is

k8s-ci-robot · 2016-11-28T20:57:42Z

Jenkins GCI GKE smoke e2e failed for commit 3515f173e82520fa67b134e85dda4c850f7dbc33. Full PR test history.

The magic incantation to run this job again is @k8s-bot gci gke e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

k8s-ci-robot · 2016-11-28T20:58:31Z

Jenkins GKE smoke e2e failed for commit 3515f173e82520fa67b134e85dda4c850f7dbc33. Full PR test history.

The magic incantation to run this job again is @k8s-bot cvm gke e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

k8s-ci-robot · 2016-11-28T20:59:41Z

Jenkins CRI GCE Node e2e failed for commit 3515f173e82520fa67b134e85dda4c850f7dbc33. Full PR test history.

The magic incantation to run this job again is @k8s-bot cri node e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

0xmichalis · 2016-12-02T21:22:19Z

@kubernetes/kubectl @kubernetes/sig-cli

adohe-zz · 2016-12-04T10:02:26Z

@fabianofranz 👍 I would take a deep look the next Monday.

soltysh · 2016-12-05T10:10:19Z

pkg/client/unversioned/clientcmd/loader.go

@@ -47,8 +47,9 @@ const (
 	RecommendedSchemaName       = "schema"
 )

-var RecommendedHomeFile = path.Join(homedir.HomeDir(), RecommendedHomeDir, RecommendedFileName)
-var RecommendedSchemaFile = path.Join(homedir.HomeDir(), RecommendedHomeDir, RecommendedSchemaName)
+var RecommenderConfigDir = path.Join(homedir.HomeDir(), RecommendedHomeDir)


var ( ... )

please 😸

soltysh · 2016-12-05T10:11:45Z

pkg/kubectl/cmd/cmd.go

@@ -225,25 +227,25 @@ func NewKubectlCommand(f cmdutil.Factory, in io.Reader, out, err io.Writer) *cob
 		{
 			Message: "Basic Commands (Beginner):",
 			Commands: []*cobra.Command{
-				NewCmdCreate(f, out, err),
+				NewCmdCreate(f, out, errOut),


We should be passing out and errOut to every single command, as a rule of thumb. Not all our commands write errors to stderr, but should.

yes, this should be a basic rule.

soltysh · 2016-12-05T10:17:42Z

pkg/kubectl/plugin/loader.go

+
+	list := PluginList{}
+	err := filepath.Walk(l.pluginsDir, func(path string, fileInfo os.FileInfo, err error) error {
+		glog.V(9).Infof("Checking plugin in %s...", path)


I'd stick with 5 & 6 levels, 7 and above usually contain HTTP data, which will hide this log.

soltysh · 2016-12-05T10:23:03Z

pkg/kubectl/plugin/plugin.go

+
+package plugin
+
+type Plugin struct {


This needs to be documented in details so that the plugin writers know what to return here.

soltysh · 2016-12-05T10:25:31Z

pkg/kubectl/plugins/examples/kubectl-foo

@@ -0,0 +1,21 @@
+#!/usr/bin/env ruby
+
+if ARGV[0] == "metadata"


If this serves as an example, put detailed comments what/why and where?

soltysh · 2016-12-05T10:27:51Z

pkg/kubectl/plugins/examples/kubectl-foo

  puts "I'm the magnificent foo plugin."
-  puts ""
-  puts "I can use the API from #{api_host}:"
-  puts `curl -s #{api_endpoint}`


I liked this example, b/c it was simple and quite self-explanatory, why removing it?

I wanted to make this one as dumb as possible (doesn't even access the API), and then I wrote the more complex kubectl aging which uses API data.

soltysh · 2016-12-05T10:34:03Z

This is looking really nice, but most importantly thorough doc is needed for users writing the plugins.

adohe-zz · 2016-12-05T11:14:13Z

@soltysh this is just a PoC, still a lot to do, and of course thorough doc is necessary.

adohe-zz · 2016-12-05T11:27:16Z

The Helm plugin guide could be a good reference:
https://github.com/kubernetes/helm/blob/master/docs/plugins.md

adohe-zz · 2016-12-05T11:58:17Z

pkg/kubectl/plugins/plugin.go

+	"github.com/golang/glog"
+)
+
+type Plugin struct {


some comments are necessary here.

adohe-zz · 2016-12-05T12:24:26Z

pkg/kubectl/plugins/loader.go

+			return nil
+		}
+
+		cmd := exec.Command(path, "metadata")


Compared with Helm file based metadata, I am worrying the performance, since this lookup happens every time command execute.

@adohe Absolutely, this is one of my major concerns. While this makes plugins much more portable (everything is in the executable), if definitely doesn't scale well. I'll think about an intermediate idea, maybe cache the metadata, look for a descriptor file first and fallback to the metadata call, something like that.

fabianofranz · 2016-12-05T18:14:05Z

This is looking really nice, but most importantly thorough doc is needed for users writing the plugins.

some comments are necessary here

Definitely, I just didn't do it given the PoC nature of this PR. But it needs both docs and very good inline comments. ;)

fabianofranz · 2017-04-26T20:37:06Z

@liggitt @soltysh @pwittrock @monopole PTAL. Plugins are now loaded under kubectl plugin. In a separate commit for easier review.

soltysh

A small nit, but overall lgtm!

soltysh · 2017-04-27T11:09:50Z

pkg/kubectl/cmd/plugin.go

+	plugin_long = templates.LongDesc(`
+		Runs a command-line plugin.
+
+		Plugins are subcommands that are not part of the major command-line distribution 


It's worth mentioning from where these plugins are loaded, I think.

Yeah it started like that but I didn't like the results, too clumsy for a command help. Ahead we'll probably want something better than relying on just the help as the default command run, then we can think of something better. And we will have proper docs which we can link to.

soltysh · 2017-04-27T11:14:33Z

@fabianofranz you need to run hack/update-generated-docs.sh as well.

fabianofranz · 2017-04-27T19:33:28Z

@k8s-bot verify test this

fabianofranz · 2017-04-27T21:01:50Z

@k8s-bot unit test this

fabianofranz · 2017-04-28T04:10:08Z

@k8s-bot unit test this

fabianofranz · 2017-04-28T14:41:56Z

@k8s-bot gce etcd3 e2e test this
@k8s-bot unit test this

fabianofranz · 2017-04-28T15:48:57Z

@soltysh @monopole @pwittrock @liggitt FYI on tests green, I'd like to get this merged. I'm already putting together the next PR that will include parts of post-V0 like global flags through env vars, better support for plugin flags and so on.

0xmichalis · 2017-04-28T16:14:53Z

Very cool work @fabianofranz ! Can't wait to use this. lgtm but I will let one of the main reviewers tag it.

monopole · 2017-04-28T18:20:58Z

/lgtm

smarterclayton · 2017-04-28T19:06:17Z

/approve

for remaining code given agreement from sig owners and that this is initial

k8s-github-robot · 2017-04-28T19:06:23Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fabianofranz, monopole, smarterclayton, soltysh

Needs approval from an approver in each of these OWNERS Files:

~~docs/OWNERS~~ [smarterclayton]
~~hack/OWNERS~~ [smarterclayton]
~~pkg/kubectl/OWNERS~~ [fabianofranz,smarterclayton]
~~staging/src/k8s.io/client-go/OWNERS~~ [smarterclayton]
~~test/fixtures/pkg/kubectl/OWNERS~~ [fabianofranz,smarterclayton,soltysh]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

k8s-github-robot · 2017-04-28T19:23:58Z

Automatic merge from submit-queue

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 25, 2016

fabianofranz mentioned this pull request Nov 25, 2016

Add a proposal for kubectl extension #30086

Closed

k8s-github-robot assigned adohe-zz Nov 25, 2016

k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-label-needed labels Nov 25, 2016

fabianofranz changed the title ~~[WIP] [DO-NOT-MERGE] kubectl plugins~~ [WIP] [DO-NOT-MERGE] kubectl binary plugins Nov 28, 2016

fabianofranz force-pushed the kubectl_plugins branch 3 times, most recently from 3515f17 to 806fb34 Compare November 28, 2016 20:39

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 30, 2016

fabianofranz force-pushed the kubectl_plugins branch from 806fb34 to ad600b7 Compare December 2, 2016 19:59

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 2, 2016

fabianofranz mentioned this pull request Dec 2, 2016

Proposal: kubectl extension kubernetes/community#122

Merged

soltysh reviewed Dec 5, 2016

View reviewed changes

adohe-zz reviewed Dec 5, 2016

View reviewed changes

pkg/kubectl/plugins/plugin.go Outdated

"github.com/golang/glog"

)

type Plugin struct {

Copy link

adohe-zz Dec 5, 2016

Choose a reason for hiding this comment

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

some comments are necessary here.

adohe-zz reviewed Dec 5, 2016

View reviewed changes

k8s-github-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed labels Dec 10, 2016

fabianofranz force-pushed the kubectl_plugins branch from ad600b7 to fd83635 Compare December 16, 2016 21:12

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 23, 2017

k8s-github-robot unassigned adohe-zz Jan 30, 2017

fabianofranz force-pushed the kubectl_plugins branch 2 times, most recently from 32b955c to 096285f Compare April 27, 2017 03:00

soltysh approved these changes Apr 27, 2017

View reviewed changes

fabianofranz force-pushed the kubectl_plugins branch 2 times, most recently from e4ca8b0 to 2ee97cb Compare April 27, 2017 15:22

k8s-github-robot added the kind/old-docs label Apr 27, 2017

fabianofranz force-pushed the kubectl_plugins branch 2 times, most recently from 00f8f61 to 1c2b1ae Compare April 27, 2017 18:51

fabianofranz force-pushed the kubectl_plugins branch from 1c2b1ae to f1153bd Compare April 28, 2017 03:34

fabianofranz added 2 commits April 28, 2017 01:34

Basic support for kubectl plugins

2b178ad

Plugins are loaded under the 'kubectl plugin' command

2158473

fabianofranz force-pushed the kubectl_plugins branch from f1153bd to 2158473 Compare April 28, 2017 04:34

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 28, 2017

fabianofranz added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 28, 2017

k8s-github-robot merged commit d4ece0a into kubernetes:master Apr 28, 2017

sjenning mentioned this pull request May 8, 2017

nil pointer dereference in kubectl/plugins/loader.go:70 #45507

Closed

surajssd mentioned this pull request Jun 16, 2017

Hook into kubectl commandline as extension kedgeproject/kedge#28

Open

fabianofranz mentioned this pull request Jul 31, 2017

kubectl binary plugins kubernetes/enhancements#371

Closed

kubectl binary plugins #37499

kubectl binary plugins #37499

Conversation

fabianofranz commented Nov 25, 2016 • edited Loading

k8s-ci-robot commented Nov 28, 2016

k8s-ci-robot commented Nov 28, 2016

k8s-ci-robot commented Nov 28, 2016

0xmichalis commented Dec 2, 2016

adohe-zz commented Dec 4, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soltysh commented Dec 5, 2016

adohe-zz commented Dec 5, 2016

adohe-zz commented Dec 5, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabianofranz commented Dec 5, 2016

fabianofranz commented Apr 26, 2017

soltysh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soltysh commented Apr 27, 2017

fabianofranz commented Apr 27, 2017

fabianofranz commented Apr 27, 2017

fabianofranz commented Apr 28, 2017

fabianofranz commented Apr 28, 2017

fabianofranz commented Apr 28, 2017

0xmichalis commented Apr 28, 2017

monopole commented Apr 28, 2017

smarterclayton commented Apr 28, 2017

k8s-github-robot commented Apr 28, 2017

k8s-github-robot commented Apr 28, 2017

fabianofranz commented Nov 25, 2016 •

edited

Loading