From c5b26925aea042a11c98880e12fcf793e7db8b0d Mon Sep 17 00:00:00 2001
From: anastasia <anastasia.macmood@canonical.com>
Date: Thu, 12 Apr 2018 18:30:31 +1000
Subject: [PATCH 1/6] cloud cred worker/manifold

---
 .../credentialvalidator.go                    |   2 +-
 .../credentialvalidator.go                    |   8 +-
 .../credentialvalidator_test.go               |   6 +-
 cmd/jujud/agent/engine_test.go                |   1 +
 cmd/jujud/agent/machine_test.go               |  16 +-
 cmd/jujud/agent/model/manifolds.go            |  21 ++-
 cmd/jujud/agent/model/manifolds_test.go       |   3 +
 worker/credentialvalidator/manifold.go        |  83 ++++++++
 worker/credentialvalidator/manifold_test.go   | 177 ++++++++++++++++++
 worker/credentialvalidator/package_test.go    |  14 ++
 worker/credentialvalidator/shim.go            |  27 +++
 worker/credentialvalidator/util_test.go       | 155 +++++++++++++++
 worker/credentialvalidator/validate_test.go   |  34 ++++
 worker/credentialvalidator/worker.go          | 162 ++++++++++++++++
 worker/credentialvalidator/worker_test.go     | 165 ++++++++++++++++
 15 files changed, 863 insertions(+), 11 deletions(-)
 create mode 100644 worker/credentialvalidator/manifold.go
 create mode 100644 worker/credentialvalidator/manifold_test.go
 create mode 100644 worker/credentialvalidator/package_test.go
 create mode 100644 worker/credentialvalidator/shim.go
 create mode 100644 worker/credentialvalidator/util_test.go
 create mode 100644 worker/credentialvalidator/validate_test.go
 create mode 100644 worker/credentialvalidator/worker.go
 create mode 100644 worker/credentialvalidator/worker_test.go

diff --git a/api/credentialvalidator/credentialvalidator.go b/api/credentialvalidator/credentialvalidator.go
index a1c4346a731..9bf23dc343a 100644
--- a/api/credentialvalidator/credentialvalidator.go
+++ b/api/credentialvalidator/credentialvalidator.go
@@ -59,7 +59,7 @@ func (c *Facade) ModelCredential() (base.StoredCredential, bool, error) {
 func (c *Facade) WatchCredential(credentialID string) (watcher.NotifyWatcher, error) {
 	in := names.NewCloudCredentialTag(credentialID).String()
 	var result params.NotifyWatchResult
-	err := c.facade.FacadeCall("WatchCredential", in, &result)
+	err := c.facade.FacadeCall("WatchCredential", params.Entity{in}, &result)
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
diff --git a/apiserver/facades/agent/credentialvalidator/credentialvalidator.go b/apiserver/facades/agent/credentialvalidator/credentialvalidator.go
index 57fd562335a..5b35f1bcbc5 100644
--- a/apiserver/facades/agent/credentialvalidator/credentialvalidator.go
+++ b/apiserver/facades/agent/credentialvalidator/credentialvalidator.go
@@ -17,7 +17,8 @@ var logger = loggo.GetLogger("juju.api.credentialvalidator")
 // CredentialValidator defines the methods on credentialvalidator API endpoint.
 type CredentialValidator interface {
 	ModelCredential() (params.ModelCredential, error)
-	WatchCredential(string) (params.NotifyWatchResult, error)
+	WatchCredential(params.Entity) (params.NotifyWatchResult, error)
+	//WatchCredential(string) (params.NotifyWatchResult, error)
 }
 
 type CredentialValidatorAPI struct {
@@ -48,16 +49,15 @@ func internalNewCredentialValidatorAPI(backend Backend, resources facade.Resourc
 // changes to the given cloud credentials.
 // The order of returned watchers is important and corresponds directly to the
 // order of supplied cloud credentials collection.
-func (api *CredentialValidatorAPI) WatchCredential(tag string) (params.NotifyWatchResult, error) {
+func (api *CredentialValidatorAPI) WatchCredential(tag params.Entity) (params.NotifyWatchResult, error) {
 	fail := func(failure error) (params.NotifyWatchResult, error) {
 		return params.NotifyWatchResult{}, common.ServerError(failure)
 	}
 
-	credentialTag, err := names.ParseCloudCredentialTag(tag)
+	credentialTag, err := names.ParseCloudCredentialTag(tag.Tag)
 	if err != nil {
 		return fail(err)
 	}
-
 	// Is credential used by the model that has created this backend?
 	isUsed, err := api.backend.ModelUsesCredential(credentialTag)
 	if err != nil {
diff --git a/apiserver/facades/agent/credentialvalidator/credentialvalidator_test.go b/apiserver/facades/agent/credentialvalidator/credentialvalidator_test.go
index 30f0b1741d3..729425085ba 100644
--- a/apiserver/facades/agent/credentialvalidator/credentialvalidator_test.go
+++ b/apiserver/facades/agent/credentialvalidator/credentialvalidator_test.go
@@ -67,7 +67,7 @@ func (s *CredentialValidatorSuite) TestModelCredentialNotNeeded(c *gc.C) {
 }
 
 func (s *CredentialValidatorSuite) TestWatchCredential(c *gc.C) {
-	result, err := s.api.WatchCredential(credentialTag.String())
+	result, err := s.api.WatchCredential(params.Entity{credentialTag.String()})
 	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(result, gc.DeepEquals, params.NotifyWatchResult{"1", nil})
 	c.Assert(s.resources.Count(), gc.Equals, 1)
@@ -75,13 +75,13 @@ func (s *CredentialValidatorSuite) TestWatchCredential(c *gc.C) {
 
 func (s *CredentialValidatorSuite) TestWatchCredentialNotUsedInThisModel(c *gc.C) {
 	s.backend.isUsed = false
-	_, err := s.api.WatchCredential(credentialTag.String())
+	_, err := s.api.WatchCredential(params.Entity{credentialTag.String()})
 	c.Assert(err, gc.ErrorMatches, common.ErrPerm.Error())
 	c.Assert(s.resources.Count(), gc.Equals, 0)
 }
 
 func (s *CredentialValidatorSuite) TestWatchCredentialInvalidTag(c *gc.C) {
-	_, err := s.api.WatchCredential("my-tag")
+	_, err := s.api.WatchCredential(params.Entity{"my-tag"})
 	c.Assert(err, gc.ErrorMatches, `"my-tag" is not a valid tag`)
 	c.Assert(s.resources.Count(), gc.Equals, 0)
 }
diff --git a/cmd/jujud/agent/engine_test.go b/cmd/jujud/agent/engine_test.go
index c7b94773ded..38c04810328 100644
--- a/cmd/jujud/agent/engine_test.go
+++ b/cmd/jujud/agent/engine_test.go
@@ -34,6 +34,7 @@ var (
 		"action-pruner",
 		"charm-revision-updater",
 		"compute-provisioner",
+		"credential-validator-flag",
 		"environ-tracker",
 		"firewaller",
 		"instance-poller",
diff --git a/cmd/jujud/agent/machine_test.go b/cmd/jujud/agent/machine_test.go
index df390f13cd0..d09c6895c0b 100644
--- a/cmd/jujud/agent/machine_test.go
+++ b/cmd/jujud/agent/machine_test.go
@@ -1041,8 +1041,20 @@ func (s *MachineSuite) TestHostedModelWorkers(c *gc.C) {
 		return &minModelWorkersEnviron{}, nil
 	})
 
-	st, closer := s.setUpNewModel(c)
-	defer closer()
+	st := s.Factory.MakeModel(c, &factory.ModelParams{
+		ConfigAttrs: coretesting.Attrs{
+			"max-status-history-age":  "2h",
+			"max-status-history-size": "4M",
+			"max-action-results-age":  "2h",
+			"max-action-results-size": "4M",
+		},
+		CloudCredential: names.NewCloudCredentialTag("dummy/admin/cred"),
+	})
+	defer func() {
+		err := st.Close()
+		c.Check(err, jc.ErrorIsNil)
+	}()
+
 	uuid := st.ModelUUID()
 
 	tracker := agenttest.NewEngineTracker()
diff --git a/cmd/jujud/agent/model/manifolds.go b/cmd/jujud/agent/model/manifolds.go
index db9eb0c4339..41b6b6dec64 100644
--- a/cmd/jujud/agent/model/manifolds.go
+++ b/cmd/jujud/agent/model/manifolds.go
@@ -34,6 +34,7 @@ import (
 	"github.com/juju/juju/worker/charmrevision"
 	"github.com/juju/juju/worker/charmrevision/charmrevisionmanifold"
 	"github.com/juju/juju/worker/cleaner"
+	"github.com/juju/juju/worker/credentialvalidator"
 	"github.com/juju/juju/worker/dependency"
 	"github.com/juju/juju/worker/environ"
 	"github.com/juju/juju/worker/firewaller"
@@ -80,7 +81,7 @@ type ManifoldsConfig struct {
 	Clock clock.Clock
 
 	// InstPollerAggregationDelay is the delay before sending a batch of
-	// requests in the instancpoller.Worker's aggregate loop.
+	// requests in the instancepoller.Worker's aggregate loop.
 	InstPollerAggregationDelay time.Duration
 
 	// RunFlagDuration defines for how long this controller will ask
@@ -170,6 +171,14 @@ func commonManifolds(config ManifoldsConfig) dependency.Manifolds {
 			NewFacade: singular.NewFacade,
 			NewWorker: singular.NewWorker,
 		}),
+		// Cloud credential validator runs on all models, and
+		// determines if model's cloud credential is valid.
+		credentialValidatorFlagName: ifNotUpgrading(ifNotDead(credentialvalidator.Manifold(credentialvalidator.ManifoldConfig{
+			APICallerName: apiCallerName,
+			Check:         credentialvalidator.IsValid,
+			NewFacade:     credentialvalidator.NewFacade,
+			NewWorker:     credentialvalidator.NewWorker,
+		}))),
 
 		// The migration workers collaborate to run migrations;
 		// and to create a mechanism for running other workers
@@ -519,6 +528,14 @@ var (
 			modelUpgradedFlagName,
 		},
 	}.Decorate
+
+	// ifCredentialValid wraps a manifold such that it only runs if
+	// the model has a valid credential.
+	ifCredentialValid = engine.Housing{
+		Flags: []string{
+			credentialValidatorFlagName,
+		},
+	}.Decorate
 )
 
 const (
@@ -560,4 +577,6 @@ const (
 	caasOperatorProvisionerName = "caas-operator-provisioner"
 	caasUnitProvisionerName     = "caas-unit-provisioner"
 	caasBrokerTrackerName       = "caas-broker-tracker"
+
+	credentialValidatorFlagName = "credential-validator-flag"
 )
diff --git a/cmd/jujud/agent/model/manifolds_test.go b/cmd/jujud/agent/model/manifolds_test.go
index 31f08769eed..5e965abe594 100644
--- a/cmd/jujud/agent/model/manifolds_test.go
+++ b/cmd/jujud/agent/model/manifolds_test.go
@@ -39,6 +39,7 @@ func (s *ManifoldsSuite) TestIAASNames(c *gc.C) {
 		"charm-revision-updater",
 		"clock",
 		"compute-provisioner",
+		"credential-validator-flag",
 		"environ-tracker",
 		"firewaller",
 		"instance-poller",
@@ -84,6 +85,7 @@ func (s *ManifoldsSuite) TestCAASNames(c *gc.C) {
 		"caas-unit-provisioner",
 		"charm-revision-updater",
 		"clock",
+		"credential-validator-flag",
 		"is-responsible-flag",
 		"log-forwarder",
 		"migration-fortress",
@@ -107,6 +109,7 @@ func (s *ManifoldsSuite) TestFlagDependencies(c *gc.C) {
 		"api-caller",
 		"api-config-watcher",
 		"clock",
+		"credential-validator-flag",
 		"is-responsible-flag",
 		"not-alive-flag",
 		"not-dead-flag",
diff --git a/worker/credentialvalidator/manifold.go b/worker/credentialvalidator/manifold.go
new file mode 100644
index 00000000000..96e5d2cca8d
--- /dev/null
+++ b/worker/credentialvalidator/manifold.go
@@ -0,0 +1,83 @@
+// Copyright 2018 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package credentialvalidator
+
+import (
+	"github.com/juju/errors"
+	worker "gopkg.in/juju/worker.v1"
+
+	"github.com/juju/juju/api/base"
+	"github.com/juju/juju/cmd/jujud/agent/engine"
+	"github.com/juju/juju/worker/dependency"
+)
+
+// ManifoldConfig holds the dependencies and configuration for a
+// Worker manifold.
+type ManifoldConfig struct {
+	APICallerName string
+	Check         Predicate
+
+	NewFacade func(base.APICaller) (Facade, error)
+	NewWorker func(Config) (worker.Worker, error)
+}
+
+// validate is called by start to check for bad configuration.
+func (config ManifoldConfig) validate() error {
+	if config.APICallerName == "" {
+		return errors.NotValidf("empty APICallerName")
+	}
+	if config.Check == nil {
+		return errors.NotValidf("nil Check")
+	}
+	if config.NewFacade == nil {
+		return errors.NotValidf("nil NewFacade")
+	}
+	if config.NewWorker == nil {
+		return errors.NotValidf("nil NewWorker")
+	}
+	return nil
+}
+
+// start is a StartFunc for a Worker manifold.
+func (config ManifoldConfig) start(context dependency.Context) (worker.Worker, error) {
+	if err := config.validate(); err != nil {
+		return nil, errors.Trace(err)
+	}
+	var apiCaller base.APICaller
+	if err := context.Get(config.APICallerName, &apiCaller); err != nil {
+		return nil, errors.Trace(err)
+	}
+	facade, err := config.NewFacade(apiCaller)
+	if err != nil {
+		return nil, errors.Trace(err)
+	}
+	worker, err := config.NewWorker(Config{
+		Facade: facade,
+		Check:  config.Check,
+	})
+	if err != nil {
+		return nil, errors.Trace(err)
+	}
+	return worker, nil
+}
+
+// Manifold packages a Worker for use in a dependency.Engine.
+func Manifold(config ManifoldConfig) dependency.Manifold {
+	return dependency.Manifold{
+		Inputs: []string{config.APICallerName},
+		Start:  config.start,
+		Output: engine.FlagOutput,
+		Filter: filterErrors,
+	}
+}
+
+func filterErrors(err error) error {
+	cause := errors.Cause(err)
+	if cause == ErrChanged || cause == ErrModelCredentialChanged {
+		return dependency.ErrBounce
+	} else if cause == ErrModelDoesNotNeedCredential {
+		return dependency.ErrUninstall
+	}
+	return err
+}
diff --git a/worker/credentialvalidator/manifold_test.go b/worker/credentialvalidator/manifold_test.go
new file mode 100644
index 00000000000..02a68a5a140
--- /dev/null
+++ b/worker/credentialvalidator/manifold_test.go
@@ -0,0 +1,177 @@
+// Copyright 2018 Canonical Ltd.
+// Licensed under the LGPLv3, see LICENCE file for details.
+
+package credentialvalidator_test
+
+import (
+	"github.com/juju/errors"
+	"github.com/juju/testing"
+	jc "github.com/juju/testing/checkers"
+	gc "gopkg.in/check.v1"
+	worker "gopkg.in/juju/worker.v1"
+
+	"github.com/juju/juju/api/base"
+	"github.com/juju/juju/cmd/jujud/agent/engine"
+	"github.com/juju/juju/worker/credentialvalidator"
+	"github.com/juju/juju/worker/dependency"
+	dt "github.com/juju/juju/worker/dependency/testing"
+)
+
+type ManifoldSuite struct {
+	testing.IsolationSuite
+}
+
+var _ = gc.Suite(&ManifoldSuite{})
+
+func (*ManifoldSuite) TestInputs(c *gc.C) {
+	manifold := credentialvalidator.Manifold(validManifoldConfig())
+	c.Check(manifold.Inputs, jc.DeepEquals, []string{"api-caller"})
+}
+
+func (*ManifoldSuite) TestOutputBadWorker(c *gc.C) {
+	manifold := credentialvalidator.Manifold(credentialvalidator.ManifoldConfig{})
+	in := &struct{ worker.Worker }{}
+	var out engine.Flag
+	err := manifold.Output(in, &out)
+	c.Check(err, gc.ErrorMatches, "expected in to implement Flag; got a .*")
+}
+
+func (*ManifoldSuite) TestOutputBadTarget(c *gc.C) {
+	manifold := credentialvalidator.Manifold(credentialvalidator.ManifoldConfig{})
+	in := &credentialvalidator.Worker{}
+	var out bool
+	err := manifold.Output(in, &out)
+	c.Check(err, gc.ErrorMatches, "expected out to be a \\*Flag; got a .*")
+}
+
+func (*ManifoldSuite) TestOutputBadInput(c *gc.C) {
+	manifold := credentialvalidator.Manifold(credentialvalidator.ManifoldConfig{})
+	in := &credentialvalidator.Worker{}
+	var out engine.Flag
+	err := manifold.Output(in, &out)
+	c.Check(err, jc.ErrorIsNil)
+	c.Check(out, gc.Equals, in)
+}
+
+func (*ManifoldSuite) TestFilterNil(c *gc.C) {
+	manifold := credentialvalidator.Manifold(credentialvalidator.ManifoldConfig{})
+	err := manifold.Filter(nil)
+	c.Check(err, jc.ErrorIsNil)
+}
+
+func (*ManifoldSuite) TestFilterErrChanged(c *gc.C) {
+	manifold := credentialvalidator.Manifold(credentialvalidator.ManifoldConfig{})
+	err := manifold.Filter(credentialvalidator.ErrChanged)
+	c.Check(err, gc.Equals, dependency.ErrBounce)
+}
+
+func (*ManifoldSuite) TestFilterErrModelCredentialChanged(c *gc.C) {
+	manifold := credentialvalidator.Manifold(credentialvalidator.ManifoldConfig{})
+	err := manifold.Filter(credentialvalidator.ErrModelCredentialChanged)
+	c.Check(err, gc.Equals, dependency.ErrBounce)
+}
+
+func (*ManifoldSuite) TestFilterErrModelDoesNotNeedCredential(c *gc.C) {
+	manifold := credentialvalidator.Manifold(credentialvalidator.ManifoldConfig{})
+	err := manifold.Filter(credentialvalidator.ErrModelDoesNotNeedCredential)
+	c.Check(err, gc.Equals, dependency.ErrUninstall)
+}
+
+func (*ManifoldSuite) TestFilterOther(c *gc.C) {
+	manifold := credentialvalidator.Manifold(credentialvalidator.ManifoldConfig{})
+	expect := errors.New("whatever")
+	actual := manifold.Filter(expect)
+	c.Check(actual, gc.Equals, expect)
+}
+
+func (*ManifoldSuite) TestStartMissingAPICallerName(c *gc.C) {
+	config := validManifoldConfig()
+	config.APICallerName = ""
+	checkManifoldNotValid(c, config, "empty APICallerName not valid")
+}
+
+func (*ManifoldSuite) TestStartMissingCheck(c *gc.C) {
+	config := validManifoldConfig()
+	config.Check = nil
+	checkManifoldNotValid(c, config, "nil Check not valid")
+}
+
+func (*ManifoldSuite) TestStartMissingNewFacade(c *gc.C) {
+	config := validManifoldConfig()
+	config.NewFacade = nil
+	checkManifoldNotValid(c, config, "nil NewFacade not valid")
+}
+
+func (*ManifoldSuite) TestStartMissingNewWorker(c *gc.C) {
+	config := validManifoldConfig()
+	config.NewWorker = nil
+	checkManifoldNotValid(c, config, "nil NewWorker not valid")
+}
+
+func (*ManifoldSuite) TestStartMissingAPICaller(c *gc.C) {
+	context := dt.StubContext(nil, map[string]interface{}{
+		"api-caller": dependency.ErrMissing,
+	})
+	manifold := credentialvalidator.Manifold(validManifoldConfig())
+
+	worker, err := manifold.Start(context)
+	c.Check(worker, gc.IsNil)
+	c.Check(errors.Cause(err), gc.Equals, dependency.ErrMissing)
+}
+
+func (*ManifoldSuite) TestStartNewFacadeError(c *gc.C) {
+	expectCaller := &stubCaller{}
+	context := dt.StubContext(nil, map[string]interface{}{
+		"api-caller": expectCaller,
+	})
+	config := validManifoldConfig()
+	config.NewFacade = func(caller base.APICaller) (credentialvalidator.Facade, error) {
+		c.Check(caller, gc.Equals, expectCaller)
+		return nil, errors.New("bort")
+	}
+	manifold := credentialvalidator.Manifold(config)
+
+	worker, err := manifold.Start(context)
+	c.Check(worker, gc.IsNil)
+	c.Check(err, gc.ErrorMatches, "bort")
+}
+
+func (*ManifoldSuite) TestStartNewWorkerError(c *gc.C) {
+	context := dt.StubContext(nil, map[string]interface{}{
+		"api-caller": &stubCaller{},
+	})
+	expectFacade := &struct{ credentialvalidator.Facade }{}
+	config := validManifoldConfig()
+	config.NewFacade = func(base.APICaller) (credentialvalidator.Facade, error) {
+		return expectFacade, nil
+	}
+	config.NewWorker = func(workerConfig credentialvalidator.Config) (worker.Worker, error) {
+		c.Check(workerConfig.Facade, gc.Equals, expectFacade)
+		c.Check(workerConfig.Check, gc.NotNil) // uncomparable
+		return nil, errors.New("snerk")
+	}
+	manifold := credentialvalidator.Manifold(config)
+
+	worker, err := manifold.Start(context)
+	c.Check(worker, gc.IsNil)
+	c.Check(err, gc.ErrorMatches, "snerk")
+}
+
+func (*ManifoldSuite) TestStartSuccess(c *gc.C) {
+	context := dt.StubContext(nil, map[string]interface{}{
+		"api-caller": &stubCaller{},
+	})
+	expectWorker := &struct{ worker.Worker }{}
+	config := validManifoldConfig()
+	config.NewFacade = func(base.APICaller) (credentialvalidator.Facade, error) {
+		return &struct{ credentialvalidator.Facade }{}, nil
+	}
+	config.NewWorker = func(credentialvalidator.Config) (worker.Worker, error) {
+		return expectWorker, nil
+	}
+	manifold := credentialvalidator.Manifold(config)
+
+	worker, err := manifold.Start(context)
+	c.Check(err, jc.ErrorIsNil)
+	c.Check(worker, gc.Equals, expectWorker)
+}
diff --git a/worker/credentialvalidator/package_test.go b/worker/credentialvalidator/package_test.go
new file mode 100644
index 00000000000..467bd538a96
--- /dev/null
+++ b/worker/credentialvalidator/package_test.go
@@ -0,0 +1,14 @@
+// Copyright 2018 Canonical Ltd.
+// Licensed under the LGPLv3, see LICENCE file for details.
+
+package credentialvalidator_test
+
+import (
+	stdtesting "testing"
+
+	gc "gopkg.in/check.v1"
+)
+
+func TestPackage(t *stdtesting.T) {
+	gc.TestingT(t)
+}
diff --git a/worker/credentialvalidator/shim.go b/worker/credentialvalidator/shim.go
new file mode 100644
index 00000000000..a1d58143fcf
--- /dev/null
+++ b/worker/credentialvalidator/shim.go
@@ -0,0 +1,27 @@
+// Copyright 2018 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package credentialvalidator
+
+import (
+	"github.com/juju/errors"
+	worker "gopkg.in/juju/worker.v1"
+
+	"github.com/juju/juju/api/base"
+	"github.com/juju/juju/api/credentialvalidator"
+)
+
+// NewFacade creates a *credentialvalidator.Facade and returns it as a Facade.
+func NewFacade(apiCaller base.APICaller) (Facade, error) {
+	facade := credentialvalidator.NewFacade(apiCaller)
+	return facade, nil
+}
+
+// NewWorker creates a *Worker and returns it as a worker.Worker.
+func NewWorker(config Config) (worker.Worker, error) {
+	worker, err := New(config)
+	if err != nil {
+		return nil, errors.Trace(err)
+	}
+	return worker, nil
+}
diff --git a/worker/credentialvalidator/util_test.go b/worker/credentialvalidator/util_test.go
new file mode 100644
index 00000000000..7d17695a308
--- /dev/null
+++ b/worker/credentialvalidator/util_test.go
@@ -0,0 +1,155 @@
+// Copyright 2018 Canonical Ltd.
+// Licensed under the LGPLv3, see LICENCE file for details.
+
+package credentialvalidator_test
+
+import (
+	"github.com/juju/errors"
+	"github.com/juju/testing"
+	jc "github.com/juju/testing/checkers"
+	gc "gopkg.in/check.v1"
+	"gopkg.in/juju/names.v2"
+	worker "gopkg.in/juju/worker.v1"
+
+	"github.com/juju/juju/api/base"
+	"github.com/juju/juju/watcher"
+	"github.com/juju/juju/worker/credentialvalidator"
+	dt "github.com/juju/juju/worker/dependency/testing"
+	"github.com/juju/juju/worker/workertest"
+)
+
+// mockFacade implements credentialvalidator.Facade for use in the tests.
+type mockFacade struct {
+	*testing.Stub
+	credentials []base.StoredCredential
+	exists      bool
+}
+
+// ModelCredential is part of the credentialvalidator.Facade interface.
+func (m *mockFacade) ModelCredential() (base.StoredCredential, bool, error) {
+	m.AddCall("ModelCredential")
+	if err := m.NextErr(); err != nil {
+		return base.StoredCredential{}, false, err
+	}
+	return m.nextCredential(), m.exists, nil
+}
+
+// nextCredential consumes a credential and returns it, or panics.
+func (m *mockFacade) nextCredential() base.StoredCredential {
+	credential := m.credentials[0]
+	m.credentials = m.credentials[1:]
+	return credential
+}
+
+// WatchCredential is part of the credentialvalidator.Facade interface.
+func (mock *mockFacade) WatchCredential(id string) (watcher.NotifyWatcher, error) {
+	mock.AddCall("WatchCredential", id)
+	if err := mock.NextErr(); err != nil {
+		return nil, err
+	}
+	return newMockWatcher(), nil
+}
+
+// newMockWatcher returns a watcher.NotifyWatcher that always
+// sends 3 changes and then sits quietly until killed.
+func newMockWatcher() *mockWatcher {
+	const count = 3
+	changes := make(chan struct{}, count)
+	for i := 0; i < count; i++ {
+		changes <- struct{}{}
+	}
+	return &mockWatcher{
+		Worker:  workertest.NewErrorWorker(nil),
+		changes: changes,
+	}
+}
+
+// mockWatcher implements watcher.NotifyWatcher for use in the tests.
+type mockWatcher struct {
+	worker.Worker
+	changes chan struct{}
+}
+
+// Changes is part of the watcher.NotifyWatcher interface.
+func (mock *mockWatcher) Changes() watcher.NotifyChannel {
+	return mock.changes
+}
+
+// modelUUID is the model tag we're using in the tests.
+var modelUUID = "01234567-89ab-cdef-0123-456789abcdef"
+
+// credentialTag is the credential tag we're using in the tests.
+// needs to fit fmt.Sprintf("%s/%s/%s", cloudName, userName, credentialName)
+var credentialTag = names.NewCloudCredentialTag("cloud/user/credential").String()
+
+// panicCheck is a Config.Check value that should not be called.
+func panicCheck(base.StoredCredential) bool { panic("unexpected") }
+
+// neverCheck is a Config.Check value that always returns false.
+func neverCheck(base.StoredCredential) bool { return false }
+
+// panicFacade is a NewFacade that should not be called.
+func panicFacade(base.APICaller) (credentialvalidator.Facade, error) {
+	panic("panicFacade")
+}
+
+// panicWorker is a NewWorker that should not be called.
+func panicWorker(credentialvalidator.Config) (worker.Worker, error) {
+	panic("panicWorker")
+}
+
+// validConfig returns a minimal config stuffed with dummy objects that
+// will explode when used.
+func validConfig() credentialvalidator.Config {
+	return credentialvalidator.Config{
+		Facade: struct{ credentialvalidator.Facade }{},
+		Check:  panicCheck,
+	}
+}
+
+// checkNotValid checks that the supplied credentialvalidator.Config fails to
+// Validate, and cannot be used to construct a credentialvalidator.Worker.
+func checkNotValid(c *gc.C, config credentialvalidator.Config, expect string) {
+	check := func(err error) {
+		c.Check(err, gc.ErrorMatches, expect)
+		c.Check(err, jc.Satisfies, errors.IsNotValid)
+	}
+
+	err := config.Validate()
+	check(err)
+
+	worker, err := credentialvalidator.New(config)
+	c.Check(worker, gc.IsNil)
+	check(err)
+}
+
+// validManifoldConfig returns a minimal config stuffed with dummy objects
+// that will explode when used.
+func validManifoldConfig() credentialvalidator.ManifoldConfig {
+	return credentialvalidator.ManifoldConfig{
+		APICallerName: "api-caller",
+		Check:         panicCheck,
+		NewFacade:     panicFacade,
+		NewWorker:     panicWorker,
+	}
+}
+
+// checkManifoldNotValid checks that the supplied ManifoldConfig creates
+// a manifold that cannot be started.
+func checkManifoldNotValid(c *gc.C, config credentialvalidator.ManifoldConfig, expect string) {
+	manifold := credentialvalidator.Manifold(config)
+	worker, err := manifold.Start(dt.StubContext(nil, nil))
+	c.Check(worker, gc.IsNil)
+	c.Check(err, gc.ErrorMatches, expect)
+	c.Check(err, jc.Satisfies, errors.IsNotValid)
+}
+
+// stubCaller is a base.APICaller that only implements ModelTag.
+type stubCaller struct {
+	base.APICaller
+}
+
+// ModelTag is part of the base.APICaller interface.
+func (*stubCaller) ModelTag() (names.ModelTag, bool) {
+	return names.NewModelTag(modelUUID), true
+}
diff --git a/worker/credentialvalidator/validate_test.go b/worker/credentialvalidator/validate_test.go
new file mode 100644
index 00000000000..3a6fb8ee6b9
--- /dev/null
+++ b/worker/credentialvalidator/validate_test.go
@@ -0,0 +1,34 @@
+// Copyright 2018 Canonical Ltd.
+// Licensed under the LGPLv3, see LICENCE file for details.
+
+package credentialvalidator_test
+
+import (
+	"github.com/juju/testing"
+	jc "github.com/juju/testing/checkers"
+	gc "gopkg.in/check.v1"
+)
+
+type ValidateSuite struct {
+	testing.IsolationSuite
+}
+
+var _ = gc.Suite(&ValidateSuite{})
+
+func (*ValidateSuite) TestValid(c *gc.C) {
+	config := validConfig()
+	err := config.Validate()
+	c.Check(err, jc.ErrorIsNil)
+}
+
+func (*ValidateSuite) TestNilFacade(c *gc.C) {
+	config := validConfig()
+	config.Facade = nil
+	checkNotValid(c, config, "nil Facade not valid")
+}
+
+func (*ValidateSuite) TestNilCheck(c *gc.C) {
+	config := validConfig()
+	config.Check = nil
+	checkNotValid(c, config, "nil Check not valid")
+}
diff --git a/worker/credentialvalidator/worker.go b/worker/credentialvalidator/worker.go
new file mode 100644
index 00000000000..53d0442043e
--- /dev/null
+++ b/worker/credentialvalidator/worker.go
@@ -0,0 +1,162 @@
+// Copyright 2018 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package credentialvalidator
+
+import (
+	"github.com/juju/errors"
+	"github.com/juju/loggo"
+	"gopkg.in/juju/worker.v1"
+
+	"github.com/juju/juju/api/base"
+	"github.com/juju/juju/watcher"
+	"github.com/juju/juju/worker/catacomb"
+)
+
+var logger = loggo.GetLogger("juju.api.credentialvalidator")
+
+// ErrChanged indicates that a Worker has bounced because its
+// Check result has changed.
+var ErrChanged = errors.New("cloud credential validity has changed")
+
+// ErrModelCredentialChanged indicates that a Worker has bounced
+// because model credential was replaced.
+var ErrModelCredentialChanged = errors.New("unexpected model credential")
+
+// ErrModelDoesNotNeedCredential indicates that a Worker has been uninstalled
+// since the model does not have a cloud credential set as the model is
+// on the cloud that does not require authentication.
+var ErrModelDoesNotNeedCredential = errors.New("model is on the cloud that does not need auth")
+
+// Predicate defines a predicate.
+type Predicate func(base.StoredCredential) bool
+
+// Facade exposes functionality required by a Worker to access and watch
+// a cloud credential that a model uses.
+type Facade interface {
+
+	// ModelCredential gets model's cloud credential.
+	// Models that are on the clouds that do not require auth will return
+	// false to signify that credential was not set.
+	ModelCredential() (base.StoredCredential, bool, error)
+
+	// WatchCredential gets cloud credential watcher.
+	WatchCredential(string) (watcher.NotifyWatcher, error)
+}
+
+// IsValid returns true when the given credential is valid.
+func IsValid(c base.StoredCredential) bool {
+	return c.Valid
+}
+
+// Config holds the dependencies and configuration for a Worker.
+type Config struct {
+	Facade Facade
+	Check  Predicate
+}
+
+// Validate returns an error if the config cannot be expected to
+// drive a functional Worker.
+func (config Config) Validate() error {
+	if config.Facade == nil {
+		return errors.NotValidf("nil Facade")
+	}
+	if config.Check == nil {
+		return errors.NotValidf("nil Check")
+	}
+	return nil
+}
+
+// New returns a Worker that tracks the result of the configured
+// Check on the Model's cloud credential, as exposed by the Facade.
+func New(config Config) (*Worker, error) {
+	if err := config.Validate(); err != nil {
+		return nil, errors.Trace(err)
+	}
+	modelCredential, exists, err := config.Facade.ModelCredential()
+	if err != nil {
+		return nil, errors.Trace(err)
+	}
+	if !exists {
+		return nil, ErrModelDoesNotNeedCredential
+	}
+	watcher, err := config.Facade.WatchCredential(modelCredential.CloudCredential)
+	if err != nil {
+		return nil, errors.Trace(err)
+	}
+
+	w := &Worker{
+		config:          config,
+		watcher:         watcher,
+		modelCredential: modelCredential,
+	}
+
+	if err := catacomb.Invoke(catacomb.Plan{
+		Site: &w.catacomb,
+		Init: []worker.Worker{watcher},
+		Work: w.loop,
+	}); err != nil {
+		return nil, errors.Trace(err)
+	}
+	return w, nil
+}
+
+// Worker implements worker.Worker and util.Flag, and exits
+// with ErrChanged whenever the result of its configured Check of
+// the Model's cloud credential changes.
+type Worker struct {
+	catacomb        catacomb.Catacomb
+	config          Config
+	watcher         watcher.NotifyWatcher
+	modelCredential base.StoredCredential
+}
+
+// Kill is part of the worker.Worker interface.
+func (w *Worker) Kill() {
+	w.catacomb.Kill(nil)
+}
+
+// Wait is part of the worker.Worker interface.
+func (w *Worker) Wait() error {
+	return w.catacomb.Wait()
+}
+
+// Check is part of the util.Flag interface.
+func (w *Worker) Check() bool {
+	return w.config.Check(w.modelCredential)
+}
+
+func (w *Worker) loop() error {
+	for {
+		select {
+		case <-w.catacomb.Dying():
+			return w.catacomb.ErrDying()
+		case _, ok := <-w.watcher.Changes():
+			if !ok {
+				return w.catacomb.ErrDying()
+			}
+			mc, exists, err := w.config.Facade.ModelCredential()
+			if err != nil {
+				return errors.Trace(err)
+			}
+			if !exists {
+				// Really should not happen in practice since the uninstall
+				// should have occurred back in the constructor.
+				// If the credential came back as not set here, it could be a bigger problem:
+				// the model must have had a credential at some stage and now it's removed.
+				return ErrModelDoesNotNeedCredential
+			}
+			if mc.CloudCredential != w.modelCredential.CloudCredential {
+				// Model is now using different credential than when this worker was created.
+				// TODO (anastasiamac 2018-04-05) - It cannot happen yet
+				// but when it can, make sure that this worker still behaves...
+				// Also consider - is it appropriate to bounce the worker in that case
+				// or just change it's variables to reflect new credential?
+				return ErrModelCredentialChanged
+			}
+			if w.Check() != w.config.Check(mc) {
+				return ErrChanged
+			}
+		}
+	}
+}
diff --git a/worker/credentialvalidator/worker_test.go b/worker/credentialvalidator/worker_test.go
new file mode 100644
index 00000000000..510640bd44c
--- /dev/null
+++ b/worker/credentialvalidator/worker_test.go
@@ -0,0 +1,165 @@
+// Copyright 2018 Canonical Ltd.
+// Licensed under the LGPLv3, see LICENCE file for details.
+
+package credentialvalidator_test
+
+import (
+	"github.com/juju/errors"
+	"github.com/juju/testing"
+	jc "github.com/juju/testing/checkers"
+	gc "gopkg.in/check.v1"
+	"gopkg.in/juju/names.v2"
+
+	"github.com/juju/juju/api/base"
+	"github.com/juju/juju/worker/credentialvalidator"
+	"github.com/juju/juju/worker/workertest"
+)
+
+type WorkerSuite struct {
+	testing.IsolationSuite
+
+	facade   *mockFacade
+	modelTag names.ModelTag
+}
+
+var _ = gc.Suite(&WorkerSuite{})
+
+func (s *WorkerSuite) SetUpTest(c *gc.C) {
+	s.IsolationSuite.SetUpTest(c)
+
+	s.facade = &mockFacade{
+		Stub: &testing.Stub{},
+		credentials: []base.StoredCredential{
+			{credentialTag, true},
+		},
+		exists: true,
+	}
+	s.modelTag = names.NewModelTag(modelUUID)
+}
+
+func (s *WorkerSuite) TestCredentialValidityPanicOnStartup(c *gc.C) {
+	s.facade.SetErrors(errors.New("gaah"))
+	config := credentialvalidator.Config{
+		Facade: s.facade,
+		Check:  panicCheck,
+	}
+	worker, err := credentialvalidator.New(config)
+	c.Check(worker, gc.IsNil)
+	c.Check(err, gc.ErrorMatches, "gaah")
+	s.facade.CheckCallNames(c, "ModelCredential")
+}
+
+func (s *WorkerSuite) TestWatchError(c *gc.C) {
+	s.facade.SetErrors(nil, errors.New("boff"))
+	config := credentialvalidator.Config{
+		Facade: s.facade,
+		Check:  neverCheck,
+	}
+	worker, err := credentialvalidator.New(config)
+	c.Check(worker, gc.IsNil)
+	c.Check(err, gc.ErrorMatches, "boff")
+	s.facade.CheckCallNames(c, "ModelCredential", "WatchCredential")
+}
+
+func (s *WorkerSuite) TestModelCredentialErrorWhileRunning(c *gc.C) {
+	s.facade.SetErrors(nil, nil, errors.New("glug"))
+	config := credentialvalidator.Config{
+		Facade: s.facade,
+		Check:  neverCheck,
+	}
+	worker, err := credentialvalidator.New(config)
+	c.Assert(err, jc.ErrorIsNil)
+	c.Check(worker.Check(), jc.IsFalse)
+
+	err = workertest.CheckKilled(c, worker)
+	c.Check(err, gc.ErrorMatches, "glug")
+	s.facade.CheckCallNames(c, "ModelCredential", "WatchCredential", "ModelCredential")
+}
+
+func (s *WorkerSuite) TestModelCredentialNotNeeded(c *gc.C) {
+	s.facade.exists = false
+	config := credentialvalidator.Config{
+		Facade: s.facade,
+		Check:  neverCheck,
+	}
+	worker, err := credentialvalidator.New(config)
+	c.Assert(err, gc.ErrorMatches, "model is on the cloud that does not need auth")
+	c.Assert(worker, gc.IsNil)
+	s.facade.CheckCallNames(c, "ModelCredential")
+}
+
+func (s *WorkerSuite) TestCredentialChangeToInvalid(c *gc.C) {
+	s.facade.credentials = []base.StoredCredential{
+		{credentialTag, true},
+		{credentialTag, false},
+	}
+
+	config := credentialvalidator.Config{
+		Facade: s.facade,
+		Check:  credentialvalidator.IsValid,
+	}
+	worker, err := credentialvalidator.New(config)
+	c.Assert(err, jc.ErrorIsNil)
+	c.Check(worker.Check(), jc.IsTrue)
+
+	err = workertest.CheckKilled(c, worker)
+	c.Check(err, gc.Equals, credentialvalidator.ErrChanged)
+	s.facade.CheckCallNames(c, "ModelCredential", "WatchCredential", "ModelCredential")
+}
+
+func (s *WorkerSuite) TestCredentialChangeFromInvalid(c *gc.C) {
+	s.facade.credentials = []base.StoredCredential{
+		{credentialTag, false},
+		{credentialTag, true},
+	}
+
+	config := credentialvalidator.Config{
+		Facade: s.facade,
+		Check:  credentialvalidator.IsValid,
+	}
+	worker, err := credentialvalidator.New(config)
+	c.Assert(err, jc.ErrorIsNil)
+	c.Check(worker.Check(), jc.IsFalse)
+
+	err = workertest.CheckKilled(c, worker)
+	c.Check(err, gc.Equals, credentialvalidator.ErrChanged)
+	s.facade.CheckCallNames(c, "ModelCredential", "WatchCredential", "ModelCredential")
+}
+
+func (s *WorkerSuite) TestModelCredentialReplaced(c *gc.C) {
+	s.facade.credentials = []base.StoredCredential{
+		{credentialTag, true},
+		{names.NewCloudCredentialTag("such/different/credential").String(), false},
+	}
+	config := credentialvalidator.Config{
+		Facade: s.facade,
+		Check:  credentialvalidator.IsValid,
+	}
+	worker, err := credentialvalidator.New(config)
+	c.Assert(err, jc.ErrorIsNil)
+	c.Check(worker.Check(), jc.IsTrue)
+
+	err = workertest.CheckKilled(c, worker)
+	c.Check(err, gc.Equals, credentialvalidator.ErrModelCredentialChanged)
+	s.facade.CheckCallNames(c, "ModelCredential", "WatchCredential", "ModelCredential")
+}
+
+func (s *WorkerSuite) TestNoRelevantCredentialChange(c *gc.C) {
+	s.facade.credentials = []base.StoredCredential{
+		{credentialTag, true},
+		{credentialTag, true},
+		{credentialTag, true},
+		{credentialTag, true},
+	}
+	config := credentialvalidator.Config{
+		Facade: s.facade,
+		Check:  credentialvalidator.IsValid,
+	}
+	worker, err := credentialvalidator.New(config)
+	c.Assert(err, jc.ErrorIsNil)
+	c.Check(worker.Check(), jc.IsTrue)
+
+	workertest.CheckAlive(c, worker)
+	workertest.CleanKill(c, worker)
+	s.facade.CheckCallNames(c, "ModelCredential", "WatchCredential", "ModelCredential", "ModelCredential", "ModelCredential")
+}

From 42f8034289743ec75f30e3f26c36d406c8d35ab5 Mon Sep 17 00:00:00 2001
From: anastasia <anastasia.macmood@canonical.com>
Date: Fri, 13 Apr 2018 11:17:53 +1000
Subject: [PATCH 2/6] review comments.

---
 .../agent/credentialvalidator/credentialvalidator.go       | 7 ++-----
 worker/credentialvalidator/manifold.go                     | 2 +-
 worker/credentialvalidator/manifold_test.go                | 2 +-
 worker/credentialvalidator/util_test.go                    | 6 ++----
 worker/credentialvalidator/worker.go                       | 3 ++-
 worker/credentialvalidator/worker_test.go                  | 4 +---
 6 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/apiserver/facades/agent/credentialvalidator/credentialvalidator.go b/apiserver/facades/agent/credentialvalidator/credentialvalidator.go
index 5b35f1bcbc5..ac0025cf1f4 100644
--- a/apiserver/facades/agent/credentialvalidator/credentialvalidator.go
+++ b/apiserver/facades/agent/credentialvalidator/credentialvalidator.go
@@ -18,7 +18,6 @@ var logger = loggo.GetLogger("juju.api.credentialvalidator")
 type CredentialValidator interface {
 	ModelCredential() (params.ModelCredential, error)
 	WatchCredential(params.Entity) (params.NotifyWatchResult, error)
-	//WatchCredential(string) (params.NotifyWatchResult, error)
 }
 
 type CredentialValidatorAPI struct {
@@ -45,10 +44,8 @@ func internalNewCredentialValidatorAPI(backend Backend, resources facade.Resourc
 	}, nil
 }
 
-// WatchCredential returns a collection of NotifyWatchers that observe
-// changes to the given cloud credentials.
-// The order of returned watchers is important and corresponds directly to the
-// order of supplied cloud credentials collection.
+// WatchCredential returns a NotifyWatcher that observes
+// changes to a given cloud credential.
 func (api *CredentialValidatorAPI) WatchCredential(tag params.Entity) (params.NotifyWatchResult, error) {
 	fail := func(failure error) (params.NotifyWatchResult, error) {
 		return params.NotifyWatchResult{}, common.ServerError(failure)
diff --git a/worker/credentialvalidator/manifold.go b/worker/credentialvalidator/manifold.go
index 96e5d2cca8d..bf76ffa40d8 100644
--- a/worker/credentialvalidator/manifold.go
+++ b/worker/credentialvalidator/manifold.go
@@ -5,7 +5,7 @@ package credentialvalidator
 
 import (
 	"github.com/juju/errors"
-	worker "gopkg.in/juju/worker.v1"
+	"gopkg.in/juju/worker.v1"
 
 	"github.com/juju/juju/api/base"
 	"github.com/juju/juju/cmd/jujud/agent/engine"
diff --git a/worker/credentialvalidator/manifold_test.go b/worker/credentialvalidator/manifold_test.go
index 02a68a5a140..0f1553bfa37 100644
--- a/worker/credentialvalidator/manifold_test.go
+++ b/worker/credentialvalidator/manifold_test.go
@@ -8,7 +8,7 @@ import (
 	"github.com/juju/testing"
 	jc "github.com/juju/testing/checkers"
 	gc "gopkg.in/check.v1"
-	worker "gopkg.in/juju/worker.v1"
+	"gopkg.in/juju/worker.v1"
 
 	"github.com/juju/juju/api/base"
 	"github.com/juju/juju/cmd/jujud/agent/engine"
diff --git a/worker/credentialvalidator/util_test.go b/worker/credentialvalidator/util_test.go
index 7d17695a308..640499496b6 100644
--- a/worker/credentialvalidator/util_test.go
+++ b/worker/credentialvalidator/util_test.go
@@ -12,6 +12,7 @@ import (
 	worker "gopkg.in/juju/worker.v1"
 
 	"github.com/juju/juju/api/base"
+	coretesting "github.com/juju/juju/testing"
 	"github.com/juju/juju/watcher"
 	"github.com/juju/juju/worker/credentialvalidator"
 	dt "github.com/juju/juju/worker/dependency/testing"
@@ -75,9 +76,6 @@ func (mock *mockWatcher) Changes() watcher.NotifyChannel {
 	return mock.changes
 }
 
-// modelUUID is the model tag we're using in the tests.
-var modelUUID = "01234567-89ab-cdef-0123-456789abcdef"
-
 // credentialTag is the credential tag we're using in the tests.
 // needs to fit fmt.Sprintf("%s/%s/%s", cloudName, userName, credentialName)
 var credentialTag = names.NewCloudCredentialTag("cloud/user/credential").String()
@@ -151,5 +149,5 @@ type stubCaller struct {
 
 // ModelTag is part of the base.APICaller interface.
 func (*stubCaller) ModelTag() (names.ModelTag, bool) {
-	return names.NewModelTag(modelUUID), true
+	return coretesting.ModelTag, true
 }
diff --git a/worker/credentialvalidator/worker.go b/worker/credentialvalidator/worker.go
index 53d0442043e..5b8b567d415 100644
--- a/worker/credentialvalidator/worker.go
+++ b/worker/credentialvalidator/worker.go
@@ -21,7 +21,7 @@ var ErrChanged = errors.New("cloud credential validity has changed")
 
 // ErrModelCredentialChanged indicates that a Worker has bounced
 // because model credential was replaced.
-var ErrModelCredentialChanged = errors.New("unexpected model credential")
+var ErrModelCredentialChanged = errors.New("model credential changed")
 
 // ErrModelDoesNotNeedCredential indicates that a Worker has been uninstalled
 // since the model does not have a cloud credential set as the model is
@@ -144,6 +144,7 @@ func (w *Worker) loop() error {
 				// should have occurred back in the constructor.
 				// If the credential came back as not set here, it could be a bigger problem:
 				// the model must have had a credential at some stage and now it's removed.
+				logger.Warningf("model credential was unexpectedly unset for the model that resides on the cloud that requires auth")
 				return ErrModelDoesNotNeedCredential
 			}
 			if mc.CloudCredential != w.modelCredential.CloudCredential {
diff --git a/worker/credentialvalidator/worker_test.go b/worker/credentialvalidator/worker_test.go
index 510640bd44c..8591d7ef215 100644
--- a/worker/credentialvalidator/worker_test.go
+++ b/worker/credentialvalidator/worker_test.go
@@ -18,8 +18,7 @@ import (
 type WorkerSuite struct {
 	testing.IsolationSuite
 
-	facade   *mockFacade
-	modelTag names.ModelTag
+	facade *mockFacade
 }
 
 var _ = gc.Suite(&WorkerSuite{})
@@ -34,7 +33,6 @@ func (s *WorkerSuite) SetUpTest(c *gc.C) {
 		},
 		exists: true,
 	}
-	s.modelTag = names.NewModelTag(modelUUID)
 }
 
 func (s *WorkerSuite) TestCredentialValidityPanicOnStartup(c *gc.C) {

From c0c9249c2affdab0c92f72a4bd8742c6a5277c6b Mon Sep 17 00:00:00 2001
From: anastasia <anastasia.macmood@canonical.com>
Date: Fri, 13 Apr 2018 14:28:30 +1000
Subject: [PATCH 3/6] Rename error to be more descriptive.

---
 worker/credentialvalidator/manifold.go      | 2 +-
 worker/credentialvalidator/manifold_test.go | 2 +-
 worker/credentialvalidator/worker.go        | 9 +++++----
 worker/credentialvalidator/worker_test.go   | 4 ++--
 4 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/worker/credentialvalidator/manifold.go b/worker/credentialvalidator/manifold.go
index bf76ffa40d8..a59b079fd8b 100644
--- a/worker/credentialvalidator/manifold.go
+++ b/worker/credentialvalidator/manifold.go
@@ -74,7 +74,7 @@ func Manifold(config ManifoldConfig) dependency.Manifold {
 
 func filterErrors(err error) error {
 	cause := errors.Cause(err)
-	if cause == ErrChanged || cause == ErrModelCredentialChanged {
+	if cause == ErrValidityChanged || cause == ErrModelCredentialChanged {
 		return dependency.ErrBounce
 	} else if cause == ErrModelDoesNotNeedCredential {
 		return dependency.ErrUninstall
diff --git a/worker/credentialvalidator/manifold_test.go b/worker/credentialvalidator/manifold_test.go
index 0f1553bfa37..5d421775c45 100644
--- a/worker/credentialvalidator/manifold_test.go
+++ b/worker/credentialvalidator/manifold_test.go
@@ -61,7 +61,7 @@ func (*ManifoldSuite) TestFilterNil(c *gc.C) {
 
 func (*ManifoldSuite) TestFilterErrChanged(c *gc.C) {
 	manifold := credentialvalidator.Manifold(credentialvalidator.ManifoldConfig{})
-	err := manifold.Filter(credentialvalidator.ErrChanged)
+	err := manifold.Filter(credentialvalidator.ErrValidityChanged)
 	c.Check(err, gc.Equals, dependency.ErrBounce)
 }
 
diff --git a/worker/credentialvalidator/worker.go b/worker/credentialvalidator/worker.go
index 5b8b567d415..e0772c738f8 100644
--- a/worker/credentialvalidator/worker.go
+++ b/worker/credentialvalidator/worker.go
@@ -15,9 +15,10 @@ import (
 
 var logger = loggo.GetLogger("juju.api.credentialvalidator")
 
-// ErrChanged indicates that a Worker has bounced because its
-// Check result has changed.
-var ErrChanged = errors.New("cloud credential validity has changed")
+// ErrValidityChanged indicates that a Worker has bounced because its
+// credential validity has changed: either a valid credential became invalid
+// or invalid credential became valid.
+var ErrValidityChanged = errors.New("cloud credential validity has changed")
 
 // ErrModelCredentialChanged indicates that a Worker has bounced
 // because model credential was replaced.
@@ -156,7 +157,7 @@ func (w *Worker) loop() error {
 				return ErrModelCredentialChanged
 			}
 			if w.Check() != w.config.Check(mc) {
-				return ErrChanged
+				return ErrValidityChanged
 			}
 		}
 	}
diff --git a/worker/credentialvalidator/worker_test.go b/worker/credentialvalidator/worker_test.go
index 8591d7ef215..18904eaf051 100644
--- a/worker/credentialvalidator/worker_test.go
+++ b/worker/credentialvalidator/worker_test.go
@@ -101,7 +101,7 @@ func (s *WorkerSuite) TestCredentialChangeToInvalid(c *gc.C) {
 	c.Check(worker.Check(), jc.IsTrue)
 
 	err = workertest.CheckKilled(c, worker)
-	c.Check(err, gc.Equals, credentialvalidator.ErrChanged)
+	c.Check(err, gc.Equals, credentialvalidator.ErrValidityChanged)
 	s.facade.CheckCallNames(c, "ModelCredential", "WatchCredential", "ModelCredential")
 }
 
@@ -120,7 +120,7 @@ func (s *WorkerSuite) TestCredentialChangeFromInvalid(c *gc.C) {
 	c.Check(worker.Check(), jc.IsFalse)
 
 	err = workertest.CheckKilled(c, worker)
-	c.Check(err, gc.Equals, credentialvalidator.ErrChanged)
+	c.Check(err, gc.Equals, credentialvalidator.ErrValidityChanged)
 	s.facade.CheckCallNames(c, "ModelCredential", "WatchCredential", "ModelCredential")
 }
 

From db22ec4946a4e8be459c5faeefaccd6e38678200 Mon Sep 17 00:00:00 2001
From: anastasia <anastasia.macmood@canonical.com>
Date: Fri, 13 Apr 2018 14:30:46 +1000
Subject: [PATCH 4/6] Change licence to AGPL

---
 .../facades/agent/credentialvalidator/credentialvalidator.go    | 1 +
 worker/credentialvalidator/manifold_test.go                     | 2 +-
 worker/credentialvalidator/package_test.go                      | 2 +-
 worker/credentialvalidator/util_test.go                         | 2 +-
 worker/credentialvalidator/validate_test.go                     | 2 +-
 worker/credentialvalidator/worker_test.go                       | 2 +-
 6 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/apiserver/facades/agent/credentialvalidator/credentialvalidator.go b/apiserver/facades/agent/credentialvalidator/credentialvalidator.go
index ac0025cf1f4..fcde5705a4f 100644
--- a/apiserver/facades/agent/credentialvalidator/credentialvalidator.go
+++ b/apiserver/facades/agent/credentialvalidator/credentialvalidator.go
@@ -1,5 +1,6 @@
 // Copyright 2018 Canonical Ltd.
 // Licensed under the AGPLv3, see LICENCE file for details.
+
 package credentialvalidator
 
 import (
diff --git a/worker/credentialvalidator/manifold_test.go b/worker/credentialvalidator/manifold_test.go
index 5d421775c45..1320d91fe71 100644
--- a/worker/credentialvalidator/manifold_test.go
+++ b/worker/credentialvalidator/manifold_test.go
@@ -1,5 +1,5 @@
 // Copyright 2018 Canonical Ltd.
-// Licensed under the LGPLv3, see LICENCE file for details.
+// Licensed under the AGPLv3, see LICENCE file for details.
 
 package credentialvalidator_test
 
diff --git a/worker/credentialvalidator/package_test.go b/worker/credentialvalidator/package_test.go
index 467bd538a96..7c51a5304eb 100644
--- a/worker/credentialvalidator/package_test.go
+++ b/worker/credentialvalidator/package_test.go
@@ -1,5 +1,5 @@
 // Copyright 2018 Canonical Ltd.
-// Licensed under the LGPLv3, see LICENCE file for details.
+// Licensed under the AGPLv3, see LICENCE file for details.
 
 package credentialvalidator_test
 
diff --git a/worker/credentialvalidator/util_test.go b/worker/credentialvalidator/util_test.go
index 640499496b6..0310854b67b 100644
--- a/worker/credentialvalidator/util_test.go
+++ b/worker/credentialvalidator/util_test.go
@@ -1,5 +1,5 @@
 // Copyright 2018 Canonical Ltd.
-// Licensed under the LGPLv3, see LICENCE file for details.
+// Licensed under the AGPLv3, see LICENCE file for details.
 
 package credentialvalidator_test
 
diff --git a/worker/credentialvalidator/validate_test.go b/worker/credentialvalidator/validate_test.go
index 3a6fb8ee6b9..2d7e1e1f15d 100644
--- a/worker/credentialvalidator/validate_test.go
+++ b/worker/credentialvalidator/validate_test.go
@@ -1,5 +1,5 @@
 // Copyright 2018 Canonical Ltd.
-// Licensed under the LGPLv3, see LICENCE file for details.
+// Licensed under the AGPLv3, see LICENCE file for details.
 
 package credentialvalidator_test
 
diff --git a/worker/credentialvalidator/worker_test.go b/worker/credentialvalidator/worker_test.go
index 18904eaf051..40960443c4e 100644
--- a/worker/credentialvalidator/worker_test.go
+++ b/worker/credentialvalidator/worker_test.go
@@ -1,5 +1,5 @@
 // Copyright 2018 Canonical Ltd.
-// Licensed under the LGPLv3, see LICENCE file for details.
+// Licensed under the AGPLv3, see LICENCE file for details.
 
 package credentialvalidator_test
 

From 6aca7fa4f65945c400f81c9f8789abbced750b5f Mon Sep 17 00:00:00 2001
From: anastasia <anastasia.macmood@canonical.com>
Date: Fri, 13 Apr 2018 14:41:56 +1000
Subject: [PATCH 5/6] export config.validate()

---
 worker/credentialvalidator/manifold.go  | 6 +++---
 worker/credentialvalidator/util_test.go | 5 +----
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/worker/credentialvalidator/manifold.go b/worker/credentialvalidator/manifold.go
index a59b079fd8b..59abb324932 100644
--- a/worker/credentialvalidator/manifold.go
+++ b/worker/credentialvalidator/manifold.go
@@ -22,8 +22,8 @@ type ManifoldConfig struct {
 	NewWorker func(Config) (worker.Worker, error)
 }
 
-// validate is called by start to check for bad configuration.
-func (config ManifoldConfig) validate() error {
+// Validate is called by start to check for bad configuration.
+func (config ManifoldConfig) Validate() error {
 	if config.APICallerName == "" {
 		return errors.NotValidf("empty APICallerName")
 	}
@@ -41,7 +41,7 @@ func (config ManifoldConfig) validate() error {
 
 // start is a StartFunc for a Worker manifold.
 func (config ManifoldConfig) start(context dependency.Context) (worker.Worker, error) {
-	if err := config.validate(); err != nil {
+	if err := config.Validate(); err != nil {
 		return nil, errors.Trace(err)
 	}
 	var apiCaller base.APICaller
diff --git a/worker/credentialvalidator/util_test.go b/worker/credentialvalidator/util_test.go
index 0310854b67b..377fc7244db 100644
--- a/worker/credentialvalidator/util_test.go
+++ b/worker/credentialvalidator/util_test.go
@@ -15,7 +15,6 @@ import (
 	coretesting "github.com/juju/juju/testing"
 	"github.com/juju/juju/watcher"
 	"github.com/juju/juju/worker/credentialvalidator"
-	dt "github.com/juju/juju/worker/dependency/testing"
 	"github.com/juju/juju/worker/workertest"
 )
 
@@ -135,9 +134,7 @@ func validManifoldConfig() credentialvalidator.ManifoldConfig {
 // checkManifoldNotValid checks that the supplied ManifoldConfig creates
 // a manifold that cannot be started.
 func checkManifoldNotValid(c *gc.C, config credentialvalidator.ManifoldConfig, expect string) {
-	manifold := credentialvalidator.Manifold(config)
-	worker, err := manifold.Start(dt.StubContext(nil, nil))
-	c.Check(worker, gc.IsNil)
+	err := config.Validate()
 	c.Check(err, gc.ErrorMatches, expect)
 	c.Check(err, jc.Satisfies, errors.IsNotValid)
 }

From d4afb173b73835eb1d2fb0d84bf5483992209130 Mon Sep 17 00:00:00 2001
From: anastasia <anastasia.macmood@canonical.com>
Date: Fri, 13 Apr 2018 16:03:27 +1000
Subject: [PATCH 6/6] remove chck from config and generally restructure based
 on reviews

---
 cmd/jujud/agent/model/manifolds.go          |   1 -
 worker/credentialvalidator/manifold.go      |   5 -
 worker/credentialvalidator/manifold_test.go |  24 -----
 worker/credentialvalidator/shim.go          |  12 ---
 worker/credentialvalidator/util_test.go     |  10 +-
 worker/credentialvalidator/validate_test.go |   6 --
 worker/credentialvalidator/worker.go        | 112 ++++++++------------
 worker/credentialvalidator/worker_test.go   |  84 +++++----------
 8 files changed, 74 insertions(+), 180 deletions(-)

diff --git a/cmd/jujud/agent/model/manifolds.go b/cmd/jujud/agent/model/manifolds.go
index 41b6b6dec64..8e6174aa80c 100644
--- a/cmd/jujud/agent/model/manifolds.go
+++ b/cmd/jujud/agent/model/manifolds.go
@@ -175,7 +175,6 @@ func commonManifolds(config ManifoldsConfig) dependency.Manifolds {
 		// determines if model's cloud credential is valid.
 		credentialValidatorFlagName: ifNotUpgrading(ifNotDead(credentialvalidator.Manifold(credentialvalidator.ManifoldConfig{
 			APICallerName: apiCallerName,
-			Check:         credentialvalidator.IsValid,
 			NewFacade:     credentialvalidator.NewFacade,
 			NewWorker:     credentialvalidator.NewWorker,
 		}))),
diff --git a/worker/credentialvalidator/manifold.go b/worker/credentialvalidator/manifold.go
index 59abb324932..33c1b1b52fc 100644
--- a/worker/credentialvalidator/manifold.go
+++ b/worker/credentialvalidator/manifold.go
@@ -16,7 +16,6 @@ import (
 // Worker manifold.
 type ManifoldConfig struct {
 	APICallerName string
-	Check         Predicate
 
 	NewFacade func(base.APICaller) (Facade, error)
 	NewWorker func(Config) (worker.Worker, error)
@@ -27,9 +26,6 @@ func (config ManifoldConfig) Validate() error {
 	if config.APICallerName == "" {
 		return errors.NotValidf("empty APICallerName")
 	}
-	if config.Check == nil {
-		return errors.NotValidf("nil Check")
-	}
 	if config.NewFacade == nil {
 		return errors.NotValidf("nil NewFacade")
 	}
@@ -54,7 +50,6 @@ func (config ManifoldConfig) start(context dependency.Context) (worker.Worker, e
 	}
 	worker, err := config.NewWorker(Config{
 		Facade: facade,
-		Check:  config.Check,
 	})
 	if err != nil {
 		return nil, errors.Trace(err)
diff --git a/worker/credentialvalidator/manifold_test.go b/worker/credentialvalidator/manifold_test.go
index 1320d91fe71..c2a564c8584 100644
--- a/worker/credentialvalidator/manifold_test.go
+++ b/worker/credentialvalidator/manifold_test.go
@@ -36,23 +36,6 @@ func (*ManifoldSuite) TestOutputBadWorker(c *gc.C) {
 	c.Check(err, gc.ErrorMatches, "expected in to implement Flag; got a .*")
 }
 
-func (*ManifoldSuite) TestOutputBadTarget(c *gc.C) {
-	manifold := credentialvalidator.Manifold(credentialvalidator.ManifoldConfig{})
-	in := &credentialvalidator.Worker{}
-	var out bool
-	err := manifold.Output(in, &out)
-	c.Check(err, gc.ErrorMatches, "expected out to be a \\*Flag; got a .*")
-}
-
-func (*ManifoldSuite) TestOutputBadInput(c *gc.C) {
-	manifold := credentialvalidator.Manifold(credentialvalidator.ManifoldConfig{})
-	in := &credentialvalidator.Worker{}
-	var out engine.Flag
-	err := manifold.Output(in, &out)
-	c.Check(err, jc.ErrorIsNil)
-	c.Check(out, gc.Equals, in)
-}
-
 func (*ManifoldSuite) TestFilterNil(c *gc.C) {
 	manifold := credentialvalidator.Manifold(credentialvalidator.ManifoldConfig{})
 	err := manifold.Filter(nil)
@@ -90,12 +73,6 @@ func (*ManifoldSuite) TestStartMissingAPICallerName(c *gc.C) {
 	checkManifoldNotValid(c, config, "empty APICallerName not valid")
 }
 
-func (*ManifoldSuite) TestStartMissingCheck(c *gc.C) {
-	config := validManifoldConfig()
-	config.Check = nil
-	checkManifoldNotValid(c, config, "nil Check not valid")
-}
-
 func (*ManifoldSuite) TestStartMissingNewFacade(c *gc.C) {
 	config := validManifoldConfig()
 	config.NewFacade = nil
@@ -147,7 +124,6 @@ func (*ManifoldSuite) TestStartNewWorkerError(c *gc.C) {
 	}
 	config.NewWorker = func(workerConfig credentialvalidator.Config) (worker.Worker, error) {
 		c.Check(workerConfig.Facade, gc.Equals, expectFacade)
-		c.Check(workerConfig.Check, gc.NotNil) // uncomparable
 		return nil, errors.New("snerk")
 	}
 	manifold := credentialvalidator.Manifold(config)
diff --git a/worker/credentialvalidator/shim.go b/worker/credentialvalidator/shim.go
index a1d58143fcf..c795cdc1384 100644
--- a/worker/credentialvalidator/shim.go
+++ b/worker/credentialvalidator/shim.go
@@ -4,9 +4,6 @@
 package credentialvalidator
 
 import (
-	"github.com/juju/errors"
-	worker "gopkg.in/juju/worker.v1"
-
 	"github.com/juju/juju/api/base"
 	"github.com/juju/juju/api/credentialvalidator"
 )
@@ -16,12 +13,3 @@ func NewFacade(apiCaller base.APICaller) (Facade, error) {
 	facade := credentialvalidator.NewFacade(apiCaller)
 	return facade, nil
 }
-
-// NewWorker creates a *Worker and returns it as a worker.Worker.
-func NewWorker(config Config) (worker.Worker, error) {
-	worker, err := New(config)
-	if err != nil {
-		return nil, errors.Trace(err)
-	}
-	return worker, nil
-}
diff --git a/worker/credentialvalidator/util_test.go b/worker/credentialvalidator/util_test.go
index 377fc7244db..d78a708e920 100644
--- a/worker/credentialvalidator/util_test.go
+++ b/worker/credentialvalidator/util_test.go
@@ -79,12 +79,6 @@ func (mock *mockWatcher) Changes() watcher.NotifyChannel {
 // needs to fit fmt.Sprintf("%s/%s/%s", cloudName, userName, credentialName)
 var credentialTag = names.NewCloudCredentialTag("cloud/user/credential").String()
 
-// panicCheck is a Config.Check value that should not be called.
-func panicCheck(base.StoredCredential) bool { panic("unexpected") }
-
-// neverCheck is a Config.Check value that always returns false.
-func neverCheck(base.StoredCredential) bool { return false }
-
 // panicFacade is a NewFacade that should not be called.
 func panicFacade(base.APICaller) (credentialvalidator.Facade, error) {
 	panic("panicFacade")
@@ -100,7 +94,6 @@ func panicWorker(credentialvalidator.Config) (worker.Worker, error) {
 func validConfig() credentialvalidator.Config {
 	return credentialvalidator.Config{
 		Facade: struct{ credentialvalidator.Facade }{},
-		Check:  panicCheck,
 	}
 }
 
@@ -115,7 +108,7 @@ func checkNotValid(c *gc.C, config credentialvalidator.Config, expect string) {
 	err := config.Validate()
 	check(err)
 
-	worker, err := credentialvalidator.New(config)
+	worker, err := credentialvalidator.NewWorker(config)
 	c.Check(worker, gc.IsNil)
 	check(err)
 }
@@ -125,7 +118,6 @@ func checkNotValid(c *gc.C, config credentialvalidator.Config, expect string) {
 func validManifoldConfig() credentialvalidator.ManifoldConfig {
 	return credentialvalidator.ManifoldConfig{
 		APICallerName: "api-caller",
-		Check:         panicCheck,
 		NewFacade:     panicFacade,
 		NewWorker:     panicWorker,
 	}
diff --git a/worker/credentialvalidator/validate_test.go b/worker/credentialvalidator/validate_test.go
index 2d7e1e1f15d..552005c7574 100644
--- a/worker/credentialvalidator/validate_test.go
+++ b/worker/credentialvalidator/validate_test.go
@@ -26,9 +26,3 @@ func (*ValidateSuite) TestNilFacade(c *gc.C) {
 	config.Facade = nil
 	checkNotValid(c, config, "nil Facade not valid")
 }
-
-func (*ValidateSuite) TestNilCheck(c *gc.C) {
-	config := validConfig()
-	config.Check = nil
-	checkNotValid(c, config, "nil Check not valid")
-}
diff --git a/worker/credentialvalidator/worker.go b/worker/credentialvalidator/worker.go
index e0772c738f8..396f9659990 100644
--- a/worker/credentialvalidator/worker.go
+++ b/worker/credentialvalidator/worker.go
@@ -29,9 +29,6 @@ var ErrModelCredentialChanged = errors.New("model credential changed")
 // on the cloud that does not require authentication.
 var ErrModelDoesNotNeedCredential = errors.New("model is on the cloud that does not need auth")
 
-// Predicate defines a predicate.
-type Predicate func(base.StoredCredential) bool
-
 // Facade exposes functionality required by a Worker to access and watch
 // a cloud credential that a model uses.
 type Facade interface {
@@ -45,15 +42,9 @@ type Facade interface {
 	WatchCredential(string) (watcher.NotifyWatcher, error)
 }
 
-// IsValid returns true when the given credential is valid.
-func IsValid(c base.StoredCredential) bool {
-	return c.Valid
-}
-
 // Config holds the dependencies and configuration for a Worker.
 type Config struct {
 	Facade Facade
-	Check  Predicate
 }
 
 // Validate returns an error if the config cannot be expected to
@@ -62,93 +53,78 @@ func (config Config) Validate() error {
 	if config.Facade == nil {
 		return errors.NotValidf("nil Facade")
 	}
-	if config.Check == nil {
-		return errors.NotValidf("nil Check")
-	}
 	return nil
 }
 
-// New returns a Worker that tracks the result of the configured
-// Check on the Model's cloud credential, as exposed by the Facade.
-func New(config Config) (*Worker, error) {
+// NewWorker returns a Worker that tracks the validity of the Model's cloud
+// credential, as exposed by the Facade.
+func NewWorker(config Config) (worker.Worker, error) {
 	if err := config.Validate(); err != nil {
 		return nil, errors.Trace(err)
 	}
-	modelCredential, exists, err := config.Facade.ModelCredential()
-	if err != nil {
-		return nil, errors.Trace(err)
-	}
-	if !exists {
-		return nil, ErrModelDoesNotNeedCredential
-	}
-	watcher, err := config.Facade.WatchCredential(modelCredential.CloudCredential)
-	if err != nil {
-		return nil, errors.Trace(err)
-	}
 
-	w := &Worker{
-		config:          config,
-		watcher:         watcher,
-		modelCredential: modelCredential,
-	}
+	v := &validator{validatorFacade: config.Facade}
 
-	if err := catacomb.Invoke(catacomb.Plan{
-		Site: &w.catacomb,
-		Init: []worker.Worker{watcher},
-		Work: w.loop,
-	}); err != nil {
+	err := catacomb.Invoke(catacomb.Plan{
+		Site: &v.catacomb,
+		Work: v.loop,
+	})
+	if err != nil {
 		return nil, errors.Trace(err)
 	}
-	return w, nil
+	return v, nil
 }
 
-// Worker implements worker.Worker and util.Flag, and exits
-// with ErrChanged whenever the result of its configured Check of
-// the Model's cloud credential changes.
-type Worker struct {
+type validator struct {
 	catacomb        catacomb.Catacomb
-	config          Config
-	watcher         watcher.NotifyWatcher
-	modelCredential base.StoredCredential
+	validatorFacade Facade
 }
 
 // Kill is part of the worker.Worker interface.
-func (w *Worker) Kill() {
-	w.catacomb.Kill(nil)
+func (v *validator) Kill() {
+	v.catacomb.Kill(nil)
 }
 
 // Wait is part of the worker.Worker interface.
-func (w *Worker) Wait() error {
-	return w.catacomb.Wait()
+func (v *validator) Wait() error {
+	return v.catacomb.Wait()
 }
 
-// Check is part of the util.Flag interface.
-func (w *Worker) Check() bool {
-	return w.config.Check(w.modelCredential)
-}
+func (v *validator) loop() error {
+	modelCredential := func() (base.StoredCredential, error) {
+		mc, exists, err := v.validatorFacade.ModelCredential()
+		if err != nil {
+			return base.StoredCredential{}, errors.Trace(err)
+		}
+		if !exists {
+			logger.Warningf("model credential is not set for the model")
+			return base.StoredCredential{}, ErrModelDoesNotNeedCredential
+		}
+		return mc, nil
+	}
 
-func (w *Worker) loop() error {
+	originalCredential, err := modelCredential()
+	if err != nil {
+		return errors.Trace(err)
+	}
+
+	w, err := v.validatorFacade.WatchCredential(originalCredential.CloudCredential)
+	if err != nil {
+		return errors.Trace(err)
+	}
 	for {
 		select {
-		case <-w.catacomb.Dying():
-			return w.catacomb.ErrDying()
-		case _, ok := <-w.watcher.Changes():
+		case <-v.catacomb.Dying():
+			return v.catacomb.ErrDying()
+		case _, ok := <-w.Changes():
 			if !ok {
-				return w.catacomb.ErrDying()
+				return v.catacomb.ErrDying()
 			}
-			mc, exists, err := w.config.Facade.ModelCredential()
+			updatedCredential, err := modelCredential()
 			if err != nil {
 				return errors.Trace(err)
 			}
-			if !exists {
-				// Really should not happen in practice since the uninstall
-				// should have occurred back in the constructor.
-				// If the credential came back as not set here, it could be a bigger problem:
-				// the model must have had a credential at some stage and now it's removed.
-				logger.Warningf("model credential was unexpectedly unset for the model that resides on the cloud that requires auth")
-				return ErrModelDoesNotNeedCredential
-			}
-			if mc.CloudCredential != w.modelCredential.CloudCredential {
+			if originalCredential.CloudCredential != updatedCredential.CloudCredential {
 				// Model is now using different credential than when this worker was created.
 				// TODO (anastasiamac 2018-04-05) - It cannot happen yet
 				// but when it can, make sure that this worker still behaves...
@@ -156,7 +132,7 @@ func (w *Worker) loop() error {
 				// or just change it's variables to reflect new credential?
 				return ErrModelCredentialChanged
 			}
-			if w.Check() != w.config.Check(mc) {
+			if originalCredential.Valid != updatedCredential.Valid {
 				return ErrValidityChanged
 			}
 		}
diff --git a/worker/credentialvalidator/worker_test.go b/worker/credentialvalidator/worker_test.go
index 40960443c4e..fe15994122d 100644
--- a/worker/credentialvalidator/worker_test.go
+++ b/worker/credentialvalidator/worker_test.go
@@ -19,6 +19,7 @@ type WorkerSuite struct {
 	testing.IsolationSuite
 
 	facade *mockFacade
+	config credentialvalidator.Config
 }
 
 var _ = gc.Suite(&WorkerSuite{})
@@ -33,56 +34,47 @@ func (s *WorkerSuite) SetUpTest(c *gc.C) {
 		},
 		exists: true,
 	}
-}
-
-func (s *WorkerSuite) TestCredentialValidityPanicOnStartup(c *gc.C) {
-	s.facade.SetErrors(errors.New("gaah"))
-	config := credentialvalidator.Config{
+	s.config = credentialvalidator.Config{
 		Facade: s.facade,
-		Check:  panicCheck,
 	}
-	worker, err := credentialvalidator.New(config)
-	c.Check(worker, gc.IsNil)
-	c.Check(err, gc.ErrorMatches, "gaah")
+}
+
+func (s *WorkerSuite) TestModelCredentialError(c *gc.C) {
+	s.facade.SetErrors(errors.New("mc fail"))
+
+	worker, err := testWorker(s.config)
+	c.Assert(err, jc.ErrorIsNil)
+	err = workertest.CheckKilled(c, worker)
+	c.Check(err, gc.ErrorMatches, "mc fail")
 	s.facade.CheckCallNames(c, "ModelCredential")
 }
 
 func (s *WorkerSuite) TestWatchError(c *gc.C) {
-	s.facade.SetErrors(nil, errors.New("boff"))
-	config := credentialvalidator.Config{
-		Facade: s.facade,
-		Check:  neverCheck,
-	}
-	worker, err := credentialvalidator.New(config)
-	c.Check(worker, gc.IsNil)
-	c.Check(err, gc.ErrorMatches, "boff")
+	s.facade.SetErrors(nil, errors.New("watch fail"))
+
+	worker, err := testWorker(s.config)
+	c.Assert(err, jc.ErrorIsNil)
+	err = workertest.CheckKilled(c, worker)
+	c.Check(err, gc.ErrorMatches, "watch fail")
 	s.facade.CheckCallNames(c, "ModelCredential", "WatchCredential")
 }
 
 func (s *WorkerSuite) TestModelCredentialErrorWhileRunning(c *gc.C) {
-	s.facade.SetErrors(nil, nil, errors.New("glug"))
-	config := credentialvalidator.Config{
-		Facade: s.facade,
-		Check:  neverCheck,
-	}
-	worker, err := credentialvalidator.New(config)
+	s.facade.SetErrors(nil, nil, errors.New("mc fail"))
+	worker, err := testWorker(s.config)
 	c.Assert(err, jc.ErrorIsNil)
-	c.Check(worker.Check(), jc.IsFalse)
 
 	err = workertest.CheckKilled(c, worker)
-	c.Check(err, gc.ErrorMatches, "glug")
+	c.Check(err, gc.ErrorMatches, "mc fail")
 	s.facade.CheckCallNames(c, "ModelCredential", "WatchCredential", "ModelCredential")
 }
 
 func (s *WorkerSuite) TestModelCredentialNotNeeded(c *gc.C) {
 	s.facade.exists = false
-	config := credentialvalidator.Config{
-		Facade: s.facade,
-		Check:  neverCheck,
-	}
-	worker, err := credentialvalidator.New(config)
+	worker, err := testWorker(s.config)
+	c.Assert(err, jc.ErrorIsNil)
+	err = workertest.CheckKilled(c, worker)
 	c.Assert(err, gc.ErrorMatches, "model is on the cloud that does not need auth")
-	c.Assert(worker, gc.IsNil)
 	s.facade.CheckCallNames(c, "ModelCredential")
 }
 
@@ -92,13 +84,8 @@ func (s *WorkerSuite) TestCredentialChangeToInvalid(c *gc.C) {
 		{credentialTag, false},
 	}
 
-	config := credentialvalidator.Config{
-		Facade: s.facade,
-		Check:  credentialvalidator.IsValid,
-	}
-	worker, err := credentialvalidator.New(config)
+	worker, err := testWorker(s.config)
 	c.Assert(err, jc.ErrorIsNil)
-	c.Check(worker.Check(), jc.IsTrue)
 
 	err = workertest.CheckKilled(c, worker)
 	c.Check(err, gc.Equals, credentialvalidator.ErrValidityChanged)
@@ -111,13 +98,8 @@ func (s *WorkerSuite) TestCredentialChangeFromInvalid(c *gc.C) {
 		{credentialTag, true},
 	}
 
-	config := credentialvalidator.Config{
-		Facade: s.facade,
-		Check:  credentialvalidator.IsValid,
-	}
-	worker, err := credentialvalidator.New(config)
+	worker, err := testWorker(s.config)
 	c.Assert(err, jc.ErrorIsNil)
-	c.Check(worker.Check(), jc.IsFalse)
 
 	err = workertest.CheckKilled(c, worker)
 	c.Check(err, gc.Equals, credentialvalidator.ErrValidityChanged)
@@ -129,13 +111,8 @@ func (s *WorkerSuite) TestModelCredentialReplaced(c *gc.C) {
 		{credentialTag, true},
 		{names.NewCloudCredentialTag("such/different/credential").String(), false},
 	}
-	config := credentialvalidator.Config{
-		Facade: s.facade,
-		Check:  credentialvalidator.IsValid,
-	}
-	worker, err := credentialvalidator.New(config)
+	worker, err := testWorker(s.config)
 	c.Assert(err, jc.ErrorIsNil)
-	c.Check(worker.Check(), jc.IsTrue)
 
 	err = workertest.CheckKilled(c, worker)
 	c.Check(err, gc.Equals, credentialvalidator.ErrModelCredentialChanged)
@@ -149,15 +126,12 @@ func (s *WorkerSuite) TestNoRelevantCredentialChange(c *gc.C) {
 		{credentialTag, true},
 		{credentialTag, true},
 	}
-	config := credentialvalidator.Config{
-		Facade: s.facade,
-		Check:  credentialvalidator.IsValid,
-	}
-	worker, err := credentialvalidator.New(config)
+	worker, err := testWorker(s.config)
 	c.Assert(err, jc.ErrorIsNil)
-	c.Check(worker.Check(), jc.IsTrue)
 
 	workertest.CheckAlive(c, worker)
 	workertest.CleanKill(c, worker)
 	s.facade.CheckCallNames(c, "ModelCredential", "WatchCredential", "ModelCredential", "ModelCredential", "ModelCredential")
 }
+
+var testWorker = credentialvalidator.NewWorker