support for zero weighted services in a route

Author: Rajat Chopra

images/router/haproxy/conf/haproxy-config.template

@@ -288,7 +288,8 @@ backend be_edge_http_{{$cfgIdx}}
   {{ end }}
   http-request set-header Forwarded for=%[src];host=%[req.hdr(host)];proto=%[req.hdr(X-Forwarded-Proto)]
     {{ range $serviceUnitName, $weight := $cfg.ServiceUnitNames }}
-      {{ with $serviceUnit := index $.ServiceUnits $serviceUnitName }}
+      {{ if ne $weight 0 }}
+        {{ with $serviceUnit := index $.ServiceUnits $serviceUnitName }}
           {{ range $idx, $endpoint := endpointsForAlias $cfg $serviceUnit }}
             {{ with $healthIntv := index $cfg.Annotations "router.openshift.io/haproxy.health.check.interval" }}
               {{ if (matchPattern "[1-9][0-9]*(us|ms|s|m|h|d)?" $healthIntv) }}
@@ -304,7 +305,8 @@ backend be_edge_http_{{$cfgIdx}}
               {{ end }}
             {{ end }}
           {{ end }}
-      {{ end }}
+        {{ end }}
+      {{ end }}{{/* end if weight != 0 */}}
     {{ end }}{{/* end iterate over services */}}
   {{ end }}{{/* end if tls==edge/none */}}
 
@@ -329,8 +331,9 @@ backend be_tcp_{{$cfgIdx}}
   hash-type consistent
   timeout check 5000ms
     {{ range $serviceUnitName, $weight := $cfg.ServiceUnitNames }}
-      {{ with $serviceUnit := index $.ServiceUnits $serviceUnitName }}
-        {{ range $idx, $endpoint := endpointsForAlias $cfg $serviceUnit }}
+      {{ if ne $weight 0 }}
+        {{ with $serviceUnit := index $.ServiceUnits $serviceUnitName }}
+          {{ range $idx, $endpoint := endpointsForAlias $cfg $serviceUnit }}
             {{ with $healthIntv := index $cfg.Annotations "router.openshift.io/haproxy.health.check.interval" }}
               {{ if (matchPattern "[1-9][0-9]*(us|ms|s|m|h|d)?" $healthIntv) }}
   server {{$endpoint.IdHash}} {{$endpoint.IP}}:{{$endpoint.Port}} check inter {{$healthIntv}} weight {{$weight}}
@@ -344,8 +347,9 @@ backend be_tcp_{{$cfgIdx}}
   server {{$endpoint.IdHash}} {{$endpoint.IP}}:{{$endpoint.Port}} check inter 5000ms weight {{$weight}}
               {{ end }}
             {{ end }}
-        {{ end }}
-      {{ end }}
+          {{ end }}{{/* end range endpointsForAlias */}}
+        {{ end }}{{/* end get ServiceUnit from serviceUnitName */}}
+      {{ end }}{{/* end if weight != 0 */}}
     {{ end }}{{/* end iterate over services*/}}
   {{ end }}{{/*end tls==passthrough*/}}
 
@@ -375,8 +379,9 @@ backend be_secure_{{$cfgIdx}}
   http-request set-header Forwarded for=%[src];host=%[req.hdr(host)];proto=%[req.hdr(X-Forwarded-Proto)]
   cookie {{$cfg.RoutingKeyName}} insert indirect nocache httponly secure
     {{ range $serviceUnitName, $weight := $cfg.ServiceUnitNames }}
-      {{ with $serviceUnit := index $.ServiceUnits $serviceUnitName }}
-        {{ range $idx, $endpoint := endpointsForAlias $cfg $serviceUnit }}
+      {{ if ne $weight 0 }}
+        {{ with $serviceUnit := index $.ServiceUnits $serviceUnitName }}
+          {{ range $idx, $endpoint := endpointsForAlias $cfg $serviceUnit }}
             {{ with $healthIntv := index $cfg.Annotations "router.openshift.io/haproxy.health.check.interval" }}
               {{ if (matchPattern "[1-9][0-9]*(us|ms|s|m|h|d)?" $healthIntv) }}
   server {{$endpoint.IdHash}} {{$endpoint.IP}}:{{$endpoint.Port}} ssl check inter {{$healthIntv}} verify required ca-file {{ $workingDir }}/cacerts/{{$cfgIdx}}.pem cookie {{$endpoint.IdHash}} weight {{$weight}}
@@ -389,10 +394,11 @@ backend be_secure_{{$cfgIdx}}
               {{ else }}
   server {{$endpoint.IdHash}} {{$endpoint.IP}}:{{$endpoint.Port}} ssl check inter 5000ms verify required ca-file {{$workingDir}}/cacerts/{{$cfgIdx}}.pem cookie {{$endpoint.IdHash}} weight {{$weight}}
               {{ end }}
-            {{ end }}
-        {{ end }}
-      {{ end }}
-    {{ end }}
+            {{ end }}{{/* end get health interval annotation */}}
+          {{ end }}{{/* end range endpointsForAlias */}}
+        {{ end }}{{/* end get serviceUnit from its name */}}
+      {{ end }}{{/* end if weight != 0 */}}
+    {{ end }}{{/* end range over serviceUnitNames */}}
   {{ end }}{{/* end tls==reencrypt */}}
 {{ end }}{{/* end loop over routes */}}
 {{ end }}{{/* end haproxy config template */}}

pkg/route/api/validation/validation.go

@@ -46,8 +46,8 @@ func ValidateRoute(route *routeapi.Route) field.ErrorList {
 	if route.Spec.To.Kind != "Service" {
 		result = append(result, field.Invalid(specPath.Child("to", "kind"), route.Spec.To.Kind, "must reference a Service"))
 	}
-	if route.Spec.To.Weight != nil && (*route.Spec.To.Weight < 1 || *route.Spec.To.Weight > 256) {
-		result = append(result, field.Invalid(specPath.Child("to", "weight"), route.Spec.To.Weight, "weight must be an integer between 1 and 256"))
+	if route.Spec.To.Weight != nil && (*route.Spec.To.Weight < 0 || *route.Spec.To.Weight > 256) {
+		result = append(result, field.Invalid(specPath.Child("to", "weight"), route.Spec.To.Weight, "weight must be an integer between 0 and 256"))
 	}
 
 	if len(route.Spec.AlternateBackends) > 3 {
@@ -60,8 +60,8 @@ func ValidateRoute(route *routeapi.Route) field.ErrorList {
 		if svc.Kind != "Service" {
 			result = append(result, field.Invalid(specPath.Child("alternateBackends", "kind"), svc.Kind, "must reference a Service"))
 		}
-		if svc.Weight != nil && (*svc.Weight < 1 || *svc.Weight > 256) {
-			result = append(result, field.Invalid(specPath.Child("alternateBackends", "weight"), svc.Weight, "weight must be an integer between 1 and 256"))
+		if svc.Weight != nil && (*svc.Weight < 0 || *svc.Weight > 256) {
+			result = append(result, field.Invalid(specPath.Child("alternateBackends", "weight"), svc.Weight, "weight must be an integer between 0 and 256"))
 		}
 	}
 

test/extended/router/weighted.go

@@ -96,6 +96,15 @@ var _ = g.Describe("[networking][router] weighted openshift router", func() {
 			if weightedRatio < 5 && weightedRatio > 0.2 {
 				e2e.Failf("Unexpected weighted ratio for incoming traffic: %v (%d/%d)", weightedRatio, trafficEP1, trafficEP2)
 			}
+
+			g.By(fmt.Sprintf("checking that zero weights are also respected by the router"))
+			host = "zeroweighted.example.com"
+			req, _ = requestViaReverseProxy("GET", routerURL, host)
+			resp, err = http.DefaultClient.Do(req)
+			o.Expect(err).NotTo(o.HaveOccurred())
+			if resp.StatusCode != http.StatusServiceUnavailable {
+				e2e.Failf("Expected zero weighted route to return a 503, but got %v", resp.StatusCode)
+			}
 		})
 	})
 })

test/extended/testdata/weighted-router.yaml

@@ -61,6 +61,29 @@ items:
     ports:
     - targetPort: 8080
 
+# a route that has multiple services but all weights are zero
+- apiVersion: v1
+  kind: Route
+  metadata:
+    name: zeroweightedroute
+    labels:
+      test: router
+      select: weighted
+    annotations:
+      haproxy.router.openshift.io/balance: roundrobin
+  spec:
+    host: zeroweighted.example.com
+    to:
+      name: weightedendpoints1
+      kind: Service
+      weight: 0
+    alternateBackends:
+    - name: weightedendpoints2
+      kind: Service
+      weight: 0
+    ports:
+    - targetPort: 8080
+
 # two services that can be routed to
 - apiVersion: v1
   kind: Service