Merge lp:~axwalk/juju-core/wire-up-prechecker-take2 into lp:~go-bot/juju-core/trunk
- wire-up-prechecker-take2
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Andrew Wilkins |
Approved revision: | no longer in the source branch. |
Merged at revision: | 2344 |
Proposed branch: | lp:~axwalk/juju-core/wire-up-prechecker-take2 |
Merge into: | lp:~go-bot/juju-core/trunk |
Diff against target: |
1450 lines (+380/-224) 41 files modified
agent/agent.go (+4/-4) agent/bootstrap.go (+3/-3) agent/bootstrap_test.go (+6/-5) cmd/jujud/agent.go (+2/-1) cmd/jujud/agent_test.go (+3/-2) cmd/jujud/bootstrap.go (+2/-1) cmd/jujud/bootstrap_test.go (+8/-7) cmd/plugins/juju-restore/restore.go (+1/-1) environs/errors.go (+0/-24) environs/interface.go (+4/-22) environs/jujutest/livetests.go (+3/-13) environs/statepolicy.go (+34/-0) juju/conn.go (+2/-2) juju/conn_test.go (+3/-3) provider/azure/environ.go (+0/-12) provider/azure/environ_test.go (+0/-9) provider/dummy/environs.go (+18/-5) provider/ec2/ec2.go (+0/-12) provider/ec2/local_test.go (+0/-11) provider/local/environ.go (+0/-12) provider/local/environ_test.go (+0/-16) provider/manual/environ.go (+4/-0) provider/openstack/local_test.go (+0/-11) provider/openstack/provider.go (+0/-12) state/addmachine.go (+14/-0) state/api/agent/machine_test.go (+2/-1) state/apiserver/client/api_test.go (+2/-1) state/compat_test.go (+1/-1) state/conn_test.go (+16/-1) state/environ_test.go (+1/-1) state/export_test.go (+10/-2) state/initialize_test.go (+8/-8) state/megawatcher_internal_test.go (+1/-1) state/open.go (+12/-6) state/policy.go (+63/-0) state/prechecker_test.go (+132/-0) state/settings_test.go (+1/-1) state/state.go (+1/-0) state/state_test.go (+10/-10) state/unit_test.go (+3/-3) worker/provisioner/provisioner_test.go (+6/-0) |
To merge this branch: | bzr merge lp:~axwalk/juju-core/wire-up-prechecker-take2 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Juju Engineering | Pending | ||
Review via email:
|
Commit message
Wire up prechecker
We introduce the concept of a policy
into state: state.State is given a
state.Policy interface when opened,
which is consulted during certain
operations to validate or modify
behaviour.
The Policy interface contains only
one method, Prechecker, which is called
to obtain a Prechecker. The Prechecker,
if non-nil, is used to ensure that an
instance with the specified parameters
can potentially be instantiated.
There is currently only one implementation
of Prechecker, and that is the manual
provider's Environ. The manual provider
thus prevents add-machine without ssh-
placement.
Description of the change
Wire up prechecker
We introduce the concept of a policy
into state: state.State is given a
state.Policy interface when opened,
which is consulted during certain
operations to validate or modify
behaviour.
The Policy interface contains only
one method, Prechecker, which is called
to obtain a Prechecker. The Prechecker,
if non-nil, is used to ensure that an
instance with the specified parameters
can potentially be instantiated.
There is currently only one implementation
of Prechecker, and that is the manual
provider's Environ. The manual provider
thus prevents add-machine without ssh-
placement.
This CL supersedes https:/
The primary differences are:
- up-to-date with latest codebase
- introduction of Policy, and removal of
raciness/lack of environ config updates
- policy is not set on State in juju.NewConn
methods; prechecking simply will not occur for
old environments
- Environ does not embed Prechecker, and there
is no new EnvironBase struct; this may be
proposed as a follow-up
- there is no container prechecking, as otherwise
we have no lightweight means of deploying charms
that do not require addressability
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andrew Wilkins (axwalk) wrote : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
William Reade (fwereade) wrote : | # |
On 2014/02/11 06:26:20, axw wrote:
> Please take a look.
Couple of preliminary comments: I'm not actually sure it's correct to
disable containers anywhere, because they still have value as proxy
charms: kapil in particular doesn't want to waste an instance on an ELB
charm, and hulk-smash is a pretty weak alternative.
There's definitely value in prechecking environ instances though.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andrew Wilkins (axwalk) wrote : | # |
On 2014/02/14 09:28:53, fwereade wrote:
> On 2014/02/11 06:26:20, axw wrote:
> > Please take a look.
> Couple of preliminary comments: I'm not actually sure it's correct to
disable
> containers anywhere, because they still have value as proxy charms:
kapil in
> particular doesn't want to waste an instance on an ELB charm, and
hulk-smash is
> a pretty weak alternative.
Agreed that hulk smash is a crappy alternative, and I know the use case
is there; I was hoping to address it when the time came (this is all
server side).
It doesn't seem ideal to allow containers for everyone, letting users
shoot themselves, to enable ELB, proxies, whatever. It'd be nice to have
something inside the charm that indicates what it requires (or doesn't
require?) so that we know to let those special cases through. OTOH, are
users likely to "add-machine lxc" if they don't know what they're doing?
In any case, there's work to be done. I can change it to allow
containers to begin with.
> There's definitely value in prechecking environ instances though.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
William Reade (fwereade) wrote : | # |
On 2014/02/14 09:39:44, axw wrote:
> It doesn't seem ideal to allow containers for everyone, letting users
shoot
> themselves, to enable ELB, proxies, whatever. It'd be nice to have
something
> inside the charm that indicates what it requires (or doesn't require?)
so that
> we know to let those special cases through. OTOH, are users likely to
> "add-machine lxc" if they don't know what they're doing?
Yeah, I think a charm flag (i-dont-
best approach. Once we have one of those we can do it all internally,
and expose a cmdline flag for add-machine, for those users who really do
know what they're doing.
> In any case, there's work to be done. I can change it to allow
containers to
> begin with.
Ideal, thanks.
> > There's definitely value in prechecking environ instances though.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
William Reade (fwereade) wrote : | # |
possible approach question, please discuss with waigani:
the issue with setting a prechecker, even if it's watched, is raciness.
I was willing to handwave it a bit before, but now we have waigani's use
case which requires that we get *less* racy wrt environ config... how
about setting not a prechecker, but something like a getter: eg
`func(map[
for your purposes really [0] but critically important for waigani's --
and the only difference is in the type that needs to be returned
(whether it has SetConfig or Precheck). That'd need a bit of finesse but
it'd let you use basically the same implementation for basically the
same task -- *and* mean that you don't need to do the watching followup
(at the cost of an extra db hits when prechecking -- worth it, I think).
*and* we wouldn't need to mess around with locks in state :).
[0] you *could* use it to assert that the environ hasn't changed since
you checked validity of the upcoming op, but the value of that is
somewhat limited I think
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andrew Wilkins (axwalk) wrote : | # |
On 2014/02/17 10:00:17, fwereade wrote:
> possible approach question, please discuss with waigani:
> the issue with setting a prechecker, even if it's watched, is
raciness. I was
> willing to handwave it a bit before, but now we have waigani's use
case which
> requires that we get *less* racy wrt environ config... how about
setting not a
> prechecker, but something like a getter: eg
`func(map[
> (Prechecker, error)`? This is irrelevant for your purposes really [0]
but
> critically important for waigani's -- and the only difference is in
the type
> that needs to be returned (whether it has SetConfig or Precheck).
That'd need a
> bit of finesse but it'd let you use basically the same implementation
for
> basically the same task -- *and* mean that you don't need to do the
watching
> followup (at the cost of an extra db hits when prechecking -- worth
it, I
> think).
I might just create one getter type that can do Precheck or Validate,
but otherwise I like this alternative. I'll look into implementing this,
and discuss with waigani in our standup tomorrow.
> *and* we wouldn't need to mess around with locks in state :).
> [0] you *could* use it to assert that the environ hasn't changed since
you
> checked validity of the upcoming op, but the value of that is somewhat
limited I
> think
Yeah, I don't think we need to go that far.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andrew Wilkins (axwalk) wrote : | # |
Please take a look.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
William Reade (fwereade) wrote : | # |
Looking very nice, basically just quibbles. And I'm not quite clear
about the distinction between a nil policy and a PolicyBase{} -- what
exactly governs which you use in a given situation?
https:/
File cmd/jujud/
https:/
cmd/jujud/
state.Policy(nil))
I'm wondering whether it might be good to have a package that implements
environStatePolicy, and includes a state.Open wrapper that always passes
the environ policy in. I'm not sure I can see a use case for alternative
policies at the moment, and if one does show up I think it'll be pretty
easy to add.
As it is I feel it's a bit icky having the nil policies everywhere -- I
think we probably want all our states to work the same in-test and out.
Open to counterarguments though.
https:/
File cmd/jujud/
https:/
cmd/jujud/
I don't really like the nil-return-
sort of NotImplemented error that we handle explicitly in
precheckInstance, please?
https:/
File juju/conn.go (right):
https:/
juju/conn.go:57: policy := state.PolicyBase{}
This bit doesn't have to use the mooted wrapper, ofc.
*but* I don't quite understand what the benefit is of having a null
policy here. What breaks down if we use a real one?
https:/
File state/policy.go (right):
https:/
state/policy.go:17: // for any of the interfaces, but should only return
I think you a word.
https:/
state/policy.go:43: return nil, nil
yeah, `nil, nil` scares me. Please change it :).
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andrew Wilkins (axwalk) wrote : | # |
Please take a look.
https:/
File cmd/jujud/
https:/
cmd/jujud/
state.Policy(nil))
On 2014/02/18 15:30:18, fwereade wrote:
> I'm wondering whether it might be good to have a package that
implements
> environStatePolicy, and includes a state.Open wrapper that always
passes the
> environ policy in. I'm not sure I can see a use case for alternative
policies at
> the moment, and if one does show up I think it'll be pretty easy to
add.
In any case, juju-core/agent can't use it directly. The policy
implementation requires environs, which depends on agent (which seems a
bit unfortunate.)
I'll just move the policy implementation into environs. The wrappers
aren't really all that useful since they can't be used by both
production and testing code.
> As it is I feel it's a bit icky having the nil policies everywhere --
I think we
> probably want all our states to work the same in-test and out. Open to
> counterarguments though.
No problems. I'd prefer to keep the ones in juju-core/state
self-contained though.
https:/
File cmd/jujud/
https:/
cmd/jujud/
On 2014/02/18 15:30:18, fwereade wrote:
> I don't really like the nil-return-
specific sort of
> NotImplemented error that we handle explicitly in precheckInstance,
please?
Done.
https:/
File juju/conn.go (right):
https:/
juju/conn.go:57: policy := state.PolicyBase{}
On 2014/02/18 15:30:18, fwereade wrote:
> This bit doesn't have to use the mooted wrapper, ofc.
> *but* I don't quite understand what the benefit is of having a null
policy here.
> What breaks down if we use a real one?
Nothing actually, this is basically a holdover from when Environ had to
be updated.
I'll move the policy implementation to its own package as suggested and
update this.
https:/
File state/policy.go (right):
https:/
state/policy.go:17: // for any of the interfaces, but should only return
On 2014/02/18 15:30:18, fwereade wrote:
> I think you a word.
Done.
https:/
state/policy.go:43: return nil, nil
On 2014/02/18 15:30:18, fwereade wrote:
> yeah, `nil, nil` scares me. Please change it :).
Done. Must return an error that satisfies errors.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
William Reade (fwereade) wrote : | # |
LGTM, thanks
https:/
File cmd/jujud/
https:/
cmd/jujud/
state.Policy(nil))
On 2014/02/19 07:08:45, axw wrote:
> In any case, juju-core/agent can't use it directly. The policy
implementation
> requires environs, which depends on agent (which seems a bit
unfortunate.)
Grar. Navigate the twisty passages as you must, then. Cheers.
> No problems. I'd prefer to keep the ones in juju-core/state
self-contained
> though.
SGTM
https:/
File state/policy.go (right):
https:/
state/policy.go:58: logger.
ignoring")
I think this is really bad behaviour on the policy's part; probably bad
enough to justify erroring out of the AddMachine, because the
implementation is crazy enough that all bets should be considered to be
off.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andrew Wilkins (axwalk) wrote : | # |
Please take a look.
https:/
File state/policy.go (right):
https:/
state/policy.go:58: logger.
ignoring")
On 2014/02/19 13:01:39, fwereade wrote:
> I think this is really bad behaviour on the policy's part; probably
bad enough
> to justify erroring out of the AddMachine, because the implementation
is crazy
> enough that all bets should be considered to be off.
Done.
Preview Diff
1 | === modified file 'agent/agent.go' |
2 | --- agent/agent.go 2014-02-05 08:01:22 +0000 |
3 | +++ agent/agent.go 2014-02-20 08:24:11 +0000 |
4 | @@ -75,7 +75,7 @@ |
5 | |
6 | // OpenState tries to open a direct connection to the state database using |
7 | // the given Conf. |
8 | - OpenState() (*state.State, error) |
9 | + OpenState(policy state.Policy) (*state.State, error) |
10 | |
11 | // Write writes the agent configuration. |
12 | Write() error |
13 | @@ -429,7 +429,7 @@ |
14 | return st, password, nil |
15 | } |
16 | |
17 | -func (c *configInternal) OpenState() (*state.State, error) { |
18 | +func (c *configInternal) OpenState(policy state.Policy) (*state.State, error) { |
19 | info := state.Info{ |
20 | Addrs: c.stateDetails.addresses, |
21 | Password: c.stateDetails.password, |
22 | @@ -437,7 +437,7 @@ |
23 | Tag: c.tag, |
24 | } |
25 | if info.Password != "" { |
26 | - st, err := state.Open(&info, state.DefaultDialOpts()) |
27 | + st, err := state.Open(&info, state.DefaultDialOpts(), policy) |
28 | if err == nil { |
29 | return st, nil |
30 | } |
31 | @@ -448,5 +448,5 @@ |
32 | } |
33 | } |
34 | info.Password = c.oldPassword |
35 | - return state.Open(&info, state.DefaultDialOpts()) |
36 | + return state.Open(&info, state.DefaultDialOpts(), policy) |
37 | } |
38 | |
39 | === modified file 'agent/bootstrap.go' |
40 | --- agent/bootstrap.go 2014-02-05 08:01:22 +0000 |
41 | +++ agent/bootstrap.go 2014-02-20 08:24:11 +0000 |
42 | @@ -29,7 +29,7 @@ |
43 | // InitializeState returns the newly initialized state and bootstrap |
44 | // machine. If it fails, the state may well be irredeemably compromised. |
45 | type StateInitializer interface { |
46 | - InitializeState(envCfg *config.Config, machineCfg BootstrapMachineConfig, timeout state.DialOpts) (*state.State, *state.Machine, error) |
47 | + InitializeState(envCfg *config.Config, machineCfg BootstrapMachineConfig, timeout state.DialOpts, policy state.Policy) (*state.State, *state.Machine, error) |
48 | } |
49 | |
50 | // MarshalBootstrapJobs may be used to marshal a set of |
51 | @@ -67,7 +67,7 @@ |
52 | |
53 | const bootstrapMachineId = "0" |
54 | |
55 | -func (c *configInternal) InitializeState(envCfg *config.Config, machineCfg BootstrapMachineConfig, timeout state.DialOpts) (*state.State, *state.Machine, error) { |
56 | +func (c *configInternal) InitializeState(envCfg *config.Config, machineCfg BootstrapMachineConfig, timeout state.DialOpts, policy state.Policy) (*state.State, *state.Machine, error) { |
57 | if c.Tag() != names.MachineTag(bootstrapMachineId) { |
58 | return nil, nil, fmt.Errorf("InitializeState not called with bootstrap machine's configuration") |
59 | } |
60 | @@ -76,7 +76,7 @@ |
61 | CACert: c.caCert, |
62 | } |
63 | logger.Debugf("initializing address %v", info.Addrs) |
64 | - st, err := state.Initialize(&info, envCfg, timeout) |
65 | + st, err := state.Initialize(&info, envCfg, timeout, policy) |
66 | if err != nil { |
67 | return nil, nil, fmt.Errorf("failed to initialize state: %v", err) |
68 | } |
69 | |
70 | === modified file 'agent/bootstrap_test.go' |
71 | --- agent/bootstrap_test.go 2014-02-05 08:01:22 +0000 |
72 | +++ agent/bootstrap_test.go 2014-02-20 08:24:11 +0000 |
73 | @@ -8,6 +8,7 @@ |
74 | |
75 | "launchpad.net/juju-core/agent" |
76 | "launchpad.net/juju-core/constraints" |
77 | + "launchpad.net/juju-core/environs" |
78 | "launchpad.net/juju-core/environs/config" |
79 | "launchpad.net/juju-core/instance" |
80 | "launchpad.net/juju-core/state" |
81 | @@ -71,7 +72,7 @@ |
82 | envCfg, err := config.New(config.NoDefaults, envAttrs) |
83 | c.Assert(err, gc.IsNil) |
84 | |
85 | - st, m, err := cfg.InitializeState(envCfg, mcfg, state.DialOpts{}) |
86 | + st, m, err := cfg.InitializeState(envCfg, mcfg, state.DialOpts{}, environs.NewStatePolicy()) |
87 | c.Assert(err, gc.IsNil) |
88 | defer st.Close() |
89 | |
90 | @@ -106,7 +107,7 @@ |
91 | c.Assert(newCfg.Tag(), gc.Equals, "machine-0") |
92 | c.Assert(agent.Password(newCfg), gc.Not(gc.Equals), pwHash) |
93 | c.Assert(agent.Password(newCfg), gc.Not(gc.Equals), testing.DefaultMongoPassword) |
94 | - st1, err := cfg.OpenState() |
95 | + st1, err := cfg.OpenState(environs.NewStatePolicy()) |
96 | c.Assert(err, gc.IsNil) |
97 | defer st1.Close() |
98 | } |
99 | @@ -136,13 +137,13 @@ |
100 | envCfg, err := config.New(config.NoDefaults, envAttrs) |
101 | c.Assert(err, gc.IsNil) |
102 | |
103 | - st, _, err := cfg.InitializeState(envCfg, mcfg, state.DialOpts{}) |
104 | + st, _, err := cfg.InitializeState(envCfg, mcfg, state.DialOpts{}, environs.NewStatePolicy()) |
105 | c.Assert(err, gc.IsNil) |
106 | err = st.SetAdminMongoPassword("") |
107 | c.Check(err, gc.IsNil) |
108 | st.Close() |
109 | |
110 | - st, _, err = cfg.InitializeState(envCfg, mcfg, state.DialOpts{}) |
111 | + st, _, err = cfg.InitializeState(envCfg, mcfg, state.DialOpts{}, environs.NewStatePolicy()) |
112 | if err == nil { |
113 | st.Close() |
114 | } |
115 | @@ -156,7 +157,7 @@ |
116 | Tag: "", |
117 | Password: password, |
118 | } |
119 | - st, err := state.Open(info, state.DialOpts{}) |
120 | + st, err := state.Open(info, state.DialOpts{}, environs.NewStatePolicy()) |
121 | c.Assert(err, gc.IsNil) |
122 | defer st.Close() |
123 | _, err = st.Machine("0") |
124 | |
125 | === modified file 'cmd/jujud/agent.go' |
126 | --- cmd/jujud/agent.go 2014-01-28 14:31:58 +0000 |
127 | +++ cmd/jujud/agent.go 2014-02-20 08:24:11 +0000 |
128 | @@ -12,6 +12,7 @@ |
129 | |
130 | "launchpad.net/juju-core/agent" |
131 | "launchpad.net/juju-core/cmd" |
132 | + "launchpad.net/juju-core/environs" |
133 | "launchpad.net/juju-core/errors" |
134 | "launchpad.net/juju-core/state" |
135 | "launchpad.net/juju-core/state/api" |
136 | @@ -146,7 +147,7 @@ |
137 | } |
138 | |
139 | func openState(agentConfig agent.Config, a Agent) (*state.State, AgentState, error) { |
140 | - st, err := agentConfig.OpenState() |
141 | + st, err := agentConfig.OpenState(environs.NewStatePolicy()) |
142 | if err != nil { |
143 | return nil, nil, err |
144 | } |
145 | |
146 | === modified file 'cmd/jujud/agent_test.go' |
147 | --- cmd/jujud/agent_test.go 2014-02-19 02:25:30 +0000 |
148 | +++ cmd/jujud/agent_test.go 2014-02-20 08:24:11 +0000 |
149 | @@ -13,6 +13,7 @@ |
150 | "launchpad.net/juju-core/agent" |
151 | agenttools "launchpad.net/juju-core/agent/tools" |
152 | "launchpad.net/juju-core/cmd" |
153 | + "launchpad.net/juju-core/environs" |
154 | envtesting "launchpad.net/juju-core/environs/testing" |
155 | envtools "launchpad.net/juju-core/environs/tools" |
156 | "launchpad.net/juju-core/juju/testing" |
157 | @@ -323,7 +324,7 @@ |
158 | func (s *agentSuite) assertCanOpenState(c *gc.C, tag, dataDir string) { |
159 | config, err := agent.ReadConf(dataDir, tag) |
160 | c.Assert(err, gc.IsNil) |
161 | - st, err := config.OpenState() |
162 | + st, err := config.OpenState(environs.NewStatePolicy()) |
163 | c.Assert(err, gc.IsNil) |
164 | st.Close() |
165 | } |
166 | @@ -331,7 +332,7 @@ |
167 | func (s *agentSuite) assertCannotOpenState(c *gc.C, tag, dataDir string) { |
168 | config, err := agent.ReadConf(dataDir, tag) |
169 | c.Assert(err, gc.IsNil) |
170 | - _, err = config.OpenState() |
171 | + _, err = config.OpenState(environs.NewStatePolicy()) |
172 | expectErr := fmt.Sprintf("cannot log in to juju database as %q: unauthorized mongo access: auth fails", tag) |
173 | c.Assert(err, gc.ErrorMatches, expectErr) |
174 | } |
175 | |
176 | === modified file 'cmd/jujud/bootstrap.go' |
177 | --- cmd/jujud/bootstrap.go 2014-02-05 08:01:22 +0000 |
178 | +++ cmd/jujud/bootstrap.go 2014-02-20 08:24:11 +0000 |
179 | @@ -15,6 +15,7 @@ |
180 | "launchpad.net/juju-core/agent" |
181 | "launchpad.net/juju-core/cmd" |
182 | "launchpad.net/juju-core/constraints" |
183 | + "launchpad.net/juju-core/environs" |
184 | "launchpad.net/juju-core/environs/bootstrap" |
185 | "launchpad.net/juju-core/environs/cloudinit" |
186 | "launchpad.net/juju-core/environs/config" |
187 | @@ -96,7 +97,7 @@ |
188 | Jobs: jobs, |
189 | InstanceId: bsState.StateInstances[0], |
190 | Characteristics: characteristics, |
191 | - }, state.DefaultDialOpts()) |
192 | + }, state.DefaultDialOpts(), environs.NewStatePolicy()) |
193 | if err != nil { |
194 | return err |
195 | } |
196 | |
197 | === modified file 'cmd/jujud/bootstrap_test.go' |
198 | --- cmd/jujud/bootstrap_test.go 2014-02-05 08:01:22 +0000 |
199 | +++ cmd/jujud/bootstrap_test.go 2014-02-20 08:24:11 +0000 |
200 | @@ -13,6 +13,7 @@ |
201 | |
202 | "launchpad.net/juju-core/agent" |
203 | "launchpad.net/juju-core/constraints" |
204 | + "launchpad.net/juju-core/environs" |
205 | "launchpad.net/juju-core/environs/bootstrap" |
206 | "launchpad.net/juju-core/environs/jujutest" |
207 | "launchpad.net/juju-core/errors" |
208 | @@ -118,7 +119,7 @@ |
209 | Addrs: []string{testing.MgoServer.Addr()}, |
210 | CACert: []byte(testing.CACert), |
211 | Password: testPasswordHash(), |
212 | - }, state.DefaultDialOpts()) |
213 | + }, state.DefaultDialOpts(), environs.NewStatePolicy()) |
214 | c.Assert(err, gc.IsNil) |
215 | defer st.Close() |
216 | machines, err := st.AllMachines() |
217 | @@ -145,7 +146,7 @@ |
218 | Addrs: []string{testing.MgoServer.Addr()}, |
219 | CACert: []byte(testing.CACert), |
220 | Password: testPasswordHash(), |
221 | - }, state.DefaultDialOpts()) |
222 | + }, state.DefaultDialOpts(), environs.NewStatePolicy()) |
223 | c.Assert(err, gc.IsNil) |
224 | defer st.Close() |
225 | cons, err := st.EnvironConstraints() |
226 | @@ -174,7 +175,7 @@ |
227 | Addrs: []string{testing.MgoServer.Addr()}, |
228 | CACert: []byte(testing.CACert), |
229 | Password: testPasswordHash(), |
230 | - }, state.DefaultDialOpts()) |
231 | + }, state.DefaultDialOpts(), environs.NewStatePolicy()) |
232 | c.Assert(err, gc.IsNil) |
233 | defer st.Close() |
234 | m, err := st.Machine("0") |
235 | @@ -199,7 +200,7 @@ |
236 | Addrs: []string{testing.MgoServer.Addr()}, |
237 | CACert: []byte(testing.CACert), |
238 | Password: testPasswordHash(), |
239 | - }, state.DefaultDialOpts()) |
240 | + }, state.DefaultDialOpts(), environs.NewStatePolicy()) |
241 | c.Assert(err, gc.IsNil) |
242 | defer st.Close() |
243 | m, err := st.Machine("0") |
244 | @@ -208,7 +209,7 @@ |
245 | } |
246 | |
247 | func testOpenState(c *gc.C, info *state.Info, expectErrType error) { |
248 | - st, err := state.Open(info, state.DefaultDialOpts()) |
249 | + st, err := state.Open(info, state.DefaultDialOpts(), environs.NewStatePolicy()) |
250 | if st != nil { |
251 | st.Close() |
252 | } |
253 | @@ -236,7 +237,7 @@ |
254 | |
255 | // Check we can log in to mongo as admin. |
256 | info.Tag, info.Password = "", testPasswordHash() |
257 | - st, err := state.Open(info, state.DefaultDialOpts()) |
258 | + st, err := state.Open(info, state.DefaultDialOpts(), environs.NewStatePolicy()) |
259 | c.Assert(err, gc.IsNil) |
260 | // Reset password so the tests can continue to use the same server. |
261 | defer st.Close() |
262 | @@ -254,7 +255,7 @@ |
263 | machineConf1, err := agent.ReadConf(machineConf.DataDir(), "machine-0") |
264 | c.Assert(err, gc.IsNil) |
265 | |
266 | - st, err = machineConf1.OpenState() |
267 | + st, err = machineConf1.OpenState(environs.NewStatePolicy()) |
268 | c.Assert(err, gc.IsNil) |
269 | defer st.Close() |
270 | } |
271 | |
272 | === modified file 'cmd/plugins/juju-restore/restore.go' |
273 | --- cmd/plugins/juju-restore/restore.go 2014-02-13 02:46:58 +0000 |
274 | +++ cmd/plugins/juju-restore/restore.go 2014-02-20 08:24:11 +0000 |
275 | @@ -187,7 +187,7 @@ |
276 | CACert: caCert, |
277 | Tag: creds.Tag, |
278 | Password: creds.Password, |
279 | - }, state.DefaultDialOpts()) |
280 | + }, state.DefaultDialOpts(), environs.NewStatePolicy()) |
281 | if err != nil { |
282 | return fmt.Errorf("cannot open state: %v", err) |
283 | } |
284 | |
285 | === modified file 'environs/errors.go' |
286 | --- environs/errors.go 2013-09-27 08:14:36 +0000 |
287 | +++ environs/errors.go 2014-02-20 08:24:11 +0000 |
288 | @@ -12,27 +12,3 @@ |
289 | ErrNoInstances = errors.New("no instances found") |
290 | ErrPartialInstances = errors.New("only some instances were found") |
291 | ) |
292 | - |
293 | -// containersUnsupportedError indicates that the environment does not support |
294 | -// creation of containers. |
295 | -type containersUnsupportedError struct { |
296 | - msg string |
297 | -} |
298 | - |
299 | -func (e *containersUnsupportedError) Error() string { |
300 | - return e.msg |
301 | -} |
302 | - |
303 | -// IsContainersUnsupportedError reports whether the error |
304 | -// was created by NewContainersUnsupportedError. |
305 | -func IsContainersUnsupportedError(err error) bool { |
306 | - _, ok := err.(*containersUnsupportedError) |
307 | - return ok |
308 | -} |
309 | - |
310 | -// NewContainersUnsupportedError returns a new error |
311 | -// which satisfies IsContainersUnsupported and reports |
312 | -// the given message. |
313 | -func NewContainersUnsupported(msg string) error { |
314 | - return &containersUnsupportedError{msg: msg} |
315 | -} |
316 | |
317 | === modified file 'environs/interface.go' |
318 | --- environs/interface.go 2014-02-13 03:16:09 +0000 |
319 | +++ environs/interface.go 2014-02-20 08:24:11 +0000 |
320 | @@ -71,28 +71,6 @@ |
321 | Config() *config.Config |
322 | } |
323 | |
324 | -// Prechecker is an optional interface that an Environ may implement, |
325 | -// in order to support pre-flight checking of instance/container creation. |
326 | -// |
327 | -// Prechecker's methods are best effort, and not guaranteed to eliminate |
328 | -// all invalid parameters. If a precheck method returns nil, it is not |
329 | -// guaranteed that the constraints are valid; if a non-nil error is |
330 | -// returned, then the constraints are definitely invalid. |
331 | -type Prechecker interface { |
332 | - // PrecheckInstance performs a preflight check on the specified |
333 | - // series and constraints, ensuring that they are possibly valid for |
334 | - // creating an instance in this environment. |
335 | - PrecheckInstance(series string, cons constraints.Value) error |
336 | - |
337 | - // PrecheckContainer performs a preflight check on the container type, |
338 | - // ensuring that the environment is possibly capable of creating a |
339 | - // container of the specified type and series. |
340 | - // |
341 | - // The container type must be a valid ContainerType as specified |
342 | - // in the instance package, and != instance.NONE. |
343 | - PrecheckContainer(series string, kind instance.ContainerType) error |
344 | -} |
345 | - |
346 | // An Environ represents a juju environment as specified |
347 | // in the environments.yaml file. |
348 | // |
349 | @@ -180,6 +158,10 @@ |
350 | |
351 | // Provider returns the EnvironProvider that created this Environ. |
352 | Provider() EnvironProvider |
353 | + |
354 | + // TODO(axw) 2014-02-11 #pending-review |
355 | + // Embed state.Prechecker, and introduce an EnvironBase |
356 | + // that embeds a no-op prechecker implementation. |
357 | } |
358 | |
359 | // BootstrapContext is an interface that is passed to |
360 | |
361 | === modified file 'environs/jujutest/livetests.go' |
362 | --- environs/jujutest/livetests.go 2014-02-13 02:46:58 +0000 |
363 | +++ environs/jujutest/livetests.go 2014-02-20 08:24:11 +0000 |
364 | @@ -159,22 +159,12 @@ |
365 | func (t *LiveTests) TestPrechecker(c *gc.C) { |
366 | // Providers may implement Prechecker. If they do, then they should |
367 | // return nil for empty constraints (excluding the null provider). |
368 | - prechecker, ok := t.Env.(environs.Prechecker) |
369 | + prechecker, ok := t.Env.(state.Prechecker) |
370 | if !ok { |
371 | return |
372 | } |
373 | - |
374 | - const series = "precise" |
375 | - var cons constraints.Value |
376 | - c.Check(prechecker.PrecheckInstance(series, cons), gc.IsNil) |
377 | - |
378 | - err := prechecker.PrecheckContainer(series, instance.LXC) |
379 | - // If err is nil, that is fine, some providers support containers. |
380 | - if err != nil { |
381 | - // But for ones that don't, they should have a standard error format. |
382 | - c.Check(err, gc.ErrorMatches, ".*provider does not support .*containers") |
383 | - c.Check(err, jc.Satisfies, environs.IsContainersUnsupportedError) |
384 | - } |
385 | + err := prechecker.PrecheckInstance("precise", constraints.Value{}) |
386 | + c.Assert(err, gc.IsNil) |
387 | } |
388 | |
389 | // TestStartStop is similar to Tests.TestStartStop except |
390 | |
391 | === added file 'environs/statepolicy.go' |
392 | --- environs/statepolicy.go 1970-01-01 00:00:00 +0000 |
393 | +++ environs/statepolicy.go 2014-02-20 08:24:11 +0000 |
394 | @@ -0,0 +1,34 @@ |
395 | +// Copyright 2014 Canonical Ltd. |
396 | +// Licensed under the AGPLv3, see LICENCE file for details. |
397 | + |
398 | +package environs |
399 | + |
400 | +import ( |
401 | + "launchpad.net/juju-core/environs/config" |
402 | + "launchpad.net/juju-core/errors" |
403 | + "launchpad.net/juju-core/state" |
404 | +) |
405 | + |
406 | +// environStatePolicy implements state.Policy in |
407 | +// terms of environs.Environ and related types. |
408 | +type environStatePolicy struct{} |
409 | + |
410 | +var _ state.Policy = environStatePolicy{} |
411 | + |
412 | +// NewStatePolicy returns a state.Policy that is |
413 | +// implemented in terms of Environ and related |
414 | +// types. |
415 | +func NewStatePolicy() state.Policy { |
416 | + return environStatePolicy{} |
417 | +} |
418 | + |
419 | +func (environStatePolicy) Prechecker(cfg *config.Config) (state.Prechecker, error) { |
420 | + env, err := New(cfg) |
421 | + if err != nil { |
422 | + return nil, err |
423 | + } |
424 | + if p, ok := env.(state.Prechecker); ok { |
425 | + return p, nil |
426 | + } |
427 | + return nil, errors.NewNotImplementedError("Prechecker") |
428 | +} |
429 | |
430 | === modified file 'juju/conn.go' |
431 | --- juju/conn.go 2014-01-28 17:07:55 +0000 |
432 | +++ juju/conn.go 2014-02-20 08:24:11 +0000 |
433 | @@ -54,7 +54,7 @@ |
434 | |
435 | info.Password = password |
436 | opts := state.DefaultDialOpts() |
437 | - st, err := state.Open(info, opts) |
438 | + st, err := state.Open(info, opts, environs.NewStatePolicy()) |
439 | if errors.IsUnauthorizedError(err) { |
440 | log.Noticef("juju: authorization error while connecting to state server; retrying") |
441 | // We can't connect with the administrator password,; |
442 | @@ -66,7 +66,7 @@ |
443 | // connecting to mongo before the state has been |
444 | // initialized and the initial password set. |
445 | for a := redialStrategy.Start(); a.Next(); { |
446 | - st, err = state.Open(info, opts) |
447 | + st, err = state.Open(info, opts, environs.NewStatePolicy()) |
448 | if !errors.IsUnauthorizedError(err) { |
449 | break |
450 | } |
451 | |
452 | === modified file 'juju/conn_test.go' |
453 | --- juju/conn_test.go 2014-02-13 02:46:58 +0000 |
454 | +++ juju/conn_test.go 2014-02-20 08:24:11 +0000 |
455 | @@ -140,7 +140,7 @@ |
456 | info, _, err := env.StateInfo() |
457 | c.Assert(err, gc.IsNil) |
458 | info.Password = utils.UserPasswordHash("side-effect secret", utils.CompatSalt) |
459 | - st, err := state.Open(info, state.DefaultDialOpts()) |
460 | + st, err := state.Open(info, state.DefaultDialOpts(), environs.NewStatePolicy()) |
461 | c.Assert(err, gc.IsNil) |
462 | defer assertClose(c, st) |
463 | |
464 | @@ -230,7 +230,7 @@ |
465 | info, _, err := env.StateInfo() |
466 | c.Assert(err, gc.IsNil) |
467 | info.Password = utils.UserPasswordHash("nutkin", utils.CompatSalt) |
468 | - st, err := state.Open(info, state.DefaultDialOpts()) |
469 | + st, err := state.Open(info, state.DefaultDialOpts(), environs.NewStatePolicy()) |
470 | c.Assert(err, gc.IsNil) |
471 | assertClose(c, st) |
472 | |
473 | @@ -242,7 +242,7 @@ |
474 | // Check that the password has now been changed to the original |
475 | // admin password. |
476 | info.Password = "nutkin" |
477 | - st1, err := state.Open(info, state.DefaultDialOpts()) |
478 | + st1, err := state.Open(info, state.DefaultDialOpts(), environs.NewStatePolicy()) |
479 | c.Assert(err, gc.IsNil) |
480 | assertClose(c, st1) |
481 | |
482 | |
483 | === modified file 'provider/azure/environ.go' |
484 | --- provider/azure/environ.go 2014-01-29 06:45:16 +0000 |
485 | +++ provider/azure/environ.go 2014-02-20 08:24:11 +0000 |
486 | @@ -123,18 +123,6 @@ |
487 | return key, nil |
488 | } |
489 | |
490 | -// PrecheckInstance is specified in the environs.Prechecker interface. |
491 | -func (*azureEnviron) PrecheckInstance(series string, cons constraints.Value) error { |
492 | - return nil |
493 | -} |
494 | - |
495 | -// PrecheckContainer is specified in the environs.Prechecker interface. |
496 | -func (*azureEnviron) PrecheckContainer(series string, kind instance.ContainerType) error { |
497 | - // This check can either go away or be relaxed when the azure |
498 | - // provider manages container addressibility. |
499 | - return environs.NewContainersUnsupported("azure provider does not support containers") |
500 | -} |
501 | - |
502 | // Name is specified in the Environ interface. |
503 | func (env *azureEnviron) Name() string { |
504 | return env.name |
505 | |
506 | === modified file 'provider/azure/environ_test.go' |
507 | --- provider/azure/environ_test.go 2014-01-29 06:45:16 +0000 |
508 | +++ provider/azure/environ_test.go 2014-02-20 08:24:11 +0000 |
509 | @@ -93,15 +93,6 @@ |
510 | c.Check(env.Name(), gc.Equals, env.name) |
511 | } |
512 | |
513 | -func (*environSuite) TestPrecheck(c *gc.C) { |
514 | - env := azureEnviron{name: "foo"} |
515 | - var cons constraints.Value |
516 | - err := env.PrecheckInstance("saucy", cons) |
517 | - c.Check(err, gc.IsNil) |
518 | - err = env.PrecheckContainer("saucy", instance.LXC) |
519 | - c.Check(err, gc.ErrorMatches, "azure provider does not support containers") |
520 | -} |
521 | - |
522 | func (*environSuite) TestConfigReturnsConfig(c *gc.C) { |
523 | cfg := new(config.Config) |
524 | ecfg := azureEnvironConfig{Config: cfg} |
525 | |
526 | === modified file 'provider/dummy/environs.go' |
527 | --- provider/dummy/environs.go 2014-02-20 04:59:31 +0000 |
528 | +++ provider/dummy/environs.go 2014-02-20 08:24:11 +0000 |
529 | @@ -146,8 +146,9 @@ |
530 | // environProvider represents the dummy provider. There is only ever one |
531 | // instance of this type (providerInstance) |
532 | type environProvider struct { |
533 | - mu sync.Mutex |
534 | - ops chan<- Operation |
535 | + mu sync.Mutex |
536 | + ops chan<- Operation |
537 | + statePolicy state.Policy |
538 | // We have one state for each environment name |
539 | state map[int]*environState |
540 | maxStateId int |
541 | @@ -164,6 +165,7 @@ |
542 | id int |
543 | name string |
544 | ops chan<- Operation |
545 | + statePolicy state.Policy |
546 | mu sync.Mutex |
547 | maxId int // maximum instance id allocated so far. |
548 | insts map[instance.Id]*dummyInstance |
549 | @@ -225,6 +227,7 @@ |
550 | if testing.MgoServer.Addr() != "" { |
551 | testing.MgoServer.Reset() |
552 | } |
553 | + providerInstance.statePolicy = environs.NewStatePolicy() |
554 | } |
555 | |
556 | func (state *environState) destroy() { |
557 | @@ -262,10 +265,11 @@ |
558 | // newState creates the state for a new environment with the |
559 | // given name and starts an http server listening for |
560 | // storage requests. |
561 | -func newState(name string, ops chan<- Operation) *environState { |
562 | +func newState(name string, ops chan<- Operation, policy state.Policy) *environState { |
563 | s := &environState{ |
564 | name: name, |
565 | ops: ops, |
566 | + statePolicy: policy, |
567 | insts: make(map[instance.Id]*dummyInstance), |
568 | globalPorts: make(map[instance.Port]bool), |
569 | } |
570 | @@ -287,6 +291,15 @@ |
571 | go http.Serve(l, mux) |
572 | } |
573 | |
574 | +// SetStatePolicy sets the state.Policy to use when a |
575 | +// state server is initialised by dummy. |
576 | +func SetStatePolicy(policy state.Policy) { |
577 | + p := &providerInstance |
578 | + p.mu.Lock() |
579 | + defer p.mu.Unlock() |
580 | + p.statePolicy = policy |
581 | +} |
582 | + |
583 | // Listen closes the previously registered listener (if any). |
584 | // Subsequent operations on any dummy environment can be received on c |
585 | // (if not nil). |
586 | @@ -450,7 +463,7 @@ |
587 | panic(fmt.Errorf("cannot share a state between two dummy environs; old %q; new %q", old.name, name)) |
588 | } |
589 | } |
590 | - state := newState(name, p.ops) |
591 | + state := newState(name, p.ops, p.statePolicy) |
592 | p.maxStateId++ |
593 | state.id = p.maxStateId |
594 | p.state[state.id] = state |
595 | @@ -574,7 +587,7 @@ |
596 | // so that we can call it here. |
597 | |
598 | info := stateInfo() |
599 | - st, err := state.Initialize(info, cfg, state.DefaultDialOpts()) |
600 | + st, err := state.Initialize(info, cfg, state.DefaultDialOpts(), estate.statePolicy) |
601 | if err != nil { |
602 | panic(err) |
603 | } |
604 | |
605 | === modified file 'provider/ec2/ec2.go' |
606 | --- provider/ec2/ec2.go 2014-02-13 02:46:58 +0000 |
607 | +++ provider/ec2/ec2.go 2014-02-20 08:24:11 +0000 |
608 | @@ -306,18 +306,6 @@ |
609 | return s3 |
610 | } |
611 | |
612 | -// PrecheckInstance is specified in the environs.Prechecker interface. |
613 | -func (e *environ) PrecheckInstance(series string, cons constraints.Value) error { |
614 | - return nil |
615 | -} |
616 | - |
617 | -// PrecheckContainer is specified in the environs.Prechecker interface. |
618 | -func (e *environ) PrecheckContainer(series string, kind instance.ContainerType) error { |
619 | - // This check can either go away or be relaxed when the ec2 |
620 | - // provider manages container addressibility. |
621 | - return environs.NewContainersUnsupported("ec2 provider does not support containers") |
622 | -} |
623 | - |
624 | func (e *environ) Name() string { |
625 | return e.name |
626 | } |
627 | |
628 | === modified file 'provider/ec2/local_test.go' |
629 | --- provider/ec2/local_test.go 2014-02-13 02:46:58 +0000 |
630 | +++ provider/ec2/local_test.go 2014-02-20 08:24:11 +0000 |
631 | @@ -229,17 +229,6 @@ |
632 | t.srv.stopServer(c) |
633 | } |
634 | |
635 | -func (t *localServerSuite) TestPrecheck(c *gc.C) { |
636 | - env := t.Prepare(c) |
637 | - prechecker, ok := env.(environs.Prechecker) |
638 | - c.Assert(ok, jc.IsTrue) |
639 | - var cons constraints.Value |
640 | - err := prechecker.PrecheckInstance("precise", cons) |
641 | - c.Check(err, gc.IsNil) |
642 | - err = prechecker.PrecheckContainer("precise", instance.LXC) |
643 | - c.Check(err, gc.ErrorMatches, "ec2 provider does not support containers") |
644 | -} |
645 | - |
646 | func (t *localServerSuite) TestBootstrapInstanceUserDataAndState(c *gc.C) { |
647 | env := t.Prepare(c) |
648 | envtesting.UploadFakeTools(c, env.Storage()) |
649 | |
650 | === modified file 'provider/local/environ.go' |
651 | --- provider/local/environ.go 2014-02-18 01:29:47 +0000 |
652 | +++ provider/local/environ.go 2014-02-20 08:24:11 +0000 |
653 | @@ -82,18 +82,6 @@ |
654 | return fmt.Sprintf("/etc/rsyslog.d/25-juju-%s.conf", env.config.namespace()) |
655 | } |
656 | |
657 | -// PrecheckInstance is specified in the environs.Prechecker interface. |
658 | -func (*localEnviron) PrecheckInstance(series string, cons constraints.Value) error { |
659 | - return nil |
660 | -} |
661 | - |
662 | -// PrecheckContainer is specified in the environs.Prechecker interface. |
663 | -func (*localEnviron) PrecheckContainer(series string, kind instance.ContainerType) error { |
664 | - // This check can either go away or be relaxed when the local |
665 | - // provider can do nested containers. |
666 | - return environs.NewContainersUnsupported("local provider does not support nested containers") |
667 | -} |
668 | - |
669 | func ensureNotRoot() error { |
670 | if checkIfRoot() { |
671 | return fmt.Errorf("bootstrapping a local environment must not be done as root") |
672 | |
673 | === modified file 'provider/local/environ_test.go' |
674 | --- provider/local/environ_test.go 2014-02-18 17:29:25 +0000 |
675 | +++ provider/local/environ_test.go 2014-02-20 08:24:11 +0000 |
676 | @@ -20,7 +20,6 @@ |
677 | "launchpad.net/juju-core/environs/jujutest" |
678 | envtesting "launchpad.net/juju-core/environs/testing" |
679 | "launchpad.net/juju-core/environs/tools" |
680 | - "launchpad.net/juju-core/instance" |
681 | "launchpad.net/juju-core/provider/local" |
682 | "launchpad.net/juju-core/state" |
683 | coretesting "launchpad.net/juju-core/testing" |
684 | @@ -76,21 +75,6 @@ |
685 | c.Assert(strings.Contains(url, "/tools"), jc.IsTrue) |
686 | } |
687 | |
688 | -func (s *environSuite) TestPrecheck(c *gc.C) { |
689 | - testConfig := minimalConfig(c) |
690 | - environ, err := local.Provider.Open(testConfig) |
691 | - c.Assert(err, gc.IsNil) |
692 | - var cons constraints.Value |
693 | - prechecker, ok := environ.(environs.Prechecker) |
694 | - c.Assert(ok, jc.IsTrue) |
695 | - |
696 | - err = prechecker.PrecheckInstance("precise", cons) |
697 | - c.Check(err, gc.IsNil) |
698 | - |
699 | - err = prechecker.PrecheckContainer("precise", instance.LXC) |
700 | - c.Check(err, gc.ErrorMatches, "local provider does not support nested containers") |
701 | -} |
702 | - |
703 | type localJujuTestSuite struct { |
704 | baseProviderSuite |
705 | jujutest.Tests |
706 | |
707 | === modified file 'provider/manual/environ.go' |
708 | --- provider/manual/environ.go 2014-02-13 03:16:09 +0000 |
709 | +++ provider/manual/environ.go 2014-02-20 08:24:11 +0000 |
710 | @@ -231,6 +231,10 @@ |
711 | return err |
712 | } |
713 | |
714 | +func (*manualEnviron) PrecheckInstance(series string, cons constraints.Value) error { |
715 | + return errors.New(`use "juju add-machine ssh:[user@]<host>" to provision machines`) |
716 | +} |
717 | + |
718 | func (e *manualEnviron) OpenPorts(ports []instance.Port) error { |
719 | return nil |
720 | } |
721 | |
722 | === modified file 'provider/openstack/local_test.go' |
723 | --- provider/openstack/local_test.go 2014-02-18 01:29:47 +0000 |
724 | +++ provider/openstack/local_test.go 2014-02-20 08:24:11 +0000 |
725 | @@ -252,17 +252,6 @@ |
726 | s.LoggingSuite.TearDownTest(c) |
727 | } |
728 | |
729 | -func (s *localServerSuite) TestPrecheck(c *gc.C) { |
730 | - var cons constraints.Value |
731 | - env := s.Prepare(c) |
732 | - prechecker, ok := env.(environs.Prechecker) |
733 | - c.Assert(ok, jc.IsTrue) |
734 | - err := prechecker.PrecheckInstance("precise", cons) |
735 | - c.Check(err, gc.IsNil) |
736 | - err = prechecker.PrecheckContainer("precise", instance.LXC) |
737 | - c.Check(err, gc.ErrorMatches, "openstack provider does not support containers") |
738 | -} |
739 | - |
740 | // If the bootstrap node is configured to require a public IP address, |
741 | // bootstrapping fails if an address cannot be allocated. |
742 | func (s *localServerSuite) TestBootstrapFailsWhenPublicIPError(c *gc.C) { |
743 | |
744 | === modified file 'provider/openstack/provider.go' |
745 | --- provider/openstack/provider.go 2014-02-13 02:46:58 +0000 |
746 | +++ provider/openstack/provider.go 2014-02-20 08:24:11 +0000 |
747 | @@ -483,18 +483,6 @@ |
748 | return nova |
749 | } |
750 | |
751 | -// PrecheckInstance is specified in the environs.Prechecker interface. |
752 | -func (*environ) PrecheckInstance(series string, cons constraints.Value) error { |
753 | - return nil |
754 | -} |
755 | - |
756 | -// PrecheckContainer is specified in the environs.Prechecker interface. |
757 | -func (*environ) PrecheckContainer(series string, kind instance.ContainerType) error { |
758 | - // This check can either go away or be relaxed when the openstack |
759 | - // provider manages container addressibility. |
760 | - return environs.NewContainersUnsupported("openstack provider does not support containers") |
761 | -} |
762 | - |
763 | func (e *environ) Name() string { |
764 | return e.name |
765 | } |
766 | |
767 | === modified file 'state/addmachine.go' |
768 | --- state/addmachine.go 2014-01-25 11:45:42 +0000 |
769 | +++ state/addmachine.go 2014-02-20 08:24:11 +0000 |
770 | @@ -218,6 +218,11 @@ |
771 | if err != nil { |
772 | return nil, nil, err |
773 | } |
774 | + if template.InstanceId == "" { |
775 | + if err := st.precheckInstance(template.Series, template.Constraints); err != nil { |
776 | + return nil, nil, err |
777 | + } |
778 | + } |
779 | seq, err := st.sequence("machine") |
780 | if err != nil { |
781 | return nil, nil, err |
782 | @@ -331,6 +336,15 @@ |
783 | if err != nil { |
784 | return nil, nil, err |
785 | } |
786 | + if containerType == "" { |
787 | + return nil, nil, fmt.Errorf("no container type specified") |
788 | + } |
789 | + if parentTemplate.InstanceId == "" { |
790 | + if err := st.precheckInstance(parentTemplate.Series, parentTemplate.Constraints); err != nil { |
791 | + return nil, nil, err |
792 | + } |
793 | + } |
794 | + |
795 | parentDoc := machineDocForTemplate(parentTemplate, strconv.Itoa(seq)) |
796 | newId, err := st.newContainerId(parentDoc.Id, containerType) |
797 | if err != nil { |
798 | |
799 | === modified file 'state/api/agent/machine_test.go' |
800 | --- state/api/agent/machine_test.go 2013-11-05 10:25:28 +0000 |
801 | +++ state/api/agent/machine_test.go 2014-02-20 08:24:11 +0000 |
802 | @@ -9,6 +9,7 @@ |
803 | |
804 | gc "launchpad.net/gocheck" |
805 | |
806 | + "launchpad.net/juju-core/environs" |
807 | "launchpad.net/juju-core/errors" |
808 | "launchpad.net/juju-core/juju/testing" |
809 | "launchpad.net/juju-core/state" |
810 | @@ -83,7 +84,7 @@ |
811 | } |
812 | |
813 | func tryOpenState(info *state.Info) error { |
814 | - st, err := state.Open(info, state.DialOpts{}) |
815 | + st, err := state.Open(info, state.DialOpts{}, environs.NewStatePolicy()) |
816 | if err == nil { |
817 | st.Close() |
818 | } |
819 | |
820 | === modified file 'state/apiserver/client/api_test.go' |
821 | --- state/apiserver/client/api_test.go 2014-01-23 20:30:35 +0000 |
822 | +++ state/apiserver/client/api_test.go 2014-02-20 08:24:11 +0000 |
823 | @@ -11,6 +11,7 @@ |
824 | gc "launchpad.net/gocheck" |
825 | |
826 | "launchpad.net/juju-core/constraints" |
827 | + "launchpad.net/juju-core/environs" |
828 | "launchpad.net/juju-core/environs/config" |
829 | "launchpad.net/juju-core/errors" |
830 | "launchpad.net/juju-core/instance" |
831 | @@ -111,7 +112,7 @@ |
832 | stateInfo.Password = password |
833 | st, err := state.Open(stateInfo, state.DialOpts{ |
834 | Timeout: 25 * time.Millisecond, |
835 | - }) |
836 | + }, environs.NewStatePolicy()) |
837 | if err == nil { |
838 | st.Close() |
839 | } |
840 | |
841 | === modified file 'state/compat_test.go' |
842 | --- state/compat_test.go 2014-01-09 10:37:28 +0000 |
843 | +++ state/compat_test.go 2014-02-20 08:24:11 +0000 |
844 | @@ -36,7 +36,7 @@ |
845 | func (s *compatSuite) SetUpTest(c *gc.C) { |
846 | s.LoggingSuite.SetUpTest(c) |
847 | s.MgoSuite.SetUpTest(c) |
848 | - s.state = TestingInitialize(c, nil) |
849 | + s.state = TestingInitialize(c, nil, Policy(nil)) |
850 | env, err := s.state.Environment() |
851 | c.Assert(err, gc.IsNil) |
852 | s.env = env |
853 | |
854 | === modified file 'state/conn_test.go' |
855 | --- state/conn_test.go 2013-12-03 14:05:19 +0000 |
856 | +++ state/conn_test.go 2014-02-20 08:24:11 +0000 |
857 | @@ -9,6 +9,8 @@ |
858 | "labix.org/v2/mgo" |
859 | gc "launchpad.net/gocheck" |
860 | |
861 | + "launchpad.net/juju-core/environs/config" |
862 | + "launchpad.net/juju-core/errors" |
863 | "launchpad.net/juju-core/state" |
864 | "launchpad.net/juju-core/testing" |
865 | "launchpad.net/juju-core/testing/testbase" |
866 | @@ -32,6 +34,7 @@ |
867 | units *mgo.Collection |
868 | stateServers *mgo.Collection |
869 | State *state.State |
870 | + policy mockPolicy |
871 | } |
872 | |
873 | func (cs *ConnSuite) SetUpSuite(c *gc.C) { |
874 | @@ -47,7 +50,8 @@ |
875 | func (cs *ConnSuite) SetUpTest(c *gc.C) { |
876 | cs.LoggingSuite.SetUpTest(c) |
877 | cs.MgoSuite.SetUpTest(c) |
878 | - cs.State = state.TestingInitialize(c, nil) |
879 | + cs.policy = mockPolicy{} |
880 | + cs.State = state.TestingInitialize(c, nil, &cs.policy) |
881 | cs.annotations = cs.MgoSuite.Session.DB("juju").C("annotations") |
882 | cs.charms = cs.MgoSuite.Session.DB("juju").C("charms") |
883 | cs.machines = cs.MgoSuite.Session.DB("juju").C("machines") |
884 | @@ -88,3 +92,14 @@ |
885 | func (s *ConnSuite) AddMetaCharm(c *gc.C, name, metaYaml string, revsion int) *state.Charm { |
886 | return state.AddCustomCharm(c, s.State, name, "metadata.yaml", metaYaml, "quantal", revsion) |
887 | } |
888 | + |
889 | +type mockPolicy struct { |
890 | + getPrechecker func(*config.Config) (state.Prechecker, error) |
891 | +} |
892 | + |
893 | +func (p *mockPolicy) Prechecker(cfg *config.Config) (state.Prechecker, error) { |
894 | + if p.getPrechecker != nil { |
895 | + return p.getPrechecker(cfg) |
896 | + } |
897 | + return nil, errors.NewNotImplementedError("Prechecker") |
898 | +} |
899 | |
900 | === modified file 'state/environ_test.go' |
901 | --- state/environ_test.go 2014-02-03 09:44:12 +0000 |
902 | +++ state/environ_test.go 2014-02-20 08:24:11 +0000 |
903 | @@ -40,7 +40,7 @@ |
904 | s.State.Close() |
905 | s.MgoSuite.TearDownTest(c) |
906 | s.MgoSuite.SetUpTest(c) |
907 | - s.State = state.TestingInitialize(c, nil) |
908 | + s.State = state.TestingInitialize(c, nil, state.Policy(nil)) |
909 | env, err := s.State.Environment() |
910 | c.Assert(err, gc.IsNil) |
911 | uuidB := env.UUID() |
912 | |
913 | === modified file 'state/export_test.go' |
914 | --- state/export_test.go 2014-02-17 12:57:06 +0000 |
915 | +++ state/export_test.go 2014-02-20 08:24:11 +0000 |
916 | @@ -94,14 +94,22 @@ |
917 | }) |
918 | } |
919 | |
920 | +// SetPolicy updates the State's policy field to the |
921 | +// given Policy, and returns the old value. |
922 | +func SetPolicy(st *State, p Policy) Policy { |
923 | + old := st.policy |
924 | + st.policy = p |
925 | + return old |
926 | +} |
927 | + |
928 | // TestingInitialize initializes the state and returns it. If state was not |
929 | // already initialized, and cfg is nil, the minimal default environment |
930 | // configuration will be used. |
931 | -func TestingInitialize(c *gc.C, cfg *config.Config) *State { |
932 | +func TestingInitialize(c *gc.C, cfg *config.Config, policy Policy) *State { |
933 | if cfg == nil { |
934 | cfg = testing.EnvironConfig(c) |
935 | } |
936 | - st, err := Initialize(TestingStateInfo(), cfg, TestingDialOpts()) |
937 | + st, err := Initialize(TestingStateInfo(), cfg, TestingDialOpts(), policy) |
938 | c.Assert(err, gc.IsNil) |
939 | return st |
940 | } |
941 | |
942 | === modified file 'state/initialize_test.go' |
943 | --- state/initialize_test.go 2013-12-11 09:14:12 +0000 |
944 | +++ state/initialize_test.go 2014-02-20 08:24:11 +0000 |
945 | @@ -37,7 +37,7 @@ |
946 | s.LoggingSuite.SetUpTest(c) |
947 | s.MgoSuite.SetUpTest(c) |
948 | var err error |
949 | - s.State, err = state.Open(state.TestingStateInfo(), state.TestingDialOpts()) |
950 | + s.State, err = state.Open(state.TestingStateInfo(), state.TestingDialOpts(), state.Policy(nil)) |
951 | c.Assert(err, gc.IsNil) |
952 | } |
953 | |
954 | @@ -59,7 +59,7 @@ |
955 | |
956 | cfg := testing.EnvironConfig(c) |
957 | initial := cfg.AllAttrs() |
958 | - st, err := state.Initialize(state.TestingStateInfo(), cfg, state.TestingDialOpts()) |
959 | + st, err := state.Initialize(state.TestingStateInfo(), cfg, state.TestingDialOpts(), state.Policy(nil)) |
960 | c.Assert(err, gc.IsNil) |
961 | c.Assert(st, gc.NotNil) |
962 | err = st.Close() |
963 | @@ -85,7 +85,7 @@ |
964 | func (s *InitializeSuite) TestDoubleInitializeConfig(c *gc.C) { |
965 | cfg := testing.EnvironConfig(c) |
966 | initial := cfg.AllAttrs() |
967 | - st := state.TestingInitialize(c, cfg) |
968 | + st := state.TestingInitialize(c, cfg, state.Policy(nil)) |
969 | st.Close() |
970 | |
971 | // A second initialize returns an open *State, but ignores its params. |
972 | @@ -93,7 +93,7 @@ |
973 | // for originally... |
974 | cfg, err := cfg.Apply(map[string]interface{}{"authorized-keys": "something-else"}) |
975 | c.Assert(err, gc.IsNil) |
976 | - st, err = state.Initialize(state.TestingStateInfo(), cfg, state.TestingDialOpts()) |
977 | + st, err = state.Initialize(state.TestingStateInfo(), cfg, state.TestingDialOpts(), state.Policy(nil)) |
978 | c.Assert(err, gc.IsNil) |
979 | c.Assert(st, gc.NotNil) |
980 | st.Close() |
981 | @@ -108,11 +108,11 @@ |
982 | good := testing.EnvironConfig(c) |
983 | bad, err := good.Apply(map[string]interface{}{"admin-secret": "foo"}) |
984 | |
985 | - _, err = state.Initialize(state.TestingStateInfo(), bad, state.TestingDialOpts()) |
986 | + _, err = state.Initialize(state.TestingStateInfo(), bad, state.TestingDialOpts(), state.Policy(nil)) |
987 | c.Assert(err, gc.ErrorMatches, "admin-secret should never be written to the state") |
988 | |
989 | // admin-secret blocks SetEnvironConfig. |
990 | - st := state.TestingInitialize(c, good) |
991 | + st := state.TestingInitialize(c, good, state.Policy(nil)) |
992 | st.Close() |
993 | err = s.State.SetEnvironConfig(bad, good) |
994 | c.Assert(err, gc.ErrorMatches, "admin-secret should never be written to the state") |
995 | @@ -131,11 +131,11 @@ |
996 | bad, err := config.New(config.NoDefaults, attrs) |
997 | c.Assert(err, gc.IsNil) |
998 | |
999 | - _, err = state.Initialize(state.TestingStateInfo(), bad, state.TestingDialOpts()) |
1000 | + _, err = state.Initialize(state.TestingStateInfo(), bad, state.TestingDialOpts(), state.Policy(nil)) |
1001 | c.Assert(err, gc.ErrorMatches, "agent-version must always be set in state") |
1002 | |
1003 | // Bad agent-version blocks SetEnvironConfig. |
1004 | - st := state.TestingInitialize(c, good) |
1005 | + st := state.TestingInitialize(c, good, state.Policy(nil)) |
1006 | st.Close() |
1007 | err = s.State.SetEnvironConfig(bad, good) |
1008 | c.Assert(err, gc.ErrorMatches, "agent-version must always be set in state") |
1009 | |
1010 | === modified file 'state/megawatcher_internal_test.go' |
1011 | --- state/megawatcher_internal_test.go 2014-01-24 13:28:02 +0000 |
1012 | +++ state/megawatcher_internal_test.go 2014-02-20 08:24:11 +0000 |
1013 | @@ -47,7 +47,7 @@ |
1014 | func (s *storeManagerStateSuite) SetUpTest(c *gc.C) { |
1015 | s.LoggingSuite.SetUpTest(c) |
1016 | s.MgoSuite.SetUpTest(c) |
1017 | - s.State = TestingInitialize(c, nil) |
1018 | + s.State = TestingInitialize(c, nil, Policy(nil)) |
1019 | s.State.AddUser("admin", "pass") |
1020 | } |
1021 | |
1022 | |
1023 | === modified file 'state/open.go' |
1024 | --- state/open.go 2014-02-07 18:38:02 +0000 |
1025 | +++ state/open.go 2014-02-20 08:24:11 +0000 |
1026 | @@ -69,8 +69,13 @@ |
1027 | // Open connects to the server described by the given |
1028 | // info, waits for it to be initialized, and returns a new State |
1029 | // representing the environment connected to. |
1030 | -// It returns unauthorizedError if access is unauthorized. |
1031 | -func Open(info *Info, opts DialOpts) (*State, error) { |
1032 | +// |
1033 | +// A policy may be provided, which will be used to validate and |
1034 | +// modify behaviour of certain operations in state. A nil policy |
1035 | +// may be provided. |
1036 | +// |
1037 | +// Open returns unauthorizedError if access is unauthorized. |
1038 | +func Open(info *Info, opts DialOpts, policy Policy) (*State, error) { |
1039 | logger.Infof("opening state; mongo addresses: %q; entity %q", info.Addrs, info.Tag) |
1040 | if len(info.Addrs) == 0 { |
1041 | return nil, stderrors.New("no mongo addresses") |
1042 | @@ -110,7 +115,7 @@ |
1043 | return nil, err |
1044 | } |
1045 | logger.Infof("connection established") |
1046 | - st, err := newState(session, info) |
1047 | + st, err := newState(session, info, policy) |
1048 | if err != nil { |
1049 | session.Close() |
1050 | return nil, err |
1051 | @@ -122,8 +127,8 @@ |
1052 | // Initialize sets up an initial empty state and returns it. |
1053 | // This needs to be performed only once for a given environment. |
1054 | // It returns unauthorizedError if access is unauthorized. |
1055 | -func Initialize(info *Info, cfg *config.Config, opts DialOpts) (rst *State, err error) { |
1056 | - st, err := Open(info, opts) |
1057 | +func Initialize(info *Info, cfg *config.Config, opts DialOpts, policy Policy) (rst *State, err error) { |
1058 | + st, err := Open(info, opts, policy) |
1059 | if err != nil { |
1060 | return nil, err |
1061 | } |
1062 | @@ -217,7 +222,7 @@ |
1063 | return false |
1064 | } |
1065 | |
1066 | -func newState(session *mgo.Session, info *Info) (*State, error) { |
1067 | +func newState(session *mgo.Session, info *Info, policy Policy) (*State, error) { |
1068 | db := session.DB("juju") |
1069 | pdb := session.DB("presence") |
1070 | if info.Tag != "" { |
1071 | @@ -235,6 +240,7 @@ |
1072 | } |
1073 | st := &State{ |
1074 | info: info, |
1075 | + policy: policy, |
1076 | db: db, |
1077 | environments: db.C("environments"), |
1078 | charms: db.C("charms"), |
1079 | |
1080 | === added file 'state/policy.go' |
1081 | --- state/policy.go 1970-01-01 00:00:00 +0000 |
1082 | +++ state/policy.go 2014-02-20 08:24:11 +0000 |
1083 | @@ -0,0 +1,63 @@ |
1084 | +// Copyright 2014 Canonical Ltd. |
1085 | +// Licensed under the AGPLv3, see LICENCE file for details. |
1086 | + |
1087 | +package state |
1088 | + |
1089 | +import ( |
1090 | + "fmt" |
1091 | + |
1092 | + "launchpad.net/juju-core/constraints" |
1093 | + "launchpad.net/juju-core/environs/config" |
1094 | + "launchpad.net/juju-core/errors" |
1095 | +) |
1096 | + |
1097 | +// Policy is an interface provided to State that may |
1098 | +// be consulted by State to validate or modify the |
1099 | +// behaviour of certain operations. |
1100 | +// |
1101 | +// If a Policy implementation does not implement one |
1102 | +// of the methods, it must return an error that |
1103 | +// satisfies errors.IsNotImplemented, and will thus |
1104 | +// be ignored. Any other error will cause an error |
1105 | +// in the use of the policy. |
1106 | +type Policy interface { |
1107 | + // Prechecker takes a *config.Config and returns |
1108 | + // a (possibly nil) Prechecker or an error. |
1109 | + Prechecker(*config.Config) (Prechecker, error) |
1110 | +} |
1111 | + |
1112 | +// Prechecker is a policy interface that is provided to State |
1113 | +// to perform pre-flight checking of instance creation. |
1114 | +type Prechecker interface { |
1115 | + // PrecheckInstance performs a preflight check on the specified |
1116 | + // series and constraints, ensuring that they are possibly valid for |
1117 | + // creating an instance in this environment. |
1118 | + // |
1119 | + // PrecheckInstance is best effort, and not guaranteed to eliminate |
1120 | + // all invalid parameters. If PrecheckInstance returns nil, it is not |
1121 | + // guaranteed that the constraints are valid; if a non-nil error is |
1122 | + // returned, then the constraints are definitely invalid. |
1123 | + PrecheckInstance(series string, cons constraints.Value) error |
1124 | +} |
1125 | + |
1126 | +// precheckInstance calls the state's assigned policy, if non-nil, to obtain |
1127 | +// a Prechecker, and calls PrecheckInstance if a non-nil Prechecker is returned. |
1128 | +func (st *State) precheckInstance(series string, cons constraints.Value) error { |
1129 | + if st.policy == nil { |
1130 | + return nil |
1131 | + } |
1132 | + cfg, err := st.EnvironConfig() |
1133 | + if err != nil { |
1134 | + return err |
1135 | + } |
1136 | + prechecker, err := st.policy.Prechecker(cfg) |
1137 | + if errors.IsNotImplementedError(err) { |
1138 | + return nil |
1139 | + } else if err != nil { |
1140 | + return err |
1141 | + } |
1142 | + if prechecker == nil { |
1143 | + return fmt.Errorf("policy returned nil prechecker without an error") |
1144 | + } |
1145 | + return prechecker.PrecheckInstance(series, cons) |
1146 | +} |
1147 | |
1148 | === added file 'state/prechecker_test.go' |
1149 | --- state/prechecker_test.go 1970-01-01 00:00:00 +0000 |
1150 | +++ state/prechecker_test.go 2014-02-20 08:24:11 +0000 |
1151 | @@ -0,0 +1,132 @@ |
1152 | +// Copyright 2014 Canonical Ltd. |
1153 | +// Licensed under the AGPLv3, see LICENCE file for details. |
1154 | + |
1155 | +package state_test |
1156 | + |
1157 | +import ( |
1158 | + "fmt" |
1159 | + |
1160 | + gc "launchpad.net/gocheck" |
1161 | + |
1162 | + "launchpad.net/juju-core/constraints" |
1163 | + "launchpad.net/juju-core/environs/config" |
1164 | + "launchpad.net/juju-core/errors" |
1165 | + "launchpad.net/juju-core/instance" |
1166 | + "launchpad.net/juju-core/state" |
1167 | +) |
1168 | + |
1169 | +type PrecheckerSuite struct { |
1170 | + ConnSuite |
1171 | + prechecker mockPrechecker |
1172 | +} |
1173 | + |
1174 | +var _ = gc.Suite(&PrecheckerSuite{}) |
1175 | + |
1176 | +type mockPrechecker struct { |
1177 | + precheckInstanceError error |
1178 | + precheckInstanceSeries string |
1179 | + precheckInstanceConstraints constraints.Value |
1180 | +} |
1181 | + |
1182 | +func (p *mockPrechecker) PrecheckInstance(series string, cons constraints.Value) error { |
1183 | + p.precheckInstanceSeries = series |
1184 | + p.precheckInstanceConstraints = cons |
1185 | + return p.precheckInstanceError |
1186 | +} |
1187 | + |
1188 | +func (s *PrecheckerSuite) SetUpTest(c *gc.C) { |
1189 | + s.ConnSuite.SetUpTest(c) |
1190 | + s.prechecker = mockPrechecker{} |
1191 | + s.policy.getPrechecker = func(*config.Config) (state.Prechecker, error) { |
1192 | + return &s.prechecker, nil |
1193 | + } |
1194 | +} |
1195 | + |
1196 | +func (s *PrecheckerSuite) TestPrecheckInstance(c *gc.C) { |
1197 | + // PrecheckInstance should be called with the specified |
1198 | + // series, and the specified constraints merged with the |
1199 | + // environment constraints, when attempting to create an |
1200 | + // instance. |
1201 | + envCons := constraints.MustParse("mem=4G") |
1202 | + template, err := s.addOneMachine(c, envCons) |
1203 | + c.Assert(err, gc.IsNil) |
1204 | + c.Assert(s.prechecker.precheckInstanceSeries, gc.Equals, template.Series) |
1205 | + cons := template.Constraints.WithFallbacks(envCons) |
1206 | + c.Assert(s.prechecker.precheckInstanceConstraints, gc.DeepEquals, cons) |
1207 | +} |
1208 | + |
1209 | +func (s *PrecheckerSuite) TestPrecheckErrors(c *gc.C) { |
1210 | + // Ensure that AddOneMachine fails when PrecheckInstance returns an error. |
1211 | + s.prechecker.precheckInstanceError = fmt.Errorf("no instance for you") |
1212 | + _, err := s.addOneMachine(c, constraints.Value{}) |
1213 | + c.Assert(err, gc.ErrorMatches, ".*no instance for you") |
1214 | + |
1215 | + // If the policy's Prechecker method fails, that will be returned first. |
1216 | + s.policy.getPrechecker = func(*config.Config) (state.Prechecker, error) { |
1217 | + return nil, fmt.Errorf("no prechecker for you") |
1218 | + } |
1219 | + _, err = s.addOneMachine(c, constraints.Value{}) |
1220 | + c.Assert(err, gc.ErrorMatches, ".*no prechecker for you") |
1221 | +} |
1222 | + |
1223 | +func (s *PrecheckerSuite) TestPrecheckPrecheckerUnimplemented(c *gc.C) { |
1224 | + var precheckerErr error |
1225 | + s.policy.getPrechecker = func(*config.Config) (state.Prechecker, error) { |
1226 | + return nil, precheckerErr |
1227 | + } |
1228 | + _, err := s.addOneMachine(c, constraints.Value{}) |
1229 | + c.Assert(err, gc.ErrorMatches, "cannot add a new machine: policy returned nil prechecker without an error") |
1230 | + precheckerErr = errors.NewNotImplementedError("Prechecker") |
1231 | + _, err = s.addOneMachine(c, constraints.Value{}) |
1232 | + c.Assert(err, gc.IsNil) |
1233 | +} |
1234 | + |
1235 | +func (s *PrecheckerSuite) TestPrecheckNoPolicy(c *gc.C) { |
1236 | + s.policy.getPrechecker = func(*config.Config) (state.Prechecker, error) { |
1237 | + c.Errorf("should not have been invoked") |
1238 | + return nil, nil |
1239 | + } |
1240 | + state.SetPolicy(s.State, nil) |
1241 | + _, err := s.addOneMachine(c, constraints.Value{}) |
1242 | + c.Assert(err, gc.IsNil) |
1243 | +} |
1244 | + |
1245 | +func (s *PrecheckerSuite) addOneMachine(c *gc.C, envCons constraints.Value) (state.MachineTemplate, error) { |
1246 | + err := s.State.SetEnvironConstraints(envCons) |
1247 | + c.Assert(err, gc.IsNil) |
1248 | + oneJob := []state.MachineJob{state.JobHostUnits} |
1249 | + extraCons := constraints.MustParse("cpu-cores=4") |
1250 | + template := state.MachineTemplate{ |
1251 | + Series: "precise", |
1252 | + Constraints: extraCons, |
1253 | + Jobs: oneJob, |
1254 | + } |
1255 | + _, err = s.State.AddOneMachine(template) |
1256 | + return template, err |
1257 | +} |
1258 | + |
1259 | +func (s *PrecheckerSuite) TestPrecheckInstanceInjectMachine(c *gc.C) { |
1260 | + template := state.MachineTemplate{ |
1261 | + InstanceId: instance.Id("bootstrap"), |
1262 | + Series: "precise", |
1263 | + Nonce: state.BootstrapNonce, |
1264 | + Jobs: []state.MachineJob{state.JobManageEnviron}, |
1265 | + } |
1266 | + _, err := s.State.AddOneMachine(template) |
1267 | + c.Assert(err, gc.IsNil) |
1268 | + // PrecheckInstance should not have been called, as we've |
1269 | + // injected a machine with an existing instance. |
1270 | + c.Assert(s.prechecker.precheckInstanceSeries, gc.Equals, "") |
1271 | +} |
1272 | + |
1273 | +func (s *PrecheckerSuite) TestPrecheckContainerNewMachine(c *gc.C) { |
1274 | + // Attempting to add a container to a new machine should cause |
1275 | + // PrecheckInstance to be called. |
1276 | + template := state.MachineTemplate{ |
1277 | + Series: "precise", |
1278 | + Jobs: []state.MachineJob{state.JobHostUnits}, |
1279 | + } |
1280 | + _, err := s.State.AddMachineInsideNewMachine(template, template, instance.LXC) |
1281 | + c.Assert(err, gc.IsNil) |
1282 | + c.Assert(s.prechecker.precheckInstanceSeries, gc.Equals, template.Series) |
1283 | +} |
1284 | |
1285 | === modified file 'state/settings_test.go' |
1286 | --- state/settings_test.go 2014-02-07 16:32:23 +0000 |
1287 | +++ state/settings_test.go 2014-02-20 08:24:11 +0000 |
1288 | @@ -53,7 +53,7 @@ |
1289 | s.LoggingSuite.SetUpTest(c) |
1290 | s.MgoSuite.SetUpTest(c) |
1291 | // TODO(dfc) this logic is duplicated with the metawatcher_test. |
1292 | - state, err := Open(TestingStateInfo(), TestingDialOpts()) |
1293 | + state, err := Open(TestingStateInfo(), TestingDialOpts(), Policy(nil)) |
1294 | c.Assert(err, gc.IsNil) |
1295 | |
1296 | s.state = state |
1297 | |
1298 | === modified file 'state/state.go' |
1299 | --- state/state.go 2014-02-05 17:46:02 +0000 |
1300 | +++ state/state.go 2014-02-20 08:24:11 +0000 |
1301 | @@ -44,6 +44,7 @@ |
1302 | // managed by juju. |
1303 | type State struct { |
1304 | info *Info |
1305 | + policy Policy |
1306 | db *mgo.Database |
1307 | environments *mgo.Collection |
1308 | charms *mgo.Collection |
1309 | |
1310 | === modified file 'state/state_test.go' |
1311 | --- state/state_test.go 2014-02-14 11:39:37 +0000 |
1312 | +++ state/state_test.go 2014-02-20 08:24:11 +0000 |
1313 | @@ -54,7 +54,7 @@ |
1314 | func (s *StateSuite) TestDialAgain(c *gc.C) { |
1315 | // Ensure idempotent operations on Dial are working fine. |
1316 | for i := 0; i < 2; i++ { |
1317 | - st, err := state.Open(state.TestingStateInfo(), state.TestingDialOpts()) |
1318 | + st, err := state.Open(state.TestingStateInfo(), state.TestingDialOpts(), state.Policy(nil)) |
1319 | c.Assert(err, gc.IsNil) |
1320 | c.Assert(st.Close(), gc.IsNil) |
1321 | } |
1322 | @@ -1854,7 +1854,7 @@ |
1323 | } |
1324 | |
1325 | func tryOpenState(info *state.Info) error { |
1326 | - st, err := state.Open(info, state.TestingDialOpts()) |
1327 | + st, err := state.Open(info, state.TestingDialOpts(), state.Policy(nil)) |
1328 | if err == nil { |
1329 | st.Close() |
1330 | } |
1331 | @@ -1881,7 +1881,7 @@ |
1332 | info.Addrs = []string{"0.1.2.3:1234"} |
1333 | st, err := state.Open(info, state.DialOpts{ |
1334 | Timeout: 1 * time.Millisecond, |
1335 | - }) |
1336 | + }, state.Policy(nil)) |
1337 | if err == nil { |
1338 | st.Close() |
1339 | } |
1340 | @@ -1897,7 +1897,7 @@ |
1341 | t0 := time.Now() |
1342 | st, err := state.Open(info, state.DialOpts{ |
1343 | Timeout: 1 * time.Millisecond, |
1344 | - }) |
1345 | + }, state.Policy(nil)) |
1346 | if err == nil { |
1347 | st.Close() |
1348 | } |
1349 | @@ -1993,7 +1993,7 @@ |
1350 | |
1351 | func testSetMongoPassword(c *gc.C, getEntity func(st *state.State) (entity, error)) { |
1352 | info := state.TestingStateInfo() |
1353 | - st, err := state.Open(info, state.TestingDialOpts()) |
1354 | + st, err := state.Open(info, state.TestingDialOpts(), state.Policy(nil)) |
1355 | c.Assert(err, gc.IsNil) |
1356 | defer st.Close() |
1357 | // Turn on fully-authenticated mode. |
1358 | @@ -2014,7 +2014,7 @@ |
1359 | |
1360 | // Check that we can log in with the correct password. |
1361 | info.Password = "foo" |
1362 | - st1, err := state.Open(info, state.TestingDialOpts()) |
1363 | + st1, err := state.Open(info, state.TestingDialOpts(), state.Policy(nil)) |
1364 | c.Assert(err, gc.IsNil) |
1365 | defer st1.Close() |
1366 | |
1367 | @@ -2634,7 +2634,7 @@ |
1368 | // interact with the closed state, causing it to return an |
1369 | // unexpected error (often "Closed explictly"). |
1370 | func testWatcherDiesWhenStateCloses(c *gc.C, startWatcher func(c *gc.C, st *state.State) waiter) { |
1371 | - st, err := state.Open(state.TestingStateInfo(), state.TestingDialOpts()) |
1372 | + st, err := state.Open(state.TestingStateInfo(), state.TestingDialOpts(), state.Policy(nil)) |
1373 | c.Assert(err, gc.IsNil) |
1374 | watcher := startWatcher(c, st) |
1375 | err = st.Close() |
1376 | @@ -2676,7 +2676,7 @@ |
1377 | c.Assert(err, gc.NotNil) |
1378 | c.Assert(info, gc.IsNil) |
1379 | |
1380 | - st, err := state.Open(state.TestingStateInfo(), state.TestingDialOpts()) |
1381 | + st, err := state.Open(state.TestingStateInfo(), state.TestingDialOpts(), state.Policy(nil)) |
1382 | c.Assert(err, gc.IsNil) |
1383 | defer st.Close() |
1384 | |
1385 | @@ -2695,7 +2695,7 @@ |
1386 | c.Assert(err, gc.IsNil) |
1387 | c.Assert(info, jc.DeepEquals, &state.StateServerInfo{}) |
1388 | |
1389 | - st, err := state.Open(state.TestingStateInfo(), state.TestingDialOpts()) |
1390 | + st, err := state.Open(state.TestingStateInfo(), state.TestingDialOpts(), state.Policy(nil)) |
1391 | c.Assert(err, gc.IsNil) |
1392 | defer st.Close() |
1393 | |
1394 | @@ -2726,7 +2726,7 @@ |
1395 | c.Assert(info.MachineIds, gc.HasLen, 0) |
1396 | c.Assert(info.VotingMachineIds, gc.HasLen, 0) |
1397 | |
1398 | - st, err := state.Open(state.TestingStateInfo(), state.TestingDialOpts()) |
1399 | + st, err := state.Open(state.TestingStateInfo(), state.TestingDialOpts(), state.Policy(nil)) |
1400 | c.Assert(err, gc.IsNil) |
1401 | defer st.Close() |
1402 | |
1403 | |
1404 | === modified file 'state/unit_test.go' |
1405 | --- state/unit_test.go 2014-01-15 06:52:56 +0000 |
1406 | +++ state/unit_test.go 2014-02-20 08:24:11 +0000 |
1407 | @@ -633,7 +633,7 @@ |
1408 | c.Assert(err, gc.IsNil) |
1409 | |
1410 | info := state.TestingStateInfo() |
1411 | - st, err := state.Open(info, state.TestingDialOpts()) |
1412 | + st, err := state.Open(info, state.TestingDialOpts(), state.Policy(nil)) |
1413 | c.Assert(err, gc.IsNil) |
1414 | defer st.Close() |
1415 | // Turn on fully-authenticated mode. |
1416 | @@ -663,7 +663,7 @@ |
1417 | // Connect as the machine entity. |
1418 | info.Tag = m.Tag() |
1419 | info.Password = "foo" |
1420 | - st1, err := state.Open(info, state.TestingDialOpts()) |
1421 | + st1, err := state.Open(info, state.TestingDialOpts(), state.Policy(nil)) |
1422 | c.Assert(err, gc.IsNil) |
1423 | defer st1.Close() |
1424 | |
1425 | @@ -678,7 +678,7 @@ |
1426 | // that entity, change the password for a new unit. |
1427 | info.Tag = unit.Tag() |
1428 | info.Password = "bar" |
1429 | - st2, err := state.Open(info, state.TestingDialOpts()) |
1430 | + st2, err := state.Open(info, state.TestingDialOpts(), state.Policy(nil)) |
1431 | c.Assert(err, gc.IsNil) |
1432 | defer st2.Close() |
1433 | |
1434 | |
1435 | === modified file 'worker/provisioner/provisioner_test.go' |
1436 | --- worker/provisioner/provisioner_test.go 2014-01-28 12:29:38 +0000 |
1437 | +++ worker/provisioner/provisioner_test.go 2014-02-20 08:24:11 +0000 |
1438 | @@ -57,6 +57,12 @@ |
1439 | } |
1440 | |
1441 | func (s *CommonProvisionerSuite) SetUpTest(c *gc.C) { |
1442 | + // Disable the default state policy, because the |
1443 | + // provisioner needs to be able to test pathological |
1444 | + // scenarios where a machine exists in state with |
1445 | + // invalid environment config. |
1446 | + dummy.SetStatePolicy(nil) |
1447 | + |
1448 | s.JujuConnSuite.SetUpTest(c) |
1449 | // Create the operations channel with more than enough space |
1450 | // for those tests that don't listen on it. |
Reviewers: mp+205700_ code.launchpad. net,
Message:
Please take a look.
Description:
Wire up prechecker
This change is to set a Prechecker on
the State object used by the apiserver
in cmd/jujud. The Environ itself is a
Prechecker, and that is what is assigned
to the State object.
State now calls the prechecker when
deciding to create instance/container
machine entries in state. The prechecker
calls are elided for machines with
instance IDs (i.e. "injected" machines).
All Environs, apart from MAAS, disallow
containers. The null provider disallows
everything (unless done "manually").
This CL supersedes https:/ /codereview. appspot. com/14032043/
The primary differences are:
- up-to-date with latest codebase
- prechecker is not set on State in juju.NewConn
methods; prechecking simply will not occur for
old environments
- Environ does not embed Prechecker, and there
is no new EnvironBase struct; this will be
proposed as a follow-up
The old CL stalled because previously an environment
config would be invalid at first startup of jujud,
because it would lack secrets. This has changed
with synchronous bootstrap and is no longer an issue.
https:/ /code.launchpad .net/~axwalk/ juju-core/ wire-up- prechecker- take2/+ merge/205700
(do not edit description out of merge proposal)
Please review this at https:/ /codereview. appspot. com/61520045/
Affected files (+251, -34 lines): machine. go interface. go jujutest/ livetests. go azure/environ. go ec2/local_ test.go local/environ. go local/environ_ test.go manual/ environ. go openstack/ local_test. go openstack/ provider. go r_test. go
A [revision details]
M cmd/jujud/agent.go
M cmd/jujud/
M environs/
M environs/
M provider/
M provider/ec2/ec2.go
M provider/
M provider/
M provider/
M provider/
M provider/
M provider/
M state/addmachine.go
A state/prechecke
M state/state.go