Add lb-method support in vs and vsr

Author: Raul Marrero

docs/virtualserver-and-virtualserverroute.md

@@ -7,11 +7,23 @@ This document is the reference documentation for the resources. To see additiona
 **Feature Status**: The VirtualServer and VirtualServerRoute resources are available as a preview feature: it is suitable for experimenting and testing; however, it must be used with caution in production environments. Additionally, while the feature is in preview, we might introduce some backward-incompatible changes to the resources specification in the next releases. 
 
 ## Contents
-* [Prerequisites](#Prerequisites)
-* [VirtualServer Specification](#VirtualServer-Specification)
-* [VirtualServerRoute Specification](#VirtualServerRoute-Specification)
-* [Using VirtualServer and VirtualServerRoute](#Using-VirtualServer-and-VirtualServerRoute)
-* [Customization via ConfigMap](#Customization-via-ConfigMap)
+- [VirtualServer and VirtualServerRoute Resources](#VirtualServer-and-VirtualServerRoute-Resources)
+  - [Contents](#Contents)
+  - [Prerequisites](#Prerequisites)
+  - [VirtualServer Specification](#VirtualServer-Specification)
+    - [VirtualServer.TLS](#VirtualServerTLS)
+    - [VirtualServer.Route](#VirtualServerRoute)
+  - [VirtualServerRoute Specification](#VirtualServerRoute-Specification)
+    - [VirtualServerRoute.Subroute](#VirtualServerRouteSubroute)
+  - [Common Parts of the VirtualServer and VirtualServerRoute](#Common-Parts-of-the-VirtualServer-and-VirtualServerRoute)
+    - [Upstream](#Upstream)
+    - [Split](#Split)
+    - [Rules](#Rules)
+    - [Condition](#Condition)
+    - [Match](#Match)
+  - [Using VirtualServer and VirtualServerRoute](#Using-VirtualServer-and-VirtualServerRoute)
+    - [Validation](#Validation)
+  - [Customization via ConfigMap](#Customization-via-ConfigMap)
 
 ## Prerequisites 
 
@@ -164,13 +176,16 @@ The upstream defines a destination for the routing configuration. For example:
 name: tea
 service: tea-svc
 port: 80
+lb-method: "round_robin"
 ``` 
 
 | Field | Description | Type | Required |
 | ----- | ----------- | ---- | -------- |
 | `name` | The name of the upstream. Must be a valid DNS label as defined in RFC 1035. For example, `hello` and `upstream-123` are valid. The name must be unique among all upstreams of the resource. | `string` | Yes |
 | `service` | The name of a [service](https://kubernetes.io/docs/concepts/services-networking/service/). The service must belong to the same namespace as the resource. If the service doesn't exist, NGINX will assume the service has zero endpoints and return a `502` response for requests for this upstream. | `string` | Yes |
 | `port` | The port of the service. If the service doesn't define that port, NGINX will assume the service has zero endpoints and return a `502` response for requests for this upstream. The port must fall into the range `1..65553`. | `uint16` | Yes |
+| `lb-method` | The load [balancing method](https://docs.nginx.com/nginx/admin-guide/load-balancer/http-load-balancer/#choosing-a-load-balancing-method). To use the round-robin method, specify `round_robin`. The default is specified in the `lb-method` ConfigMap key. | `string` | No |
+
 
 ### Split
 

internal/configs/virtualserver.go

@@ -92,7 +92,7 @@ func generateVirtualServerConfig(virtualServerEx *VirtualServerEx, tlsPemFileNam
 	for _, u := range virtualServerEx.VirtualServer.Spec.Upstreams {
 		upstreamName := virtualServerUpstreamNamer.GetNameForUpstream(u.Name)
 		endpointsKey := GenerateEndpointsKey(virtualServerEx.VirtualServer.Namespace, u.Service, u.Port)
-		ups := generateUpstream(upstreamName, virtualServerEx.Endpoints[endpointsKey], isPlus, baseCfgParams)
+		ups := generateUpstream(upstreamName, u.LBMethod, virtualServerEx.Endpoints[endpointsKey], isPlus, baseCfgParams)
 		upstreams = append(upstreams, ups)
 	}
 	// generate upstreams for each VirtualServerRoute
@@ -101,7 +101,7 @@ func generateVirtualServerConfig(virtualServerEx *VirtualServerEx, tlsPemFileNam
 		for _, u := range vsr.Spec.Upstreams {
 			upstreamName := upstreamNamer.GetNameForUpstream(u.Name)
 			endpointsKey := GenerateEndpointsKey(vsr.Namespace, u.Service, u.Port)
-			ups := generateUpstream(upstreamName, virtualServerEx.Endpoints[endpointsKey], isPlus, baseCfgParams)
+			ups := generateUpstream(upstreamName, u.LBMethod, virtualServerEx.Endpoints[endpointsKey], isPlus, baseCfgParams)
 			upstreams = append(upstreams, ups)
 		}
 	}
@@ -195,7 +195,7 @@ func generateVirtualServerConfig(virtualServerEx *VirtualServerEx, tlsPemFileNam
 	}
 }
 
-func generateUpstream(upstreamName string, endpoints []string, isPlus bool, cfgParams *ConfigParams) version2.Upstream {
+func generateUpstream(upstreamName string, lBMethod string, endpoints []string, isPlus bool, cfgParams *ConfigParams) version2.Upstream {
 	var upsServers []version2.UpstreamServer
 
 	for _, e := range endpoints {
@@ -219,12 +219,22 @@ func generateUpstream(upstreamName string, endpoints []string, isPlus bool, cfgP
 	ups := version2.Upstream{
 		Name:     upstreamName,
 		Servers:  upsServers,
-		LBMethod: cfgParams.LBMethod,
+		LBMethod: generateLBMethod(lBMethod, cfgParams.LBMethod),
 	}
 
 	return ups
 }
 
+func generateLBMethod(method string, defaultMethod string) string {
+	if method == "" {
+		return defaultMethod
+	} else if method == "round_robin" {
+		return ""
+	}
+
+	return method
+}
+
 func generateLocation(path string, upstreamName string, cfgParams *ConfigParams) version2.Location {
 	loc := version2.Location{
 		Path:                 path,

internal/configs/virtualserver_test.go

@@ -754,7 +754,7 @@ func TestGenerateUpstream(t *testing.T) {
 		LBMethod: "random",
 	}
 
-	result := generateUpstream(name, endpoints, isPlus, &cfgParams)
+	result := generateUpstream(name, "", endpoints, isPlus, &cfgParams)
 	if !reflect.DeepEqual(result, expected) {
 		t.Errorf("generateUpstream() returned %v but expected %v", result, expected)
 	}
@@ -780,7 +780,7 @@ func TestGenerateUpstreamForZeroEndpoints(t *testing.T) {
 		},
 	}
 
-	result := generateUpstream(name, endpoints, isPlus, &cfgParams)
+	result := generateUpstream(name, "", endpoints, isPlus, &cfgParams)
 	if !reflect.DeepEqual(result, expectedForNGINX) {
 		t.Errorf("generateUpstream(isPlus=%v) returned %v but expected %v", isPlus, result, expectedForNGINX)
 	}
@@ -791,7 +791,7 @@ func TestGenerateUpstreamForZeroEndpoints(t *testing.T) {
 		Servers: nil,
 	}
 
-	result = generateUpstream(name, endpoints, isPlus, &cfgParams)
+	result = generateUpstream(name, "", endpoints, isPlus, &cfgParams)
 	if !reflect.DeepEqual(result, expectedForNGINXPlus) {
 		t.Errorf("generateUpstream(isPlus=%v) returned %v but expected %v", isPlus, result, expectedForNGINXPlus)
 	}
@@ -1436,3 +1436,31 @@ func TestGetNameForSourceForRulesRouteMapFromCondition(t *testing.T) {
 		}
 	}
 }
+
+func TestGenerateLBMethod(t *testing.T) {
+	defaultMethod := "random two least_conn"
+
+	tests := []struct {
+		input    string
+		expected string
+	}{
+		{
+			input:    "",
+			expected: defaultMethod,
+		},
+		{
+			input:    "round_robin",
+			expected: "",
+		},
+		{
+			input:    "random",
+			expected: "random",
+		},
+	}
+	for _, test := range tests {
+		result := generateLBMethod(test.input, defaultMethod)
+		if result != test.expected {
+			t.Errorf("generateLBMethod() returned %q but expected %q for input '%v'", result, test.expected, test.input)
+		}
+	}
+}

internal/k8s/controller.go

@@ -590,7 +590,7 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) {
 
 	vs := obj.(*conf_v1alpha1.VirtualServer)
 
-	validationErr := validation.ValidateVirtualServer(vs)
+	validationErr := validation.ValidateVirtualServer(vs, lbc.isNginxPlus)
 	if validationErr != nil {
 		err := lbc.configurator.DeleteVirtualServer(key)
 		if err != nil {
@@ -648,7 +648,7 @@ func (lbc *LoadBalancerController) syncVirtualServerRoute(task task) {
 
 	vsr := obj.(*conf_v1alpha1.VirtualServerRoute)
 
-	validationErr := validation.ValidateVirtualServerRoute(vsr)
+	validationErr := validation.ValidateVirtualServerRoute(vsr, lbc.isNginxPlus)
 	if validationErr != nil {
 		lbc.recorder.Eventf(vsr, api_v1.EventTypeWarning, "Rejected", "VirtualServerRoute %s is invalid and was rejected: %v", key, validationErr)
 	}
@@ -1271,7 +1271,7 @@ func (lbc *LoadBalancerController) getVirtualServers() []*conf_v1alpha1.VirtualS
 	for _, obj := range lbc.virtualServerLister.List() {
 		vs := obj.(*conf_v1alpha1.VirtualServer)
 
-		err := validation.ValidateVirtualServer(vs)
+		err := validation.ValidateVirtualServer(vs, lbc.isNginxPlus)
 		if err != nil {
 			glog.V(3).Infof("Skipping invalid VirtualServer %s/%s: %v", vs.Namespace, vs.Name, err)
 			continue
@@ -1289,7 +1289,7 @@ func (lbc *LoadBalancerController) getVirtualServerRoutes() []*conf_v1alpha1.Vir
 	for _, obj := range lbc.virtualServerRouteLister.List() {
 		vsr := obj.(*conf_v1alpha1.VirtualServerRoute)
 
-		err := validation.ValidateVirtualServerRoute(vsr)
+		err := validation.ValidateVirtualServerRoute(vsr, lbc.isNginxPlus)
 		if err != nil {
 			glog.V(3).Infof("Skipping invalid VirtualServerRoute %s/%s: %v", vsr.Namespace, vsr.Name, err)
 			continue
@@ -1541,7 +1541,7 @@ func (lbc *LoadBalancerController) createVirtualServer(virtualServer *conf_v1alp
 
 		vsr := obj.(*conf_v1alpha1.VirtualServerRoute)
 
-		err = validation.ValidateVirtualServerRouteForVirtualServer(vsr, virtualServer.Spec.Host, r.Path)
+		err = validation.ValidateVirtualServerRouteForVirtualServer(vsr, virtualServer.Spec.Host, r.Path, lbc.isNginxPlus)
 		if err != nil {
 			glog.Warningf("VirtualServer %s/%s references invalid VirtualServerRoute %s: %v", virtualServer.Name, virtualServer.Namespace, vsrKey, err)
 			virtualServerRouteErrors = append(virtualServerRouteErrors, newVirtualServerRouteErrorFromVSR(vsr, err))

pkg/apis/configuration/v1alpha1/types.go

@@ -25,9 +25,10 @@ type VirtualServerSpec struct {
 
 // Upstream defines an upstream.
 type Upstream struct {
-	Name    string `json:"name"`
-	Service string `json:"service"`
-	Port    uint16 `json:"port"`
+	Name     string `json:"name"`
+	Service  string `json:"service"`
+	Port     uint16 `json:"port"`
+	LBMethod string `json:"lb-method"`
 }
 
 // Route defines a route.

pkg/apis/configuration/validation/validation.go

@@ -5,26 +5,28 @@ import (
 	"regexp"
 	"strings"
 
+	"github.com/nginxinc/kubernetes-ingress/internal/configs"
+
 	"github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1alpha1"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 )
 
 // ValidateVirtualServer validates a VirtualServer.
-func ValidateVirtualServer(virtualServer *v1alpha1.VirtualServer) error {
-	allErrs := validateVirtualServerSpec(&virtualServer.Spec, field.NewPath("spec"))
+func ValidateVirtualServer(virtualServer *v1alpha1.VirtualServer, isPlus bool) error {
+	allErrs := validateVirtualServerSpec(&virtualServer.Spec, field.NewPath("spec"), isPlus)
 	return allErrs.ToAggregate()
 }
 
 // validateVirtualServerSpec validates a VirtualServerSpec.
-func validateVirtualServerSpec(spec *v1alpha1.VirtualServerSpec, fieldPath *field.Path) field.ErrorList {
+func validateVirtualServerSpec(spec *v1alpha1.VirtualServerSpec, fieldPath *field.Path, isPlus bool) field.ErrorList {
 	allErrs := field.ErrorList{}
 
 	allErrs = append(allErrs, validateHost(spec.Host, fieldPath.Child("host"))...)
 	allErrs = append(allErrs, validateTLS(spec.TLS, fieldPath.Child("tls"))...)
 
-	upstreamErrs, upstreamNames := validateUpstreams(spec.Upstreams, fieldPath.Child("upstreams"))
+	upstreamErrs, upstreamNames := validateUpstreams(spec.Upstreams, fieldPath.Child("upstreams"), isPlus)
 	allErrs = append(allErrs, upstreamErrs...)
 
 	allErrs = append(allErrs, validateVirtualServerRoutes(spec.Routes, fieldPath.Child("routes"), upstreamNames)...)
@@ -55,6 +57,27 @@ func validateTLS(tls *v1alpha1.TLS, fieldPath *field.Path) field.ErrorList {
 	return validateSecretName(tls.Secret, fieldPath.Child("secret"))
 }
 
+func validateUpstreamLBMethod(lBMethod string, fieldPath *field.Path, isPlus bool) field.ErrorList {
+	allErrs := field.ErrorList{}
+	if lBMethod == "" {
+		return allErrs
+	}
+
+	if isPlus {
+		_, err := configs.ParseLBMethodForPlus(lBMethod)
+		if err != nil {
+			return append(allErrs, field.Invalid(fieldPath, lBMethod, err.Error()))
+		}
+	} else {
+		_, err := configs.ParseLBMethod(lBMethod)
+		if err != nil {
+			return append(allErrs, field.Invalid(fieldPath, lBMethod, err.Error()))
+		}
+	}
+
+	return allErrs
+}
+
 // validateSecretName checks if a secret name is valid.
 // It performs the same validation as ValidateSecretName from k8s.io/kubernetes/pkg/apis/core/validation/validation.go.
 func validateSecretName(name string, fieldPath *field.Path) field.ErrorList {
@@ -71,7 +94,7 @@ func validateSecretName(name string, fieldPath *field.Path) field.ErrorList {
 	return allErrs
 }
 
-func validateUpstreams(upstreams []v1alpha1.Upstream, fieldPath *field.Path) (allErrs field.ErrorList, upstreamNames sets.String) {
+func validateUpstreams(upstreams []v1alpha1.Upstream, fieldPath *field.Path, isPlus bool) (allErrs field.ErrorList, upstreamNames sets.String) {
 	allErrs = field.ErrorList{}
 	upstreamNames = sets.String{}
 
@@ -88,6 +111,7 @@ func validateUpstreams(upstreams []v1alpha1.Upstream, fieldPath *field.Path) (al
 		}
 
 		allErrs = append(allErrs, validateServiceName(u.Service, idxPath.Child("service"))...)
+		allErrs = append(allErrs, validateUpstreamLBMethod(u.LBMethod, idxPath.Child("lb-method"), isPlus)...)
 
 		for _, msg := range validation.IsValidPortNum(int(u.Port)) {
 			allErrs = append(allErrs, field.Invalid(idxPath.Child("port"), u.Port, msg))
@@ -410,23 +434,23 @@ func isValidMatchValue(value string) []string {
 }
 
 // ValidateVirtualServerRoute validates a VirtualServerRoute.
-func ValidateVirtualServerRoute(virtualServerRoute *v1alpha1.VirtualServerRoute) error {
-	allErrs := validateVirtualServerRouteSpec(&virtualServerRoute.Spec, field.NewPath("spec"), "", "/")
+func ValidateVirtualServerRoute(virtualServerRoute *v1alpha1.VirtualServerRoute, isPlus bool) error {
+	allErrs := validateVirtualServerRouteSpec(&virtualServerRoute.Spec, field.NewPath("spec"), "", "/", isPlus)
 	return allErrs.ToAggregate()
 }
 
 // ValidateVirtualServerRouteForVirtualServer validates a VirtualServerRoute for a VirtualServer represented by its host and path prefix.
-func ValidateVirtualServerRouteForVirtualServer(virtualServerRoute *v1alpha1.VirtualServerRoute, virtualServerHost string, pathPrefix string) error {
-	allErrs := validateVirtualServerRouteSpec(&virtualServerRoute.Spec, field.NewPath("spec"), virtualServerHost, pathPrefix)
+func ValidateVirtualServerRouteForVirtualServer(virtualServerRoute *v1alpha1.VirtualServerRoute, virtualServerHost string, pathPrefix string, isPlus bool) error {
+	allErrs := validateVirtualServerRouteSpec(&virtualServerRoute.Spec, field.NewPath("spec"), virtualServerHost, pathPrefix, isPlus)
 	return allErrs.ToAggregate()
 }
 
-func validateVirtualServerRouteSpec(spec *v1alpha1.VirtualServerRouteSpec, fieldPath *field.Path, virtualServerHost string, pathPrefix string) field.ErrorList {
+func validateVirtualServerRouteSpec(spec *v1alpha1.VirtualServerRouteSpec, fieldPath *field.Path, virtualServerHost string, pathPrefix string, isPlus bool) field.ErrorList {
 	allErrs := field.ErrorList{}
 
 	allErrs = append(allErrs, validateVirtualServerRouteHost(spec.Host, virtualServerHost, fieldPath.Child("host"))...)
 
-	upstreamErrs, upstreamNames := validateUpstreams(spec.Upstreams, fieldPath.Child("upstreams"))
+	upstreamErrs, upstreamNames := validateUpstreams(spec.Upstreams, fieldPath.Child("upstreams"), isPlus)
 	allErrs = append(allErrs, upstreamErrs...)
 
 	allErrs = append(allErrs, validateVirtualServerRouteSubroutes(spec.Subroutes, fieldPath.Child("subroutes"), upstreamNames, pathPrefix)...)