Skip to content

Commit

Permalink
Prevent LoadBalancer updates on follower.
Browse files Browse the repository at this point in the history
Follower instances of Contour do not run loadBalancerStatusWriter and
therefore do not read from the channel that receives status updates.
The LoadBalancer status updates are still watched and sent to a channel,
causing the go routine to block.

This led to LoadBalancer updates piling up and consuming memory, eventually
causing an out-of-memory condition and killing the Contour process.

Signed-off-by: Tero Saarni <[email protected]>
  • Loading branch information
tsaarni committed Jan 20, 2025
1 parent 32dad5f commit 8b2af7d
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 4 deletions.
9 changes: 6 additions & 3 deletions cmd/contour/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -703,6 +703,10 @@ func (s *Server) doServe() error {
return err
}

notifier := &leadership.Notifier{
ToNotify: []leadership.NeedLeaderElectionNotification{contourHandler, observer},
}

// Register an informer to watch envoy's service if we haven't been given static details.
if lbAddress := contourConfiguration.Ingress.StatusAddress; len(lbAddress) > 0 {
s.log.WithField("loadbalancer-address", lbAddress).Info("Using supplied information for Ingress status")
Expand All @@ -723,6 +727,8 @@ func (s *Server) doServe() error {
s.log.WithError(err).WithField("resource", "services").Fatal("failed to create informer")
}

notifier.ToNotify = append(notifier.ToNotify, serviceHandler)

s.log.WithField("envoy-service-name", contourConfiguration.Envoy.Service.Name).
WithField("envoy-service-namespace", contourConfiguration.Envoy.Service.Namespace).
Info("Watching Service for Ingress status")
Expand All @@ -740,9 +746,6 @@ func (s *Server) doServe() error {
return err
}

notifier := &leadership.Notifier{
ToNotify: []leadership.NeedLeaderElectionNotification{contourHandler, observer},
}
if err := s.mgr.Add(notifier); err != nil {
return err
}
Expand Down
10 changes: 9 additions & 1 deletion internal/k8s/statusaddress.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package k8s
import (
"fmt"
"sync"
"sync/atomic"

"github.com/sirupsen/logrus"
core_v1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -186,6 +187,7 @@ type ServiceStatusLoadBalancerWatcher struct {
ServiceName string
LBStatus chan core_v1.LoadBalancerStatus
Log logrus.FieldLogger
leader atomic.Bool
}

func (s *ServiceStatusLoadBalancerWatcher) OnAdd(obj any, _ bool) {
Expand Down Expand Up @@ -234,8 +236,14 @@ func (s *ServiceStatusLoadBalancerWatcher) OnDelete(obj any) {
})
}

func (s *ServiceStatusLoadBalancerWatcher) OnElectedLeader() {
s.leader.Store(true)
}

func (s *ServiceStatusLoadBalancerWatcher) notify(lbstatus core_v1.LoadBalancerStatus) {
s.LBStatus <- lbstatus
if s.leader.Load() {
s.LBStatus <- lbstatus
}
}

func coreToNetworkingLBStatus(lbs core_v1.LoadBalancerStatus) networking_v1.IngressLoadBalancerStatus {
Expand Down
28 changes: 28 additions & 0 deletions internal/k8s/statusaddress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ func TestServiceStatusLoadBalancerWatcherOnAdd(t *testing.T) {
LBStatus: lbstatus,
Log: fixture.NewTestLogger(t),
}
sw.OnElectedLeader()

recv := func() (core_v1.LoadBalancerStatus, bool) {
select {
Expand Down Expand Up @@ -89,6 +90,7 @@ func TestServiceStatusLoadBalancerWatcherOnUpdate(t *testing.T) {
LBStatus: lbstatus,
Log: fixture.NewTestLogger(t),
}
sw.OnElectedLeader()

recv := func() (core_v1.LoadBalancerStatus, bool) {
select {
Expand Down Expand Up @@ -139,6 +141,7 @@ func TestServiceStatusLoadBalancerWatcherOnDelete(t *testing.T) {
LBStatus: lbstatus,
Log: fixture.NewTestLogger(t),
}
sw.OnElectedLeader()

recv := func() (core_v1.LoadBalancerStatus, bool) {
select {
Expand Down Expand Up @@ -179,6 +182,31 @@ func TestServiceStatusLoadBalancerWatcherOnDelete(t *testing.T) {
assert.Equal(t, want, got)
}

func TestServiceStatusLoadBalancerWatcherNoNotificationsOnFollower(t *testing.T) {
lbstatus := make(chan core_v1.LoadBalancerStatus, 1)

sw := ServiceStatusLoadBalancerWatcher{
ServiceName: "envoy",
LBStatus: lbstatus,
Log: fixture.NewTestLogger(t),
}

recv := func() bool {
select {
case <-sw.LBStatus:
return true
default:
return false
}
}

// assert that when not elected leader, no notifications are sent.
var svc core_v1.Service
svc.Name = "envoy"
sw.OnAdd(&svc, false)
assert.False(t, recv())
}

func TestStatusAddressUpdater(t *testing.T) {
const objName = "someobjfoo"

Expand Down

0 comments on commit 8b2af7d

Please sign in to comment.