Merge lp:~mfoord/juju-core/jenv-warning into lp:~go-bot/juju-core/trunk

[Michael Foord]

cmd/juju: warn user existing jenv file is being used.

Warn the user when bootstrap takes environ config from an existing .jenv file instead of environments.yaml.

Mitigates issue lp:1282642

https://codereview.appspot.com/71490045/

cmd/juju: warn about existing jenv file

Warn the user when using an existing .jenv file instead of
environments.yaml.

Mitigates issue lp:1282642

https://codereview.appspot.com/71490045/

[Michael Foord (mfoord)]

Reviewers: mp+209421_code.launchpad.net,

Message:
Please take a look.

Description:

https://code.launchpad.net/~mfoord/juju-core/jenv-warning/+merge/209421

(do not edit description out of merge proposal)

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

Affected files (+27, -0 lines):
   A [revision details]
   M cmd/juju/bootstrap.go
   M cmd/juju/bootstrap_test.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: tarmac-20140227172835-l54f642dgfcocrj0
+New revision: <email address hidden>

Index: cmd/juju/bootstrap.go
=== modified file 'cmd/juju/bootstrap.go'
--- cmd/juju/bootstrap.go 2014-02-13 03:16:09 +0000
+++ cmd/juju/bootstrap.go 2014-03-05 11:00:23 +0000
@@ -115,6 +115,7 @@
   var existing bool
   if _, err := store.ReadInfo(c.EnvName); !errors.IsNotFoundError(err) {
    existing = true
+ logger.Warningf("Found and using existing .jenv file")
   }
   environ, err := environs.PrepareFromName(c.EnvName, ctx, store)
   if err != nil {

Index: cmd/juju/bootstrap_test.go
=== modified file 'cmd/juju/bootstrap_test.go'
--- cmd/juju/bootstrap_test.go 2014-02-13 02:46:58 +0000
+++ cmd/juju/bootstrap_test.go 2014-03-05 14:45:19 +0000
@@ -8,6 +8,7 @@
   "fmt"
   "strings"

+ "github.com/loggo/loggo"
   gc "launchpad.net/gocheck"

   "launchpad.net/juju-core/cmd"
@@ -26,6 +27,7 @@
   "launchpad.net/juju-core/errors"
   "launchpad.net/juju-core/provider/dummy"
   coretesting "launchpad.net/juju-core/testing"
+ jc "launchpad.net/juju-core/testing/checkers"
   "launchpad.net/juju-core/testing/testbase"
   coretools "launchpad.net/juju-core/tools"
   "launchpad.net/juju-core/version"
@@ -315,6 +317,28 @@
   c.Check(coretesting.Stdout(ctx2), gc.Equals, "")
  }

