Code review comment for lp:~axwalk/juju-core/lp1296739-local-sudo-env

Revision history for this message
Andrew Wilkins (axwalk) wrote :

Reviewers: mp+212538_code.launchpad.net,

Message:
Please take a look.

Description:
provider/local: use "env" to propagate JUJU_HOME

Sudo may not allow environment variables to be
set directly, so we must use "env" to set JUJU_HOME
when re-executing juju as root.

Fixes lp:1296739

https://code.launchpad.net/~axwalk/juju-core/lp1296739-local-sudo-env/+merge/212538

(do not edit description out of merge proposal)

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

Affected files (+4, -1 lines):
   A [revision details]
   M provider/local/environ.go
   M provider/local/environ_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: tarmac-20140324200845-j6jqyuz0uwfvu29o
+New revision: <email address hidden>

Index: provider/local/environ.go
=== modified file 'provider/local/environ.go'
--- provider/local/environ.go 2014-03-24 04:05:24 +0000
+++ provider/local/environ.go 2014-03-25 02:46:17 +0000
@@ -406,7 +406,7 @@
     return err
    }
    args := []string{
- osenv.JujuHomeEnvKey + "=" + osenv.JujuHome(),
+ "env", osenv.JujuHomeEnvKey + "=" + osenv.JujuHome(),
     juju, "destroy-environment", "-y", "--force", env.Name(),
    }
    cmd := exec.Command("sudo", args...)

Index: provider/local/environ_test.go
=== modified file 'provider/local/environ_test.go'
--- provider/local/environ_test.go 2014-03-24 04:05:24 +0000
+++ provider/local/environ_test.go 2014-03-25 02:46:17 +0000
@@ -206,6 +206,7 @@
   c.Assert(err, gc.IsNil)
   expected := []string{
    s.fakesudo,
+ "env",
    "JUJU_HOME=" + osenv.JujuHome(),
    os.Args[0],
    "destroy-environment",

« Back to merge proposal