1. origin
  2. Commit af969529ab4af1776cd78dda61d142ee10d09cf9
Browse code

Bug 1275003: expose: Set route port based on service target port

Route should be using the port used from the endpoints of the service
and not the port the service is exposing.

kargakis authored on 2015/10/28 22:49:29
Showing 4 changed files
pkg/cmd/cli/cmd/expose.go
History View file @ af96952
... ... 
@@ -100,14 +100,15 @@ func validate(cmd *cobra.Command, f *clientcmd.Factory, args []string) error {
100 100 
 			if len(cmdutil.GetFlagString(cmd, "protocol")) == 0 {
101 101 
 				cmd.Flags().Set("protocol", "TCP")
102 102 
 			}
103 
-			return validateFlags(cmd, "service/v1")
103 
+			return validateFlags(cmd, generator)
104 104 
 		case "":
105 105 
 			// Default exposing services as a route
106 
-			cmd.Flags().Set("generator", "route/v1")
106 
+			generator = "route/v1"
107 
+			cmd.Flags().Set("generator", generator)
107 108 
 			fallthrough
108 109 
 		case "route/v1":
109 110 
 			// We need to validate services exposed as routes
110 
-			if err := validateFlags(cmd, "route/v1"); err != nil {
111 
+			if err := validateFlags(cmd, generator); err != nil {
111 112 
 				return err
112 113 
 			}
113 114 
 			svc, err := kc.Services(info.Namespace).Get(info.Name)
... ... 
@@ -118,12 +119,14 @@ func validate(cmd *cobra.Command, f *clientcmd.Factory, args []string) error {
118 118 
 			supportsTCP := false
119 119 
 			for _, port := range svc.Spec.Ports {
120 120 
 				if port.Protocol == kapi.ProtocolTCP {
121 
+					// Pass service target port as the route port
122 
+					cmd.Flags().Set("port", port.TargetPort.String())
121 123 
 					supportsTCP = true
122 124 
 					break
123 125 
 				}
124 126 
 			}
125 127 
 			if !supportsTCP {
126 
-				return fmt.Errorf("service %s doesn't support TCP", info.Name)
128 
+				return fmt.Errorf("service %q doesn't support TCP", info.Name)
127 129 
 			}
128 130 
 		}
129 131
... ... 
@@ -133,14 +136,15 @@ func validate(cmd *cobra.Command, f *clientcmd.Factory, args []string) error {
133 133 
 			return fmt.Errorf("cannot expose a %s as a route", mapping.Kind)
134 134 
 		case "":
135 135 
 			// Default exposing everything except services as a service
136 
-			cmd.Flags().Set("generator", "service/v1")
136 
+			generator = "service/v2"
137 
+			cmd.Flags().Set("generator", generator)
137 138 
 			fallthrough
138 139 
 		case "service/v1", "service/v2":
139 140 
 			// Set default protocol back for generating services
140 141 
 			if len(cmdutil.GetFlagString(cmd, "protocol")) == 0 {
141 142 
 				cmd.Flags().Set("protocol", "TCP")
142 143 
 			}
143 
-			return validateFlags(cmd, "service/v1")
144 
+			return validateFlags(cmd, generator)
144 145 
 		}
145 146 
 	}
146 147
... ... 
@@ -179,6 +183,12 @@ func validateFlags(cmd *cobra.Command, generator string) error {
179 179 
 		if len(cmdutil.GetFlagString(cmd, "port")) != 0 {
180 180 
 			invalidFlags = append(invalidFlags, "--port")
181 181 
 		}
182 
+		if len(cmdutil.GetFlagString(cmd, "load-balancer-ip")) != 0 {
183 
+			invalidFlags = append(invalidFlags, "--load-balancer-ip")
184 
+		}
185 
+		if len(cmdutil.GetFlagString(cmd, "session-affinity")) != 0 {
186 
+			invalidFlags = append(invalidFlags, "--session-affinity")
187 
+		}
182 188 
 		if cmdutil.GetFlagBool(cmd, "create-external-load-balancer") {
183 189 
 			invalidFlags = append(invalidFlags, "--create-external-load-balancer")
184 190 
 		}
pkg/route/generator/generate.go
History View file @ af96952
... ... 
@@ -2,7 +2,6 @@ package generator
2 2
3 3 
 import (
4 4 
 	"fmt"
5 
-	"strconv"
6 5 
 	"strings"
7 6
8 7 
 	kapi "k8s.io/kubernetes/pkg/api"
... ... 
@@ -68,10 +67,8 @@ func (RouteGenerator) Generate(genericParams map[string]interface{}) (runtime.Ob
68 68 
 	if !found || len(portString) == 0 {
69 69 
 		portString = strings.Split(params["ports"], ",")[0]
70 70 
 	}
71 
-
72 
-	port, err := strconv.ParseInt(portString, 10, 0)
73 
-	if err != nil {
74 
-		return nil, err
71 
+	if len(portString) == 0 {
72 
+		return nil, fmt.Errorf("exposed service does not have any target ports specified")
75 73 
 	}
76 74
77 75 
 	return &api.Route{
... ... 
@@ -85,10 +82,7 @@ func (RouteGenerator) Generate(genericParams map[string]interface{}) (runtime.Ob
85 85 
 				Name: params["default-name"],
86 86 
 			},
87 87 
 			Port: &api.RoutePort{
88 
-				TargetPort: util.IntOrString{
89 
-					Kind:   util.IntstrInt,
90 
-					IntVal: int(port),
91 
-				},
88 
+				TargetPort: util.NewIntOrStringFromString(portString),
92 89 
 			},
93 90 
 		},
94 91 
 	}, nil
pkg/route/generator/generate_test.go
History View file @ af96952
... ... 
@@ -39,8 +39,8 @@ func TestGenerateRoute(t *testing.T) {
39 39 
 					},
40 40 
 					Port: &routeapi.RoutePort{
41 41 
 						TargetPort: util.IntOrString{
42 
-							Kind:   util.IntstrInt,
43 
-							IntVal: 80,
42 
+							Kind:   util.IntstrString,
43 
+							StrVal: "80",
44 44 
 						},
45 45 
 					},
46 46 
 				},
... ... 
@@ -68,8 +68,8 @@ func TestGenerateRoute(t *testing.T) {
68 68 
 					},
69 69 
 					Port: &routeapi.RoutePort{
70 70 
 						TargetPort: util.IntOrString{
71 
-							Kind:   util.IntstrInt,
72 
-							IntVal: 80,
71 
+							Kind:   util.IntstrString,
72 
+							StrVal: "80",
73 73 
 						},
74 74 
 					},
75 75 
 				},
test/cmd/basicresources.sh
History View file @ af96952
... ... 
@@ -78,7 +78,7 @@ oc delete svc external
78 78 
 # Expose multiport service and verify we set a port in the route
79 79 
 oc create -f test/fixtures/multiport-service.yaml
80 80 
 oc expose svc/frontend --name route-with-set-port
81 
-os::util::get_object_assert 'route route-with-set-port' "{{.spec.port.targetPort}}" '5432'
81 
+os::util::get_object_assert 'route route-with-set-port' "{{.spec.port.targetPort}}" '8080'
82 82 
 echo "expose: ok"
83 83
84 84 
 oc delete all --all