Allow `kubectl expose` to be polymorphic to the source of the selector #5977

smarterclayton · 2015-03-26T02:37:20Z

Allows exposing new resource types to be exposed (OpenShift deployment
controllers, copying services, and other resources that will have
pod label selectors).

Also fixed a bug where the selector wasn't exposed because of parameter
defaulting.

googlebot · 2015-03-26T02:37:22Z

Thanks for your pull request.

It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA) at https://cla.developers.google.com/.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check the information on your CLA or see this help article on setting the email on your git commits.

Once you've done that, please reply here to let us know. If you signed the CLA as a corporation, please let us know the company's name.

j3ffml · 2015-03-26T17:22:44Z

pkg/kubectl/cmd/expose.go

 // Create a service for a replicated streaming application on port 4100 balancing UDP traffic and named 'video-stream'.
 $ kubectl expose streamer --port=4100 --protocol=udp --service-name=video-stream`
 )

 func (f *Factory) NewCmdExposeService(out io.Writer) *cobra.Command {
 	cmd := &cobra.Command{
-		Use:     "expose NAME --port=port [--protocol=TCP|UDP] [--container-port=number-or-name] [--service-name=name] [--public-ip=ip] [--create-external-load-balancer=bool]",
+		Use:     "expose [RESOURCE] NAME --port=port [--protocol=TCP|UDP] [--container-port=number-or-name] [--service-name=name] [--public-ip=ip] [--create-external-load-balancer=bool]",


My take is that if the command is going to be polymorphic to either rc or service, it should require explicitly providing the resource type. Remembering that the resource defaults to rc if omitted is an unnecessary burden for users.

Isn't it going to be the default option? I would say that you expose a replication controller by default. You can expose other things. Although I guess a pod is also something you would expose (except you can't expose just one safely without doing a label query first). I'm ok with forcing someone to always type RC though.

----- Original Message -----

// Create a service for a replicated streaming application on port 4100
balancing UDP traffic and named 'video-stream'.
$ kubectl expose streamer --port=4100 --protocol=udp
--service-name=video-stream`
)

func (f *Factory) NewCmdExposeService(out io.Writer) *cobra.Command {
cmd := &cobra.Command{

Use: "expose NAME --port=port [--protocol=TCP|UDP]
[--container-port=number-or-name] [--service-name=name] [--public-ip=ip]
[--create-external-load-balancer=bool]",

Use: "expose [RESOURCE] NAME --port=port [--protocol=TCP|UDP]
[--container-port=number-or-name] [--service-name=name] [--public-ip=ip]
[--create-external-load-balancer=bool]",

My take is that if the command is going to be polymorphic to either rc or
service, it should require explicitly providing the resource type.
Remembering that the resource defaults to rc if omitted is an unnecessary
burden for users.

Reply to this email directly or view it on GitHub:
https://github.com/GoogleCloudPlatform/kubernetes/pull/5977/files#r27234846

I erred on the side of not breaking the input pattern of existing commands. We probably want to break CLI before 1.0. If we don't think a default is valuable let's just change it now.

----- Original Message -----

Isn't it going to be the default option? I would say that you expose a
replication controller by default. You can expose other things. Although
I guess a pod is also something you would expose (except you can't expose
just one safely without doing a label query first). I'm ok with forcing
someone to always type RC though.

----- Original Message -----

// Create a service for a replicated streaming application on port 4100
balancing UDP traffic and named 'video-stream'.
$ kubectl expose streamer --port=4100 --protocol=udp
--service-name=video-stream`
)

func (f *Factory) NewCmdExposeService(out io.Writer) *cobra.Command {
cmd := &cobra.Command{

Use: "expose NAME --port=port [--protocol=TCP|UDP]
[--container-port=number-or-name] [--service-name=name] [--public-ip=ip]
[--create-external-load-balancer=bool]",

Use: "expose [RESOURCE] NAME --port=port [--protocol=TCP|UDP]
[--container-port=number-or-name] [--service-name=name] [--public-ip=ip]
[--create-external-load-balancer=bool]",

My take is that if the command is going to be polymorphic to either rc or
service, it should require explicitly providing the resource type.
Remembering that the resource defaults to rc if omitted is an unnecessary
burden for users.

Reply to this email directly or view it on GitHub:
https://github.com/GoogleCloudPlatform/kubernetes/pull/5977/files#r27234846

Fixed ptal

----- Original Message -----

I erred on the side of not breaking the input pattern of existing commands.
We probably want to break CLI before 1.0. If we don't think a default is
valuable let's just change it now.

----- Original Message -----

Isn't it going to be the default option? I would say that you expose a
replication controller by default. You can expose other things.
Although
I guess a pod is also something you would expose (except you can't expose
just one safely without doing a label query first). I'm ok with forcing
someone to always type RC though.

----- Original Message -----

// Create a service for a replicated streaming application on port
4100
balancing UDP traffic and named 'video-stream'.
$ kubectl expose streamer --port=4100 --protocol=udp
--service-name=video-stream`
)

func (f *Factory) NewCmdExposeService(out io.Writer) *cobra.Command {
cmd := &cobra.Command{

Use: "expose NAME --port=port [--protocol=TCP|UDP]
[--container-port=number-or-name] [--service-name=name]
[--public-ip=ip]
[--create-external-load-balancer=bool]",

Use: "expose [RESOURCE] NAME --port=port [--protocol=TCP|UDP]
[--container-port=number-or-name] [--service-name=name]
[--public-ip=ip]
[--create-external-load-balancer=bool]",

My take is that if the command is going to be polymorphic to either rc or
service, it should require explicitly providing the resource type.
Remembering that the resource defaults to rc if omitted is an unnecessary
burden for users.

Reply to this email directly or view it on GitHub:
https://github.com/GoogleCloudPlatform/kubernetes/pull/5977/files#r27234846

j3ffml · 2015-03-26T18:27:32Z

docs/man/man1/kubectl-expose.1

@@ -16,8 +16,9 @@ kubectl expose \- Take a replicated application and expose it as Kubernetes Serv
 Take a replicated application and expose it as Kubernetes Service.

 .PP
-Looks up a ReplicationController by name, and uses the selector for that replication controller
-as the selector for a new Service on the specified port.
+Looks up a replication controller by name, and uses the selector for that replication controller


Wording a bit clunky. Consider changing to

Looks up a replication controller or service by name, and uses the selector for that resource as the selector for a new service on the specified port.

Done

----- Original Message -----

@@ -16,8 +16,9 @@ kubectl expose - Take a replicated application and
expose it as Kubernetes Serv
Take a replicated application and expose it as Kubernetes Service.

.PP
-Looks up a ReplicationController by name, and uses the selector for that
replication controller
-as the selector for a new Service on the specified port.
+Looks up a replication controller by name, and uses the selector for that
replication controller

Wording a bit clunky. Consider changing to

Looks up a replication controller or service by name, and uses the selector for that resource as the selector for a new service on the specified port.

Reply to this email directly or view it on GitHub:
https://github.com/GoogleCloudPlatform/kubernetes/pull/5977/files#r27241736

Allows exposing new resource types to be exposed (OpenShift deployment controllers, copying services, and other resources that will have pod label selectors). Also fixed a bug where the selector wasn't exposed because of parameter defaulting.

j3ffml · 2015-03-26T18:44:15Z

pkg/kubectl/cmd/expose.go

+	switch l := len(args); {
+	case l == 2:
+		resource, name = args[0], args[1]
+	default:


Nit: seems like if/else would be cleaner

j3ffml · 2015-03-26T20:23:18Z

LGTM. Trappable is green, nit inconsequential. Merging.

…ther_resources Allow `kubectl expose` to be polymorphic to the source of the selector

Refs: kubernetes#5977

googlebot added the cla: no label Mar 26, 2015

smarterclayton force-pushed the allow_expose_to_handle_other_resources branch from f641086 to b6dcb79 Compare March 26, 2015 14:20

smarterclayton assigned j3ffml Mar 26, 2015

j3ffml reviewed Mar 26, 2015
View reviewed changes

smarterclayton force-pushed the allow_expose_to_handle_other_resources branch from b6dcb79 to 086081d Compare March 26, 2015 18:21

j3ffml reviewed Mar 26, 2015
View reviewed changes

smarterclayton force-pushed the allow_expose_to_handle_other_resources branch from 086081d to 64f91f7 Compare March 26, 2015 18:36

j3ffml reviewed Mar 26, 2015
View reviewed changes

j3ffml added a commit that referenced this pull request Mar 26, 2015

Merge pull request #5977 from smarterclayton/allow_expose_to_handle_o…

653b7e6

…ther_resources Allow `kubectl expose` to be polymorphic to the source of the selector

j3ffml merged commit 653b7e6 into kubernetes:master Mar 26, 2015

kbeecher pushed a commit to endocode/kubernetes that referenced this pull request Apr 27, 2015

Modify REST framework to support deleting all resources in a namespace

e685d1e

Refs: kubernetes#5977

kbeecher mentioned this pull request Apr 27, 2015

WIP Modify REST framework to support deleting all resources in a namespace #7372

Closed

smarterclayton unassigned j3ffml Aug 12, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow `kubectl expose` to be polymorphic to the source of the selector #5977

Allow `kubectl expose` to be polymorphic to the source of the selector #5977

smarterclayton commented Mar 26, 2015

googlebot commented Mar 26, 2015

j3ffml Mar 26, 2015

smarterclayton Mar 26, 2015

smarterclayton Mar 26, 2015

smarterclayton Mar 26, 2015

j3ffml Mar 26, 2015

smarterclayton Mar 26, 2015

j3ffml Mar 26, 2015

j3ffml commented Mar 26, 2015

Allow kubectl expose to be polymorphic to the source of the selector #5977

Allow kubectl expose to be polymorphic to the source of the selector #5977

Conversation

smarterclayton commented Mar 26, 2015

googlebot commented Mar 26, 2015

j3ffml Mar 26, 2015

Choose a reason for hiding this comment

smarterclayton Mar 26, 2015

Choose a reason for hiding this comment

smarterclayton Mar 26, 2015

Choose a reason for hiding this comment

smarterclayton Mar 26, 2015

Choose a reason for hiding this comment

j3ffml Mar 26, 2015

Choose a reason for hiding this comment

smarterclayton Mar 26, 2015

Choose a reason for hiding this comment

j3ffml Mar 26, 2015

Choose a reason for hiding this comment

j3ffml commented Mar 26, 2015

Allow `kubectl expose` to be polymorphic to the source of the selector #5977

Allow `kubectl expose` to be polymorphic to the source of the selector #5977