cmd/jujud: delay stop for possible upgrades

cmd/jujud: delay stop for possible upgrades

In particular, we need to be careful that
an agent which consistently fails early does
not prevent upgrades from happening.

https://code.launchpad.net/~rogpeppe/juju-core/057-upgrader-delay-stop/+merge/124488

(do not edit description out of merge proposal)

[rog, 2012-09-14 17:52:22]

Please take a look.

[niemeyer, 2012-09-17 10:15:03]

LGTM

https://codereview.appspot.com/6492123/diff/2001/cmd/jujud/upgrade.go
File cmd/jujud/upgrade.go (right):

https://codereview.appspot.com/6492123/diff/2001/cmd/jujud/upgrade.go#newcode16
cmd/jujud/upgrade.go:16: var upgraderDelay = 5 * time.Minute
upgraderKillDelay?

https://codereview.appspot.com/6492123/diff/2001/cmd/jujud/upgrade.go#newcode96
cmd/jujud/upgrade.go:96: // first able.
// Rather than using worker.WaitForEnviron, invalid environments are
// managed explicitly so that all configuration changes are observed
// by the loop below.

https://codereview.appspot.com/6492123/diff/2001/cmd/jujud/upgrade.go#newcode211
cmd/jujud/upgrade.go:211: return fmt.Errorf("download aborted of %q",
downloadTools.URL)
"upgrader: aborted download of %q"

https://codereview.appspot.com/6492123/diff/2001/environs/dummy/environs.go
File environs/dummy/environs.go (right):

https://codereview.appspot.com/6492123/diff/2001/environs/dummy/environs.go#n...
environs/dummy/environs.go:239: // SetStorageDelay causes any Get or http
download
s/Get or http download operation from the storage/storage download operation/

https://codereview.appspot.com/6492123/diff/2001/environs/dummy/environs.go#n...
environs/dummy/environs.go:242: // The delay happens before the operation is
started;
I don't think this has to be explained. It's more confusing to read it here and
ponder about what it means than simply assume that it does what you think it
does.

https://codereview.appspot.com/6492123/diff/2001/worker/firewaller/firewaller.go
File worker/firewaller/firewaller.go (right):

https://codereview.appspot.com/6492123/diff/2001/worker/firewaller/firewaller...
worker/firewaller/firewaller.go:236: return "firewaller worker"
Consistency is very poor here:

    "provisioning worker"
    "firewaller worker"
    "upgrader"
    "machine %d worker"
    "uniter worker for %s"



We only really need two variants, at most:

    "fooer"
    "fooer for %v"

[rog, 2012-09-17 11:24:21]

*** Submitted:

cmd/jujud: delay stop for possible upgrades

In particular, we need to be careful that
an agent which consistently fails early does
not prevent upgrades from happening.

R=niemeyer
CC=
https://codereview.appspot.com/6492123

https://codereview.appspot.com/6492123/diff/2001/cmd/jujud/upgrade.go
File cmd/jujud/upgrade.go (right):

https://codereview.appspot.com/6492123/diff/2001/cmd/jujud/upgrade.go#newcode16
cmd/jujud/upgrade.go:16: var upgraderDelay = 5 * time.Minute
On 2012/09/17 10:15:03, niemeyer wrote:
> upgraderKillDelay?

Done.

https://codereview.appspot.com/6492123/diff/2001/cmd/jujud/upgrade.go#newcode96
cmd/jujud/upgrade.go:96: // first able.
On 2012/09/17 10:15:03, niemeyer wrote:
> // Rather than using worker.WaitForEnviron, invalid environments are
> // managed explicitly so that all configuration changes are observed
> // by the loop below.

Done.

https://codereview.appspot.com/6492123/diff/2001/cmd/jujud/upgrade.go#newcode211
cmd/jujud/upgrade.go:211: return fmt.Errorf("download aborted of %q",
downloadTools.URL)
On 2012/09/17 10:15:03, niemeyer wrote:
> "upgrader: aborted download of %q"

Done.

https://codereview.appspot.com/6492123/diff/2001/environs/dummy/environs.go
File environs/dummy/environs.go (right):

https://codereview.appspot.com/6492123/diff/2001/environs/dummy/environs.go#n...
environs/dummy/environs.go:239: // SetStorageDelay causes any Get or http
download
On 2012/09/17 10:15:03, niemeyer wrote:
> s/Get or http download operation from the storage/storage download operation/

Done.

https://codereview.appspot.com/6492123/diff/2001/environs/dummy/environs.go#n...
environs/dummy/environs.go:242: // The delay happens before the operation is
started;
On 2012/09/17 10:15:03, niemeyer wrote:
> I don't think this has to be explained. It's more confusing to read it here
and
> ponder about what it means than simply assume that it does what you think it
> does.

Done.