Code review comment for lp://staging/~dimitern/juju-core/027-state-supports-nonced-provisioning

Revision history for this message
Dimiter Naydenov (dimitern) wrote :

*** Submitted:

state: add nonced provisioning support

Next step in nonced provisioning; adding support
in state: machine.SetInstanceId() renamed to
SetProvisioned(), taking instance id and nonce.
It allows you change it only once, so some tests
that assumed too much had to be changed (e.g.
changing instance id several times just to
trigger a machine change). Also CheckProvisioned()
is added to machine, taking a nonce and returning
true is returned only when the instance id is set
and the nonce matches.

In the upcoming CL we'll bring everything
together - PA generating an unique nonce,
and checking it.

R=fwereade, TheMue, rog, dfc
CC=
https://codereview.appspot.com/8561044

https://codereview.appspot.com/8561044/diff/7001/state/apiserver/api_test.go
File state/apiserver/api_test.go (right):

https://codereview.appspot.com/8561044/diff/7001/state/apiserver/api_test.go#newcode912
state/apiserver/api_test.go:912: stm, err :=
s.State.AddMachine("series", state.JobHostUnits)
On 2013/04/09 17:52:09, rog wrote:
> i have to say i found the logic here really hard to follow. i know the
logic
> comes straight from state, but i'd like a few comments. the use of
oldId and
> newId make things harder too, i think. i'd just use the string
constants.

Added comments, hopefully now it's easier to follow.

https://codereview.appspot.com/8561044/diff/7001/state/machine.go
File state/machine.go (right):

https://codereview.appspot.com/8561044/diff/7001/state/machine.go#newcode439
state/machine.go:439: // an instance id and the given nonce.
On 2013/04/09 17:52:09, rog wrote:
> i think i'd say "if the machine has been provisioned with the given
nonce", as
> it's not possible to call SetProvisioned *without* an instance id.

Done.

https://codereview.appspot.com/8561044/

« Back to merge proposal