New FindTools API call for upgrades

New FindTools API call for upgrades

The upgrade command was directly instantiating
an environ instance in order to call FindTools.
A new API call is provided to allow this to be
moved to the server side where it belongs. The
existing 1.16 compatibility code has grown a fair
bit to accommodate this change, but is being
removed after 1.18 anyway.

A consequence of this work is a change to how
upgrades internally implement --upload-tools. 
There was an inconsistency in that bootstrap
and sync-tools commands used Prepare, and
upgrade used the env directly from state. Now
all 3 commands share a common implementation,
using Prepare. It is reasonable to expect that 
users running juju upgradde-juju --upload-tools
will have the environ config locally.

Other work involved moving common tools test
code so it can be shared.

https://code.launchpad.net/~wallyworld/juju-core/upgrade-remove-legacy/+merge/209582

Requires: https://code.launchpad.net/~wallyworld/juju-core/enable-major-upgrades/+merge/209378

(do not edit description out of merge proposal)

[wallyworld, 2014-03-06 05:50:42]

Please take a look.

[fwereade, 2014-03-06 15:24:02]

pre-review comment: man, we should have in-env storage, so that clients can
upload tools directly to the env rather than messing around with provider creds.

[mfoord, 2014-03-06 15:39:06]

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

https://codereview.appspot.com/71920043/diff/1/cmd/juju/bootstrap.go#oldcode116
cmd/juju/bootstrap.go:116: if _, err := store.ReadInfo(c.EnvName);
!errors.IsNotFoundError(err) {
We're in the process of landing a branch that warns the user when we use a
pre-existing .jenv file (and so ignore their environments.yaml). We're issuing
the warning in this block of code.

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

This is as a mitigation for launchpad issue: 1282642

[wallyworld, 2014-03-10 00:48:11]

Please take a look.

[wallyworld, 2014-03-11 01:25:08]

Please take a look.

[thumper, 2014-03-11 02:30:35]

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

https://codereview.appspot.com/71920043/diff/40001/cmd/juju/upgradejuju.go#ne...
cmd/juju/upgradejuju.go:179: availableTools, err = findTools1dot17(cfg)
Why 1dot17 and not 1dot16?

https://codereview.appspot.com/71920043/diff/40001/cmd/juju/upgradejuju.go#ne...
cmd/juju/upgradejuju.go:354: log.Warningf("running in 1.16 compatibility mode")
gah... log needs to die.

https://codereview.appspot.com/71920043/diff/40001/environs/tools/testing/tes...
File environs/tools/testing/testing.go (right):

https://codereview.appspot.com/71920043/diff/40001/environs/tools/testing/tes...
environs/tools/testing/testing.go:159: func generateMetadata(c *gc.C, verses
...version.Binary) []metadataFile {
why not versions?

https://codereview.appspot.com/71920043/diff/40001/environs/tools/testing/tes...
environs/tools/testing/testing.go:180: func UploadToStorage(c *gc.C, stor
storage.Storage, verses ...version.Binary) map[version.Binary]string {
ditto

https://codereview.appspot.com/71920043/diff/40001/state/api/params/internal.go
File state/api/params/internal.go (right):

https://codereview.appspot.com/71920043/diff/40001/state/api/params/internal....
state/api/params/internal.go:419: MajorVersion int
what about architecture? surely that matters?

[wallyworld, 2014-03-11 02:57:24]

Please take a look.

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

https://codereview.appspot.com/71920043/diff/40001/cmd/juju/upgradejuju.go#ne...
cmd/juju/upgradejuju.go:179: availableTools, err = findTools1dot17(cfg)
On 2014/03/11 02:30:35, thumper wrote:
> Why 1dot17 and not 1dot16?

The 1.16 code forks off earlier than this. 1.17 has client.EnvironmentGet() and
so gets to here. But only 1.18 has client.FindTools()

https://codereview.appspot.com/71920043/diff/40001/cmd/juju/upgradejuju.go#ne...
cmd/juju/upgradejuju.go:354: log.Warningf("running in 1.16 compatibility mode")
On 2014/03/11 02:30:35, thumper wrote:
> gah... log needs to die.

Cut and paste fail.
Done.

https://codereview.appspot.com/71920043/diff/40001/environs/tools/testing/tes...
File environs/tools/testing/testing.go (right):

https://codereview.appspot.com/71920043/diff/40001/environs/tools/testing/tes...
environs/tools/testing/testing.go:159: func generateMetadata(c *gc.C, verses
...version.Binary) []metadataFile {
On 2014/03/11 02:30:35, thumper wrote:
> why not versions?

Don't know. Code was copied from elsewhere. I guess someone thought vers is the
singular and verses the plural.

https://codereview.appspot.com/71920043/diff/40001/state/api/params/internal.go
File state/api/params/internal.go (right):

https://codereview.appspot.com/71920043/diff/40001/state/api/params/internal....
state/api/params/internal.go:419: MajorVersion int
On 2014/03/11 02:30:35, thumper wrote:
> what about architecture? surely that matters?

This is replicating the behaviour of the current, very old code which finds
tools based on major version number and filters later. I thought we could add
extra params later without breaking the api by appending fields to the struct.
Nonetheless, I added in extra fields even though they are not used by
upgradejuju.

[wallyworld, 2014-03-11 03:02:08]

Please take a look.

[thumper, 2014-03-11 03:48:52]

LGTM

I have a feeling there will be more work soon around finding the appropriate
tools for the cloud, but that is outside the scope of this change.