Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(341)

Issue 6347044: environs/ec2: bootstrap (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 9 months ago by dfc
Modified:
11 years, 8 months ago
Reviewers:
mp+112274
Visibility:
Public.

Description

environs/ec2: bootstrap This propsal is an incremental improvement towards ec2 bootstrapping. I've taken the great work Roger did and incorporated Williams testing changes. juju bootstrap is now able to create machine/0 with a provisioner running. The provisioner currently does not do anything as it has no secrets. Once the problem of pushing the real secrets to ec2 is solved, changing the HasProvisioner flag in live_test.go should allow this test to pass in the real -amazon environment. https://code.launchpad.net/~dave-cheney/juju-core/go-environs-ec2-cloudinit-fixes/+merge/112274 (do not edit description out of merge proposal)

Patch Set 1 #

Patch Set 2 : environs/ec2: bootstrap #

Patch Set 3 : environs/ec2: bootstrap #

Patch Set 4 : environs/ec2: bootstrap #

Patch Set 5 : environs/ec2: bootstrap #

Patch Set 6 : environs/ec2: bootstrap #

Patch Set 7 : environs/ec2: bootstrap #

Patch Set 8 : environs/ec2: bootstrap #

Patch Set 9 : environs/ec2: bootstrap #

Patch Set 10 : environs/ec2: bootstrap #

Patch Set 11 : environs/ec2: bootstrap #

Patch Set 12 : environs/ec2: bootstrap #

Total comments: 32

Patch Set 13 : environs/ec2: bootstrap #

Total comments: 2

Patch Set 14 : environs/ec2: bootstrap #

Patch Set 15 : environs/ec2: bootstrap #

Patch Set 16 : environs/ec2: bootstrap #

Total comments: 6

Patch Set 17 : environs/ec2: bootstrap #

Total comments: 14

Patch Set 18 : environs/ec2: bootstrap #

Total comments: 2

Patch Set 19 : environs/ec2: bootstrap #

Unified diffs Side-by-side diffs Delta from patch set Stats (+150 lines, -20 lines) Patch
A [revision details] View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +2 lines, -0 lines 0 comments Download
M environs/dummy/environs_test.go View 1 2 3 4 5 6 7 8 9 1 chunk +4 lines, -3 lines 0 comments Download
M environs/ec2/cloudinit.go View 1 2 3 4 5 6 7 8 9 10 11 12 13 3 chunks +34 lines, -8 lines 0 comments Download
M environs/ec2/cloudinit_test.go View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +5 lines, -1 line 0 comments Download
M environs/ec2/live_test.go View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +1 line, -0 lines 0 comments Download
M environs/ec2/local_test.go View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 4 chunks +7 lines, -6 lines 0 comments Download
M environs/jujutest/livetests.go View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 2 chunks +92 lines, -2 lines 0 comments Download
M environs/jujutest/test.go View 1 2 3 4 5 6 7 8 9 1 chunk +5 lines, -0 lines 0 comments Download

Messages

Total messages: 27
dfc
Please take a look.
11 years, 9 months ago (2012-07-05 01:27:17 UTC) #1
niemeyer
We've talked about this on the channel yesterday.
11 years, 8 months ago (2012-07-05 21:30:40 UTC) #2
dfc
Hello, I would like to repropose this as a WIP that allows us to have ...
11 years, 8 months ago (2012-07-06 06:28:45 UTC) #3
dfc
Please take a look.
11 years, 8 months ago (2012-07-06 06:29:26 UTC) #4
niemeyer
Almost there. https://codereview.appspot.com/6347044/diff/5003/environs/dummy/environs_test.go File environs/dummy/environs_test.go (right): https://codereview.appspot.com/6347044/diff/5003/environs/dummy/environs_test.go#newcode28 environs/dummy/environs_test.go:28: HasProvisioner: false, Isn't CanOpenState enough? https://codereview.appspot.com/6347044/diff/5003/environs/ec2/cloudinit.go File ...
11 years, 8 months ago (2012-07-06 19:44:56 UTC) #5
niemeyer
https://codereview.appspot.com/6347044/diff/5003/environs/dummy/environs_test.go File environs/dummy/environs_test.go (right): https://codereview.appspot.com/6347044/diff/5003/environs/dummy/environs_test.go#newcode28 environs/dummy/environs_test.go:28: HasProvisioner: false, On 2012/07/06 19:44:56, niemeyer wrote: > Isn't ...
11 years, 8 months ago (2012-07-06 19:45:54 UTC) #6
dfc
Please take a look. https://codereview.appspot.com/6347044/diff/5003/environs/ec2/cloudinit.go File environs/ec2/cloudinit.go (right): https://codereview.appspot.com/6347044/diff/5003/environs/ec2/cloudinit.go#newcode119 environs/ec2/cloudinit.go:119: svc := upstart.NewService("juju-provision-agent") On 2012/07/06 ...
11 years, 8 months ago (2012-07-09 00:52:57 UTC) #7
niemeyer
https://codereview.appspot.com/6347044/diff/5003/environs/ec2/cloudinit.go File environs/ec2/cloudinit.go (right): https://codereview.appspot.com/6347044/diff/5003/environs/ec2/cloudinit.go#newcode121 environs/ec2/cloudinit.go:121: // we don't have to second-guess upstart's quoting rules. ...
11 years, 8 months ago (2012-07-09 22:59:37 UTC) #8
dfc
Please take a look. https://codereview.appspot.com/6347044/diff/5003/environs/ec2/cloudinit.go File environs/ec2/cloudinit.go (right): https://codereview.appspot.com/6347044/diff/5003/environs/ec2/cloudinit.go#newcode121 environs/ec2/cloudinit.go:121: // we don't have to ...
11 years, 8 months ago (2012-07-10 00:13:19 UTC) #9
dfc
Thank you for your comments. I have split the test into two individual tests now.
11 years, 8 months ago (2012-07-10 03:19:57 UTC) #10
dfc
Please take a look.
11 years, 8 months ago (2012-07-10 03:20:37 UTC) #11
niemeyer
Thanks Dave. That's almost it. There's just some stuff that doesn't have to be done ...
11 years, 8 months ago (2012-07-10 03:43:41 UTC) #12
dfc
Please take a look. https://codereview.appspot.com/6347044/diff/21003/environs/jujutest/livetests.go File environs/jujutest/livetests.go (right): https://codereview.appspot.com/6347044/diff/21003/environs/jujutest/livetests.go#newcode98 environs/jujutest/livetests.go:98: c.Assert(err, ErrorMatches, "environment is already ...
11 years, 8 months ago (2012-07-10 07:03:58 UTC) #13
rog
looking good. it's a pity we can't actually exercise the tests. https://codereview.appspot.com/6347044/diff/5003/environs/jujutest/livetests.go File environs/jujutest/livetests.go (right): ...
11 years, 8 months ago (2012-07-10 08:07:56 UTC) #14
dfc
Thanks for your review Rog. I'd say this one is polished to a fine sheen.
11 years, 8 months ago (2012-07-10 08:24:24 UTC) #15
dfc
Please take a look. https://codereview.appspot.com/6347044/diff/7023/environs/ec2/cloudinit_test.go File environs/ec2/cloudinit_test.go (right): https://codereview.appspot.com/6347044/diff/7023/environs/ec2/cloudinit_test.go#newcode64 environs/ec2/cloudinit_test.go:64: t.checkScripts(c, `jujud provisioning --zookeeper-servers 'localhost:2181'`) ...
11 years, 8 months ago (2012-07-10 08:24:46 UTC) #16
rog
LGTM On 10 July 2012 09:24, <dave@cheney.net> wrote: > Please take a look. > > ...
11 years, 8 months ago (2012-07-10 08:40:22 UTC) #17
niemeyer
https://codereview.appspot.com/6347044/diff/5003/environs/jujutest/livetests.go File environs/jujutest/livetests.go (right): https://codereview.appspot.com/6347044/diff/5003/environs/jujutest/livetests.go#newcode94 environs/jujutest/livetests.go:94: func (t *LiveTests) testProvisioning(c *C, st *state.State) { On ...
11 years, 8 months ago (2012-07-10 13:10:16 UTC) #18
niemeyer
LGTM. Only a trivial before submitting it please: https://codereview.appspot.com/6347044/diff/14005/environs/jujutest/livetests.go File environs/jujutest/livetests.go (right): https://codereview.appspot.com/6347044/diff/14005/environs/jujutest/livetests.go#newcode100 environs/jujutest/livetests.go:100: // ...
11 years, 8 months ago (2012-07-10 13:13:46 UTC) #19
rog
On 10 July 2012 14:10, <n13m3y3r@gmail.com> wrote: > > https://codereview.appspot.com/6347044/diff/5003/environs/jujutest/livetests.go > File environs/jujutest/livetests.go (right): > ...
11 years, 8 months ago (2012-07-10 13:21:52 UTC) #20
gustavo.niemeyer
On Tue, Jul 10, 2012 at 10:21 AM, roger peppe <rogpeppe@gmail.com> wrote: > perhaps the ...
11 years, 8 months ago (2012-07-10 13:32:21 UTC) #21
rog
On 10 July 2012 14:31, Gustavo Niemeyer <gustavo.niemeyer@canonical.com> wrote: > On Tue, Jul 10, 2012 ...
11 years, 8 months ago (2012-07-10 15:26:01 UTC) #22
gustavo.niemeyer
On Tue, Jul 10, 2012 at 12:26 PM, roger peppe <rogpeppe@gmail.com> wrote: > the call ...
11 years, 8 months ago (2012-07-10 15:29:27 UTC) #23
rog
On 10 July 2012 16:29, Gustavo Niemeyer <gustavo.niemeyer@canonical.com> wrote: > On Tue, Jul 10, 2012 ...
11 years, 8 months ago (2012-07-10 16:05:42 UTC) #24
gustavo.niemeyer
On Tue, Jul 10, 2012 at 1:05 PM, roger peppe <rogpeppe@gmail.com> wrote: > I just ...
11 years, 8 months ago (2012-07-10 20:54:51 UTC) #25
dfc
Thank you for your review. Gustavo/Roger - i'll repropose patch set 12 (local ec2 PA ...
11 years, 8 months ago (2012-07-10 21:59:31 UTC) #26
dfc
11 years, 8 months ago (2012-07-10 22:13:07 UTC) #27
*** Submitted:

environs/ec2: bootstrap

This propsal is an incremental improvement towards ec2 bootstrapping.
I've taken the great work Roger did and incorporated Williams testing
changes. juju bootstrap is now able to create machine/0 with a provisioner 
running. The provisioner currently does not do anything as it has no secrets.

Once the problem of pushing the real secrets to ec2 is solved, changing the
HasProvisioner flag in live_test.go should allow this test to pass in the 
real -amazon environment.

R=niemeyer, rog, gustavo.niemeyer
CC=
https://codereview.appspot.com/6347044
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b