Code review comment for lp://staging/~gz/juju-core/revert_r1188

Revision history for this message
Martin Packman (gz) wrote :

Reviewers: mp+160208_code.launchpad.net,

Message:
Please take a look.

Description:
Undo change to cloudinit update logic on raring

See the merge proposal that added this workaround for more details:

<https://codereview.appspot.com/8648047/>

In short smoser has doubts about the fix, and it is unlikely to be an
issue once raring has been released. There will be no upstart upgrades
without first fixing the underlying problem.

https://code.launchpad.net/~gz/juju-core/revert_r1188/+merge/160208

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M environs/cloudinit/cloudinit.go
   M environs/cloudinit/cloudinit_test.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-19 16:55:38 +0000
+++ environs/cloudinit/cloudinit.go 2013-04-22 20:34:59 +0000
@@ -106,12 +106,6 @@
   if err := verifyConfig(cfg); err != nil {
    return nil, err
   }
- // TODO(dimitern) this is needed for raring, due to LP bug #1103881
- if cfg.Tools.Series == "raring" {
- addScripts(c, "apt-get upgrade -y")
- } else {
- c.SetAptUpgrade(true)
- }
   c.AddSSHAuthorizedKeys(cfg.AuthorizedKeys)
   c.AddPackage("git")

@@ -184,9 +178,9 @@
   }

   // general options
+ c.SetAptUpgrade(true)
   c.SetAptUpdate(true)
   c.SetOutput(cloudinit.OutAll, "| tee -a
/var/log/cloud-init-output.log", "")
-
   return c, nil
  }

Index: environs/cloudinit/cloudinit_test.go
=== modified file 'environs/cloudinit/cloudinit_test.go'
--- environs/cloudinit/cloudinit_test.go 2013-04-19 15:46:34 +0000
+++ environs/cloudinit/cloudinit_test.go 2013-04-22 20:34:59 +0000
@@ -129,7 +129,6 @@
    },
    setEnvConfig: true,
    expectScripts: `
-apt-get upgrade -y
  set -xe
  mkdir -p /var/lib/juju
  mkdir -p /var/log/juju
@@ -249,10 +248,7 @@
    err = goyaml.Unmarshal(data, &x)
    c.Assert(err, IsNil)

- // TODO(dimitern) raring does apt-get upgrade differently, due to LP bug
#1103881

- if test.cfg.Tools.Series != "raring" {
- c.Check(x["apt_upgrade"], Equals, true)
- }
+ c.Check(x["apt_upgrade"], Equals, true)
    c.Check(x["apt_update"], Equals, true)

    scripts := getScripts(x)

« Back to merge proposal