diff --git a/docs/virtualserver-and-virtualserverroute.md b/docs/virtualserver-and-virtualserverroute.md index 40d3adff56..7b3648636a 100644 --- a/docs/virtualserver-and-virtualserverroute.md +++ b/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,6 +176,7 @@ 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 | @@ -171,6 +184,8 @@ port: 80 | `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 diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go index 9fe954fd82..393401a898 100644 --- a/internal/configs/virtualserver.go +++ b/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, diff --git a/internal/configs/virtualserver_test.go b/internal/configs/virtualserver_test.go index 3b5f95222d..e191fcbc3e 100644 --- a/internal/configs/virtualserver_test.go +++ b/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) + } + } +} diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index 0707ef9ed2..2c5cc4f012 100644 --- a/internal/k8s/controller.go +++ b/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)) diff --git a/pkg/apis/configuration/v1alpha1/types.go b/pkg/apis/configuration/v1alpha1/types.go index 2d3614857b..0ecd1817a1 100644 --- a/pkg/apis/configuration/v1alpha1/types.go +++ b/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. diff --git a/pkg/apis/configuration/validation/validation.go b/pkg/apis/configuration/validation/validation.go index 0014e6fdb3..e68bc3eced 100644 --- a/pkg/apis/configuration/validation/validation.go +++ b/pkg/apis/configuration/validation/validation.go @@ -5,6 +5,8 @@ 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" @@ -12,19 +14,19 @@ import ( ) // 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)...) diff --git a/pkg/apis/configuration/validation/validation_test.go b/pkg/apis/configuration/validation/validation_test.go index bacb1c46da..474bbfe345 100644 --- a/pkg/apis/configuration/validation/validation_test.go +++ b/pkg/apis/configuration/validation/validation_test.go @@ -23,9 +23,10 @@ func TestValidateVirtualServer(t *testing.T) { }, Upstreams: []v1alpha1.Upstream{ { - Name: "first", - Service: "service-1", - Port: 80, + Name: "first", + Service: "service-1", + LBMethod: "random", + Port: 80, }, { Name: "second", @@ -46,7 +47,7 @@ func TestValidateVirtualServer(t *testing.T) { }, } - err := ValidateVirtualServer(&virtualServer) + err := ValidateVirtualServer(&virtualServer, false) if err != nil { t.Errorf("ValidateVirtualServer() returned error %v for valid input %v", err, virtualServer) } @@ -148,9 +149,9 @@ func TestValidateUpstreams(t *testing.T) { msg: "2 valid upstreams", }, } - + isPlus := false for _, test := range tests { - allErrs, resultUpstreamNames := validateUpstreams(test.upstreams, field.NewPath("upstreams")) + allErrs, resultUpstreamNames := validateUpstreams(test.upstreams, field.NewPath("upstreams"), isPlus) if len(allErrs) > 0 { t.Errorf("validateUpstreams() returned errors %v for valid input for the case of %s", allErrs, test.msg) } @@ -223,8 +224,9 @@ func TestValidateUpstreamsFails(t *testing.T) { }, } + isPlus := false for _, test := range tests { - allErrs, resultUpstreamNames := validateUpstreams(test.upstreams, field.NewPath("upstreams")) + allErrs, resultUpstreamNames := validateUpstreams(test.upstreams, field.NewPath("upstreams"), isPlus) if len(allErrs) == 0 { t.Errorf("validateUpstreams() returned no errors for the case of %s", test.msg) } @@ -1218,8 +1220,8 @@ func TestValidateVirtualServerRoute(t *testing.T) { }, }, } - - err := ValidateVirtualServerRoute(&virtualServerRoute) + isPlus := false + err := ValidateVirtualServerRoute(&virtualServerRoute, isPlus) if err != nil { t.Errorf("ValidateVirtualServerRoute() returned error %v for valid input %v", err, virtualServerRoute) } @@ -1260,7 +1262,8 @@ func TestValidateVirtualServerRouteForVirtualServer(t *testing.T) { virtualServerHost := "example.com" pathPrefix := "/test" - err := ValidateVirtualServerRouteForVirtualServer(&virtualServerRoute, virtualServerHost, pathPrefix) + isPlus := false + err := ValidateVirtualServerRouteForVirtualServer(&virtualServerRoute, virtualServerHost, pathPrefix, isPlus) if err != nil { t.Errorf("ValidateVirtualServerRouteForVirtualServer() returned error %v for valid input %v", err, virtualServerRoute) } @@ -1378,3 +1381,59 @@ func TestValidateVirtualServerRouteSubroutesFails(t *testing.T) { } } } + +func TestValidateUpstreamLBMethod(t *testing.T) { + tests := []struct { + method string + isPlus bool + }{ + { + method: "round_robin", + isPlus: false, + }, + { + method: "", + isPlus: false, + }, + { + method: "ip_hash", + isPlus: true, + }, + { + method: "", + isPlus: true, + }, + } + + for _, test := range tests { + allErrs := validateUpstreamLBMethod(test.method, field.NewPath("lb-method"), test.isPlus) + + if len(allErrs) != 0 { + t.Errorf("validateUpstreamLBMethod() returned errors for method %s", test.method) + } + } +} + +func TestValidateUpstreamLBMethodFails(t *testing.T) { + tests := []struct { + method string + isPlus bool + }{ + { + method: "wrong", + isPlus: false, + }, + { + method: "wrong", + isPlus: true, + }, + } + + for _, test := range tests { + allErrs := validateUpstreamLBMethod(test.method, field.NewPath("lb-method"), test.isPlus) + + if len(allErrs) == 0 { + t.Errorf("validateUpstreamLBMethod() returned no errors for method %s", test.method) + } + } +}