juju-core

Merge lp:~wallyworld/juju-core/instance-type-constraint into lp:~go-bot/juju-core/trunk

  1. instance-type-constraint
  2. Merge into trunk
Proposed by Ian Booth
Status: Merged
Approved by: Ian Booth
Approved revision: no longer in the source branch.
Merged at revision: 2659
Proposed branch: lp:~wallyworld/juju-core/instance-type-constraint
Merge into: lp:~go-bot/juju-core/trunk
Prerequisite: lp:~wallyworld/juju-core/constraints-validation-merge
Diff against target: 1085 lines (+562/-52)
31 files modified
cmd/juju/bootstrap.go (+8/-0)
cmd/juju/bootstrap_test.go (+8/-0)
environs/interface.go (+4/-0)
environs/statepolicy.go (+9/-0)
provider/azure/environ.go (+12/-0)
provider/azure/environ_test.go (+9/-0)
provider/dummy/environs.go (+8/-0)
provider/ec2/ec2.go (+14/-0)
provider/ec2/local_test.go (+19/-0)
provider/joyent/environ_instance.go (+13/-0)
provider/joyent/local_test.go (+9/-0)
provider/local/environ.go (+14/-0)
provider/local/environ_test.go (+11/-0)
provider/maas/environ.go (+12/-0)
provider/maas/environ_test.go (+12/-0)
provider/manual/environ.go (+13/-0)
provider/manual/environ_test.go (+9/-0)
provider/openstack/local_test.go (+19/-0)
provider/openstack/provider.go (+15/-0)
state/addmachine.go (+18/-22)
state/conn_test.go (+12/-3)
state/constraints.go (+24/-21)
state/constraintsvalidation_test.go (+104/-0)
state/export_test.go (+5/-0)
state/machine.go (+7/-0)
state/machine_test.go (+37/-0)
state/policy.go (+51/-0)
state/service.go (+9/-6)
state/service_test.go (+33/-0)
state/state.go (+7/-0)
state/state_test.go (+37/-0)
To merge this branch: bzr merge lp:~wallyworld/juju-core/instance-type-constraint
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+216244@code.launchpad.net

Commit message

Add constraints validation to providers

Each provider has a constraints validator which
is used when setting constraints on a machine or
service, as well as when constraints are merged.
The validation step allows conflicting constraints
like instance-type and mem to be rejected, and also
unsupported constraints to be logged with a warning.
The merge step allows things like instance-type to
mask other incompatible constraints like mem or arch,
and visa versa.

https://codereview.appspot.com/88780043/

Description of the change

Add constraints validation to providers

Each provider has a constraints validator which
is used when setting constraints on a machine or
service, as well as when constraints are merged.
The validation step allows conflicting constraints
like instance-type and mem to be rejected, and also
unsupported constraints to be logged with a warning.
The merge step allows things like instance-type to
mask other incompatible constraints like mem or arch,
and visa versa.

https://codereview.appspot.com/88780043/

To post a comment you must log in.
Revision history for this message
Ian Booth (wallyworld) wrote :

Reviewers: mp+216244_code.launchpad.net,

Message:
Please take a look.

Description:
Add constraints validation to providers

Each provider has a constraints validator which
is used when setting constraints on a machine or
service, as well as when constraints are merged.
The validation step allows conflicting constraints
like instance-type and mem to be rejected, and also
unsupported constraints to be logged with a warning.
The merge step allows things like instance-type to
mask other incompatible constraints like mem or arch,
and visa versa.

https://code.launchpad.net/~wallyworld/juju-core/instance-type-constraint/+merge/216244

Requires:
https://code.launchpad.net/~wallyworld/juju-core/constraints-validation-merge/+merge/215807

(do not edit description out of merge proposal)

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

Affected files (+529, -61 lines):
   A [revision details]
   M environs/instances/image.go
   M environs/interface.go
   M environs/statepolicy.go
   M provider/azure/environ.go
   M provider/azure/environ_test.go
   M provider/dummy/environs.go
   M provider/ec2/ec2.go
   M provider/ec2/local_test.go
   M provider/joyent/environ_instance.go
   M provider/joyent/local_test.go
   M provider/local/environ.go
   M provider/local/environ_test.go
   M provider/maas/environ.go
   M provider/maas/environ_test.go
   M provider/manual/environ.go
   M provider/manual/environ_test.go
   M provider/openstack/local_test.go
   M provider/openstack/provider.go
   M state/addmachine.go
   M state/conn_test.go
   M state/constraints.go
   A state/constraintsvalidation_test.go
   M state/export_test.go
   M state/machine.go
   M state/machine_test.go
   M state/policy.go
   M state/service.go
   M state/service_test.go
   M state/state_test.go

Revision history for this message
Ian Booth (wallyworld) wrote :

Please take a look.

https://codereview.appspot.com/88780043/

Revision history for this message
Ian Booth (wallyworld) wrote :

Please take a look.

https://codereview.appspot.com/88780043/

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

I think there's a SetEnvironConstraints somewhere that needs to be
validated, and I'm a bit uncertain about some of the what-we-support
choices; but this is generally looking very solid. Thanks.

I also think we need some more integration with the Prechecker -- those
probably ought to check the constraints internally -- but that's fine as
a followup.

https://codereview.appspot.com/88780043/diff/40001/cmd/juju/bootstrap.go
File cmd/juju/bootstrap.go (right):

https://codereview.appspot.com/88780043/diff/40001/cmd/juju/bootstrap.go#newcode107
cmd/juju/bootstrap.go:107: logger.Warningf("%v", err)
Not for this CL, but we really do need a channel by which we can pass
this sort of warning back to a user on the other side of the API
(bootstrap is fine, I know, but all the other times....).

https://codereview.appspot.com/88780043/diff/40001/provider/ec2/ec2.go
File provider/ec2/ec2.go (right):

https://codereview.appspot.com/88780043/diff/40001/provider/ec2/ec2.go#newcode369
provider/ec2/ec2.go:369: []string{constraints.Mem,
constraints.CpuCores})
CpuPower too, I think

https://codereview.appspot.com/88780043/diff/40001/provider/ec2/local_test.go
File provider/ec2/local_test.go (right):

https://codereview.appspot.com/88780043/diff/40001/provider/ec2/local_test.go#newcode378
provider/ec2/local_test.go:378: c.Assert(cons, gc.DeepEquals,
constraints.MustParse("arch=i386 instance-type=foo cpu-power=10"))
Ideally instance-type's value will itself be validated here -- such that
cc2.x8large is rejected in the regions that don't provide them, etc etc.
That'd be reasonable as a followup, possibly alongside the API that
lists available instance-type values (also maybe good to do the same for
arch?).

https://codereview.appspot.com/88780043/diff/40001/provider/local/environ.go
File provider/local/environ.go (right):

https://codereview.appspot.com/88780043/diff/40001/provider/local/environ.go#newcode306
provider/local/environ.go:306: constraints.Tags,
Really, mem and cpu-cores are basically unsupported too (even if we
*could* do something with them in some cases).

This seems fine as it is (progress not perfection) but we might want a
followup that massages some of these a bit.

https://codereview.appspot.com/88780043/diff/40001/provider/manual/environ.go
File provider/manual/environ.go (right):

https://codereview.appspot.com/88780043/diff/40001/provider/manual/environ.go#newcode273
provider/manual/environ.go:273: constraints.Tags,
Not sure about this. Shouldn't these all be supported? Constraints for
add-machine are a bit weird, but constraints for deploying service units
on particular machines absolutely should be accepted and matched against
the machine's hardware characteristics, I think.

https://codereview.appspot.com/88780043/diff/40001/state/machine.go
File state/machine.go (right):

