Code review comment for lp://staging/~rogpeppe/juju-core/281-set-xe

Revision history for this message
Roger Peppe (rogpeppe) wrote :

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,
  }

« Back to merge proposal