Code review comment for lp:~thumper/juju-core/verify-storage-move

Revision history for this message
Tim Penhey (thumper) wrote :

Reviewers: mp+188974_code.launchpad.net,

Message:
Please take a look.

Description:
Fail earlier when bootstrapping if up already.

I broke apart the VerifyBootstrap method into two:
   EnsureNotBootstrapped
   VerifyStorage
Well I didn't create the second, just didn't call it
from the EnsureNotBootstrapped call.

The ensure method is now called in the command bootstrap
prior to any upload tools checks.

However the implication of this was much futher reaching
than I expected. Many tests assumed that bootstrap.Bootstrap
was enough to bootstrap and environment, and that it would
complain if already bootstrapped. Since the complaining was
moved into the command function, some changes had to be made.
Since some of the tests were refactored to check the storage,
this required making the dummy provider actually write the
provider-state into storage like a normal provider.

I moved the VerifyStorage out of emptystorage.go and into open.go
as it seemed better fitting there (and the comment actually
makes sense now).

Also used the environs constant for the VerificationFilename.

https://code.launchpad.net/~thumper/juju-core/verify-storage-move/+merge/188974

(do not edit description out of merge proposal)

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

Affected files (+119, -74 lines):
   A [revision details]
   M cmd/juju/bootstrap.go
   M cmd/juju/bootstrap_test.go
   M environs/bootstrap/bootstrap.go
   M environs/emptystorage.go
   M environs/emptystorage_test.go
   M environs/jujutest/livetests.go
   M environs/jujutest/tests.go
   M environs/open.go
   M environs/open_test.go
   M provider/dummy/environs.go

« Back to merge proposal