Merge pull request #287 from isaachawley/slow-start-support

Support slow-start for Plus

Author: pleshakov

examples/customization/README.md

@@ -63,6 +63,7 @@ The table below summarizes all of the options. For some of them, there are examp
 | `nginx.com/health-checks` | N/A | Enables active health checks. | `False` | [Support for Active Health Checks](../health-checks). |
 | `nginx.com/health-checks-mandatory` | N/A | Configures active health checks as mandatory. | `False` | [Support for Active Health Checks](../health-checks). |
 | `nginx.com/health-checks-mandatory-queue` | N/A | When active health checks are mandatory, configures a queue for temporary storing incoming requests during the time when NGINX Plus is checking the health of the endpoints after a configuration reload. | `0` | [Support for Active Health Checks](../health-checks). |
+| `nginx.com/slow-start` | N/A | Sets the upstream server [slow-start period](https://docs.nginx.com/nginx/admin-guide/load-balancer/http-load-balancer/#server-slow-start). By default, slow-start is activated after a server becomes [available](https://docs.nginx.com/nginx/admin-guide/load-balancer/http-health-check/#passive-health-checks) or [healthy](https://docs.nginx.com/nginx/admin-guide/load-balancer/http-health-check/#active-health-checks). To enable slow-start for newly added servers, configure [mandatory active health checks](../health-checks). | `"0s"` | |
 
 ## Using ConfigMaps
 

nginx-controller/nginx/config.go

@@ -38,6 +38,7 @@ type Config struct {
 	HealthCheckEnabled            bool
 	HealthCheckMandatory          bool
 	HealthCheckMandatoryQueue     int64
+	SlowStart                     string
 
 	// http://nginx.org/en/docs/http/ngx_http_realip_module.html
 	RealIPHeader    string

nginx-controller/nginx/configurator.go

@@ -349,22 +349,38 @@ func (cnf *Configurator) createConfig(ingEx *IngressEx) Config {
 		}
 		if cnf.isPlus() {
 			ingCfg.HealthCheckEnabled = healthCheckEnabled
-			if healthCheckMandatory, exists, err := GetMapKeyAsBool(ingEx.Ingress.Annotations, "nginx.com/health-checks-mandatory", ingEx.Ingress); exists {
-				if err != nil {
-					glog.Error(err)
-				}
-				ingCfg.HealthCheckMandatory = healthCheckMandatory
-				if healthCheckQueue, exists, err := GetMapKeyAsInt(ingEx.Ingress.Annotations, "nginx.com/health-checks-mandatory-queue", ingEx.Ingress); exists {
-					if err != nil {
-						glog.Error(err)
-					}
-					ingCfg.HealthCheckMandatoryQueue = healthCheckQueue
-				}
-			}
 		} else {
 			glog.Warning("Annotation 'nginx.com/health-checks' requires NGINX Plus")
 		}
 	}
+	if ingCfg.HealthCheckEnabled {
+		if healthCheckMandatory, exists, err := GetMapKeyAsBool(ingEx.Ingress.Annotations, "nginx.com/health-checks-mandatory", ingEx.Ingress); exists {
+			if err != nil {
+				glog.Error(err)
+			}
+			ingCfg.HealthCheckMandatory = healthCheckMandatory
+		}
+	}
+	if ingCfg.HealthCheckMandatory {
+		if healthCheckQueue, exists, err := GetMapKeyAsInt(ingEx.Ingress.Annotations, "nginx.com/health-checks-mandatory-queue", ingEx.Ingress); exists {
+			if err != nil {
+				glog.Error(err)
+			}
+			ingCfg.HealthCheckMandatoryQueue = healthCheckQueue
+		}
+	}
+
+	if slowStart, exists := ingEx.Ingress.Annotations["nginx.com/slow-start"]; exists {
+		if parsedSlowStart, err := ParseSlowStart(slowStart); err != nil {
+			glog.Errorf("Ingress %s/%s: Invalid value nginx.org/slow-start: got %q: %v", ingEx.Ingress.GetNamespace(), ingEx.Ingress.GetName(), slowStart, err)
+		} else {
+			if cnf.isPlus() {
+				ingCfg.SlowStart = parsedSlowStart
+			} else {
+				glog.Warning("Annotation 'nginx.com/slow-start' requires NGINX Plus")
+			}
+		}
+	}
 
 	if serverTokens, exists, err := GetMapKeyAsBool(ingEx.Ingress.Annotations, "nginx.org/server-tokens", ingEx.Ingress); exists {
 		if err != nil {
@@ -732,6 +748,7 @@ func (cnf *Configurator) createUpstream(ingEx *IngressEx, name string, backend *
 				Port:        addressport[1],
 				MaxFails:    cfg.MaxFails,
 				FailTimeout: cfg.FailTimeout,
+				SlowStart:   cfg.SlowStart,
 			})
 		}
 		if len(upsServers) > 0 {
@@ -905,6 +922,7 @@ func (cnf *Configurator) updatePlusEndpoints(ingEx *IngressEx) error {
 	cfg := plus.ServerConfig{
 		MaxFails:    ingCfg.MaxFails,
 		FailTimeout: ingCfg.FailTimeout,
+		SlowStart:   ingCfg.SlowStart,
 	}
 
 	if ingEx.Ingress.Spec.Backend != nil {

nginx-controller/nginx/extensions.go

@@ -1,7 +1,9 @@
 package nginx
 
 import (
+	"errors"
 	"fmt"
+	"regexp"
 	"strings"
 )
 
@@ -59,3 +61,28 @@ func validateHashLBMethod(method string) (string, error) {
 	}
 	return "", fmt.Errorf("Invalid load balancing method: %q", method)
 }
+
+// http://nginx.org/en/docs/syntax.html
+var validTimeSuffixes = []string{
+	"ms",
+	"s",
+	"m",
+	"h",
+	"d",
+	"w",
+	"M",
+	"y",
+}
+
+var durationEscaped = strings.Join(validTimeSuffixes, "|")
+var validNginxTime = regexp.MustCompile(`^([0-9]+([` + durationEscaped + `]?){0,1} *)+$`)
+
+// ParseSlowStart ensures that the slow_start value in the annotation is valid.
+func ParseSlowStart(s string) (string, error) {
+	s = strings.TrimSpace(s)
+
+	if validNginxTime.MatchString(s) {
+		return s, nil
+	}
+	return "", errors.New("Invalid time string")
+}

nginx-controller/nginx/extensions_test.go

@@ -81,3 +81,24 @@ func TestParseLBMethodForPlus(t *testing.T) {
 		}
 	}
 }
+
+
+func TestParseSlowStart(t *testing.T) {
+	var testsWithValidInput = []string{"1", "1m10s", "11 11", "5m 30s", "1s", "100m", "5w", "15m", "11M", "3h", "100y", "600"}
+	var invalidInput = []string{"ss", "rM", "m0m", "s1s", "-5s", "", "1L"}
+	for _, test := range testsWithValidInput {
+		result, err := ParseSlowStart(test)
+		if err != nil {
+			t.Errorf("TestParseSlowStart(%q) returned an error for valid input", test)
+		}
+		if test != result {
+			t.Errorf("TestParseSlowStart(%q) returned %q expected %q", test, result, test)
+		}
+	}
+	for _, test := range invalidInput {
+		result, err := ParseSlowStart(test)
+		if err == nil {
+			t.Errorf("TestParseSlowStart(%q) didn't return error. Returned: %q", test, result)
+		}
+	}
+}

nginx-controller/nginx/nginx.go

@@ -51,6 +51,7 @@ type UpstreamServer struct {
 	Port        string
 	MaxFails    int64
 	FailTimeout string
+	SlowStart   string
 }
 
 // HealthCheck describes an active HTTP health check

nginx-controller/nginx/plus/nginx_api.go

@@ -14,6 +14,7 @@ type NginxAPIController struct {
 type ServerConfig struct {
 	MaxFails    int64
 	FailTimeout string
+	SlowStart   string
 }
 
 func NewNginxAPIController(httpClient *http.Client, endpoint string, local bool) (*NginxAPIController, error) {
@@ -37,6 +38,7 @@ func (nginx *NginxAPIController) UpdateServers(upstream string, servers []string
 			Server:      s,
 			MaxFails:    config.MaxFails,
 			FailTimeout: config.FailTimeout,
+			SlowStart:   config.SlowStart,
 		})
 	}
 

nginx-controller/nginx/plus/nginx_client.go

@@ -25,6 +25,7 @@ type UpstreamServer struct {
 	Server      string `json:"server"`
 	MaxFails    int64  `json:"max_fails"`
 	FailTimeout string `json:"fail_timeout,omitempty"`
+	SlowStart   string `json:"slow_start,omitempty"`
 }
 
 type apiErrorResponse struct {

nginx-controller/nginx/templates/nginx-plus.ingress.tmpl

@@ -3,7 +3,8 @@ upstream {{$upstream.Name}} {
 	zone {{$upstream.Name}} 256k;
 	{{if $upstream.LBMethod }}{{$upstream.LBMethod}};{{end}}
 	{{range $server := $upstream.UpstreamServers}}
-	server {{$server.Address}}:{{$server.Port}} max_fails={{$server.MaxFails}} fail_timeout={{$server.FailTimeout}};{{end}}
+	server {{$server.Address}}:{{$server.Port}} max_fails={{$server.MaxFails}} fail_timeout={{$server.FailTimeout}}
+	    {{- if $server.SlowStart}} slow_start={{$server.SlowStart}}{{end}};{{end}}
 	{{if $upstream.StickyCookie}}
 	sticky cookie {{$upstream.StickyCookie}};
 	{{end}}

nginx-controller/nginx/templates/templates_test.go

@@ -21,6 +21,7 @@ var testUps = nginx.Upstream{
 			Port:        "8181",
 			MaxFails:    0,
 			FailTimeout: "1s",
+			SlowStart:   "5s",
 		},
 	},
 }