api: move route.service.strategy to route.loadBalancerPolicy

Updates #1119
Fixes #1668

This PR moves the per service lb strategy to a per route lb policy.
The lb policy type provides a place to hold the load balancer policy
strategy.

Signed-off-by: Dave Cheney <dave@cheney.net>

apis/projectcontour/v1/httpproxy.go

@@ -122,10 +122,14 @@ type Route struct {
 	RetryPolicy *RetryPolicy `json:"retryPolicy,omitempty"`
 	// The health check policy for this route.
 	HealthCheckPolicy *HTTPHealthCheckPolicy `json:"healthCheckPolicy,omitempty"`
+	// The load balancing policy for this route.
+	LoadBalancerPolicy *LoadBalancerPolicy `json:"loadBalancerPolicy,omitempty"`
 }
 
 // TCPProxy contains the set of services to proxy TCP connections.
 type TCPProxy struct {
+	// The load balancing policy for the backend services.
+	LoadBalancerPolicy *LoadBalancerPolicy `json:"loadBalancerPolicy,omitempty"`
 	// Services are the services to proxy traffic
 	Services []Service `json:"services,omitempty"`
 	// Include specifies that this tcpproxy should be delegated to another HTTPProxy.
@@ -141,8 +145,6 @@ type Service struct {
 	Port int `json:"port"`
 	// Weight defines percentage of traffic to balance traffic
 	Weight uint32 `json:"weight,omitempty"`
-	// LB Algorithm to apply.
-	Strategy string `json:"strategy,omitempty"`
 	// UpstreamValidation defines how to verify the backend service's certificate
 	UpstreamValidation *UpstreamValidation `json:"validation,omitempty"`
 	// If Mirror is true the Service will receive a read only mirror of the traffic for this route.
@@ -188,6 +190,11 @@ type RetryPolicy struct {
 	PerTryTimeout string `json:"perTryTimeout,omitempty"`
 }
 
+// LoadBalancerPolicy defines the load balancing policy.
+type LoadBalancerPolicy struct {
+	Strategy string `json:"strategy,omitempty"`
+}
+
 // UpstreamValidation defines how to verify the backend service's certificate
 type UpstreamValidation struct {
 	// Name of the Kubernetes secret be used to validate the certificate presented by the backend

docs/ingressroute-to-httpproxy.md

@@ -234,11 +234,51 @@ See #899
 
 ### Load balancing strategies
 
-No change.
+Per service load balancing strategy has moved to a per route strategy that applies to all services for that route.
+
+Before:
+```yaml
+apiVersion: contour.heptio.com/v1beta1
+kind: IngressRoute
+metadata:
+  name: lb-strategy
+  namespace: default
+spec:
+  virtualhost:
+    fqdn: strategy.bar.com
+  routes:
+    - match: /
+      services:
+        - name: s1-strategy
+          port: 80
+          strategy: WeightedLeastRequest
+        - name: s2-strategy
+          port: 80
+          strategy: WeightedLeastRequest
+```
+After:
+```yaml
+apiVersion: projectcontour.io/v1
+kind: HTTPProxy
+metadata:
+  name: lb-strategy
+  namespace: default
+spec:
+  virtualhost:
+    fqdn: strategy.bar.com
+  routes:
+    - services:
+        - name: s1-strategy
+          port: 80
+        - name: s2-strategy
+          port: 80
+      loadBalancerStrategy:
+        strategy: WeightedLeastRequest
+```
 
 ### Session affinity
 
-No change.
+See above.
 
 ### Per upstream health check
 

examples/example-workload/httpproxy/03-routes/httpproxy-lb-strategy.yaml

@@ -13,4 +13,5 @@ spec:
           port: 80
         - name: s2-strategy
           port: 80
-          strategy: WeightedLeastRequest
+      loadBalancerPolicy:
+        strategy: WeightedLeastRequest

internal/dag/builder.go

@@ -604,11 +604,11 @@ func (b *Builder) computeRoutes(sw *ObjectStatusWriter, proxy *projcontour.HTTPP
 			}
 
 			c := &Cluster{
-				Upstream:             s,
-				LoadBalancerStrategy: service.Strategy,
-				Weight:               service.Weight,
-				HealthCheckPolicy:    healthCheckPolicy(route.HealthCheckPolicy),
-				UpstreamValidation:   uv,
+				Upstream:           s,
+				LoadBalancerPolicy: loadBalancerPolicy(route.LoadBalancerPolicy),
+				Weight:             service.Weight,
+				HealthCheckPolicy:  healthCheckPolicy(route.HealthCheckPolicy),
+				UpstreamValidation: uv,
 			}
 			if service.Mirror && r.MirrorPolicy != nil {
 				sw.SetInvalid("only one service per route may be nominated as mirror")
@@ -778,11 +778,11 @@ func (b *Builder) processIngressRoutes(sw *ObjectStatusWriter, ir *ingressroutev
 					}
 				}
 				r.Clusters = append(r.Clusters, &Cluster{
-					Upstream:             s,
-					LoadBalancerStrategy: service.Strategy,
-					Weight:               service.Weight,
-					HealthCheckPolicy:    ingressrouteHealthCheckPolicy(service.HealthCheck),
-					UpstreamValidation:   uv,
+					Upstream:           s,
+					LoadBalancerPolicy: service.Strategy,
+					Weight:             service.Weight,
+					HealthCheckPolicy:  ingressrouteHealthCheckPolicy(service.HealthCheck),
+					UpstreamValidation: uv,
 				})
 			}
 
@@ -878,8 +878,8 @@ func (b *Builder) processTCPProxy(sw *ObjectStatusWriter, ir *ingressroutev1.Ing
 				return
 			}
 			proxy.Clusters = append(proxy.Clusters, &Cluster{
-				Upstream:             s,
-				LoadBalancerStrategy: service.Strategy,
+				Upstream:           s,
+				LoadBalancerPolicy: service.Strategy,
 			})
 		}
 		b.lookupSecureVirtualHost(host).TCPProxy = &proxy
@@ -933,8 +933,8 @@ func (b *Builder) processTCPProxyHTTPProxy(sw *ObjectStatusWriter, httpproxy *pr
 				return
 			}
 			proxy.Clusters = append(proxy.Clusters, &Cluster{
-				Upstream:             s,
-				LoadBalancerStrategy: service.Strategy,
+				Upstream:           s,
+				LoadBalancerPolicy: loadBalancerPolicy(httpproxy.Spec.TCPProxy.LoadBalancerPolicy),
 			})
 		}
 		b.lookupSecureVirtualHost(host).TCPProxy = &proxy

internal/dag/dag.go

@@ -337,7 +337,7 @@ type Cluster struct {
 
 	// The load balancer type to use when picking a host in the cluster.
 	// See https://www.envoyproxy.io/docs/envoy/latest/api-v2/api/v2/cds.proto#envoy-api-enum-cluster-lbpolicy
-	LoadBalancerStrategy string
+	LoadBalancerPolicy string
 
 	// Cluster health check policy.
 	*HealthCheckPolicy

internal/dag/policy.go

@@ -119,6 +119,24 @@ func healthCheckPolicy(hc *projcontour.HTTPHealthCheckPolicy) *HealthCheckPolicy
 	}
 }
 
+// loadBalancerPolicy returns the load balancer strategy or
+// blank if no valid strategy is supplied.
+func loadBalancerPolicy(lbp *projcontour.LoadBalancerPolicy) string {
+	if lbp == nil {
+		return ""
+	}
+	switch lbp.Strategy {
+	case "WeightedLeastRequest":
+		return "WeightedLeastRequest"
+	case "Random":
+		return "Random"
+	case "Cookie":
+		return "Cookie"
+	default:
+		return ""
+	}
+}
+
 func parseTimeout(timeout string) time.Duration {
 	if timeout == "" {
 		// Blank is interpreted as no timeout specified, use envoy defaults

internal/dag/policy_test.go

@@ -340,6 +340,53 @@ func TestTimeoutPolicy(t *testing.T) {
 	}
 }
 
+func TestLoadBalancerPolicy(t *testing.T) {
+	tests := map[string]struct {
+		lbp  *projcontour.LoadBalancerPolicy
+		want string
+	}{
+		"nil": {
+			lbp:  nil,
+			want: "",
+		},
+		"empty": {
+			lbp:  &projcontour.LoadBalancerPolicy{},
+			want: "",
+		},
+		"WeightedLeastRequest": {
+			lbp: &projcontour.LoadBalancerPolicy{
+				Strategy: "WeightedLeastRequest",
+			},
+			want: "WeightedLeastRequest",
+		},
+		"Random": {
+			lbp: &projcontour.LoadBalancerPolicy{
+				Strategy: "Random",
+			},
+			want: "Random",
+		},
+		"Cookie": {
+			lbp: &projcontour.LoadBalancerPolicy{
+				Strategy: "Cookie",
+			},
+			want: "Cookie",
+		},
+		"unknown": {
+			lbp: &projcontour.LoadBalancerPolicy{
+				Strategy: "please",
+			},
+			want: "",
+		},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			got := loadBalancerPolicy(tc.lbp)
+			assert.Equal(t, tc.want, got)
+		})
+	}
+}
+
 func TestParseTimeout(t *testing.T) {
 	tests := map[string]struct {
 		duration string