Improve the image metadata generation command

Improve the image metadata generation command

juju metadata generate-image was originally written
as a developer tool to aid simplestreams development.
It now is useful for setting up private clouds so has
been given a facelift to make it more user friendly.
The key feature is it will now use the current environment
to pick up most of what it needs, and the user just has to
supply the image id and architecture (if something different
to amd64 is required).

https://code.launchpad.net/~wallyworld/juju-core/improve-image-metadata-command/+merge/190517

(do not edit description out of merge proposal)

[wallyworld, 2013-10-11 01:26:14]

Please take a look.

[axw, 2013-10-15 07:08:33]

https://codereview.appspot.com/14502059/diff/1/cmd/plugins/juju-metadata/imag...
File cmd/plugins/juju-metadata/imagemetadata.go (right):

https://codereview.appspot.com/14502059/diff/1/cmd/plugins/juju-metadata/imag...
cmd/plugins/juju-metadata/imagemetadata.go:72: return fmt.Errorf("environment %q
cannot provide region and endpoint", environ.Name())
What if the user has specified -s, -r, and -e on the command line? Is this
really an error then?

Just a thought: perhaps ImageMetadataCommand could implement HasRegion. If
environ doesn't implement HasRegion, fall back to ImageMetadataCommand.

https://codereview.appspot.com/14502059/diff/1/cmd/plugins/juju-metadata/imag...
File cmd/plugins/juju-metadata/imagemetadata_test.go (right):

https://codereview.appspot.com/14502059/diff/1/cmd/plugins/juju-metadata/imag...
cmd/plugins/juju-metadata/imagemetadata_test.go:20: "path/filepath"
Move me please

https://codereview.appspot.com/14502059/diff/1/cmd/plugins/juju-metadata/imag...
cmd/plugins/juju-metadata/imagemetadata_test.go:42: restore :=
testbase.PatchEnvironment("AWS_ACCESS_KEY_ID", "access")
You can just use s.PatchEnvironment here, which wraps testbase.PatchEnvironment
& AddCleanup.

[wallyworld, 2013-10-16 02:57:49]

Please take a look.

https://codereview.appspot.com/14502059/diff/1/cmd/plugins/juju-metadata/imag...
File cmd/plugins/juju-metadata/imagemetadata.go (right):

https://codereview.appspot.com/14502059/diff/1/cmd/plugins/juju-metadata/imag...
cmd/plugins/juju-metadata/imagemetadata.go:72: return fmt.Errorf("environment %q
cannot provide region and endpoint", environ.Name())
On 2013/10/15 07:08:34, axw wrote:
> What if the user has specified -s, -r, and -e on the command line? Is this
> really an error then?
> 

Hmmm. I thought yes when I wrote this. But I changed it so that it works more as
expected.

> Just a thought: perhaps ImageMetadataCommand could implement HasRegion. If
> environ doesn't implement HasRegion, fall back to ImageMetadataCommand.

See my changes - I think it's ok as is.

https://codereview.appspot.com/14502059/diff/1/cmd/plugins/juju-metadata/imag...
File cmd/plugins/juju-metadata/imagemetadata_test.go (right):

https://codereview.appspot.com/14502059/diff/1/cmd/plugins/juju-metadata/imag...
cmd/plugins/juju-metadata/imagemetadata_test.go:20: "path/filepath"
On 2013/10/15 07:08:34, axw wrote:
> Move me please

Done.

https://codereview.appspot.com/14502059/diff/1/cmd/plugins/juju-metadata/imag...
cmd/plugins/juju-metadata/imagemetadata_test.go:42: restore :=
testbase.PatchEnvironment("AWS_ACCESS_KEY_ID", "access")
On 2013/10/15 07:08:34, axw wrote:
> You can just use s.PatchEnvironment here, which wraps
testbase.PatchEnvironment
> & AddCleanup.

Done.

[axw, 2013-10-16 03:13:39]

On 2013/10/16 02:57:49, wallyworld wrote:
> Please take a look.
> 
>
https://codereview.appspot.com/14502059/diff/1/cmd/plugins/juju-metadata/imag...
> File cmd/plugins/juju-metadata/imagemetadata.go (right):
> 
>
https://codereview.appspot.com/14502059/diff/1/cmd/plugins/juju-metadata/imag...
> cmd/plugins/juju-metadata/imagemetadata.go:72: return fmt.Errorf("environment
%q
> cannot provide region and endpoint", environ.Name())
> On 2013/10/15 07:08:34, axw wrote:
> > What if the user has specified -s, -r, and -e on the command line? Is this
> > really an error then?
> > 
> 
> Hmmm. I thought yes when I wrote this. But I changed it so that it works more
as
> expected.
> 
> > Just a thought: perhaps ImageMetadataCommand could implement HasRegion. If
> > environ doesn't implement HasRegion, fall back to ImageMetadataCommand.
> 
> See my changes - I think it's ok as is.
> 
>
https://codereview.appspot.com/14502059/diff/1/cmd/plugins/juju-metadata/imag...
> File cmd/plugins/juju-metadata/imagemetadata_test.go (right):
> 
>
https://codereview.appspot.com/14502059/diff/1/cmd/plugins/juju-metadata/imag...
> cmd/plugins/juju-metadata/imagemetadata_test.go:20: "path/filepath"
> On 2013/10/15 07:08:34, axw wrote:
> > Move me please
> 
> Done.
> 
>
https://codereview.appspot.com/14502059/diff/1/cmd/plugins/juju-metadata/imag...
> cmd/plugins/juju-metadata/imagemetadata_test.go:42: restore :=
> testbase.PatchEnvironment("AWS_ACCESS_KEY_ID", "access")
> On 2013/10/15 07:08:34, axw wrote:
> > You can just use s.PatchEnvironment here, which wraps
> testbase.PatchEnvironment
> > & AddCleanup.
> 
> Done.

Thanks, LGTM