juju-core

Merge lp:~dimitern/juju-core/148-apiuniter-fix-configsettings into lp:~go-bot/juju-core/trunk

  1. 148-apiuniter-fix-configsettings
  2. Merge into trunk
Proposed by Dimiter Naydenov
Status: Merged
Approved by: Dimiter Naydenov
Approved revision: no longer in the source branch.
Merged at revision: 1893
Proposed branch: lp:~dimitern/juju-core/148-apiuniter-fix-configsettings
Merge into: lp:~go-bot/juju-core/trunk
Prerequisite: lp:~dimitern/juju-core/147-apiprovisioner-blank-env-secrets
Diff against target: 747 lines (+177/-113)
16 files modified
state/api/params/internal.go (+37/-22)
state/api/provisioner/provisioner.go (+1/-1)
state/api/uniter/relationunit.go (+3/-3)
state/api/uniter/relationunit_test.go (+2/-2)
state/api/uniter/settings.go (+6/-6)
state/api/uniter/settings_test.go (+11/-11)
state/api/uniter/unit.go (+2/-10)
state/apiserver/provisioner/provisioner.go (+2/-2)
state/apiserver/provisioner/provisioner_test.go (+1/-1)
state/apiserver/uniter/uniter.go (+24/-19)
state/apiserver/uniter/uniter_test.go (+64/-12)
worker/uniter/context.go (+2/-2)
worker/uniter/context_test.go (+15/-15)
worker/uniter/jujuc/context.go (+2/-2)
worker/uniter/jujuc/relation-get.go (+1/-1)
worker/uniter/jujuc/util_test.go (+4/-4)
To merge this branch: bzr merge lp:~dimitern/juju-core/148-apiuniter-fix-configsettings
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+187824@code.launchpad.net

Commit message

apiuniter: Fix lp:1231457

This changes several params types, while keeping
the field names for backward compatibility.
And changes the result of ConfigSettings() call
to return map[string]interface{}, because charm
settings are different from relation settings
(which are always strings), and coercing charm
settings to stings discards non-string config
settings, so this is fixed here.

https://codereview.appspot.com/13908044/

R=fwereade

Description of the change

apiuniter: Fix lp:1231457

This changes several params types, while keeping
the field names for backward compatibility.
And changes the result of ConfigSettings() call
to return map[string]interface{}, because charm
settings are different from relation settings
(which are always strings), and coercing charm
settings to stings discards non-string config
settings, so this is fixed here.

https://codereview.appspot.com/13908044/

To post a comment you must log in.
Revision history for this message
Dimiter Naydenov (dimitern) wrote :

Reviewers: mp+187824_code.launchpad.net,

Message:
Please take a look.

Description:
apiuniter: Fix lp:1231457

This changes several params types, while keeping
the field names for backward compatibility.
And changes the result of ConfigSettings() call
to return map[string]interface{}, because charm
settings are different from relation settings
(which are always strings), and coercing charm
settings to stings discards non-string config
settings, so this is fixed here.

https://code.launchpad.net/~dimitern/juju-core/148-apiuniter-fix-configsettings/+merge/187824

Requires:
https://code.launchpad.net/~dimitern/juju-core/147-apiprovisioner-blank-env-secrets/+merge/187738

(do not edit description out of merge proposal)

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

Affected files (+121, -111 lines):
   A [revision details]
   M state/api/params/internal.go
   M state/api/provisioner/provisioner.go
   M state/api/uniter/relationunit.go
   M state/api/uniter/relationunit_test.go
   M state/api/uniter/settings.go
   M state/api/uniter/settings_test.go
   M state/api/uniter/unit.go
   M state/apiserver/provisioner/provisioner.go
   M state/apiserver/provisioner/provisioner_test.go
   M state/apiserver/uniter/uniter.go
   M state/apiserver/uniter/uniter_test.go
   M worker/uniter/context.go
   M worker/uniter/context_test.go
   M worker/uniter/jujuc/context.go
   M worker/uniter/jujuc/relation-get.go
   M worker/uniter/jujuc/util_test.go

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

LGTM with an error return on unexpected relation settings.

https://codereview.appspot.com/13908044/diff/1/state/api/params/internal.go
File state/api/params/internal.go (right):

https://codereview.appspot.com/13908044/diff/1/state/api/params/internal.go#newcode138
state/api/params/internal.go:138: // EnvironConfig holds an environment
configuration.
Man, our vocabulary is still kinda screwed up here... "Config" means too
many things. Not much we can do about it right now though.

https://codereview.appspot.com/13908044/diff/1/state/api/uniter/unit.go
File state/api/uniter/unit.go (right):

https://codereview.appspot.com/13908044/diff/1/state/api/uniter/unit.go#newcode143
state/api/uniter/unit.go:143: return
map[string]interface{}(result.Settings), nil
can you not do charm.Settings(result.Settings)?

https://codereview.appspot.com/13908044/diff/1/state/apiserver/uniter/uniter.go
File state/apiserver/uniter/uniter.go (left):

