From 2beb9f670dd3740db26c8f862399e2b1b1b29254 Mon Sep 17 00:00:00 2001 From: Maxim Date: Tue, 7 Sep 2021 10:47:16 -0700 Subject: [PATCH] Trigger syncIngress on Service addition/deletion #7346 (#7374) Normally Ingress sinchronization for Services is triggered when corresponding Service's Endpoints are added, deleted or modified. Services of type ExternalName, however, do not have any endpoints and hence do not trigger Ingress synchronization as only Update events are being watched. This commit makes sure that Update and Delete Service events also enqueue a syncIngress task. --- internal/ingress/controller/store/store.go | 18 +++ .../servicebackend/service_externalname.go | 124 ++++++++++-------- 2 files changed, 88 insertions(+), 54 deletions(-) diff --git a/internal/ingress/controller/store/store.go b/internal/ingress/controller/store/store.go index f65098a21..dd18594e9 100644 --- a/internal/ingress/controller/store/store.go +++ b/internal/ingress/controller/store/store.go @@ -660,6 +660,24 @@ func New( } serviceHandler := cache.ResourceEventHandlerFuncs{ + AddFunc: func(obj interface{}) { + svc := obj.(*corev1.Service) + if svc.Spec.Type == corev1.ServiceTypeExternalName { + updateCh.In() <- Event{ + Type: CreateEvent, + Obj: obj, + } + } + }, + DeleteFunc: func(obj interface{}) { + svc := obj.(*corev1.Service) + if svc.Spec.Type == corev1.ServiceTypeExternalName { + updateCh.In() <- Event{ + Type: DeleteEvent, + Obj: obj, + } + } + }, UpdateFunc: func(old, cur interface{}) { oldSvc := old.(*corev1.Service) curSvc := cur.(*corev1.Service) diff --git a/test/e2e/servicebackend/service_externalname.go b/test/e2e/servicebackend/service_externalname.go index 2c33c020f..d2a921cd3 100644 --- a/test/e2e/servicebackend/service_externalname.go +++ b/test/e2e/servicebackend/service_externalname.go @@ -34,6 +34,27 @@ import ( "k8s.io/ingress-nginx/test/e2e/framework" ) +func buildHTTPBinExternalNameService(f *framework.Framework, portName string) *corev1.Service { + return &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: framework.HTTPBinService, + Namespace: f.Namespace, + }, + Spec: corev1.ServiceSpec{ + ExternalName: "httpbin.org", + Type: corev1.ServiceTypeExternalName, + Ports: []corev1.ServicePort{ + { + Name: portName, + Port: 80, + TargetPort: intstr.FromInt(80), + Protocol: "TCP", + }, + }, + }, + } +} + var _ = framework.IngressNginxDescribe("[Service] Type ExternalName", func() { f := framework.NewDefaultFramework("type-externalname") @@ -107,24 +128,7 @@ var _ = framework.IngressNginxDescribe("[Service] Type ExternalName", func() { ginkgo.It("should return 200 for service type=ExternalName with a port defined", func() { host := "echo" - svc := &corev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: framework.HTTPBinService, - Namespace: f.Namespace, - }, - Spec: corev1.ServiceSpec{ - ExternalName: "httpbin.org", - Type: corev1.ServiceTypeExternalName, - Ports: []corev1.ServicePort{ - { - Name: host, - Port: 80, - TargetPort: intstr.FromInt(80), - Protocol: "TCP", - }, - }, - }, - } + svc := buildHTTPBinExternalNameService(f, host) f.EnsureService(svc) annotations := map[string]string{ @@ -179,24 +183,7 @@ var _ = framework.IngressNginxDescribe("[Service] Type ExternalName", func() { ginkgo.It("should return 200 for service type=ExternalName using a port name", func() { host := "echo" - svc := &corev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: framework.HTTPBinService, - Namespace: f.Namespace, - }, - Spec: corev1.ServiceSpec{ - ExternalName: "httpbin.org", - Type: corev1.ServiceTypeExternalName, - Ports: []corev1.ServicePort{ - { - Name: host, - Port: 80, - TargetPort: intstr.FromInt(80), - Protocol: "TCP", - }, - }, - }, - } + svc := buildHTTPBinExternalNameService(f, host) f.EnsureService(svc) annotations := map[string]string{ @@ -260,24 +247,7 @@ var _ = framework.IngressNginxDescribe("[Service] Type ExternalName", func() { ginkgo.It("should update the external name after a service update", func() { host := "echo" - svc := &corev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: framework.HTTPBinService, - Namespace: f.Namespace, - }, - Spec: corev1.ServiceSpec{ - ExternalName: "httpbin.org", - Type: corev1.ServiceTypeExternalName, - Ports: []corev1.ServicePort{ - { - Name: host, - Port: 80, - TargetPort: intstr.FromInt(80), - Protocol: "TCP", - }, - }, - }, - } + svc := buildHTTPBinExternalNameService(f, host) f.EnsureService(svc) annotations := map[string]string{ @@ -336,4 +306,50 @@ var _ = framework.IngressNginxDescribe("[Service] Type ExternalName", func() { assert.Nil(ginkgo.GinkgoT(), err) assert.Contains(ginkgo.GinkgoT(), output, `{"address":"eu.httpbin.org"`) }) + + ginkgo.It("should sync ingress on external name service addition/deletion", func() { + host := "echo" + + // Create the Ingress first + ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.HTTPBinService, 80, nil) + f.EnsureIngress(ing) + + f.WaitForNginxServer(host, + func(server string) bool { + return strings.Contains(server, "proxy_pass http://upstream_balancer;") + }) + + // Nginx should return 503 without the underlying service being available + f.HTTPTestClient(). + GET("/get"). + WithHeader("Host", host). + Expect(). + Status(http.StatusServiceUnavailable) + + // Now create the service + svc := buildHTTPBinExternalNameService(f, host) + f.EnsureService(svc) + + framework.Sleep() + + // 503 should change to 200 OK + f.HTTPTestClient(). + GET("/get"). + WithHeader("Host", host). + Expect(). + Status(http.StatusOK) + + // And back to 503 after deleting the service + + err := f.KubeClientSet.CoreV1().Services(f.Namespace).Delete(context.TODO(), framework.HTTPBinService, metav1.DeleteOptions{}) + assert.Nil(ginkgo.GinkgoT(), err, "unexpected error deleting httpbin service") + + framework.Sleep() + + f.HTTPTestClient(). + GET("/get"). + WithHeader("Host", host). + Expect(). + Status(http.StatusServiceUnavailable) + }) })