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

Issue 6347044: environs/ec2: bootstrap (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years, 1 month ago by dfc
Modified:
7 years, 1 month 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.
7 years, 1 month ago (2012-07-05 01:27:17 UTC) #1
niemeyer
We've talked about this on the channel yesterday.
7 years, 1 month 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 ...
7 years, 1 month ago (2012-07-06 06:28:45 UTC) #3
dfc
Please take a look.
7 years, 1 month 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 ...
7 years, 1 month 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 ...
7 years, 1 month 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 ...
7 years, 1 month 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. ...
7 years, 1 month 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 ...
7 years, 1 month 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.
7 years, 1 month ago (2012-07-10 03:19:57 UTC) #10
dfc
Please take a look.
7 years, 1 month 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 ...
7 years, 1 month 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 ...
7 years, 1 month 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): ...
7 years, 1 month 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.
7 years, 1 month 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'`) ...
7 years, 1 month 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. > > ...
7 years, 1 month 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 ...
7 years, 1 month 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: // ...
7 years, 1 month 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): > ...
7 years, 1 month 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 ...
7 years, 1 month 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 ...
7 years, 1 month 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 ...
7 years, 1 month 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 ...
7 years, 1 month 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 ...
7 years, 1 month 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 ...
7 years, 1 month ago (2012-07-10 21:59:31 UTC) #26
dfc
7 years, 1 month 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