Skip to content

Commit

Permalink
Fix test failures resulting from forward merge
Browse files Browse the repository at this point in the history
worker_test.go setup now included 'expectTrackedDB...' to be called.
This needed to be added to new tests introduced in the forward merge

worker_namespace_tests were not aware of the new 'IsLoopbackPreferred'
method on the interface. Add some expect statements to account. Also
write some new tests for the different LoopbackPreferred values.

In doing so, extract some repeated code into a helper function,
'startWorker'
  • Loading branch information
jack-w-shaw committed Feb 5, 2024
1 parent d9c6919 commit 6499355
Show file tree
Hide file tree
Showing 2 changed files with 122 additions and 63 deletions.
163 changes: 103 additions & 60 deletions internal/worker/dbaccessor/worker_namespace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ func (s *namespaceSuite) TestEnsureNamespaceForModelNotFound(c *gc.C) {

// If this is an existing node, we do not
// invoke the address or cluster options.
mgrExp.IsExistingNode().Return(true, nil).Times(1)
mgrExp.IsExistingNode().Return(true, nil)
mgrExp.IsLoopbackPreferred().Return(false)
mgrExp.IsLoopbackBound(gomock.Any()).Return(true, nil).Times(2)
mgrExp.WithLogFuncOption().Return(nil)
mgrExp.WithTracingOption().Return(nil)
Expand All @@ -72,6 +73,29 @@ func (s *namespaceSuite) TestEnsureNamespaceForModelNotFound(c *gc.C) {

workertest.CleanKill(c, w)
}
func (s *namespaceSuite) startWorker(c *gc.C, ctx context.Context) *dbWorker {
trackedWorkerDB := newWorkerTrackedDB(s.TxnRunner())

w := s.newWorkerWithDB(c, trackedWorkerDB)

err := s.TxnRunner().StdTxn(ctx, func(ctx context.Context, tx *sql.Tx) error {
stmt := "INSERT INTO model_list (uuid) VALUES (?);"
result, err := tx.ExecContext(ctx, stmt, "foo")
c.Assert(err, jc.ErrorIsNil)

num, err := result.RowsAffected()
c.Assert(err, jc.ErrorIsNil)
c.Assert(num, gc.Equals, int64(1))

return nil
})
c.Assert(err, jc.ErrorIsNil)

dbw := w.(*dbWorker)
ensureStartup(c, dbw)

return dbw
}

func (s *namespaceSuite) TestEnsureNamespaceForModel(c *gc.C) {
defer s.setupMocks(c).Finish()
Expand All @@ -85,6 +109,7 @@ func (s *namespaceSuite) TestEnsureNamespaceForModel(c *gc.C) {
// If this is an existing node, we do not
// invoke the address or cluster options.
mgrExp.IsExistingNode().Return(true, nil)
mgrExp.IsLoopbackPreferred().Return(false)
mgrExp.IsLoopbackBound(gomock.Any()).Return(true, nil).Times(2)
mgrExp.WithLogFuncOption().Return(nil)
mgrExp.WithTracingOption().Return(nil)
Expand All @@ -95,34 +120,49 @@ func (s *namespaceSuite) TestEnsureNamespaceForModel(c *gc.C) {

s.hub.EXPECT().Subscribe(apiserver.DetailsTopic, gomock.Any()).Return(func() {}, nil)

trackedWorkerDB := newWorkerTrackedDB(s.TxnRunner())

w := s.newWorkerWithDB(c, trackedWorkerDB)
defer workertest.DirtyKill(c, w)

ctx, cancel := context.WithTimeout(context.Background(), testing.LongWait)
defer cancel()

err := s.TxnRunner().StdTxn(ctx, func(ctx context.Context, tx *sql.Tx) error {
stmt := "INSERT INTO model_list (uuid) VALUES (?);"
result, err := tx.ExecContext(ctx, stmt, "foo")
c.Assert(err, jc.ErrorIsNil)
dbw := s.startWorker(c, ctx)

num, err := result.RowsAffected()
c.Assert(err, jc.ErrorIsNil)
c.Assert(num, gc.Equals, int64(1))

return nil
})
err := dbw.ensureNamespace("foo")
c.Assert(err, jc.ErrorIsNil)

dbw := w.(*dbWorker)
ensureStartup(c, dbw)
workertest.CleanKill(c, dbw)
}

err = dbw.ensureNamespace("foo")
func (s *namespaceSuite) TestEnsureNamespaceForModelLoopbackPreferred(c *gc.C) {
defer s.setupMocks(c).Finish()

s.expectClock()

dataDir := c.MkDir()
mgrExp := s.nodeManager.EXPECT()
mgrExp.EnsureDataDir().Return(dataDir, nil).MinTimes(1)

// If this is an existing node, we do not
// invoke the address or cluster options.
mgrExp.IsExistingNode().Return(true, nil)
mgrExp.IsLoopbackPreferred().Return(true)
mgrExp.IsLoopbackBound(gomock.Any()).Return(true, nil).Times(1)
mgrExp.WithLogFuncOption().Return(nil)
mgrExp.WithTracingOption().Return(nil)

s.client.EXPECT().Cluster(gomock.Any()).Return(nil, nil)

s.expectNodeStartupAndShutdown()

s.hub.EXPECT().Subscribe(apiserver.DetailsTopic, gomock.Any()).Return(func() {}, nil)

ctx, cancel := context.WithTimeout(context.Background(), testing.LongWait)
defer cancel()

dbw := s.startWorker(c, ctx)

err := dbw.ensureNamespace("foo")
c.Assert(err, jc.ErrorIsNil)

workertest.CleanKill(c, w)
workertest.CleanKill(c, dbw)
}

func (s *namespaceSuite) TestEnsureNamespaceForModelWithCache(c *gc.C) {
Expand All @@ -137,6 +177,7 @@ func (s *namespaceSuite) TestEnsureNamespaceForModelWithCache(c *gc.C) {
// If this is an existing node, we do not
// invoke the address or cluster options.
mgrExp.IsExistingNode().Return(true, nil)
mgrExp.IsLoopbackPreferred().Return(false)
mgrExp.IsLoopbackBound(gomock.Any()).Return(true, nil).Times(2)
mgrExp.WithLogFuncOption().Return(nil)
mgrExp.WithTracingOption().Return(nil)
Expand Down Expand Up @@ -198,6 +239,7 @@ func (s *namespaceSuite) TestCloseDatabaseForController(c *gc.C) {
// If this is an existing node, we do not
// invoke the address or cluster options.
mgrExp.IsExistingNode().Return(true, nil)
mgrExp.IsLoopbackPreferred().Return(false)
mgrExp.IsLoopbackBound(gomock.Any()).Return(true, nil).Times(2)
mgrExp.WithLogFuncOption().Return(nil)
mgrExp.WithTracingOption().Return(nil)
Expand All @@ -208,34 +250,15 @@ func (s *namespaceSuite) TestCloseDatabaseForController(c *gc.C) {

s.hub.EXPECT().Subscribe(apiserver.DetailsTopic, gomock.Any()).Return(func() {}, nil)

trackedWorkerDB := newWorkerTrackedDB(s.TxnRunner())

w := s.newWorkerWithDB(c, trackedWorkerDB)
defer workertest.DirtyKill(c, w)

ctx, cancel := context.WithTimeout(context.Background(), testing.LongWait)
defer cancel()

err := s.TxnRunner().StdTxn(ctx, func(ctx context.Context, tx *sql.Tx) error {
stmt := "INSERT INTO model_list (uuid) VALUES (?);"
result, err := tx.ExecContext(ctx, stmt, "foo")
c.Assert(err, jc.ErrorIsNil)

num, err := result.RowsAffected()
c.Assert(err, jc.ErrorIsNil)
c.Assert(num, gc.Equals, int64(1))
dbw := s.startWorker(c, ctx)

return nil
})
c.Assert(err, jc.ErrorIsNil)

dbw := w.(*dbWorker)
ensureStartup(c, dbw)

err = dbw.closeDatabase(database.ControllerNS)
err := dbw.closeDatabase(database.ControllerNS)
c.Assert(err, gc.ErrorMatches, "cannot close controller database")

workertest.CleanKill(c, w)
workertest.CleanKill(c, dbw)
}

func (s *namespaceSuite) TestCloseDatabaseForModel(c *gc.C) {
Expand All @@ -250,6 +273,7 @@ func (s *namespaceSuite) TestCloseDatabaseForModel(c *gc.C) {
// If this is an existing node, we do not
// invoke the address or cluster options.
mgrExp.IsExistingNode().Return(true, nil)
mgrExp.IsLoopbackPreferred().Return(false)
mgrExp.IsLoopbackBound(gomock.Any()).Return(true, nil).Times(2)
mgrExp.WithLogFuncOption().Return(nil)
mgrExp.WithTracingOption().Return(nil)
Expand All @@ -260,37 +284,55 @@ func (s *namespaceSuite) TestCloseDatabaseForModel(c *gc.C) {

s.hub.EXPECT().Subscribe(apiserver.DetailsTopic, gomock.Any()).Return(func() {}, nil)

trackedWorkerDB := newWorkerTrackedDB(s.TxnRunner())

w := s.newWorkerWithDB(c, trackedWorkerDB)
defer workertest.DirtyKill(c, w)

ctx, cancel := context.WithTimeout(context.Background(), testing.LongWait)
defer cancel()

err := s.TxnRunner().StdTxn(ctx, func(ctx context.Context, tx *sql.Tx) error {
stmt := "INSERT INTO model_list (uuid) VALUES (?);"
result, err := tx.ExecContext(ctx, stmt, "foo")
c.Assert(err, jc.ErrorIsNil)
dbw := s.startWorker(c, ctx)

num, err := result.RowsAffected()
c.Assert(err, jc.ErrorIsNil)
c.Assert(num, gc.Equals, int64(1))
_, err := dbw.GetDB("foo")
c.Assert(err, jc.ErrorIsNil)

return nil
})
err = dbw.closeDatabase("foo")
c.Assert(err, jc.ErrorIsNil)

dbw := w.(*dbWorker)
ensureStartup(c, dbw)
workertest.CleanKill(c, dbw)
}

_, err = dbw.GetDB("foo")
func (s *namespaceSuite) TestCloseDatabaseForModelLoopbackPreferred(c *gc.C) {
defer s.setupMocks(c).Finish()

s.expectClock()

dataDir := c.MkDir()
mgrExp := s.nodeManager.EXPECT()
mgrExp.EnsureDataDir().Return(dataDir, nil).MinTimes(1)

// If this is an existing node, we do not
// invoke the address or cluster options.
mgrExp.IsExistingNode().Return(true, nil)
mgrExp.IsLoopbackPreferred().Return(true)
mgrExp.IsLoopbackBound(gomock.Any()).Return(true, nil).Times(1)
mgrExp.WithLogFuncOption().Return(nil)
mgrExp.WithTracingOption().Return(nil)

s.client.EXPECT().Cluster(gomock.Any()).Return(nil, nil)

s.expectNodeStartupAndShutdown()

s.hub.EXPECT().Subscribe(apiserver.DetailsTopic, gomock.Any()).Return(func() {}, nil)

ctx, cancel := context.WithTimeout(context.Background(), testing.LongWait)
defer cancel()

dbw := s.startWorker(c, ctx)

_, err := dbw.GetDB("foo")
c.Assert(err, jc.ErrorIsNil)

err = dbw.closeDatabase("foo")
c.Assert(err, jc.ErrorIsNil)

workertest.CleanKill(c, w)
workertest.CleanKill(c, dbw)
}

func (s *namespaceSuite) TestCloseDatabaseForUnknownModel(c *gc.C) {
Expand All @@ -305,6 +347,7 @@ func (s *namespaceSuite) TestCloseDatabaseForUnknownModel(c *gc.C) {
// If this is an existing node, we do not
// invoke the address or cluster options.
mgrExp.IsExistingNode().Return(true, nil)
mgrExp.IsLoopbackPreferred().Return(false)
mgrExp.IsLoopbackBound(gomock.Any()).Return(true, nil).Times(2)
mgrExp.WithLogFuncOption().Return(nil)
mgrExp.WithTracingOption().Return(nil)
Expand Down
22 changes: 19 additions & 3 deletions internal/worker/dbaccessor/worker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ func (s *workerSuite) TestKilledGetDBErrDying(c *gc.C) {
mgrExp.EnsureDataDir().Return(c.MkDir(), nil)
mgrExp.IsExistingNode().Return(true, nil).Times(1)
mgrExp.IsLoopbackBound(gomock.Any()).Return(true, nil).Times(2)
mgrExp.IsLoopbackPreferred().Return(false)
mgrExp.WithLogFuncOption().Return(nil)
mgrExp.WithTracingOption().Return(nil)

Expand Down Expand Up @@ -295,7 +296,9 @@ func (s *workerSuite) TestWorkerStartupExistingNode(c *gc.C) {
func (s *workerSuite) TestWorkerStartupExistingNodeWithLoopbackPreferred(c *gc.C) {
defer s.setupMocks(c).Finish()

dbDone := make(chan struct{})
s.expectClock()
s.expectTrackedDBUpdateNodeAndKill(dbDone)

mgrExp := s.nodeManager.EXPECT()
mgrExp.EnsureDataDir().Return(c.MkDir(), nil)
Expand All @@ -320,7 +323,10 @@ func (s *workerSuite) TestWorkerStartupExistingNodeWithLoopbackPreferred(c *gc.C
s.hub.EXPECT().Subscribe(apiserver.DetailsTopic, gomock.Any()).Return(func() {}, nil)

w := s.newWorker(c)
defer workertest.DirtyKill(c, w)
defer func() {
close(dbDone)
workertest.DirtyKill(c, w)
}()

ensureStartup(c, w.(*dbWorker))

Expand Down Expand Up @@ -474,7 +480,9 @@ func (s *workerSuite) newWorker(c *gc.C) worker.Worker {
func (s *workerSuite) TestWorkerStartupAsBootstrapNodeThenReconfigureWithLoopbackPreferred(c *gc.C) {
defer s.setupMocks(c).Finish()

dbDone := make(chan struct{})
s.expectClock()
s.expectTrackedDBUpdateNodeAndKill(dbDone)

dataDir := c.MkDir()
mgrExp := s.nodeManager.EXPECT()
Expand All @@ -496,7 +504,10 @@ func (s *workerSuite) TestWorkerStartupAsBootstrapNodeThenReconfigureWithLoopbac
s.client.EXPECT().Cluster(gomock.Any()).Return(nil, nil)

w := s.newWorker(c)
defer workertest.DirtyKill(c, w)
defer func() {
close(dbDone)
workertest.DirtyKill(c, w)
}()
dbw := w.(*dbWorker)

ensureStartup(c, dbw)
Expand All @@ -520,7 +531,9 @@ func (s *workerSuite) TestWorkerStartupAsBootstrapNodeThenReconfigureWithLoopbac
func (s *workerSuite) TestWorkerStartupAsBootstrapNodeThenReconfigureWithLoopbackPreferredAndNotLoopbackBound(c *gc.C) {
defer s.setupMocks(c).Finish()

dbDone := make(chan struct{})
s.expectClock()
s.expectTrackedDBUpdateNodeAndKill(dbDone)

dataDir := c.MkDir()
mgrExp := s.nodeManager.EXPECT()
Expand All @@ -545,7 +558,10 @@ func (s *workerSuite) TestWorkerStartupAsBootstrapNodeThenReconfigureWithLoopbac
s.client.EXPECT().Cluster(gomock.Any()).Return(nil, nil)

w := s.newWorker(c)
defer workertest.DirtyKill(c, w)
defer func() {
close(dbDone)
workertest.DirtyKill(c, w)
}()
dbw := w.(*dbWorker)

ensureStartup(c, dbw)
Expand Down

0 comments on commit 6499355

Please sign in to comment.