Added explicit --journal flag to mongod

Added explicit --journal flag to mongod

Mongod has Write Ahead logging on by default for 
64 bits systems:
http://docs.mongodb.org/manual/tutorial/manage-journaling/
juju seems to be assuming this for every arch
so the specific --journal flag was added.

https://code.launchpad.net/~hduran-8/juju-core/add_mongodb_wal/+merge/219567

(do not edit description out of merge proposal)

[hduran, 2014-05-14 18:50:35]

Please take a look.

[wwitzel3, 2014-05-14 19:08:07]

LGTM

assuming the existing tests exercise the changed code paths.

[fwereade, 2014-05-15 09:46:37]

NOT LGTM without a test. We're invoking it differently, something should have
failed and needed to be changed (and even if you can argue against that case, we
need a test that we *are* writing --journal).

Does this/can this handle upgrading environments?

[hduran, 2014-05-15 10:24:12]

On 2014/05/15 09:46:37, fwereade wrote:
> NOT LGTM without a test. We're invoking it differently, something should have
> failed and needed to be changed (and even if you can argue against that case,
we
> need a test that we *are* writing --journal).
> 
> Does this/can this handle upgrading environments?

I agree I did not notice we actually have tests to see if a flag is being passed
which I find is very nice, si I will add one.
But if I understand correctly you are here asking for other kind of testing too,
such as seeing if mongod properly starts when this flag is used?

[fwereade, 2014-05-16 08:31:23]

On 2014/05/15 10:24:12, hduran wrote:
> On 2014/05/15 09:46:37, fwereade wrote:
> > NOT LGTM without a test. We're invoking it differently, something should
have
> > failed and needed to be changed (and even if you can argue against that
case,
> we
> > need a test that we *are* writing --journal).
> > 
> > Does this/can this handle upgrading environments?
> 
> I agree I did not notice we actually have tests to see if a flag is being
passed
> which I find is very nice, si I will add one.
> But if I understand correctly you are here asking for other kind of testing
too,
> such as seeing if mongod properly starts when this flag is used?

No, I'm fine letting CI handle that -- I'm just talking about making sure the
unit tests actually test the units :).

[hduran, 2014-05-16 14:37:18]

Please take a look.

[fwereade, 2014-05-16 21:01:18]

LGTM with trivials

https://codereview.appspot.com/97370044/diff/30001/agent/mongo/mongo_test.go
File agent/mongo/mongo_test.go (right):

https://codereview.appspot.com/97370044/diff/30001/agent/mongo/mongo_test.go#...
agent/mongo/mongo_test.go:231: c.Assert(strings.Contains(svc.Cmd, "--journal"),
jc.IsTrue)
just for pedantry's sake, please check for whitespace/end-string around
--journal

https://codereview.appspot.com/97370044/diff/30001/agent/mongo/mongo_test.go#...
agent/mongo/mongo_test.go:246: // c.Assert(strings.Contains(cmd.Path,
"--journal"), jc.IsTrue)
d

[hduran, 2014-05-19 01:37:31]

Please take a look.

[fwereade, 2014-05-19 07:30:15]

Still LGTM; if I LGTM-with-requests it means I trust you to make the changes
sanely and land them.

You *should* always feel free to request a new review, but I don't think you
really needed one here :).