+func (s *BootstrapSuite) TestBootstrapJenvWarning(c *gc.C) {
+ env, fake := makeEmptyFakeHome(c)
+ defer fake.Restore()
+ defaultSeriesVersion := version.Current
+ defaultSeriesVersion.Series = env.Config().DefaultSeries()
+
+ store, err := configstore.Default()
+ c.Assert(err, gc.IsNil)
+ ctx := coretesting.Context(c)
+ environs.PrepareFromName("peckham", ctx, store)
+
+ logger := "jenv.warning.test"
+ testWriter := &loggo.TestWriter{}
+ loggo.RegisterWriter(logger, testWriter, loggo.WARNING)
+ defer loggo.RemoveWriter(logger)
+
+ opc, errc := runCommand(ctx, new(BootstrapCommand), "-e", "peckham")
+ c.Assert(<-errc, gc.IsNil)
+ c.Assert((<-opc).(dummy.OpPutFile).Env, gc.Equals, "peckham")
+ c.Assert(testWriter.Log, jc.LogMatches, []string{"Found and using
existing .jenv file"})
+}
+
  func (s *BootstrapSuite) TestInvalidLocalSource(c *gc.C) {
   s.PatchValue(&version.Current.Number, version.MustParse("1.2.0"))
   env, fake := makeEmptyFakeHome(c)

[John A Meinel (jameinel)]

LGTM with a tweak to the message

https://codereview.appspot.com/71490045/diff/1/cmd/juju/bootstrap.go
File cmd/juju/bootstrap.go (right):

https://codereview.appspot.com/71490045/diff/1/cmd/juju/bootstrap.go#newcode118
cmd/juju/bootstrap.go:118: logger.Warningf("Found and using existing
.jenv file")
I think this would be easier for users as:

"Using existing environment file: /path/to/.juju/environments/foo.jenv"

https://codereview.appspot.com/71490045/

[Michael Foord (mfoord)]

Please take a look.

https://codereview.appspot.com/71490045/

[Michael Foord (mfoord)]

Please take a look.

https://codereview.appspot.com/71490045/

[Michael Foord (mfoord)]

Please take a look.

https://codereview.appspot.com/71490045/

[Dimiter Naydenov (dimitern)]

LGTM with two minor suggestions.

https://codereview.appspot.com/71490045/diff/40001/environs/configstore/disk.go
File environs/configstore/disk.go (right):

https://codereview.appspot.com/71490045/diff/40001/environs/configstore/disk.go#newcode162
environs/configstore/disk.go:162: // Location returns the location of
the environInfo in human readable format
s/format/format./

https://codereview.appspot.com/71490045/diff/40001/environs/configstore/interface.go
File environs/configstore/interface.go (right):

https://codereview.appspot.com/71490045/diff/40001/environs/configstore/interface.go#newcode78
environs/configstore/interface.go:78: // information in a human readable
format
s/format/format./ ?

https://codereview.appspot.com/71490045/

[Michael Foord (mfoord)]

Please take a look.

https://codereview.appspot.com/71490045/diff/40001/environs/configstore/disk.go
File environs/configstore/disk.go (right):

https://codereview.appspot.com/71490045/diff/40001/environs/configstore/disk.go#newcode162
environs/configstore/disk.go:162: // Location returns the location of
the environInfo in human readable format
On 2014/03/06 11:37:24, dimitern wrote:
> s/format/format./

Done.

https://codereview.appspot.com/71490045/diff/40001/environs/configstore/interface.go
File environs/configstore/interface.go (right):

https://codereview.appspot.com/71490045/diff/40001/environs/configstore/interface.go#newcode78
environs/configstore/interface.go:78: // information in a human readable
format
On 2014/03/06 11:37:24, dimitern wrote:
> s/format/format./ ?

Done.

https://codereview.appspot.com/71490045/

[Roger Peppe (rogpeppe)]

LGTM with a couple of suggestions below.

https://codereview.appspot.com/71490045/diff/40001/cmd/juju/bootstrap.go
File cmd/juju/bootstrap.go (right):

https://codereview.appspot.com/71490045/diff/40001/cmd/juju/bootstrap.go#newcode119
cmd/juju/bootstrap.go:119: logger.Warningf("using existing environment
file: %q", loc)
with a slightly shorter variable name, it perhaps seems
more reasonable inline here. Also, given that Location
is documented to be human readable, perhaps we don't
need to quote it.

logger.Warningf("using existing environment at %s", info.Location())

?

https://codereview.appspot.com/71490045/diff/40001/cmd/juju/bootstrap_test.go
File cmd/juju/bootstrap_test.go (right):

https://codereview.appspot.com/71490045/diff/40001/cmd/juju/bootstrap_test.go#newcode338
cmd/juju/bootstrap_test.go:338: c.Assert((<-opc).(dummy.OpPutFile).Env,
gc.Equals, "peckham")
I don't think you need this (and it makes the test slightly more
fragile)

https://codereview.appspot.com/71490045/

[Michael Foord (mfoord)]

On 2014/03/06 12:14:58, rog wrote:
> LGTM with a couple of suggestions below.

https://codereview.appspot.com/71490045/diff/40001/cmd/juju/bootstrap.go
> File cmd/juju/bootstrap.go (right):

https://codereview.appspot.com/71490045/diff/40001/cmd/juju/bootstrap.go#newcode119
> cmd/juju/bootstrap.go:119: logger.Warningf("using existing environment
file:
> %q", loc)
> with a slightly shorter variable name, it perhaps seems
> more reasonable inline here.

I don't know what this means I'm afraid.

> Also, given that Location
> is documented to be human readable, perhaps we don't
> need to quote it.

> logger.Warningf("using existing environment at %s", info.Location())

Well, I don't know that how Location() is *documented* should affect how
we treat it. Either the quotes are helpful or they aren't. For copying
and pasting to the shell they *may* be useful (the location could
contain spaces), so I'm inclined to leave them.

> ?

https://codereview.appspot.com/71490045/diff/40001/cmd/juju/bootstrap_test.go
> File cmd/juju/bootstrap_test.go (right):

https://codereview.appspot.com/71490045/diff/40001/cmd/juju/bootstrap_test.go#newcode338
> cmd/juju/bootstrap_test.go:338:
c.Assert((<-opc).(dummy.OpPutFile).Env,
> gc.Equals, "peckham")
> I don't think you need this (and it makes the test slightly more
fragile)

We need to read from *one* of the channels returned by runCommand, or
the command isn't actually run. It maybe that just reading the err
channel is enough, in which case this assert is superfluous. I'll check.

https://codereview.appspot.com/71490045/

[Michael Foord (mfoord)]

Please take a look.

https://codereview.appspot.com/71490045/

[Michael Foord (mfoord)]

Please take a look.

https://codereview.appspot.com/71490045/

[William Reade (fwereade)]

a few comments, but not really enough to block it. thoughts though?

https://codereview.appspot.com/71490045/diff/100001/cmd/juju/bootstrap.go
File cmd/juju/bootstrap.go (right):

https://codereview.appspot.com/71490045/diff/100001/cmd/juju/bootstrap.go#newcode119
cmd/juju/bootstrap.go:119: logger.Warningf("using existing environment
file: %q", loc)
perhaps "ignoring environments.yaml: using bootstrap config in %q"

https://codereview.appspot.com/71490045/diff/100001/environs/configstore/disk.go
File environs/configstore/disk.go (right):

https://codereview.appspot.com/71490045/diff/100001/environs/configstore/disk.go#newcode163
environs/configstore/disk.go:163: func (info *environInfo) Location()
string {
Path()?

https://codereview.appspot.com/71490045/diff/100001/environs/configstore/disk_test.go
File environs/configstore/disk_test.go (right):

https://codereview.appspot.com/71490045/diff/100001/environs/configstore/disk_test.go#newcode113
environs/configstore/disk_test.go:113: c.Assert(info.Location(),
gc.Matches, ".*/someenv.jenv")
if we know the full path, which we should, I'd really like to assert it
specifically

https://codereview.appspot.com/71490045/diff/100001/environs/configstore/mem.go
File environs/configstore/mem.go (right):

https://codereview.appspot.com/71490045/diff/100001/environs/configstore/mem.go#newcode76
environs/configstore/mem.go:76: return "memory"
ah. hmm. I see. I think I'd really rather have Path() (string, error);
return ("", ErrNotADiskStore); and handle that error in the command.

overkill?

https://codereview.appspot.com/71490045/

[Wayne Witzel III (wwitzel3)]

On 2014/03/06 15:17:19, fwereade wrote:
> a few comments, but not really enough to block it. thoughts though?

https://codereview.appspot.com/71490045/diff/100001/cmd/juju/bootstrap.go
> File cmd/juju/bootstrap.go (right):

https://codereview.appspot.com/71490045/diff/100001/cmd/juju/bootstrap.go#newcode119
> cmd/juju/bootstrap.go:119: logger.Warningf("using existing environment
file:
> %q", loc)
> perhaps "ignoring environments.yaml: using bootstrap config in %q"

Updated message to the user and updated Location to return a quoted
string.

https://codereview.appspot.com/71490045/diff/100001/environs/configstore/disk.go
> File environs/configstore/disk.go (right):

https://codereview.appspot.com/71490045/diff/100001/environs/configstore/disk.go#newcode163
> environs/configstore/disk.go:163: func (info *environInfo) Location()
string {
> Path()?

https://codereview.appspot.com/71490045/diff/100001/environs/configstore/disk_test.go
> File environs/configstore/disk_test.go (right):

https://codereview.appspot.com/71490045/diff/100001/environs/configstore/disk_test.go#newcode113
> environs/configstore/disk_test.go:113: c.Assert(info.Location(),
gc.Matches,
> ".*/someenv.jenv")
> if we know the full path, which we should, I'd really like to assert
it
> specifically

Updated test to use full path.

https://codereview.appspot.com/71490045/diff/100001/environs/configstore/mem.go
> File environs/configstore/mem.go (right):

https://codereview.appspot.com/71490045/diff/100001/environs/configstore/mem.go#newcode76
> environs/configstore/mem.go:76: return "memory"
> ah. hmm. I see. I think I'd really rather have Path() (string, error);
return
> ("", ErrNotADiskStore); and handle that error in the command.

> overkill?

https://codereview.appspot.com/71490045/

[Michael Foord (mfoord)]

Please take a look.

https://codereview.appspot.com/71490045/

[Wayne Witzel III (wwitzel3)]

On 2014/03/06 15:37:19, fuzzyman wrote:
> Please take a look.

LGMT

https://codereview.appspot.com/71490045/

[Go Bot (go-bot)]

The attempt to merge lp:~mfoord/juju-core/jenv-warning into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core 0.019s

----------------------------------------------------------------------
PANIC: bootstrap_test.go:29: bootstrapSuite.SetUpSuite

... Panic: MgoSuite tests must be run with MgoTestPackage (PC=0x414011)

/usr/lib/go/src/pkg/runtime/panic.c:229
  in panic
/home/tarmac/trees/src/launchpad.net/juju-core/testing/mgo.go:214
  in MgoSuite.SetUpSuite
bootstrap_test.go:31
  in bootstrapSuite.SetUpSuite
OOPS: 24 passed, 1 FIXTURE-PANICKED, 3 MISSED
--- FAIL: Test (0.01 seconds)
FAIL
FAIL launchpad.net/juju-core/agent 0.210s
ok launchpad.net/juju-core/agent/tools 0.163s

----------------------------------------------------------------------
FAIL: bzr_test.go:26: BzrSuite.SetUpTest

bzr_test.go:28:
    c.Assert(s.b.Init(), gc.IsNil)
... value *errors.errorString = &errors.errorString{s:"error running \"bzr init\": exec: \"bzr\": executable file not found in $PATH"} ("error running \"bzr init\": exec: \"bzr\": executable file not found in $PATH")

----------------------------------------------------------------------
PANIC: bzr_test.go:128: BzrSuite.TestCheckClean

... Panic: Fixture has panicked (see related PANIC)
OOPS: 0 passed, 1 FAILED, 8 MISSED
--- FAIL: Test (0.00 seconds)
FAIL
FAIL launchpad.net/juju-core/bzr 0.013s
ok launchpad.net/juju-core/cert 2.132s
ok launchpad.net/juju-core/charm 0.320s
? launchpad.net/juju-core/charm/hooks [no test files]
? launchpad.net/juju-core/charm/testing [no test files]
ok launchpad.net/juju-core/cloudinit 0.024s
ok launchpad.net/juju-core/cloudinit/sshinit 0.909s
ok launchpad.net/juju-core/cmd 0.159s
&{/tmp/gocheck-8674665223082153551/1 }

----------------------------------------------------------------------
PANIC: deletecharm_test.go:45: DeleteCharmSuite.TestRun

[LOG] 87.28418 INFO juju store: Store opened. Connecting to:
[LOG] 97.85392 ERROR juju store: Error connecting to MongoDB: no reachable servers
[LOG] 97.85401 INFO juju store: Store opened. Connecting to:
[LOG] 8.41398 ERROR juju store: Error connecting to MongoDB: no reachable servers
deletecharm_test.go:66:
    c.Assert(err, gc.IsNil)
... value *errors.errorString = &errors.errorString{s:"no reachable servers"} ("no reachable servers")

... Panic: runtime error: invalid memory address or nil pointer dereference (PC=0x413F61)

/usr/lib/go/src/pkg/runtime/panic.c:229
  in panic
/usr/lib/go/src/pkg/runtime/panic.c:487
  in panicstring
/usr/lib/go/src/pkg/runtime/os_linux.c:236
  in sigpanic
/home/tarmac/trees/src/launchpad.net/juju-core/store/store.go:122
  in Store.Close
/usr/lib/go/src/pkg/runtime/panic.c:184
  in rundefer
/usr/lib/go/src/pkg/runtime/panic.c:493
  in Goexit
/home/tarmac/trees/src/launchpad.net/gocheck/gocheck.go:88
  in C.stopNow
/home/tarmac/trees/src/launchpad.net/gocheck/helpers.go:164
  in C.Assert
deletecharm_test.go:66
  in DeleteCharmSuite.TestRun
OOPS: 2 passed, 1 PANICKED
--- FAIL: Test (21.13 seconds)
FAIL
FAIL launchpad.net/juju-core/cmd/charm-admin 21.276s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/ch...

[Go Bot (go-bot)]

The attempt to merge lp:~mfoord/juju-core/jenv-warning into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core 0.012s
ok launchpad.net/juju-core/agent 1.059s
ok launchpad.net/juju-core/agent/tools 0.171s
ok launchpad.net/juju-core/bzr 4.856s
ok launchpad.net/juju-core/cert 2.444s
ok launchpad.net/juju-core/charm 0.342s
? launchpad.net/juju-core/charm/hooks [no test files]
? launchpad.net/juju-core/charm/testing [no test files]
ok launchpad.net/juju-core/cloudinit 0.025s
ok launchpad.net/juju-core/cloudinit/sshinit 0.799s
ok launchpad.net/juju-core/cmd 0.144s
ok launchpad.net/juju-core/cmd/charm-admin 0.265s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]

----------------------------------------------------------------------
PANIC: bootstrap_test.go:108: BootstrapSuite.TestAllowRetries

[LOG] 97.31167 DEBUG juju.environs.tools no architecture specified when finding tools, looking for any
[LOG] 97.31168 DEBUG juju.environs.tools no series specified when finding tools, looking for any
[LOG] 97.31172 DEBUG juju.environs.simplestreams fetchData failed for "file:///tmp/gocheck-4751997750760398084/0/tools/streams/v1/index.sjson": stat /tmp/gocheck-4751997750760398084/0/tools/streams/v1/index.sjson: no such file or directory
[LOG] 97.31175 DEBUG juju.environs.simplestreams cannot load index "file:///tmp/gocheck-4751997750760398084/0/tools/streams/v1/index.sjson": invalid URL "file:///tmp/gocheck-4751997750760398084/0/tools/streams/v1/index.sjson" not found
[LOG] 97.31177 DEBUG juju.environs.simplestreams fetchData failed for "file:///tmp/gocheck-4751997750760398084/0/tools/streams/v1/index.json": stat /tmp/gocheck-4751997750760398084/0/tools/streams/v1/index.json: no such file or directory
[LOG] 97.31180 DEBUG juju.environs.simplestreams cannot load index "file:///tmp/gocheck-4751997750760398084/0/tools/streams/v1/index.json": invalid URL "file:///tmp/gocheck-4751997750760398084/0/tools/streams/v1/index.json" not found
[LOG] 97.31241 INFO juju.environs.tools Writing tools/streams/v1/index.json
[LOG] 97.31266 INFO juju.environs.tools Writing tools/streams/v1/com.ubuntu.juju:released:tools.json
test 0: no tools uploaded, first check has no retries; no matching binary in source; sync fails with no second attempt

[LOG] 97.31308 INFO juju.provider.dummy reset environment
[LOG] 97.31556 INFO juju Reset successfully reset admin password
[LOG] 97.31600 DEBUG juju.environs.configstore Making /tmp/gocheck-4751997750760398084/1/.juju/environments
clearing private storage
removing files: []
[LOG] 97.44200 DEBUG juju.environs.tools no architecture specified when finding tools, looking for any
[LOG] 97.44202 DEBUG juju.environs.tools no series specified when finding tools, looking for any
[LOG] 97.44205 DEBUG juju.environs.simplestreams fetchData failed for "file:///tmp/gocheck-4751997750760398084/2/tools/streams/v1/index.sjson": stat /tmp/gocheck-4751997750760398084/2/tools/streams/v1/index.sjson: no such file or directory
[LOG] 97.44207 DEBUG juju.environs.simplestreams cannot load index "file:///tmp/gocheck-4751997750760398084/...

[Go Bot (go-bot)]

The attempt to merge lp:~mfoord/juju-core/jenv-warning into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core 0.018s
ok launchpad.net/juju-core/agent 0.964s
ok launchpad.net/juju-core/agent/tools 0.196s

----------------------------------------------------------------------
FAIL: bzr_test.go:26: BzrSuite.SetUpTest

bzr_test.go:28:
    c.Assert(s.b.Init(), gc.IsNil)
... value *errors.errorString = &errors.errorString{s:"error running \"bzr init\": exec: \"bzr\": executable file not found in $PATH"} ("error running \"bzr init\": exec: \"bzr\": executable file not found in $PATH")

----------------------------------------------------------------------
PANIC: bzr_test.go:128: BzrSuite.TestCheckClean

... Panic: Fixture has panicked (see related PANIC)
OOPS: 0 passed, 1 FAILED, 8 MISSED
--- FAIL: Test (0.00 seconds)
FAIL
FAIL launchpad.net/juju-core/bzr 0.012s
ok launchpad.net/juju-core/cert 2.684s
ok launchpad.net/juju-core/charm 0.371s
? launchpad.net/juju-core/charm/hooks [no test files]
? launchpad.net/juju-core/charm/testing [no test files]
ok launchpad.net/juju-core/cloudinit 0.025s
ok launchpad.net/juju-core/cloudinit/sshinit 0.952s
ok launchpad.net/juju-core/cmd 0.164s
ok launchpad.net/juju-core/cmd/charm-admin 0.750s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]

----------------------------------------------------------------------
FAIL: bootstrap_test.go:320: BootstrapSuite.TestBootstrapJenvWarning

[LOG] 60.02503 DEBUG juju.environs.tools no architecture specified when finding tools, looking for any
[LOG] 60.02503 DEBUG juju.environs.tools no series specified when finding tools, looking for any
[LOG] 60.02507 DEBUG juju.environs.simplestreams fetchData failed for "file:///tmp/gocheck-4751997750760398084/21/tools/streams/v1/index.sjson": stat /tmp/gocheck-4751997750760398084/21/tools/streams/v1/index.sjson: no such file or directory
[LOG] 60.02510 DEBUG juju.environs.simplestreams cannot load index "file:///tmp/gocheck-4751997750760398084/21/tools/streams/v1/index.sjson": invalid URL "file:///tmp/gocheck-4751997750760398084/21/tools/streams/v1/index.sjson" not found
[LOG] 60.02512 DEBUG juju.environs.simplestreams fetchData failed for "file:///tmp/gocheck-4751997750760398084/21/tools/streams/v1/index.json": stat /tmp/gocheck-4751997750760398084/21/tools/streams/v1/index.json: no such file or directory
[LOG] 60.02515 DEBUG juju.environs.simplestreams cannot load index "file:///tmp/gocheck-4751997750760398084/21/tools/streams/v1/index.json": invalid URL "file:///tmp/gocheck-4751997750760398084/21/tools/streams/v1/index.json" not found
[LOG] 60.02575 INFO juju.environs.tools Writing tools/streams/v1/index.json
[LOG] 60.02592 INFO juju.environs.tools Writing tools/streams/v1/com.ubuntu.juju:released:tools.json
[LOG] 60.02616 INFO juju.provider.dummy reset environment
[LOG] 60.02827 INFO juju Reset successfully reset admin password
[LOG] 60.02879 DEBUG juju.environs.configstore Making /tmp/gocheck-4751997750760398084/22/.juju/environments
clearing private storage
removing files: []
[LOG] 60.14035 DEBUG juju....

[Go Bot (go-bot)]

The attempt to merge lp:~mfoord/juju-core/jenv-warning into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core 0.013s
ok launchpad.net/juju-core/agent 1.050s
ok launchpad.net/juju-core/agent/tools 0.163s
ok launchpad.net/juju-core/bzr 4.773s
ok launchpad.net/juju-core/cert 2.225s
ok launchpad.net/juju-core/charm 0.338s
? launchpad.net/juju-core/charm/hooks [no test files]
? launchpad.net/juju-core/charm/testing [no test files]
ok launchpad.net/juju-core/cloudinit 0.028s
ok launchpad.net/juju-core/cloudinit/sshinit 0.821s
ok launchpad.net/juju-core/cmd 0.170s
ok launchpad.net/juju-core/cmd/charm-admin 0.734s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]

----------------------------------------------------------------------
FAIL: bootstrap_test.go:320: BootstrapSuite.TestBootstrapJenvWarning

[LOG] 48.74008 DEBUG juju.environs.tools no architecture specified when finding tools, looking for any
[LOG] 48.74009 DEBUG juju.environs.tools no series specified when finding tools, looking for any
[LOG] 48.74012 DEBUG juju.environs.simplestreams fetchData failed for "file:///tmp/gocheck-7504504064263669287/21/tools/streams/v1/index.sjson": stat /tmp/gocheck-7504504064263669287/21/tools/streams/v1/index.sjson: no such file or directory
[LOG] 48.74016 DEBUG juju.environs.simplestreams cannot load index "file:///tmp/gocheck-7504504064263669287/21/tools/streams/v1/index.sjson": invalid URL "file:///tmp/gocheck-7504504064263669287/21/tools/streams/v1/index.sjson" not found
[LOG] 48.74017 DEBUG juju.environs.simplestreams fetchData failed for "file:///tmp/gocheck-7504504064263669287/21/tools/streams/v1/index.json": stat /tmp/gocheck-7504504064263669287/21/tools/streams/v1/index.json: no such file or directory
[LOG] 48.74020 DEBUG juju.environs.simplestreams cannot load index "file:///tmp/gocheck-7504504064263669287/21/tools/streams/v1/index.json": invalid URL "file:///tmp/gocheck-7504504064263669287/21/tools/streams/v1/index.json" not found
[LOG] 48.74074 INFO juju.environs.tools Writing tools/streams/v1/index.json
[LOG] 48.74088 INFO juju.environs.tools Writing tools/streams/v1/com.ubuntu.juju:released:tools.json
[LOG] 48.74114 INFO juju.provider.dummy reset environment
[LOG] 48.74319 INFO juju Reset successfully reset admin password
[LOG] 48.74369 DEBUG juju.environs.configstore Making /tmp/gocheck-7504504064263669287/22/.juju/environments
clearing private storage
removing files: []
[LOG] 48.83331 DEBUG juju.environs ConfigForName found bootstrap config map[string]interface {}{"admin-secret":"arble", "default-series":"raring", "image-metadata-url":"", "logging-config":"<root>=WARNING;juju=DEBUG;unit=DEBUG", "ssl-hostname-verification":true, "state-server":false, "tools-metadata-url":"", "type":"dummy", "authorized-keys":"i-am-a-key", "bootstrap-retry-delay":5, "bootstrap-timeout":600, "broken":"", "ca-cert":"-----BEGIN CERTIFICATE-----\nMIIB2DCCAYSgAwIBAgIBADALBgkqhkiG9w0BAQUwRTENMAsGA1UEChMEanVqdTE0\nMDIGA1UEAwwranVqdS1nZW5lcmF0ZWQgQ0EgZm9yIGVudmlyb25tZW50ICJwZWNr\naGFtIjAeFw0xNDAzMDYxODU0MjlaFw0yNDAzMDYxODU5MjlaMEUxDTALB...

[Michael Foord (mfoord)]

Please take a look.

https://codereview.appspot.com/71490045/

[Roger Peppe (rogpeppe)]

On 2014/03/07 09:52:30, fuzzyman wrote:
> Please take a look.

still LGTM

https://codereview.appspot.com/71490045/

[Go Bot (go-bot)]

The attempt to merge lp:~mfoord/juju-core/jenv-warning into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core 0.013s
ok launchpad.net/juju-core/agent 1.084s
ok launchpad.net/juju-core/agent/mongo 0.591s
ok launchpad.net/juju-core/agent/tools 0.225s
ok launchpad.net/juju-core/bzr 5.111s
ok launchpad.net/juju-core/cert 2.886s
ok launchpad.net/juju-core/charm 0.389s
? launchpad.net/juju-core/charm/hooks [no test files]
? launchpad.net/juju-core/charm/testing [no test files]
ok launchpad.net/juju-core/cloudinit 0.025s
ok launchpad.net/juju-core/cloudinit/sshinit 0.834s
ok launchpad.net/juju-core/cmd 0.166s
ok launchpad.net/juju-core/cmd/charm-admin 0.250s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]

----------------------------------------------------------------------
FAIL: bootstrap_test.go:320: BootstrapSuite.TestBootstrapJenvWarning

[LOG] 3.17739 DEBUG juju.environs.tools no architecture specified when finding tools, looking for any
[LOG] 3.17740 DEBUG juju.environs.tools no series specified when finding tools, looking for any
[LOG] 3.17748 DEBUG juju.environs.simplestreams fetchData failed for "file:///tmp/gocheck-1976235410884491574/21/tools/streams/v1/index.sjson": stat /tmp/gocheck-1976235410884491574/21/tools/streams/v1/index.sjson: no such file or directory
[LOG] 3.17752 DEBUG juju.environs.simplestreams cannot load index "file:///tmp/gocheck-1976235410884491574/21/tools/streams/v1/index.sjson": invalid URL "file:///tmp/gocheck-1976235410884491574/21/tools/streams/v1/index.sjson" not found
[LOG] 3.17756 DEBUG juju.environs.simplestreams fetchData failed for "file:///tmp/gocheck-1976235410884491574/21/tools/streams/v1/index.json": stat /tmp/gocheck-1976235410884491574/21/tools/streams/v1/index.json: no such file or directory
[LOG] 3.17760 DEBUG juju.environs.simplestreams cannot load index "file:///tmp/gocheck-1976235410884491574/21/tools/streams/v1/index.json": invalid URL "file:///tmp/gocheck-1976235410884491574/21/tools/streams/v1/index.json" not found
[LOG] 3.17845 INFO juju.environs.tools Writing tools/streams/v1/index.json
[LOG] 3.17872 INFO juju.environs.tools Writing tools/streams/v1/com.ubuntu.juju:released:tools.json
[LOG] 3.17918 INFO juju.provider.dummy reset environment
[LOG] 3.18227 INFO juju Reset successfully reset admin password
[LOG] 3.18332 DEBUG juju.environs.configstore Making /tmp/gocheck-1976235410884491574/22/.juju/environments
clearing private storage
removing files: []
[LOG] 3.30682 DEBUG juju.environs ConfigForName found bootstrap config map[string]interface {}{"bootstrap-retry-delay":5, "charm-store-auth":"", "development":false, "image-stream":"", "logging-config":"<root>=WARNING;juju=DEBUG;unit=DEBUG", "name":"peckham", "state-id":"33", "syslog-port":6514, "type":"dummy", "admin-secret":"arble", "bootstrap-addresses-delay":10, "broken":"", "firewall-mode":"instance", "ssl-hostname-verification":true, "state-port":37017, "tools-url":"", "api-port":17070, "image-metadata-url":"", "secret":"pork", "authorized-keys":"i-am-a-key", "bootstrap-timeout":600, "ca-cert":"-----BE...

[Go Bot (go-bot)]

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

=== modified file 'CONTRIBUTING'
--- CONTRIBUTING	2014-01-28 04:58:43 +0000
+++ CONTRIBUTING	2014-03-07 12:02:53 +0000
@@ -82,7 +82,7 @@
 
         "labix.org/v2/mgo"
         gc "launchpad.net/gocheck"
-        "github.com/loggo/loggo"
+        "github.com/juju/loggo"
 
         "launchpad.net/juju-core/state"
         "launchpad.net/juju-core/worker"

=== modified file 'agent/mongo/mongo.go'
--- agent/mongo/mongo.go	2014-03-06 20:43:44 +0000
+++ agent/mongo/mongo.go	2014-03-07 12:02:53 +0000
@@ -7,7 +7,7 @@
 	"path"
 	"path/filepath"
 
-	"github.com/loggo/loggo"
+	"github.com/juju/loggo"
 
 	"launchpad.net/juju-core/upstart"
 	"launchpad.net/juju-core/utils"

=== modified file 'cmd/juju/bootstrap.go'
--- cmd/juju/bootstrap.go	2014-02-13 03:16:09 +0000
+++ cmd/juju/bootstrap.go	2014-03-07 12:02:53 +0000
@@ -113,8 +113,9 @@
 		return err
 	}
 	var existing bool
-	if _, err := store.ReadInfo(c.EnvName); !errors.IsNotFoundError(err) {
+	if environInfo, err := store.ReadInfo(c.EnvName); !errors.IsNotFoundError(err) {
 		existing = true
+		logger.Warningf("ignoring environments.yaml: using bootstrap config in %s", environInfo.Location())
 	}
 	environ, err := environs.PrepareFromName(c.EnvName, ctx, store)
 	if err != nil {

=== modified file 'cmd/juju/bootstrap_test.go'
--- cmd/juju/bootstrap_test.go	2014-02-13 02:46:58 +0000
+++ cmd/juju/bootstrap_test.go	2014-03-07 12:02:53 +0000
@@ -8,6 +8,7 @@
 	"fmt"
 	"strings"
 
+	"github.com/juju/loggo"
 	gc "launchpad.net/gocheck"
 
 	"launchpad.net/juju-core/cmd"
@@ -26,6 +27,7 @@
 	"launchpad.net/juju-core/errors"
 	"launchpad.net/juju-core/provider/dummy"
 	coretesting "launchpad.net/juju-core/testing"
+	jc "launchpad.net/juju-core/testing/checkers"
 	"launchpad.net/juju-core/testing/testbase"
 	coretools "launchpad.net/juju-core/tools"
 	"launchpad.net/juju-core/version"
@@ -315,6 +317,27 @@
 	c.Check(coretesting.Stdout(ctx2), gc.Equals, "")
 }
 
+func (s *BootstrapSuite) TestBootstrapJenvWarning(c *gc.C) {
+	env, fake := makeEmptyFakeHome(c)
+	defer fake.Restore()
+	defaultSeriesVersion := version.Current
+	defaultSeriesVersion.Series = env.Config().DefaultSeries()
+
+	store, err := configstore.Default()
+	c.Assert(err, gc.IsNil)
+	ctx := coretesting.Context(c)
+	environs.PrepareFromName("peckham", ctx, store)
+
+	logger := "jenv.warning.test"
+	testWriter := &loggo.TestWriter{}
+	loggo.RegisterWriter(logger, testWriter, loggo.WARNING)
+	defer loggo.RemoveWriter(logger)
+
+	_, errc := runCommand(ctx, new(BootstrapCommand), "-e", "peckham")
+	c.Assert(<-errc, gc.IsNil)
+	c.Assert(testWriter.Log, jc.LogMatches, []string{"ignoring environments.yaml: using bootstrap config in .*"})
+}
+
 func (s *BootstrapSuite) TestInvalidLocalSource(c *gc.C) {
 	s.PatchValue(&version.Current.Number, version.MustParse("1.2.0"))
 	env, fake := makeEmptyFakeHome(c)

=== modified file 'environs/configstore/disk.go'
--- environs/configstore/disk.go	2014-03-05 19:41:34 +0000
+++ environs/configstore/disk.go	2014-03-07 12:02:53 +0000
@@ -159,6 +159,11 @@
 	info.Password = creds.Password
 }
 
+// Location returns the location of the environInfo in human readable format.
+func (info *environInfo) Location() string {
+	return fmt.Sprintf("file %q", info.path)
+}
+
 // Write implements EnvironInfo.Write.
 func (info *environInfo) Write() error {
 	data, err := goyaml.Marshal(info)

=== modified file 'environs/configstore/disk_test.go'
--- environs/configstore/disk_test.go	2013-10-02 04:06:06 +0000
+++ environs/configstore/disk_test.go	2014-03-07 12:02:53 +0000
@@ -110,6 +110,7 @@
 		Addresses: []string{"example.com", "kremvax.ru"},
 		CACert:    "first line\nsecond line",
 	})
+	c.Assert(info.Location(), gc.Equals, fmt.Sprintf("file %q", dir+"/environments/someenv.jenv"))
 	c.Assert(info.BootstrapConfig(), gc.DeepEquals, map[string]interface{}{
 		"secret": "blah",
 		"arble":  "bletch",

=== modified file 'environs/configstore/interface.go'
--- environs/configstore/interface.go	2013-09-26 09:09:29 +0000
+++ environs/configstore/interface.go	2014-03-07 12:02:53 +0000
@@ -74,6 +74,10 @@
 	// associated with the environment.
 	SetAPICredentials(APICredentials)
 
+	// Location returns the location of the source of the environment
+	// information in a human readable format.
+	Location() string
+
 	// Write writes the current information to persistent storage.
 	// A subsequent call to ConfigStorage.ReadInfo
 	// can retrieve it. After this call succeeds, Initialized will return true.

=== modified file 'environs/configstore/mem.go'
--- environs/configstore/mem.go	2013-09-25 16:32:36 +0000
+++ environs/configstore/mem.go	2014-03-07 12:02:53 +0000
@@ -71,6 +71,11 @@
 	return nil, errors.NotFoundf("environment %q", envName)
 }
 
+// Location implements EnvironInfo.Location.
+func (info *memInfo) Location() string {
+	return "memory"
+}
+
 // Write implements EnvironInfo.Write.
 func (info *memInfo) Write() error {
 	m := info.store

=== modified file 'environs/configstore/mem_test.go'
--- environs/configstore/mem_test.go	2013-09-20 10:33:19 +0000
+++ environs/configstore/mem_test.go	2014-03-07 12:02:53 +0000
@@ -20,3 +20,9 @@
 		return configstore.NewMem()
 	}
 }
+
+func (s *memInterfaceSuite) TestMemInfoLocation(c *gc.C) {
+	memStore := configstore.NewMem()
+	memInfo, _ := memStore.CreateInfo("foo")
+	c.Assert(memInfo.Location(), gc.Equals, "memory")
+}