Merge into trunk : local-default-root-dir : Code : juju-core

Status:	Merged
Approved by:	Tim Penhey on 2013-07-16
Approved revision:	no longer in the source branch.
Merged at revision:	1466
Proposed branch:	lp:~thumper/juju-core/local-default-root-dir
Merge into:	lp:~go-bot/juju-core/trunk
Prerequisite:	lp:~thumper/juju-core/local-sudo-caller
Diff against target:	152 lines (+19/-22) 5 files modified environs/local/config_test.go (+2/-2) environs/local/environ.go (+5/-0) environs/local/environ_test.go (+6/-13) environs/local/environprovider.go (+3/-3) environs/local/environprovider_test.go (+3/-4)
To merge this branch:	bzr merge lp:~thumper/juju-core/local-default-root-dir
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Juju Engineering		2013-07-16	Pending
Review via email: mp+174908@code.launchpad.net

Commit message

Change the default root-dir to be in $HOME

Instead of having the root-dir in /usr/lib/juju, make it
in ~/.juju (well, $JUJU_HOME) by default. The root dir
is now the environment name under the ~/.juju dir.

https://codereview.appspot.com/11319044/

Description of the change

Change the default root-dir to be in $HOME

Instead of having the root-dir in /usr/lib/juju, make it
in ~/.juju (well, $JUJU_HOME) by default. The root dir
is now the environment name under the ~/.juju dir.

https://codereview.appspot.com/11319044/

Revision history for this message

Tim Penhey (thumper) wrote on 2013-07-16:

#

Download full text (5.4 KiB)

Reviewers: mp+174908_code.launchpad.net,

Message:
Please take a look.

Description:
Change the default root-dir to be in $HOME

Instead of having the root-dir in /usr/lib/juju, make it
in ~/.juju (well, $JUJU_HOME) by default. The root dir
is now the environment name under the ~/.juju dir.

https://code.launchpad.net/~thumper/juju-core/local-default-root-dir/+merge/174908

Requires:
https://code.launchpad.net/~thumper/juju-core/local-sudo-caller/+merge/174904

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M environs/local/config_test.go
   M environs/local/environ.go
   M environs/local/environ_test.go
   M environs/local/environprovider.go
   M environs/local/environprovider_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/local/config_test.go
=== modified file 'environs/local/config_test.go'
--- environs/local/config_test.go 2013-07-16 02:54:50 +0000
+++ environs/local/config_test.go 2013-07-16 03:23:32 +0000
@@ -59,7 +59,7 @@
valid, err := local.Provider.Validate(testConfig, nil)
c.Assert(err, gc.IsNil)

- expectedRootDir := filepath.Join(environs.DataDir, "tester-test")
+ expectedRootDir := filepath.Join(os.Getenv("HOME"), ".juju", "test")
   unknownAttrs := valid.UnknownAttrs()
   c.Assert(unknownAttrs["root-dir"], gc.Equals, expectedRootDir)
  }

Index: environs/local/environ.go
=== modified file 'environs/local/environ.go'
--- environs/local/environ.go 2013-07-05 03:50:29 +0000
+++ environs/local/environ.go 2013-07-12 01:53:18 +0000
@@ -75,6 +75,11 @@
   defer env.localMutex.Unlock()
   env.config = config
   env.name = config.Name()
+
+ if err := config.createDirs(); err != nil {
+ return err
+ }
+
   sharedStorageListener, err :=
createLocalStorageListener(config.sharedStorageDir())
   if err != nil {
    return err

Index: environs/local/environ_test.go
=== modified file 'environs/local/environ_test.go'
--- environs/local/environ_test.go 2013-07-15 02:58:47 +0000
+++ environs/local/environ_test.go 2013-07-16 03:23:32 +0000
@@ -9,7 +9,6 @@
   "launchpad.net/juju-core/environs"
   "launchpad.net/juju-core/environs/jujutest"
   "launchpad.net/juju-core/environs/local"
- jc "launchpad.net/juju-core/testing/checkers"
  )

type environSuite struct {
@@ -20,20 +19,18 @@

func (*environSuite) TestOpenFailsWithoutDirs(c *gc.C) {
testConfig := minimalConfig(c)
+ testConfig, err := testConfig.Apply(map[string]interface{}{
+ "root-dir": "/usr/lib/juju",
+ })
+ c.Assert(err, gc.IsNil)

   environ, err := local.Provider.Open(testConfig)
- c.Assert(err, gc.ErrorMatches, "storage directory .* does not exist,
bootstrap first")
+ c.Assert(err, gc.ErrorMatches, "mkdir .* permission denied")
   c.Assert(environ, gc.IsNil)
  }

func (s *environSuite) TestNameAndStorage(c *gc.C) {
- c.Logf("root: %s", environs.DataDir)
- c.Asser...

Reviewers: mp+174908_code.launchpad.net,

Message:
Please take a look.

Description:
Change the default root-dir to be in $HOME

Instead of having the root-dir in /usr/lib/juju, make it
in ~/.juju (well, $JUJU_HOME) by default.  The root dir
is now the environment name under the ~/.juju dir.

https://code.launchpad.net/~thumper/juju-core/local-default-root-dir/+merge/174908

Requires:
https://code.launchpad.net/~thumper/juju-core/local-sudo-caller/+merge/174904

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M environs/local/config_test.go
   M environs/local/environ.go
   M environs/local/environ_test.go
   M environs/local/environprovider.go
   M environs/local/environprovider_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: tim.penhey@canonical.com-20130716031308-p4wy7pcwdgufgcbx
+New revision: tim.penhey@canonical.com-20130716032332-cz0okxylj35ia4dd

Index: environs/local/config_test.go
=== modified file 'environs/local/config_test.go'
--- environs/local/config_test.go	2013-07-16 02:54:50 +0000
+++ environs/local/config_test.go	2013-07-16 03:23:32 +0000
@@ -59,7 +59,7 @@
  	valid, err := local.Provider.Validate(testConfig, nil)
  	c.Assert(err, gc.IsNil)

-	expectedRootDir := filepath.Join(environs.DataDir, "tester-test")
+	expectedRootDir := filepath.Join(os.Getenv("HOME"), ".juju", "test")
  	unknownAttrs := valid.UnknownAttrs()
  	c.Assert(unknownAttrs["root-dir"], gc.Equals, expectedRootDir)
  }

Index: environs/local/environ.go
=== modified file 'environs/local/environ.go'
--- environs/local/environ.go	2013-07-05 03:50:29 +0000
+++ environs/local/environ.go	2013-07-12 01:53:18 +0000
@@ -75,6 +75,11 @@
  	defer env.localMutex.Unlock()
  	env.config = config
  	env.name = config.Name()
+
+	if err := config.createDirs(); err != nil {
+		return err
+	}
+
  	sharedStorageListener, err :=  
createLocalStorageListener(config.sharedStorageDir())
  	if err != nil {
  		return err

Index: environs/local/environ_test.go
=== modified file 'environs/local/environ_test.go'
--- environs/local/environ_test.go	2013-07-15 02:58:47 +0000
+++ environs/local/environ_test.go	2013-07-16 03:23:32 +0000
@@ -9,7 +9,6 @@
  	"launchpad.net/juju-core/environs"
  	"launchpad.net/juju-core/environs/jujutest"
  	"launchpad.net/juju-core/environs/local"
-	jc "launchpad.net/juju-core/testing/checkers"
  )

type environSuite struct {
@@ -20,20 +19,18 @@

func (*environSuite) TestOpenFailsWithoutDirs(c *gc.C) {
  	testConfig := minimalConfig(c)
+	testConfig, err := testConfig.Apply(map[string]interface{}{
+		"root-dir": "/usr/lib/juju",
+	})
+	c.Assert(err, gc.IsNil)

environ, err := local.Provider.Open(testConfig)
-	c.Assert(err, gc.ErrorMatches, "storage directory .* does not exist,  
bootstrap first")
+	c.Assert(err, gc.ErrorMatches, "mkdir .* permission denied")
  	c.Assert(environ, gc.IsNil)
  }

func (s *environSuite) TestNameAndStorage(c *gc.C) {
-	c.Logf("root: %s", environs.DataDir)
-	c.Assert(environs.DataDir, jc.IsDirectory)
-
  	testConfig := minimalConfig(c)
-	err := local.CreateDirs(c, testConfig)
-	c.Assert(err, gc.IsNil)
-
  	environ, err := local.Provider.Open(testConfig)
  	c.Assert(err, gc.IsNil)
  	c.Assert(environ.Name(), gc.Equals, "test")
@@ -48,9 +45,6 @@

func (s *localJujuTestSuite) SetUpTest(c *gc.C) {
  	s.baseProviderSuite.SetUpTest(c)
-	// Construct the directories first.
-	err := local.CreateDirs(c, minimalConfig(c))
-	c.Assert(err, gc.IsNil)
  	s.Tests.SetUpTest(c)
  }

Index: environs/local/environprovider.go
=== modified file 'environs/local/environprovider.go'
--- environs/local/environprovider.go	2013-07-12 09:58:12 +0000
+++ environs/local/environprovider.go	2013-07-16 03:23:32 +0000
@@ -5,7 +5,6 @@

import (
  	"fmt"
-	"path/filepath"

"launchpad.net/loggo"

@@ -65,7 +64,7 @@
  	}
  	dir := utils.NormalizePath(localConfig.rootDir())
  	if dir == "." {
-		dir = filepath.Join(environs.DataDir, localConfig.namespace())
+		dir = config.JujuHomePath(cfg.Name())
  		localConfig.attrs["root-dir"] = dir
  	}

@@ -80,7 +79,8 @@
  local:
    type: local
    # Override the directory that is used for the storage files and database.
-  # The default location is /var/lib/juju/<USER>-<ENV>
+  # The default location is $JUJU_HOME/<ENV>.
+  # $JUJU_HOME defaults to ~/.juju
    # root-dir: ~/.juju/local

`[1:]

Index: environs/local/environprovider_test.go
=== modified file 'environs/local/environprovider_test.go'
--- environs/local/environprovider_test.go	2013-07-15 02:58:47 +0000
+++ environs/local/environprovider_test.go	2013-07-16 03:23:32 +0000
@@ -7,22 +7,21 @@
  	gc "launchpad.net/gocheck"
  	"launchpad.net/loggo"

-	envtesting "launchpad.net/juju-core/environs/testing"
  	"launchpad.net/juju-core/testing"
  )

type baseProviderSuite struct {
  	testing.LoggingSuite
-	restoreDataDir func()
+	home *testing.FakeHome
  }

func (s *baseProviderSuite) SetUpTest(c *gc.C) {
  	s.LoggingSuite.SetUpTest(c)
+	s.home = testing.MakeFakeHomeNoEnvironments(c, "test")
  	loggo.GetLogger("juju.environs.local").SetLogLevel(loggo.TRACE)
-	s.restoreDataDir = envtesting.PatchDataDir(c.MkDir())
  }

func (s *baseProviderSuite) TearDownTest(c *gc.C) {
-	s.restoreDataDir()
+	s.home.Restore()
  	s.LoggingSuite.TearDownTest(c)
  }

Revision history for this message

Jeroen T. Vermeulen (jtv) wrote on 2013-07-16:

#

LGTM but for one thing that worries me: is this about /usr/lib/juju or
about /var/lib/juju?

https://codereview.appspot.com/11319044/diff/1/environs/local/environ_test.go
File environs/local/environ_test.go (left):

https://codereview.appspot.com/11319044/diff/1/environs/local/environ_test.go#oldcode21
environs/local/environ_test.go:21: func (*environSuite)
TestOpenFailsWithoutDirs(c *gc.C) {

This test's name may need changing now.

https://codereview.appspot.com/11319044/diff/1/environs/local/environprovider_test.go
File environs/local/environprovider_test.go (left):

https://codereview.appspot.com/11319044/diff/1/environs/local/environprovider_test.go#oldcode16
environs/local/environprovider_test.go:16: restoreDataDir func()

You're removing the overriding of DataDir here — but be aware that that
is *not* the old /usr/lib/juju that you're replacing here. This one is
about /var/lib/juju. So you may still need both.

https://codereview.appspot.com/11319044/

Revision history for this message

William Reade (fwereade) wrote on 2013-07-16:

#

LGTM

https://codereview.appspot.com/11319044/

Revision history for this message

Tim Penhey (thumper) wrote on 2013-07-16:

#

https://codereview.appspot.com/11319044/diff/1/environs/local/environ_test.go
File environs/local/environ_test.go (left):

https://codereview.appspot.com/11319044/diff/1/environs/local/environ_test.go#oldcode21
environs/local/environ_test.go:21: func (*environSuite)
TestOpenFailsWithoutDirs(c *gc.C) {
On 2013/07/16 04:13:58, jtv.canonical wrote:

> This test's name may need changing now.

Done.

https://codereview.appspot.com/11319044/diff/1/environs/local/environprovider_test.go
File environs/local/environprovider_test.go (left):

https://codereview.appspot.com/11319044/diff/1/environs/local/environprovider_test.go#oldcode16
environs/local/environprovider_test.go:16: restoreDataDir func()
On 2013/07/16 04:13:58, jtv.canonical wrote:

> You're removing the overriding of DataDir here — but be aware that
that is *not*
> the old /usr/lib/juju that you're replacing here. This one is about
> /var/lib/juju. So you may still need both.

Sorry about the confusing directory name. I should have said
/var/lib/juju, and we don't actually use that now.

https://codereview.appspot.com/11319044/

juju-core

Merge lp:~thumper/juju-core/local-default-root-dir into lp:~go-bot/juju-core/trunk

Commit message

Description of the change

Preview Diff

Subscribers

1	=== modified file 'environs/local/config_test.go'
2	--- environs/local/config_test.go 2013-07-16 22:03:30 +0000
3	+++ environs/local/config_test.go 2013-07-16 22:03:30 +0000
4	@@ -10,7 +10,7 @@
5	"syscall"
6
7	gc "launchpad.net/gocheck"
8	- "launchpad.net/juju-core/environs"
9	+
10	"launchpad.net/juju-core/environs/config"
11	"launchpad.net/juju-core/environs/local"
12	"launchpad.net/juju-core/testing"
13	@@ -59,7 +59,7 @@
14	valid, err := local.Provider.Validate(testConfig, nil)
15	c.Assert(err, gc.IsNil)
16
17	- expectedRootDir := filepath.Join(environs.DataDir, "tester-test")
18	+ expectedRootDir := filepath.Join(os.Getenv("HOME"), ".juju", "test")
19	unknownAttrs := valid.UnknownAttrs()
20	c.Assert(unknownAttrs["root-dir"], gc.Equals, expectedRootDir)
21	}
22
23	=== modified file 'environs/local/environ.go'
24	--- environs/local/environ.go 2013-07-05 03:50:29 +0000
25	+++ environs/local/environ.go 2013-07-16 22:03:30 +0000
26	@@ -75,6 +75,11 @@
27	defer env.localMutex.Unlock()
28	env.config = config
29	env.name = config.Name()
30	+
31	+ if err := config.createDirs(); err != nil {
32	+ return err
33	+ }
34	+
35	sharedStorageListener, err := createLocalStorageListener(config.sharedStorageDir())
36	if err != nil {
37	return err
38
39	=== modified file 'environs/local/environ_test.go'
40	--- environs/local/environ_test.go 2013-07-15 02:58:47 +0000
41	+++ environs/local/environ_test.go 2013-07-16 22:03:30 +0000
42	@@ -6,10 +6,8 @@
43	import (
44	gc "launchpad.net/gocheck"
45
46	- "launchpad.net/juju-core/environs"
47	"launchpad.net/juju-core/environs/jujutest"
48	"launchpad.net/juju-core/environs/local"
49	- jc "launchpad.net/juju-core/testing/checkers"
50	)
51
52	type environSuite struct {
53	@@ -18,22 +16,20 @@
54
55	var _ = gc.Suite(&environSuite{})
56
57	-func (environSuite) TestOpenFailsWithoutDirs(c gc.C) {
58	+func (environSuite) TestOpenFailsWithProtectedDirectories(c gc.C) {
59	testConfig := minimalConfig(c)
60	+ testConfig, err := testConfig.Apply(map[string]interface{}{
61	+ "root-dir": "/usr/lib/juju",
62	+ })
63	+ c.Assert(err, gc.IsNil)
64
65	environ, err := local.Provider.Open(testConfig)
66	- c.Assert(err, gc.ErrorMatches, "storage directory .* does not exist, bootstrap first")
67	+ c.Assert(err, gc.ErrorMatches, "mkdir .* permission denied")
68	c.Assert(environ, gc.IsNil)
69	}
70
71	func (s environSuite) TestNameAndStorage(c gc.C) {
72	- c.Logf("root: %s", environs.DataDir)
73	- c.Assert(environs.DataDir, jc.IsDirectory)
74	-
75	testConfig := minimalConfig(c)
76	- err := local.CreateDirs(c, testConfig)
77	- c.Assert(err, gc.IsNil)
78	-
79	environ, err := local.Provider.Open(testConfig)
80	c.Assert(err, gc.IsNil)
81	c.Assert(environ.Name(), gc.Equals, "test")
82	@@ -48,9 +44,6 @@
83
84	func (s localJujuTestSuite) SetUpTest(c gc.C) {
85	s.baseProviderSuite.SetUpTest(c)
86	- // Construct the directories first.
87	- err := local.CreateDirs(c, minimalConfig(c))
88	- c.Assert(err, gc.IsNil)
89	s.Tests.SetUpTest(c)
90	}
91
92
93	=== modified file 'environs/local/environprovider.go'
94	--- environs/local/environprovider.go 2013-07-12 09:58:12 +0000
95	+++ environs/local/environprovider.go 2013-07-16 22:03:30 +0000
96	@@ -5,7 +5,6 @@
97
98	import (
99	"fmt"
100	- "path/filepath"
101
102	"launchpad.net/loggo"
103
104	@@ -65,7 +64,7 @@
105	}
106	dir := utils.NormalizePath(localConfig.rootDir())
107	if dir == "." {
108	- dir = filepath.Join(environs.DataDir, localConfig.namespace())
109	+ dir = config.JujuHomePath(cfg.Name())
110	localConfig.attrs["root-dir"] = dir
111	}
112
113	@@ -80,7 +79,8 @@
114	local:
115	type: local
116	# Override the directory that is used for the storage files and database.
117	- # The default location is /var/lib/juju/<USER>-<ENV>
118	+ # The default location is $JUJU_HOME/<ENV>.
119	+ # $JUJU_HOME defaults to ~/.juju
120	# root-dir: ~/.juju/local
121
122	`[1:]
123
124	=== modified file 'environs/local/environprovider_test.go'
125	--- environs/local/environprovider_test.go 2013-07-15 02:58:47 +0000
126	+++ environs/local/environprovider_test.go 2013-07-16 22:03:30 +0000
127	@@ -7,22 +7,21 @@
128	gc "launchpad.net/gocheck"
129	"launchpad.net/loggo"
130
131	- envtesting "launchpad.net/juju-core/environs/testing"
132	"launchpad.net/juju-core/testing"
133	)
134
135	type baseProviderSuite struct {
136	testing.LoggingSuite
137	- restoreDataDir func()
138	+ home *testing.FakeHome
139	}
140
141	func (s baseProviderSuite) SetUpTest(c gc.C) {
142	s.LoggingSuite.SetUpTest(c)
143	+ s.home = testing.MakeFakeHomeNoEnvironments(c, "test")
144	loggo.GetLogger("juju.environs.local").SetLogLevel(loggo.TRACE)
145	- s.restoreDataDir = envtesting.PatchDataDir(c.MkDir())
146	}
147
148	func (s baseProviderSuite) TearDownTest(c gc.C) {
149	- s.restoreDataDir()
150	+ s.home.Restore()
151	s.LoggingSuite.TearDownTest(c)
152	}