juju-core

Merge lp:~wallyworld/juju-core/merge-new-image-metadata into lp:~go-bot/juju-core/trunk

  1. merge-new-image-metadata
  2. Merge into trunk
Proposed by Ian Booth
Status: Merged
Approved by: Ian Booth
Approved revision: no longer in the source branch.
Merged at revision: 1994
Proposed branch: lp:~wallyworld/juju-core/merge-new-image-metadata
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 1144 lines (+501/-212)
11 files modified
cmd/plugins/juju-metadata/imagemetadata.go (+8/-6)
cmd/plugins/juju-metadata/imagemetadata_test.go (+18/-19)
cmd/plugins/juju-metadata/validateimagemetadata_test.go (+4/-6)
environs/imagemetadata/generate.go (+84/-30)
environs/imagemetadata/generate_test.go (+173/-7)
environs/imagemetadata/marshal.go (+16/-22)
environs/imagemetadata/marshal_test.go (+9/-6)
environs/imagemetadata/simplestreams.go (+28/-19)
environs/imagemetadata/simplestreams_test.go (+138/-84)
environs/imagemetadata/validation_test.go (+6/-4)
environs/simplestreams/simplestreams.go (+17/-9)
To merge this branch: bzr merge lp:~wallyworld/juju-core/merge-new-image-metadata
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+190898@code.launchpad.net

Commit message

Merge new image metadata with existing

When image id metadata is generated, any existing metadata is
now loaded and then the new metadata is merged in. This allows
the generate-image plugin to be run multiple times to build up
metadata for multiple images. Some fairly extensive refactoring
to how image metadata search constraints are set up was required.

https://codereview.appspot.com/14663043/

Description of the change

Merge new image metadata with existing

When image id metadata is generated, any existing metadata is
now loaded and then the new metadata is merged in. This allows
the generate-image plugin to be run multiple times to build up
metadata for multiple images. Some fairly extensive refactoring
to how image metadata search constraints are set up was required.

https://codereview.appspot.com/14663043/

To post a comment you must log in.
Revision history for this message
Ian Booth (wallyworld) wrote :

Reviewers: mp+190898_code.launchpad.net,

Message:
Please take a look.

Description:
Merge new image metadata with existing

When image id metadata is generated, any existing metadata is
now loaded and then the new metadata is merged in. This allows
the generate-image plugin to be run multiple times to build up
metadata for multiple images. Some fairly extensive refactoring
to how image metadata search constraints are set up was required.

https://code.launchpad.net/~wallyworld/juju-core/merge-new-image-metadata/+merge/190898

(do not edit description out of merge proposal)

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

Affected files (+843, -367 lines):
   A [revision details]
   M cmd/plugins/juju-metadata/imagemetadata.go
   M cmd/plugins/juju-metadata/imagemetadata_test.go
   M cmd/plugins/juju-metadata/validateimagemetadata_test.go
   M environs/imagemetadata/generate.go
   A environs/imagemetadata/generate_test.go
   A environs/imagemetadata/marshal.go
   A environs/imagemetadata/marshal_test.go
   M environs/imagemetadata/simplestreams.go
   M environs/imagemetadata/simplestreams_test.go
   A environs/imagemetadata/testing/testing.go
   M environs/imagemetadata/validation_test.go
   M environs/simplestreams/simplestreams.go
   M environs/tools/marshal.go

Revision history for this message
Ian Booth (wallyworld) wrote :

Please take a look.

https://codereview.appspot.com/14663043/

Revision history for this message
Tim Penhey (thumper) wrote :
Download full text (5.6 KiB)

https://codereview.appspot.com/14663043/diff/4001/cmd/plugins/juju-metadata/imagemetadata.go
File cmd/plugins/juju-metadata/imagemetadata.go (right):

https://codereview.appspot.com/14663043/diff/4001/cmd/plugins/juju-metadata/imagemetadata.go#newcode64
cmd/plugins/juju-metadata/imagemetadata.go:64: if store, err :=
configstore.Default(); err == nil {
What should we be doing if there is no configstore? Is it an error?

It seems that either we allow to use to say "don't use an environment"
or we should fail if we can't open the environment defined by the
command.

https://codereview.appspot.com/14663043/diff/4001/cmd/plugins/juju-metadata/imagemetadata.go#newcode131
cmd/plugins/juju-metadata/imagemetadata.go:131: %q
why are the directories quoted?

https://codereview.appspot.com/14663043/diff/4001/cmd/plugins/juju-metadata/imagemetadata_test.go
File cmd/plugins/juju-metadata/imagemetadata_test.go (right):

https://codereview.appspot.com/14663043/diff/4001/cmd/plugins/juju-metadata/imagemetadata_test.go#newcode39
cmd/plugins/juju-metadata/imagemetadata_test.go:39: os.Clearenv()
Why clear the entire environment?

https://codereview.appspot.com/14663043/diff/4001/cmd/plugins/juju-metadata/imagemetadata_test.go#newcode42
cmd/plugins/juju-metadata/imagemetadata_test.go:42: restore :=
testbase.PatchEnvironment("AWS_ACCESS_KEY_ID", "access")
just use:

s.PatchEnvironment("AWS_ACCESS_KEY_ID", "access")

This adds the restore cleanup for you.

https://codereview.appspot.com/14663043/diff/4001/cmd/plugins/juju-metadata/imagemetadata_test.go#newcode49
cmd/plugins/juju-metadata/imagemetadata_test.go:49: for _, envstring :=
range s.environ {
I'd be tempted to make this a function by itself, and call "addCleanup"
just after the os.Clearenv().

Adding the home.Restore() as a cleanup also means you don't need a
TestTearDown.

https://codereview.appspot.com/14663043/diff/4001/cmd/plugins/juju-metadata/imagemetadata_test.go#newcode88
cmd/plugins/juju-metadata/imagemetadata_test.go:88:
c.Assert(strings.Contains(content, fmt.Sprintf(`"region": %q`,
expected.region)), jc.IsTrue)
Do you realise that we have a checker for Contains?

c.Assert(content, jc.Contains, fmt.Sprintf(`"region": %q`,
expected.region))

https://codereview.appspot.com/14663043/diff/4001/cmd/plugins/juju-metadata/imagemetadata_test.go#newcode129
cmd/plugins/juju-metadata/imagemetadata_test.go:129: errOut :=
ctx.Stdout.(*bytes.Buffer).String()
there is also testing.Stdout(ctx) which does the horrible cast for you.

https://codereview.appspot.com/14663043/diff/4001/cmd/plugins/juju-metadata/validateimagemetadata_test.go
File cmd/plugins/juju-metadata/validateimagemetadata_test.go (right):

https://codereview.appspot.com/14663043/diff/4001/cmd/plugins/juju-metadata/validateimagemetadata_test.go#newcode101
cmd/plugins/juju-metadata/validateimagemetadata_test.go:101:
s.AddCleanup(func(*gc.C) { restore() })
s.PatchEnvironment("AWS_ACCESS_KEY_ID", "access")

https://codereview.appspot.com/14663043/diff/4001/cmd/plugins/juju-metadata/validateimagemetadata_test.go#newcode133
cmd/plugins/juju-metadata/validateimagemetadata_test.go:133:
testbase.PatchEnvironment("AWS_ACCESS_KEY_ID", "")
s.PatchEnvironment...

Read more...

Revision history for this message
Ian Booth (wallyworld) wrote :
Download full text (6.7 KiB)

https://codereview.appspot.com/14663043/diff/4001/cmd/plugins/juju-metadata/imagemetadata.go
File cmd/plugins/juju-metadata/imagemetadata.go (right):

https://codereview.appspot.com/14663043/diff/4001/cmd/plugins/juju-metadata/imagemetadata.go#newcode64
cmd/plugins/juju-metadata/imagemetadata.go:64: if store, err :=
configstore.Default(); err == nil {
On 2013/10/16 00:47:59, thumper wrote:
> What should we be doing if there is no configstore? Is it an error?

No. It just means that the user wants to run the command without an
environment. It is not an error as far as this command is concerned.

> It seems that either we allow to use to say "don't use an environment"
or we
> should fail if we can't open the environment defined by the command.

I'd rather keep the semantics simple for the user. A written, the
command works quite nicely without an environment, so long as all the
parameters are specified. But it does complain if an env name is
specified and the env cannot be opened.

https://codereview.appspot.com/14663043/diff/4001/cmd/plugins/juju-metadata/imagemetadata.go#newcode131
cmd/plugins/juju-metadata/imagemetadata.go:131: %q
On 2013/10/16 00:47:59, thumper wrote:
> why are the directories quoted?

Seemed like a good idea. I can unquote.

https://codereview.appspot.com/14663043/diff/4001/cmd/plugins/juju-metadata/imagemetadata_test.go
File cmd/plugins/juju-metadata/imagemetadata_test.go (right):

https://codereview.appspot.com/14663043/diff/4001/cmd/plugins/juju-metadata/imagemetadata_test.go#newcode39
cmd/plugins/juju-metadata/imagemetadata_test.go:39: os.Clearenv()
On 2013/10/16 00:47:59, thumper wrote:
> Why clear the entire environment?

We don't need *any* env vars set for the test and there's so many
combinations of AWS and EC2 env vars it's just easier.

https://codereview.appspot.com/14663043/diff/4001/cmd/plugins/juju-metadata/imagemetadata_test.go#newcode42
cmd/plugins/juju-metadata/imagemetadata_test.go:42: restore :=
testbase.PatchEnvironment("AWS_ACCESS_KEY_ID", "access")
On 2013/10/16 00:47:59, thumper wrote:
> just use:

> s.PatchEnvironment("AWS_ACCESS_KEY_ID", "access")

> This adds the restore cleanup for you.

will do

https://codereview.appspot.com/14663043/diff/4001/cmd/plugins/juju-metadata/imagemetadata_test.go#newcode88
cmd/plugins/juju-metadata/imagemetadata_test.go:88:
c.Assert(strings.Contains(content, fmt.Sprintf(`"region": %q`,
expected.region)), jc.IsTrue)
On 2013/10/16 00:47:59, thumper wrote:
> Do you realise that we have a checker for Contains?

> c.Assert(content, jc.Contains, fmt.Sprintf(`"region": %q`,
expected.region))

I do now

https://codereview.appspot.com/14663043/diff/4001/cmd/plugins/juju-metadata/imagemetadata_test.go#newcode129
cmd/plugins/juju-metadata/imagemetadata_test.go:129: errOut :=
ctx.Stdout.(*bytes.Buffer).String()
On 2013/10/16 00:47:59, thumper wrote:
> there is also testing.Stdout(ctx) which does the horrible cast for
 you.

\o/

https://codereview.appspot.com/14663043/diff/4001/cmd/plugins/juju-metadata/validateimagemetadata_test.go
File cmd/plugins/juju-metadata/validateimagemetadata_test.go (right):

https://codereview.appspot.com/14663043/diff/4001/cmd/plugins/juju-metadata...

Read more...

Revision history for this message
Ian Booth (wallyworld) wrote :

Please take a look.

https://codereview.appspot.com/14663043/

Revision history for this message
Ian Booth (wallyworld) wrote :

Please take a look.

https://codereview.appspot.com/14663043/

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

LGTM

https://codereview.appspot.com/14663043/

Revision history for this message
Go Bot (go-bot) wrote :
Download full text (6.9 KiB)

The attempt to merge lp:~wallyworld/juju-core/merge-new-image-metadata into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core/agent 1.264s
ok launchpad.net/juju-core/agent/tools 0.259s
ok launchpad.net/juju-core/bzr 7.130s
ok launchpad.net/juju-core/cert 2.848s
ok launchpad.net/juju-core/charm 0.613s
? launchpad.net/juju-core/charm/hooks [no test files]
ok launchpad.net/juju-core/cloudinit 0.031s
ok launchpad.net/juju-core/cmd 0.228s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/juju 171.617s
ok launchpad.net/juju-core/cmd/jujud 48.162s
FAIL launchpad.net/juju-core/cmd/plugins/juju-metadata [build failed]
ok launchpad.net/juju-core/constraints 0.043s
ok launchpad.net/juju-core/container/lxc 0.348s
? launchpad.net/juju-core/container/lxc/mock [no test files]
ok launchpad.net/juju-core/downloader 5.296s
ok launchpad.net/juju-core/environs 3.600s
ok launchpad.net/juju-core/environs/bootstrap 5.091s
ok launchpad.net/juju-core/environs/cloudinit 0.500s
ok launchpad.net/juju-core/environs/config 0.788s
ok launchpad.net/juju-core/environs/configstore 0.045s
ok launchpad.net/juju-core/environs/filestorage 0.032s
ok launchpad.net/juju-core/environs/httpstorage 0.880s
ok launchpad.net/juju-core/environs/imagemetadata 0.532s
? launchpad.net/juju-core/environs/imagemetadata/testing [no test files]
ok launchpad.net/juju-core/environs/instances 0.052s
ok launchpad.net/juju-core/environs/jujutest 0.259s
ok launchpad.net/juju-core/environs/manual 4.176s
ok launchpad.net/juju-core/environs/simplestreams 0.385s
? launchpad.net/juju-core/environs/simplestreams/testing [no test files]
ok launchpad.net/juju-core/environs/sshstorage 1.125s
ok launchpad.net/juju-core/environs/storage 1.118s
ok launchpad.net/juju-core/environs/sync 28.576s
ok launchpad.net/juju-core/environs/testing 0.200s
ok launchpad.net/juju-core/environs/tools 7.098s
? launchpad.net/juju-core/environs/tools/testing [no test files]
ok launchpad.net/juju-core/errors 0.016s
ok launchpad.net/juju-core/instance 0.023s
? launchpad.net/juju-core/instance/testing [no test files]
ok launchpad.net/juju-core/juju 17.758s
ok launchpad.net/juju-core/juju/osenv 0.018s
? launchpad.net/juju-core/juju/testing [no test files]
ok launchpad.net/juju-core/log 0.016s
ok launchpad.net/juju-core/log/syslog 0.020s
ok launchpad.net/juju-core/names 0.023s
? launchpad.net/juju-core/provider [no test files]
? launchpad.net/juju-core/provider/all [no test files]
ok launchpad.net/juju-core/provider/azure 6.727s
ok launchpad.net/juju-core/provider/common 0.263s
ok launchpad.net/juju-core/provider/dummy 22.424s
ok launchpad.net/juju-core/provider/ec2 5.660s
ok launchpad.net/juju-core/provider/ec2/httpstorage 0.200s
ok launchpad.net/juju-core/provider/local 2.251s
ok launchpad.net/juju-core/provider/maas 10.070s
ok launchpad.net/juju-core/provider/null 1.205s
ok launchpad.net/juju-core/provider/openstack 13.886s
ok launchpad.net/juju-core/rpc 0.078s
ok launchpad.net/juju...

Read more...

Revision history for this message
Go Bot (go-bot) wrote :
Download full text (19.2 KiB)

The attempt to merge lp:~wallyworld/juju-core/merge-new-image-metadata into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core/agent 0.914s
ok launchpad.net/juju-core/agent/tools 0.255s
ok launchpad.net/juju-core/bzr 7.287s
ok launchpad.net/juju-core/cert 3.762s
ok launchpad.net/juju-core/charm 0.576s
? launchpad.net/juju-core/charm/hooks [no test files]
ok launchpad.net/juju-core/cloudinit 0.027s
ok launchpad.net/juju-core/cmd 0.275s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/juju 173.522s
ok launchpad.net/juju-core/cmd/jujud 48.185s
ok launchpad.net/juju-core/cmd/plugins/juju-metadata 2.549s
ok launchpad.net/juju-core/constraints 0.032s
ok launchpad.net/juju-core/container/lxc 0.324s
? launchpad.net/juju-core/container/lxc/mock [no test files]
ok launchpad.net/juju-core/downloader 5.305s
ok launchpad.net/juju-core/environs 3.236s
ok launchpad.net/juju-core/environs/bootstrap 5.048s
ok launchpad.net/juju-core/environs/cloudinit 0.521s
ok launchpad.net/juju-core/environs/config 0.858s
ok launchpad.net/juju-core/environs/configstore 0.041s
ok launchpad.net/juju-core/environs/filestorage 0.040s
ok launchpad.net/juju-core/environs/httpstorage 0.913s
ok launchpad.net/juju-core/environs/imagemetadata 0.491s
? launchpad.net/juju-core/environs/imagemetadata/testing [no test files]
ok launchpad.net/juju-core/environs/instances 0.055s
ok launchpad.net/juju-core/environs/jujutest 0.253s
ok launchpad.net/juju-core/environs/manual 4.230s
ok launchpad.net/juju-core/environs/simplestreams 0.338s
? launchpad.net/juju-core/environs/simplestreams/testing [no test files]
ok launchpad.net/juju-core/environs/sshstorage 1.218s
ok launchpad.net/juju-core/environs/storage 1.177s
ok launchpad.net/juju-core/environs/sync 29.731s
ok launchpad.net/juju-core/environs/testing 0.224s
ok launchpad.net/juju-core/environs/tools 7.134s
? launchpad.net/juju-core/environs/tools/testing [no test files]
ok launchpad.net/juju-core/errors 0.017s
ok launchpad.net/juju-core/instance 0.024s
? launchpad.net/juju-core/instance/testing [no test files]
ok launchpad.net/juju-core/juju 17.834s
ok launchpad.net/juju-core/juju/osenv 0.028s
? launchpad.net/juju-core/juju/testing [no test files]
ok launchpad.net/juju-core/log 0.016s
ok launchpad.net/juju-core/log/syslog 0.031s
ok launchpad.net/juju-core/names 0.025s
? launchpad.net/juju-core/provider [no test files]
? launchpad.net/juju-core/provider/all [no test files]
ok launchpad.net/juju-core/provider/azure 6.501s
ok launchpad.net/juju-core/provider/common 0.327s
ok launchpad.net/juju-core/provider/dummy 21.903s
ok launchpad.net/juju-core/provider/ec2 5.529s
ok launchpad.net/juju-core/provider/ec2/httpstorage 0.224s
ok launchpad.net/juju-core/provider/local 2.309s
ok launchpad.net/juju-core/provider/maas 10.245s
ok launchpad.net/juju-core/provider/null 1.246s
ok launchpad.net/juju-core/provider/openstack 13.733s
ok launchpad.net/juju-core/rpc 0.087s
ok launchpad.net/juju-core/rp...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'cmd/plugins/juju-metadata/imagemetadata.go'
2--- cmd/plugins/juju-metadata/imagemetadata.go 2013-10-16 03:31:36 +0000
3+++ cmd/plugins/juju-metadata/imagemetadata.go 2013-10-17 05:10:43 +0000
4@@ -17,6 +17,7 @@
5 "launchpad.net/juju-core/environs/filestorage"
6 "launchpad.net/juju-core/environs/imagemetadata"
7 "launchpad.net/juju-core/environs/simplestreams"
8+ "launchpad.net/juju-core/utils"
9 )
10
11 // ImageMetadataCommand is used to write out simplestreams image metadata information.
12@@ -125,17 +126,17 @@
13
14 var helpDoc = `
15 image metadata files have been written to:
16-%q.
17+%s.
18 For Juju to use this metadata, the files need to be put into the
19 image metadata search path. There are 2 options:
20
21 1. Use tools-url in $JUJU_HOME/environments.yaml
22 Configure a http server to serve the contents of
23-%q
24+%s
25 and set the value of tools-url accordingly.
26
27 2. Upload the contents of
28-%q
29+%s
30 to your cloud's private storage (for ec2 and openstack).
31 eg for openstack
32 "cd %s; swift upload %s streams/v1/*"
33@@ -145,7 +146,7 @@
34 func (c *ImageMetadataCommand) Run(context *cmd.Context) error {
35 out := context.Stdout
36
37- im := imagemetadata.ImageMetadata{
38+ im := &imagemetadata.ImageMetadata{
39 Id: c.ImageId,
40 Arch: c.Arch,
41 }
42@@ -157,11 +158,12 @@
43 if err != nil {
44 return err
45 }
46- err = imagemetadata.WriteMetadata(c.Series, &im, &cloudSpec, targetStorage)
47+ err = imagemetadata.MergeAndWriteMetadata(c.Series, []*imagemetadata.ImageMetadata{im}, &cloudSpec, targetStorage)
48 if err != nil {
49 return fmt.Errorf("image metadata files could not be created: %v", err)
50 }
51 dest := filepath.Join(c.Dir, "streams", "v1")
52- fmt.Fprintf(out, fmt.Sprintf(helpDoc, dest, c.Dir, c.Dir, c.Dir, c.privateStorage))
53+ dir := utils.NormalizePath(c.Dir)
54+ fmt.Fprintf(out, fmt.Sprintf(helpDoc, dest, dir, dir, dir, c.privateStorage))
55 return nil
56 }
57
58=== modified file 'cmd/plugins/juju-metadata/imagemetadata_test.go'
59--- cmd/plugins/juju-metadata/imagemetadata_test.go 2013-10-16 03:31:36 +0000
60+++ cmd/plugins/juju-metadata/imagemetadata_test.go 2013-10-17 05:10:43 +0000
61@@ -4,7 +4,6 @@
62 package main
63
64 import (
65- "bytes"
66 "encoding/json"
67 "fmt"
68 "io/ioutil"
69@@ -89,10 +88,10 @@
70 c.Assert(err, gc.IsNil)
71 c.Assert(indices.(map[string]interface{})["format"], gc.Equals, "index:1.0")
72 prodId := fmt.Sprintf("com.ubuntu.cloud:server:%s:%s", seriesVersions[expected.series], expected.arch)
73- c.Assert(strings.Contains(content, prodId), jc.IsTrue)
74- c.Assert(strings.Contains(content, fmt.Sprintf(`"region": %q`, expected.region)), jc.IsTrue)
75- c.Assert(strings.Contains(content, fmt.Sprintf(`"endpoint": %q`, expected.endpoint)), jc.IsTrue)
76- c.Assert(strings.Contains(content, fmt.Sprintf(`"path": "streams/v1/%s"`, imageFileName)), jc.IsTrue)
77+ c.Assert(content, jc.Contains, prodId)
78+ c.Assert(content, jc.Contains, fmt.Sprintf(`"region": %q`, expected.region))
79+ c.Assert(content, jc.Contains, fmt.Sprintf(`"endpoint": %q`, expected.endpoint))
80+ c.Assert(content, jc.Contains, fmt.Sprintf(`"path": "streams/v1/%s"`, imageFileName))
81
82 imagepath := filepath.Join(s.dir, "streams", "v1", imageFileName)
83 data, err = ioutil.ReadFile(imagepath)
84@@ -102,8 +101,8 @@
85 err = json.Unmarshal(data, &images)
86 c.Assert(err, gc.IsNil)
87 c.Assert(images.(map[string]interface{})["format"], gc.Equals, "products:1.0")
88- c.Assert(strings.Contains(content, prodId), gc.Equals, true)
89- c.Assert(strings.Contains(content, `"id": "1234"`), gc.Equals, true)
90+ c.Assert(content, jc.Contains, prodId)
91+ c.Assert(content, jc.Contains, `"id": "1234"`)
92 }
93
94 const (
95@@ -117,12 +116,12 @@
96 &ImageMetadataCommand{}, ctx, []string{
97 "-d", s.dir, "-i", "1234", "-r", "region", "-a", "arch", "-u", "endpoint", "-s", "raring"})
98 c.Assert(code, gc.Equals, 0)
99- errOut := ctx.Stdout.(*bytes.Buffer).String()
100+ out := testing.Stdout(ctx)
101 expected := expectedMetadata{
102 series: "raring",
103 arch: "arch",
104 }
105- s.assertCommandOutput(c, expected, errOut, defaultIndexFileName, defaultImageFileName)
106+ s.assertCommandOutput(c, expected, out, defaultIndexFileName, defaultImageFileName)
107 }
108
109 func (s *ImageMetadataSuite) TestImageMetadataFilesDefaultArch(c *gc.C) {
110@@ -131,12 +130,12 @@
111 &ImageMetadataCommand{}, ctx, []string{
112 "-d", s.dir, "-i", "1234", "-r", "region", "-u", "endpoint", "-s", "raring"})
113 c.Assert(code, gc.Equals, 0)
114- errOut := ctx.Stdout.(*bytes.Buffer).String()
115+ out := testing.Stdout(ctx)
116 expected := expectedMetadata{
117 series: "raring",
118 arch: "amd64",
119 }
120- s.assertCommandOutput(c, expected, errOut, defaultIndexFileName, defaultImageFileName)
121+ s.assertCommandOutput(c, expected, out, defaultIndexFileName, defaultImageFileName)
122 }
123
124 func (s *ImageMetadataSuite) TestImageMetadataFilesDefaultSeries(c *gc.C) {
125@@ -145,12 +144,12 @@
126 &ImageMetadataCommand{}, ctx, []string{
127 "-d", s.dir, "-i", "1234", "-r", "region", "-a", "arch", "-u", "endpoint"})
128 c.Assert(code, gc.Equals, 0)
129- errOut := ctx.Stdout.(*bytes.Buffer).String()
130+ out := testing.Stdout(ctx)
131 expected := expectedMetadata{
132 series: "precise",
133 arch: "arch",
134 }
135- s.assertCommandOutput(c, expected, errOut, defaultIndexFileName, defaultImageFileName)
136+ s.assertCommandOutput(c, expected, out, defaultIndexFileName, defaultImageFileName)
137 }
138
139 func (s *ImageMetadataSuite) TestImageMetadataFilesUsingEnv(c *gc.C) {
140@@ -158,14 +157,14 @@
141 code := cmd.Main(
142 &ImageMetadataCommand{}, ctx, []string{"-d", s.dir, "-e", "ec2", "-i", "1234"})
143 c.Assert(code, gc.Equals, 0)
144- errOut := ctx.Stdout.(*bytes.Buffer).String()
145+ out := testing.Stdout(ctx)
146 expected := expectedMetadata{
147 series: "precise",
148 arch: "amd64",
149 region: "us-east-1",
150 endpoint: "https://ec2.us-east-1.amazonaws.com",
151 }
152- s.assertCommandOutput(c, expected, errOut, defaultIndexFileName, defaultImageFileName)
153+ s.assertCommandOutput(c, expected, out, defaultIndexFileName, defaultImageFileName)
154 }
155
156 func (s *ImageMetadataSuite) TestImageMetadataFilesUsingEnvWithRegionOverride(c *gc.C) {
157@@ -174,14 +173,14 @@
158 &ImageMetadataCommand{}, ctx, []string{
159 "-d", s.dir, "-e", "ec2", "-r", "us-west-1", "-u", "https://ec2.us-west-1.amazonaws.com", "-i", "1234"})
160 c.Assert(code, gc.Equals, 0)
161- errOut := ctx.Stdout.(*bytes.Buffer).String()
162+ out := testing.Stdout(ctx)
163 expected := expectedMetadata{
164 series: "precise",
165 arch: "amd64",
166 region: "us-west-1",
167 endpoint: "https://ec2.us-west-1.amazonaws.com",
168 }
169- s.assertCommandOutput(c, expected, errOut, defaultIndexFileName, defaultImageFileName)
170+ s.assertCommandOutput(c, expected, out, defaultIndexFileName, defaultImageFileName)
171 }
172
173 func (s *ImageMetadataSuite) TestImageMetadataFilesUsingEnvWithNoHasRegion(c *gc.C) {
174@@ -190,14 +189,14 @@
175 &ImageMetadataCommand{}, ctx, []string{
176 "-d", s.dir, "-e", "azure", "-r", "region", "-u", "endpoint", "-i", "1234"})
177 c.Assert(code, gc.Equals, 0)
178- errOut := ctx.Stdout.(*bytes.Buffer).String()
179+ out := testing.Stdout(ctx)
180 expected := expectedMetadata{
181 series: "raring",
182 arch: "amd64",
183 region: "region",
184 endpoint: "endpoint",
185 }
186- s.assertCommandOutput(c, expected, errOut, defaultIndexFileName, defaultImageFileName)
187+ s.assertCommandOutput(c, expected, out, defaultIndexFileName, defaultImageFileName)
188 }
189
190 type errTestParams struct {
191
192=== modified file 'cmd/plugins/juju-metadata/validateimagemetadata_test.go'
193--- cmd/plugins/juju-metadata/validateimagemetadata_test.go 2013-10-17 04:53:59 +0000
194+++ cmd/plugins/juju-metadata/validateimagemetadata_test.go 2013-10-17 05:10:43 +0000
195@@ -66,7 +66,7 @@
196 }
197
198 func (s *ValidateImageMetadataSuite) makeLocalMetadata(c *gc.C, id, region, series, endpoint string) error {
199- im := imagemetadata.ImageMetadata{
200+ im := &imagemetadata.ImageMetadata{
201 Id: id,
202 Arch: "amd64",
203 }
204@@ -78,7 +78,7 @@
205 if err != nil {
206 return err
207 }
208- err = imagemetadata.WriteMetadata(series, &im, &cloudSpec, targetStorage)
209+ err = imagemetadata.MergeAndWriteMetadata(series, []*imagemetadata.ImageMetadata{im}, &cloudSpec, targetStorage)
210 if err != nil {
211 return err
212 }
213@@ -105,10 +105,8 @@
214 s.LoggingSuite.SetUpTest(c)
215 s.metadataDir = c.MkDir()
216 s.home = coretesting.MakeFakeHome(c, metadataTestEnvConfig)
217- restore := testbase.PatchEnvironment("AWS_ACCESS_KEY_ID", "access")
218- s.AddCleanup(func(*gc.C) { restore() })
219- restore = testbase.PatchEnvironment("AWS_SECRET_ACCESS_KEY", "secret")
220- s.AddCleanup(func(*gc.C) { restore() })
221+ s.PatchEnvironment("AWS_ACCESS_KEY_ID", "access")
222+ s.PatchEnvironment("AWS_SECRET_ACCESS_KEY", "secret")
223 }
224
225 func (s *ValidateImageMetadataSuite) TearDownTest(c *gc.C) {
226
227=== modified file 'environs/imagemetadata/generate.go'
228--- environs/imagemetadata/generate.go 2013-10-11 06:44:35 +0000
229+++ environs/imagemetadata/generate.go 2013-10-17 05:10:43 +0000
230@@ -5,16 +5,96 @@
231
232 import (
233 "bytes"
234+ "fmt"
235 "time"
236
237 "launchpad.net/juju-core/environs/simplestreams"
238 "launchpad.net/juju-core/environs/storage"
239+ "launchpad.net/juju-core/errors"
240 )
241
242-func WriteMetadata(series string, im *ImageMetadata, cloudSpec *simplestreams.CloudSpec, metadataStore storage.Storage) error {
243- metadataInfo, err := generateMetadata(series, im, cloudSpec)
244- if err != nil {
245- return err
246+// MergeAndWriteMetadata reads the existing metadata from storage (if any),
247+// and merges it with supplied metadata, writing the resulting metadata is written to storage.
248+func MergeAndWriteMetadata(series string, metadata []*ImageMetadata, cloudSpec *simplestreams.CloudSpec,
249+ metadataStore storage.Storage) error {
250+
251+ existingMetadata, err := readMetadata(metadataStore)
252+ if err != nil {
253+ return err
254+ }
255+ seriesVersion, err := simplestreams.SeriesVersion(series)
256+ if err != nil {
257+ return err
258+ }
259+ toWrite, allCloudSpec := mergeMetadata(seriesVersion, cloudSpec, metadata, existingMetadata)
260+ return writeMetadata(toWrite, allCloudSpec, metadataStore)
261+}
262+
263+// readMetadata reads the image metadata from metadataStore.
264+func readMetadata(metadataStore storage.Storage) ([]*ImageMetadata, error) {
265+ // Read any existing metadata so we can merge the new tools metadata with what's there.
266+ dataSource := storage.NewStorageSimpleStreamsDataSource(metadataStore, "")
267+ imageConstraint := NewImageConstraint(simplestreams.LookupParams{})
268+ existingMetadata, err := Fetch(
269+ []simplestreams.DataSource{dataSource}, simplestreams.DefaultIndexPath, imageConstraint, false)
270+ if err != nil && !errors.IsNotFoundError(err) {
271+ return nil, err
272+ }
273+ return existingMetadata, nil
274+}
275+
276+func mapKey(im *ImageMetadata) string {
277+ return fmt.Sprintf("%s-%s", im.productId(), im.RegionName)
278+}
279+
280+// mergeMetadata merges the newMetadata into existingMetadata, overwriting existing matching image records.
281+func mergeMetadata(seriesVersion string, cloudSpec *simplestreams.CloudSpec, newMetadata,
282+ existingMetadata []*ImageMetadata) ([]*ImageMetadata, []simplestreams.CloudSpec) {
283+
284+ var toWrite = make([]*ImageMetadata, len(newMetadata))
285+ imageIds := make(map[string]bool)
286+ for i, im := range newMetadata {
287+ newRecord := *im
288+ newRecord.Version = seriesVersion
289+ newRecord.RegionName = cloudSpec.Region
290+ newRecord.Endpoint = cloudSpec.Endpoint
291+ toWrite[i] = &newRecord
292+ imageIds[mapKey(&newRecord)] = true
293+ }
294+ regions := make(map[string]bool)
295+ var allCloudSpecs = []simplestreams.CloudSpec{*cloudSpec}
296+ for _, im := range existingMetadata {
297+ if _, ok := imageIds[mapKey(im)]; ok {
298+ continue
299+ }
300+ toWrite = append(toWrite, im)
301+ if _, ok := regions[im.RegionName]; ok {
302+ continue
303+ }
304+ regions[im.RegionName] = true
305+ existingCloudSpec := simplestreams.CloudSpec{im.RegionName, im.Endpoint}
306+ allCloudSpecs = append(allCloudSpecs, existingCloudSpec)
307+ }
308+ return toWrite, allCloudSpecs
309+}
310+
311+type MetadataFile struct {
312+ Path string
313+ Data []byte
314+}
315+
316+// writeMetadata generates some basic simplestreams metadata using the specified cloud and image details and writes
317+// it to the supplied store.
318+func writeMetadata(metadata []*ImageMetadata, cloudSpec []simplestreams.CloudSpec,
319+ metadataStore storage.Storage) error {
320+
321+ index, products, err := MarshalImageMetadataJSON(metadata, cloudSpec, time.Now())
322+ if err != nil {
323+ return err
324+ }
325+ metadataInfo := []MetadataFile{
326+ {simplestreams.UnsignedIndex, index},
327+ {ProductMetadataPath, products},
328 }
329 for _, md := range metadataInfo {
330 err = metadataStore.Put(md.Path, bytes.NewReader(md.Data), int64(len(md.Data)))
331@@ -24,29 +104,3 @@
332 }
333 return nil
334 }
335-
336-type MetadataFile struct {
337- Path string
338- Data []byte
339-}
340-
341-// generateMetadata generates some basic simplestreams metadata using the specified cloud and image details.
342-func generateMetadata(series string, im *ImageMetadata, cloudSpec *simplestreams.CloudSpec) ([]MetadataFile, error) {
343- metadata := &ImageMetadata{
344- Id: im.Id,
345- Arch: im.Arch,
346- Release: series,
347- RegionName: cloudSpec.Region,
348- Endpoint: cloudSpec.Endpoint,
349- }
350-
351- index, products, err := MarshalImageMetadataJSON([]*ImageMetadata{metadata}, cloudSpec, time.Now())
352- if err != nil {
353- return nil, err
354- }
355- objects := []MetadataFile{
356- {simplestreams.UnsignedIndex, index},
357- {ProductMetadataPath, products},
358- }
359- return objects, nil
360-}
361
362=== modified file 'environs/imagemetadata/generate_test.go'
363--- environs/imagemetadata/generate_test.go 2013-10-16 04:11:38 +0000
364+++ environs/imagemetadata/generate_test.go 2013-10-17 05:10:43 +0000
365@@ -4,12 +4,15 @@
366 package imagemetadata_test
367
368 import (
369+ "sort"
370+
371 gc "launchpad.net/gocheck"
372
373 "launchpad.net/juju-core/environs/filestorage"
374 "launchpad.net/juju-core/environs/imagemetadata"
375 "launchpad.net/juju-core/environs/imagemetadata/testing"
376 "launchpad.net/juju-core/environs/simplestreams"
377+ "launchpad.net/juju-core/environs/storage"
378 "launchpad.net/juju-core/testing/testbase"
379 )
380
381@@ -19,10 +22,27 @@
382 testbase.LoggingSuite
383 }
384
385+func assertFetch(c *gc.C, stor storage.Storage, series, arch, region, endpoint, id string) {
386+ cons := imagemetadata.NewImageConstraint(simplestreams.LookupParams{
387+ CloudSpec: simplestreams.CloudSpec{region, endpoint},
388+ Series: []string{series},
389+ Arches: []string{arch},
390+ })
391+ dataSource := storage.NewStorageSimpleStreamsDataSource(stor, "")
392+ metadata, err := imagemetadata.Fetch(
393+ []simplestreams.DataSource{dataSource}, simplestreams.DefaultIndexPath, cons, false)
394+ c.Assert(err, gc.IsNil)
395+ c.Assert(metadata, gc.HasLen, 1)
396+ c.Assert(metadata[0].Id, gc.Equals, id)
397+}
398+
399 func (s *generateSuite) TestWriteMetadata(c *gc.C) {
400- im := &imagemetadata.ImageMetadata{
401- Id: "1234",
402- Arch: "amd64",
403+ im := []*imagemetadata.ImageMetadata{
404+ {
405+ Id: "1234",
406+ Arch: "amd64",
407+ Version: "13.04",
408+ },
409 }
410 cloudSpec := &simplestreams.CloudSpec{
411 Region: "region",
412@@ -31,10 +51,156 @@
413 dir := c.MkDir()
414 targetStorage, err := filestorage.NewFileStorageWriter(dir, filestorage.UseDefaultTmpDir)
415 c.Assert(err, gc.IsNil)
416- err = imagemetadata.WriteMetadata("raring", im, cloudSpec, targetStorage)
417+ err = imagemetadata.MergeAndWriteMetadata("raring", im, cloudSpec, targetStorage)
418 c.Assert(err, gc.IsNil)
419 metadata := testing.ParseMetadata(c, dir)
420 c.Assert(metadata, gc.HasLen, 1)
421- im.RegionName = cloudSpec.Region
422- c.Assert(im, gc.DeepEquals, metadata[0])
423-}
424+ im[0].RegionName = cloudSpec.Region
425+ im[0].Endpoint = cloudSpec.Endpoint
426+ c.Assert(im[0], gc.DeepEquals, metadata[0])
427+ assertFetch(c, targetStorage, "raring", "amd64", "region", "endpoint", "1234")
428+}
429+
430+func (s *generateSuite) TestWriteMetadataMergeOverwriteSameArch(c *gc.C) {
431+ existingImageMetadata := []*imagemetadata.ImageMetadata{
432+ {
433+ Id: "1234",
434+ Arch: "amd64",
435+ Version: "13.04",
436+ },
437+ }
438+ cloudSpec := &simplestreams.CloudSpec{
439+ Region: "region",
440+ Endpoint: "endpoint",
441+ }
442+ dir := c.MkDir()
443+ targetStorage, err := filestorage.NewFileStorageWriter(dir, filestorage.UseDefaultTmpDir)
444+ c.Assert(err, gc.IsNil)
445+ err = imagemetadata.MergeAndWriteMetadata("raring", existingImageMetadata, cloudSpec, targetStorage)
446+ c.Assert(err, gc.IsNil)
447+ newImageMetadata := []*imagemetadata.ImageMetadata{
448+ {
449+ Id: "abcd",
450+ Arch: "amd64",
451+ Version: "13.04",
452+ },
453+ {
454+ Id: "xyz",
455+ Arch: "arm",
456+ Version: "13.04",
457+ },
458+ }
459+ err = imagemetadata.MergeAndWriteMetadata("raring", newImageMetadata, cloudSpec, targetStorage)
460+ c.Assert(err, gc.IsNil)
461+ metadata := testing.ParseMetadata(c, dir)
462+ c.Assert(metadata, gc.HasLen, 2)
463+ for _, im := range newImageMetadata {
464+ im.RegionName = cloudSpec.Region
465+ im.Endpoint = cloudSpec.Endpoint
466+ }
467+ c.Assert(metadata, gc.DeepEquals, newImageMetadata)
468+ assertFetch(c, targetStorage, "raring", "amd64", "region", "endpoint", "abcd")
469+ assertFetch(c, targetStorage, "raring", "arm", "region", "endpoint", "xyz")
470+}
471+
472+func (s *generateSuite) TestWriteMetadataMergeDifferentSeries(c *gc.C) {
473+ existingImageMetadata := []*imagemetadata.ImageMetadata{
474+ {
475+ Id: "1234",
476+ Arch: "amd64",
477+ Version: "13.04",
478+ },
479+ }
480+ cloudSpec := &simplestreams.CloudSpec{
481+ Region: "region",
482+ Endpoint: "endpoint",
483+ }
484+ dir := c.MkDir()
485+ targetStorage, err := filestorage.NewFileStorageWriter(dir, filestorage.UseDefaultTmpDir)
486+ c.Assert(err, gc.IsNil)
487+ err = imagemetadata.MergeAndWriteMetadata("raring", existingImageMetadata, cloudSpec, targetStorage)
488+ c.Assert(err, gc.IsNil)
489+ newImageMetadata := []*imagemetadata.ImageMetadata{
490+ {
491+ Id: "abcd",
492+ Arch: "amd64",
493+ Version: "12.04",
494+ },
495+ {
496+ Id: "xyz",
497+ Arch: "arm",
498+ Version: "12.04",
499+ },
500+ }
501+ err = imagemetadata.MergeAndWriteMetadata("precise", newImageMetadata, cloudSpec, targetStorage)
502+ c.Assert(err, gc.IsNil)
503+ metadata := testing.ParseMetadata(c, dir)
504+ c.Assert(metadata, gc.HasLen, 3)
505+ newImageMetadata = append(newImageMetadata, existingImageMetadata[0])
506+ for _, im := range newImageMetadata {
507+ im.RegionName = cloudSpec.Region
508+ im.Endpoint = cloudSpec.Endpoint
509+ }
510+ sort.Sort(byId(metadata))
511+ sort.Sort(byId(newImageMetadata))
512+ c.Assert(metadata, gc.DeepEquals, newImageMetadata)
513+ assertFetch(c, targetStorage, "raring", "amd64", "region", "endpoint", "1234")
514+ assertFetch(c, targetStorage, "precise", "amd64", "region", "endpoint", "abcd")
515+}
516+
517+func (s *generateSuite) TestWriteMetadataMergeDifferentRegion(c *gc.C) {
518+ existingImageMetadata := []*imagemetadata.ImageMetadata{
519+ {
520+ Id: "1234",
521+ Arch: "amd64",
522+ Version: "13.04",
523+ },
524+ }
525+ cloudSpec := &simplestreams.CloudSpec{
526+ Region: "region",
527+ Endpoint: "endpoint",
528+ }
529+ dir := c.MkDir()
530+ targetStorage, err := filestorage.NewFileStorageWriter(dir, filestorage.UseDefaultTmpDir)
531+ c.Assert(err, gc.IsNil)
532+ err = imagemetadata.MergeAndWriteMetadata("raring", existingImageMetadata, cloudSpec, targetStorage)
533+ c.Assert(err, gc.IsNil)
534+ newImageMetadata := []*imagemetadata.ImageMetadata{
535+ {
536+ Id: "abcd",
537+ Arch: "amd64",
538+ Version: "13.04",
539+ },
540+ {
541+ Id: "xyz",
542+ Arch: "arm",
543+ Version: "13.04",
544+ },
545+ }
546+ cloudSpec = &simplestreams.CloudSpec{
547+ Region: "region2",
548+ Endpoint: "endpoint2",
549+ }
550+ err = imagemetadata.MergeAndWriteMetadata("raring", newImageMetadata, cloudSpec, targetStorage)
551+ c.Assert(err, gc.IsNil)
552+ metadata := testing.ParseMetadata(c, dir)
553+ c.Assert(metadata, gc.HasLen, 3)
554+ for _, im := range newImageMetadata {
555+ im.RegionName = "region2"
556+ im.Endpoint = "endpoint2"
557+ }
558+ existingImageMetadata[0].RegionName = "region"
559+ existingImageMetadata[0].Endpoint = "endpoint"
560+ newImageMetadata = append(newImageMetadata, existingImageMetadata[0])
561+ sort.Sort(byId(metadata))
562+ sort.Sort(byId(newImageMetadata))
563+ c.Assert(metadata, gc.DeepEquals, newImageMetadata)
564+ assertFetch(c, targetStorage, "raring", "amd64", "region", "endpoint", "1234")
565+ assertFetch(c, targetStorage, "raring", "amd64", "region2", "endpoint2", "abcd")
566+}
567+
568+type byId []*imagemetadata.ImageMetadata
569+
570+func (b byId) Len() int { return len(b) }
571+func (b byId) Swap(i, j int) { b[i], b[j] = b[j], b[i] }
572+func (b byId) Less(i, j int) bool { return b[i].Id < b[j].Id }
573
574=== modified file 'environs/imagemetadata/marshal.go'
575--- environs/imagemetadata/marshal.go 2013-10-16 04:11:38 +0000
576+++ environs/imagemetadata/marshal.go 2013-10-17 05:10:43 +0000
577@@ -18,7 +18,9 @@
578 // MarshalImageMetadataJSON marshals image metadata to index and products JSON.
579 //
580 // updated is the time at which the JSON file was updated.
581-func MarshalImageMetadataJSON(metadata []*ImageMetadata, cloudSpec *simplestreams.CloudSpec, updated time.Time) (index, products []byte, err error) {
582+func MarshalImageMetadataJSON(metadata []*ImageMetadata, cloudSpec []simplestreams.CloudSpec,
583+ updated time.Time) (index, products []byte, err error) {
584+
585 if index, err = MarshalImageMetadataIndexJSON(metadata, cloudSpec, updated); err != nil {
586 return nil, nil, err
587 }
588@@ -31,13 +33,12 @@
589 // MarshalImageMetadataIndexJSON marshals image metadata to index JSON.
590 //
591 // updated is the time at which the JSON file was updated.
592-func MarshalImageMetadataIndexJSON(metadata []*ImageMetadata, cloudSpec *simplestreams.CloudSpec, updated time.Time) (out []byte, err error) {
593+func MarshalImageMetadataIndexJSON(metadata []*ImageMetadata, cloudSpec []simplestreams.CloudSpec,
594+ updated time.Time) (out []byte, err error) {
595+
596 productIds := make([]string, len(metadata))
597 for i, t := range metadata {
598- productIds[i], err = t.productId()
599- if err != nil {
600- return nil, err
601- }
602+ productIds[i] = t.productId()
603 }
604 var indices simplestreams.Indices
605 indices.Updated = updated.Format(time.RFC1123Z)
606@@ -50,7 +51,7 @@
607 DataType: "image-ids",
608 ProductsFilePath: ProductMetadataPath,
609 ProductIds: set.NewStrings(productIds...).SortedValues(),
610- Clouds: []simplestreams.CloudSpec{*cloudSpec},
611+ Clouds: cloudSpec,
612 },
613 }
614 return json.MarshalIndent(&indices, "", " ")
615@@ -66,31 +67,24 @@
616 cloud.Products = make(map[string]simplestreams.MetadataCatalog)
617 itemsversion := updated.Format("20060201") // YYYYMMDD
618 for _, t := range metadata {
619- id, err := t.productId()
620- if err != nil {
621- return nil, err
622- }
623- version, err := simplestreams.SeriesVersion(t.Release)
624- if err != nil {
625- return nil, err
626- }
627 toWrite := &ImageMetadata{
628- Id: t.Id,
629+ Id: t.Id,
630+ RegionName: t.RegionName,
631+ Endpoint: t.Endpoint,
632 }
633- if catalog, ok := cloud.Products[id]; ok {
634- catalog.Items[itemsversion].Items[t.Id] = t
635+ if catalog, ok := cloud.Products[t.productId()]; ok {
636+ catalog.Items[itemsversion].Items[t.Id] = toWrite
637 } else {
638 catalog = simplestreams.MetadataCatalog{
639- Arch: t.Arch,
640- RegionName: t.RegionName,
641- Version: version,
642+ Arch: t.Arch,
643+ Version: t.Version,
644 Items: map[string]*simplestreams.ItemCollection{
645 itemsversion: &simplestreams.ItemCollection{
646 Items: map[string]interface{}{t.Id: toWrite},
647 },
648 },
649 }
650- cloud.Products[id] = catalog
651+ cloud.Products[t.productId()] = catalog
652 }
653 }
654 return json.MarshalIndent(&cloud, "", " ")
655
656=== modified file 'environs/imagemetadata/marshal_test.go'
657--- environs/imagemetadata/marshal_test.go 2013-10-11 06:44:35 +0000
658+++ environs/imagemetadata/marshal_test.go 2013-10-17 05:10:43 +0000
659@@ -10,11 +10,14 @@
660
661 "launchpad.net/juju-core/environs/imagemetadata"
662 "launchpad.net/juju-core/environs/simplestreams"
663+ "launchpad.net/juju-core/testing/testbase"
664 )
665
666 var _ = gc.Suite(&marshalSuite{})
667
668-type marshalSuite struct{}
669+type marshalSuite struct {
670+ testbase.LoggingSuite
671+}
672
673 var expectedIndex = `{
674 "index": {
675@@ -90,23 +93,23 @@
676 var imageMetadataForTesting = []*imagemetadata.ImageMetadata{
677 &imagemetadata.ImageMetadata{
678 Id: "1234",
679- Release: "saucy",
680+ Version: "13.10",
681 Arch: "arm",
682 },
683 &imagemetadata.ImageMetadata{
684 Id: "5678",
685- Release: "precise",
686+ Version: "12.04",
687 Arch: "arm",
688 },
689 &imagemetadata.ImageMetadata{
690 Id: "abcd",
691- Release: "precise",
692+ Version: "12.04",
693 Arch: "amd64",
694 },
695 }
696
697 func (s *marshalSuite) TestMarshalIndex(c *gc.C) {
698- cloudSpec := &simplestreams.CloudSpec{Region: "region", Endpoint: "endpoint"}
699+ cloudSpec := []simplestreams.CloudSpec{{Region: "region", Endpoint: "endpoint"}}
700 index, err := imagemetadata.MarshalImageMetadataIndexJSON(imageMetadataForTesting, cloudSpec, time.Unix(0, 0).UTC())
701 c.Assert(err, gc.IsNil)
702 c.Assert(string(index), gc.Equals, expectedIndex)
703@@ -119,7 +122,7 @@
704 }
705
706 func (s *marshalSuite) TestMarshal(c *gc.C) {
707- cloudSpec := &simplestreams.CloudSpec{Region: "region", Endpoint: "endpoint"}
708+ cloudSpec := []simplestreams.CloudSpec{{Region: "region", Endpoint: "endpoint"}}
709 index, products, err := imagemetadata.MarshalImageMetadataJSON(imageMetadataForTesting, cloudSpec, time.Unix(0, 0).UTC())
710 c.Assert(err, gc.IsNil)
711 c.Assert(string(index), gc.Equals, expectedIndex)
712
713=== modified file 'environs/imagemetadata/simplestreams.go'
714--- environs/imagemetadata/simplestreams.go 2013-10-17 04:53:59 +0000
715+++ environs/imagemetadata/simplestreams.go 2013-10-17 05:10:43 +0000
716@@ -88,9 +88,11 @@
717 }
718
719 func NewImageConstraint(params simplestreams.LookupParams) *ImageConstraint {
720- if len(params.Series) != 1 {
721- // This can only happen as a result of a coding error.
722- panic(fmt.Sprintf("image constraint requires a single series, got %v", params.Series))
723+ if len(params.Series) == 0 {
724+ params.Series = simplestreams.SupportedSeries()
725+ }
726+ if len(params.Arches) == 0 {
727+ params.Arches = []string{"amd64", "i386", "arm"}
728 }
729 return &ImageConstraint{LookupParams: params}
730 }
731@@ -101,13 +103,18 @@
732 if stream != "" {
733 stream = "." + stream
734 }
735- version, err := simplestreams.SeriesVersion(ic.Series[0])
736- if err != nil {
737- return nil, err
738- }
739- ids := make([]string, len(ic.Arches))
740+
741+ nrArches := len(ic.Arches)
742+ nrSeries := len(ic.Series)
743+ ids := make([]string, nrArches*nrSeries)
744 for i, arch := range ic.Arches {
745- ids[i] = fmt.Sprintf("com.ubuntu.cloud%s:server:%s:%s", stream, version, arch)
746+ for j, series := range ic.Series {
747+ version, err := simplestreams.SeriesVersion(series)
748+ if err != nil {
749+ return nil, err
750+ }
751+ ids[j*nrArches+i] = fmt.Sprintf("com.ubuntu.cloud%s:server:%s:%s", stream, version, arch)
752+ }
753 }
754 return ids, nil
755 }
756@@ -118,18 +125,18 @@
757 Storage string `json:"root_store,omitempty"`
758 VType string `json:"virt,omitempty"`
759 Arch string `json:"arch,omitempty"`
760- Release string `json:"-"`
761+ Version string `json:"version,omitempty"`
762 RegionAlias string `json:"crsn,omitempty"`
763 RegionName string `json:"region,omitempty"`
764 Endpoint string `json:"endpoint,omitempty"`
765 }
766
767-func (t *ImageMetadata) productId() (string, error) {
768- seriesVersion, err := simplestreams.SeriesVersion(t.Release)
769- if err != nil {
770- return "", err
771- }
772- return fmt.Sprintf("com.ubuntu.cloud:server:%s:%s", seriesVersion, t.Arch), nil
773+func (im *ImageMetadata) String() string {
774+ return fmt.Sprintf("%#v", im)
775+}
776+
777+func (im *ImageMetadata) productId() string {
778+ return fmt.Sprintf("com.ubuntu.cloud:server:%s:%s", im.Version, im.Arch)
779 }
780
781 // Fetch returns a list of images for the specified cloud matching the constraint.
782@@ -157,6 +164,8 @@
783 type imageKey struct {
784 vtype string
785 arch string
786+ version string
787+ region string
788 storage string
789 }
790
791@@ -168,14 +177,14 @@
792 imagesMap := make(map[imageKey]*ImageMetadata, len(matchingImages))
793 for _, val := range matchingImages {
794 im := val.(*ImageMetadata)
795- imagesMap[imageKey{im.VType, im.Arch, im.Storage}] = im
796+ imagesMap[imageKey{im.VType, im.Arch, im.Version, im.RegionName, im.Storage}] = im
797 }
798 for _, val := range images {
799 im := val.(*ImageMetadata)
800- if cons.Params().Region != im.RegionName {
801+ if cons != nil && cons.Params().Region != "" && cons.Params().Region != im.RegionName {
802 continue
803 }
804- if _, ok := imagesMap[imageKey{im.VType, im.Arch, im.Storage}]; !ok {
805+ if _, ok := imagesMap[imageKey{im.VType, im.Arch, im.Version, im.RegionName, im.Storage}]; !ok {
806 matchingImages = append(matchingImages, im)
807 }
808 }
809
810=== modified file 'environs/imagemetadata/simplestreams_test.go'
811--- environs/imagemetadata/simplestreams_test.go 2013-10-10 11:40:54 +0000
812+++ environs/imagemetadata/simplestreams_test.go 2013-10-17 05:10:43 +0000
813@@ -107,91 +107,138 @@
814 }
815
816 var fetchTests = []struct {
817- region string
818- series string
819- arches []string
820- images []*imagemetadata.ImageMetadata
821+ region string
822+ version string
823+ arches []string
824+ images []*imagemetadata.ImageMetadata
825 }{
826 {
827- region: "us-east-1",
828- series: "precise",
829- arches: []string{"amd64", "arm"},
830- images: []*imagemetadata.ImageMetadata{
831- {
832- Id: "ami-442ea674",
833- VType: "hvm",
834- Arch: "amd64",
835- RegionName: "us-east-1",
836- Endpoint: "https://ec2.us-east-1.amazonaws.com",
837- Storage: "ebs",
838- },
839- {
840- Id: "ami-442ea684",
841- VType: "pv",
842- Arch: "amd64",
843- RegionName: "us-east-1",
844- Endpoint: "https://ec2.us-east-1.amazonaws.com",
845- Storage: "instance",
846- },
847- {
848- Id: "ami-442ea699",
849- VType: "pv",
850- Arch: "arm",
851- RegionName: "us-east-1",
852- Endpoint: "https://ec2.us-east-1.amazonaws.com",
853- Storage: "ebs",
854- },
855- },
856- },
857- {
858- region: "us-east-1",
859- series: "precise",
860- arches: []string{"amd64"},
861- images: []*imagemetadata.ImageMetadata{
862- {
863- Id: "ami-442ea674",
864- VType: "hvm",
865- Arch: "amd64",
866- RegionName: "us-east-1",
867- Endpoint: "https://ec2.us-east-1.amazonaws.com",
868- Storage: "ebs",
869- },
870- {
871- Id: "ami-442ea684",
872- VType: "pv",
873- Arch: "amd64",
874- RegionName: "us-east-1",
875- Endpoint: "https://ec2.us-east-1.amazonaws.com",
876- Storage: "instance",
877- },
878- },
879- },
880- {
881- region: "us-east-1",
882- series: "precise",
883- arches: []string{"arm"},
884- images: []*imagemetadata.ImageMetadata{
885- {
886- Id: "ami-442ea699",
887- VType: "pv",
888- Arch: "arm",
889- RegionName: "us-east-1",
890- Endpoint: "https://ec2.us-east-1.amazonaws.com",
891- Storage: "ebs",
892- },
893- },
894- },
895- {
896- region: "us-east-1",
897- series: "precise",
898- arches: []string{"amd64"},
899- images: []*imagemetadata.ImageMetadata{
900- {
901- Id: "ami-442ea674",
902- VType: "hvm",
903- Arch: "amd64",
904- RegionName: "us-east-1",
905- Endpoint: "https://ec2.us-east-1.amazonaws.com",
906+ region: "us-east-1",
907+ version: "12.04",
908+ arches: []string{"amd64", "arm"},
909+ images: []*imagemetadata.ImageMetadata{
910+ {
911+ Id: "ami-442ea674",
912+ VType: "hvm",
913+ Arch: "amd64",
914+ RegionName: "us-east-1",
915+ Endpoint: "https://ec2.us-east-1.amazonaws.com",
916+ Storage: "ebs",
917+ },
918+ {
919+ Id: "ami-442ea684",
920+ VType: "pv",
921+ Arch: "amd64",
922+ RegionName: "us-east-1",
923+ Endpoint: "https://ec2.us-east-1.amazonaws.com",
924+ Storage: "instance",
925+ },
926+ {
927+ Id: "ami-442ea699",
928+ VType: "pv",
929+ Arch: "arm",
930+ RegionName: "us-east-1",
931+ Endpoint: "https://ec2.us-east-1.amazonaws.com",
932+ Storage: "ebs",
933+ },
934+ },
935+ },
936+ {
937+ region: "us-east-1",
938+ version: "12.04",
939+ arches: []string{"amd64"},
940+ images: []*imagemetadata.ImageMetadata{
941+ {
942+ Id: "ami-442ea674",
943+ VType: "hvm",
944+ Arch: "amd64",
945+ RegionName: "us-east-1",
946+ Endpoint: "https://ec2.us-east-1.amazonaws.com",
947+ Storage: "ebs",
948+ },
949+ {
950+ Id: "ami-442ea684",
951+ VType: "pv",
952+ Arch: "amd64",
953+ RegionName: "us-east-1",
954+ Endpoint: "https://ec2.us-east-1.amazonaws.com",
955+ Storage: "instance",
956+ },
957+ },
958+ },
959+ {
960+ region: "us-east-1",
961+ version: "12.04",
962+ arches: []string{"arm"},
963+ images: []*imagemetadata.ImageMetadata{
964+ {
965+ Id: "ami-442ea699",
966+ VType: "pv",
967+ Arch: "arm",
968+ RegionName: "us-east-1",
969+ Endpoint: "https://ec2.us-east-1.amazonaws.com",
970+ Storage: "ebs",
971+ },
972+ },
973+ },
974+ {
975+ region: "us-east-1",
976+ version: "12.04",
977+ arches: []string{"amd64"},
978+ images: []*imagemetadata.ImageMetadata{
979+ {
980+ Id: "ami-442ea674",
981+ VType: "hvm",
982+ Arch: "amd64",
983+ RegionName: "us-east-1",
984+ Endpoint: "https://ec2.us-east-1.amazonaws.com",
985+ Storage: "ebs",
986+ },
987+ {
988+ Id: "ami-442ea684",
989+ VType: "pv",
990+ Arch: "amd64",
991+ RegionName: "us-east-1",
992+ Endpoint: "https://ec2.us-east-1.amazonaws.com",
993+ Storage: "instance",
994+ },
995+ },
996+ },
997+ {
998+ version: "12.04",
999+ arches: []string{"amd64"},
1000+ images: []*imagemetadata.ImageMetadata{
1001+ {
1002+ Id: "ami-26745463",
1003+ VType: "pv",
1004+ Arch: "amd64",
1005+ RegionName: "au-east-2",
1006+ Endpoint: "https://somewhere-else",
1007+ Storage: "ebs",
1008+ },
1009+ {
1010+ Id: "ami-442ea674",
1011+ VType: "hvm",
1012+ Arch: "amd64",
1013+ RegionName: "us-east-1",
1014+ Endpoint: "https://ec2.us-east-1.amazonaws.com",
1015+ Storage: "ebs",
1016+ },
1017+ {
1018+ Id: "ami-442ea675",
1019+ VType: "hvm",
1020+ Arch: "amd64",
1021+ RegionAlias: "uswest3",
1022+ RegionName: "us-west-3",
1023+ Endpoint: "https://ec2.us-west-3.amazonaws.com",
1024+ Storage: "ebs",
1025+ },
1026+ {
1027+ Id: "ami-26745464",
1028+ VType: "pv",
1029+ Arch: "amd64",
1030+ RegionName: "au-east-1",
1031+ Endpoint: "https://somewhere",
1032 Storage: "ebs",
1033 },
1034 {
1035@@ -209,8 +256,12 @@
1036 func (s *simplestreamsSuite) TestFetch(c *gc.C) {
1037 for i, t := range fetchTests {
1038 c.Logf("test %d", i)
1039+ cloudSpec := simplestreams.CloudSpec{t.region, "https://ec2.us-east-1.amazonaws.com"}
1040+ if t.region == "" {
1041+ cloudSpec = simplestreams.EmptyCloudSpec
1042+ }
1043 imageConstraint := imagemetadata.NewImageConstraint(simplestreams.LookupParams{
1044- CloudSpec: simplestreams.CloudSpec{t.region, "https://ec2.us-east-1.amazonaws.com"},
1045+ CloudSpec: cloudSpec,
1046 Series: []string{"precise"},
1047 Arches: t.arches,
1048 })
1049@@ -218,6 +269,9 @@
1050 if !c.Check(err, gc.IsNil) {
1051 continue
1052 }
1053+ for _, testImage := range t.images {
1054+ testImage.Version = t.version
1055+ }
1056 c.Check(images, gc.DeepEquals, t.images)
1057 }
1058 }
1059
1060=== modified file 'environs/imagemetadata/validation_test.go'
1061--- environs/imagemetadata/validation_test.go 2013-10-11 06:44:35 +0000
1062+++ environs/imagemetadata/validation_test.go 2013-10-17 05:10:43 +0000
1063@@ -20,9 +20,11 @@
1064 var _ = gc.Suite(&ValidateSuite{})
1065
1066 func (s *ValidateSuite) makeLocalMetadata(c *gc.C, id, region, series, endpoint string) error {
1067- im := imagemetadata.ImageMetadata{
1068- Id: id,
1069- Arch: "amd64",
1070+ metadata := []*imagemetadata.ImageMetadata{
1071+ {
1072+ Id: id,
1073+ Arch: "amd64",
1074+ },
1075 }
1076 cloudSpec := simplestreams.CloudSpec{
1077 Region: region,
1078@@ -30,7 +32,7 @@
1079 }
1080 targetStorage, err := filestorage.NewFileStorageWriter(s.metadataDir, filestorage.UseDefaultTmpDir)
1081 c.Assert(err, gc.IsNil)
1082- err = imagemetadata.WriteMetadata(series, &im, &cloudSpec, targetStorage)
1083+ err = imagemetadata.MergeAndWriteMetadata(series, metadata, &cloudSpec, targetStorage)
1084 if err != nil {
1085 return err
1086 }
1087
1088=== modified file 'environs/simplestreams/simplestreams.go'
1089--- environs/simplestreams/simplestreams.go 2013-10-17 04:53:59 +0000
1090+++ environs/simplestreams/simplestreams.go 2013-10-17 05:10:43 +0000
1091@@ -36,6 +36,9 @@
1092 Endpoint string `json:"endpoint"`
1093 }
1094
1095+// EmptyCloudSpec is used when we want all records regardless of cloud to be loaded.
1096+var EmptyCloudSpec = CloudSpec{}
1097+
1098 // HasRegion is implemented by instances which can provide a region to which they belong.
1099 // A region is defined by region name and endpoint.
1100 type HasRegion interface {
1101@@ -86,6 +89,9 @@
1102
1103 // SeriesVersion returns the version number for the specified Ubuntu series.
1104 func SeriesVersion(series string) (string, error) {
1105+ if series == "" {
1106+ panic("cannot pass empty series to SeriesVersion()")
1107+ }
1108 seriesVersionsMutex.Lock()
1109 defer seriesVersionsMutex.Unlock()
1110 if vers, ok := seriesVersions[series]; ok {
1111@@ -556,13 +562,15 @@
1112 if len(candidates) == 0 {
1113 return "", errors.NotFoundf("index file missing %q data", indexRef.valueParams.DataType)
1114 }
1115- // Restrict by cloud spec.
1116- hasRightCloud := func(metadata *IndexMetadata) bool {
1117- return metadata.hasCloud(cons.Params().CloudSpec)
1118- }
1119- candidates = candidates.filter(hasRightCloud)
1120- if len(candidates) == 0 {
1121- return "", errors.NotFoundf("index file has no data for cloud %v", cons.Params().CloudSpec)
1122+ // Restrict by cloud spec, if required.
1123+ if cons.Params().CloudSpec != EmptyCloudSpec {
1124+ hasRightCloud := func(metadata *IndexMetadata) bool {
1125+ return metadata.hasCloud(cons.Params().CloudSpec)
1126+ }
1127+ candidates = candidates.filter(hasRightCloud)
1128+ if len(candidates) == 0 {
1129+ return "", errors.NotFoundf("index file has no data for cloud %v", cons.Params().CloudSpec)
1130+ }
1131 }
1132 // Restrict by product IDs.
1133 hasProduct := func(metadata *IndexMetadata) bool {
1134@@ -880,8 +888,8 @@
1135
1136 // GetCloudMetadataWithFormat loads the entire cloud metadata encoded using the specified format.
1137 // Exported for testing.
1138-func (indexRef *IndexReference) GetCloudMetadataWithFormat(ic LookupConstraint, format string, requireSigned bool) (*CloudMetadata, error) {
1139- productFilesPath, err := indexRef.GetProductsPath(ic)
1140+func (indexRef *IndexReference) GetCloudMetadataWithFormat(cons LookupConstraint, format string, requireSigned bool) (*CloudMetadata, error) {
1141+ productFilesPath, err := indexRef.GetProductsPath(cons)
1142 if err != nil {
1143 return nil, err
1144 }

Subscribers

People subscribed via source and target branches

to status/vote changes: