Merge lp://staging/~rogpeppe/juju-core/281-set-xe into lp://staging/~juju/juju-core/trunk

Reviewers: mp+158356_code.launchpad.net,

Message:
Please take a look.

Description:
environs/cloudinit: fail properly in cloudinit

If one command fails, we should not carry on and
try more. We also cause the executing commands to
be printed, so it should be more obvious which command
caused the script to fail.

We also raise the shortAttempt timeout level - I have
seen live timeouts exceeding 20s.

https://code.launchpad.net/~rogpeppe/juju-core/281-set-xe/+merge/158356

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/8661043/

Affected files:
   A [revision details]
   M environs/cloudinit/cloudinit.go
   M environs/cloudinit/cloudinit_test.go
   M environs/ec2/ec2.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision:
<email address hidden>
+New revision: <email address hidden>

Index: environs/cloudinit/cloudinit.go
=== modified file 'environs/cloudinit/cloudinit.go'
--- environs/cloudinit/cloudinit.go 2013-04-11 02:13:40 +0000
+++ environs/cloudinit/cloudinit.go 2013-04-11 12:43:17 +0000
@@ -107,6 +107,7 @@
   c.AddPackage("git")

   addScripts(c,
+ "set -xe", // ensure we run all the scripts or abort.
    fmt.Sprintf("mkdir -p %s", cfg.DataDir),
    "mkdir -p /var/log/juju")

Index: environs/cloudinit/cloudinit_test.go
=== modified file 'environs/cloudinit/cloudinit_test.go'
--- environs/cloudinit/cloudinit_test.go 2013-04-11 07:54:47 +0000
+++ environs/cloudinit/cloudinit_test.go 2013-04-11 12:44:36 +0000
@@ -73,6 +73,7 @@
    },
    setEnvConfig: true,
    expectScripts: `
+set -xe
  mkdir -p /var/lib/juju
  mkdir -p /var/log/juju
  bin='/var/lib/juju/tools/1\.2\.3-precise-amd64'
@@ -127,6 +128,7 @@
    },
    setEnvConfig: true,
    expectScripts: `
+set -xe
  mkdir -p /var/lib/juju
  mkdir -p /var/log/juju
  bin='/var/lib/juju/tools/1\.2\.3-raring-amd64'
@@ -177,6 +179,7 @@
     },
    },
    expectScripts: `
+set -xe
  mkdir -p /var/lib/juju
  mkdir -p /var/log/juju
  bin='/var/lib/juju/tools/1\.2\.3-linux-amd64'

Index: environs/ec2/ec2.go
=== modified file 'environs/ec2/ec2.go'
--- environs/ec2/ec2.go 2013-04-11 02:13:40 +0000
+++ environs/ec2/ec2.go 2013-04-11 12:43:17 +0000
@@ -34,7 +34,7 @@
  // a security group after termination). The former failure mode is
  // dealt with by shortAttempt, the latter by longAttempt.
  var shortAttempt = trivial.AttemptStrategy{
- Total: 5 * time.Second,
+ Total: 40 * time.Second,
   Delay: 200 * time.Millisecond,
  }

LGTM. What do you know - I learn something new about bash all the time
:)

https://codereview.appspot.com/8661043/

LGTM. What do you know - I learn something new about bash all the time
:)

https://codereview.appspot.com/8661043/

as agreed online, we can't have a 40s timeout on the cli if EC2 goes
down; if you're fixing live tests, let the live tests specifically be
forgiving of the errors we think they should be.

The set -ex LGTM, though.

https://codereview.appspot.com/8661043/

*** Submitted:

environs/cloudinit: fail properly in cloudinit

If one command fails, we should not carry on and
try more. We also cause the executing commands to
be printed, so it should be more obvious which command
caused the script to fail.

R=dimitern, fwereade
CC=
https://codereview.appspot.com/8661043

https://codereview.appspot.com/8661043/