Code review comment for lp://staging/~danilo/juju-core/bootstrap-verify

Reviewers: mp+166481_code.launchpad.net,

Message:
Please take a look.

Description:
Verify storage writing on bootstrap

Replicates a mechanism used by python juju to test if private bucket
storage is writeable by juju on bootstrap. It, however, uses a
different content inside the bootstrap-verify file since we want to use
the differences to detect a Python environment and fail on it (this is
coming in a follow-up branch).

This has been tested with ec2, but not with openstack or maas. Any
suggestions on how to best do that are welcome.

https://code.launchpad.net/~danilo/juju-core/bootstrap-verify/+merge/166481

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M environs/dummy/environs.go
   M environs/ec2/ec2.go
   M environs/maas/environ.go
   M environs/openstack/provider.go
   M environs/storage.go
   M environs/storage_test.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/storage.go
=== modified file 'environs/storage.go'
--- environs/storage.go 2013-05-02 15:55:42 +0000
+++ environs/storage.go 2013-05-30 10:54:02 +0000
@@ -6,6 +6,7 @@
  import (
   "fmt"
   "io"
+ "strings"
  )

  // EmptyStorage holds a StorageReader object that contains no files and
@@ -14,6 +15,9 @@

  type emptyStorage struct{}

+const VERIFICATION_FILENAME string = "bootstrap-verify"
+const VERIFICATION_CONTENT = "juju-core storage writing verified: ok"
+
  func (s emptyStorage) Get(name string) (io.ReadCloser, error) {
   return nil, &NotFoundError{fmt.Errorf("file %q not found", name)}
  }
@@ -25,3 +29,10 @@
  func (s emptyStorage) List(prefix string) ([]string, error) {
   return nil, nil
  }
+
+func VerifyStorage(storage Storage) error {
+ reader := strings.NewReader(VERIFICATION_CONTENT)
+ err := storage.Put(VERIFICATION_FILENAME, reader,
+ int64(len(VERIFICATION_CONTENT)))
+ return err
+}

Index: environs/storage_test.go
=== modified file 'environs/storage_test.go'
--- environs/storage_test.go 2013-05-02 15:55:42 +0000
+++ environs/storage_test.go 2013-05-30 11:09:25 +0000
@@ -4,6 +4,9 @@
  package environs_test

  import (
+ "fmt"
+ "io"
+ "io/ioutil"
   . "launchpad.net/gocheck"
   "launchpad.net/juju-core/environs"
  )
@@ -29,3 +32,46 @@
   c.Assert(names, IsNil)
   c.Assert(err, IsNil)
  }
+
+type MockStorage struct {
+ StorageRequests []string
+}
+
+func (ms *MockStorage) Put(filename string, reader io.Reader, length
int64) error {
+ var content []byte
+ content, _ = ioutil.ReadAll(reader)
+ log_message := fmt.Sprintf(
+ "Put('%s', '%s', %d)", filename, content, length)
+ ms.StorageRequests = append(ms.StorageRequests, log_message)
+ return nil
+}
+
+func (ms *MockStorage) Get(name string) (io.ReadCloser, error) {
+ log_message := fmt.Sprintf("Get('%s')", name)
+ ms.StorageRequests = append(ms.StorageRequests, log_message)
+ return nil, &environs.NotFoundError{fmt.Errorf("file %q not found", name)}
+}
+
+func (ms *MockStorage) List(prefix string) ([]string, error) {
+ return nil, nil
+}
+
+func (ms *MockStorage) Remove(file string) error {
+ return nil
+}
+
+func (ms *MockStorage) URL(name string) (string, error) {
+ return "", fmt.Errorf("file %q not found", name)
+}
+
+type VerifyStorageSuite struct{}
+
+var _ = Suite(&VerifyStorageSuite{})
+
+func (s *VerifyStorageSuite) TestVerifyStorage(c *C) {
+ storage := MockStorage{}
+ error := environs.VerifyStorage(&storage)
+ c.Assert(error, IsNil)
+ c.Assert(storage.StorageRequests[0], Equals,
+ "Put('bootstrap-verify', 'juju-core storage writing verified: ok', 38)")
+}

Index: environs/dummy/environs.go
=== modified file 'environs/dummy/environs.go'
--- environs/dummy/environs.go 2013-05-02 15:55:42 +0000
+++ environs/dummy/environs.go 2013-05-28 14:22:25 +0000
@@ -433,6 +433,9 @@
   if _, ok := e.Config().CACert(); !ok {
    return fmt.Errorf("no CA certificate in environment configuration")
   }
+ if err := environs.VerifyStorage(e.Storage()); err != nil {
+ return fmt.Errorf("Provider storage is not writeable.")
+ }

   possibleTools, err := environs.FindBootstrapTools(e, cons)
   if err != nil {

Index: environs/ec2/ec2.go
=== modified file 'environs/ec2/ec2.go'
--- environs/ec2/ec2.go 2013-05-21 22:11:27 +0000
+++ environs/ec2/ec2.go 2013-05-28 14:22:25 +0000
@@ -258,6 +258,12 @@
   if err == nil {
    return fmt.Errorf("environment is already bootstrapped")
   }
+
+ err = environs.VerifyStorage(e.Storage())
+ if err != nil {
+ return fmt.Errorf("Provider storage is not writeable.")
+ }
+
   if !environs.IsNotFoundError(err) {
    return fmt.Errorf("cannot query old bootstrap state: %v", err)
   }

Index: environs/maas/environ.go
=== modified file 'environs/maas/environ.go'
--- environs/maas/environ.go 2013-05-02 15:55:42 +0000
+++ environs/maas/environ.go 2013-05-28 14:22:25 +0000
@@ -113,6 +113,10 @@
  func (env *maasEnviron) Bootstrap(cons constraints.Value) error {
   // TODO(fwereade): this should check for an existing environment before
   // starting a new one -- even given raciness, it's better than nothing.
+ if err := environs.VerifyStorage(env.Storage()); err != nil {
+ return fmt.Errorf("Provider storage is not writeable.")
+ }
+
   inst, err := env.startBootstrapNode(cons)
   if err != nil {
    return err

Index: environs/openstack/provider.go
=== modified file 'environs/openstack/provider.go'
--- environs/openstack/provider.go 2013-05-22 23:25:13 +0000
+++ environs/openstack/provider.go 2013-05-28 14:22:25 +0000
@@ -489,6 +489,10 @@
   if !environs.IsNotFoundError(err) {
    return fmt.Errorf("cannot query old bootstrap state: %v", err)
   }
+ err = environs.VerifyStorage(e.Storage())
+ if err != nil {
+ return fmt.Errorf("Provider storage is not writeable.")
+ }

   possibleTools, err := environs.FindBootstrapTools(e, cons)
   if err != nil {