Merge pull request #15461 from kargakis/relax-exposing-multiport-objects

docs/man/man1/kubectl-expose.1

@@ -13,12 +13,13 @@ kubectl expose \- Take a replication controller, service or pod and expose it as
 
 .SH DESCRIPTION
 .PP
-Take a replication controller, service or pod and expose it as a new Kubernetes Service.
+Take a replication controller, service, or pod and expose it as a new Kubernetes service.
 
 .PP
-Looks up a replication controller, service or pod by name and uses the selector for that resource as the
-selector for a new Service on the specified port. If no labels are specified, the new service will
-re\-use the labels from the resource it exposes.
+Looks up a replication controller, service, or pod by name and uses the selector for that resource as the
+selector for a new service on the specified port. Note that if no port is specified via \-\-port and the
+exposed resource has multiple ports, all will be re\-used by the new service. Also if no labels are specified,
+the new service will re\-use the labels from the resource it exposes.
 
 
 .SH OPTIONS

docs/user-guide/kubectl/kubectl_expose.md

@@ -38,11 +38,12 @@ Take a replication controller, service or pod and expose it as a new Kubernetes
 ### Synopsis
 
 
-Take a replication controller, service or pod and expose it as a new Kubernetes Service.
+Take a replication controller, service, or pod and expose it as a new Kubernetes service.
 
-Looks up a replication controller, service or pod by name and uses the selector for that resource as the
-selector for a new Service on the specified port. If no labels are specified, the new service will
-re-use the labels from the resource it exposes.
+Looks up a replication controller, service, or pod by name and uses the selector for that resource as the
+selector for a new service on the specified port. Note that if no port is specified via --port and the
+exposed resource has multiple ports, all will be re-used by the new service. Also if no labels are specified,
+the new service will re-use the labels from the resource it exposes.
 
 ```
 kubectl expose (-f FILENAME | TYPE NAME) [--port=port] [--protocol=TCP|UDP] [--target-port=number-or-name] [--name=name] [----external-ip=external-ip-of-service] [--type=type]
@@ -125,7 +126,7 @@ $ kubectl expose rc streamer --port=4100 --protocol=udp --name=video-stream
 
 * [kubectl](kubectl.md)	 - kubectl controls the Kubernetes cluster manager
 
-###### Auto generated by spf13/cobra at 2015-10-10 14:16:24.22183637 +0000 UTC
+###### Auto generated by spf13/cobra at 2015-10-14 10:34:09.969832007 +0000 UTC
 
 <!-- BEGIN MUNGE: GENERATED_ANALYTICS -->
 [![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/docs/user-guide/kubectl/kubectl_expose.md?pixel)]()

hack/test-cmd.sh

@@ -213,6 +213,8 @@ runTests() {
   rc_container_image_field=".spec.template.spec.containers"
   port_field="(index .spec.ports 0).port"
   port_name="(index .spec.ports 0).name"
+  second_port_field="(index .spec.ports 1).port"
+  second_port_name="(index .spec.ports 1).name"
   image_field="(index .spec.containers 0).image"
   hpa_min_field=".spec.minReplicas"
   hpa_max_field=".spec.maxReplicas"
@@ -781,6 +783,17 @@ __EOF__
   # Clean-up
   kubectl delete svc kubernetes-serve-hostnam "${kube_flags[@]}"
 
+  ### Expose multiport object as a new service
+  # Pre-condition: don't use --port flag
+  output_message=$(kubectl expose -f docs/admin/high-availability/etcd.yaml --selector=test=etcd 2>&1 "${kube_flags[@]}")
+  # Post-condition: expose succeeded
+  kube::test::if_has_string "${output_message}" '\"etcd-server\" exposed'
+  # Post-condition: generated service has both ports from the exposed pod
+  kube::test::get_object_assert 'service etcd-server' "{{$port_name}} {{$port_field}}" 'port-1 2380'
+  kube::test::get_object_assert 'service etcd-server' "{{$second_port_name}} {{$second_port_field}}" 'port-2 4001'
+  # Clean-up
+  kubectl delete svc etcd-server "${kube_flags[@]}"
+
   ### Delete replication controller with id
   # Pre-condition: frontend replication controller is running
   kube::test::get_object_assert rc "{{range.items}}{{$id_field}}:{{end}}" 'frontend:'

pkg/kubectl/cmd/cmd_test.go

@@ -235,6 +235,8 @@ func NewAPIFactory() (*cmdutil.Factory, *testFactory, runtime.Codec) {
 	}
 	rf := cmdutil.NewFactory(nil)
 	f.PodSelectorForObject = rf.PodSelectorForObject
+	f.PortsForObject = rf.PortsForObject
+	f.LabelsForObject = rf.LabelsForObject
 	f.CanBeExposed = rf.CanBeExposed
 	return f, t, testapi.Default.Codec()
 }

pkg/kubectl/cmd/expose.go

@@ -19,6 +19,7 @@ package cmd
 import (
 	"fmt"
 	"io"
+	"strings"
 
 	"github.com/spf13/cobra"
 
@@ -35,11 +36,12 @@ type ExposeOptions struct {
 }
 
 const (
-	expose_long = `Take a replication controller, service or pod and expose it as a new Kubernetes Service.
+	expose_long = `Take a replication controller, service, or pod and expose it as a new Kubernetes service.
 
-Looks up a replication controller, service or pod by name and uses the selector for that resource as the
-selector for a new Service on the specified port. If no labels are specified, the new service will
-re-use the labels from the resource it exposes.`
+Looks up a replication controller, service, or pod by name and uses the selector for that resource as the
+selector for a new service on the specified port. Note that if no port is specified via --port and the 
+exposed resource has multiple ports, all will be re-used by the new service. Also if no labels are specified,
+the new service will re-use the labels from the resource it exposes.`
 
 	expose_example = `# Create a service for a replicated nginx, which serves on port 80 and connects to the containers on port 8000.
 $ kubectl expose rc nginx --port=80 --target-port=8000
@@ -163,7 +165,7 @@ func RunExpose(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []str
 		case 1:
 			params["port"] = ports[0]
 		default:
-			return cmdutil.UsageError(cmd, fmt.Sprintf("multiple ports to choose from: %v, please explicitly specify a port using the --port flag.", ports))
+			params["ports"] = strings.Join(ports, ",")
 		}
 	}
 	if kubectl.IsZero(params["labels"]) {

pkg/kubectl/cmd/expose_test.go

@@ -286,6 +286,54 @@ func TestRunExposeService(t *testing.T) {
 			expected: "service \"a-name-that-is-toooo-big\" exposed",
 			status:   200,
 		},
+		{
+			name: "expose-multiport-object",
+			args: []string{"service", "foo"},
+			ns:   "test",
+			calls: map[string]string{
+				"GET":  "/namespaces/test/services/foo",
+				"POST": "/namespaces/test/services",
+			},
+			input: &api.Service{
+				ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: "", Labels: map[string]string{"svc": "multiport"}},
+				Spec: api.ServiceSpec{
+					Ports: []api.ServicePort{
+						{
+							Protocol:   api.ProtocolTCP,
+							Port:       80,
+							TargetPort: util.NewIntOrStringFromInt(80),
+						},
+						{
+							Protocol:   api.ProtocolTCP,
+							Port:       443,
+							TargetPort: util.NewIntOrStringFromInt(443),
+						},
+					},
+				},
+			},
+			flags: map[string]string{"selector": "svc=fromfoo", "generator": "service/v2", "name": "fromfoo", "dry-run": "true"},
+			output: &api.Service{
+				ObjectMeta: api.ObjectMeta{Name: "fromfoo", Namespace: "", Labels: map[string]string{"svc": "multiport"}},
+				Spec: api.ServiceSpec{
+					Ports: []api.ServicePort{
+						{
+							Name:       "port-1",
+							Protocol:   api.ProtocolTCP,
+							Port:       80,
+							TargetPort: util.NewIntOrStringFromInt(80),
+						},
+						{
+							Name:       "port-2",
+							Protocol:   api.ProtocolTCP,
+							Port:       443,
+							TargetPort: util.NewIntOrStringFromInt(443),
+						},
+					},
+					Selector: map[string]string{"svc": "fromfoo"},
+				},
+			},
+			status: 200,
+		},
 	}
 
 	for _, test := range tests {

pkg/kubectl/service.go

@@ -19,6 +19,7 @@ package kubectl
 import (
 	"fmt"
 	"strconv"
+	"strings"
 
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/runtime"
@@ -52,7 +53,12 @@ func paramNames() []GeneratorParam {
 		{"default-name", true},
 		{"name", false},
 		{"selector", true},
-		{"port", true},
+		// port will be used if a user specifies --port OR the exposed object
+		// has one port
+		{"port", false},
+		// ports will be used iff a user doesn't specify --port AND the
+		// exposed object has multiple ports
+		{"ports", false},
 		{"labels", false},
 		{"external-ip", false},
 		{"create-external-load-balancer", false},
@@ -100,19 +106,41 @@ func generate(genericParams map[string]interface{}) (runtime.Object, error) {
 			return nil, fmt.Errorf("'name' is a required parameter.")
 		}
 	}
-	portString, found := params["port"]
-	if !found {
-		return nil, fmt.Errorf("'port' is a required parameter.")
-	}
-	port, err := strconv.Atoi(portString)
-	if err != nil {
-		return nil, err
-	}
+	ports := []api.ServicePort{}
 	servicePortName, found := params["port-name"]
 	if !found {
 		// Leave the port unnamed.
 		servicePortName = ""
 	}
+	// ports takes precedence over port since it will be
+	// specified only when the user hasn't specified a port
+	// via --port and the exposed object has multiple ports.
+	var portString string
+	if portString, found = params["ports"]; !found {
+		portString, found = params["port"]
+		if !found {
+			return nil, fmt.Errorf("'port' is a required parameter.")
+		}
+	}
+	portStringSlice := strings.Split(portString, ",")
+	for i, stillPortString := range portStringSlice {
+		port, err := strconv.Atoi(stillPortString)
+		if err != nil {
+			return nil, err
+		}
+		name := servicePortName
+		// If we are going to assign multiple ports to a service, we need to
+		// generate a different name for each one.
+		if len(portStringSlice) > 1 {
+			name = fmt.Sprintf("port-%d", i+1)
+		}
+		ports = append(ports, api.ServicePort{
+			Name:     name,
+			Port:     port,
+			Protocol: api.Protocol(params["protocol"]),
+		})
+	}
+
 	service := api.Service{
 		ObjectMeta: api.ObjectMeta{
 			Name:   name,
@@ -120,27 +148,31 @@ func generate(genericParams map[string]interface{}) (runtime.Object, error) {
 		},
 		Spec: api.ServiceSpec{
 			Selector: selector,
-			Ports: []api.ServicePort{
-				{
-					Name:     servicePortName,
-					Port:     port,
-					Protocol: api.Protocol(params["protocol"]),
-				},
-			},
+			Ports:    ports,
 		},
 	}
-	targetPort, found := params["target-port"]
+	targetPortString, found := params["target-port"]
 	if !found {
-		targetPort, found = params["container-port"]
+		targetPortString, found = params["container-port"]
 	}
-	if found && len(targetPort) > 0 {
-		if portNum, err := strconv.Atoi(targetPort); err != nil {
-			service.Spec.Ports[0].TargetPort = util.NewIntOrStringFromString(targetPort)
+	if found && len(targetPortString) > 0 {
+		var targetPort util.IntOrString
+		if portNum, err := strconv.Atoi(targetPortString); err != nil {
+			targetPort = util.NewIntOrStringFromString(targetPortString)
 		} else {
-			service.Spec.Ports[0].TargetPort = util.NewIntOrStringFromInt(portNum)
+			targetPort = util.NewIntOrStringFromInt(portNum)
+		}
+		// Use the same target-port for every port
+		for i := range service.Spec.Ports {
+			service.Spec.Ports[i].TargetPort = targetPort
 		}
 	} else {
-		service.Spec.Ports[0].TargetPort = util.NewIntOrStringFromInt(port)
+		// If --target-port or --container-port haven't been specified, this
+		// should be the same as Port
+		for i := range service.Spec.Ports {
+			port := service.Spec.Ports[i].Port
+			service.Spec.Ports[i].TargetPort = util.NewIntOrStringFromInt(port)
+		}
 	}
 	if params["create-external-load-balancer"] == "true" {
 		service.Spec.Type = api.ServiceTypeLoadBalancer

pkg/kubectl/service_test.go

@@ -303,6 +303,107 @@ func TestGenerateService(t *testing.T) {
 				},
 			},
 		},
+		{
+			generator: ServiceGeneratorV1{},
+			params: map[string]interface{}{
+				"selector":       "foo=bar",
+				"name":           "test",
+				"ports":          "80,443",
+				"protocol":       "TCP",
+				"container-port": "foobar",
+			},
+			expected: api.Service{
+				ObjectMeta: api.ObjectMeta{
+					Name: "test",
+				},
+				Spec: api.ServiceSpec{
+					Selector: map[string]string{
+						"foo": "bar",
+					},
+					Ports: []api.ServicePort{
+						{
+							Name:       "port-1",
+							Port:       80,
+							Protocol:   api.ProtocolTCP,
+							TargetPort: util.NewIntOrStringFromString("foobar"),
+						},
+						{
+							Name:       "port-2",
+							Port:       443,
+							Protocol:   api.ProtocolTCP,
+							TargetPort: util.NewIntOrStringFromString("foobar"),
+						},
+					},
+				},
+			},
+		},
+		{
+			generator: ServiceGeneratorV2{},
+			params: map[string]interface{}{
+				"selector":    "foo=bar",
+				"name":        "test",
+				"ports":       "80,443",
+				"protocol":    "UDP",
+				"target-port": "1234",
+			},
+			expected: api.Service{
+				ObjectMeta: api.ObjectMeta{
+					Name: "test",
+				},
+				Spec: api.ServiceSpec{
+					Selector: map[string]string{
+						"foo": "bar",
+					},
+					Ports: []api.ServicePort{
+						{
+							Name:       "port-1",
+							Port:       80,
+							Protocol:   api.ProtocolUDP,
+							TargetPort: util.NewIntOrStringFromInt(1234),
+						},
+						{
+							Name:       "port-2",
+							Port:       443,
+							Protocol:   api.ProtocolUDP,
+							TargetPort: util.NewIntOrStringFromInt(1234),
+						},
+					},
+				},
+			},
+		},
+		{
+			generator: ServiceGeneratorV2{},
+			params: map[string]interface{}{
+				"selector": "foo=bar",
+				"name":     "test",
+				"ports":    "80,443",
+				"protocol": "TCP",
+			},
+			expected: api.Service{
+				ObjectMeta: api.ObjectMeta{
+					Name: "test",
+				},
+				Spec: api.ServiceSpec{
+					Selector: map[string]string{
+						"foo": "bar",
+					},
+					Ports: []api.ServicePort{
+						{
+							Name:       "port-1",
+							Port:       80,
+							Protocol:   api.ProtocolTCP,
+							TargetPort: util.NewIntOrStringFromInt(80),
+						},
+						{
+							Name:       "port-2",
+							Port:       443,
+							Protocol:   api.ProtocolTCP,
+							TargetPort: util.NewIntOrStringFromInt(443),
+						},
+					},
+				},
+			},
+		},
 	}
 	for _, test := range tests {
 		obj, err := test.generator.Generate(test.params)