From bae49a46570afdfbbac41750517384144cc88495 Mon Sep 17 00:00:00 2001 From: Manuel Alejandro de Brito Fontes Date: Sat, 18 May 2019 06:08:05 -0400 Subject: [PATCH] Refactor ListIngresses to add filters --- internal/ingress/controller/controller.go | 30 +++++++++---------- .../ingress/controller/controller_test.go | 8 +++-- internal/ingress/controller/nginx.go | 5 ++-- internal/ingress/controller/store/store.go | 12 ++++++-- .../ingress/controller/store/store_test.go | 2 +- internal/ingress/status/status.go | 5 ++-- internal/ingress/status/status_test.go | 3 +- 7 files changed, 39 insertions(+), 26 deletions(-) diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index 4af63c17f..5d2b46810 100755 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -122,8 +122,7 @@ func (n *NGINXController) syncIngress(interface{}) error { return nil } - ings := n.store.ListIngresses() - + ings := n.store.ListIngresses(nil) hosts, servers, pcfg := n.getConfiguration(ings) if n.runningConfig.Equal(pcfg) { @@ -202,37 +201,36 @@ func (n *NGINXController) syncIngress(interface{}) error { // CheckIngress returns an error in case the provided ingress, when added // to the current configuration, generates an invalid configuration func (n *NGINXController) CheckIngress(ing *extensions.Ingress) error { + //TODO: this is wrong if n == nil { return fmt.Errorf("cannot check ingress on a nil ingress controller") } + if ing == nil { // no ingress to add, no state change return nil } + if !class.IsValid(ing) { klog.Infof("ignoring ingress %v in %v based on annotation %v", ing.Name, ing.ObjectMeta.Namespace, class.IngressKey) return nil } + if n.cfg.Namespace != "" && ing.ObjectMeta.Namespace != n.cfg.Namespace { klog.Infof("ignoring ingress %v in namespace %v different from the namespace watched %s", ing.Name, ing.ObjectMeta.Namespace, n.cfg.Namespace) return nil } - ings := n.store.ListIngresses() - newIngress := &ingress.Ingress{ - Ingress: *ing, - ParsedAnnotations: annotations.NewAnnotationExtractor(n.store).Extract(ing), + filter := func(toCheck *ingress.Ingress) bool { + return toCheck.ObjectMeta.Namespace == ing.ObjectMeta.Namespace && + toCheck.ObjectMeta.Name == ing.ObjectMeta.Name } - for i, ingress := range ings { - if ingress.Ingress.ObjectMeta.Name == ing.ObjectMeta.Name && ingress.Ingress.ObjectMeta.Namespace == ing.ObjectMeta.Namespace { - ings[i] = newIngress - newIngress = nil - } - } - if newIngress != nil { - ings = append(ings, newIngress) - } + ings := n.store.ListIngresses(filter) + ings = append(ings, &ingress.Ingress{ + Ingress: *ing, + ParsedAnnotations: annotations.NewAnnotationExtractor(n.store).Extract(ing), + }) _, _, pcfg := n.getConfiguration(ings) @@ -244,12 +242,14 @@ func (n *NGINXController) CheckIngress(ing *extensions.Ingress) error { n.metricCollector.IncCheckErrorCount(ing.ObjectMeta.Namespace, ing.Name) return err } + err = n.testTemplate(content) if err != nil { n.metricCollector.IncCheckErrorCount(ing.ObjectMeta.Namespace, ing.Name) } else { n.metricCollector.IncCheckCount(ing.ObjectMeta.Namespace, ing.Name) } + return err } diff --git a/internal/ingress/controller/controller_test.go b/internal/ingress/controller/controller_test.go index 43387f585..3ef4d6338 100644 --- a/internal/ingress/controller/controller_test.go +++ b/internal/ingress/controller/controller_test.go @@ -30,12 +30,13 @@ import ( "time" "github.com/eapache/channels" - "k8s.io/api/core/v1" corev1 "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" 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/fake" + "k8s.io/ingress-nginx/internal/file" "k8s.io/ingress-nginx/internal/ingress" "k8s.io/ingress-nginx/internal/ingress/annotations" @@ -76,7 +77,7 @@ func (fakeIngressStore) GetServiceEndpoints(key string) (*corev1.Endpoints, erro return nil, fmt.Errorf("test error") } -func (fis fakeIngressStore) ListIngresses() []*ingress.Ingress { +func (fis fakeIngressStore) ListIngresses(store.IngressFilterFunc) []*ingress.Ingress { return fis.ingresses } @@ -209,7 +210,8 @@ func TestCheckIngress(t *testing.T) { nginx.store = fakeIngressStore{ ingresses: []*ingress.Ingress{ { - Ingress: *ing, + Ingress: *ing, + ParsedAnnotations: &annotations.Ingress{}, }, }, } diff --git a/internal/ingress/controller/nginx.go b/internal/ingress/controller/nginx.go index 34c23ad50..20cd90cb1 100755 --- a/internal/ingress/controller/nginx.go +++ b/internal/ingress/controller/nginx.go @@ -45,6 +45,9 @@ import ( v1core "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/tools/record" "k8s.io/client-go/util/flowcontrol" + "k8s.io/klog" + "k8s.io/kubernetes/pkg/util/filesystem" + adm_controler "k8s.io/ingress-nginx/internal/admission/controller" "k8s.io/ingress-nginx/internal/file" "k8s.io/ingress-nginx/internal/ingress" @@ -62,8 +65,6 @@ import ( "k8s.io/ingress-nginx/internal/nginx" "k8s.io/ingress-nginx/internal/task" "k8s.io/ingress-nginx/internal/watch" - "k8s.io/klog" - "k8s.io/kubernetes/pkg/util/filesystem" ) const ( diff --git a/internal/ingress/controller/store/store.go b/internal/ingress/controller/store/store.go index e537b1894..b756fb9fd 100755 --- a/internal/ingress/controller/store/store.go +++ b/internal/ingress/controller/store/store.go @@ -56,6 +56,9 @@ import ( "k8s.io/ingress-nginx/internal/k8s" ) +// IngressFilterFunc decides if an Ingress should be ommited or not +type IngressFilterFunc func(*ingress.Ingress) bool + // Storer is the interface that wraps the required methods to gather information // about ingresses, services, secrets and ingress annotations. type Storer interface { @@ -75,7 +78,7 @@ type Storer interface { GetServiceEndpoints(key string) (*corev1.Endpoints, error) // ListIngresses returns a list of all Ingresses in the store. - ListIngresses() []*ingress.Ingress + ListIngresses(IngressFilterFunc) []*ingress.Ingress // GetRunningControllerPodsCount returns the number of Running ingress-nginx controller Pods. GetRunningControllerPodsCount() int @@ -758,11 +761,16 @@ func (s *k8sStore) getIngress(key string) (*extensions.Ingress, error) { } // ListIngresses returns the list of Ingresses -func (s *k8sStore) ListIngresses() []*ingress.Ingress { +func (s *k8sStore) ListIngresses(filter IngressFilterFunc) []*ingress.Ingress { // filter ingress rules ingresses := make([]*ingress.Ingress, 0) for _, item := range s.listers.IngressWithAnnotation.List() { ing := item.(*ingress.Ingress) + + if filter != nil && filter(ing) { + continue + } + ingresses = append(ingresses, ing) } diff --git a/internal/ingress/controller/store/store_test.go b/internal/ingress/controller/store/store_test.go index a8f8a2a64..425c6004e 100755 --- a/internal/ingress/controller/store/store_test.go +++ b/internal/ingress/controller/store/store_test.go @@ -1029,7 +1029,7 @@ func TestListIngresses(t *testing.T) { } s.listers.IngressWithAnnotation.Add(ingressWithNginxClass) - ingresses := s.ListIngresses() + ingresses := s.ListIngresses(nil) if s := len(ingresses); s != 3 { t.Errorf("Expected 3 Ingresses but got %v", s) diff --git a/internal/ingress/status/status.go b/internal/ingress/status/status.go index 399e3e39a..6dcb7baed 100644 --- a/internal/ingress/status/status.go +++ b/internal/ingress/status/status.go @@ -35,6 +35,7 @@ import ( "k8s.io/kubernetes/pkg/kubelet/util/sliceutils" "k8s.io/ingress-nginx/internal/ingress" + "k8s.io/ingress-nginx/internal/ingress/controller/store" "k8s.io/ingress-nginx/internal/k8s" "k8s.io/ingress-nginx/internal/task" ) @@ -52,7 +53,7 @@ type Syncer interface { type ingressLister interface { // ListIngresses returns the list of Ingresses - ListIngresses() []*ingress.Ingress + ListIngresses(store.IngressFilterFunc) []*ingress.Ingress } // Config ... @@ -255,7 +256,7 @@ func sliceToStatus(endpoints []string) []apiv1.LoadBalancerIngress { // updateStatus changes the status information of Ingress rules func (s *statusSync) updateStatus(newIngressPoint []apiv1.LoadBalancerIngress) { - ings := s.IngressLister.ListIngresses() + ings := s.IngressLister.ListIngresses(nil) p := pool.NewLimited(10) defer p.Close() diff --git a/internal/ingress/status/status_test.go b/internal/ingress/status/status_test.go index c070dc21f..26a0bc3e6 100644 --- a/internal/ingress/status/status_test.go +++ b/internal/ingress/status/status_test.go @@ -28,6 +28,7 @@ import ( "k8s.io/ingress-nginx/internal/ingress" "k8s.io/ingress-nginx/internal/ingress/annotations/class" + "k8s.io/ingress-nginx/internal/ingress/controller/store" "k8s.io/ingress-nginx/internal/k8s" "k8s.io/ingress-nginx/internal/task" ) @@ -232,7 +233,7 @@ func buildExtensionsIngresses() []extensions.Ingress { type testIngressLister struct { } -func (til *testIngressLister) ListIngresses() []*ingress.Ingress { +func (til *testIngressLister) ListIngresses(store.IngressFilterFunc) []*ingress.Ingress { var ingresses []*ingress.Ingress ingresses = append(ingresses, &ingress.Ingress{ Ingress: extensions.Ingress{