juju-core

Merge lp:~thumper/juju-core/verify-storage-move into lp:~go-bot/juju-core/trunk

  1. verify-storage-move
  2. Merge into trunk
Proposed by Tim Penhey
Status: Merged
Approved by: Tim Penhey
Approved revision: no longer in the source branch.
Merged at revision: 1939
Proposed branch: lp:~thumper/juju-core/verify-storage-move
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 478 lines (+116/-73)
10 files modified
cmd/juju/bootstrap.go (+6/-0)
cmd/juju/bootstrap_test.go (+29/-8)
environs/bootstrap/bootstrap.go (+10/-19)
environs/emptystorage.go (+0/-25)
environs/emptystorage_test.go (+2/-2)
environs/jujutest/livetests.go (+6/-2)
environs/jujutest/tests.go (+9/-5)
environs/open.go (+30/-2)
environs/open_test.go (+3/-3)
provider/dummy/environs.go (+21/-7)
To merge this branch: bzr merge lp:~thumper/juju-core/verify-storage-move
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+188974@code.launchpad.net

Commit message

Fail earlier when bootstrapping if up already.

I broke apart the VerifyBootstrap method into two:
  EnsureNotBootstrapped
  VerifyStorage
Well I didn't create the second, just didn't call it
from the EnsureNotBootstrapped call.

The ensure method is now called in the command bootstrap
prior to any upload tools checks.

However the implication of this was much futher reaching
than I expected. Many tests assumed that bootstrap.Bootstrap
was enough to bootstrap and environment, and that it would
complain if already bootstrapped. Since the complaining was
moved into the command function, some changes had to be made.
Since some of the tests were refactored to check the storage,
this required making the dummy provider actually write the
provider-state into storage like a normal provider.

I moved the VerifyStorage out of emptystorage.go and into open.go
as it seemed better fitting there (and the comment actually
makes sense now).

Also used the environs constant for the VerificationFilename.

https://codereview.appspot.com/14279044/

Description of the change

Fail earlier when bootstrapping if up already.

I broke apart the VerifyBootstrap method into two:
  EnsureNotBootstrapped
  VerifyStorage
Well I didn't create the second, just didn't call it
from the EnsureNotBootstrapped call.

The ensure method is now called in the command bootstrap
prior to any upload tools checks.

However the implication of this was much futher reaching
than I expected. Many tests assumed that bootstrap.Bootstrap
was enough to bootstrap and environment, and that it would
complain if already bootstrapped. Since the complaining was
moved into the command function, some changes had to be made.
Since some of the tests were refactored to check the storage,
this required making the dummy provider actually write the
provider-state into storage like a normal provider.

I moved the VerifyStorage out of emptystorage.go and into open.go
as it seemed better fitting there (and the comment actually
makes sense now).

Also used the environs constant for the VerificationFilename.

https://codereview.appspot.com/14279044/

To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :

Reviewers: mp+188974_code.launchpad.net,

Message:
Please take a look.

Description:
Fail earlier when bootstrapping if up already.

I broke apart the VerifyBootstrap method into two:
   EnsureNotBootstrapped
   VerifyStorage
Well I didn't create the second, just didn't call it
from the EnsureNotBootstrapped call.

The ensure method is now called in the command bootstrap
prior to any upload tools checks.

However the implication of this was much futher reaching
than I expected. Many tests assumed that bootstrap.Bootstrap
was enough to bootstrap and environment, and that it would
complain if already bootstrapped. Since the complaining was
moved into the command function, some changes had to be made.
Since some of the tests were refactored to check the storage,
this required making the dummy provider actually write the
provider-state into storage like a normal provider.

I moved the VerifyStorage out of emptystorage.go and into open.go
as it seemed better fitting there (and the comment actually
makes sense now).

Also used the environs constant for the VerificationFilename.

https://code.launchpad.net/~thumper/juju-core/verify-storage-move/+merge/188974

(do not edit description out of merge proposal)

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

Affected files (+119, -74 lines):
   A [revision details]
   M cmd/juju/bootstrap.go
   M cmd/juju/bootstrap_test.go
   M environs/bootstrap/bootstrap.go
   M environs/emptystorage.go
   M environs/emptystorage_test.go
   M environs/jujutest/livetests.go
   M environs/jujutest/tests.go
   M environs/open.go
   M environs/open_test.go
   M provider/dummy/environs.go

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

On 2013/10/03 03:19:01, dfc wrote:
> LGTM. Some nice boy scout work in there as well.

https://codereview.appspot.com/14279044/diff/1/environs/bootstrap/bootstrap.go
> File environs/bootstrap/bootstrap.go (right):

https://codereview.appspot.com/14279044/diff/1/environs/bootstrap/bootstrap.go#newcode92
> environs/bootstrap/bootstrap.go:92: _, err :=
 common.LoadState(env.Storage())
> this method hurt my head, but it makes sense in a double negative sort
 of way.

LGTM

https://codereview.appspot.com/14279044/

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

https://codereview.appspot.com/14279044/diff/1/environs/bootstrap/bootstrap.go
File environs/bootstrap/bootstrap.go (left):

https://codereview.appspot.com/14279044/diff/1/environs/bootstrap/bootstrap.go#oldcode96
environs/bootstrap/bootstrap.go:96: // TODO(rog) this feels like a
layering violation - providers
Is this TODO no longer applicable? Seems to be...

https://codereview.appspot.com/14279044/

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

https://codereview.appspot.com/14279044/diff/1/environs/bootstrap/bootstrap.go
File environs/bootstrap/bootstrap.go (left):

https://codereview.appspot.com/14279044/diff/1/environs/bootstrap/bootstrap.go#oldcode96
environs/bootstrap/bootstrap.go:96: // TODO(rog) this feels like a
layering violation - providers
On 2013/10/03 03:21:00, axw1 wrote:
> Is this TODO no longer applicable? Seems to be...

I took it out because everything expects it to be there.
It is how bootstrap checks to see if the environment
is already bootstrapped. I think for consistency across
providers this comment no longer makes sense.

https://codereview.appspot.com/14279044/

Revision history for this message
Roger Peppe (rogpeppe) wrote :

I have reservations about this.
I don't think we should be further entrenching the arbitrary way in
which the ec2 provider happened to mark its server instances.
Some thoughts below.

https://codereview.appspot.com/14279044/diff/1/environs/bootstrap/bootstrap.go
File environs/bootstrap/bootstrap.go (left):

https://codereview.appspot.com/14279044/diff/1/environs/bootstrap/bootstrap.go#oldcode96
environs/bootstrap/bootstrap.go:96: // TODO(rog) this feels like a
layering violation - providers
On 2013/10/03 03:38:01, thumper wrote:
> On 2013/10/03 03:21:00, axw1 wrote:
> > Is this TODO no longer applicable? Seems to be...

> I took it out because everything expects it to be there.
> It is how bootstrap checks to see if the environment
> is already bootstrapped. I think for consistency across
> providers this comment no longer makes sense.

The comment *is* still relevant IMHO. It has long been
the plan to move away from using file storage on ec2 to
indicate state servers (it could be done nicely with
instance tags) and therefore allow the possibility of
having an environment with no local storage at all.

It is *currently* how bootstrap checks to see if the
environment is already bootstrapped, but there's no
particular necessity for it to be this way.
It would be better to have an Environ.IsBootstrapped
call instead.

https://codereview.appspot.com/14279044/diff/1/environs/bootstrap/bootstrap.go
File environs/bootstrap/bootstrap.go (right):

