juju-core

Merge lp:~axwalk/juju-core/lp1235717-refactor-simplestreams-writemetadata into lp:~go-bot/juju-core/trunk

  1. lp1235717-refactor-simplestreams-writemetadata
  2. Merge into trunk
Proposed by Andrew Wilkins
Status: Merged
Approved by: Andrew Wilkins
Approved revision: no longer in the source branch.
Merged at revision: 1988
Proposed branch: lp:~axwalk/juju-core/lp1235717-refactor-simplestreams-writemetadata
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 896 lines (+427/-142)
10 files modified
cmd/jujud/agent_test.go (+1/-1)
cmd/plugins/juju-metadata/toolsmetadata.go (+21/-2)
cmd/plugins/juju-metadata/toolsmetadata_test.go (+16/-14)
environs/sync/sync.go (+1/-1)
environs/testing/tools.go (+17/-7)
environs/tools/simplestreams.go (+132/-82)
environs/tools/simplestreams_test.go (+226/-15)
environs/tools/tools.go (+1/-6)
testing/targz.go (+8/-10)
worker/upgrader/upgrader_test.go (+4/-4)
To merge this branch: bzr merge lp:~axwalk/juju-core/lp1235717-refactor-simplestreams-writemetadata
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+189767@code.launchpad.net

Commit message

Fix tools metadata generation for null provider

There's two functional changes here:
 - If there's existing tools metadata, use it
   if it has sha256/size in it, and the tools
   haven't been replaced.
 - Don't use net/http or the tools URL when
   fetching tools to compute sha256/size. That
   doesn't work with sshstorage.
 - Don't fetch existing tools to resolve metadata
   during normal tools syncing operations. Only
   the "juju metadata generate-tools" command
   will do this now.

Additionally, I have refactored
environs/tools/simplestreams.go, to make the
intent of WriteMetadata (now MergeAndWriteMetadata)
more obvious. That function is now composed of
several other more specific functions.

Fixes #1235717

https://codereview.appspot.com/14527043/

Description of the change

Fix tools metadata generation for null provider

There's two functional changes here:
 - If there's existing tools metadata, use it
   if it has sha256/size in it, and the tools
   haven't been replaced.
 - Don't use net/http or the tools URL when
   fetching tools to compute sha256/size. That
   doesn't work with sshstorage.
 - Don't fetch existing tools to resolve metadata
   during normal tools syncing operations. Only
   the "juju metadata generate-tools" command
   will do this now.

Additionally, I have refactored
environs/tools/simplestreams.go, to make the
intent of WriteMetadata (now MergeAndWriteMetadata)
more obvious. That function is now composed of
several other more specific functions.

Fixes #1235717

https://codereview.appspot.com/14527043/

To post a comment you must log in.
Revision history for this message
Andrew Wilkins (axwalk) wrote :

Reviewers: mp+189767_code.launchpad.net,

Message:
Please take a look.

Description:
Fix tools metadata generation for null provider

There's two functional changes here:
  - If there's existing tools metadata, use it
    if it has sha256/size in it, and the tools
    haven't been replaced.
  - Don't use net/http or the tools URL when
    fetching tools to compute sha256/size. That
    doesn't work with sshstorage.

Additionally, I have refactored
environs/tools/simplestreams.go, to make the
intent of WriteMetadata (now MergeAndWriteMetadata)
more obvious. That function is now composed of
several other more specific functions.

Fixes #1235717

https://code.launchpad.net/~axwalk/juju-core/lp1235717-refactor-simplestreams-writemetadata/+merge/189767

(do not edit description out of merge proposal)

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

Affected files (+171, -122 lines):
   A [revision details]
   M cmd/jujud/agent_test.go
   M cmd/plugins/juju-metadata/toolsmetadata.go
   M cmd/plugins/juju-metadata/toolsmetadata_test.go
   M environs/sync/sync.go
   M environs/testing/tools.go
   M environs/tools/simplestreams.go
   M environs/tools/simplestreams_test.go
   M environs/tools/tools.go
   M worker/upgrader/upgrader_test.go

Revision history for this message
Andrew Wilkins (axwalk) wrote :

Please take a look.

https://codereview.appspot.com/14527043/

Revision history for this message
Roger Peppe (rogpeppe) wrote :
Download full text (3.5 KiB)

This looks great in general, with a few remarks and suggestions below.

https://codereview.appspot.com/14527043/diff/5001/cmd/plugins/juju-metadata/toolsmetadata.go
File cmd/plugins/juju-metadata/toolsmetadata.go (right):

https://codereview.appspot.com/14527043/diff/5001/cmd/plugins/juju-metadata/toolsmetadata.go#newcode76
cmd/plugins/juju-metadata/toolsmetadata.go:76: return
tools.MergeAndWriteMetadata(targetStorage, toolsList,
tools.ResolveFlag(c.fetch))
I'd prefer to see explicit values for the resolve flag here - using it
as a boolean gets around the advantages of using the named constants, I
think.

https://codereview.appspot.com/14527043/diff/5001/environs/tools/simplestreams.go
File environs/tools/simplestreams.go (right):

https://codereview.appspot.com/14527043/diff/5001/environs/tools/simplestreams.go#newcode217
environs/tools/simplestreams.go:217: // the metadata locally.
s/metadata/size and hash/ ?

https://codereview.appspot.com/14527043/diff/5001/environs/tools/simplestreams.go#newcode220
environs/tools/simplestreams.go:220: if md.Size == 0 {
if md.Size != 0 {
     continue
}

to save an indent level?

https://codereview.appspot.com/14527043/diff/5001/environs/tools/simplestreams.go#newcode240
environs/tools/simplestreams.go:240: func MergeMetadata(newMetadata,
oldMetadata []*ToolsMetadata) []*ToolsMetadata {
I'm not sure that "new" and "old" are appropriate prefixes here - they
seem to imply that the arguments are treated differently, but isn't
MergeMetadata commutative?

Personally I'd use something like m0 and m1, but go with your own
thoughts for naming.

https://codereview.appspot.com/14527043/diff/5001/environs/tools/simplestreams.go#newcode292
environs/tools/simplestreams.go:292: type ResolveFlag bool
I know we do this elsewhere, but I've come to dislike
this idiom - it's an unsatisfactory halfway house between
named constants and bools. There's still code that
uses the bool semantics, which means that these names aren't
really helping much because you have to know their values
anyway.

I'd prefer either to use int and iota or to stick with the bool
and lose the named constants.

https://codereview.appspot.com/14527043/diff/5001/environs/tools/simplestreams.go#newcode300
environs/tools/simplestreams.go:300: // and merges it with metadata
generated from the given tools list.
It's not clear what targetTools is here. It sounds like it's
a list of tools in the target storage, but it doesn't actually
seem like that's the case.

https://codereview.appspot.com/14527043/diff/5001/environs/tools/simplestreams.go#newcode301
environs/tools/simplestreams.go:301: // If resolve is true, incomplete
metadata is resolved by fetching the tools
// If resolve is Resolve

https://codereview.appspot.com/14527043/diff/5001/environs/tools/simplestreams.go#newcode304
environs/tools/simplestreams.go:304: func MergeAndWriteMetadata(stor
storage.Storage, targetTools coretools.List, resolve ResolveFlag) error
{
s/stor/target/ ?

(given that we refer to "the target storage" above)

https://codereview.appspot.com/14527043/diff/5001/environs/tools/simplestreams_test.go
File environs/tools/simplestreams_test.go (right):

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

Read more...

Revision history for this message
Andrew Wilkins (axwalk) wrote :
Download full text (5.2 KiB)

https://codereview.appspot.com/14527043/diff/5001/cmd/plugins/juju-metadata/toolsmetadata.go
File cmd/plugins/juju-metadata/toolsmetadata.go (right):

https://codereview.appspot.com/14527043/diff/5001/cmd/plugins/juju-metadata/toolsmetadata.go#newcode76
cmd/plugins/juju-metadata/toolsmetadata.go:76: return
tools.MergeAndWriteMetadata(targetStorage, toolsList,
tools.ResolveFlag(c.fetch))
On 2013/10/09 12:08:57, rog wrote:
> I'd prefer to see explicit values for the resolve flag here - using it
 as a
> boolean gets around the advantages of using the named constants, I
 think.

Done.

https://codereview.appspot.com/14527043/diff/5001/environs/tools/simplestreams.go
File environs/tools/simplestreams.go (right):

https://codereview.appspot.com/14527043/diff/5001/environs/tools/simplestreams.go#newcode217
environs/tools/simplestreams.go:217: // the metadata locally.
On 2013/10/09 12:08:57, rog wrote:
> s/metadata/size and hash/ ?

Done.

https://codereview.appspot.com/14527043/diff/5001/environs/tools/simplestreams.go#newcode220
environs/tools/simplestreams.go:220: if md.Size == 0 {
On 2013/10/09 12:08:57, rog wrote:
> if md.Size != 0 {
> continue
> }

> to save an indent level?

Done.

https://codereview.appspot.com/14527043/diff/5001/environs/tools/simplestreams.go#newcode240
environs/tools/simplestreams.go:240: func MergeMetadata(newMetadata,
oldMetadata []*ToolsMetadata) []*ToolsMetadata {
On 2013/10/09 12:08:57, rog wrote:
> I'm not sure that "new" and "old" are appropriate prefixes here - they
 seem to
> imply that the arguments are treated differently, but isn't
 MergeMetadata
> commutative?

> Personally I'd use something like m0 and m1, but go with your own
thoughts for
> naming.

It's not commutative, per the last sentence of the doc comment:
"If both entries have information, prefer the entry from newMetadata".

The idea behind that is that if both have size&hash, then the one from
"newMetadata" is considered newer. That's to cater for copying over the
top of existing tools (say, a dev build).

https://codereview.appspot.com/14527043/diff/5001/environs/tools/simplestreams.go#newcode292
environs/tools/simplestreams.go:292: type ResolveFlag bool
On 2013/10/09 12:08:57, rog wrote:
> I know we do this elsewhere, but I've come to dislike
> this idiom - it's an unsatisfactory halfway house between
> named constants and bools. There's still code that
> uses the bool semantics, which means that these names aren't
> really helping much because you have to know their values
> anyway.

> I'd prefer either to use int and iota or to stick with the bool
> and lose the named constants.

Consider a bool as (conceptually) a 1-bit unsigned int; then what's the
difference between that and int with constant named values? The only
difference I see is that the bool is range limited to [0, 1], which is a
very good thing.

I'd be happy to drop the named type. Would that be a reasonable
alternative? i.e.:

const (
     DontResolve = false
     Resolve = true
)

func MergeAndWriteMetadata(stor storage.Storage, targetTools
coretools.List, resolve bool) error {
...
}

then people can use bools if they want, without much remorse, or they
can use the named constant...

Read more...

Revision history for this message
Andrew Wilkins (axwalk) wrote :

Please take a look.

https://codereview.appspot.com/14527043/

Revision history for this message
Roger Peppe (rogpeppe) wrote :
Download full text (3.9 KiB)

LGTM with some thoughts below.

For the future, I think that the "resolve" mode is unnecessary and
should go - if we have items in the target bucket that we don't have
hashes for, we should just ignore them. This will actually work fine in
practice, I think, even if we abort a SyncTools half way through,
because when we retry the SyncTools, any items in the target bucket that
we didn't get around to adding to the target bucket simplestreams data
should have metadata in the source bucket's simplestreams data.

In general, I think retro-discovering hash data by trying to find out
what the data currently is is a flawed idea - the hash of some tools
should be generated on first upload and preserved forever after.

https://codereview.appspot.com/14527043/diff/5001/environs/tools/simplestreams.go
File environs/tools/simplestreams.go (right):

https://codereview.appspot.com/14527043/diff/5001/environs/tools/simplestreams.go#newcode240
environs/tools/simplestreams.go:240: func MergeMetadata(newMetadata,
oldMetadata []*ToolsMetadata) []*ToolsMetadata {
On 2013/10/09 14:28:25, axw wrote:
> On 2013/10/09 12:08:57, rog wrote:
> > I'm not sure that "new" and "old" are appropriate prefixes here -
 they seem to
> > imply that the arguments are treated differently, but isn't
 MergeMetadata
> > commutative?
> >
> > Personally I'd use something like m0 and m1, but go with your own
 thoughts for
> > naming.

> It's not commutative, per the last sentence of the doc comment:
> "If both entries have information, prefer the entry from newMetadata".

> The idea behind that is that if both have size&hash, then the one from
> "newMetadata" is considered newer. That's to cater for copying over
the top of
> existing tools (say, a dev build).

Hmm, that *should* never happen - a version should always correspond
only to one particular tarball. I think if we do get a clash like this,
then we have a problem that I suspect our current logic won't cope with.

Perhaps this function should check for inconsistencies and return an
error if it finds one.

https://codereview.appspot.com/14527043/diff/5001/environs/tools/simplestreams.go#newcode292
environs/tools/simplestreams.go:292: type ResolveFlag bool
On 2013/10/09 14:28:25, axw wrote:
> On 2013/10/09 12:08:57, rog wrote:
> > I know we do this elsewhere, but I've come to dislike
> > this idiom - it's an unsatisfactory halfway house between
> > named constants and bools. There's still code that
> > uses the bool semantics, which means that these names aren't
> > really helping much because you have to know their values
> > anyway.
> >
> > I'd prefer either to use int and iota or to stick with the bool
> > and lose the named constants.

> Consider a bool as (conceptually) a 1-bit unsigned int; then what's
the
> difference between that and int with constant named values? The only
difference
> I see is that the bool is range limited to [0, 1], which is a very
good thing.

The main difference I see is that various pieces of code will use a
boolean directly, or a boolean test and the sense of the code isn't
clear in that case. For example, in another piece of code in this CL,
there's a boolean field called "fetch" which is passed direc...

Read more...

Revision history for this message
Andrew Wilkins (axwalk) wrote :
Download full text (4.6 KiB)

On 2013/10/09 16:29:42, rog wrote:
> LGTM with some thoughts below.

Thanks. I'll change the newMetadata/oldMetadata thing as suggested, but
I think I'll leave the others to a followup.

> For the future, I think that the "resolve" mode is unnecessary and
should go -
> if we have items in the target bucket that we don't have hashes for,
we should
> just ignore them. This will actually work fine in practice, I think,
even if we
> abort a SyncTools half way through, because when we retry the
SyncTools, any
> items in the target bucket that we didn't get around to adding to the
target
> bucket simplestreams data should have metadata in the source bucket's
> simplestreams data.

> In general, I think retro-discovering hash data by trying to find out
what the
> data currently is is a flawed idea - the hash of some tools should be
generated
> on first upload and preserved forever after.

I'd like to say I agree, however I know the requirements here are a bit
subtle.
I think it'd be good to talk with wallyworld at SFO to nut it out.

https://codereview.appspot.com/14527043/diff/5001/environs/tools/simplestreams.go
> File environs/tools/simplestreams.go (right):

https://codereview.appspot.com/14527043/diff/5001/environs/tools/simplestreams.go#newcode240
> environs/tools/simplestreams.go:240: func MergeMetadata(newMetadata,
 oldMetadata
> []*ToolsMetadata) []*ToolsMetadata {
> On 2013/10/09 14:28:25, axw wrote:
> > On 2013/10/09 12:08:57, rog wrote:
> > > I'm not sure that "new" and "old" are appropriate prefixes here -
 they seem
> to
> > > imply that the arguments are treated differently, but isn't
 MergeMetadata
> > > commutative?
> > >
> > > Personally I'd use something like m0 and m1, but go with your own
 thoughts
> for
> > > naming.
> >
> > It's not commutative, per the last sentence of the doc comment:
> > "If both entries have information, prefer the entry from
 newMetadata".
> >
> > The idea behind that is that if both have size&hash, then the one
 from
> > "newMetadata" is considered newer. That's to cater for copying over
 the top of
> > existing tools (say, a dev build).

> Hmm, that *should* never happen - a version should always correspond
only to one
> particular tarball. I think if we do get a clash like this, then we
have a
> problem that I suspect our current logic won't cope with.

> Perhaps this function should check for inconsistencies and return an
error if it
> finds one.

Yeah, that sounds sensible. The tools won't be copied over if they are
already in the target, so the "newMetadata" entry won't have a
size/hash. So this code is only handling a theoretical case, anyway.
I'll make this change and repropose.

https://codereview.appspot.com/14527043/diff/5001/environs/tools/simplestreams.go#newcode292
> environs/tools/simplestreams.go:292: type ResolveFlag bool
> On 2013/10/09 14:28:25, axw wrote:
> > On 2013/10/09 12:08:57, rog wrote:
> > > I know we do this elsewhere, but I've come to dislike
> > > this idiom - it's an unsatisfactory halfway house between
> > > named constants and bools. There's still code that
> > > uses the bool semantics, which means that these names aren't
> > > really helping much because you have to know th...

Read more...

Revision history for this message
Andrew Wilkins (axwalk) wrote :

Please take a look.

https://codereview.appspot.com/14527043/

Revision history for this message
Andrew Wilkins (axwalk) wrote :

Please take a look.

https://codereview.appspot.com/14527043/

Revision history for this message
Andrew Wilkins (axwalk) wrote :

Please take a look.

https://codereview.appspot.com/14527043/

Revision history for this message
Andrew Wilkins (axwalk) wrote :
Download full text (5.0 KiB)

On 2013/10/10 02:26:45, axw wrote:
> On 2013/10/09 16:29:42, rog wrote:
> > LGTM with some thoughts below.

> Thanks. I'll change the newMetadata/oldMetadata thing as suggested,
but I think
> I'll leave the others to a followup.

> > For the future, I think that the "resolve" mode is unnecessary and
should go -
> > if we have items in the target bucket that we don't have hashes for,
we should
> > just ignore them. This will actually work fine in practice, I think,
even if
> we
> > abort a SyncTools half way through, because when we retry the
SyncTools, any
> > items in the target bucket that we didn't get around to adding to
the target
> > bucket simplestreams data should have metadata in the source
bucket's
> > simplestreams data.
> >
> > In general, I think retro-discovering hash data by trying to find
out what the
> > data currently is is a flawed idea - the hash of some tools should
be
> generated
> > on first upload and preserved forever after.

> I'd like to say I agree, however I know the requirements here are a
bit subtle.
> I think it'd be good to talk with wallyworld at SFO to nut it out.

I discussed this with wallyworld on IRC, and he agreed that it should go
in 1.18.
After 1.16, all tools in storage should have corresponding metadata with
size and hash.

> >

https://codereview.appspot.com/14527043/diff/5001/environs/tools/simplestreams.go
> > File environs/tools/simplestreams.go (right):
> >
> >

https://codereview.appspot.com/14527043/diff/5001/environs/tools/simplestreams.go#newcode240
> > environs/tools/simplestreams.go:240: func MergeMetadata(newMetadata,
> oldMetadata
> > []*ToolsMetadata) []*ToolsMetadata {
> > On 2013/10/09 14:28:25, axw wrote:
> > > On 2013/10/09 12:08:57, rog wrote:
> > > > I'm not sure that "new" and "old" are appropriate prefixes here
 - they
> seem
> > to
> > > > imply that the arguments are treated differently, but isn't
 MergeMetadata
> > > > commutative?
> > > >
> > > > Personally I'd use something like m0 and m1, but go with your
 own thoughts
> > for
> > > > naming.
> > >
> > > It's not commutative, per the last sentence of the doc comment:
> > > "If both entries have information, prefer the entry from
 newMetadata".
> > >
> > > The idea behind that is that if both have size&hash, then the one
 from
> > > "newMetadata" is considered newer. That's to cater for copying
 over the top
> of
> > > existing tools (say, a dev build).
> >
> > Hmm, that *should* never happen - a version should always correspond
 only to
> one
> > particular tarball. I think if we do get a clash like this, then we
 have a
> > problem that I suspect our current logic won't cope with.
> >
> > Perhaps this function should check for inconsistencies and return an
 error if
> it
> > finds one.

> Yeah, that sounds sensible. The tools won't be copied over if they are
already
> in the target, so the "newMetadata" entry won't have a size/hash. So
this code
> is only handling a theoretical case, anyway. I'll make this change and
> repropose.

> >

https://codereview.appspot.com/14527043/diff/5001/environs/tools/simplestreams.go#newcode292
> > environs/tools/simplestreams.go:292: type ResolveFlag bool
> > On 2013/10/09 14:28:25, axw wrote:
> ...

Read more...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'cmd/jujud/agent_test.go'
--- cmd/jujud/agent_test.go 2013-10-01 12:51:12 +0000
+++ cmd/jujud/agent_test.go 2013-10-16 05:27:02 +0000
@@ -192,7 +192,7 @@
192func (s *agentSuite) primeAgent(c *gc.C, tag, password string) (agent.Config, *coretools.Tools) {192func (s *agentSuite) primeAgent(c *gc.C, tag, password string) (agent.Config, *coretools.Tools) {
193 stor := s.Conn.Environ.Storage()193 stor := s.Conn.Environ.Storage()
194 agentTools := envtesting.PrimeTools(c, stor, s.DataDir(), version.Current)194 agentTools := envtesting.PrimeTools(c, stor, s.DataDir(), version.Current)
195 err := envtools.WriteMetadata(coretools.List{agentTools}, true, stor)195 err := envtools.MergeAndWriteMetadata(stor, coretools.List{agentTools})
196 c.Assert(err, gc.IsNil)196 c.Assert(err, gc.IsNil)
197 tools1, err := agenttools.ChangeAgentTools(s.DataDir(), tag, version.Current)197 tools1, err := agenttools.ChangeAgentTools(s.DataDir(), tag, version.Current)
198 c.Assert(err, gc.IsNil)198 c.Assert(err, gc.IsNil)
199199
=== modified file 'cmd/plugins/juju-metadata/toolsmetadata.go'
--- cmd/plugins/juju-metadata/toolsmetadata.go 2013-09-19 02:53:44 +0000
+++ cmd/plugins/juju-metadata/toolsmetadata.go 2013-10-16 05:27:02 +0000
@@ -12,9 +12,11 @@
12 "launchpad.net/juju-core/cmd"12 "launchpad.net/juju-core/cmd"
13 "launchpad.net/juju-core/environs/config"13 "launchpad.net/juju-core/environs/config"
14 "launchpad.net/juju-core/environs/filestorage"14 "launchpad.net/juju-core/environs/filestorage"
15 "launchpad.net/juju-core/environs/storage"
15 "launchpad.net/juju-core/environs/sync"16 "launchpad.net/juju-core/environs/sync"
16 "launchpad.net/juju-core/environs/tools"17 "launchpad.net/juju-core/environs/tools"
17 "launchpad.net/juju-core/provider/ec2/httpstorage"18 "launchpad.net/juju-core/provider/ec2/httpstorage"
19 coretools "launchpad.net/juju-core/tools"
18 "launchpad.net/juju-core/utils"20 "launchpad.net/juju-core/utils"
19 "launchpad.net/juju-core/version"21 "launchpad.net/juju-core/version"
20)22)
@@ -42,7 +44,6 @@
4244
43func (c *ToolsMetadataCommand) SetFlags(f *gnuflag.FlagSet) {45func (c *ToolsMetadataCommand) SetFlags(f *gnuflag.FlagSet) {
44 c.EnvCommandBase.SetFlags(f)46 c.EnvCommandBase.SetFlags(f)
45 f.BoolVar(&c.fetch, "fetch", true, "fetch tools and compute content size and hash")
46 f.StringVar(&c.metadataDir, "d", "", "local directory in which to store metadata")47 f.StringVar(&c.metadataDir, "d", "", "local directory in which to store metadata")
47}48}
4849
@@ -73,5 +74,23 @@
73 if err != nil {74 if err != nil {
74 return err75 return err
75 }76 }
76 return tools.WriteMetadata(toolsList, c.fetch, targetStorage)77 return mergeAndWriteMetadata(targetStorage, toolsList)
78}
79
80// This is essentially the same as tools.MergeAndWriteMetadata, but also
81// resolves metadata for existing tools by fetching them and computing
82// size/sha256 locally.
83func mergeAndWriteMetadata(stor storage.Storage, toolsList coretools.List) error {
84 existing, err := tools.ReadMetadata(stor)
85 if err != nil {
86 return err
87 }
88 metadata := tools.MetadataFromTools(toolsList)
89 if metadata, err = tools.MergeMetadata(metadata, existing); err != nil {
90 return err
91 }
92 if err = tools.ResolveMetadata(stor, metadata); err != nil {
93 return err
94 }
95 return tools.WriteMetadata(stor, metadata)
77}96}
7897
=== modified file 'cmd/plugins/juju-metadata/toolsmetadata_test.go'
--- cmd/plugins/juju-metadata/toolsmetadata_test.go 2013-09-30 19:52:11 +0000
+++ cmd/plugins/juju-metadata/toolsmetadata_test.go 2013-10-16 05:27:02 +0000
@@ -22,10 +22,12 @@
22 ttesting "launchpad.net/juju-core/environs/tools/testing"22 ttesting "launchpad.net/juju-core/environs/tools/testing"
23 "launchpad.net/juju-core/provider/dummy"23 "launchpad.net/juju-core/provider/dummy"
24 coretesting "launchpad.net/juju-core/testing"24 coretesting "launchpad.net/juju-core/testing"
25 "launchpad.net/juju-core/testing/testbase"
25 "launchpad.net/juju-core/version"26 "launchpad.net/juju-core/version"
26)27)
2728
28type ToolsMetadataSuite struct {29type ToolsMetadataSuite struct {
30 testbase.LoggingSuite
29 home *coretesting.FakeHome31 home *coretesting.FakeHome
30 env environs.Environ32 env environs.Environ
31}33}
@@ -33,7 +35,13 @@
33var _ = gc.Suite(&ToolsMetadataSuite{})35var _ = gc.Suite(&ToolsMetadataSuite{})
3436
35func (s *ToolsMetadataSuite) SetUpTest(c *gc.C) {37func (s *ToolsMetadataSuite) SetUpTest(c *gc.C) {
38 s.LoggingSuite.SetUpTest(c)
36 s.home = coretesting.MakeSampleHome(c)39 s.home = coretesting.MakeSampleHome(c)
40 s.AddCleanup(func(*gc.C) {
41 s.home.Restore()
42 dummy.Reset()
43 loggo.ResetLoggers()
44 })
37 env, err := environs.PrepareFromName("erewhemos", configstore.NewMem())45 env, err := environs.PrepareFromName("erewhemos", configstore.NewMem())
38 c.Assert(err, gc.IsNil)46 c.Assert(err, gc.IsNil)
39 s.env = env47 s.env = env
@@ -41,12 +49,6 @@
41 loggo.GetLogger("").SetLogLevel(loggo.INFO)49 loggo.GetLogger("").SetLogLevel(loggo.INFO)
42}50}
4351
44func (s *ToolsMetadataSuite) TearDownTest(c *gc.C) {
45 dummy.Reset()
46 loggo.ResetLoggers()
47 s.home.Restore()
48}
49
50var currentVersionStrings = []string{52var currentVersionStrings = []string{
51 // only these ones will make it into the JSON files.53 // only these ones will make it into the JSON files.
52 version.Current.Number.String() + "-quantal-amd64",54 version.Current.Number.String() + "-quantal-amd64",
@@ -66,13 +68,13 @@
6668
67func makeExpectedOutputCommon() string {69func makeExpectedOutputCommon() string {
68 expected := `Finding tools\.\.\.70 expected := `Finding tools\.\.\.
69.*Fetching tools to generate hash: .*/tools/.*juju-1\.12\.0-precise-amd64\.tgz71.*Fetching tools to generate hash: 1\.12\.0-precise-amd64
70.*Fetching tools to generate hash: .*/tools/.*juju-1\.12\.0-precise-i386\.tgz72.*Fetching tools to generate hash: 1\.12\.0-precise-i386
71.*Fetching tools to generate hash: .*/tools/.*juju-1\.12\.0-raring-amd64\.tgz73.*Fetching tools to generate hash: 1\.12\.0-raring-amd64
72.*Fetching tools to generate hash: .*/tools/.*juju-1\.12\.0-raring-i386\.tgz74.*Fetching tools to generate hash: 1\.12\.0-raring-i386
73.*Fetching tools to generate hash: .*/tools/.*juju-1\.13\.0-precise-amd64\.tgz75.*Fetching tools to generate hash: 1\.13\.0-precise-amd64
74`76`
75 f := ".*Fetching tools to generate hash: .*/tools/.*juju-%s\\.tgz\n"77 f := ".*Fetching tools to generate hash: %s\n"
76 for _, v := range currentVersionStrings {78 for _, v := range currentVersionStrings {
77 expected += fmt.Sprintf(f, regexp.QuoteMeta(v))79 expected += fmt.Sprintf(f, regexp.QuoteMeta(v))
78 }80 }
@@ -145,8 +147,8 @@
145 output := ctx.Stdout.(*bytes.Buffer).String()147 output := ctx.Stdout.(*bytes.Buffer).String()
146 expectedOutput := fmt.Sprintf(`148 expectedOutput := fmt.Sprintf(`
147Finding tools\.\.\.149Finding tools\.\.\.
148.*Fetching tools to generate hash: .*/tools/releases/juju-%s\.tgz150.*Fetching tools to generate hash: %s
149.*Fetching tools to generate hash: .*/tools/releases/juju-%s\.tgz151.*Fetching tools to generate hash: %s
150.*Writing tools/streams/v1/index\.json152.*Writing tools/streams/v1/index\.json
151.*Writing tools/streams/v1/com\.ubuntu\.juju:released:tools\.json153.*Writing tools/streams/v1/com\.ubuntu\.juju:released:tools\.json
152`[1:], regexp.QuoteMeta(versionStrings[0]), regexp.QuoteMeta(versionStrings[1]))154`[1:], regexp.QuoteMeta(versionStrings[0]), regexp.QuoteMeta(versionStrings[1]))
153155
=== modified file 'environs/sync/sync.go'
--- environs/sync/sync.go 2013-10-10 11:40:54 +0000
+++ environs/sync/sync.go 2013-10-16 05:27:02 +0000
@@ -120,7 +120,7 @@
120 logger.Infof("generating tools metadata")120 logger.Infof("generating tools metadata")
121 if !syncContext.DryRun {121 if !syncContext.DryRun {
122 targetTools = append(targetTools, missing...)122 targetTools = append(targetTools, missing...)
123 err = envtools.WriteMetadata(targetTools, true, targetStorage)123 err = envtools.MergeAndWriteMetadata(targetStorage, targetTools)
124 if err != nil {124 if err != nil {
125 return err125 return err
126 }126 }
127127
=== modified file 'environs/testing/tools.go'
--- environs/testing/tools.go 2013-10-03 13:19:40 +0000
+++ environs/testing/tools.go 2013-10-16 05:27:02 +0000
@@ -112,16 +112,25 @@
112112
113// UploadFakeToolsVersions puts fake tools in the supplied storage for the supplied versions.113// UploadFakeToolsVersions puts fake tools in the supplied storage for the supplied versions.
114func UploadFakeToolsVersions(stor storage.Storage, versions ...version.Binary) ([]*coretools.Tools, error) {114func UploadFakeToolsVersions(stor storage.Storage, versions ...version.Binary) ([]*coretools.Tools, error) {
115 // Leave existing tools alone.
116 existingTools := make(map[version.Binary]*coretools.Tools)
117 existing, _ := envtools.ReadList(stor, 1, -1)
118 for _, tools := range existing {
119 existingTools[tools.Version] = tools
120 }
115 var agentTools coretools.List = make(coretools.List, len(versions))121 var agentTools coretools.List = make(coretools.List, len(versions))
116 for i, version := range versions {122 for i, version := range versions {
117 t, err := uploadFakeToolsVersion(stor, version)123 if tools, ok := existingTools[version]; ok {
118 if err != nil {124 agentTools[i] = tools
119 return nil, err125 } else {
126 t, err := uploadFakeToolsVersion(stor, version)
127 if err != nil {
128 return nil, err
129 }
130 agentTools[i] = t
120 }131 }
121 agentTools[i] = t
122 }132 }
123 err := envtools.WriteMetadata(agentTools, true, stor)133 if err := envtools.MergeAndWriteMetadata(stor, agentTools); err != nil {
124 if err != nil {
125 return nil, err134 return nil, err
126 }135 }
127 return agentTools, nil136 return agentTools, nil
@@ -144,7 +153,7 @@
144 }153 }
145 agentTools[i] = t154 agentTools[i] = t
146 }155 }
147 err := envtools.WriteMetadata(agentTools, true, stor)156 err := envtools.MergeAndWriteMetadata(stor, agentTools)
148 if err != nil {157 if err != nil {
149 panic(err)158 panic(err)
150 }159 }
@@ -182,6 +191,7 @@
182191
183// RemoveFakeTools deletes the fake tools from the supplied storage.192// RemoveFakeTools deletes the fake tools from the supplied storage.
184func RemoveFakeTools(c *gc.C, stor storage.Storage) {193func RemoveFakeTools(c *gc.C, stor storage.Storage) {
194 c.Logf("removing fake tools")
185 toolsVersion := version.Current195 toolsVersion := version.Current
186 name := envtools.StorageName(toolsVersion)196 name := envtools.StorageName(toolsVersion)
187 err := stor.Remove(name)197 err := stor.Remove(name)
188198
=== modified file 'environs/tools/simplestreams.go'
--- environs/tools/simplestreams.go 2013-10-10 11:40:54 +0000
+++ environs/tools/simplestreams.go 2013-10-16 05:27:02 +0000
@@ -12,7 +12,6 @@
12 "fmt"12 "fmt"
13 "hash"13 "hash"
14 "io"14 "io"
15 "net/http"
16 "strings"15 "strings"
17 "time"16 "time"
1817
@@ -138,7 +137,7 @@
138 Arch string `json:"arch"`137 Arch string `json:"arch"`
139 Size int64 `json:"size"`138 Size int64 `json:"size"`
140 Path string `json:"path"`139 Path string `json:"path"`
141 FullPath string `json:"-,omitempty"`140 FullPath string `json:"-"`
142 FileType string `json:"ftype"`141 FileType string `json:"ftype"`
143 SHA256 string `json:"sha256"`142 SHA256 string `json:"sha256"`
144}143}
@@ -147,6 +146,16 @@
147 return fmt.Sprintf("%+v", *t)146 return fmt.Sprintf("%+v", *t)
148}147}
149148
149// binary returns the tools metadata's binary version,
150// which may be used for map lookup.
151func (t *ToolsMetadata) binary() version.Binary {
152 return version.Binary{
153 Number: version.MustParse(t.Version),
154 Series: t.Release,
155 Arch: t.Arch,
156 }
157}
158
150func (t *ToolsMetadata) productId() (string, error) {159func (t *ToolsMetadata) productId() (string, error) {
151 seriesVersion, err := simplestreams.SeriesVersion(t.Release)160 seriesVersion, err := simplestreams.SeriesVersion(t.Release)
152 if err != nil {161 if err != nil {
@@ -199,10 +208,10 @@
199func appendMatchingTools(source simplestreams.DataSource, matchingTools []interface{},208func appendMatchingTools(source simplestreams.DataSource, matchingTools []interface{},
200 tools map[string]interface{}, cons simplestreams.LookupConstraint) []interface{} {209 tools map[string]interface{}, cons simplestreams.LookupConstraint) []interface{} {
201210
202 toolsMap := make(map[string]*ToolsMetadata, len(matchingTools))211 toolsMap := make(map[version.Binary]*ToolsMetadata, len(matchingTools))
203 for _, val := range matchingTools {212 for _, val := range matchingTools {
204 tm := val.(*ToolsMetadata)213 tm := val.(*ToolsMetadata)
205 toolsMap[fmt.Sprintf("%s-%s-%s", tm.Release, tm.Version, tm.Arch)] = tm214 toolsMap[tm.binary()] = tm
206 }215 }
207 for _, val := range tools {216 for _, val := range tools {
208 tm := val.(*ToolsMetadata)217 tm := val.(*ToolsMetadata)
@@ -227,7 +236,7 @@
227 }236 }
228 }237 }
229 }238 }
230 if _, ok := toolsMap[fmt.Sprintf("%s-%s-%s", tm.Release, tm.Version, tm.Arch)]; !ok {239 if _, ok := toolsMap[tm.binary()]; !ok {
231 tm.FullPath, _ = source.URL(tm.Path)240 tm.FullPath, _ = source.URL(tm.Path)
232 matchingTools = append(matchingTools, tm)241 matchingTools = append(matchingTools, tm)
233 }242 }
@@ -240,101 +249,142 @@
240 Data []byte249 Data []byte
241}250}
242251
243func WriteMetadata(toolsList coretools.List, fetch bool, metadataStore storage.Storage) error {252// MetadataFromTools returns a tools metadata list derived from the
244 // Read any existing metadata so we can merge the new tools metadata with what's there.253// given tools list. The size and sha256 will not be computed if
245 // The metadata from toolsList is already present, the existing data is overwritten.254// missing.
246 dataSource := storage.NewStorageSimpleStreamsDataSource(metadataStore, "tools")255func MetadataFromTools(toolsList coretools.List) []*ToolsMetadata {
247 toolsConstraint, err := makeToolsConstraint(simplestreams.CloudSpec{}, -1, -1, coretools.Filter{})
248 if err != nil {
249 return err
250 }
251 existingMetadata, err := Fetch([]simplestreams.DataSource{dataSource}, simplestreams.DefaultIndexPath, toolsConstraint, false)
252 if err != nil && !errors.IsNotFoundError(err) {
253 return err
254 }
255 newToolsVersions := make(map[string]bool)
256 for _, tool := range toolsList {
257 newToolsVersions[tool.Version.String()] = true
258 }
259 // Merge in existing records.
260 for _, toolsMetadata := range existingMetadata {
261 vers := version.Binary{version.MustParse(toolsMetadata.Version), toolsMetadata.Release, toolsMetadata.Arch}
262 if _, ok := newToolsVersions[vers.String()]; ok {
263 continue
264 }
265 tool := &coretools.Tools{
266 Version: vers,
267 SHA256: toolsMetadata.SHA256,
268 Size: toolsMetadata.Size,
269 }
270 toolsList = append(toolsList, tool)
271 }
272 metadataInfo, err := generateMetadata(toolsList, fetch)
273 if err != nil {
274 return err
275 }
276 for _, md := range metadataInfo {
277 logger.Infof("Writing %s", "tools/"+md.Path)
278 err = metadataStore.Put("tools/"+md.Path, bytes.NewReader(md.Data), int64(len(md.Data)))
279 if err != nil {
280 return err
281 }
282 }
283 return nil
284}
285
286func generateMetadata(toolsList coretools.List, fetch bool) ([]MetadataFile, error) {
287 metadata := make([]*ToolsMetadata, len(toolsList))256 metadata := make([]*ToolsMetadata, len(toolsList))
288 for i, t := range toolsList {257 for i, t := range toolsList {
289 var size int64
290 var sha256hex string
291 var err error
292 if fetch && t.Size == 0 {
293 logger.Infof("Fetching tools to generate hash: %v", t.URL)
294 var sha256hash hash.Hash
295 size, sha256hash, err = fetchToolsHash(t.URL)
296 if err != nil {
297 return nil, err
298 }
299 sha256hex = fmt.Sprintf("%x", sha256hash.Sum(nil))
300 } else {
301 size = t.Size
302 sha256hex = t.SHA256
303 }
304
305 path := fmt.Sprintf("releases/juju-%s-%s-%s.tgz", t.Version.Number, t.Version.Series, t.Version.Arch)258 path := fmt.Sprintf("releases/juju-%s-%s-%s.tgz", t.Version.Number, t.Version.Series, t.Version.Arch)
306 metadata[i] = &ToolsMetadata{259 metadata[i] = &ToolsMetadata{
307 Release: t.Version.Series,260 Release: t.Version.Series,
308 Version: t.Version.Number.String(),261 Version: t.Version.Number.String(),
309 Arch: t.Version.Arch,262 Arch: t.Version.Arch,
263 FullPath: t.URL,
310 Path: path,264 Path: path,
311 FileType: "tar.gz",265 FileType: "tar.gz",
312 Size: size,266 Size: t.Size,
313 SHA256: sha256hex,267 SHA256: t.SHA256,
314 }268 }
315 }269 }
316270 return metadata
271}
272
273// ResolveMetadata resolves incomplete metadata
274// by fetching the tools from storage and computing
275// the size and hash locally.
276func ResolveMetadata(stor storage.StorageReader, metadata []*ToolsMetadata) error {
277 for _, md := range metadata {
278 if md.Size != 0 {
279 continue
280 }
281 binary := md.binary()
282 logger.Infof("Fetching tools to generate hash: %v", binary)
283 var sha256hash hash.Hash
284 size, sha256hash, err := fetchToolsHash(stor, binary)
285 if err != nil {
286 return err
287 }
288 md.Size = size
289 md.SHA256 = fmt.Sprintf("%x", sha256hash.Sum(nil))
290 }
291 return nil
292}
293
294// MergeMetadata merges the given tools metadata.
295// If metadata for the same tools version exists in both lists,
296// an entry with non-empty size/SHA256 takes precedence; if
297// the two entries have different sizes/hashes, then an error is
298// returned.
299func MergeMetadata(tmlist1, tmlist2 []*ToolsMetadata) ([]*ToolsMetadata, error) {
300 merged := make(map[version.Binary]*ToolsMetadata)
301 for _, tm := range tmlist1 {
302 merged[tm.binary()] = tm
303 }
304 for _, tm := range tmlist2 {
305 binary := tm.binary()
306 if existing, ok := merged[binary]; ok {
307 if tm.Size != 0 {
308 if existing.Size == 0 {
309 merged[binary] = tm
310 } else if existing.Size != tm.Size || existing.SHA256 != tm.SHA256 {
311 return nil, fmt.Errorf(
312 "metadata mismatch for %s: sizes=(%v,%v) sha256=(%v,%v)",
313 binary.String(),
314 existing.Size, tm.Size,
315 existing.SHA256, tm.SHA256,
316 )
317 }
318 }
319 } else {
320 merged[binary] = tm
321 }
322 }
323 list := make([]*ToolsMetadata, 0, len(merged))
324 for _, metadata := range merged {
325 list = append(list, metadata)
326 }
327 return list, nil
328}
329
330// ReadMetadata returns the tools metadata from the given storage.
331func ReadMetadata(store storage.StorageReader) ([]*ToolsMetadata, error) {
332 dataSource := storage.NewStorageSimpleStreamsDataSource(store, "tools")
333 toolsConstraint, err := makeToolsConstraint(simplestreams.CloudSpec{}, -1, -1, coretools.Filter{})
334 if err != nil {
335 return nil, err
336 }
337 metadata, err := Fetch([]simplestreams.DataSource{dataSource}, simplestreams.DefaultIndexPath, toolsConstraint, false)
338 if err != nil && !errors.IsNotFoundError(err) {
339 return nil, err
340 }
341 return metadata, nil
342}
343
344// WriteMetadata writes the given tools metadata to the given storage.
345func WriteMetadata(stor storage.Storage, metadata []*ToolsMetadata) error {
317 index, products, err := MarshalToolsMetadataJSON(metadata, time.Now())346 index, products, err := MarshalToolsMetadataJSON(metadata, time.Now())
318 if err != nil {347 if err != nil {
319 return nil, err348 return err
320 }349 }
321 objects := []MetadataFile{350 metadataInfo := []MetadataFile{
322 {simplestreams.UnsignedIndex, index},351 {simplestreams.UnsignedIndex, index},
323 {ProductMetadataPath, products},352 {ProductMetadataPath, products},
324 }353 }
325 return objects, nil354 for _, md := range metadataInfo {
326}355 logger.Infof("Writing %s", "tools/"+md.Path)
327356 err = stor.Put("tools/"+md.Path, bytes.NewReader(md.Data), int64(len(md.Data)))
328// fetchToolsHash fetches the file at the specified URL,357 if err != nil {
329// and calculates its size in bytes and computes a SHA256358 return err
330// hash of its contents.359 }
331func fetchToolsHash(url string) (size int64, sha256hash hash.Hash, err error) {360 }
332 resp, err := http.Get(url)361 return nil
362}
363
364// MergeAndWriteMetadata reads the existing metadata from storage (if any),
365// and merges it with metadata generated from the given tools list. The
366// resulting metadata is written to storage.
367func MergeAndWriteMetadata(stor storage.Storage, tools coretools.List) error {
368 existing, err := ReadMetadata(stor)
369 if err != nil {
370 return err
371 }
372 metadata := MetadataFromTools(tools)
373 if metadata, err = MergeMetadata(metadata, existing); err != nil {
374 return err
375 }
376 return WriteMetadata(stor, metadata)
377}
378
379// fetchToolsHash fetches the tools from storage and calculates
380// its size in bytes and computes a SHA256 hash of its contents.
381func fetchToolsHash(stor storage.StorageReader, ver version.Binary) (size int64, sha256hash hash.Hash, err error) {
382 r, err := storage.Get(stor, StorageName(ver))
333 if err != nil {383 if err != nil {
334 return 0, nil, err384 return 0, nil, err
335 }385 }
386 defer r.Close()
336 sha256hash = sha256.New()387 sha256hash = sha256.New()
337 size, err = io.Copy(sha256hash, resp.Body)388 size, err = io.Copy(sha256hash, r)
338 resp.Body.Close()
339 return size, sha256hash, err389 return size, sha256hash, err
340}390}
341391
=== modified file 'environs/tools/simplestreams_test.go'
--- environs/tools/simplestreams_test.go 2013-10-10 11:40:54 +0000
+++ environs/tools/simplestreams_test.go 2013-10-16 05:27:02 +0000
@@ -7,8 +7,8 @@
7 "bytes"7 "bytes"
8 "flag"8 "flag"
9 "fmt"9 "fmt"
10 "io"
10 "net/http"11 "net/http"
11 "path/filepath"
12 "reflect"12 "reflect"
13 "strings"13 "strings"
14 "testing"14 "testing"
@@ -20,8 +20,10 @@
20 "launchpad.net/juju-core/environs/jujutest"20 "launchpad.net/juju-core/environs/jujutest"
21 "launchpad.net/juju-core/environs/simplestreams"21 "launchpad.net/juju-core/environs/simplestreams"
22 sstesting "launchpad.net/juju-core/environs/simplestreams/testing"22 sstesting "launchpad.net/juju-core/environs/simplestreams/testing"
23 "launchpad.net/juju-core/environs/storage"
23 "launchpad.net/juju-core/environs/tools"24 "launchpad.net/juju-core/environs/tools"
24 ttesting "launchpad.net/juju-core/environs/tools/testing"25 ttesting "launchpad.net/juju-core/environs/tools/testing"
26 "launchpad.net/juju-core/testing/testbase"
25 coretools "launchpad.net/juju-core/tools"27 coretools "launchpad.net/juju-core/tools"
26 "launchpad.net/juju-core/version"28 "launchpad.net/juju-core/version"
27)29)
@@ -280,14 +282,9 @@
280 c.Assert(toolsMetadata[0], gc.DeepEquals, expectedMetadata)282 c.Assert(toolsMetadata[0], gc.DeepEquals, expectedMetadata)
281}283}
282284
283func assertMetadataMatches(c *gc.C, toolList coretools.List, metadata []*tools.ToolsMetadata) {285func assertMetadataMatches(c *gc.C, storageDir string, toolList coretools.List, metadata []*tools.ToolsMetadata) {
284 var expectedMetadata []*tools.ToolsMetadata = make([]*tools.ToolsMetadata, len(toolList))286 var expectedMetadata []*tools.ToolsMetadata = make([]*tools.ToolsMetadata, len(toolList))
285 for i, tool := range toolList {287 for i, tool := range toolList {
286 if tool.URL != "" {
287 size, sha256 := ttesting.SHA256sum(c, tool.URL)
288 tool.SHA256 = sha256
289 tool.Size = size
290 }
291 expectedMetadata[i] = &tools.ToolsMetadata{288 expectedMetadata[i] = &tools.ToolsMetadata{
292 Release: tool.Version.Series,289 Release: tool.Version.Series,
293 Version: tool.Version.Number.String(),290 Version: tool.Version.Number.String(),
@@ -316,10 +313,10 @@
316 dir := c.MkDir()313 dir := c.MkDir()
317 writer, err := filestorage.NewFileStorageWriter(dir, filestorage.UseDefaultTmpDir)314 writer, err := filestorage.NewFileStorageWriter(dir, filestorage.UseDefaultTmpDir)
318 c.Assert(err, gc.IsNil)315 c.Assert(err, gc.IsNil)
319 err = tools.WriteMetadata(toolsList, false, writer)316 err = tools.MergeAndWriteMetadata(writer, toolsList)
320 c.Assert(err, gc.IsNil)317 c.Assert(err, gc.IsNil)
321 metadata := ttesting.ParseMetadata(c, dir)318 metadata := ttesting.ParseMetadata(c, dir)
322 assertMetadataMatches(c, toolsList, metadata)319 assertMetadataMatches(c, dir, toolsList, metadata)
323}320}
324321
325func (s *simplestreamsSuite) TestWriteMetadata(c *gc.C) {322func (s *simplestreamsSuite) TestWriteMetadata(c *gc.C) {
@@ -338,15 +335,16 @@
338 SHA256: "abcd",335 SHA256: "abcd",
339 }, {336 }, {
340 Version: version.MustParseBinary("2.0.1-raring-amd64"),337 Version: version.MustParseBinary("2.0.1-raring-amd64"),
341 URL: "file://" + filepath.Join(dir, "tools/releases/juju-2.0.1-raring-amd64.tgz"),338 // The URL is not used for generating metadata.
339 URL: "bogus://",
342 },340 },
343 }341 }
344 writer, err := filestorage.NewFileStorageWriter(dir, filestorage.UseDefaultTmpDir)342 writer, err := filestorage.NewFileStorageWriter(dir, filestorage.UseDefaultTmpDir)
345 c.Assert(err, gc.IsNil)343 c.Assert(err, gc.IsNil)
346 err = tools.WriteMetadata(toolsList, true, writer)344 err = tools.MergeAndWriteMetadata(writer, toolsList)
347 c.Assert(err, gc.IsNil)345 c.Assert(err, gc.IsNil)
348 metadata := ttesting.ParseMetadata(c, dir)346 metadata := ttesting.ParseMetadata(c, dir)
349 assertMetadataMatches(c, toolsList, metadata)347 assertMetadataMatches(c, dir, toolsList, metadata)
350}348}
351349
352func (s *simplestreamsSuite) TestWriteMetadataMergeWithExisting(c *gc.C) {350func (s *simplestreamsSuite) TestWriteMetadataMergeWithExisting(c *gc.C) {
@@ -364,7 +362,7 @@
364 }362 }
365 writer, err := filestorage.NewFileStorageWriter(dir, filestorage.UseDefaultTmpDir)363 writer, err := filestorage.NewFileStorageWriter(dir, filestorage.UseDefaultTmpDir)
366 c.Assert(err, gc.IsNil)364 c.Assert(err, gc.IsNil)
367 err = tools.WriteMetadata(existingToolsList, true, writer)365 err = tools.MergeAndWriteMetadata(writer, existingToolsList)
368 c.Assert(err, gc.IsNil)366 c.Assert(err, gc.IsNil)
369 newToolsList := coretools.List{367 newToolsList := coretools.List{
370 existingToolsList[0],368 existingToolsList[0],
@@ -374,11 +372,11 @@
374 SHA256: "def",372 SHA256: "def",
375 },373 },
376 }374 }
377 err = tools.WriteMetadata(newToolsList, true, writer)375 err = tools.MergeAndWriteMetadata(writer, newToolsList)
378 c.Assert(err, gc.IsNil)376 c.Assert(err, gc.IsNil)
379 requiredToolsList := append(existingToolsList, newToolsList[1])377 requiredToolsList := append(existingToolsList, newToolsList[1])
380 metadata := ttesting.ParseMetadata(c, dir)378 metadata := ttesting.ParseMetadata(c, dir)
381 assertMetadataMatches(c, requiredToolsList, metadata)379 assertMetadataMatches(c, dir, requiredToolsList, metadata)
382}380}
383381
384type productSpecSuite struct{}382type productSpecSuite struct{}
@@ -480,6 +478,219 @@
480 c.Assert(item.(*tools.ToolsMetadata).Size, gc.Equals, int64(9223372036854775807))478 c.Assert(item.(*tools.ToolsMetadata).Size, gc.Equals, int64(9223372036854775807))
481}479}
482480
481type metadataHelperSuite struct {
482 testbase.LoggingSuite
483}
484
485var _ = gc.Suite(&metadataHelperSuite{})
486
487func (*metadataHelperSuite) TestMetadataFromTools(c *gc.C) {
488 metadata := tools.MetadataFromTools(nil)
489 c.Assert(metadata, gc.HasLen, 0)
490
491 toolsList := coretools.List{{
492 Version: version.MustParseBinary("1.2.3-precise-amd64"),
493 Size: 123,
494 SHA256: "abc",
495 }, {
496 Version: version.MustParseBinary("2.0.1-raring-amd64"),
497 URL: "file:///tmp/releases/juju-2.0.1-raring-amd64.tgz",
498 Size: 456,
499 SHA256: "xyz",
500 }}
501 metadata = tools.MetadataFromTools(toolsList)
502 c.Assert(metadata, gc.HasLen, len(toolsList))
503 for i, t := range toolsList {
504 md := metadata[i]
505 c.Assert(md.Release, gc.Equals, t.Version.Series)
506 c.Assert(md.Version, gc.Equals, t.Version.Number.String())
507 c.Assert(md.Arch, gc.Equals, t.Version.Arch)
508 c.Assert(md.FullPath, gc.Equals, t.URL)
509 c.Assert(md.Path, gc.Equals, tools.StorageName(t.Version)[len("tools/"):])
510 c.Assert(md.FileType, gc.Equals, "tar.gz")
511 c.Assert(md.Size, gc.Equals, t.Size)
512 c.Assert(md.SHA256, gc.Equals, t.SHA256)
513 }
514}
515
516type countingStorage struct {
517 storage.StorageReader
518 counter int
519}
520
521func (c *countingStorage) Get(name string) (io.ReadCloser, error) {
522 c.counter++
523 return c.StorageReader.Get(name)
524}
525
526func (*metadataHelperSuite) TestResolveMetadata(c *gc.C) {
527 var versionStrings = []string{"1.2.3-precise-amd64"}
528 dir := c.MkDir()
529 ttesting.MakeTools(c, dir, "releases", versionStrings)
530 toolsList := coretools.List{{
531 Version: version.MustParseBinary(versionStrings[0]),
532 Size: 123,
533 SHA256: "abc",
534 }}
535
536 stor, err := filestorage.NewFileStorageReader(dir)
537 c.Assert(err, gc.IsNil)
538 err = tools.ResolveMetadata(stor, nil)
539 c.Assert(err, gc.IsNil)
540
541 // We already have size/sha256, so ensure that storage isn't consulted.
542 countingStorage := &countingStorage{StorageReader: stor}
543 metadata := tools.MetadataFromTools(toolsList)
544 err = tools.ResolveMetadata(countingStorage, metadata)
545 c.Assert(err, gc.IsNil)
546 c.Assert(countingStorage.counter, gc.Equals, 0)
547
548 // Now clear size/sha256, and check that it is called, and
549 // the size/sha256 sum are updated.
550 metadata[0].Size = 0
551 metadata[0].SHA256 = ""
552 err = tools.ResolveMetadata(countingStorage, metadata)
553 c.Assert(err, gc.IsNil)
554 c.Assert(countingStorage.counter, gc.Equals, 1)
555 c.Assert(metadata[0].Size, gc.Not(gc.Equals), 0)
556 c.Assert(metadata[0].SHA256, gc.Not(gc.Equals), "")
557}
558
559func (*metadataHelperSuite) TestMergeMetadata(c *gc.C) {
560 md1 := &tools.ToolsMetadata{
561 Release: "precise",
562 Version: "1.2.3",
563 Arch: "amd64",
564 Path: "path1",
565 }
566 md2 := &tools.ToolsMetadata{
567 Release: "precise",
568 Version: "1.2.3",
569 Arch: "amd64",
570 Path: "path2",
571 }
572 md3 := &tools.ToolsMetadata{
573 Release: "raring",
574 Version: "1.2.3",
575 Arch: "amd64",
576 Path: "path3",
577 }
578
579 withSize := func(md *tools.ToolsMetadata, size int64) *tools.ToolsMetadata {
580 clone := *md
581 clone.Size = size
582 return &clone
583 }
584 withSHA256 := func(md *tools.ToolsMetadata, sha256 string) *tools.ToolsMetadata {
585 clone := *md
586 clone.SHA256 = sha256
587 return &clone
588 }
589
590 type mdlist []*tools.ToolsMetadata
591 type test struct {
592 name string
593 lhs, rhs, merged []*tools.ToolsMetadata
594 err string
595 }
596 tests := []test{{
597 name: "non-empty lhs, empty rhs",
598 lhs: mdlist{md1},
599 rhs: nil,
600 merged: mdlist{md1},
601 }, {
602 name: "empty lhs, non-empty rhs",
603 lhs: nil,
604 rhs: mdlist{md2},
605 merged: mdlist{md2},
606 }, {
607 name: "identical lhs, rhs",
608 lhs: mdlist{md1},
609 rhs: mdlist{md1},
610 merged: mdlist{md1},
611 }, {
612 name: "same tools in lhs and rhs, neither have size: prefer lhs",
613 lhs: mdlist{md1},
614 rhs: mdlist{md2},
615 merged: mdlist{md1},
616 }, {
617 name: "same tools in lhs and rhs, only lhs has a size: prefer lhs",
618 lhs: mdlist{withSize(md1, 123)},
619 rhs: mdlist{md2},
620 merged: mdlist{withSize(md1, 123)},
621 }, {
622 name: "same tools in lhs and rhs, only rhs has a size: prefer rhs",
623 lhs: mdlist{md1},
624 rhs: mdlist{withSize(md2, 123)},
625 merged: mdlist{withSize(md2, 123)},
626 }, {
627 name: "same tools in lhs and rhs, both have the same size: prefer lhs",
628 lhs: mdlist{withSize(md1, 123)},
629 rhs: mdlist{withSize(md2, 123)},
630 merged: mdlist{withSize(md1, 123)},
631 }, {
632 name: "same tools in lhs and rhs, both have different sizes: error",
633 lhs: mdlist{withSize(md1, 123)},
634 rhs: mdlist{withSize(md2, 456)},
635 err: "metadata mismatch for 1\\.2\\.3-precise-amd64: sizes=\\(123,456\\) sha256=\\(,\\)",
636 }, {
637 name: "same tools in lhs and rhs, both have same size but different sha256: error",
638 lhs: mdlist{withSHA256(withSize(md1, 123), "a")},
639 rhs: mdlist{withSHA256(withSize(md2, 123), "b")},
640 err: "metadata mismatch for 1\\.2\\.3-precise-amd64: sizes=\\(123,123\\) sha256=\\(a,b\\)",
641 }, {
642 name: "lhs is a proper superset of rhs: union of lhs and rhs",
643 lhs: mdlist{md1, md3},
644 rhs: mdlist{md1},
645 merged: mdlist{md1, md3},
646 }, {
647 name: "rhs is a proper superset of lhs: union of lhs and rhs",
648 lhs: mdlist{md1},
649 rhs: mdlist{md1, md3},
650 merged: mdlist{md1, md3},
651 }}
652 for i, test := range tests {
653 c.Logf("test %d: %s", i, test.name)
654 merged, err := tools.MergeMetadata(test.lhs, test.rhs)
655 if test.err == "" {
656 c.Assert(err, gc.IsNil)
657 c.Assert(merged, gc.DeepEquals, test.merged)
658 } else {
659 c.Assert(err, gc.ErrorMatches, test.err)
660 c.Assert(merged, gc.IsNil)
661 }
662 }
663}
664
665func (*metadataHelperSuite) TestReadWriteMetadata(c *gc.C) {
666 metadata := []*tools.ToolsMetadata{{
667 Release: "precise",
668 Version: "1.2.3",
669 Arch: "amd64",
670 Path: "path1",
671 }, {
672 Release: "raring",
673 Version: "1.2.3",
674 Arch: "amd64",
675 Path: "path2",
676 }}
677
678 stor, err := filestorage.NewFileStorageWriter(c.MkDir(), filestorage.UseDefaultTmpDir)
679 c.Assert(err, gc.IsNil)
680 out, err := tools.ReadMetadata(stor)
681 c.Assert(out, gc.HasLen, 0)
682 c.Assert(err, gc.IsNil) // non-existence is not an error
683 err = tools.WriteMetadata(stor, metadata)
684 c.Assert(err, gc.IsNil)
685 out, err = tools.ReadMetadata(stor)
686 for _, md := range out {
687 // FullPath is set by ReadMetadata.
688 c.Assert(md.FullPath, gc.Not(gc.Equals), "")
689 md.FullPath = ""
690 }
691 c.Assert(out, gc.DeepEquals, metadata)
692}
693
483type signedSuite struct {694type signedSuite struct {
484 origKey string695 origKey string
485}696}
486697
=== modified file 'environs/tools/tools.go'
--- environs/tools/tools.go 2013-09-26 23:07:24 +0000
+++ environs/tools/tools.go 2013-10-16 05:27:02 +0000
@@ -121,13 +121,8 @@
121 }121 }
122 list = make(coretools.List, len(toolsMetadata))122 list = make(coretools.List, len(toolsMetadata))
123 for i, metadata := range toolsMetadata {123 for i, metadata := range toolsMetadata {
124 binary := version.Binary{
125 Number: version.MustParse(metadata.Version),
126 Arch: metadata.Arch,
127 Series: metadata.Release,
128 }
129 list[i] = &coretools.Tools{124 list[i] = &coretools.Tools{
130 Version: binary,125 Version: metadata.binary(),
131 URL: metadata.FullPath,126 URL: metadata.FullPath,
132 Size: metadata.Size,127 Size: metadata.Size,
133 SHA256: metadata.SHA256,128 SHA256: metadata.SHA256,
134129
=== modified file 'testing/targz.go'
--- testing/targz.go 2013-09-25 00:18:35 +0000
+++ testing/targz.go 2013-10-16 05:27:02 +0000
@@ -11,7 +11,6 @@
11 "fmt"11 "fmt"
12 "io"12 "io"
13 "os"13 "os"
14 "time"
15)14)
1615
17// TarFile represents a file to be archived.16// TarFile represents a file to be archived.
@@ -33,17 +32,16 @@
33 if ftype == 0 {32 if ftype == 0 {
34 panic(fmt.Errorf("unexpected mode %v", mode))33 panic(fmt.Errorf("unexpected mode %v", mode))
35 }34 }
35 // NOTE: Do not set attributes (e.g. times) dynamically, as various
36 // tests expect the contents of fake tools archives to be unchanging.
36 return &TarFile{37 return &TarFile{
37 Header: tar.Header{38 Header: tar.Header{
38 Typeflag: ftype,39 Typeflag: ftype,
39 Name: name,40 Name: name,
40 Size: int64(len(contents)),41 Size: int64(len(contents)),
41 Mode: int64(mode & 0777),42 Mode: int64(mode & 0777),
42 ModTime: time.Now(),43 Uname: "ubuntu",
43 AccessTime: time.Now(),44 Gname: "ubuntu",
44 ChangeTime: time.Now(),
45 Uname: "ubuntu",
46 Gname: "ubuntu",
47 },45 },
48 Contents: contents,46 Contents: contents,
49 }47 }
5048
=== modified file 'worker/upgrader/upgrader_test.go'
--- worker/upgrader/upgrader_test.go 2013-10-02 16:28:16 +0000
+++ worker/upgrader/upgrader_test.go 2013-10-16 05:27:02 +0000
@@ -83,7 +83,7 @@
83 c.Assert(err, gc.IsNil)83 c.Assert(err, gc.IsNil)
84 stor := s.Conn.Environ.Storage()84 stor := s.Conn.Environ.Storage()
85 agentTools := envtesting.PrimeTools(c, stor, s.DataDir(), vers)85 agentTools := envtesting.PrimeTools(c, stor, s.DataDir(), vers)
86 err = envtools.WriteMetadata(coretools.List{agentTools}, true, stor)86 err = envtools.MergeAndWriteMetadata(stor, coretools.List{agentTools})
87 _, err = s.machine.AgentTools()87 _, err = s.machine.AgentTools()
88 c.Assert(err, jc.Satisfies, errors.IsNotFoundError)88 c.Assert(err, jc.Satisfies, errors.IsNotFoundError)
8989
@@ -119,7 +119,7 @@
119 oldTools := envtesting.PrimeTools(c, stor, s.DataDir(), version.MustParseBinary("5.4.3-precise-amd64"))119 oldTools := envtesting.PrimeTools(c, stor, s.DataDir(), version.MustParseBinary("5.4.3-precise-amd64"))
120 newTools := envtesting.AssertUploadFakeToolsVersions(120 newTools := envtesting.AssertUploadFakeToolsVersions(
121 c, stor, version.MustParseBinary("5.4.5-precise-amd64"))[0]121 c, stor, version.MustParseBinary("5.4.5-precise-amd64"))[0]
122 err := envtools.WriteMetadata(coretools.List{oldTools, newTools}, true, stor)122 err := envtools.MergeAndWriteMetadata(stor, coretools.List{oldTools, newTools})
123 c.Assert(err, gc.IsNil)123 c.Assert(err, gc.IsNil)
124 err = statetesting.SetAgentVersion(s.State, newTools.Version.Number)124 err = statetesting.SetAgentVersion(s.State, newTools.Version.Number)
125 c.Assert(err, gc.IsNil)125 c.Assert(err, gc.IsNil)
@@ -147,7 +147,7 @@
147 oldTools := envtesting.PrimeTools(c, stor, s.DataDir(), version.MustParseBinary("5.4.3-precise-amd64"))147 oldTools := envtesting.PrimeTools(c, stor, s.DataDir(), version.MustParseBinary("5.4.3-precise-amd64"))
148 newTools := envtesting.AssertUploadFakeToolsVersions(148 newTools := envtesting.AssertUploadFakeToolsVersions(
149 c, stor, version.MustParseBinary("5.4.5-precise-amd64"))[0]149 c, stor, version.MustParseBinary("5.4.5-precise-amd64"))[0]
150 err := envtools.WriteMetadata(coretools.List{oldTools, newTools}, true, stor)150 err := envtools.MergeAndWriteMetadata(stor, coretools.List{oldTools, newTools})
151 c.Assert(err, gc.IsNil)151 c.Assert(err, gc.IsNil)
152 err = statetesting.SetAgentVersion(s.State, newTools.Version.Number)152 err = statetesting.SetAgentVersion(s.State, newTools.Version.Number)
153 c.Assert(err, gc.IsNil)153 c.Assert(err, gc.IsNil)
@@ -202,7 +202,7 @@
202 }202 }
203 stor := s.Conn.Environ.Storage()203 stor := s.Conn.Environ.Storage()
204 newTools := envtesting.PrimeTools(c, stor, s.DataDir(), version.MustParseBinary("5.4.3-precise-amd64"))204 newTools := envtesting.PrimeTools(c, stor, s.DataDir(), version.MustParseBinary("5.4.3-precise-amd64"))
205 err := envtools.WriteMetadata(coretools.List{newTools}, true, stor)205 err := envtools.MergeAndWriteMetadata(stor, coretools.List{newTools})
206 c.Assert(err, gc.IsNil)206 c.Assert(err, gc.IsNil)
207 ugErr := &upgrader.UpgradeReadyError{207 ugErr := &upgrader.UpgradeReadyError{
208 AgentName: "anAgent",208 AgentName: "anAgent",

Subscribers

People subscribed via source and target branches

to status/vote changes: