Skip to content

Commit

Permalink
Multiple refactors on cplb.tcpproxy
Browse files Browse the repository at this point in the history
1- Simplify tcpproxy by reomivng useless interfaces:
The original tcpproxy allowed different types of routes and needed to do
a bunch of interfacing for it to work. Since we only implement one kind
of route and one kind of target, we remove all the interfaces and merge
both structs into a unique struct.

2- Remove proxy.AddRoute: We only used it once and setRoutes can cover
that use case

3- Lock tcpproxy.Proxy when modifying routes to make it thread safe.
Prior to this we relied on the proxy being called only from one
goroutine. Now it can be called concurrently, not that we expect to do
that but a lock gives us extra safety.

4- Panic if tcpproxy.SetRoutes gets and empty route list. We now check
this in cplb_unix.go.

5- Remove the route feeding goroutine for round robin, since we added a
lock to make proxy.SetRoutes threadsafe we don't need that anymore and
it can be made much simpler by adding a lock.

Signed-off-by: Juan-Luis de Sousa-Valadas Castaño <jvaladas@mirantis.com>
  • Loading branch information
juanluisvaladas committed Dec 30, 2024
1 parent 4a06ba3 commit b133769
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 139 deletions.
7 changes: 4 additions & 3 deletions inttest/cplb-userspace/cplbuserspace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,15 +154,16 @@ func (s *CPLBUserSpaceSuite) TestK0sGetsUp() {
s.T().Log("Testing that the load balancer is actually balancing the load")
// Other stuff may be querying the controller, running the HTTPS request 15 times
// should be more than we need.
attempt := 0
signatures := make(map[string]int)
url := url.URL{Scheme: "https", Host: net.JoinHostPort(lb, strconv.Itoa(6443))}
for range 15 {
for len(signatures) < 3 {
signature, err := getServerCertSignature(ctx, url.String())
s.Require().NoError(err)
signatures[signature] = 1
attempt++
s.Require().LessOrEqual(attempt, 15, "Failed to get a signature from all controllers")
}

s.Require().Len(signatures, 3, "Expected 3 different signatures, got %d", len(signatures))
}

// getLBAddress returns the IP address of the controller 0 and it adds 100 to
Expand Down
24 changes: 16 additions & 8 deletions pkg/component/controller/cplb/cplb_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func (k *Keepalived) Init(_ context.Context) error {
}

// Start generates the keepalived configuration and starts the keepalived process
func (k *Keepalived) Start(_ context.Context) error {
func (k *Keepalived) Start(ctx context.Context) error {
if k.Config == nil || (len(k.Config.VRRPInstances) == 0 && len(k.Config.VirtualServers) == 0) {
k.log.Warn("No VRRP instances or virtual servers defined, skipping keepalived start")
return nil
Expand Down Expand Up @@ -154,8 +154,7 @@ func (k *Keepalived) Start(_ context.Context) error {
if len(k.Config.VirtualServers) > 0 {
k.watchReconcilerUpdatesKeepalived()
} else {

if err := k.watchReconcilerUpdatesReverseProxy(); err != nil {
if err := k.watchReconcilerUpdatesReverseProxy(ctx); err != nil {
k.log.WithError(err).Error("failed to watch reconciler updates")
}
}
Expand Down Expand Up @@ -347,18 +346,23 @@ func (k *Keepalived) generateKeepalivedTemplate() error {
return nil
}

func (k *Keepalived) watchReconcilerUpdatesReverseProxy() error {
func (k *Keepalived) watchReconcilerUpdatesReverseProxy(ctx context.Context) error {
k.proxy = tcpproxy.Proxy{}
// We don't know how long until we get the first update, so initially we
// forward everything to localhost
k.proxy.AddRoute(fmt.Sprintf(":%d", k.Config.UserSpaceProxyPort), tcpproxy.To(fmt.Sprintf("127.0.0.1:%d", k.APIPort)))
k.proxy.SetRoutes(fmt.Sprintf(":%d", k.Config.UserSpaceProxyPort), []tcpproxy.Route{tcpproxy.To(fmt.Sprintf("127.0.0.1:%d", k.APIPort))})

if err := k.proxy.Start(); err != nil {
return fmt.Errorf("failed to start proxy: %w", err)
}

fmt.Println("Waiting for updateCh")
<-k.updateCh
k.log.Info("Waiting for the first cplb-reconciler update")

select {
case <-ctx.Done():
return errors.New("context cancelled while starting the reverse proxy")
case <-k.updateCh:
}
k.setProxyRoutes()

// Do not create the iptables rules until we have the first update and the
Expand All @@ -374,11 +378,15 @@ func (k *Keepalived) watchReconcilerUpdatesReverseProxy() error {
}

func (k *Keepalived) setProxyRoutes() {
routes := []tcpproxy.Target{}
routes := []tcpproxy.Route{}
for _, addr := range k.reconciler.GetIPs() {
routes = append(routes, tcpproxy.To(fmt.Sprintf("%s:%d", addr, k.APIPort)))
}

if len(routes) == 0 {
k.log.Error("No API servers available, leave previous configuration")
return
}
k.proxy.SetRoutes(fmt.Sprintf(":%d", k.Config.UserSpaceProxyPort), routes)
}

Expand Down
Loading

0 comments on commit b133769

Please sign in to comment.