Code review comment for lp:~dimitern/juju-core/382-api-provisioner-machine-nics

LGTM. I am a little panicked around future-proofing though, I think
these api structs will want to look a little different, or maybe quite a
bit different, when we add other clouds and better networking support.
We seem to be reserving some pretty generic API names 'AddNetworks" and
will probably want a number of revisions. We may get to several -Ex
suffixes or something.

https://codereview.appspot.com/84570043/diff/1/state/api/params/internal.go
File state/api/params/internal.go (right):

https://codereview.appspot.com/84570043/diff/1/state/api/params/internal.go#newcode398
state/api/params/internal.go:398: NetworkName string
This will probably be a non-name id for clouds other than MAAS.
Shouldn't matter apart from naming.

https://codereview.appspot.com/84570043/diff/1/state/api/provisioner/machine.go
File state/api/provisioner/machine.go (right):

https://codereview.appspot.com/84570043/diff/1/state/api/provisioner/machine.go#newcode85
state/api/provisioner/machine.go:85: if interfaces[i].MachineTag !=
m.tag {
Why the conditional?

https://codereview.appspot.com/84570043/diff/1/state/api/provisioner/provisioner_test.go
File state/api/provisioner/provisioner_test.go (right):

https://codereview.appspot.com/84570043/diff/1/state/api/provisioner/provisioner_test.go#newcode359
state/api/provisioner/provisioner_test.go:359: c.Assert(net.Name(),
gc.Equals, expectParams.Name)
Can use c.Check for these two after the err Assert.

https://codereview.appspot.com/84570043/diff/1/state/api/provisioner/provisioner_test.go#newcode412
state/api/provisioner/provisioner_test.go:412: // Ensure only the first
error is reported.
This feels like a seperate testcase really, the current test is > one
screen high which is a bit of a personal bugbear of mine. Sharing setup
steps is not that hard.

https://codereview.appspot.com/84570043/diff/1/state/apiserver/provisioner/provisioner_test.go
File state/apiserver/provisioner/provisioner_test.go (right):

https://codereview.appspot.com/84570043/diff/1/state/apiserver/provisioner/provisioner_test.go#newcode828
state/apiserver/provisioner/provisioner_test.go:828:
c.Assert(net.Name(), gc.Equals, "vlan42")
Can use c.Check, and below.

https://codereview.appspot.com/84570043/diff/1/state/apiserver/provisioner/provisioner_test.go#newcode839
state/apiserver/provisioner/provisioner_test.go:839: // Test it fails
with a non-environment-manager.
This feels like a seperate testcase.

https://codereview.appspot.com/84570043/diff/1/state/apiserver/provisioner/provisioner_test.go#newcode850
state/apiserver/provisioner/provisioner_test.go:850: func (s
*withoutStateServerSuite) TestAddNetworkInterface(c *gc.C) {
Giant complicated test again...

https://codereview.appspot.com/84570043/