juju-core

Merge lp:~axwalk/juju-core/provider-unit-assignment-policy into lp:~go-bot/juju-core/trunk

  1. provider-unit-assignment-policy
  2. Merge into trunk
Proposed by Andrew Wilkins
Status: Merged
Approved by: Andrew Wilkins
Approved revision: no longer in the source branch.
Merged at revision: 2812
Proposed branch: lp:~axwalk/juju-core/provider-unit-assignment-policy
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 544 lines (+383/-50)
8 files modified
environs/statepolicy.go (+13/-0)
state/apiserver/provisioner/provisioner.go (+6/-27)
state/conn_test.go (+8/-0)
state/distribution.go (+77/-0)
state/distribution_test.go (+185/-0)
state/policy.go (+30/-8)
state/service.go (+7/-3)
state/unit.go (+57/-12)
To merge this branch: bzr merge lp:~axwalk/juju-core/provider-unit-assignment-policy
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+221481@code.launchpad.net

Commit message

Introduce InstanceDistributor policy

This change introduces an InstanceDistributor
policy, which will be used to implement automatic
Availability Zone management for AWS and OpenStack.

When an attempt is made to assign a unit to a clean
machine, we gather the instances that have already
been provisioned, and provide those and the unit's
distribution group to the provider. The provider
may select as many or as few of the candidates as it
wishes to allow; the assignment policy will then
choose one of them for assigning to. If no candidates
are returned, the unit will be assigned to a new
machine.

There are currently no implementations of this policy;
they will be forthcoming when manual AZ support lands
in the amazon/openstack providers.

https://codereview.appspot.com/99660047/

Description of the change

Introduce InstanceDistributor policy

This change introduces an InstanceDistributor
policy, which will be used to implement automatic
Availability Zone management for AWS and OpenStack.

When an attempt is made to assign a unit to a clean
machine, we gather the instances that have already
been provisioned, and provide those and the unit's
distribution group to the provider. The provider
may select as many or as few of the candidates as it
wishes to allow; the assignment policy will then
choose one of them for assigning to. If no candidates
are returned, the unit will be assigned to a new
machine.

There are currently no implementations of this policy;
they will be forthcoming when manual AZ support lands
in the amazon/openstack providers.

https://codereview.appspot.com/99660047/

To post a comment you must log in.
Revision history for this message
Andrew Wilkins (axwalk) wrote :

Reviewers: mp+221481_code.launchpad.net,

Message:
Please take a look.

Description:
Introduce InstanceDistributor policy

This change introduces an InstanceDistributor
policy, which will be used to implement automatic
Availability Zone management for AWS and OpenStack.

When an attempt is made to assign a unit to a clean
machine, we gather the instances that have already
been provisioned, and provide those and the unit's
distribution group to the provider. The provider
may select as many or as few of the candidates as it
wishes to allow; the assignment policy will then
choose one of them for assigning to. If no candidates
are returned, the unit will be assigned to a new
machine.

There are currently no implementations of this policy;
they will be forthcoming when manual AZ support lands
in the amazon/openstack providers.

https://code.launchpad.net/~axwalk/juju-core/provider-unit-assignment-policy/+merge/221481

(do not edit description out of merge proposal)

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

Affected files (+379, -40 lines):
   A [revision details]
   M environs/statepolicy.go
   M state/apiserver/provisioner/provisioner.go
   M state/conn_test.go
   A state/distribution.go
   A state/distribution_test.go
   M state/policy.go
   M state/unit.go

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

LGTM with a bit of mindful rearrangement.

https://codereview.appspot.com/99660047/diff/1/state/distribution.go
File state/distribution.go (right):

https://codereview.appspot.com/99660047/diff/1/state/distribution.go#newcode42
state/distribution.go:42: distributionGroup, err :=
service.ServiceInstances()
fwiw, this doesn't feel *quite* right because we don't actually ever
need/use the *Service directly -- the AllUnits could be figured out from
the unit name alone, so grabbing the service doc is just an unnecessary
db hit (I think?).

But it's a balance: using the methods we already have for
familarity/clarity's sake, vs writing more primitives that operate more
on ids and less on instantiated objects and *might* eventually lead us
in a leaner/cleaner direction. I have a slight preference for the latter
but I'll leave it to your judgment.

https://codereview.appspot.com/99660047/diff/1/state/distribution.go#newcode54
state/distribution.go:54: func (service *Service) ServiceInstances()
([]instance.Id, error) {
Suggestion: ServiceInstances(*state.Service). I retain a mild discomfort
over spreading the methods on a particular type across files, but just
making the receiver a regular param pretty much eliminates that feeling
of ick. I'm not sure it's *entirely* rational, but...

Anyway, same applies to Unit.distribute above. Doesn't feel like it's
quite in the right place.

https://codereview.appspot.com/99660047/diff/1/state/policy.go
File state/policy.go (right):

https://codereview.appspot.com/99660047/diff/1/state/policy.go#newcode204
state/policy.go:204: // TODO(axw) move this comment
where to? ;p

https://codereview.appspot.com/99660047/

Revision history for this message
Andrew Wilkins (axwalk) wrote :

Please take a look.

https://codereview.appspot.com/99660047/diff/1/state/distribution.go
File state/distribution.go (right):

https://codereview.appspot.com/99660047/diff/1/state/distribution.go#newcode42
state/distribution.go:42: distributionGroup, err :=
service.ServiceInstances()
On 2014/05/30 08:51:12, fwereade wrote:
> fwiw, this doesn't feel *quite* right because we don't actually ever
 need/use
> the *Service directly -- the AllUnits could be figured out from the
 unit name
> alone, so grabbing the service doc is just an unnecessary db hit (I
 think?).

Indeed it is unnecessary. I've changed ServiceInstances to take a
service name, and extracted the logic of Service.AllUnits out to a free
function which takes a service name.

> But it's a balance: using the methods we already have for
familarity/clarity's
> sake, vs writing more primitives that operate more on ids and less on
> instantiated objects and *might* eventually lead us in a
leaner/cleaner
> direction. I have a slight preference for the latter but I'll leave it
to your
> judgment.

https://codereview.appspot.com/99660047/diff/1/state/distribution.go#newcode54
state/distribution.go:54: func (service *Service) ServiceInstances()
([]instance.Id, error) {
On 2014/05/30 08:51:12, fwereade wrote:
> Suggestion: ServiceInstances(*state.Service). I retain a mild
 discomfort over
> spreading the methods on a particular type across files, but just
 making the
> receiver a regular param pretty much eliminates that feeling of ick.
 I'm not
> sure it's *entirely* rational, but...

> Anyway, same applies to Unit.distribute above. Doesn't feel like it's
quite in
> the right place.

Changed both to free functions.

https://codereview.appspot.com/99660047/diff/1/state/policy.go
File state/policy.go (right):

https://codereview.appspot.com/99660047/diff/1/state/policy.go#newcode204
state/policy.go:204: // TODO(axw) move this comment
On 2014/05/30 08:51:12, fwereade wrote:
> where to? ;p

Yeah, it's probably best just left here.

https://codereview.appspot.com/99660047/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'environs/statepolicy.go'
--- environs/statepolicy.go 2014-04-24 12:33:19 +0000
+++ environs/statepolicy.go 2014-05-30 12:33:30 +0000
@@ -4,6 +4,8 @@
4package environs4package environs
55
6import (6import (
7 "github.com/juju/errors"
8
7 "launchpad.net/juju-core/constraints"9 "launchpad.net/juju-core/constraints"
8 "launchpad.net/juju-core/environs/config"10 "launchpad.net/juju-core/environs/config"
9 "launchpad.net/juju-core/state"11 "launchpad.net/juju-core/state"
@@ -44,3 +46,14 @@
44 }46 }
45 return env.ConstraintsValidator()47 return env.ConstraintsValidator()
46}48}
49
50func (environStatePolicy) InstanceDistributor(cfg *config.Config) (state.InstanceDistributor, error) {
51 env, err := New(cfg)
52 if err != nil {
53 return nil, err
54 }
55 if p, ok := env.(state.InstanceDistributor); ok {
56 return p, nil
57 }
58 return nil, errors.NotImplementedf("InstanceDistributor")
59}
4760
=== modified file 'state/apiserver/provisioner/provisioner.go'
--- state/apiserver/provisioner/provisioner.go 2014-05-13 12:57:53 +0000
+++ state/apiserver/provisioner/provisioner.go 2014-05-30 12:33:30 +0000
@@ -416,33 +416,12 @@
416 if !unit.IsPrincipal() {416 if !unit.IsPrincipal() {
417 continue417 continue
418 }418 }
419 service, err := unit.Service()419 instanceIds, err := state.ServiceInstances(st, unit.ServiceName())
420 if err != nil {420 if err != nil {
421 return nil, err421 return nil, err
422 }422 }
423 allUnits, err := service.AllUnits()423 for _, instanceId := range instanceIds {
424 if err != nil {424 instanceIdSet.Add(string(instanceId))
425 return nil, err
426 }
427 for _, unit := range allUnits {
428 machineId, err := unit.AssignedMachineId()
429 if state.IsNotAssigned(err) {
430 continue
431 } else if err != nil {
432 return nil, err
433 }
434 machine, err := st.Machine(machineId)
435 if err != nil {
436 return nil, err
437 }
438 instanceId, err := machine.InstanceId()
439 if err == nil {
440 instanceIdSet.Add(string(instanceId))
441 } else if state.IsNotProvisionedError(err) {
442 continue
443 } else {
444 return nil, err
445 }
446 }425 }
447 }426 }
448 instanceIds := make([]instance.Id, instanceIdSet.Size())427 instanceIds := make([]instance.Id, instanceIdSet.Size())
449428
=== modified file 'state/conn_test.go'
--- state/conn_test.go 2014-05-20 04:27:02 +0000
+++ state/conn_test.go 2014-05-30 12:33:30 +0000
@@ -105,6 +105,7 @@
105 getConfigValidator func(string) (state.ConfigValidator, error)105 getConfigValidator func(string) (state.ConfigValidator, error)
106 getEnvironCapability func(*config.Config) (state.EnvironCapability, error)106 getEnvironCapability func(*config.Config) (state.EnvironCapability, error)
107 getConstraintsValidator func(*config.Config) (constraints.Validator, error)107 getConstraintsValidator func(*config.Config) (constraints.Validator, error)
108 getInstanceDistributor func(*config.Config) (state.InstanceDistributor, error)
108}109}
109110
110func (p *mockPolicy) Prechecker(cfg *config.Config) (state.Prechecker, error) {111func (p *mockPolicy) Prechecker(cfg *config.Config) (state.Prechecker, error) {
@@ -134,3 +135,10 @@
134 }135 }
135 return nil, errors.NewNotImplemented(nil, "ConstraintsValidator")136 return nil, errors.NewNotImplemented(nil, "ConstraintsValidator")
136}137}
138
139func (p *mockPolicy) InstanceDistributor(cfg *config.Config) (state.InstanceDistributor, error) {
140 if p.getInstanceDistributor != nil {
141 return p.getInstanceDistributor(cfg)
142 }
143 return nil, errors.NewNotImplemented(nil, "InstanceDistributor")
144}
137145
=== added file 'state/distribution.go'
--- state/distribution.go 1970-01-01 00:00:00 +0000
+++ state/distribution.go 2014-05-30 12:33:30 +0000
@@ -0,0 +1,77 @@
1// Copyright 2014 Canonical Ltd.
2// Licensed under the AGPLv3, see LICENCE file for details.
3
4package state
5
6import (
7 "fmt"
8
9 "github.com/juju/errors"
10
11 "launchpad.net/juju-core/instance"
12)
13
14// distributeuUnit takes a unit and set of clean, possibly empty, instances
15// and asks the InstanceDistributor policy (if any) which ones are suitable
16// for assigning the unit to. If there is no InstanceDistributor, or the
17// distribution group is empty, then all of the candidates will be returned.
18func distributeUnit(u *Unit, candidates []instance.Id) ([]instance.Id, error) {
19 if len(candidates) == 0 {
20 return nil, nil
21 }
22 if u.st.policy == nil {
23 return candidates, nil
24 }
25 cfg, err := u.st.EnvironConfig()
26 if err != nil {
27 return nil, err
28 }
29 distributor, err := u.st.policy.InstanceDistributor(cfg)
30 if errors.IsNotImplemented(err) {
31 return candidates, nil
32 } else if err != nil {
33 return nil, err
34 }
35 if distributor == nil {
36 return nil, fmt.Errorf("policy returned nil instance distributor without an error")
37 }
38 distributionGroup, err := ServiceInstances(u.st, u.doc.Service)
39 if err != nil {
40 return nil, err
41 }
42 if len(distributionGroup) == 0 {
43 return candidates, nil
44 }
45 return distributor.DistributeInstances(candidates, distributionGroup)
46}
47
48// ServiceInstances returns the instance IDs of provisioned
49// machines that are assigned units of the specified service.
50func ServiceInstances(st *State, service string) ([]instance.Id, error) {
51 units, err := allUnits(st, service)
52 if err != nil {
53 return nil, err
54 }
55 instanceIds := make([]instance.Id, 0, len(units))
56 for _, unit := range units {
57 machineId, err := unit.AssignedMachineId()
58 if IsNotAssigned(err) {
59 continue
60 } else if err != nil {
61 return nil, err
62 }
63 machine, err := st.Machine(machineId)
64 if err != nil {
65 return nil, err
66 }
67 instanceId, err := machine.InstanceId()
68 if err == nil {
69 instanceIds = append(instanceIds, instanceId)
70 } else if IsNotProvisionedError(err) {
71 continue
72 } else {
73 return nil, err
74 }
75 }
76 return instanceIds, nil
77}
078
=== added file 'state/distribution_test.go'
--- state/distribution_test.go 1970-01-01 00:00:00 +0000
+++ state/distribution_test.go 2014-05-30 12:33:30 +0000
@@ -0,0 +1,185 @@
1// Copyright 2014 Canonical Ltd.
2// Licensed under the AGPLv3, see LICENCE file for details.
3
4package state_test
5
6import (
7 "fmt"
8
9 "github.com/juju/errors"
10 jc "github.com/juju/testing/checkers"
11 gc "launchpad.net/gocheck"
12
13 "launchpad.net/juju-core/environs/config"
14 "launchpad.net/juju-core/instance"
15 "launchpad.net/juju-core/state"
16)
17
18type InstanceDistributorSuite struct {
19 ConnSuite
20 distributor mockInstanceDistributor
21 wordpress *state.Service
22 machines []*state.Machine
23}
24
25var _ = gc.Suite(&InstanceDistributorSuite{})
26
27type mockInstanceDistributor struct {
28 candidates []instance.Id
29 distributionGroup []instance.Id
30 result []instance.Id
31 err error
32}
33
34func (p *mockInstanceDistributor) DistributeInstances(candidates, distributionGroup []instance.Id) ([]instance.Id, error) {
35 p.candidates = candidates
36 p.distributionGroup = distributionGroup
37 result := p.result
38 if result == nil {
39 result = candidates
40 }
41 return result, p.err
42}
43
44func (s *InstanceDistributorSuite) SetUpTest(c *gc.C) {
45 s.ConnSuite.SetUpTest(c)
46 s.distributor = mockInstanceDistributor{}
47 s.policy.getInstanceDistributor = func(*config.Config) (state.InstanceDistributor, error) {
48 return &s.distributor, nil
49 }
50 s.wordpress = s.AddTestingServiceWithNetworks(
51 c,
52 "wordpress",
53 s.AddTestingCharm(c, "wordpress"),
54 []string{"net1", "net2"},
55 []string{"net3", "net4"},
56 )
57 s.machines = make([]*state.Machine, 3)
58 for i := range s.machines {
59 var err error
60 s.machines[i], err = s.State.AddOneMachine(state.MachineTemplate{
61 Series: "quantal",
62 Jobs: []state.MachineJob{state.JobHostUnits},
63 })
64 c.Assert(err, gc.IsNil)
65 }
66}
67
68func (s *InstanceDistributorSuite) setupScenario(c *gc.C) {
69 // Assign a unit so we have a non-empty distribution group, and
70 // provision all instances so we have candidates.
71 unit, err := s.wordpress.AddUnit()
72 c.Assert(err, gc.IsNil)
73 err = unit.AssignToMachine(s.machines[0])
74 c.Assert(err, gc.IsNil)
75 for i, m := range s.machines {
76 instId := instance.Id(fmt.Sprintf("i-blah-%d", i))
77 err = m.SetProvisioned(instId, "fake-nonce", nil)
78 c.Assert(err, gc.IsNil)
79 }
80}
81
82func (s *InstanceDistributorSuite) TestDistributeInstances(c *gc.C) {
83 s.setupScenario(c)
84 unit, err := s.wordpress.AddUnit()
85 c.Assert(err, gc.IsNil)
86 _, err = unit.AssignToCleanMachine()
87 c.Assert(err, gc.IsNil)
88 c.Assert(s.distributor.candidates, jc.SameContents, []instance.Id{"i-blah-1", "i-blah-2"})
89 c.Assert(s.distributor.distributionGroup, jc.SameContents, []instance.Id{"i-blah-0"})
90 s.distributor.result = []instance.Id{}
91 _, err = unit.AssignToCleanMachine()
92 c.Assert(err, gc.ErrorMatches, eligibleMachinesInUse)
93}
94
95func (s *InstanceDistributorSuite) TestDistributeInstancesInvalidInstances(c *gc.C) {
96 s.setupScenario(c)
97 unit, err := s.wordpress.AddUnit()
98 c.Assert(err, gc.IsNil)
99 s.distributor.result = []instance.Id{"notthere"}
100 _, err = unit.AssignToCleanMachine()
101 c.Assert(err, gc.ErrorMatches, `cannot assign unit "wordpress/1" to clean machine: invalid instance returned: notthere`)
102}
103
104func (s *InstanceDistributorSuite) TestDistributeInstancesNoEmptyMachines(c *gc.C) {
105 for i := range s.machines {
106 // Assign a unit so we have a non-empty distribution group.
107 unit, err := s.wordpress.AddUnit()
108 c.Assert(err, gc.IsNil)
109 m, err := unit.AssignToCleanMachine()
110 c.Assert(err, gc.IsNil)
111 instId := instance.Id(fmt.Sprintf("i-blah-%d", i))
112 err = m.SetProvisioned(instId, "fake-nonce", nil)
113 c.Assert(err, gc.IsNil)
114 }
115
116 // InstanceDistributor is not called if there are no empty instances.
117 s.distributor.err = fmt.Errorf("no assignment for you")
118 unit, err := s.wordpress.AddUnit()
119 c.Assert(err, gc.IsNil)
120 _, err = unit.AssignToCleanMachine()
121 c.Assert(err, gc.ErrorMatches, eligibleMachinesInUse)
122}
123
124func (s *InstanceDistributorSuite) TestDistributeInstancesErrors(c *gc.C) {
125 s.setupScenario(c)
126 unit, err := s.wordpress.AddUnit()
127 c.Assert(err, gc.IsNil)
128
129 // Ensure that assignment fails when DistributeInstances returns an error.
130 s.distributor.err = fmt.Errorf("no assignment for you")
131 _, err = unit.AssignToCleanMachine()
132 c.Assert(err, gc.ErrorMatches, ".*no assignment for you")
133 _, err = unit.AssignToCleanEmptyMachine()
134 c.Assert(err, gc.ErrorMatches, ".*no assignment for you")
135 // If the policy's InstanceDistributor method fails, that will be returned first.
136 s.policy.getInstanceDistributor = func(*config.Config) (state.InstanceDistributor, error) {
137 return nil, fmt.Errorf("incapable of InstanceDistributor")
138 }
139 _, err = unit.AssignToCleanMachine()
140 c.Assert(err, gc.ErrorMatches, ".*incapable of InstanceDistributor")
141}
142
143func (s *InstanceDistributorSuite) TestDistributeInstancesEmptyDistributionGroup(c *gc.C) {
144 s.distributor.err = fmt.Errorf("no assignment for you")
145
146 // InstanceDistributor is not called if the distribution group is empty.
147 unit0, err := s.wordpress.AddUnit()
148 c.Assert(err, gc.IsNil)
149 _, err = unit0.AssignToCleanMachine()
150 c.Assert(err, gc.IsNil)
151
152 // Distribution group is still empty, because the machine assigned to has
153 // not been provisioned.
154 unit1, err := s.wordpress.AddUnit()
155 c.Assert(err, gc.IsNil)
156 _, err = unit1.AssignToCleanMachine()
157 c.Assert(err, gc.IsNil)
158}
159
160func (s *InstanceDistributorSuite) TestInstanceDistributorUnimplemented(c *gc.C) {
161 s.setupScenario(c)
162 var distributorErr error
163 s.policy.getInstanceDistributor = func(*config.Config) (state.InstanceDistributor, error) {
164 return nil, distributorErr
165 }
166 unit, err := s.wordpress.AddUnit()
167 c.Assert(err, gc.IsNil)
168 _, err = unit.AssignToCleanMachine()
169 c.Assert(err, gc.ErrorMatches, `cannot assign unit "wordpress/1" to clean machine: policy returned nil instance distributor without an error`)
170 distributorErr = errors.NotImplementedf("InstanceDistributor")
171 _, err = unit.AssignToCleanMachine()
172 c.Assert(err, gc.IsNil)
173}
174
175func (s *InstanceDistributorSuite) TestDistributeInstancesNoPolicy(c *gc.C) {
176 s.policy.getInstanceDistributor = func(*config.Config) (state.InstanceDistributor, error) {
177 c.Errorf("should not have been invoked")
178 return nil, nil
179 }
180 state.SetPolicy(s.State, nil)
181 unit, err := s.wordpress.AddUnit()
182 c.Assert(err, gc.IsNil)
183 _, err = unit.AssignToCleanMachine()
184 c.Assert(err, gc.IsNil)
185}
0186
=== modified file 'state/policy.go'
--- state/policy.go 2014-05-13 04:30:48 +0000
+++ state/policy.go 2014-05-30 12:33:30 +0000
@@ -10,6 +10,7 @@
1010
11 "launchpad.net/juju-core/constraints"11 "launchpad.net/juju-core/constraints"
12 "launchpad.net/juju-core/environs/config"12 "launchpad.net/juju-core/environs/config"
13 "launchpad.net/juju-core/instance"
13)14)
1415
15// Policy is an interface provided to State that may16// Policy is an interface provided to State that may
@@ -22,21 +23,24 @@
22// be ignored. Any other error will cause an error23// be ignored. Any other error will cause an error
23// in the use of the policy.24// in the use of the policy.
24type Policy interface {25type Policy interface {
25 // Prechecker takes a *config.Config and returns26 // Prechecker takes a *config.Config and returns a Prechecker or an error.
26 // a (possibly nil) Prechecker or an error.
27 Prechecker(*config.Config) (Prechecker, error)27 Prechecker(*config.Config) (Prechecker, error)
2828
29 // ConfigValidator takes a provider type name and returns29 // ConfigValidator takes a provider type name and returns a ConfigValidator
30 // a (possibly nil) ConfigValidator or an error.30 // or an error.
31 ConfigValidator(providerType string) (ConfigValidator, error)31 ConfigValidator(providerType string) (ConfigValidator, error)
3232
33 // EnvironCapability takes a *config.Config and returns33 // EnvironCapability takes a *config.Config and returns an EnvironCapability
34 // a (possibly nil) EnvironCapability or an error.34 // or an error.
35 EnvironCapability(*config.Config) (EnvironCapability, error)35 EnvironCapability(*config.Config) (EnvironCapability, error)
3636
37 // ConstraintsValidator takes a *config.Config and returns37 // ConstraintsValidator takes a *config.Config and returns a
38 // a (possibly nil) constraints.Validator or an error.38 // constraints.Validator or an error.
39 ConstraintsValidator(*config.Config) (constraints.Validator, error)39 ConstraintsValidator(*config.Config) (constraints.Validator, error)
40
41 // InstanceDistributor takes a *config.Config and returns an
42 // InstanceDistributor or an error.
43 InstanceDistributor(*config.Config) (InstanceDistributor, error)
40}44}
4145
42// Prechecker is a policy interface that is provided to State46// Prechecker is a policy interface that is provided to State
@@ -187,3 +191,21 @@
187 }191 }
188 return capability.SupportsUnitPlacement()192 return capability.SupportsUnitPlacement()
189}193}
194
195// InstanceDistributor is a policy interface that is provided
196// to State to perform distribution of units across instances
197// for high availability.
198type InstanceDistributor interface {
199 // DistributeInstance takes a set of clean, empty
200 // instances, and a distribution group, and returns
201 // the subset of candidates which the policy will
202 // allow entry into the distribution group.
203 //
204 // The AssignClean and AssignCleanEmpty unit
205 // assignment policies will attempt to assign a
206 // unit to each of the resulting instances until
207 // one is successful. If no instances can be assigned
208 // to (e.g. because of concurrent deployments), then
209 // a new machine will be allocated.
210 DistributeInstances(candidates, distributionGroup []instance.Id) ([]instance.Id, error)
211}
190212
=== modified file 'state/service.go'
--- state/service.go 2014-05-26 21:16:46 +0000
+++ state/service.go 2014-05-30 12:33:30 +0000
@@ -723,13 +723,17 @@
723723
724// AllUnits returns all units of the service.724// AllUnits returns all units of the service.
725func (s *Service) AllUnits() (units []*Unit, err error) {725func (s *Service) AllUnits() (units []*Unit, err error) {
726 return allUnits(s.st, s.doc.Name)
727}
728
729func allUnits(st *State, service string) (units []*Unit, err error) {
726 docs := []unitDoc{}730 docs := []unitDoc{}
727 err = s.st.units.Find(bson.D{{"service", s.doc.Name}}).All(&docs)731 err = st.units.Find(bson.D{{"service", service}}).All(&docs)
728 if err != nil {732 if err != nil {
729 return nil, fmt.Errorf("cannot get all units from service %q: %v", s, err)733 return nil, fmt.Errorf("cannot get all units from service %q: %v", service, err)
730 }734 }
731 for i := range docs {735 for i := range docs {
732 units = append(units, newUnit(s.st, &docs[i]))736 units = append(units, newUnit(st, &docs[i]))
733 }737 }
734 return units, nil738 return units, nil
735}739}
736740
=== modified file 'state/unit.go'
--- state/unit.go 2014-05-27 05:36:00 +0000
+++ state/unit.go 2014-05-30 12:33:30 +0000
@@ -1277,14 +1277,63 @@
1277 return nil, err1277 return nil, err
1278 }1278 }
12791279
1280 // TODO(rog) Fix so this is more efficient when there are concurrent uses.1280 // Find all of the candidate machines, and associated
1281 // Possible solution: pick the highest and the smallest id of all1281 // instances for those that are provisioned. Instances
1282 // unused machines, and try to assign to the first one >= a random id in the1282 // will be distributed across in preference to
1283 // middle.1283 // unprovisioned machines.
1284 iter := query.Batch(1).Prefetch(0).Iter()1284 var mdocs []*machineDoc
1285 var mdoc machineDoc1285 if err := query.All(&mdocs); err != nil {
1286 for iter.Next(&mdoc) {1286 assignContextf(&err, u, context)
1287 m := newMachine(u.st, &mdoc)1287 return nil, err
1288 }
1289 var unprovisioned []*Machine
1290 var instances []instance.Id
1291 instanceMachines := make(map[instance.Id]*Machine)
1292 for _, mdoc := range mdocs {
1293 m := newMachine(u.st, mdoc)
1294 instance, err := m.InstanceId()
1295 if IsNotProvisionedError(err) {
1296 unprovisioned = append(unprovisioned, m)
1297 } else if err != nil {
1298 assignContextf(&err, u, context)
1299 return nil, err
1300 } else {
1301 instances = append(instances, instance)
1302 instanceMachines[instance] = m
1303 }
1304 }
1305
1306 // Filter the list of instances that are suitable for
1307 // distribution, and then map them back to machines.
1308 //
1309 // TODO(axw) 2014-05-30 #1324904
1310 // Shuffle machines to reduce likelihood of collisions.
1311 // The partition of provisioned/unprovisioned machines
1312 // must be maintained.
1313 if instances, err = distributeUnit(u, instances); err != nil {
1314 assignContextf(&err, u, context)
1315 return nil, err
1316 }
1317 machines := make([]*Machine, len(instances), len(instances)+len(unprovisioned))
1318 for i, instance := range instances {
1319 m, ok := instanceMachines[instance]
1320 if !ok {
1321 err := fmt.Errorf("invalid instance returned: %v", instance)
1322 assignContextf(&err, u, context)
1323 return nil, err
1324 }
1325 machines[i] = m
1326 }
1327 machines = append(machines, unprovisioned...)
1328
1329 // TODO(axw) 2014-05-30 #1253704
1330 // We should not select a machine that is in the process
1331 // of being provisioned. There's no point asserting that
1332 // the machine hasn't been provisioned, as there'll still
1333 // be a period of time during which the machine may be
1334 // provisioned without the fact having yet been recorded
1335 // in state.
1336 for _, m := range machines {
1288 err := u.assignToMachine(m, true)1337 err := u.assignToMachine(m, true)
1289 if err == nil {1338 if err == nil {
1290 return m, nil1339 return m, nil
@@ -1294,10 +1343,6 @@
1294 return nil, err1343 return nil, err
1295 }1344 }
1296 }1345 }
1297 if err := iter.Err(); err != nil {
1298 assignContextf(&err, u, context)
1299 return nil, err
1300 }
1301 return nil, noCleanMachines1346 return nil, noCleanMachines
1302}1347}
13031348

Subscribers

People subscribed via source and target branches

to status/vote changes: