fix expose multi protocols issue

docs/man/man1/kubectl-expose.1

@@ -89,8 +89,8 @@ Possible resources include (case insensitive):
     The port that the service should serve on. Copied from the resource being exposed, if unspecified
 
 .PP
-\fB\-\-protocol\fP="TCP"
-    The network protocol for the service to be created. Default is 'tcp'.
+\fB\-\-protocol\fP=""
+    The network protocol for the service to be created. Default is 'TCP'.
 
 .PP
 \fB\-\-record\fP=false

docs/user-guide/kubectl/kubectl_expose.md

@@ -97,7 +97,7 @@ kubectl expose deployment nginx --port=80 --target-port=8000
       --output-version="": Output the formatted object with the given group version (for ex: 'extensions/v1beta1').
       --overrides="": An inline JSON override for the generated object. If this is non-empty, it is used to override the generated object. Requires that the object supply a valid apiVersion field.
       --port="": The port that the service should serve on. Copied from the resource being exposed, if unspecified
-      --protocol="TCP": The network protocol for the service to be created. Default is 'tcp'.
+      --protocol="": The network protocol for the service to be created. Default is 'TCP'.
       --record[=false]: Record current kubectl command in the resource annotation.
   -R, --recursive[=false]: If true, process directory recursively.
       --save-config[=false]: If true, the configuration of current object will be saved in its annotation. This is useful when you want to perform kubectl apply on this object in the future.
@@ -143,7 +143,7 @@ kubectl expose deployment nginx --port=80 --target-port=8000
 
 * [kubectl](kubectl.md)	 - kubectl controls the Kubernetes cluster manager
 
-###### Auto generated by spf13/cobra on 3-May-2016
+###### Auto generated by spf13/cobra on 11-May-2016
 
 <!-- BEGIN MUNGE: GENERATED_ANALYTICS -->
 [![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/docs/user-guide/kubectl/kubectl_expose.md?pixel)]()

docs/yaml/kubectl/kubectl_expose.yaml

@@ -59,9 +59,8 @@ options:
   usage: |
     The port that the service should serve on. Copied from the resource being exposed, if unspecified
 - name: protocol
-  default_value: TCP
   usage: |
-    The network protocol for the service to be created. Default is 'tcp'.
+    The network protocol for the service to be created. Default is 'TCP'.
 - name: record
   default_value: "false"
   usage: Record current kubectl command in the resource annotation.

pkg/kubectl/cmd/cmd_test.go

@@ -315,6 +315,7 @@ func NewAPIFactory() (*cmdutil.Factory, *testFactory, runtime.Codec) {
 	rf := cmdutil.NewFactory(nil)
 	f.MapBasedSelectorForObject = rf.MapBasedSelectorForObject
 	f.PortsForObject = rf.PortsForObject
+	f.ProtocolsForObject = rf.ProtocolsForObject
 	f.LabelsForObject = rf.LabelsForObject
 	f.CanBeExposed = rf.CanBeExposed
 	return f, t, testapi.Default.Codec()

pkg/kubectl/cmd/expose.go

@@ -101,7 +101,7 @@ func NewCmdExposeService(f *cmdutil.Factory, out io.Writer) *cobra.Command {
 	}
 	cmdutil.AddPrinterFlags(cmd)
 	cmd.Flags().String("generator", "service/v2", "The name of the API generator to use. There are 2 generators: 'service/v1' and 'service/v2'. The only difference between them is that service port in v1 is named 'default', while it is left unnamed in v2. Default is 'service/v2'.")
-	cmd.Flags().String("protocol", "TCP", "The network protocol for the service to be created. Default is 'tcp'.")
+	cmd.Flags().String("protocol", "", "The network protocol for the service to be created. Default is 'TCP'.")
 	cmd.Flags().String("port", "", "The port that the service should serve on. Copied from the resource being exposed, if unspecified")
 	cmd.Flags().String("type", "", "Type for this service: ClusterIP, NodePort, or LoadBalancer. Default is 'ClusterIP'.")
 	// TODO: remove create-external-load-balancer in code on or after Aug 25, 2016.
@@ -198,6 +198,19 @@ func RunExpose(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []str
 				params["ports"] = strings.Join(ports, ",")
 			}
 		}
+
+		// Always try to derive protocols from the exposed object, may use
+		// different protocols for different ports.
+		if _, found := params["protocol"]; found {
+			protocolsMap, err := f.ProtocolsForObject(info.Object)
+			if err != nil {
+				return cmdutil.UsageError(cmd, fmt.Sprintf("couldn't find protocol via introspection: %s", err))
+			}
+			if protocols := kubectl.MakeProtocols(protocolsMap); !kubectl.IsZero(protocols) {
+				params["protocols"] = protocols
+			}
+		}
+
 		if kubectl.IsZero(params["labels"]) {
 			labels, err := f.LabelsForObject(info.Object)
 			if err != nil {

pkg/kubectl/cmd/expose_test.go

@@ -304,6 +304,65 @@ func TestRunExposeService(t *testing.T) {
 			},
 			status: 200,
 		},
+		{
+			name: "expose-multiprotocol-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: intstr.FromInt(80),
+						},
+						{
+							Protocol:   api.ProtocolUDP,
+							Port:       8080,
+							TargetPort: intstr.FromInt(8080),
+						},
+						{
+							Protocol:   api.ProtocolUDP,
+							Port:       8081,
+							TargetPort: intstr.FromInt(8081),
+						},
+					},
+				},
+			},
+			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: intstr.FromInt(80),
+						},
+						{
+							Name:       "port-2",
+							Protocol:   api.ProtocolUDP,
+							Port:       8080,
+							TargetPort: intstr.FromInt(8080),
+						},
+						{
+							Name:       "port-3",
+							Protocol:   api.ProtocolUDP,
+							Port:       8081,
+							TargetPort: intstr.FromInt(8081),
+						},
+					},
+					Selector: map[string]string{"svc": "fromfoo"},
+				},
+			},
+			status: 200,
+		},
 	}
 
 	for _, test := range tests {

pkg/kubectl/cmd/util/factory.go

@@ -109,6 +109,8 @@ type Factory struct {
 	MapBasedSelectorForObject func(object runtime.Object) (string, error)
 	// PortsForObject returns the ports associated with the provided object
 	PortsForObject func(object runtime.Object) ([]string, error)
+	// ProtocolsForObject returns the <port, protocol> mapping associated with the provided object
+	ProtocolsForObject func(object runtime.Object) (map[string]string, error)
 	// LabelsForObject returns the labels associated with the provided object
 	LabelsForObject func(object runtime.Object) (map[string]string, error)
 	// LogsForObject returns a request for the logs associated with the provided object
@@ -423,6 +425,27 @@ func NewFactory(optionalClientConfig clientcmd.ClientConfig) *Factory {
 				return nil, fmt.Errorf("cannot extract ports from %v", gvk)
 			}
 		},
+		ProtocolsForObject: func(object runtime.Object) (map[string]string, error) {
+			// TODO: replace with a swagger schema based approach (identify pod selector via schema introspection)
+			switch t := object.(type) {
+			case *api.ReplicationController:
+				return getProtocols(t.Spec.Template.Spec), nil
+			case *api.Pod:
+				return getProtocols(t.Spec), nil
+			case *api.Service:
+				return getServiceProtocols(t.Spec), nil
+			case *extensions.Deployment:
+				return getProtocols(t.Spec.Template.Spec), nil
+			case *extensions.ReplicaSet:
+				return getProtocols(t.Spec.Template.Spec), nil
+			default:
+				gvk, err := api.Scheme.ObjectKind(object)
+				if err != nil {
+					return nil, err
+				}
+				return nil, fmt.Errorf("cannot extract protocols from %v", gvk)
+			}
+		},
 		LabelsForObject: func(object runtime.Object) (map[string]string, error) {
 			return meta.NewAccessor().Labels(object)
 		},
@@ -744,6 +767,16 @@ func getPorts(spec api.PodSpec) []string {
 	return result
 }
 
+func getProtocols(spec api.PodSpec) map[string]string {
+	result := make(map[string]string)
+	for _, container := range spec.Containers {
+		for _, port := range container.Ports {
+			result[strconv.Itoa(int(port.ContainerPort))] = string(port.Protocol)
+		}
+	}
+	return result
+}
+
 // Extracts the ports exposed by a service from the given service spec.
 func getServicePorts(spec api.ServiceSpec) []string {
 	result := []string{}
@@ -753,6 +786,15 @@ func getServicePorts(spec api.ServiceSpec) []string {
 	return result
 }
 
+// Extracts the protocols exposed by a service from the given service spec.
+func getServiceProtocols(spec api.ServiceSpec) map[string]string {
+	result := make(map[string]string)
+	for _, servicePort := range spec.Ports {
+		result[strconv.Itoa(int(servicePort.Port))] = string(servicePort.Protocol)
+	}
+	return result
+}
+
 type clientSwaggerSchema struct {
 	c        *client.Client
 	cacheDir string

pkg/kubectl/cmd/util/factory_test.go

@@ -102,6 +102,48 @@ func TestPortsForObject(t *testing.T) {
 	}
 }
 
+func TestProtocolsForObject(t *testing.T) {
+	f := NewFactory(nil)
+
+	pod := &api.Pod{
+		ObjectMeta: api.ObjectMeta{Name: "baz", Namespace: "test", ResourceVersion: "12"},
+		Spec: api.PodSpec{
+			Containers: []api.Container{
+				{
+					Ports: []api.ContainerPort{
+						{
+							ContainerPort: 101,
+							Protocol:      api.ProtocolTCP,
+						},
+						{
+							ContainerPort: 102,
+							Protocol:      api.ProtocolUDP,
+						},
+					},
+				},
+			},
+		},
+	}
+
+	expected := "101/TCP,102/UDP"
+	protocolsMap, err := f.ProtocolsForObject(pod)
+	if err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+	got := kubectl.MakeProtocols(protocolsMap)
+	expectedSlice := strings.Split(expected, ",")
+	gotSlice := strings.Split(got, ",")
+
+	sort.Strings(expectedSlice)
+	sort.Strings(gotSlice)
+
+	for i, protocol := range gotSlice {
+		if protocol != expectedSlice[i] {
+			t.Fatalf("Protocols mismatch! Expected %s, got %s", expectedSlice[i], protocol)
+		}
+	}
+}
+
 func TestLabelsForObject(t *testing.T) {
 	f := NewFactory(nil)
 

pkg/kubectl/generate.go

@@ -134,6 +134,34 @@ func MakeParams(cmd *cobra.Command, params []GeneratorParam) map[string]interfac
 	return result
 }
 
+func MakeProtocols(protocols map[string]string) string {
+	out := []string{}
+	for key, value := range protocols {
+		out = append(out, fmt.Sprintf("%s/%s", key, value))
+	}
+	return strings.Join(out, ",")
+}
+
+func ParseProtocols(protocols interface{}) (map[string]string, error) {
+	protocolsString, isString := protocols.(string)
+	if !isString {
+		return nil, fmt.Errorf("expected string, found %v", protocols)
+	}
+	if len(protocolsString) == 0 {
+		return nil, fmt.Errorf("no protocols passed")
+	}
+	portProtocolMap := map[string]string{}
+	protocolsSlice := strings.Split(protocolsString, ",")
+	for ix := range protocolsSlice {
+		portProtocol := strings.Split(protocolsSlice[ix], "/")
+		if len(portProtocol) != 2 {
+			return nil, fmt.Errorf("unexpected port protocol mapping: %s", protocolsSlice[ix])
+		}
+		portProtocolMap[portProtocol[0]] = portProtocol[1]
+	}
+	return portProtocolMap, nil
+}
+
 func MakeLabels(labels map[string]string) string {
 	out := []string{}
 	for key, value := range labels {

pkg/kubectl/service.go

@@ -65,6 +65,9 @@ func paramNames() []GeneratorParam {
 		{"load-balancer-ip", false},
 		{"type", false},
 		{"protocol", false},
+		// protocols will be used to keep port-protocol mapping derived from
+		// exposed object
+		{"protocols", false},
 		{"container-port", false}, // alias of target-port
 		{"target-port", false},
 		{"port-name", false},
@@ -112,6 +115,15 @@ func generate(genericParams map[string]interface{}) (runtime.Object, error) {
 		// Leave the port unnamed.
 		servicePortName = ""
 	}
+
+	protocolsString, found := params["protocols"]
+	var portProtocolMap map[string]string
+	if found && len(protocolsString) > 0 {
+		portProtocolMap, err = ParseProtocols(protocolsString)
+		if err != nil {
+			return nil, err
+		}
+	}
 	// 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.
@@ -122,6 +134,7 @@ func generate(genericParams map[string]interface{}) (runtime.Object, error) {
 			return nil, fmt.Errorf("'port' is a required parameter.")
 		}
 	}
+
 	portStringSlice := strings.Split(portString, ",")
 	for i, stillPortString := range portStringSlice {
 		port, err := strconv.Atoi(stillPortString)
@@ -134,10 +147,26 @@ func generate(genericParams map[string]interface{}) (runtime.Object, error) {
 		if len(portStringSlice) > 1 {
 			name = fmt.Sprintf("port-%d", i+1)
 		}
+		protocol := params["protocol"]
+
+		switch {
+		case len(protocol) == 0 && len(portProtocolMap) == 0:
+			// Default to TCP, what the flag was doing previously.
+			protocol = "TCP"
+		case len(protocol) > 0 && len(portProtocolMap) > 0:
+			// User has specified the --protocol while exposing a multiprotocol resource
+			// We should stomp multiple protocols with the one specified ie. do nothing
+		case len(protocol) == 0 && len(portProtocolMap) > 0:
+			// no --protocol and we expose a multiprotocol resource
+			protocol = "TCP" // have the default so we can stay sane
+			if exposeProtocol, found := portProtocolMap[stillPortString]; found {
+				protocol = exposeProtocol
+			}
+		}
 		ports = append(ports, api.ServicePort{
 			Name:     name,
 			Port:     int32(port),
-			Protocol: api.Protocol(params["protocol"]),
+			Protocol: api.Protocol(protocol),
 		})
 	}