Merge lp:~dimitern/juju-core/400-lp-1307513-multiple-nics-same-mac into lp:~go-bot/juju-core/trunk
- 400-lp-1307513-multiple-nics-same-mac
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Dimiter Naydenov |
Approved revision: | no longer in the source branch. |
Merged at revision: | 2655 |
Proposed branch: | lp:~dimitern/juju-core/400-lp-1307513-multiple-nics-same-mac |
Merge into: | lp:~go-bot/juju-core/trunk |
Diff against target: |
1776 lines (+456/-256) 31 files modified
environs/broker.go (+2/-27) environs/network/network.go (+37/-0) juju/testing/instance.go (+4/-3) provider/azure/environ.go (+2/-1) provider/common/bootstrap_test.go (+5/-4) provider/common/mock_test.go (+3/-2) provider/dummy/environs.go (+13/-11) provider/ec2/ec2.go (+2/-1) provider/joyent/environ_instance.go (+2/-1) provider/local/environ.go (+2/-1) provider/maas/environ.go (+36/-39) provider/maas/environ_test.go (+20/-2) provider/manual/environ.go (+2/-1) provider/openstack/provider.go (+2/-1) state/api/params/internal.go (+6/-1) state/api/provisioner/provisioner_test.go (+31/-11) state/apiserver/provisioner/provisioner.go (+1/-0) state/apiserver/provisioner/provisioner_test.go (+34/-10) state/machine.go (+47/-35) state/machine_test.go (+43/-29) state/networkinterfaces.go (+46/-6) state/networkinterfaces_test.go (+11/-2) state/networks.go (+22/-3) state/networks_test.go (+15/-5) state/open.go (+1/-0) state/state.go (+19/-25) state/state_test.go (+27/-21) worker/provisioner/kvm-broker.go (+2/-1) worker/provisioner/lxc-broker.go (+2/-1) worker/provisioner/provisioner_task.go (+6/-4) worker/provisioner/provisioner_test.go (+11/-8) |
To merge this branch: | bzr merge lp:~dimitern/juju-core/400-lp-1307513-multiple-nics-same-mac |
Related bugs: | |
Related blueprints: |
Support MaaS VLANs in Juju
(Essential)
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Juju Engineering | Pending | ||
Review via email: mp+216453@code.launchpad.net |
Commit message
various: Support multiple NICs with the same MAC
Fixes bug #1307513, allowing more than one network
interface with the same MAC address, and introduces
the concept of virtual and physical NIC.
This is needed because you can have more than one
network interface with the same MAC address on the
same machine (but it has to be on different networks):
one physical and multiple virtual NICs. Changes were
needed mostly in state to use a different primary
key for networkinterfaces collection. Also added a
unique index on "interfacename" and "networkname",
improved tests.
Fixed the MAAS provider to configure networks properly,
so that you can have more than one VLAN virtual NIC
on the same physical NIC.
Introduced environs/network package and defined an
Id type to make it apparent that network ids are
provider-specific (like instance.Id). Also moved
environs.
In a follow-up, machine addresses and networks will
be integrated, so we can link a network interface's
name and MAC address to its network (IP) address.
Finally, networks and machine interfaces will be
visible in juju status.
https:/
R=fwereade
Description of the change
various: Support multiple NICs with the same MAC
Fixes bug #1307513, allowing more than one network
interface with the same MAC address, and introduces
the concept of virtual and physical NIC.
This is needed because you can have more than one
network interface with the same MAC address on the
same machine (but it has to be on different networks):
one physical and multiple virtual NICs. Changes were
needed mostly in state to use a different primary
key for networkinterfaces collection. Also added a
unique index on "interfacename" and "networkname",
improved tests.
Fixed the MAAS provider to configure networks properly,
so that you can have more than one VLAN virtual NIC
on the same physical NIC.
Introduced environs/network package and defined an
Id type to make it apparent that network ids are
provider-specific (like instance.Id). Also moved
environs.
In a follow-up, machine addresses and networks will
be integrated, so we can link a network interface's
name and MAC address to its network (IP) address.
Finally, networks and machine interfaces will be
visible in juju status.
Dimiter Naydenov (dimitern) wrote : | # |
William Reade (fwereade) wrote : | # |
mostly looking good, but (1) IsVirtual on network still seems ill-formed
to me; and (2) are we explicitly ignoring compatibility with earlier
1.19? these new fields in state worry me a little, the people who want
to work with networks will likely be upgrading from there...
https:/
File environs/broker.go (right):
https:/
environs/
We could make this clearer if we had a type for it, like instance.Id --
as it is I think ProviderId is not much of a win over NetworkId. But
`NetworkId NetworkId` should be less confusing.
https:/
File state/api/
https:/
state/api/
I'm less certain about this one. If it's possible for virtual/physical
interfaces to be mixed, this is meaningless; if it's not possible, it's
not necessary on the interface definition. Right?
https:/
File state/machine.go (right):
https:/
state/machine.
hey, I'm feeling more strongly about this now: please assert that the
machine is Dead in this method, just so that if (when) someone tries to
use it from elsewhere they get something pointing out the race with
adding interfaces.
https:/
state/machine.
interface")
This is on the state server, let's not panic even if it should never
happen.
Dimiter Naydenov (dimitern) wrote : | # |
On 2014/04/18 14:41:17, fwereade wrote:
> mostly looking good, but (1) IsVirtual on network still seems
ill-formed to me;
> and (2) are we explicitly ignoring compatibility with earlier 1.19?
these new
> fields in state worry me a little, the people who want to work with
networks
> will likely be upgrading from there...
1) I'll drop IsVirtual from networks and keep it on interfaces only.
2) 1.19.0 is a dev release which was out recently, so I think we can get
away with saying "if you used networks, please redeploy your environment
when upgrading to 1.19.1" - I'll add a note to the bug and ping sinzui.
Dimiter Naydenov (dimitern) wrote : | # |
Please take a look.
https:/
File environs/broker.go (right):
https:/
environs/
On 2014/04/18 14:41:18, fwereade wrote:
> We could make this clearer if we had a type for it, like instance.Id
-- as it is
> I think ProviderId is not much of a win over NetworkId. But `NetworkId
> NetworkId` should be less confusing.
I wanted to make the code use consistent naming - NetworkId is only used
here. I like the idea of type NetworkId string and then perhaps use
ProviderId NetworkId as a compromise?
https:/
File state/api/
https:/
state/api/
On 2014/04/18 14:41:18, fwereade wrote:
> I'm less certain about this one. If it's possible for virtual/physical
> interfaces to be mixed, this is meaningless; if it's not possible,
it's not
> necessary on the interface definition. Right?
Frankly, I don't know whether it's possible to mix physical and virtual
interfaces on the same network, perhaps it is. But the actual flag makes
more sense for NICs, rather than networks.
I'll drop it from networks and leave it only on interfaces.
https:/
File state/machine.go (right):
https:/
state/machine.
On 2014/04/18 14:41:18, fwereade wrote:
> hey, I'm feeling more strongly about this now: please assert that the
machine is
> Dead in this method, just so that if (when) someone tries to use it
from
> elsewhere they get something pointing out the race with adding
interfaces.
Done.
https:/
state/machine.
interface")
On 2014/04/18 14:41:18, fwereade wrote:
> This is on the state server, let's not panic even if it should never
happen.
I'll log an error instead.
William Reade (fwereade) wrote : | # |
Thanks, LGTM with trivials.
https:/
File environs/broker.go (right):
https:/
environs/
On 2014/04/18 15:37:26, dimitern wrote:
> On 2014/04/18 14:41:18, fwereade wrote:
> > We could make this clearer if we had a type for it, like instance.Id
-- as it
> is
> > I think ProviderId is not much of a win over NetworkId. But
`NetworkId
> > NetworkId` should be less confusing.
> I wanted to make the code use consistent naming - NetworkId is only
used here. I
> like the idea of type NetworkId string and then perhaps use ProviderId
NetworkId
> as a compromise?
Yeah, that works for me. Thanks.
https:/
File state/machine.go (right):
https:/
state/machine.
On 2014/04/18 15:37:27, dimitern wrote:
> On 2014/04/18 14:41:18, fwereade wrote:
> > hey, I'm feeling more strongly about this now: please assert that
the machine
> is
> > Dead in this method, just so that if (when) someone tries to use it
from
> > elsewhere they get something pointing out the race with adding
interfaces.
> Done.
Sorry, I expressed myself poorly. The op is nice, and could/should
happily stay (other invariants *should* make it unnecessary, but +1 to
defence in depth), but what I'm really looking for is a quick check
against m.doc.Life so we can abort with an error before even running the
txn.
https:/
File state/api/
https:/
state/api/
hmm, instance.NetworkId doesn't feel quite right -- I'd say
environs.NetworkId, with the expectation that soon enough we'll want an
environs/network package and we can just call it network.Id.
Or if you want to start the package with a single type, and start
gradually migrating stuff over to it as you go in future CLs, that would
also work for me.
https:/
File state/machine.go (right):
https:/
state/machine.
m.st.networkInt
.One(&doc) perhaps? I suspect you're grabbing the data anyway, even if
you're not storing it, so you may as well return a doc that definitely
completely matches what's in the db.
I know it doesn't make a difference *now* but it feels like an
encouragement to subtle bugs.
Dimiter Naydenov (dimitern) wrote : | # |
Please take a look.
https:/
File state/machine.go (right):
https:/
state/machine.
On 2014/04/18 16:02:54, fwereade wrote:
> On 2014/04/18 15:37:27, dimitern wrote:
> > On 2014/04/18 14:41:18, fwereade wrote:
> > > hey, I'm feeling more strongly about this now: please assert that
the
> machine
> > is
> > > Dead in this method, just so that if (when) someone tries to use
it from
> > > elsewhere they get something pointing out the race with adding
interfaces.
> >
> > Done.
> Sorry, I expressed myself poorly. The op is nice, and could/should
happily stay
> (other invariants *should* make it unnecessary, but +1 to defence in
depth), but
> what I'm really looking for is a quick check against m.doc.Life so we
can abort
> with an error before even running the txn.
As discussed, I'm adding a check for m.doc.Life == Dead at the
beginning.
https:/
File state/api/
https:/
state/api/
On 2014/04/18 16:02:54, fwereade wrote:
> hmm, instance.NetworkId doesn't feel quite right -- I'd say
environs.NetworkId,
> with the expectation that soon enough we'll want an environs/network
package and
> we can just call it network.Id.
> Or if you want to start the package with a single type, and start
gradually
> migrating stuff over to it as you go in future CLs, that would also
work for me.
I tried adding it to environs, but there are import loops. I like the
idea of having environs/network package, so I added that and moved
environs.
https:/
File state/machine.go (right):
https:/
state/machine.
m.st.networkInt
On 2014/04/18 16:02:54, fwereade wrote:
> .One(&doc) perhaps? I suspect you're grabbing the data anyway, even if
you're
> not storing it, so you may as well return a doc that definitely
completely
> matches what's in the db.
> I know it doesn't make a difference *now* but it feels like an
encouragement to
> subtle bugs.
Done.
Dimiter Naydenov (dimitern) wrote : | # |
Please take a look.
https:/
File state/machine.go (right):
https:/
state/machine.
On 2014/04/18 16:02:54, fwereade wrote:
> On 2014/04/18 15:37:27, dimitern wrote:
> > On 2014/04/18 14:41:18, fwereade wrote:
> > > hey, I'm feeling more strongly about this now: please assert that
the
> machine
> > is
> > > Dead in this method, just so that if (when) someone tries to use
it from
> > > elsewhere they get something pointing out the race with adding
interfaces.
> >
> > Done.
> Sorry, I expressed myself poorly. The op is nice, and could/should
happily stay
> (other invariants *should* make it unnecessary, but +1 to defence in
depth), but
> what I'm really looking for is a quick check against m.doc.Life so we
can abort
> with an error before even running the txn.
As discussed, I'm adding a check for m.doc.Life == Dead at the
beginning.
https:/
File state/api/
https:/
state/api/
On 2014/04/18 16:02:54, fwereade wrote:
> hmm, instance.NetworkId doesn't feel quite right -- I'd say
environs.NetworkId,
> with the expectation that soon enough we'll want an environs/network
package and
> we can just call it network.Id.
> Or if you want to start the package with a single type, and start
gradually
> migrating stuff over to it as you go in future CLs, that would also
work for me.
I tried adding it to environs, but there are import loops. I like the
idea of having environs/network package, so I added that and moved
environs.
https:/
File state/machine.go (right):
https:/
state/machine.
m.st.networkInt
On 2014/04/18 16:02:54, fwereade wrote:
> .One(&doc) perhaps? I suspect you're grabbing the data anyway, even if
you're
> not storing it, so you may as well return a doc that definitely
completely
> matches what's in the db.
> I know it doesn't make a difference *now* but it feels like an
encouragement to
> subtle bugs.
Done.
Go Bot (go-bot) wrote : | # |
The attempt to merge lp:~dimitern/juju-core/400-lp-1307513-multiple-nics-same-mac into lp:juju-core failed. Below is the output from the failed tests.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
? launchpad.
? launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
? launchpad.
? launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
? launchpad.
ok launchpad.
? launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
? launchpad.
ok launchpad.
? launchpad.
? launchpad.
? launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
? launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
? launchpad.
ok launchpad.
? launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
? launchpad.
ok launchpad.
ok launchpad.
? launchpad.
Preview Diff
1 | === modified file 'environs/broker.go' |
2 | --- environs/broker.go 2014-04-11 13:54:51 +0000 |
3 | +++ environs/broker.go 2014-04-18 16:46:53 +0000 |
4 | @@ -6,6 +6,7 @@ |
5 | import ( |
6 | "launchpad.net/juju-core/constraints" |
7 | "launchpad.net/juju-core/environs/cloudinit" |
8 | + "launchpad.net/juju-core/environs/network" |
9 | "launchpad.net/juju-core/instance" |
10 | "launchpad.net/juju-core/tools" |
11 | ) |
12 | @@ -33,32 +34,6 @@ |
13 | DistributionGroup func() ([]instance.Id, error) |
14 | } |
15 | |
16 | -// NetworkInfo describes a single network interface available on an |
17 | -// instance. For providers that support networks, this will be |
18 | -// available at StartInstance() time. |
19 | -type NetworkInfo struct { |
20 | - // MACAddress is the network interface's hardware MAC address |
21 | - // (e.g. "aa:bb:cc:dd:ee:ff"). |
22 | - MACAddress string |
23 | - |
24 | - // CIDR of the network, in 123.45.67.89/24 format. |
25 | - CIDR string |
26 | - |
27 | - // NetworkName is juju-internal name of the network. |
28 | - NetworkName string |
29 | - |
30 | - // NetworkId is a provider-specific network id. |
31 | - NetworkId string |
32 | - |
33 | - // VLANTag needs to be between 1 and 4094 for VLANs and 0 for |
34 | - // normal networks. It's defined by IEEE 802.1Q standard. |
35 | - VLANTag int |
36 | - |
37 | - // InterfaceName is the OS-specific network device name (e.g. |
38 | - // "eth0" or "eth1.42" for a VLAN virtual interface). |
39 | - InterfaceName string |
40 | -} |
41 | - |
42 | // TODO(wallyworld) - we want this in the environs/instance package but import loops |
43 | // stop that from being possible right now. |
44 | type InstanceBroker interface { |
45 | @@ -68,7 +43,7 @@ |
46 | // unique within an environment, is used by juju to protect against the |
47 | // consequences of multiple instances being started with the same machine |
48 | // id. |
49 | - StartInstance(args StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, []NetworkInfo, error) |
50 | + StartInstance(args StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, []network.Info, error) |
51 | |
52 | // StopInstances shuts down the given instances. |
53 | StopInstances([]instance.Instance) error |
54 | |
55 | === added directory 'environs/network' |
56 | === added file 'environs/network/network.go' |
57 | --- environs/network/network.go 1970-01-01 00:00:00 +0000 |
58 | +++ environs/network/network.go 2014-04-18 16:46:53 +0000 |
59 | @@ -0,0 +1,37 @@ |
60 | +// Copyright 2014 Canonical Ltd. |
61 | +// Licensed under the AGPLv3, see LICENCE file for details. |
62 | + |
63 | +package network |
64 | + |
65 | +// Id defines a provider-specific network id. |
66 | +type Id string |
67 | + |
68 | +// Info describes a single network interface available on an instance. |
69 | +// For providers that support networks, this will be available at |
70 | +// StartInstance() time. |
71 | +type Info struct { |
72 | + // MACAddress is the network interface's hardware MAC address |
73 | + // (e.g. "aa:bb:cc:dd:ee:ff"). |
74 | + MACAddress string |
75 | + |
76 | + // CIDR of the network, in 123.45.67.89/24 format. |
77 | + CIDR string |
78 | + |
79 | + // NetworkName is juju-internal name of the network. |
80 | + NetworkName string |
81 | + |
82 | + // ProviderId is a provider-specific network id. |
83 | + ProviderId Id |
84 | + |
85 | + // VLANTag needs to be between 1 and 4094 for VLANs and 0 for |
86 | + // normal networks. It's defined by IEEE 802.1Q standard. |
87 | + VLANTag int |
88 | + |
89 | + // InterfaceName is the OS-specific network device name (e.g. |
90 | + // "eth0" or "eth1.42" for a VLAN virtual interface). |
91 | + InterfaceName string |
92 | + |
93 | + // IsVirtual is true when the interface is a virtual device, as |
94 | + // opposed to a physical device. |
95 | + IsVirtual bool |
96 | +} |
97 | |
98 | === modified file 'juju/testing/instance.go' |
99 | --- juju/testing/instance.go 2014-04-14 16:41:28 +0000 |
100 | +++ juju/testing/instance.go 2014-04-18 16:46:53 +0000 |
101 | @@ -11,6 +11,7 @@ |
102 | "launchpad.net/juju-core/constraints" |
103 | "launchpad.net/juju-core/environs" |
104 | "launchpad.net/juju-core/environs/config" |
105 | + "launchpad.net/juju-core/environs/network" |
106 | "launchpad.net/juju-core/environs/tools" |
107 | "launchpad.net/juju-core/instance" |
108 | "launchpad.net/juju-core/names" |
109 | @@ -60,7 +61,7 @@ |
110 | func StartInstance( |
111 | env environs.Environ, machineId string, |
112 | ) ( |
113 | - instance.Instance, *instance.HardwareCharacteristics, []environs.NetworkInfo, error, |
114 | + instance.Instance, *instance.HardwareCharacteristics, []network.Info, error, |
115 | ) { |
116 | return StartInstanceWithConstraints(env, machineId, constraints.Value{}) |
117 | } |
118 | @@ -84,7 +85,7 @@ |
119 | func StartInstanceWithConstraints( |
120 | env environs.Environ, machineId string, cons constraints.Value, |
121 | ) ( |
122 | - instance.Instance, *instance.HardwareCharacteristics, []environs.NetworkInfo, error, |
123 | + instance.Instance, *instance.HardwareCharacteristics, []network.Info, error, |
124 | ) { |
125 | return StartInstanceWithConstraintsAndNetworks(env, machineId, cons, nil, nil) |
126 | } |
127 | @@ -111,7 +112,7 @@ |
128 | env environs.Environ, machineId string, cons constraints.Value, |
129 | includeNetworks, excludeNetworks []string, |
130 | ) ( |
131 | - instance.Instance, *instance.HardwareCharacteristics, []environs.NetworkInfo, error, |
132 | + instance.Instance, *instance.HardwareCharacteristics, []network.Info, error, |
133 | ) { |
134 | series := config.PreferredSeries(env.Config()) |
135 | agentVersion, ok := env.Config().AgentVersion() |
136 | |
137 | === modified file 'provider/azure/environ.go' |
138 | --- provider/azure/environ.go 2014-04-04 15:55:19 +0000 |
139 | +++ provider/azure/environ.go 2014-04-18 16:46:53 +0000 |
140 | @@ -19,6 +19,7 @@ |
141 | "launchpad.net/juju-core/environs/config" |
142 | "launchpad.net/juju-core/environs/imagemetadata" |
143 | "launchpad.net/juju-core/environs/instances" |
144 | + "launchpad.net/juju-core/environs/network" |
145 | "launchpad.net/juju-core/environs/simplestreams" |
146 | "launchpad.net/juju-core/environs/storage" |
147 | envtools "launchpad.net/juju-core/environs/tools" |
148 | @@ -508,7 +509,7 @@ |
149 | } |
150 | |
151 | // StartInstance is specified in the InstanceBroker interface. |
152 | -func (env *azureEnviron) StartInstance(args environs.StartInstanceParams) (_ instance.Instance, _ *instance.HardwareCharacteristics, _ []environs.NetworkInfo, err error) { |
153 | +func (env *azureEnviron) StartInstance(args environs.StartInstanceParams) (_ instance.Instance, _ *instance.HardwareCharacteristics, _ []network.Info, err error) { |
154 | if args.MachineConfig.HasNetworks() { |
155 | return nil, nil, nil, fmt.Errorf("starting instances with networks is not supported yet.") |
156 | } |
157 | |
158 | === modified file 'provider/common/bootstrap_test.go' |
159 | --- provider/common/bootstrap_test.go 2014-04-11 10:12:20 +0000 |
160 | +++ provider/common/bootstrap_test.go 2014-04-18 16:46:53 +0000 |
161 | @@ -17,6 +17,7 @@ |
162 | "launchpad.net/juju-core/environs" |
163 | "launchpad.net/juju-core/environs/cloudinit" |
164 | "launchpad.net/juju-core/environs/config" |
165 | + "launchpad.net/juju-core/environs/network" |
166 | "launchpad.net/juju-core/environs/storage" |
167 | envtesting "launchpad.net/juju-core/environs/testing" |
168 | "launchpad.net/juju-core/instance" |
169 | @@ -80,7 +81,7 @@ |
170 | startInstance := func( |
171 | cons constraints.Value, _, _ []string, possibleTools tools.List, mcfg *cloudinit.MachineConfig, |
172 | ) ( |
173 | - instance.Instance, *instance.HardwareCharacteristics, []environs.NetworkInfo, error, |
174 | + instance.Instance, *instance.HardwareCharacteristics, []network.Info, error, |
175 | ) { |
176 | c.Assert(cons, gc.DeepEquals, checkCons) |
177 | c.Assert(mcfg, gc.DeepEquals, environs.NewBootstrapMachineConfig(mcfg.SystemPrivateSSHKey)) |
178 | @@ -105,7 +106,7 @@ |
179 | startInstance := func( |
180 | _ constraints.Value, _, _ []string, _ tools.List, _ *cloudinit.MachineConfig, |
181 | ) ( |
182 | - instance.Instance, *instance.HardwareCharacteristics, []environs.NetworkInfo, error, |
183 | + instance.Instance, *instance.HardwareCharacteristics, []network.Info, error, |
184 | ) { |
185 | stor.putErr = fmt.Errorf("suddenly a wild blah") |
186 | return &mockInstance{id: "i-blah"}, nil, nil, nil |
187 | @@ -138,7 +139,7 @@ |
188 | startInstance := func( |
189 | _ constraints.Value, _, _ []string, _ tools.List, _ *cloudinit.MachineConfig, |
190 | ) ( |
191 | - instance.Instance, *instance.HardwareCharacteristics, []environs.NetworkInfo, error, |
192 | + instance.Instance, *instance.HardwareCharacteristics, []network.Info, error, |
193 | ) { |
194 | stor.putErr = fmt.Errorf("suddenly a wild blah") |
195 | return &mockInstance{id: "i-blah"}, nil, nil, nil |
196 | @@ -179,7 +180,7 @@ |
197 | startInstance := func( |
198 | _ constraints.Value, _, _ []string, _ tools.List, mcfg *cloudinit.MachineConfig, |
199 | ) ( |
200 | - instance.Instance, *instance.HardwareCharacteristics, []environs.NetworkInfo, error, |
201 | + instance.Instance, *instance.HardwareCharacteristics, []network.Info, error, |
202 | ) { |
203 | return &mockInstance{id: checkInstanceId}, &checkHardware, nil, nil |
204 | } |
205 | |
206 | === modified file 'provider/common/mock_test.go' |
207 | --- provider/common/mock_test.go 2014-04-04 15:55:19 +0000 |
208 | +++ provider/common/mock_test.go 2014-04-18 16:46:53 +0000 |
209 | @@ -10,6 +10,7 @@ |
210 | "launchpad.net/juju-core/environs" |
211 | "launchpad.net/juju-core/environs/cloudinit" |
212 | "launchpad.net/juju-core/environs/config" |
213 | + "launchpad.net/juju-core/environs/network" |
214 | "launchpad.net/juju-core/environs/simplestreams" |
215 | "launchpad.net/juju-core/environs/storage" |
216 | "launchpad.net/juju-core/instance" |
217 | @@ -17,7 +18,7 @@ |
218 | ) |
219 | |
220 | type allInstancesFunc func() ([]instance.Instance, error) |
221 | -type startInstanceFunc func(constraints.Value, []string, []string, tools.List, *cloudinit.MachineConfig) (instance.Instance, *instance.HardwareCharacteristics, []environs.NetworkInfo, error) |
222 | +type startInstanceFunc func(constraints.Value, []string, []string, tools.List, *cloudinit.MachineConfig) (instance.Instance, *instance.HardwareCharacteristics, []network.Info, error) |
223 | type stopInstancesFunc func([]instance.Instance) error |
224 | type getToolsSourcesFunc func() ([]simplestreams.DataSource, error) |
225 | type configFunc func() *config.Config |
226 | @@ -49,7 +50,7 @@ |
227 | func (env *mockEnviron) AllInstances() ([]instance.Instance, error) { |
228 | return env.allInstances() |
229 | } |
230 | -func (env *mockEnviron) StartInstance(args environs.StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, []environs.NetworkInfo, error) { |
231 | +func (env *mockEnviron) StartInstance(args environs.StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, []network.Info, error) { |
232 | return env.startInstance( |
233 | args.Constraints, |
234 | args.MachineConfig.IncludeNetworks, |
235 | |
236 | === modified file 'provider/dummy/environs.go' |
237 | --- provider/dummy/environs.go 2014-04-14 16:41:28 +0000 |
238 | +++ provider/dummy/environs.go 2014-04-18 16:46:53 +0000 |
239 | @@ -40,6 +40,7 @@ |
240 | "launchpad.net/juju-core/environs/bootstrap" |
241 | "launchpad.net/juju-core/environs/config" |
242 | "launchpad.net/juju-core/environs/imagemetadata" |
243 | + "launchpad.net/juju-core/environs/network" |
244 | "launchpad.net/juju-core/environs/simplestreams" |
245 | "launchpad.net/juju-core/environs/storage" |
246 | "launchpad.net/juju-core/environs/tools" |
247 | @@ -115,7 +116,7 @@ |
248 | Constraints constraints.Value |
249 | IncludeNetworks []string |
250 | ExcludeNetworks []string |
251 | - NetworkInfo []environs.NetworkInfo |
252 | + NetworkInfo []network.Info |
253 | Info *state.Info |
254 | APIInfo *api.Info |
255 | Secret string |
256 | @@ -693,7 +694,7 @@ |
257 | } |
258 | |
259 | // StartInstance is specified in the InstanceBroker interface. |
260 | -func (e *environ) StartInstance(args environs.StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, []environs.NetworkInfo, error) { |
261 | +func (e *environ) StartInstance(args environs.StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, []network.Info, error) { |
262 | |
263 | defer delay() |
264 | machineId := args.MachineConfig.MachineId |
265 | @@ -766,23 +767,24 @@ |
266 | } |
267 | } |
268 | // Simulate networks added when requested. |
269 | - networkInfo := make([]environs.NetworkInfo, len(args.MachineConfig.IncludeNetworks)) |
270 | - for i, network := range args.MachineConfig.IncludeNetworks { |
271 | - if strings.HasPrefix(network, "bad-") { |
272 | + networkInfo := make([]network.Info, len(args.MachineConfig.IncludeNetworks)) |
273 | + for i, netName := range args.MachineConfig.IncludeNetworks { |
274 | + if strings.HasPrefix(netName, "bad-") { |
275 | // Simulate we didn't get correct information for the network. |
276 | - networkInfo[i] = environs.NetworkInfo{ |
277 | - NetworkId: network, |
278 | - NetworkName: network, |
279 | + networkInfo[i] = network.Info{ |
280 | + ProviderId: network.Id(netName), |
281 | + NetworkName: netName, |
282 | CIDR: "invalid", |
283 | } |
284 | } else { |
285 | - networkInfo[i] = environs.NetworkInfo{ |
286 | - NetworkId: network, |
287 | - NetworkName: network, |
288 | + networkInfo[i] = network.Info{ |
289 | + ProviderId: network.Id(netName), |
290 | + NetworkName: netName, |
291 | CIDR: fmt.Sprintf("0.%d.2.0/24", i+1), |
292 | InterfaceName: fmt.Sprintf("eth%d", i), |
293 | VLANTag: i, |
294 | MACAddress: fmt.Sprintf("aa:bb:cc:dd:ee:f%d", i), |
295 | + IsVirtual: i > 0, |
296 | } |
297 | } |
298 | } |
299 | |
300 | === modified file 'provider/ec2/ec2.go' |
301 | --- provider/ec2/ec2.go 2014-04-09 16:36:12 +0000 |
302 | +++ provider/ec2/ec2.go 2014-04-18 16:46:53 +0000 |
303 | @@ -18,6 +18,7 @@ |
304 | "launchpad.net/juju-core/environs/config" |
305 | "launchpad.net/juju-core/environs/imagemetadata" |
306 | "launchpad.net/juju-core/environs/instances" |
307 | + "launchpad.net/juju-core/environs/network" |
308 | "launchpad.net/juju-core/environs/simplestreams" |
309 | "launchpad.net/juju-core/environs/storage" |
310 | envtools "launchpad.net/juju-core/environs/tools" |
311 | @@ -393,7 +394,7 @@ |
312 | const ebsStorage = "ebs" |
313 | |
314 | // StartInstance is specified in the InstanceBroker interface. |
315 | -func (e *environ) StartInstance(args environs.StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, []environs.NetworkInfo, error) { |
316 | +func (e *environ) StartInstance(args environs.StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, []network.Info, error) { |
317 | if args.MachineConfig.HasNetworks() { |
318 | return nil, nil, nil, fmt.Errorf("starting instances with networks is not supported yet.") |
319 | } |
320 | |
321 | === modified file 'provider/joyent/environ_instance.go' |
322 | --- provider/joyent/environ_instance.go 2014-04-04 15:55:19 +0000 |
323 | +++ provider/joyent/environ_instance.go 2014-04-18 16:46:53 +0000 |
324 | @@ -15,6 +15,7 @@ |
325 | "launchpad.net/juju-core/environs" |
326 | "launchpad.net/juju-core/environs/imagemetadata" |
327 | "launchpad.net/juju-core/environs/instances" |
328 | + "launchpad.net/juju-core/environs/network" |
329 | "launchpad.net/juju-core/environs/simplestreams" |
330 | "launchpad.net/juju-core/instance" |
331 | "launchpad.net/juju-core/names" |
332 | @@ -50,7 +51,7 @@ |
333 | return fmt.Sprintf("juju-%s-%s", env.Name(), names.MachineTag(machineId)) |
334 | } |
335 | |
336 | -func (env *joyentEnviron) StartInstance(args environs.StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, []environs.NetworkInfo, error) { |
337 | +func (env *joyentEnviron) StartInstance(args environs.StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, []network.Info, error) { |
338 | |
339 | if args.MachineConfig.HasNetworks() { |
340 | return nil, nil, nil, fmt.Errorf("starting instances with networks is not supported yet.") |
341 | |
342 | === modified file 'provider/local/environ.go' |
343 | --- provider/local/environ.go 2014-04-14 16:41:28 +0000 |
344 | +++ provider/local/environ.go 2014-04-18 16:46:53 +0000 |
345 | @@ -30,6 +30,7 @@ |
346 | "launchpad.net/juju-core/environs/config" |
347 | "launchpad.net/juju-core/environs/filestorage" |
348 | "launchpad.net/juju-core/environs/httpstorage" |
349 | + "launchpad.net/juju-core/environs/network" |
350 | "launchpad.net/juju-core/environs/simplestreams" |
351 | "launchpad.net/juju-core/environs/storage" |
352 | envtools "launchpad.net/juju-core/environs/tools" |
353 | @@ -301,7 +302,7 @@ |
354 | } |
355 | |
356 | // StartInstance is specified in the InstanceBroker interface. |
357 | -func (env *localEnviron) StartInstance(args environs.StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, []environs.NetworkInfo, error) { |
358 | +func (env *localEnviron) StartInstance(args environs.StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, []network.Info, error) { |
359 | if args.MachineConfig.HasNetworks() { |
360 | return nil, nil, nil, fmt.Errorf("starting instances with networks is not supported yet.") |
361 | } |
362 | |
363 | === modified file 'provider/maas/environ.go' |
364 | --- provider/maas/environ.go 2014-04-11 15:20:32 +0000 |
365 | +++ provider/maas/environ.go 2014-04-18 16:46:53 +0000 |
366 | @@ -22,6 +22,7 @@ |
367 | "launchpad.net/juju-core/environs" |
368 | "launchpad.net/juju-core/environs/config" |
369 | "launchpad.net/juju-core/environs/imagemetadata" |
370 | + "launchpad.net/juju-core/environs/network" |
371 | "launchpad.net/juju-core/environs/simplestreams" |
372 | "launchpad.net/juju-core/environs/storage" |
373 | envtools "launchpad.net/juju-core/environs/tools" |
374 | @@ -331,56 +332,54 @@ |
375 | return `sed -i "s/iface eth0 inet dhcp/source \/etc\/network\/eth0.config/" /etc/network/interfaces` |
376 | } |
377 | |
378 | -// setupNetworks prepares a []environs.NetworkInfo for the given instance. |
379 | -func (environ *maasEnviron) setupNetworks(inst instance.Instance) ([]environs.NetworkInfo, error) { |
380 | +// setupNetworks prepares a []network.Info for the given instance. |
381 | +func (environ *maasEnviron) setupNetworks(inst instance.Instance) ([]network.Info, error) { |
382 | // Get the instance network interfaces first. |
383 | interfaces, err := environ.getInstanceNetworkInterfaces(inst) |
384 | if err != nil { |
385 | return nil, fmt.Errorf("getInstanceNetworkInterfaces failed: %v", err) |
386 | } |
387 | logger.Debugf("node %q has network interfaces %v", inst.Id(), interfaces) |
388 | - networkInfoMap := make(map[string]environs.NetworkInfo) |
389 | - for macAddress, interfaceName := range interfaces { |
390 | - networkInfoMap[macAddress] = environs.NetworkInfo{ |
391 | - MACAddress: macAddress, |
392 | - InterfaceName: interfaceName, |
393 | - } |
394 | - } |
395 | networks, err := environ.getInstanceNetworks(inst) |
396 | if err != nil { |
397 | return nil, fmt.Errorf("getInstanceNetworks failed: %v", err) |
398 | } |
399 | logger.Debugf("node %q has networks %v", inst.Id(), networks) |
400 | - for _, network := range networks { |
401 | + var tempNetworkInfo []network.Info |
402 | + for _, netw := range networks { |
403 | netCIDR := &net.IPNet{ |
404 | - IP: net.ParseIP(network.IP), |
405 | - Mask: net.IPMask(net.ParseIP(network.Mask)), |
406 | + IP: net.ParseIP(netw.IP), |
407 | + Mask: net.IPMask(net.ParseIP(netw.Mask)), |
408 | } |
409 | - macs, err := environ.getNetworkMACs(network.Name) |
410 | + macs, err := environ.getNetworkMACs(netw.Name) |
411 | if err != nil { |
412 | return nil, fmt.Errorf("getNetworkMACs failed: %v", err) |
413 | } |
414 | + logger.Debugf("network %q has MACs: %v", netw.Name, macs) |
415 | for _, mac := range macs { |
416 | - if _, ok := interfaces[mac]; ok { |
417 | - info := networkInfoMap[mac] |
418 | - info.CIDR = netCIDR.String() |
419 | - info.VLANTag = network.VLANTag |
420 | - info.NetworkId = network.Name |
421 | - info.NetworkName = network.Name |
422 | - networkInfoMap[mac] = info |
423 | + if interfaceName, ok := interfaces[mac]; ok { |
424 | + tempNetworkInfo = append(tempNetworkInfo, network.Info{ |
425 | + MACAddress: mac, |
426 | + InterfaceName: interfaceName, |
427 | + CIDR: netCIDR.String(), |
428 | + VLANTag: netw.VLANTag, |
429 | + ProviderId: network.Id(netw.Name), |
430 | + NetworkName: netw.Name, |
431 | + IsVirtual: netw.VLANTag > 0, |
432 | + }) |
433 | } |
434 | } |
435 | } |
436 | // Verify we filled-in everything for all networks/interfaces |
437 | // and drop incomplete records. |
438 | - var networkInfo []environs.NetworkInfo |
439 | - for _, info := range networkInfoMap { |
440 | - if info.NetworkId == "" || info.NetworkName == "" || info.CIDR == "" { |
441 | + var networkInfo []network.Info |
442 | + for _, info := range tempNetworkInfo { |
443 | + if info.ProviderId == "" || info.NetworkName == "" || info.CIDR == "" { |
444 | logger.Warningf("ignoring network interface %q: missing network information", info.InterfaceName) |
445 | continue |
446 | } |
447 | if info.MACAddress == "" || info.InterfaceName == "" { |
448 | - logger.Warningf("ignoring network %q: missing network interface information", info.NetworkId) |
449 | + logger.Warningf("ignoring network %q: missing network interface information", info.ProviderId) |
450 | continue |
451 | } |
452 | networkInfo = append(networkInfo, info) |
453 | @@ -391,7 +390,7 @@ |
454 | |
455 | // StartInstance is specified in the InstanceBroker interface. |
456 | func (environ *maasEnviron) StartInstance(args environs.StartInstanceParams) ( |
457 | - instance.Instance, *instance.HardwareCharacteristics, []environs.NetworkInfo, error, |
458 | + instance.Instance, *instance.HardwareCharacteristics, []network.Info, error, |
459 | ) { |
460 | var inst *maasInstance |
461 | var err error |
462 | @@ -413,7 +412,7 @@ |
463 | } |
464 | } |
465 | }() |
466 | - var networkInfo []environs.NetworkInfo |
467 | + var networkInfo []network.Info |
468 | if args.MachineConfig.HasNetworks() { |
469 | networkInfo, err = environ.setupNetworks(inst) |
470 | if err != nil { |
471 | @@ -454,7 +453,7 @@ |
472 | |
473 | // newCloudinitConfig creates a cloudinit.Config structure |
474 | // suitable as a base for initialising a MAAS node. |
475 | -func newCloudinitConfig(hostname string, networkInfo []environs.NetworkInfo) (*cloudinit.Config, error) { |
476 | +func newCloudinitConfig(hostname string, networkInfo []network.Info) (*cloudinit.Config, error) { |
477 | info := machineInfo{hostname} |
478 | runCmd, err := info.cloudinitRunCmd() |
479 | if err != nil { |
480 | @@ -477,11 +476,7 @@ |
481 | |
482 | // setupNetworksOnBoot prepares a script to enable and start all given |
483 | // networks on boot. |
484 | -// |
485 | -// TODO(dimitern) To support more than one VLAN on the same physical |
486 | -// interface, we need model changes to allow muliple NICs with the |
487 | -// same MAC address, but different network name. |
488 | -func setupNetworksOnBoot(cloudcfg *cloudinit.Config, networkInfo []environs.NetworkInfo) { |
489 | +func setupNetworksOnBoot(cloudcfg *cloudinit.Config, networkInfo []network.Info) { |
490 | const ifaceConfig = `cat >> /etc/network/interfaces << EOF |
491 | |
492 | auto %s |
493 | @@ -494,6 +489,10 @@ |
494 | script := func(line string, args ...interface{}) { |
495 | cloudcfg.AddScripts(fmt.Sprintf(line, args...)) |
496 | } |
497 | + // Because eth0 is already configured in the br0 bridge, we |
498 | + // don't want to break that. |
499 | + configured := set.NewStrings("eth0") |
500 | + |
501 | // In order to support VLANs, we need to include 8021q module |
502 | // configure vconfig's set_name_type |
503 | script("modprobe 8021q") |
504 | @@ -501,14 +500,12 @@ |
505 | script("vconfig set_name_type DEV_PLUS_VID_NO_PAD") |
506 | // Now prepare each interface configuration |
507 | for _, info := range networkInfo { |
508 | - // Because eth0 is already configured in the br0 bridge, we |
509 | - // don't want to break that. |
510 | - if info.InterfaceName == "eth0" { |
511 | - continue |
512 | + if !configured.Contains(info.InterfaceName) { |
513 | + // Register and bring up the physical interface. |
514 | + script(ifaceConfig, info.InterfaceName, info.InterfaceName) |
515 | + script("ifup %s", info.InterfaceName) |
516 | + configured.Add(info.InterfaceName) |
517 | } |
518 | - // Register and bring up the interface. |
519 | - script(ifaceConfig, info.InterfaceName, info.InterfaceName) |
520 | - script("ifup %s", info.InterfaceName) |
521 | if info.VLANTag > 0 { |
522 | // We have a VLAN and need to create and register it after |
523 | // its parent interface was brought up. |
524 | |
525 | === modified file 'provider/maas/environ_test.go' |
526 | --- provider/maas/environ_test.go 2014-04-10 17:46:52 +0000 |
527 | +++ provider/maas/environ_test.go 2014-04-18 16:46:53 +0000 |
528 | @@ -10,8 +10,8 @@ |
529 | gc "launchpad.net/gocheck" |
530 | "launchpad.net/gomaasapi" |
531 | |
532 | - "launchpad.net/juju-core/environs" |
533 | "launchpad.net/juju-core/environs/config" |
534 | + "launchpad.net/juju-core/environs/network" |
535 | envtesting "launchpad.net/juju-core/environs/testing" |
536 | "launchpad.net/juju-core/provider/maas" |
537 | coretesting "launchpad.net/juju-core/testing" |
538 | @@ -197,10 +197,17 @@ |
539 | } |
540 | |
541 | func (*environSuite) TestNewCloudinitConfig(c *gc.C) { |
542 | - nwInfo := []environs.NetworkInfo{ |
543 | + nwInfo := []network.Info{ |
544 | + // physical eth0 won't be touched, but it can have VLANs on it. |
545 | {InterfaceName: "eth0", VLANTag: 0}, |
546 | + {InterfaceName: "eth0", VLANTag: 99}, |
547 | + // physical NIC given explicitly, then a couple of virtual ones using it. |
548 | + {InterfaceName: "eth1", VLANTag: 0}, |
549 | {InterfaceName: "eth1", VLANTag: 42}, |
550 | + {InterfaceName: "eth1", VLANTag: 69}, |
551 | {InterfaceName: "eth2", VLANTag: 0}, |
552 | + // physical NIC not given, ensure it gets brought up first, before the virtual one. |
553 | + {InterfaceName: "eth3", VLANTag: 123}, |
554 | } |
555 | cloudcfg, err := maas.NewCloudinitConfig("testing.invalid", nwInfo) |
556 | c.Assert(err, gc.IsNil) |
557 | @@ -216,12 +223,23 @@ |
558 | "modprobe 8021q", |
559 | "sh -c 'grep -q 8021q /etc/modules || echo 8021q >> /etc/modules'", |
560 | "vconfig set_name_type DEV_PLUS_VID_NO_PAD", |
561 | + "vconfig add eth0 99", |
562 | + "cat >> /etc/network/interfaces << EOF\n\nauto eth0.99\niface eth0.99 inet dhcp\nEOF\n", |
563 | + "ifup eth0.99", |
564 | "cat >> /etc/network/interfaces << EOF\n\nauto eth1\niface eth1 inet dhcp\nEOF\n", |
565 | "ifup eth1", |
566 | "vconfig add eth1 42", |
567 | "cat >> /etc/network/interfaces << EOF\n\nauto eth1.42\niface eth1.42 inet dhcp\nEOF\n", |
568 | "ifup eth1.42", |
569 | + "vconfig add eth1 69", |
570 | + "cat >> /etc/network/interfaces << EOF\n\nauto eth1.69\niface eth1.69 inet dhcp\nEOF\n", |
571 | + "ifup eth1.69", |
572 | "cat >> /etc/network/interfaces << EOF\n\nauto eth2\niface eth2 inet dhcp\nEOF\n", |
573 | "ifup eth2", |
574 | + "cat >> /etc/network/interfaces << EOF\n\nauto eth3\niface eth3 inet dhcp\nEOF\n", |
575 | + "ifup eth3", |
576 | + "vconfig add eth3 123", |
577 | + "cat >> /etc/network/interfaces << EOF\n\nauto eth3.123\niface eth3.123 inet dhcp\nEOF\n", |
578 | + "ifup eth3.123", |
579 | }) |
580 | } |
581 | |
582 | === modified file 'provider/manual/environ.go' |
583 | --- provider/manual/environ.go 2014-04-11 17:51:58 +0000 |
584 | +++ provider/manual/environ.go 2014-04-18 16:46:53 +0000 |
585 | @@ -21,6 +21,7 @@ |
586 | "launchpad.net/juju-core/environs/config" |
587 | "launchpad.net/juju-core/environs/httpstorage" |
588 | "launchpad.net/juju-core/environs/manual" |
589 | + "launchpad.net/juju-core/environs/network" |
590 | "launchpad.net/juju-core/environs/simplestreams" |
591 | "launchpad.net/juju-core/environs/sshstorage" |
592 | "launchpad.net/juju-core/environs/storage" |
593 | @@ -65,7 +66,7 @@ |
594 | var errNoStartInstance = errors.New("manual provider cannot start instances") |
595 | var errNoStopInstance = errors.New("manual provider cannot stop instances") |
596 | |
597 | -func (*manualEnviron) StartInstance(args environs.StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, []environs.NetworkInfo, error) { |
598 | +func (*manualEnviron) StartInstance(args environs.StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, []network.Info, error) { |
599 | return nil, nil, nil, errNoStartInstance |
600 | } |
601 | |
602 | |
603 | === modified file 'provider/openstack/provider.go' |
604 | --- provider/openstack/provider.go 2014-04-12 05:53:58 +0000 |
605 | +++ provider/openstack/provider.go 2014-04-18 16:46:53 +0000 |
606 | @@ -27,6 +27,7 @@ |
607 | "launchpad.net/juju-core/environs/config" |
608 | "launchpad.net/juju-core/environs/imagemetadata" |
609 | "launchpad.net/juju-core/environs/instances" |
610 | + "launchpad.net/juju-core/environs/network" |
611 | "launchpad.net/juju-core/environs/simplestreams" |
612 | "launchpad.net/juju-core/environs/storage" |
613 | envtools "launchpad.net/juju-core/environs/tools" |
614 | @@ -745,7 +746,7 @@ |
615 | } |
616 | |
617 | // StartInstance is specified in the InstanceBroker interface. |
618 | -func (e *environ) StartInstance(args environs.StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, []environs.NetworkInfo, error) { |
619 | +func (e *environ) StartInstance(args environs.StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, []network.Info, error) { |
620 | |
621 | if args.MachineConfig.HasNetworks() { |
622 | return nil, nil, nil, fmt.Errorf("starting instances with networks is not supported yet.") |
623 | |
624 | === modified file 'state/api/params/internal.go' |
625 | --- state/api/params/internal.go 2014-04-11 15:16:00 +0000 |
626 | +++ state/api/params/internal.go 2014-04-18 16:46:53 +0000 |
627 | @@ -7,6 +7,7 @@ |
628 | "time" |
629 | |
630 | "launchpad.net/juju-core/constraints" |
631 | + "launchpad.net/juju-core/environs/network" |
632 | "launchpad.net/juju-core/instance" |
633 | "launchpad.net/juju-core/tools" |
634 | "launchpad.net/juju-core/utils/exec" |
635 | @@ -317,7 +318,7 @@ |
636 | Tag string |
637 | |
638 | // ProviderId is the provider-specific network id. |
639 | - ProviderId string |
640 | + ProviderId network.Id |
641 | |
642 | // CIDR of the network, in "123.45.67.89/12" format. |
643 | CIDR string |
644 | @@ -340,6 +341,10 @@ |
645 | |
646 | // NetworkTag is this interface's network tag. |
647 | NetworkTag string |
648 | + |
649 | + // IsVirtual is true when the interface is a virtual device, as |
650 | + // opposed to a physical device. |
651 | + IsVirtual bool |
652 | } |
653 | |
654 | // InstanceInfo holds a machine tag, provider-specific instance id, a |
655 | |
656 | === modified file 'state/api/provisioner/provisioner_test.go' |
657 | --- state/api/provisioner/provisioner_test.go 2014-04-14 12:36:13 +0000 |
658 | +++ state/api/provisioner/provisioner_test.go 2014-04-18 16:46:53 +0000 |
659 | @@ -230,6 +230,11 @@ |
660 | CIDR: "0.2.2.0/24", |
661 | VLANTag: 42, |
662 | }, { |
663 | + Tag: "network-vlan69", |
664 | + ProviderId: "vlan69", |
665 | + CIDR: "0.3.2.0/24", |
666 | + VLANTag: 69, |
667 | + }, { |
668 | Tag: "network-vlan42", // duplicated; ignored |
669 | ProviderId: "vlan42", |
670 | CIDR: "0.2.2.0/24", |
671 | @@ -239,18 +244,32 @@ |
672 | MACAddress: "aa:bb:cc:dd:ee:f0", |
673 | NetworkTag: "network-net1", |
674 | InterfaceName: "eth0", |
675 | + IsVirtual: false, |
676 | }, { |
677 | MACAddress: "aa:bb:cc:dd:ee:f1", |
678 | NetworkTag: "network-net1", |
679 | InterfaceName: "eth1", |
680 | - }, { |
681 | - MACAddress: "aa:bb:cc:dd:ee:f2", |
682 | - NetworkTag: "network-vlan42", |
683 | - InterfaceName: "eth2", |
684 | - }, { |
685 | - MACAddress: "aa:bb:cc:dd:ee:f2", // duplicated; ignored |
686 | - NetworkTag: "network-vlan42", |
687 | - InterfaceName: "eth2", |
688 | + IsVirtual: false, |
689 | + }, { |
690 | + MACAddress: "aa:bb:cc:dd:ee:f1", |
691 | + NetworkTag: "network-vlan42", |
692 | + InterfaceName: "eth1.42", |
693 | + IsVirtual: true, |
694 | + }, { |
695 | + MACAddress: "aa:bb:cc:dd:ee:f1", |
696 | + NetworkTag: "network-vlan69", |
697 | + InterfaceName: "eth1.69", |
698 | + IsVirtual: true, |
699 | + }, { |
700 | + MACAddress: "aa:bb:cc:dd:ee:f1", // duplicated mac+net; ignored |
701 | + NetworkTag: "network-vlan42", |
702 | + InterfaceName: "eth2", |
703 | + IsVirtual: true, |
704 | + }, { |
705 | + MACAddress: "aa:bb:cc:dd:ee:f4", |
706 | + NetworkTag: "network-net1", |
707 | + InterfaceName: "eth1", // duplicated name+machine id; ignored |
708 | + IsVirtual: false, |
709 | }} |
710 | |
711 | err = apiMachine.SetInstanceInfo("i-will", "fake_nonce", &hwChars, networks, ifaces) |
712 | @@ -273,7 +292,7 @@ |
713 | |
714 | // Check the networks are created. |
715 | for i, _ := range networks { |
716 | - if i == 2 { |
717 | + if i == 3 { |
718 | // Last one was ignored, so skip it. |
719 | break |
720 | } |
721 | @@ -291,16 +310,17 @@ |
722 | // And the network interfaces as well. |
723 | ifacesMachine, err = notProvisionedMachine.NetworkInterfaces() |
724 | c.Assert(err, gc.IsNil) |
725 | - c.Assert(ifacesMachine, gc.HasLen, 3) |
726 | + c.Assert(ifacesMachine, gc.HasLen, 4) |
727 | actual := make([]params.NetworkInterface, len(ifacesMachine)) |
728 | for i, iface := range ifacesMachine { |
729 | actual[i].InterfaceName = iface.InterfaceName() |
730 | actual[i].NetworkTag = iface.NetworkTag() |
731 | actual[i].MACAddress = iface.MACAddress() |
732 | + actual[i].IsVirtual = iface.IsVirtual() |
733 | c.Check(iface.MachineTag(), gc.Equals, notProvisionedMachine.Tag()) |
734 | c.Check(iface.MachineId(), gc.Equals, notProvisionedMachine.Id()) |
735 | } |
736 | - c.Assert(actual, jc.SameContents, ifaces[:3]) // skip [3] as it's ignored. |
737 | + c.Assert(actual, jc.SameContents, ifaces[:4]) // skip the rest as they are ignored. |
738 | } |
739 | |
740 | func (s *provisionerSuite) TestSeries(c *gc.C) { |
741 | |
742 | === modified file 'state/apiserver/provisioner/provisioner.go' |
743 | --- state/apiserver/provisioner/provisioner.go 2014-04-11 15:10:48 +0000 |
744 | +++ state/apiserver/provisioner/provisioner.go 2014-04-18 16:46:53 +0000 |
745 | @@ -435,6 +435,7 @@ |
746 | MACAddress: iface.MACAddress, |
747 | NetworkName: networkName, |
748 | InterfaceName: iface.InterfaceName, |
749 | + IsVirtual: iface.IsVirtual, |
750 | } |
751 | } |
752 | return stateNetworks, stateInterfaces, nil |
753 | |
754 | === modified file 'state/apiserver/provisioner/provisioner_test.go' |
755 | --- state/apiserver/provisioner/provisioner_test.go 2014-04-17 12:53:23 +0000 |
756 | +++ state/apiserver/provisioner/provisioner_test.go 2014-04-18 16:46:53 +0000 |
757 | @@ -862,6 +862,11 @@ |
758 | CIDR: "0.2.2.0/24", |
759 | VLANTag: 42, |
760 | }, { |
761 | + Tag: "network-vlan69", |
762 | + ProviderId: "vlan69", |
763 | + CIDR: "0.3.2.0/24", |
764 | + VLANTag: 69, |
765 | + }, { |
766 | Tag: "network-vlan42", // duplicated; ignored |
767 | ProviderId: "vlan42", |
768 | CIDR: "0.2.2.0/24", |
769 | @@ -871,18 +876,32 @@ |
770 | MACAddress: "aa:bb:cc:dd:ee:f0", |
771 | NetworkTag: "network-net1", |
772 | InterfaceName: "eth0", |
773 | + IsVirtual: false, |
774 | }, { |
775 | MACAddress: "aa:bb:cc:dd:ee:f1", |
776 | NetworkTag: "network-net1", |
777 | InterfaceName: "eth1", |
778 | + IsVirtual: false, |
779 | + }, { |
780 | + MACAddress: "aa:bb:cc:dd:ee:f1", |
781 | + NetworkTag: "network-vlan42", |
782 | + InterfaceName: "eth1.42", |
783 | + IsVirtual: true, |
784 | + }, { |
785 | + MACAddress: "aa:bb:cc:dd:ee:f0", |
786 | + NetworkTag: "network-vlan69", |
787 | + InterfaceName: "eth0.69", |
788 | + IsVirtual: true, |
789 | + }, { |
790 | + MACAddress: "aa:bb:cc:dd:ee:f1", // duplicated mac+net; ignored |
791 | + NetworkTag: "network-vlan42", |
792 | + InterfaceName: "eth2", |
793 | + IsVirtual: true, |
794 | }, { |
795 | MACAddress: "aa:bb:cc:dd:ee:f2", |
796 | - NetworkTag: "network-vlan42", |
797 | - InterfaceName: "eth2", |
798 | - }, { |
799 | - MACAddress: "aa:bb:cc:dd:ee:f2", // duplicated; ignored |
800 | - NetworkTag: "network-vlan42", |
801 | - InterfaceName: "eth2", |
802 | + NetworkTag: "network-net1", |
803 | + InterfaceName: "eth1", // duplicated name+machine id; ignored for machine 1. |
804 | + IsVirtual: false, |
805 | }} |
806 | args := params.InstancesInfo{Machines: []params.InstanceInfo{{ |
807 | Tag: s.machines[0].Tag(), |
808 | @@ -939,21 +958,26 @@ |
809 | c.Check(gotHardware, gc.DeepEquals, &hwChars) |
810 | ifacesMachine1, err := s.machines[1].NetworkInterfaces() |
811 | c.Assert(err, gc.IsNil) |
812 | - c.Assert(ifacesMachine1, gc.HasLen, 3) |
813 | + c.Assert(ifacesMachine1, gc.HasLen, 4) |
814 | actual := make([]params.NetworkInterface, len(ifacesMachine1)) |
815 | for i, iface := range ifacesMachine1 { |
816 | actual[i].InterfaceName = iface.InterfaceName() |
817 | actual[i].NetworkTag = iface.NetworkTag() |
818 | actual[i].MACAddress = iface.MACAddress() |
819 | + actual[i].IsVirtual = iface.IsVirtual() |
820 | c.Check(iface.MachineId(), gc.Equals, s.machines[1].Id()) |
821 | c.Check(iface.MachineTag(), gc.Equals, s.machines[1].Tag()) |
822 | } |
823 | - c.Assert(actual, jc.SameContents, ifaces[:3]) |
824 | + c.Assert(actual, jc.SameContents, ifaces[:4]) |
825 | ifacesMachine2, err := s.machines[2].NetworkInterfaces() |
826 | c.Assert(err, gc.IsNil) |
827 | - c.Assert(ifacesMachine2, gc.HasLen, 0) |
828 | + c.Assert(ifacesMachine2, gc.HasLen, 1) |
829 | + c.Assert(ifacesMachine2[0].InterfaceName(), gc.Equals, ifaces[5].InterfaceName) |
830 | + c.Assert(ifacesMachine2[0].MACAddress(), gc.Equals, ifaces[5].MACAddress) |
831 | + c.Assert(ifacesMachine2[0].NetworkTag(), gc.Equals, ifaces[5].NetworkTag) |
832 | + c.Assert(ifacesMachine2[0].MachineId(), gc.Equals, s.machines[2].Id()) |
833 | for i, _ := range networks { |
834 | - if i == 2 { |
835 | + if i == 3 { |
836 | // Last one was ignored, so don't check. |
837 | break |
838 | } |
839 | |
840 | === modified file 'state/machine.go' |
841 | --- state/machine.go 2014-04-17 16:18:46 +0000 |
842 | +++ state/machine.go 2014-04-18 16:46:53 +0000 |
843 | @@ -567,16 +567,21 @@ |
844 | } |
845 | |
846 | func (m *Machine) removeNetworkInterfacesOps() ([]txn.Op, error) { |
847 | - var doc struct { |
848 | - MACAddress string `bson:"_id"` |
849 | + if m.doc.Life != Dead { |
850 | + return nil, fmt.Errorf("machine is not dead") |
851 | } |
852 | - ops := []txn.Op{} |
853 | + var doc networkInterfaceDoc |
854 | + ops := []txn.Op{{ |
855 | + C: m.st.machines.Name, |
856 | + Id: m.doc.Id, |
857 | + Assert: isDeadDoc, |
858 | + }} |
859 | sel := bson.D{{"machineid", m.doc.Id}} |
860 | iter := m.st.networkInterfaces.Find(sel).Select(bson.D{{"_id", 1}}).Iter() |
861 | for iter.Next(&doc) { |
862 | ops = append(ops, txn.Op{ |
863 | C: m.st.networkInterfaces.Name, |
864 | - Id: doc.MACAddress, |
865 | + Id: doc.Id, |
866 | Remove: true, |
867 | }) |
868 | } |
869 | @@ -842,7 +847,7 @@ |
870 | |
871 | // Add the networks and interfaces first. |
872 | for _, network := range networks { |
873 | - _, err := m.st.AddNetwork(network.Name, network.ProviderId, network.CIDR, network.VLANTag) |
874 | + _, err := m.st.AddNetwork(network) |
875 | if err != nil && errors.IsAlreadyExists(err) { |
876 | // Ignore already existing networks. |
877 | continue |
878 | @@ -851,7 +856,7 @@ |
879 | } |
880 | } |
881 | for _, iface := range interfaces { |
882 | - _, err := m.AddNetworkInterface(iface.MACAddress, iface.InterfaceName, iface.NetworkName) |
883 | + _, err := m.AddNetworkInterface(iface) |
884 | if err != nil && errors.IsAlreadyExists(err) { |
885 | // Ignore already existing network interfaces. |
886 | continue |
887 | @@ -996,34 +1001,31 @@ |
888 | return ifaces, nil |
889 | } |
890 | |
891 | -// AddNetworkInterface creates a new network interface on the given |
892 | -// network for the machine. The machine must be alive and not yet |
893 | +// AddNetworkInterface creates a new network interface with the given |
894 | +// args for this machine. The machine must be alive and not yet |
895 | // provisioned, and there must be no other interface with the same MAC |
896 | -// address or the same name on that machine for this to succeed. If a |
897 | -// network interface already exists, the returned error satisfies |
898 | -// errors.IsAlreadyExists. |
899 | -func (m *Machine) AddNetworkInterface(macAddress, interfaceName, networkName string) (iface *NetworkInterface, err error) { |
900 | - defer errors.Contextf(&err, "cannot add network interface %q to machine %q", interfaceName, m.doc.Id) |
901 | +// address on the same network, or the same name on that machine for |
902 | +// this to succeed. If a network interface already exists, the |
903 | +// returned error satisfies errors.IsAlreadyExists. |
904 | +func (m *Machine) AddNetworkInterface(args NetworkInterfaceInfo) (iface *NetworkInterface, err error) { |
905 | + defer errors.Contextf(&err, "cannot add network interface %q to machine %q", args.InterfaceName, m.doc.Id) |
906 | |
907 | - if macAddress == "" { |
908 | + if args.MACAddress == "" { |
909 | return nil, fmt.Errorf("MAC address must be not empty") |
910 | } |
911 | - if _, err = net.ParseMAC(macAddress); err != nil { |
912 | + if _, err = net.ParseMAC(args.MACAddress); err != nil { |
913 | return nil, err |
914 | } |
915 | - if interfaceName == "" { |
916 | + if args.InterfaceName == "" { |
917 | return nil, fmt.Errorf("interface name must be not empty") |
918 | } |
919 | aliveAndNotProvisioned := append(isAliveDoc, bson.D{{"nonce", ""}}...) |
920 | - doc := &networkInterfaceDoc{ |
921 | - MACAddress: macAddress, |
922 | - InterfaceName: interfaceName, |
923 | - NetworkName: networkName, |
924 | - MachineId: m.doc.Id, |
925 | - } |
926 | + doc := newNetworkInterfaceDoc(args) |
927 | + doc.MachineId = m.doc.Id |
928 | + doc.Id = bson.NewObjectId() |
929 | ops := []txn.Op{{ |
930 | C: m.st.networks.Name, |
931 | - Id: networkName, |
932 | + Id: args.NetworkName, |
933 | Assert: txn.DocExists, |
934 | }, { |
935 | C: m.st.machines.Name, |
936 | @@ -1031,7 +1033,7 @@ |
937 | Assert: aliveAndNotProvisioned, |
938 | }, { |
939 | C: m.st.networkInterfaces.Name, |
940 | - Id: macAddress, |
941 | + Id: doc.Id, |
942 | Assert: txn.DocMissing, |
943 | Insert: doc, |
944 | }} |
945 | @@ -1039,10 +1041,7 @@ |
946 | err = m.st.runTransaction(ops) |
947 | switch err { |
948 | case txn.ErrAborted: |
949 | - if err = m.st.networkInterfaces.FindId(macAddress).One(nil); err == nil { |
950 | - return nil, errors.AlreadyExistsf("interface with MAC address %q", macAddress) |
951 | - } |
952 | - if _, err = m.st.Network(networkName); err != nil { |
953 | + if _, err = m.st.Network(args.NetworkName); err != nil { |
954 | return nil, err |
955 | } |
956 | if err = m.Refresh(); err != nil { |
957 | @@ -1053,17 +1052,30 @@ |
958 | msg := "machine already provisioned: dynamic network interfaces not currently supported" |
959 | return nil, fmt.Errorf(msg) |
960 | } |
961 | + // Should never happen. |
962 | + logger.Errorf("unhandled assert while adding network interface doc %#v", doc) |
963 | case nil: |
964 | - // We have a unique key restriction on the InterfaceName |
965 | - // field, which will cause the insert to fail if there is |
966 | - // another record with the same interface name in the table. |
967 | + // We have a unique key restrictions on the following fields: |
968 | + // - InterfaceName, MachineId |
969 | + // - MACAddress, NetworkName |
970 | + // These will cause the insert to fail if there is another record |
971 | + // with the same combination of values in the table. |
972 | // The txn logic does not report insertion errors, so we check |
973 | // that the record has actually been inserted correctly before |
974 | // reporting success. |
975 | - if err = m.st.networkInterfaces.FindId(macAddress).One(nil); err != nil { |
976 | - return nil, errors.AlreadyExistsf("%q on machine %q", interfaceName, m.doc.Id) |
977 | - } |
978 | - return newNetworkInterface(m.st, doc), nil |
979 | + if err = m.st.networkInterfaces.FindId(doc.Id).One(&doc); err == nil { |
980 | + return newNetworkInterface(m.st, doc), nil |
981 | + } |
982 | + sel := bson.D{{"interfacename", args.InterfaceName}, {"machineid", m.doc.Id}} |
983 | + if err = m.st.networkInterfaces.Find(sel).One(nil); err == nil { |
984 | + return nil, errors.AlreadyExistsf("%q on machine %q", args.InterfaceName, m.doc.Id) |
985 | + } |
986 | + sel = bson.D{{"macaddress", args.MACAddress}, {"networkname", args.NetworkName}} |
987 | + if err = m.st.networkInterfaces.Find(sel).One(nil); err == nil { |
988 | + return nil, errors.AlreadyExistsf("MAC address %q on network %q", args.MACAddress, args.NetworkName) |
989 | + } |
990 | + // Should never happen. |
991 | + logger.Errorf("unknown error while adding network interface doc %#v", doc) |
992 | } |
993 | return nil, err |
994 | } |
995 | |
996 | === modified file 'state/machine_test.go' |
997 | --- state/machine_test.go 2014-04-14 12:36:13 +0000 |
998 | +++ state/machine_test.go 2014-04-18 16:46:53 +0000 |
999 | @@ -12,6 +12,7 @@ |
1000 | gc "launchpad.net/gocheck" |
1001 | |
1002 | "launchpad.net/juju-core/constraints" |
1003 | + "launchpad.net/juju-core/environs/network" |
1004 | "launchpad.net/juju-core/errors" |
1005 | "launchpad.net/juju-core/instance" |
1006 | "launchpad.net/juju-core/state" |
1007 | @@ -389,12 +390,22 @@ |
1008 | } |
1009 | |
1010 | func addNetworkAndInterface(c *gc.C, st *state.State, machine *state.Machine, |
1011 | - networkName, providerId, cidr string, vlanTag int, |
1012 | + networkName, providerId, cidr string, vlanTag int, isVirtual bool, |
1013 | mac, ifaceName string, |
1014 | ) (*state.Network, *state.NetworkInterface) { |
1015 | - net, err := st.AddNetwork(networkName, providerId, cidr, vlanTag) |
1016 | + net, err := st.AddNetwork(state.NetworkInfo{ |
1017 | + Name: networkName, |
1018 | + ProviderId: network.Id(providerId), |
1019 | + CIDR: cidr, |
1020 | + VLANTag: vlanTag, |
1021 | + }) |
1022 | c.Assert(err, gc.IsNil) |
1023 | - iface, err := machine.AddNetworkInterface(mac, ifaceName, networkName) |
1024 | + iface, err := machine.AddNetworkInterface(state.NetworkInterfaceInfo{ |
1025 | + MACAddress: mac, |
1026 | + InterfaceName: ifaceName, |
1027 | + NetworkName: networkName, |
1028 | + IsVirtual: isVirtual, |
1029 | + }) |
1030 | c.Assert(err, gc.IsNil) |
1031 | return net, iface |
1032 | } |
1033 | @@ -419,11 +430,11 @@ |
1034 | |
1035 | net1, _ := addNetworkAndInterface( |
1036 | c, s.State, machine, |
1037 | - "net1", "net1", "0.1.2.0/24", 0, |
1038 | + "net1", "net1", "0.1.2.0/24", 0, false, |
1039 | "aa:bb:cc:dd:ee:f0", "eth0") |
1040 | net2, _ := addNetworkAndInterface( |
1041 | c, s.State, machine, |
1042 | - "net2", "net2", "0.2.2.0/24", 0, |
1043 | + "net2", "net2", "0.2.2.0/24", 0, false, |
1044 | "aa:bb:cc:dd:ee:f1", "eth1") |
1045 | |
1046 | nets, err = machine.Networks() |
1047 | @@ -448,15 +459,15 @@ |
1048 | // And a few networks and NICs. |
1049 | _, iface0 := addNetworkAndInterface( |
1050 | c, s.State, machine, |
1051 | - "net1", "net1", "0.1.2.0/24", 0, |
1052 | + "net1", "net1", "0.1.2.0/24", 0, false, |
1053 | "aa:bb:cc:dd:ee:f0", "eth0") |
1054 | _, iface1 := addNetworkAndInterface( |
1055 | c, s.State, machine, |
1056 | - "vlan42", "vlan42", "0.1.2.0/30", 42, |
1057 | + "vlan42", "vlan42", "0.1.2.0/30", 42, true, |
1058 | "aa:bb:cc:dd:ee:f1", "eth0.42") |
1059 | _, iface2 := addNetworkAndInterface( |
1060 | c, s.State, machine, |
1061 | - "net2", "net2", "0.2.2.0/24", 0, |
1062 | + "net2", "net2", "0.2.2.0/24", 0, false, |
1063 | "aa:bb:cc:dd:ee:f2", "eth1") |
1064 | |
1065 | ifaces, err = machine.NetworkInterfaces() |
1066 | @@ -476,49 +487,47 @@ |
1067 | } |
1068 | |
1069 | var addNetworkInterfaceErrorsTests = []struct { |
1070 | - macAddress string |
1071 | - interfaceName string |
1072 | - networkName string |
1073 | - beforeAdding func(*gc.C, *state.Machine) |
1074 | - expectErr string |
1075 | + args state.NetworkInterfaceInfo |
1076 | + beforeAdding func(*gc.C, *state.Machine) |
1077 | + expectErr string |
1078 | }{{ |
1079 | - "", "eth1", "net1", |
1080 | + state.NetworkInterfaceInfo{"", "eth1", "net1", false}, |
1081 | nil, |
1082 | `cannot add network interface "eth1" to machine "2": MAC address must be not empty`, |
1083 | }, { |
1084 | - "invalid", "eth1", "net1", |
1085 | + state.NetworkInterfaceInfo{"invalid", "eth1", "net1", false}, |
1086 | nil, |
1087 | `cannot add network interface "eth1" to machine "2": invalid MAC address: invalid`, |
1088 | }, { |
1089 | - "aa:bb:cc:dd:ee:f0", "eth1", "net1", |
1090 | + state.NetworkInterfaceInfo{"aa:bb:cc:dd:ee:f0", "eth1", "net1", false}, |
1091 | nil, |
1092 | - `cannot add network interface "eth1" to machine "2": interface with MAC address "aa:bb:cc:dd:ee:f0" already exists`, |
1093 | + `cannot add network interface "eth1" to machine "2": MAC address "aa:bb:cc:dd:ee:f0" on network "net1" already exists`, |
1094 | }, { |
1095 | - "aa:bb:cc:dd:ee:ff", "", "net1", |
1096 | + state.NetworkInterfaceInfo{"aa:bb:cc:dd:ee:ff", "", "net1", false}, |
1097 | nil, |
1098 | `cannot add network interface "" to machine "2": interface name must be not empty`, |
1099 | }, { |
1100 | - "aa:bb:cc:dd:ee:ff", "eth0", "net1", |
1101 | + state.NetworkInterfaceInfo{"aa:bb:cc:dd:ee:ff", "eth0", "net1", false}, |
1102 | nil, |
1103 | `cannot add network interface "eth0" to machine "2": "eth0" on machine "2" already exists`, |
1104 | }, { |
1105 | - "aa:bb:cc:dd:ee:ff", "eth1", "missing", |
1106 | + state.NetworkInterfaceInfo{"aa:bb:cc:dd:ee:ff", "eth1", "missing", false}, |
1107 | nil, |
1108 | `cannot add network interface "eth1" to machine "2": network "missing" not found`, |
1109 | }, { |
1110 | - "aa:bb:cc:dd:ee:f1", "eth1", "net1", |
1111 | + state.NetworkInterfaceInfo{"aa:bb:cc:dd:ee:f1", "eth1", "net1", false}, |
1112 | func(c *gc.C, m *state.Machine) { |
1113 | c.Check(m.SetProvisioned("i-am", "fake_nonce", nil), gc.IsNil) |
1114 | }, |
1115 | `cannot add network interface "eth1" to machine "2": machine already provisioned: dynamic network interfaces not currently supported`, |
1116 | }, { |
1117 | - "aa:bb:cc:dd:ee:f1", "eth1", "net1", |
1118 | + state.NetworkInterfaceInfo{"aa:bb:cc:dd:ee:f1", "eth1", "net1", false}, |
1119 | func(c *gc.C, m *state.Machine) { |
1120 | c.Check(m.EnsureDead(), gc.IsNil) |
1121 | }, |
1122 | `cannot add network interface "eth1" to machine "2": machine is not alive`, |
1123 | }, { |
1124 | - "aa:bb:cc:dd:ee:f1", "eth1", "net1", |
1125 | + state.NetworkInterfaceInfo{"aa:bb:cc:dd:ee:f1", "eth1", "net1", false}, |
1126 | func(c *gc.C, m *state.Machine) { |
1127 | c.Check(m.Remove(), gc.IsNil) |
1128 | }, |
1129 | @@ -534,18 +543,20 @@ |
1130 | c.Assert(err, gc.IsNil) |
1131 | addNetworkAndInterface( |
1132 | c, s.State, machine, |
1133 | - "net1", "provider-net1", "0.1.2.0/24", 0, |
1134 | + "net1", "provider-net1", "0.1.2.0/24", 0, false, |
1135 | "aa:bb:cc:dd:ee:f0", "eth0") |
1136 | + ifaces, err := machine.NetworkInterfaces() |
1137 | + c.Assert(err, gc.IsNil) |
1138 | + c.Assert(ifaces, gc.HasLen, 1) |
1139 | |
1140 | for i, test := range addNetworkInterfaceErrorsTests { |
1141 | - c.Logf("test %d: mac=%q, name=%q, network=%q", |
1142 | - i, test.macAddress, test.interfaceName, test.networkName) |
1143 | + c.Logf("test %d: %#v", i, test.args) |
1144 | |
1145 | if test.beforeAdding != nil { |
1146 | test.beforeAdding(c, machine) |
1147 | } |
1148 | |
1149 | - _, err = machine.AddNetworkInterface(test.macAddress, test.interfaceName, test.networkName) |
1150 | + _, err = machine.AddNetworkInterface(test.args) |
1151 | c.Check(err, gc.ErrorMatches, test.expectErr) |
1152 | if strings.Contains(test.expectErr, "not found") { |
1153 | c.Check(err, jc.Satisfies, errors.IsNotFound) |
1154 | @@ -685,9 +696,11 @@ |
1155 | |
1156 | func (s *MachineSuite) TestMachineSetInstanceInfoSuccess(c *gc.C) { |
1157 | c.Assert(s.machine.CheckProvisioned("fake_nonce"), gc.Equals, false) |
1158 | - networks := []state.NetworkInfo{{Name: "net1", ProviderId: "net1", CIDR: "0.1.2.0/24", VLANTag: 0}} |
1159 | + networks := []state.NetworkInfo{ |
1160 | + {Name: "net1", ProviderId: "net1", CIDR: "0.1.2.0/24", VLANTag: 0}, |
1161 | + } |
1162 | interfaces := []state.NetworkInterfaceInfo{ |
1163 | - {MACAddress: "aa:bb:cc:dd:ee:ff", NetworkName: "net1", InterfaceName: "eth0"}, |
1164 | + {MACAddress: "aa:bb:cc:dd:ee:ff", NetworkName: "net1", InterfaceName: "eth0", IsVirtual: false}, |
1165 | } |
1166 | err := s.machine.SetInstanceInfo("umbrella/0", "fake_nonce", nil, networks, interfaces) |
1167 | c.Assert(err, gc.IsNil) |
1168 | @@ -705,6 +718,7 @@ |
1169 | c.Check(ifaces[0].NetworkName(), gc.Equals, interfaces[0].NetworkName) |
1170 | c.Check(ifaces[0].MACAddress(), gc.Equals, interfaces[0].MACAddress) |
1171 | c.Check(ifaces[0].MachineTag(), gc.Equals, s.machine.Tag()) |
1172 | + c.Check(ifaces[0].IsVirtual(), gc.Equals, interfaces[0].IsVirtual) |
1173 | } |
1174 | |
1175 | func (s *MachineSuite) TestMachineSetProvisionedWhenNotAlive(c *gc.C) { |
1176 | |
1177 | === modified file 'state/networkinterfaces.go' |
1178 | --- state/networkinterfaces.go 2014-04-11 15:10:48 +0000 |
1179 | +++ state/networkinterfaces.go 2014-04-18 16:46:53 +0000 |
1180 | @@ -4,6 +4,10 @@ |
1181 | package state |
1182 | |
1183 | import ( |
1184 | + "fmt" |
1185 | + |
1186 | + "labix.org/v2/mgo/bson" |
1187 | + |
1188 | "launchpad.net/juju-core/names" |
1189 | ) |
1190 | |
1191 | @@ -27,25 +31,49 @@ |
1192 | |
1193 | // NetworkName is this interface's network name. |
1194 | NetworkName string |
1195 | + |
1196 | + // IsVirtual is true when the interface is a virtual device, as |
1197 | + // opposed to a physical device. |
1198 | + IsVirtual bool |
1199 | } |
1200 | |
1201 | // networkInterfaceDoc represents a network interface for a machine on |
1202 | // a given network. |
1203 | -// |
1204 | -// TODO(dimitern) To allow multiple virtual (e.g. VLAN) interfaces on |
1205 | -// the same MAC address, we need to change the key and uniqueness |
1206 | -// constraints. |
1207 | type networkInterfaceDoc struct { |
1208 | - MACAddress string `bson:"_id"` |
1209 | + Id bson.ObjectId `bson:"_id"` |
1210 | + MACAddress string |
1211 | InterfaceName string |
1212 | NetworkName string |
1213 | MachineId string |
1214 | + IsVirtual bool |
1215 | } |
1216 | |
1217 | func newNetworkInterface(st *State, doc *networkInterfaceDoc) *NetworkInterface { |
1218 | return &NetworkInterface{st, *doc} |
1219 | } |
1220 | |
1221 | +func newNetworkInterfaceDoc(args NetworkInterfaceInfo) *networkInterfaceDoc { |
1222 | + // This does not set the machine id. |
1223 | + return &networkInterfaceDoc{ |
1224 | + MACAddress: args.MACAddress, |
1225 | + InterfaceName: args.InterfaceName, |
1226 | + NetworkName: args.NetworkName, |
1227 | + IsVirtual: args.IsVirtual, |
1228 | + } |
1229 | +} |
1230 | + |
1231 | +// GoString implements fmt.GoStringer. |
1232 | +func (ni *NetworkInterface) GoString() string { |
1233 | + return fmt.Sprintf( |
1234 | + "&state.NetworkInterface{machineId: %q, mac: %q, name: %q, networkName: %q, isVirtual: %v}", |
1235 | + ni.MachineId(), ni.MACAddress(), ni.InterfaceName(), ni.NetworkName(), ni.IsVirtual()) |
1236 | +} |
1237 | + |
1238 | +// Id returns the internal juju-specific id of the interface. |
1239 | +func (ni *NetworkInterface) Id() string { |
1240 | + return ni.doc.Id.String() |
1241 | +} |
1242 | + |
1243 | // MACAddress returns the MAC address of the interface. |
1244 | func (ni *NetworkInterface) MACAddress() string { |
1245 | return ni.doc.MACAddress |
1246 | @@ -56,7 +84,7 @@ |
1247 | return ni.doc.InterfaceName |
1248 | } |
1249 | |
1250 | -// NetworkId returns the network name of the interface. |
1251 | +// NetworkName returns the network name of the interface. |
1252 | func (ni *NetworkInterface) NetworkName() string { |
1253 | return ni.doc.NetworkName |
1254 | } |
1255 | @@ -75,3 +103,15 @@ |
1256 | func (ni *NetworkInterface) MachineTag() string { |
1257 | return names.MachineTag(ni.doc.MachineId) |
1258 | } |
1259 | + |
1260 | +// IsVirtual returns whether the interface represents a virtual |
1261 | +// device. |
1262 | +func (ni *NetworkInterface) IsVirtual() bool { |
1263 | + return ni.doc.IsVirtual |
1264 | +} |
1265 | + |
1266 | +// IsPhysical returns whether the interface represents a physical |
1267 | +// device. |
1268 | +func (ni *NetworkInterface) IsPhysical() bool { |
1269 | + return !ni.doc.IsVirtual |
1270 | +} |
1271 | |
1272 | === modified file 'state/networkinterfaces_test.go' |
1273 | --- state/networkinterfaces_test.go 2014-04-10 14:40:34 +0000 |
1274 | +++ state/networkinterfaces_test.go 2014-04-18 16:46:53 +0000 |
1275 | @@ -4,6 +4,7 @@ |
1276 | package state_test |
1277 | |
1278 | import ( |
1279 | + jc "github.com/juju/testing/checkers" |
1280 | gc "launchpad.net/gocheck" |
1281 | |
1282 | "launchpad.net/juju-core/state" |
1283 | @@ -23,17 +24,25 @@ |
1284 | var err error |
1285 | s.machine, err = s.State.AddMachine("quantal", state.JobHostUnits) |
1286 | c.Assert(err, gc.IsNil) |
1287 | - s.network, err = s.State.AddNetwork("net1", "net1", "0.1.2.3/24", 42) |
1288 | + s.network, err = s.State.AddNetwork(state.NetworkInfo{"net1", "net1", "0.1.2.3/24", 42}) |
1289 | c.Assert(err, gc.IsNil) |
1290 | - s.iface, err = s.machine.AddNetworkInterface("aa:bb:cc:dd:ee:ff", "eth0", "net1") |
1291 | + s.iface, err = s.machine.AddNetworkInterface(state.NetworkInterfaceInfo{ |
1292 | + MACAddress: "aa:bb:cc:dd:ee:ff", |
1293 | + InterfaceName: "eth0", |
1294 | + NetworkName: "net1", |
1295 | + IsVirtual: true, |
1296 | + }) |
1297 | c.Assert(err, gc.IsNil) |
1298 | } |
1299 | |
1300 | func (s *NetworkInterfaceSuite) TestGetterMethods(c *gc.C) { |
1301 | + c.Assert(s.iface.Id(), gc.Not(gc.Equals), "") |
1302 | c.Assert(s.iface.MACAddress(), gc.Equals, "aa:bb:cc:dd:ee:ff") |
1303 | c.Assert(s.iface.InterfaceName(), gc.Equals, "eth0") |
1304 | c.Assert(s.iface.NetworkName(), gc.Equals, s.network.Name()) |
1305 | c.Assert(s.iface.NetworkTag(), gc.Equals, s.network.Tag()) |
1306 | c.Assert(s.iface.MachineId(), gc.Equals, s.machine.Id()) |
1307 | c.Assert(s.iface.MachineTag(), gc.Equals, s.machine.Tag()) |
1308 | + c.Assert(s.iface.IsVirtual(), jc.IsTrue) |
1309 | + c.Assert(s.iface.IsPhysical(), jc.IsFalse) |
1310 | } |
1311 | |
1312 | === modified file 'state/networks.go' |
1313 | --- state/networks.go 2014-04-11 15:10:48 +0000 |
1314 | +++ state/networks.go 2014-04-18 16:46:53 +0000 |
1315 | @@ -4,8 +4,11 @@ |
1316 | package state |
1317 | |
1318 | import ( |
1319 | + "fmt" |
1320 | + |
1321 | "labix.org/v2/mgo/bson" |
1322 | |
1323 | + "launchpad.net/juju-core/environs/network" |
1324 | "launchpad.net/juju-core/names" |
1325 | ) |
1326 | |
1327 | @@ -21,7 +24,7 @@ |
1328 | Name string |
1329 | |
1330 | // ProviderId is a provider-specific network id. |
1331 | - ProviderId string |
1332 | + ProviderId network.Id |
1333 | |
1334 | // CIDR of the network, in 123.45.67.89/24 format. |
1335 | CIDR string |
1336 | @@ -38,7 +41,7 @@ |
1337 | // included networks. |
1338 | Name string `bson:"_id"` |
1339 | |
1340 | - ProviderId string |
1341 | + ProviderId network.Id |
1342 | CIDR string |
1343 | VLANTag int |
1344 | } |
1345 | @@ -47,13 +50,29 @@ |
1346 | return &Network{st, *doc} |
1347 | } |
1348 | |
1349 | +func newNetworkDoc(args NetworkInfo) *networkDoc { |
1350 | + return &networkDoc{ |
1351 | + Name: args.Name, |
1352 | + ProviderId: args.ProviderId, |
1353 | + CIDR: args.CIDR, |
1354 | + VLANTag: args.VLANTag, |
1355 | + } |
1356 | +} |
1357 | + |
1358 | +// GoString implements fmt.GoStringer. |
1359 | +func (n *Network) GoString() string { |
1360 | + return fmt.Sprintf( |
1361 | + "&state.Network{name: %q, providerId: %q, cidr: %q, vlanTag: %v}", |
1362 | + n.Name(), n.ProviderId(), n.CIDR(), n.VLANTag()) |
1363 | +} |
1364 | + |
1365 | // Name returns the network name. |
1366 | func (n *Network) Name() string { |
1367 | return n.doc.Name |
1368 | } |
1369 | |
1370 | // ProviderId returns the provider-specific id of the network. |
1371 | -func (n *Network) ProviderId() string { |
1372 | +func (n *Network) ProviderId() network.Id { |
1373 | return n.doc.ProviderId |
1374 | } |
1375 | |
1376 | |
1377 | === modified file 'state/networks_test.go' |
1378 | --- state/networks_test.go 2014-04-10 14:40:34 +0000 |
1379 | +++ state/networks_test.go 2014-04-18 16:46:53 +0000 |
1380 | @@ -24,15 +24,15 @@ |
1381 | var err error |
1382 | s.machine, err = s.State.AddMachine("quantal", state.JobHostUnits) |
1383 | c.Assert(err, gc.IsNil) |
1384 | - s.network, err = s.State.AddNetwork("net1", "net1", "0.1.2.3/24", 0) |
1385 | + s.network, err = s.State.AddNetwork(state.NetworkInfo{"net1", "net1", "0.1.2.3/24", 0}) |
1386 | c.Assert(err, gc.IsNil) |
1387 | - s.vlan, err = s.State.AddNetwork("vlan", "vlan", "0.1.2.3/30", 42) |
1388 | + s.vlan, err = s.State.AddNetwork(state.NetworkInfo{"vlan", "vlan", "0.1.2.3/30", 42}) |
1389 | c.Assert(err, gc.IsNil) |
1390 | } |
1391 | |
1392 | func (s *NetworkSuite) TestGetterMethods(c *gc.C) { |
1393 | c.Assert(s.network.Name(), gc.Equals, "net1") |
1394 | - c.Assert(s.network.ProviderId(), gc.Equals, "net1") |
1395 | + c.Assert(string(s.network.ProviderId()), gc.Equals, "net1") |
1396 | c.Assert(s.network.Tag(), gc.Equals, "network-net1") |
1397 | c.Assert(s.network.CIDR(), gc.Equals, "0.1.2.3/24") |
1398 | c.Assert(s.network.VLANTag(), gc.Equals, 0) |
1399 | @@ -46,9 +46,19 @@ |
1400 | c.Assert(err, gc.IsNil) |
1401 | c.Assert(ifaces, gc.HasLen, 0) |
1402 | |
1403 | - iface0, err := s.machine.AddNetworkInterface("aa:bb:cc:dd:ee:f0", "eth0", "net1") |
1404 | + iface0, err := s.machine.AddNetworkInterface(state.NetworkInterfaceInfo{ |
1405 | + MACAddress: "aa:bb:cc:dd:ee:f0", |
1406 | + InterfaceName: "eth0", |
1407 | + NetworkName: "net1", |
1408 | + IsVirtual: false, |
1409 | + }) |
1410 | c.Assert(err, gc.IsNil) |
1411 | - iface1, err := s.machine.AddNetworkInterface("aa:bb:cc:dd:ee:f1", "eth1", "net1") |
1412 | + iface1, err := s.machine.AddNetworkInterface(state.NetworkInterfaceInfo{ |
1413 | + MACAddress: "aa:bb:cc:dd:ee:f1", |
1414 | + InterfaceName: "eth1", |
1415 | + NetworkName: "net1", |
1416 | + IsVirtual: false, |
1417 | + }) |
1418 | c.Assert(err, gc.IsNil) |
1419 | |
1420 | ifaces, err = s.network.Interfaces() |
1421 | |
1422 | === modified file 'state/open.go' |
1423 | --- state/open.go 2014-04-17 17:30:48 +0000 |
1424 | +++ state/open.go 2014-04-18 16:46:53 +0000 |
1425 | @@ -209,6 +209,7 @@ |
1426 | {"users", []string{"name"}, false}, |
1427 | {"networks", []string{"providerid"}, true}, |
1428 | {"networkinterfaces", []string{"interfacename", "machineid"}, true}, |
1429 | + {"networkinterfaces", []string{"macaddress", "networkname"}, true}, |
1430 | {"networkinterfaces", []string{"networkname"}, false}, |
1431 | {"networkinterfaces", []string{"machineid"}, false}, |
1432 | } |
1433 | |
1434 | === modified file 'state/state.go' |
1435 | --- state/state.go 2014-04-17 16:18:46 +0000 |
1436 | +++ state/state.go 2014-04-18 16:46:53 +0000 |
1437 | @@ -1014,47 +1014,41 @@ |
1438 | return svc, nil |
1439 | } |
1440 | |
1441 | -// AddNetwork creates a new network with the given name, |
1442 | -// provider-specific id, CIDR and VLAN tag. If a network with the same |
1443 | -// name or provider id already exists in state, an error satisfying |
1444 | -// errors.IsAlreadyExists is returned. |
1445 | -func (st *State) AddNetwork(name, providerId, cidr string, vlanTag int) (n *Network, err error) { |
1446 | - defer errors.Contextf(&err, "cannot add network %q", name) |
1447 | - if cidr != "" { |
1448 | - _, _, err := net.ParseCIDR(cidr) |
1449 | +// AddNetwork creates a new network with the given params. If a |
1450 | +// network with the same name or provider id already exists in state, |
1451 | +// an error satisfying errors.IsAlreadyExists is returned. |
1452 | +func (st *State) AddNetwork(args NetworkInfo) (n *Network, err error) { |
1453 | + defer errors.Contextf(&err, "cannot add network %q", args.Name) |
1454 | + if args.CIDR != "" { |
1455 | + _, _, err := net.ParseCIDR(args.CIDR) |
1456 | if err != nil { |
1457 | return nil, err |
1458 | } |
1459 | } |
1460 | - if name == "" { |
1461 | + if args.Name == "" { |
1462 | return nil, fmt.Errorf("name must be not empty") |
1463 | } |
1464 | - if !names.IsNetwork(name) { |
1465 | + if !names.IsNetwork(args.Name) { |
1466 | return nil, fmt.Errorf("invalid name") |
1467 | } |
1468 | - if providerId == "" { |
1469 | + if args.ProviderId == "" { |
1470 | return nil, fmt.Errorf("provider id must be not empty") |
1471 | } |
1472 | - if vlanTag < 0 || vlanTag > 4094 { |
1473 | - return nil, fmt.Errorf("invalid VLAN tag %d: must be between 0 and 4094", vlanTag) |
1474 | - } |
1475 | - doc := &networkDoc{ |
1476 | - Name: name, |
1477 | - ProviderId: providerId, |
1478 | - CIDR: cidr, |
1479 | - VLANTag: vlanTag, |
1480 | - } |
1481 | + if args.VLANTag < 0 || args.VLANTag > 4094 { |
1482 | + return nil, fmt.Errorf("invalid VLAN tag %d: must be between 0 and 4094", args.VLANTag) |
1483 | + } |
1484 | + doc := newNetworkDoc(args) |
1485 | ops := []txn.Op{{ |
1486 | C: st.networks.Name, |
1487 | - Id: name, |
1488 | + Id: args.Name, |
1489 | Assert: txn.DocMissing, |
1490 | Insert: doc, |
1491 | }} |
1492 | err = st.runTransaction(ops) |
1493 | switch err { |
1494 | case txn.ErrAborted: |
1495 | - if _, err = st.Network(name); err == nil { |
1496 | - return nil, errors.AlreadyExistsf("network %q", name) |
1497 | + if _, err = st.Network(args.Name); err == nil { |
1498 | + return nil, errors.AlreadyExistsf("network %q", args.Name) |
1499 | } else if err != nil { |
1500 | return nil, err |
1501 | } |
1502 | @@ -1065,8 +1059,8 @@ |
1503 | // logic does not report insertion errors, so we check that |
1504 | // the record has actually been inserted correctly before |
1505 | // reporting success. |
1506 | - if _, err = st.Network(name); err != nil { |
1507 | - return nil, errors.AlreadyExistsf("network with provider id %q", providerId) |
1508 | + if _, err = st.Network(args.Name); err != nil { |
1509 | + return nil, errors.AlreadyExistsf("network with provider id %q", args.ProviderId) |
1510 | } |
1511 | return newNetwork(st, doc), nil |
1512 | } |
1513 | |
1514 | === modified file 'state/state_test.go' |
1515 | --- state/state_test.go 2014-04-17 15:32:49 +0000 |
1516 | +++ state/state_test.go 2014-04-18 16:46:53 +0000 |
1517 | @@ -963,34 +963,31 @@ |
1518 | } |
1519 | |
1520 | var addNetworkErrorsTests = []struct { |
1521 | - name string |
1522 | - providerId string |
1523 | - cidr string |
1524 | - vlanTag int |
1525 | - expectErr string |
1526 | + args state.NetworkInfo |
1527 | + expectErr string |
1528 | }{{ |
1529 | - "", "provider-id", "0.3.1.0/24", 0, |
1530 | + state.NetworkInfo{"", "provider-id", "0.3.1.0/24", 0}, |
1531 | `cannot add network "": name must be not empty`, |
1532 | }, { |
1533 | - "-invalid-", "provider-id", "0.3.1.0/24", 0, |
1534 | + state.NetworkInfo{"-invalid-", "provider-id", "0.3.1.0/24", 0}, |
1535 | `cannot add network "-invalid-": invalid name`, |
1536 | }, { |
1537 | - "net2", "", "0.3.1.0/24", 0, |
1538 | + state.NetworkInfo{"net2", "", "0.3.1.0/24", 0}, |
1539 | `cannot add network "net2": provider id must be not empty`, |
1540 | }, { |
1541 | - "net2", "provider-id", "invalid", 0, |
1542 | + state.NetworkInfo{"net2", "provider-id", "invalid", 0}, |
1543 | `cannot add network "net2": invalid CIDR address: invalid`, |
1544 | }, { |
1545 | - "net2", "provider-id", "0.3.1.0/24", -1, |
1546 | + state.NetworkInfo{"net2", "provider-id", "0.3.1.0/24", -1}, |
1547 | `cannot add network "net2": invalid VLAN tag -1: must be between 0 and 4094`, |
1548 | }, { |
1549 | - "net2", "provider-id", "0.3.1.0/24", 9999, |
1550 | + state.NetworkInfo{"net2", "provider-id", "0.3.1.0/24", 9999}, |
1551 | `cannot add network "net2": invalid VLAN tag 9999: must be between 0 and 4094`, |
1552 | }, { |
1553 | - "net1", "provider-id", "0.3.1.0/24", 0, |
1554 | + state.NetworkInfo{"net1", "provider-id", "0.3.1.0/24", 0}, |
1555 | `cannot add network "net1": network "net1" already exists`, |
1556 | }, { |
1557 | - "net2", "provider-net1", "0.3.1.0/24", 0, |
1558 | + state.NetworkInfo{"net2", "provider-net1", "0.3.1.0/24", 0}, |
1559 | `cannot add network "net2": network with provider id "provider-net1" already exists`, |
1560 | }} |
1561 | |
1562 | @@ -1005,22 +1002,21 @@ |
1563 | |
1564 | net1, _ := addNetworkAndInterface( |
1565 | c, s.State, machine, |
1566 | - "net1", "provider-net1", "0.1.2.0/24", 0, |
1567 | + "net1", "provider-net1", "0.1.2.0/24", 0, false, |
1568 | "aa:bb:cc:dd:ee:f0", "eth0") |
1569 | |
1570 | net, err := s.State.Network("net1") |
1571 | c.Assert(err, gc.IsNil) |
1572 | c.Assert(net, gc.DeepEquals, net1) |
1573 | c.Assert(net.Name(), gc.Equals, "net1") |
1574 | - c.Assert(net.ProviderId(), gc.Equals, "provider-net1") |
1575 | + c.Assert(string(net.ProviderId()), gc.Equals, "provider-net1") |
1576 | _, err = s.State.Network("missing") |
1577 | c.Assert(err, jc.Satisfies, errors.IsNotFound) |
1578 | c.Assert(err, gc.ErrorMatches, `network "missing" not found`) |
1579 | |
1580 | for i, test := range addNetworkErrorsTests { |
1581 | - c.Logf("test %d: name=%q, providerId=%q, cidr=%q, vlanTag=%d", |
1582 | - i, test.name, test.providerId, test.cidr, test.vlanTag) |
1583 | - _, err := s.State.AddNetwork(test.name, test.providerId, test.cidr, test.vlanTag) |
1584 | + c.Logf("test %d: %#v", i, test.args) |
1585 | + _, err := s.State.AddNetwork(test.args) |
1586 | c.Check(err, gc.ErrorMatches, test.expectErr) |
1587 | if strings.Contains(test.expectErr, "already exists") { |
1588 | c.Check(err, jc.Satisfies, errors.IsAlreadyExists) |
1589 | @@ -2280,10 +2276,15 @@ |
1590 | rel, err := s.State.AddRelation(eps...) |
1591 | c.Assert(err, gc.IsNil) |
1592 | c.Assert(rel.String(), gc.Equals, "wordpress:db ser-vice2:server") |
1593 | - net1, err := s.State.AddNetwork("net1", "provider-id", "0.1.2.0/24", 0) |
1594 | + net1, err := s.State.AddNetwork(state.NetworkInfo{ |
1595 | + Name: "net1", |
1596 | + ProviderId: "provider-id", |
1597 | + CIDR: "0.1.2.0/24", |
1598 | + VLANTag: 0, |
1599 | + }) |
1600 | c.Assert(err, gc.IsNil) |
1601 | c.Assert(net1.Tag(), gc.Equals, "network-net1") |
1602 | - c.Assert(net1.ProviderId(), gc.Equals, "provider-id") |
1603 | + c.Assert(string(net1.ProviderId()), gc.Equals, "provider-id") |
1604 | |
1605 | // environment tag is dynamically generated |
1606 | env, err := s.State.Environment() |
1607 | @@ -2375,7 +2376,12 @@ |
1608 | c.Assert(err, gc.IsNil) |
1609 | |
1610 | // Parse a network name. |
1611 | - net1, err := s.State.AddNetwork("net1", "provider-id", "0.1.2.0/24", 0) |
1612 | + net1, err := s.State.AddNetwork(state.NetworkInfo{ |
1613 | + Name: "net1", |
1614 | + ProviderId: "provider-id", |
1615 | + CIDR: "0.1.2.0/24", |
1616 | + VLANTag: 0, |
1617 | + }) |
1618 | c.Assert(err, gc.IsNil) |
1619 | coll, id, err = state.ParseTag(s.State, net1.Tag()) |
1620 | c.Assert(coll, gc.Equals, "networks") |
1621 | |
1622 | === modified file 'worker/provisioner/kvm-broker.go' |
1623 | --- worker/provisioner/kvm-broker.go 2014-04-08 03:37:14 +0000 |
1624 | +++ worker/provisioner/kvm-broker.go 2014-04-18 16:46:53 +0000 |
1625 | @@ -12,6 +12,7 @@ |
1626 | "launchpad.net/juju-core/container" |
1627 | "launchpad.net/juju-core/container/kvm" |
1628 | "launchpad.net/juju-core/environs" |
1629 | + "launchpad.net/juju-core/environs/network" |
1630 | "launchpad.net/juju-core/instance" |
1631 | "launchpad.net/juju-core/tools" |
1632 | ) |
1633 | @@ -54,7 +55,7 @@ |
1634 | } |
1635 | |
1636 | // StartInstance is specified in the Broker interface. |
1637 | -func (broker *kvmBroker) StartInstance(args environs.StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, []environs.NetworkInfo, error) { |
1638 | +func (broker *kvmBroker) StartInstance(args environs.StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, []network.Info, error) { |
1639 | if args.MachineConfig.HasNetworks() { |
1640 | return nil, nil, nil, fmt.Errorf("starting kvm containers with networks is not supported yet.") |
1641 | } |
1642 | |
1643 | === modified file 'worker/provisioner/lxc-broker.go' |
1644 | --- worker/provisioner/lxc-broker.go 2014-04-08 03:37:14 +0000 |
1645 | +++ worker/provisioner/lxc-broker.go 2014-04-18 16:46:53 +0000 |
1646 | @@ -12,6 +12,7 @@ |
1647 | "launchpad.net/juju-core/container" |
1648 | "launchpad.net/juju-core/container/lxc" |
1649 | "launchpad.net/juju-core/environs" |
1650 | + "launchpad.net/juju-core/environs/network" |
1651 | "launchpad.net/juju-core/instance" |
1652 | "launchpad.net/juju-core/state/api/params" |
1653 | "launchpad.net/juju-core/tools" |
1654 | @@ -55,7 +56,7 @@ |
1655 | } |
1656 | |
1657 | // StartInstance is specified in the Broker interface. |
1658 | -func (broker *lxcBroker) StartInstance(args environs.StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, []environs.NetworkInfo, error) { |
1659 | +func (broker *lxcBroker) StartInstance(args environs.StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, []network.Info, error) { |
1660 | if args.MachineConfig.HasNetworks() { |
1661 | return nil, nil, nil, fmt.Errorf("starting lxc containers with networks is not supported yet.") |
1662 | } |
1663 | |
1664 | === modified file 'worker/provisioner/provisioner_task.go' |
1665 | --- worker/provisioner/provisioner_task.go 2014-04-17 13:21:44 +0000 |
1666 | +++ worker/provisioner/provisioner_task.go 2014-04-18 16:46:53 +0000 |
1667 | @@ -11,6 +11,7 @@ |
1668 | "launchpad.net/juju-core/constraints" |
1669 | "launchpad.net/juju-core/environs" |
1670 | "launchpad.net/juju-core/environs/cloudinit" |
1671 | + "launchpad.net/juju-core/environs/network" |
1672 | "launchpad.net/juju-core/environs/tools" |
1673 | "launchpad.net/juju-core/instance" |
1674 | "launchpad.net/juju-core/names" |
1675 | @@ -421,7 +422,7 @@ |
1676 | return nil |
1677 | } |
1678 | |
1679 | -func (task *provisionerTask) prepareNetworkAndInterfaces(networkInfo []environs.NetworkInfo) ( |
1680 | +func (task *provisionerTask) prepareNetworkAndInterfaces(networkInfo []network.Info) ( |
1681 | networks []params.Network, ifaces []params.NetworkInterface) { |
1682 | if len(networkInfo) == 0 { |
1683 | return nil, nil |
1684 | @@ -429,19 +430,20 @@ |
1685 | visitedNetworks := set.NewStrings() |
1686 | for _, info := range networkInfo { |
1687 | networkTag := names.NetworkTag(info.NetworkName) |
1688 | - if !visitedNetworks.Contains(info.NetworkId) { |
1689 | + if !visitedNetworks.Contains(networkTag) { |
1690 | networks = append(networks, params.Network{ |
1691 | Tag: networkTag, |
1692 | - ProviderId: info.NetworkId, |
1693 | + ProviderId: info.ProviderId, |
1694 | CIDR: info.CIDR, |
1695 | VLANTag: info.VLANTag, |
1696 | }) |
1697 | - visitedNetworks.Add(info.NetworkId) |
1698 | + visitedNetworks.Add(networkTag) |
1699 | } |
1700 | ifaces = append(ifaces, params.NetworkInterface{ |
1701 | InterfaceName: info.InterfaceName, |
1702 | MACAddress: info.MACAddress, |
1703 | NetworkTag: networkTag, |
1704 | + IsVirtual: info.IsVirtual, |
1705 | }) |
1706 | } |
1707 | return networks, ifaces |
1708 | |
1709 | === modified file 'worker/provisioner/provisioner_test.go' |
1710 | --- worker/provisioner/provisioner_test.go 2014-04-14 12:36:13 +0000 |
1711 | +++ worker/provisioner/provisioner_test.go 2014-04-18 16:46:53 +0000 |
1712 | @@ -14,6 +14,7 @@ |
1713 | "launchpad.net/juju-core/constraints" |
1714 | "launchpad.net/juju-core/environs" |
1715 | "launchpad.net/juju-core/environs/config" |
1716 | + "launchpad.net/juju-core/environs/network" |
1717 | "launchpad.net/juju-core/environs/simplestreams" |
1718 | "launchpad.net/juju-core/environs/tools" |
1719 | "launchpad.net/juju-core/errors" |
1720 | @@ -165,7 +166,7 @@ |
1721 | return s.checkStartInstanceCustom(c, m, "pork", s.defaultConstraints, nil, nil, nil, true) |
1722 | } |
1723 | |
1724 | -func (s *CommonProvisionerSuite) checkStartInstanceCustom(c *gc.C, m *state.Machine, secret string, cons constraints.Value, includeNetworks, excludeNetworks []string, networkInfo []environs.NetworkInfo, waitInstanceId bool) (inst instance.Instance) { |
1725 | +func (s *CommonProvisionerSuite) checkStartInstanceCustom(c *gc.C, m *state.Machine, secret string, cons constraints.Value, includeNetworks, excludeNetworks []string, networkInfo []network.Info, waitInstanceId bool) (inst instance.Instance) { |
1726 | s.BackingState.StartSync() |
1727 | for { |
1728 | select { |
1729 | @@ -473,20 +474,22 @@ |
1730 | // Add and provision a machine with networks specified. |
1731 | includeNetworks := []string{"net1", "net2"} |
1732 | excludeNetworks := []string{"net3", "net4"} |
1733 | - expectNetworkInfo := []environs.NetworkInfo{{ |
1734 | + expectNetworkInfo := []network.Info{{ |
1735 | MACAddress: "aa:bb:cc:dd:ee:f0", |
1736 | InterfaceName: "eth0", |
1737 | - NetworkId: "net1", |
1738 | + ProviderId: "net1", |
1739 | NetworkName: "net1", |
1740 | VLANTag: 0, |
1741 | CIDR: "0.1.2.0/24", |
1742 | + IsVirtual: false, |
1743 | }, { |
1744 | MACAddress: "aa:bb:cc:dd:ee:f1", |
1745 | InterfaceName: "eth1", |
1746 | - NetworkId: "net2", |
1747 | + ProviderId: "net2", |
1748 | NetworkName: "net2", |
1749 | VLANTag: 1, |
1750 | CIDR: "0.2.2.0/24", |
1751 | + IsVirtual: true, |
1752 | }} |
1753 | m, err := s.addMachineWithRequestedNetworks(includeNetworks, excludeNetworks) |
1754 | c.Assert(err, gc.IsNil) |
1755 | @@ -519,9 +522,9 @@ |
1756 | // Add and provision a machine with networks specified. |
1757 | includeNetworks := []string{"bad-net1"} |
1758 | // "bad-" prefix for networks causes dummy provider to report |
1759 | - // invalid NetworkInfo. |
1760 | - expectNetworkInfo := []environs.NetworkInfo{ |
1761 | - {NetworkId: "bad-net1", NetworkName: "bad-net1", CIDR: "invalid"}, |
1762 | + // invalid network.Info. |
1763 | + expectNetworkInfo := []network.Info{ |
1764 | + {ProviderId: "bad-net1", NetworkName: "bad-net1", CIDR: "invalid"}, |
1765 | } |
1766 | m, err := s.addMachineWithRequestedNetworks(includeNetworks, nil) |
1767 | c.Assert(err, gc.IsNil) |
1768 | @@ -924,7 +927,7 @@ |
1769 | retryCount map[string]int |
1770 | } |
1771 | |
1772 | -func (b *mockBroker) StartInstance(args environs.StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, []environs.NetworkInfo, error) { |
1773 | +func (b *mockBroker) StartInstance(args environs.StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, []network.Info, error) { |
1774 | // All machines except machines 3, 4 are provisioned successfully the first time. |
1775 | // Machines 3 is provisioned after some attempts have been made. |
1776 | // Machine 4 is never provisioned. |
Reviewers: mp+216453_ code.launchpad. net,
Message:
Please take a look.
Description:
various: Support multiple NICs with the same MAC
Fixes bug #1307513, allowing more than one network
interface with the same MAC address, and introduces
the concept of virtual and physical network and NIC.
This is needed because you can have more than one
network interface with the same MAC address on the
same machine (but it has to be on different networks):
one physical and multiple virtual NICs. Changes were
needed mostly in state to use a different primary
key for networkinterfaces collection. Also added a
unique index on "interfacename" and "networkname",
improved tests.
Fixed the MAAS provider to configure networks properly,
so that you can have more than one VLAN virtual NIC
on the same physical NIC.
In a follow-up, machine addresses and networks will
be integrated, so we can link a network interface's
name and MAC address to its network (IP) address.
Finally, networks and machine interfaces will be
visible in juju status.
https:/ /code.launchpad .net/~dimitern/ juju-core/ 400-lp- 1307513- multiple- nics-same- mac/+merge/ 216453
(do not edit description out of merge proposal)
Please review this at https:/ /codereview. appspot. com/89260044/
Affected files (+408, -180 lines): dummy/environs. go maas/environ. go maas/environ_ test.go params/ internal. go provisioner/ provisioner_ test.go /provisioner/ provisioner. go /provisioner/ provisioner_ test.go test.go terfaces. go terfaces_ test.go test.go provisioner/ provisioner_ task.go provisioner/ provisioner_ test.go
A [revision details]
M environs/broker.go
M provider/
M provider/
M provider/
M state/api/
M state/api/
M state/apiserver
M state/apiserver
M state/machine.go
M state/machine_
M state/networkin
M state/networkin
M state/networks.go
M state/networks_
M state/open.go
M state/state.go
M state/state_test.go
M worker/
M worker/