Implement Storage and PublicStorage for local.

Implement Storage and PublicStorage for local.

In the restructuring of the directories, I realized that we
should just have a configurable root directory for the
environment, and have other directories off that.

The local config now just has a root-dir.

The namespace takes into consideration the root user, and
will look for the SUDO_USER environment variable if set
to identify the user calling sudo.

I also renamed the storage to "storage" and "sharedStorage"
based on email conversations.

The directories are no longer created when the config is
validated as the config may be being parsed and created
when the user isn't root, especially when the default directory
is in a restricted area.

There are now two tests in the config_local.go file that are
only run when run as root.  To run these locally I had to make
sure that the GOPATH was set:
  sudo GOPATH=/home/tim/go go test -gocheck.f configRootTest

https://code.launchpad.net/~thumper/juju-core/local-provider-storage/+merge/173386

Requires: https://code.launchpad.net/~thumper/juju-core/upstart-services/+merge/173121

(do not edit description out of merge proposal)

[thumper, 2013-07-07 22:55:30]

Please take a look.

[wallyworld, 2013-07-08 02:20:11]

LGTM with some more tests as suggested

https://codereview.appspot.com/10987043/diff/1/environs/local/config.go
File environs/local/config.go (right):

https://codereview.appspot.com/10987043/diff/1/environs/local/config.go#newco...
environs/local/config.go:93: if uidStr != "" && gidStr != "" {
I think we should log a warning or even an error if uidStr or gidStr is ""

https://codereview.appspot.com/10987043/diff/1/environs/local/config_test.go
File environs/local/config_test.go (right):

https://codereview.appspot.com/10987043/diff/1/environs/local/config_test.go#...
environs/local/config_test.go:108: // Skip if not running as root.
Could we please add a skip if not running on Linux

https://codereview.appspot.com/10987043/diff/1/environs/local/config_test.go#...
environs/local/config_test.go:144: // Check that the dirs are owned by root.
typo in comment

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

https://codereview.appspot.com/10987043/diff/1/environs/local/environ_test.go...
environs/local/environ_test.go:39: c.Assert(environ.PublicStorage(), gc.NotNil)
I'd like to see some smoke tests that the Storage() and PublicStorage() can
write a file and read it back, just to test that everything is wired up
correctly within the provider.

https://codereview.appspot.com/10987043/diff/1/environs/local/environprovider.go
File environs/local/environprovider.go (right):

https://codereview.appspot.com/10987043/diff/1/environs/local/environprovider...
environs/local/environprovider.go:72: dir = filepath.Join(defaultRootDir,
localConfig.namespace())
Can we add a check that the dir exists

[thumper, 2013-07-08 04:01:01]

Please take a look.

https://codereview.appspot.com/10987043/diff/1/environs/local/config.go
File environs/local/config.go (right):

https://codereview.appspot.com/10987043/diff/1/environs/local/config.go#newco...
environs/local/config.go:93: if uidStr != "" && gidStr != "" {
On 2013/07/08 02:20:12, wallyworld wrote:
> I think we should log a warning or even an error if uidStr or gidStr is ""

It is possible that someone is actually running this as root.  It is perfectly
valid, and as such, we should allow it without warnings IMO.

https://codereview.appspot.com/10987043/diff/1/environs/local/config_test.go
File environs/local/config_test.go (right):

https://codereview.appspot.com/10987043/diff/1/environs/local/config_test.go#...
environs/local/config_test.go:108: // Skip if not running as root.
On 2013/07/08 02:20:12, wallyworld wrote:
> Could we please add a skip if not running on Linux

Done.  Although I think that if we are skipping on non-linux, there are many
other tests that need to be skipped.

https://codereview.appspot.com/10987043/diff/1/environs/local/config_test.go#...
environs/local/config_test.go:144: // Check that the dirs are owned by root.
On 2013/07/08 02:20:12, wallyworld wrote:
> typo in comment

Done.

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

https://codereview.appspot.com/10987043/diff/1/environs/local/environ_test.go...
environs/local/environ_test.go:39: c.Assert(environ.PublicStorage(), gc.NotNil)
On 2013/07/08 02:20:12, wallyworld wrote:
> I'd like to see some smoke tests that the Storage() and PublicStorage() can
> write a file and read it back, just to test that everything is wired up
> correctly within the provider.

I've done this by hooking up the environs/jujutest Tests suite,
and skipped the bootstrap and start/stop tests, leaving just the
storage test for now.

https://codereview.appspot.com/10987043/diff/1/environs/local/environprovider.go
File environs/local/environprovider.go (right):

https://codereview.appspot.com/10987043/diff/1/environs/local/environprovider...
environs/local/environprovider.go:72: dir = filepath.Join(defaultRootDir,
localConfig.namespace())
On 2013/07/08 02:20:12, wallyworld wrote:
> Can we add a check that the dir exists

No, we explicitly don't do this at this stage because it may
not exist.  This is why we check the directories when we
try to create the storage.

[dave_cheney.net, 2013-07-08 07:34:38]

LGTM.

https://codereview.appspot.com/10987043/diff/8001/environs/local/config.go
File environs/local/config.go (right):

https://codereview.appspot.com/10987043/diff/8001/environs/local/config.go#ne...
environs/local/config.go:33: user := os.Getenv("USER")
will this fail on win32 ?

https://codereview.appspot.com/10987043/diff/8001/environs/local/config_test.go
File environs/local/config_test.go (right):

https://codereview.appspot.com/10987043/diff/8001/environs/local/config_test....
environs/local/config_test.go:110: if runtime.GOOS != "linux" {
i think this is the first place we've used c.Skip. Maybe this should be done
above when calling gc.Suite(&configRootSuite{})

[mue, 2013-07-08 08:50:56]

LGTM, only one remark.

https://codereview.appspot.com/10987043/diff/8001/environs/local/config.go
File environs/local/config.go (right):

https://codereview.appspot.com/10987043/diff/8001/environs/local/config.go#ne...
environs/local/config.go:33: user := os.Getenv("USER")
On 2013/07/08 07:34:39, dfc wrote:
> will this fail on win32 ?

If I'm right this only runs on server-side, so no win32. Or am I wrong?

https://codereview.appspot.com/10987043/diff/8001/environs/local/environprovi...
File environs/local/environprovider.go (right):

https://codereview.appspot.com/10987043/diff/8001/environs/local/environprovi...
environs/local/environprovider.go:86: # Override the directory that is used for
the storage files and mongo database.
s/and mongo database/and database/

No need to mention the used technology here. We already changed it once and
fought a long time with the references.

[thumper, 2013-07-08 22:41:11]

https://codereview.appspot.com/10987043/diff/8001/environs/local/config.go
File environs/local/config.go (right):

https://codereview.appspot.com/10987043/diff/8001/environs/local/config.go#ne...
environs/local/config.go:33: user := os.Getenv("USER")
On 2013/07/08 08:50:56, mue wrote:
> On 2013/07/08 07:34:39, dfc wrote:
> > will this fail on win32 ?
> 
> If I'm right this only runs on server-side, so no win32. Or am I wrong?

It won't fail, but unlikely to ever be usable on anything except linux as it is
using LXC containers.

I'm not sure what windows uses for the user, but for this, it really isn't going
to matter.

https://codereview.appspot.com/10987043/diff/8001/environs/local/config_test.go
File environs/local/config_test.go (right):

https://codereview.appspot.com/10987043/diff/8001/environs/local/config_test....
environs/local/config_test.go:110: if runtime.GOOS != "linux" {
On 2013/07/08 07:34:39, dfc wrote:
> i think this is the first place we've used c.Skip. Maybe this should be done
> above when calling gc.Suite(&configRootSuite{})

It is certainly easier to skip the test as it is being called, rather than at
the registration phase.

https://codereview.appspot.com/10987043/diff/8001/environs/local/environprovi...
File environs/local/environprovider.go (right):

https://codereview.appspot.com/10987043/diff/8001/environs/local/environprovi...
environs/local/environprovider.go:86: # Override the directory that is used for
the storage files and mongo database.
On 2013/07/08 08:50:56, mue wrote:
> s/and mongo database/and database/
> 
> No need to mention the used technology here. We already changed it once and
> fought a long time with the references.

Done.