simplify customhttperrors e2e test and add regression test and fix a bug

internal/ingress/types_equals.go

@@ -449,6 +449,22 @@ func (l1 *Location) Equal(l2 *Location) bool {
 		return false
 	}
 
+	if len(l1.CustomHTTPErrors) != len(l2.CustomHTTPErrors) {
+		return false
+	}
+	for _, code1 := range l1.CustomHTTPErrors {
+		found := false
+		for _, code2 := range l2.CustomHTTPErrors {
+			if code1 == code2 {
+				found = true
+				break
+			}
+		}
+		if !found {
+			return false
+		}
+	}
+
 	if !(&l1.ModSecurity).Equal(&l2.ModSecurity) {
 		return false
 	}

test/e2e/annotations/customhttperrors.go

@@ -22,20 +22,23 @@ import (
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
+
+	extensions "k8s.io/api/extensions/v1beta1"
+
 	"k8s.io/ingress-nginx/test/e2e/framework"
 )
 
 var _ = framework.IngressNginxDescribe("Annotations - custom-http-errors", func() {
 	f := framework.NewDefaultFramework("custom-http-errors")
 
 	BeforeEach(func() {
-		f.NewEchoDeploymentWithReplicas(2)
+		f.NewEchoDeploymentWithReplicas(1)
 	})
 
 	AfterEach(func() {
 	})
 
-	It("should set proxy_intercept_errors", func() {
+	It("configures Nginx correctly", func() {
 		host := "customerrors.foo.com"
 
 		errorCodes := []string{"404", "500"}
@@ -47,73 +50,51 @@ var _ = framework.IngressNginxDescribe("Annotations - custom-http-errors", func(
 		ing := framework.NewSingleIngress(host, "/", host, f.IngressController.Namespace, "http-svc", 80, &annotations)
 		f.EnsureIngress(ing)
 
-		f.WaitForNginxServer(host,
-			func(server string) bool {
-				return Expect(server).Should(ContainSubstring("proxy_intercept_errors on;"))
-			})
-	})
+		var serverConfig string
+		f.WaitForNginxServer(host, func(sc string) bool {
+			serverConfig = sc
+			return strings.Contains(serverConfig, fmt.Sprintf("server_name %s", host))
+		})
 
-	It("should create error routes", func() {
-		host := "customerrors.foo.com"
-		errorCodes := []string{"404", "500"}
-
-		annotations := map[string]string{
-			"nginx.ingress.kubernetes.io/custom-http-errors": strings.Join(errorCodes, ","),
-		}
-
-		ing := framework.NewSingleIngress(host, "/test", host, f.IngressController.Namespace, "http-svc", 80, &annotations)
-		f.EnsureIngress(ing)
+		By("turning on proxy_intercept_errors directive")
+		Expect(serverConfig).Should(ContainSubstring("proxy_intercept_errors on;"))
 
+		By("configuring error_page directive")
 		for _, code := range errorCodes {
-			f.WaitForNginxServer(host,
-				func(server string) bool {
-					return Expect(server).Should(ContainSubstring(fmt.Sprintf("@custom_%s", code)))
-				})
-		}
-	})
-
-	It("should set up error_page routing", func() {
-		host := "customerrors.foo.com"
-		errorCodes := []string{"404", "500"}
-
-		annotations := map[string]string{
-			"nginx.ingress.kubernetes.io/custom-http-errors": strings.Join(errorCodes, ","),
+			Expect(serverConfig).Should(ContainSubstring(fmt.Sprintf("error_page %s = @custom_%s", code, code)))
 		}
 
-		ing := framework.NewSingleIngress(host, "/test", host, f.IngressController.Namespace, "http-svc", 80, &annotations)
-		f.EnsureIngress(ing)
-
+		By("creating error locations")
 		for _, code := range errorCodes {
-			f.WaitForNginxServer(host,
-				func(server string) bool {
-					return Expect(server).Should(ContainSubstring(fmt.Sprintf("error_page %s = @custom_%s", code, code)))
-				})
+			Expect(serverConfig).Should(ContainSubstring(fmt.Sprintf("location @custom_%s", code)))
 		}
-	})
 
-	It("should create only one of each error route", func() {
-		host := "customerrors.foo.com"
-		errorCodes := [][]string{{"404", "500"}, {"400", "404"}}
-
-		for i, codeSet := range errorCodes {
-			annotations := map[string]string{
-				"nginx.ingress.kubernetes.io/custom-http-errors": strings.Join(codeSet, ","),
+		By("updating configuration when only custom-http-error value changes")
+		err := framework.UpdateIngress(f.KubeClientSet, f.IngressController.Namespace, host, func(ingress *extensions.Ingress) error {
+			ingress.ObjectMeta.Annotations["nginx.ingress.kubernetes.io/custom-http-errors"] = "503"
+			return nil
+		})
+		Expect(err).ToNot(HaveOccurred())
+		f.WaitForNginxServer(host, func(sc string) bool {
+			if serverConfig != sc {
+				serverConfig = sc
+				return true
 			}
-
-			ing := framework.NewSingleIngress(
-				fmt.Sprintf("%s-%d", host, i), fmt.Sprintf("/test-%d", i),
-				host, f.IngressController.Namespace, "http-svc", 80, &annotations)
-			f.EnsureIngress(ing)
-		}
-
-		for _, codeSet := range errorCodes {
-			for _, code := range codeSet {
-				f.WaitForNginxServer(host,
-					func(server string) bool {
-						count := strings.Count(server, fmt.Sprintf("location @custom_%s", code))
-						return Expect(count).Should(Equal(1))
-					})
-			}
-		}
+			return false
+		})
+		Expect(serverConfig).Should(ContainSubstring("location @custom_503"))
+		Expect(serverConfig).ShouldNot(ContainSubstring("location @custom_400"))
+		Expect(serverConfig).ShouldNot(ContainSubstring("location @custom_500"))
+
+		By("ignoring duplicate values (503 in this case) per server")
+		annotations["nginx.ingress.kubernetes.io/custom-http-errors"] = "404, 503"
+		ing = framework.NewSingleIngress(fmt.Sprintf("%s-else", host), "/else", host, f.IngressController.Namespace, "http-svc", 80, &annotations)
+		f.EnsureIngress(ing)
+		f.WaitForNginxServer(host, func(sc string) bool {
+			serverConfig = sc
+			return strings.Contains(serverConfig, "location /else")
+		})
+		count := strings.Count(serverConfig, fmt.Sprintf("location @custom_503"))
+		Expect(count).Should(Equal(1))
 	})
 })