Merge into trunk : bootstrap-verify : Code : juju-core

Status:	Superseded
Proposed branch:	lp://staging/~danilo/juju-core/bootstrap-verify
Merge into:	lp://staging/~juju/juju-core/trunk
Diff against target:	247 lines (+114/-2) 9 files modified cmd/juju/bootstrap_test.go (+4/-0) environs/dummy/environs.go (+7/-1) environs/dummy/storage.go (+6/-1) environs/ec2/ec2.go (+5/-0) environs/jujutest/livetests.go (+17/-0) environs/maas/environ.go (+4/-0) environs/openstack/provider.go (+4/-0) environs/storage.go (+22/-0) environs/storage_test.go (+45/-0)
To merge this branch:	bzr merge lp://staging/~danilo/juju-core/bootstrap-verify
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Juju Engineering		2013-05-30	Pending
Review via email: mp+166481@code.staging.launchpad.net

Revision history for this message

Данило Шеган (danilo) wrote on 2013-05-30:

#

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, &envir...

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:  
dimiter.naydenov@canonical.com-20130528074910-13d16hcybf14v760
+New revision: danilo.segan@canonical.com-20130530110925-z44imw05j0rrq383

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 {

Revision history for this message

William Reade (fwereade) wrote on 2013-05-30:

#

Since you *did* implement this for the dummy provider, you could add a
jujutest test [0] that something actually is written to environ storage.
Wouldn't fix MAAS, which doesn't use those, but that should I think be
testable directly?

[0] these (mostly?) get run against ec2, openstack, dummy. But they're
also somewhat crazy and confusing. I don't really mind how this is
tested, so long as it is :).

PS are we expecting a followup that checks there's no pyjuju
check-writable file?

https://codereview.appspot.com/9876043/diff/1/environs/dummy/environs.go
File environs/dummy/environs.go (right):

