Live Nginx (re)configuration without reloading (#2174)

internal/ingress/controller/config/config.go

@@ -604,23 +604,24 @@ func (cfg Configuration) BuildLogFormatUpstream() string {
 
 // TemplateConfig contains the nginx configuration to render the file nginx.conf
 type TemplateConfig struct {
-	ProxySetHeaders         map[string]string
-	AddHeaders              map[string]string
-	MaxOpenFiles            int
-	BacklogSize             int
-	Backends                []*ingress.Backend
-	PassthroughBackends     []*ingress.SSLPassthroughBackend
-	Servers                 []*ingress.Server
-	TCPBackends             []ingress.L4Service
-	UDPBackends             []ingress.L4Service
-	HealthzURI              string
-	CustomErrors            bool
-	Cfg                     Configuration
-	IsIPV6Enabled           bool
-	IsSSLPassthroughEnabled bool
-	RedirectServers         map[string]string
-	ListenPorts             *ListenPorts
-	PublishService          *apiv1.Service
+	ProxySetHeaders             map[string]string
+	AddHeaders                  map[string]string
+	MaxOpenFiles                int
+	BacklogSize                 int
+	Backends                    []*ingress.Backend
+	PassthroughBackends         []*ingress.SSLPassthroughBackend
+	Servers                     []*ingress.Server
+	TCPBackends                 []ingress.L4Service
+	UDPBackends                 []ingress.L4Service
+	HealthzURI                  string
+	CustomErrors                bool
+	Cfg                         Configuration
+	IsIPV6Enabled               bool
+	IsSSLPassthroughEnabled     bool
+	RedirectServers             map[string]string
+	ListenPorts                 *ListenPorts
+	PublishService              *apiv1.Service
+	DynamicConfigurationEnabled bool
 }
 
 // ListenPorts describe the ports required to run the

internal/ingress/controller/controller.go

@@ -95,6 +95,8 @@ type Configuration struct {
 	FakeCertificateSHA  string
 
 	SyncRateLimit float32
+
+	DynamicConfigurationEnabled bool
 }
 
 // GetPublishService returns the configured service used to set ingress status
@@ -167,6 +169,15 @@ func (n *NGINXController) syncIngress(item interface{}) error {
 	if !n.isForceReload() && n.runningConfig.Equal(&pcfg) {
 		glog.V(3).Infof("skipping backend reload (no changes detected)")
 		return nil
+	} else if !n.isForceReload() && n.cfg.DynamicConfigurationEnabled && n.IsDynamicallyConfigurable(&pcfg) {
+		err := n.ConfigureDynamically(&pcfg)
+		if err == nil {
+			glog.Infof("dynamic reconfiguration succeeded, skipping reload")
+			n.runningConfig = &pcfg
+			return nil
+		}
+
+		glog.Warningf("falling back to reload, could not dynamically reconfigure: %v", err)
 	}
 
 	glog.Infof("backend reload required")
@@ -182,6 +193,19 @@ func (n *NGINXController) syncIngress(item interface{}) error {
 	incReloadCount()
 	setSSLExpireTime(servers)
 
+	if n.isForceReload() && n.cfg.DynamicConfigurationEnabled {
+		go func() {
+			// it takes time for Nginx to start listening on the port
+			time.Sleep(1 * time.Second)
+			err := n.ConfigureDynamically(&pcfg)
+			if err == nil {
+				glog.Infof("dynamic reconfiguration succeeded")
+			} else {
+				glog.Warningf("could not dynamically reconfigure: %v", err)
+			}
+		}()
+	}
+
 	n.runningConfig = &pcfg
 	n.SetForceReload(false)
 

internal/ingress/controller/nginx.go

@@ -18,10 +18,12 @@ package controller
 
 import (
 	"bytes"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"io/ioutil"
 	"net"
+	"net/http"
 	"os"
 	"os/exec"
 	"strconv"
@@ -606,23 +608,24 @@ func (n *NGINXController) OnUpdate(ingressCfg ingress.Configuration) error {
 	cfg.SSLDHParam = sslDHParam
 
 	tc := ngx_config.TemplateConfig{
-		ProxySetHeaders:         setHeaders,
-		AddHeaders:              addHeaders,
-		MaxOpenFiles:            maxOpenFiles,
-		BacklogSize:             sysctlSomaxconn(),
-		Backends:                ingressCfg.Backends,
-		PassthroughBackends:     ingressCfg.PassthroughBackends,
-		Servers:                 ingressCfg.Servers,
-		TCPBackends:             ingressCfg.TCPEndpoints,
-		UDPBackends:             ingressCfg.UDPEndpoints,
-		HealthzURI:              ngxHealthPath,
-		CustomErrors:            len(cfg.CustomHTTPErrors) > 0,
-		Cfg:                     cfg,
-		IsIPV6Enabled:           n.isIPV6Enabled && !cfg.DisableIpv6,
-		RedirectServers:         redirectServers,
-		IsSSLPassthroughEnabled: n.cfg.EnableSSLPassthrough,
-		ListenPorts:             n.cfg.ListenPorts,
-		PublishService:          n.GetPublishService(),
+		ProxySetHeaders:             setHeaders,
+		AddHeaders:                  addHeaders,
+		MaxOpenFiles:                maxOpenFiles,
+		BacklogSize:                 sysctlSomaxconn(),
+		Backends:                    ingressCfg.Backends,
+		PassthroughBackends:         ingressCfg.PassthroughBackends,
+		Servers:                     ingressCfg.Servers,
+		TCPBackends:                 ingressCfg.TCPEndpoints,
+		UDPBackends:                 ingressCfg.UDPEndpoints,
+		HealthzURI:                  ngxHealthPath,
+		CustomErrors:                len(cfg.CustomHTTPErrors) > 0,
+		Cfg:                         cfg,
+		IsIPV6Enabled:               n.isIPV6Enabled && !cfg.DisableIpv6,
+		RedirectServers:             redirectServers,
+		IsSSLPassthroughEnabled:     n.cfg.EnableSSLPassthrough,
+		ListenPorts:                 n.cfg.ListenPorts,
+		PublishService:              n.GetPublishService(),
+		DynamicConfigurationEnabled: n.cfg.DynamicConfigurationEnabled,
 	}
 
 	content, err := n.t.Write(tc)
@@ -745,3 +748,43 @@ func (n *NGINXController) setupSSLProxy() {
 		}
 	}()
 }
+
+// IsDynamicallyConfigurable decides if the new configuration can be dynamically configured without reloading
+func (n *NGINXController) IsDynamicallyConfigurable(pcfg *ingress.Configuration) bool {
+	var copyOfRunningConfig ingress.Configuration = *n.runningConfig
+	var copyOfPcfg ingress.Configuration = *pcfg
+
+	copyOfRunningConfig.Backends = []*ingress.Backend{}
+	copyOfPcfg.Backends = []*ingress.Backend{}
+
+	return copyOfRunningConfig.Equal(&copyOfPcfg)
+}
+
+// ConfigureDynamically JSON encodes new Backends and POSTs it to an internal HTTP endpoint
+// that is handled by Lua
+func (n *NGINXController) ConfigureDynamically(pcfg *ingress.Configuration) error {
+	buf, err := json.Marshal(pcfg.Backends)
+	if err != nil {
+		return err
+	}
+
+	glog.V(2).Infof("posting backends configuration: %s", buf)
+
+	url := fmt.Sprintf("http://localhost:%d/configuration/backends", n.cfg.ListenPorts.Status)
+	resp, err := http.Post(url, "application/json", bytes.NewReader(buf))
+	if err != nil {
+		return err
+	}
+
+	defer func() {
+		if err := resp.Body.Close(); err != nil {
+			glog.Warningf("error while closing response body: \n%v", err)
+		}
+	}()
+
+	if resp.StatusCode != http.StatusCreated {
+		return fmt.Errorf("Unexpected error code: %d", resp.StatusCode)
+	}
+
+	return nil
+}

internal/ingress/controller/nginx_test.go

@@ -16,7 +16,78 @@ limitations under the License.
 
 package controller
 
-import "testing"
+import (
+	"testing"
+
+	"k8s.io/ingress-nginx/internal/ingress"
+)
+
+func TestIsDynamicallyConfigurable(t *testing.T) {
+	backends := []*ingress.Backend{{
+		Name: "fakenamespace-myapp-80",
+		Endpoints: []ingress.Endpoint{
+			{
+				Address: "10.0.0.1",
+				Port:    "8080",
+			},
+			{
+				Address: "10.0.0.2",
+				Port:    "8080",
+			},
+		},
+	}}
+
+	servers := []*ingress.Server{{
+		Hostname: "myapp.fake",
+		Locations: []*ingress.Location{
+			{
+				Path:    "/",
+				Backend: "fakenamespace-myapp-80",
+			},
+		},
+	}}
+
+	commonConfig := &ingress.Configuration{
+		Backends: backends,
+		Servers:  servers,
+	}
+
+	n := &NGINXController{
+		runningConfig: &ingress.Configuration{
+			Backends: backends,
+			Servers:  servers,
+		},
+	}
+
+	newConfig := commonConfig
+	if !n.IsDynamicallyConfigurable(newConfig) {
+		t.Errorf("When new config is same as the running config it should be deemed as dynamically configurable")
+	}
+
+	newConfig = &ingress.Configuration{
+		Backends: []*ingress.Backend{{Name: "another-backend-8081"}},
+		Servers:  []*ingress.Server{{Hostname: "myapp1.fake"}},
+	}
+	if n.IsDynamicallyConfigurable(newConfig) {
+		t.Errorf("Expected to not be dynamically configurable when there's more than just backends change")
+	}
+
+	newConfig = &ingress.Configuration{
+		Backends: []*ingress.Backend{{Name: "a-backend-8080"}},
+		Servers:  servers,
+	}
+	if !n.IsDynamicallyConfigurable(newConfig) {
+		t.Errorf("Expected to be dynamically configurable when only backends change")
+	}
+
+	if !n.runningConfig.Equal(commonConfig) {
+		t.Errorf("Expected running config to not change")
+	}
+
+	if !newConfig.Equal(&ingress.Configuration{Backends: []*ingress.Backend{{Name: "a-backend-8080"}}, Servers: servers}) {
+		t.Errorf("Expected new config to not change")
+	}
+}
 
 func TestNginxHashBucketSize(t *testing.T) {
 	tests := []struct {

internal/ingress/controller/template/template.go

@@ -307,7 +307,7 @@ func buildLoadBalancingConfig(b interface{}, fallbackLoadBalancing string) strin
 // (specified through the nginx.ingress.kubernetes.io/rewrite-to annotation)
 // If the annotation nginx.ingress.kubernetes.io/add-base-url:"true" is specified it will
 // add a base tag in the head of the response from the service
-func buildProxyPass(host string, b interface{}, loc interface{}) string {
+func buildProxyPass(host string, b interface{}, loc interface{}, dynamicConfigurationEnabled bool) string {
 	backends, ok := b.([]*ingress.Backend)
 	if !ok {
 		glog.Errorf("expected an '[]*ingress.Backend' type but %T was returned", b)
@@ -323,14 +323,19 @@ func buildProxyPass(host string, b interface{}, loc interface{}) string {
 	path := location.Path
 	proto := "http"
 
-	upstreamName := location.Backend
+	upstreamName := "upstream_balancer"
+
+	if !dynamicConfigurationEnabled {
+		upstreamName = location.Backend
+	}
+
 	for _, backend := range backends {
 		if backend.Name == location.Backend {
 			if backend.Secure || backend.SSLPassthrough {
 				proto = "https"
 			}
 
-			if isSticky(host, location, backend.SessionAffinity.CookieSessionAffinity.Locations) {
+			if !dynamicConfigurationEnabled && isSticky(host, location, backend.SessionAffinity.CookieSessionAffinity.Locations) {
 				upstreamName = fmt.Sprintf("sticky-%v", upstreamName)
 			}
 
@@ -340,6 +345,7 @@ func buildProxyPass(host string, b interface{}, loc interface{}) string {
 
 	// defProxyPass returns the default proxy_pass, just the name of the upstream
 	defProxyPass := fmt.Sprintf("proxy_pass %s://%s;", proto, upstreamName)
+
 	// if the path in the ingress rule is equals to the target: no special rewrite
 	if path == location.Rewrite.Target {
 		return defProxyPass

internal/ingress/controller/template/template_test.go

@@ -37,72 +37,262 @@ import (
 )
 
 var (
-	// TODO: add tests for secure endpoints
+	// TODO: add tests for SSLPassthrough
 	tmplFuncTestcases = map[string]struct {
-		Path             string
-		Target           string
-		Location         string
-		ProxyPass        string
-		AddBaseURL       bool
-		BaseURLScheme    string
-		Sticky           bool
-		XForwardedPrefix bool
+		Path                        string
+		Target                      string
+		Location                    string
+		ProxyPass                   string
+		AddBaseURL                  bool
+		BaseURLScheme               string
+		Sticky                      bool
+		XForwardedPrefix            bool
+		DynamicConfigurationEnabled bool
+		SecureBackend               bool
 	}{
-		"invalid redirect / to /": {"/", "/", "/", "proxy_pass http://upstream-name;", false, "", false, false},
-		"redirect / to /jenkins": {"/", "/jenkins", "~* /",
+		"when secure backend enabled": {
+			"/",
+			"/",
+			"/",
+			"proxy_pass https://upstream-name;",
+			false,
+			"",
+			false,
+			false,
+			false,
+			true},
+		"when secure backend and stickeness enabled": {
+			"/",
+			"/",
+			"/",
+			"proxy_pass https://sticky-upstream-name;",
+			false,
+			"",
+			true,
+			false,
+			false,
+			true},
+		"when secure backend and dynamic config enabled": {
+			"/",
+			"/",
+			"/",
+			"proxy_pass https://upstream_balancer;",
+			false,
+			"",
+			false,
+			false,
+			true,
+			true},
+		"when secure backend, stickeness and dynamic config enabled": {
+			"/",
+			"/",
+			"/",
+			"proxy_pass https://upstream_balancer;",
+			false,
+			"",
+			true,
+			false,
+			true,
+			true},
+		"invalid redirect / to / with dynamic config enabled": {
+			"/",
+			"/",
+			"/",
+			"proxy_pass http://upstream_balancer;",
+			false,
+			"",
+			false,
+			false,
+			true,
+			false},
+		"invalid redirect / to /": {
+			"/",
+			"/",
+			"/",
+			"proxy_pass http://upstream-name;",
+			false,
+			"",
+			false,
+			false,
+			false,
+			false},
+		"redirect / to /jenkins": {
+			"/",
+			"/jenkins",
+			"~* /",
 			`
 	    rewrite /(.*) /jenkins/$1 break;
 	    proxy_pass http://upstream-name;
-	    `, false, "", false, false},
-		"redirect /something to /": {"/something", "/", `~* ^/something\/?(?<baseuri>.*)`, `
+	    `,
+			false,
+			"",
+			false,
+			false,
+			false,
+			false},
+		"redirect /something to /": {
+			"/something",
+			"/",
+			`~* ^/something\/?(?<baseuri>.*)`,
+			`
 	    rewrite /something/(.*) /$1 break;
 	    rewrite /something / break;
 	    proxy_pass http://upstream-name;
-	    `, false, "", false, false},
-		"redirect /end-with-slash/ to /not-root": {"/end-with-slash/", "/not-root", "~* ^/end-with-slash/(?<baseuri>.*)", `
+	    `,
+			false,
+			"",
+			false,
+			false,
+			false,
+			false},
+		"redirect /end-with-slash/ to /not-root": {
+			"/end-with-slash/",
+			"/not-root",
+			"~* ^/end-with-slash/(?<baseuri>.*)",
+			`
 	    rewrite /end-with-slash/(.*) /not-root/$1 break;
 	    proxy_pass http://upstream-name;
-	    `, false, "", false, false},
-		"redirect /something-complex to /not-root": {"/something-complex", "/not-root", `~* ^/something-complex\/?(?<baseuri>.*)`, `
+	    `,
+			false,
+			"",
+			false,
+			false,
+			false,
+			false},
+		"redirect /something-complex to /not-root": {
+			"/something-complex",
+			"/not-root",
+			`~* ^/something-complex\/?(?<baseuri>.*)`,
+			`
 	    rewrite /something-complex/(.*) /not-root/$1 break;
 	    proxy_pass http://upstream-name;
-	    `, false, "", false, false},
-		"redirect / to /jenkins and rewrite": {"/", "/jenkins", "~* /", `
+	    `,
+			false,
+			"",
+			false,
+			false,
+			false,
+			false},
+		"redirect / to /jenkins and rewrite": {
+			"/",
+			"/jenkins",
+			"~* /",
+			`
 	    rewrite /(.*) /jenkins/$1 break;
 	    proxy_pass http://upstream-name;
 	    subs_filter '(<(?:H|h)(?:E|e)(?:A|a)(?:D|d)(?:[^">]|"[^"]*")*>)' '$1<base href="$scheme://$http_host/$baseuri">' ro;
-	    `, true, "", false, false},
-		"redirect /something to / and rewrite": {"/something", "/", `~* ^/something\/?(?<baseuri>.*)`, `
+	    `,
+			true,
+			"",
+			false,
+			false,
+			false,
+			false},
+		"redirect /something to / and rewrite": {
+			"/something",
+			"/",
+			`~* ^/something\/?(?<baseuri>.*)`,
+			`
 	    rewrite /something/(.*) /$1 break;
 	    rewrite /something / break;
 	    proxy_pass http://upstream-name;
 	    subs_filter '(<(?:H|h)(?:E|e)(?:A|a)(?:D|d)(?:[^">]|"[^"]*")*>)' '$1<base href="$scheme://$http_host/something/$baseuri">' ro;
-	    `, true, "", false, false},
-		"redirect /end-with-slash/ to /not-root and rewrite": {"/end-with-slash/", "/not-root", `~* ^/end-with-slash/(?<baseuri>.*)`, `
+	    `,
+			true,
+			"",
+			false,
+			false,
+			false,
+			false},
+		"redirect /end-with-slash/ to /not-root and rewrite": {
+			"/end-with-slash/",
+			"/not-root",
+			`~* ^/end-with-slash/(?<baseuri>.*)`,
+			`
 	    rewrite /end-with-slash/(.*) /not-root/$1 break;
 	    proxy_pass http://upstream-name;
 	    subs_filter '(<(?:H|h)(?:E|e)(?:A|a)(?:D|d)(?:[^">]|"[^"]*")*>)' '$1<base href="$scheme://$http_host/end-with-slash/$baseuri">' ro;
-	    `, true, "", false, false},
-		"redirect /something-complex to /not-root and rewrite": {"/something-complex", "/not-root", `~* ^/something-complex\/?(?<baseuri>.*)`, `
+	    `,
+			true,
+			"",
+			false,
+			false,
+			false,
+			false},
+		"redirect /something-complex to /not-root and rewrite": {
+			"/something-complex",
+			"/not-root",
+			`~* ^/something-complex\/?(?<baseuri>.*)`,
+			`
 	    rewrite /something-complex/(.*) /not-root/$1 break;
 	    proxy_pass http://upstream-name;
 	    subs_filter '(<(?:H|h)(?:E|e)(?:A|a)(?:D|d)(?:[^">]|"[^"]*")*>)' '$1<base href="$scheme://$http_host/something-complex/$baseuri">' ro;
-	    `, true, "", false, false},
-		"redirect /something to / and rewrite with specific scheme": {"/something", "/", `~* ^/something\/?(?<baseuri>.*)`, `
+	    `,
+			true,
+			"",
+			false,
+			false,
+			false,
+			false},
+		"redirect /something to / and rewrite with specific scheme": {
+			"/something",
+			"/",
+			`~* ^/something\/?(?<baseuri>.*)`,
+			`
 	    rewrite /something/(.*) /$1 break;
 	    rewrite /something / break;
 	    proxy_pass http://upstream-name;
 	    subs_filter '(<(?:H|h)(?:E|e)(?:A|a)(?:D|d)(?:[^">]|"[^"]*")*>)' '$1<base href="http://$http_host/something/$baseuri">' ro;
-	    `, true, "http", false, false},
-		"redirect / to /something with sticky enabled": {"/", "/something", `~* /`, `
+	    `,
+			true,
+			"http",
+			false,
+			false,
+			false,
+			false},
+		"redirect / to /something with sticky enabled": {
+			"/",
+			"/something",
+			`~* /`,
+			`
 	    rewrite /(.*) /something/$1 break;
 	    proxy_pass http://sticky-upstream-name;
-	    `, false, "http", true, false},
-		"add the X-Forwarded-Prefix header": {"/there", "/something", `~* ^/there\/?(?<baseuri>.*)`, `
+	    `,
+			false,
+			"http",
+			true,
+			false,
+			false,
+			false},
+		"redirect / to /something with sticky and dynamic config enabled": {
+			"/",
+			"/something",
+			`~* /`,
+			`
+	    rewrite /(.*) /something/$1 break;
+	    proxy_pass http://upstream_balancer;
+	    `,
+			false,
+			"http",
+			true,
+			false,
+			true,
+			false},
+		"add the X-Forwarded-Prefix header": {
+			"/there",
+			"/something",
+			`~* ^/there\/?(?<baseuri>.*)`,
+			`
 	    rewrite /there/(.*) /something/$1 break;
 	    proxy_set_header X-Forwarded-Prefix "/there/";
 	    proxy_pass http://sticky-upstream-name;
-	    `, false, "http", true, true},
+	    `,
+			false,
+			"http",
+			true,
+			true,
+			false,
+			false},
 	}
 )
 
@@ -151,24 +341,25 @@ func TestBuildProxyPass(t *testing.T) {
 			XForwardedPrefix: tc.XForwardedPrefix,
 		}
 
-		backends := []*ingress.Backend{}
+		backend := &ingress.Backend{
+			Name:   defaultBackend,
+			Secure: tc.SecureBackend,
+		}
+
 		if tc.Sticky {
-			backends = []*ingress.Backend{
-				{
-					Name: defaultBackend,
-					SessionAffinity: ingress.SessionAffinityConfig{
-						AffinityType: "cookie",
-						CookieSessionAffinity: ingress.CookieSessionAffinity{
-							Locations: map[string][]string{
-								defaultHost: {tc.Path},
-							},
-						},
+			backend.SessionAffinity = ingress.SessionAffinityConfig{
+				AffinityType: "cookie",
+				CookieSessionAffinity: ingress.CookieSessionAffinity{
+					Locations: map[string][]string{
+						defaultHost: {tc.Path},
 					},
 				},
 			}
 		}
 
-		pp := buildProxyPass(defaultHost, backends, loc)
+		backends := []*ingress.Backend{backend}
+
+		pp := buildProxyPass(defaultHost, backends, loc, tc.DynamicConfigurationEnabled)
 		if !strings.EqualFold(tc.ProxyPass, pp) {
 			t.Errorf("%s: expected \n'%v'\nbut returned \n'%v'", k, tc.ProxyPass, pp)
 		}