From facb7647fd15d44e4e28269d5adce46bae558a1e Mon Sep 17 00:00:00 2001 From: Vighneswar Rao Bojja Date: Tue, 13 Aug 2019 16:47:54 +0530 Subject: [PATCH] Add slow-start support in vs/vsr --- docs/virtualserver-and-virtualserverroute.md | 1 + internal/configs/version2/config.go | 1 + .../version2/nginx-plus.virtualserver.tmpl | 2 +- internal/configs/version2/templates_test.go | 1 + internal/configs/virtualserver.go | 41 ++++++++++-- internal/configs/virtualserver_test.go | 62 +++++++++++++++++-- pkg/apis/configuration/v1alpha1/types.go | 1 + .../configuration/validation/validation.go | 1 + 8 files changed, 101 insertions(+), 9 deletions(-) diff --git a/docs/virtualserver-and-virtualserverroute.md b/docs/virtualserver-and-virtualserverroute.md index 28cc3a56da..ea88c8d624 100644 --- a/docs/virtualserver-and-virtualserverroute.md +++ b/docs/virtualserver-and-virtualserverroute.md @@ -217,6 +217,7 @@ tls: | `client-max-body-size` | Sets the maximum allowed size of the client request body. See the [client_max_body_size](https://nginx.org/en/docs/http/ngx_http_core_module.html#client_max_body_size) directive. The default is set in the `client-max-body-size` ConfigMap key. | `string` | No | | `tls` | The TLS configuration for the Upstream. | [`tls`](#UpstreamTLS) | No | | `healthCheck` | The health check configuration for the Upstream. See the [health_check](http://nginx.org/en/docs/http/ngx_http_upstream_hc_module.html#health_check) directive. Note: this feature is supported only in NGINX Plus. | [`healthcheck`](#UpstreamHealthcheck) | No | +| `slow-start` | The slow start allows an upstream server to gradually recover its weight from 0 to its nominal value after it has been recovered or became available or when the server becomes available after a period of time it was considered unavailable. By default, the slow start is disabled. See the [slow_start](https://nginx.org/en/docs/http/ngx_http_upstream_module.html#slow_start) parameter of the server directive. Note: The parameter cannot be used along with the `random`, `hash` or `ip_hash` load balancing methods and will be ignored. | `string` | No | ### Upstream.TLS diff --git a/internal/configs/version2/config.go b/internal/configs/version2/config.go index b7ad0f86b4..a7df8b3321 100644 --- a/internal/configs/version2/config.go +++ b/internal/configs/version2/config.go @@ -25,6 +25,7 @@ type UpstreamServer struct { MaxConns int FailTimeout string Resolve bool + SlowStart string } // Server defines a server. diff --git a/internal/configs/version2/nginx-plus.virtualserver.tmpl b/internal/configs/version2/nginx-plus.virtualserver.tmpl index a7f55e2273..c21426bdd4 100644 --- a/internal/configs/version2/nginx-plus.virtualserver.tmpl +++ b/internal/configs/version2/nginx-plus.virtualserver.tmpl @@ -5,7 +5,7 @@ upstream {{ $u.Name }} { {{ if $u.LBMethod }}{{ $u.LBMethod }};{{ end }} {{ range $s := $u.Servers }} - server {{ $s.Address }} max_fails={{ $s.MaxFails }} fail_timeout={{ $s.FailTimeout }} max_conns={{ $s.MaxConns }}{{ if $s.Resolve }} resolve{{ end }}; + server {{ $s.Address }} max_fails={{ $s.MaxFails }} fail_timeout={{ $s.FailTimeout }}{{ if $s.SlowStart }} slow_start={{ $s.SlowStart }}{{ end }} max_conns={{ $s.MaxConns }}{{ if $s.Resolve }} resolve{{ end }}; {{ end }} {{ if $u.Keepalive }} diff --git a/internal/configs/version2/templates_test.go b/internal/configs/version2/templates_test.go index 24ec37735c..d5dd077bfd 100644 --- a/internal/configs/version2/templates_test.go +++ b/internal/configs/version2/templates_test.go @@ -15,6 +15,7 @@ var virtualServerCfg = VirtualServerConfig{ MaxFails: 5, FailTimeout: "10s", MaxConns: 31, + SlowStart: "10s", }, }, LBMethod: "random", diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go index 647ed3ad83..b306f41577 100644 --- a/internal/configs/virtualserver.go +++ b/internal/configs/virtualserver.go @@ -14,6 +14,15 @@ import ( const nginx502Server = "unix:/var/run/nginx-502-server.sock" +var incompatibleLBMethodsForSlowStart = map[string]bool{ + "random": true, + "ip_hash": true, + "random two": true, + "random two least_conn": true, + "random two least_time=header": true, + "random two least_time=last_byte": true, +} + // VirtualServerEx holds a VirtualServer along with the resources that are referenced in this VirtualServer. type VirtualServerEx struct { VirtualServer *conf_v1alpha1.VirtualServer @@ -211,7 +220,7 @@ func generateVirtualServerConfig(virtualServerEx *VirtualServerEx, tlsPemFileNam endpoints := generateEndpointsForUpstream(upstreamNamespace, u, virtualServerEx, isResolverConfigured, isPlus) // isExternalNameSvc is always false for OSS _, isExternalNameSvc := virtualServerEx.ExternalNameSvcs[GenerateExternalNameSvcKey(upstreamNamespace, u.Service)] - ups := generateUpstream(upstreamName, u, isExternalNameSvc, endpoints, baseCfgParams) + ups := generateUpstream(upstreamName, u, isExternalNameSvc, endpoints, baseCfgParams, isPlus) upstreams = append(upstreams, ups) crUpstreams[upstreamName] = u @@ -231,7 +240,7 @@ func generateVirtualServerConfig(virtualServerEx *VirtualServerEx, tlsPemFileNam endpoints := generateEndpointsForUpstream(upstreamNamespace, u, virtualServerEx, isResolverConfigured, isPlus) // isExternalNameSvc is always false for OSS _, isExternalNameSvc := virtualServerEx.ExternalNameSvcs[GenerateExternalNameSvcKey(upstreamNamespace, u.Service)] - ups := generateUpstream(upstreamName, u, isExternalNameSvc, endpoints, baseCfgParams) + ups := generateUpstream(upstreamName, u, isExternalNameSvc, endpoints, baseCfgParams, isPlus) upstreams = append(upstreams, ups) crUpstreams[upstreamName] = u @@ -332,8 +341,9 @@ func generateVirtualServerConfig(virtualServerEx *VirtualServerEx, tlsPemFileNam } } -func generateUpstream(upstreamName string, upstream conf_v1alpha1.Upstream, isExternalNameSvc bool, endpoints []string, cfgParams *ConfigParams) version2.Upstream { +func generateUpstream(upstreamName string, upstream conf_v1alpha1.Upstream, isExternalNameSvc bool, endpoints []string, cfgParams *ConfigParams, isPlus bool) version2.Upstream { var upsServers []version2.UpstreamServer + lbMethod := generateLBMethod(upstream.LBMethod, cfgParams.LBMethod) for _, e := range endpoints { s := version2.UpstreamServer{ @@ -343,18 +353,41 @@ func generateUpstream(upstreamName string, upstream conf_v1alpha1.Upstream, isEx MaxConns: generateIntFromPointer(upstream.MaxConns, cfgParams.MaxConns), Resolve: isExternalNameSvc, } + + if isPlus { + s.SlowStart = generateSlowStartForPlus(upstream, lbMethod) + } + upsServers = append(upsServers, s) } return version2.Upstream{ Name: upstreamName, Servers: upsServers, - LBMethod: generateLBMethod(upstream.LBMethod, cfgParams.LBMethod), + LBMethod: lbMethod, Keepalive: generateIntFromPointer(upstream.Keepalive, cfgParams.Keepalive), UpstreamZoneSize: cfgParams.UpstreamZoneSize, } } +func generateSlowStartForPlus(upstream conf_v1alpha1.Upstream, lbMethod string) string { + if upstream.SlowStart == "" { + return "" + } + + if strings.HasPrefix(lbMethod, "hash") { + glog.Warningf("slow-start will be disabled for the Upstream %v", upstream.Name) + return "" + } + + if _, exists := incompatibleLBMethodsForSlowStart[lbMethod]; exists { + glog.Warningf("slow-start will be disabled for the Upstream %v", upstream.Name) + return "" + } + + return upstream.SlowStart +} + func generateLBMethod(method string, defaultMethod string) string { if method == "" { return defaultMethod diff --git a/internal/configs/virtualserver_test.go b/internal/configs/virtualserver_test.go index 660b501d79..9eb0065aab 100644 --- a/internal/configs/virtualserver_test.go +++ b/internal/configs/virtualserver_test.go @@ -767,7 +767,7 @@ func TestGenerateVirtualServerConfigForVirtualServerWithRules(t *testing.T) { func TestGenerateUpstream(t *testing.T) { name := "test-upstream" - upstream := conf_v1alpha1.Upstream{Service: name, Port: 80} + upstream := conf_v1alpha1.Upstream{Service: name, Port: 80, SlowStart: "10s"} endpoints := []string{ "192.168.10.10:8080", } @@ -788,6 +788,7 @@ func TestGenerateUpstream(t *testing.T) { MaxFails: 1, MaxConns: 0, FailTimeout: "10s", + SlowStart: "", }, }, LBMethod: "random", @@ -795,7 +796,7 @@ func TestGenerateUpstream(t *testing.T) { UpstreamZoneSize: "256k", } - result := generateUpstream(name, upstream, false, endpoints, &cfgParams) + result := generateUpstream(name, upstream, false, endpoints, &cfgParams, true) if !reflect.DeepEqual(result, expected) { t.Errorf("generateUpstream() returned %v but expected %v", result, expected) } @@ -859,7 +860,7 @@ func TestGenerateUpstreamWithKeepalive(t *testing.T) { } for _, test := range tests { - result := generateUpstream(name, test.upstream, false, endpoints, test.cfgParams) + result := generateUpstream(name, test.upstream, false, endpoints, test.cfgParams, false) if !reflect.DeepEqual(result, test.expected) { t.Errorf("generateUpstream() returned %v but expected %v for the case of %v", result, test.expected, test.msg) } @@ -882,7 +883,7 @@ func TestGenerateUpstreamForExternalNameService(t *testing.T) { }, } - result := generateUpstream(name, upstream, true, endpoints, &cfgParams) + result := generateUpstream(name, upstream, true, endpoints, &cfgParams, false) if !reflect.DeepEqual(result, expected) { t.Errorf("generateUpstream() returned %v but expected %v", result, expected) } @@ -1962,3 +1963,56 @@ func TestGenerateEndpointsForUpstream(t *testing.T) { } } } + +func TestGenerateSlowStartForPlusWithInCompatibleLBMethods(t *testing.T) { + serviceName := "test-slowstart-with-incompatible-LBMethods" + upstream := conf_v1alpha1.Upstream{Service: serviceName, Port: 80, SlowStart: "10s"} + expected := "" + + var tests = []string{ + "random", + "ip_hash", + "hash 123", + "random two", + "random two least_conn", + "random two least_time=header", + "random two least_time=last_byte", + } + + for _, lbMethod := range tests { + result := generateSlowStartForPlus(upstream, lbMethod) + + if !reflect.DeepEqual(result, expected) { + t.Errorf("generateSlowStartForPlus returned %v, but expected %v for lbMethod %v", result, expected, lbMethod) + } + } + +} + +func TestGenerateSlowStartForPlus(t *testing.T) { + serviceName := "test-slowstart" + + tests := []struct { + upstream conf_v1alpha1.Upstream + lbMethod string + expected string + }{ + { + upstream: conf_v1alpha1.Upstream{Service: serviceName, Port: 80, SlowStart: "", LBMethod: "least_conn"}, + lbMethod: "least_conn", + expected: "", + }, + { + upstream: conf_v1alpha1.Upstream{Service: serviceName, Port: 80, SlowStart: "10s", LBMethod: "least_conn"}, + lbMethod: "least_conn", + expected: "10s", + }, + } + + for _, test := range tests { + result := generateSlowStartForPlus(test.upstream, test.lbMethod) + if !reflect.DeepEqual(result, test.expected) { + t.Errorf("generateSlowStartForPlus returned %v, but expected %v", result, test.expected) + } + } +} diff --git a/pkg/apis/configuration/v1alpha1/types.go b/pkg/apis/configuration/v1alpha1/types.go index 31493c975f..8c2afee82b 100644 --- a/pkg/apis/configuration/v1alpha1/types.go +++ b/pkg/apis/configuration/v1alpha1/types.go @@ -42,6 +42,7 @@ type Upstream struct { ClientMaxBodySize string `json:"client-max-body-size"` TLS UpstreamTLS `json:"tls"` HealthCheck *HealthCheck `json:"healthCheck"` + SlowStart string `json:"slow-start"` } // UpstreamTLS defines a TLS configuration for an Upstream. diff --git a/pkg/apis/configuration/validation/validation.go b/pkg/apis/configuration/validation/validation.go index b821934263..e47e711ab8 100644 --- a/pkg/apis/configuration/validation/validation.go +++ b/pkg/apis/configuration/validation/validation.go @@ -331,6 +331,7 @@ func validateUpstreams(upstreams []v1alpha1.Upstream, fieldPath *field.Path, isP allErrs = append(allErrs, validatePositiveIntOrZeroFromPointer(u.MaxConns, idxPath.Child("max-conns"))...) allErrs = append(allErrs, validateSize(u.ClientMaxBodySize, idxPath.Child("client-max-body-size"))...) allErrs = append(allErrs, validateUpstreamHealthCheck(u.HealthCheck, idxPath.Child("healthCheck"))...) + allErrs = append(allErrs, validateTime(u.SlowStart, idxPath.Child("slow-start"))...) for _, msg := range validation.IsValidPortNum(int(u.Port)) { allErrs = append(allErrs, field.Invalid(idxPath.Child("port"), u.Port, msg))