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

Issue 6570063: machiner: eat provisioner and firewaller for lunch

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 6 months ago by rog
Modified:
11 years, 6 months ago
Reviewers:
mp+127005
Visibility:
Public.

Description

machiner: eat provisioner and firewaller for lunch Three in one. https://code.launchpad.net/~rogpeppe/juju-core/092-machiner-absorb-provisioner/+merge/127005 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 12

Patch Set 2 : machiner: eat provisioner and firewaller for lunch #

Unified diffs Side-by-side diffs Delta from patch set Stats (+125 lines, -220 lines) Patch
A [revision details] View 1 1 chunk +2 lines, -0 lines 0 comments Download
M cmd/jujud/machine.go View 1 5 chunks +28 lines, -8 lines 0 comments Download
M cmd/jujud/machine_test.go View 1 2 chunks +92 lines, -0 lines 0 comments Download
M cmd/jujud/main.go View 1 chunk +0 lines, -1 line 0 comments Download
M cmd/jujud/main_test.go View 2 chunks +1 line, -4 lines 0 comments Download
D cmd/jujud/provisioning.go View 1 chunk +0 lines, -76 lines 0 comments Download
D cmd/jujud/provisioning_test.go View 1 chunk +0 lines, -113 lines 0 comments Download
M cmd/jujud/unit.go View 1 2 chunks +2 lines, -2 lines 0 comments Download
M environs/cloudinit/cloudinit.go View 2 chunks +0 lines, -8 lines 0 comments Download
M environs/cloudinit/cloudinit_test.go View 4 chunks +0 lines, -7 lines 0 comments Download
M environs/ec2/ec2.go View 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 3
fwereade
LGTM
11 years, 6 months ago (2012-09-28 15:54:51 UTC) #1
niemeyer
This is great! LGTM https://codereview.appspot.com/6570063/diff/1/cmd/jujud/machine.go File cmd/jujud/machine.go (right): https://codereview.appspot.com/6570063/diff/1/cmd/jujud/machine.go#newcode96 cmd/jujud/machine.go:96: log.Printf("machine agent running tasks: %v", ...
11 years, 6 months ago (2012-09-28 15:59:38 UTC) #2
rog
11 years, 6 months ago (2012-09-28 16:10:50 UTC) #3
*** Submitted:

machiner: eat provisioner and firewaller for lunch

Three in one.

R=fwereade, niemeyer
CC=
https://codereview.appspot.com/6570063

https://codereview.appspot.com/6570063/diff/1/cmd/jujud/machine.go
File cmd/jujud/machine.go (right):

https://codereview.appspot.com/6570063/diff/1/cmd/jujud/machine.go#newcode96
cmd/jujud/machine.go:96: log.Printf("machine agent running tasks: %v",
m.Workers())
On 2012/09/28 15:59:39, niemeyer wrote:
> "requested workers for machine agent: %v"

Done.

https://codereview.appspot.com/6570063/diff/1/cmd/jujud/machine.go#newcode98
cmd/jujud/machine.go:98: for _, w := range m.Workers() {
On 2012/09/28 15:59:39, niemeyer wrote:
> Beautiful implementation, thanks.

*blush*

https://codereview.appspot.com/6570063/diff/1/cmd/jujud/machine.go#newcode109
cmd/jujud/machine.go:109: log.Printf("ignoring unknown worker task %q", w)
On 2012/09/28 15:59:39, niemeyer wrote:
> "ignoring unknown worker: %s", w

Done. and made other log messages more consistent.

https://codereview.appspot.com/6570063/diff/1/cmd/jujud/machine.go#newcode114
cmd/jujud/machine.go:114: log.Printf("final tasks: %#v", tasks)
On 2012/09/28 15:59:39, niemeyer wrote:
> d

Done.

https://codereview.appspot.com/6570063/diff/1/cmd/jujud/machine_test.go
File cmd/jujud/machine_test.go (right):

https://codereview.appspot.com/6570063/diff/1/cmd/jujud/machine_test.go#newco...
cmd/jujud/machine_test.go:127: // Check that the provisioner is alive by doing a
rudimentary check
On 2012/09/28 15:59:39, niemeyer wrote:
> provisioner and firewaller

Done.
Sign in to reply to this message.

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