Merge pull request #2794 from ElvinEfendi/enable-dynamic-confoguration

enable dynamic backend configuration by default

cmd/nginx/flags.go

@@ -143,7 +143,7 @@ extension for this to succeed.`)
 			`Customized address to set as the load-balancer status of Ingress objects this controller satisfies.
 Requires the update-status parameter.`)
 
-		dynamicConfigurationEnabled = flags.Bool("enable-dynamic-configuration", false,
+		dynamicConfigurationEnabled = flags.Bool("enable-dynamic-configuration", true,
 			`Dynamically refresh backends on topology changes instead of reloading NGINX.
 Feature backed by OpenResty Lua libraries.`)
 

docs/user-guide/cli-arguments.md

@@ -15,7 +15,7 @@ They are set in the container spec of the `nginx-ingress-controller` Deployment
 | --default-server-port int         | Port to use for exposing the default server (catch-all). (default 8181) |
 | --default-ssl-certificate string  | Secret containing a SSL certificate to be used by the default HTTPS server (catch-all). Takes the form "namespace/name". |
 | --election-id string              | Election id to use for Ingress status updates. (default "ingress-controller-leader") |
-| --enable-dynamic-configuration    | Dynamically refresh backends on topology changes instead of reloading NGINX. Feature backed by OpenResty Lua libraries. |
+| --enable-dynamic-configuration    | Dynamically refresh backends on topology changes instead of reloading NGINX. Feature backed by OpenResty Lua libraries. (enabled by default) |
 | --enable-ssl-chain-completion     | Autocomplete SSL certificate chains with missing intermediate CA certificates. A valid certificate chain is required to enable OCSP stapling. Certificates uploaded to Kubernetes must have the "Authority Information Access" X.509 v3 extension for this to succeed. (default true) |
 | --enable-ssl-passthrough          | Enable SSL Passthrough. |
 | --force-namespace-isolation       | Force namespace isolation. Prevents Ingress objects from referencing Secrets and ConfigMaps located in a different namespace than their own. May be used together with watch-namespace. |

docs/user-guide/nginx-configuration/configmap.md

@@ -92,7 +92,7 @@ The following table shows a configuration option's name, type, and the default v
 |[worker-processes](#worker-processes)|string|`<Number of CPUs>`|
 |[worker-cpu-affinity](#worker-cpu-affinity)|string|""|
 |[worker-shutdown-timeout](#worker-shutdown-timeout)|string|"10s"|
-|[load-balance](#load-balance)|string|"least_conn"|
+|[load-balance](#load-balance)|string|"round_robin"|
 |[variables-hash-bucket-size](#variables-hash-bucket-size)|int|128|
 |[variables-hash-max-size](#variables-hash-max-size)|int|2048|
 |[upstream-keepalive-connections](#upstream-keepalive-connections)|int|32|
@@ -520,11 +520,11 @@ Sets the algorithm to use for load balancing.
 The value can either be:
 
 - round_robin: to use the default round robin loadbalancer
-- least_conn: to use the least connected method
-- ip_hash: to use a hash of the server for routing.
-- ewma: to use the peak ewma method for routing (only available with `enable-dynamic-configuration` flag) 
+- least_conn: to use the least connected method (_note_ that this is available only in non-dynamic mode: `--enable-dynamic-configuration=false`)
+- ip_hash: to use a hash of the server for routing (_note_ that this is available only in non-dynamic mode: `--enable-dynamic-configuration=false`, but alternatively you can consider using `nginx.ingress.kubernetes.io/upstream-hash-by`)
+- ewma: to use the Peak EWMA method for routing ([implementation](https://github.com/kubernetes/ingress-nginx/blob/master/rootfs/etc/nginx/lua/balancer/ewma.lua))
 
-The default is least_conn.
+The default is `round_robin`.
 
 _References:_
 [http://nginx.org/en/docs/http/load_balancing.html](http://nginx.org/en/docs/http/load_balancing.html)

internal/ingress/controller/config/config.go

@@ -77,7 +77,7 @@ const (
 	sslSessionCacheSize = "10m"
 
 	// Default setting for load balancer algorithm
-	defaultLoadBalancerAlgorithm = "least_conn"
+	defaultLoadBalancerAlgorithm = ""
 
 	// Parameters for a shared memory zone that will keep states for various keys.
 	// http://nginx.org/en/docs/http/ngx_http_limit_conn_module.html#limit_conn_zone

internal/ingress/controller/template/configmap_test.go

@@ -155,7 +155,7 @@ func TestMergeConfigMapToStruct(t *testing.T) {
 func TestDefaultLoadBalance(t *testing.T) {
 	conf := map[string]string{}
 	to := ReadConfig(conf)
-	if to.LoadBalanceAlgorithm != "least_conn" {
+	if to.LoadBalanceAlgorithm != "" {
 		t.Errorf("default load balance algorithm wrong")
 	}
 }

internal/ingress/controller/template/template.go

@@ -378,7 +378,7 @@ func buildLoadBalancingConfig(b interface{}, fallbackLoadBalancing string) strin
 		return fmt.Sprintf("%s;", backend.LoadBalancing)
 	}
 
-	if fallbackLoadBalancing == "round_robin" {
+	if fallbackLoadBalancing == "round_robin" || fallbackLoadBalancing == "" {
 		return ""
 	}
 

test/e2e/annotations/affinity.go

@@ -32,11 +32,15 @@ import (
 	"k8s.io/ingress-nginx/test/e2e/framework"
 )
 
+// TODO(elvinefendi) merge this with Affinity tests in test/e2e/lua/dynamic_configuration.go
 var _ = framework.IngressNginxDescribe("Annotations - Affinity", func() {
 	f := framework.NewDefaultFramework("affinity")
 
 	BeforeEach(func() {
-		err := f.NewEchoDeploymentWithReplicas(2)
+		err := f.DisableDynamicConfiguration()
+		Expect(err).NotTo(HaveOccurred())
+
+		err = f.NewEchoDeploymentWithReplicas(2)
 		Expect(err).NotTo(HaveOccurred())
 	})
 

test/e2e/framework/framework.go

@@ -420,3 +420,19 @@ func NewSingleIngress(name, path, host, ns, service string, port int, annotation
 
 	return ing
 }
+
+// DisableDynamicConfiguration disables dynamic configuration
+func (f *Framework) DisableDynamicConfiguration() error {
+	return UpdateDeployment(f.KubeClientSet, f.IngressController.Namespace, "nginx-ingress-controller", 1,
+		func(deployment *appsv1beta1.Deployment) error {
+			args := deployment.Spec.Template.Spec.Containers[0].Args
+			args = append(args, "--enable-dynamic-configuration=false")
+			deployment.Spec.Template.Spec.Containers[0].Args = args
+			_, err := f.KubeClientSet.AppsV1beta1().Deployments(f.IngressController.Namespace).Update(deployment)
+			if err != nil {
+				return err
+			}
+
+			return nil
+		})
+}

test/e2e/lua/dynamic_configuration.go

@@ -27,11 +27,9 @@ import (
 	. "github.com/onsi/gomega"
 	"github.com/parnurzeal/gorequest"
 
-	appsv1beta1 "k8s.io/api/apps/v1beta1"
 	extensions "k8s.io/api/extensions/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
-	"k8s.io/client-go/kubernetes"
 
 	"k8s.io/ingress-nginx/internal/net/dns"
 	"k8s.io/ingress-nginx/test/e2e/framework"
@@ -50,10 +48,7 @@ var _ = framework.IngressNginxDescribe("Dynamic Configuration", func() {
 	f := framework.NewDefaultFramework("dynamic-configuration")
 
 	BeforeEach(func() {
-		err := enableDynamicConfiguration(f.IngressController.Namespace, f.KubeClientSet)
-		Expect(err).NotTo(HaveOccurred())
-
-		err = f.NewEchoDeploymentWithReplicas(1)
+		err := f.NewEchoDeploymentWithReplicas(1)
 		Expect(err).NotTo(HaveOccurred())
 
 		err = f.WaitForNginxConfiguration(func(cfg string) bool {
@@ -369,21 +364,6 @@ var _ = framework.IngressNginxDescribe("Dynamic Configuration", func() {
 	})
 })
 
-func enableDynamicConfiguration(namespace string, kubeClientSet kubernetes.Interface) error {
-	return framework.UpdateDeployment(kubeClientSet, namespace, "nginx-ingress-controller", 1,
-		func(deployment *appsv1beta1.Deployment) error {
-			args := deployment.Spec.Template.Spec.Containers[0].Args
-			args = append(args, "--enable-dynamic-configuration")
-			deployment.Spec.Template.Spec.Containers[0].Args = args
-			_, err := kubeClientSet.AppsV1beta1().Deployments(namespace).Update(deployment)
-			if err != nil {
-				return err
-			}
-
-			return nil
-		})
-}
-
 func ensureIngress(f *framework.Framework, host string) (*extensions.Ingress, error) {
 	return f.EnsureIngress(framework.NewSingleIngress(host, "/", host, f.IngressController.Namespace, "http-svc", 80, &map[string]string{
 		"nginx.ingress.kubernetes.io/load-balance": "ewma",