Merge lp:~dimitern/juju-core/350-api-service-deploy-with-networks into lp:~go-bot/juju-core/trunk

[Dimiter Naydenov]

state/api: Add Client.ServiceDeployWithNetworks

This adds a new client API call: ServiceDeployWithNetworks,
which works exactly like ServiceDeploy, but takes two extra
arguments: IncludeNetworks and ExcludeNetworks.
These specify a list of VLANs/networks to enable or disable
at boot time for the machine being deployed, and also record
that into the service document.

AddUnits is also changed (and its tests) to respect networks
set on the service and pass them through MachineTemplate when
deploying or a new machine or container.

https://codereview.appspot.com/76910044/

R=gz, rogpeppe

state/api: Add Client.ServiceDeployWithNetworks

This adds a new client API call: ServiceDeployWithNetworks,
which works exactly like ServiceDeploy, but takes two extra
arguments: IncludeNetworks and ExcludeNetworks.
These specify a list of VLANs/networks to enable or disable
at boot time for the machine being deployed, and also record
that into the service document.

AddUnits is also changed (and its tests) to respect networks
set on the service and pass them through MachineTemplate when
deploying or a new machine or container.

https://codereview.appspot.com/76910044/

[Dimiter Naydenov (dimitern)]

Reviewers: mp+211773_code.launchpad.net,

Message:
Please take a look.

Description:
state/api: Add Client.ServiceDeployWithNetworks

This adds a new client API call: ServiceDeployWithNetworks,
which works exactly like ServiceDeploy, but takes two extra
arguments (required) IncludedNetworks and ExcludedNetworks.
These specify a list of VLANs/networks to enable or disable
at boot time for the machine being deployed, and also record
that into the service document.

Right now, the new API does not change the service and/or
machine documents to add the networks, because there's another
branch mgz is doing for that right now.

https://code.launchpad.net/~dimitern/juju-core/350-api-service-deploy-with-networks/+merge/211773

(do not edit description out of merge proposal)

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

Affected files (+162, -37 lines):
   A [revision details]
   M juju/deploy.go
   M state/api/client.go
   M state/api/params/params.go
   M state/apiserver/client/client.go
   M state/apiserver/client/client_test.go
   M state/apiserver/client/perm_test.go

[William Reade (fwereade)]

1) it's hard to approve this without an implementation in the background

2) should we be talking network tags?

https://codereview.appspot.com/76910044/diff/20001/state/apiserver/client/client.go
File state/apiserver/client/client.go (right):

https://codereview.appspot.com/76910044/diff/20001/state/apiserver/client/client.go#newcode243
state/apiserver/client/client.go:243: return c.serviceDeploy(&args, nil)
might be nicer to copy args into a ServiceDeployWithNetworks...

https://codereview.appspot.com/76910044/diff/20001/state/apiserver/client/client.go#newcode256
state/apiserver/client/client.go:256: func (c *Client)
serviceDeploy(args0 *params.ServiceDeploy, args1
*params.ServiceDeployWithNetworks) error {
...and just handle one arg type here?

https://codereview.appspot.com/76910044/

[Roger Peppe (rogpeppe)]

https://codereview.appspot.com/76910044/diff/20001/state/api/params/params.go
File state/api/params/params.go (right):

https://codereview.appspot.com/76910044/diff/20001/state/api/params/params.go#newcode155
state/api/params/params.go:155: type ServiceDeployWithNetworks struct {
I suggest that instead of making a new set of parameters, we could just
add the networks parameters to ServiceDeploy, and make
ServiceDeployWithNetworks an alias of Deploy.

We can document the new fields as recognised only in 1.18 (1.19? 1.20?)
API servers.

This means that clients that care that the networks fields are
recognised can use ServiceDeployWithNetworks, but existing clients can
continue to use Deploy, and eventually we should be able to deprecate
ServiceDeployWithNetworks.

This adds minimal clutter to the code, and provides the same
functionality AFAICS.

https://codereview.appspot.com/76910044/

[Dimiter Naydenov (dimitern)]

Please take a look.

https://codereview.appspot.com/76910044/diff/20001/state/api/params/params.go
File state/api/params/params.go (right):

https://codereview.appspot.com/76910044/diff/20001/state/api/params/params.go#newcode155
state/api/params/params.go:155: type ServiceDeployWithNetworks struct {
On 2014/03/20 14:06:23, rog wrote:
> I suggest that instead of making a new set of parameters, we could
just add the
> networks parameters to ServiceDeploy, and make
ServiceDeployWithNetworks an
> alias of Deploy.

> We can document the new fields as recognised only in 1.18 (1.19?
1.20?) API
> servers.

> This means that clients that care that the networks fields are
recognised can
> use ServiceDeployWithNetworks, but existing clients can continue to
use Deploy,
> and eventually we should be able to deprecate
ServiceDeployWithNetworks.

> This adds minimal clutter to the code, and provides the same
functionality
> AFAICS.

Done.

https://codereview.appspot.com/76910044/diff/20001/state/apiserver/client/client.go
File state/apiserver/client/client.go (right):

https://codereview.appspot.com/76910044/diff/20001/state/apiserver/client/client.go#newcode243
state/apiserver/client/client.go:243: return c.serviceDeploy(&args, nil)
On 2014/03/20 13:42:50, fwereade wrote:
> might be nicer to copy args into a ServiceDeployWithNetworks...

Done as rog suggested - ServiceDeployWithNetworks as an alias for
ServiceDeploy.

https://codereview.appspot.com/76910044/diff/20001/state/apiserver/client/client.go#newcode256
state/apiserver/client/client.go:256: func (c *Client)
serviceDeploy(args0 *params.ServiceDeploy, args1
*params.ServiceDeployWithNetworks) error {
On 2014/03/20 13:42:50, fwereade wrote:
> ...and just handle one arg type here?

Done differently.

https://codereview.appspot.com/76910044/

[Roger Peppe (rogpeppe)]

much nicer, thanks.
LGTM with one suggestion below.

https://codereview.appspot.com/76910044/diff/40001/state/apiserver/client/client.go
File state/apiserver/client/client.go (right):

https://codereview.appspot.com/76910044/diff/40001/state/apiserver/client/client.go#newcode298
state/apiserver/client/client.go:298: if len(args.IncludedNetworks) == 0
&& len(args.ExcludedNetworks) == 0 {
I'd remove this and make the two commands exactly the same. Then clients
can use this call for all Deploy purposes if they wish.

https://codereview.appspot.com/76910044/

[John A Meinel (jameinel)]

https://codereview.appspot.com/76910044/diff/40001/state/apiserver/client/client.go
File state/apiserver/client/client.go (right):

https://codereview.appspot.com/76910044/diff/40001/state/apiserver/client/client.go#newcode298
state/apiserver/client/client.go:298: if len(args.IncludedNetworks) == 0
&& len(args.ExcludedNetworks) == 0 {
On 2014/03/20 15:45:03, rog wrote:
> I'd remove this and make the two commands exactly the same. Then
clients can use
> this call for all Deploy purposes if they wish.

As William pointed out in IRC, I think the "ideal" would be to have API
versioning and then we'd just have ServiceDeploy that accepts
everything. However, I think lacking that I agree with Roger. I'd rather
have just 1 API that is always called.
(So we effectively have API versioning, just via a clumsy renaming of
the API each time.)

https://codereview.appspot.com/76910044/

[Dimiter Naydenov (dimitern)]

Please take a look.

https://codereview.appspot.com/76910044/diff/40001/state/apiserver/client/client.go
File state/apiserver/client/client.go (right):

https://codereview.appspot.com/76910044/diff/40001/state/apiserver/client/client.go#newcode298
state/apiserver/client/client.go:298: if len(args.IncludedNetworks) == 0
&& len(args.ExcludedNetworks) == 0 {
On 2014/03/20 15:45:03, rog wrote:
> I'd remove this and make the two commands exactly the same. Then
clients can use
> this call for all Deploy purposes if they wish.

Done.

https://codereview.appspot.com/76910044/diff/40001/state/apiserver/client/client.go#newcode298
state/apiserver/client/client.go:298: if len(args.IncludedNetworks) == 0
&& len(args.ExcludedNetworks) == 0 {
On 2014/03/20 20:38:22, jameinel wrote:
> On 2014/03/20 15:45:03, rog wrote:
> > I'd remove this and make the two commands exactly the same. Then
clients can
> use
> > this call for all Deploy purposes if they wish.

> As William pointed out in IRC, I think the "ideal" would be to have
API
> versioning and then we'd just have ServiceDeploy that accepts
everything.
> However, I think lacking that I agree with Roger. I'd rather have just
1 API
> that is always called.
> (So we effectively have API versioning, just via a clumsy renaming of
the API
> each time.)

Done as rog suggested. Also, I'm not landing this until mgz's branch
lands. Then I'll integrate the two and drop the related TODOs.

https://codereview.appspot.com/76910044/

[William Reade (fwereade)]

On 2014/03/20 20:38:21, jameinel wrote:
> As William pointed out in IRC, I think the "ideal" would be to have
API
> versioning and then we'd just have ServiceDeploy that accepts
everything.
> However, I think lacking that I agree with Roger. I'd rather have just
1 API
> that is always called.
> (So we effectively have API versioning, just via a clumsy renaming of
the API
> each time.)

I can bear it on this occasion, but this is only suitable in the
ultra-short-term: it will now look as though the old version supports
networks, even though it doesn't (especially if we start autogenerating
docs etc).

https://codereview.appspot.com/76910044/

[Dimiter Naydenov (dimitern)]

Please take a look.

https://codereview.appspot.com/76910044/

[Martin Packman (gz)]

LGTM.

https://codereview.appspot.com/76910044/

=== modified file 'juju/conn_test.go'
--- juju/conn_test.go	2014-03-25 15:48:47 +0000
+++ juju/conn_test.go	2014-03-26 10:43:53 +0000
@@ -398,15 +398,30 @@
 	c.Assert(sch.Revision(), gc.Equals, rev+1)
 }
 
+func (s *ConnSuite) assertAssignedMachineNetworks(c *gc.C, unit *state.Unit, expectInclude, expectExclude []string) {
+	machineId, err := unit.AssignedMachineId()
+	c.Assert(err, gc.IsNil)
+	machine, err := s.conn.State.Machine(machineId)
+	c.Assert(err, gc.IsNil)
+	include, exclude, err := machine.Networks()
+	c.Assert(err, gc.IsNil)
+	c.Assert(include, jc.DeepEquals, expectInclude)
+	c.Assert(exclude, jc.DeepEquals, expectExclude)
+}
+
 func (s *ConnSuite) TestAddUnits(c *gc.C) {
+	withNets := []string{"net1", "net2"}
+	withoutNets := []string{"net3", "net4"}
 	curl := coretesting.Charms.ClonedURL(s.repo.Path, "quantal", "riak")
 	sch, err := s.conn.PutCharm(curl, s.repo, false)
 	c.Assert(err, gc.IsNil)
-	svc, err := s.conn.State.AddService("testriak", "user-admin", sch, nil, nil)
+	svc, err := s.conn.State.AddService("testriak", "user-admin", sch, withNets, withoutNets)
 	c.Assert(err, gc.IsNil)
 	units, err := juju.AddUnits(s.conn.State, svc, 2, "")
 	c.Assert(err, gc.IsNil)
 	c.Assert(units, gc.HasLen, 2)
+	s.assertAssignedMachineNetworks(c, units[0], withNets, withoutNets)
+	s.assertAssignedMachineNetworks(c, units[1], withNets, withoutNets)
 
 	id0, err := units[0].AssignedMachineId()
 	c.Assert(err, gc.IsNil)
@@ -419,16 +434,19 @@
 
 	units, err = juju.AddUnits(s.conn.State, svc, 1, "0")
 	c.Assert(err, gc.IsNil)
+	s.assertAssignedMachineNetworks(c, units[0], withNets, withoutNets)
 	id2, err := units[0].AssignedMachineId()
 	c.Assert(id2, gc.Equals, id0)
 
 	units, err = juju.AddUnits(s.conn.State, svc, 1, "lxc:0")
 	c.Assert(err, gc.IsNil)
+	s.assertAssignedMachineNetworks(c, units[0], withNets, withoutNets)
 	id3, err := units[0].AssignedMachineId()
 	c.Assert(id3, gc.Equals, id0+"/lxc/0")
 
 	units, err = juju.AddUnits(s.conn.State, svc, 1, "lxc:"+id3)
 	c.Assert(err, gc.IsNil)
+	s.assertAssignedMachineNetworks(c, units[0], withNets, withoutNets)
 	id4, err := units[0].AssignedMachineId()
 	c.Assert(id4, gc.Equals, id0+"/lxc/0/lxc/0")
 

=== modified file 'juju/deploy.go'
--- juju/deploy.go	2014-03-25 15:06:31 +0000
+++ juju/deploy.go	2014-03-26 10:43:53 +0000
@@ -28,6 +28,10 @@
 	// - a new container on an existing machine eg "lxc:1"
 	// Use string to avoid ambiguity around machine 0.
 	ToMachineSpec string
+	// IncludeNetworks holds a list of networks to start on boot.
+	IncludeNetworks []string
+	// ExcludeNetworks holds a list of networks to disable on boot.
+	ExcludeNetworks []string
 }
 
 // DeployService takes a charm and various parameters and deploys it.
@@ -49,8 +53,13 @@
 	}
 	// TODO(fwereade): transactional State.AddService including settings, constraints
 	// (minimumUnitCount, initialMachineIds?).
-	// TODO(gz): pass through real includeNetworks and excludeNetworks vals
-	service, err := st.AddService(args.ServiceName, "user-admin", args.Charm, nil, nil)
+	service, err := st.AddService(
+		args.ServiceName,
+		"user-admin",
+		args.Charm,
+		args.IncludeNetworks,
+		args.ExcludeNetworks,
+	)
 	if err != nil {
 		return nil, err
 	}
@@ -81,6 +90,11 @@
 	units := make([]*state.Unit, n)
 	// Hard code for now till we implement a different approach.
 	policy := state.AssignCleanEmpty
+	// All units should have the same networks as the service.
+	includeNetworks, excludeNetworks, err := svc.Networks()
+	if err != nil {
+		return nil, fmt.Errorf("cannot get service %q networks: %v", svc.Name(), err)
+	}
 	// TODO what do we do if we fail half-way through this process?
 	for i := 0; i < n; i++ {
 		unit, err := svc.AddUnit()
@@ -116,9 +130,11 @@
 				// Create the new machine marked as dirty so that
 				// nothing else will grab it before we assign the unit to it.
 				template := state.MachineTemplate{
-					Series: unit.Series(),
-					Jobs:   []state.MachineJob{state.JobHostUnits},
-					Dirty:  true,
+					Series:          unit.Series(),
+					Jobs:            []state.MachineJob{state.JobHostUnits},
+					Dirty:           true,
+					IncludeNetworks: includeNetworks,
+					ExcludeNetworks: excludeNetworks,
 				}
 				m, err = st.AddMachineInsideMachine(template, mid, containerType)
 			} else {

=== modified file 'state/api/client.go'
--- state/api/client.go	2014-03-26 07:01:48 +0000
+++ state/api/client.go	2014-03-26 10:43:53 +0000
@@ -256,12 +256,29 @@
 	return c.call("ServiceUnexpose", params, nil)
 }
 
+// ServiceDeployWithNetworks works exactly like ServiceDeploy, but
+// allows specifying networks to either include or exclude on the
+// machine where the charm is deployed.
+func (c *Client) ServiceDeployWithNetworks(charmURL string, serviceName string, numUnits int, configYAML string, cons constraints.Value, toMachineSpec string, includeNetworks, excludeNetworks []string) error {
+	params := params.ServiceDeploy{
+		ServiceName:     serviceName,
+		CharmUrl:        charmURL,
+		NumUnits:        numUnits,
+		ConfigYAML:      configYAML,
+		Constraints:     cons,
+		ToMachineSpec:   toMachineSpec,
+		IncludeNetworks: includeNetworks,
+		ExcludeNetworks: excludeNetworks,
+	}
+	return c.st.Call("Client", "", "ServiceDeployWithNetworks", params, nil)
+}
+
 // ServiceDeploy obtains the charm, either locally or from the charm store,
 // and deploys it.
-func (c *Client) ServiceDeploy(charmUrl string, serviceName string, numUnits int, configYAML string, cons constraints.Value, toMachineSpec string) error {
+func (c *Client) ServiceDeploy(charmURL string, serviceName string, numUnits int, configYAML string, cons constraints.Value, toMachineSpec string) error {
 	params := params.ServiceDeploy{
 		ServiceName:   serviceName,
-		CharmUrl:      charmUrl,
+		CharmUrl:      charmURL,
 		NumUnits:      numUnits,
 		ConfigYAML:    configYAML,
 		Constraints:   cons,

=== modified file 'state/api/params/params.go'
--- state/api/params/params.go	2014-03-26 07:01:48 +0000
+++ state/api/params/params.go	2014-03-26 10:43:53 +0000
@@ -149,6 +149,10 @@
 	ConfigYAML    string // Takes precedence over config if both are present.
 	Constraints   constraints.Value
 	ToMachineSpec string
+	// The following fields are supported from 1.17.7 onwards and
+	// ignored before that.
+	IncludeNetworks []string
+	ExcludeNetworks []string
 }
 
 // ServiceUpdate holds the parameters for making the ServiceUpdate call.

=== modified file 'state/apiserver/client/client.go'
--- state/apiserver/client/client.go	2014-03-26 07:01:48 +0000
+++ state/apiserver/client/client.go	2014-03-26 10:43:53 +0000
@@ -282,16 +282,25 @@
 
 	_, err = juju.DeployService(c.api.state,
 		juju.DeployServiceParams{
-			ServiceName:    args.ServiceName,
-			Charm:          ch,
-			NumUnits:       args.NumUnits,
-			ConfigSettings: settings,
-			Constraints:    args.Constraints,
-			ToMachineSpec:  args.ToMachineSpec,
+			ServiceName:     args.ServiceName,
+			Charm:           ch,
+			NumUnits:        args.NumUnits,
+			ConfigSettings:  settings,
+			Constraints:     args.Constraints,
+			ToMachineSpec:   args.ToMachineSpec,
+			IncludeNetworks: args.IncludeNetworks,
+			ExcludeNetworks: args.ExcludeNetworks,
 		})
 	return err
 }
 
+// ServiceDeployWithNetworks works exactly like ServiceDeploy, but
+// allows specifying networks to include or exclude on the machine
+// where the charm gets deployed.
+func (c *Client) ServiceDeployWithNetworks(args params.ServiceDeploy) error {
+	return c.ServiceDeploy(args)
+}
+
 // ServiceUpdate updates the service attributes, including charm URL,
 // minimum number of units, settings and constraints.
 // All parameters in params.ServiceUpdate except the service name are optional.

=== modified file 'state/apiserver/client/client_test.go'
--- state/apiserver/client/client_test.go	2014-03-26 07:01:48 +0000
+++ state/apiserver/client/client_test.go	2014-03-26 10:43:53 +0000
@@ -680,6 +680,50 @@
 	}
 }
 
+func (s *clientSuite) TestClientServiceDeployWithNetworks(c *gc.C) {
+	store, restore := makeMockCharmStore()
+	defer restore()
+	curl, bundle := addCharm(c, store, "dummy")
+	mem4g := constraints.MustParse("mem=4G")
+	err := s.APIState.Client().ServiceDeployWithNetworks(
+		curl.String(), "service", 3, "", mem4g, "", []string{"net1", "net2"}, []string{"net3"},
+	)
+	c.Assert(err, gc.IsNil)
+	service := s.assertPrincipalDeployed(c, "service", curl, false, bundle, mem4g)
+
+	include, exclude, err := service.Networks()
+	c.Assert(err, gc.IsNil)
+	c.Assert(include, gc.DeepEquals, []string{"net1", "net2"})
+	c.Assert(exclude, gc.DeepEquals, []string{"net3"})
+}
+
+func (s *clientSuite) assertPrincipalDeployed(c *gc.C, serviceName string, curl *charm.URL, forced bool, bundle charm.Charm, cons constraints.Value) *state.Service {
+	service, err := s.State.Service(serviceName)
+	c.Assert(err, gc.IsNil)
+	charm, force, err := service.Charm()
+	c.Assert(err, gc.IsNil)
+	c.Assert(force, gc.Equals, forced)
+	c.Assert(charm.URL(), gc.DeepEquals, curl)
+	c.Assert(charm.Meta(), gc.DeepEquals, bundle.Meta())
+	c.Assert(charm.Config(), gc.DeepEquals, bundle.Config())
+
+	serviceCons, err := service.Constraints()
+	c.Assert(err, gc.IsNil)
+	c.Assert(serviceCons, gc.DeepEquals, cons)
+	units, err := service.AllUnits()
+	c.Assert(err, gc.IsNil)
+	for _, unit := range units {
+		mid, err := unit.AssignedMachineId()
+		c.Assert(err, gc.IsNil)
+		machine, err := s.State.Machine(mid)
+		c.Assert(err, gc.IsNil)
+		machineCons, err := machine.Constraints()
+		c.Assert(err, gc.IsNil)
+		c.Assert(machineCons, gc.DeepEquals, cons)
+	}
+	return service
+}
+
 func (s *clientSuite) TestClientServiceDeployPrincipal(c *gc.C) {
 	// TODO(fwereade): test ToMachineSpec directly on srvClient, when we
 	// manage to extract it as a package and can thus do it conveniently.
@@ -691,29 +735,7 @@
 		curl.String(), "service", 3, "", mem4g, "",
 	)
 	c.Assert(err, gc.IsNil)
-	service, err := s.State.Service("service")
-	c.Assert(err, gc.IsNil)
-	charm, force, err := service.Charm()
-	c.Assert(err, gc.IsNil)
-	c.Assert(force, gc.Equals, false)
-	c.Assert(charm.URL(), gc.DeepEquals, curl)
-	c.Assert(charm.Meta(), gc.DeepEquals, bundle.Meta())
-	c.Assert(charm.Config(), gc.DeepEquals, bundle.Config())
-
-	cons, err := service.Constraints()
-	c.Assert(err, gc.IsNil)
-	c.Assert(cons, gc.DeepEquals, mem4g)
-	units, err := service.AllUnits()
-	c.Assert(err, gc.IsNil)
-	for _, unit := range units {
-		mid, err := unit.AssignedMachineId()
-		c.Assert(err, gc.IsNil)
-		machine, err := s.State.Machine(mid)
-		c.Assert(err, gc.IsNil)
-		cons, err := machine.Constraints()
-		c.Assert(err, gc.IsNil)
-		c.Assert(cons, gc.DeepEquals, mem4g)
-	}
+	s.assertPrincipalDeployed(c, "service", curl, false, bundle, mem4g)
 }
 
 func (s *clientSuite) TestClientServiceDeploySubordinate(c *gc.C) {

=== modified file 'state/apiserver/client/perm_test.go'
--- state/apiserver/client/perm_test.go	2014-03-13 07:54:56 +0000
+++ state/apiserver/client/perm_test.go	2014-03-26 10:43:53 +0000
@@ -69,6 +69,10 @@
 	op:    opClientServiceDeploy,
 	allow: []string{"user-admin", "user-other"},
 }, {
+	about: "Client.ServiceDeployWithNetworks",
+	op:    opClientServiceDeployWithNetworks,
+	allow: []string{"user-admin", "user-other"},
+}, {
 	about: "Client.ServiceUpdate",
 	op:    opClientServiceUpdate,
 	allow: []string{"user-admin", "user-other"},
@@ -321,6 +325,14 @@
 	return func() {}, err
 }
 
+func opClientServiceDeployWithNetworks(c *gc.C, st *api.State, mst *state.State) (func(), error) {
+	err := st.Client().ServiceDeployWithNetworks("mad:bad/url-1", "x", 1, "", constraints.Value{}, "", nil, nil)
+	if err.Error() == `charm URL has invalid schema: "mad:bad/url-1"` {
+		err = nil
+	}
+	return func() {}, err
+}
+
 func opClientServiceUpdate(c *gc.C, st *api.State, mst *state.State) (func(), error) {
 	args := params.ServiceUpdate{
 		ServiceName:     "no-such-charm",

=== modified file 'state/assign_test.go'
--- state/assign_test.go	2014-03-13 07:54:56 +0000
+++ state/assign_test.go	2014-03-26 10:43:53 +0000
@@ -28,7 +28,13 @@
 
 func (s *AssignSuite) SetUpTest(c *gc.C) {
 	s.ConnSuite.SetUpTest(c)
-	wordpress := s.AddTestingService(c, "wordpress", s.AddTestingCharm(c, "wordpress"))
+	wordpress := s.AddTestingServiceWithNetworks(
+		c,
+		"wordpress",
+		s.AddTestingCharm(c, "wordpress"),
+		[]string{"net1", "net2"},
+		[]string{"net3", "net4"},
+	)
 	s.wordpress = wordpress
 }
 
@@ -312,12 +318,21 @@
 }
 
 func (s *AssignSuite) assertAssignedUnit(c *gc.C, unit *state.Unit) string {
+	// Get service networks.
+	service, err := unit.Service()
+	c.Assert(err, gc.IsNil)
+	includeNetworks, excludeNetworks, err := service.Networks()
+	c.Assert(err, gc.IsNil)
 	// Check the machine on the unit is set.
 	machineId, err := unit.AssignedMachineId()
 	c.Assert(err, gc.IsNil)
 	// Check that the principal is set on the machine.
 	machine, err := s.State.Machine(machineId)
 	c.Assert(err, gc.IsNil)
+	include, exclude, err := machine.Networks()
+	c.Assert(err, gc.IsNil)
+	c.Assert(include, gc.DeepEquals, includeNetworks)
+	c.Assert(exclude, gc.DeepEquals, excludeNetworks)
 	machineUnits, err := machine.Units()
 	c.Assert(err, gc.IsNil)
 	c.Assert(machineUnits, gc.HasLen, 1)

=== modified file 'state/unit.go'
--- state/unit.go	2014-03-19 23:07:33 +0000
+++ state/unit.go	2014-03-26 10:43:53 +0000
@@ -991,11 +991,12 @@
 	return &cons, nil
 }
 
-// AssignToNewMachineOrContainer assigns the unit to a new machine, with constraints
-// determined according to the service and environment constraints at the time of unit creation.
-// If a container is required, a clean, empty machine instance is required on which to create
-// the container. An existing clean, empty instance is first searched for, and if not found,
-// a new one is created.
+// AssignToNewMachineOrContainer assigns the unit to a new machine,
+// with constraints determined according to the service and
+// environment constraints at the time of unit creation. If a
+// container is required, a clean, empty machine instance is required
+// on which to create the container. An existing clean, empty instance
+// is first searched for, and if not found, a new one is created.
 func (u *Unit) AssignToNewMachineOrContainer() (err error) {
 	defer assignContextf(&err, u, "new machine or container")
 	if u.doc.Principal != "" {
@@ -1026,10 +1027,20 @@
 	} else if err != nil {
 		return err
 	}
+	svc, err := u.Service()
+	if err != nil {
+		return err
+	}
+	includeNetworks, excludeNetworks, err := svc.Networks()
+	if err != nil {
+		return err
+	}
 	template := MachineTemplate{
-		Series:      u.doc.Series,
-		Constraints: *cons,
-		Jobs:        []MachineJob{JobHostUnits},
+		Series:          u.doc.Series,
+		Constraints:     *cons,
+		Jobs:            []MachineJob{JobHostUnits},
+		IncludeNetworks: includeNetworks,
+		ExcludeNetworks: excludeNetworks,
 	}
 	err = u.assignToNewMachine(template, host.Id, *cons.Container)
 	if err == machineNotCleanErr {
@@ -1059,10 +1070,20 @@
 	if cons.HasContainer() {
 		containerType = *cons.Container
 	}
+	svc, err := u.Service()
+	if err != nil {
+		return err
+	}
+	includeNetworks, excludeNetworks, err := svc.Networks()
+	if err != nil {
+		return err
+	}
 	template := MachineTemplate{
-		Series:      u.doc.Series,
-		Constraints: *cons,
-		Jobs:        []MachineJob{JobHostUnits},
+		Series:          u.doc.Series,
+		Constraints:     *cons,
+		Jobs:            []MachineJob{JobHostUnits},
+		IncludeNetworks: includeNetworks,
+		ExcludeNetworks: excludeNetworks,
 	}
 	return u.assignToNewMachine(template, "", containerType)
 }