cmd/jujud: introduce stater

cmd/jujud: introduce stater

Prereq: https://codereview.appspot.com/7134051/

This proposal introduces a new jujud subcommand, stater. The stater command replaces the current shell scripting built into cloudinit.

In future proposals the stater will become a long lived command that is responsible for the configuration of the mongodb upstart job.

https://code.launchpad.net/~dave-cheney/juju-core/071-cmd-jujud-stater-I/+merge/143638

Requires: https://code.launchpad.net/~dave-cheney/juju-core/069-worker-stater-I/+merge/143629

(do not edit description out of merge proposal)

[dave_cheney.net, 2013-01-18 04:11:27]

Please take a look.

[fwereade, 2013-01-18 07:39:31]

https://codereview.appspot.com/7149043/diff/4001/cmd/jujud/main.go
File cmd/jujud/main.go (right):

https://codereview.appspot.com/7149043/diff/4001/cmd/jujud/main.go#newcode22
cmd/jujud/main.go:22: jujud.Register(&StaterCommand{})
Either this needs a *stater.Config (-1), or the command itself needs to set one
up itself (+1); ISTM that it doesn't, and the tests poke all that info in
directly. Did I miss something?

https://codereview.appspot.com/7149043/diff/4001/cmd/jujud/stater.go
File cmd/jujud/stater.go (right):

https://codereview.appspot.com/7149043/diff/4001/cmd/jujud/stater.go#newcode12
cmd/jujud/stater.go:12: *stater.Config
Why embed a (nil) pointer when you could just embed directly...

https://codereview.appspot.com/7149043/diff/4001/cmd/jujud/stater.go#newcode22
cmd/jujud/stater.go:22: }
...initialize the fields here...

https://codereview.appspot.com/7149043/diff/4001/cmd/jujud/stater.go#newcode27
cmd/jujud/stater.go:27: sr := stater.NewStater(s.Config)
...and pass &s.Config here?

https://codereview.appspot.com/7149043/diff/4001/cmd/jujud/stater_test.go
File cmd/jujud/stater_test.go (left):

https://codereview.appspot.com/7149043/diff/4001/cmd/jujud/stater_test.go#old...
cmd/jujud/stater_test.go:16: ctx := &cmd.Context{
There are a lot of these scattered through the codebase, but maybe it's not
worth factoring them into one place. Thoughts?

https://codereview.appspot.com/7149043/diff/4001/environs/cloudinit/cloudinit.go
File environs/cloudinit/cloudinit.go (left):

https://codereview.appspot.com/7149043/diff/4001/environs/cloudinit/cloudinit...
environs/cloudinit/cloudinit.go:135: if err := addMongoToBoot(c, cfg); err !=
nil {
I don't think this change is justified yet -- we don't pass any of the necessary
information in. (Does this actually run correctly? I don't see how it can...)

[dave_cheney.net, 2013-01-18 20:18:48]

Please take a look.

https://codereview.appspot.com/7149043/diff/4001/cmd/jujud/main.go
File cmd/jujud/main.go (right):

https://codereview.appspot.com/7149043/diff/4001/cmd/jujud/main.go#newcode22
cmd/jujud/main.go:22: jujud.Register(&StaterCommand{})
On 2013/01/18 07:39:31, fwereade wrote:
> Either this needs a *stater.Config (-1), or the command itself needs to set
one
> up itself (+1); ISTM that it doesn't, and the tests poke all that info in
> directly. Did I miss something?

a nil configuration supplies valid defaults, right now providing your own config
is a convenience for the tests, but may be necessary in the future.

https://codereview.appspot.com/7149043/diff/4001/cmd/jujud/stater.go
File cmd/jujud/stater.go (right):

https://codereview.appspot.com/7149043/diff/4001/cmd/jujud/stater.go#newcode12
cmd/jujud/stater.go:12: *stater.Config
On 2013/01/18 07:39:31, fwereade wrote:
> Why embed a (nil) pointer when you could just embed directly...

It doesn't make a lot of difference, a nil configuration and a configuration of
zero values has the same effect.

https://codereview.appspot.com/7149043/diff/4001/cmd/jujud/stater.go#newcode22
cmd/jujud/stater.go:22: }
On 2013/01/18 07:39:31, fwereade wrote:
> ...initialize the fields here...

Not sure I follow, there are no fields to initialise at the moment as the
default (nil) configuration provides the correct values.

https://codereview.appspot.com/7149043/diff/4001/cmd/jujud/stater_test.go
File cmd/jujud/stater_test.go (left):

https://codereview.appspot.com/7149043/diff/4001/cmd/jujud/stater_test.go#old...
cmd/jujud/stater_test.go:16: ctx := &cmd.Context{
On 2013/01/18 07:39:31, fwereade wrote:
> There are a lot of these scattered through the codebase, but maybe it's not
> worth factoring them into one place. Thoughts?

Yup, will do in a follow up CL.

https://codereview.appspot.com/7149043/diff/4001/environs/cloudinit/cloudinit.go
File environs/cloudinit/cloudinit.go (left):

https://codereview.appspot.com/7149043/diff/4001/environs/cloudinit/cloudinit...
environs/cloudinit/cloudinit.go:135: if err := addMongoToBoot(c, cfg); err !=
nil {
On 2013/01/18 07:39:31, fwereade wrote:
> I don't think this change is justified yet -- we don't pass any of the
necessary
> information in. (Does this actually run correctly? I don't see how it can...)

The default (nil) configuration matches the logic that was replaced here.

I believe this change is ready to land.

lucky(~/src/launchpad.net/juju-core) % juju bootstrap --upload-tools -v
2013/01/19 07:04:36 JUJU environs/ec2: opening environment "ap-southeast-2"
2013/01/19 07:04:36 JUJU environs/ec2: bootstrapping environment
"ap-southeast-2"
2013/01/19 07:04:44 JUJU environs: putting tools
tools/juju-1.9.7-quantal-amd64.tgz (3260kB)
2013/01/19 07:05:19 JUJU environs/ec2: starting machine 0 in "ap-southeast-2"
running tools version "1.9.7-quantal-amd64" from
"https://s3-ap-southeast-2.amazonaws.com/juju-syd-en-ee-ii/tools/juju-1.9.7-quantal-amd64.tgz?AWSAccessKeyId=AKIAJ4SOKUWG25EDMAOA&Expires=1674072318&Signature=zIG0%2BD6px73OmV7peg3MB%2F9hBdI%3D"
2013/01/19 07:05:22 JUJU environs/ec2: started instance "i-78a58242"
lucky(~/src/launchpad.net/juju-core) % juju ssh 0 -- grep -A 100 fetching
/var/log/cloud-init-output.log
2013/01/18 20:07:19 JUJU fetching mongo from:
http://juju-dist.s3.amazonaws.com/tools/mongo-2.2.0-quantal-amd64.tgz
2013/01/18 20:07:20 JUJU unpacking mongo to: /opt
2013/01/18 20:07:21 JUJU:DEBUG making directory: /opt/mongo/bin
2013/01/18 20:07:21 JUJU:DEBUG unpacking file: /opt/mongo/bin/mongorestore
2013/01/18 20:07:27 JUJU:DEBUG unpacking file: /opt/mongo/bin/mongooplog
2013/01/18 20:07:29 JUJU:DEBUG unpacking file: /opt/mongo/bin/mongos
2013/01/18 20:07:30 JUJU:DEBUG unpacking file: /opt/mongo/bin/bsondump
2013/01/18 20:07:31 JUJU:DEBUG unpacking file: /opt/mongo/bin/mongotop
2013/01/18 20:07:32 JUJU:DEBUG unpacking file: /opt/mongo/bin/mongoperf
2013/01/18 20:07:33 JUJU:DEBUG unpacking file: /opt/mongo/bin/mongoexport
2013/01/18 20:07:34 JUJU:DEBUG unpacking file: /opt/mongo/bin/mongod
2013/01/18 20:07:37 JUJU:DEBUG unpacking file: /opt/mongo/bin/mongodump
2013/01/18 20:07:38 JUJU:DEBUG unpacking file: /opt/mongo/bin/mongoimport
2013/01/18 20:07:40 JUJU:DEBUG unpacking file: /opt/mongo/bin/mongo
2013/01/18 20:07:41 JUJU:DEBUG unpacking file: /opt/mongo/bin/mongofiles
2013/01/18 20:07:42 JUJU:DEBUG unpacking file: /opt/mongo/bin/mongostat
2013/01/18 20:07:44 JUJU creating mongo data directory: /var/lib/juju/db
2013/01/18 20:07:44 JUJU creating mongo journal directory:
/var/lib/juju/db/journal
2013/01/18 20:07:44 JUJU:DEBUG creating journal file:
/var/lib/juju/db/journal/prealloc.0, size 1048576
2013/01/18 20:07:44 JUJU:DEBUG creating journal file:
/var/lib/juju/db/journal/prealloc.1, size 1048576
2013/01/18 20:07:44 JUJU:DEBUG creating journal file:
/var/lib/juju/db/journal/prealloc.2, size 1048576
juju-db start/running, process 4275
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100    10  100    10    0     0    801      0 --:--:-- --:--:-- --:--:--   833
2013/01/18 20:07:44 JUJU state: opening state; mongo addresses:
["localhost:37017"]; entity ""
2013/01/18 20:07:44 JUJU state: connecting to 127.0.0.1:37017
2013/01/18 20:07:44 JUJU state: connection failed: dial tcp 127.0.0.1:37017:
connection refused
2013/01/18 20:07:44 JUJU state: connecting to 127.0.0.1:37017
2013/01/18 20:07:44 JUJU state: connection failed: dial tcp 127.0.0.1:37017:
connection refused
2013/01/18 20:07:44 JUJU state: connecting to 127.0.0.1:37017
2013/01/18 20:07:44 JUJU state: connection failed: dial tcp 127.0.0.1:37017:
connection refused
2013/01/18 20:07:45 JUJU state: connecting to 127.0.0.1:37017
2013/01/18 20:07:45 JUJU state: connection established
2013/01/18 20:07:45 JUJU:DEBUG state/watcher: reading most recent document to
ignore past history...
2013/01/18 20:07:45 JUJU:DEBUG state/presence: synchronizing watcher knowledge
with database...
2013/01/18 20:07:45 JUJU:DEBUG state/watcher: loading new events from changelog
collection...
2013/01/18 20:07:45 JUJU state: storing no-secrets environment configuration
jujud-machine-0 start/running, process 4301
Cloud-init v. 0.7 finished at Fri, 18 Jan 2013 20:07:45 +0000. Datasource
DataSourceEc2.  Up 115.99 seconds
Connection to ec2-54-252-26-170.ap-southeast-2.compute.amazonaws.com closed.

[rog, 2013-01-23 17:32:30]

i'm not sure that a new jujud subcommand is the way to go.
i believe the right approach is to make the state server
a job run by the machiner.

let's talk live about this.