Merge lp:~axwalk/juju-core/lp1233924-azure-disable-daily-stream into lp:~go-bot/juju-core/trunk

Proposed by Andrew Wilkins
Status: Merged
Approved by: Andrew Wilkins
Approved revision: no longer in the source branch.
Merged at revision: 1923
Proposed branch: lp:~axwalk/juju-core/lp1233924-azure-disable-daily-stream
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 0 lines
To merge this branch: bzr merge lp:~axwalk/juju-core/lp1233924-azure-disable-daily-stream
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+188777@code.launchpad.net

Commit message

provider/azure: remove default daily stream

The simplestreams.GetMetadata function was
being called with three datasources: storage,
daily and released. In our tests, storage was
empty, but daily and released were not; daily
did not, however, have any matching image ids.
There's an issue with simplestreams.GetMetadata
not attempting the next datasource if the
metadata index is valid, but does not contain
any matching product IDs.

Since the behaviour of simplestreams.GetMetadata
is potentially useful for private clouds, we
will just remove the daily stream from the defaults
for now. If people really want it, they can get
at it with image-stream and image-metadata-url.

Fixes #1233924

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

Description of the change

provider/azure: remove default daily stream

The simplestreams.GetMetadata function was
being called with three datasources: storage,
daily and released. In our tests, storage was
empty, but daily and released were not; daily
did not, however, have any matching image ids.
There's an issue with simplestreams.GetMetadata
not attempting the next datasource if the
metadata index is valid, but does not contain
any matching product IDs.

Since the behaviour of simplestreams.GetMetadata
is potentially useful for private clouds, we
will just remove the daily stream from the defaults
for now. If people really want it, they can get
at it with image-stream and image-metadata-url.

Fixes #1233924

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

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

Reviewers: mp+188777_code.launchpad.net,

Message:
Please take a look.

Description:
provider/azure: remove default daily stream

The simplestreams.GetMetadata function was
being called with three datasources: storage,
daily and released. In our tests, storage was
empty, but daily and released were not; daily
did not, however, have any matching image ids.
There's an issue with simplestreams.GetMetadata
not attempting the next datasource if the
metadata index is valid, but does not contain
any matching product IDs.

Since the behaviour of simplestreams.GetMetadata
is potentially useful for private clouds, we
will just remove the daily stream from the defaults
for now. If people really want it, they can get
at it with image-stream and image-metadata-url.

Fixes #1233924

https://code.launchpad.net/~axwalk/juju-core/lp1233924-azure-disable-daily-stream/+merge/188777

(do not edit description out of merge proposal)

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

Affected files (+7, -3 lines):
   A [revision details]
   M provider/azure/environ.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-20131002053203-75cl5uyqbq7bozdw
+New revision: <email address hidden>

Index: provider/azure/environ.go
=== modified file 'provider/azure/environ.go'
--- provider/azure/environ.go 2013-09-30 19:40:06 +0000
+++ provider/azure/environ.go 2013-10-02 07:37:51 +0000
@@ -900,9 +900,11 @@
  // It contains the central databases for the released and daily streams,
but this may
  // become more configurable. This variable is here as a placeholder, but
also
  // as an injection point for tests.
-var baseURLs = []string{
- "http://cloud-images.ubuntu.com/daily",
-}
+//
+// XXX Due to datasource fallback issues, the default daily stream has
been removed.
+// This var now only serves as an injection point for tests. See also:
+// https://bugs.launchpad.net/juju-core/+bug/1233924
+var baseURLs = []string{}

  // GetImageSources returns a list of sources which are used to search for
