diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index 9e7a7f833..0e3d1d24a 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -169,41 +169,41 @@ 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 + } + + if n.cfg.DynamicConfigurationEnabled && n.IsDynamicConfigurationEnough(&pcfg) && !n.isForceReload() { + glog.Infof("skipping reload") + } else { + glog.Infof("backend reload required") + + err := n.OnUpdate(pcfg) + if err != nil { + incReloadErrorCount() + glog.Errorf("unexpected failure restarting the backend: \n%v", err) + return err } - glog.Warningf("falling back to reload, could not dynamically reconfigure: %v", err) + glog.Infof("ingress backend successfully reloaded...") + incReloadCount() + setSSLExpireTime(servers) } - glog.Infof("backend reload required") + if n.cfg.DynamicConfigurationEnabled { + isFirstSync := n.runningConfig.Equal(&ingress.Configuration{}) + go func(isFirstSync bool) { + if isFirstSync { + glog.Infof("first sync of Nginx configuration") - err := n.OnUpdate(pcfg) - if err != nil { - incReloadErrorCount() - glog.Errorf("unexpected failure restarting the backend: \n%v", err) - return err - } - - glog.Infof("ingress backend successfully reloaded...") - 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) + // 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) } - }() + }(isFirstSync) } n.runningConfig = &pcfg diff --git a/internal/ingress/controller/nginx.go b/internal/ingress/controller/nginx.go index 7b8edc53d..a424f0f0e 100644 --- a/internal/ingress/controller/nginx.go +++ b/internal/ingress/controller/nginx.go @@ -751,8 +751,8 @@ func (n *NGINXController) setupSSLProxy() { }() } -// IsDynamicallyConfigurable decides if the new configuration can be dynamically configured without reloading -func (n *NGINXController) IsDynamicallyConfigurable(pcfg *ingress.Configuration) bool { +// IsDynamicConfigurationEnough decides if the new configuration changes can be dynamically applied without reloading +func (n *NGINXController) IsDynamicConfigurationEnough(pcfg *ingress.Configuration) bool { var copyOfRunningConfig ingress.Configuration = *n.runningConfig var copyOfPcfg ingress.Configuration = *pcfg diff --git a/internal/ingress/controller/nginx_test.go b/internal/ingress/controller/nginx_test.go index 7be8ce135..7b4877a8c 100644 --- a/internal/ingress/controller/nginx_test.go +++ b/internal/ingress/controller/nginx_test.go @@ -22,7 +22,7 @@ import ( "k8s.io/ingress-nginx/internal/ingress" ) -func TestIsDynamicallyConfigurable(t *testing.T) { +func TestIsDynamicConfigurationEnough(t *testing.T) { backends := []*ingress.Backend{{ Name: "fakenamespace-myapp-80", Endpoints: []ingress.Endpoint{ @@ -60,7 +60,7 @@ func TestIsDynamicallyConfigurable(t *testing.T) { } newConfig := commonConfig - if !n.IsDynamicallyConfigurable(newConfig) { + if !n.IsDynamicConfigurationEnough(newConfig) { t.Errorf("When new config is same as the running config it should be deemed as dynamically configurable") } @@ -68,7 +68,7 @@ func TestIsDynamicallyConfigurable(t *testing.T) { Backends: []*ingress.Backend{{Name: "another-backend-8081"}}, Servers: []*ingress.Server{{Hostname: "myapp1.fake"}}, } - if n.IsDynamicallyConfigurable(newConfig) { + if n.IsDynamicConfigurationEnough(newConfig) { t.Errorf("Expected to not be dynamically configurable when there's more than just backends change") } @@ -76,7 +76,7 @@ func TestIsDynamicallyConfigurable(t *testing.T) { Backends: []*ingress.Backend{{Name: "a-backend-8080"}}, Servers: servers, } - if !n.IsDynamicallyConfigurable(newConfig) { + if !n.IsDynamicConfigurationEnough(newConfig) { t.Errorf("Expected to be dynamically configurable when only backends change") } diff --git a/test/e2e/e2e.go b/test/e2e/e2e.go index 4639b7a3d..6a628d64d 100644 --- a/test/e2e/e2e.go +++ b/test/e2e/e2e.go @@ -32,6 +32,7 @@ import ( // tests to run _ "k8s.io/ingress-nginx/test/e2e/annotations" _ "k8s.io/ingress-nginx/test/e2e/defaultbackend" + _ "k8s.io/ingress-nginx/test/e2e/lua" _ "k8s.io/ingress-nginx/test/e2e/settings" _ "k8s.io/ingress-nginx/test/e2e/ssl" ) diff --git a/test/e2e/framework/framework.go b/test/e2e/framework/framework.go index f007c3d28..2d8831d2f 100644 --- a/test/e2e/framework/framework.go +++ b/test/e2e/framework/framework.go @@ -19,14 +19,17 @@ import ( "strings" "time" + appsv1beta1 "k8s.io/api/apps/v1beta1" "k8s.io/api/core/v1" apiextcs "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/kubernetes" restclient "k8s.io/client-go/rest" "github.com/golang/glog" + "github.com/pkg/errors" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -255,3 +258,24 @@ func (f *Framework) matchNginxConditions(name string, matcher func(cfg string) b return false, nil } } + +// UpdateDeployment runs the given updateFunc on the deployment and waits for it to be updated +func UpdateDeployment(kubeClientSet kubernetes.Interface, namespace string, name string, replicas int, updateFunc func(d *appsv1beta1.Deployment) error) error { + deployment, err := kubeClientSet.AppsV1beta1().Deployments(namespace).Get(name, metav1.GetOptions{}) + if err != nil { + return err + } + + if err = updateFunc(deployment); err != nil { + return err + } + + err = WaitForPodsReady(kubeClientSet, 60*time.Second, replicas, namespace, metav1.ListOptions{ + LabelSelector: fields.SelectorFromSet(fields.Set(deployment.Spec.Template.ObjectMeta.Labels)).String(), + }) + if err != nil { + return errors.Wrap(err, "failed to wait for nginx-ingress-controller pods to restart") + } + + return nil +} diff --git a/test/e2e/framework/k8s.go b/test/e2e/framework/k8s.go index 82c34a666..198bce78d 100644 --- a/test/e2e/framework/k8s.go +++ b/test/e2e/framework/k8s.go @@ -25,6 +25,7 @@ import ( k8sErrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/kubernetes" ) // EnsureSecret creates a Secret object or returns it if it already exists. @@ -75,10 +76,15 @@ func (f *Framework) EnsureDeployment(deployment *extensions.Deployment) (*extens return d, nil } -// WaitForPodsReady waits for a given amount of time until a group of Pods is running. +// WaitForPodsReady waits for a given amount of time until a group of Pods is running in the framework's namespace. func (f *Framework) WaitForPodsReady(timeout time.Duration, expectedReplicas int, opts metav1.ListOptions) error { + return WaitForPodsReady(f.KubeClientSet, timeout, expectedReplicas, f.Namespace.Name, opts) +} + +// WaitForPodsReady waits for a given amount of time until a group of Pods is running in the given namespace. +func WaitForPodsReady(kubeClientSet kubernetes.Interface, timeout time.Duration, expectedReplicas int, namespace string, opts metav1.ListOptions) error { return wait.Poll(time.Second, timeout, func() (bool, error) { - pl, err := f.KubeClientSet.CoreV1().Pods(f.Namespace.Name).List(opts) + pl, err := kubeClientSet.CoreV1().Pods(namespace).List(opts) if err != nil { return false, err } diff --git a/test/e2e/lua/dynamic_configuration.go b/test/e2e/lua/dynamic_configuration.go new file mode 100644 index 000000000..3e04749c1 --- /dev/null +++ b/test/e2e/lua/dynamic_configuration.go @@ -0,0 +1,248 @@ +/* +Copyright 2018 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package lua + +import ( + "fmt" + "net/http" + "strings" + "time" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + "github.com/parnurzeal/gorequest" + + appsv1beta1 "k8s.io/api/apps/v1beta1" + extensions "k8s.io/api/extensions/v1beta1" + v1beta1 "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/test/e2e/framework" +) + +var _ = framework.IngressNginxDescribe("Dynamic Configuration", func() { + f := framework.NewDefaultFramework("dynamic-configuration") + + BeforeEach(func() { + err := enableDynamicConfiguration(f.KubeClientSet) + Expect(err).NotTo(HaveOccurred()) + + err = f.NewEchoDeploymentWithReplicas(1) + Expect(err).NotTo(HaveOccurred()) + + host := "foo.com" + ing, err := ensureIngress(f, host) + Expect(err).NotTo(HaveOccurred()) + Expect(ing).NotTo(BeNil()) + + time.Sleep(5 * time.Second) + + err = f.WaitForNginxServer(host, + func(server string) bool { + return strings.Contains(server, "proxy_pass http://upstream_balancer;") + }) + Expect(err).NotTo(HaveOccurred()) + + resp, _, errs := gorequest.New(). + Get(f.NginxHTTPURL). + Set("Host", host). + End() + Expect(len(errs)).Should(BeNumerically("==", 0)) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + + log, err := f.NginxLogs() + Expect(err).ToNot(HaveOccurred()) + Expect(log).ToNot(ContainSubstring("could not dynamically reconfigure")) + Expect(log).To(ContainSubstring("first sync of Nginx configuration")) + }) + + AfterEach(func() { + err := disableDynamicConfiguration(f.KubeClientSet) + Expect(err).NotTo(HaveOccurred()) + }) + + Context("when only backends change", func() { + It("should handle endpoints only changes", func() { + resp, _, errs := gorequest.New(). + Get(fmt.Sprintf("%s?id=endpoints_only_changes", f.NginxHTTPURL)). + Set("Host", "foo.com"). + End() + Expect(len(errs)).Should(BeNumerically("==", 0)) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + + replicas := 2 + err := framework.UpdateDeployment(f.KubeClientSet, f.Namespace.Name, "http-svc", replicas, + func(deployment *appsv1beta1.Deployment) error { + deployment.Spec.Replicas = framework.NewInt32(int32(replicas)) + _, err := f.KubeClientSet.AppsV1beta1().Deployments(f.Namespace.Name).Update(deployment) + return err + }) + Expect(err).NotTo(HaveOccurred()) + + time.Sleep(5 * time.Second) + log, err := f.NginxLogs() + Expect(err).ToNot(HaveOccurred()) + Expect(log).ToNot(BeEmpty()) + index := strings.Index(log, "id=endpoints_only_changes") + restOfLogs := log[index:] + + By("POSTing new backends to Lua endpoint") + Expect(restOfLogs).To(ContainSubstring("dynamic reconfiguration succeeded")) + Expect(restOfLogs).ToNot(ContainSubstring("could not dynamically reconfigure")) + + By("skipping Nginx reload") + Expect(restOfLogs).ToNot(ContainSubstring("backend reload required")) + Expect(restOfLogs).ToNot(ContainSubstring("ingress backend successfully reloaded")) + Expect(restOfLogs).To(ContainSubstring("skipping reload")) + Expect(restOfLogs).ToNot(ContainSubstring("first sync of Nginx configuration")) + }) + + It("should handle annotation changes", func() { + ingress, err := f.KubeClientSet.ExtensionsV1beta1().Ingresses(f.Namespace.Name).Get("foo.com", metav1.GetOptions{}) + Expect(err).ToNot(HaveOccurred()) + + ingress.ObjectMeta.Annotations["nginx.ingress.kubernetes.io/load-balance"] = "round_robin" + _, err = f.KubeClientSet.ExtensionsV1beta1().Ingresses(f.Namespace.Name).Update(ingress) + Expect(err).ToNot(HaveOccurred()) + + time.Sleep(5 * time.Second) + log, err := f.NginxLogs() + Expect(err).ToNot(HaveOccurred()) + Expect(log).ToNot(BeEmpty()) + index := strings.Index(log, fmt.Sprintf("reason: 'UPDATE' Ingress %s/foo.com", f.Namespace.Name)) + restOfLogs := log[index:] + + By("POSTing new backends to Lua endpoint") + Expect(restOfLogs).To(ContainSubstring("dynamic reconfiguration succeeded")) + Expect(restOfLogs).ToNot(ContainSubstring("could not dynamically reconfigure")) + + By("skipping Nginx reload") + Expect(restOfLogs).ToNot(ContainSubstring("backend reload required")) + Expect(restOfLogs).ToNot(ContainSubstring("ingress backend successfully reloaded")) + Expect(restOfLogs).To(ContainSubstring("skipping reload")) + Expect(restOfLogs).ToNot(ContainSubstring("first sync of Nginx configuration")) + }) + }) + + It("should handle a non backend update", func() { + ingress, err := f.KubeClientSet.ExtensionsV1beta1().Ingresses(f.Namespace.Name).Get("foo.com", metav1.GetOptions{}) + Expect(err).ToNot(HaveOccurred()) + + ingress.Spec.TLS = []v1beta1.IngressTLS{ + { + Hosts: []string{"foo.com"}, + SecretName: "foo.com", + }, + } + + _, _, _, err = framework.CreateIngressTLSSecret(f.KubeClientSet, + ingress.Spec.TLS[0].Hosts, + ingress.Spec.TLS[0].SecretName, + ingress.Namespace) + Expect(err).ToNot(HaveOccurred()) + + _, err = f.KubeClientSet.ExtensionsV1beta1().Ingresses(f.Namespace.Name).Update(ingress) + Expect(err).ToNot(HaveOccurred()) + + time.Sleep(5 * time.Second) + log, err := f.NginxLogs() + Expect(err).ToNot(HaveOccurred()) + Expect(log).ToNot(BeEmpty()) + + By("reloading Nginx") + Expect(log).To(ContainSubstring("ingress backend successfully reloaded")) + + By("POSTing new backends to Lua endpoint") + Expect(log).To(ContainSubstring("dynamic reconfiguration succeeded")) + + By("still be proxying requests through Lua balancer") + err = f.WaitForNginxServer("foo.com", + func(server string) bool { + return strings.Contains(server, "proxy_pass http://upstream_balancer;") + }) + Expect(err).NotTo(HaveOccurred()) + + By("generating the respective ssl listen directive") + err = f.WaitForNginxServer("foo.com", + func(server string) bool { + return strings.Contains(server, "server_name foo.com") && + strings.Contains(server, "listen 443") + }) + Expect(err).ToNot(HaveOccurred()) + }) +}) + +func enableDynamicConfiguration(kubeClientSet kubernetes.Interface) error { + return framework.UpdateDeployment(kubeClientSet, "ingress-nginx", "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("ingress-nginx").Update(deployment) + return err + }) +} + +func disableDynamicConfiguration(kubeClientSet kubernetes.Interface) error { + return framework.UpdateDeployment(kubeClientSet, "ingress-nginx", "nginx-ingress-controller", 1, + func(deployment *appsv1beta1.Deployment) error { + args := deployment.Spec.Template.Spec.Containers[0].Args + var newArgs []string + for _, arg := range args { + if arg != "--enable-dynamic-configuration" { + newArgs = append(newArgs, arg) + } + } + deployment.Spec.Template.Spec.Containers[0].Args = newArgs + _, err := kubeClientSet.AppsV1beta1().Deployments("ingress-nginx").Update(deployment) + return err + }) +} + +func ensureIngress(f *framework.Framework, host string) (*extensions.Ingress, error) { + return f.EnsureIngress(&v1beta1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: host, + Namespace: f.Namespace.Name, + Annotations: map[string]string{ + "nginx.ingress.kubernetes.io/load-balance": "ewma", + }, + }, + Spec: v1beta1.IngressSpec{ + Rules: []v1beta1.IngressRule{ + { + Host: host, + IngressRuleValue: v1beta1.IngressRuleValue{ + HTTP: &v1beta1.HTTPIngressRuleValue{ + Paths: []v1beta1.HTTPIngressPath{ + { + Path: "/", + Backend: v1beta1.IngressBackend{ + ServiceName: "http-svc", + ServicePort: intstr.FromInt(80), + }, + }, + }, + }, + }, + }, + }, + }, + }) +}