https://codereview.appspot.com/88780043/diff/40001/state/machine.go#newcode1097
state/machine.go:1097: logger.Warningf("setting constraints on machine
%q: %v", m.Tag(), err)
m.Id()?

https://codereview.appspot.com/88780043/

Revision history for this message
Ian Booth (wallyworld) wrote :

On 2014/04/18 08:25:04, fwereade wrote:
> I think there's a SetEnvironConstraints somewhere that needs to be
 validated,
> and I'm a bit uncertain about some of the what-we-support choices; but
 this is
> generally looking very solid. Thanks.

I added checks to the SetEnvironConstraints method.

> I also think we need some more integration with the Prechecker --
those probably
> ought to check the constraints internally -- but that's fine as a
followup.

Next branch adds stuff to Prechecker.

https://codereview.appspot.com/88780043/

Revision history for this message
Ian Booth (wallyworld) wrote :
Download full text (3.2 KiB)

Please take a look.

https://codereview.appspot.com/88780043/diff/40001/provider/ec2/ec2.go
File provider/ec2/ec2.go (right):

https://codereview.appspot.com/88780043/diff/40001/provider/ec2/ec2.go#newcode369
provider/ec2/ec2.go:369: []string{constraints.Mem,
constraints.CpuCores})
On 2014/04/18 08:25:04, fwereade wrote:
> CpuPower too, I think

Done.

https://codereview.appspot.com/88780043/diff/40001/provider/ec2/local_test.go
File provider/ec2/local_test.go (right):

https://codereview.appspot.com/88780043/diff/40001/provider/ec2/local_test.go#newcode378
provider/ec2/local_test.go:378: c.Assert(cons, gc.DeepEquals,
constraints.MustParse("arch=i386 instance-type=foo cpu-power=10"))
On 2014/04/18 08:25:04, fwereade wrote:
> Ideally instance-type's value will itself be validated here -- such
 that
> cc2.x8large is rejected in the regions that don't provide them, etc
 etc. That'd
> be reasonable as a followup, possibly alongside the API that lists
 available
> instance-type values (also maybe good to do the same for arch?).

The instance-type check is done in the next branch as part of the
PrecheckInstance() step. I thought about doing it on the
constraints.Validate() step but this would involve plugging provider
specific behaviour into the constraints package and we already have the
pre check stuff that will do the job, and will cause an error before a
new machine record is added to state. It won't prevent service
constraints from being set with a bad instance type however. But I
thought the implementation chosen is a viable approach nonetheless.

https://codereview.appspot.com/88780043/diff/40001/provider/local/environ.go
File provider/local/environ.go (right):

https://codereview.appspot.com/88780043/diff/40001/provider/local/environ.go#newcode306
provider/local/environ.go:306: constraints.Tags,
On 2014/04/18 08:25:04, fwereade wrote:
> Really, mem and cpu-cores are basically unsupported too (even if we
 *could* do
> something with them in some cases).

> This seems fine as it is (progress not perfection) but we might want a
followup
> that massages some of these a bit.

I think we should support mem, and I recall that we actually use it to
set the container size (or were going to). I'll add cpu cores to the
list though.

https://codereview.appspot.com/88780043/diff/40001/provider/manual/environ.go
File provider/manual/environ.go (right):

https://codereview.appspot.com/88780043/diff/40001/provider/manual/environ.go#newcode273
provider/manual/environ.go:273: constraints.Tags,
On 2014/04/18 08:25:04, fwereade wrote:
> Not sure about this. Shouldn't these all be supported? Constraints for
> add-machine are a bit weird, but constraints for deploying service
 units on
> particular machines absolutely should be accepted and matched against
 the
> machine's hardware characteristics, I think.

Tags I don't think we need, nor instance-type or cpu-power. I'll remove
the rest.

https://codereview.appspot.com/88780043/diff/40001/state/machine.go
File state/machine.go (right):

https://codereview.appspot.com/88780043/diff/40001/state/machine.go#newcode1097
state/machine.go:1097: logger.Warningf("setting constraints on machine
%q: %v", m.Tag(), err)
On 2014...

Read more...

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

LGTM if you promise to do a followup that implements constraint
vocabularies ;).

https://codereview.appspot.com/88780043/diff/40001/provider/ec2/local_test.go
File provider/ec2/local_test.go (right):

https://codereview.appspot.com/88780043/diff/40001/provider/ec2/local_test.go#newcode378
provider/ec2/local_test.go:378: c.Assert(cons, gc.DeepEquals,
constraints.MustParse("arch=i386 instance-type=foo cpu-power=10"))
On 2014/04/18 13:51:19, wallyworld wrote:
> On 2014/04/18 08:25:04, fwereade wrote:
> > Ideally instance-type's value will itself be validated here -- such
 that
> > cc2.x8large is rejected in the regions that don't provide them, etc
 etc.
> That'd
> > be reasonable as a followup, possibly alongside the API that lists
 available
> > instance-type values (also maybe good to do the same for arch?).

> The instance-type check is done in the next branch as part of the
> PrecheckInstance() step. I thought about doing it on the
constraints.Validate()
> step but this would involve plugging provider specific behaviour into
the
> constraints package and we already have the pre check stuff that will
do the
> job, and will cause an error before a new machine record is added to
state. It
> won't prevent service constraints from being set with a bad instance
type
> however. But I thought the implementation chosen is a viable approach
> nonetheless.

Hmm. ISTM that there are several, uh, vocab-constrained constraints --
arch, instance-type, and tags -- and each of them vary by cloud, and
it'll almost certainly be to our benefit to be able to reject the crazy
ones before they get into state anywhere at all -- I'm really quite
concerned about the delay between the bad specification and the actual
error reporting.

It can go into a followup, this clearly remains a good direction, but I
think we do need to be able to fail fast on those values. Some of them
are potentially tricky, ofc -- the set of arches available depends on
the juju tools available, and the images, but we've at least partly
addressed that issue already iirc.

https://codereview.appspot.com/88780043/diff/60001/state/service.go
File state/service.go (right):

