MINOR: update to management of ingress statuses

Author: ivanmatmati

deploy/tests/tnr/routeacl/suite_test.go

@@ -18,7 +18,9 @@ import (
 	"os"
 	"testing"
 
+	"github.com/haproxytech/kubernetes-ingress/pkg/annotations"
 	c "github.com/haproxytech/kubernetes-ingress/pkg/controller"
+	"github.com/haproxytech/kubernetes-ingress/pkg/haproxy"
 	"github.com/haproxytech/kubernetes-ingress/pkg/haproxy/env"
 	"github.com/haproxytech/kubernetes-ingress/pkg/ingress"
 	"github.com/haproxytech/kubernetes-ingress/pkg/k8s"
@@ -35,6 +37,13 @@ type UseBackendSuite struct {
 	test Test
 }
 
+type updateStatusManager struct{}
+
+func (m *updateStatusManager) AddIngress(ingress *ingress.Ingress) {}
+func (m *updateStatusManager) Update(k store.K8s, h haproxy.HAProxy, a annotations.Annotations) (reload bool, err error) {
+	return
+}
+
 func TestUseBackend(t *testing.T) {
 	suite.Run(t, new(UseBackendSuite))
 }
@@ -118,7 +127,7 @@ func (suite *UseBackendSuite) UseBackendFixture() (eventChan chan k8s.SyncDataEv
 		WithEventChan(eventChan).
 		WithStore(s).
 		WithHaproxyEnv(haproxyEnv).
-		WithUpdatePublishServiceFunc(func(ingresses []*ingress.Ingress, publishServiceAddresses []string) {}).
+		WithUpdateStatusManager(&updateStatusManager{}).
 		WithArgs(osArgs).Build()
 
 	go controller.Start()

main.go

@@ -113,7 +113,6 @@ func main() {
 		WithHaproxyCfgFile(haproxyConf).
 		WithEventChan(eventChan).
 		WithStore(s).
-		WithUpdatePublishServiceFunc(k.UpdatePublishService).
 		WithClientSet(k.GetClientset()).
 		WithRestClientSet(k.GetRestClientset()).
 		WithArgs(osArgs).Build()

pkg/controller/builder.go

@@ -36,6 +36,7 @@ import (
 	"github.com/haproxytech/kubernetes-ingress/pkg/haproxy/rules"
 	"github.com/haproxytech/kubernetes-ingress/pkg/ingress"
 	"github.com/haproxytech/kubernetes-ingress/pkg/k8s"
+	"github.com/haproxytech/kubernetes-ingress/pkg/status"
 	"github.com/haproxytech/kubernetes-ingress/pkg/store"
 	"github.com/haproxytech/kubernetes-ingress/pkg/utils"
 )
@@ -54,6 +55,7 @@ type Builder struct {
 	haproxyCfgFile           []byte
 	store                    store.K8s
 	osArgs                   utils.OSArgs
+	updateStatusManager      status.UpdateStatusManager
 }
 
 var defaultEnv = env.Env{
@@ -123,11 +125,6 @@ func (builder *Builder) WithArgs(osArgs utils.OSArgs) *Builder {
 	return builder
 }
 
-func (builder *Builder) WithUpdatePublishServiceFunc(updatePublishServiceFunc func(ingresses []*ingress.Ingress, publishServiceAddresses []string)) *Builder {
-	builder.updatePublishServiceFunc = updatePublishServiceFunc
-	return builder
-}
-
 func (builder *Builder) WithClientSet(clientSet *kubernetes.Clientset) *Builder {
 	builder.clientSet = clientSet
 	return builder
@@ -143,6 +140,11 @@ func (builder *Builder) WithGatewayManager(gatewayManager gateway.GatewayManager
 	return builder
 }
 
+func (builder *Builder) WithUpdateStatusManager(updateStatusManager status.UpdateStatusManager) *Builder {
+	builder.updateStatusManager = updateStatusManager
+	return builder
+}
+
 func (builder *Builder) Build() *HAProxyController {
 	if builder.haproxyCfgFile == nil {
 		logger.Panic(errors.New("no HAProxy Config file provided"))
@@ -170,6 +172,10 @@ func (builder *Builder) Build() *HAProxyController {
 	if gatewayManager == nil {
 		gatewayManager = gateway.New(builder.store, haproxy.HAProxyClient, builder.osArgs, builder.restClientSet)
 	}
+	updateStatusManager := builder.updateStatusManager
+	if updateStatusManager == nil {
+		updateStatusManager = status.New(builder.clientSet)
+	}
 	return &HAProxyController{
 		osArgs:                   builder.osArgs,
 		haproxy:                  haproxy,
@@ -181,6 +187,7 @@ func (builder *Builder) Build() *HAProxyController {
 		chShutdown:               chShutdown,
 		updatePublishServiceFunc: builder.updatePublishServiceFunc,
 		gatewayManager:           gatewayManager,
+		updateStatusManager:      updateStatusManager,
 	}
 }
 

pkg/controller/controller.go

@@ -27,6 +27,7 @@ import (
 	"github.com/haproxytech/kubernetes-ingress/pkg/ingress"
 	"github.com/haproxytech/kubernetes-ingress/pkg/k8s"
 	"github.com/haproxytech/kubernetes-ingress/pkg/route"
+	"github.com/haproxytech/kubernetes-ingress/pkg/status"
 	"github.com/haproxytech/kubernetes-ingress/pkg/store"
 	"github.com/haproxytech/kubernetes-ingress/pkg/utils"
 )
@@ -50,6 +51,7 @@ type HAProxyController struct {
 	restart                  bool
 	reload                   bool
 	ready                    bool
+	updateStatusManager      status.UpdateStatusManager
 }
 
 // Wrapping a Native-Client transaction and commit it.
@@ -111,29 +113,25 @@ func (c *HAProxyController) updateHAProxy() {
 		logger.Error(route.CustomRoutesReset(c.haproxy))
 	}
 
-	ingresses := []*ingress.Ingress{}
 	for _, namespace := range c.store.Namespaces {
 		if !namespace.Relevant {
 			continue
 		}
 		c.store.SecretsProcessed = map[string]struct{}{}
 		for _, ingResource := range namespace.Ingresses {
 			i := ingress.New(c.store, ingResource, c.osArgs.IngressClass, c.osArgs.EmptyIngressClass, c.annotations)
-			if i == nil {
+			if !i.Supported(c.store, c.annotations) {
 				logger.Debugf("ingress '%s/%s' ignored: no matching IngressClass", ingResource.Namespace, ingResource.Name)
-				continue
+				ingResource.Addresses = []string{""}
+			} else {
+				c.reload = i.Update(c.store, c.haproxy, c.annotations) || c.reload
 			}
 			if ingResource.Status == store.ADDED || ingResource.ClassUpdated {
-				ingresses = append(ingresses, i)
+				c.updateStatusManager.AddIngress(i)
 			}
-			c.reload = i.Update(c.store, c.haproxy, c.annotations) || c.reload
 		}
 	}
 
-	if len(ingresses) > 0 {
-		go c.updatePublishServiceFunc(ingresses, c.store.PublishServiceAddresses)
-	}
-
 	gatewayReload := c.gatewayManager.ManageGateway()
 	c.reload = gatewayReload || c.reload
 

pkg/controller/handler.go

@@ -51,6 +51,7 @@ func (c *HAProxyController) initHandlers() {
 		},
 		&handler.PatternFiles{},
 		&handler.BackendCfgSnippet{},
+		c.updateStatusManager,
 	}
 	if c.osArgs.PprofEnabled {
 		c.updateHandlers = append(c.updateHandlers, handler.Pprof{})

pkg/ingress/ingress.go

@@ -39,20 +39,16 @@ type Ingress struct {
 // If the k8s ingress resource is not assigned to the controller (no matching IngressClass)
 // then New will return nil
 func New(k store.K8s, resource *store.Ingress, class string, emptyClass bool, a annotations.Annotations) *Ingress {
-	i := &Ingress{resource: resource, controllerClass: class, allowEmptyClass: emptyClass, annotations: a}
-	if i.resource == nil || !i.supported(k, a) {
-		return nil
-	}
-	return i
+	return &Ingress{resource: resource, controllerClass: class, allowEmptyClass: emptyClass, annotations: a}
 }
 
-// supported verifies if the IngressClass matches the ControllerClass
+// Supported verifies if the IngressClass matches the ControllerClass
 // and in such case returns true otherwise false
 //
 // According to https://github.com/kubernetes/api/blob/master/networking/v1/types.go#L257
 // ingress.class annotation should have precedence over the IngressClass mechanism implemented
 // in "networking.k8s.io".
-func (i Ingress) supported(k8s store.K8s, a annotations.Annotations) (supported bool) {
+func (i Ingress) Supported(k8s store.K8s, a annotations.Annotations) (supported bool) {
 	var igClassAnn, igClassSpec string
 	igClassAnn = a.String("ingress.class", i.resource.Annotations)
 	if igClassResource := k8s.IngressClasses[i.resource.Class]; igClassResource != nil {
@@ -256,3 +252,7 @@ func (i *Ingress) Update(k store.K8s, h haproxy.HAProxy, a annotations.Annotatio
 	}
 	return
 }
+
+func (i Ingress) GetAddresses() []string {
+	return i.resource.Addresses
+}

pkg/ingress/status.go

@@ -7,7 +7,6 @@ import (
 
 	"github.com/haproxytech/kubernetes-ingress/pkg/annotations"
 	"github.com/haproxytech/kubernetes-ingress/pkg/store"
-	"github.com/haproxytech/kubernetes-ingress/pkg/utils"
 	networkingv1 "k8s.io/api/networking/v1"
 	k8serror "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -20,20 +19,16 @@ func NewStatusIngressUpdater(client *kubernetes.Clientset, k store.K8s, class st
 	return func(ingresses []*store.Ingress, publishServiceAddresses []string) {
 		for _, ingress := range ingresses {
 			if ing := New(k, ingress, class, emptyClass, a); ing != nil {
-				logger.Error(ing.UpdateStatus(client, publishServiceAddresses))
+				logger.Error(ing.UpdateStatus(client))
 			}
 		}
 	}
 }
 
-func (i *Ingress) UpdateStatus(client *kubernetes.Clientset, addresses []string) (err error) {
+func (i *Ingress) UpdateStatus(client *kubernetes.Clientset) (err error) {
 	var lbi []networkingv1.IngressLoadBalancerIngress
 
-	if utils.EqualSliceStringsWithoutOrder(i.resource.Addresses, addresses) {
-		return
-	}
-
-	for _, addr := range addresses {
+	for _, addr := range i.resource.Addresses {
 		if net.ParseIP(addr) == nil {
 			lbi = append(lbi, networkingv1.IngressLoadBalancerIngress{Hostname: addr})
 		} else {
@@ -61,12 +56,5 @@ func (i *Ingress) UpdateStatus(client *kubernetes.Clientset, addresses []string)
 	}
 	logger.Tracef("Successful update of LoadBalancer status in ingress %s/%s", i.resource.Namespace, i.resource.Name)
 	// Allow to store the publish service addresses affected to the ingress for future comparison in update test.
-	i.resource.Addresses = addresses
 	return nil
 }
-
-func UpdatePublishService(ingresses []*Ingress, api *kubernetes.Clientset, publishServiceAddresses []string) {
-	for _, i := range ingresses {
-		logger.Error(i.UpdateStatus(api, publishServiceAddresses))
-	}
-}

pkg/k8s/main.go

@@ -30,7 +30,6 @@ import (
 
 	crclientset "github.com/haproxytech/kubernetes-ingress/crs/generated/clientset/versioned"
 	crinformers "github.com/haproxytech/kubernetes-ingress/crs/generated/informers/externalversions"
-	"github.com/haproxytech/kubernetes-ingress/pkg/ingress"
 	"github.com/haproxytech/kubernetes-ingress/pkg/utils"
 	crdclientset "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
 	errGw "k8s.io/apimachinery/pkg/api/errors"
@@ -57,7 +56,6 @@ type K8s interface {
 	GetRestClientset() client.Client
 	GetClientset() *k8sclientset.Clientset
 	MonitorChanges(eventChan chan SyncDataEvent, stop chan struct{}, osArgs utils.OSArgs, gatewayAPIInstalled bool)
-	UpdatePublishService(ingresses []*ingress.Ingress, publishServiceAddresses []string)
 	IsGatewayAPIInstalled(gatewayControllerName string) bool
 }
 
@@ -152,13 +150,6 @@ func (k k8s) GetClientset() *k8sclientset.Clientset {
 	return k.builtInClient
 }
 
-func (k k8s) UpdatePublishService(ingresses []*ingress.Ingress, publishServiceAddresses []string) {
-	clientSet := k.GetClientset()
-	for _, i := range ingresses {
-		logger.Error(i.UpdateStatus(clientSet, publishServiceAddresses))
-	}
-}
-
 func (k k8s) MonitorChanges(eventChan chan SyncDataEvent, stop chan struct{}, osArgs utils.OSArgs, gatewayAPIInstalled bool) {
 	informersSynced := &[]cache.InformerSynced{}
 	k.runPodInformer(eventChan, stop, informersSynced)

pkg/status/updatestatus.go

@@ -0,0 +1,80 @@
+package status
+
+import (
+	"github.com/haproxytech/kubernetes-ingress/pkg/annotations"
+	"github.com/haproxytech/kubernetes-ingress/pkg/haproxy"
+	"github.com/haproxytech/kubernetes-ingress/pkg/ingress"
+	"github.com/haproxytech/kubernetes-ingress/pkg/store"
+	"github.com/haproxytech/kubernetes-ingress/pkg/utils"
+	"k8s.io/client-go/kubernetes"
+)
+
+type UpdateStatusManager interface {
+	AddIngress(ingress *ingress.Ingress)
+	Update(k store.K8s, h haproxy.HAProxy, a annotations.Annotations) (reload bool, err error)
+}
+
+type UpdateStatusManagerImpl struct {
+	updateIngresses []*ingress.Ingress
+	client          *kubernetes.Clientset
+}
+
+func New(client *kubernetes.Clientset) UpdateStatusManager {
+	return &UpdateStatusManagerImpl{
+		client: client,
+	}
+}
+
+func (m *UpdateStatusManagerImpl) AddIngress(ingress *ingress.Ingress) {
+	m.updateIngresses = append(m.updateIngresses, ingress)
+}
+
+func (m *UpdateStatusManagerImpl) Update(k store.K8s, h haproxy.HAProxy, a annotations.Annotations) (reload bool, err error) {
+	errs := utils.Errors{}
+	defer func() {
+		err = errs.Result()
+	}()
+
+	ingresses := m.updateIngresses
+
+	if k.UpdateAllIngresses {
+		ingresses = nil
+		for _, namespace := range k.Namespaces {
+			if !namespace.Relevant {
+				continue
+			}
+
+			for _, ingResource := range namespace.Ingresses {
+				previousAddresses := ingResource.Addresses
+				i := ingress.New(k, ingResource, "haproxy", false, a)
+				supported := i.Supported(k, a)
+				if supported {
+					ingResource.Addresses = k.PublishServiceAddresses
+				}
+				// If ingress is not managed by us, three cases can occur:
+				// - it has no adddresses.
+				// - it has addresses and they are different (maybe managed by someone else).
+				// - it has addresses and they are ours. We managed it but not now anymore.
+				// You can see we can't easily manage the case when while the IC is stopped, the ingress switches to unmanaged state (ingress class change) and the publish service addresses have also changed.
+				if !supported && !utils.EqualSliceStringsWithoutOrder(previousAddresses, ingResource.Addresses) {
+					continue
+				}
+				ingresses = append(ingresses, i)
+			}
+		}
+	}
+
+	if len(ingresses) > 0 {
+		go func() {
+			for _, ing := range ingresses {
+				if ing != nil {
+					errs.Add(ing.UpdateStatus(m.client))
+				}
+			}
+		}()
+	}
+
+	k.UpdateAllIngresses = false
+	m.updateIngresses = nil
+	return
+}

pkg/store/convert.go

@@ -60,7 +60,7 @@ type ingressNetworkingV1Strategy struct {
 }
 
 func (n ingressNetworkingV1Strategy) ConvertIngress() *Ingress {
-	return &Ingress{
+	ing := &Ingress{
 		IngressCore: IngressCore{
 			APIVersion:  NETWORKINGV1,
 			Namespace:   n.ig.GetNamespace(),
@@ -138,6 +138,16 @@ func (n ingressNetworkingV1Strategy) ConvertIngress() *Ingress {
 			}(n.ig.Spec.TLS),
 		},
 	}
+	addresses := []string{}
+	for _, ingLoadBalancer := range n.ig.Status.LoadBalancer.Ingress {
+		address := ingLoadBalancer.IP
+		if ingLoadBalancer.Hostname != "" {
+			address = ingLoadBalancer.Hostname
+		}
+		addresses = append(addresses, address)
+	}
+	ing.Addresses = addresses
+	return ing
 }
 
 func (n ingressNetworkingV1Strategy) ConvertClass() *IngressClass {