https://codereview.appspot.com/9876043/diff/1/environs/dummy/environs.go#newcode437
environs/dummy/environs.go:437: return fmt.Errorf("Provider storage is
not writeable.")
FWIW, this is extending an antipattern -- the dummy provider doesn't do
the same stuff as the others anyway, and faking it up doesn't seem that
helpful -- but I'm not suggesting you change this today.

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

Revision history for this message

Данило Шеган (danilo) wrote on 2013-06-04:

#

Please take a look.

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

Revision history for this message

Данило Шеган (danilo) wrote on 2013-06-04:

#

On 2013/05/30 16:03:43, fwereade wrote:
> Since you *did* implement this for the dummy provider, you could add a
jujutest
> test [0] that something actually is written to environ storage.
Wouldn't fix
> MAAS, which doesn't use those, but that should I think be testable
directly?

At the very least, I've extended the dummy provider's OpPutFile to log
the filename as well and I watch out for that operation in the bootstrap
jujutest. I can add in the file contents as well, which might make
sense for the follow up branch (but if I am doing it for it, I'd rather
do it there).

I'd add a separate test for bootstrap verify, but it would be running
exactly the same code and I am uncertain if extending the test suite run
for this makes sense.

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

Revision history for this message

Данило Шеган (danilo) wrote on 2013-06-04:

#

https://codereview.appspot.com/9876043/diff/1/environs/dummy/environs.go
File environs/dummy/environs.go (right):

https://codereview.appspot.com/9876043/diff/1/environs/dummy/environs.go#newcode437
environs/dummy/environs.go:437: return fmt.Errorf("Provider storage is
not writeable.")
What would you suggest I do instead? Ignore the error?

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

Revision history for this message

Frank Mueller (themue) wrote on 2013-06-04:

#

LGTM with some minor comments.

https://codereview.appspot.com/9876043/diff/4001/environs/ec2/ec2.go
File environs/ec2/ec2.go (right):

https://codereview.appspot.com/9876043/diff/4001/environs/ec2/ec2.go#newcode268
environs/ec2/ec2.go:268: return fmt.Errorf("Provider storage is not
writeable.")
Lowercase, "provider storage ...", and not dot at the end.

https://codereview.appspot.com/9876043/diff/4001/environs/maas/environ.go
File environs/maas/environ.go (right):

https://codereview.appspot.com/9876043/diff/4001/environs/maas/environ.go#newcode117
environs/maas/environ.go:117: return fmt.Errorf("Provider storage is not
writeable.")
Dito, lowercase and dot.

https://codereview.appspot.com/9876043/diff/4001/environs/openstack/provider.go
File environs/openstack/provider.go (right):

https://codereview.appspot.com/9876043/diff/4001/environs/openstack/provider.go#newcode494
environs/openstack/provider.go:494: return fmt.Errorf("Provider storage
is not writeable.")
And again lowercase and dot.

https://codereview.appspot.com/9876043/diff/4001/environs/storage.go
File environs/storage.go (right):

https://codereview.appspot.com/9876043/diff/4001/environs/storage.go#newcode18
environs/storage.go:18: const VERIFICATION_FILENAME string =
"bootstrap-verify"
Typical camel-case is used. And it only starts with an uppercase char if
it shall be exported.

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

Revision history for this message

John A Meinel (jameinel) wrote on 2013-06-04:

#

I think this is reasonable, but is it ok to just write something and
look for an error code rather than also trying to read it back in (or
seeing if one already exists)?

https://codereview.appspot.com/9876043/diff/4001/environs/storage.go
File environs/storage.go (right):

https://codereview.appspot.com/9876043/diff/4001/environs/storage.go#newcode19
environs/storage.go:19: const VERIFICATION_CONTENT = "juju-core storage
writing verified: ok"
If you are writing it out for long-term storage, it is usually nice to
put a '\n' on the end of it.

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

Revision history for this message

William Reade (fwereade) wrote on 2013-06-05:

#

On 2013/06/04 12:31:29, Danilo wrote:
> On 2013/05/30 16:03:43, fwereade wrote:
> > Since you *did* implement this for the dummy provider, you could add
a
> jujutest
> > test [0] that something actually is written to environ storage.
Wouldn't fix
> > MAAS, which doesn't use those, but that should I think be testable
directly?

> At the very least, I've extended the dummy provider's OpPutFile to log
the
> filename as well and I watch out for that operation in the bootstrap
jujutest.
> I can add in the file contents as well, which might make sense for the
follow up
> branch (but if I am doing it for it, I'd rather do it there).

> I'd add a separate test for bootstrap verify, but it would be running
exactly
> the same code and I am uncertain if extending the test suite run for
this makes
> sense.

I don't see an environs/jujutest change anywhere... if we're not syncing
the check against all providers, it doesn't help.

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

Revision history for this message

William Reade (fwereade) wrote on 2013-06-05:

#

https://codereview.appspot.com/9876043/diff/1/environs/dummy/environs.go
File environs/dummy/environs.go (right):

https://codereview.appspot.com/9876043/diff/1/environs/dummy/environs.go#newcode437
environs/dummy/environs.go:437: return fmt.Errorf("Provider storage is
not writeable.")
On 2013/06/04 12:31:38, Danilo wrote:
> What would you suggest I do instead? Ignore the error?

Sorry, I was commenting a bit obliquely on the fundamental issues with
the dummy provider. Implementing this functionality here is only helpful
if it's to support a jujutest test case, which would then be able to
verify that all providers (except MAAS... different discussion) fulfil
the same expectations. That is: without a jujutest case, I don't think
it's meaningful to do this.

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

Revision history for this message

Dimiter Naydenov (dimitern) wrote on 2013-06-07:

#

Almost there and nice! Some minor suggestions, but big +1 on fwereade's
comments about changing jujutest.

https://codereview.appspot.com/9876043/diff/4001/environs/dummy/environs.go
File environs/dummy/environs.go (right):

https://codereview.appspot.com/9876043/diff/4001/environs/dummy/environs.go#newcode440
environs/dummy/environs.go:440: return fmt.Errorf("Provider storage is
not writeable.")
s/Provider/provider/ and no dot at the end.

https://codereview.appspot.com/9876043/diff/4001/environs/ec2/ec2.go
File environs/ec2/ec2.go (right):

https://codereview.appspot.com/9876043/diff/4001/environs/ec2/ec2.go#newcode268
environs/ec2/ec2.go:268: return fmt.Errorf("Provider storage is not
writeable.")
On 2013/06/04 12:42:52, mue wrote:
> Lowercase, "provider storage ...", and not dot at the end.

+1

https://codereview.appspot.com/9876043/diff/4001/environs/maas/environ.go
File environs/maas/environ.go (right):

https://codereview.appspot.com/9876043/diff/4001/environs/maas/environ.go#newcode117
environs/maas/environ.go:117: return fmt.Errorf("Provider storage is not
writeable.")
On 2013/06/04 12:42:52, mue wrote:
> Dito, lowercase and dot.

+1

https://codereview.appspot.com/9876043/diff/4001/environs/openstack/provider.go
File environs/openstack/provider.go (right):

https://codereview.appspot.com/9876043/diff/4001/environs/openstack/provider.go#newcode494
environs/openstack/provider.go:494: return fmt.Errorf("Provider storage
is not writeable.")
On 2013/06/04 12:42:52, mue wrote:
> And again lowercase and dot.

+1

https://codereview.appspot.com/9876043/diff/4001/environs/storage.go
File environs/storage.go (right):

https://codereview.appspot.com/9876043/diff/4001/environs/storage.go#newcode18
environs/storage.go:18: const VERIFICATION_FILENAME string =
"bootstrap-verify"
Please, this is not C :)
const VerificationFilename string = "bootstrap-verify"

https://codereview.appspot.com/9876043/diff/4001/environs/storage.go#newcode19
environs/storage.go:19: const VERIFICATION_CONTENT = "juju-core storage
writing verified: ok"
ditto: VerificationContent

https://codereview.appspot.com/9876043/diff/4001/environs/storage_test.go
File environs/storage_test.go (right):

https://codereview.appspot.com/9876043/diff/4001/environs/storage_test.go#newcode36
environs/storage_test.go:36: type MockStorage struct {
does it have to be exported?

https://codereview.appspot.com/9876043/diff/4001/environs/storage_test.go#newcode44
environs/storage_test.go:44: "Put('%s', '%s', %d)", filename, content,
length)
if you're doing the call multiline, please:
log_message := fmt.Sprintf(
   "...",
   filename,
   content,
   length,
)

https://codereview.appspot.com/9876043/diff/4001/environs/storage_test.go#newcode67
environs/storage_test.go:67: type VerifyStorageSuite struct{}
again, the suite doesn't need to be exported (verifyStorageSuite or even
just storageSuite)

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

Almost there and nice! Some minor suggestions, but big +1 on fwereade's
comments about changing jujutest.

https://codereview.appspot.com/9876043/diff/4001/environs/dummy/environs.go
File environs/dummy/environs.go (right):

https://codereview.appspot.com/9876043/diff/4001/environs/dummy/environs.go#newcode440
environs/dummy/environs.go:440: return fmt.Errorf("Provider storage is
not writeable.")
s/Provider/provider/ and no dot at the end.

https://codereview.appspot.com/9876043/diff/4001/environs/ec2/ec2.go
File environs/ec2/ec2.go (right):

https://codereview.appspot.com/9876043/diff/4001/environs/ec2/ec2.go#newcode268
environs/ec2/ec2.go:268: return fmt.Errorf("Provider storage is not
writeable.")
On 2013/06/04 12:42:52, mue wrote:
> Lowercase, "provider storage ...", and not dot at the end.

+1

https://codereview.appspot.com/9876043/diff/4001/environs/maas/environ.go
File environs/maas/environ.go (right):

https://codereview.appspot.com/9876043/diff/4001/environs/maas/environ.go#newcode117
environs/maas/environ.go:117: return fmt.Errorf("Provider storage is not
writeable.")
On 2013/06/04 12:42:52, mue wrote:
> Dito, lowercase and dot.

+1

https://codereview.appspot.com/9876043/diff/4001/environs/openstack/provider.go
File environs/openstack/provider.go (right):

https://codereview.appspot.com/9876043/diff/4001/environs/openstack/provider.go#newcode494
environs/openstack/provider.go:494: return fmt.Errorf("Provider storage
is not writeable.")
On 2013/06/04 12:42:52, mue wrote:
> And again lowercase and dot.

+1

https://codereview.appspot.com/9876043/diff/4001/environs/storage.go
File environs/storage.go (right):

https://codereview.appspot.com/9876043/diff/4001/environs/storage.go#newcode18
environs/storage.go:18: const VERIFICATION_FILENAME string =
"bootstrap-verify"
Please, this is not C :)
const VerificationFilename string = "bootstrap-verify"

https://codereview.appspot.com/9876043/diff/4001/environs/storage.go#newcode19
environs/storage.go:19: const VERIFICATION_CONTENT = "juju-core storage
writing verified: ok"
ditto: VerificationContent

https://codereview.appspot.com/9876043/diff/4001/environs/storage_test.go
File environs/storage_test.go (right):

https://codereview.appspot.com/9876043/diff/4001/environs/storage_test.go#newcode36
environs/storage_test.go:36: type MockStorage struct {
does it have to be exported?

https://codereview.appspot.com/9876043/diff/4001/environs/storage_test.go#newcode44
environs/storage_test.go:44: "Put('%s', '%s', %d)", filename, content,
length)
if you're doing the call multiline, please:
log_message := fmt.Sprintf(
   "...",
   filename,
   content,
   length,
)

https://codereview.appspot.com/9876043/diff/4001/environs/storage_test.go#newcode67
environs/storage_test.go:67: type VerifyStorageSuite struct{}
again, the suite doesn't need to be exported (verifyStorageSuite or even
just storageSuite)

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

Revision history for this message

Данило Шеган (danilo) wrote on 2013-06-13:

#

Download full text (3.7 KiB)

Please take a look.

https://codereview.appspot.com/9876043/diff/1/environs/dummy/environs.go
File environs/dummy/environs.go (right):

https://codereview.appspot.com/9876043/diff/1/environs/dummy/environs.go#newcode437
environs/dummy/environs.go:437: return fmt.Errorf("Provider storage is
not writeable.")
I've added a minimal jujutest that bootstraps the environment, and then
makes sure that the bootstrap-verify file is written in. It has been
tested with at least ec2 and openstack providers.

https://codereview.appspot.com/9876043/diff/4001/environs/dummy/environs.go
File environs/dummy/environs.go (right):

https://codereview.appspot.com/9876043/diff/4001/environs/dummy/environs.go#newcode440
environs/dummy/environs.go:440: return fmt.Errorf("Provider storage is
not writeable.")
On 2013/06/07 12:35:20, dimitern wrote:
> s/Provider/provider/ and no dot at the end.

Fixed already after review by Frank.

https://codereview.appspot.com/9876043/diff/4001/environs/ec2/ec2.go
File environs/ec2/ec2.go (right):

https://codereview.appspot.com/9876043/diff/4001/environs/ec2/ec2.go#newcode268
environs/ec2/ec2.go:268: return fmt.Errorf("Provider storage is not
writeable.")
Fixed.

https://codereview.appspot.com/9876043/diff/4001/environs/maas/environ.go
File environs/maas/environ.go (right):

https://codereview.appspot.com/9876043/diff/4001/environs/maas/environ.go#newcode117
environs/maas/environ.go:117: return fmt.Errorf("Provider storage is not
writeable.")
Fixed.

https://codereview.appspot.com/9876043/diff/4001/environs/openstack/provider.go
File environs/openstack/provider.go (right):

https://codereview.appspot.com/9876043/diff/4001/environs/openstack/provider.go#newcode494
environs/openstack/provider.go:494: return fmt.Errorf("Provider storage
is not writeable.")
And fixed.

https://codereview.appspot.com/9876043/diff/4001/environs/storage.go
File environs/storage.go (right):

https://codereview.appspot.com/9876043/diff/4001/environs/storage.go#newcode18
environs/storage.go:18: const VERIFICATION_FILENAME string =
"bootstrap-verify"
On 2013/06/07 12:35:20, dimitern wrote:
> Please, this is not C :)
> const VerificationFilename string = "bootstrap-verify"

This is actually quite common naming pattern in Python ;) Naming
patterns are there to make it easier for us to see this in an arbitrary
place in code and understand that this is a constant. I know that Go
makes everything starting with a capital exported, but that means that
there's no good naming pattern for constants.

Anyway, changed to verificationFilename, as Frank suggested originally.

https://codereview.appspot.com/9876043/diff/4001/environs/storage.go#newcode19
environs/storage.go:19: const VERIFICATION_CONTENT = "juju-core storage
writing verified: ok"
On 2013/06/04 17:15:34, jameinel wrote:
> If you are writing it out for long-term storage, it is usually nice to
put a
> '\n' on the end of it.

Good point, fixed.

https://codereview.appspot.com/9876043/diff/4001/environs/storage.go#newcode19
environs/storage.go:19: const VERIFICATION_CONTENT = "juju-core storage
writing verified: ok"
On 2013/06/07 12:35:20, dimitern wrote:
> ditto: VerificationContent

Fixed to verificationConte...

juju-core

Merge lp://staging/~danilo/juju-core/bootstrap-verify into lp://staging/~juju/juju-core/trunk

Commit message

Description of the change

Preview Diff

Subscribers