Code review comment for lp://staging/~wallyworld/juju-core/openstack-live-test-fixes

Reviewers: mp+160275_code.launchpad.net,

Message:
Please take a look.

Description:
Remove hard coded Canonistack image id

Now that we have cloud image metadata for Canonistack, the hard coded
image id and instance type used for live tests are no longer required.

Another change is that the clean up for the live tests was changed to
only remove the fake tools from the public bucket otherwise we were
deleting the entire public bucket and this removed all the legitimate
tools and image metadata files.

https://code.launchpad.net/~wallyworld/juju-core/openstack-live-test-fixes/+merge/160275

Requires:
https://code.launchpad.net/~wallyworld/juju-core/openstack-image-lookup/+merge/159301

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M environs/openstack/live_test.go
   M environs/openstack/local_test.go
   M environs/openstack/provider.go
   M environs/openstack/provider_test.go
   M environs/testing/tools.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: <email address hidden>
+New revision: <email address hidden>

Index: environs/openstack/live_test.go
=== modified file 'environs/openstack/live_test.go'
--- environs/openstack/live_test.go 2013-04-19 02:39:42 +0000
+++ environs/openstack/live_test.go 2013-04-23 04:03:33 +0000
@@ -70,8 +70,6 @@
     // this flag to True.
     HasProvisioner: false,
    },
- testImageId: testImageDetails.ImageId,
- testFlavor: testImageDetails.Flavor,
   })
  }

@@ -82,8 +80,6 @@
   coretesting.LoggingSuite
   jujutest.LiveTests
   cred *identity.Credentials
- testImageId string
- testFlavor string
   writeablePublicStorage environs.Storage
  }

@@ -118,8 +114,7 @@
    return
   }
   if t.writeablePublicStorage != nil {
- err := openstack.DeleteStorageContent(t.writeablePublicStorage)
- c.Check(err, IsNil)
+ envtesting.RemoveFakeTools(c, t.writeablePublicStorage)
   }
   t.LiveTests.TearDownSuite(c)
   t.LoggingSuite.TearDownSuite(c)

Index: environs/openstack/local_test.go
=== modified file 'environs/openstack/local_test.go'
--- environs/openstack/local_test.go 2013-04-19 02:39:42 +0000
+++ environs/openstack/local_test.go 2013-04-23 04:03:33 +0000
@@ -100,9 +100,7 @@
   config["default-instance-type"] = "m1.small"
   Suite(&localLiveSuite{
    LiveTests: LiveTests{
- cred: cred,
- testImageId: "1",
- testFlavor: "m1.small",
+ cred: cred,
     LiveTests: jujutest.LiveTests{
      TestConfig: jujutest.TestConfig{config},
     },
@@ -183,9 +181,10 @@
  type localServerSuite struct {
   coretesting.LoggingSuite
   jujutest.Tests
- cred *identity.Credentials
- srv localServer
- env environs.Environ
+ cred *identity.Credentials
+ srv localServer
+ env environs.Environ
+ writeablePublicStorage environs.Storage
  }

  func (s *localServerSuite) SetUpSuite(c *C) {
@@ -206,8 +205,8 @@
    "auth-url": s.cred.URL,
   })
   s.Tests.SetUpTest(c)
- writeablePublicStorage := openstack.WritablePublicStorage(s.Env)
- envtesting.UploadFakeTools(c, writeablePublicStorage)
+ s.writeablePublicStorage = openstack.WritablePublicStorage(s.Env)
+ envtesting.UploadFakeTools(c, s.writeablePublicStorage)
   s.env = s.Tests.Env
   openstack.UseTestImageData(s.env, false)
  }
@@ -216,6 +215,9 @@
   if s.env != nil {
    openstack.RemoveTestImageData(s.env)
   }
+ if s.writeablePublicStorage != nil {
+ envtesting.RemoveFakeTools(c, s.writeablePublicStorage)
+ }
   s.Tests.TearDownTest(c)
   s.srv.stop()
   s.LoggingSuite.TearDownTest(c)

Index: environs/openstack/provider.go
=== modified file 'environs/openstack/provider.go'
--- environs/openstack/provider.go 2013-04-22 03:26:40 +0000
+++ environs/openstack/provider.go 2013-04-23 04:03:33 +0000
@@ -77,8 +77,10 @@
    # auth-url: https://yourkeystoneurl:443/v2.0/
    # override if your workstation is running a different series to which
you are deploying
    # default-series: precise
- default-image-id: c876e5fe-abb0-41f0-8f29-f0b47481f523
- default-instance-type: "m1.small"
+ # The attributes below allow user specified defaults to be used if a
suitable image
+ # or instance type cannot be found.
+ # default-image-id: <fallback image id>
+ # default-instance-type: <fallback flavor name>
    # The following are used for userpass authentication (the default)
    auth-mode: userpass
    # Usually set via the env variable OS_USERNAME, but can be specified here

Index: environs/openstack/provider_test.go
=== modified file 'environs/openstack/provider_test.go'
--- environs/openstack/provider_test.go 2013-02-27 00:53:19 +0000
+++ environs/openstack/provider_test.go 2013-04-23 04:03:33 +0000
@@ -13,8 +13,6 @@

  // Out-of-the-box, we support live testing using Canonistack or HP Cloud.
  var testConstraints = map[string]openstack.ImageDetails{
- "canonistack": openstack.ImageDetails{
- Flavor: "m1.tiny", ImageId: "c876e5fe-abb0-41f0-8f29-f0b47481f523"},
   "hpcloud": openstack.ImageDetails{
    Flavor: "standard.xsmall", ImageId: "75845"},
  }
@@ -35,14 +33,6 @@
      t.Fatalf("Unknown vendor %s. Must be one of %s", *vendor, keys)
     }
    } else {
- if *imageId == "" {
- t.Fatalf("Must specify image id to use for test instance, "+
- "eg %s for Canonistack", "-image
c876e5fe-abb0-41f0-8f29-f0b47481f523")
- }
- if *flavor == "" {
- t.Fatalf("Must specify flavor to use for test instance, "+
- "eg %s for Canonistack", "-flavor m1.tiny")
- }
     testImageDetails = openstack.ImageDetails{*flavor, *imageId}
    }
    cred, err := identity.CompleteCredentialsFromEnv()

Index: environs/testing/tools.go
=== modified file 'environs/testing/tools.go'
--- environs/testing/tools.go 2013-04-17 20:41:15 +0000
+++ environs/testing/tools.go 2013-04-23 04:03:33 +0000
@@ -71,6 +71,20 @@
   }
  }

+// RemoveFakeTools deletes the fake tools from the supplied storage.
+func RemoveFakeTools(c *C, storage environs.Storage) {
+ toolsVersion := version.Current
+ name := tools.StorageName(toolsVersion)
+ err := storage.Remove(name)
+ c.Check(err, IsNil)
+ if version.Current.Series != config.DefaultSeries {
+ toolsVersion.Series = config.DefaultSeries
+ name := tools.StorageName(toolsVersion)
+ err := storage.Remove(name)
+ c.Check(err, IsNil)
+ }
+}
+
  // RemoveTools deletes all tools from the supplied storage.
  func RemoveTools(c *C, storage environs.Storage) {
   names, err := storage.List("tools/juju-")
@@ -78,7 +92,7 @@
   c.Logf("removing files: %v", names)
   for _, name := range names {
    err = storage.Remove(name)
- c.Assert(err, IsNil)
+ c.Check(err, IsNil)
   }
  }