From 498e8545b606623cad602c8784a76868075e0d80 Mon Sep 17 00:00:00 2001 From: Eli Mallon Date: Tue, 15 Sep 2020 04:48:32 -0700 Subject: [PATCH] feat: update more than one LoadBalancer ip Co-authored-by: kevinpollet --- pkg/provider/kubernetes/ingress/client.go | 58 +++++++++----- .../kubernetes/ingress/client_mock_test.go | 2 +- .../kubernetes/ingress/client_test.go | 78 +++++++++++++++++++ pkg/provider/kubernetes/ingress/kubernetes.go | 4 +- 4 files changed, 119 insertions(+), 23 deletions(-) diff --git a/pkg/provider/kubernetes/ingress/client.go b/pkg/provider/kubernetes/ingress/client.go index fc54c48d5..975b5b03f 100644 --- a/pkg/provider/kubernetes/ingress/client.go +++ b/pkg/provider/kubernetes/ingress/client.go @@ -54,7 +54,7 @@ type Client interface { GetService(namespace, name string) (*corev1.Service, bool, error) GetSecret(namespace, name string) (*corev1.Secret, bool, error) GetEndpoints(namespace, name string) (*corev1.Endpoints, bool, error) - UpdateIngressStatus(ing *networkingv1beta1.Ingress, ip, hostname string) error + UpdateIngressStatus(ing *networkingv1beta1.Ingress, ingStatus []corev1.LoadBalancerIngress) error GetServerVersion() (*version.Version, error) } @@ -230,13 +230,13 @@ func extensionsToNetworking(ing proto.Marshaler) (*networkingv1beta1.Ingress, er } // UpdateIngressStatus updates an Ingress with a provided status. -func (c *clientWrapper) UpdateIngressStatus(src *networkingv1beta1.Ingress, ip, hostname string) error { +func (c *clientWrapper) UpdateIngressStatus(src *networkingv1beta1.Ingress, ingStatus []corev1.LoadBalancerIngress) error { if !c.isWatchedNamespace(src.Namespace) { return fmt.Errorf("failed to get ingress %s/%s: namespace is not within watched namespaces", src.Namespace, src.Name) } if src.GetObjectKind().GroupVersionKind().Group != "networking.k8s.io" { - return c.updateIngressStatusOld(src, ip, hostname) + return c.updateIngressStatusOld(src, ingStatus) } ing, err := c.factories[c.lookupNamespace(src.Namespace)].Networking().V1beta1().Ingresses().Lister().Ingresses(src.Namespace).Get(src.Name) @@ -244,16 +244,15 @@ func (c *clientWrapper) UpdateIngressStatus(src *networkingv1beta1.Ingress, ip, return fmt.Errorf("failed to get ingress %s/%s: %w", src.Namespace, src.Name, err) } - if len(ing.Status.LoadBalancer.Ingress) > 0 { - if ing.Status.LoadBalancer.Ingress[0].Hostname == hostname && ing.Status.LoadBalancer.Ingress[0].IP == ip { - // If status is already set, skip update - log.Debugf("Skipping status update on ingress %s/%s", ing.Namespace, ing.Name) - return nil - } + logger := log.WithoutContext().WithField("namespace", ing.Namespace).WithField("ingress", ing.Name) + + if isLoadBalancerIngressEquals(ing.Status.LoadBalancer.Ingress, ingStatus) { + logger.Debug("Skipping ingress status update") + return nil } ingCopy := ing.DeepCopy() - ingCopy.Status = networkingv1beta1.IngressStatus{LoadBalancer: corev1.LoadBalancerStatus{Ingress: []corev1.LoadBalancerIngress{{IP: ip, Hostname: hostname}}}} + ingCopy.Status = networkingv1beta1.IngressStatus{LoadBalancer: corev1.LoadBalancerStatus{Ingress: ingStatus}} ctx, cancel := context.WithTimeout(context.Background(), defaultTimeout) defer cancel() @@ -263,26 +262,25 @@ func (c *clientWrapper) UpdateIngressStatus(src *networkingv1beta1.Ingress, ip, return fmt.Errorf("failed to update ingress status %s/%s: %w", src.Namespace, src.Name, err) } - log.Infof("Updated status on ingress %s/%s", src.Namespace, src.Name) + logger.Info("Updated ingress status") return nil } -func (c *clientWrapper) updateIngressStatusOld(src *networkingv1beta1.Ingress, ip, hostname string) error { +func (c *clientWrapper) updateIngressStatusOld(src *networkingv1beta1.Ingress, ingStatus []corev1.LoadBalancerIngress) error { ing, err := c.factories[c.lookupNamespace(src.Namespace)].Extensions().V1beta1().Ingresses().Lister().Ingresses(src.Namespace).Get(src.Name) if err != nil { return fmt.Errorf("failed to get ingress %s/%s: %w", src.Namespace, src.Name, err) } - if len(ing.Status.LoadBalancer.Ingress) > 0 { - if ing.Status.LoadBalancer.Ingress[0].Hostname == hostname && ing.Status.LoadBalancer.Ingress[0].IP == ip { - // If status is already set, skip update - log.Debugf("Skipping status update on ingress %s/%s", ing.Namespace, ing.Name) - return nil - } + logger := log.WithoutContext().WithField("namespace", ing.Namespace).WithField("ingress", ing.Name) + + if isLoadBalancerIngressEquals(ing.Status.LoadBalancer.Ingress, ingStatus) { + logger.Debug("Skipping ingress status update") + return nil } ingCopy := ing.DeepCopy() - ingCopy.Status = extensionsv1beta1.IngressStatus{LoadBalancer: corev1.LoadBalancerStatus{Ingress: []corev1.LoadBalancerIngress{{IP: ip, Hostname: hostname}}}} + ingCopy.Status = extensionsv1beta1.IngressStatus{LoadBalancer: corev1.LoadBalancerStatus{Ingress: ingStatus}} ctx, cancel := context.WithTimeout(context.Background(), defaultTimeout) defer cancel() @@ -292,10 +290,30 @@ func (c *clientWrapper) updateIngressStatusOld(src *networkingv1beta1.Ingress, i return fmt.Errorf("failed to update ingress status %s/%s: %w", src.Namespace, src.Name, err) } - log.Infof("Updated status on ingress %s/%s", src.Namespace, src.Name) + logger.Info("Updated ingress status") return nil } +// isLoadBalancerIngressEquals returns true if the given slices are equal, false otherwise. +func isLoadBalancerIngressEquals(aSlice []corev1.LoadBalancerIngress, bSlice []corev1.LoadBalancerIngress) bool { + if len(aSlice) != len(bSlice) { + return false + } + + aMap := make(map[string]struct{}) + for _, aIngress := range aSlice { + aMap[aIngress.Hostname+aIngress.IP] = struct{}{} + } + + for _, bIngress := range bSlice { + if _, exists := aMap[bIngress.Hostname+bIngress.IP]; !exists { + return false + } + } + + return true +} + // GetService returns the named service from the given namespace. func (c *clientWrapper) GetService(namespace, name string) (*corev1.Service, bool, error) { if !c.isWatchedNamespace(namespace) { diff --git a/pkg/provider/kubernetes/ingress/client_mock_test.go b/pkg/provider/kubernetes/ingress/client_mock_test.go index 3ef9093db..4b15cad86 100644 --- a/pkg/provider/kubernetes/ingress/client_mock_test.go +++ b/pkg/provider/kubernetes/ingress/client_mock_test.go @@ -125,6 +125,6 @@ func (c clientMock) WatchAll(namespaces []string, stopCh <-chan struct{}) (<-cha return c.watchChan, nil } -func (c clientMock) UpdateIngressStatus(_ *networkingv1beta1.Ingress, _, _ string) error { +func (c clientMock) UpdateIngressStatus(_ *networkingv1beta1.Ingress, _ []corev1.LoadBalancerIngress) error { return c.apiIngressStatusError } diff --git a/pkg/provider/kubernetes/ingress/client_test.go b/pkg/provider/kubernetes/ingress/client_test.go index f582502a7..4b838c5f5 100644 --- a/pkg/provider/kubernetes/ingress/client_test.go +++ b/pkg/provider/kubernetes/ingress/client_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" kubeerror "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime/schema" ) @@ -47,3 +48,80 @@ func TestTranslateNotFoundError(t *testing.T) { }) } } + +func TestIsLoadBalancerIngressEquals(t *testing.T) { + testCases := []struct { + desc string + aSlice []corev1.LoadBalancerIngress + bSlice []corev1.LoadBalancerIngress + expectedEqual bool + }{ + { + desc: "both slices are empty", + expectedEqual: true, + }, + { + desc: "not the same length", + bSlice: []corev1.LoadBalancerIngress{ + {IP: "192.168.1.1", Hostname: "traefik"}, + }, + expectedEqual: false, + }, + { + desc: "same ordered content", + aSlice: []corev1.LoadBalancerIngress{ + {IP: "192.168.1.1", Hostname: "traefik"}, + }, + bSlice: []corev1.LoadBalancerIngress{ + {IP: "192.168.1.1", Hostname: "traefik"}, + }, + expectedEqual: true, + }, + { + desc: "same unordered content", + aSlice: []corev1.LoadBalancerIngress{ + {IP: "192.168.1.1", Hostname: "traefik"}, + {IP: "192.168.1.2", Hostname: "traefik2"}, + }, + bSlice: []corev1.LoadBalancerIngress{ + {IP: "192.168.1.2", Hostname: "traefik2"}, + {IP: "192.168.1.1", Hostname: "traefik"}, + }, + expectedEqual: true, + }, + { + desc: "different ordered content", + aSlice: []corev1.LoadBalancerIngress{ + {IP: "192.168.1.1", Hostname: "traefik"}, + {IP: "192.168.1.2", Hostname: "traefik2"}, + }, + bSlice: []corev1.LoadBalancerIngress{ + {IP: "192.168.1.1", Hostname: "traefik"}, + {IP: "192.168.1.2", Hostname: "traefik"}, + }, + expectedEqual: false, + }, + { + desc: "different unordered content", + aSlice: []corev1.LoadBalancerIngress{ + {IP: "192.168.1.1", Hostname: "traefik"}, + {IP: "192.168.1.2", Hostname: "traefik2"}, + }, + bSlice: []corev1.LoadBalancerIngress{ + {IP: "192.168.1.2", Hostname: "traefik3"}, + {IP: "192.168.1.1", Hostname: "traefik"}, + }, + expectedEqual: false, + }, + } + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + gotEqual := isLoadBalancerIngressEquals(test.aSlice, test.bSlice) + assert.Equal(t, test.expectedEqual, gotEqual) + }) + } +} diff --git a/pkg/provider/kubernetes/ingress/kubernetes.go b/pkg/provider/kubernetes/ingress/kubernetes.go index eefc68679..838abe091 100644 --- a/pkg/provider/kubernetes/ingress/kubernetes.go +++ b/pkg/provider/kubernetes/ingress/kubernetes.go @@ -301,7 +301,7 @@ func (p *Provider) updateIngressStatus(ing *networkingv1beta1.Ingress, k8sClient return errors.New("publishedService or ip or hostname must be defined") } - return k8sClient.UpdateIngressStatus(ing, p.IngressEndpoint.IP, p.IngressEndpoint.Hostname) + return k8sClient.UpdateIngressStatus(ing, []corev1.LoadBalancerIngress{{IP: p.IngressEndpoint.IP, Hostname: p.IngressEndpoint.Hostname}}) } serviceInfo := strings.Split(p.IngressEndpoint.PublishedService, "/") @@ -326,7 +326,7 @@ func (p *Provider) updateIngressStatus(ing *networkingv1beta1.Ingress, k8sClient return fmt.Errorf("missing service: %s", p.IngressEndpoint.PublishedService) } - return k8sClient.UpdateIngressStatus(ing, service.Status.LoadBalancer.Ingress[0].IP, service.Status.LoadBalancer.Ingress[0].Hostname) + return k8sClient.UpdateIngressStatus(ing, service.Status.LoadBalancer.Ingress) } func (p *Provider) shouldProcessIngress(providerIngressClass string, ingress *networkingv1beta1.Ingress, ingressClass *networkingv1beta1.IngressClass) bool {