simplestreams image metadata.
  func (env *azureEnviron) GetImageSources() ([]simplestreams.DataSource,
error) {

Revision history for this message
John A Meinel (jameinel) wrote :

LGTM though a small comment tweak

https://codereview.appspot.com/14266043/diff/1/provider/azure/environ.go
File provider/azure/environ.go (right):

https://codereview.appspot.com/14266043/diff/1/provider/azure/environ.go#newcode904
provider/azure/environ.go:904: // XXX Due to datasource fallback issues,
the default daily stream has been removed.
I would just say "Note" and not "XXX"

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

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

https://codereview.appspot.com/14266043/diff/1/provider/azure/environ.go
File provider/azure/environ.go (right):

https://codereview.appspot.com/14266043/diff/1/provider/azure/environ.go#newcode904
provider/azure/environ.go:904: // XXX Due to datasource fallback issues,
the default daily stream has been removed.
On 2013/10/02 07:56:50, jameinel wrote:
> I would just say "Note" and not "XXX"

Done.

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

Revision history for this message
Go Bot (go-bot) wrote :

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

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

The attempt to merge lp:~axwalk/juju-core/lp1233924-azure-disable-daily-stream into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core/agent 0.281s
ok launchpad.net/juju-core/agent/tools 0.248s
ok launchpad.net/juju-core/bzr 6.845s
ok launchpad.net/juju-core/cert 3.021s
ok launchpad.net/juju-core/charm 0.502s
? launchpad.net/juju-core/charm/hooks [no test files]
ok launchpad.net/juju-core/cloudinit 0.029s
ok launchpad.net/juju-core/cmd 0.192s
? 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 157.136s
ok launchpad.net/juju-core/cmd/jujud 41.511s
ok launchpad.net/juju-core/cmd/plugins/juju-metadata 1.728s
ok launchpad.net/juju-core/constraints 0.028s
ok launchpad.net/juju-core/container/lxc 0.284s
? launchpad.net/juju-core/container/lxc/mock [no test files]
ok launchpad.net/juju-core/downloader 5.362s
ok launchpad.net/juju-core/environs 2.516s
ok launchpad.net/juju-core/environs/bootstrap 5.531s
ok launchpad.net/juju-core/environs/cloudinit 0.456s
ok launchpad.net/juju-core/environs/config 0.707s
ok launchpad.net/juju-core/environs/configstore 0.041s
ok launchpad.net/juju-core/environs/filestorage 0.029s
ok launchpad.net/juju-core/environs/httpstorage 0.772s
ok launchpad.net/juju-core/environs/imagemetadata 0.546s
ok launchpad.net/juju-core/environs/instances 0.053s
ok launchpad.net/juju-core/environs/jujutest 0.237s
ok launchpad.net/juju-core/environs/manual 4.918s
ok launchpad.net/juju-core/environs/simplestreams 0.364s
? launchpad.net/juju-core/environs/simplestreams/testing [no test files]
ok launchpad.net/juju-core/environs/sshstorage 1.043s
ok launchpad.net/juju-core/environs/storage 1.056s
ok launchpad.net/juju-core/environs/sync 27.227s
ok launchpad.net/juju-core/environs/testing 0.204s
ok launchpad.net/juju-core/environs/tools 5.877s
? launchpad.net/juju-core/environs/tools/testing [no test files]
ok launchpad.net/juju-core/errors 0.015s
ok launchpad.net/juju-core/instance 0.027s
? launchpad.net/juju-core/instance/testing [no test files]
ok launchpad.net/juju-core/juju 23.056s
ok launchpad.net/juju-core/juju/osenv 0.034s
? launchpad.net/juju-core/juju/testing [no test files]
ok launchpad.net/juju-core/log 0.015s
ok launchpad.net/juju-core/log/syslog 0.024s
ok launchpad.net/juju-core/names 0.036s
? launchpad.net/juju-core/provider [no test files]
? launchpad.net/juju-core/provider/all [no test files]

----------------------------------------------------------------------
FAIL: environ_test.go:1287: environSuite.TestGetImageMetadataSources

environ_test.go:1296:
    c.Assert(len(sources), gc.Equals, 3)
... obtained int = 2
... expected int = 3

OOPS: 161 passed, 1 FAILED
--- FAIL: TestAzureProvider (6.20 seconds)
FAIL
FAIL launchpad.net/juju-core/provider/azure 6.454s
ok launchpad.net/juju-core/provider/common 0.334s
ok launchpad.net/juju-core/provider/dummy 20.947s
ok launchpad.net/juju-core/provider/ec2 4.895s
ok launchpad.net/juju-core/provider/ec2/httpstorage 0.203s
ok launchpad.net/juju-...

Read more...

Preview Diff

Empty

Subscribers

People subscribed via source and target branches

to status/vote changes: