Merge lp:~thumper/juju-core/pass-through-agent-config into lp:~go-bot/juju-core/trunk

[Tim Penhey]

Pass agent config through to workers.

Broke this change out of the following work.
Instead of passing through elements of the agent
config through to the workers separately, pass the
entire agent config through. This has the benefit
of simplifying a lot of call sites.

https://codereview.appspot.com/13477043/

Pass agent config through to workers.

Broke this change out of the following work.
Instead of passing through elements of the agent
config through to the workers separately, pass the
entire agent config through. This has the benefit
of simplifying a lot of call sites.

https://codereview.appspot.com/13477043/

[Tim Penhey (thumper)]

Reviewers: mp+183543_code.launchpad.net,

Message:
Please take a look.

Description:
Pass agent config through to workers.

Broke this change out of the following work.
Instead of passing through elements of the agent
config through to the workers separately, pass the
entire agent config through. This has the benefit
of simplifying a lot of call sites.

https://code.launchpad.net/~thumper/juju-core/pass-through-agent-config/+merge/183543

Requires:
https://code.launchpad.net/~thumper/juju-core/agent-config-formatters/+merge/183074

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/13477043/

Affected files:
   A [revision details]
   M agent/agent.go
   M cmd/jujud/agent.go
   M cmd/jujud/deploy_test.go
   M cmd/jujud/machine.go
   M cmd/jujud/machine_test.go
   M cmd/jujud/unit.go
   M cmd/plugins/juju-metadata/metadata.go
   M cmd/plugins/juju-metadata/metadataplugin_test.go
   A cmd/plugins/juju-metadata/toolsmetadata.go
   A cmd/plugins/juju-metadata/toolsmetadata_test.go
   A environs/filestorage/filestorage.go
   M environs/localstorage/storage.go
   M environs/localstorage/storage_test.go
   A environs/simplestreams/json.go
   A environs/simplestreams/json_test.go
   M environs/simplestreams/simplestreams.go
   M environs/simplestreams/simplestreams_test.go
   M environs/sync/sync.go
   M environs/tools/boilerplate.go
   A environs/tools/marshal.go
   A environs/tools/marshal_test.go
   M environs/tools/simplestreams.go
   M environs/tools/simplestreams_test.go
   M provider/azure/config.go
   M state/api/params/internal.go
   M state/api/uniter/relationunit.go
   M state/api/uniter/unit.go
   M state/api/uniter/uniter_test.go
   M state/apiserver/uniter/uniter.go
   M state/apiserver/uniter/uniter_test.go
   M worker/deployer/deployer.go
   M worker/deployer/deployer_test.go
   M worker/deployer/export_test.go
   M worker/deployer/simple.go
   M worker/deployer/simple_test.go
   M worker/machiner/machiner.go
   M worker/machiner/machiner_test.go
   M worker/uniter/modes.go
   M worker/upgrader/upgrader.go
   M worker/upgrader/upgrader_test.go

[Tim Penhey (thumper)]

Please take a look.

https://codereview.appspot.com/13477043/

[Ian Booth (wallyworld)]

Seems fairly mechanical. LGTM

https://codereview.appspot.com/13477043/

[Andrew Wilkins (axwalk)]

LGTM

https://codereview.appspot.com/13477043/diff/2/agent/agent.go
File agent/agent.go (right):

https://codereview.appspot.com/13477043/diff/2/agent/agent.go#newcode219
agent/agent.go:219: return c.caCert
Perhaps to minimise problems through API misuse, this should be a copy
of the slice. Readonly slices would be good here...

https://codereview.appspot.com/13477043/diff/2/worker/deployer/simple.go
File worker/deployer/simple.go (right):

https://codereview.appspot.com/13477043/diff/2/worker/deployer/simple.go#newcode63
worker/deployer/simple.go:63: logDir: "/var/log/juju",
Will these things be moving into agent.Config?

https://codereview.appspot.com/13477043/diff/2/worker/upgrader/upgrader.go
File worker/upgrader/upgrader.go (right):

https://codereview.appspot.com/13477043/diff/2/worker/upgrader/upgrader.go#newcode71
worker/upgrader/upgrader.go:71: func NewUpgrader(st *upgrader.State,
agentConfig agent.Config) *Upgrader {
If the name is changing, the comment needs to change too, doesn't it?

https://codereview.appspot.com/13477043/

[Tim Penhey (thumper)]

Running the tests then submitting.

https://codereview.appspot.com/13477043/diff/2/agent/agent.go
File agent/agent.go (right):

https://codereview.appspot.com/13477043/diff/2/agent/agent.go#newcode219
agent/agent.go:219: return c.caCert
On 2013/09/03 01:33:12, axw1 wrote:
> Perhaps to minimise problems through API misuse, this should be a copy
of the
> slice. Readonly slices would be good here...

Done.

https://codereview.appspot.com/13477043/diff/2/worker/deployer/simple.go
File worker/deployer/simple.go (right):

https://codereview.appspot.com/13477043/diff/2/worker/deployer/simple.go#newcode63
worker/deployer/simple.go:63: logDir: "/var/log/juju",
On 2013/09/03 01:33:12, axw1 wrote:
> Will these things be moving into agent.Config?

Possibly, and probably, just not right now.

https://codereview.appspot.com/13477043/diff/2/worker/upgrader/upgrader.go
File worker/upgrader/upgrader.go (right):

https://codereview.appspot.com/13477043/diff/2/worker/upgrader/upgrader.go#newcode71
worker/upgrader/upgrader.go:71: func NewUpgrader(st *upgrader.State,
agentConfig agent.Config) *Upgrader {
On 2013/09/03 01:33:12, axw1 wrote:
> If the name is changing, the comment needs to change too, doesn't it?

Good call.

https://codereview.appspot.com/13477043/

=== modified file 'agent/agent.go'
--- agent/agent.go	2013-08-29 07:11:02 +0000
+++ agent/agent.go	2013-09-03 02:24:16 +0000
@@ -36,6 +36,10 @@
 	// TODO: make this one of the key/value pairs.
 	Nonce() string
 
+	// CACert returns the CA certificate that is used to validate the state or
+	// API servier's certificate.
+	CACert() []byte
+
 	// OpenAPI tries to connect to an API end-point.  If a non-empty
 	// newPassword is returned, the password used to connect to the state
 	// should be changed accordingly - the caller should set the entity's
@@ -211,6 +215,13 @@
 	return c.nonce
 }
 
+func (c *configInternal) CACert() []byte {
+	// Give the caller their own copy of the cert to avoid any possibility of
+	// modifying the config's copy.
+	result := append([]byte{}, c.caCert...)
+	return result
+}
+
 func (c *configInternal) APIServerDetails() (port int, cert, key []byte) {
 	return c.apiPort, c.stateServerCert, c.stateServerKey
 }

=== modified file 'cmd/jujud/agent.go'
--- cmd/jujud/agent.go	2013-08-22 23:29:46 +0000
+++ cmd/jujud/agent.go	2013-09-03 02:24:16 +0000
@@ -212,18 +212,6 @@
 // running the tests and (2) get access to the *State used internally, so that
 // tests can be run without waiting for the 5s watcher refresh time to which we would
 // otherwise be restricted.
-var newDeployContext = func(st *apideployer.State, dataDir string) (deployer.Context, error) {
-	caCert, err := st.CACert()
-	if err != nil {
-		return nil, err
-	}
-	return deployer.NewSimpleContext(dataDir, caCert, st), nil
-}
-
-func newDeployer(st *apideployer.State, machineTag string, dataDir string) (worker.Worker, error) {
-	ctx, err := newDeployContext(st, dataDir)
-	if err != nil {
-		return nil, err
-	}
-	return deployer.NewDeployer(st, ctx, machineTag), nil
+var newDeployContext = func(st *apideployer.State, agentConfig agent.Config) deployer.Context {
+	return deployer.NewSimpleContext(agentConfig, st)
 }

=== modified file 'cmd/jujud/deploy_test.go'
--- cmd/jujud/deploy_test.go	2013-08-21 22:43:46 +0000
+++ cmd/jujud/deploy_test.go	2013-09-03 02:24:16 +0000
@@ -11,6 +11,7 @@
 
 	gc "launchpad.net/gocheck"
 
+	"launchpad.net/juju-core/agent"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/testing"
 	"launchpad.net/juju-core/utils/set"
@@ -23,10 +24,11 @@
 // a sync and observe changes to the set of desired units (and thereby run
 // deployment tests in a reasonable amount of time).
 type fakeContext struct {
-	mu       sync.Mutex
-	deployed set.Strings
-	st       *state.State
-	inited   chan struct{}
+	mu          sync.Mutex
+	deployed    set.Strings
+	st          *state.State
+	agentConfig agent.Config
+	inited      chan struct{}
 }
 
 func (ctx *fakeContext) DeployUnit(unitName, _ string) error {
@@ -76,3 +78,7 @@
 		}
 	}
 }
+
+func (ctx *fakeContext) AgentConfig() agent.Config {
+	return ctx.agentConfig
+}

