Merge lp:~dimitern/juju-core/381-state-machine-nics into lp:~go-bot/juju-core/trunk

Proposed by Dimiter Naydenov
Status: Merged
Approved by: Dimiter Naydenov
Approved revision: no longer in the source branch.
Merged at revision: 2567
Proposed branch: lp:~dimitern/juju-core/381-state-machine-nics
Merge into: lp:~go-bot/juju-core/trunk
Prerequisite: lp:~dimitern/juju-core/380-provisioner-honors-networks
Diff against target: 1257 lines (+689/-130)
23 files modified
juju/conn_test.go (+7/-7)
state/addmachine.go (+5/-1)
state/api/provisioner/machine.go (+4/-4)
state/api/provisioner/provisioner_test.go (+2/-2)
state/apiserver/provisioner/provisioner.go (+3/-3)
state/apiserver/provisioner/provisioner_test.go (+2/-2)
state/assign_test.go (+1/-1)
state/cleanup.go (+0/-13)
state/compat_test.go (+8/-8)
state/machine.go (+133/-7)
state/machine_test.go (+144/-6)
state/networkinterfaces.go (+45/-0)
state/networkinterfaces_test.go (+37/-0)
state/networks.go (+68/-0)
state/networks_test.go (+56/-0)
state/open.go (+27/-23)
state/requestednetworks.go (+19/-16)
state/service.go (+2/-2)
state/state.go (+81/-28)
state/state_test.go (+38/-0)
state/statecmd/machineconfig.go (+2/-2)
worker/provisioner/provisioner_task.go (+1/-1)
worker/provisioner/provisioner_test.go (+4/-4)
To merge this branch: bzr merge lp:~dimitern/juju-core/381-state-machine-nics
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+213796@code.launchpad.net

Commit message

state: Add machine networks and interfaces

Added a couple of entities (and related methods) in state:
 * Network - holds a name, CIDR and VLAN tag; used to
   describe actual configured networks that machines
   need to be on.
 * NetworkInterface - linked to a machine id and network
   name, holds the network interface name to MAC address
   mapping in state (MAC address as key for uniqueness);
   machines support adding one or more NICs.

Renamed machine.Network() and st.networks to RequestedNetworks()
and st.requestedNetworks (in code only, schema remains the same).
This helps avoid confusion between the requested networks
(to include/exclude on a machine, coming from the service; they
are now called "requested networks"), and actually configured
networks on a running machine (just "networks").

For now networks are immutable, can't be removed and NICs can be
added to them only if their machine is not provisioned yet.
NICs are only removed when the machine itself is removed.

Provisioner API change: renamed Networks() to RequestedNetworks()
which is fine, because Networks() was added recently and not
released yet.

We'll use the added machine NICs to show enabled networks
in juju status later on, and they will be set at StartInstance()
time inside the provisioner. Internally, MaaS provider changes
will be needed to get NIC-to-MAC mapping for an acquired node
and the list of networks for it.

https://codereview.appspot.com/83070047/

R=fwereade

Description of the change

state: Add machine networks and interfaces

Added a couple of entities (and related methods) in state:
 * Network - holds a name, CIDR and VLAN tag; used to
   describe actual configured networks that machines
   need to be on.
 * NetworkInterface - linked to a machine id and network
   name, holds the network interface name to MAC address
   mapping in state (MAC address as key for uniqueness);
   machines support adding one or more NICs.

Renamed machine.Network() and st.networks to RequestedNetworks()
and st.requestedNetworks (in code only, schema remains the same).
This helps avoid confusion between the requested networks
(to include/exclude on a machine, coming from the service; they
are now called "requested networks"), and actually configured
networks on a running machine (just "networks").

For now networks are immutable, can't be removed and NICs can be
added to them only if their machine is not provisioned yet.
NICs are only removed when the machine itself is removed.

Provisioner API change: renamed Networks() to RequestedNetworks()
which is fine, because Networks() was added recently and not
released yet.

We'll use the added machine NICs to show enabled networks
in juju status later on, and they will be set at StartInstance()
time inside the provisioner. Internally, MaaS provider changes
will be needed to get NIC-to-MAC mapping for an acquired node
and the list of networks for it.

https://codereview.appspot.com/83070047/

To post a comment you must log in.
Revision history for this message
Dimiter Naydenov (dimitern) wrote :
Download full text (5.9 KiB)

Reviewers: mp+213796_code.launchpad.net,

Message:
Please take a look.

Description:
state: Add machine.(Set)NetworkInterfaces

Added two new methods and a field on the machine document:
  * machineDoc.NetworkInterfaces []state.NetworkInterface
    (contains configured NICs for the machine with their
    name, CIDR, VLAN tag and MAC address);
  * machine.NetworkInterfaces() returns previously set NICs;
  * machine.SetNetworkInterfaces() will be called by the
    provisioner at StartInstance() time set discovered network
    interfaces (from the provider) when include/excludeNetworks
    is specified.

This is needed to show actual network interfaces and their linked
networks in juju status, as well as to prepare the parameters
needed by cloudinit scripts to set up the NICs at boot time
(only when networks are present).

https://code.launchpad.net/~dimitern/juju-core/381-state-machine-nics/+merge/213796

Requires:
https://code.launchpad.net/~dimitern/juju-core/380-provisioner-honors-networks/+merge/213647

(do not edit description out of merge proposal)

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

Affected files (+1986, -1111 lines):
   A [revision details]
   M agent/agent.go
   M agent/agent_test.go
   M agent/bootstrap.go
   M agent/bootstrap_test.go
   M agent/mongo/mongo.go
   M agent/mongo/mongo_test.go
   M cmd/cmd.go
   M cmd/envcmd/environmentcommand.go
   M cmd/envcmd/environmentcommand_test.go
   M cmd/envcmd/export_test.go
   M cmd/juju/addmachine.go
   M cmd/juju/addrelation.go
   M cmd/juju/addunit.go
   A cmd/juju/adduser.go
   A cmd/juju/adduser_test.go
   M cmd/juju/authorisedkeys_add.go
   M cmd/juju/authorisedkeys_delete.go
   M cmd/juju/authorisedkeys_import.go
   M cmd/juju/authorisedkeys_list.go
   M cmd/juju/bootstrap.go
   M cmd/juju/bootstrap_test.go
   M cmd/juju/cmd_test.go
   M cmd/juju/constraints.go
   M cmd/juju/debuglog_test.go
   M cmd/juju/deploy.go
   M cmd/juju/destroymachine.go
   M cmd/juju/destroyrelation.go
   M cmd/juju/destroyservice.go
   M cmd/juju/destroyunit.go
   M cmd/juju/endpoint.go
   M cmd/juju/ensureavailability.go
   M cmd/juju/environment.go
   M cmd/juju/expose.go
   M cmd/juju/get.go
   M cmd/juju/plugin.go
   M cmd/juju/publish.go
   M cmd/juju/publish_test.go
   A cmd/juju/removeuser.go
   A cmd/juju/removeuser_test.go
   M cmd/juju/resolved.go
   M cmd/juju/retryprovisioning.go
   M cmd/juju/run.go
   M cmd/juju/run_test.go
   M cmd/juju/scp_test.go
   M cmd/juju/set.go
   M cmd/juju/ssh.go
   M cmd/juju/ssh_test.go
   M cmd/juju/status.go
   M cmd/juju/status_test.go
   M cmd/juju/switch.go
   M cmd/juju/switch_test.go
   M cmd/juju/synctools.go
   M cmd/juju/unexpose.go
   M cmd/juju/unset.go
   M cmd/juju/upgradecharm.go
   M cmd/juju/upgradejuju.go
   M cmd/jujud/bootstrap.go
   M cmd/jujud/bootstrap_test.go
   M cmd/jujud/machine_test.go
   M cmd/jujud/run_test.go
   M cmd/plugins/juju-metadata/imagemetadata.go
   M cmd/plugins/juju-metadata/toolsmetadata.go
   M cmd/plugins/juju-metadata/validateimagemetadata.go
   M cmd/plugins/juju-metadata/validatetoolsmetadata.go
   M cmd/plugins/juju-restore/restore.go
   M cmd/supercommand.go
   M dependencies.tsv
   M...

Read more...

Revision history for this message
Dimiter Naydenov (dimitern) wrote :
Revision history for this message
William Reade (fwereade) wrote :

Couple of tweaks please.

https://codereview.appspot.com/83070047/diff/20001/state/machine.go
File state/machine.go (right):

https://codereview.appspot.com/83070047/diff/20001/state/machine.go#newcode915
state/machine.go:915: Update: bson.D{{"$set",
bson.D{{"networkinterfaces", ifaces}}}},
I get really nervous when we use the same struct in the interface and in
the actual data storage -- it's just too easy for changes to break
stuff.

And... can we put it on a separate doc please? It's just the usual
read/write/watch usage patterns issue.

https://codereview.appspot.com/83070047/

Revision history for this message
Dimiter Naydenov (dimitern) wrote :

Please take a look.

https://codereview.appspot.com/83070047/diff/20001/state/machine.go
File state/machine.go (right):

https://codereview.appspot.com/83070047/diff/20001/state/machine.go#newcode915
state/machine.go:915: Update: bson.D{{"$set",
bson.D{{"networkinterfaces", ifaces}}}},
On 2014/04/02 10:18:45, fwereade wrote:
> I get really nervous when we use the same struct in the interface and
in the
> actual data storage -- it's just too easy for changes to break stuff.

> And... can we put it on a separate doc please? It's just the usual
> read/write/watch usage patterns issue.

Changed as discussed - separate docs for networks and NICs, linked to a
machine.

https://codereview.appspot.com/83070047/

Revision history for this message
William Reade (fwereade) wrote :
Download full text (7.2 KiB)

Some naming quibbles; and quite a lot of freakouts wrt races. Most can
be eliminated by dropping Remove methods, I think.

https://codereview.appspot.com/83070047/diff/40001/juju/conn_test.go
File juju/conn_test.go (right):

https://codereview.appspot.com/83070047/diff/40001/juju/conn_test.go#newcode406
juju/conn_test.go:406: include, exclude, err := machine.LinkedNetworks()
Can we call this RequestedNetworks or something? "Linked" doesn't fit my
brain here.

https://codereview.appspot.com/83070047/diff/40001/state/addmachine.go
File state/addmachine.go (right):

https://codereview.appspot.com/83070047/diff/40001/state/addmachine.go#newcode425
state/addmachine.go:425: createLinkedNetworksOp(st,
machineGlobalKey(mdoc.Id),
can we have a TODO and bug for the lack of sanity-checking here? ideally
we'd be checking names against actual networks.

https://codereview.appspot.com/83070047/diff/40001/state/cleanup.go
File state/cleanup.go (right):

https://codereview.appspot.com/83070047/diff/40001/state/cleanup.go#newcode140
state/cleanup.go:140: // depend upon the supplied machine, and removes
the machine from state. It's
whoops, please s/removes the machine from state/sets the machine to
Dead/

https://codereview.appspot.com/83070047/diff/40001/state/cleanup.go#newcode157
state/cleanup.go:157: if err :=
st.cleanupLinkedNetworks(machineGlobalKey(machineId)); err != nil {
Sorry I missed this before, but I don't see the point of this code. All
it does is leave an extant machine doc without linked networks -- not
fatal, sure, but not very useful either.

We should just run the appropriate ops when we remove a service/machine,
and leave cleanup out of it.

https://codereview.appspot.com/83070047/diff/40001/state/machine.go
File state/machine.go (right):

https://codereview.appspot.com/83070047/diff/40001/state/machine.go#newcode890
state/machine.go:890: }
Possibly this is something more like a NetworkSpecification?

https://codereview.appspot.com/83070047/diff/40001/state/machine.go#newcode893
state/machine.go:893: func (m *Machine) MachineNetworks()
([]*MachineNetwork, error) {
and I think this is more like just Networks()

https://codereview.appspot.com/83070047/diff/40001/state/machine.go#newcode939
state/machine.go:939: Name: name,
InterfaceName?

https://codereview.appspot.com/83070047/diff/40001/state/machine.go#newcode952
state/machine.go:952: }}
I think we need an assert that the machine is alive. IIRC machines can't
fast-forward from alive to removed, so that should be sufficient to
maintain integrity in the face of remove-all-interfaces ops called at
machine-Remove time.

https://codereview.appspot.com/83070047/diff/40001/state/machine_test.go
File state/machine_test.go (right):

https://codereview.appspot.com/83070047/diff/40001/state/machine_test.go#newcode394
state/machine_test.go:394: net, err := st.AddMachineNetwork(networkName,
cidr, vlanTag)
I don't think this is really a MachineNetwork, is it? it feels like it's
the heart of the actual network model in juju and deserves the name
Network...

https://codereview.appspot.com/83070047/diff/40001/state/machine_test.go#newcode396
state/machine_test.go:396: iface, err :=
machine.AddNetw...

Read more...

Revision history for this message
Dimiter Naydenov (dimitern) wrote :
Download full text (10.0 KiB)

Please take a look.

https://codereview.appspot.com/83070047/diff/40001/juju/conn_test.go
File juju/conn_test.go (right):

https://codereview.appspot.com/83070047/diff/40001/juju/conn_test.go#newcode406
juju/conn_test.go:406: include, exclude, err := machine.LinkedNetworks()
On 2014/04/03 16:37:09, fwereade wrote:
> Can we call this RequestedNetworks or something? "Linked" doesn't fit
my brain
> here.

Done.

https://codereview.appspot.com/83070047/diff/40001/state/addmachine.go
File state/addmachine.go (right):

https://codereview.appspot.com/83070047/diff/40001/state/addmachine.go#newcode425
state/addmachine.go:425: createLinkedNetworksOp(st,
machineGlobalKey(mdoc.Id),
On 2014/04/03 16:37:09, fwereade wrote:
> can we have a TODO and bug for the lack of sanity-checking here?
ideally we'd be
> checking names against actual networks.

Done.

https://codereview.appspot.com/83070047/diff/40001/state/cleanup.go
File state/cleanup.go (right):

https://codereview.appspot.com/83070047/diff/40001/state/cleanup.go#newcode140
state/cleanup.go:140: // depend upon the supplied machine, and removes
the machine from state. It's
On 2014/04/03 16:37:09, fwereade wrote:
> whoops, please s/removes the machine from state/sets the machine to
Dead/

Done.

https://codereview.appspot.com/83070047/diff/40001/state/cleanup.go#newcode157
state/cleanup.go:157: if err :=
st.cleanupLinkedNetworks(machineGlobalKey(machineId)); err != nil {
On 2014/04/03 16:37:09, fwereade wrote:
> Sorry I missed this before, but I don't see the point of this code.
All it does
> is leave an extant machine doc without linked networks -- not fatal,
sure, but
> not very useful either.

> We should just run the appropriate ops when we remove a
service/machine, and
> leave cleanup out of it.

Removed.

https://codereview.appspot.com/83070047/diff/40001/state/machine.go
File state/machine.go (right):

https://codereview.appspot.com/83070047/diff/40001/state/machine.go#newcode890
state/machine.go:890: }
On 2014/04/03 16:37:09, fwereade wrote:
> Possibly this is something more like a NetworkSpecification?

Do you mean rename the method or something else? I thought you suggested
to rename it to RequestedNetworks earlier.

https://codereview.appspot.com/83070047/diff/40001/state/machine.go#newcode893
state/machine.go:893: func (m *Machine) MachineNetworks()
([]*MachineNetwork, error) {
On 2014/04/03 16:37:09, fwereade wrote:
> and I think this is more like just Networks()

So, let's call this Networks() and the former RequestedNetworks() or
even maybe ServiceNetworks()?

https://codereview.appspot.com/83070047/diff/40001/state/machine.go#newcode939
state/machine.go:939: Name: name,
On 2014/04/03 16:37:09, fwereade wrote:
> InterfaceName?

Done.

https://codereview.appspot.com/83070047/diff/40001/state/machine.go#newcode952
state/machine.go:952: }}
On 2014/04/03 16:37:09, fwereade wrote:
> I think we need an assert that the machine is alive. IIRC machines
can't
> fast-forward from alive to removed, so that should be sufficient to
maintain
> integrity in the face of remove-all-interfaces ops called at
machine-Remove
> time.

Done.

https://codereview.appspot.com/83070047/diff/40001/state/mach...

Revision history for this message
William Reade (fwereade) wrote :
Download full text (5.5 KiB)

LGTM with relatively trivial fixes:

https://codereview.appspot.com/83070047/diff/40001/state/machine_test.go
File state/machine_test.go (right):

https://codereview.appspot.com/83070047/diff/40001/state/machine_test.go#newcode394
state/machine_test.go:394: net, err := st.AddMachineNetwork(networkName,
cidr, vlanTag)
On 2014/04/04 09:14:43, dimitern wrote:
> On 2014/04/03 16:37:09, fwereade wrote:
> > I don't think this is really a MachineNetwork, is it? it feels like
it's the
> > heart of the actual network model in juju and deserves the name
Network...

> Done, although arguably this specific definition of network concerns
only
> machines (i.e. NICs and the networks they're on are a property of a
machine).

I'm pretty sure netName/cidr/vlanTag defines a network -- and
ifceName/MAC/machine/network defines an interface. Am I ludicrously
wrong about something?

https://codereview.appspot.com/83070047/diff/40001/state/machine_test.go#newcode396
state/machine_test.go:396: iface, err :=
machine.AddNetworkInterface(mac, ifaceName, networkName)
On 2014/04/04 09:14:43, dimitern wrote:
> On 2014/04/03 16:37:09, fwereade wrote:
> > wondering if this should be net.AddMachine? just a thought, open to
> > counterarguments

> That's a fair point, I'm not sure. Networks (as of now) are created
when you
> need to add a NIC to a machine, but they stay past the lifetime of the
initial
> machine. On another hand, the machine comes first in the sequence of
things
> created in state (although this might change in the future).

I don't think we need to get this perfect now, because (yay!) this bit
of state at least is properly isolated.

https://codereview.appspot.com/83070047/diff/40001/state/networkinterfaces.go
File state/networkinterfaces.go (right):

https://codereview.appspot.com/83070047/diff/40001/state/networkinterfaces.go#newcode69
state/networkinterfaces.go:69: func removeNetworkInterfacesOps(st
*State, machineId string) []txn.Op {
On 2014/04/04 09:14:43, dimitern wrote:
> On 2014/04/03 16:37:09, fwereade wrote:
> > This is racy in isolation -- if we move it into the machine's Remove
method,
> > though, I think we're good. (with the check-machine-alive assert in
> > AddNetworkInterface, anyway)

> I don't quite follow - removeNetworkInterfacesOps() is called in
> machine.Remove() and run with the other []txn.Op already. Adding an
assert on
> machine.Life is only needed to keep dying or dead machines from having
NICs
> added (which can't happen right now as NICs are immutable and only
changed after
> provisioning), but if NICs can change at run-time (as can
> HardwareCharacteristics btw), that sounds exactly like a job for the
Networker
> worker we'll have after MVP.

I'd quite like that assert-alive-on-add to be in place anyway, because
it'll be way too easy to forget when we make this dynamic.

And this method in particular will be too easy to just call when we add
Remove -- but if we move it onto Machine and include a dead-check at the
start we at least put a little roadblock in the way of thoughtless later
refactoring.

> As with MachineNetworks, I think dropping Remove() entirely here, for
this CL is
> perhaps what we need.

+1 to that. But please...

Read more...

Revision history for this message
Dimiter Naydenov (dimitern) wrote :
Download full text (5.0 KiB)

Please take a look.

https://codereview.appspot.com/83070047/diff/40001/state/networkinterfaces.go
File state/networkinterfaces.go (right):

https://codereview.appspot.com/83070047/diff/40001/state/networkinterfaces.go#newcode69
state/networkinterfaces.go:69: func removeNetworkInterfacesOps(st
*State, machineId string) []txn.Op {
On 2014/04/04 09:56:22, fwereade wrote:
> On 2014/04/04 09:14:43, dimitern wrote:
> > On 2014/04/03 16:37:09, fwereade wrote:
> > > This is racy in isolation -- if we move it into the machine's
Remove method,
> > > though, I think we're good. (with the check-machine-alive assert
in
> > > AddNetworkInterface, anyway)
> >
> > I don't quite follow - removeNetworkInterfacesOps() is called in
> > machine.Remove() and run with the other []txn.Op already. Adding an
assert on
> > machine.Life is only needed to keep dying or dead machines from
having NICs
> > added (which can't happen right now as NICs are immutable and only
changed
> after
> > provisioning), but if NICs can change at run-time (as can
> > HardwareCharacteristics btw), that sounds exactly like a job for the
Networker
> > worker we'll have after MVP.