https://codereview.appspot.com/13908044/diff/1/state/apiserver/uniter/uniter.go#oldcode797
state/apiserver/uniter/uniter.go:797: // All relation settings should be
strings.
Hmm. This is almost legitimate-panic territory. Best not, since we're
running in the API server and will cause a lot of collateral damage, but
I think we should at least error out -- if it happens, we're clearly not
in Kansas any more.

https://codereview.appspot.com/13908044/

Revision history for this message
Dimiter Naydenov (dimitern) wrote :

Please take a look.

https://codereview.appspot.com/13908044/diff/1/state/api/uniter/unit.go
File state/api/uniter/unit.go (right):

https://codereview.appspot.com/13908044/diff/1/state/api/uniter/unit.go#newcode143
state/api/uniter/unit.go:143: return
map[string]interface{}(result.Settings), nil
On 2013/09/26 16:11:52, fwereade wrote:
> can you not do charm.Settings(result.Settings)?

Done.

https://codereview.appspot.com/13908044/diff/1/state/apiserver/uniter/uniter.go
File state/apiserver/uniter/uniter.go (left):

https://codereview.appspot.com/13908044/diff/1/state/apiserver/uniter/uniter.go#oldcode797
state/apiserver/uniter/uniter.go:797: // All relation settings should be
strings.
On 2013/09/26 16:11:52, fwereade wrote:
> Hmm. This is almost legitimate-panic territory. Best not, since we're
 running in
> the API server and will cause a lot of collateral damage, but I think
 we should
> at least error out -- if it happens, we're clearly not in Kansas any
 more.

Done. Also added a couple of tests for the error.

https://codereview.appspot.com/13908044/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'state/api/params/internal.go'
2--- state/api/params/internal.go 2013-09-24 08:55:01 +0000
3+++ state/api/params/internal.go 2013-09-26 17:24:28 +0000
4@@ -105,28 +105,43 @@
5 Results []BoolResult
6 }
7
8-// Settings holds charm config options names and values.
9-type Settings map[string]string
10-
11-// SettingsResult holds a charm settings map or an error.
12-type SettingsResult struct {
13- Error *Error
14- Settings Settings
15-}
16-
17-// SettingsResults holds the result of an API calls that returns
18-// settings for multiple entities.
19-type SettingsResults struct {
20- Results []SettingsResult
21-}
22-
23-// Config holds configuration with string keys and arbitrary values.
24-type Config map[string]interface{}
25-
26-// ConfigResult holds a configuration map or an error.
27-type ConfigResult struct {
28+// RelationSettings holds relation settings names and values.
29+type RelationSettings map[string]string
30+
31+// RelationSettingsResult holds a relation settings map or an error.
32+type RelationSettingsResult struct {
33+ Error *Error
34+ Settings RelationSettings
35+}
36+
37+// RelationSettingsResults holds the result of an API calls that
38+// returns settings for multiple relations.
39+type RelationSettingsResults struct {
40+ Results []RelationSettingsResult
41+}
42+
43+// ConfigSettings holds unit, service or cham configuration settings
44+// with string keys and arbitrary values.
45+type ConfigSettings map[string]interface{}
46+
47+// ConfigSettingsResult holds a configuration map or an error.
48+type ConfigSettingsResult struct {
49+ Error *Error
50+ Settings ConfigSettings
51+}
52+
53+// ConfigSettingsResults holds multiple configuration maps or errors.
54+type ConfigSettingsResults struct {
55+ Results []ConfigSettingsResult
56+}
57+
58+// EnvironConfig holds an environment configuration.
59+type EnvironConfig map[string]interface{}
60+
61+// EnvironConfigResult holds environment configuration or an error.
62+type EnvironConfigResult struct {
63 Error *Error
64- Config Config
65+ Config EnvironConfig
66 }
67
68 // RelationUnit holds a relation and a unit tag.
69@@ -164,7 +179,7 @@
70 type RelationUnitSettings struct {
71 Relation string
72 Unit string
73- Settings Settings
74+ Settings RelationSettings
75 }
76
77 // RelationUnitsSettings holds the arguments for making a EnterScope
78
79=== modified file 'state/api/provisioner/provisioner.go'
80--- state/api/provisioner/provisioner.go 2013-09-26 10:38:59 +0000
81+++ state/api/provisioner/provisioner.go 2013-09-26 17:24:28 +0000
82@@ -71,7 +71,7 @@
83
84 // EnvironConfig returns the current environment configuration.
85 func (st *State) EnvironConfig() (*config.Config, error) {
86- var result params.ConfigResult
87+ var result params.EnvironConfigResult
88 err := st.caller.Call("Provisioner", "", "EnvironConfig", nil, &result)
89 if err != nil {
90 return nil, err
91
92=== modified file 'state/api/uniter/relationunit.go'
93--- state/api/uniter/relationunit.go 2013-09-10 17:00:19 +0000
94+++ state/api/uniter/relationunit.go 2013-09-26 17:24:28 +0000
95@@ -105,7 +105,7 @@
96 // Settings returns a Settings which allows access to the unit's settings
97 // within the relation.
98 func (ru *RelationUnit) Settings() (*Settings, error) {
99- var results params.SettingsResults
100+ var results params.RelationSettingsResults
101 args := params.RelationUnits{
102 RelationUnits: []params.RelationUnit{{
103 Relation: ru.relation.tag,
104@@ -133,9 +133,9 @@
105 // unit is not grounds for an error, because the unit settings are
106 // guaranteed to persist for the lifetime of the relation, regardless
107 // of the lifetime of the unit.
108-func (ru *RelationUnit) ReadSettings(uname string) (params.Settings, error) {
109+func (ru *RelationUnit) ReadSettings(uname string) (params.RelationSettings, error) {
110 tag := names.UnitTag(uname)
111- var results params.SettingsResults
112+ var results params.RelationSettingsResults
113 args := params.RelationUnitPairs{
114 RelationUnitPairs: []params.RelationUnitPair{{
115 Relation: ru.relation.tag,
116
117=== modified file 'state/api/uniter/relationunit_test.go'
118--- state/api/uniter/relationunit_test.go 2013-09-13 11:35:33 +0000
119+++ state/api/uniter/relationunit_test.go 2013-09-26 17:24:28 +0000
120@@ -196,7 +196,7 @@
121
122 gotSettings, err := apiRelUnit.Settings()
123 c.Assert(err, gc.IsNil)
124- c.Assert(gotSettings.Map(), gc.DeepEquals, params.Settings{
125+ c.Assert(gotSettings.Map(), gc.DeepEquals, params.RelationSettings{
126 "some": "settings",
127 "other": "things",
128 })
129@@ -230,7 +230,7 @@
130 s.assertInScope(c, myRelUnit, true)
131 gotSettings, err = apiRelUnit.ReadSettings("mysql/0")
132 c.Assert(err, gc.IsNil)
133- c.Assert(gotSettings, gc.DeepEquals, params.Settings{
134+ c.Assert(gotSettings, gc.DeepEquals, params.RelationSettings{
135 "some": "settings",
136 "other": "things",
137 })
138
139=== modified file 'state/api/uniter/settings.go'
140--- state/api/uniter/settings.go 2013-09-06 16:22:34 +0000
141+++ state/api/uniter/settings.go 2013-09-26 17:24:28 +0000
142@@ -15,12 +15,12 @@
143 st *State
144 relationTag string
145 unitTag string
146- settings params.Settings
147+ settings params.RelationSettings
148 }
149
150-func newSettings(st *State, relationTag, unitTag string, settings params.Settings) *Settings {
151+func newSettings(st *State, relationTag, unitTag string, settings params.RelationSettings) *Settings {
152 if settings == nil {
153- settings = make(params.Settings)
154+ settings = make(params.RelationSettings)
155 }
156 return &Settings{
157 st: st,
158@@ -36,8 +36,8 @@
159 // not return map[string]interface{}, but since all values are
160 // expected to be strings anyway, we need to fix the uniter code
161 // accordingly when migrating to the API.
162-func (s *Settings) Map() params.Settings {
163- settingsCopy := make(params.Settings)
164+func (s *Settings) Map() params.RelationSettings {
165+ settingsCopy := make(params.RelationSettings)
166 for k, v := range s.settings {
167 if v != "" {
168 // Skip deleted keys.
169@@ -75,7 +75,7 @@
170 // without overwritting.
171 func (s *Settings) Write() error {
172 // First make a copy of the map, including deleted keys.
173- settingsCopy := make(params.Settings)
174+ settingsCopy := make(params.RelationSettings)
175 for k, v := range s.settings {
176 settingsCopy[k] = v
177 }
178
179=== modified file 'state/api/uniter/settings_test.go'
180--- state/api/uniter/settings_test.go 2013-09-06 15:39:38 +0000
181+++ state/api/uniter/settings_test.go 2013-09-26 17:24:28 +0000
182@@ -34,7 +34,7 @@
183 c.Assert(theMap, gc.HasLen, 0)
184
185 // And also accepts a populated map, and returns a converted map.
186- rawSettings := params.Settings{
187+ rawSettings := params.RelationSettings{
188 "some": "settings",
189 "other": "stuff",
190 }
191@@ -47,15 +47,15 @@
192 settings := uniter.NewSettings(s.uniter, "blah", "foo", nil)
193
194 settings.Set("foo", "bar")
195- c.Assert(settings.Map(), gc.DeepEquals, params.Settings{
196+ c.Assert(settings.Map(), gc.DeepEquals, params.RelationSettings{
197 "foo": "bar",
198 })
199 settings.Set("foo", "qaz")
200- c.Assert(settings.Map(), gc.DeepEquals, params.Settings{
201+ c.Assert(settings.Map(), gc.DeepEquals, params.RelationSettings{
202 "foo": "qaz",
203 })
204 settings.Set("bar", "Cheers")
205- c.Assert(settings.Map(), gc.DeepEquals, params.Settings{
206+ c.Assert(settings.Map(), gc.DeepEquals, params.RelationSettings{
207 "foo": "qaz",
208 "bar": "Cheers",
209 })
210@@ -67,27 +67,27 @@
211 settings.Set("foo", "qaz")
212 settings.Set("abc", "tink")
213 settings.Set("bar", "tonk")
214- c.Assert(settings.Map(), gc.DeepEquals, params.Settings{
215+ c.Assert(settings.Map(), gc.DeepEquals, params.RelationSettings{
216 "foo": "qaz",
217 "abc": "tink",
218 "bar": "tonk",
219 })
220 settings.Delete("abc")
221- c.Assert(settings.Map(), gc.DeepEquals, params.Settings{
222+ c.Assert(settings.Map(), gc.DeepEquals, params.RelationSettings{
223 "foo": "qaz",
224 "bar": "tonk",
225 })
226 settings.Delete("bar")
227- c.Assert(settings.Map(), gc.DeepEquals, params.Settings{
228+ c.Assert(settings.Map(), gc.DeepEquals, params.RelationSettings{
229 "foo": "qaz",
230 })
231 settings.Set("abc", "123")
232- c.Assert(settings.Map(), gc.DeepEquals, params.Settings{
233+ c.Assert(settings.Map(), gc.DeepEquals, params.RelationSettings{
234 "foo": "qaz",
235 "abc": "123",
236 })
237 settings.Delete("missing")
238- c.Assert(settings.Map(), gc.DeepEquals, params.Settings{
239+ c.Assert(settings.Map(), gc.DeepEquals, params.RelationSettings{
240 "foo": "qaz",
241 "abc": "123",
242 })
243@@ -112,7 +112,7 @@
244 c.Assert(err, gc.IsNil)
245 settings, err := apiRelUnit.Settings()
246 c.Assert(err, gc.IsNil)
247- c.Assert(settings.Map(), gc.DeepEquals, params.Settings{
248+ c.Assert(settings.Map(), gc.DeepEquals, params.RelationSettings{
249 "some": "stuff",
250 "other": "things",
251 })
252@@ -126,7 +126,7 @@
253 c.Assert(err, gc.IsNil)
254 settings, err = apiRelUnit.Settings()
255 c.Assert(err, gc.IsNil)
256- c.Assert(settings.Map(), gc.DeepEquals, params.Settings{
257+ c.Assert(settings.Map(), gc.DeepEquals, params.RelationSettings{
258 "foo": "qaz",
259 "other": "days",
260 })
261
262=== modified file 'state/api/uniter/unit.go'
263--- state/api/uniter/unit.go 2013-09-17 18:18:44 +0000
264+++ state/api/uniter/unit.go 2013-09-26 17:24:28 +0000
265@@ -120,20 +120,12 @@
266 return service, nil
267 }
268
269-func convertSettings(input params.Settings) charm.Settings {
270- result := make(charm.Settings)
271- for k, v := range input {
272- result[k] = v
273- }
274- return result
275-}
276-
277 // ConfigSettings returns the complete set of service charm config settings
278 // available to the unit. Unset values will be replaced with the default
279 // value for the associated option, and may thus be nil when no default is
280 // specified.
281 func (u *Unit) ConfigSettings() (charm.Settings, error) {
282- var results params.SettingsResults
283+ var results params.ConfigSettingsResults
284 args := params.Entities{
285 Entities: []params.Entity{{Tag: u.tag}},
286 }
287@@ -148,7 +140,7 @@
288 if result.Error != nil {
289 return nil, result.Error
290 }
291- return convertSettings(result.Settings), nil
292+ return charm.Settings(result.Settings), nil
293 }
294
295 // ServiceName returns the service name.
296
297=== modified file 'state/apiserver/provisioner/provisioner.go'
298--- state/apiserver/provisioner/provisioner.go 2013-09-26 16:01:47 +0000
299+++ state/apiserver/provisioner/provisioner.go 2013-09-26 17:24:28 +0000
300@@ -152,8 +152,8 @@
301 }
302
303 // EnvironConfig returns the current environment's configuration.
304-func (p *ProvisionerAPI) EnvironConfig() (params.ConfigResult, error) {
305- result := params.ConfigResult{}
306+func (p *ProvisionerAPI) EnvironConfig() (params.EnvironConfigResult, error) {
307+ result := params.EnvironConfigResult{}
308 config, err := p.st.EnvironConfig()
309 if err != nil {
310 return result, err
311
312=== modified file 'state/apiserver/provisioner/provisioner_test.go'
313--- state/apiserver/provisioner/provisioner_test.go 2013-09-26 11:15:31 +0000
314+++ state/apiserver/provisioner/provisioner_test.go 2013-09-26 17:24:28 +0000
315@@ -421,7 +421,7 @@
316 result, err := s.provisioner.EnvironConfig()
317 c.Assert(err, gc.IsNil)
318 c.Assert(result.Error, gc.IsNil)
319- c.Assert(result.Config, gc.DeepEquals, params.Config(envConfig.AllAttrs()))
320+ c.Assert(result.Config, gc.DeepEquals, params.EnvironConfig(envConfig.AllAttrs()))
321
322 // Now test it with a non-environment manager and make sure
323 // the secret attributes are masked.
324
325=== modified file 'state/apiserver/uniter/uniter.go'
326--- state/apiserver/uniter/uniter.go 2013-09-12 14:58:40 +0000
327+++ state/apiserver/uniter/uniter.go 2013-09-26 17:24:28 +0000
328@@ -6,6 +6,8 @@
329 package uniter
330
331 import (
332+ "fmt"
333+
334 "launchpad.net/juju-core/charm"
335 "launchpad.net/juju-core/environs"
336 "launchpad.net/juju-core/errors"
337@@ -489,13 +491,13 @@
338
339 // ConfigSettings returns the complete set of service charm config
340 // settings available to each given unit.
341-func (u *UniterAPI) ConfigSettings(args params.Entities) (params.SettingsResults, error) {
342- result := params.SettingsResults{
343- Results: make([]params.SettingsResult, len(args.Entities)),
344+func (u *UniterAPI) ConfigSettings(args params.Entities) (params.ConfigSettingsResults, error) {
345+ result := params.ConfigSettingsResults{
346+ Results: make([]params.ConfigSettingsResult, len(args.Entities)),
347 }
348 canAccess, err := u.accessUnit()
349 if err != nil {
350- return params.SettingsResults{}, err
351+ return params.ConfigSettingsResults{}, err
352 }
353 for i, entity := range args.Entities {
354 err := common.ErrPerm
355@@ -506,7 +508,7 @@
356 var settings charm.Settings
357 settings, err = unit.ConfigSettings()
358 if err == nil {
359- result.Results[i].Settings = convertSettings(settings)
360+ result.Results[i].Settings = params.ConfigSettings(settings)
361 }
362 }
363 }
364@@ -791,25 +793,28 @@
365 return result, nil
366 }
367
368-func convertSettings(settings map[string]interface{}) params.Settings {
369- result := make(params.Settings)
370+func convertRelationSettings(settings map[string]interface{}) (params.RelationSettings, error) {
371+ result := make(params.RelationSettings)
372 for k, v := range settings {
373 // All relation settings should be strings.
374- sval, _ := v.(string)
375+ sval, ok := v.(string)
376+ if !ok {
377+ return nil, fmt.Errorf("unexpected relation setting %q: expected string, got %T", k, v)
378+ }
379 result[k] = sval
380 }
381- return result
382+ return result, nil
383 }
384
385 // ReadSettings returns the local settings of each given set of
386 // relation/unit.
387-func (u *UniterAPI) ReadSettings(args params.RelationUnits) (params.SettingsResults, error) {
388- result := params.SettingsResults{
389- Results: make([]params.SettingsResult, len(args.RelationUnits)),
390+func (u *UniterAPI) ReadSettings(args params.RelationUnits) (params.RelationSettingsResults, error) {
391+ result := params.RelationSettingsResults{
392+ Results: make([]params.RelationSettingsResult, len(args.RelationUnits)),
393 }
394 canAccess, err := u.accessUnit()
395 if err != nil {
396- return params.SettingsResults{}, err
397+ return params.RelationSettingsResults{}, err
398 }
399 for i, arg := range args.RelationUnits {
400 relUnit, err := u.getRelationUnit(canAccess, arg.Relation, arg.Unit)
401@@ -817,7 +822,7 @@
402 var settings *state.Settings
403 settings, err = relUnit.Settings()
404 if err == nil {
405- result.Results[i].Settings = convertSettings(settings.Map())
406+ result.Results[i].Settings, err = convertRelationSettings(settings.Map())
407 }
408 }
409 result.Results[i].Error = common.ServerError(err)
410@@ -845,13 +850,13 @@
411
412 // ReadRemoteSettings returns the remote settings of each given set of
413 // relation/local unit/remote unit.
414-func (u *UniterAPI) ReadRemoteSettings(args params.RelationUnitPairs) (params.SettingsResults, error) {
415- result := params.SettingsResults{
416- Results: make([]params.SettingsResult, len(args.RelationUnitPairs)),
417+func (u *UniterAPI) ReadRemoteSettings(args params.RelationUnitPairs) (params.RelationSettingsResults, error) {
418+ result := params.RelationSettingsResults{
419+ Results: make([]params.RelationSettingsResult, len(args.RelationUnitPairs)),
420 }
421 canAccess, err := u.accessUnit()
422 if err != nil {
423- return params.SettingsResults{}, err
424+ return params.RelationSettingsResults{}, err
425 }
426 for i, arg := range args.RelationUnitPairs {
427 relUnit, err := u.getRelationUnit(canAccess, arg.Relation, arg.LocalUnit)
428@@ -862,7 +867,7 @@
429 var settings map[string]interface{}
430 settings, err = relUnit.ReadSettings(remoteUnit)
431 if err == nil {
432- result.Results[i].Settings = convertSettings(settings)
433+ result.Results[i].Settings, err = convertRelationSettings(settings)
434 }
435 }
436 }
437
438=== modified file 'state/apiserver/uniter/uniter_test.go'
439--- state/apiserver/uniter/uniter_test.go 2013-09-18 15:57:30 +0000
440+++ state/apiserver/uniter/uniter_test.go 2013-09-26 17:24:28 +0000
441@@ -808,10 +808,10 @@
442 }}
443 result, err := s.uniter.ConfigSettings(args)
444 c.Assert(err, gc.IsNil)
445- c.Assert(result, gc.DeepEquals, params.SettingsResults{
446- Results: []params.SettingsResult{
447+ c.Assert(result, gc.DeepEquals, params.ConfigSettingsResults{
448+ Results: []params.ConfigSettingsResult{
449 {Error: apiservertesting.ErrUnauthorized},
450- {Settings: params.Settings{"blog-title": "My Title"}},
451+ {Settings: params.ConfigSettings{"blog-title": "My Title"}},
452 {Error: apiservertesting.ErrUnauthorized},
453 },
454 })
455@@ -1097,10 +1097,10 @@
456 }}
457 result, err := s.uniter.ReadSettings(args)
458 c.Assert(err, gc.IsNil)
459- c.Assert(result, gc.DeepEquals, params.SettingsResults{
460- Results: []params.SettingsResult{
461+ c.Assert(result, gc.DeepEquals, params.RelationSettingsResults{
462+ Results: []params.RelationSettingsResult{
463 {Error: apiservertesting.ErrUnauthorized},
464- {Settings: params.Settings{
465+ {Settings: params.RelationSettings{
466 "some": "settings",
467 }},
468 {Error: apiservertesting.ErrUnauthorized},
469@@ -1116,6 +1116,31 @@
470 })
471 }
472
473+func (s *uniterSuite) TestReadSettingsWithNonStringValuesFails(c *gc.C) {
474+ rel := s.addRelation(c, "wordpress", "mysql")
475+ relUnit, err := rel.Unit(s.wordpressUnit)
476+ c.Assert(err, gc.IsNil)
477+ settings := map[string]interface{}{
478+ "other": "things",
479+ "invalid-bool": false,
480+ }
481+ err = relUnit.EnterScope(settings)
482+ c.Assert(err, gc.IsNil)
483+ s.assertInScope(c, relUnit, true)
484+
485+ args := params.RelationUnits{RelationUnits: []params.RelationUnit{
486+ {Relation: rel.Tag(), Unit: "unit-wordpress-0"},
487+ }}
488+ expectErr := `unexpected relation setting "invalid-bool": expected string, got bool`
489+ result, err := s.uniter.ReadSettings(args)
490+ c.Assert(err, gc.IsNil)
491+ c.Assert(result, gc.DeepEquals, params.RelationSettingsResults{
492+ Results: []params.RelationSettingsResult{
493+ {Error: &params.Error{Message: expectErr}},
494+ },
495+ })
496+}
497+
498 func (s *uniterSuite) TestReadRemoteSettings(c *gc.C) {
499 rel := s.addRelation(c, "wordpress", "mysql")
500 relUnit, err := rel.Unit(s.wordpressUnit)
501@@ -1146,8 +1171,8 @@
502 // We don't set the remote unit settings on purpose to test the error.
503 expectErr := `cannot read settings for unit "mysql/0" in relation "wordpress:db mysql:server": settings not found`
504 c.Assert(err, gc.IsNil)
505- c.Assert(result, gc.DeepEquals, params.SettingsResults{
506- Results: []params.SettingsResult{
507+ c.Assert(result, gc.DeepEquals, params.RelationSettingsResults{
508+ Results: []params.RelationSettingsResult{
509 {Error: apiservertesting.ErrUnauthorized},
510 {Error: apiservertesting.ErrUnauthorized},
511 {Error: &params.Error{Message: expectErr}},
512@@ -1182,15 +1207,42 @@
513 }}}
514 result, err = s.uniter.ReadRemoteSettings(args)
515 c.Assert(err, gc.IsNil)
516- c.Assert(result, gc.DeepEquals, params.SettingsResults{
517- Results: []params.SettingsResult{
518- {Settings: params.Settings{
519+ c.Assert(result, gc.DeepEquals, params.RelationSettingsResults{
520+ Results: []params.RelationSettingsResult{
521+ {Settings: params.RelationSettings{
522 "other": "things",
523 }},
524 },
525 })
526 }
527
528+func (s *uniterSuite) TestReadRemoteSettingsWithNonStringValuesFails(c *gc.C) {
529+ rel := s.addRelation(c, "wordpress", "mysql")
530+ relUnit, err := rel.Unit(s.mysqlUnit)
531+ c.Assert(err, gc.IsNil)
532+ settings := map[string]interface{}{
533+ "other": "things",
534+ "invalid-bool": false,
535+ }
536+ err = relUnit.EnterScope(settings)
537+ c.Assert(err, gc.IsNil)
538+ s.assertInScope(c, relUnit, true)
539+
540+ args := params.RelationUnitPairs{RelationUnitPairs: []params.RelationUnitPair{{
541+ Relation: rel.Tag(),
542+ LocalUnit: "unit-wordpress-0",
543+ RemoteUnit: "unit-mysql-0",
544+ }}}
545+ expectErr := `unexpected relation setting "invalid-bool": expected string, got bool`
546+ result, err := s.uniter.ReadRemoteSettings(args)
547+ c.Assert(err, gc.IsNil)
548+ c.Assert(result, gc.DeepEquals, params.RelationSettingsResults{
549+ Results: []params.RelationSettingsResult{
550+ {Error: &params.Error{Message: expectErr}},
551+ },
552+ })
553+}
554+
555 func (s *uniterSuite) TestUpdateSettings(c *gc.C) {
556 rel := s.addRelation(c, "wordpress", "mysql")
557 relUnit, err := rel.Unit(s.wordpressUnit)
558@@ -1202,7 +1254,7 @@
559 err = relUnit.EnterScope(settings)
560 s.assertInScope(c, relUnit, true)
561
562- newSettings := params.Settings{
563+ newSettings := params.RelationSettings{
564 "some": "different",
565 "other": "",
566 }
567
568=== modified file 'worker/uniter/context.go'
569--- worker/uniter/context.go 2013-09-13 10:38:02 +0000
570+++ worker/uniter/context.go 2013-09-26 17:24:28 +0000
571@@ -275,7 +275,7 @@
572 }
573
574 // SettingsMap is a map from unit name to relation settings.
575-type SettingsMap map[string]params.Settings
576+type SettingsMap map[string]params.RelationSettings
577
578 // ContextRelation is the implementation of jujuc.ContextRelation.
579 type ContextRelation struct {
580@@ -367,7 +367,7 @@
581 return ctx.settings, nil
582 }
583
584-func (ctx *ContextRelation) ReadSettings(unit string) (settings params.Settings, err error) {
585+func (ctx *ContextRelation) ReadSettings(unit string) (settings params.RelationSettings, err error) {
586 settings, member := ctx.members[unit]
587 if settings == nil {
588 if settings = ctx.cache[unit]; settings == nil {
589
590=== modified file 'worker/uniter/context_test.go'
591--- worker/uniter/context_test.go 2013-09-17 03:12:12 +0000
592+++ worker/uniter/context_test.go 2013-09-26 17:24:28 +0000
593@@ -275,10 +275,10 @@
594 // Check that the changes to the local settings nodes have been discarded.
595 node0, err = s.relctxs[0].Settings()
596 c.Assert(err, gc.IsNil)
597- c.Assert(node0.Map(), gc.DeepEquals, params.Settings{"relation-name": "db0"})
598+ c.Assert(node0.Map(), gc.DeepEquals, params.RelationSettings{"relation-name": "db0"})
599 node1, err = s.relctxs[1].Settings()
600 c.Assert(err, gc.IsNil)
601- c.Assert(node1.Map(), gc.DeepEquals, params.Settings{"relation-name": "db1"})
602+ c.Assert(node1.Map(), gc.DeepEquals, params.RelationSettings{"relation-name": "db1"})
603
604 // Check that the changes have been written to state.
605 settings0, err := s.relunits[0].ReadSettings("u/0")
606@@ -303,13 +303,13 @@
607 // Check that the changes to the local settings nodes are still there.
608 node0, err = s.relctxs[0].Settings()
609 c.Assert(err, gc.IsNil)
610- c.Assert(node0.Map(), gc.DeepEquals, params.Settings{
611+ c.Assert(node0.Map(), gc.DeepEquals, params.RelationSettings{
612 "relation-name": "db0",
613 "baz": "3",
614 })
615 node1, err = s.relctxs[1].Settings()
616 c.Assert(err, gc.IsNil)
617- c.Assert(node1.Map(), gc.DeepEquals, params.Settings{
618+ c.Assert(node1.Map(), gc.DeepEquals, params.RelationSettings{
619 "relation-name": "db1",
620 "qux": "4",
621 })
622@@ -383,13 +383,13 @@
623 "u/4": {"qux": "4"},
624 })
625 c.Assert(ctx.UnitNames(), gc.DeepEquals, []string{"u/2", "u/4"})
626- assertSettings := func(unit string, expect params.Settings) {
627+ assertSettings := func(unit string, expect params.RelationSettings) {
628 actual, err := ctx.ReadSettings(unit)
629 c.Assert(err, gc.IsNil)
630 c.Assert(actual, gc.DeepEquals, expect)
631 }
632- assertSettings("u/2", params.Settings{"baz": "2"})
633- assertSettings("u/4", params.Settings{"qux": "4"})
634+ assertSettings("u/2", params.RelationSettings{"baz": "2"})
635+ assertSettings("u/4", params.RelationSettings{"qux": "4"})
636
637 // Send a second update; check that members are only added, not removed.
638 ctx.UpdateMembers(uniter.SettingsMap{
639@@ -400,10 +400,10 @@
640 c.Assert(ctx.UnitNames(), gc.DeepEquals, []string{"u/1", "u/2", "u/3", "u/4"})
641
642 // Check that all settings remain cached.
643- assertSettings("u/1", params.Settings{"foo": "1"})
644- assertSettings("u/2", params.Settings{"abc": "2"})
645- assertSettings("u/3", params.Settings{"bar": "3"})
646- assertSettings("u/4", params.Settings{"qux": "4"})
647+ assertSettings("u/1", params.RelationSettings{"foo": "1"})
648+ assertSettings("u/2", params.RelationSettings{"abc": "2"})
649+ assertSettings("u/3", params.RelationSettings{"bar": "3"})
650+ assertSettings("u/4", params.RelationSettings{"qux": "4"})
651
652 // Delete a member, and check that it is no longer a member...
653 ctx.DeleteMember("u/2")
654@@ -454,7 +454,7 @@
655 ctx.UpdateMembers(uniter.SettingsMap{"u/1": {"entirely": "different"}})
656 m, err = ctx.ReadSettings("u/1")
657 c.Assert(err, gc.IsNil)
658- c.Assert(m, gc.DeepEquals, params.Settings{"entirely": "different"})
659+ c.Assert(m, gc.DeepEquals, params.RelationSettings{"entirely": "different"})
660 }
661
662 func (s *ContextRelationSuite) TestNonMemberCaching(c *gc.C) {
663@@ -700,7 +700,7 @@
664 return context
665 }
666
667-func convertSettings(settings params.Settings) map[string]interface{} {
668+func convertSettings(settings params.RelationSettings) map[string]interface{} {
669 result := make(map[string]interface{})
670 for k, v := range settings {
671 result[k] = v
672@@ -708,8 +708,8 @@
673 return result
674 }
675
676-func convertMap(settingsMap map[string]interface{}) params.Settings {
677- result := make(params.Settings)
678+func convertMap(settingsMap map[string]interface{}) params.RelationSettings {
679+ result := make(params.RelationSettings)
680 for k, v := range settingsMap {
681 result[k] = v.(string)
682 }
683
684=== modified file 'worker/uniter/jujuc/context.go'
685--- worker/uniter/jujuc/context.go 2013-09-12 16:29:52 +0000
686+++ worker/uniter/jujuc/context.go 2013-09-26 17:24:28 +0000
687@@ -77,12 +77,12 @@
688 UnitNames() []string
689
690 // ReadSettings returns the settings of any remote unit in the relation.
691- ReadSettings(unit string) (params.Settings, error)
692+ ReadSettings(unit string) (params.RelationSettings, error)
693 }
694
695 // Settings is implemented by types that manipulate unit settings.
696 type Settings interface {
697- Map() params.Settings
698+ Map() params.RelationSettings
699 Set(string, string)
700 Delete(string)
701 }
702
703=== modified file 'worker/uniter/jujuc/relation-get.go'
704--- worker/uniter/jujuc/relation-get.go 2013-09-12 16:29:52 +0000
705+++ worker/uniter/jujuc/relation-get.go 2013-09-26 17:24:28 +0000
706@@ -78,7 +78,7 @@
707 if !found {
708 return fmt.Errorf("unknown relation id")
709 }
710- var settings params.Settings
711+ var settings params.RelationSettings
712 if c.UnitName == c.ctx.UnitName() {
713 node, err := r.Settings()
714 if err != nil {
715
716=== modified file 'worker/uniter/jujuc/util_test.go'
717--- worker/uniter/jujuc/util_test.go 2013-09-12 16:29:52 +0000
718+++ worker/uniter/jujuc/util_test.go 2013-09-26 17:24:28 +0000
719@@ -166,7 +166,7 @@
720 return s
721 }
722
723-func (r *ContextRelation) ReadSettings(name string) (params.Settings, error) {
724+func (r *ContextRelation) ReadSettings(name string) (params.RelationSettings, error) {
725 s, found := r.units[name]
726 if !found {
727 return nil, fmt.Errorf("unknown unit %s", name)
728@@ -174,7 +174,7 @@
729 return s.Map(), nil
730 }
731
732-type Settings params.Settings
733+type Settings params.RelationSettings
734
735 func (s Settings) Get(k string) (interface{}, bool) {
736 v, f := s[k]
737@@ -189,8 +189,8 @@
738 delete(s, k)
739 }
740
741-func (s Settings) Map() params.Settings {
742- r := params.Settings{}
743+func (s Settings) Map() params.RelationSettings {
744+ r := params.RelationSettings{}
745 for k, v := range s {
746 r[k] = v
747 }

Subscribers

People subscribed via source and target branches

to status/vote changes: