Merge lp:~dimitern/juju-core/460-network-constraints into lp:~go-bot/juju-core/trunk

[Dimiter Naydenov]

various: Introduce networks constraints

Following the removal of --exclude-networks argument
from juju deploy, now we're introducting a complete
way to specify networks with constraints:

--constraints networks=net1,net2,^net3,^net4

Means "find a machine with net1 and net2 networks,
but not with net3 or net4 networks".

Changes are in 3 places: constraints package, state
(for saving/loading) and cmd/juju/deploy for the CLI.

Now, to specify networks to include/exclude on deploy:
juju deploy svc --networks=logging,db --constraints networks=^dmz,storage

Means: "deploy svc on machines with logging, db, and storage
networks, but not with dmz network".

Only the those, specified with --networks will be enabled
on the machine, once the networker worker is done. The ones
specified with constraints are used as a filter for instance
selection.

https://codereview.appspot.com/102830044/

[Dimiter Naydenov (dimitern)]

Reviewers: mp+221390_code.launchpad.net,

Message:
Please take a look.

Description:
various: Introduce networks constraints

Following the removal of --exclude-networks argument
from juju deploy, now we're introducting a complete
way to specify networks with constraints:

--constraints networks=net1,net2,^net3,^net4

Means "find a machine with net1 and net2 networks,
but not with net3 or net4 networks".

Changes are in 3 places: constraints package, state
(for saving/loading) and cmd/juju/deploy for the CLI.

Now, to specify networks to include/exclude on deploy:
juju deploy svc --networks=logging,db --constraints
networks=^dmz,storage

Means: "deploy svc on machines with logging, db, and storage
networks, but not with dmz network".

Only the those, specified with --networks will be enabled
on the machine, once the networker worker is done. The ones
specified with constraints are used as a filter for instance
selection.

https://code.launchpad.net/~dimitern/juju-core/460-network-constraints/+merge/221390

(do not edit description out of merge proposal)

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

Affected files (+271, -131 lines):
   A [revision details]
   M cmd/juju/deploy.go
   M cmd/juju/deploy_test.go
   M cmd/juju/help_topics.go
   M constraints/constraints.go
   M constraints/constraints_test.go
   M state/constraints.go
   M state/constraintsvalidation_test.go

[William Reade (fwereade)]

If you dropped the changes to cmd/juju I'd approve this (with just a
little bit of extra checking of the values when you parse network
constraints).

https://codereview.appspot.com/102830044/diff/1/cmd/juju/deploy.go
File cmd/juju/deploy.go (right):

https://codereview.appspot.com/102830044/diff/1/cmd/juju/deploy.go#newcode95
cmd/juju/deploy.go:95: juju deploy mysql --networks=storage,mynet
--constraints ^logging,db
--constraints networks=^logging,db

https://codereview.appspot.com/102830044/diff/1/cmd/juju/deploy.go#newcode191
cmd/juju/deploy.go:191: includeNetworks :=
mergeNetworks(parseNetworks(c.Networks),
c.Constraints.IncludeNetworks())
This is definitely wrong. This'd put the ones in constraints into the
set that got configured on the machine, instead of the ones used *only*
for machine *selection*, as they should be.

https://codereview.appspot.com/102830044/diff/1/cmd/juju/deploy.go#newcode192
cmd/juju/deploy.go:192: excludeNetworks := mergeNetworks(nil,
c.Constraints.ExcludeNetworks())
And I'm pretty sure this is wrong too. The information's already in
constraints and doesn't need to be duplicated.

https://codereview.appspot.com/102830044/diff/1/cmd/juju/deploy.go#newcode195
cmd/juju/deploy.go:195: env, err := environs.New(conf)
I'm a bit concerned about this. I know we currently need the environ to
talk to the charm store, but I think this will change as we move the
charm store into the environment; I'd prefer not to add more
dependencies on it at this layer (apart from everything else, we can't
assume that the average user has permission to look at the env config).
Just do the checking in the API server (which you have to do anyway, you
might get any request from the GUI/other client).

https://codereview.appspot.com/102830044/diff/1/cmd/juju/deploy.go#newcode240
cmd/juju/deploy.go:240: excludeNetworks,
includeNetworks could stay, if it had the right information in it, but
I'm certain we don't want this. If we haven't released a stable version
with --include/exclude-networks, we should just drop this field. What
are dev versions for, if not for making mistakes? ;p

https://codereview.appspot.com/102830044/diff/1/cmd/juju/deploy.go#newcode297
cmd/juju/deploy.go:297: parts := strings.Split(networksValue, ",")
cmd.StringsValue

https://codereview.appspot.com/102830044/diff/1/cmd/juju/deploy.go#newcode314
cmd/juju/deploy.go:314: // It's not a tag, convert it.
We should *always* turn the user's strings into tags. If they try to
specify network-foo, send network-network-foo over the API.

https://codereview.appspot.com/102830044/diff/1/constraints/constraints.go
File constraints/constraints.go (right):

https://codereview.appspot.com/102830044/diff/1/constraints/constraints.go#newcode514
constraints/constraints.go:514: return &items, nil
I'm wondering whether we should be restricting the values here, at least
in the networks case. Remember these are meant to refer to juju network
names, not provider ones (where we can't control validity).

Tags may be a bit different; not sure if we can sanely restrict them,
even if we'll get surprising results if they ever contain ",", but I
think we can and should keep network names locked down.

ht...

[Dimiter Naydenov (dimitern)]

Sorry, what I meant by "follow-up" is actually a prerequisite:
https://codereview.appspot.com/93670046

What I've said below is done, it's there.

I'll repropose this later with the changes to cmd/juju mostly.

https://codereview.appspot.com/102830044/diff/1/cmd/juju/deploy.go
File cmd/juju/deploy.go (right):

https://codereview.appspot.com/102830044/diff/1/cmd/juju/deploy.go#newcode95
cmd/juju/deploy.go:95: juju deploy mysql --networks=storage,mynet
--constraints ^logging,db
On 2014/05/29 18:22:36, fwereade wrote:
> --constraints networks=^logging,db

Done in a follow-up.

https://codereview.appspot.com/102830044/diff/1/cmd/juju/deploy.go#newcode191
cmd/juju/deploy.go:191: includeNetworks :=
mergeNetworks(parseNetworks(c.Networks),
c.Constraints.IncludeNetworks())
On 2014/05/29 18:22:36, fwereade wrote:
> This is definitely wrong. This'd put the ones in constraints into the
set that
> got configured on the machine, instead of the ones used *only* for
machine
> *selection*, as they should be.

Taking your suggestion, I'm dropping cmd/juju changes from this branch
and will propose it as a follow-up, fixing these issues.

https://codereview.appspot.com/102830044/diff/1/cmd/juju/deploy.go#newcode192
cmd/juju/deploy.go:192: excludeNetworks := mergeNetworks(nil,
c.Constraints.ExcludeNetworks())
On 2014/05/29 18:22:36, fwereade wrote:
> And I'm pretty sure this is wrong too. The information's already in
constraints
> and doesn't need to be duplicated.

Done in a follow-up.

https://codereview.appspot.com/102830044/diff/1/cmd/juju/deploy.go#newcode195
cmd/juju/deploy.go:195: env, err := environs.New(conf)
On 2014/05/29 18:22:36, fwereade wrote:
> I'm a bit concerned about this. I know we currently need the environ
to talk to
> the charm store, but I think this will change as we move the charm
store into
> the environment; I'd prefer not to add more dependencies on it at this
layer
> (apart from everything else, we can't assume that the average user has
> permission to look at the env config). Just do the checking in the API
server
> (which you have to do anyway, you might get any request from the
GUI/other
> client).

Check the follow-up. Doing this check here, in addition to a check in
the API server (sorry, I thought I had that there already - will add it
there as well) will allow us to bail out early, before the API call with
a nicer CLI-specific message, like "cannot use --networks: not supported
by the environmen" instead of "cannot deploy "wordpress" with networks:
not supported by the environment". I'm with having only the latter, less
specific error message.

https://codereview.appspot.com/102830044/diff/1/cmd/juju/deploy.go#newcode240
cmd/juju/deploy.go:240: excludeNetworks,
On 2014/05/29 18:22:36, fwereade wrote:
> includeNetworks could stay, if it had the right information in it, but
I'm
> certain we don't want this. If we haven't released a stable version
with
> --include/exclude-networks, we should just drop this field. What are
dev
> versions for, if not for making mistakes? ;p

Good point. In the follow-up, I'll drop that and modify
service.Networks() to return include/excludeNetworks as before, but
taking both requested networks and...

[Dimiter Naydenov (dimitern)]

This got migrated to github:

https://github.com/juju/juju/pull/13

Lots of changes done, suggestions applied.

https://codereview.appspot.com/102830044/

=== modified file 'cmd/juju/deploy.go'
--- cmd/juju/deploy.go	2014-05-28 15:07:03 +0000
+++ cmd/juju/deploy.go	2014-05-29 14:17:10 +0000
@@ -22,6 +22,7 @@
 	"launchpad.net/juju-core/names"
 	"launchpad.net/juju-core/state/api"
 	"launchpad.net/juju-core/state/api/params"
+	"launchpad.net/juju-core/utils/set"
 )
 
 type DeployCommand struct {
@@ -88,16 +89,20 @@
    juju deploy mysql --to 24/lxc/3 (deploy to lxc container 3 on host machine 24)
    juju deploy mysql --to lxc:25   (deploy to a new lxc container on host machine 25)
 
-   juju deploy mysql -n 5 --constraints mem=8G (deploy 5 instances of mysql with at least 8 GB of RAM each)
+   juju deploy mysql -n 5 --constraints mem=8G
+   (deploy 5 instances of mysql with at least 8 GB of RAM each)
 
-   juju deploy mysql --networks=storage,mynet
+   juju deploy mysql --networks=storage,mynet --constraints ^logging,db
+   (deploy mysql on machines with "storage", "mynet" and "db" networks,
+    but not on machines with "logging" network)
 
 Like constraints, service-specific network requirements can be
 specified with the --networks argument, which takes a comma-delimited
-list of provider-specific network names/labels.
-These instruct juju to ensure to add all the networks specified with
---networks to all new machines deployed to host units of the service.
-Not supported on all providers.
+list of juju-specific network names. Networks can also be specified with
+constraints, but they only define what machine to pick, not what networks
+to configure on it. The --networks argument instructs juju to add all the
+networks specified with it to all new machines deployed to host units of
+the service. Not supported on all providers.
 
 See Also:
    juju help constraints
@@ -182,16 +187,17 @@
 	if c.BumpRevision {
 		ctx.Infof("--upgrade (or -u) is deprecated and ignored; charms are always deployed with a unique revision.")
 	}
-	var includeNetworks []string
-	if c.Networks != "" {
-		includeNetworks = parseNetworks(c.Networks)
 
+	includeNetworks := mergeNetworks(parseNetworks(c.Networks), c.Constraints.IncludeNetworks())
+	excludeNetworks := mergeNetworks(nil, c.Constraints.ExcludeNetworks())
+	haveNetworks := len(includeNetworks) > 0 || len(excludeNetworks) > 0
+	if haveNetworks {
 		env, err := environs.New(conf)
 		if err != nil {
 			return err
 		}
 		if !env.SupportNetworks() {
-			return errors.New("cannot use --networks: not supported by the environment")
+			return errors.New("cannot use --networks/--constraints networks=...: not supported by the environment")
 		}
 	}
 
@@ -231,11 +237,11 @@
 		c.Constraints,
 		c.ToMachineSpec,
 		includeNetworks,
-		nil,
+		excludeNetworks,
 	)
 	if params.IsCodeNotImplemented(err) {
-		if len(includeNetworks) > 0 {
-			return errors.New("cannot use --networks: not supported by the API server")
+		if haveNetworks {
+			return errors.New("cannot use --networks/--constraints networks=...: not supported by the API server")
 		}
 		err = client.ServiceDeploy(
 			curl.String(),
@@ -282,15 +288,36 @@
 	return curl, nil
 }
 
-// parseNetworks returns a list of network tags by parsing the
+// parseNetworks returns a list of networks by parsing the
 // comma-delimited string value of --networks argument.
 func parseNetworks(networksValue string) (networks []string) {
+	if networksValue == "" {
+		return networks
+	}
 	parts := strings.Split(networksValue, ",")
 	for _, part := range parts {
 		network := strings.TrimSpace(part)
 		if network != "" {
-			networks = append(networks, names.NetworkTag(network))
+			networks = append(networks, network)
 		}
 	}
 	return networks
 }
+
+// mergeNetworks returns a sorted list of unique network tags, merging
+// the given slices.
+func mergeNetworks(fromNetworks, fromConstraints []string) []string {
+	result := set.NewStrings()
+	mergeFrom := func(networks []string) {
+		for _, netName := range networks {
+			if _, _, err := names.ParseTag(netName, names.NetworkTagKind); err != nil {
+				// It's not a tag, convert it.
+				netName = names.NetworkTag(netName)
+			}
+			result.Add(netName)
+		}
+	}
+	mergeFrom(fromNetworks)
+	mergeFrom(fromConstraints)
+	return result.SortedValues()
+}

=== modified file 'cmd/juju/deploy_test.go'
--- cmd/juju/deploy_test.go	2014-05-28 15:07:03 +0000
+++ cmd/juju/deploy_test.go	2014-05-29 14:17:10 +0000
@@ -161,25 +161,28 @@
 
 func (s *DeploySuite) TestConstraints(c *gc.C) {
 	coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
-	err := runDeploy(c, "local:dummy", "--constraints", "mem=2G cpu-cores=2")
+	err := runDeploy(c, "local:dummy", "--constraints", "mem=2G cpu-cores=2 networks=net1,^net2")
 	c.Assert(err, gc.IsNil)
 	curl := charm.MustParseURL("local:precise/dummy-1")
 	service, _ := s.AssertService(c, "dummy", curl, 1, 0)
 	cons, err := service.Constraints()
 	c.Assert(err, gc.IsNil)
-	c.Assert(cons, gc.DeepEquals, constraints.MustParse("mem=2G cpu-cores=2"))
+	c.Assert(cons, jc.DeepEquals, constraints.MustParse("mem=2G cpu-cores=2 networks=net1,^net2"))
 }
 
 func (s *DeploySuite) TestNetworks(c *gc.C) {
 	coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
-	err := runDeploy(c, "local:dummy", "--networks", ", net1, net2 , ")
+	err := runDeploy(c, "local:dummy", "--networks", ", net1, net2 , ", "--constraints", "mem=2G cpu-cores=2 networks=net1,net0,^net3,^net4")
 	c.Assert(err, gc.IsNil)
 	curl := charm.MustParseURL("local:precise/dummy-1")
 	service, _ := s.AssertService(c, "dummy", curl, 1, 0)
 	includeNetworks, excludeNetworks, err := service.Networks()
 	c.Assert(err, gc.IsNil)
-	c.Assert(includeNetworks, gc.DeepEquals, []string{"net1", "net2"})
-	c.Assert(excludeNetworks, gc.HasLen, 0)
+	c.Assert(includeNetworks, jc.DeepEquals, []string{"net0", "net1", "net2"})
+	c.Assert(excludeNetworks, jc.DeepEquals, []string{"net3", "net4"})
+	cons, err := service.Constraints()
+	c.Assert(err, gc.IsNil)
+	c.Assert(cons, jc.DeepEquals, constraints.MustParse("mem=2G cpu-cores=2 networks=net1,net0,^net3,^net4"))
 }
 
 func (s *DeploySuite) TestSubordinateConstraints(c *gc.C) {

=== modified file 'cmd/juju/help_topics.go'
--- cmd/juju/help_topics.go	2014-05-15 10:12:36 +0000
+++ cmd/juju/help_topics.go	2014-05-29 14:17:10 +0000
@@ -324,6 +324,13 @@
    Multiple tags must be delimited by a comma. Tags are currently only supported
    by the MaaS environment.
 
+networks
+   Networks defines the list of networks to ensure are available or not on the
+   machine. Both positive and negative network constraints can be specified, the
+   later have a "^" prefix to the name. Multiple netwokrs must be delimited by a
+   comma. Not supported on all providers. Example: networks=storage,db,^logging
+   specifies to enable "storage" and "db" networks but not "logging" on the machine.
+
 Example:
 
    juju add-machine --constraints "arch=amd64 mem=8G tags=foo,bar"

=== modified file 'constraints/constraints.go'
--- constraints/constraints.go	2014-05-13 04:50:10 +0000
+++ constraints/constraints.go	2014-05-29 14:17:10 +0000
@@ -27,6 +27,7 @@
 	RootDisk     = "root-disk"
 	Tags         = "tags"
 	InstanceType = "instance-type"
+	Networks     = "networks"
 )
 
 // Value describes a user's requirements of the hardware on which units
@@ -70,6 +71,12 @@
 	// InstanceType, if not nil, indicates that the specified cloud instance type
 	// be used. Only valid for clouds which support instance types.
 	InstanceType *string `json:"instance-type,omitempty" yaml:"instance-type,omitempty"`
+
+	// Networks, if not nil, holds a list of juju network names that
+	// should be available (or not) on the machine. Positive and
+	// negative values are accepted, and the difference is the latter
+	// have a "^" prefix to the name.
+	Networks *[]string `json:"networks,omitempty" yaml:"networks,omitempty"`
 }
 
 // fieldNames records a mapping from the constraint tag to struct field name.
@@ -107,6 +114,40 @@
 	return v.InstanceType != nil && *v.InstanceType != ""
 }
 
+// extractNetworks returns a list of positive (toInclude=true),
+// negative (toExclude=true), or both from the networks constraint (if
+// set).
+func (v *Value) extractNetworks(toInclude, toExclude bool) []string {
+	var nets []string
+	if v.Networks != nil {
+		for _, name := range *v.Networks {
+			if strings.HasPrefix(name, "^") {
+				if toExclude {
+					nets = append(nets, strings.TrimPrefix(name, "^"))
+				}
+			} else {
+				if toInclude {
+					nets = append(nets, name)
+				}
+			}
+		}
+	}
+	return nets
+}
+
+// IncludeNetworks returns a list of networks to include when starting
+// a machine, if specified.
+func (v *Value) IncludeNetworks() []string {
+	return v.extractNetworks(true, false)
+}
+
+// ExcludeNetworks returns a list of networks to exclude when starting
+// a machine, if specified. They are given in the networks constraint
+// with a "^" prefix to the name, which is stripped before returning.
+func (v *Value) ExcludeNetworks() []string {
+	return v.extractNetworks(false, true)
+}
+
 // String expresses a constraints.Value in the language in which it was specified.
 func (v Value) String() string {
 	var strs []string
@@ -143,6 +184,10 @@
 		s := strings.Join(*v.Tags, ",")
 		strs = append(strs, "tags="+s)
 	}
+	if v.Networks != nil {
+		s := strings.Join(*v.Networks, ",")
+		strs = append(strs, "networks="+s)
+	}
 	return strings.Join(strs, " ")
 }
 
@@ -270,6 +315,8 @@
 		err = v.setTags(str)
 	case InstanceType:
 		err = v.setInstanceType(str)
+	case Networks:
+		err = v.setNetworks(str)
 	default:
 		return fmt.Errorf("unknown constraint %q", name)
 	}
@@ -309,7 +356,9 @@
 		case RootDisk:
 			v.RootDisk, err = parseUint64(vstr)
 		case Tags:
-			v.Tags, err = parseYamlTags(val)
+			v.Tags, err = parseYamlStrings("tags", val)
+		case Networks:
+			v.Networks, err = parseYamlStrings("networks", val)
 		default:
 			return false
 		}
@@ -397,7 +446,15 @@
 	if v.Tags != nil {
 		return fmt.Errorf("already set")
 	}
-	v.Tags = parseTags(str)
+	v.Tags = parseCommaDelimited(str)
+	return nil
+}
+
+func (v *Value) setNetworks(str string) error {
+	if v.Networks != nil {
+		return fmt.Errorf("already set")
+	}
+	v.Networks = parseCommaDelimited(str)
 	return nil
 }
 
@@ -431,8 +488,9 @@
 	return &value, nil
 }
 
-// parseTags returns the tags in the value s.  We expect the tags to be comma delimited strings.
-func parseTags(s string) *[]string {
+// parseCommaDelimited returns the items in the value s. We expect the
+// tags to be comma delimited strings. It is used for tags and networks.
+func parseCommaDelimited(s string) *[]string {
 	if s == "" {
 		return &[]string{}
 	}
@@ -440,20 +498,20 @@
 	return &t
 }
 
-func parseYamlTags(val interface{}) (*[]string, error) {
+func parseYamlStrings(entityName string, val interface{}) (*[]string, error) {
 	ifcs, ok := val.([]interface{})
 	if !ok {
-		return nil, fmt.Errorf("unexpected type passed to tags: %T", val)
+		return nil, fmt.Errorf("unexpected type passed to %s: %T", entityName, val)
 	}
-	tags := make([]string, len(ifcs))
+	items := make([]string, len(ifcs))
 	for n, ifc := range ifcs {
 		s, ok := ifc.(string)
 		if !ok {
-			return nil, fmt.Errorf("unexpected type passed as a tag: %T", ifc)
+			return nil, fmt.Errorf("unexpected type passed as in %s: %T", entityName, ifc)
 		}
-		tags[n] = s
+		items[n] = s
 	}
-	return &tags, nil
+	return &items, nil
 }
 
 var mbSuffixes = map[string]float64{

=== modified file 'constraints/constraints_test.go'
--- constraints/constraints_test.go	2014-04-24 00:00:36 +0000
+++ constraints/constraints_test.go	2014-05-29 14:17:10 +0000
@@ -263,6 +263,21 @@
 		args:    []string{"tags="},
 	},
 
+	// networks
+	{
+		summary: "single network",
+		args:    []string{"networks=net1"},
+	}, {
+		summary: "multiple networks - positive",
+		args:    []string{"networks=net1,net2"},
+	}, {
+		summary: "multiple networks - positive and negative",
+		args:    []string{"networks=net1,^net2,net3,^net4"},
+	}, {
+		summary: "no networks",
+		args:    []string{"networks="},
+	},
+
 	// instance type
 	{
 		summary: "set instance type",
@@ -277,12 +292,12 @@
 		summary: "kitchen sink together",
 		args: []string{
 			"root-disk=8G mem=2T  arch=i386  cpu-cores=4096 cpu-power=9001 container=lxc " +
-				"tags=foo,bar instance-type=foo"},
+				"tags=foo,bar networks=net1,^net2 instance-type=foo"},
 	}, {
 		summary: "kitchen sink separately",
 		args: []string{
 			"root-disk=8G", "mem=2T", "cpu-cores=4096", "cpu-power=9001", "arch=armhf",
-			"container=lxc", "tags=foo,bar", "instance-type=foo"},
+			"container=lxc", "tags=foo,bar", "networks=net1,^net2", "instance-type=foo"},
 	},
 }
 
@@ -302,15 +317,26 @@
 	}
 }
 
-func (s *ConstraintsSuite) TestParseMissingTags(c *gc.C) {
+func (s *ConstraintsSuite) TestParseMissingTagsAndNetworks(c *gc.C) {
 	con := constraints.MustParse("arch=amd64 mem=4G cpu-cores=1 root-disk=8G")
 	c.Check(con.Tags, gc.IsNil)
+	c.Check(con.Networks, gc.IsNil)
 }
 
-func (s *ConstraintsSuite) TestParseNoTags(c *gc.C) {
-	con := constraints.MustParse("arch=amd64 mem=4G cpu-cores=1 root-disk=8G tags=")
+func (s *ConstraintsSuite) TestParseNoTagsNoNetworks(c *gc.C) {
+	con := constraints.MustParse("arch=amd64 mem=4G cpu-cores=1 root-disk=8G tags= networks=")
 	c.Assert(con.Tags, gc.Not(gc.IsNil))
+	c.Assert(con.Networks, gc.Not(gc.IsNil))
 	c.Check(*con.Tags, gc.HasLen, 0)
+	c.Check(*con.Networks, gc.HasLen, 0)
+}
+
+func (s *ConstraintsSuite) TestIncludeExcludeNetworks(c *gc.C) {
+	con := constraints.MustParse("networks=net1,^net2,net3,^net4")
+	c.Assert(con.Networks, gc.Not(gc.IsNil))
+	c.Check(*con.Networks, gc.HasLen, 4)
+	c.Check(con.IncludeNetworks(), jc.SameContents, []string{"net1", "net3"})
+	c.Check(con.ExcludeNetworks(), jc.SameContents, []string{"net2", "net4"})
 }
 
 func (s *ConstraintsSuite) TestIsEmpty(c *gc.C) {
@@ -322,6 +348,8 @@
 	c.Check(&con, jc.Satisfies, constraints.IsEmpty)
 	con = constraints.MustParse("tags=")
 	c.Check(&con, gc.Not(jc.Satisfies), constraints.IsEmpty)
+	con = constraints.MustParse("networks=")
+	c.Check(&con, gc.Not(jc.Satisfies), constraints.IsEmpty)
 	con = constraints.MustParse("mem=")
 	c.Check(&con, gc.Not(jc.Satisfies), constraints.IsEmpty)
 	con = constraints.MustParse("arch=")
@@ -378,6 +406,9 @@
 	{"Tags1", constraints.Value{Tags: nil}},
 	{"Tags2", constraints.Value{Tags: &[]string{}}},
 	{"Tags3", constraints.Value{Tags: &[]string{"foo", "bar"}}},
+	{"Networks1", constraints.Value{Networks: nil}},
+	{"Networks2", constraints.Value{Networks: &[]string{}}},
+	{"Networks3", constraints.Value{Networks: &[]string{"net1", "^net2"}}},
 	{"InstanceType1", constraints.Value{InstanceType: strp("")}},
 	{"InstanceType2", constraints.Value{InstanceType: strp("foo")}},
 	{"All", constraints.Value{
@@ -388,6 +419,7 @@
 		Mem:          uint64p(18000000000),
 		RootDisk:     uint64p(24000000000),
 		Tags:         &[]string{"foo", "bar"},
+		Networks:     &[]string{"net1", "^net2"},
 		InstanceType: strp("foo"),
 	}},
 }
@@ -399,7 +431,7 @@
 		val := constraints.ConstraintsValue{&cons}
 		err := val.Set(t.Value.String())
 		c.Check(err, gc.IsNil)
-		c.Check(cons, gc.DeepEquals, t.Value)
+		c.Check(cons, jc.DeepEquals, t.Value)
 	}
 }
 
@@ -408,7 +440,7 @@
 		c.Logf("test %s", t.Name)
 		cons, err := constraints.Parse(t.Value.String())
 		c.Check(err, gc.IsNil)
-		c.Check(cons, gc.DeepEquals, t.Value)
+		c.Check(cons, jc.DeepEquals, t.Value)
 	}
 }
 
@@ -420,7 +452,7 @@
 		var cons constraints.Value
 		err = json.Unmarshal(data, &cons)
 		c.Check(err, gc.IsNil)
-		c.Check(cons, gc.DeepEquals, t.Value)
+		c.Check(cons, jc.DeepEquals, t.Value)
 	}
 }
 
@@ -432,7 +464,7 @@
 		var cons constraints.Value
 		err = goyaml.Unmarshal(data, &cons)
 		c.Check(err, gc.IsNil)
-		c.Check(cons, gc.DeepEquals, t.Value)
+		c.Check(cons, jc.DeepEquals, t.Value)
 	}
 }
 
@@ -466,57 +498,53 @@
 	c.Check(cons.HasInstanceType(), jc.IsTrue)
 }
 
+const initialWithoutCons = "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cpu-cores=4 networks=net1,^net2 tags=foo container=lxc instance-type=bar"
+
 var withoutTests = []struct {
 	initial string
 	without []string
 	final   string
-}{
-	{
-		initial: "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cpu-cores=4 tags=foo container=lxc instance-type=bar",
-		without: []string{"root-disk"},
-		final:   "mem=4G arch=amd64 cpu-power=1000 cpu-cores=4 tags=foo container=lxc instance-type=bar",
-	},
-	{
-		initial: "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cpu-cores=4 tags=foo container=lxc instance-type=bar",
-		without: []string{"mem"},
-		final:   "root-disk=8G arch=amd64 cpu-power=1000 cpu-cores=4 tags=foo container=lxc instance-type=bar",
-	},
-	{
-		initial: "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cpu-cores=4 tags=foo container=lxc instance-type=bar",
-		without: []string{"arch"},
-		final:   "root-disk=8G mem=4G cpu-power=1000 cpu-cores=4 tags=foo container=lxc instance-type=bar",
-	},
-	{
-		initial: "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cpu-cores=4 tags=foo container=lxc instance-type=bar",
-		without: []string{"cpu-power"},
-		final:   "root-disk=8G mem=4G arch=amd64 cpu-cores=4 tags=foo container=lxc instance-type=bar",
-	},
-	{
-		initial: "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cpu-cores=4 tags=foo container=lxc instance-type=bar",
-		without: []string{"cpu-cores"},
-		final:   "root-disk=8G mem=4G arch=amd64 cpu-power=1000 tags=foo container=lxc instance-type=bar",
-	},
-	{
-		initial: "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cpu-cores=4 tags=foo container=lxc instance-type=bar",
-		without: []string{"tags"},
-		final:   "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cpu-cores=4 container=lxc instance-type=bar",
-	},
-	{
-		initial: "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cpu-cores=4 tags=foo container=lxc instance-type=bar",
-		without: []string{"container"},
-		final:   "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cpu-cores=4 tags=foo instance-type=bar",
-	},
-	{
-		initial: "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cpu-cores=4 tags=foo container=lxc instance-type=bar",
-		without: []string{"instance-type"},
-		final:   "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cpu-cores=4 tags=foo container=lxc",
-	},
-	{
-		initial: "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cpu-cores=4 tags=foo container=lxc instance-type=bar",
-		without: []string{"root-disk", "mem", "arch"},
-		final:   "cpu-power=1000 cpu-cores=4 tags=foo container=lxc instance-type=bar",
-	},
-}
+}{{
+	initial: initialWithoutCons,
+	without: []string{"root-disk"},
+	final:   "mem=4G arch=amd64 cpu-power=1000 cpu-cores=4 tags=foo networks=net1,^net2 container=lxc instance-type=bar",
+}, {
+	initial: initialWithoutCons,
+	without: []string{"mem"},
+	final:   "root-disk=8G arch=amd64 cpu-power=1000 cpu-cores=4 tags=foo networks=net1,^net2 container=lxc instance-type=bar",
+}, {
+	initial: initialWithoutCons,
+	without: []string{"arch"},
+	final:   "root-disk=8G mem=4G cpu-power=1000 cpu-cores=4 tags=foo networks=net1,^net2 container=lxc instance-type=bar",
+}, {
+	initial: initialWithoutCons,
+	without: []string{"cpu-power"},
+	final:   "root-disk=8G mem=4G arch=amd64 cpu-cores=4 tags=foo networks=net1,^net2 container=lxc instance-type=bar",
+}, {
+	initial: initialWithoutCons,
+	without: []string{"cpu-cores"},
+	final:   "root-disk=8G mem=4G arch=amd64 cpu-power=1000 tags=foo networks=net1,^net2 container=lxc instance-type=bar",
+}, {
+	initial: initialWithoutCons,
+	without: []string{"tags"},
+	final:   "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cpu-cores=4 networks=net1,^net2 container=lxc instance-type=bar",
+}, {
+	initial: initialWithoutCons,
+	without: []string{"networks"},
+	final:   "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cpu-cores=4 tags=foo container=lxc instance-type=bar",
+}, {
+	initial: initialWithoutCons,
+	without: []string{"container"},
+	final:   "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cpu-cores=4 tags=foo networks=net1,^net2 instance-type=bar",
+}, {
+	initial: initialWithoutCons,
+	without: []string{"instance-type"},
+	final:   "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cpu-cores=4 tags=foo networks=net1,^net2 container=lxc",
+}, {
+	initial: initialWithoutCons,
+	without: []string{"root-disk", "mem", "arch"},
+	final:   "cpu-power=1000 cpu-cores=4 tags=foo networks=net1,^net2 container=lxc instance-type=bar",
+}}
 
 func (s *ConstraintsSuite) TestWithout(c *gc.C) {
 	for i, t := range withoutTests {
@@ -524,7 +552,7 @@
 		initial := constraints.MustParse(t.initial)
 		final, err := constraints.Without(initial, t.without...)
 		c.Assert(err, gc.IsNil)
-		c.Check(final, gc.DeepEquals, constraints.MustParse(t.final))
+		c.Check(final, jc.DeepEquals, constraints.MustParse(t.final))
 	}
 }
 
@@ -537,7 +565,7 @@
 func (s *ConstraintsSuite) TestAttributesWithValues(c *gc.C) {
 	for i, consStr := range []string{
 		"",
-		"root-disk=8G mem=4G arch=amd64 cpu-power=1000 cpu-cores=4 instance-type=foo tags=foo,bar",
+		"root-disk=8G mem=4G arch=amd64 cpu-power=1000 cpu-cores=4 instance-type=foo tags=foo,bar networks=net1,^net2",
 	} {
 		c.Logf("test %d", i)
 		cons := constraints.MustParse(consStr)
@@ -576,6 +604,11 @@
 		} else {
 			assertMissing("tags")
 		}
+		if cons.Networks != nil {
+			c.Check(obtained["networks"], gc.DeepEquals, *cons.Networks)
+		} else {
+			assertMissing("networks")
+		}
 		if cons.InstanceType != nil {
 			c.Check(obtained["instance-type"], gc.Equals, *cons.InstanceType)
 		} else {
@@ -590,13 +623,13 @@
 	expected []string
 }{
 	{
-		cons:     "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cpu-cores=4",
-		attrs:    []string{"root-disk", "tags", "mem"},
-		expected: []string{"root-disk", "mem"},
+		cons:     "root-disk=8G mem=4G arch=amd64 cpu-power=1000 networks=net1,^net2 cpu-cores=4",
+		attrs:    []string{"root-disk", "tags", "mem", "networks"},
+		expected: []string{"root-disk", "mem", "networks"},
 	},
 	{
 		cons:     "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cpu-cores=4",
-		attrs:    []string{"tags"},
+		attrs:    []string{"tags", "networks"},
 		expected: []string{},
 	},
 }
@@ -606,6 +639,6 @@
 		c.Logf("test %d", i)
 		cons := constraints.MustParse(t.cons)
 		obtained := constraints.HasAny(cons, t.attrs...)
-		c.Check(obtained, gc.DeepEquals, t.expected)
+		c.Check(obtained, jc.DeepEquals, t.expected)
 	}
 }

=== modified file 'state/constraints.go'
--- state/constraints.go	2014-05-13 04:30:48 +0000
+++ state/constraints.go	2014-05-29 14:17:10 +0000
@@ -25,6 +25,7 @@
 	InstanceType *string
 	Container    *instance.ContainerType
 	Tags         *[]string `bson:",omitempty"`
+	Networks     *[]string `bson:",omitempty"`
 }
 
 func (doc constraintsDoc) value() constraints.Value {
@@ -36,6 +37,7 @@
 		RootDisk:     doc.RootDisk,
 		Container:    doc.Container,
 		Tags:         doc.Tags,
+		Networks:     doc.Networks,
 		InstanceType: doc.InstanceType,
 	}
 }
@@ -49,6 +51,7 @@
 		RootDisk:     cons.RootDisk,
 		Container:    cons.Container,
 		Tags:         cons.Tags,
+		Networks:     cons.Networks,
 		InstanceType: cons.InstanceType,
 	}
 }

=== modified file 'state/constraintsvalidation_test.go'
--- state/constraintsvalidation_test.go	2014-04-18 13:49:33 +0000
+++ state/constraintsvalidation_test.go	2014-05-29 14:17:10 +0000
@@ -4,6 +4,7 @@
 package state_test
 
 import (
+	jc "github.com/juju/testing/checkers"
 	gc "launchpad.net/gocheck"
 
 	"launchpad.net/juju-core/constraints"
@@ -39,50 +40,56 @@
 	consFallback string
 	cons         string
 	expected     string
-}{
-	{
-		cons:         "root-disk=8G mem=4G arch=amd64",
-		consFallback: "cpu-power=1000 cpu-cores=4",
-		expected:     "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cpu-cores=4",
-	}, {
-		cons:         "root-disk=8G mem=4G arch=amd64",
-		consFallback: "cpu-power=1000 cpu-cores=4 mem=8G",
-		expected:     "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cpu-cores=4",
-	}, {
-		consFallback: "root-disk=8G cpu-cores=4 instance-type=foo",
-		expected:     "root-disk=8G cpu-cores=4 instance-type=foo",
-	}, {
-		cons:     "root-disk=8G cpu-cores=4 instance-type=foo",
-		expected: "root-disk=8G cpu-cores=4 instance-type=foo",
-	}, {
-		consFallback: "root-disk=8G instance-type=foo",
-		cons:         "root-disk=8G cpu-cores=4 instance-type=bar",
-		expected:     "root-disk=8G cpu-cores=4 instance-type=bar",
-	}, {
-		consFallback: "root-disk=8G mem=4G",
-		cons:         "root-disk=8G cpu-cores=4 instance-type=bar",
-		expected:     "root-disk=8G cpu-cores=4 instance-type=bar",
-	}, {
-		consFallback: "root-disk=8G cpu-cores=4 instance-type=bar",
-		cons:         "root-disk=8G mem=4G",
-		expected:     "root-disk=8G cpu-cores=4 mem=4G",
-	}, {
-		consFallback: "root-disk=8G cpu-cores=4 instance-type=bar",
-		cons:         "root-disk=8G arch=amd64 mem=4G",
-		expected:     "root-disk=8G cpu-cores=4 arch=amd64 mem=4G",
-	},
-}
+}{{
+	cons:         "root-disk=8G mem=4G arch=amd64",
+	consFallback: "cpu-power=1000 cpu-cores=4",
+	expected:     "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cpu-cores=4",
+}, {
+	cons:         "root-disk=8G mem=4G arch=amd64",
+	consFallback: "cpu-power=1000 cpu-cores=4 mem=8G",
+	expected:     "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cpu-cores=4",
+}, {
+	consFallback: "root-disk=8G cpu-cores=4 instance-type=foo",
+	expected:     "root-disk=8G cpu-cores=4 instance-type=foo",
+}, {
+	cons:     "root-disk=8G cpu-cores=4 instance-type=foo",
+	expected: "root-disk=8G cpu-cores=4 instance-type=foo",
+}, {
+	consFallback: "root-disk=8G instance-type=foo",
+	cons:         "root-disk=8G cpu-cores=4 instance-type=bar",
+	expected:     "root-disk=8G cpu-cores=4 instance-type=bar",
+}, {
+	consFallback: "root-disk=8G mem=4G",
+	cons:         "root-disk=8G cpu-cores=4 instance-type=bar",
+	expected:     "root-disk=8G cpu-cores=4 instance-type=bar",
+}, {
+	consFallback: "root-disk=8G cpu-cores=4 instance-type=bar",
+	cons:         "root-disk=8G mem=4G",
+	expected:     "root-disk=8G cpu-cores=4 mem=4G",
+}, {
+	consFallback: "root-disk=8G cpu-cores=4 instance-type=bar",
+	cons:         "root-disk=8G arch=amd64 mem=4G",
+	expected:     "root-disk=8G cpu-cores=4 arch=amd64 mem=4G",
+}, {
+	consFallback: "cpu-cores=4 mem=4G networks=net1,^net3",
+	cons:         "networks=net2,^net4 mem=4G",
+	expected:     "cpu-cores=4 mem=4G networks=net2,^net4",
+}, {
+	consFallback: "cpu-cores=4 mem=2G networks=",
+	cons:         "mem=4G networks=net1,^net3",
+	expected:     "cpu-cores=4 mem=4G networks=net1,^net3",
+}}
 
 func (s *constraintsValidationSuite) TestMachineConstraints(c *gc.C) {
 	for i, t := range setConstraintsTests {
-		c.Logf("test %d", i)
+		c.Logf("test %d: fallback: %q, cons: %q", i, t.consFallback, t.cons)
 		err := s.State.SetEnvironConstraints(constraints.MustParse(t.consFallback))
 		c.Check(err, gc.IsNil)
 		m, err := s.addOneMachine(c, constraints.MustParse(t.cons))
 		c.Check(err, gc.IsNil)
 		cons, err := m.Constraints()
 		c.Check(err, gc.IsNil)
-		c.Check(cons, gc.DeepEquals, constraints.MustParse(t.expected))
+		c.Check(cons, jc.DeepEquals, constraints.MustParse(t.expected))
 	}
 }
 
@@ -90,15 +97,15 @@
 	charm := s.AddTestingCharm(c, "wordpress")
 	service := s.AddTestingService(c, "wordpress", charm)
 	for i, t := range setConstraintsTests {
-		c.Logf("test %d", i)
+		c.Logf("test %d: fallback: %q, cons: %q", i, t.consFallback, t.cons)
 		err := s.State.SetEnvironConstraints(constraints.MustParse(t.consFallback))
 		c.Check(err, gc.IsNil)
 		err = service.SetConstraints(constraints.MustParse(t.cons))
 		c.Check(err, gc.IsNil)
 		u, err := service.AddUnit()
 		c.Check(err, gc.IsNil)
-		cons, err := state.UnitConstraints(u)
+		ucons, err := state.UnitConstraints(u)
 		c.Check(err, gc.IsNil)
-		c.Check(*cons, gc.DeepEquals, constraints.MustParse(t.expected))
+		c.Check(*ucons, jc.DeepEquals, constraints.MustParse(t.expected))
 	}
 }