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

Issue 8786043: provisioner: handle machine life

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 11 months ago by fwereade
Modified:
10 years, 11 months ago
Reviewers:
mp+159045, thumper, TheMue
Visibility:
Public.

Description

provisioner: handle machine life * Dead machines are removed. * Dying machines that have not already been provisioned are removed. As part of this, it turned out that the LifecycleWatcher was not following its supposed behaviour (of reporting Dead machines, just once, in the first event, lest their responsible entity entirely lose track of them), so I fixed that... and noted to my disgust that those awful, awful tests didn't even cover the case I changed. So I killed them all, and it felt Good. Then I tidied up the provisioner tests a bit and, before you know it, that's 1000 lines of diff :/. https://code.launchpad.net/~fwereade/juju-core/provisioner-remove-machines/+merge/159045 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 7

Patch Set 2 : provisioner: handle machine life #

Unified diffs Side-by-side diffs Delta from patch set Stats (+326 lines, -487 lines) Patch
A [revision details] View 1 1 chunk +2 lines, -0 lines 0 comments Download
M state/state_test.go View 1 chunk +157 lines, -336 lines 0 comments Download
M state/watcher.go View 1 chunk +1 line, -1 line 0 comments Download
M worker/provisioner/provisioner.go View 7 chunks +26 lines, -21 lines 0 comments Download
M worker/provisioner/provisioner_test.go View 11 chunks +140 lines, -129 lines 0 comments Download

Messages

Total messages: 6
fwereade
Please take a look.
10 years, 11 months ago (2013-04-16 00:33:16 UTC) #1
thumper
LGTM - only real comment was the inline interface in the stop function. https://codereview.appspot.com/8786043/diff/1/state/state_test.go File ...
10 years, 11 months ago (2013-04-16 01:07:45 UTC) #2
TheMue
LGTM, like it. Only minor comments. https://codereview.appspot.com/8786043/diff/1/state/state_test.go File state/state_test.go (right): https://codereview.appspot.com/8786043/diff/1/state/state_test.go#newcode641 state/state_test.go:641: case <-time.After(50 * ...
10 years, 11 months ago (2013-04-16 10:06:39 UTC) #3
fwereade
*** Submitted: provisioner: handle machine life * Dead machines are removed. * Dying machines that ...
10 years, 11 months ago (2013-04-16 12:31:04 UTC) #4
fwereade
On 2013/04/16 12:31:04, fwereade wrote: > *** Submitted: > > provisioner: handle machine life > ...
10 years, 11 months ago (2013-04-16 12:32:09 UTC) #5
fwereade
10 years, 11 months ago (2013-04-16 13:44:50 UTC) #6
On 2013/04/16 12:32:09, fwereade wrote:
> Hell, sorry, I got tangled in my branches, thought I was doing something else.
> Expect an imminent followup to address review comments...

Followup at https://codereview.appspot.com/8688044
Sign in to reply to this message.

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