Merge lp:~thumper/juju-core/apiserver-knows-logdir into lp:~go-bot/juju-core/trunk

Proposed by Tim Penhey
Status: Merged
Approved by: Tim Penhey
Approved revision: no longer in the source branch.
Merged at revision: 2564
Proposed branch: lp:~thumper/juju-core/apiserver-knows-logdir
Merge into: lp:~go-bot/juju-core/trunk
Prerequisite: lp:~thumper/juju-core/tailer-tweaks
Diff against target: 111 lines (+16/-5)
6 files modified
cmd/jujud/machine.go (+3/-1)
juju/testing/conn.go (+3/-0)
provider/dummy/environs.go (+2/-1)
state/apiserver/apiserver.go (+3/-1)
state/apiserver/login_test.go (+1/-1)
state/apiserver/server_test.go (+4/-1)
To merge this branch: bzr merge lp:~thumper/juju-core/apiserver-knows-logdir
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+214155@code.launchpad.net

Commit message

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://codereview.appspot.com/84350043/

Description of the change

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://codereview.appspot.com/84350043/

To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :
Download full text (5.5 KiB)

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() *environCo...

Read more...

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

On 2014/04/04 04:47:42, thumper wrote:
> Please take a look.

LGTM

https://codereview.appspot.com/84350043/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'cmd/jujud/machine.go'
--- cmd/jujud/machine.go 2014-04-02 12:52:25 +0000
+++ cmd/jujud/machine.go 2014-04-04 04:45:42 +0000
@@ -381,7 +381,9 @@
381 return nil, &fatalError{"configuration does not have state server cert/key"}381 return nil, &fatalError{"configuration does not have state server cert/key"}
382 }382 }
383 dataDir := agentConfig.DataDir()383 dataDir := agentConfig.DataDir()
384 return apiserver.NewServer(st, fmt.Sprintf(":%d", port), cert, key, dataDir)384 logDir := agentConfig.LogDir()
385 return apiserver.NewServer(
386 st, fmt.Sprintf(":%d", port), cert, key, dataDir, logDir)
385 })387 })
386 a.startWorkerAfterUpgrade(runner, "cleaner", func() (worker.Worker, error) {388 a.startWorkerAfterUpgrade(runner, "cleaner", func() (worker.Worker, error) {
387 return cleaner.NewCleaner(st), nil389 return cleaner.NewCleaner(st), nil
388390
=== modified file 'juju/testing/conn.go'
--- juju/testing/conn.go 2014-04-02 01:59:59 +0000
+++ juju/testing/conn.go 2014-04-04 04:45:42 +0000
@@ -60,6 +60,7 @@
60 ConfigStore configstore.Storage60 ConfigStore configstore.Storage
61 BackingState *state.State // The State being used by the API server61 BackingState *state.State // The State being used by the API server
62 RootDir string // The faked-up root directory.62 RootDir string // The faked-up root directory.
63 LogDir string
63 oldHome string64 oldHome string
64 oldJujuHome string65 oldJujuHome string
65 environ environs.Environ66 environ environs.Environ
@@ -211,6 +212,8 @@
211 // sanity check we've got the correct environment.212 // sanity check we've got the correct environment.
212 c.Assert(environ.Name(), gc.Equals, "dummyenv")213 c.Assert(environ.Name(), gc.Equals, "dummyenv")
213 s.PatchValue(&dummy.DataDir, s.DataDir())214 s.PatchValue(&dummy.DataDir, s.DataDir())
215 s.LogDir = c.MkDir()
216 s.PatchValue(&dummy.LogDir, s.LogDir)
214217
215 versions := PreferredDefaultVersions(environ.Config(), version.Current)218 versions := PreferredDefaultVersions(environ.Config(), version.Current)
216 versions = append(versions, version.Current)219 versions = append(versions, version.Current)
217220
=== modified file 'provider/dummy/environs.go'
--- provider/dummy/environs.go 2014-04-03 04:46:51 +0000
+++ provider/dummy/environs.go 2014-04-04 04:45:42 +0000
@@ -501,6 +501,7 @@
501501
502// Override for testing - the data directory with which the state api server is initialised.502// Override for testing - the data directory with which the state api server is initialised.
503var DataDir = ""503var DataDir = ""
504var LogDir = ""
504505
505func (e *environ) ecfg() *environConfig {506func (e *environ) ecfg() *environConfig {
506 e.ecfgMutex.Lock()507 e.ecfgMutex.Lock()
@@ -607,7 +608,7 @@
607 if err != nil {608 if err != nil {
608 panic(err)609 panic(err)
609 }610 }
610 estate.apiServer, err = apiserver.NewServer(st, "localhost:0", []byte(testing.ServerCert), []byte(testing.ServerKey), DataDir)611 estate.apiServer, err = apiserver.NewServer(st, "localhost:0", []byte(testing.ServerCert), []byte(testing.ServerKey), DataDir, LogDir)
611 if err != nil {612 if err != nil {
612 panic(err)613 panic(err)
613 }614 }
614615
=== modified file 'state/apiserver/apiserver.go'
--- state/apiserver/apiserver.go 2014-03-13 22:47:05 +0000
+++ state/apiserver/apiserver.go 2014-04-04 04:45:42 +0000
@@ -30,12 +30,13 @@
30 state *state.State30 state *state.State
31 addr net.Addr31 addr net.Addr
32 dataDir string32 dataDir string
33 logDir string
33}34}
3435
35// Serve serves the given state by accepting requests on the given36// Serve serves the given state by accepting requests on the given
36// listener, using the given certificate and key (in PEM format) for37// listener, using the given certificate and key (in PEM format) for
37// authentication.38// authentication.
38func NewServer(s *state.State, addr string, cert, key []byte, datadir string) (*Server, error) {39func NewServer(s *state.State, addr string, cert, key []byte, datadir, logDir string) (*Server, error) {
39 lis, err := net.Listen("tcp", addr)40 lis, err := net.Listen("tcp", addr)
40 if err != nil {41 if err != nil {
41 return nil, err42 return nil, err
@@ -49,6 +50,7 @@
49 state: s,50 state: s,
50 addr: lis.Addr(),51 addr: lis.Addr(),
51 dataDir: datadir,52 dataDir: datadir,
53 logDir: logDir,
52 }54 }
53 // TODO(rog) check that *srvRoot is a valid type for using55 // TODO(rog) check that *srvRoot is a valid type for using
54 // as an RPC server.56 // as an RPC server.
5557
=== 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 04:45:42 +0000
@@ -54,7 +54,7 @@
54 "localhost:0",54 "localhost:0",
55 []byte(coretesting.ServerCert),55 []byte(coretesting.ServerCert),
56 []byte(coretesting.ServerKey),56 []byte(coretesting.ServerKey),
57 "",57 "", "",
58 )58 )
59 c.Assert(err, gc.IsNil)59 c.Assert(err, gc.IsNil)
60 info := &api.Info{60 info := &api.Info{
6161
=== 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 04:45:42 +0000
@@ -36,7 +36,10 @@
36func (s *serverSuite) TestStop(c *gc.C) {36func (s *serverSuite) TestStop(c *gc.C) {
37 // Start our own instance of the server so we have37 // Start our own instance of the server so we have
38 // a handle on it to stop it.38 // a handle on it to stop it.
39 srv, err := apiserver.NewServer(s.State, "localhost:0", []byte(coretesting.ServerCert), []byte(coretesting.ServerKey), "")39 srv, err := apiserver.NewServer(
40 s.State, "localhost:0",
41 []byte(coretesting.ServerCert), []byte(coretesting.ServerKey),
42 "", "")
40 c.Assert(err, gc.IsNil)43 c.Assert(err, gc.IsNil)
41 defer srv.Stop()44 defer srv.Stop()
4245

Subscribers

People subscribed via source and target branches

to status/vote changes: