Merge lp:~jameinel/juju-core/api-set-creds-1199915 into lp:~go-bot/juju-core/trunk

[John A Meinel]

cmd/jujud: Ensure valid agent.conf after upgrade

This fixes the agent.conf files for the machine and unit agents after upgrading from 1.10 to 1.11. I ran into a few differences, the basics are:

1) Machine Agent: machine 0 always starts the state worker, when
upgrading it has an APIInfo section, but it
  1a) Didn't have Machine.SetPassword in the DB set so we didn't have a
  way to connect via the api
  1b) Didn't have a password set in apiinfo, and 1.10 had already
  changed the password after connecting to the MongoDB
  1c) Had changed their password already

So to solve this one, we grab the Password from StateInfo.Password,
call Agent.SetPassword(), and set the value in APIInfo.Password.

2) Machine Agent: machine != 0. Were only connecting to the state after
connecting to the API. Because of (1a,b) they couldn't connect to
notice that they had work to do.

So on top of what we did for (1), I added a step of:

  2a) If we are unable to connect to the API, call ensureStateWorker().
  This gives us an opportunity to set our API password and save it in
  agent.conf.

3) Unit agents:

  3a) Didn't even have an APIInfo section.
  3b) Did have a Mongo password, and had reset their Mongo Password

So we copy the state info into the api info section, and change the
port. I only use the default port (17070). I think it would be possible
to query something in state for what the official API is
(state.State.Environ().APIInfo() comes to mind, but I'm not sure what
the correct value is.)

I felt that changing the port would solve all current 1.10 users, and
if we change things in the future, we can change the code. (I also just
discovered that we have Conf.APIPort which may or may not be set in all
the various permutations, I don't have the energy to track down 3
different versions now.)

4) Unit agents bootstrapped with 1.11.2 tools

  4a) Would have a valid APIInfo section
  4b) But would not have StateInfo.Password or APIInfo.Password set.
  This is because changing the Password now occurs when we connect to
  the API, not when we first connect to the Mongo DB.

I added checks for this, to make sure that 1.11.3 doesn't cause us to
call Unit.SetPassword("").

I did a few upgrades from 1.10 => this code, and 1.11.2 => this code,
and straight bootstrapping from this code. It all seems to be up and
running. I don't think I tried bootstrapping 1.10 with this code and
upgrading.

I hopefully captured all of these config edge cases in the test suite,
so we don't have to test all the permutations manually in the future.
We also need to think very carefully when we add stuff about how we
will get upgrade to notice. I'm starting to think we want to tweak how
upgrade works. So instead of just replacing the tools, it has a
"juju upgraded" or some other command that runs the first time you are
upgraded so we can put logic into that place and have it centralized.

https://codereview.appspot.com/11137044/

cmd/jujud: Ensure valid agent.conf after upgrade

This fixes the agent.conf files for the machine and unit agents after upgrading
from 1.10 to 1.11. I ran into a few differences, the basics are:

1) Machine Agent: machine 0 always starts the state worker, when
upgrading it has an APIInfo section, but it
  1a) Didn't have Machine.SetPassword in the DB set so we didn't have a
  way to connect via the api
  1b) Didn't have a password set in apiinfo, and 1.10 had already
  changed the password after connecting to the MongoDB
  1c) Had changed their password already

So to solve this one, we grab the Password from StateInfo.Password,
call Agent.SetPassword(), and set the value in APIInfo.Password.

2) Machine Agent: machine != 0. Were only connecting to the state after
connecting to the API. Because of (1a,b) they couldn't connect to
notice that they had work to do.

So on top of what we did for (1), I added a step of:

  2a) If we are unable to connect to the API, call ensureStateWorker().
  This gives us an opportunity to set our API password and save it in
  agent.conf.

3) Unit agents:

  3a) Didn't even have an APIInfo section.
  3b) Did have a Mongo password, and had reset their Mongo Password

So we copy the state info into the api info section, and change the
port. I only use the default port (17070). I think it would be possible
to query something in state for what the official API is
(state.State.Environ().APIInfo() comes to mind, but I'm not sure what
the correct value is.)

I felt that changing the port would solve all current 1.10 users, and
if we change things in the future, we can change the code. (I also just
discovered that we have Conf.APIPort which may or may not be set in all
the various permutations, I don't have the energy to track down 3
different versions now.)

4) Unit agents bootstrapped with 1.11.2 tools

  4a) Would have a valid APIInfo section
  4b) But would not have StateInfo.Password or APIInfo.Password set.
  This is because changing the Password now occurs when we connect to
  the API, not when we first connect to the Mongo DB.

I added checks for this, to make sure that 1.11.3 doesn't cause us to
call Unit.SetPassword("").

I did a few upgrades from 1.10 => this code, and 1.11.2 => this code,
and straight bootstrapping from this code. It all seems to be up and
running. I don't think I tried bootstrapping 1.10 with this code and
upgrading.

I hopefully captured all of these config edge cases in the test suite,
so we don't have to test all the permutations manually in the future.
We also need to think very carefully when we add stuff about how we
will get upgrade to notice. I'm starting to think we want to tweak how
upgrade works. So instead of just replacing the tools, it has a
"juju upgraded" or some other command that runs the first time you are
upgraded so we can put logic into that place and have it centralized.

https://codereview.appspot.com/11137044/

[John A Meinel (jameinel)]

Reviewers: mp+174620_code.launchpad.net,

Message:
Please take a look.

Description:
cmd/jujud: Ensure valid agent.conf after upgrade

This fixes the agent.conf files for the machine and unit agents after
upgrading
from 1.10 to 1.11. I ran into a few differences, the basics are:

1) Machine Agent: machine 0 always starts the state worker, when
upgrading it has an APIInfo section, but it
   1a) Didn't have Machine.SetPassword in the DB set so we didn't have a
   way to connect via the api
   1b) Didn't have a password set in apiinfo, and 1.10 had already
   changed the password after connecting to the MongoDB
   1c) Had changed their password already

So to solve this one, we grab the Password from StateInfo.Password,
call Agent.SetPassword(), and set the value in APIInfo.Password.

2) Machine Agent: machine != 0. Were only connecting to the state after
connecting to the API. Because of (1a,b) they couldn't connect to
notice that they had work to do.

So on top of what we did for (1), I added a step of:

   2a) If we are unable to connect to the API, call ensureStateWorker().
   This gives us an opportunity to set our API password and save it in
   agent.conf.

3) Unit agents:

   3a) Didn't even have an APIInfo section.
   3b) Did have a Mongo password, and had reset their Mongo Password

So we copy the state info into the api info section, and change the
port. I only use the default port (17070). I think it would be possible
to query something in state for what the official API is
(state.State.Environ().APIInfo() comes to mind, but I'm not sure what
the correct value is.)

I felt that changing the port would solve all current 1.10 users, and
if we change things in the future, we can change the code. (I also just
discovered that we have Conf.APIPort which may or may not be set in all
the various permutations, I don't have the energy to track down 3
different versions now.)

4) Unit agents bootstrapped with 1.11.2 tools

   4a) Would have a valid APIInfo section
   4b) But would not have StateInfo.Password or APIInfo.Password set.
   This is because changing the Password now occurs when we connect to
   the API, not when we first connect to the Mongo DB.

I added checks for this, to make sure that 1.11.3 doesn't cause us to
call Unit.SetPassword("").

I did a few upgrades from 1.10 => this code, and 1.11.2 => this code,
and straight bootstrapping from this code. It all seems to be up and
running. I don't think I tried bootstrapping 1.10 with this code and
upgrading.

I hopefully captured all of these config edge cases in the test suite,
so we don't have to test all the permutations manually in the future.
We also need to think very carefully when we add stuff about how we
will get upgrade to notice. I'm starting to think we want to tweak how
upgrade works. So instead of just replacing the tools, it has a
"juju upgraded" or some other command that runs the first time you are
upgraded so we can put logic into that place and have it centralized.

https://code.launchpad.net/~jameinel/juju-core/api-set-creds-1199915/+merge/174620

(do not edit description out of merge proposal)

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

Affe...

[Frank Mueller (themue)]

LGTM, only few remarks.

https://codereview.appspot.com/11137044/diff/1/cmd/jujud/upgradevalidation.go
File cmd/jujud/upgradevalidation.go (right):

https://codereview.appspot.com/11137044/diff/1/cmd/jujud/upgradevalidation.go#newcode91
cmd/jujud/upgradevalidation.go:91: //???
We should at least log the error (which should not happen).

https://codereview.appspot.com/11137044/diff/1/cmd/jujud/upgradevalidation.go#newcode134
cmd/jujud/upgradevalidation.go:134: // This is unexpected as all
AgentState objects (Machine and Unit)
Isn't this worth a panic()? How does the system continue with this
error?

https://codereview.appspot.com/11137044/diff/1/cmd/jujud/upgradevalidation_test.go
File cmd/jujud/upgradevalidation_test.go (right):

https://codereview.appspot.com/11137044/diff/1/cmd/jujud/upgradevalidation_test.go#newcode47
cmd/jujud/upgradevalidation_test.go:47: func (s
*UpgradeValidationMachineSuite) Create1_10Machine(c *gc.C)
(*state.Machine, *agent.Conf) {
Needed a few seconds to interpret 1_10 right. ;) How about
Create1Dot10Machine?

https://codereview.appspot.com/11137044/

[William Reade (fwereade)]

Looking very nice, thanks for this; I have a couple of questions.

https://codereview.appspot.com/11137044/diff/1/cmd/jujud/machine.go
File cmd/jujud/machine.go (right):

https://codereview.appspot.com/11137044/diff/1/cmd/jujud/machine.go#newcode145
cmd/jujud/machine.go:145: // TODO: Once we can reliably trust that we
have API passwords
Give this a TODO(jam), it's nice to be able to know who to ask even when
blame info is lost through unrelated changes.

https://codereview.appspot.com/11137044/diff/1/cmd/jujud/machine_test.go
File cmd/jujud/machine_test.go (right):

https://codereview.appspot.com/11137044/diff/1/cmd/jujud/machine_test.go#newcode73
cmd/jujud/machine_test.go:73: err = conf.Write()
thanks, good catch

https://codereview.appspot.com/11137044/diff/1/cmd/jujud/upgradevalidation.go
File cmd/jujud/upgradevalidation.go (right):

https://codereview.appspot.com/11137044/diff/1/cmd/jujud/upgradevalidation.go#newcode110
cmd/jujud/upgradevalidation.go:110: }
Can you use State.APIAddresses for this? Or something like that...
defined in state/open.go I think.

https://codereview.appspot.com/11137044/diff/1/cmd/jujud/upgradevalidation.go#newcode134
cmd/jujud/upgradevalidation.go:134: // This is unexpected as all
AgentState objects (Machine and Unit)
On 2013/07/15 13:31:43, mue wrote:
> Isn't this worth a panic()? How does the system continue with this
error?

Yeah, I think this is a panic. Someone completely screwed up, do not
pass go, do not collect $200.

