Skip to content

Commit

Permalink
api: defer controller manager init (#4699)
Browse files Browse the repository at this point in the history
We've had a race condition here all along, but apparently the
latest changes exacerabated it (maybe just due to sheer number
of reconcilers we have now).

This moves the actual start of the controller manager to after
the controllers are initialized (which itself happens in two
phases).

It'd be nice to fully redo this initialization sequence in the
future, but we'd need to de-couple it from using the store to
initialize everything.
  • Loading branch information
milas committed Jun 29, 2021
1 parent 77c5854 commit 2af3565
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 12 deletions.
12 changes: 10 additions & 2 deletions internal/controllers/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func (c *ControllerBuilder) OnChange(_ context.Context, _ store.RStore, _ store.
return nil
}

func (c *ControllerBuilder) SetUp(_ context.Context, _ store.RStore) error {
func (c *ControllerBuilder) SetUp(ctx context.Context, st store.RStore) error {
mgr := c.tscm.GetManager()
client := c.tscm.GetClient()

Expand All @@ -63,6 +63,15 @@ func (c *ControllerBuilder) SetUp(_ context.Context, _ store.RStore) error {
return fmt.Errorf("error starting controller: %v", err)
}
}

// start the controller manager now that all the controllers are initialized
go func() {
if err := mgr.Start(ctx); err != nil && !errors.Is(err, context.Canceled) {
err = fmt.Errorf("controller manager stopped unexpectedly: %v", err)
st.Dispatch(store.NewErrorAction(err))
}
}()

return nil
}

Expand All @@ -73,5 +82,4 @@ func (c *ControllerBuilder) TearDown(ctx context.Context) {
td.TearDown(ctx)
}
}

}
11 changes: 1 addition & 10 deletions internal/controllers/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package controllers

import (
"context"
"errors"
"fmt"
"time"

Expand Down Expand Up @@ -57,7 +56,7 @@ func (m *TiltServerControllerManager) GetClient() ctrlclient.Client {
return m.manager.GetClient()
}

func (m *TiltServerControllerManager) SetUp(ctx context.Context, st store.RStore) error {
func (m *TiltServerControllerManager) SetUp(ctx context.Context, _ store.RStore) error {
ctx, m.cancel = context.WithCancel(ctx)

// controller-runtime internals don't really make use of verbosity levels, so in lieu of a better
Expand Down Expand Up @@ -103,14 +102,6 @@ func (m *TiltServerControllerManager) SetUp(ctx context.Context, st store.RStore

// provide the deferred client with the real client now that it has been initialized
m.deferredClient.initialize(mgr.GetClient())

go func() {
if err := mgr.Start(ctx); err != nil && !errors.Is(err, context.Canceled) {
err = fmt.Errorf("controller manager stopped unexpectedly: %v", err)
st.Dispatch(store.NewErrorAction(err))
}
}()

m.manager = mgr

return nil
Expand Down

0 comments on commit 2af3565

Please sign in to comment.