Addressed PR review comments.

sdg0919 · May 31, 2018 · c66d668 · c66d668
1 parent 3ea2f9e
commit c66d668
Show file tree

Hide file tree

Showing 20 changed files with 24 additions and 88 deletions.
diff --git a/apiserver/common/firewall/mock_test.go b/apiserver/common/firewall/mock_test.go
@@ -162,10 +162,6 @@ type mockWatcher struct {
 	tomb.Tomb
 }
 
-func (w *mockWatcher) doneWhenDying() {
-	<-w.Tomb.Dying()
-}
-
 func (w *mockWatcher) Kill() {
 	w.MethodCall(w, "Kill")
 	w.Tomb.Kill(nil)
@@ -193,7 +189,7 @@ type mockStringsWatcher struct {
 func newMockStringsWatcher() *mockStringsWatcher {
 	w := &mockStringsWatcher{changes: make(chan []string, 1)}
 	w.Tomb.Go(func() error {
-		w.doneWhenDying()
+		<-w.Tomb.Dying()
 		return nil
 	})
 	return w
@@ -209,7 +205,7 @@ func newMockNotifyWatcher() *mockNotifyWatcher {
 	// Initial event
 	w.changes <- struct{}{}
 	w.Tomb.Go(func() error {
-		w.doneWhenDying()
+		<-w.Tomb.Dying()
 		return nil
 	})
 	return w
@@ -306,7 +302,7 @@ func (r *mockRelation) WatchRelationEgressNetworks() state.StringsWatcher {
 func newMockRelationUnitsWatcher() *mockRelationUnitsWatcher {
 	w := &mockRelationUnitsWatcher{changes: make(chan params.RelationUnitsChange, 1)}
 	w.Tomb.Go(func() error {
-		w.doneWhenDying()
+		<-w.Tomb.Dying()
 		return nil
 	})
 	return w
@@ -433,7 +429,7 @@ type mockAddressWatcher struct {
 func newMockAddressWatcher() *mockAddressWatcher {
 	w := &mockAddressWatcher{changes: make(chan struct{}, 1)}
 	w.Tomb.Go(func() error {
-		w.doneWhenDying()
+		<-w.Tomb.Dying()
 		return nil
 	})
 	return w

diff --git a/apiserver/facades/controller/caasoperatorprovisioner/mock_test.go b/apiserver/facades/controller/caasoperatorprovisioner/mock_test.go
@@ -69,10 +69,6 @@ type mockWatcher struct {
 	tomb.Tomb
 }
 
-func (w *mockWatcher) doneWhenDying() {
-	<-w.Tomb.Dying()
-}
-
 func (w *mockWatcher) Kill() {
 	w.MethodCall(w, "Kill")
 	w.Tomb.Kill(nil)
@@ -95,7 +91,7 @@ type mockStringsWatcher struct {
 func newMockStringsWatcher() *mockStringsWatcher {
 	w := &mockStringsWatcher{changes: make(chan []string, 1)}
 	w.Tomb.Go(func() error {
-		w.doneWhenDying()
+		<-w.Tomb.Dying()
 		return nil
 	})
 	return w

diff --git a/apiserver/facades/controller/firewaller/mock_test.go b/apiserver/facades/controller/firewaller/mock_test.go
@@ -125,10 +125,6 @@ type mockWatcher struct {
 	tomb.Tomb
 }
 
-func (w *mockWatcher) doneWhenDying() {
-	<-w.Tomb.Dying()
-}
-
 func (w *mockWatcher) Kill() {
 	w.MethodCall(w, "Kill")
 	w.Tomb.Kill(nil)
@@ -156,7 +152,7 @@ type mockStringsWatcher struct {
 func newMockStringsWatcher() *mockStringsWatcher {
 	w := &mockStringsWatcher{changes: make(chan []string, 1)}
 	w.Tomb.Go(func() error {
-		w.doneWhenDying()
+		<-w.Tomb.Dying()
 		return nil
 	})
 	return w
@@ -172,7 +168,7 @@ func newMockNotifyWatcher() *mockNotifyWatcher {
 	// Initial event
 	w.changes <- struct{}{}
 	w.Tomb.Go(func() error {
-		w.doneWhenDying()
+		<-w.Tomb.Dying()
 		return nil
 	})
 	return w
@@ -252,7 +248,7 @@ func (r *mockRelation) SetStatus(info status.StatusInfo) error {
 func newMockRelationUnitsWatcher() *mockRelationUnitsWatcher {
 	w := &mockRelationUnitsWatcher{changes: make(chan params.RelationUnitsChange, 1)}
 	w.Tomb.Go(func() error {
-		w.doneWhenDying()
+		<-w.Tomb.Dying()
 		return nil
 	})
 	return w

diff --git a/apiserver/facades/controller/remoterelations/mock_test.go b/apiserver/facades/controller/remoterelations/mock_test.go
@@ -404,10 +404,6 @@ type mockWatcher struct {
 	tomb.Tomb
 }
 
-func (w *mockWatcher) doneWhenDying() {
-	<-w.Tomb.Dying()
-}
-
 func (w *mockWatcher) Kill() {
 	w.MethodCall(w, "Kill")
 	w.Tomb.Kill(nil)
@@ -430,7 +426,7 @@ type mockStringsWatcher struct {
 func newMockStringsWatcher() *mockStringsWatcher {
 	w := &mockStringsWatcher{changes: make(chan []string, 1)}
 	w.Tomb.Go(func() error {
-		w.doneWhenDying()
+		<-w.Tomb.Dying()
 		return nil
 	})
 	return w
@@ -451,7 +447,7 @@ func newMockRelationUnitsWatcher() *mockRelationUnitsWatcher {
 		changes: make(chan params.RelationUnitsChange, 1),
 	}
 	w.Tomb.Go(func() error {
-		w.doneWhenDying()
+		<-w.Tomb.Dying()
 		return nil
 	})
 	return w

diff --git a/apiserver/pinger.go b/apiserver/pinger.go
@@ -68,9 +68,6 @@ func (pt *pingTimeout) Ping() {
 
 // Stop terminates the ping timeout.
 func (pt *pingTimeout) Stop() error {
-	if err := pt.tomb.Err(); err != tomb.ErrStillAlive {
-		return err
-	}
 	pt.tomb.Kill(nil)
 	return pt.tomb.Wait()
 }

diff --git a/cmd/jujud/agent/caasoperator.go b/cmd/jujud/agent/caasoperator.go
@@ -113,15 +113,15 @@ func (op *CaasOperatorAgent) Stop() error {
 	return op.Wait()
 }
 
-// Finished signals the machine agent is finished
-func (op *CaasOperatorAgent) Finished(err error) {
+// Done signals the machine agent is finished
+func (op *CaasOperatorAgent) Done(err error) {
 	op.errReason = err
 	close(op.dead)
 }
 
 // Run implements Command.
 func (op *CaasOperatorAgent) Run(ctx *cmd.Context) (err error) {
-	defer op.Finished(err)
+	defer op.Done(err)
 	if err := op.ReadConfig(op.Tag().String()); err != nil {
 		return err
 	}

diff --git a/cmd/jujud/agent/machine.go b/cmd/jujud/agent/machine.go
@@ -374,8 +374,8 @@ func (a *MachineAgent) Stop() error {
 	return a.Wait()
 }
 
-// Finished signals the machine agent is finished
-func (a *MachineAgent) Finished(err error) {
+// Done signals the machine agent is finished
+func (a *MachineAgent) Done(err error) {
 	a.errReason = err
 	close(a.dead)
 }
@@ -431,7 +431,7 @@ func upgradeCertificateDNSNames(config agent.ConfigSetter) error {
 
 // Run runs a machine agent.
 func (a *MachineAgent) Run(*cmd.Context) (err error) {
-	defer a.Finished(err)
+	defer a.Done(err)
 	useMultipleCPUs()
 	if err := a.ReadConfig(a.Tag().String()); err != nil {
 		return errors.Errorf("cannot read agent configuration: %v", err)

diff --git a/cmd/jujud/agent/unit.go b/cmd/jujud/agent/unit.go
@@ -139,8 +139,8 @@ func (a *UnitAgent) Wait() error {
 	return a.errReason
 }
 
-// Finished signals the unit agent is finished
-func (a *UnitAgent) Finished(err error) {
+// Done signals the unit agent is finished
+func (a *UnitAgent) Done(err error) {
 	a.errReason = err
 	close(a.dead)
 }
@@ -155,7 +155,7 @@ func (a *UnitAgent) isInitialUpgradeCheckPending() bool {
 
 // Run runs a unit agent.
 func (a *UnitAgent) Run(ctx *cmd.Context) (err error) {
-	defer a.Finished(err)
+	defer a.Done(err)
 	if err := a.ReadConfig(a.Tag().String()); err != nil {
 		return err
 	}

diff --git a/mongo/oplog.go b/mongo/oplog.go
@@ -199,9 +199,6 @@ func (t *OplogTailer) Dying() <-chan struct{} {
 // Stop shuts down the OplogTailer. It will block until shutdown is
 // complete.
 func (t *OplogTailer) Stop() error {
-	if err := t.tomb.Err(); err != tomb.ErrStillAlive {
-		return err
-	}
 	t.tomb.Kill(nil)
 	return t.tomb.Wait()
 }

diff --git a/state/logs.go b/state/logs.go
@@ -401,9 +401,6 @@ func (t *logTailer) Dying() <-chan struct{} {
 
 // Stop implements the LogTailer interface.
 func (t *logTailer) Stop() error {
-	if err := t.tomb.Err(); err != tomb.ErrStillAlive {
-		return err
-	}
 	t.tomb.Kill(nil)
 	return t.tomb.Wait()
 }

diff --git a/state/testing/watcher.go b/state/testing/watcher.go
@@ -333,9 +333,6 @@ func (w *MockNotifyWatcher) Changes() <-chan struct{} {
 }
 
 func (w *MockNotifyWatcher) Stop() error {
-	if err := w.tomb.Err(); err != tomb.ErrStillAlive {
-		return err
-	}
 	w.Kill()
 	return w.Wait()
 }
@@ -372,9 +369,6 @@ func (w *MockStringsWatcher) Changes() <-chan []string {
 }
 
 func (w *MockStringsWatcher) Stop() error {
-	if err := w.tomb.Err(); err != tomb.ErrStillAlive {
-		return err
-	}
 	w.Kill()
 	return w.Wait()
 }

diff --git a/state/watcher/helpers.go b/state/watcher/helpers.go
@@ -4,8 +4,6 @@
 package watcher
 
 import (
-	"fmt"
-
 	"github.com/juju/errors"
 	"gopkg.in/tomb.v2"
 )
@@ -30,7 +28,6 @@ func Stop(w Stopper, t *tomb.Tomb) {
 			// wrap any other error.
 			err = errors.Trace(err)
 		}
-		fmt.Printf(">> Error is: %s\n", err)
 		t.Kill(err)
 	}
 }

diff --git a/state/watcher/watchertest/notify.go b/state/watcher/watchertest/notify.go
@@ -26,9 +26,6 @@ func (w *NotifyWatcher) Changes() <-chan struct{} {
 }
 
 func (w *NotifyWatcher) Stop() error {
-	if err := w.tomb.Err(); err != tomb.ErrStillAlive {
-		return err
-	}
 	w.Kill()
 	return w.Wait()
 }

diff --git a/watcher/legacy/notifyworker_test.go b/watcher/legacy/notifyworker_test.go
@@ -8,7 +8,6 @@ import (
 	"sync"
 	"time"
 
-	"github.com/juju/loggo"
 	jc "github.com/juju/testing/checkers"
 	gc "gopkg.in/check.v1"
 	worker "gopkg.in/juju/worker.v1"
@@ -19,8 +18,6 @@ import (
 	"github.com/juju/juju/watcher/legacy"
 )
 
-var logger = loggo.GetLogger("juju.watcher.legacy")
-
 type NotifyWorkerSuite struct {
 	coretesting.BaseSuite
 	worker worker.Worker
@@ -145,11 +142,9 @@ func (tnw *testNotifyWatcher) Err() error {
 }
 
 func (tnw *testNotifyWatcher) Stop() error {
-	logger.Criticalf(">> Stop!")
 	tnw.mu.Lock()
 	defer tnw.mu.Unlock()
 	if !tnw.stopped {
-		logger.Criticalf(">> Stopping changes.")
 		close(tnw.changes)
 	}
 	tnw.stopped = true
@@ -158,7 +153,6 @@ func (tnw *testNotifyWatcher) Stop() error {
 
 func (tnw *testNotifyWatcher) SetStopError(err error) {
 	tnw.mu.Lock()
-	logger.Criticalf(">> setting error: %s", err)
 	tnw.stopError = err
 	tnw.mu.Unlock()
 }

diff --git a/watcher/watchertest/notify.go b/watcher/watchertest/notify.go
@@ -6,7 +6,6 @@ package watchertest
 import (
 	"time"
 
-	"github.com/juju/loggo"
 	jc "github.com/juju/testing/checkers"
 	gc "gopkg.in/check.v1"
 	tomb "gopkg.in/tomb.v2"
@@ -15,8 +14,6 @@ import (
 	"github.com/juju/juju/watcher"
 )
 
-var logger = loggo.GetLogger("veebers")
-
 type MockNotifyWatcher struct {
 	tomb tomb.Tomb
 	ch   <-chan struct{}
@@ -36,9 +33,6 @@ func (w *MockNotifyWatcher) Changes() watcher.NotifyChannel {
 }
 
 func (w *MockNotifyWatcher) Stop() error {
-	if err := w.tomb.Err(); err != tomb.ErrStillAlive {
-		return err
-	}
 	w.Kill()
 	return w.Wait()
 }

diff --git a/watcher/watchertest/strings.go b/watcher/watchertest/strings.go
@@ -34,9 +34,6 @@ func (w *MockStringsWatcher) Changes() watcher.StringsChannel {
 }
 
 func (w *MockStringsWatcher) Stop() error {
-	if err := w.tomb.Err(); err != tomb.ErrStillAlive {
-		return err
-	}
 	w.Kill()
 	return w.Wait()
 }

diff --git a/worker/caasoperatorprovisioner/mock_test.go b/worker/caasoperatorprovisioner/mock_test.go
@@ -129,10 +129,6 @@ type mockWatcher struct {
 	terminated bool
 }
 
-func (w *mockWatcher) doneWhenDying() {
-	<-w.Tomb.Dying()
-}
-
 func (w *mockWatcher) killed() bool {
 	w.mu.Lock()
 	defer w.mu.Unlock()
@@ -164,7 +160,7 @@ type mockStringsWatcher struct {
 func newMockStringsWatcher() *mockStringsWatcher {
 	w := &mockStringsWatcher{changes: make(chan []string, 5)}
 	w.Tomb.Go(func() error {
-		w.doneWhenDying()
+		<-w.Tomb.Dying()
 		return nil
 	})
 	return w

diff --git a/worker/meterstatus/isolated_test.go b/worker/meterstatus/isolated_test.go
@@ -117,7 +117,6 @@ func (s *IsolatedWorkerSuite) SetUpTest(c *gc.C) {
 	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(wrk, gc.NotNil)
 	s.worker = wrk
-	time.Sleep(1 * time.Second)
 }
 
 func (s *IsolatedWorkerSuite) TearDownTest(c *gc.C) {

diff --git a/worker/metrics/spool/listener.go b/worker/metrics/spool/listener.go
@@ -46,9 +46,6 @@ func NewSocketListener(socketPath string, handler ConnectionHandler) (*socketLis
 // Stop closes the listener and releases all resources
 // used by the socketListener.
 func (l *socketListener) Stop() error {
-	if err := l.t.Err(); err != tomb.ErrStillAlive {
-		return err
-	}
 	l.t.Kill(nil)
 	err := l.listener.Close()
 	if err != nil {