https://codereview.appspot.com/11137044/diff/1/cmd/jujud/upgradevalidation.go#newcode165
cmd/jujud/upgradevalidation.go:165: if err :=
setter.SetPassword(password); err != nil {
Hmm. The original idea IIRC was that we save the oldPassword (for
recovery purposes), write the new password to disk, and then really set
the new password. Not quite sure this mechanism and that one are
properly joined-up?

https://codereview.appspot.com/11137044/diff/1/cmd/jujud/upgradevalidation_test.go
File cmd/jujud/upgradevalidation_test.go (right):

https://codereview.appspot.com/11137044/diff/1/cmd/jujud/upgradevalidation_test.go#newcode47
cmd/jujud/upgradevalidation_test.go:47: func (s
*UpgradeValidationMachineSuite) Create1_10Machine(c *gc.C)
(*state.Machine, *agent.Conf) {
On 2013/07/15 13:31:43, mue wrote:
> Needed a few seconds to interpret 1_10 right. ;) How about
Create1Dot10Machine?

-1 myself, but happy to follow consensus

https://codereview.appspot.com/11137044/diff/1/cmd/jujud/upgradevalidation_test.go#newcode226
cmd/jujud/upgradevalidation_test.go:226: func (s
*UpgradeValidationUnitSuite) TestEnsureAPIInfo111(c *gc.C) {
1_11 for consistency with the above?

https://codereview.appspot.com/11137044/diff/1/cmd/jujud/upgradevalidation_test.go#newcode247
cmd/jujud/upgradevalidation_test.go:247: func (s
*UpgradeValidationUnitSuite) TestEnsureAPIInfo111Noop(c *gc.C) {
ditto

https://codereview.appspot.com/11137044/

[John A Meinel (jameinel)]

https://codereview.appspot.com/11137044/diff/1/cmd/jujud/upgradevalidation.go
File cmd/jujud/upgradevalidation.go (right):

https://codereview.appspot.com/11137044/diff/1/cmd/jujud/upgradevalidation.go#newcode165
cmd/jujud/upgradevalidation.go:165: if err :=
setter.SetPassword(password); err != nil {
On 2013/07/15 13:53:51, fwereade wrote:
> Hmm. The original idea IIRC was that we save the oldPassword (for
recovery
> purposes), write the new password to disk, and then really set the new
password.
> Not quite sure this mechanism and that one are properly joined-up?

We already have OldPassword available (or StateInfo.Password). We use
the fact that APIInfo.Password isn't set yet to indicate we need to call
state.State.SetPassword().

If we write to disk first, and then crash, the next startup won't call
SetPassword.

I did think carefully about it and had a comment, but if you feel it
needs more clarification I can try to do so.

https://codereview.appspot.com/11137044/

[John A Meinel (jameinel)]

Please take a look.

https://codereview.appspot.com/11137044/diff/1/cmd/jujud/machine.go
File cmd/jujud/machine.go (right):

https://codereview.appspot.com/11137044/diff/1/cmd/jujud/machine.go#newcode145
cmd/jujud/machine.go:145: // TODO: Once we can reliably trust that we
have API passwords
On 2013/07/15 13:53:51, fwereade wrote:
> Give this a TODO(jam), it's nice to be able to know who to ask even
when blame
> info is lost through unrelated changes.

Done.

https://codereview.appspot.com/11137044/diff/1/cmd/jujud/upgradevalidation.go
File cmd/jujud/upgradevalidation.go (right):

https://codereview.appspot.com/11137044/diff/1/cmd/jujud/upgradevalidation.go#newcode110
cmd/jujud/upgradevalidation.go:110: }
On 2013/07/15 13:53:51, fwereade wrote:
> Can you use State.APIAddresses for this? Or something like that...
defined in
> state/open.go I think.

I don't have easy access to a raw state.State object here (I currently
take something like state.Machine or state.Unit)
Looking at the implementation of state.APIAddresses it is just doing
what I'm doing only without "net". Explicitly searching for ":" and
splitting on it.

So I will pull in conf.APIPort(), but I'd rather avoid reworking what
objects are in play just to get a function that may or may not properly
handle ipv6 in the future.

https://codereview.appspot.com/11137044/diff/1/cmd/jujud/upgradevalidation.go#newcode134
cmd/jujud/upgradevalidation.go:134: // This is unexpected as all
AgentState objects (Machine and Unit)
On 2013/07/15 13:53:51, fwereade wrote:
> On 2013/07/15 13:31:43, mue wrote:
> > Isn't this worth a panic()? How does the system continue with this
error?

> Yeah, I think this is a panic. Someone completely screwed up, do not
pass go, do
> not collect $200.

Done.

https://codereview.appspot.com/11137044/diff/1/cmd/jujud/upgradevalidation_test.go
File cmd/jujud/upgradevalidation_test.go (right):

https://codereview.appspot.com/11137044/diff/1/cmd/jujud/upgradevalidation_test.go#newcode47
cmd/jujud/upgradevalidation_test.go:47: func (s
*UpgradeValidationMachineSuite) Create1_10Machine(c *gc.C)
(*state.Machine, *agent.Conf) {
On 2013/07/15 13:53:51, fwereade wrote:
> On 2013/07/15 13:31:43, mue wrote:
> > Needed a few seconds to interpret 1_10 right. ;) How about
> Create1Dot10Machine?

> -1 myself, but happy to follow consensus

I kept it as 1_10 and matched the 111 to 1_11 later in the file.

https://codereview.appspot.com/11137044/diff/1/cmd/jujud/upgradevalidation_test.go#newcode226
cmd/jujud/upgradevalidation_test.go:226: func (s
*UpgradeValidationUnitSuite) TestEnsureAPIInfo111(c *gc.C) {
On 2013/07/15 13:53:51, fwereade wrote:
> 1_11 for consistency with the above?

Done.

https://codereview.appspot.com/11137044/diff/1/cmd/jujud/upgradevalidation_test.go#newcode247
cmd/jujud/upgradevalidation_test.go:247: func (s
*UpgradeValidationUnitSuite) TestEnsureAPIInfo111Noop(c *gc.C) {
On 2013/07/15 13:53:51, fwereade wrote:
> ditto

Done.

https://codereview.appspot.com/11137044/

[William Reade (fwereade)]

LGTM, but please do a little bit of investigation to see if there's a
reaosnably convenient way to reuse the state.APIAddresses code. If not I
can live with it assuming we manage to get rid of this workaround sooner
rather than later -- and I can't see why we *would* keep it around for a
long time, so I'm happy accepting your judgment here.

https://codereview.appspot.com/11137044/

[John A Meinel (jameinel)]

Please take a look.

https://codereview.appspot.com/11137044/

[John A Meinel (jameinel)]

On 2013/07/16 13:42:49, jameinel wrote:
> Please take a look.

I did end up doing state.APIAddresses.

https://codereview.appspot.com/11137044/

[Go Bot (go-bot)]

Attempt to merge into lp:juju-core failed due to conflicts:

text conflict in cmd/jujud/upgradevalidation.go

[Go Bot (go-bot)]

The attempt to merge lp:~jameinel/juju-core/api-set-creds-1199915 into lp:juju-core failed. Below is the output from the failed tests.

----------------------------------------------------------------------
PANIC: agent_test.go:0: openSuite.TearDownTest

... Panic: watcher iteration error: unauthorized db:juju ns:juju.txns.log lock type:0 client:127.0.0.1 (PC=0x41175F)

/usr/lib/go/src/pkg/runtime/proc.c:1443
  in panic
/home/tarmac/trees/src/launchpad.net/juju-core/environs/dummy/environs.go:213
  in environState.destroy
/home/tarmac/trees/src/launchpad.net/juju-core/environs/dummy/environs.go:189
  in Reset
/home/tarmac/trees/src/launchpad.net/juju-core/juju/testing/conn.go:235
  in JujuConnSuite.tearDownConn
/home/tarmac/trees/src/launchpad.net/juju-core/juju/testing/conn.go:134
  in JujuConnSuite.TearDownTest

----------------------------------------------------------------------
PANIC: agent_test.go:384: openSuite.TestOpenAPIFallbackPassword

[LOG] 86.66806 INFO juju environs/testing: uploading FAKE tools 1.11.3-precise-amd64
[LOG] 86.66819 INFO juju environs: reading tools with major version 1
[LOG] 86.66821 DEBUG juju.environs.tools reading v1.* tools
[LOG] 86.66823 INFO juju environs: falling back to public bucket
[LOG] 86.66824 DEBUG juju.environs.tools reading v1.* tools
[LOG] 86.66829 DEBUG juju.environs.tools found 1.11.3-precise-amd64
[LOG] 86.66831 INFO juju environs: filtering tools by series: precise
[LOG] 86.66834 INFO juju environs: filtering tools by version: 1.11.3
[LOG] 86.66837 INFO juju environs/dummy: would pick tools from 1.11.3-precise-amd64
[LOG] 86.69442 INFO juju state: opening state; mongo addresses: ["localhost:38047"]; entity ""
[LOG] 86.69843 INFO juju state: connection established
[LOG] 91.48213 INFO juju state: initializing environment
[LOG] 70.72201 INFO juju state/api: listening on "127.0.0.1:50666"
[LOG] 70.75192 INFO juju state: opening state; mongo addresses: ["localhost:38047"]; entity ""
[LOG] 70.75555 INFO juju state: connection established
[LOG] 70.75608 INFO juju juju: authorization error while connecting to state server; retrying
[LOG] 70.75614 INFO juju state: opening state; mongo addresses: ["localhost:38047"]; entity ""
[LOG] 70.75931 INFO juju state: connection established
[LOG] 70.81147 INFO juju state/api: dialing "wss://127.0.0.1:50666/"
[LOG] 70.81616 INFO juju state/api: connection established
[LOG] 70.81640 INFO juju rpc: discarding action method reflect.Method{Name:"apiRootForEntity", PkgPath:"launchpad.net/juju-core/state/apiserver", Type:(*reflect.commonType)(0x800ad8), Func:reflect.Value{typ:(*reflect.commonType)(0x800ad8), val:(unsafe.Pointer)(0x6b547b), flag:0x131}, Index:1}
[LOG] 70.81651 DEBUG juju rpc/jsoncodec: <- {"RequestId":1,"Type":"Admin","Request":"Login","Params":{"AuthTag":"user-admin","Password":"dummy-secret"}}
[LOG] 70.81700 INFO juju rpc: discarding obtainer method reflect.Method{Name:"AuthClient", PkgPath:"", Type:(*reflect.commonType)(0x746b50), Func:reflect.Value{typ:(*reflect.commonType)(0x746b50), val:(unsafe.Pointer)(0x6b68d1), flag:0x130}, Index:1}
[LOG] 70.81703 INFO juju rpc: discarding obtainer method reflect.Method{Name:"AuthEnvironManager", PkgPath:"", ...

=== modified file 'cmd/jujud/machine.go'
--- cmd/jujud/machine.go	2013-07-17 02:38:48 +0000
+++ cmd/jujud/machine.go	2013-07-17 07:05:30 +0000
@@ -116,6 +116,7 @@
 		// instances of the API server have been started, we
 		// should follow the normal course of things and ignore
 		// the fact that this was once the bootstrap machine.
+		log.Infof("Starting StateWorker for machine-0")
 		ensureStateWorker()
 	}
 	a.runner.StartWorker("api", func() (worker.Worker, error) {
@@ -144,6 +145,17 @@
 func (a *MachineAgent) APIWorker(ensureStateWorker func()) (worker.Worker, error) {
 	st, entity, err := openAPIState(a.Conf.Conf, a)
 	if err != nil {
+		// There was an error connecting to the API,
+		// https://launchpad.net/bugs/1199915 means that we may just
+		// not have an API password set. So force a state connection at
+		// this point.
+		// TODO(jam): Once we can reliably trust that we have API
+		//            passwords set, and we no longer need state
+		//            connections (and possibly agents will be blocked
+		//            from connecting directly to state) we can remove
+		//            this. Currently needed because 1.10 does not set
+		//            the API password and 1.11 requires it
+		ensureStateWorker()
 		return nil, err
 	}
 	m := entity.(*machineagent.Machine)
@@ -167,6 +179,11 @@
 	if err != nil {
 		return nil, err
 	}
+	// If this fails, other bits will fail, so we just log the error, and
+	// let the other failures actually restart runners
+	if err := EnsureAPIInfo(a.Conf.Conf, st, entity); err != nil {
+		log.Warningf("failed to EnsureAPIInfo: %v", err)
+	}
 	reportOpenedState(st)
 	m := entity.(*state.Machine)
 	// TODO(rog) use more discriminating test for errors

=== modified file 'cmd/jujud/machine_test.go'
--- cmd/jujud/machine_test.go	2013-07-15 23:01:31 +0000
+++ cmd/jujud/machine_test.go	2013-07-17 07:05:30 +0000
@@ -70,7 +70,7 @@
 	c.Assert(err, IsNil)
 	conf, tools := s.agentSuite.primeAgent(c, state.MachineTag(m.Id()), "machine-password")
 	conf.MachineNonce = state.BootstrapNonce
-	conf.Write()
+	err = conf.Write()
 	c.Assert(err, IsNil)
 	return m, conf, tools
 }

=== modified file 'cmd/jujud/unit.go'
--- cmd/jujud/unit.go	2013-06-27 09:26:02 +0000
+++ cmd/jujud/unit.go	2013-07-17 07:05:30 +0000
@@ -5,15 +5,20 @@
 
 import (
 	"fmt"
+
 	"launchpad.net/gnuflag"
+	"launchpad.net/loggo"
+	"launchpad.net/tomb"
+
 	"launchpad.net/juju-core/cmd"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api"
 	"launchpad.net/juju-core/worker"
 	"launchpad.net/juju-core/worker/uniter"
-	"launchpad.net/tomb"
 )
 
+var agentLogger = loggo.GetLogger("juju.jujud")
+
 // UnitAgent is a cmd.Command responsible for running a unit agent.
 type UnitAgent struct {
 	cmd.CommandBase
@@ -63,6 +68,7 @@
 	if err := a.Conf.read(a.Tag()); err != nil {
 		return err
 	}
+	agentLogger.Infof("unit agent %v start", a.Tag())
 	a.runner.StartWorker("toplevel", func() (worker.Worker, error) {
 		// TODO(rog) go1.1: use method expression
 		return a.Workers()
@@ -78,6 +84,11 @@
 	if err != nil {
 		return nil, err
 	}
+	if err := EnsureAPIInfo(a.Conf.Conf, st, entity); err != nil {
+		// We suppress this error, because it is probably more interesting
+		// to see other failures, but we log it, in case it is a root cause
+		agentLogger.Warningf("error while calling EnsureAPIInfo: %v", err)
+	}
 	unit := entity.(*state.Unit)
 	dataDir := a.Conf.DataDir
 	runner := worker.NewRunner(allFatal, moreImportant)

=== modified file 'cmd/jujud/unit_test.go'
--- cmd/jujud/unit_test.go	2013-07-09 11:31:00 +0000
+++ cmd/jujud/unit_test.go	2013-07-17 07:05:30 +0000
@@ -85,14 +85,9 @@
 	c.Assert(err, ErrorMatches, `unrecognized args: \["thundering typhoons"\]`)
 }
 
-func (s *UnitSuite) TestRunStop(c *C) {
-	unit, _, _ := s.primeAgent(c)
-	a := s.newAgent(c, unit)
-	go func() { c.Check(a.Run(nil), IsNil) }()
-	defer func() { c.Check(a.Stop(), IsNil) }()
+func waitForUnitStarted(stateConn *state.State, unit *state.Unit, c *C) {
 	timeout := time.After(5 * time.Second)
 
-waitStarted:
 	for {
 		select {
 		case <-timeout:
@@ -108,9 +103,9 @@
 				continue
 			case params.StatusStarted:
 				c.Logf("started!")
-				break waitStarted
+				return
 			case params.StatusDown:
-				s.State.StartSync()
+				stateConn.StartSync()
 				c.Logf("unit is still down")
 			default:
 				c.Fatalf("unexpected status %s %s", st, info)
@@ -119,6 +114,14 @@
 	}
 }
 
+func (s *UnitSuite) TestRunStop(c *C) {
+	unit, _, _ := s.primeAgent(c)
+	a := s.newAgent(c, unit)
+	go func() { c.Check(a.Run(nil), IsNil) }()
+	defer func() { c.Check(a.Stop(), IsNil) }()
+	waitForUnitStarted(s.State, unit, c)
+}
+
 func (s *UnitSuite) TestUpgrade(c *C) {
 	unit, _, currentTools := s.primeAgent(c)
 	a := s.newAgent(c, unit)

=== modified file 'cmd/jujud/upgradevalidation.go'
--- cmd/jujud/upgradevalidation.go	2013-07-17 02:35:07 +0000
+++ cmd/jujud/upgradevalidation.go	2013-07-17 07:05:30 +0000
@@ -10,10 +10,12 @@
 
 	"launchpad.net/loggo"
 
+	"launchpad.net/juju-core/agent"
 	"launchpad.net/juju-core/container/lxc"
 	"launchpad.net/juju-core/environs/provider"
 	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/state"
+	"launchpad.net/juju-core/state/api"
 	"launchpad.net/juju-core/utils"
 	"launchpad.net/juju-core/utils/fslock"
 )
@@ -89,3 +91,86 @@
 	//       only supports debian-based anyway
 	return utils.AptGetInstall("lxc")
 }
+
+type passwordSetter interface {
+	SetPassword(password string) error
+}
+
+// apiAddresser just implements APIAddresses
+// This is implemented by the standard *state.State object
+type apiAddresser interface {
+	APIAddresses() ([]string, error)
+}
+
+func apiInfoFromStateInfo(stInfo *state.Info, stConn apiAddresser) *api.Info {
+	addrs, err := stConn.APIAddresses()
+	if err != nil {
+		validationLogger.Warningf("Unable to get the list of valid API Addresses: %v", err)
+	}
+	return &api.Info{
+		Addrs:    addrs,
+		CACert:   stInfo.CACert,
+		Tag:      stInfo.Tag,
+		Password: stInfo.Password,
+	}
+}
+
+// EnsureAPIInfo makes sure we can connect as an agent to the API server 1.10
+// did not set an API password for machine agents, 1.11 sets it the same as the
+// mongo password.  1.10 also does not set any API Info at all for Unit agents.
+// conf is the agent.conf for this machine/unit agent.  agentConn is the direct
+// connection to the State database
+func EnsureAPIInfo(conf *agent.Conf, stConn *state.State, agentConn AgentState) error {
+	// In 1.10 Machine Agents will have an API Info section, but will not
+	// have properly configured the Password field, so when upgrading to
+	// 1.11 we must set that field
+	// In 1.10 Unit Agents will not have an API Info section at all, so we
+	// must set everything from the StateInfo (correcting the Addrs for API
+	// port vs Mongo port)
+	// A new 1.11 Unit Agent will have an OldPassword but no Password set
+	// (in StateInfo or APIInfo). Because we only change the password on
+	// API connect, not on DB connect. We don't want to make extra
+	// SetPassword calls when we already have a valid password set
+	if conf.APIInfo != nil && (conf.APIInfo.Password != "" || conf.StateInfo.Password == "") {
+		// We must have set it earlier
+		return nil
+	}
+	setter, ok := agentConn.(passwordSetter)
+	if !ok {
+		panic("AgentState is missing a SetPassword method?")
+	}
+	if conf.APIInfo == nil {
+		// Unit agents didn't get any APIInfo in 1.10
+		conf.APIInfo = apiInfoFromStateInfo(conf.StateInfo, stConn)
+		validationLogger.Infof(
+			"agent.conf APIInfo is not set. Setting to {Addrs: %s, Tag: %s}",
+			conf.APIInfo.Addrs,
+			conf.APIInfo.Tag,
+		)
+	} else {
+		validationLogger.Infof("agent.conf APIInfo password is not set. Setting to state password")
+		conf.APIInfo.Password = conf.StateInfo.Password
+	}
+	password := conf.StateInfo.Password
+	if password == "" {
+		// In 1.11 we don't set a new password on connect (because it is
+		// done in the API code, and we don't have any API workers).
+		// We want to make sure the API user has the correct password
+		// (OldPassword). We *don't* want to set APIInfo.Password
+		// because then when we do connect via the API we wouldn't
+		// change the password. So this is only used for SetPassword
+		validationLogger.Infof(
+			"agent.conf StateInfo password is \"\". Setting Agent password for %s to OldPassword",
+			conf.APIInfo.Tag)
+		password = conf.OldPassword
+	}
+	// We set the actual password before writing it to disk, because
+	// otherwise we would not set it correctly in the future
+	if err := setter.SetPassword(password); err != nil {
+		return err
+	}
+	if err := conf.Write(); err != nil {
+		return err
+	}
+	return nil
+}

=== added file 'cmd/jujud/upgradevalidation_test.go'
--- cmd/jujud/upgradevalidation_test.go	1970-01-01 00:00:00 +0000
+++ cmd/jujud/upgradevalidation_test.go	2013-07-17 07:05:30 +0000
@@ -0,0 +1,335 @@
+// Copyright 2012, 2013 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package main
+
+import (
+	"fmt"
+	"time"
+
+	gc "launchpad.net/gocheck"
+
+	"launchpad.net/juju-core/agent"
+	"launchpad.net/juju-core/constraints"
+	"launchpad.net/juju-core/container/lxc"
+	"launchpad.net/juju-core/state"
+	"launchpad.net/juju-core/state/api"
+	"launchpad.net/juju-core/testing"
+)
+
+var _ = gc.Suite(&UpgradeValidationMachineSuite{})
+
+type UpgradeValidationMachineSuite struct {
+	agentSuite
+	lxc.TestSuite
+}
+
+func (s *UpgradeValidationMachineSuite) SetUpSuite(c *gc.C) {
+	s.agentSuite.SetUpSuite(c)
+	s.TestSuite.SetUpSuite(c)
+}
+
+func (s *UpgradeValidationMachineSuite) TearDownSuite(c *gc.C) {
+	s.TestSuite.TearDownSuite(c)
+	s.agentSuite.TearDownSuite(c)
+}
+
+func (s *UpgradeValidationMachineSuite) SetUpTest(c *gc.C) {
+	s.agentSuite.SetUpTest(c)
+	s.TestSuite.SetUpTest(c)
+}
+
+func (s *UpgradeValidationMachineSuite) TearDownTest(c *gc.C) {
+	s.TestSuite.TearDownTest(c)
+	s.agentSuite.TearDownTest(c)
+}
+
+func (s *UpgradeValidationMachineSuite) Create1_10Machine(c *gc.C) (*state.Machine, *agent.Conf) {
+	// Given the current connection to state, create a new machine, and 'reset'
+	// the configuration so that it looks like how juju 1.10 would have
+	// configured it
+	m, err := s.State.InjectMachine("series", constraints.Value{}, "ardbeg-0", state.JobHostUnits)
+	c.Assert(err, gc.IsNil)
+	err = m.SetMongoPassword("machine-password")
+	c.Assert(err, gc.IsNil)
+	// We intentionally do *not* call m.SetPassword here, as it was not
+	// done in 1.10, we also intentionally set the APIInfo.Password back to
+	// the empty string.
+	conf, _ := s.agentSuite.primeAgent(c, m.Tag(), "machine-password")
+	conf.MachineNonce = state.BootstrapNonce
+	conf.APIInfo.Password = ""
+	conf.Write()
+	c.Assert(conf.StateInfo.Tag, gc.Equals, m.Tag())
+	c.Assert(conf.StateInfo.Password, gc.Equals, "machine-password")
+	c.Assert(err, gc.IsNil)
+	return m, conf
+}
+
+func (s *UpgradeValidationMachineSuite) TestEnsureAPIInfo(c *gc.C) {
+	m, conf := s.Create1_10Machine(c)
+	// Opening the API should fail as is
+	apiState, newPassword, err := conf.OpenAPI(api.DialOpts{})
+	c.Assert(apiState, gc.IsNil)
+	c.Assert(newPassword, gc.Equals, "")
+	c.Assert(err, gc.NotNil)
+	c.Assert(err, gc.ErrorMatches, "invalid entity name or password")
+
+	err = EnsureAPIInfo(conf, s.State, m)
+	c.Assert(err, gc.IsNil)
+	// After EnsureAPIInfo we should be able to connect
+	apiState, newPassword, err = conf.OpenAPI(api.DialOpts{})
+	c.Assert(err, gc.IsNil)
+	c.Assert(apiState, gc.NotNil)
+	// We shouldn't need to set a new password
+	c.Assert(newPassword, gc.Equals, "")
+}
+
+func (s *UpgradeValidationMachineSuite) TestEnsureAPIInfoNoOp(c *gc.C) {
+	m, conf := s.Create1_10Machine(c)
+	// Set the API password to something, and record it, ensure that
+	// EnsureAPIInfo doesn't change it on us
+	m.SetPassword("frobnizzle")
+	conf.APIInfo.Password = "frobnizzle"
+	// We matched them, so we should be able to open the API
+	apiState, newPassword, err := conf.OpenAPI(api.DialOpts{})
+	c.Assert(apiState, gc.NotNil)
+	c.Assert(newPassword, gc.Equals, "")
+	c.Assert(err, gc.IsNil)
+	apiState.Close()
+
+	err = EnsureAPIInfo(conf, s.State, m)
+	c.Assert(err, gc.IsNil)
+	// After EnsureAPIInfo we should still be able to connect
+	apiState, newPassword, err = conf.OpenAPI(api.DialOpts{})
+	c.Assert(err, gc.IsNil)
+	c.Assert(apiState, gc.NotNil)
+	// We shouldn't need to set a new password
+	c.Assert(newPassword, gc.Equals, "")
+	// The password hasn't been changed
+	c.Assert(conf.APIInfo.Password, gc.Equals, "frobnizzle")
+}
+
+// Test that MachineAgent enforces the API password on startup
+func (s *UpgradeValidationMachineSuite) TestAgentEnsuresAPIInfo(c *gc.C) {
+	m, _ := s.Create1_10Machine(c)
+	// This is similar to assertJobWithState, however we need to control
+	// how the machine is initialized, so it looks like a 1.10 upgrade
+	a := &MachineAgent{}
+	s.initAgent(c, a, "--machine-id", m.Id())
+
+	agentStates := make(chan *state.State, 1000)
+	undo := sendOpenedStates(agentStates)
+	defer undo()
+
+	done := make(chan error)
+	go func() {
+		done <- a.Run(nil)
+	}()
+
+	select {
+	case agentState := <-agentStates:
+		c.Assert(agentState, gc.NotNil)
+		c.Assert(a.Conf.Conf.APIInfo.Password, gc.Equals, "machine-password")
+	case <-time.After(testing.LongWait):
+		c.Fatalf("state not opened")
+	}
+	err := a.Stop()
+	c.Assert(err, gc.IsNil)
+	c.Assert(<-done, gc.IsNil)
+}
+
+// Test that MachineAgent enforces the API password on startup even for machine>0
+func (s *UpgradeValidationMachineSuite) TestAgentEnsuresAPIInfoOnWorkers(c *gc.C) {
+	// create a machine-0, then create a new machine-1
+	_, _ = s.Create1_10Machine(c)
+	m1, _ := s.Create1_10Machine(c)
+
+	a := &MachineAgent{}
+	s.initAgent(c, a, "--machine-id", m1.Id())
+
+	agentStates := make(chan *state.State, 1000)
+	undo := sendOpenedStates(agentStates)
+	defer undo()
+
+	done := make(chan error)
+	go func() {
+		done <- a.Run(nil)
+	}()
+
+	select {
+	case agentState := <-agentStates:
+		c.Assert(agentState, gc.NotNil)
+		c.Assert(a.Conf.Conf.APIInfo.Password, gc.Equals, "machine-password")
+	case <-time.After(testing.LongWait):
+		c.Fatalf("state not opened")
+	}
+	err := a.Stop()
+	c.Assert(err, gc.IsNil)
+	c.Assert(<-done, gc.IsNil)
+}
+
+var _ = gc.Suite(&UpgradeValidationUnitSuite{})
+
+type UpgradeValidationUnitSuite struct {
+	agentSuite
+	testing.GitSuite
+}
+
+func (s *UpgradeValidationUnitSuite) SetUpTest(c *gc.C) {
+	s.agentSuite.SetUpTest(c)
+	s.GitSuite.SetUpTest(c)
+}
+
+func (s *UpgradeValidationUnitSuite) TearDownTest(c *gc.C) {
+	s.GitSuite.SetUpTest(c)
+	s.agentSuite.TearDownTest(c)
+}
+
+func (s *UpgradeValidationUnitSuite) Create1_10Unit(c *gc.C) (*state.Unit, *agent.Conf) {
+	svc, err := s.State.AddService("wordpress", s.AddTestingCharm(c, "wordpress"))
+	c.Assert(err, gc.IsNil)
+	unit, err := svc.AddUnit()
+	c.Assert(err, gc.IsNil)
+	err = unit.SetMongoPassword("unit-password")
+	c.Assert(err, gc.IsNil)
+	// We do not call SetPassword for the unit agent, and we force the
+	// APIInfo to be empty
+	conf, _ := s.agentSuite.primeAgent(c, unit.Tag(), "unit-password")
+	conf.APIInfo = nil
+	c.Assert(conf.Write(), gc.IsNil)
+	return unit, conf
+}
+
+func (s *UpgradeValidationUnitSuite) TestEnsureAPIInfo(c *gc.C) {
+	u, conf := s.Create1_10Unit(c)
+	// Opening the API should fail as is
+	c.Assert(func() { conf.OpenAPI(api.DialOpts{}) }, gc.PanicMatches, ".*nil pointer dereference")
+
+	err := EnsureAPIInfo(conf, s.State, u)
+	c.Assert(err, gc.IsNil)
+	// The test suite runs the API on non-standard ports. Fix it
+	apiAddresses, err := s.State.APIAddresses()
+	c.Assert(err, gc.IsNil)
+	c.Assert(conf.APIInfo.Addrs, gc.DeepEquals, apiAddresses)
+	conf.APIInfo.Addrs = s.APIInfo(c).Addrs
+	apiState, newPassword, err := conf.OpenAPI(api.DialOpts{})
+	c.Assert(err, gc.IsNil)
+	c.Assert(apiState, gc.NotNil)
+	// We shouldn't need to set a new password
+	c.Assert(newPassword, gc.Equals, "")
+}
+
+func (s *UpgradeValidationUnitSuite) TestEnsureAPIInfo1_11(c *gc.C) {
+	// In 1.11 State.Password is actually "", and the valid password is
+	// OldPassword. This is because in 1.11 we only change the password in
+	// OpenAPI which we don't call until we actually have agent workers
+	// But we don't want to set the actual entity password to the empty string
+	u, conf := s.Create1_10Unit(c)
+	conf.OldPassword = conf.StateInfo.Password
+	conf.StateInfo.Password = ""
+
+	err := EnsureAPIInfo(conf, s.State, u)
+	c.Assert(err, gc.IsNil)
+	// The test suite runs the API on non-standard ports. Fix it
+	apiAddresses, err := s.State.APIAddresses()
+	c.Assert(err, gc.IsNil)
+	c.Assert(conf.APIInfo.Addrs, gc.DeepEquals, apiAddresses)
+	conf.APIInfo.Addrs = s.APIInfo(c).Addrs
+	apiState, newPassword, err := conf.OpenAPI(api.DialOpts{})
+	c.Assert(err, gc.IsNil)
+	c.Assert(apiState, gc.NotNil)
+	// It should want to set a new Password
+	c.Assert(newPassword, gc.Not(gc.Equals), "")
+}
+
+func (s *UpgradeValidationUnitSuite) TestEnsureAPIInfo1_11Noop(c *gc.C) {
+	// We should notice if APIInfo is 'valid' in that it matches StateInfo
+	// even though in 1.1 both Password fields are empty.
+	u, conf := s.Create1_10Unit(c)
+	conf.OldPassword = conf.StateInfo.Password
+	conf.StateInfo.Password = ""
+	u.SetPassword(conf.OldPassword)
+	testAPIInfo := s.APIInfo(c)
+	conf.APIInfo = &api.Info{
+		Addrs:    testAPIInfo.Addrs,
+		Tag:      u.Tag(),
+		Password: "",
+		CACert:   testAPIInfo.CACert,
+	}
+
+	err := EnsureAPIInfo(conf, s.State, u)
+	c.Assert(err, gc.IsNil)
+	// We should not have changed the API Addrs or Password
+	c.Assert(conf.APIInfo.Password, gc.Equals, "")
+	c.Assert(conf.APIInfo.Addrs, gc.DeepEquals, testAPIInfo.Addrs)
+	apiState, newPassword, err := conf.OpenAPI(api.DialOpts{})
+	c.Assert(err, gc.IsNil)
+	c.Assert(apiState, gc.NotNil)
+	// It should want to set a new Password
+	c.Assert(newPassword, gc.Not(gc.Equals), "")
+}
+
+// Test that UnitAgent enforces the API password on startup
+func (s *UpgradeValidationUnitSuite) TestAgentEnsuresAPIInfo(c *gc.C) {
+	unit, _ := s.Create1_10Unit(c)
+	a := &UnitAgent{}
+	s.initAgent(c, a, "--unit-name", unit.Name())
+	go func() { c.Check(a.Run(nil), gc.IsNil) }()
+	waitForUnitStarted(s.State, unit, c)
+	c.Check(a.Stop(), gc.IsNil)
+	c.Check(a.Conf.APIInfo.Password, gc.Equals, "unit-password")
+}
+
+var _ = gc.Suite(&UpgradeValidationSuite{})
+
+type UpgradeValidationSuite struct {
+	testing.LoggingSuite
+}
+
+type mockAddresser struct {
+	Addrs []string
+	Err   error
+}
+
+func (m *mockAddresser) APIAddresses() ([]string, error) {
+	return m.Addrs, m.Err
+}
+
+func (s *UpgradeValidationSuite) TestapiInfoFromStateInfo(c *gc.C) {
+	cert := []byte("stuff")
+	stInfo := &state.Info{
+		Addrs:    []string{"example.invalid:37070"},
+		CACert:   cert,
+		Tag:      "machine-0",
+		Password: "abcdefh",
+	}
+	apiAddresses := []string{"example.invalid:17070", "another.invalid:1234"}
+	apiInfo := apiInfoFromStateInfo(stInfo, &mockAddresser{Addrs: apiAddresses})
+	c.Assert(*apiInfo, gc.DeepEquals, api.Info{
+		Addrs:    apiAddresses,
+		CACert:   cert,
+		Tag:      "machine-0",
+		Password: "abcdefh",
+	})
+
+}
+
+func (s *UpgradeValidationSuite) TestapiInfoFromStateInfoSwallowsError(c *gc.C) {
+	// No reason for it to die just because of this
+	cert := []byte("stuff")
+	stInfo := &state.Info{
+		Addrs:    []string{"example.invalid:37070"},
+		CACert:   cert,
+		Tag:      "machine-0",
+		Password: "abcdefh",
+	}
+	apiAddresses := []string{}
+	apiInfo := apiInfoFromStateInfo(stInfo, &mockAddresser{Addrs: apiAddresses, Err: fmt.Errorf("bad")})
+	c.Assert(*apiInfo, gc.DeepEquals, api.Info{
+		Addrs:    []string{},
+		CACert:   cert,
+		Tag:      "machine-0",
+		Password: "abcdefh",
+	})
+
+}