=== modified file 'cmd/jujud/machine.go'
--- cmd/jujud/machine.go	2013-08-29 05:36:08 +0000
+++ cmd/jujud/machine.go	2013-09-03 02:24:16 +0000
@@ -26,6 +26,7 @@
 	"launchpad.net/juju-core/upstart"
 	"launchpad.net/juju-core/worker"
 	"launchpad.net/juju-core/worker/cleaner"
+	"launchpad.net/juju-core/worker/deployer"
 	"launchpad.net/juju-core/worker/firewaller"
 	"launchpad.net/juju-core/worker/machiner"
 	"launchpad.net/juju-core/worker/minunitsworker"
@@ -142,7 +143,8 @@
 //
 // If a state worker is necessary, APIWorker calls ensureStateWorker.
 func (a *MachineAgent) APIWorker(ensureStateWorker func()) (worker.Worker, error) {
-	st, entity, err := openAPIState(a.Conf.config, a)
+	agentConfig := a.Conf.config
+	st, entity, err := openAPIState(agentConfig, a)
 	if err != nil {
 		// There was an error connecting to the API,
 		// https://launchpad.net/bugs/1199915 means that we may just
@@ -168,21 +170,18 @@
 	// Only the machiner currently connects to the API.
 	// Add other workers here as they are ready.
 	runner.StartWorker("machiner", func() (worker.Worker, error) {
-		return machiner.NewMachiner(st.Machiner(), a.Tag()), nil
+		return machiner.NewMachiner(st.Machiner(), agentConfig), nil
 	})
 	runner.StartWorker("upgrader", func() (worker.Worker, error) {
-		// TODO(rog) use id instead of *Machine (or introduce Clone method)
-		return upgrader.New(st.Upgrader(), a.Tag(), a.Conf.dataDir), nil
+		return upgrader.NewUpgrader(st.Upgrader(), agentConfig), nil
 	})
 	for _, job := range entity.Jobs() {
 		switch job {
 		case params.JobHostUnits:
-			deployerTask, err := newDeployer(st.Deployer(), a.Tag(), a.Conf.dataDir)
-			if err != nil {
-				return nil, err
-			}
 			runner.StartWorker("deployer", func() (worker.Worker, error) {
-				return deployerTask, nil
+				apiDeployer := st.Deployer()
+				context := newDeployContext(apiDeployer, agentConfig)
+				return deployer.NewDeployer(apiDeployer, context), nil
 			})
 		case params.JobManageEnviron:
 			// Not yet implemented with the API.

=== modified file 'cmd/jujud/machine_test.go'
--- cmd/jujud/machine_test.go	2013-09-02 08:35:00 +0000
+++ cmd/jujud/machine_test.go	2013-09-03 02:24:16 +0000
@@ -248,10 +248,11 @@
 		inited: make(chan struct{}),
 	}
 	orig := newDeployContext
-	newDeployContext = func(dst *apideployer.State, dataDir string) (deployer.Context, error) {
+	newDeployContext = func(dst *apideployer.State, agentConfig agent.Config) deployer.Context {
 		ctx.st = st
+		ctx.agentConfig = agentConfig
 		close(ctx.inited)
-		return ctx, nil
+		return ctx
 	}
 	return ctx, func() { newDeployContext = orig }
 }

=== modified file 'cmd/jujud/unit.go'
--- cmd/jujud/unit.go	2013-08-21 04:12:32 +0000
+++ cmd/jujud/unit.go	2013-09-03 02:24:16 +0000
@@ -93,14 +93,14 @@
 }
 
 func (a *UnitAgent) APIWorkers() (worker.Worker, error) {
-	st, entity, err := openAPIState(a.Conf.config, a)
+	agentConfig := a.Conf.config
+	st, _, err := openAPIState(agentConfig, a)
 	if err != nil {
 		return nil, err
 	}
-	dataDir := a.Conf.dataDir
 	runner := worker.NewRunner(allFatal, moreImportant)
 	runner.StartWorker("upgrader", func() (worker.Worker, error) {
-		return upgrader.New(st.Upgrader(), entity.Tag(), dataDir), nil
+		return upgrader.NewUpgrader(st.Upgrader(), agentConfig), nil
 	})
 	return newCloseWorker(runner, st), nil
 }

=== modified file 'worker/deployer/deployer.go'
--- worker/deployer/deployer.go	2013-08-28 14:03:08 +0000
+++ worker/deployer/deployer.go	2013-09-03 02:24:16 +0000
@@ -8,6 +8,7 @@
 
 	"launchpad.net/loggo"
 
+	"launchpad.net/juju-core/agent"
 	"launchpad.net/juju-core/errors"
 	"launchpad.net/juju-core/names"
 	apideployer "launchpad.net/juju-core/state/api/deployer"
@@ -24,10 +25,9 @@
 // to changes in a set of state units; and for the final removal of its agents'
 // units from state when they are no longer needed.
 type Deployer struct {
-	st         *apideployer.State
-	ctx        Context
-	machineTag string
-	deployed   set.Strings
+	st       *apideployer.State
+	ctx      Context
+	deployed set.Strings
 }
 
 // Context abstracts away the differences between different unit deployment
@@ -46,15 +46,18 @@
 
 	// DeployedUnits returns the names of all units deployed by the manager.
 	DeployedUnits() ([]string, error)
+
+	// AgentConfig returns the agent config for the machine agent that is
+	// running the deployer.
+	AgentConfig() agent.Config
 }
 
 // NewDeployer returns a Worker that deploys and recalls unit agents
 // via ctx, taking a machine id to operate on.
-func NewDeployer(st *apideployer.State, ctx Context, machineTag string) worker.Worker {
+func NewDeployer(st *apideployer.State, ctx Context) worker.Worker {
 	d := &Deployer{
-		st:         st,
-		ctx:        ctx,
-		machineTag: machineTag,
+		st:  st,
+		ctx: ctx,
 	}
 	return worker.NewStringsWorker(d)
 }
@@ -64,7 +67,8 @@
 }
 
 func (d *Deployer) SetUp() (watcher.StringsWatcher, error) {
-	machine, err := d.st.Machine(d.machineTag)
+	machineTag := d.ctx.AgentConfig().Tag()
+	machine, err := d.st.Machine(machineTag)
 	if err != nil {
 		return nil, err
 	}

=== modified file 'worker/deployer/deployer_test.go'
--- worker/deployer/deployer_test.go	2013-08-01 13:49:08 +0000
+++ worker/deployer/deployer_test.go	2013-09-03 02:24:16 +0000
@@ -13,7 +13,6 @@
 
 	"launchpad.net/juju-core/errors"
 	jujutesting "launchpad.net/juju-core/juju/testing"
-	"launchpad.net/juju-core/names"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api"
 	apideployer "launchpad.net/juju-core/state/api/deployer"
@@ -71,6 +70,12 @@
 	s.JujuConnSuite.TearDownTest(c)
 }
 
+func (s *deployerSuite) makeDeployerAndContext(c *gc.C) (worker.Worker, deployer.Context) {
+	// Create a deployer acting on behalf of the machine.
+	ctx := s.getContextForMachine(c, s.machine.Tag())
+	return deployer.NewDeployer(s.deployerState, ctx), ctx
+}
+
 func (s *deployerSuite) TestDeployRecallRemovePrincipals(c *gc.C) {
 	// Create a machine, and a couple of units.
 	svc, err := s.State.AddService("wordpress", s.AddTestingCharm(c, "wordpress"))
@@ -80,9 +85,7 @@
 	u1, err := svc.AddUnit()
 	c.Assert(err, gc.IsNil)
 
-	// Create a deployer acting on behalf of the machine.
-	ctx := s.getContext(c)
-	dep := deployer.NewDeployer(s.deployerState, ctx, s.machine.Tag())
+	dep, ctx := s.makeDeployerAndContext(c)
 	defer stop(c, dep)
 
 	// Assign one unit, and wait for it to be deployed.
@@ -147,8 +150,7 @@
 
 	// When the deployer is started, in each case (1) no unit agent is deployed
 	// and (2) the non-Alive unit is been removed from state.
-	ctx := s.getContext(c)
-	dep := deployer.NewDeployer(s.deployerState, ctx, s.machine.Tag())
+	dep, ctx := s.makeDeployerAndContext(c)
 	defer stop(c, dep)
 	s.waitFor(c, isRemoved(s.State, u0.Name()))
 	s.waitFor(c, isRemoved(s.State, u1.Name()))
@@ -181,14 +183,11 @@
 func (s *deployerSuite) TestDeployRecallRemoveSubordinates(c *gc.C) {
 	// Create a deployer acting on behalf of the principal.
 	u, rus := s.prepareSubordinates(c)
-	ctx := s.getContext(c)
-	machineId, err := u.AssignedMachineId()
-	c.Assert(err, gc.IsNil)
-	dep := deployer.NewDeployer(s.deployerState, ctx, names.MachineTag(machineId))
+	dep, ctx := s.makeDeployerAndContext(c)
 	defer stop(c, dep)
 
 	// Add a subordinate, and wait for it to be deployed.
-	err = rus[0].EnterScope(nil)
+	err := rus[0].EnterScope(nil)
 	c.Assert(err, gc.IsNil)
 	sub0, err := s.State.Unit("subsvc0/0")
 	c.Assert(err, gc.IsNil)
@@ -218,7 +217,7 @@
 
 func (s *deployerSuite) TestNonAliveSubordinates(c *gc.C) {
 	// Add two subordinate units and set them to Dead/Dying respectively.
-	u, rus := s.prepareSubordinates(c)
+	_, rus := s.prepareSubordinates(c)
 	err := rus[0].EnterScope(nil)
 	c.Assert(err, gc.IsNil)
 	sub0, err := s.State.Unit("subsvc0/0")
@@ -234,10 +233,7 @@
 
 	// When we start a new deployer, neither unit will be deployed and
 	// both will be removed.
-	ctx := s.getContext(c)
-	machineId, err := u.AssignedMachineId()
-	c.Assert(err, gc.IsNil)
-	dep := deployer.NewDeployer(s.deployerState, ctx, names.MachineTag(machineId))
+	dep, _ := s.makeDeployerAndContext(c)
 	defer stop(c, dep)
 	s.waitFor(c, isRemoved(s.State, sub0.Name()))
 	s.waitFor(c, isRemoved(s.State, sub1.Name()))

=== modified file 'worker/deployer/export_test.go'
--- worker/deployer/export_test.go	2013-07-25 11:03:16 +0000
+++ worker/deployer/export_test.go	2013-09-03 02:24:16 +0000
@@ -3,6 +3,10 @@
 
 package deployer
 
+import (
+	"launchpad.net/juju-core/agent"
+)
+
 type fakeAddresser struct{}
 
 func (*fakeAddresser) StateAddresses() ([]string, error) {
@@ -13,12 +17,11 @@
 	return []string{"a1:123", "a2:123"}, nil
 }
 
-func NewTestSimpleContext(initDir, dataDir, logDir, syslogConfigDir string) *SimpleContext {
+func NewTestSimpleContext(agentConfig agent.Config, initDir, logDir, syslogConfigDir string) *SimpleContext {
 	return &SimpleContext{
 		addresser:       &fakeAddresser{},
-		caCert:          []byte("test-cert"),
+		agentConfig:     agentConfig,
 		initDir:         initDir,
-		dataDir:         dataDir,
 		logDir:          logDir,
 		syslogConfigDir: syslogConfigDir,
 	}

=== modified file 'worker/deployer/simple.go'
--- worker/deployer/simple.go	2013-08-30 02:38:39 +0000
+++ worker/deployer/simple.go	2013-09-03 02:24:16 +0000
@@ -28,18 +28,14 @@
 	// the given unit is deployed.
 	addresser Addresser
 
-	// caCert holds the CA certificate that will be used
-	// to validate the state server's certificate, in PEM format.
-	caCert []byte
+	// agentConfig returns the agent config for the machine agent that is
+	// running the deployer.
+	agentConfig agent.Config
 
 	// initDir specifies the directory used by upstart on the local system.
 	// It is typically set to "/etc/init".
 	initDir string
 
-	// dataDir specifies the directory used by juju to store its state. It
-	// is typically set to "/var/lib/juju".
-	dataDir string
-
 	// logDir specifies the directory to which installed units will write
 	// their log files. It is typically set to "/var/log/juju".
 	logDir string
@@ -58,21 +54,20 @@
 // NewSimpleContext returns a new SimpleContext, acting on behalf of the
 // specified deployer, that deploys unit agents as upstart jobs in
 // "/etc/init" logging to "/var/log/juju". Paths to which agents and tools
-// are installed are relative to dataDir; if dataDir is empty, it will be
-// set to "/var/lib/juju".
-func NewSimpleContext(dataDir string, caCert []byte, addresser Addresser) *SimpleContext {
-	if dataDir == "" {
-		dataDir = "/var/lib/juju"
-	}
+// are installed are relative to dataDir.
+func NewSimpleContext(agentConfig agent.Config, addresser Addresser) *SimpleContext {
 	return &SimpleContext{
-		addresser: addresser,
-		caCert:    caCert,
-		initDir:   "/etc/init",
-		dataDir:   dataDir,
-		logDir:    "/var/log/juju",
+		addresser:   addresser,
+		agentConfig: agentConfig,
+		initDir:     "/etc/init",
+		logDir:      "/var/log/juju",
 	}
 }
 
+func (ctx *SimpleContext) AgentConfig() agent.Config {
+	return ctx.agentConfig
+}
+
 func (ctx *SimpleContext) DeployUnit(unitName, initialPassword string) (err error) {
 	// Check sanity.
 	svc := ctx.upstartService(unitName)
@@ -82,8 +77,9 @@
 
 	// Link the current tools for use by the new agent.
 	tag := names.UnitTag(unitName)
-	_, err = tools.ChangeAgentTools(ctx.dataDir, tag, version.Current)
-	toolsDir := tools.ToolsDir(ctx.dataDir, tag)
+	dataDir := ctx.agentConfig.DataDir()
+	_, err = tools.ChangeAgentTools(dataDir, tag, version.Current)
+	toolsDir := tools.ToolsDir(dataDir, tag)
 	defer removeOnErr(&err, toolsDir)
 
 	// Retrieve the state addresses.
@@ -100,13 +96,13 @@
 	logger.Debugf("API addresses: %q", apiAddrs)
 	conf, err := agent.NewAgentConfig(
 		agent.AgentConfigParams{
-			DataDir:        ctx.dataDir,
+			DataDir:        dataDir,
 			Tag:            tag,
 			Password:       initialPassword,
 			Nonce:          "unused",
 			StateAddresses: stateAddrs,
 			APIAddresses:   apiAddrs,
-			CACert:         ctx.caCert,
+			CACert:         ctx.agentConfig.CACert(),
 		})
 	if err != nil {
 		return err
@@ -132,7 +128,7 @@
 
 	cmd := strings.Join([]string{
 		path.Join(toolsDir, "jujud"), "unit",
-		"--data-dir", conf.DataDir(),
+		"--data-dir", dataDir,
 		"--unit-name", unitName,
 		"--debug", // TODO: propagate debug state sensibly
 	}, " ")
@@ -176,7 +172,8 @@
 		return err
 	}
 	tag := names.UnitTag(unitName)
-	agentDir := agent.Dir(ctx.dataDir, tag)
+	dataDir := ctx.agentConfig.DataDir()
+	agentDir := agent.Dir(dataDir, tag)
 	if err := os.RemoveAll(agentDir); err != nil {
 		return err
 	}
@@ -189,7 +186,7 @@
 			logger.Warningf("installer: cannot restart syslog daemon: %v", err)
 		}
 	}()
-	toolsDir := tools.ToolsDir(ctx.dataDir, tag)
+	toolsDir := tools.ToolsDir(dataDir, tag)
 	return os.Remove(toolsDir)
 }
 

=== modified file 'worker/deployer/simple_test.go'
--- worker/deployer/simple_test.go	2013-08-28 22:59:45 +0000
+++ worker/deployer/simple_test.go	2013-09-03 02:24:16 +0000
@@ -183,7 +183,13 @@
 }
 
 func (fix *SimpleToolsFixture) getContext(c *gc.C) *deployer.SimpleContext {
-	return deployer.NewTestSimpleContext(fix.initDir, fix.dataDir, fix.logDir, fix.syslogConfigDir)
+	config := agentConfig("machine-tag", fix.dataDir)
+	return deployer.NewTestSimpleContext(config, fix.initDir, fix.logDir, fix.syslogConfigDir)
+}
+
+func (fix *SimpleToolsFixture) getContextForMachine(c *gc.C, machineTag string) *deployer.SimpleContext {
+	config := agentConfig(machineTag, fix.dataDir)
+	return deployer.NewTestSimpleContext(config, fix.initDir, fix.logDir, fix.syslogConfigDir)
 }
 
 func (fix *SimpleToolsFixture) paths(tag string) (confPath, agentDir, toolsDir, syslogConfPath string) {
@@ -279,3 +285,25 @@
 	err = os.MkdirAll(toolsDir, 0755)
 	c.Assert(err, gc.IsNil)
 }
+
+type mockConfig struct {
+	agent.Config
+	tag     string
+	datadir string
+}
+
+func (mock *mockConfig) Tag() string {
+	return mock.tag
+}
+
+func (mock *mockConfig) DataDir() string {
+	return mock.datadir
+}
+
+func (mock *mockConfig) CACert() []byte {
+	return []byte(testing.CACert)
+}
+
+func agentConfig(tag, datadir string) agent.Config {
+	return &mockConfig{tag: tag, datadir: datadir}
+}

=== modified file 'worker/machiner/machiner.go'
--- worker/machiner/machiner.go	2013-08-28 14:03:08 +0000
+++ worker/machiner/machiner.go	2013-09-03 02:24:16 +0000
@@ -5,6 +5,7 @@
 import (
 	"launchpad.net/loggo"
 
+	"launchpad.net/juju-core/agent"
 	"launchpad.net/juju-core/errors"
 	"launchpad.net/juju-core/state/api/machiner"
 	"launchpad.net/juju-core/state/api/params"
@@ -24,8 +25,8 @@
 // NewMachiner returns a Worker that will wait for the identified machine
 // to become Dying and make it Dead; or until the machine becomes Dead by
 // other means.
-func NewMachiner(st *machiner.State, tag string) worker.Worker {
-	mr := &Machiner{st: st, tag: tag}
+func NewMachiner(st *machiner.State, agentConfig agent.Config) worker.Worker {
+	mr := &Machiner{st: st, tag: agentConfig.Tag()}
 	return worker.NewNotifyWorker(mr)
 }
 

=== modified file 'worker/machiner/machiner_test.go'
--- worker/machiner/machiner_test.go	2013-08-19 11:20:02 +0000
+++ worker/machiner/machiner_test.go	2013-09-03 02:24:16 +0000
@@ -5,6 +5,10 @@
 
 import (
 	gc "launchpad.net/gocheck"
+	stdtesting "testing"
+	"time"
+
+	"launchpad.net/juju-core/agent"
 	"launchpad.net/juju-core/juju/testing"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api"
@@ -13,8 +17,6 @@
 	coretesting "launchpad.net/juju-core/testing"
 	"launchpad.net/juju-core/worker"
 	"launchpad.net/juju-core/worker/machiner"
-	stdtesting "testing"
-	"time"
 )
 
 // worstCase is used for timeouts when timing out
@@ -87,13 +89,30 @@
 
 var _ worker.NotifyWatchHandler = (*machiner.Machiner)(nil)
 
+type mockConfig struct {
+	agent.Config
+	tag string
+}
+
+func (mock *mockConfig) Tag() string {
+	return mock.tag
+}
+
+func agentConfig(tag string) agent.Config {
+	return &mockConfig{tag: tag}
+}
+
 func (s *MachinerSuite) TestNotFoundOrUnauthorized(c *gc.C) {
-	mr := machiner.NewMachiner(s.machinerState, "eleventy-one")
+	mr := machiner.NewMachiner(s.machinerState, agentConfig("eleventy-one"))
 	c.Assert(mr.Wait(), gc.Equals, worker.ErrTerminateAgent)
 }
 
+func (s *MachinerSuite) makeMachiner() worker.Worker {
+	return machiner.NewMachiner(s.machinerState, agentConfig(s.apiMachine.Tag()))
+}
+
 func (s *MachinerSuite) TestRunStop(c *gc.C) {
-	mr := machiner.NewMachiner(s.machinerState, s.apiMachine.Tag())
+	mr := s.makeMachiner()
 	c.Assert(worker.Stop(mr), gc.IsNil)
 	c.Assert(s.apiMachine.Refresh(), gc.IsNil)
 	c.Assert(s.apiMachine.Life(), gc.Equals, params.Alive)
@@ -105,21 +124,21 @@
 	c.Assert(status, gc.Equals, params.StatusPending)
 	c.Assert(info, gc.Equals, "")
 
-	mr := machiner.NewMachiner(s.machinerState, s.apiMachine.Tag())
+	mr := s.makeMachiner()
 	defer worker.Stop(mr)
 
 	s.waitMachineStatus(c, s.machine, params.StatusStarted)
 }
 
 func (s *MachinerSuite) TestSetsStatusWhenDying(c *gc.C) {
-	mr := machiner.NewMachiner(s.machinerState, s.apiMachine.Tag())
+	mr := s.makeMachiner()
 	defer worker.Stop(mr)
 	c.Assert(s.machine.Destroy(), gc.IsNil)
 	s.waitMachineStatus(c, s.machine, params.StatusStopped)
 }
 
 func (s *MachinerSuite) TestSetDead(c *gc.C) {
-	mr := machiner.NewMachiner(s.machinerState, s.machine.Tag())
+	mr := s.makeMachiner()
 	defer worker.Stop(mr)
 	c.Assert(s.machine.Destroy(), gc.IsNil)
 	s.State.StartSync()

=== modified file 'worker/upgrader/upgrader.go'
--- worker/upgrader/upgrader.go	2013-08-29 11:29:57 +0000
+++ worker/upgrader/upgrader.go	2013-09-03 02:24:16 +0000
@@ -11,6 +11,7 @@
 	"launchpad.net/loggo"
 	"launchpad.net/tomb"
 
+	"launchpad.net/juju-core/agent"
 	agenttools "launchpad.net/juju-core/agent/tools"
 	"launchpad.net/juju-core/state/api/upgrader"
 	"launchpad.net/juju-core/state/watcher"
@@ -60,18 +61,17 @@
 	tag     string
 }
 
-// New returns a new upgrader worker. It watches changes to the
-// current version of the current agent (with the given tag)
-// and tries to download the tools for any new version
-// into the given data directory.
-// If an upgrade is needed, the worker will exit with an
-// UpgradeReadyError holding details of the requested upgrade. The tools
-// will have been downloaded and unpacked.
-func New(st *upgrader.State, tag, dataDir string) *Upgrader {
+// NewUpgrader returns a new upgrader worker. It watches changes to the
+// current version of the current agent (with the given tag) and tries to
+// download the tools for any new version into the given data directory.  If
+// an upgrade is needed, the worker will exit with an UpgradeReadyError
+// holding details of the requested upgrade. The tools will have been
+// downloaded and unpacked.
+func NewUpgrader(st *upgrader.State, agentConfig agent.Config) *Upgrader {
 	u := &Upgrader{
 		st:      st,
-		dataDir: dataDir,
-		tag:     tag,
+		dataDir: agentConfig.DataDir(),
+		tag:     agentConfig.Tag(),
 	}
 	go func() {
 		defer u.tomb.Done()

=== modified file 'worker/upgrader/upgrader_test.go'
--- worker/upgrader/upgrader_test.go	2013-08-22 21:59:32 +0000
+++ worker/upgrader/upgrader_test.go	2013-09-03 02:24:16 +0000
@@ -14,6 +14,7 @@
 
 	gc "launchpad.net/gocheck"
 
+	"launchpad.net/juju-core/agent"
 	agenttools "launchpad.net/juju-core/agent/tools"
 	envtools "launchpad.net/juju-core/environs/tools"
 	"launchpad.net/juju-core/errors"
@@ -99,6 +100,29 @@
 	return &coretools.Tools{URL: url, Version: vers}
 }
 
+type mockConfig struct {
+	agent.Config
+	tag     string
+	datadir string
+}
+
+func (mock *mockConfig) Tag() string {
+	return mock.tag
+}
+
+func (mock *mockConfig) DataDir() string {
+	return mock.datadir
+}
+
+func agentConfig(tag, datadir string) agent.Config {
+	return &mockConfig{tag: tag, datadir: datadir}
+}
+
+func (s *UpgraderSuite) makeUpgrader() *upgrader.Upgrader {
+	config := agentConfig(s.machine.Tag(), s.DataDir())
+	return upgrader.NewUpgrader(s.state.Upgrader(), config)
+}
+
 func (s *UpgraderSuite) TestUpgraderSetsTools(c *gc.C) {
 	vers := version.MustParseBinary("5.4.3-foo-bar")
 	err := statetesting.SetAgentVersion(s.State, vers.Number)
@@ -108,7 +132,7 @@
 	_, err = s.machine.AgentTools()
 	c.Assert(err, jc.Satisfies, errors.IsNotFoundError)
 
-	u := upgrader.New(s.state.Upgrader(), s.machine.Tag(), s.DataDir())
+	u := s.makeUpgrader()
 	statetesting.AssertStop(c, u)
 	s.machine.Refresh()
 	gotTools, err := s.machine.AgentTools()
@@ -127,7 +151,7 @@
 	err = statetesting.SetAgentVersion(s.State, vers.Number)
 	c.Assert(err, gc.IsNil)
 
-	u := upgrader.New(s.state.Upgrader(), s.machine.Tag(), s.DataDir())
+	u := s.makeUpgrader()
 	statetesting.AssertStop(c, u)
 	s.machine.Refresh()
 	gotTools, err := s.machine.AgentTools()
@@ -147,7 +171,7 @@
 	// it's been stopped.
 	dummy.SetStorageDelay(coretesting.ShortWait)
 
-	u := upgrader.New(s.state.Upgrader(), s.machine.Tag(), s.DataDir())
+	u := s.makeUpgrader()
 	err = u.Stop()
 	c.Assert(err, gc.DeepEquals, &upgrader.UpgradeReadyError{
 		AgentName: s.machine.Tag(),
@@ -173,7 +197,7 @@
 		return retryc
 	}
 	dummy.Poison(s.Conn.Environ.Storage(), envtools.StorageName(newTools.Version), fmt.Errorf("a non-fatal dose"))
-	u := upgrader.New(s.state.Upgrader(), s.machine.Tag(), s.DataDir())
+	u := s.makeUpgrader()
 	defer u.Stop()
 
 	for i := 0; i < 3; i++ {