juju-core

Merge lp:~axwalk/juju-core/local-provider-fixes into lp:~go-bot/juju-core/trunk

  1. local-provider-fixes
  2. Merge into trunk
Proposed by Andrew Wilkins
Status: Merged
Approved by: Andrew Wilkins
Approved revision: no longer in the source branch.
Merged at revision: 2253
Proposed branch: lp:~axwalk/juju-core/local-provider-fixes
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 41 lines (+5/-4)
3 files modified
provider/local/config.go (+1/-2)
provider/local/environ.go (+3/-1)
provider/local/environprovider.go (+1/-1)
To merge this branch: bzr merge lp:~axwalk/juju-core/local-provider-fixes
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+202973@code.launchpad.net

Commit message

provider/local: fix namespace, destroy

There was an error in the namespace attribute check.
We must check that it is either "" or nil.

When Destroy() is called, it calls the CLI again with sudo.
We must pass -y to prevent juju from prompting the user.

https://codereview.appspot.com/56260044/

Description of the change

provider/local: fix namespace, destroy

There was an error in the namespace attribute check.
We must check that it is either "" or nil.

When Destroy() is called, it calls the CLI again with sudo.
We must pass -y to prevent juju from prompting the user.

https://codereview.appspot.com/56260044/

To post a comment you must log in.
Revision history for this message
Andrew Wilkins (axwalk) wrote :

Reviewers: mp+202973_code.launchpad.net,

Message:
Please take a look.

Description:
provider/local: fix namespace, destroy

There was an error in the namespace attribute check.
We must check that it is either "" or nil.

When Destroy() is called, it calls the CLI again with sudo.
We must pass -y to prevent juju from prompting the user.

https://code.launchpad.net/~axwalk/juju-core/local-provider-fixes/+merge/202973

(do not edit description out of merge proposal)

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

Affected files (+6, -2 lines):
   A [revision details]
   M provider/local/environ.go
   M provider/local/environprovider.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-20140124005805-5wkfrpv3q7zaknxv
+New revision: <email address hidden>

Index: provider/local/environ.go
=== modified file 'provider/local/environ.go'
--- provider/local/environ.go 2014-01-23 21:20:27 +0000
+++ provider/local/environ.go 2014-01-24 01:27:26 +0000
@@ -405,7 +405,9 @@
    if err != nil {
     return err
    }
- cmd := exec.Command("sudo", append([]string{juju}, os.Args[1:]...)...)
+ args := append([]string{juju}, os.Args[1:]...)
+ args = append(args, "-y")
+ cmd := exec.Command("sudo", args...)
    cmd.Stdout = os.Stdout
    cmd.Stderr = os.Stderr
    return cmd.Run()

Index: provider/local/environprovider.go
=== modified file 'provider/local/environprovider.go'
--- provider/local/environprovider.go 2014-01-23 05:45:58 +0000
+++ provider/local/environprovider.go 2014-01-24 01:27:26 +0000
@@ -47,7 +47,7 @@
   // Set the "namespace" attribute. We do this here, and not in Prepare,
   // for backwards compatibility: older versions did not store the namespace
   // in config.
- if namespace := cfg.UnknownAttrs()["namespace"]; namespace == "" {
+ if namespace, _ := cfg.UnknownAttrs()["namespace"].(string); namespace
== "" {
    var err error
    namespace = fmt.Sprintf("%s-%s", os.Getenv("USER"), cfg.Name())
    cfg, err = cfg.Apply(map[string]interface{}{"namespace": namespace})

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

Please take a look.

https://codereview.appspot.com/56260044/

Revision history for this message
Tim Penhey (thumper) wrote :

On 2014/01/24 02:04:00, axw wrote:
> Please take a look.

LGTM

https://codereview.appspot.com/56260044/

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

Please take a look.

https://codereview.appspot.com/56260044/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'provider/local/config.go'
2--- provider/local/config.go 2014-01-23 05:21:21 +0000
3+++ provider/local/config.go 2014-01-24 01:51:06 +0000
4@@ -38,8 +38,7 @@
5 "bootstrap-ip": schema.Omit,
6 "storage-port": 8040,
7 "shared-storage-port": 8041,
8- // namespace has a default of "", for backwards compatibility.
9- "namespace": "",
10+ "namespace": "",
11 }
12 )
13
14
15=== modified file 'provider/local/environ.go'
16--- provider/local/environ.go 2014-01-23 21:20:27 +0000
17+++ provider/local/environ.go 2014-01-24 01:51:06 +0000
18@@ -405,7 +405,9 @@
19 if err != nil {
20 return err
21 }
22- cmd := exec.Command("sudo", append([]string{juju}, os.Args[1:]...)...)
23+ args := append([]string{juju}, os.Args[1:]...)
24+ args = append(args, "-y")
25+ cmd := exec.Command("sudo", args...)
26 cmd.Stdout = os.Stdout
27 cmd.Stderr = os.Stderr
28 return cmd.Run()
29
30=== modified file 'provider/local/environprovider.go'
31--- provider/local/environprovider.go 2014-01-23 05:45:58 +0000
32+++ provider/local/environprovider.go 2014-01-24 01:51:06 +0000
33@@ -47,7 +47,7 @@
34 // Set the "namespace" attribute. We do this here, and not in Prepare,
35 // for backwards compatibility: older versions did not store the namespace
36 // in config.
37- if namespace := cfg.UnknownAttrs()["namespace"]; namespace == "" {
38+ if namespace, _ := cfg.UnknownAttrs()["namespace"].(string); namespace == "" {
39 var err error
40 namespace = fmt.Sprintf("%s-%s", os.Getenv("USER"), cfg.Name())
41 cfg, err = cfg.Apply(map[string]interface{}{"namespace": namespace})

Subscribers

People subscribed via source and target branches

to status/vote changes: