Merge pull request #5457 from kargakis/expose-right-port

Merged by openshift-bot

Author: OpenShift Bot

pkg/cmd/cli/cmd/expose.go

@@ -100,14 +100,15 @@ func validate(cmd *cobra.Command, f *clientcmd.Factory, args []string) error {
 			if len(cmdutil.GetFlagString(cmd, "protocol")) == 0 {
 				cmd.Flags().Set("protocol", "TCP")
 			}
-			return validateFlags(cmd, "service/v1")
+			return validateFlags(cmd, generator)
 		case "":
 			// Default exposing services as a route
-			cmd.Flags().Set("generator", "route/v1")
+			generator = "route/v1"
+			cmd.Flags().Set("generator", generator)
 			fallthrough
 		case "route/v1":
 			// We need to validate services exposed as routes
-			if err := validateFlags(cmd, "route/v1"); err != nil {
+			if err := validateFlags(cmd, generator); err != nil {
 				return err
 			}
 			svc, err := kc.Services(info.Namespace).Get(info.Name)
@@ -118,12 +119,14 @@ func validate(cmd *cobra.Command, f *clientcmd.Factory, args []string) error {
 			supportsTCP := false
 			for _, port := range svc.Spec.Ports {
 				if port.Protocol == kapi.ProtocolTCP {
+					// Pass service target port as the route port
+					cmd.Flags().Set("port", port.TargetPort.String())
 					supportsTCP = true
 					break
 				}
 			}
 			if !supportsTCP {
-				return fmt.Errorf("service %s doesn't support TCP", info.Name)
+				return fmt.Errorf("service %q doesn't support TCP", info.Name)
 			}
 		}
 
@@ -133,14 +136,15 @@ func validate(cmd *cobra.Command, f *clientcmd.Factory, args []string) error {
 			return fmt.Errorf("cannot expose a %s as a route", mapping.Kind)
 		case "":
 			// Default exposing everything except services as a service
-			cmd.Flags().Set("generator", "service/v1")
+			generator = "service/v2"
+			cmd.Flags().Set("generator", generator)
 			fallthrough
 		case "service/v1", "service/v2":
 			// Set default protocol back for generating services
 			if len(cmdutil.GetFlagString(cmd, "protocol")) == 0 {
 				cmd.Flags().Set("protocol", "TCP")
 			}
-			return validateFlags(cmd, "service/v1")
+			return validateFlags(cmd, generator)
 		}
 	}
 
@@ -179,6 +183,12 @@ func validateFlags(cmd *cobra.Command, generator string) error {
 		if len(cmdutil.GetFlagString(cmd, "port")) != 0 {
 			invalidFlags = append(invalidFlags, "--port")
 		}
+		if len(cmdutil.GetFlagString(cmd, "load-balancer-ip")) != 0 {
+			invalidFlags = append(invalidFlags, "--load-balancer-ip")
+		}
+		if len(cmdutil.GetFlagString(cmd, "session-affinity")) != 0 {
+			invalidFlags = append(invalidFlags, "--session-affinity")
+		}
 		if cmdutil.GetFlagBool(cmd, "create-external-load-balancer") {
 			invalidFlags = append(invalidFlags, "--create-external-load-balancer")
 		}

pkg/route/generator/generate.go

@@ -2,7 +2,6 @@ package generator
 
 import (
 	"fmt"
-	"strconv"
 	"strings"
 
 	kapi "k8s.io/kubernetes/pkg/api"
@@ -68,10 +67,8 @@ func (RouteGenerator) Generate(genericParams map[string]interface{}) (runtime.Ob
 	if !found || len(portString) == 0 {
 		portString = strings.Split(params["ports"], ",")[0]
 	}
-
-	port, err := strconv.ParseInt(portString, 10, 0)
-	if err != nil {
-		return nil, err
+	if len(portString) == 0 {
+		return nil, fmt.Errorf("exposed service does not have any target ports specified")
 	}
 
 	return &api.Route{
@@ -85,10 +82,7 @@ func (RouteGenerator) Generate(genericParams map[string]interface{}) (runtime.Ob
 				Name: params["default-name"],
 			},
 			Port: &api.RoutePort{
-				TargetPort: util.IntOrString{
-					Kind:   util.IntstrInt,
-					IntVal: int(port),
-				},
+				TargetPort: util.NewIntOrStringFromString(portString),
 			},
 		},
 	}, nil

pkg/route/generator/generate_test.go

@@ -39,8 +39,8 @@ func TestGenerateRoute(t *testing.T) {
 					},
 					Port: &routeapi.RoutePort{
 						TargetPort: util.IntOrString{
-							Kind:   util.IntstrInt,
-							IntVal: 80,
+							Kind:   util.IntstrString,
+							StrVal: "80",
 						},
 					},
 				},
@@ -68,8 +68,8 @@ func TestGenerateRoute(t *testing.T) {
 					},
 					Port: &routeapi.RoutePort{
 						TargetPort: util.IntOrString{
-							Kind:   util.IntstrInt,
-							IntVal: 80,
+							Kind:   util.IntstrString,
+							StrVal: "80",
 						},
 					},
 				},

test/cmd/basicresources.sh

@@ -78,7 +78,7 @@ oc delete svc external
 # Expose multiport service and verify we set a port in the route
 oc create -f test/fixtures/multiport-service.yaml
 oc expose svc/frontend --name route-with-set-port
-os::util::get_object_assert 'route route-with-set-port' "{{.spec.port.targetPort}}" '5432'
+os::util::get_object_assert 'route route-with-set-port' "{{.spec.port.targetPort}}" '8080'
 echo "expose: ok"
 
 oc delete all --all