Code review comment for lp:~thumper/juju-core/provisioner-reget-state-info

Revision history for this message
William Reade (fwereade) wrote :

Strongly -1 on the clever bits here. Just barf on failure and I'll be
happy :).

https://codereview.appspot.com/9824047/diff/1/worker/provisioner/provisioner_task.go
File worker/provisioner/provisioner_task.go (right):

https://codereview.appspot.com/9824047/diff/1/worker/provisioner/provisioner_task.go#newcode304
worker/provisioner/provisioner_task.go:304: // Don't return a real
error, just try again next time.
This is problematic, because the watcher won't report that machine again
until it changes -- which it won't. For this to work, we need the
provisioner to be restarted regularly... but I'm hoping for a
provisioner that can run for weeks without breaking sweat.

https://codereview.appspot.com/9824047/diff/1/worker/provisioner/provisioner_task.go#newcode360
worker/provisioner/provisioner_task.go:360: // info if config becomes
invalid.
Honestly, I think this is sufficient justification to nuke the whole
provisioner; it'll be restarted in a few seconds, and for now I'd rather
not even try to provision a machine (which can be expected to cost our
users actual money) unless I'm as sure as I can be that I'll be able to
start a functioning machine agent...

https://codereview.appspot.com/9824047/

« Back to merge proposal