Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix bug with lua e2e test suite #2867

Merged
merged 1 commit into from
Jul 28, 2018
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 16 additions & 19 deletions test/e2e/lua/dynamic_configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/intstr"

"k8s.io/ingress-nginx/internal/net/dns"
"k8s.io/ingress-nginx/test/e2e/framework"
)

Expand All @@ -42,6 +41,7 @@ const (
logBackendReloadSuccess = "Backend successfully reloaded"
logSkipBackendReload = "Changes handled by the dynamic configuration, skipping backend reload"
logInitialConfigSync = "Initial synchronization of the NGINX configuration"
waitForLuaSync = 2 * time.Second
)

var _ = framework.IngressNginxDescribe("Dynamic Configuration", func() {
Expand All @@ -51,16 +51,6 @@ var _ = framework.IngressNginxDescribe("Dynamic Configuration", func() {
err := f.NewEchoDeploymentWithReplicas(1)
Expect(err).NotTo(HaveOccurred())

err = f.WaitForNginxConfiguration(func(cfg string) bool {
servers, err := dns.GetSystemNameServers()
Expect(err).NotTo(HaveOccurred())
ips := []string{}
for _, server := range servers {
ips = append(ips, fmt.Sprintf("\"%v\"", server))
}
return strings.Contains(cfg, "configuration.nameservers = { "+strings.Join(ips, ", ")+" }")
})

Copy link
Member Author

@ElvinEfendi ElvinEfendi Jul 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you can see I wasn't asserting on err here and the check was timing out (because the condition never matches, executing dns.GetSystemNameServers() returns resolvers for the current host not for the running pod) silently after 5 minutes, and this was happening before every test. Therefore e2e test runtime doubled.

host := "foo.com"
ing, err := ensureIngress(f, host)
Expect(err).NotTo(HaveOccurred())
Expand All @@ -72,8 +62,7 @@ var _ = framework.IngressNginxDescribe("Dynamic Configuration", func() {
})
Expect(err).NotTo(HaveOccurred())

// give some time for Lua to sync the backend
time.Sleep(5 * time.Second)
time.Sleep(waitForLuaSync)

resp, _, errs := gorequest.New().
Get(f.IngressController.HTTPURL).
Expand All @@ -88,6 +77,14 @@ var _ = framework.IngressNginxDescribe("Dynamic Configuration", func() {
Expect(log).To(ContainSubstring(logDynamicConfigSuccess))
})

It("should set nameservers for Lua", func() {
err := f.WaitForNginxConfiguration(func(cfg string) bool {
r := regexp.MustCompile(`configuration.nameservers = { [".,0-9a-zA-Z]+ }`)
return r.MatchString(cfg)
})
Expect(err).NotTo(HaveOccurred())
})

Context("when only backends change", func() {
It("should handle endpoints only changes", func() {
resp, _, errs := gorequest.New().
Expand All @@ -100,7 +97,7 @@ var _ = framework.IngressNginxDescribe("Dynamic Configuration", func() {
replicas := 2
err := framework.UpdateDeployment(f.KubeClientSet, f.IngressController.Namespace, "http-svc", replicas, nil)
Expect(err).NotTo(HaveOccurred())
time.Sleep(5 * time.Second)
time.Sleep(waitForLuaSync)

log, err := f.NginxLogs()
Expect(err).ToNot(HaveOccurred())
Expand Down Expand Up @@ -131,7 +128,7 @@ var _ = framework.IngressNginxDescribe("Dynamic Configuration", func() {
replicas = 4
err = framework.UpdateDeployment(f.KubeClientSet, f.IngressController.Namespace, "http-svc", replicas, nil)
Expect(err).NotTo(HaveOccurred())
time.Sleep(5 * time.Second)
time.Sleep(waitForLuaSync)

resp, _, errs := gorequest.New().
Get(f.IngressController.HTTPURL).
Expand Down Expand Up @@ -170,7 +167,7 @@ var _ = framework.IngressNginxDescribe("Dynamic Configuration", func() {
ingress.ObjectMeta.Annotations["nginx.ingress.kubernetes.io/load-balance"] = "round_robin"
_, err = f.KubeClientSet.ExtensionsV1beta1().Ingresses(f.IngressController.Namespace).Update(ingress)
Expect(err).ToNot(HaveOccurred())
time.Sleep(5 * time.Second)
time.Sleep(waitForLuaSync)

log, err := f.NginxLogs()
Expect(err).ToNot(HaveOccurred())
Expand Down Expand Up @@ -246,7 +243,7 @@ var _ = framework.IngressNginxDescribe("Dynamic Configuration", func() {

err = framework.UpdateDeployment(f.KubeClientSet, f.IngressController.Namespace, "http-svc", 2, nil)
Expect(err).NotTo(HaveOccurred())
time.Sleep(10 * time.Second)
time.Sleep(waitForLuaSync)

resp, body, errs := gorequest.New().
Get(fmt.Sprintf("%s?a-unique-request-uri", f.IngressController.HTTPURL)).
Expand Down Expand Up @@ -290,7 +287,7 @@ var _ = framework.IngressNginxDescribe("Dynamic Configuration", func() {
By("Increasing the number of service replicas")
err = framework.UpdateDeployment(f.KubeClientSet, f.IngressController.Namespace, "http-svc", 2, nil)
Expect(err).NotTo(HaveOccurred())
time.Sleep(5 * time.Second)
time.Sleep(waitForLuaSync)

By("Making a first request")
host := "foo.com"
Expand Down Expand Up @@ -346,7 +343,7 @@ var _ = framework.IngressNginxDescribe("Dynamic Configuration", func() {
}
_, err = f.KubeClientSet.ExtensionsV1beta1().Ingresses(f.IngressController.Namespace).Update(ingress)
Expect(err).ToNot(HaveOccurred())
time.Sleep(5 * time.Second)
time.Sleep(waitForLuaSync)

By("Making a request")
host := "foo.com"
Expand Down