https://codereview.appspot.com/14279044/diff/1/environs/bootstrap/bootstrap.go#newcode91
environs/bootstrap/bootstrap.go:91: func EnsureNotBootstrapped(env
environs.Environ) error {
We usually use the word "ensure" to mean "ensure that something is so".
That's not the case here.

I'd prefer:

// IsBootstrapped returns whether the environment is
// bootstrapped.
func IsBootstrapped(env environs.Environ) (bool, error)

https://codereview.appspot.com/14279044/diff/1/environs/open.go
File environs/open.go (right):

https://codereview.appspot.com/14279044/diff/1/environs/open.go#newcode29
environs/open.go:29: VerifyStorageError error = fmt.Errorf(
s/ error//

https://codereview.appspot.com/14279044/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'cmd/juju/bootstrap.go'
--- cmd/juju/bootstrap.go 2013-10-02 05:18:02 +0000
+++ cmd/juju/bootstrap.go 2013-10-03 03:45:40 +0000
@@ -78,6 +78,12 @@
78 return fmt.Errorf("failed to enable bootstrap storage: %v", err)78 return fmt.Errorf("failed to enable bootstrap storage: %v", err)
79 }79 }
80 }80 }
81 // Check to see if the environment is already bootstrapped
82 // before potentially uploading any tools.
83 if err := bootstrap.EnsureNotBootstrapped(environ); err != nil {
84 return err
85 }
86
81 // TODO (wallyworld): 2013-09-20 bug 122793187 // TODO (wallyworld): 2013-09-20 bug 1227931
82 // We can set a custom tools data source instead of doing an88 // We can set a custom tools data source instead of doing an
83 // unecessary upload.89 // unecessary upload.
8490
=== modified file 'cmd/juju/bootstrap_test.go'
--- cmd/juju/bootstrap_test.go 2013-10-02 18:34:59 +0000
+++ cmd/juju/bootstrap_test.go 2013-10-03 03:45:40 +0000
@@ -53,6 +53,13 @@
53 s.LoggingSuite.TearDownSuite(c)53 s.LoggingSuite.TearDownSuite(c)
54}54}
5555
56func (s *BootstrapSuite) TearDownTest(c *gc.C) {
57 s.ToolsFixture.TearDownTest(c)
58 s.MgoSuite.TearDownTest(c)
59 s.LoggingSuite.TearDownTest(c)
60 dummy.Reset()
61}
62
56type bootstrapRetryTest struct {63type bootstrapRetryTest struct {
57 info string64 info string
58 args []string65 args []string
@@ -101,13 +108,6 @@
101 c.Assert(err, gc.ErrorMatches, test.err)108 c.Assert(err, gc.ErrorMatches, test.err)
102}109}
103110
104func (s *BootstrapSuite) TearDownTest(c *gc.C) {
105 s.ToolsFixture.TearDownTest(c)
106 s.MgoSuite.TearDownTest(c)
107 s.LoggingSuite.TearDownTest(c)
108 dummy.Reset()
109}
110
111func (s *BootstrapSuite) TestTest(c *gc.C) {111func (s *BootstrapSuite) TestTest(c *gc.C) {
112 uploadTools = mockUploadTools112 uploadTools = mockUploadTools
113 defer func() { uploadTools = sync.Upload }()113 defer func() { uploadTools = sync.Upload }()
@@ -190,7 +190,11 @@
190 }190 }
191 opPutBootstrapVerifyFile := (<-opc).(dummy.OpPutFile)191 opPutBootstrapVerifyFile := (<-opc).(dummy.OpPutFile)
192 c.Check(opPutBootstrapVerifyFile.Env, gc.Equals, "peckham")192 c.Check(opPutBootstrapVerifyFile.Env, gc.Equals, "peckham")
193 c.Check(opPutBootstrapVerifyFile.FileName, gc.Equals, "bootstrap-verify")193 c.Check(opPutBootstrapVerifyFile.FileName, gc.Equals, environs.VerificationFilename)
194
195 opPutBootstrapInitFile := (<-opc).(dummy.OpPutFile)
196 c.Check(opPutBootstrapInitFile.Env, gc.Equals, "peckham")
197 c.Check(opPutBootstrapInitFile.FileName, gc.Equals, "provider-state")
194198
195 opBootstrap := (<-opc).(dummy.OpBootstrap)199 opBootstrap := (<-opc).(dummy.OpBootstrap)
196 c.Check(opBootstrap.Env, gc.Equals, "peckham")200 c.Check(opBootstrap.Env, gc.Equals, "peckham")
@@ -265,6 +269,23 @@
265 },269 },
266}}270}}
267271
272func (s *BootstrapSuite) TestBootstrapTwice(c *gc.C) {
273 restore := createToolsStore(c)
274 defer restore()
275 _, fake := makeEmptyFakeHome(c)
276 defer fake.Restore()
277
278 ctx := coretesting.Context(c)
279 code := cmd.Main(&BootstrapCommand{}, ctx, nil)
280 c.Check(code, gc.Equals, 0)
281
282 ctx2 := coretesting.Context(c)
283 code2 := cmd.Main(&BootstrapCommand{}, ctx2, nil)
284 c.Check(code2, gc.Equals, 1)
285 c.Check(coretesting.Stderr(ctx2), gc.Equals, "error: environment is already bootstrapped\n")
286 c.Check(coretesting.Stdout(ctx2), gc.Equals, "")
287}
288
268func (s *BootstrapSuite) TestAutoSync(c *gc.C) {289func (s *BootstrapSuite) TestAutoSync(c *gc.C) {
269 // Prepare a mock storage for testing and store the290 // Prepare a mock storage for testing and store the
270 // dummy tools in there.291 // dummy tools in there.
271292
=== modified file 'environs/bootstrap/bootstrap.go'
--- environs/bootstrap/bootstrap.go 2013-10-02 10:39:12 +0000
+++ environs/bootstrap/bootstrap.go 2013-10-03 03:45:40 +0000
@@ -42,11 +42,8 @@
42 if _, hasCAKey := cfg.CAPrivateKey(); !hasCAKey {42 if _, hasCAKey := cfg.CAPrivateKey(); !hasCAKey {
43 return fmt.Errorf("environment configuration has no ca-private-key")43 return fmt.Errorf("environment configuration has no ca-private-key")
44 }44 }
45 // If the state file exists, it might actually have just been45 // Write out the bootstrap-init file, and confirm storage is writeable.
46 // removed by Destroy, and eventual consistency has not caught46 if err := environs.VerifyStorage(environ.Storage()); err != nil {
47 // up yet, so we retry to verify if that is happening.
48 err := verifyBootstrapInit(environ)
49 if err != nil {
50 return err47 return err
51 }48 }
5249
@@ -89,24 +86,18 @@
89 return environ.Bootstrap(cons, newestTools)86 return environ.Bootstrap(cons, newestTools)
90}87}
9188
92// verifyBootstrapInit does the common initial check before bootstrapping, to89// EnsureNotBootstrapped returns null if the environment is not bootstrapped,
93// confirm that the environment isn't already running, and that the storage90// and an error if it is or if the function was not able to tell.
94// works.91func EnsureNotBootstrapped(env environs.Environ) error {
95func verifyBootstrapInit(env environs.Environ) error {92 _, err := common.LoadState(env.Storage())
96 // TODO(rog) this feels like a layering violation - providers93 // If there is no error loading the bootstrap state, then we are bootstrapped.
97 // should not necessarily be required to store their bootstrap
98 // state in a file. This verification should probably
99 // be moved into provider and called by the providers themselves.
100 stor := env.Storage()
101 _, err := common.LoadState(stor)
102 if err == nil {94 if err == nil {
103 return fmt.Errorf("environment is already bootstrapped")95 return fmt.Errorf("environment is already bootstrapped")
104 }96 }
105 if err != environs.ErrNotBootstrapped {97 if err == environs.ErrNotBootstrapped {
106 return fmt.Errorf("cannot query old bootstrap state: %v", err)98 return nil
107 }99 }
108100 return err
109 return environs.VerifyStorage(stor)
110}101}
111102
112// ConfigureBootstrapMachine adds the initial machine into state. As a part103// ConfigureBootstrapMachine adds the initial machine into state. As a part
113104
=== modified file 'environs/emptystorage.go'
--- environs/emptystorage.go 2013-09-18 07:13:09 +0000
+++ environs/emptystorage.go 2013-10-03 03:45:40 +0000
@@ -6,11 +6,9 @@
6import (6import (
7 "fmt"7 "fmt"
8 "io"8 "io"
9 "strings"
109
11 "launchpad.net/juju-core/environs/storage"10 "launchpad.net/juju-core/environs/storage"
12 "launchpad.net/juju-core/errors"11 "launchpad.net/juju-core/errors"
13 "launchpad.net/juju-core/log"
14 "launchpad.net/juju-core/utils"12 "launchpad.net/juju-core/utils"
15)13)
1614
@@ -20,16 +18,6 @@
2018
21type emptyStorage struct{}19type emptyStorage struct{}
2220
23// File named `verificationFilename` in the storage will contain
24// `verificationContent`. This is also used to differentiate between
25// Python Juju and juju-core environments, so change the content with
26// care (and update CheckEnvironment below when you do that).
27const verificationFilename string = "bootstrap-verify"
28const verificationContent = "juju-core storage writing verified: ok\n"
29
30var VerifyStorageError error = fmt.Errorf(
31 "provider storage is not writable")
32
33func (s emptyStorage) Get(name string) (io.ReadCloser, error) {21func (s emptyStorage) Get(name string) (io.ReadCloser, error) {
34 return nil, errors.NotFoundf("file %q", name)22 return nil, errors.NotFoundf("file %q", name)
35}23}
@@ -51,16 +39,3 @@
51func (s emptyStorage) ShouldRetry(err error) bool {39func (s emptyStorage) ShouldRetry(err error) bool {
52 return false40 return false
53}41}
54
55func VerifyStorage(stor storage.Storage) error {
56 reader := strings.NewReader(verificationContent)
57 err := stor.Put(verificationFilename, reader,
58 int64(len(verificationContent)))
59 if err != nil {
60 log.Debugf(
61 "environs: failed to write bootstrap-verify file: %v",
62 err)
63 return VerifyStorageError
64 }
65 return nil
66}
6742
=== modified file 'environs/emptystorage_test.go'
--- environs/emptystorage_test.go 2013-10-01 16:53:02 +0000
+++ environs/emptystorage_test.go 2013-10-03 03:45:40 +0000
@@ -68,7 +68,7 @@
68 stor := environ.Storage()68 stor := environ.Storage()
69 err = environs.VerifyStorage(stor)69 err = environs.VerifyStorage(stor)
70 c.Assert(err, gc.IsNil)70 c.Assert(err, gc.IsNil)
71 reader, err := storage.Get(stor, "bootstrap-verify")71 reader, err := storage.Get(stor, environs.VerificationFilename)
72 c.Assert(err, gc.IsNil)72 c.Assert(err, gc.IsNil)
73 defer reader.Close()73 defer reader.Close()
74 contents, err := ioutil.ReadAll(reader)74 contents, err := ioutil.ReadAll(reader)
@@ -84,7 +84,7 @@
84 c.Assert(err, gc.IsNil)84 c.Assert(err, gc.IsNil)
85 stor := environ.Storage()85 stor := environ.Storage()
86 someError := errors.Unauthorizedf("you shall not pass")86 someError := errors.Unauthorizedf("you shall not pass")
87 dummy.Poison(stor, "bootstrap-verify", someError)87 dummy.Poison(stor, environs.VerificationFilename, someError)
88 err = environs.VerifyStorage(stor)88 err = environs.VerifyStorage(stor)
89 c.Assert(err, gc.Equals, environs.VerifyStorageError)89 c.Assert(err, gc.Equals, environs.VerifyStorageError)
90}90}
9191
=== modified file 'environs/jujutest/livetests.go'
--- environs/jujutest/livetests.go 2013-10-02 10:39:12 +0000
+++ environs/jujutest/livetests.go 2013-10-03 03:45:40 +0000
@@ -137,7 +137,9 @@
137 c.Assert(err, gc.IsNil)137 c.Assert(err, gc.IsNil)
138 }138 }
139 envtesting.UploadFakeTools(c, t.Env.Storage())139 envtesting.UploadFakeTools(c, t.Env.Storage())
140 err := bootstrap.Bootstrap(t.Env, cons)140 err := bootstrap.EnsureNotBootstrapped(t.Env)
141 c.Assert(err, gc.IsNil)
142 err = bootstrap.Bootstrap(t.Env, cons)
141 c.Assert(err, gc.IsNil)143 c.Assert(err, gc.IsNil)
142 t.bootstrapped = true144 t.bootstrapped = true
143}145}
@@ -385,9 +387,11 @@
385}387}
386388
387func (t *LiveTests) TestBootstrapMultiple(c *gc.C) {389func (t *LiveTests) TestBootstrapMultiple(c *gc.C) {
390 // bootstrap.Bootstrap no longer raises errors if the environment is
391 // already up, this has been moved into the bootstrap command.
388 t.BootstrapOnce(c)392 t.BootstrapOnce(c)
389393
390 err := bootstrap.Bootstrap(t.Env, constraints.Value{})394 err := bootstrap.EnsureNotBootstrapped(t.Env)
391 c.Assert(err, gc.ErrorMatches, "environment is already bootstrapped")395 c.Assert(err, gc.ErrorMatches, "environment is already bootstrapped")
392396
393 c.Logf("destroy env")397 c.Logf("destroy env")
394398
=== modified file 'environs/jujutest/tests.go'
--- environs/jujutest/tests.go 2013-10-01 12:03:33 +0000
+++ environs/jujutest/tests.go 2013-10-03 03:45:40 +0000
@@ -131,19 +131,21 @@
131func (t *Tests) TestBootstrap(c *gc.C) {131func (t *Tests) TestBootstrap(c *gc.C) {
132 e := t.Prepare(c)132 e := t.Prepare(c)
133 envtesting.UploadFakeTools(c, e.Storage())133 envtesting.UploadFakeTools(c, e.Storage())
134 err := bootstrap.Bootstrap(e, constraints.Value{})134 err := bootstrap.EnsureNotBootstrapped(e)
135 c.Assert(err, gc.IsNil)
136 err = bootstrap.Bootstrap(e, constraints.Value{})
135 c.Assert(err, gc.IsNil)137 c.Assert(err, gc.IsNil)
136138
137 info, apiInfo, err := e.StateInfo()139 info, apiInfo, err := e.StateInfo()
138 c.Check(info.Addrs, gc.Not(gc.HasLen), 0)140 c.Check(info.Addrs, gc.Not(gc.HasLen), 0)
139 c.Check(apiInfo.Addrs, gc.Not(gc.HasLen), 0)141 c.Check(apiInfo.Addrs, gc.Not(gc.HasLen), 0)
140142
141 err = bootstrap.Bootstrap(e, constraints.Value{})143 err = bootstrap.EnsureNotBootstrapped(e)
142 c.Assert(err, gc.ErrorMatches, "environment is already bootstrapped")144 c.Assert(err, gc.ErrorMatches, "environment is already bootstrapped")
143145
144 e2 := t.Open(c)146 e2 := t.Open(c)
145 envtesting.UploadFakeTools(c, e2.Storage())147 envtesting.UploadFakeTools(c, e2.Storage())
146 err = bootstrap.Bootstrap(e2, constraints.Value{})148 err = bootstrap.EnsureNotBootstrapped(e2)
147 c.Assert(err, gc.ErrorMatches, "environment is already bootstrapped")149 c.Assert(err, gc.ErrorMatches, "environment is already bootstrapped")
148150
149 info2, apiInfo2, err := e2.StateInfo()151 info2, apiInfo2, err := e2.StateInfo()
@@ -157,11 +159,13 @@
157 e3 := t.Prepare(c)159 e3 := t.Prepare(c)
158 envtesting.UploadFakeTools(c, e3.Storage())160 envtesting.UploadFakeTools(c, e3.Storage())
159161
162 err = bootstrap.EnsureNotBootstrapped(e3)
163 c.Assert(err, gc.IsNil)
160 err = bootstrap.Bootstrap(e3, constraints.Value{})164 err = bootstrap.Bootstrap(e3, constraints.Value{})
161 c.Assert(err, gc.IsNil)165 c.Assert(err, gc.IsNil)
162166
163 err = bootstrap.Bootstrap(e3, constraints.Value{})167 err = bootstrap.EnsureNotBootstrapped(e3)
164 c.Assert(err, gc.NotNil)168 c.Assert(err, gc.ErrorMatches, "environment is already bootstrapped")
165}169}
166170
167var noRetry = utils.AttemptStrategy{}171var noRetry = utils.AttemptStrategy{}
168172
=== modified file 'environs/open.go'
--- environs/open.go 2013-10-02 18:41:54 +0000
+++ environs/open.go 2013-10-03 03:45:40 +0000
@@ -6,6 +6,7 @@
6import (6import (
7 "fmt"7 "fmt"
8 "io/ioutil"8 "io/ioutil"
9 "strings"
9 "time"10 "time"
1011
11 "launchpad.net/juju-core/cert"12 "launchpad.net/juju-core/cert"
@@ -15,7 +16,21 @@
15 "launchpad.net/juju-core/errors"16 "launchpad.net/juju-core/errors"
16)17)
1718
18var InvalidEnvironmentError = fmt.Errorf("environment is not a juju-core environment")19// File named `VerificationFilename` in the storage will contain
20// `verificationContent`. This is also used to differentiate between
21// Python Juju and juju-core environments, so change the content with
22// care (and update CheckEnvironment below when you do that).
23const (
24 VerificationFilename = "bootstrap-verify"
25 verificationContent = "juju-core storage writing verified: ok\n"
26)
27
28var (
29 VerifyStorageError error = fmt.Errorf(
30 "provider storage is not writable")
31 InvalidEnvironmentError = fmt.Errorf(
32 "environment is not a juju-core environment")
33)
1934
20// ConfigForName returns the configuration for the environment with the35// ConfigForName returns the configuration for the environment with the
21// given name from the default environments file. If the name is blank,36// given name from the default environments file. If the name is blank,
@@ -199,6 +214,19 @@
199 return nil214 return nil
200}215}
201216
217// VerifyStorage writes the bootstrap init file to the storage to indicate
218// that the storage is writable.
219func VerifyStorage(stor storage.Storage) error {
220 reader := strings.NewReader(verificationContent)
221 err := stor.Put(VerificationFilename, reader,
222 int64(len(verificationContent)))
223 if err != nil {
224 logger.Warningf("failed to write bootstrap-verify file: %v", err)
225 return VerifyStorageError
226 }
227 return nil
228}
229
202// CheckEnvironment checks if an environment has a bootstrap-verify230// CheckEnvironment checks if an environment has a bootstrap-verify
203// that is written by juju-core commands (as compared to one being231// that is written by juju-core commands (as compared to one being
204// written by Python juju).232// written by Python juju).
@@ -210,7 +238,7 @@
210// Returns InvalidEnvironmentError on failure, nil otherwise.238// Returns InvalidEnvironmentError on failure, nil otherwise.
211func CheckEnvironment(environ Environ) error {239func CheckEnvironment(environ Environ) error {
212 stor := environ.Storage()240 stor := environ.Storage()
213 reader, err := storage.Get(stor, verificationFilename)241 reader, err := storage.Get(stor, VerificationFilename)
214 if errors.IsNotFoundError(err) {242 if errors.IsNotFoundError(err) {
215 // When verification file does not exist, this is a juju-core243 // When verification file does not exist, this is a juju-core
216 // environment.244 // environment.
217245
=== modified file 'environs/open_test.go'
--- environs/open_test.go 2013-10-02 18:41:54 +0000
+++ environs/open_test.go 2013-10-03 03:45:40 +0000
@@ -297,7 +297,7 @@
297 // When the bootstrap-verify file does not exist, it still believes297 // When the bootstrap-verify file does not exist, it still believes
298 // the environment is a juju-core one because earlier versions298 // the environment is a juju-core one because earlier versions
299 // did not create that file.299 // did not create that file.
300 err = stor.Remove("bootstrap-verify")300 err = stor.Remove(environs.VerificationFilename)
301 c.Assert(err, gc.IsNil)301 c.Assert(err, gc.IsNil)
302 err = environs.CheckEnvironment(environ)302 err = environs.CheckEnvironment(environ)
303 c.Assert(err, gc.IsNil)303 c.Assert(err, gc.IsNil)
@@ -318,7 +318,7 @@
318 // When fetching the verification file from storage fails,318 // When fetching the verification file from storage fails,
319 // we get an InvalidEnvironmentError.319 // we get an InvalidEnvironmentError.
320 someError := errors.Unauthorizedf("you shall not pass")320 someError := errors.Unauthorizedf("you shall not pass")
321 dummy.Poison(stor, "bootstrap-verify", someError)321 dummy.Poison(stor, environs.VerificationFilename, someError)
322 err = environs.CheckEnvironment(environ)322 err = environs.CheckEnvironment(environ)
323 c.Assert(err, gc.Equals, someError)323 c.Assert(err, gc.Equals, someError)
324}324}
@@ -333,7 +333,7 @@
333 stor := environ.Storage()333 stor := environ.Storage()
334 content := "bad verification content"334 content := "bad verification content"
335 reader := strings.NewReader(content)335 reader := strings.NewReader(content)
336 err = stor.Put("bootstrap-verify", reader, int64(len(content)))336 err = stor.Put(environs.VerificationFilename, reader, int64(len(content)))
337 c.Assert(err, gc.IsNil)337 c.Assert(err, gc.IsNil)
338338
339 // When the bootstrap-verify file contains unexpected content,339 // When the bootstrap-verify file contains unexpected content,
340340
=== modified file 'provider/dummy/environs.go'
--- provider/dummy/environs.go 2013-10-03 00:40:14 +0000
+++ provider/dummy/environs.go 2013-10-03 03:45:40 +0000
@@ -33,6 +33,8 @@
33 "sync"33 "sync"
34 "time"34 "time"
3535
36 "launchpad.net/loggo"
37
36 "launchpad.net/juju-core/constraints"38 "launchpad.net/juju-core/constraints"
37 "launchpad.net/juju-core/environs"39 "launchpad.net/juju-core/environs"
38 "launchpad.net/juju-core/environs/cloudinit"40 "launchpad.net/juju-core/environs/cloudinit"
@@ -42,7 +44,6 @@
42 "launchpad.net/juju-core/environs/storage"44 "launchpad.net/juju-core/environs/storage"
43 "launchpad.net/juju-core/environs/tools"45 "launchpad.net/juju-core/environs/tools"
44 "launchpad.net/juju-core/instance"46 "launchpad.net/juju-core/instance"
45 "launchpad.net/juju-core/log"
46 "launchpad.net/juju-core/names"47 "launchpad.net/juju-core/names"
47 "launchpad.net/juju-core/provider"48 "launchpad.net/juju-core/provider"
48 "launchpad.net/juju-core/provider/common"49 "launchpad.net/juju-core/provider/common"
@@ -55,6 +56,8 @@
55 "launchpad.net/juju-core/utils"56 "launchpad.net/juju-core/utils"
56)57)
5758
59var logger = loggo.GetLogger("juju.provider.dummy")
60
58// SampleConfig() returns an environment configuration with all required61// SampleConfig() returns an environment configuration with all required
59// attributes set.62// attributes set.
60func SampleConfig() testing.Attrs {63func SampleConfig() testing.Attrs {
@@ -208,7 +211,7 @@
208// operation listener. All opened environments after Reset will share211// operation listener. All opened environments after Reset will share
209// the same underlying state.212// the same underlying state.
210func Reset() {213func Reset() {
211 log.Infof("environs/dummy: reset environment")214 logger.Infof("reset environment")
212 p := &providerInstance215 p := &providerInstance
213 p.mu.Lock()216 p.mu.Lock()
214 defer p.mu.Unlock()217 defer p.mu.Unlock()
@@ -538,7 +541,7 @@
538 return fmt.Errorf("no CA certificate in environment configuration")541 return fmt.Errorf("no CA certificate in environment configuration")
539 }542 }
540543
541 log.Infof("environs/dummy: would pick tools from %s", possibleTools)544 logger.Infof("would pick tools from %s", possibleTools)
542 cfg, err := environs.BootstrapConfig(e.Config())545 cfg, err := environs.BootstrapConfig(e.Config())
543 if err != nil {546 if err != nil {
544 return fmt.Errorf("cannot make bootstrap config: %v", err)547 return fmt.Errorf("cannot make bootstrap config: %v", err)
@@ -553,6 +556,17 @@
553 if estate.bootstrapped {556 if estate.bootstrapped {
554 return fmt.Errorf("environment is already bootstrapped")557 return fmt.Errorf("environment is already bootstrapped")
555 }558 }
559 // Write the bootstrap file just like a normal provider. However
560 // we need to release the mutex for the save state to work, so regain
561 // it after the call.
562 estate.mu.Unlock()
563 if err := common.SaveState(e.Storage(), &common.BootstrapState{StateInstances: []instance.Id{"localhost"}}); err != nil {
564 logger.Errorf("failed to save state instances: %v", err)
565 estate.mu.Lock() // otherwise defered unlock will fail
566 return err
567 }
568 estate.mu.Lock() // back at it
569
556 if e.ecfg().stateServer() {570 if e.ecfg().stateServer() {
557 // TODO(rog) factor out relevant code from cmd/jujud/bootstrap.go571 // TODO(rog) factor out relevant code from cmd/jujud/bootstrap.go
558 // so that we can call it here.572 // so that we can call it here.
@@ -653,7 +667,7 @@
653667
654 defer delay()668 defer delay()
655 machineId := machineConfig.MachineId669 machineId := machineConfig.MachineId
656 log.Infof("environs/dummy: dummy startinstance, machine %s", machineId)670 logger.Infof("dummy startinstance, machine %s", machineId)
657 if err := e.checkBroken("StartInstance"); err != nil {671 if err := e.checkBroken("StartInstance"); err != nil {
658 return nil, nil, err672 return nil, nil, err
659 }673 }
@@ -675,7 +689,7 @@
675 if machineConfig.APIInfo.Tag != names.MachineTag(machineId) {689 if machineConfig.APIInfo.Tag != names.MachineTag(machineId) {
676 return nil, nil, fmt.Errorf("entity tag must match started machine")690 return nil, nil, fmt.Errorf("entity tag must match started machine")
677 }691 }
678 log.Infof("environs/dummy: would pick tools from %s", possibleTools)692 logger.Infof("would pick tools from %s", possibleTools)
679 series := possibleTools.OneSeries()693 series := possibleTools.OneSeries()
680 i := &dummyInstance{694 i := &dummyInstance{
681 id: instance.Id(fmt.Sprintf("%s-%d", e.name, estate.maxId)),695 id: instance.Id(fmt.Sprintf("%s-%d", e.name, estate.maxId)),
@@ -899,7 +913,7 @@
899913
900func (inst *dummyInstance) OpenPorts(machineId string, ports []instance.Port) error {914func (inst *dummyInstance) OpenPorts(machineId string, ports []instance.Port) error {
901 defer delay()915 defer delay()
902 log.Infof("environs/dummy: openPorts %s, %#v", machineId, ports)916 logger.Infof("openPorts %s, %#v", machineId, ports)
903 if inst.firewallMode != config.FwInstance {917 if inst.firewallMode != config.FwInstance {
904 return fmt.Errorf("invalid firewall mode %q for opening ports on instance",918 return fmt.Errorf("invalid firewall mode %q for opening ports on instance",
905 inst.firewallMode)919 inst.firewallMode)
@@ -970,7 +984,7 @@
970// pause execution to simulate the latency of a real provider984// pause execution to simulate the latency of a real provider
971func delay() {985func delay() {
972 if providerDelay > 0 {986 if providerDelay > 0 {
973 log.Infof("environs/dummy: pausing for %v", providerDelay)987 logger.Infof("pausing for %v", providerDelay)
974 <-time.After(providerDelay)988 <-time.After(providerDelay)
975 }989 }
976}990}

Subscribers

People subscribed via source and target branches

to status/vote changes: