Kubectl plugin for service catalog #840

simonleung8 · 2017-05-12T00:02:36Z

Plugin commands create-service-broker, create-service, bind-service
To use, compile the commands, and set the env var KUBECTL_PLUGINS_PATH and SERVICE_CATALOG_URL

Example:
export KUBECTL_PLUGINS_PATH=$PWD/kubernetes-incubator/service-catalog/bin
export SERVICE_CATALOG_URL={SERVICE_CATALOG_URL}

** if SERVICE_CATALOG_URL is not set, it defaults to minikube IP 192.168.99.100:30080

USAGE:

  kubectl plugin create-service-broker BROKER_NAME BROKER_URL

  kubectl plugin create-service-instance SERVICE_CLASS_NAME PLAN_NAME NAMESPACE INSTANCE_NAME

  kubectl plugin bind-service INSTANCE_NAME BINDING_NAME NAMESPACE
  *secret name defaults to the BINDING_NAME

Currently I do not intent to include unit tests for these plugins, since the kubectl plugin architecture might still be changing. However, integration test make sense and is coming soon.

Current kubectl plugin limitation:

Plugins must be invoked with the plugin command
No custom command flags allowed, kubectl global flags are allowed.

duglin · 2017-05-12T02:14:46Z

VERY COOL!

On the syntax:

you said you can't do flags for the plugin, can you do optional args? If so, we could then add [userid] [password] to the create-service-broker
it should probably be: create-service-instance to avoid confusion with normal kube services
on bind-service I think you meant to have INSTANCE_NAME instead of SERVICE_CLASS_NAME. So bind-service INSTANCE_NAME SECRET_NAME. When we have podpreset then we'll add another parameter.
Can we use the global --namespace flag from kubectl instead of having people add NAMESPACE/ to the instance name?

pmorie

Nice job. I would like to see a demo of these and discuss before we merge this PR, but looks like a promising start.

pmorie · 2017-05-12T03:51:36Z

kubectl_plugins/bind-service/bind-service.go

@@ -0,0 +1,104 @@
+package main


I assume kubectl_plugins here is just a place to start hacking on this and not a statement that this code should live where it currently is in this PR.

Precedent for this type of code in kubernetes: the scheduler lives in plugin/cmd/kube-scheduler. How about plugin/cmd/kubectl/<binary-name> for these?

@pmorie I have moved the plugins into the dir you have recommended plugin/cmd/kubectl/...

pmorie · 2017-05-12T03:53:39Z

kubectl_plugins/bind-service/bind-service.go

+const USAGE = `
+
+Usage:
+  kubectl plugin bind-service SERVICE_CLASS_NAME SECRET_NAME NAMESPACE/INSTANCE_NAME


namespace is a function of the entire binding, not just the instance name. It's preferable to use --namespace for the whole operation.

Also, this operation should not need to take the service class at all.

Suggest the following order for arguments:

bind-service INSTANCE_NAME SECRET_NAME --namespace <namespace>

As of right now, we are not able to leverage the global option flag --namespace. While kubectl will accept the flag, it is not being passed on the plugin. I have made "namespace" into an argument, with the intention to eventually make it into an option flag, I believe kubectl will soon expose this feature (work is being done).

Regarding your comment of binding service not needing a service class, I wasn't able to get it to work without provide the service class name. This is the error I get without one:

Error binding service instance (Binding.servicecatalog.k8s.io "ups-binding" is invalid: Spec.instanceRef.name: Invalid value: "": a DNS-1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*'))

UPDATE: Sorry for the confusion, I see what you and @duglin meant now, I have renamed it to INSTANCE_NAME from SERVICE_CLASS_NAME

pmorie · 2017-05-12T03:55:12Z

kubectl_plugins/create-service-broker/create-service-broker.go

+INVALID USAGE:
+
+Usage:
+  kubectl plugin create-service-broker BROKER_NAME BROKER_URL


For a follow-up: We should plumb through auth somehow. We should assume basic auth will be one of many options. Basic auth is stored in a secret, so we will need to plumb through the namespace and name of the secret.

I will do this in a follow up, however, since option flag is not yet available, the syntax might not be as good as we want. We could have 1 single optional argument and everything (namespace/secret..) will have to go into that argument.

pmorie · 2017-05-12T03:56:16Z

kubectl_plugins/create-service/create-service.go

+const USAGE = `
+
+Usage:
+  kubectl plugin create-service SERVICE_CLASS_NAME PLAN_NAME NAMESPACE/INSTANCE_NAME


We should decide whether we want create-service-instance, provision-service-instance, both, or something else.

Same comments about --namespace apply here.

I am going with create-service-instance for now.

See my comment above regarding --namespace

pmorie · 2017-05-15T15:42:32Z

Added to 1.0.0 for now because I'm not sure if work is still happening on this or when we expect it to be merged, but I would like ot have this before we call service-catalog 1.0.0

duglin · 2017-05-15T15:43:33Z

what do you mean by "if" ?

simonleung8 · 2017-05-15T22:15:12Z

Currently the secret name will default to the binding name during bind_service

pmorie · 2017-05-25T22:01:49Z

@simonleung8 is this still something you are working on?

simonleung8 · 2017-05-25T22:13:57Z

@pmorie Yes, it seems like folks are generally ok with the direction this is going, I am going to finish refining the code and the UX.

arschles

@simonleung8 looks like a good start. I made 1 small comment, and also I don't think that the binary should be checked in.

arschles · 2017-06-05T18:30:31Z

plugin/cmd/kubectl/utils/utils.go

+func CheckNamespaceExists(name, proxyURL string) error {
+	resp, err := http.Get(fmt.Sprintf("%s/api/v1/namespaces/%s", proxyURL, name))
+	if err != nil {
+		fmt.Errorf("Error looking up namespace from core api server (%s)", err)


this needs a return before the fmt.Errorf

simonleung8 · 2017-06-05T18:48:58Z

@arschles Thanks, cleaned up the code

fabianofranz · 2017-06-07T20:16:30Z

plugin/cmd/kubectl/utils/utils.go

+	proxyURL := "http://127.0.0.1"
+	proxyPort := "8881"
+
+	kubeProxy := exec.Command("kubectl", "proxy", "-p", proxyPort)


Here you can use the KUBECTL_PLUGINS_CALLER env var, which will give you the full path to the kubectl binary.

fabianofranz · 2017-06-07T20:21:13Z

plugin/cmd/kubectl/utils/ui.go

+import "fmt"
+
+func Green(str string) string {
+	return fmt.Sprintf("\x1b[32;1m%s\x1b[0m", str)


Note that these aren't supported and will look bad on most Windows terminals. One of the major reasons we didn't add broader support to colors in kubectl yet. If you absolutely want them, I suggest using a library like this one.

fabianofranz · 2017-06-07T20:27:16Z

plugin/cmd/kubectl/bind-service/bind-service.go

+)
+
+const USAGE = `Usage:
+  kubectl plugin bind-service INSTANCE_NAME BINDING_NAME NAMESPACE`


The NAMESPACE you want here is not the global --namespace flag but one specific to this command, correct?

Just mentioning because you can find the value from the global flag in the KUBECTL_PLUGINS_GLOBAL_FLAG_NAMESPACE and there's also KUBECTL_PLUGINS_CURRENT_NAMESPACE which will given the "namespace in use" concept (the one coming from the kubeconfig, or overriden by the --namespace flag, etc).

fabianofranz · 2017-06-07T20:28:08Z

I know we're still lacking docs for kubectl plugins, but I added a couple comments here. Hope it helps. ;)

simonleung8 · 2017-06-07T23:08:02Z

@fabianofranz Thanks, good info!

fabianofranz · 2017-06-13T21:58:34Z

Full flags support in plugins + API proxy are PR'ed:

kubernetes/kubernetes#47151
kubernetes/kubernetes#47267

MHBauer · 2017-08-09T18:59:18Z

rebased to bring the 1.7 rebase into scope.

fabianofranz · 2017-08-09T21:13:54Z

kubernetes/kubernetes#47267 flags support is merged.
kubernetes/kubernetes#47151 API proxy not merged and not getting broader acception given security concerns. Most likely plugins will not have a proxy mechanism and/or have token env vars set, and will have to deal with accessing the API themselves.

So I'd like to submit a few adjustments here to reflect the current status of plugins. Should I do it by adding commits to this PR, or let's move on to get this merged and I open a separate one after that?

MHBauer · 2017-08-10T22:26:28Z

@fabianofranz I have not tested this one yet myself, adding commits is fine by me if you know what we shoudl be doing.

MorganBauer · 2017-09-02T01:17:08Z

rebased on the names stuff. still builds and runs nice.

MHBauer · 2017-09-20T22:41:26Z

rebasing on the new api changes.

MHBauer · 2017-10-24T20:18:53Z

more work was done on the side in simonleung8#1

I have a simple reconciliation and proposal to PR soon.

Signed-off-by: Doug Davis <dug@us.ibm.com>

n3wscott · 2017-11-13T18:51:28Z

Out of date, can be closed or still wanted?

duglin · 2017-11-13T19:03:22Z

@jberkhahn is still doing stuff, but perhaps its moved to a new PR. Jonathan?

pmorie · 2018-01-04T22:36:00Z

@jberkhahn can we close this PR now that #1621 is open?

pmorie · 2018-01-20T14:18:33Z

Okay, I think we can def close this one now that #1664 is merged.

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 12, 2017

simonleung8 mentioned this pull request May 12, 2017

Make it possible to add porcelain commands to kubectl for service-catalog #806

Closed

pmorie reviewed May 12, 2017

View reviewed changes

MHBauer self-requested a review May 12, 2017 18:41

MHBauer added the in-progress label May 12, 2017

pmorie added this to the 1.0.0 milestone May 15, 2017

simonleung8 force-pushed the plugin-cmds branch 3 times, most recently from d3ed19a to 275eee9 Compare May 15, 2017 22:13

duglin changed the title ~~Kubectl plugin for service catalog - WIP~~ WIP: Kubectl plugin for service catalog May 24, 2017

simonleung8 force-pushed the plugin-cmds branch from 275eee9 to 7a8b4a4 Compare June 4, 2017 21:47

arschles reviewed Jun 5, 2017

View reviewed changes

simonleung8 force-pushed the plugin-cmds branch 2 times, most recently from 6282d90 to 9838341 Compare June 5, 2017 18:48

simonleung8 force-pushed the plugin-cmds branch 2 times, most recently from 6e2eb13 to ec1f5b6 Compare June 5, 2017 18:53

simonleung8 changed the title ~~WIP: Kubectl plugin for service catalog~~ Kubectl plugin for service catalog Jun 5, 2017

fabianofranz reviewed Jun 7, 2017

View reviewed changes

duglin force-pushed the plugin-cmds branch 2 times, most recently from 5672f1f to 6aa63bd Compare July 11, 2017 19:59

jmrodri mentioned this pull request Jul 13, 2017

Configure broker relist behavior on a per-broker basis #705

Closed

arschles mentioned this pull request Aug 1, 2017

Proposal for Broker relist API #1086

Closed

fabianofranz mentioned this pull request Aug 3, 2017

Proposal - kubectl commands for the Service Catalog kubernetes/community#481

Closed

MHBauer force-pushed the plugin-cmds branch from 6aa63bd to 935494b Compare August 9, 2017 18:58

MHBauer force-pushed the plugin-cmds branch from 935494b to 447e34c Compare September 1, 2017 23:34

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 1, 2017

MHBauer force-pushed the plugin-cmds branch from 235bc2e to d3a0e16 Compare September 2, 2017 01:10

MHBauer force-pushed the plugin-cmds branch from d3a0e16 to 7193ec2 Compare September 20, 2017 22:43

Simon Leung and others added 8 commits October 26, 2017 14:28

kubectl plugin create-service-broker

f940936

kubectl plugin create-service-instance

9056d40

kubectl plugin bind-service

12f2652

Add plugins to Makefile and fix lint issues

16a4788

Signed-off-by: Doug Davis <dug@us.ibm.com>

Fix some more "verify" issues

d6066ca

Signed-off-by: Doug Davis <dug@us.ibm.com>

rebase based on name change

af5d0b0

use standard build

1ac451f

remove hard coded test url

25f1494

MHBauer force-pushed the plugin-cmds branch from 4546769 to 25f1494 Compare October 26, 2017 21:34

pmorie closed this Jan 20, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Kubectl plugin for service catalog #840

Kubectl plugin for service catalog #840

simonleung8 commented May 12, 2017 •

edited by duglin

duglin commented May 12, 2017

pmorie left a comment

pmorie May 12, 2017

simonleung8 May 15, 2017

pmorie May 12, 2017

simonleung8 May 15, 2017 •

edited

pmorie May 12, 2017

simonleung8 May 15, 2017

pmorie May 12, 2017

simonleung8 May 15, 2017

pmorie commented May 15, 2017

duglin commented May 15, 2017

simonleung8 commented May 15, 2017

pmorie commented May 25, 2017

simonleung8 commented May 25, 2017

arschles left a comment

arschles Jun 5, 2017

simonleung8 commented Jun 5, 2017

fabianofranz Jun 7, 2017

fabianofranz Jun 7, 2017

fabianofranz Jun 7, 2017

fabianofranz commented Jun 7, 2017

simonleung8 commented Jun 7, 2017

fabianofranz commented Jun 13, 2017

MHBauer commented Aug 9, 2017

fabianofranz commented Aug 9, 2017

MHBauer commented Aug 10, 2017

MorganBauer commented Sep 2, 2017

MHBauer commented Sep 20, 2017

MHBauer commented Oct 24, 2017

n3wscott commented Nov 13, 2017

duglin commented Nov 13, 2017

pmorie commented Jan 4, 2018

pmorie commented Jan 20, 2018

Kubectl plugin for service catalog #840

Kubectl plugin for service catalog #840

Conversation

simonleung8 commented May 12, 2017 • edited by duglin

duglin commented May 12, 2017

pmorie left a 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

simonleung8 May 15, 2017 • edited

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

pmorie commented May 15, 2017

duglin commented May 15, 2017

simonleung8 commented May 15, 2017

pmorie commented May 25, 2017

simonleung8 commented May 25, 2017

arschles left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonleung8 commented Jun 5, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabianofranz commented Jun 7, 2017

simonleung8 commented Jun 7, 2017

fabianofranz commented Jun 13, 2017

MHBauer commented Aug 9, 2017

fabianofranz commented Aug 9, 2017

MHBauer commented Aug 10, 2017

MorganBauer commented Sep 2, 2017

MHBauer commented Sep 20, 2017

MHBauer commented Oct 24, 2017

n3wscott commented Nov 13, 2017

duglin commented Nov 13, 2017

pmorie commented Jan 4, 2018

pmorie commented Jan 20, 2018

simonleung8 commented May 12, 2017 •

edited by duglin

simonleung8 May 15, 2017 •

edited