juju-core

Merge lp://staging/~natefinch/juju-core/026-EnsureMongoServer into lp://staging/~go-bot/juju-core/trunk

026-EnsureMongoServer
Merge into trunk

Proposed by Nate Finch on 2014-01-16

Status:	Work in progress
Proposed branch:	lp://staging/~natefinch/juju-core/026-EnsureMongoServer
Merge into:	lp://staging/~go-bot/juju-core/trunk
Diff against target:	756 lines (+448/-26) (has conflicts) 10 files modified agent/agent.go (+10/-0) cmd/jujud/machine.go (+136/-14) cmd/jujud/machine_test.go (+9/-0) environs/cloudinit/cloudinit.go (+16/-3) provider/local/config.go (+0/-5) provider/local/environ.go (+210/-0) replicaset/replicaset.go (+4/-0) testing/mockupstart.go (+42/-0) upstart/service.go (+13/-2) upstart/upstart.go (+8/-2) Text conflict in cmd/jujud/machine.go Text conflict in cmd/jujud/machine_test.go Text conflict in environs/cloudinit/cloudinit.go Text conflict in provider/local/environ.go Text conflict in replicaset/replicaset.go
To merge this branch:	bzr merge lp://staging/~natefinch/juju-core/026-EnsureMongoServer
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Juju Engineering		2014-01-16	Pending
Review via email: mp+201961@code.staging.launchpad.net

Description of the change

EnsureMongoServer function for MachineAgent

Implement a method to allow the machine agent to create & run the upstart script for mongo.

https://codereview.appspot.com/53220043/

Revision history for this message

Roger Peppe (rogpeppe) wrote on 2014-01-16:

Looks good so far, with a couple of suggestions/queries below.

https://codereview.appspot.com/53220043/diff/20001/agent/agent.go
File agent/agent.go (right):

https://codereview.appspot.com/53220043/diff/20001/agent/agent.go#newcode232
agent/agent.go:232: func MongoDbDir(dir string) string {
or this could be a method on Config.

at the least, dir should be named "dataDir"

https://codereview.appspot.com/53220043/diff/20001/cmd/jujud/machine.go
File cmd/jujud/machine.go (right):

https://codereview.appspot.com/53220043/diff/20001/cmd/jujud/machine.go#newcode386
cmd/jujud/machine.go:386: if err := conf.Install(); err != nil {
Do we really want to do this every time the machine agent bounces?
If possible I'd prefer to avoid bouncing mongod in that case.

https://codereview.appspot.com/53220043/diff/20001/replicaset/replicaset.go
File replicaset/replicaset.go (right):

https://codereview.appspot.com/53220043/diff/20001/replicaset/replicaset.go#newcode257
replicaset/replicaset.go:257: State MemberState `bson:"State"`
"state", surely?

https://codereview.appspot.com/53220043/

Revision history for this message

Nate Finch (natefinch) wrote on 2014-01-16:

Reviewers: mp+201961_code.launchpad.net, rog,

Message:
Some more clean up, and now add the "ensure" part of ensure mongo
server. Also consolidated the code so we only do this in machine agent.

Also, just ignore the stuff in replicaset.go, it's not really part of
this change and I'll remove it.

Description:
EnxureMongoServer function for MachineAgent

IMplement a method to allow the machine agent to create & run the
upstart script for mongo.

https://code.launchpad.net/~natefinch/juju-core/026-EnsureMongoServer/+merge/201961

(do not edit description out of merge proposal)

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

Affected files (+130, -58 lines):
   A [revision details]
   M agent/agent.go
   cmd/jujud/machine.go
   M environs/cloudinit/cloudinit.go
   M provider/local/config.go
   M provider/local/environ.go
   M replicaset/replicaset.go
   M replicaset/replicaset_test.go
   M upstart/service.go

Revision history for this message

Nate Finch (natefinch) wrote on 2014-01-17:

Please take a look.

https://codereview.appspot.com/53220043/

Revision history for this message

William Reade (fwereade) wrote on 2014-01-22:

couple of quick comments, I think I may have misunderstood the StatePort
bits, can we chat when you're free?

https://codereview.appspot.com/53220043/diff/60001/agent/agent.go
File agent/agent.go (right):

https://codereview.appspot.com/53220043/diff/60001/agent/agent.go#newcode99
agent/agent.go:99: StatePort() int
I'm a bit bothered about storing this in agent config. And actually
about storing *any* of this stuff in agent config -- we have manual
bootstrap now, and I don't see why this is needed any more.

https://codereview.appspot.com/53220043/diff/60001/provider/local/environ.go
File provider/local/environ.go (right):

https://codereview.appspot.com/53220043/diff/60001/provider/local/environ.go#newcode427
provider/local/environ.go:427: journalDir :=
filepath.Join(env.config.rootDir(), "db/journal")
"db", "journal"?

https://codereview.appspot.com/53220043/diff/60001/upstart/service.go
File upstart/service.go (right):

https://codereview.appspot.com/53220043/diff/60001/upstart/service.go#newcode26
upstart/service.go:26: func MongoUpstartService(name, dataDir string,
port int) *Conf {
Hmm, I'm really not sure this is the right package for this file. Isn't
this stuff all specific to instance config?

https://codereview.appspot.com/53220043/

lp://staging/~natefinch/juju-core/026-EnsureMongoServer updated on 2014-01-28

2185. By Nate Finch on 2014-01-23: Add code to mock out upstart so we can successfully run the new machine agent code during tests. Also added code to only run EnsureMongoServer when the machine agent has the ManageEnviron job
2186. By Nate Finch on 2014-01-28: fix a couple build errors
2187. By Nate Finch on 2014-01-28: make sure the bootstrap node doesn't try to connect to the API to get its state
2188. By Nate Finch on 2014-01-28: I should learn to hit save before commiting

Unmerged revisions

2188. By Nate Finch on 2014-01-28: I should learn to hit save before commiting
2187. By Nate Finch on 2014-01-28: make sure the bootstrap node doesn't try to connect to the API to get its state
2186. By Nate Finch on 2014-01-28: fix a couple build errors
2185. By Nate Finch on 2014-01-23: Add code to mock out upstart so we can successfully run the new machine agent code during tests. Also added code to only run EnsureMongoServer when the machine agent has the ManageEnviron job
2184. By Nate Finch on 2014-01-16: add the ensure part of ensuremongoserver, also remove the old upstart stuff so we always run it from machine agent
2183. By Nate Finch on 2014-01-16: twiddle the comment to help kick rietveld
2182. By Nate Finch on 2014-01-16: line got deleted somehow
2181. By Nate Finch on 2014-01-16: create ensuremongoserver function to create/rewrite the mongo upstart script from the machine agent
2180. By Nate Finch on 2014-01-15: Let's spell replSet correctly, eh?
2179. By Nate Finch on 2014-01-07: uncomment some tests, change mongo upstart to always set mongo in replicaset mode