> I'd quite like that assert-alive-on-add to be in place anyway, because
it'll be
> way too easy to forget when we make this dynamic.

> And this method in particular will be too easy to just call when we
add Remove
> -- but if we move it onto Machine and include a dead-check at the
start we at
> least put a little roadblock in the way of thoughtless later
refactoring.

> > As with MachineNetworks, I think dropping Remove() entirely here,
for this CL
> is
> > perhaps what we need.

> +1 to that. But please make this func a Machine method, and add the
alive
> assert.

Alive assert is already in place, moving this to a machine method.

https://codereview.appspot.com/83070047/diff/60001/state/addmachine.go
File state/addmachine.go (right):

https://codereview.appspot.com/83070047/diff/60001/state/addmachine.go#newcode425
state/addmachine.go:425: // TODO(dimitern) Once we can add networks
independently
On 2014/04/04 09:56:23, fwereade wrote:
> and a bug please :)

Done.

https://codereview.appspot.com/83070047/diff/60001/state/machine.go
File state/machine.go (right):

https://codereview.appspot.com/83070047/diff/60001/state/machine.go#newcode567
state/machine.go:567: // Remove sets the machine to Dead. It will fail
if the machine is not
On 2014/04/04 09:56:23, fwereade wrote:
> Nah, this one removes it from state (and requires that it's already
dead before
> it's called).

Done.

https://codereview.appspot.com/83070047/diff/60001/state/machine.go#newcode897
state/machine.go:897: func (m *Machine) Networks() ([]*Network, error) {
On 2014/04/04 09:56:23, fwereade wrote:
> Mildly forward-looking quibble, doesn't need to be done now: do we
need this? I
> presume it's being done for use in status; but should we not perhaps
actually be
> showing the machine's addresses *annotated* with what network that
address is
> on? ie we will want to combine the info from NetworkInterfaces with
that from
> Addresses and show that, and have a separate top-level section in
status for
> Networks?

For now we can't really link IP address...

Read more...

Revision history for this message
Dimiter Naydenov (dimitern) wrote :
Revision history for this message
William Reade (fwereade) wrote :

LGTM, one last quibble

https://codereview.appspot.com/83070047/diff/60001/state/machine.go
File state/machine.go (right):

https://codereview.appspot.com/83070047/diff/60001/state/machine.go#newcode941
state/machine.go:941: aliveAndNotProvisioned := append(isAliveDoc,
bson.D{{"nonce", ""}}...)
On 2014/04/04 11:12:19, dimitern wrote:
> On 2014/04/04 09:56:23, fwereade wrote:
> > I think this is fine, assuming we have the provisioner pass the
interfaces
> into
> > SetProvisioned, and inside the APi server call AddNetworkInterface
before
> > SetProvisioned?

> I was thinking of having AddNetworkInterface on the Provisioner API
later and
> call it as needed before calling SetProvisioned (and changing machine
state to
> error if it happens, like we do after StartInstance errors).

> Is there a particular reason you want NICs passed to SetProvisioned?

(1) chatty APIs suck (2) the quicker we can get all the instance info
away from the provisioner and into the api server the better.

...in fact ...I'm almost tempted to say that this functionality should
be of a transaction with the SetProvisioned method in state *anyway* --
consider what happens if a machine has a NIC set up but then the process
fails before SetProvisioned.

That can be a minor refactoring when you hook up the provisioner,
though.

https://codereview.appspot.com/83070047/diff/80001/state/machine.go
File state/machine.go (right):

https://codereview.appspot.com/83070047/diff/80001/state/machine.go#newcode567
state/machine.go:567: func (m *Machine) removeNetworkInterfacesOps()
([]txn.Op, error) {
just add a quick pedantic check that the machine really is Dead in here
too

https://codereview.appspot.com/83070047/

Revision history for this message
Dimiter Naydenov (dimitern) wrote :

Please take a look.

https://codereview.appspot.com/83070047/diff/60001/state/machine.go
File state/machine.go (right):

https://codereview.appspot.com/83070047/diff/60001/state/machine.go#newcode941
state/machine.go:941: aliveAndNotProvisioned := append(isAliveDoc,
bson.D{{"nonce", ""}}...)
On 2014/04/04 12:10:03, fwereade wrote:
> On 2014/04/04 11:12:19, dimitern wrote:
> > On 2014/04/04 09:56:23, fwereade wrote:
> > > I think this is fine, assuming we have the provisioner pass the
interfaces
> > into
> > > SetProvisioned, and inside the APi server call AddNetworkInterface
before
> > > SetProvisioned?
> >
> > I was thinking of having AddNetworkInterface on the Provisioner API
later and
> > call it as needed before calling SetProvisioned (and changing
machine state to
> > error if it happens, like we do after StartInstance errors).
> >
> > Is there a particular reason you want NICs passed to SetProvisioned?

> (1) chatty APIs suck (2) the quicker we can get all the instance info
away from
> the provisioner and into the api server the better.

> ...in fact ...I'm almost tempted to say that this functionality should
be of a
> transaction with the SetProvisioned method in state *anyway* --
consider what
> happens if a machine has a NIC set up but then the process fails
before
> SetProvisioned.

> That can be a minor refactoring when you hook up the provisioner,
though.

Yes, I agree provisioning should be atomic, but it's not right now
because of StartInstance and SetProvisioned getting called in sequence
(and adding a couple of extra API calls will make it only a bit worse).
I'm in favor of fixing it in general, but this is out of scope for now.

https://codereview.appspot.com/83070047/diff/80001/state/machine.go
File state/machine.go (right):

https://codereview.appspot.com/83070047/diff/80001/state/machine.go#newcode567
state/machine.go:567: func (m *Machine) removeNetworkInterfacesOps()
([]txn.Op, error) {
On 2014/04/04 12:10:03, fwereade wrote:
> just add a quick pedantic check that the machine really is Dead in
here too

Done inside machine.Remove().

https://codereview.appspot.com/83070047/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'juju/conn_test.go'
2--- juju/conn_test.go 2014-03-26 10:41:38 +0000
3+++ juju/conn_test.go 2014-04-04 13:09:14 +0000
4@@ -398,12 +398,12 @@
5 c.Assert(sch.Revision(), gc.Equals, rev+1)
6 }
7
8-func (s *ConnSuite) assertAssignedMachineNetworks(c *gc.C, unit *state.Unit, expectInclude, expectExclude []string) {
9+func (s *ConnSuite) assertAssignedMachineRequestedNetworks(c *gc.C, unit *state.Unit, expectInclude, expectExclude []string) {
10 machineId, err := unit.AssignedMachineId()
11 c.Assert(err, gc.IsNil)
12 machine, err := s.conn.State.Machine(machineId)
13 c.Assert(err, gc.IsNil)
14- include, exclude, err := machine.Networks()
15+ include, exclude, err := machine.RequestedNetworks()
16 c.Assert(err, gc.IsNil)
17 c.Assert(include, jc.DeepEquals, expectInclude)
18 c.Assert(exclude, jc.DeepEquals, expectExclude)
19@@ -420,8 +420,8 @@
20 units, err := juju.AddUnits(s.conn.State, svc, 2, "")
21 c.Assert(err, gc.IsNil)
22 c.Assert(units, gc.HasLen, 2)
23- s.assertAssignedMachineNetworks(c, units[0], withNets, withoutNets)
24- s.assertAssignedMachineNetworks(c, units[1], withNets, withoutNets)
25+ s.assertAssignedMachineRequestedNetworks(c, units[0], withNets, withoutNets)
26+ s.assertAssignedMachineRequestedNetworks(c, units[1], withNets, withoutNets)
27
28 id0, err := units[0].AssignedMachineId()
29 c.Assert(err, gc.IsNil)
30@@ -434,19 +434,19 @@
31
32 units, err = juju.AddUnits(s.conn.State, svc, 1, "0")
33 c.Assert(err, gc.IsNil)
34- s.assertAssignedMachineNetworks(c, units[0], withNets, withoutNets)
35+ s.assertAssignedMachineRequestedNetworks(c, units[0], withNets, withoutNets)
36 id2, err := units[0].AssignedMachineId()
37 c.Assert(id2, gc.Equals, id0)
38
39 units, err = juju.AddUnits(s.conn.State, svc, 1, "lxc:0")
40 c.Assert(err, gc.IsNil)
41- s.assertAssignedMachineNetworks(c, units[0], withNets, withoutNets)
42+ s.assertAssignedMachineRequestedNetworks(c, units[0], withNets, withoutNets)
43 id3, err := units[0].AssignedMachineId()
44 c.Assert(id3, gc.Equals, id0+"/lxc/0")
45
46 units, err = juju.AddUnits(s.conn.State, svc, 1, "lxc:"+id3)
47 c.Assert(err, gc.IsNil)
48- s.assertAssignedMachineNetworks(c, units[0], withNets, withoutNets)
49+ s.assertAssignedMachineRequestedNetworks(c, units[0], withNets, withoutNets)
50 id4, err := units[0].AssignedMachineId()
51 c.Assert(id4, gc.Equals, id0+"/lxc/0/lxc/0")
52
53
54=== modified file 'state/addmachine.go'
55--- state/addmachine.go 2014-04-03 10:09:20 +0000
56+++ state/addmachine.go 2014-04-04 13:09:14 +0000
57@@ -422,7 +422,11 @@
58 createStatusOp(st, machineGlobalKey(mdoc.Id), statusDoc{
59 Status: params.StatusPending,
60 }),
61- createNetworksOp(st, machineGlobalKey(mdoc.Id),
62+ // TODO(dimitern) 2014-04-04 bug #1302498
63+ // Once we can add networks independently of machine
64+ // provisioning, we should check the given networks are valid
65+ // and known before setting them.
66+ createRequestedNetworksOp(st, machineGlobalKey(mdoc.Id),
67 template.IncludeNetworks,
68 template.ExcludeNetworks,
69 ),
70
71=== modified file 'state/api/provisioner/machine.go'
72--- state/api/provisioner/machine.go 2014-04-03 02:57:11 +0000
73+++ state/api/provisioner/machine.go 2014-04-04 13:09:14 +0000
74@@ -55,12 +55,12 @@
75 return nil
76 }
77
78-// Networks returns a pair of lists of networks to enable/disable on
79-// the machine.
80-func (m *Machine) Networks() (includeNetworks, excludeNetworks []string, err error) {
81+// RequestedNetworks returns a pair of lists of networks to
82+// enable/disable on the machine.
83+func (m *Machine) RequestedNetworks() (includeNetworks, excludeNetworks []string, err error) {
84 var results params.NetworksResults
85 args := params.Entities{Entities: []params.Entity{{m.tag}}}
86- err = m.st.call("Networks", args, &results)
87+ err = m.st.call("RequestedNetworks", args, &results)
88 if err != nil {
89 return nil, nil, err
90 }
91
92=== modified file 'state/api/provisioner/provisioner_test.go'
93--- state/api/provisioner/provisioner_test.go 2014-04-03 02:57:11 +0000
94+++ state/api/provisioner/provisioner_test.go 2014-04-04 13:09:14 +0000
95@@ -329,7 +329,7 @@
96
97 apiMachine, err := s.provisioner.Machine(netsMachine.Tag())
98 c.Assert(err, gc.IsNil)
99- includeNetworks, excludeNetworks, err := apiMachine.Networks()
100+ includeNetworks, excludeNetworks, err := apiMachine.RequestedNetworks()
101 c.Assert(err, gc.IsNil)
102 c.Assert(includeNetworks, gc.DeepEquals, template.IncludeNetworks)
103 c.Assert(excludeNetworks, gc.DeepEquals, template.ExcludeNetworks)
104@@ -337,7 +337,7 @@
105 // Now try machine 0.
106 apiMachine, err = s.provisioner.Machine(s.machine.Tag())
107 c.Assert(err, gc.IsNil)
108- includeNetworks, excludeNetworks, err = apiMachine.Networks()
109+ includeNetworks, excludeNetworks, err = apiMachine.RequestedNetworks()
110 c.Assert(err, gc.IsNil)
111 c.Assert(includeNetworks, gc.HasLen, 0)
112 c.Assert(excludeNetworks, gc.HasLen, 0)
113
114=== modified file 'state/apiserver/provisioner/provisioner.go'
115--- state/apiserver/provisioner/provisioner.go 2014-04-03 02:57:11 +0000
116+++ state/apiserver/provisioner/provisioner.go 2014-04-04 13:09:14 +0000
117@@ -407,8 +407,8 @@
118 return result, nil
119 }
120
121-// Networks returns the networks for each given machine entity.
122-func (p *ProvisionerAPI) Networks(args params.Entities) (params.NetworksResults, error) {
123+// RequestedNetworks returns the requested networks for each given machine entity.
124+func (p *ProvisionerAPI) RequestedNetworks(args params.Entities) (params.NetworksResults, error) {
125 result := params.NetworksResults{
126 Results: make([]params.NetworkResult, len(args.Entities)),
127 }
128@@ -421,7 +421,7 @@
129 if err == nil {
130 var includeNetworks []string
131 var excludeNetworks []string
132- includeNetworks, excludeNetworks, err = machine.Networks()
133+ includeNetworks, excludeNetworks, err = machine.RequestedNetworks()
134 if err == nil {
135 result.Results[i].IncludeNetworks = includeNetworks
136 result.Results[i].ExcludeNetworks = excludeNetworks
137
138=== modified file 'state/apiserver/provisioner/provisioner_test.go'
139--- state/apiserver/provisioner/provisioner_test.go 2014-04-03 02:57:11 +0000
140+++ state/apiserver/provisioner/provisioner_test.go 2014-04-04 13:09:14 +0000
141@@ -769,7 +769,7 @@
142 netsMachine, err := s.State.AddOneMachine(template)
143 c.Assert(err, gc.IsNil)
144
145- includeNetsMachine0, excludeNetsMachine0, err := s.machines[0].Networks()
146+ includeNetsMachine0, excludeNetsMachine0, err := s.machines[0].RequestedNetworks()
147 c.Assert(err, gc.IsNil)
148
149 args := params.Entities{Entities: []params.Entity{
150@@ -779,7 +779,7 @@
151 {Tag: "unit-foo-0"},
152 {Tag: "service-bar"},
153 }}
154- result, err := s.provisioner.Networks(args)
155+ result, err := s.provisioner.RequestedNetworks(args)
156 c.Assert(err, gc.IsNil)
157 c.Assert(result, gc.DeepEquals, params.NetworksResults{
158 Results: []params.NetworkResult{
159
160=== modified file 'state/assign_test.go'
161--- state/assign_test.go 2014-03-26 10:41:38 +0000
162+++ state/assign_test.go 2014-04-04 13:09:14 +0000
163@@ -329,7 +329,7 @@
164 // Check that the principal is set on the machine.
165 machine, err := s.State.Machine(machineId)
166 c.Assert(err, gc.IsNil)
167- include, exclude, err := machine.Networks()
168+ include, exclude, err := machine.RequestedNetworks()
169 c.Assert(err, gc.IsNil)
170 c.Assert(include, gc.DeepEquals, includeNetworks)
171 c.Assert(exclude, gc.DeepEquals, excludeNetworks)
172
173=== modified file 'state/cleanup.go'
174--- state/cleanup.go 2014-03-24 13:37:25 +0000
175+++ state/cleanup.go 2014-04-04 13:09:14 +0000
176@@ -154,9 +154,6 @@
177 if err := st.cleanupContainers(machine); err != nil {
178 return err
179 }
180- if err := st.cleanupNetworks(machineGlobalKey(machineId)); err != nil {
181- return err
182- }
183 for _, unitName := range machine.doc.Principals {
184 if err := st.obliterateUnit(unitName); err != nil {
185 return err
186@@ -209,16 +206,6 @@
187 return nil
188 }
189
190-// cleanupNetworks removes associated networks for a machine or
191-// service, given by its global key.
192-func (st *State) cleanupNetworks(globalKey string) error {
193- op := removeNetworksOp(st, globalKey)
194- if err := st.runTransaction([]txn.Op{op}); err != nil {
195- logger.Warningf("cannot remove networks document for %q: %v", globalKey, err)
196- }
197- return nil
198-}
199-
200 // obliterateUnit removes a unit from state completely. It is not safe or
201 // sane to obliterate any unit in isolation; its only reasonable use is in
202 // the context of machine obliteration, in which we can be sure that unclean
203
204=== modified file 'state/compat_test.go'
205--- state/compat_test.go 2014-03-25 16:12:23 +0000
206+++ state/compat_test.go 2014-04-04 13:09:14 +0000
207@@ -75,9 +75,9 @@
208 service, err := s.state.AddService("mysql", "user-admin", charm, nil, nil)
209 c.Assert(err, gc.IsNil)
210 // In 1.17.7+ all services have associated document in the
211- // networks collection. We remove it here to test backwards
212- // compatibility.
213- ops := []txn.Op{removeNetworksOp(s.state, service.globalKey())}
214+ // requested networks collection. We remove it here to test
215+ // backwards compatibility.
216+ ops := []txn.Op{removeRequestedNetworksOp(s.state, service.globalKey())}
217 err = s.state.runTransaction(ops)
218 c.Assert(err, gc.IsNil)
219
220@@ -88,18 +88,18 @@
221 c.Assert(exclude, gc.HasLen, 0)
222 }
223
224-func (s *compatSuite) TestGetMachineWithoutNetworksIsOK(c *gc.C) {
225+func (s *compatSuite) TestGetMachineWithoutRequestedNetworksIsOK(c *gc.C) {
226 machine, err := s.state.AddMachine("quantal", JobHostUnits)
227 c.Assert(err, gc.IsNil)
228 // In 1.17.7+ all machines have associated document in the
229- // networks collection. We remove it here to test backwards
230- // compatibility.
231- ops := []txn.Op{removeNetworksOp(s.state, machine.globalKey())}
232+ // requested networks collection. We remove it here to test
233+ // backwards compatibility.
234+ ops := []txn.Op{removeRequestedNetworksOp(s.state, machine.globalKey())}
235 err = s.state.runTransaction(ops)
236 c.Assert(err, gc.IsNil)
237
238 // Now check the trying to fetch machine's networks is OK.
239- include, exclude, err := machine.Networks()
240+ include, exclude, err := machine.RequestedNetworks()
241 c.Assert(err, gc.IsNil)
242 c.Assert(include, gc.HasLen, 0)
243 c.Assert(exclude, gc.HasLen, 0)
244
245=== modified file 'state/machine.go'
246--- state/machine.go 2014-04-04 00:12:12 +0000
247+++ state/machine.go 2014-04-04 13:09:14 +0000
248@@ -5,6 +5,7 @@
249
250 import (
251 "fmt"
252+ "net"
253 "strings"
254 "time"
255
256@@ -563,8 +564,28 @@
257 return fmt.Errorf("machine %s cannot advance lifecycle: %v", m, ErrExcessiveContention)
258 }
259
260-// Remove removes the machine from state. It will fail if the machine is not
261-// Dead.
262+func (m *Machine) removeNetworkInterfacesOps() ([]txn.Op, error) {
263+ var doc struct {
264+ MACAddress string `bson:"_id"`
265+ }
266+ ops := []txn.Op{}
267+ sel := bson.D{{"machineid", m.doc.Id}}
268+ iter := m.st.networkInterfaces.Find(sel).Select(bson.D{{"_id", 1}}).Iter()
269+ for iter.Next(&doc) {
270+ ops = append(ops, txn.Op{
271+ C: m.st.networkInterfaces.Name,
272+ Id: doc.MACAddress,
273+ Remove: true,
274+ })
275+ }
276+ if err := iter.Err(); err != nil {
277+ return nil, err
278+ }
279+ return ops, nil
280+}
281+
282+// Remove removes the machine from state. It will fail if the machine
283+// is not Dead.
284 func (m *Machine) Remove() (err error) {
285 defer utils.ErrorContextf(&err, "cannot remove machine %s", m.doc.Id)
286 if m.doc.Life != Dead {
287@@ -578,15 +599,25 @@
288 Remove: true,
289 },
290 {
291+ C: m.st.machines.Name,
292+ Id: m.doc.Id,
293+ Assert: isDeadDoc,
294+ },
295+ {
296 C: m.st.instanceData.Name,
297 Id: m.doc.Id,
298 Remove: true,
299 },
300 removeStatusOp(m.st, m.globalKey()),
301 removeConstraintsOp(m.st, m.globalKey()),
302- removeNetworksOp(m.st, m.globalKey()),
303+ removeRequestedNetworksOp(m.st, m.globalKey()),
304 annotationRemoveOp(m.st, m.globalKey()),
305 }
306+ ifacesOps, err := m.removeNetworkInterfacesOps()
307+ if err != nil {
308+ return err
309+ }
310+ ops = append(ops, ifacesOps...)
311 ops = append(ops, removeContainerRefOps(m.st, m.Id())...)
312 // The only abort conditions in play indicate that the machine has already
313 // been removed.
314@@ -886,10 +917,105 @@
315 return nil
316 }
317
318-// Networks returns the list of networks the machine should be on
319-// (includeNetworks) or not (excludeNetworks).
320-func (m *Machine) Networks() (includeNetworks, excludeNetworks []string, err error) {
321- return readNetworks(m.st, m.globalKey())
322+// RequestedNetworks returns the list of networks the machine should
323+// be on (includeNetworks) or not (excludeNetworks).
324+func (m *Machine) RequestedNetworks() (includeNetworks, excludeNetworks []string, err error) {
325+ return readRequestedNetworks(m.st, m.globalKey())
326+}
327+
328+// Networks returns the list of configured networks on the machine.
329+func (m *Machine) Networks() ([]*Network, error) {
330+ includeNetworks, _, err := m.RequestedNetworks()
331+ if err != nil {
332+ return nil, err
333+ }
334+ docs := []networkDoc{}
335+ sel := bson.D{{"_id", bson.D{{"$in", includeNetworks}}}}
336+ err = m.st.networks.Find(sel).All(&docs)
337+ if err != nil {
338+ return nil, err
339+ }
340+ networks := make([]*Network, len(docs))
341+ for i, doc := range docs {
342+ networks[i] = newNetwork(m.st, &doc)
343+ }
344+ return networks, nil
345+}
346+
347+// NetworkInterfaces returns the list of configured network interfaces
348+// of the machine.
349+func (m *Machine) NetworkInterfaces() ([]*NetworkInterface, error) {
350+ docs := []networkInterfaceDoc{}
351+ err := m.st.networkInterfaces.Find(bson.D{{"machineid", m.doc.Id}}).All(&docs)
352+ if err != nil {
353+ return nil, err
354+ }
355+ ifaces := make([]*NetworkInterface, len(docs))
356+ for i, doc := range docs {
357+ ifaces[i] = newNetworkInterface(m.st, &doc)
358+ }
359+ return ifaces, nil
360+}
361+
362+// AddNetworkInterface creates a new network interface on the given
363+// network for the machine. The machine must be alive and not yet
364+// provisioned, and there must be no other interface with the same MAC
365+// address for this to succeed.
366+func (m *Machine) AddNetworkInterface(macAddress, name, networkName string) (iface *NetworkInterface, err error) {
367+ defer utils.ErrorContextf(&err, "cannot add network interface to machine %s", m.doc.Id)
368+
369+ if _, err := net.ParseMAC(macAddress); err != nil {
370+ return nil, err
371+ }
372+ if name == "" {
373+ return nil, fmt.Errorf("interface name must be not empty")
374+ }
375+ aliveAndNotProvisioned := append(isAliveDoc, bson.D{{"nonce", ""}}...)
376+ doc := &networkInterfaceDoc{
377+ MACAddress: macAddress,
378+ InterfaceName: name,
379+ NetworkName: networkName,
380+ MachineId: m.doc.Id,
381+ }
382+ ops := []txn.Op{{
383+ C: m.st.networks.Name,
384+ Id: networkName,
385+ Assert: txn.DocExists,
386+ }, {
387+ C: m.st.machines.Name,
388+ Id: m.doc.Id,
389+ Assert: aliveAndNotProvisioned,
390+ }, {
391+ C: m.st.networkInterfaces.Name,
392+ Id: macAddress,
393+ Assert: txn.DocMissing,
394+ Insert: doc,
395+ }}
396+
397+ for i := 0; i < 5; i++ {
398+ err = m.st.runTransaction(ops)
399+ switch err {
400+ case txn.ErrAborted:
401+ _, err = m.st.Network(networkName)
402+ if err != nil {
403+ return nil, err
404+ }
405+ if err = m.Refresh(); err != nil {
406+ return nil, err
407+ } else if m.doc.Life != Alive {
408+ return nil, fmt.Errorf("machine is not alive")
409+ } else if m.doc.Nonce != "" {
410+ return nil, fmt.Errorf("machine already provisioned: dynamic network interfaces not currently supported")
411+ }
412+ // Network and machine both OK, so the other assert must have failed.
413+ return nil, fmt.Errorf("interface with MAC address %q already exists", macAddress)
414+ case nil:
415+ return newNetworkInterface(m.st, doc), nil
416+ default:
417+ return nil, err
418+ }
419+ }
420+ return nil, ErrExcessiveContention
421 }
422
423 // CheckProvisioned returns true if the machine was provisioned with the given nonce.
424
425=== modified file 'state/machine_test.go'
426--- state/machine_test.go 2014-04-01 23:56:19 +0000
427+++ state/machine_test.go 2014-04-04 13:09:14 +0000
428@@ -4,6 +4,7 @@
429 package state_test
430
431 import (
432+ "fmt"
433 "sort"
434
435 jc "github.com/juju/testing/checkers"
436@@ -223,10 +224,13 @@
437 c.Assert(err, jc.Satisfies, errors.IsNotFoundError)
438 _, err = s.machine.Containers()
439 c.Assert(err, jc.Satisfies, errors.IsNotFoundError)
440- include, exclude, err := s.machine.Networks()
441+ include, exclude, err := s.machine.RequestedNetworks()
442 c.Assert(err, gc.IsNil)
443 c.Assert(include, gc.HasLen, 0)
444 c.Assert(exclude, gc.HasLen, 0)
445+ ifaces, err := s.machine.NetworkInterfaces()
446+ c.Assert(err, gc.IsNil)
447+ c.Assert(ifaces, gc.HasLen, 0)
448 err = s.machine.Remove()
449 c.Assert(err, gc.IsNil)
450 }
451@@ -352,10 +356,10 @@
452 c.Assert(alive, gc.Equals, false)
453 }
454
455-func (s *MachineSuite) TestMachineNetworks(c *gc.C) {
456- // s.machine is created without networks, so check
457+func (s *MachineSuite) TestRequestedNetworks(c *gc.C) {
458+ // s.machine is created without requested networks, so check
459 // they're empty when we read them.
460- include, exclude, err := s.machine.Networks()
461+ include, exclude, err := s.machine.RequestedNetworks()
462 c.Assert(err, gc.IsNil)
463 c.Assert(include, gc.HasLen, 0)
464 c.Assert(exclude, gc.HasLen, 0)
465@@ -368,7 +372,7 @@
466 ExcludeNetworks: []string{"private-net", "logging"},
467 })
468 c.Assert(err, gc.IsNil)
469- include, exclude, err = machine.Networks()
470+ include, exclude, err = machine.RequestedNetworks()
471 c.Assert(err, gc.IsNil)
472 c.Assert(include, jc.DeepEquals, []string{"net1", "mynet"})
473 c.Assert(exclude, jc.DeepEquals, []string{"private-net", "logging"})
474@@ -378,12 +382,146 @@
475 c.Assert(err, gc.IsNil)
476 err = machine.Remove()
477 c.Assert(err, gc.IsNil)
478- include, exclude, err = machine.Networks()
479+ include, exclude, err = machine.RequestedNetworks()
480 c.Assert(err, gc.IsNil)
481 c.Assert(include, gc.HasLen, 0)
482 c.Assert(exclude, gc.HasLen, 0)
483 }
484
485+func addNetworkAndInterface(c *gc.C, st *state.State, machine *state.Machine,
486+ networkName, cidr string, vlanTag int,
487+ mac, ifaceName string) (*state.Network, *state.NetworkInterface) {
488+ net, err := st.AddNetwork(networkName, cidr, vlanTag)
489+ c.Assert(err, gc.IsNil)
490+ iface, err := machine.AddNetworkInterface(mac, ifaceName, networkName)
491+ c.Assert(err, gc.IsNil)
492+ return net, iface
493+}
494+
495+func (s *MachineSuite) TestNetworks(c *gc.C) {
496+ // s.machine is created without networks, so check
497+ // they're empty when we read them.
498+ nets, err := s.machine.Networks()
499+ c.Assert(err, gc.IsNil)
500+ c.Assert(nets, gc.HasLen, 0)
501+
502+ // Now create a testing machine with requested networks, because
503+ // Networks() uses them to determine which networks are bound to
504+ // the machine.
505+ machine, err := s.State.AddOneMachine(state.MachineTemplate{
506+ Series: "quantal",
507+ Jobs: []state.MachineJob{state.JobHostUnits},
508+ IncludeNetworks: []string{"net1", "net2"},
509+ ExcludeNetworks: []string{"net3", "net4"},
510+ })
511+ c.Assert(err, gc.IsNil)
512+
513+ net1, _ := addNetworkAndInterface(
514+ c, s.State, machine,
515+ "net1", "0.1.2.0/24", 0,
516+ "aa:bb:cc:dd:ee:f0", "eth0")
517+ net2, _ := addNetworkAndInterface(
518+ c, s.State, machine,
519+ "net2", "0.2.2.0/24", 0,
520+ "aa:bb:cc:dd:ee:f1", "eth1")
521+
522+ nets, err = machine.Networks()
523+ c.Assert(err, gc.IsNil)
524+ c.Assert(nets, jc.DeepEquals, []*state.Network{net1, net2})
525+}
526+
527+func (s *MachineSuite) TestMachineNetworkInterfaces(c *gc.C) {
528+ // s.machine is created without network interfaces, so check
529+ // they're empty when we read them.
530+ ifaces, err := s.machine.NetworkInterfaces()
531+ c.Assert(err, gc.IsNil)
532+ c.Assert(ifaces, gc.HasLen, 0)
533+
534+ machine, err := s.State.AddOneMachine(state.MachineTemplate{
535+ Series: "quantal",
536+ Jobs: []state.MachineJob{state.JobHostUnits},
537+ IncludeNetworks: []string{"net1", "vlan42", "net2"},
538+ })
539+ c.Assert(err, gc.IsNil)
540+
541+ // And a few networks and NICs.
542+ _, iface0 := addNetworkAndInterface(
543+ c, s.State, machine,
544+ "net1", "0.1.2.0/24", 0,
545+ "aa:bb:cc:dd:ee:f0", "eth0")
546+ _, iface1 := addNetworkAndInterface(
547+ c, s.State, machine,
548+ "vlan42", "0.1.2.0/30", 42,
549+ "aa:bb:cc:dd:ee:f1", "eth0.42")
550+ _, iface2 := addNetworkAndInterface(
551+ c, s.State, machine,
552+ "net2", "0.2.2.0/24", 0,
553+ "aa:bb:cc:dd:ee:f2", "eth1")
554+
555+ ifaces, err = machine.NetworkInterfaces()
556+ c.Assert(err, gc.IsNil)
557+ c.Assert(ifaces, jc.DeepEquals, []*state.NetworkInterface{
558+ iface0, iface1, iface2,
559+ })
560+
561+ // Make sure interfaces get removed with the machine.
562+ err = machine.EnsureDead()
563+ c.Assert(err, gc.IsNil)
564+ err = machine.Remove()
565+ c.Assert(err, gc.IsNil)
566+ ifaces, err = machine.NetworkInterfaces()
567+ c.Assert(err, gc.IsNil)
568+ c.Assert(ifaces, gc.HasLen, 0)
569+}
570+
571+func (s *MachineSuite) TestAddNetworkInterfaceErrors(c *gc.C) {
572+ machine, err := s.State.AddOneMachine(state.MachineTemplate{
573+ Series: "quantal",
574+ Jobs: []state.MachineJob{state.JobHostUnits},
575+ IncludeNetworks: []string{"net1"},
576+ })
577+ c.Assert(err, gc.IsNil)
578+ addNetworkAndInterface(
579+ c, s.State, machine,
580+ "net1", "0.1.2.0/24", 0,
581+ "aa:bb:cc:dd:ee:f0", "eth0")
582+
583+ errorPrefix := fmt.Sprintf("cannot add network interface to machine %s: ", machine.Id())
584+ _, err = machine.AddNetworkInterface("invalid", "eth0", "net1")
585+ expectErr := errorPrefix + "invalid MAC address: invalid"
586+ c.Assert(err, gc.ErrorMatches, expectErr)
587+
588+ _, err = machine.AddNetworkInterface("aa:bb:cc:dd:ee:ff", "", "net1")
589+ expectErr = errorPrefix + "interface name must be not empty"
590+ c.Assert(err, gc.ErrorMatches, expectErr)
591+
592+ _, err = machine.AddNetworkInterface("aa:bb:cc:dd:ee:f0", "eth2", "net1")
593+ expectErr = errorPrefix + `interface with MAC address "aa:bb:cc:dd:ee:f0" already exists`
594+ c.Assert(err, gc.ErrorMatches, expectErr)
595+
596+ _, err = machine.AddNetworkInterface("aa:bb:cc:dd:ee:f0", "eth2", "invalid")
597+ expectErr = errorPrefix + `network "invalid" not found`
598+ c.Assert(err, gc.ErrorMatches, expectErr)
599+
600+ err = machine.SetProvisioned("i-am", "fake_nonce", nil)
601+ c.Assert(err, gc.IsNil)
602+ _, err = machine.AddNetworkInterface("aa:bb:cc:dd:ee:ff", "eth2", "net1")
603+ expectErr = errorPrefix + "machine already provisioned: dynamic network interfaces not currently supported"
604+ c.Assert(err, gc.ErrorMatches, expectErr)
605+
606+ err = machine.EnsureDead()
607+ c.Assert(err, gc.IsNil)
608+ _, err = machine.AddNetworkInterface("aa:bb:cc:dd:ee:ff", "eth2", "net1")
609+ expectErr = errorPrefix + "machine is not alive"
610+ c.Assert(err, gc.ErrorMatches, expectErr)
611+
612+ err = machine.Remove()
613+ c.Assert(err, gc.IsNil)
614+ _, err = machine.AddNetworkInterface("aa:bb:cc:dd:ee:ff", "eth2", "net1")
615+ expectErr = errorPrefix + fmt.Sprintf("machine %s not found", machine.Id())
616+ c.Assert(err, gc.ErrorMatches, expectErr)
617+}
618+
619 func (s *MachineSuite) TestMachineInstanceId(c *gc.C) {
620 machine, err := s.State.AddMachine("quantal", state.JobHostUnits)
621 c.Assert(err, gc.IsNil)
622
623=== added file 'state/networkinterfaces.go'
624--- state/networkinterfaces.go 1970-01-01 00:00:00 +0000
625+++ state/networkinterfaces.go 2014-04-04 13:09:14 +0000
626@@ -0,0 +1,45 @@
627+// Copyright 2014 Canonical Ltd.
628+// Licensed under the AGPLv3, see LICENCE file for details.
629+
630+package state
631+
632+// NetworkInterface represents the state of a machine network
633+// interface.
634+type NetworkInterface struct {
635+ st *State
636+ doc networkInterfaceDoc
637+}
638+
639+// networkInterfaceDoc represents a network interface for a machine on
640+// a given network.
641+type networkInterfaceDoc struct {
642+ MACAddress string `bson:"_id"`
643+ // InterfaceName is the network interface name (e.g. "eth0").
644+ InterfaceName string
645+ NetworkName string
646+ MachineId string
647+}
648+
649+func newNetworkInterface(st *State, doc *networkInterfaceDoc) *NetworkInterface {
650+ return &NetworkInterface{st, *doc}
651+}
652+
653+// MACAddress returns the MAC address of the interface.
654+func (ni *NetworkInterface) MACAddress() string {
655+ return ni.doc.MACAddress
656+}
657+
658+// InterfaceName returns the name of the interface.
659+func (ni *NetworkInterface) InterfaceName() string {
660+ return ni.doc.InterfaceName
661+}
662+
663+// NetworkName returns the machine network name of the interface.
664+func (ni *NetworkInterface) NetworkName() string {
665+ return ni.doc.NetworkName
666+}
667+
668+// MachineId returns the machine id of the interface.
669+func (ni *NetworkInterface) MachineId() string {
670+ return ni.doc.MachineId
671+}
672
673=== added file 'state/networkinterfaces_test.go'
674--- state/networkinterfaces_test.go 1970-01-01 00:00:00 +0000
675+++ state/networkinterfaces_test.go 2014-04-04 13:09:14 +0000
676@@ -0,0 +1,37 @@
677+// Copyright 2014 Canonical Ltd.
678+// Licensed under the AGPLv3, see LICENCE file for details.
679+
680+package state_test
681+
682+import (
683+ gc "launchpad.net/gocheck"
684+
685+ "launchpad.net/juju-core/state"
686+)
687+
688+type NetworkInterfaceSuite struct {
689+ ConnSuite
690+ machine *state.Machine
691+ network *state.Network
692+ iface *state.NetworkInterface
693+}
694+
695+var _ = gc.Suite(&NetworkInterfaceSuite{})
696+
697+func (s *NetworkInterfaceSuite) SetUpTest(c *gc.C) {
698+ s.ConnSuite.SetUpTest(c)
699+ var err error
700+ s.machine, err = s.State.AddMachine("quantal", state.JobHostUnits)
701+ c.Assert(err, gc.IsNil)
702+ s.network, err = s.State.AddNetwork("net1", "0.1.2.3/24", 42)
703+ c.Assert(err, gc.IsNil)
704+ s.iface, err = s.machine.AddNetworkInterface("aa:bb:cc:dd:ee:ff", "eth0", "net1")
705+ c.Assert(err, gc.IsNil)
706+}
707+
708+func (s *NetworkInterfaceSuite) TestGetterMethods(c *gc.C) {
709+ c.Assert(s.iface.MACAddress(), gc.Equals, "aa:bb:cc:dd:ee:ff")
710+ c.Assert(s.iface.InterfaceName(), gc.Equals, "eth0")
711+ c.Assert(s.iface.NetworkName(), gc.Equals, s.network.Name())
712+ c.Assert(s.iface.MachineId(), gc.Equals, s.machine.Id())
713+}
714
715=== added file 'state/networks.go'
716--- state/networks.go 1970-01-01 00:00:00 +0000
717+++ state/networks.go 2014-04-04 13:09:14 +0000
718@@ -0,0 +1,68 @@
719+// Copyright 2014 Canonical Ltd.
720+// Licensed under the AGPLv3, see LICENCE file for details.
721+
722+package state
723+
724+import (
725+ "labix.org/v2/mgo/bson"
726+)
727+
728+// Network represents the state of a network.
729+type Network struct {
730+ st *State
731+ doc networkDoc
732+}
733+
734+// networkDoc represents a configured network that a machine can be a
735+// part of.
736+type networkDoc struct {
737+ // Name is the network's name. It should be one of the machine's
738+ // included networks.
739+ Name string `bson:"_id"`
740+ // CIDR holds the network CIDR in the form 192.168.100.0/24.
741+ CIDR string
742+ // VLANTag needs to be between 1 and 4094 for VLANs and 0 for
743+ // normal networks.
744+ VLANTag int
745+}
746+
747+func newNetwork(st *State, doc *networkDoc) *Network {
748+ return &Network{st, *doc}
749+}
750+
751+// Name returns the network name.
752+func (n *Network) Name() string {
753+ return n.doc.Name
754+}
755+
756+// CIDR returns the network CIDR (e.g. 192.168.50.0/24).
757+func (n *Network) CIDR() string {
758+ return n.doc.CIDR
759+}
760+
761+// VLANTag returns the network VLAN tag. Its a number between 1 and
762+// 4094 for VLANs and 0 if the network is not a VLAN.
763+func (n *Network) VLANTag() int {
764+ return n.doc.VLANTag
765+}
766+
767+// IsVLAN returns whether the network is a VLAN (has tag > 0) or a
768+// normal network.
769+func (n *Network) IsVLAN() bool {
770+ return n.doc.VLANTag > 0
771+}
772+
773+// Interfaces returns all network interfaces on the network.
774+func (n *Network) Interfaces() ([]*NetworkInterface, error) {
775+ docs := []networkInterfaceDoc{}
776+ sel := bson.D{{"networkname", n.doc.Name}}
777+ err := n.st.networkInterfaces.Find(sel).All(&docs)
778+ if err != nil {
779+ return nil, err
780+ }
781+ ifaces := make([]*NetworkInterface, len(docs))
782+ for i, doc := range docs {
783+ ifaces[i] = newNetworkInterface(n.st, &doc)
784+ }
785+ return ifaces, nil
786+}
787
788=== added file 'state/networks_test.go'
789--- state/networks_test.go 1970-01-01 00:00:00 +0000
790+++ state/networks_test.go 2014-04-04 13:09:14 +0000
791@@ -0,0 +1,56 @@
792+// Copyright 2014 Canonical Ltd.
793+// Licensed under the AGPLv3, see LICENCE file for details.
794+
795+package state_test
796+
797+import (
798+ jc "github.com/juju/testing/checkers"
799+ gc "launchpad.net/gocheck"
800+
801+ "launchpad.net/juju-core/state"
802+)
803+
804+type NetworkSuite struct {
805+ ConnSuite
806+ machine *state.Machine
807+ network *state.Network
808+ vlan *state.Network
809+}
810+
811+var _ = gc.Suite(&NetworkSuite{})
812+
813+func (s *NetworkSuite) SetUpTest(c *gc.C) {
814+ s.ConnSuite.SetUpTest(c)
815+ var err error
816+ s.machine, err = s.State.AddMachine("quantal", state.JobHostUnits)
817+ c.Assert(err, gc.IsNil)
818+ s.network, err = s.State.AddNetwork("net1", "0.1.2.3/24", 0)
819+ c.Assert(err, gc.IsNil)
820+ s.vlan, err = s.State.AddNetwork("vlan", "0.1.2.3/30", 42)
821+ c.Assert(err, gc.IsNil)
822+}
823+
824+func (s *NetworkSuite) TestGetterMethods(c *gc.C) {
825+ c.Assert(s.network.Name(), gc.Equals, "net1")
826+ c.Assert(s.network.CIDR(), gc.Equals, "0.1.2.3/24")
827+ c.Assert(s.network.VLANTag(), gc.Equals, 0)
828+ c.Assert(s.vlan.VLANTag(), gc.Equals, 42)
829+ c.Assert(s.network.IsVLAN(), jc.IsFalse)
830+ c.Assert(s.vlan.IsVLAN(), jc.IsTrue)
831+}
832+
833+func (s *NetworkSuite) TestInterfaces(c *gc.C) {
834+ ifaces, err := s.network.Interfaces()
835+ c.Assert(err, gc.IsNil)
836+ c.Assert(ifaces, gc.HasLen, 0)
837+
838+ iface0, err := s.machine.AddNetworkInterface("aa:bb:cc:dd:ee:f0", "eth0", "net1")
839+ c.Assert(err, gc.IsNil)
840+ iface1, err := s.machine.AddNetworkInterface("aa:bb:cc:dd:ee:f1", "eth1", "net1")
841+ c.Assert(err, gc.IsNil)
842+
843+ ifaces, err = s.network.Interfaces()
844+ c.Assert(err, gc.IsNil)
845+ c.Assert(ifaces, gc.HasLen, 2)
846+ c.Assert(ifaces, jc.DeepEquals, []*state.NetworkInterface{iface0, iface1})
847+}
848
849=== modified file 'state/open.go'
850--- state/open.go 2014-03-26 18:36:39 +0000
851+++ state/open.go 2014-04-04 13:09:14 +0000
852@@ -191,6 +191,8 @@
853 {"units", []string{"principal"}},
854 {"units", []string{"machineid"}},
855 {"users", []string{"name"}},
856+ {"networkinterfaces", []string{"networkname"}},
857+ {"networkinterfaces", []string{"machineid"}},
858 }
859
860 // The capped collection used for transaction logs defaults to 10MB.
861@@ -245,29 +247,31 @@
862 }
863 }
864 st := &State{
865- info: info,
866- policy: policy,
867- db: db,
868- environments: db.C("environments"),
869- charms: db.C("charms"),
870- machines: db.C("machines"),
871- containerRefs: db.C("containerRefs"),
872- instanceData: db.C("instanceData"),
873- relations: db.C("relations"),
874- relationScopes: db.C("relationscopes"),
875- services: db.C("services"),
876- networks: db.C("linkednetworks"),
877- minUnits: db.C("minunits"),
878- settings: db.C("settings"),
879- settingsrefs: db.C("settingsrefs"),
880- constraints: db.C("constraints"),
881- units: db.C("units"),
882- users: db.C("users"),
883- presence: pdb.C("presence"),
884- cleanups: db.C("cleanups"),
885- annotations: db.C("annotations"),
886- statuses: db.C("statuses"),
887- stateServers: db.C("stateServers"),
888+ info: info,
889+ policy: policy,
890+ db: db,
891+ environments: db.C("environments"),
892+ charms: db.C("charms"),
893+ machines: db.C("machines"),
894+ containerRefs: db.C("containerRefs"),
895+ instanceData: db.C("instanceData"),
896+ relations: db.C("relations"),
897+ relationScopes: db.C("relationscopes"),
898+ services: db.C("services"),
899+ requestedNetworks: db.C("linkednetworks"),
900+ networks: db.C("networks"),
901+ networkInterfaces: db.C("networkinterfaces"),
902+ minUnits: db.C("minunits"),
903+ settings: db.C("settings"),
904+ settingsrefs: db.C("settingsrefs"),
905+ constraints: db.C("constraints"),
906+ units: db.C("units"),
907+ users: db.C("users"),
908+ presence: pdb.C("presence"),
909+ cleanups: db.C("cleanups"),
910+ annotations: db.C("annotations"),
911+ statuses: db.C("statuses"),
912+ stateServers: db.C("stateServers"),
913 }
914 log := db.C("txns.log")
915 logInfo := mgo.CollectionInfo{Capped: true, MaxBytes: logSize}
916
917=== renamed file 'state/networks.go' => 'state/requestednetworks.go'
918--- state/networks.go 2014-03-25 09:15:00 +0000
919+++ state/requestednetworks.go 2014-04-04 13:09:14 +0000
920@@ -8,45 +8,48 @@
921 "labix.org/v2/mgo/txn"
922 )
923
924-// networksDoc represents the network restrictions for a service or machine.
925-// The document ID field is the globalKey of a service or a machine.
926-type networksDoc struct {
927+// requestedNetworksDoc represents the network restrictions for a
928+// service or machine. The document ID field is the globalKey of a
929+// service or a machine.
930+type requestedNetworksDoc struct {
931+ Id string `bson:"_id"`
932 IncludeNetworks []string `bson:"include"`
933 ExcludeNetworks []string `bson:"exclude"`
934 }
935
936-func newNetworksDoc(includeNetworks, excludeNetworks []string) *networksDoc {
937- return &networksDoc{
938+func newRequestedNetworksDoc(includeNetworks, excludeNetworks []string) *requestedNetworksDoc {
939+ return &requestedNetworksDoc{
940 IncludeNetworks: includeNetworks,
941 ExcludeNetworks: excludeNetworks,
942 }
943 }
944
945-func createNetworksOp(st *State, id string, includeNetworks, excludeNetworks []string) txn.Op {
946+func createRequestedNetworksOp(st *State, id string, includeNetworks, excludeNetworks []string) txn.Op {
947 return txn.Op{
948- C: st.networks.Name,
949+ C: st.requestedNetworks.Name,
950 Id: id,
951 Assert: txn.DocMissing,
952- Insert: newNetworksDoc(includeNetworks, excludeNetworks),
953+ Insert: newRequestedNetworksDoc(includeNetworks, excludeNetworks),
954 }
955 }
956
957 // While networks are immutable, there is no setNetworksOp function.
958
959-func removeNetworksOp(st *State, id string) txn.Op {
960+func removeRequestedNetworksOp(st *State, id string) txn.Op {
961 return txn.Op{
962- C: st.networks.Name,
963+ C: st.requestedNetworks.Name,
964 Id: id,
965 Remove: true,
966 }
967 }
968
969-func readNetworks(st *State, id string) (includeNetworks, excludeNetworks []string, err error) {
970- doc := networksDoc{}
971- if err = st.networks.FindId(id).One(&doc); err == mgo.ErrNotFound {
972- // In 1.17.7+ we always create a networksDoc for each service or
973- // machine we create, but in legacy databases this is not the
974- // case. We ignore the error here for backwards-compatibility.
975+func readRequestedNetworks(st *State, id string) (includeNetworks, excludeNetworks []string, err error) {
976+ doc := requestedNetworksDoc{}
977+ if err = st.requestedNetworks.FindId(id).One(&doc); err == mgo.ErrNotFound {
978+ // In 1.17.7+ we always create a requestedNetworksDoc for each
979+ // service or machine we create, but in legacy databases this
980+ // is not the case. We ignore the error here for
981+ // backwards-compatibility.
982 err = nil
983 } else if err == nil {
984 includeNetworks = doc.IncludeNetworks
985
986=== modified file 'state/service.go'
987--- state/service.go 2014-03-25 09:15:00 +0000
988+++ state/service.go 2014-04-04 13:09:14 +0000
989@@ -224,7 +224,7 @@
990 Id: s.settingsKey(),
991 Remove: true,
992 }}
993- ops = append(ops, removeNetworksOp(s.st, s.globalKey()))
994+ ops = append(ops, removeRequestedNetworksOp(s.st, s.globalKey()))
995 ops = append(ops, removeConstraintsOp(s.st, s.globalKey()))
996 return append(ops, annotationRemoveOp(s.st, s.globalKey()))
997 }
998@@ -824,7 +824,7 @@
999
1000 // Networks returns the networks a service is associated with.
1001 func (s *Service) Networks() (includeNetworks, excludeNetworks []string, err error) {
1002- return readNetworks(s.st, s.globalKey())
1003+ return readRequestedNetworks(s.st, s.globalKey())
1004 }
1005
1006 // settingsIncRefOp returns an operation that increments the ref count
1007
1008=== modified file 'state/state.go'
1009--- state/state.go 2014-04-03 02:57:11 +0000
1010+++ state/state.go 2014-04-04 13:09:14 +0000
1011@@ -8,6 +8,7 @@
1012
1013 import (
1014 "fmt"
1015+ "net"
1016 "net/url"
1017 "regexp"
1018 "sort"
1019@@ -44,33 +45,35 @@
1020 // State represents the state of an environment
1021 // managed by juju.
1022 type State struct {
1023- info *Info
1024- policy Policy
1025- db *mgo.Database
1026- environments *mgo.Collection
1027- charms *mgo.Collection
1028- machines *mgo.Collection
1029- instanceData *mgo.Collection
1030- containerRefs *mgo.Collection
1031- relations *mgo.Collection
1032- relationScopes *mgo.Collection
1033- services *mgo.Collection
1034- networks *mgo.Collection
1035- minUnits *mgo.Collection
1036- settings *mgo.Collection
1037- settingsrefs *mgo.Collection
1038- constraints *mgo.Collection
1039- units *mgo.Collection
1040- users *mgo.Collection
1041- presence *mgo.Collection
1042- cleanups *mgo.Collection
1043- annotations *mgo.Collection
1044- statuses *mgo.Collection
1045- stateServers *mgo.Collection
1046- runner *txn.Runner
1047- transactionHooks chan ([]transactionHook)
1048- watcher *watcher.Watcher
1049- pwatcher *presence.Watcher
1050+ info *Info
1051+ policy Policy
1052+ db *mgo.Database
1053+ environments *mgo.Collection
1054+ charms *mgo.Collection
1055+ machines *mgo.Collection
1056+ instanceData *mgo.Collection
1057+ containerRefs *mgo.Collection
1058+ relations *mgo.Collection
1059+ relationScopes *mgo.Collection
1060+ services *mgo.Collection
1061+ requestedNetworks *mgo.Collection
1062+ networks *mgo.Collection
1063+ networkInterfaces *mgo.Collection
1064+ minUnits *mgo.Collection
1065+ settings *mgo.Collection
1066+ settingsrefs *mgo.Collection
1067+ constraints *mgo.Collection
1068+ units *mgo.Collection
1069+ users *mgo.Collection
1070+ presence *mgo.Collection
1071+ cleanups *mgo.Collection
1072+ annotations *mgo.Collection
1073+ statuses *mgo.Collection
1074+ stateServers *mgo.Collection
1075+ runner *txn.Runner
1076+ transactionHooks chan ([]transactionHook)
1077+ watcher *watcher.Watcher
1078+ pwatcher *presence.Watcher
1079 // mu guards allManager.
1080 mu sync.Mutex
1081 allManager *multiwatcher.StoreManager
1082@@ -952,7 +955,11 @@
1083 ops := []txn.Op{
1084 env.assertAliveOp(),
1085 createConstraintsOp(st, svc.globalKey(), constraints.Value{}),
1086- createNetworksOp(st, svc.globalKey(), includeNetworks, excludeNetworks),
1087+ // TODO(dimitern) 2014-04-04 bug #1302498
1088+ // Once we can add networks independently of machine
1089+ // provisioning, we should check the given networks are valid
1090+ // and known before setting them.
1091+ createRequestedNetworksOp(st, svc.globalKey(), includeNetworks, excludeNetworks),
1092 createSettingsOp(st, svc.settingsKey(), nil),
1093 {
1094 C: st.users.Name,
1095@@ -1003,6 +1010,52 @@
1096 return svc, nil
1097 }
1098
1099+// AddNetwork creates a new network with the given name, CIDR and VLAN tag.
1100+func (st *State) AddNetwork(name, cidr string, vlanTag int) (n *Network, err error) {
1101+ defer utils.ErrorContextf(&err, "cannot add network %q", name)
1102+ if cidr != "" {
1103+ _, _, err := net.ParseCIDR(cidr)
1104+ if err != nil {
1105+ return nil, err
1106+ }
1107+ }
1108+ if name == "" {
1109+ return nil, fmt.Errorf("name must be not empty")
1110+ }
1111+ if vlanTag < 0 || vlanTag > 4094 {
1112+ return nil, fmt.Errorf("invalid VLAN tag %d: must be between 0 and 4094", vlanTag)
1113+ }
1114+ doc := &networkDoc{
1115+ Name: name,
1116+ CIDR: cidr,
1117+ VLANTag: vlanTag,
1118+ }
1119+ ops := []txn.Op{{
1120+ C: st.networks.Name,
1121+ Id: name,
1122+ Assert: txn.DocMissing,
1123+ Insert: doc,
1124+ }}
1125+ err = onAbort(st.runTransaction(ops), fmt.Errorf("already exists"))
1126+ if err != nil {
1127+ return nil, err
1128+ }
1129+ return newNetwork(st, doc), nil
1130+}
1131+
1132+// Network returns the network with the given name.
1133+func (st *State) Network(name string) (*Network, error) {
1134+ doc := &networkDoc{}
1135+ err := st.networks.FindId(name).One(doc)
1136+ if err == mgo.ErrNotFound {
1137+ return nil, errors.NotFoundf("network %q", name)
1138+ }
1139+ if err != nil {
1140+ return nil, fmt.Errorf("cannot get network %q: %v", name, err)
1141+ }
1142+ return newNetwork(st, doc), nil
1143+}
1144+
1145 // Service returns a service state by name.
1146 func (st *State) Service(name string) (service *Service, err error) {
1147 if !names.IsService(name) {
1148
1149=== modified file 'state/state_test.go'
1150--- state/state_test.go 2014-04-03 02:57:11 +0000
1151+++ state/state_test.go 2014-04-04 13:09:14 +0000
1152@@ -962,6 +962,44 @@
1153 }
1154 }
1155
1156+func (s *StateSuite) TestAddAndGetNetwork(c *gc.C) {
1157+ machine, err := s.State.AddOneMachine(state.MachineTemplate{
1158+ Series: "quantal",
1159+ Jobs: []state.MachineJob{state.JobHostUnits},
1160+ IncludeNetworks: []string{"net1", "net2"},
1161+ ExcludeNetworks: []string{"net3", "net4"},
1162+ })
1163+ c.Assert(err, gc.IsNil)
1164+
1165+ net1, _ := addNetworkAndInterface(
1166+ c, s.State, machine,
1167+ "net1", "0.1.2.0/24", 0,
1168+ "aa:bb:cc:dd:ee:f0", "eth0")
1169+
1170+ net, err := s.State.Network("net1")
1171+ c.Assert(err, gc.IsNil)
1172+ c.Assert(net, gc.DeepEquals, net1)
1173+ _, err = s.State.Network("missing")
1174+ c.Assert(err, jc.Satisfies, errors.IsNotFoundError)
1175+ c.Assert(err, gc.ErrorMatches, `network "missing" not found`)
1176+
1177+ _, err = s.State.AddNetwork("", "0.3.1.0/24", 0)
1178+ expectErr := `cannot add network "": name must be not empty`
1179+ c.Assert(err, gc.ErrorMatches, expectErr)
1180+ _, err = s.State.AddNetwork("net42", "invalid", 0)
1181+ expectErr = `cannot add network "net42": invalid CIDR address: invalid`
1182+ c.Assert(err, gc.ErrorMatches, expectErr)
1183+ _, err = s.State.AddNetwork("net69", "0.3.1.0/30", -1)
1184+ expectErr = `cannot add network "net69": invalid VLAN tag -1: must be between 0 and 4094`
1185+ c.Assert(err, gc.ErrorMatches, expectErr)
1186+ _, err = s.State.AddNetwork("net69", "0.3.1.0/30", 9999)
1187+ expectErr = `cannot add network "net69": invalid VLAN tag 9999: must be between 0 and 4094`
1188+ c.Assert(err, gc.ErrorMatches, expectErr)
1189+ _, err = s.State.AddNetwork("net1", "0.1.2.0/24", 0)
1190+ expectErr = `cannot add network "net1": already exists`
1191+ c.Assert(err, gc.ErrorMatches, expectErr)
1192+}
1193+
1194 func (s *StateSuite) TestAddService(c *gc.C) {
1195 charm := s.AddTestingCharm(c, "dummy")
1196 _, err := s.State.AddService("haha/borken", "user-admin", charm, nil, nil)
1197
1198=== modified file 'state/statecmd/machineconfig.go'
1199--- state/statecmd/machineconfig.go 2014-04-01 16:27:22 +0000
1200+++ state/statecmd/machineconfig.go 2014-04-04 13:09:14 +0000
1201@@ -74,8 +74,8 @@
1202 return nil, err
1203 }
1204
1205- // Find networks.
1206- includeNetworks, excludeNetworks, err := machine.Networks()
1207+ // Find requested networks.
1208+ includeNetworks, excludeNetworks, err := machine.RequestedNetworks()
1209 if err != nil {
1210 return nil, err
1211 }
1212
1213=== modified file 'worker/provisioner/provisioner_task.go'
1214--- worker/provisioner/provisioner_task.go 2014-04-03 02:57:11 +0000
1215+++ worker/provisioner/provisioner_task.go 2014-04-04 13:09:14 +0000
1216@@ -491,7 +491,7 @@
1217 if err != nil {
1218 return nil, err
1219 }
1220- includeNetworks, excludeNetworks, err := machine.Networks()
1221+ includeNetworks, excludeNetworks, err := machine.RequestedNetworks()
1222 if err != nil {
1223 return nil, err
1224 }
1225
1226=== modified file 'worker/provisioner/provisioner_test.go'
1227--- worker/provisioner/provisioner_test.go 2014-04-03 04:46:51 +0000
1228+++ worker/provisioner/provisioner_test.go 2014-04-04 13:09:14 +0000
1229@@ -344,10 +344,10 @@
1230 }
1231
1232 func (s *CommonProvisionerSuite) addMachine() (*state.Machine, error) {
1233- return s.addMachineWithNetworks(nil, nil)
1234+ return s.addMachineWithRequestedNetworks(nil, nil)
1235 }
1236
1237-func (s *CommonProvisionerSuite) addMachineWithNetworks(includeNetworks, excludeNetworks []string) (*state.Machine, error) {
1238+func (s *CommonProvisionerSuite) addMachineWithRequestedNetworks(includeNetworks, excludeNetworks []string) (*state.Machine, error) {
1239 return s.BackingState.AddOneMachine(state.MachineTemplate{
1240 Series: coretesting.FakeDefaultSeries,
1241 Jobs: []state.MachineJob{state.JobHostUnits},
1242@@ -459,14 +459,14 @@
1243 s.waitRemoved(c, m)
1244 }
1245
1246-func (s *ProvisionerSuite) TestProvisioningMachinesWithNetworks(c *gc.C) {
1247+func (s *ProvisionerSuite) TestProvisioningMachinesWithRequestedNetworks(c *gc.C) {
1248 p := s.newEnvironProvisioner(c)
1249 defer stop(c, p)
1250
1251 // Add and provision a machine with networks specified.
1252 includeNetworks := []string{"net1", "net2"}
1253 excludeNetworks := []string{"net3", "net4"}
1254- m, err := s.addMachineWithNetworks(includeNetworks, excludeNetworks)
1255+ m, err := s.addMachineWithRequestedNetworks(includeNetworks, excludeNetworks)
1256 c.Assert(err, gc.IsNil)
1257 inst := s.checkStartInstanceCustom(c, m, "pork", s.defaultConstraints, includeNetworks, excludeNetworks)
1258

Subscribers

People subscribed via source and target branches

to status/vote changes: