Remove app worker from runner when the worker peacefully exits;

sdg0919 · Jan 11, 2022 · 729006b · 729006b
1 parent 90e0c74
commit 729006b
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 45 deletions.
diff --git a/worker/caasapplicationprovisioner/application.go b/worker/caasapplicationprovisioner/application.go
@@ -103,6 +103,7 @@ func (a *appWorker) loop() (err error) {
 	defer func() {
 		select {
 		case <-a.catacomb.Dying():
+			// Destroying model.
 		default:
 			if err == nil {
 				// Stop and remove myself from runner.

diff --git a/worker/caasapplicationprovisioner/application_test.go b/worker/caasapplicationprovisioner/application_test.go
@@ -62,12 +62,13 @@ type testCase struct {
 	brokerApp  *caasmocks.MockApplication
 	unitFacade *mocks.MockCAASUnitProvisionerFacade
 
-	appScaleChan     chan struct{}
-	notifyReady      chan struct{}
-	appStateChan     chan struct{}
-	appChan          chan struct{}
-	appReplicasChan  chan struct{}
-	appTrustHashChan chan []string
+	appScaleChan        chan struct{}
+	notifyReady         chan struct{}
+	appStateChan        chan struct{}
+	appChan             chan struct{}
+	appReplicasChan     chan struct{}
+	appTrustHashChan    chan []string
+	shutDownCleanUpFunc func()
 }
 
 func (s *ApplicationWorkerSuite) getWorker(c *gc.C) (func(...*gomock.Call) worker.Worker, testCase, *gomock.Controller) {
@@ -80,6 +81,7 @@ func (s *ApplicationWorkerSuite) getWorker(c *gc.C) (func(...*gomock.Call) worke
 	tc.broker = mocks.NewMockCAASBroker(ctrl)
 	tc.unitFacade = mocks.NewMockCAASUnitProvisionerFacade(ctrl)
 	tc.brokerApp = caasmocks.NewMockApplication(ctrl)
+	tc.shutDownCleanUpFunc = func() {}
 
 	s.appCharmInfo = &charmscommon.CharmInfo{
 		Meta: &charm.Meta{
@@ -130,13 +132,14 @@ func (s *ApplicationWorkerSuite) getWorker(c *gc.C) (func(...*gomock.Call) worke
 
 	startFunc := func(additionalAssertCalls ...*gomock.Call) worker.Worker {
 		config := caasapplicationprovisioner.AppWorkerConfig{
-			Name:       "test",
-			Facade:     tc.facade,
-			Broker:     tc.broker,
-			ModelTag:   s.modelTag,
-			Clock:      tc.clock,
-			Logger:     s.logger,
-			UnitFacade: tc.unitFacade,
+			Name:                "test",
+			Facade:              tc.facade,
+			Broker:              tc.broker,
+			ModelTag:            s.modelTag,
+			Clock:               tc.clock,
+			Logger:              s.logger,
+			UnitFacade:          tc.unitFacade,
+			ShutDownCleanUpFunc: tc.shutDownCleanUpFunc,
 		}
 		expectedCalls := append([]*gomock.Call{},
 			// Verify charm is v2
@@ -842,7 +845,7 @@ func (s *ApplicationWorkerSuite) TestUpgrade(c *gc.C) {
 		}),
 	)
 
-	appWorker := s.startAppWorker(c, nil, facade, broker, nil)
+	appWorker := s.startAppWorker(c, nil, facade, broker, nil, func() {})
 
 	s.waitDone(c, done)
 	workertest.DirtyKill(c, appWorker)
@@ -854,15 +857,17 @@ func (s *ApplicationWorkerSuite) startAppWorker(
 	facade caasapplicationprovisioner.CAASProvisionerFacade,
 	broker caasapplicationprovisioner.CAASBroker,
 	unitFacade caasapplicationprovisioner.CAASUnitProvisionerFacade,
+	shutDownCleanUpFunc func(),
 ) worker.Worker {
 	config := caasapplicationprovisioner.AppWorkerConfig{
-		Name:       "test",
-		Facade:     facade,
-		Broker:     broker,
-		ModelTag:   s.modelTag,
-		Clock:      clk,
-		Logger:     s.logger,
-		UnitFacade: unitFacade,
+		Name:                "test",
+		Facade:              facade,
+		Broker:              broker,
+		ModelTag:            s.modelTag,
+		Clock:               clk,
+		Logger:              s.logger,
+		UnitFacade:          unitFacade,
+		ShutDownCleanUpFunc: shutDownCleanUpFunc,
 	}
 	startFunc := caasapplicationprovisioner.NewAppWorker(config)
 	c.Assert(startFunc, gc.NotNil)
@@ -886,12 +891,11 @@ func (s *ApplicationWorkerSuite) TestUpgradeInfoNotFound(c *gc.C) {
 		// Wait till charm is v2
 		facade.EXPECT().WatchApplication("test").Return(appStateWatcher, nil),
 		facade.EXPECT().ApplicationCharmInfo("test").DoAndReturn(func(appName string) (*charmscommon.CharmInfo, error) {
-			close(done)
 			return nil, errors.NotFoundf("test charm")
 		}),
 	)
-
-	appWorker := s.startAppWorker(c, nil, facade, broker, nil)
+	shutDownCleanUpFunc := func() { close(done) }
+	appWorker := s.startAppWorker(c, nil, facade, broker, nil, shutDownCleanUpFunc)
 
 	s.waitDone(c, done)
 	workertest.CleanKill(c, appWorker)
@@ -921,7 +925,7 @@ func (s *ApplicationWorkerSuite) TestUpgradeLifeNotFound(c *gc.C) {
 		}),
 	)
 
-	appWorker := s.startAppWorker(c, nil, facade, broker, nil)
+	appWorker := s.startAppWorker(c, nil, facade, broker, nil, func() {})
 
 	s.waitDone(c, done)
 	workertest.CleanKill(c, appWorker)
@@ -951,7 +955,7 @@ func (s *ApplicationWorkerSuite) TestUpgradeLifeDead(c *gc.C) {
 		}),
 	)
 
-	appWorker := s.startAppWorker(c, nil, facade, broker, nil)
+	appWorker := s.startAppWorker(c, nil, facade, broker, nil, func() {})
 
 	s.waitDone(c, done)
 	workertest.CleanKill(c, appWorker)
@@ -999,7 +1003,7 @@ func (s *ApplicationWorkerSuite) TestDeleteOperator(c *gc.C) {
 		}),
 	)
 
-	appWorker := s.startAppWorker(c, clk, facade, broker, nil)
+	appWorker := s.startAppWorker(c, clk, facade, broker, nil, func() {})
 
 	s.waitDone(c, done)
 	workertest.DirtyKill(c, appWorker)

diff --git a/worker/caasapplicationprovisioner/mock_test.go b/worker/caasapplicationprovisioner/mock_test.go
@@ -6,26 +6,12 @@ package caasapplicationprovisioner_test
 import (
 	jujutesting "github.com/juju/testing"
 	"github.com/juju/worker/v3"
-
-	"github.com/juju/juju/core/watcher"
-	"github.com/juju/juju/core/watcher/watchertest"
-	"github.com/juju/juju/worker/caasapplicationprovisioner"
 )
 
 //go:generate go run github.com/golang/mock/mockgen -package mocks -destination mocks/broker_mock.go github.com/juju/juju/worker/caasapplicationprovisioner CAASBroker
 //go:generate go run github.com/golang/mock/mockgen -package mocks -destination mocks/facade_mock.go github.com/juju/juju/worker/caasapplicationprovisioner CAASProvisionerFacade
 //go:generate go run github.com/golang/mock/mockgen -package mocks -destination mocks/unitfacade_mock.go github.com/juju/juju/worker/caasapplicationprovisioner CAASUnitProvisionerFacade
-
-type mockFacade struct {
-	caasapplicationprovisioner.CAASProvisionerFacade
-	jujutesting.Stub
-	appWatcher *watchertest.MockStringsWatcher
-}
-
-func (f *mockFacade) WatchApplications() (watcher.StringsWatcher, error) {
-	f.MethodCall(f, "WatchApplications")
-	return f.appWatcher, f.NextErr()
-}
+//go:generate go run github.com/golang/mock/mockgen -package mocks -destination mocks/runner_mock.go github.com/juju/juju/worker/caasapplicationprovisioner Runner
 
 type mockNotifyWorker struct {
 	worker.Worker

diff --git a/worker/caasapplicationprovisioner/package_test.go b/worker/caasapplicationprovisioner/package_test.go
@@ -1,7 +1,7 @@
 // Copyright 2020 Canonical Ltd.
 // Licensed under the AGPLv3, see LICENCE file for details.
 
-package caasapplicationprovisioner_test
+package caasapplicationprovisioner
 
 import (
 	stdtesting "testing"
@@ -12,3 +12,5 @@ import (
 func TestPackage(t *stdtesting.T) {
 	gc.TestingT(t)
 }
+
+var NewProvisionerWorkerForTest = newProvisionerWorker
diff --git a/worker/caasapplicationprovisioner/worker.go b/worker/caasapplicationprovisioner/worker.go
@@ -211,8 +211,13 @@ func (p *provisioner) loop() error {
 }
 
 func (p *provisioner) shutDownAppWorker(appName string) {
-	if err := p.runner.StopAndRemoveWorker(appName, p.catacomb.Dying()); err != nil && !errors.IsNotFound(err) {
+	err := p.runner.StopAndRemoveWorker(appName, p.catacomb.Dying())
+	if errors.IsNotFound(err) {
+		return
+	}
+	if err != nil {
 		p.logger.Warningf("stopping app worker %q: %v", appName, err)
+		return
 	}
-	p.logger.Debugf("removing app worker %q", appName)
+	p.logger.Debugf("removed app worker %q", appName)
 }