juju-core

Merge lp:~axwalk/juju-core/lp1233924-simplestreams-getmetadata-fallback into lp:~go-bot/juju-core/trunk

  1. lp1233924-simplestreams-getmetadata-fallback
  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: 2028
Proposed branch: lp:~axwalk/juju-core/lp1233924-simplestreams-getmetadata-fallback
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 321 lines (+107/-42)
6 files modified
environs/simplestreams/simplestreams.go (+4/-3)
environs/simplestreams/simplestreams_test.go (+43/-0)
environs/simplestreams/testing/testing.go (+24/-0)
environs/tools/tools_test.go (+12/-22)
provider/openstack/export_test.go (+2/-4)
provider/openstack/local_test.go (+22/-13)
To merge this branch: bzr merge lp:~axwalk/juju-core/lp1233924-simplestreams-getmetadata-fallback
Reviewer Review Type Date Requested Status
Tim Penhey (community) Approve
Review via email: mp+188758@code.launchpad.net

Commit message

environs/simplestreams: fix GetMetadata

GetMetadata was dropping out if a datasource
returned no matching product IDs, without an
error. I've changed it to check that a non-
empty list of products is returned before
bailing out.

Fixes #1233924

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

Description of the change

environs/simplestreams: fix GetMetadata

GetMetadata was dropping out if a datasource
returned no matching product IDs, without an
error. I've changed it to check that a non-
empty list of products is returned before
bailing out.

Fixes #1233924

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

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

Reviewers: mp+188758_code.launchpad.net,

Message:
Please take a look.

Description:
environs/simplestreams: fix GetMetadata

GetMetadata was dropping out if a datasource
returned no matching product IDs, without an
error. I've changed it to check that a non-
empty list of products is returned before
bailing out.

Fixes #1233924

https://code.launchpad.net/~axwalk/juju-core/lp1233924-simplestreams-getmetadata-fallback/+merge/188758

(do not edit description out of merge proposal)

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

Affected files (+71, -2 lines):
   A [revision details]
   M environs/simplestreams/simplestreams.go
   M environs/simplestreams/simplestreams_test.go
   M environs/simplestreams/testing/testing.go

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

Index: environs/simplestreams/simplestreams.go
=== modified file 'environs/simplestreams/simplestreams.go'
--- environs/simplestreams/simplestreams.go 2013-09-27 07:05:45 +0000
+++ environs/simplestreams/simplestreams.go 2013-10-02 03:04:13 +0000
@@ -17,6 +17,7 @@
   "io/ioutil"
   "net/http"
   "os"
+ "path"
   "reflect"
   "sort"
   "strings"
@@ -25,7 +26,6 @@
   "launchpad.net/loggo"

   "launchpad.net/juju-core/errors"
- "path"
  )

  var logger = loggo.GetLogger("juju.environs.simplestreams")
@@ -403,7 +403,7 @@
    if err != nil && len(items) == 0 && !onlySigned {
     items, err = getMaybeSignedMetadata(source, baseIndexPath, cons, false,
params)
    }
- if err == nil {
+ if err == nil && len(items) != 0 {
     break
    }
   }

Index: environs/simplestreams/simplestreams_test.go
=== modified file 'environs/simplestreams/simplestreams_test.go'
--- environs/simplestreams/simplestreams_test.go 2013-09-26 04:11:52 +0000
+++ environs/simplestreams/simplestreams_test.go 2013-10-02 03:04:13 +0000
@@ -9,9 +9,11 @@
   "testing"

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

   "launchpad.net/juju-core/environs/simplestreams"
   sstesting "launchpad.net/juju-core/environs/simplestreams/testing"
+ jc "launchpad.net/juju-core/testing/checkers"
  )

  func Test(t *testing.T) {
@@ -284,6 +286,47 @@
   c.Check(dotOFormats, gc.DeepEquals,
simplestreams.IndexMetadataSlice{array[0], array[2]})
  }

+func (s *simplestreamsSuite) TestGetMetadataNoMatching(c *gc.C) {
+ sources := []simplestreams.DataSource{
+ simplestreams.NewURLDataSource("test:/daily",
simplestreams.VerifySSLHostnames),
+ simplestreams.NewURLDataSource("test:/daily",
simplestreams.VerifySSLHostnames),
+ }
+ params := simplestreams.ValueParams{DataType: "image-ids"}
+ constraint := sstesting.NewTestConstraint(simplestreams.LookupParams{
+ CloudSpec: simplestreams.CloudSpec{
+ Region: "us-east-1",
+ Endpoint: "https://ec2.us-east-1.amazonaws.com",
+ },
+ Series: []string{"precise"}, // never match
+ Arches: []string{"arm"},
+ })
+
+ tw := &loggo.TestWriter{}
+ c.Assert(loggo.RegisterWriter("filter-tester", tw, loggo.DEB...

Read more...

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

https://codereview.appspot.com/14218044/diff/1/environs/simplestreams/simplestreams_test.go
File environs/simplestreams/simplestreams_test.go (right):

https://codereview.appspot.com/14218044/diff/1/environs/simplestreams/simplestreams_test.go#newcode300
environs/simplestreams/simplestreams_test.go:300: Series:
[]string{"precise"}, // never match
What is the never match? precise or arm? Also, perhaps better to give a
region that doesn't exist? Someone may add arm at some stage. Would
that work?

https://codereview.appspot.com/14218044/diff/1/environs/simplestreams/simplestreams_test.go#newcode326
environs/simplestreams/simplestreams_test.go:326: messages =
append(messages, messages...)
I agree with our IRC chat, ideally we would have a simple mock
datasource. What about...

type countingSource struct {
   simplestreams.DataSource
   count int
}

func (s *countingSource) URL(path string) (string, error) {
   s.count++
   return s.DataSource.URL(path)
}

Then have sources defined by:

   first :=
&countingSource{DataSource:simplestreams.NewURLDataSource("test:/daily",
simplestreams.VerifySSLHostnames)}
   second :=
&countingSource{DataSource:simplestreams.NewURLDataSource("test:/daily",
simplestreams.VerifySSLHostnames)}
   sources := []simplestream.DataSource{first, second}

   // do stuff

   c.Check(first.count, gc.Equals, 1)
   c.Check(second.count, gc.Equals, 1)

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

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

Please take a look.

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

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

https://codereview.appspot.com/14218044/diff/1/environs/simplestreams/simplestreams_test.go
File environs/simplestreams/simplestreams_test.go (right):

https://codereview.appspot.com/14218044/diff/1/environs/simplestreams/simplestreams_test.go#newcode300
environs/simplestreams/simplestreams_test.go:300: Series:
[]string{"precise"}, // never match
On 2013/10/02 04:02:00, thumper wrote:
> What is the never match? precise or arm? Also, perhaps better to give
 a region
> that doesn't exist? Someone may add arm at some stage. Would that
 work?

precise, which isn't in the daily stream I added to testing. I can't use
a non-existent region, as that causes GetMetadata to error, which we
don't want.

The series are validated, but architectures aren't. I'll put in an
invalid arch.

https://codereview.appspot.com/14218044/diff/1/environs/simplestreams/simplestreams_test.go#newcode326
environs/simplestreams/simplestreams_test.go:326: messages =
append(messages, messages...)
On 2013/10/02 04:02:00, thumper wrote:
> I agree with our IRC chat, ideally we would have a simple mock
 datasource. What
> about...

> type countingSource struct {
> simplestreams.DataSource
> count int
> }

> func (s *countingSource) URL(path string) (string, error) {
> s.count++
> return s.DataSource.URL(path)
> }

> Then have sources defined by:

> first :=

&countingSource{DataSource:simplestreams.NewURLDataSource("test:/daily",
> simplestreams.VerifySSLHostnames)}
> second :=

&countingSource{DataSource:simplestreams.NewURLDataSource("test:/daily",
> simplestreams.VerifySSLHostnames)}
> sources := []simplestream.DataSource{first, second}

> // do stuff

> c.Check(first.count, gc.Equals, 1)
> c.Check(second.count, gc.Equals, 1)

Done, tho I'm reusing a single countingSource.

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

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

On 2013/10/02 05:07:52, axw1 wrote:

https://codereview.appspot.com/14218044/diff/1/environs/simplestreams/simplestreams_test.go
> File environs/simplestreams/simplestreams_test.go (right):

https://codereview.appspot.com/14218044/diff/1/environs/simplestreams/simplestreams_test.go#newcode300
> environs/simplestreams/simplestreams_test.go:300: Series:
 []string{"precise"},
> // never match
> On 2013/10/02 04:02:00, thumper wrote:
> > What is the never match? precise or arm? Also, perhaps better to
 give a
> region
> > that doesn't exist? Someone may add arm at some stage. Would that
 work?

> precise, which isn't in the daily stream I added to testing. I can't
use a
> non-existent region, as that causes GetMetadata to error, which we
don't want.

> The series are validated, but architectures aren't. I'll put in an
invalid arch.

https://codereview.appspot.com/14218044/diff/1/environs/simplestreams/simplestreams_test.go#newcode326
> environs/simplestreams/simplestreams_test.go:326: messages =
 append(messages,
> messages...)
> On 2013/10/02 04:02:00, thumper wrote:
> > I agree with our IRC chat, ideally we would have a simple mock
 datasource.
> What
> > about...
> >
> > type countingSource struct {
> > simplestreams.DataSource
> > count int
> > }
> >
> > func (s *countingSource) URL(path string) (string, error) {
> > s.count++
> > return s.DataSource.URL(path)
> > }
> >
> > Then have sources defined by:
> >
> > first :=
> >
 &countingSource{DataSource:simplestreams.NewURLDataSource("test:/daily",
> > simplestreams.VerifySSLHostnames)}
> > second :=
> >
 &countingSource{DataSource:simplestreams.NewURLDataSource("test:/daily",
> > simplestreams.VerifySSLHostnames)}
> > sources := []simplestream.DataSource{first, second}
> >
> > // do stuff
> >
> > c.Check(first.count, gc.Equals, 1)
> > c.Check(second.count, gc.Equals, 1)
> >

> Done, tho I'm reusing a single countingSource.

So, after discussing with jam, GetMetadata *might* be how it is on
purpose. You might want to populate metadata into your private cloud,
and then expect only to be able to create images that match your private
metadata.

I'll leave this for now, until it's decided how it should be working.

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

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

> So, after discussing with jam, GetMetadata *might* be how it is on
purpose. You
> might want to populate metadata into your private cloud, and then
expect only to
> be able to create images that match your private metadata.

> I'll leave this for now, until it's decided how it should be working.

I think that it should be a configuration option to specify "don't
fall-back to other sources".

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

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

LGTM

https://codereview.appspot.com/14218044/diff/6001/environs/simplestreams/simplestreams_test.go
File environs/simplestreams/simplestreams_test.go (right):

https://codereview.appspot.com/14218044/diff/6001/environs/simplestreams/simplestreams_test.go#newcode327
environs/simplestreams/simplestreams_test.go:327: c.Assert(source.count,
gc.Equals, 2*len(sources))
Much nicer.

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

Revision history for this message
Tim Penhey (thumper) :
review: Approve
Revision history for this message
William Reade (fwereade) wrote :

Fine with the idea, I think the implementation's in the wrong place.

https://codereview.appspot.com/14218044/diff/6001/environs/simplestreams/simplestreams.go
File environs/simplestreams/simplestreams.go (right):

https://codereview.appspot.com/14218044/diff/6001/environs/simplestreams/simplestreams.go#newcode406
environs/simplestreams/simplestreams.go:406: if err == nil && len(items)
!= 0 {
I'm not sure this is the right place for it -- it's a hack-on-a-hack,
instead of removing the original hack in getmaybeSignedMetadata that
replaces errors with nil.

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

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

Please take a look.

https://codereview.appspot.com/14218044/diff/6001/environs/simplestreams/simplestreams.go
File environs/simplestreams/simplestreams.go (right):

https://codereview.appspot.com/14218044/diff/6001/environs/simplestreams/simplestreams.go#newcode406
environs/simplestreams/simplestreams.go:406: if err == nil && len(items)
!= 0 {
On 2013/10/04 10:26:42, fwereade wrote:
> I'm not sure this is the right place for it -- it's a hack-on-a-hack,
 instead of
> removing the original hack in getmaybeSignedMetadata that replaces
 errors with
> nil.

I've moved it, but I think it's much the same either way. What we have
is an internal error that is used to communicate that there are no
matching products, for one of several reasons. It can be quashed in
either GetMetadata or getMaybeSignedMetadata - doesn't really matter
which, I think.

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

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

The attempt to merge lp:~axwalk/juju-core/lp1233924-simplestreams-getmetadata-fallback into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core/agent 1.331s
ok launchpad.net/juju-core/agent/tools 0.266s
ok launchpad.net/juju-core/bzr 7.459s
ok launchpad.net/juju-core/cert 3.287s
ok launchpad.net/juju-core/charm 0.588s
? launchpad.net/juju-core/charm/hooks [no test files]
ok launchpad.net/juju-core/cloudinit 0.026s
ok launchpad.net/juju-core/cmd 0.213s
? 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 169.091s
ok launchpad.net/juju-core/cmd/jujud 54.653s
ok launchpad.net/juju-core/cmd/plugins/juju-metadata 2.711s
ok launchpad.net/juju-core/constraints 0.028s
ok launchpad.net/juju-core/container/lxc 0.291s
? launchpad.net/juju-core/container/lxc/mock [no test files]
ok launchpad.net/juju-core/downloader 5.263s
ok launchpad.net/juju-core/environs 3.169s
ok launchpad.net/juju-core/environs/bootstrap 5.058s
ok launchpad.net/juju-core/environs/cloudinit 0.539s
ok launchpad.net/juju-core/environs/config 0.792s
ok launchpad.net/juju-core/environs/configstore 0.042s
ok launchpad.net/juju-core/environs/filestorage 0.030s
ok launchpad.net/juju-core/environs/httpstorage 0.993s
ok launchpad.net/juju-core/environs/imagemetadata 0.564s
? launchpad.net/juju-core/environs/imagemetadata/testing [no test files]
ok launchpad.net/juju-core/environs/instances 0.054s
ok launchpad.net/juju-core/environs/jujutest 0.248s
ok launchpad.net/juju-core/environs/manual 4.904s
ok launchpad.net/juju-core/environs/simplestreams 0.331s
? launchpad.net/juju-core/environs/simplestreams/testing [no test files]
ok launchpad.net/juju-core/environs/sshstorage 1.067s
ok launchpad.net/juju-core/environs/storage 1.039s
ok launchpad.net/juju-core/environs/sync 28.241s
ok launchpad.net/juju-core/environs/testing 0.204s

----------------------------------------------------------------------
FAIL: tools_test.go:290: SimpleStreamsToolsSuite.TestFindBootstrapTools

[LOG] 64.15651 INFO juju.provider.dummy reset environment

test 0: no tools at all
[LOG] 64.26877 INFO juju.provider.dummy reset environment
[LOG] 64.37642 INFO juju.environs.tools filtering tools by released version
[LOG] 64.37647 INFO juju.environs.tools reading tools with major.minor version 1.0
[LOG] 64.37649 INFO juju.environs.tools filtering tools by series: precise
[LOG] 64.37651 DEBUG juju.environs.tools no architecture specified when finding tools, looking for any
[LOG] 64.37667 DEBUG juju.environs.simplestreams fetchData failed for "file:///tmp/gocheck-894385949183117216/0/streams/v1/index.sjson": cannot find URL "file:///tmp/gocheck-894385949183117216/0/streams/v1/index.sjson" not found
[LOG] 64.37668 DEBUG juju.environs.simplestreams cannot load index "file:///tmp/gocheck-894385949183117216/0/streams/v1/index.sjson": invalid URL "file:///tmp/gocheck-894385949183117216/0/streams/v1/index.sjson" not found
[LOG] 64.37674 DEBUG juju.environs.simplestreams fetchData failed for "file:///tmp/gocheck-894385949183117216/0...

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

Please take a look.

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'environs/simplestreams/simplestreams.go'
--- environs/simplestreams/simplestreams.go 2013-10-29 22:58:43 +0000
+++ environs/simplestreams/simplestreams.go 2013-11-05 04:38:36 +0000
@@ -431,6 +431,10 @@
431 break431 break
432 }432 }
433 }433 }
434 if _, ok := err.(*noMatchingProductsError); ok {
435 // no matching products is an internal error only
436 err = nil
437 }
434 return items, err438 return items, err
435}439}
436440
@@ -463,9 +467,6 @@
463 }467 }
464 if _, ok := err.(*noMatchingProductsError); ok {468 if _, ok := err.(*noMatchingProductsError); ok {
465 logger.Debugf("%v", err)469 logger.Debugf("%v", err)
466 // No matching products is not considered an error which will allow another source to be
467 // searched, so return err = nil here.
468 return items, nil
469 }470 }
470 }471 }
471 return items, err472 return items, err
472473
=== modified file 'environs/simplestreams/simplestreams_test.go'
--- environs/simplestreams/simplestreams_test.go 2013-10-24 21:05:33 +0000
+++ environs/simplestreams/simplestreams_test.go 2013-11-05 04:38:36 +0000
@@ -300,6 +300,49 @@
300 c.Check(dotOFormats, gc.DeepEquals, simplestreams.IndexMetadataSlice{array[0], array[2]})300 c.Check(dotOFormats, gc.DeepEquals, simplestreams.IndexMetadataSlice{array[0], array[2]})
301}301}
302302
303// countingSource is used to check that a DataSource has been queried.
304type countingSource struct {
305 simplestreams.DataSource
306 count int
307}
308
309func (s *countingSource) URL(path string) (string, error) {
310 s.count++
311 return s.DataSource.URL(path)
312}
313
314func (s *simplestreamsSuite) TestGetMetadataNoMatching(c *gc.C) {
315 source := &countingSource{
316 DataSource: simplestreams.NewURLDataSource(
317 "test:/daily", simplestreams.VerifySSLHostnames,
318 ),
319 }
320 sources := []simplestreams.DataSource{source, source, source}
321 params := simplestreams.ValueParams{DataType: "image-ids"}
322 constraint := sstesting.NewTestConstraint(simplestreams.LookupParams{
323 CloudSpec: simplestreams.CloudSpec{
324 Region: "us-east-1",
325 Endpoint: "https://ec2.us-east-1.amazonaws.com",
326 },
327 Series: []string{"precise"},
328 Arches: []string{"not-a-real-arch"}, // never matches
329 })
330
331 items, err := simplestreams.GetMetadata(
332 sources,
333 simplestreams.DefaultIndexPath,
334 constraint,
335 false,
336 params,
337 )
338 c.Assert(err, gc.IsNil)
339 c.Assert(items, gc.HasLen, 0)
340
341 // There should be 2 calls to each data-source:
342 // one for .sjson, one for .json.
343 c.Assert(source.count, gc.Equals, 2*len(sources))
344}
345
303func (s *simplestreamsSuite) TestMetadataCatalog(c *gc.C) {346func (s *simplestreamsSuite) TestMetadataCatalog(c *gc.C) {
304 metadata := s.AssertGetMetadata(c)347 metadata := s.AssertGetMetadata(c)
305 c.Check(len(metadata.Products), gc.Equals, 2)348 c.Check(len(metadata.Products), gc.Equals, 2)
306349
=== modified file 'environs/simplestreams/testing/testing.go'
--- environs/simplestreams/testing/testing.go 2013-10-24 18:03:53 +0000
+++ environs/simplestreams/testing/testing.go 2013-11-05 04:38:36 +0000
@@ -58,6 +58,30 @@
58-----END PGP PUBLIC KEY BLOCK-----`58-----END PGP PUBLIC KEY BLOCK-----`
5959
60var imageData = map[string]string{60var imageData = map[string]string{
61 "/daily/streams/v1/index.json": `
62 {
63 "index": {
64 "com.ubuntu.cloud:released:raring": {
65 "updated": "Wed, 01 May 2013 13:31:26 +0000",
66 "clouds": [
67 {
68 "region": "us-east-1",
69 "endpoint": "https://ec2.us-east-1.amazonaws.com"
70 }
71 ],
72 "cloudname": "aws",
73 "datatype": "image-ids",
74 "format": "products:1.0",
75 "products": [
76 "com.ubuntu.cloud:server:13.04:amd64"
77 ],
78 "path": "streams/v1/raring_metadata.json"
79 }
80 },
81 "updated": "Wed, 01 May 2013 13:31:26 +0000",
82 "format": "index:1.0"
83 }
84 `,
61 "/streams/v1/index.json": `85 "/streams/v1/index.json": `
62 {86 {
63 "index": {87 "index": {
6488
=== modified file 'environs/tools/tools_test.go'
--- environs/tools/tools_test.go 2013-11-04 02:01:19 +0000
+++ environs/tools/tools_test.go 2013-11-05 04:38:36 +0000
@@ -210,9 +210,10 @@
210}, {210}, {
211 info: "custom tools completely block public ones",211 info: "custom tools completely block public ones",
212 major: 1,212 major: 1,
213 minor: -1,
213 custom: envtesting.V220all,214 custom: envtesting.V220all,
214 public: envtesting.VAll,215 public: envtesting.VAll,
215 err: coretools.ErrNoMatches,216 expect: envtesting.V1all,
216}, {217}, {
217 info: "tools matching major version only",218 info: "tools matching major version only",
218 major: 1,219 major: 1,
@@ -235,14 +236,13 @@
235 c.Check(err, jc.Satisfies, errors.IsNotFoundError)236 c.Check(err, jc.Satisfies, errors.IsNotFoundError)
236 continue237 continue
237 }238 }
238 source := custom
239 if len(source) == 0 {
240 // We only use the public tools if there are *no* custom tools.
241 source = public
242 }
243 expect := map[version.Binary]string{}239 expect := map[version.Binary]string{}
244 for _, expected := range test.expect {240 for _, expected := range test.expect {
245 expect[expected] = source[expected]241 // If the tools exist in custom, that's preferred.
242 var ok bool
243 if expect[expected], ok = custom[expected]; !ok {
244 expect[expected] = public[expected]
245 }
246 }246 }
247 c.Check(actual.URLs(), gc.DeepEquals, expect)247 c.Check(actual.URLs(), gc.DeepEquals, expect)
248 }248 }
@@ -302,10 +302,6 @@
302 s.reset(c, attrs)302 s.reset(c, attrs)
303 version.Current = test.CliVersion303 version.Current = test.CliVersion
304 available := s.uploadCustom(c, test.Available...)304 available := s.uploadCustom(c, test.Available...)
305 if len(available) > 0 {
306 // These should never be chosen.
307 s.uploadPublic(c, envtesting.VAll...)
308 }
309305
310 params := envtools.BootstrapToolsParams{306 params := envtools.BootstrapToolsParams{
311 Version: agentVersion,307 Version: agentVersion,
@@ -393,10 +389,6 @@
393 "agent-version": test.agentVersion.String(),389 "agent-version": test.agentVersion.String(),
394 })390 })
395 available := s.uploadCustom(c, test.available...)391 available := s.uploadCustom(c, test.available...)
396 if len(available) > 0 {
397 // These should never be chosen.
398 s.uploadPublic(c, envtesting.VAll...)
399 }
400392
401 agentVersion, _ := s.env.Config().AgentVersion()393 agentVersion, _ := s.env.Config().AgentVersion()
402 actual, err := envtools.FindInstanceTools(s.env, agentVersion, test.series, &test.arch)394 actual, err := envtools.FindInstanceTools(s.env, agentVersion, test.series, &test.arch)
@@ -444,11 +436,10 @@
444 public: []version.Binary{envtesting.V100p64},436 public: []version.Binary{envtesting.V100p64},
445 seek: envtesting.V100p64,437 seek: envtesting.V100p64,
446}, {438}, {
447 info: "exact match in public blocked by custom",439 info: "exact match in public not blocked by custom",
448 custom: envtesting.V110all,440 custom: envtesting.V110all,
449 public: []version.Binary{envtesting.V100p64},441 public: []version.Binary{envtesting.V100p64},
450 seek: envtesting.V100p64,442 seek: envtesting.V100p64,
451 err: coretools.ErrNoMatches,
452}}443}}
453444
454func (s *SimpleStreamsToolsSuite) TestFindExactTools(c *gc.C) {445func (s *SimpleStreamsToolsSuite) TestFindExactTools(c *gc.C) {
@@ -463,12 +454,11 @@
463 continue454 continue
464 }455 }
465 c.Check(actual.Version, gc.Equals, test.seek)456 c.Check(actual.Version, gc.Equals, test.seek)
466 source := custom457 if _, ok := custom[actual.Version]; ok {
467 if len(source) == 0 {458 c.Check(actual.URL, gc.DeepEquals, custom[actual.Version])
468 // We only use public tools if there are *no* private tools.459 } else {
469 source = public460 c.Check(actual.URL, gc.DeepEquals, public[actual.Version])
470 }461 }
471 c.Check(actual.URL, gc.DeepEquals, source[actual.Version])
472 } else {462 } else {
473 c.Check(err, jc.Satisfies, errors.IsNotFoundError)463 c.Check(err, jc.Satisfies, errors.IsNotFoundError)
474 }464 }
475465
=== modified file 'provider/openstack/export_test.go'
--- provider/openstack/export_test.go 2013-10-24 00:20:59 +0000
+++ provider/openstack/export_test.go 2013-11-05 04:38:36 +0000
@@ -210,7 +210,7 @@
210210
211const productMetadatafile = "image-metadata/products.json"211const productMetadatafile = "image-metadata/products.json"
212212
213func UseTestImageData(e environs.Environ, cred *identity.Credentials) {213func UseTestImageData(stor storage.Storage, cred *identity.Credentials) {
214 // Put some image metadata files into the public storage.214 // Put some image metadata files into the public storage.
215 t := template.Must(template.New("").Parse(indexData))215 t := template.Must(template.New("").Parse(indexData))
216 var metadata bytes.Buffer216 var metadata bytes.Buffer
@@ -218,14 +218,12 @@
218 panic(fmt.Errorf("cannot generate index metdata: %v", err))218 panic(fmt.Errorf("cannot generate index metdata: %v", err))
219 }219 }
220 data := metadata.Bytes()220 data := metadata.Bytes()
221 stor := MetadataStorage(e)
222 stor.Put(simplestreams.DefaultIndexPath+".json", bytes.NewReader(data), int64(len(data)))221 stor.Put(simplestreams.DefaultIndexPath+".json", bytes.NewReader(data), int64(len(data)))
223 stor.Put(222 stor.Put(
224 productMetadatafile, strings.NewReader(imagesData), int64(len(imagesData)))223 productMetadatafile, strings.NewReader(imagesData), int64(len(imagesData)))
225}224}
226225
227func RemoveTestImageData(e environs.Environ) {226func RemoveTestImageData(stor storage.Storage) {
228 stor := MetadataStorage(e)
229 stor.Remove(simplestreams.DefaultIndexPath + ".json")227 stor.Remove(simplestreams.DefaultIndexPath + ".json")
230 stor.Remove(productMetadatafile)228 stor.Remove(productMetadatafile)
231}229}
232230
=== modified file 'provider/openstack/local_test.go'
--- provider/openstack/local_test.go 2013-11-04 02:01:19 +0000
+++ provider/openstack/local_test.go 2013-11-05 04:38:36 +0000
@@ -167,11 +167,11 @@
167 c.Logf("Running live tests using openstack service test double")167 c.Logf("Running live tests using openstack service test double")
168 s.srv.start(c, s.cred)168 s.srv.start(c, s.cred)
169 s.LiveTests.SetUpSuite(c)169 s.LiveTests.SetUpSuite(c)
170 openstack.UseTestImageData(s.Env, s.cred)170 openstack.UseTestImageData(openstack.ImageMetadataStorage(s.Env), s.cred)
171}171}
172172
173func (s *localLiveSuite) TearDownSuite(c *gc.C) {173func (s *localLiveSuite) TearDownSuite(c *gc.C) {
174 openstack.RemoveTestImageData(s.Env)174 openstack.RemoveTestImageData(openstack.ImageMetadataStorage(s.Env))
175 s.LiveTests.TearDownSuite(c)175 s.LiveTests.TearDownSuite(c)
176 s.srv.stop()176 s.srv.stop()
177 s.LoggingSuite.TearDownSuite(c)177 s.LoggingSuite.TearDownSuite(c)
@@ -180,6 +180,7 @@
180func (s *localLiveSuite) SetUpTest(c *gc.C) {180func (s *localLiveSuite) SetUpTest(c *gc.C) {
181 s.LoggingSuite.SetUpTest(c)181 s.LoggingSuite.SetUpTest(c)
182 s.LiveTests.SetUpTest(c)182 s.LiveTests.SetUpTest(c)
183 s.PatchValue(&imagemetadata.DefaultBaseURL, "")
183}184}
184185
185func (s *localLiveSuite) TearDownTest(c *gc.C) {186func (s *localLiveSuite) TearDownTest(c *gc.C) {
@@ -194,9 +195,10 @@
194type localServerSuite struct {195type localServerSuite struct {
195 testbase.LoggingSuite196 testbase.LoggingSuite
196 jujutest.Tests197 jujutest.Tests
197 cred *identity.Credentials198 cred *identity.Credentials
198 srv localServer199 srv localServer
199 metadataStorage storage.Storage200 toolsMetadataStorage storage.Storage
201 imageMetadataStorage storage.Storage
200}202}
201203
202func (s *localServerSuite) SetUpSuite(c *gc.C) {204func (s *localServerSuite) SetUpSuite(c *gc.C) {
@@ -226,18 +228,21 @@
226 s.Tests.SetUpTest(c)228 s.Tests.SetUpTest(c)
227 // For testing, we create a storage instance to which is uploaded tools and image metadata.229 // For testing, we create a storage instance to which is uploaded tools and image metadata.
228 env := s.Prepare(c)230 env := s.Prepare(c)
229 s.metadataStorage = openstack.MetadataStorage(env)231 s.toolsMetadataStorage = openstack.MetadataStorage(env)
230 // Put some fake metadata in place so that tests that are simply232 // Put some fake metadata in place so that tests that are simply
231 // starting instances without any need to check if those instances233 // starting instances without any need to check if those instances
232 // are running can find the metadata.234 // are running can find the metadata.
233 envtesting.UploadFakeTools(c, s.metadataStorage)235 envtesting.UploadFakeTools(c, s.toolsMetadataStorage)
234 openstack.UseTestImageData(env, s.cred)236 s.imageMetadataStorage = openstack.ImageMetadataStorage(env)
237 openstack.UseTestImageData(s.imageMetadataStorage, s.cred)
235}238}
236239
237func (s *localServerSuite) TearDownTest(c *gc.C) {240func (s *localServerSuite) TearDownTest(c *gc.C) {
238 openstack.RemoveTestImageData(s.Open(c))241 if s.imageMetadataStorage != nil {
239 if s.metadataStorage != nil {242 openstack.RemoveTestImageData(s.imageMetadataStorage)
240 envtesting.RemoveFakeToolsMetadata(c, s.metadataStorage)243 }
244 if s.toolsMetadataStorage != nil {
245 envtesting.RemoveFakeToolsMetadata(c, s.toolsMetadataStorage)
241 }246 }
242 s.Tests.TearDownTest(c)247 s.Tests.TearDownTest(c)
243 s.srv.stop()248 s.srv.stop()
@@ -535,7 +540,11 @@
535}540}
536541
537func (s *localServerSuite) TestFindImageBadDefaultImage(c *gc.C) {542func (s *localServerSuite) TestFindImageBadDefaultImage(c *gc.C) {
543 // Prevent falling over to the public datasource.
544 s.PatchValue(&imagemetadata.DefaultBaseURL, "")
545
538 env := s.Open(c)546 env := s.Open(c)
547
539 // An error occurs if no suitable image is found.548 // An error occurs if no suitable image is found.
540 _, err := openstack.FindInstanceSpec(env, "saucy", "amd64", "mem=8G")549 _, err := openstack.FindInstanceSpec(env, "saucy", "amd64", "mem=8G")
541 c.Assert(err, gc.ErrorMatches, `no "saucy" images in some-region with arches \[amd64\]`)550 c.Assert(err, gc.ErrorMatches, `no "saucy" images in some-region with arches \[amd64\]`)
@@ -739,8 +748,8 @@
739 c.Logf("Generating fake tools for: %v", url)748 c.Logf("Generating fake tools for: %v", url)
740 envtesting.UploadFakeTools(c, metadataStorage)749 envtesting.UploadFakeTools(c, metadataStorage)
741 defer envtesting.RemoveFakeTools(c, metadataStorage)750 defer envtesting.RemoveFakeTools(c, metadataStorage)
742 openstack.UseTestImageData(s.env, s.cred)751 openstack.UseTestImageData(metadataStorage, s.cred)
743 defer openstack.RemoveTestImageData(s.env)752 defer openstack.RemoveTestImageData(metadataStorage)
744753
745 err = bootstrap.Bootstrap(s.env, constraints.Value{})754 err = bootstrap.Bootstrap(s.env, constraints.Value{})
746 c.Assert(err, gc.IsNil)755 c.Assert(err, gc.IsNil)

Subscribers

People subscribed via source and target branches

to status/vote changes: