Unify providers' user-data composition functions.

Unify providers' user-data composition functions.

All the "physical" providers (EC2, OpenStack, MAAS, Azure) were generating
user data in essentially the same way.  The MAAS one looked different but
actually just added an optional bit that was still completely generic.  And so
this branch unifies the implementations.

Construction of the cloudinit.MachineConfig was lifted out of the function.
That may seem arbitrary, but it's part of a greater plan: in a later step of
the ongoing refactoring the construction can be lifted out of the providers'
internalStartInstance methods as well, doing away with the EC2 and OpenStack
providers' startInstanceParams structs.  This actually shortens some of the
data flows and reduces cognitive load, which can only be good for maintenance.

After that step we can have a fresh look at sanitizing NewMachineConfig (which
arguably should take more parameters, or not exist at all, but is in an
intermediate state that facilitates the changes happening now).  We may also
look into re-introducing some sort of dedicated Parameter Object for
internalStartInstance, but by then the cluster of methods Bootstrap /
StartInstance / internalStartInstance will look a bit different and we can
find the most maintainable solution for that new situation.

https://code.launchpad.net/~jtv/juju-core/regularize-userdata-function/+merge/175191

(do not edit description out of merge proposal)

[jtv.canonical, 2013-07-17 03:56:58]

Please take a look.

[jtv.canonical, 2013-07-17 06:11:57]

Please take a look.

[fwereade, 2013-07-17 08:51:52]

LGTM. Very nice indeed.

https://codereview.appspot.com/11409043/diff/4001/environs/azure/customdata_t...
File environs/azure/customdata_test.go (right):

https://codereview.appspot.com/11409043/diff/4001/environs/azure/customdata_t...
environs/azure/customdata_test.go:55: c.Check(err, gc.ErrorMatches, "failure
while generating custom data: .*")
While it's sometimes inconvenient, I think it's usually worth being explicit
about the full error message. That way we have half a chance of spotting really
bad ones and tidying them up before users have to see them.

[jtv.canonical, 2013-07-17 11:01:24]

OK, I'm checking for the entire error message now.

[jtv.canonical, 2013-07-17 11:04:44]

Please take a look.

[mue, 2013-07-17 11:13:58]

Cool change, LGTM. Only one very small commment.

https://codereview.appspot.com/11409043/diff/11001/environs/cloudinit.go
File environs/cloudinit.go (right):

https://codereview.appspot.com/11409043/diff/11001/environs/cloudinit.go#newc...
environs/cloudinit.go:9: cloudinit_core "launchpad.net/juju-core/cloudinit"
No friend of the underscore here, it's no Go style.

[dimitern, 2013-07-17 11:23:12]

LGTM as well, thanks for doing this. Can we consider the bug mentioned in the
deleted sections TODOs closed with this?

[dimitern, 2013-07-17 11:23:12]

LGTM as well, thanks for doing this. Can we consider the bug mentioned in the
deleted sections TODOs closed with this?

[jtv.canonical, 2013-07-17 11:30:27]

This doesn't close that whole bug, no, but it resolves one part of the
duplication mentioned there.  I'm proceeding with the other cleanups.

[rog, 2013-07-17 14:04:31]

LGTM but i think perhaps environs/cloudinit would be a better place for the new
function.

https://codereview.appspot.com/11409043/diff/11001/environs/azure/customdata.go
File environs/azure/customdata.go (right):

https://codereview.appspot.com/11409043/diff/11001/environs/azure/customdata....
environs/azure/customdata.go:17: zipData, err := environs.ComposeUserData(cfg)
just a thought: rather than environs.ComposeUserData,
why not put the function in environs/cloudinit,
as it's directly related to cloudinit stuff,

something like this, perhaps? (in environs/cloudinit)

func UserData(*cloudinit.Config) ([]byte, error)

https://codereview.appspot.com/11409043/diff/11001/environs/azure/customdata_...
File environs/azure/customdata_test.go (left):

https://codereview.appspot.com/11409043/diff/11001/environs/azure/customdata_...
environs/azure/customdata_test.go:58: func (*CustomDataSuite)
TestUserDataProducesZipFile(c *gc.C) {
when we've removed this, are we still left with a test that we are actually
calling ComposeUserData correctly?