Code review comment for lp:~thumper/juju-core/apiserver-knows-logdir

Reviewers: mp+214155_code.launchpad.net,

Message:
Please take a look.

Description:
Have the apiserver know the logdir

The agent config now knows the logdir. In some places
the entire agent config is passed through, but in order
to keep the changes small(ish), we only pass through
the logdir (additionally).

This did require a few tweaks to tests, and in particular
the addition of LogDir to the base ConnSuite.

This logdir is used by the upcoming debug-log api.

https://code.launchpad.net/~thumper/juju-core/apiserver-knows-logdir/+merge/214155

Requires:
https://code.launchpad.net/~thumper/juju-core/tailer-tweaks/+merge/214150

(do not edit description out of merge proposal)

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

Affected files (+18, -5 lines):
   A [revision details]
   M cmd/jujud/machine.go
   M juju/testing/conn.go
   M provider/dummy/environs.go
   M state/apiserver/apiserver.go
   M state/apiserver/login_test.go
   M state/apiserver/server_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: cmd/jujud/machine.go
=== modified file 'cmd/jujud/machine.go'
--- cmd/jujud/machine.go 2014-04-02 12:52:25 +0000
+++ cmd/jujud/machine.go 2014-04-04 03:48:49 +0000
@@ -381,7 +381,9 @@
       return nil, &fatalError{"configuration does not have state server
cert/key"}
      }
      dataDir := agentConfig.DataDir()
- return apiserver.NewServer(st, fmt.Sprintf(":%d", port), cert, key,
dataDir)
+ logDir := agentConfig.LogDir()
+ return apiserver.NewServer(
+ st, fmt.Sprintf(":%d", port), cert, key, dataDir, logDir)
     })
     a.startWorkerAfterUpgrade(runner, "cleaner", func() (worker.Worker,
error) {
      return cleaner.NewCleaner(st), nil

Index: juju/testing/conn.go
=== modified file 'juju/testing/conn.go'
--- juju/testing/conn.go 2014-04-02 01:59:59 +0000
+++ juju/testing/conn.go 2014-04-04 03:48:49 +0000
@@ -60,6 +60,7 @@
   ConfigStore configstore.Storage
   BackingState *state.State // The State being used by the API server
   RootDir string // The faked-up root directory.
+ LogDir string
   oldHome string
   oldJujuHome string
   environ environs.Environ
@@ -211,6 +212,8 @@
   // sanity check we've got the correct environment.
   c.Assert(environ.Name(), gc.Equals, "dummyenv")
   s.PatchValue(&dummy.DataDir, s.DataDir())
+ s.LogDir = c.MkDir()
+ s.PatchValue(&dummy.LogDir, s.LogDir)

   versions := PreferredDefaultVersions(environ.Config(), version.Current)
   versions = append(versions, version.Current)

Index: provider/dummy/environs.go
=== modified file 'provider/dummy/environs.go'
--- provider/dummy/environs.go 2014-04-03 04:46:51 +0000
+++ provider/dummy/environs.go 2014-04-04 03:48:49 +0000
@@ -501,6 +501,7 @@

  // Override for testing - the data directory with which the state api
server is initialised.
  var DataDir = ""
+var LogDir = ""

  func (e *environ) ecfg() *environConfig {
   e.ecfgMutex.Lock()
@@ -607,7 +608,7 @@
    if err != nil {
     panic(err)
    }
- estate.apiServer, err = apiserver.NewServer(st, "localhost:0",
[]byte(testing.ServerCert), []byte(testing.ServerKey), DataDir)
+ estate.apiServer, err = apiserver.NewServer(st, "localhost:0",
[]byte(testing.ServerCert), []byte(testing.ServerKey), DataDir, LogDir)
    if err != nil {
     panic(err)
    }

Index: state/apiserver/apiserver.go
=== modified file 'state/apiserver/apiserver.go'
--- state/apiserver/apiserver.go 2014-03-13 22:47:05 +0000
+++ state/apiserver/apiserver.go 2014-04-04 03:48:49 +0000
@@ -30,12 +30,13 @@
   state *state.State
   addr net.Addr
   dataDir string
+ logDir string
  }

  // Serve serves the given state by accepting requests on the given
  // listener, using the given certificate and key (in PEM format) for
  // authentication.
-func NewServer(s *state.State, addr string, cert, key []byte, datadir
string) (*Server, error) {
+func NewServer(s *state.State, addr string, cert, key []byte, datadir,
logDir string) (*Server, error) {
   lis, err := net.Listen("tcp", addr)
   if err != nil {
    return nil, err
@@ -49,6 +50,7 @@
    state: s,
    addr: lis.Addr(),
    dataDir: datadir,
+ logDir: logDir,
   }
   // TODO(rog) check that *srvRoot is a valid type for using
   // as an RPC server.

Index: state/apiserver/login_test.go
=== modified file 'state/apiserver/login_test.go'
--- state/apiserver/login_test.go 2014-04-01 23:56:19 +0000
+++ state/apiserver/login_test.go 2014-04-04 03:48:49 +0000
@@ -54,7 +54,7 @@
    "localhost:0",
    []byte(coretesting.ServerCert),
    []byte(coretesting.ServerKey),
- "",
+ "", "",
   )
   c.Assert(err, gc.IsNil)
   info := &api.Info{

Index: state/apiserver/server_test.go
=== modified file 'state/apiserver/server_test.go'
--- state/apiserver/server_test.go 2014-03-13 07:54:56 +0000
+++ state/apiserver/server_test.go 2014-04-04 03:48:49 +0000
@@ -36,7 +36,10 @@
  func (s *serverSuite) TestStop(c *gc.C) {
   // Start our own instance of the server so we have
   // a handle on it to stop it.
- srv, err := apiserver.NewServer(s.State, "localhost:0",
[]byte(coretesting.ServerCert), []byte(coretesting.ServerKey), "")
+ srv, err := apiserver.NewServer(
+ s.State, "localhost:0",
+ []byte(coretesting.ServerCert), []byte(coretesting.ServerKey),
+ "", "")
   c.Assert(err, gc.IsNil)
   defer srv.Stop()