Merge pull request juju#9481 from manadart/zone-constraints

juju#9481 ## Description of change This patch comes about following discussion of the absence of availability zone placement support in bundles. It is intended to meet the following needs: - Bundles do support constraints, so allowing zones as constraints delivers this capability. - It accommodates MAAS deployments where normal AZ distribution is required, but some zones need to be excluded from consideration. It includes the following functional changes: - "zones" is now a valid constraint keyword, intended for constraining deployment to one or more availability zones. - Machine provisioning accommodates supplied zone constraints, including within a distribution group. - Unit assignment accommodates supplied zone constraints. #### Notable Behaviour Change Note that policy-based deployment of units to empty machines changes subtly. If these conditions are met: - zone constraints are present, - there are empty machines in any of the availability zones in the constraints, - a non-empty distribution group for the application is determined and; - the distribution group would normally cause a new machine to be created. Then one of the existing empty machines is used for deployment. ## QA steps - `juju bootstrap aws zone-test --no-gui --debug --build-agent` - `juju deploy redis red0 -n 2 --constraints zones=us-east-1a` - Check that Redis comes up on new machines in the us-east-1a zone. - `juju add-machine -n 2 --series trusty --constraints zones=us-east-1a` - Check that 2 new machines come up in the us-east-1a zone. - `juju deploy redis red1 -n 2 --constraints zones=us-east-1c` - Check that the empty us-east-1a machines are not used and that Redis is deployed to new us-east-1c machines. - `juju deploy redis red2 -n 2 --constraints zones=us-east-1a` - Check that the empty machines in us-east-1a are deployed to. - `juju add-machine --series trusty --constraints zones=us-east-1a` - `juju add-machine --series trusty --constraints zones=us-east-1c` - `juju deploy redis red3 -n 2 --constraints zones=us-east-1a,us-east-1c` - Check that a unit is deployed to each of the previously added machines. - `juju add-machine lxd --constraints zones=us-east-1d` - Check that a container is created on a new machine in us-east-1d. - Copy the _mediawiki-single_ bundle and add lines `constraints: zones=us-east-1a` to the applications. - Deploy the bundle and check that all machines were created in us-east-1. - `juju deploy redis redany -n 3` - Check that Redis is deployed to new machines in 3 different zones (no regression). #### New Distribution Test - `juju add-machine --series trusty --constraints zones=us-east-1a` - `juju add-machine --series trusty --constraints zones=us-east-1a` - `juju deploy redis red4 -n 2 --constraints zones=us-east-1a,us-east-1c` - Check that one unit goes to one of the existing machines, and another is added to a new machine in us-east1c. ## Documentation changes Yes. ## Bug reference https://bugs.launchpad.net/juju/+bug/1791715
MalloZup · Nov 27, 2018 · 294ca9f · 294ca9f
2 parents 8cd3fb2 + ec541d6
commit 294ca9f
Show file tree

Hide file tree

Showing 23 changed files with 1,080 additions and 222 deletions.
diff --git a/Gopkg.lock b/Gopkg.lock
diff --git a/Gopkg.toml b/Gopkg.toml
@@ -62,7 +62,7 @@
   name = "github.com/juju/bundlechanges"
 
 [[constraint]]
-  revision = "16527177881d2374d8bf776106fb20672bd03872"
+  revision = "b351832010e628bdc7e8bf630ff7a079f5e3b57b"
   name = "github.com/juju/description"
 
 [[constraint]]

diff --git a/constraints/constraints.go b/constraints/constraints.go
@@ -32,6 +32,7 @@ const (
 	InstanceType = "instance-type"
 	Spaces       = "spaces"
 	VirtType     = "virt-type"
+	Zones        = "zones"
 )
 
 // Value describes a user's requirements of the hardware on which units
@@ -85,6 +86,10 @@ type Value struct {
 	// VirtType, if not nil or empty, indicates that a machine must run the named
 	// virtual type. Only valid for clouds with multi-hypervisor support.
 	VirtType *string `json:"virt-type,omitempty" yaml:"virt-type,omitempty"`
+
+	// Zones, if not nil, holds a list of availability zones limiting where
+	// the machine can be located.
+	Zones *[]string `json:"zones,omitempty" yaml:"zones,omitempty"`
 }
 
 var rawAliases = map[string]string{
@@ -170,8 +175,8 @@ func (v *Value) ExcludeSpaces() []string {
 	return v.extractItems(*v.Spaces, false)
 }
 
-// HaveSpaces returns whether any spaces constraints were specified.
-func (v *Value) HaveSpaces() bool {
+// HasSpaces returns whether any spaces constraints were specified.
+func (v *Value) HasSpaces() bool {
 	return v.Spaces != nil && len(*v.Spaces) > 0
 }
 
@@ -180,6 +185,11 @@ func (v *Value) HasVirtType() bool {
 	return v.VirtType != nil && *v.VirtType != ""
 }
 
+// HasZones returns whether any zone constraints were specified.
+func (v *Value) HasZones() bool {
+	return v.Zones != nil && len(*v.Zones) > 0
+}
+
 // String expresses a constraints.Value in the language in which it was specified.
 func (v Value) String() string {
 	var strs []string
@@ -223,6 +233,10 @@ func (v Value) String() string {
 	if v.VirtType != nil {
 		strs = append(strs, "virt-type="+(*v.VirtType))
 	}
+	if v.Zones != nil {
+		s := strings.Join(*v.Zones, ",")
+		strs = append(strs, "zones="+s)
+	}
 	return strings.Join(strs, " ")
 }
 
@@ -264,6 +278,11 @@ func (v Value) GoString() string {
 	if v.VirtType != nil {
 		values = append(values, fmt.Sprintf("VirtType: %q", *v.VirtType))
 	}
+	if v.Zones != nil && *v.Zones != nil {
+		values = append(values, fmt.Sprintf("Zones: %q", *v.Zones))
+	} else if v.Zones != nil {
+		values = append(values, "Zones: (*[]string)(nil)")
+	}
 	return fmt.Sprintf("{%s}", strings.Join(values, ", "))
 }
 
@@ -420,6 +439,8 @@ func (v *Value) setRaw(name, str string) error {
 		err = v.setSpaces(str)
 	case VirtType:
 		err = v.setVirtType(str)
+	case Zones:
+		err = v.setZones(str)
 	default:
 		return errors.Errorf("unknown constraint %q", name)
 	}
@@ -483,6 +504,8 @@ func (v *Value) UnmarshalYAML(unmarshal func(interface{}) error) error {
 			}
 		case VirtType:
 			v.VirtType = &vstr
+		case Zones:
+			v.Zones, err = parseYamlStrings("zones", val)
 		default:
 			return errors.Errorf("unknown constraint value: %v", k)
 		}
@@ -607,6 +630,14 @@ func (v *Value) setVirtType(str string) error {
 	return nil
 }
 
+func (v *Value) setZones(str string) error {
+	if v.Zones != nil {
+		return errors.Errorf("already set")
+	}
+	v.Zones = parseCommaDelimited(str)
+	return nil
+}
+
 func parseUint64(str string) (*uint64, error) {
 	var value uint64
 	if str != "" {

diff --git a/constraints/constraints_test.go b/constraints/constraints_test.go
@@ -313,19 +313,31 @@ var parseConstraintsTests = []struct {
 		err:     `bad "virt-type" constraint: already set`,
 	},
 
+	// Zones
+	{
+		summary: "single zone",
+		args:    []string{"zones=az1"},
+	}, {
+		summary: "multiple zones",
+		args:    []string{"zones=az1,az2"},
+	}, {
+		summary: "no zones",
+		args:    []string{"zones="},
+	},
+
 	// Everything at once.
 	{
 		summary: "kitchen sink together",
 		args: []string{
 			"root-disk=8G mem=2T  arch=i386  cores=4096 cpu-power=9001 container=lxd " +
 				"tags=foo,bar spaces=space1,^space2 instance-type=foo",
-			"virt-type=kvm"},
+			"virt-type=kvm zones=az1,az2"},
 	}, {
 		summary: "kitchen sink separately",
 		args: []string{
 			"root-disk=8G", "mem=2T", "cores=4096", "cpu-power=9001", "arch=armhf",
 			"container=lxd", "tags=foo,bar", "spaces=space1,^space2",
-			"instance-type=foo", "virt-type=kvm"},
+			"instance-type=foo", "virt-type=kvm", "zones=az1,az2"},
 	},
 }
 
@@ -407,19 +419,19 @@ func (s *ConstraintsSuite) TestParseNoTagsNoSpaces(c *gc.C) {
 	c.Check(*con.Spaces, gc.HasLen, 0)
 }
 
-func (s *ConstraintsSuite) TestIncludeExcludeAndHaveSpaces(c *gc.C) {
+func (s *ConstraintsSuite) TestIncludeExcludeAndHasSpaces(c *gc.C) {
 	con := constraints.MustParse("spaces=space1,^space2,space3,^space4")
 	c.Assert(con.Spaces, gc.Not(gc.IsNil))
 	c.Check(*con.Spaces, gc.HasLen, 4)
 	c.Check(con.IncludeSpaces(), jc.SameContents, []string{"space1", "space3"})
 	c.Check(con.ExcludeSpaces(), jc.SameContents, []string{"space2", "space4"})
-	c.Check(con.HaveSpaces(), jc.IsTrue)
+	c.Check(con.HasSpaces(), jc.IsTrue)
 	con = constraints.MustParse("mem=4G")
-	c.Check(con.HaveSpaces(), jc.IsFalse)
+	c.Check(con.HasSpaces(), jc.IsFalse)
 	con = constraints.MustParse("mem=4G spaces=space-foo,^space-bar")
 	c.Check(con.IncludeSpaces(), jc.SameContents, []string{"space-foo"})
 	c.Check(con.ExcludeSpaces(), jc.SameContents, []string{"space-bar"})
-	c.Check(con.HaveSpaces(), jc.IsTrue)
+	c.Check(con.HasSpaces(), jc.IsTrue)
 }
 
 func (s *ConstraintsSuite) TestInvalidSpaces(c *gc.C) {
@@ -438,6 +450,19 @@ func (s *ConstraintsSuite) TestInvalidSpaces(c *gc.C) {
 	}
 }
 
+func (s *ConstraintsSuite) TestHasZones(c *gc.C) {
+	con := constraints.MustParse("zones=az1,az2,az3")
+	c.Assert(con.Zones, gc.Not(gc.IsNil))
+	c.Check(*con.Zones, gc.HasLen, 3)
+	c.Check(con.HasZones(), jc.IsTrue)
+
+	con = constraints.MustParse("zones=")
+	c.Check(con.HasZones(), jc.IsFalse)
+
+	con = constraints.MustParse("spaces=space1,^space2")
+	c.Check(con.HasZones(), jc.IsFalse)
+}
+
 func (s *ConstraintsSuite) TestIsEmpty(c *gc.C) {
 	con := constraints.Value{}
 	c.Check(&con, jc.Satisfies, constraints.IsEmpty)
@@ -463,6 +488,8 @@ func (s *ConstraintsSuite) TestIsEmpty(c *gc.C) {
 	c.Check(&con, gc.Not(jc.Satisfies), constraints.IsEmpty)
 	con = constraints.MustParse("instance-type=")
 	c.Check(&con, gc.Not(jc.Satisfies), constraints.IsEmpty)
+	con = constraints.MustParse("zones=")
+	c.Check(&con, gc.Not(jc.Satisfies), constraints.IsEmpty)
 }
 
 func uint64p(i uint64) *uint64 {
@@ -510,6 +537,9 @@ var constraintsRoundtripTests = []roundTrip{
 	{"Spaces3", constraints.Value{Spaces: &[]string{"space1", "^space2"}}},
 	{"InstanceType1", constraints.Value{InstanceType: strp("")}},
 	{"InstanceType2", constraints.Value{InstanceType: strp("foo")}},
+	{"Zones1", constraints.Value{Zones: nil}},
+	{"Zones2", constraints.Value{Zones: &[]string{}}},
+	{"Zones3", constraints.Value{Zones: &[]string{"az1", "az2"}}},
 	{"All", constraints.Value{
 		Arch:         strp("i386"),
 		Container:    ctypep("lxd"),
@@ -520,6 +550,7 @@ var constraintsRoundtripTests = []roundTrip{
 		Tags:         &[]string{"foo", "bar"},
 		Spaces:       &[]string{"space1", "^space2"},
 		InstanceType: strp("foo"),
+		Zones:        &[]string{"az1", "az2"},
 	}},
 }
 
@@ -597,7 +628,8 @@ func (s *ConstraintsSuite) TestHasInstanceType(c *gc.C) {
 	c.Check(cons.HasInstanceType(), jc.IsTrue)
 }
 
-const initialWithoutCons = "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cores=4 spaces=space1,^space2 tags=foo container=lxd instance-type=bar"
+const initialWithoutCons = "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cores=4 spaces=space1,^space2 tags=foo " +
+	"container=lxd instance-type=bar zones=az1,az2"
 
 var withoutTests = []struct {
 	initial string
@@ -606,43 +638,47 @@ var withoutTests = []struct {
 }{{
 	initial: initialWithoutCons,
 	without: []string{"root-disk"},
-	final:   "mem=4G arch=amd64 cpu-power=1000 cores=4 tags=foo spaces=space1,^space2 container=lxd instance-type=bar",
+	final:   "mem=4G arch=amd64 cpu-power=1000 cores=4 tags=foo spaces=space1,^space2 container=lxd instance-type=bar zones=az1,az2",
 }, {
 	initial: initialWithoutCons,
 	without: []string{"mem"},
-	final:   "root-disk=8G arch=amd64 cpu-power=1000 cores=4 tags=foo spaces=space1,^space2 container=lxd instance-type=bar",
+	final:   "root-disk=8G arch=amd64 cpu-power=1000 cores=4 tags=foo spaces=space1,^space2 container=lxd instance-type=bar zones=az1,az2",
 }, {
 	initial: initialWithoutCons,
 	without: []string{"arch"},
-	final:   "root-disk=8G mem=4G cpu-power=1000 cores=4 tags=foo spaces=space1,^space2 container=lxd instance-type=bar",
+	final:   "root-disk=8G mem=4G cpu-power=1000 cores=4 tags=foo spaces=space1,^space2 container=lxd instance-type=bar zones=az1,az2",
 }, {
 	initial: initialWithoutCons,
 	without: []string{"cpu-power"},
-	final:   "root-disk=8G mem=4G arch=amd64 cores=4 tags=foo spaces=space1,^space2 container=lxd instance-type=bar",
+	final:   "root-disk=8G mem=4G arch=amd64 cores=4 tags=foo spaces=space1,^space2 container=lxd instance-type=bar zones=az1,az2",
 }, {
 	initial: initialWithoutCons,
 	without: []string{"cores"},
-	final:   "root-disk=8G mem=4G arch=amd64 cpu-power=1000 tags=foo spaces=space1,^space2 container=lxd instance-type=bar",
+	final:   "root-disk=8G mem=4G arch=amd64 cpu-power=1000 tags=foo spaces=space1,^space2 container=lxd instance-type=bar zones=az1,az2",
 }, {
 	initial: initialWithoutCons,
 	without: []string{"tags"},
-	final:   "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cores=4 spaces=space1,^space2 container=lxd instance-type=bar",
+	final:   "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cores=4 spaces=space1,^space2 container=lxd instance-type=bar zones=az1,az2",
 }, {
 	initial: initialWithoutCons,
 	without: []string{"spaces"},
-	final:   "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cores=4 tags=foo container=lxd instance-type=bar",
+	final:   "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cores=4 tags=foo container=lxd instance-type=bar zones=az1,az2",
 }, {
 	initial: initialWithoutCons,
 	without: []string{"container"},
-	final:   "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cores=4 tags=foo spaces=space1,^space2 instance-type=bar",
+	final:   "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cores=4 tags=foo spaces=space1,^space2 instance-type=bar zones=az1,az2",
 }, {
 	initial: initialWithoutCons,
 	without: []string{"instance-type"},
-	final:   "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cores=4 tags=foo spaces=space1,^space2 container=lxd",
+	final:   "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cores=4 tags=foo spaces=space1,^space2 container=lxd zones=az1,az2",
 }, {
 	initial: initialWithoutCons,
 	without: []string{"root-disk", "mem", "arch"},
-	final:   "cpu-power=1000 cores=4 tags=foo spaces=space1,^space2 container=lxd instance-type=bar",
+	final:   "cpu-power=1000 cores=4 tags=foo spaces=space1,^space2 container=lxd instance-type=bar zones=az1,az2",
+}, {
+	initial: initialWithoutCons,
+	without: []string{"zones"},
+	final:   "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cores=4 spaces=space1,^space2 tags=foo container=lxd instance-type=bar",
 }}
 
 func (s *ConstraintsSuite) TestWithout(c *gc.C) {

diff --git a/instance/distribution.go b/instance/distribution.go
@@ -8,16 +8,18 @@ import "github.com/juju/juju/environs/context"
 // Distributor is an interface that may be used to distribute
 // application units across instances for high availability.
 type Distributor interface {
-	// DistributeInstances takes a set of clean, empty
-	// instances, and a distribution group, and returns
-	// the subset of candidates which the policy will
-	// allow entry into the distribution group.
+	// DistributeInstances takes a set of clean, empty instances,
+	// a distribution group, and list of zones to limit the consideration to.
+	// If the input zone collection has no elements, then all availability
+	// zones are considered when attempting distribution.
+	// It returns the subset of candidates that the policy will allow to enter
+	// the distribution group.
 	//
-	// The AssignClean and AssignCleanEmpty unit
-	// assignment policies will attempt to assign a
-	// unit to each of the resulting instances until
-	// one is successful. If no instances can be assigned
-	// to (e.g. because of concurrent deployments), then
-	// a new machine will be allocated.
-	DistributeInstances(ctx context.ProviderCallContext, candidates, distributionGroup []Id) ([]Id, error)
+	// The AssignClean and AssignCleanEmpty unit assignment policies will
+	// attempt to assign a unit to each of the resulting instances until one is
+	// successful. If no instances can be assigned to (e.g. because of
+	// concurrent deployments), then a new machine will be allocated.
+	DistributeInstances(
+		ctx context.ProviderCallContext, candidates, distributionGroup []Id, limitZones []string,
+	) ([]Id, error)
 }