https://codereview.appspot.com/88780043/diff/60001/state/service.go#newcode810
state/service.go:810:
I'm also bothered that we don't have any way to communicate this back to
the user [who isn't running debug-log, at least]. Out of scope, but
irritating.

https://codereview.appspot.com/88780043/

Revision history for this message
Go Bot (go-bot) wrote :
Download full text (10.6 KiB)

The attempt to merge lp:~wallyworld/juju-core/instance-type-constraint into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core 0.021s
ok launchpad.net/juju-core/agent 1.880s
ok launchpad.net/juju-core/agent/mongo 1.290s
ok launchpad.net/juju-core/agent/tools 0.189s
ok launchpad.net/juju-core/bzr 5.146s
ok launchpad.net/juju-core/cert 2.507s
ok launchpad.net/juju-core/charm 0.398s
? launchpad.net/juju-core/charm/hooks [no test files]
? launchpad.net/juju-core/charm/testing [no test files]
ok launchpad.net/juju-core/cloudinit 0.029s
ok launchpad.net/juju-core/cloudinit/sshinit 0.703s
ok launchpad.net/juju-core/cmd 0.179s
ok launchpad.net/juju-core/cmd/charm-admin 0.243s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/envcmd 0.184s
ok launchpad.net/juju-core/cmd/juju 228.904s
ok launchpad.net/juju-core/cmd/jujud 76.448s
ok launchpad.net/juju-core/cmd/plugins/juju-metadata 8.081s
? launchpad.net/juju-core/cmd/plugins/juju-restore [no test files]
ok launchpad.net/juju-core/cmd/plugins/local 0.147s
? launchpad.net/juju-core/cmd/plugins/local/juju-local [no test files]
ok launchpad.net/juju-core/constraints 0.022s
ok launchpad.net/juju-core/container 0.044s
ok launchpad.net/juju-core/container/factory 0.043s
ok launchpad.net/juju-core/container/kvm 0.233s
ok launchpad.net/juju-core/container/kvm/mock 0.045s
? launchpad.net/juju-core/container/kvm/testing [no test files]
ok launchpad.net/juju-core/container/lxc 4.309s
? launchpad.net/juju-core/container/lxc/mock [no test files]
? launchpad.net/juju-core/container/lxc/testing [no test files]
? launchpad.net/juju-core/container/testing [no test files]
ok launchpad.net/juju-core/downloader 5.263s
ok launchpad.net/juju-core/environs 2.093s
ok launchpad.net/juju-core/environs/bootstrap 11.718s
ok launchpad.net/juju-core/environs/cloudinit 0.479s
ok launchpad.net/juju-core/environs/config 2.291s
ok launchpad.net/juju-core/environs/configstore 0.033s
ok launchpad.net/juju-core/environs/filestorage 0.027s
ok launchpad.net/juju-core/environs/httpstorage 0.618s
ok launchpad.net/juju-core/environs/imagemetadata 0.414s
? launchpad.net/juju-core/environs/imagemetadata/testing [no test files]
ok launchpad.net/juju-core/environs/instances 0.040s
ok launchpad.net/juju-core/environs/jujutest 0.172s
ok launchpad.net/juju-core/environs/manual 12.108s
? launchpad.net/juju-core/environs/network [no test files]
ok launchpad.net/juju-core/environs/simplestreams 0.232s
? launchpad.net/juju-core/environs/simplestreams/testing [no test files]
ok launchpad.net/juju-core/environs/sshstorage 0.848s
ok launchpad.net/juju-core/environs/storage 0.766s
ok launchpad.net/juju-core/environs/sync 50.416s
ok launchpad.net/juju-core/environs/testing 0.129s
ok launchpad.net/juju-core/environs/tools 4.550s
? launchpad.net/juju-core/environs/tools/testing [no test files]
ok launchpad.net/juju-core/errors 0.020s
ok launchpad.net/juju-core/instance 0.020s
? launchpad.net/juju-core/insta...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'cmd/juju/bootstrap.go'
2--- cmd/juju/bootstrap.go 2014-04-09 15:48:45 +0000
3+++ cmd/juju/bootstrap.go 2014-04-21 23:25:35 +0000
4@@ -101,6 +101,14 @@
5 if err != nil {
6 return err
7 }
8+ validator := environ.ConstraintsValidator()
9+ unsupported, err := validator.Validate(c.Constraints)
10+ if len(unsupported) > 0 {
11+ logger.Warningf("unsupported constraints: %v", err)
12+ } else if err != nil {
13+ return err
14+ }
15+
16 defer cleanup()
17 if err := bootstrap.EnsureNotBootstrapped(environ); err != nil {
18 return err
19
20=== modified file 'cmd/juju/bootstrap_test.go'
21--- cmd/juju/bootstrap_test.go 2014-04-17 12:57:32 +0000
22+++ cmd/juju/bootstrap_test.go 2014-04-21 23:25:35 +0000
23@@ -295,6 +295,10 @@
24 args: []string{"--constraints", "bad=wrong"},
25 err: `invalid value "bad=wrong" for flag --constraints: unknown constraint "bad"`,
26 }, {
27+ info: "conflicting --constraints",
28+ args: []string{"--constraints", "instance-type=foo mem=4G"},
29+ err: `ambiguous constraints: "mem" overlaps with "instance-type"`,
30+}, {
31 info: "bad --series",
32 args: []string{"--series", "1bad1"},
33 err: `invalid value "1bad1" for flag --series: invalid series name "1bad1"`,
34@@ -312,6 +316,10 @@
35 args: []string{"--constraints", "mem=4G cpu-cores=4"},
36 constraints: constraints.MustParse("mem=4G cpu-cores=4"),
37 }, {
38+ info: "unsupported constraint passed through but no error",
39+ args: []string{"--constraints", "mem=4G cpu-cores=4 cpu-power=10"},
40+ constraints: constraints.MustParse("mem=4G cpu-cores=4 cpu-power=10"),
41+}, {
42 info: "--upload-tools picks all reasonable series",
43 version: "1.2.3-saucy-amd64",
44 args: []string{"--upload-tools"},
45
46=== modified file 'environs/interface.go'
47--- environs/interface.go 2014-04-03 03:37:45 +0000
48+++ environs/interface.go 2014-04-21 23:25:35 +0000
49@@ -113,6 +113,10 @@
50 // EnvironCapability allows access to this environment's capabilities.
51 state.EnvironCapability
52
53+ // ConstraintsValidator returns a Validator instance which
54+ // is used to validate and merge constraints.
55+ ConstraintsValidator() constraints.Validator
56+
57 // SetConfig updates the Environ's configuration.
58 //
59 // Calls to SetConfig do not affect the configuration of
60
61=== modified file 'environs/statepolicy.go'
62--- environs/statepolicy.go 2014-04-03 03:37:45 +0000
63+++ environs/statepolicy.go 2014-04-21 23:25:35 +0000
64@@ -4,6 +4,7 @@
65 package environs
66
67 import (
68+ "launchpad.net/juju-core/constraints"
69 "launchpad.net/juju-core/environs/config"
70 "launchpad.net/juju-core/state"
71 )
72@@ -35,3 +36,11 @@
73 // Environ implements state.EnvironCapability.
74 return New(cfg)
75 }
76+
77+func (environStatePolicy) ConstraintsValidator(cfg *config.Config) (constraints.Validator, error) {
78+ env, err := New(cfg)
79+ if err != nil {
80+ return nil, err
81+ }
82+ return env.ConstraintsValidator(), nil
83+}
84
85=== modified file 'provider/azure/environ.go'
86--- provider/azure/environ.go 2014-04-18 16:37:28 +0000
87+++ provider/azure/environ.go 2014-04-21 23:25:35 +0000
88@@ -422,6 +422,18 @@
89 return spec.InstanceType.Id, spec.Image.Id, nil
90 }
91
92+var unsupportedConstraints = []string{
93+ constraints.CpuPower,
94+ constraints.Tags,
95+}
96+
97+// ConstraintsValidator is defined on the Environs interface.
98+func (environ *azureEnviron) ConstraintsValidator() constraints.Validator {
99+ validator := constraints.NewValidator()
100+ validator.RegisterUnsupported(unsupportedConstraints)
101+ return validator
102+}
103+
104 // createInstance creates all of the Azure entities necessary for a
105 // new instance. This includes Cloud Service, Deployment and Role.
106 //
107
108=== modified file 'provider/azure/environ_test.go'
109--- provider/azure/environ_test.go 2014-04-11 17:51:58 +0000
110+++ provider/azure/environ_test.go 2014-04-21 23:25:35 +0000
111@@ -1518,3 +1518,12 @@
112 _, stateServer = s.startInstance(c)
113 c.Assert(stateServer, jc.IsTrue)
114 }
115+
116+func (s *environSuite) TestConstraintsValidator(c *gc.C) {
117+ env := makeEnviron(c)
118+ validator := env.ConstraintsValidator()
119+ cons := constraints.MustParse("arch=amd64 tags=bar cpu-power=10")
120+ unsupported, err := validator.Validate(cons)
121+ c.Assert(err, gc.IsNil)
122+ c.Assert(unsupported, gc.DeepEquals, []string{"cpu-power", "tags"})
123+}
124
125=== modified file 'provider/dummy/environs.go'
126--- provider/dummy/environs.go 2014-04-18 16:37:28 +0000
127+++ provider/dummy/environs.go 2014-04-21 23:25:35 +0000
128@@ -693,6 +693,14 @@
129 return nil
130 }
131
132+// ConstraintsValidator is defined on the Environs interface.
133+func (e *environ) ConstraintsValidator() constraints.Validator {
134+ validator := constraints.NewValidator()
135+ validator.RegisterUnsupported([]string{constraints.CpuPower})
136+ validator.RegisterConflicts([]string{constraints.InstanceType}, []string{constraints.Mem})
137+ return validator
138+}
139+
140 // StartInstance is specified in the InstanceBroker interface.
141 func (e *environ) StartInstance(args environs.StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, []network.Info, error) {
142
143
144=== modified file 'provider/ec2/ec2.go'
145--- provider/ec2/ec2.go 2014-04-18 16:37:28 +0000
146+++ provider/ec2/ec2.go 2014-04-21 23:25:35 +0000
147@@ -358,6 +358,20 @@
148 return false
149 }
150
151+var unsupportedConstraints = []string{
152+ constraints.Tags,
153+}
154+
155+// ConstraintsValidator is defined on the Environs interface.
156+func (e *environ) ConstraintsValidator() constraints.Validator {
157+ validator := constraints.NewValidator()
158+ validator.RegisterConflicts(
159+ []string{constraints.InstanceType},
160+ []string{constraints.Mem, constraints.CpuCores, constraints.CpuPower})
161+ validator.RegisterUnsupported(unsupportedConstraints)
162+ return validator
163+}
164+
165 // MetadataLookupParams returns parameters which are used to query simplestreams metadata.
166 func (e *environ) MetadataLookupParams(region string) (*simplestreams.MetadataLookupParams, error) {
167 if region == "" {
168
169=== modified file 'provider/ec2/local_test.go'
170--- provider/ec2/local_test.go 2014-04-09 16:36:12 +0000
171+++ provider/ec2/local_test.go 2014-04-21 23:25:35 +0000
172@@ -359,6 +359,25 @@
173 }
174 }
175
176+func (t *localServerSuite) TestConstraintsValidator(c *gc.C) {
177+ env := t.Prepare(c)
178+ validator := env.ConstraintsValidator()
179+ cons := constraints.MustParse("arch=amd64 tags=foo")
180+ unsupported, err := validator.Validate(cons)
181+ c.Assert(err, gc.IsNil)
182+ c.Assert(unsupported, gc.DeepEquals, []string{"tags"})
183+}
184+
185+func (t *localServerSuite) TestConstraintsMerge(c *gc.C) {
186+ env := t.Prepare(c)
187+ validator := env.ConstraintsValidator()
188+ consA := constraints.MustParse("arch=amd64 mem=1G cpu-power=10 cpu-cores=2 tags=bar")
189+ consB := constraints.MustParse("arch=i386 instance-type=foo")
190+ cons, err := validator.Merge(consA, consB)
191+ c.Assert(err, gc.IsNil)
192+ c.Assert(cons, gc.DeepEquals, constraints.MustParse("arch=i386 instance-type=foo tags=bar"))
193+}
194+
195 func (t *localServerSuite) TestValidateImageMetadata(c *gc.C) {
196 env := t.Prepare(c)
197 params, err := env.(simplestreams.MetadataValidator).MetadataLookupParams("test")
198
199=== modified file 'provider/joyent/environ_instance.go'
200--- provider/joyent/environ_instance.go 2014-04-18 16:37:28 +0000
201+++ provider/joyent/environ_instance.go 2014-04-21 23:25:35 +0000
202@@ -12,6 +12,7 @@
203 "github.com/joyent/gocommon/client"
204 "github.com/joyent/gosdc/cloudapi"
205
206+ "launchpad.net/juju-core/constraints"
207 "launchpad.net/juju-core/environs"
208 "launchpad.net/juju-core/environs/imagemetadata"
209 "launchpad.net/juju-core/environs/instances"
210@@ -51,6 +52,18 @@
211 return fmt.Sprintf("juju-%s-%s", env.Name(), names.MachineTag(machineId))
212 }
213
214+var unsupportedConstraints = []string{
215+ constraints.CpuPower,
216+ constraints.Tags,
217+}
218+
219+// ConstraintsValidator is defined on the Environs interface.
220+func (env *joyentEnviron) ConstraintsValidator() constraints.Validator {
221+ validator := constraints.NewValidator()
222+ validator.RegisterUnsupported(unsupportedConstraints)
223+ return validator
224+}
225+
226 func (env *joyentEnviron) StartInstance(args environs.StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, []network.Info, error) {
227
228 if args.MachineConfig.HasNetworks() {
229
230=== modified file 'provider/joyent/local_test.go'
231--- provider/joyent/local_test.go 2014-04-02 11:35:49 +0000
232+++ provider/joyent/local_test.go 2014-04-21 23:25:35 +0000
233@@ -421,3 +421,12 @@
234 _, err = storage.Get(stor, "ab")
235 c.Assert(err, gc.NotNil)
236 }
237+
238+func (s *localServerSuite) TestConstraintsValidator(c *gc.C) {
239+ env := s.Prepare(c)
240+ validator := env.ConstraintsValidator()
241+ cons := constraints.MustParse("arch=amd64 tags=bar cpu-power=10")
242+ unsupported, err := validator.Validate(cons)
243+ c.Assert(err, gc.IsNil)
244+ c.Assert(unsupported, gc.DeepEquals, []string{"cpu-power", "tags"})
245+}
246
247=== modified file 'provider/local/environ.go'
248--- provider/local/environ.go 2014-04-18 16:37:28 +0000
249+++ provider/local/environ.go 2014-04-21 23:25:35 +0000
250@@ -301,6 +301,20 @@
251 return nil
252 }
253
254+var unsupportedConstraints = []string{
255+ constraints.CpuCores,
256+ constraints.CpuPower,
257+ constraints.InstanceType,
258+ constraints.Tags,
259+}
260+
261+// ConstraintsValidator is defined on the Environs interface.
262+func (env *localEnviron) ConstraintsValidator() constraints.Validator {
263+ validator := constraints.NewValidator()
264+ validator.RegisterUnsupported(unsupportedConstraints)
265+ return validator
266+}
267+
268 // StartInstance is specified in the InstanceBroker interface.
269 func (env *localEnviron) StartInstance(args environs.StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, []network.Info, error) {
270 if args.MachineConfig.HasNetworks() {
271
272=== modified file 'provider/local/environ_test.go'
273--- provider/local/environ_test.go 2014-04-08 16:27:27 +0000
274+++ provider/local/environ_test.go 2014-04-21 23:25:35 +0000
275@@ -308,3 +308,14 @@
276 c.Assert(cloudInitOutputLog, jc.DoesNotExist)
277 c.Assert(filepath.Join(rootDir, "log"), jc.IsSymlink)
278 }
279+
280+func (s *localJujuTestSuite) TestConstraintsValidator(c *gc.C) {
281+ ctx := coretesting.Context(c)
282+ env, err := local.Provider.Prepare(ctx, minimalConfig(c))
283+ c.Assert(err, gc.IsNil)
284+ validator := env.ConstraintsValidator()
285+ cons := constraints.MustParse("arch=amd64 instance-type=foo tags=bar cpu-power=10 cpu-cores=2")
286+ unsupported, err := validator.Validate(cons)
287+ c.Assert(err, gc.IsNil)
288+ c.Assert(unsupported, gc.DeepEquals, []string{"cpu-cores", "cpu-power", "instance-type", "tags"})
289+}
290
291=== modified file 'provider/maas/environ.go'
292--- provider/maas/environ.go 2014-04-18 16:37:28 +0000
293+++ provider/maas/environ.go 2014-04-21 23:25:35 +0000
294@@ -332,6 +332,18 @@
295 return `sed -i "s/iface eth0 inet dhcp/source \/etc\/network\/eth0.config/" /etc/network/interfaces`
296 }
297
298+var unsupportedConstraints = []string{
299+ constraints.CpuPower,
300+ constraints.InstanceType,
301+}
302+
303+// ConstraintsValidator is defined on the Environs interface.
304+func (environ *maasEnviron) ConstraintsValidator() constraints.Validator {
305+ validator := constraints.NewValidator()
306+ validator.RegisterUnsupported(unsupportedConstraints)
307+ return validator
308+}
309+
310 // setupNetworks prepares a []network.Info for the given instance.
311 func (environ *maasEnviron) setupNetworks(inst instance.Instance) ([]network.Info, error) {
312 // Get the instance network interfaces first.
313
314=== modified file 'provider/maas/environ_test.go'
315--- provider/maas/environ_test.go 2014-04-18 16:37:28 +0000
316+++ provider/maas/environ_test.go 2014-04-21 23:25:35 +0000
317@@ -10,6 +10,7 @@
318 gc "launchpad.net/gocheck"
319 "launchpad.net/gomaasapi"
320
321+ "launchpad.net/juju-core/constraints"
322 "launchpad.net/juju-core/environs/config"
323 "launchpad.net/juju-core/environs/network"
324 envtesting "launchpad.net/juju-core/environs/testing"
325@@ -243,3 +244,14 @@
326 "ifup eth3.123",
327 })
328 }
329+
330+func (s *environSuite) TestConstraintsValidator(c *gc.C) {
331+ cfg := getSimpleTestConfig(c, nil)
332+ env, err := maas.NewEnviron(cfg)
333+ c.Assert(err, gc.IsNil)
334+ validator := env.ConstraintsValidator()
335+ cons := constraints.MustParse("arch=amd64 cpu-power=10 instance-type=foo")
336+ unsupported, err := validator.Validate(cons)
337+ c.Assert(err, gc.IsNil)
338+ c.Assert(unsupported, gc.DeepEquals, []string{"cpu-power", "instance-type"})
339+}
340
341=== modified file 'provider/manual/environ.go'
342--- provider/manual/environ.go 2014-04-18 16:37:28 +0000
343+++ provider/manual/environ.go 2014-04-21 23:25:35 +0000
344@@ -266,6 +266,19 @@
345 return errors.New(`use "juju add-machine ssh:[user@]<host>" to provision machines`)
346 }
347
348+var unsupportedConstraints = []string{
349+ constraints.CpuPower,
350+ constraints.InstanceType,
351+ constraints.Tags,
352+}
353+
354+// ConstraintsValidator is defined on the Environs interface.
355+func (e *manualEnviron) ConstraintsValidator() constraints.Validator {
356+ validator := constraints.NewValidator()
357+ validator.RegisterUnsupported(unsupportedConstraints)
358+ return validator
359+}
360+
361 func (e *manualEnviron) OpenPorts(ports []instance.Port) error {
362 return nil
363 }
364
365=== modified file 'provider/manual/environ_test.go'
366--- provider/manual/environ_test.go 2014-04-10 21:33:04 +0000
367+++ provider/manual/environ_test.go 2014-04-21 23:25:35 +0000
368@@ -10,6 +10,7 @@
369 jc "github.com/juju/testing/checkers"
370 gc "launchpad.net/gocheck"
371
372+ "launchpad.net/juju-core/constraints"
373 "launchpad.net/juju-core/environs"
374 "launchpad.net/juju-core/environs/manual"
375 "launchpad.net/juju-core/environs/storage"
376@@ -150,3 +151,11 @@
377 func (s *environSuite) TestSupportNetworks(c *gc.C) {
378 c.Assert(s.env.SupportNetworks(), jc.IsFalse)
379 }
380+
381+func (s *environSuite) TestConstraintsValidator(c *gc.C) {
382+ validator := s.env.ConstraintsValidator()
383+ cons := constraints.MustParse("arch=amd64 instance-type=foo tags=bar cpu-power=10 cpu-cores=2 mem=1G")
384+ unsupported, err := validator.Validate(cons)
385+ c.Assert(err, gc.IsNil)
386+ c.Assert(unsupported, gc.DeepEquals, []string{"cpu-power", "instance-type", "tags"})
387+}
388
389=== modified file 'provider/openstack/local_test.go'
390--- provider/openstack/local_test.go 2014-04-17 13:21:44 +0000
391+++ provider/openstack/local_test.go 2014-04-21 23:25:35 +0000
392@@ -675,6 +675,25 @@
393 c.Assert(err, gc.ErrorMatches, `no "saucy" images in some-region with arches \[amd64\]`)
394 }
395
396+func (s *localServerSuite) TestConstraintsValidator(c *gc.C) {
397+ env := s.Open(c)
398+ validator := env.ConstraintsValidator()
399+ cons := constraints.MustParse("arch=amd64 cpu-power=10")
400+ unsupported, err := validator.Validate(cons)
401+ c.Assert(err, gc.IsNil)
402+ c.Assert(unsupported, gc.DeepEquals, []string{"cpu-power"})
403+}
404+
405+func (s *localServerSuite) TestConstraintsMerge(c *gc.C) {
406+ env := s.Open(c)
407+ validator := env.ConstraintsValidator()
408+ consA := constraints.MustParse("arch=amd64 mem=1G root-disk=10G")
409+ consB := constraints.MustParse("instance-type=foo")
410+ cons, err := validator.Merge(consA, consB)
411+ c.Assert(err, gc.IsNil)
412+ c.Assert(cons, gc.DeepEquals, constraints.MustParse("instance-type=foo"))
413+}
414+
415 func (s *localServerSuite) TestValidateImageMetadata(c *gc.C) {
416 env := s.Open(c)
417 params, err := env.(simplestreams.MetadataValidator).MetadataLookupParams("some-region")
418
419=== modified file 'provider/openstack/provider.go'
420--- provider/openstack/provider.go 2014-04-18 16:37:28 +0000
421+++ provider/openstack/provider.go 2014-04-21 23:25:35 +0000
422@@ -522,6 +522,21 @@
423 return false
424 }
425
426+var unsupportedConstraints = []string{
427+ constraints.Tags,
428+ constraints.CpuPower,
429+}
430+
431+// ConstraintsValidator is defined on the Environs interface.
432+func (e *environ) ConstraintsValidator() constraints.Validator {
433+ validator := constraints.NewValidator()
434+ validator.RegisterConflicts(
435+ []string{constraints.InstanceType},
436+ []string{constraints.Mem, constraints.Arch, constraints.RootDisk, constraints.CpuCores})
437+ validator.RegisterUnsupported(unsupportedConstraints)
438+ return validator
439+}
440+
441 func (e *environ) Storage() storage.Storage {
442 e.ecfgMutex.Lock()
443 stor := e.storageUnlocked
444
445=== modified file 'state/addmachine.go'
446--- state/addmachine.go 2014-04-21 22:57:03 +0000
447+++ state/addmachine.go 2014-04-21 23:25:35 +0000
448@@ -183,18 +183,22 @@
449 // valid and combines it with values from the state
450 // to produce a resulting template that more accurately
451 // represents the data that will be inserted into the state.
452-func (st *State) effectiveMachineTemplate(p MachineTemplate, allowStateServer bool) (MachineTemplate, error) {
453+func (st *State) effectiveMachineTemplate(p MachineTemplate, allowStateServer bool) (tmpl MachineTemplate, err error) {
454+ // First check for obvious errors.
455 if p.Series == "" {
456- return MachineTemplate{}, fmt.Errorf("no series specified")
457- }
458- cons, err := st.EnvironConstraints()
459- if err != nil {
460- return MachineTemplate{}, err
461- }
462- validator := constraints.NewValidator()
463- p.Constraints, err = validator.Merge(cons, p.Constraints)
464- if err != nil {
465- return MachineTemplate{}, err
466+ return tmpl, fmt.Errorf("no series specified")
467+ }
468+ if p.InstanceId != "" {
469+ if p.Nonce == "" {
470+ return tmpl, fmt.Errorf("cannot add a machine with an instance id and no nonce")
471+ }
472+ } else if p.Nonce != "" {
473+ return tmpl, fmt.Errorf("cannot specify a nonce without an instance id")
474+ }
475+
476+ p.Constraints, err = st.resolveConstraints(p.Constraints)
477+ if err != nil {
478+ return tmpl, err
479 }
480 // Machine constraints do not use a container constraint value.
481 // Both provisioning and deployment constraints use the same
482@@ -204,7 +208,7 @@
483 p.Constraints.Container = nil
484
485 if len(p.Jobs) == 0 {
486- return MachineTemplate{}, fmt.Errorf("no jobs specified")
487+ return tmpl, fmt.Errorf("no jobs specified")
488 }
489 jset := make(map[MachineJob]bool)
490 for _, j := range p.Jobs {
491@@ -215,16 +219,8 @@
492 }
493 if jset[JobManageEnviron] {
494 if !allowStateServer {
495- return MachineTemplate{}, errStateServerNotAllowed
496- }
497- }
498-
499- if p.InstanceId != "" {
500- if p.Nonce == "" {
501- return MachineTemplate{}, fmt.Errorf("cannot add a machine with an instance id and no nonce")
502- }
503- } else if p.Nonce != "" {
504- return MachineTemplate{}, fmt.Errorf("cannot specify a nonce without an instance id")
505+ return tmpl, errStateServerNotAllowed
506+ }
507 }
508 return p, nil
509 }
510
511=== modified file 'state/conn_test.go'
512--- state/conn_test.go 2014-04-14 12:36:13 +0000
513+++ state/conn_test.go 2014-04-21 23:25:35 +0000
514@@ -9,6 +9,7 @@
515 "labix.org/v2/mgo"
516 gc "launchpad.net/gocheck"
517
518+ "launchpad.net/juju-core/constraints"
519 "launchpad.net/juju-core/environs/config"
520 "launchpad.net/juju-core/errors"
521 "launchpad.net/juju-core/state"
522@@ -101,9 +102,10 @@
523 }
524
525 type mockPolicy struct {
526- getPrechecker func(*config.Config) (state.Prechecker, error)
527- getConfigValidator func(string) (state.ConfigValidator, error)
528- getEnvironCapability func(*config.Config) (state.EnvironCapability, error)
529+ getPrechecker func(*config.Config) (state.Prechecker, error)
530+ getConfigValidator func(string) (state.ConfigValidator, error)
531+ getEnvironCapability func(*config.Config) (state.EnvironCapability, error)
532+ getConstraintsValidator func(*config.Config) (constraints.Validator, error)
533 }
534
535 func (p *mockPolicy) Prechecker(cfg *config.Config) (state.Prechecker, error) {
536@@ -126,3 +128,10 @@
537 }
538 return nil, errors.NotImplementedf("EnvironCapability")
539 }
540+
541+func (p *mockPolicy) ConstraintsValidator(cfg *config.Config) (constraints.Validator, error) {
542+ if p.getConstraintsValidator != nil {
543+ return p.getConstraintsValidator(cfg)
544+ }
545+ return nil, errors.NewNotImplemented(nil, "ConstraintsValidator")
546+}
547
548=== modified file 'state/constraints.go'
549--- state/constraints.go 2014-04-08 05:09:40 +0000
550+++ state/constraints.go 2014-04-21 23:25:35 +0000
551@@ -17,36 +17,39 @@
552
553 // constraintsDoc is the mongodb representation of a constraints.Value.
554 type constraintsDoc struct {
555- Arch *string
556- CpuCores *uint64
557- CpuPower *uint64
558- Mem *uint64
559- RootDisk *uint64
560- Container *instance.ContainerType
561- Tags *[]string `bson:",omitempty"`
562+ Arch *string
563+ CpuCores *uint64
564+ CpuPower *uint64
565+ Mem *uint64
566+ RootDisk *uint64
567+ InstanceType *string
568+ Container *instance.ContainerType
569+ Tags *[]string `bson:",omitempty"`
570 }
571
572 func (doc constraintsDoc) value() constraints.Value {
573 return constraints.Value{
574- Arch: doc.Arch,
575- CpuCores: doc.CpuCores,
576- CpuPower: doc.CpuPower,
577- Mem: doc.Mem,
578- RootDisk: doc.RootDisk,
579- Container: doc.Container,
580- Tags: doc.Tags,
581+ Arch: doc.Arch,
582+ CpuCores: doc.CpuCores,
583+ CpuPower: doc.CpuPower,
584+ Mem: doc.Mem,
585+ RootDisk: doc.RootDisk,
586+ Container: doc.Container,
587+ Tags: doc.Tags,
588+ InstanceType: doc.InstanceType,
589 }
590 }
591
592 func newConstraintsDoc(cons constraints.Value) constraintsDoc {
593 return constraintsDoc{
594- Arch: cons.Arch,
595- CpuCores: cons.CpuCores,
596- CpuPower: cons.CpuPower,
597- Mem: cons.Mem,
598- RootDisk: cons.RootDisk,
599- Container: cons.Container,
600- Tags: cons.Tags,
601+ Arch: cons.Arch,
602+ CpuCores: cons.CpuCores,
603+ CpuPower: cons.CpuPower,
604+ Mem: cons.Mem,
605+ RootDisk: cons.RootDisk,
606+ Container: cons.Container,
607+ Tags: cons.Tags,
608+ InstanceType: cons.InstanceType,
609 }
610 }
611
612
613=== added file 'state/constraintsvalidation_test.go'
614--- state/constraintsvalidation_test.go 1970-01-01 00:00:00 +0000
615+++ state/constraintsvalidation_test.go 2014-04-21 23:25:35 +0000
616@@ -0,0 +1,104 @@
617+// Copyright 2014 Canonical Ltd.
618+// Licensed under the AGPLv3, see LICENCE file for details.
619+
620+package state_test
621+
622+import (
623+ gc "launchpad.net/gocheck"
624+
625+ "launchpad.net/juju-core/constraints"
626+ "launchpad.net/juju-core/environs/config"
627+ "launchpad.net/juju-core/state"
628+)
629+
630+type constraintsValidationSuite struct {
631+ ConnSuite
632+}
633+
634+var _ = gc.Suite(&constraintsValidationSuite{})
635+
636+func (s *constraintsValidationSuite) SetUpTest(c *gc.C) {
637+ s.ConnSuite.SetUpTest(c)
638+ s.policy.getConstraintsValidator = func(*config.Config) (constraints.Validator, error) {
639+ validator := constraints.NewValidator()
640+ validator.RegisterConflicts([]string{constraints.InstanceType}, []string{constraints.Mem, constraints.Arch})
641+ validator.RegisterUnsupported([]string{constraints.CpuPower})
642+ return validator, nil
643+ }
644+}
645+
646+func (s *constraintsValidationSuite) addOneMachine(c *gc.C, cons constraints.Value) (*state.Machine, error) {
647+ return s.State.AddOneMachine(state.MachineTemplate{
648+ Series: "quantal",
649+ Jobs: []state.MachineJob{state.JobHostUnits},
650+ Constraints: cons,
651+ })
652+}
653+
654+var setConstraintsTests = []struct {
655+ consFallback string
656+ cons string
657+ expected string
658+}{
659+ {
660+ cons: "root-disk=8G mem=4G arch=amd64",
661+ consFallback: "cpu-power=1000 cpu-cores=4",
662+ expected: "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cpu-cores=4",
663+ }, {
664+ cons: "root-disk=8G mem=4G arch=amd64",
665+ consFallback: "cpu-power=1000 cpu-cores=4 mem=8G",
666+ expected: "root-disk=8G mem=4G arch=amd64 cpu-power=1000 cpu-cores=4",
667+ }, {
668+ consFallback: "root-disk=8G cpu-cores=4 instance-type=foo",
669+ expected: "root-disk=8G cpu-cores=4 instance-type=foo",
670+ }, {
671+ cons: "root-disk=8G cpu-cores=4 instance-type=foo",
672+ expected: "root-disk=8G cpu-cores=4 instance-type=foo",
673+ }, {
674+ consFallback: "root-disk=8G instance-type=foo",
675+ cons: "root-disk=8G cpu-cores=4 instance-type=bar",
676+ expected: "root-disk=8G cpu-cores=4 instance-type=bar",
677+ }, {
678+ consFallback: "root-disk=8G mem=4G",
679+ cons: "root-disk=8G cpu-cores=4 instance-type=bar",
680+ expected: "root-disk=8G cpu-cores=4 instance-type=bar",
681+ }, {
682+ consFallback: "root-disk=8G cpu-cores=4 instance-type=bar",
683+ cons: "root-disk=8G mem=4G",
684+ expected: "root-disk=8G cpu-cores=4 mem=4G",
685+ }, {
686+ consFallback: "root-disk=8G cpu-cores=4 instance-type=bar",
687+ cons: "root-disk=8G arch=amd64 mem=4G",
688+ expected: "root-disk=8G cpu-cores=4 arch=amd64 mem=4G",
689+ },
690+}
691+
692+func (s *constraintsValidationSuite) TestMachineConstraints(c *gc.C) {
693+ for i, t := range setConstraintsTests {
694+ c.Logf("test %d", i)
695+ err := s.State.SetEnvironConstraints(constraints.MustParse(t.consFallback))
696+ c.Check(err, gc.IsNil)
697+ m, err := s.addOneMachine(c, constraints.MustParse(t.cons))
698+ c.Check(err, gc.IsNil)
699+ cons, err := m.Constraints()
700+ c.Check(err, gc.IsNil)
701+ c.Check(cons, gc.DeepEquals, constraints.MustParse(t.expected))
702+ }
703+}
704+
705+func (s *constraintsValidationSuite) TestServiceConstraints(c *gc.C) {
706+ charm := s.AddTestingCharm(c, "wordpress")
707+ service := s.AddTestingService(c, "wordpress", charm)
708+ for i, t := range setConstraintsTests {
709+ c.Logf("test %d", i)
710+ err := s.State.SetEnvironConstraints(constraints.MustParse(t.consFallback))
711+ c.Check(err, gc.IsNil)
712+ err = service.SetConstraints(constraints.MustParse(t.cons))
713+ c.Check(err, gc.IsNil)
714+ u, err := service.AddUnit()
715+ c.Check(err, gc.IsNil)
716+ cons, err := state.UnitConstraints(u)
717+ c.Check(err, gc.IsNil)
718+ c.Check(*cons, gc.DeepEquals, constraints.MustParse(t.expected))
719+ }
720+}
721
722=== modified file 'state/export_test.go'
723--- state/export_test.go 2014-04-18 11:59:59 +0000
724+++ state/export_test.go 2014-04-21 23:25:35 +0000
725@@ -15,6 +15,7 @@
726 gc "launchpad.net/gocheck"
727
728 "launchpad.net/juju-core/charm"
729+ "launchpad.net/juju-core/constraints"
730 "launchpad.net/juju-core/environs/config"
731 "launchpad.net/juju-core/instance"
732 "launchpad.net/juju-core/testing"
733@@ -261,3 +262,7 @@
734 }
735
736 var StateServerAvailable = &stateServerAvailable
737+
738+func UnitConstraints(u *Unit) (*constraints.Value, error) {
739+ return u.constraints()
740+}
741
742=== modified file 'state/machine.go'
743--- state/machine.go 2014-04-18 16:46:35 +0000
744+++ state/machine.go 2014-04-21 23:25:35 +0000
745@@ -1101,6 +1101,13 @@
746 // is already provisioned.
747 func (m *Machine) SetConstraints(cons constraints.Value) (err error) {
748 defer errors.Maskf(&err, "cannot set constraints")
749+ unsupported, err := m.st.validateConstraints(cons)
750+ if len(unsupported) > 0 {
751+ logger.Warningf(
752+ "setting constraints on machine %q: unsupported constraints: %v", m.Id(), strings.Join(unsupported, ","))
753+ } else if err != nil {
754+ return err
755+ }
756 notSetYet := bson.D{{"nonce", ""}}
757 ops := []txn.Op{
758 {
759
760=== modified file 'state/machine_test.go'
761--- state/machine_test.go 2014-04-18 16:37:28 +0000
762+++ state/machine_test.go 2014-04-21 23:25:35 +0000
763@@ -7,11 +7,13 @@
764 "sort"
765 "strings"
766
767+ "github.com/juju/loggo"
768 jc "github.com/juju/testing/checkers"
769 "labix.org/v2/mgo/bson"
770 gc "launchpad.net/gocheck"
771
772 "launchpad.net/juju-core/constraints"
773+ "launchpad.net/juju-core/environs/config"
774 "launchpad.net/juju-core/environs/network"
775 "launchpad.net/juju-core/errors"
776 "launchpad.net/juju-core/instance"
777@@ -32,6 +34,12 @@
778
779 func (s *MachineSuite) SetUpTest(c *gc.C) {
780 s.ConnSuite.SetUpTest(c)
781+ s.policy.getConstraintsValidator = func(*config.Config) (constraints.Validator, error) {
782+ validator := constraints.NewValidator()
783+ validator.RegisterConflicts([]string{constraints.InstanceType}, []string{constraints.Mem})
784+ validator.RegisterUnsupported([]string{constraints.CpuPower})
785+ return validator, nil
786+ }
787 var err error
788 s.machine0, err = s.State.AddMachine("quantal", state.JobManageEnviron)
789 s.machine, err = s.State.AddMachine("quantal", state.JobHostUnits)
790@@ -1252,6 +1260,35 @@
791 c.Assert(mcons, gc.DeepEquals, cons1)
792 }
793
794+func (s *MachineSuite) TestSetAmbiguousConstraints(c *gc.C) {
795+ machine, err := s.State.AddMachine("quantal", state.JobHostUnits)
796+ c.Assert(err, gc.IsNil)
797+ cons := constraints.MustParse("mem=4G instance-type=foo")
798+ err = machine.SetConstraints(cons)
799+ c.Assert(err, gc.ErrorMatches, `cannot set constraints: ambiguous constraints: "mem" overlaps with "instance-type"`)
800+}
801+
802+func (s *MachineSuite) TestSetUnsupportedConstraintsWarning(c *gc.C) {
803+ defer loggo.ResetWriters()
804+ logger := loggo.GetLogger("test")
805+ logger.SetLogLevel(loggo.DEBUG)
806+ tw := &loggo.TestWriter{}
807+ c.Assert(loggo.RegisterWriter("constraints-tester", tw, loggo.DEBUG), gc.IsNil)
808+
809+ machine, err := s.State.AddMachine("quantal", state.JobHostUnits)
810+ c.Assert(err, gc.IsNil)
811+ cons := constraints.MustParse("mem=4G cpu-power=10")
812+ err = machine.SetConstraints(cons)
813+ c.Assert(err, gc.IsNil)
814+ c.Assert(tw.Log, jc.LogMatches, jc.SimpleMessages{{
815+ loggo.WARNING,
816+ `setting constraints on machine "2": unsupported constraints: cpu-power`},
817+ })
818+ mcons, err := machine.Constraints()
819+ c.Assert(err, gc.IsNil)
820+ c.Assert(mcons, gc.DeepEquals, cons)
821+}
822+
823 func (s *MachineSuite) TestConstraintsLifecycle(c *gc.C) {
824 cons := constraints.MustParse("mem=1G")
825 cannotSet := `cannot set constraints: not found or not alive`
826
827=== modified file 'state/policy.go'
828--- state/policy.go 2014-04-14 12:36:13 +0000
829+++ state/policy.go 2014-04-21 23:25:35 +0000
830@@ -32,6 +32,10 @@
831 // EnvironCapability takes a *config.Config and returns
832 // a (possibly nil) EnvironCapability or an error.
833 EnvironCapability(*config.Config) (EnvironCapability, error)
834+
835+ // ConstraintsValidator takes a *config.Config and returns
836+ // a (possibly nil) constraints.Validator or an error.
837+ ConstraintsValidator(*config.Config) (constraints.Validator, error)
838 }
839
840 // Prechecker is a policy interface that is provided to State
841@@ -94,6 +98,53 @@
842 return prechecker.PrecheckInstance(series, cons)
843 }
844
845+func (st *State) constraintsValidator() (constraints.Validator, error) {
846+ // Default behaviour is to simply use a standard validator with
847+ // no environment specific behaviour built in.
848+ defaultValidator := constraints.NewValidator()
849+ if st.policy == nil {
850+ return defaultValidator, nil
851+ }
852+ cfg, err := st.EnvironConfig()
853+ if err != nil {
854+ return nil, err
855+ }
856+ validator, err := st.policy.ConstraintsValidator(cfg)
857+ if errors.IsNotImplemented(err) {
858+ return defaultValidator, nil
859+ } else if err != nil {
860+ return nil, err
861+ }
862+ if validator == nil {
863+ return nil, fmt.Errorf("policy returned nil constraints validator without an error")
864+ }
865+ return validator, nil
866+}
867+
868+// resolveConstraints combines the given constraints with the environ constraints to get
869+// a constraints which will be used to create a new instance.
870+func (st *State) resolveConstraints(cons constraints.Value) (constraints.Value, error) {
871+ validator, err := st.constraintsValidator()
872+ if err != nil {
873+ return constraints.Value{}, err
874+ }
875+ envCons, err := st.EnvironConstraints()
876+ if err != nil {
877+ return constraints.Value{}, err
878+ }
879+ return validator.Merge(envCons, cons)
880+}
881+
882+// validateConstraints returns an error if the given constraints are not valid for the
883+// current environment, and also any unsupported attributes.
884+func (st *State) validateConstraints(cons constraints.Value) ([]string, error) {
885+ validator, err := st.constraintsValidator()
886+ if err != nil {
887+ return nil, err
888+ }
889+ return validator.Validate(cons)
890+}
891+
892 // validate calls the state's assigned policy, if non-nil, to obtain
893 // a ConfigValidator, and calls Validate if a non-nil ConfigValidator is
894 // returned.
895
896=== modified file 'state/service.go'
897--- state/service.go 2014-04-21 22:57:03 +0000
898+++ state/service.go 2014-04-21 23:25:35 +0000
899@@ -8,6 +8,7 @@
900 "fmt"
901 "sort"
902 "strconv"
903+ "strings"
904
905 "labix.org/v2/mgo"
906 "labix.org/v2/mgo/bson"
907@@ -594,12 +595,7 @@
908 if err != nil {
909 return "", nil, err
910 }
911- econs, err := s.st.EnvironConstraints()
912- if err != nil {
913- return "", nil, err
914- }
915- validator := constraints.NewValidator()
916- cons, err := validator.Merge(econs, scons)
917+ cons, err := s.st.resolveConstraints(scons)
918 if err != nil {
919 return "", nil, err
920 }
921@@ -807,6 +803,13 @@
922
923 // SetConstraints replaces the current service constraints.
924 func (s *Service) SetConstraints(cons constraints.Value) (err error) {
925+ unsupported, err := s.st.validateConstraints(cons)
926+ if len(unsupported) > 0 {
927+ logger.Warningf(
928+ "setting constraints on service %q: unsupported constraints: %v", s.Name(), strings.Join(unsupported, ","))
929+ } else if err != nil {
930+ return err
931+ }
932 if s.doc.Subordinate {
933 return ErrSubordinateConstraints
934 }
935
936=== modified file 'state/service_test.go'
937--- state/service_test.go 2014-04-14 12:36:13 +0000
938+++ state/service_test.go 2014-04-21 23:25:35 +0000
939@@ -7,12 +7,14 @@
940 "fmt"
941 "sort"
942
943+ "github.com/juju/loggo"
944 jc "github.com/juju/testing/checkers"
945 "labix.org/v2/mgo"
946 gc "launchpad.net/gocheck"
947
948 "launchpad.net/juju-core/charm"
949 "launchpad.net/juju-core/constraints"
950+ "launchpad.net/juju-core/environs/config"
951 "launchpad.net/juju-core/errors"
952 "launchpad.net/juju-core/state"
953 "launchpad.net/juju-core/state/testing"
954@@ -28,6 +30,12 @@
955
956 func (s *ServiceSuite) SetUpTest(c *gc.C) {
957 s.ConnSuite.SetUpTest(c)
958+ s.policy.getConstraintsValidator = func(*config.Config) (constraints.Validator, error) {
959+ validator := constraints.NewValidator()
960+ validator.RegisterConflicts([]string{constraints.InstanceType}, []string{constraints.Mem})
961+ validator.RegisterUnsupported([]string{constraints.CpuPower})
962+ return validator, nil
963+ }
964 s.charm = s.AddTestingCharm(c, "mysql")
965 s.mysql = s.AddTestingService(c, "mysql", s.charm)
966 }
967@@ -1153,6 +1161,31 @@
968 c.Assert(&cons6, jc.Satisfies, constraints.IsEmpty)
969 }
970
971+func (s *ServiceSuite) TestSetInvalidConstraints(c *gc.C) {
972+ cons := constraints.MustParse("mem=4G instance-type=foo")
973+ err := s.mysql.SetConstraints(cons)
974+ c.Assert(err, gc.ErrorMatches, `ambiguous constraints: "mem" overlaps with "instance-type"`)
975+}
976+
977+func (s *ServiceSuite) TestSetUnsupportedConstraintsWarning(c *gc.C) {
978+ defer loggo.ResetWriters()
979+ logger := loggo.GetLogger("test")
980+ logger.SetLogLevel(loggo.DEBUG)
981+ tw := &loggo.TestWriter{}
982+ c.Assert(loggo.RegisterWriter("constraints-tester", tw, loggo.DEBUG), gc.IsNil)
983+
984+ cons := constraints.MustParse("mem=4G cpu-power=10")
985+ err := s.mysql.SetConstraints(cons)
986+ c.Assert(err, gc.IsNil)
987+ c.Assert(tw.Log, jc.LogMatches, jc.SimpleMessages{{
988+ loggo.WARNING,
989+ `setting constraints on service "mysql": unsupported constraints: cpu-power`},
990+ })
991+ scons, err := s.mysql.Constraints()
992+ c.Assert(err, gc.IsNil)
993+ c.Assert(scons, gc.DeepEquals, cons)
994+}
995+
996 func (s *ServiceSuite) TestConstraintsLifecycle(c *gc.C) {
997 // Dying.
998 unit, err := s.mysql.AddUnit()
999
1000=== modified file 'state/state.go'
1001--- state/state.go 2014-04-18 15:31:56 +0000
1002+++ state/state.go 2014-04-21 23:25:35 +0000
1003@@ -333,6 +333,13 @@
1004
1005 // SetEnvironConstraints replaces the current environment constraints.
1006 func (st *State) SetEnvironConstraints(cons constraints.Value) error {
1007+ unsupported, err := st.validateConstraints(cons)
1008+ if len(unsupported) > 0 {
1009+ logger.Warningf(
1010+ "setting environment constraints: unsupported constraints: %v", strings.Join(unsupported, ","))
1011+ } else if err != nil {
1012+ return err
1013+ }
1014 return writeConstraints(st, environGlobalKey, cons)
1015 }
1016
1017
1018=== modified file 'state/state_test.go'
1019--- state/state_test.go 2014-04-18 15:31:56 +0000
1020+++ state/state_test.go 2014-04-21 23:25:35 +0000
1021@@ -10,6 +10,7 @@
1022 "strings"
1023 "time"
1024
1025+ "github.com/juju/loggo"
1026 jc "github.com/juju/testing/checkers"
1027 "labix.org/v2/mgo"
1028 "labix.org/v2/mgo/bson"
1029@@ -49,6 +50,16 @@
1030
1031 var _ = gc.Suite(&StateSuite{})
1032
1033+func (s *StateSuite) SetUpTest(c *gc.C) {
1034+ s.ConnSuite.SetUpTest(c)
1035+ s.policy.getConstraintsValidator = func(*config.Config) (constraints.Validator, error) {
1036+ validator := constraints.NewValidator()
1037+ validator.RegisterConflicts([]string{constraints.InstanceType}, []string{constraints.Mem})
1038+ validator.RegisterUnsupported([]string{constraints.CpuPower})
1039+ return validator, nil
1040+ }
1041+}
1042+
1043 func (s *StateSuite) TestDialAgain(c *gc.C) {
1044 // Ensure idempotent operations on Dial are working fine.
1045 for i := 0; i < 2; i++ {
1046@@ -756,6 +767,7 @@
1047 Series: "quantal",
1048 Jobs: []state.MachineJob{state.JobHostUnits, state.JobHostUnits},
1049 InstanceId: "i-foo",
1050+ Nonce: "nonce",
1051 })
1052 c.Check(err, gc.ErrorMatches, fmt.Sprintf("cannot add a new machine: duplicate job: %s", state.JobHostUnits))
1053
1054@@ -1349,6 +1361,31 @@
1055 c.Assert(cons5, gc.DeepEquals, cons4)
1056 }
1057
1058+func (s *StateSuite) TestSetInvalidConstraints(c *gc.C) {
1059+ cons := constraints.MustParse("mem=4G instance-type=foo")
1060+ err := s.State.SetEnvironConstraints(cons)
1061+ c.Assert(err, gc.ErrorMatches, `ambiguous constraints: "mem" overlaps with "instance-type"`)
1062+}
1063+
1064+func (s *StateSuite) TestSetUnsupportedConstraintsWarning(c *gc.C) {
1065+ defer loggo.ResetWriters()
1066+ logger := loggo.GetLogger("test")
1067+ logger.SetLogLevel(loggo.DEBUG)
1068+ tw := &loggo.TestWriter{}
1069+ c.Assert(loggo.RegisterWriter("constraints-tester", tw, loggo.DEBUG), gc.IsNil)
1070+
1071+ cons := constraints.MustParse("mem=4G cpu-power=10")
1072+ err := s.State.SetEnvironConstraints(cons)
1073+ c.Assert(err, gc.IsNil)
1074+ c.Assert(tw.Log, jc.LogMatches, jc.SimpleMessages{{
1075+ loggo.WARNING,
1076+ `setting environment constraints: unsupported constraints: cpu-power`},
1077+ })
1078+ econs, err := s.State.EnvironConstraints()
1079+ c.Assert(err, gc.IsNil)
1080+ c.Assert(econs, gc.DeepEquals, cons)
1081+}
1082+
1083 func (s *StateSuite) TestWatchServicesBulkEvents(c *gc.C) {
1084 // Alive service...
1085 dummyCharm := s.AddTestingCharm(c, "dummy")

Subscribers

People subscribed via source and target branches

to status/vote changes: