Merge lp:~fwereade/juju-core/config-6-state-service-sane-methods into lp:~juju/juju-core/trunk

Proposed by William Reade
Status: Merged
Merged at revision: 1276
Proposed branch: lp:~fwereade/juju-core/config-6-state-service-sane-methods
Merge into: lp:~juju/juju-core/trunk
Prerequisite: lp:~fwereade/juju-core/config-5-state-service-config-yaml
Diff against target: 1196 lines (+259/-365)
21 files modified
cmd/juju/cmd_test.go (+2/-2)
cmd/juju/config_test.go (+11/-12)
cmd/juju/deploy.go (+3/-4)
cmd/juju/deploy_test.go (+6/-0)
cmd/juju/set.go (+40/-32)
juju/conn.go (+15/-13)
juju/conn_test.go (+4/-4)
state/api/client.go (+0/-1)
state/api/params/params_test.go (+1/-1)
state/apiserver/client.go (+18/-2)
state/apiserver/client_test.go (+4/-4)
state/megawatcher_internal_test.go (+13/-20)
state/service.go (+12/-39)
state/service_test.go (+69/-151)
state/statecmd/config_test.go (+3/-1)
state/statecmd/deploy_test.go (+4/-4)
state/statecmd/get.go (+26/-31)
state/unit_test.go (+7/-7)
worker/uniter/context_test.go (+3/-4)
worker/uniter/filter_test.go (+17/-29)
worker/uniter/uniter_test.go (+1/-4)
To merge this branch: bzr merge lp:~fwereade/juju-core/config-6-state-service-sane-methods
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+168580@code.launchpad.net

Description of the change

state: Service ConfigSettings methods

Config, SetConfig, and SetConfigYAML methods have been dropped in favour of
ConfigSettings and UpdateConfigSettings, which use sensible types. Clients
are expected to parse their own damn data and supply a sensible format.

https://codereview.appspot.com/10083047/

To post a comment you must log in.
Revision history for this message
William Reade (fwereade) wrote :

Reviewers: mp+168580_code.launchpad.net,

Message:
Please take a look.

Description:
state: Service ConfigSettings methods

Config, SetConfig, and SetConfigYAML methods have been dropped in favour
of
ConfigSettings and UpdateConfigSettings, which use sensible types.
Clients
are expected to parse their own damn data and supply a sensible format.

https://code.launchpad.net/~fwereade/juju-core/config-6-state-service-sane-methods/+merge/168580

Requires:
https://code.launchpad.net/~fwereade/juju-core/config-5-state-service-config-yaml/+merge/168579

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M cmd/juju/cmd_test.go
   M cmd/juju/config_test.go
   M cmd/juju/deploy.go
   M cmd/juju/deploy_test.go
   M cmd/juju/set.go
   M juju/conn.go
   M juju/conn_test.go
   M state/api/client.go
   M state/api/params/params_test.go
   M state/apiserver/client.go
   M state/apiserver/client_test.go
   M state/megawatcher_internal_test.go
   M state/service.go
   M state/service_test.go
   M state/statecmd/config_test.go
   M state/statecmd/deploy_test.go
   M state/statecmd/get.go
   M state/unit_test.go
   M worker/uniter/context_test.go
   M worker/uniter/filter_test.go
   M worker/uniter/uniter_test.go

Revision history for this message
Frank Mueller (themue) wrote :

LGTM, cleaner interface now.

https://codereview.appspot.com/10083047/

Revision history for this message
Roger Peppe (rogpeppe) wrote :

https://codereview.appspot.com/10083047/diff/1/cmd/juju/deploy.go
File cmd/juju/deploy.go (right):

https://codereview.appspot.com/10083047/diff/1/cmd/juju/deploy.go#newcode95
cmd/juju/deploy.go:95: return errors.New("must deploy at least one
unit")
personally i don't see why we require >0 units.

https://codereview.appspot.com/10083047/

Revision history for this message
Roger Peppe (rogpeppe) wrote :

LGTM

https://codereview.appspot.com/10083047/diff/1/juju/conn.go
File juju/conn.go (right):

https://codereview.appspot.com/10083047/diff/1/juju/conn.go#newcode219
juju/conn.go:219: if err := svc.UpdateConfigSettings(settings); err !=
nil {
much nicer to only have one operation here, thanks.

https://codereview.appspot.com/10083047/diff/1/state/apiserver/client.go
File state/apiserver/client.go (right):

https://codereview.appspot.com/10083047/diff/1/state/apiserver/client.go#newcode70
state/apiserver/client.go:70: changes, err :=
ch.Config().ParseSettingsYAML([]byte(p.Config), p.ServiceName)
we should check with the GUI folks that they haven't put in a workaround
for the current behaviour before this branch lands.

https://codereview.appspot.com/10083047/diff/1/state/service.go
File state/service.go (left):

https://codereview.appspot.com/10083047/diff/1/state/service.go#oldcode777
state/service.go:777: func (s *Service) SetConfigYAML(yamlData []byte)
error {
it's gone!

https://codereview.appspot.com/10083047/diff/1/state/service.go
File state/service.go (right):

https://codereview.appspot.com/10083047/diff/1/state/service.go#newcode729
state/service.go:729: return settings.Map(), nil
i'd be tempted to return settings.core to avoid yet another unnecessary
map copy (or implement Settings.sharedMap which just returns
settings.core,
amounting to the same thing).

perhaps the overhead amounts to nothing in the long run though. <hits
self>.

https://codereview.appspot.com/10083047/diff/1/state/statecmd/get.go
File state/statecmd/get.go (right):

https://codereview.appspot.com/10083047/diff/1/state/statecmd/get.go#newcode52
state/statecmd/get.go:52: info := map[string]interface{}{
this should really be a struct, i think.

https://codereview.appspot.com/10083047/diff/1/worker/uniter/context_test.go
File worker/uniter/context_test.go (right):

https://codereview.appspot.com/10083047/diff/1/worker/uniter/context_test.go#newcode582
worker/uniter/context_test.go:582: })
c.Assert(err, IsNil)

https://codereview.appspot.com/10083047/

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

*** Submitted:

state: Service ConfigSettings methods

Config, SetConfig, and SetConfigYAML methods have been dropped in favour
of
ConfigSettings and UpdateConfigSettings, which use sensible types.
Clients
are expected to parse their own damn data and supply a sensible format.

R=mue, rog
CC=
https://codereview.appspot.com/10083047

https://codereview.appspot.com/10083047/diff/1/cmd/juju/deploy.go
File cmd/juju/deploy.go (right):

https://codereview.appspot.com/10083047/diff/1/cmd/juju/deploy.go#newcode95
cmd/juju/deploy.go:95: return errors.New("must deploy at least one
unit")
On 2013/06/11 12:31:23, rog wrote:
> personally i don't see why we require >0 units.

I'm trying not to get sidetracked with such questions here ;).

https://codereview.appspot.com/10083047/diff/1/state/apiserver/client.go
File state/apiserver/client.go (right):

https://codereview.appspot.com/10083047/diff/1/state/apiserver/client.go#newcode70
state/apiserver/client.go:70: changes, err :=
ch.Config().ParseSettingsYAML([]byte(p.Config), p.ServiceName)
On 2013/06/11 12:53:12, rog wrote:
> we should check with the GUI folks that they haven't put in a
workaround for the
> current behaviour before this branch lands.

Agreed before embarking on this course of action -- they just pass it
through directly when they actually use it, so they're not affected.

https://codereview.appspot.com/10083047/diff/1/state/service.go
File state/service.go (right):

https://codereview.appspot.com/10083047/diff/1/state/service.go#newcode729
state/service.go:729: return settings.Map(), nil
On 2013/06/11 12:53:12, rog wrote:
> i'd be tempted to return settings.core to avoid yet another
unnecessary map copy
> (or implement Settings.sharedMap which just returns settings.core,
> amounting to the same thing).

> perhaps the overhead amounts to nothing in the long run though. <hits
self>.

I officially do not care about the overhead of in-memory copies of small
maps :).

https://codereview.appspot.com/10083047/diff/1/state/statecmd/get.go
File state/statecmd/get.go (right):

https://codereview.appspot.com/10083047/diff/1/state/statecmd/get.go#newcode52
state/statecmd/get.go:52: info := map[string]interface{}{
On 2013/06/11 12:53:12, rog wrote:
> this should really be a struct, i think.

I resisted the temptation to pull this whole lot into the charm
package... I think I'll leave it out for now.

https://codereview.appspot.com/10083047/diff/1/worker/uniter/context_test.go
File worker/uniter/context_test.go (right):

https://codereview.appspot.com/10083047/diff/1/worker/uniter/context_test.go#newcode582
worker/uniter/context_test.go:582: })
On 2013/06/11 12:53:12, rog wrote:
> c.Assert(err, IsNil)

Gaaah! Thank you.

https://codereview.appspot.com/10083047/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'cmd/juju/cmd_test.go'
--- cmd/juju/cmd_test.go 2013-05-02 15:55:42 +0000
+++ cmd/juju/cmd_test.go 2013-06-12 01:17:26 +0000
@@ -334,8 +334,8 @@
334 file.Close()334 file.Close()
335 com, err := initSetCommand("--config", "testconfig.yaml", "service")335 com, err := initSetCommand("--config", "testconfig.yaml", "service")
336 c.Assert(err, IsNil)336 c.Assert(err, IsNil)
337 c.Assert(com.Config.Path, Equals, "testconfig.yaml")337 c.Assert(com.SettingsYAML.Path, Equals, "testconfig.yaml")
338 actual, err := com.Config.Read(ctx)338 actual, err := com.SettingsYAML.Read(ctx)
339 c.Assert(err, IsNil)339 c.Assert(err, IsNil)
340 c.Assert(actual, DeepEquals, expected)340 c.Assert(actual, DeepEquals, expected)
341341
342342
=== modified file 'cmd/juju/config_test.go'
--- cmd/juju/config_test.go 2013-06-12 00:58:52 +0000
+++ cmd/juju/config_test.go 2013-06-12 01:17:26 +0000
@@ -9,6 +9,7 @@
99
10 . "launchpad.net/gocheck"10 . "launchpad.net/gocheck"
11 "launchpad.net/goyaml"11 "launchpad.net/goyaml"
12 "launchpad.net/juju-core/charm"
12 "launchpad.net/juju-core/cmd"13 "launchpad.net/juju-core/cmd"
13 "launchpad.net/juju-core/juju/testing"14 "launchpad.net/juju-core/juju/testing"
14 coretesting "launchpad.net/juju-core/testing"15 coretesting "launchpad.net/juju-core/testing"
@@ -67,9 +68,7 @@
67 sch := s.AddTestingCharm(c, "dummy")68 sch := s.AddTestingCharm(c, "dummy")
68 svc, err := s.State.AddService("dummy-service", sch)69 svc, err := s.State.AddService("dummy-service", sch)
69 c.Assert(err, IsNil)70 c.Assert(err, IsNil)
70 err = svc.SetConfig(map[string]string{71 err = svc.UpdateConfigSettings(charm.Settings{"title": "Nearly There"})
71 "title": "Nearly There",
72 })
73 c.Assert(err, IsNil)72 c.Assert(err, IsNil)
74 for _, t := range getTests {73 for _, t := range getTests {
75 ctx := coretesting.Context(c)74 ctx := coretesting.Context(c)
@@ -94,9 +93,9 @@
9493
95var setTests = []struct {94var setTests = []struct {
96 about string95 about string
97 args []string // command to be executed96 args []string // command to be executed
98 expect map[string]interface{} // resulting configuration of the dummy service.97 expect charm.Settings // resulting configuration of the dummy service.
99 err string // error regex98 err string // error regex
100}{{99}{{
101 about: "invalid option",100 about: "invalid option",
102 args: []string{"foo", "bar"},101 args: []string{"foo", "bar"},
@@ -112,21 +111,21 @@
112}, {111}, {
113 about: "set with options",112 about: "set with options",
114 args: []string{"username=hello"},113 args: []string{"username=hello"},
115 expect: map[string]interface{}{114 expect: charm.Settings{
116 "username": "hello",115 "username": "hello",
117 },116 },
118}, {117}, {
119 about: "set with option values containing =",118 about: "set with option values containing =",
120 args: []string{"username=hello=foo"},119 args: []string{"username=hello=foo"},
121 expect: map[string]interface{}{120 expect: charm.Settings{
122 "username": "hello=foo",121 "username": "hello=foo",
123 },122 },
124}, {123}, {
125 about: "--config $FILE test",124 about: "--config $FILE test",
126 args: []string{"--config", "testconfig.yaml"},125 args: []string{"--config", "testconfig.yaml"},
127 expect: map[string]interface{}{126 expect: charm.Settings{
128 "username": "admin001",127 "username": "admin001",
129 "skill-level": int64(9000), // yaml int types are int64128 "skill-level": int64(9000), // charm int types are int64
130 },129 },
131},130},
132}131}
@@ -147,9 +146,9 @@
147 c.Assert(ctx.Stderr.(*bytes.Buffer).String(), Matches, t.err)146 c.Assert(ctx.Stderr.(*bytes.Buffer).String(), Matches, t.err)
148 } else {147 } else {
149 c.Check(code, Equals, 0)148 c.Check(code, Equals, 0)
150 cfg, err := svc.Config()149 settings, err := svc.ConfigSettings()
151 c.Assert(err, IsNil)150 c.Assert(err, IsNil)
152 c.Assert(cfg.Map(), DeepEquals, t.expect)151 c.Assert(settings, DeepEquals, t.expect)
153 }152 }
154 }153 }
155}154}
156155
=== modified file 'cmd/juju/deploy.go'
--- cmd/juju/deploy.go 2013-05-22 19:34:42 +0000
+++ cmd/juju/deploy.go 2013-06-12 01:17:26 +0000
@@ -92,16 +92,15 @@
92 return cmd.CheckEmpty(args[2:])92 return cmd.CheckEmpty(args[2:])
93 }93 }
94 if c.NumUnits < 1 {94 if c.NumUnits < 1 {
95 // TODO improve/remove: this is misleading when deploying subordinates.
96 return errors.New("must deploy at least one unit")95 return errors.New("must deploy at least one unit")
97 }96 }
98 if c.ForceMachineId != "" {97 if c.ForceMachineId != "" {
98 if c.NumUnits > 1 {
99 return errors.New("cannot use --num-units with --force-machine")
100 }
99 if !state.IsMachineId(c.ForceMachineId) {101 if !state.IsMachineId(c.ForceMachineId) {
100 return fmt.Errorf("invalid machine id %q", c.ForceMachineId)102 return fmt.Errorf("invalid machine id %q", c.ForceMachineId)
101 }103 }
102 if c.NumUnits > 1 {
103 return fmt.Errorf("force-machine cannot be used for multiple units")
104 }
105 }104 }
106 return nil105 return nil
107}106}
108107
=== modified file 'cmd/juju/deploy_test.go'
--- cmd/juju/deploy_test.go 2013-05-27 03:00:31 +0000
+++ cmd/juju/deploy_test.go 2013-06-12 01:17:26 +0000
@@ -37,6 +37,12 @@
37 args: []string{"craziness", "burble1", "-n", "0"},37 args: []string{"craziness", "burble1", "-n", "0"},
38 err: `must deploy at least one unit`,38 err: `must deploy at least one unit`,
39 }, {39 }, {
40 args: []string{"craziness", "burble1", "--force-machine", "bigglesplop"},
41 err: `invalid machine id "bigglesplop"`,
42 }, {
43 args: []string{"craziness", "burble1", "-n", "2", "--force-machine", "123"},
44 err: `cannot use --num-units with --force-machine`,
45 }, {
40 args: []string{"craziness", "burble1", "--constraints", "gibber=plop"},46 args: []string{"craziness", "burble1", "--constraints", "gibber=plop"},
41 err: `invalid value "gibber=plop" for flag --constraints: unknown constraint "gibber"`,47 err: `invalid value "gibber=plop" for flag --constraints: unknown constraint "gibber"`,
42 },48 },
4349
=== modified file 'cmd/juju/set.go'
--- cmd/juju/set.go 2013-05-02 15:55:42 +0000
+++ cmd/juju/set.go 2013-06-12 01:17:26 +0000
@@ -9,6 +9,7 @@
9 "strings"9 "strings"
1010
11 "launchpad.net/gnuflag"11 "launchpad.net/gnuflag"
12 "launchpad.net/juju-core/charm"
12 "launchpad.net/juju-core/cmd"13 "launchpad.net/juju-core/cmd"
13 "launchpad.net/juju-core/juju"14 "launchpad.net/juju-core/juju"
14)15)
@@ -16,10 +17,9 @@
16// SetCommand updates the configuration of a service17// SetCommand updates the configuration of a service
17type SetCommand struct {18type SetCommand struct {
18 EnvCommandBase19 EnvCommandBase
19 ServiceName string20 ServiceName string
20 // either Options or Config will contain the configuration data21 SettingsStrings map[string]string
21 Options []string22 SettingsYAML cmd.FileVar
22 Config cmd.FileVar
23}23}
2424
25func (c *SetCommand) Info() *cmd.Info {25func (c *SetCommand) Info() *cmd.Info {
@@ -33,51 +33,59 @@
3333
34func (c *SetCommand) SetFlags(f *gnuflag.FlagSet) {34func (c *SetCommand) SetFlags(f *gnuflag.FlagSet) {
35 c.EnvCommandBase.SetFlags(f)35 c.EnvCommandBase.SetFlags(f)
36 f.Var(&c.Config, "config", "path to yaml-formatted service config")36 f.Var(&c.SettingsYAML, "config", "path to yaml-formatted service config")
37}37}
3838
39func (c *SetCommand) Init(args []string) error {39func (c *SetCommand) Init(args []string) error {
40 if len(args) == 0 || len(strings.Split(args[0], "=")) > 1 {40 if len(args) == 0 || len(strings.Split(args[0], "=")) > 1 {
41 return errors.New("no service name specified")41 return errors.New("no service name specified")
42 }42 }
43 if len(c.Config.Path) > 0 && len(args) > 1 {43 if c.SettingsYAML.Path != "" && len(args) > 1 {
44 return errors.New("cannot specify --config when using key=value arguments")44 return errors.New("cannot specify --config when using key=value arguments")
45 }45 }
46 c.ServiceName, c.Options = args[0], args[1:]46 c.ServiceName = args[0]
47 settings, err := parse(args[1:])
48 if err != nil {
49 return err
50 }
51 c.SettingsStrings = settings
47 return nil52 return nil
48}53}
4954
50// Run updates the configuration of a service55// Run updates the configuration of a service.
51func (c *SetCommand) Run(ctx *cmd.Context) error {56func (c *SetCommand) Run(ctx *cmd.Context) error {
52 contents, err := c.Config.Read(ctx)
53 if err != nil && err != cmd.ErrNoPath {
54 return err
55 }
56 var options map[string]string
57 if len(contents) == 0 {
58 if len(c.Options) == 0 {
59 // nothing to do.
60 return nil
61 }
62 options, err = parse(c.Options)
63 if err != nil {
64 return err
65 }
66 }
67 conn, err := juju.NewConnFromName(c.EnvName)57 conn, err := juju.NewConnFromName(c.EnvName)
68 if err != nil {58 if err != nil {
69 return err59 return err
70 }60 }
71 defer conn.Close()61 defer conn.Close()
7262 service, err := conn.State.Service(c.ServiceName)
73 svc, err := conn.State.Service(c.ServiceName)63 if err != nil {
74 if err != nil {64 return err
75 return err65 }
76 }66 ch, _, err := service.Charm()
77 if len(contents) == 0 {67 if err != nil {
78 return svc.SetConfig(options)68 return err
79 }69 }
80 return svc.SetConfigYAML(contents)70 var settings charm.Settings
71 if c.SettingsYAML.Path != "" {
72 settingsYAML, err := c.SettingsYAML.Read(ctx)
73 if err != nil {
74 return err
75 }
76 settings, err = ch.Config().ParseSettingsYAML(settingsYAML, c.ServiceName)
77 if err != nil {
78 return err
79 }
80 } else if len(c.SettingsStrings) > 0 {
81 settings, err = ch.Config().ParseSettingsStrings(c.SettingsStrings)
82 if err != nil {
83 return err
84 }
85 } else {
86 return nil
87 }
88 return service.UpdateConfigSettings(settings)
81}89}
8290
83// parse parses the option k=v strings into a map of options to be91// parse parses the option k=v strings into a map of options to be
8492
=== modified file 'juju/conn.go'
--- juju/conn.go 2013-06-10 10:30:13 +0000
+++ juju/conn.go 2013-06-12 01:17:26 +0000
@@ -200,21 +200,23 @@
200200
201// DeployService takes a charm and various parameters and deploys it.201// DeployService takes a charm and various parameters and deploys it.
202func (conn *Conn) DeployService(args DeployServiceParams) (*state.Service, error) {202func (conn *Conn) DeployService(args DeployServiceParams) (*state.Service, error) {
203 // TODO(rog) validate the configuration before adding the service.203 var err error
204 svc, err := conn.State.AddService(args.ServiceName, args.Charm)204 var settings charm.Settings
205 if err != nil {205 config := args.Charm.Config()
206 return nil, err
207 }
208 // TODO(rog) should we destroy if we return an error in any of the
209 // subsequent operations?
210 // BUG(lp:1162122): Config/ConfigYAML have no tests.
211 if args.ConfigYAML != "" {206 if args.ConfigYAML != "" {
212 if err := svc.SetConfigYAML([]byte(args.ConfigYAML)); err != nil {207 settings, err = config.ParseSettingsYAML([]byte(args.ConfigYAML), args.ServiceName)
213 return nil, err
214 }
215 } else if args.Config != nil {208 } else if args.Config != nil {
216 if err := svc.SetConfig(args.Config); err != nil {209 settings, err = config.ParseSettingsStrings(args.Config)
217 // TODO(rog) should we destroy the service here?210 }
211 if err != nil {
212 return nil, err
213 }
214 svc, err := conn.State.AddService(args.ServiceName, args.Charm)
215 if err != nil {
216 return nil, err
217 }
218 if len(settings) > 0 {
219 if err := svc.UpdateConfigSettings(settings); err != nil {
218 return nil, err220 return nil, err
219 }221 }
220 }222 }
221223
=== modified file 'juju/conn_test.go'
--- juju/conn_test.go 2013-06-10 22:46:06 +0000
+++ juju/conn_test.go 2013-06-12 01:17:26 +0000
@@ -426,12 +426,12 @@
426}426}
427427
428func (s *DeployLocalSuite) TestDeploy(c *C) {428func (s *DeployLocalSuite) TestDeploy(c *C) {
429 charm, err := s.Conn.PutCharm(s.charmUrl, s.repo, false)429 ch, err := s.Conn.PutCharm(s.charmUrl, s.repo, false)
430 c.Assert(err, IsNil)430 c.Assert(err, IsNil)
431 cons := constraints.MustParse("mem=4G")431 cons := constraints.MustParse("mem=4G")
432 args := juju.DeployServiceParams{432 args := juju.DeployServiceParams{
433 ServiceName: "bob",433 ServiceName: "bob",
434 Charm: charm,434 Charm: ch,
435 NumUnits: 3,435 NumUnits: 3,
436 Constraints: cons,436 Constraints: cons,
437 ConfigYAML: "bob: {blog-title: aspidistra flagpole}",437 ConfigYAML: "bob: {blog-title: aspidistra flagpole}",
@@ -441,9 +441,9 @@
441 scons, err := svc.Constraints()441 scons, err := svc.Constraints()
442 c.Assert(err, IsNil)442 c.Assert(err, IsNil)
443 c.Assert(scons, DeepEquals, cons)443 c.Assert(scons, DeepEquals, cons)
444 config, err := svc.Config()444 settings, err := svc.ConfigSettings()
445 c.Assert(err, IsNil)445 c.Assert(err, IsNil)
446 c.Assert(config.Map(), DeepEquals, map[string]interface{}{446 c.Assert(settings, DeepEquals, charm.Settings{
447 "blog-title": "aspidistra flagpole",447 "blog-title": "aspidistra flagpole",
448 })448 })
449449
450450
=== modified file 'state/api/client.go'
--- state/api/client.go 2013-06-05 09:36:10 +0000
+++ state/api/client.go 2013-06-12 01:17:26 +0000
@@ -105,7 +105,6 @@
105 ServiceName: serviceName,105 ServiceName: serviceName,
106 CharmUrl: charmUrl,106 CharmUrl: charmUrl,
107 NumUnits: numUnits,107 NumUnits: numUnits,
108 // BUG(lp:1162122): ConfigYAML has no tests.
109 ConfigYAML: configYAML,108 ConfigYAML: configYAML,
110 Constraints: cons,109 Constraints: cons,
111 }110 }
112111
=== modified file 'state/api/params/params_test.go'
--- state/api/params/params_test.go 2013-05-21 16:40:29 +0000
+++ state/api/params/params_test.go 2013-06-12 01:17:26 +0000
@@ -48,7 +48,7 @@
48 CharmURL: "cs:series/name",48 CharmURL: "cs:series/name",
49 Life: params.Life(state.Dying.String()),49 Life: params.Life(state.Dying.String()),
50 Constraints: constraints.MustParse("arch=arm mem=1024M"),50 Constraints: constraints.MustParse("arch=arm mem=1024M"),
51 Config: map[string]interface{}{51 Config: charm.Settings{
52 "hello": "goodbye",52 "hello": "goodbye",
53 "foo": false,53 "foo": false,
54 },54 },
5555
=== modified file 'state/apiserver/client.go'
--- state/apiserver/client.go 2013-05-24 19:04:47 +0000
+++ state/apiserver/client.go 2013-06-12 01:17:26 +0000
@@ -46,7 +46,15 @@
46 if err != nil {46 if err != nil {
47 return err47 return err
48 }48 }
49 return svc.SetConfig(p.Options)49 ch, _, err := svc.Charm()
50 if err != nil {
51 return err
52 }
53 changes, err := ch.Config().ParseSettingsStrings(p.Options)
54 if err != nil {
55 return err
56 }
57 return svc.UpdateConfigSettings(changes)
50}58}
5159
52// ServiceSetYAML implements the server side of Client.ServerSetYAML.60// ServiceSetYAML implements the server side of Client.ServerSetYAML.
@@ -55,7 +63,15 @@
55 if err != nil {63 if err != nil {
56 return err64 return err
57 }65 }
58 return svc.SetConfigYAML([]byte(p.Config))66 ch, _, err := svc.Charm()
67 if err != nil {
68 return err
69 }
70 changes, err := ch.Config().ParseSettingsYAML([]byte(p.Config), p.ServiceName)
71 if err != nil {
72 return err
73 }
74 return svc.UpdateConfigSettings(changes)
59}75}
6076
61// ServiceGet returns the configuration for a service.77// ServiceGet returns the configuration for a service.
6278
=== modified file 'state/apiserver/client_test.go'
--- state/apiserver/client_test.go 2013-06-10 22:46:06 +0000
+++ state/apiserver/client_test.go 2013-06-12 01:17:26 +0000
@@ -36,9 +36,9 @@
36 "username": "yyy",36 "username": "yyy",
37 })37 })
38 c.Assert(err, IsNil)38 c.Assert(err, IsNil)
39 conf, err := dummy.Config()39 settings, err := dummy.ConfigSettings()
40 c.Assert(err, IsNil)40 c.Assert(err, IsNil)
41 c.Assert(conf.Map(), DeepEquals, map[string]interface{}{41 c.Assert(settings, DeepEquals, charm.Settings{
42 "title": "xxx",42 "title": "xxx",
43 "username": "yyy",43 "username": "yyy",
44 })44 })
@@ -49,9 +49,9 @@
49 c.Assert(err, IsNil)49 c.Assert(err, IsNil)
50 err = s.APIState.Client().ServiceSetYAML("dummy", "dummy:\n title: aaa\n username: bbb")50 err = s.APIState.Client().ServiceSetYAML("dummy", "dummy:\n title: aaa\n username: bbb")
51 c.Assert(err, IsNil)51 c.Assert(err, IsNil)
52 conf, err := dummy.Config()52 settings, err := dummy.ConfigSettings()
53 c.Assert(err, IsNil)53 c.Assert(err, IsNil)
54 c.Assert(conf.Map(), DeepEquals, map[string]interface{}{54 c.Assert(settings, DeepEquals, charm.Settings{
55 "title": "aaa",55 "title": "aaa",
56 "username": "bbb",56 "username": "bbb",
57 })57 })
5858
=== modified file 'state/megawatcher_internal_test.go'
--- state/megawatcher_internal_test.go 2013-05-27 03:00:31 +0000
+++ state/megawatcher_internal_test.go 2013-06-12 01:17:26 +0000
@@ -85,7 +85,7 @@
85 CharmURL: serviceCharmURL(wordpress).String(),85 CharmURL: serviceCharmURL(wordpress).String(),
86 Life: params.Life(Alive.String()),86 Life: params.Life(Alive.String()),
87 Constraints: constraints.MustParse("mem=100M"),87 Constraints: constraints.MustParse("mem=100M"),
88 Config: map[string]interface{}{"blog-title": "boring"},88 Config: charm.Settings{"blog-title": "boring"},
89 })89 })
90 pairs := map[string]string{"x": "12", "y": "99"}90 pairs := map[string]string{"x": "12", "y": "99"}
91 err = wordpress.SetAnnotations(pairs)91 err = wordpress.SetAnnotations(pairs)
@@ -101,7 +101,7 @@
101 Name: "logging",101 Name: "logging",
102 CharmURL: serviceCharmURL(logging).String(),102 CharmURL: serviceCharmURL(logging).String(),
103 Life: params.Life(Alive.String()),103 Life: params.Life(Alive.String()),
104 Config: map[string]interface{}{},104 Config: charm.Settings{},
105 })105 })
106106
107 eps, err := s.State.InferEndpoints([]string{"logging", "wordpress"})107 eps, err := s.State.InferEndpoints([]string{"logging", "wordpress"})
@@ -411,7 +411,7 @@
411 Exposed: true,411 Exposed: true,
412 CharmURL: "local:series/series-wordpress-3",412 CharmURL: "local:series/series-wordpress-3",
413 Life: params.Life(Alive.String()),413 Life: params.Life(Alive.String()),
414 Config: map[string]interface{}{},414 Config: charm.Settings{},
415 },415 },
416 },416 },
417 }, {417 }, {
@@ -421,7 +421,7 @@
421 Exposed: true,421 Exposed: true,
422 CharmURL: "local:series/series-wordpress-3",422 CharmURL: "local:series/series-wordpress-3",
423 Constraints: constraints.MustParse("mem=99M"),423 Constraints: constraints.MustParse("mem=99M"),
424 Config: map[string]interface{}{"blog-title": "boring"},424 Config: charm.Settings{"blog-title": "boring"},
425 }},425 }},
426 setUp: func(c *C, st *State) {426 setUp: func(c *C, st *State) {
427 svc, err := st.AddService("wordpress", AddTestingCharm(c, st, "wordpress"))427 svc, err := st.AddService("wordpress", AddTestingCharm(c, st, "wordpress"))
@@ -438,7 +438,7 @@
438 CharmURL: "local:series/series-wordpress-3",438 CharmURL: "local:series/series-wordpress-3",
439 Life: params.Life(Alive.String()),439 Life: params.Life(Alive.String()),
440 Constraints: constraints.MustParse("mem=99M"),440 Constraints: constraints.MustParse("mem=99M"),
441 Config: map[string]interface{}{"blog-title": "boring"},441 Config: charm.Settings{"blog-title": "boring"},
442 },442 },
443 },443 },
444 }, {444 }, {
@@ -448,16 +448,12 @@
448 // Note: CharmURL has a different revision number from448 // Note: CharmURL has a different revision number from
449 // the wordpress revision in the testing repo.449 // the wordpress revision in the testing repo.
450 CharmURL: "local:series/series-wordpress-2",450 CharmURL: "local:series/series-wordpress-2",
451 Config: map[string]interface{}{"foo": "bar"},451 Config: charm.Settings{"foo": "bar"},
452 }},452 }},
453 setUp: func(c *C, st *State) {453 setUp: func(c *C, st *State) {
454 svc, err := st.AddService("wordpress", AddTestingCharm(c, st, "wordpress"))454 svc, err := st.AddService("wordpress", AddTestingCharm(c, st, "wordpress"))
455 c.Assert(err, IsNil)455 c.Assert(err, IsNil)
456 cfg, err := svc.Config()456 setServiceConfigAttr(c, svc, "blog-title", "boring")
457 c.Assert(err, IsNil)
458 cfg.Set("blog-title", "boring")
459 _, err = cfg.Write()
460 c.Assert(err, IsNil)
461 },457 },
462 change: watcher.Change{458 change: watcher.Change{
463 C: "services",459 C: "services",
@@ -468,7 +464,7 @@
468 Name: "wordpress",464 Name: "wordpress",
469 CharmURL: "local:series/series-wordpress-3",465 CharmURL: "local:series/series-wordpress-3",
470 Life: params.Life(Alive.String()),466 Life: params.Life(Alive.String()),
471 Config: map[string]interface{}{"blog-title": "boring"},467 Config: charm.Settings{"blog-title": "boring"},
472 },468 },
473 },469 },
474 },470 },
@@ -758,7 +754,7 @@
758 add: []params.EntityInfo{&params.ServiceInfo{754 add: []params.EntityInfo{&params.ServiceInfo{
759 Name: "wordpress",755 Name: "wordpress",
760 CharmURL: "local:series/series-wordpress-3",756 CharmURL: "local:series/series-wordpress-3",
761 Config: map[string]interface{}{"foo": "bar"},757 Config: charm.Settings{"foo": "bar"},
762 }},758 }},
763 setUp: func(c *C, st *State) {759 setUp: func(c *C, st *State) {
764 svc, err := st.AddService("wordpress", AddTestingCharm(c, st, "wordpress"))760 svc, err := st.AddService("wordpress", AddTestingCharm(c, st, "wordpress"))
@@ -773,7 +769,7 @@
773 &params.ServiceInfo{769 &params.ServiceInfo{
774 Name: "wordpress",770 Name: "wordpress",
775 CharmURL: "local:series/series-wordpress-3",771 CharmURL: "local:series/series-wordpress-3",
776 Config: map[string]interface{}{"blog-title": "foo"},772 Config: charm.Settings{"blog-title": "foo"},
777 },773 },
778 },774 },
779 }, {775 }, {
@@ -781,7 +777,7 @@
781 add: []params.EntityInfo{&params.ServiceInfo{777 add: []params.EntityInfo{&params.ServiceInfo{
782 Name: "wordpress",778 Name: "wordpress",
783 CharmURL: "local:series/series-wordpress-2", // Note different revno.779 CharmURL: "local:series/series-wordpress-2", // Note different revno.
784 Config: map[string]interface{}{"foo": "bar"},780 Config: charm.Settings{"foo": "bar"},
785 }},781 }},
786 setUp: func(c *C, st *State) {782 setUp: func(c *C, st *State) {
787 svc, err := st.AddService("wordpress", AddTestingCharm(c, st, "wordpress"))783 svc, err := st.AddService("wordpress", AddTestingCharm(c, st, "wordpress"))
@@ -796,7 +792,7 @@
796 &params.ServiceInfo{792 &params.ServiceInfo{
797 Name: "wordpress",793 Name: "wordpress",
798 CharmURL: "local:series/series-wordpress-2",794 CharmURL: "local:series/series-wordpress-2",
799 Config: map[string]interface{}{"foo": "bar"},795 Config: charm.Settings{"foo": "bar"},
800 },796 },
801 },797 },
802 }, {798 }, {
@@ -817,10 +813,7 @@
817}813}
818814
819func setServiceConfigAttr(c *C, svc *Service, attr string, val interface{}) {815func setServiceConfigAttr(c *C, svc *Service, attr string, val interface{}) {
820 cfg, err := svc.Config()816 err := svc.UpdateConfigSettings(charm.Settings{attr: val})
821 c.Assert(err, IsNil)
822 cfg.Set("blog-title", val)
823 _, err = cfg.Write()
824 c.Assert(err, IsNil)817 c.Assert(err, IsNil)
825}818}
826819
827820
=== modified file 'state/service.go'
--- state/service.go 2013-06-10 22:46:06 +0000
+++ state/service.go 2013-06-12 01:17:26 +0000
@@ -719,9 +719,19 @@
719 return relations, nil719 return relations, nil
720}720}
721721
722// updateConfigSettings changes a service's charm config settings. Values set722// ConfigSettings returns the raw user configuration for the service's charm.
723// Unset values are omitted.
724func (s *Service) ConfigSettings() (charm.Settings, error) {
725 settings, err := readSettings(s.st, s.settingsKey())
726 if err != nil {
727 return nil, err
728 }
729 return settings.Map(), nil
730}
731
732// UpdateConfigSettings changes a service's charm config settings. Values set
723// to nil will be deleted; unknown and invalid values will return an error.733// to nil will be deleted; unknown and invalid values will return an error.
724func (s *Service) updateConfigSettings(changes charm.Settings) error {734func (s *Service) UpdateConfigSettings(changes charm.Settings) error {
725 charm, _, err := s.Charm()735 charm, _, err := s.Charm()
726 if err != nil {736 if err != nil {
727 return err737 return err
@@ -749,43 +759,6 @@
749 return err759 return err
750}760}
751761
752// Config returns the configuration node for the service.
753func (s *Service) Config() (config *Settings, err error) {
754 config, err = readSettings(s.st, s.settingsKey())
755 if err != nil {
756 return nil, fmt.Errorf("cannot get configuration of service %q: %v", s, err)
757 }
758 return config, nil
759}
760
761// SetConfig changes a service's configuration values.
762// Values set to the empty string will be deleted.
763func (s *Service) SetConfig(options map[string]string) error {
764 charm, _, err := s.Charm()
765 if err != nil {
766 return err
767 }
768 changes, err := charm.Config().ParseSettingsStrings(options)
769 if err != nil {
770 return err
771 }
772 return s.updateConfigSettings(changes)
773}
774
775// SetConfigYAML is like Set except that the
776// configuration data is specified in YAML format.
777func (s *Service) SetConfigYAML(yamlData []byte) error {
778 charm, _, err := s.Charm()
779 if err != nil {
780 return err
781 }
782 changes, err := charm.Config().ParseSettingsYAML(yamlData, s.doc.Name)
783 if err != nil {
784 return err
785 }
786 return s.updateConfigSettings(changes)
787}
788
789var ErrSubordinateConstraints = stderrors.New("constraints do not apply to subordinate services")762var ErrSubordinateConstraints = stderrors.New("constraints do not apply to subordinate services")
790763
791// Constraints returns the current service constraints.764// Constraints returns the current service constraints.
792765
=== modified file 'state/service_test.go'
--- state/service_test.go 2013-06-10 22:46:06 +0000
+++ state/service_test.go 2013-06-12 01:17:26 +0000
@@ -226,9 +226,9 @@
226var setCharmConfigTests = []struct {226var setCharmConfigTests = []struct {
227 summary string227 summary string
228 startconfig string228 startconfig string
229 startvalues map[string]interface{}229 startvalues charm.Settings
230 endconfig string230 endconfig string
231 endvalues map[string]interface{}231 endvalues charm.Settings
232 err string232 err string
233}{233}{
234 {234 {
@@ -242,18 +242,18 @@
242 }, {242 }, {
243 summary: "add string key and preserve existing values",243 summary: "add string key and preserve existing values",
244 startconfig: stringConfig,244 startconfig: stringConfig,
245 startvalues: map[string]interface{}{"key": "foo", "other": "bar"},245 startvalues: charm.Settings{"key": "foo"},
246 endconfig: newStringConfig,246 endconfig: newStringConfig,
247 endvalues: map[string]interface{}{"key": "foo", "other": "bar"},247 endvalues: charm.Settings{"key": "foo"},
248 }, {248 }, {
249 summary: "remove string key",249 summary: "remove string key",
250 startconfig: stringConfig,250 startconfig: stringConfig,
251 startvalues: map[string]interface{}{"key": "value"},251 startvalues: charm.Settings{"key": "value"},
252 endconfig: emptyConfig,252 endconfig: emptyConfig,
253 }, {253 }, {
254 summary: "remove float key",254 summary: "remove float key",
255 startconfig: floatConfig,255 startconfig: floatConfig,
256 startvalues: map[string]interface{}{"key": 123.45},256 startvalues: charm.Settings{"key": 123.45},
257 endconfig: emptyConfig,257 endconfig: emptyConfig,
258 }, {258 }, {
259 summary: "change key type without values",259 summary: "change key type without values",
@@ -262,7 +262,7 @@
262 }, {262 }, {
263 summary: "change key type with values",263 summary: "change key type with values",
264 startconfig: stringConfig,264 startconfig: stringConfig,
265 startvalues: map[string]interface{}{"key": "value"},265 startvalues: charm.Settings{"key": "value"},
266 endconfig: floatConfig,266 endconfig: floatConfig,
267 },267 },
268}268}
@@ -281,15 +281,12 @@
281 origCh := charms[t.startconfig]281 origCh := charms[t.startconfig]
282 svc, err := s.State.AddService("wordpress", origCh)282 svc, err := s.State.AddService("wordpress", origCh)
283 c.Assert(err, IsNil)283 c.Assert(err, IsNil)
284 cfg, err := svc.Config()284 err = svc.UpdateConfigSettings(t.startvalues)
285 c.Assert(err, IsNil)
286 cfg.Update(t.startvalues)
287 _, err = cfg.Write()
288 c.Assert(err, IsNil)285 c.Assert(err, IsNil)
289286
290 newCh := charms[t.endconfig]287 newCh := charms[t.endconfig]
291 err = svc.SetCharm(newCh, false)288 err = svc.SetCharm(newCh, false)
292 var expectVals map[string]interface{}289 var expectVals charm.Settings
293 var expectCh *state.Charm290 var expectCh *state.Charm
294 if t.err != "" {291 if t.err != "" {
295 c.Assert(err, ErrorMatches, t.err)292 c.Assert(err, ErrorMatches, t.err)
@@ -304,12 +301,12 @@
304 sch, _, err := svc.Charm()301 sch, _, err := svc.Charm()
305 c.Assert(err, IsNil)302 c.Assert(err, IsNil)
306 c.Assert(sch.URL(), DeepEquals, expectCh.URL())303 c.Assert(sch.URL(), DeepEquals, expectCh.URL())
307 cfg, err = svc.Config()304 settings, err := svc.ConfigSettings()
308 c.Assert(err, IsNil)305 c.Assert(err, IsNil)
309 if len(expectVals) == 0 {306 if len(expectVals) == 0 {
310 c.Assert(cfg.Map(), HasLen, 0)307 c.Assert(settings, HasLen, 0)
311 } else {308 } else {
312 c.Assert(cfg.Map(), DeepEquals, expectVals)309 c.Assert(settings, DeepEquals, expectVals)
313 }310 }
314311
315 err = svc.Destroy()312 err = svc.Destroy()
@@ -317,138 +314,78 @@
317 }314 }
318}315}
319316
320func serviceSet(options map[string]string) func(svc *state.Service) error {317var serviceUpdateConfigSettingsTests = []struct {
321 return func(svc *state.Service) error {
322 return svc.SetConfig(options)
323 }
324}
325
326func serviceSetYAML(yaml string) func(svc *state.Service) error {
327 return func(svc *state.Service) error {
328 return svc.SetConfigYAML([]byte(yaml))
329 }
330}
331
332var serviceSetTests = []struct {
333 about string318 about string
334 initial map[string]interface{}319 initial charm.Settings
335 set func(st *state.Service) error320 update charm.Settings
336 expect map[string]interface{} // resulting configuration of the dummy service.321 expect charm.Settings
337 err string // error regex322 err string
338}{{323}{{
339 about: "unknown option",324 about: "unknown option",
340 set: serviceSet(map[string]string{"foo": "bar"}),325 update: charm.Settings{"foo": "bar"},
341 err: `unknown option "foo"`,326 err: `unknown option "foo"`,
342}, {327}, {
343 about: "set outlook",328 about: "bad type",
344 set: serviceSet(map[string]string{"outlook": "positive"}),329 update: charm.Settings{"skill-level": "profound"},
345 expect: map[string]interface{}{330 err: `option "skill-level" expected int, got "profound"`,
346 "outlook": "positive",331}, {
347 },332 about: "set string",
348}, {333 update: charm.Settings{"outlook": "positive"},
349 about: "unset outlook and set title",334 expect: charm.Settings{"outlook": "positive"},
350 initial: map[string]interface{}{335}, {
351 "outlook": "positive",336 about: "unset string and set another",
352 },337 initial: charm.Settings{"outlook": "positive"},
353 set: serviceSet(map[string]string{338 update: charm.Settings{"outlook": nil, "title": "sir"},
354 "outlook": "",339 expect: charm.Settings{"title": "sir"},
355 "title": "sir",340}, {
356 },341 about: "unset missing string",
357 ),342 update: charm.Settings{"outlook": nil},
358 expect: map[string]interface{}{343}, {
359 "title": "sir",344 about: `empty strings unset string values`,
360 },345 initial: charm.Settings{"outlook": "positive"},
361}, {346 update: charm.Settings{"outlook": "", "title": ""},
362 about: "set a default value",347}, {
363 initial: map[string]interface{}{348 about: "preserve existing value",
364 "title": "sir",349 initial: charm.Settings{"title": "sir"},
365 },350 update: charm.Settings{"username": "admin001"},
366 set: serviceSet(map[string]string{"username": "admin001"}),351 expect: charm.Settings{"username": "admin001", "title": "sir"},
367 expect: map[string]interface{}{352}, {
368 "username": "admin001",353 about: "unset a default value, set a different default",
369 "title": "sir",354 initial: charm.Settings{"username": "admin001", "title": "sir"},
370 },355 update: charm.Settings{"username": nil, "title": "My Title"},
371}, {356 expect: charm.Settings{"title": "My Title"},
372 about: "unset a default value, set a different default",357}, {
373 initial: map[string]interface{}{358 about: "non-string type",
374 "username": "admin001",359 update: charm.Settings{"skill-level": 303},
375 "title": "sir",360 expect: charm.Settings{"skill-level": int64(303)},
376 },361}, {
377 set: serviceSet(map[string]string{362 about: "unset non-string type",
378 "username": "",363 initial: charm.Settings{"skill-level": 303},
379 "title": "My Title",364 update: charm.Settings{"skill-level": nil},
380 },365}}
381 ),
382 expect: map[string]interface{}{
383 "title": "My Title",
384 },
385}, {
386 about: "gibberish",
387 set: serviceSetYAML("`"),
388 err: `cannot parse settings data: .*`,
389}, {
390 about: "bad configuration",
391 set: serviceSetYAML("{}"),
392 err: `no settings found for "dummy-service"`,
393}, {
394 about: "config with no options",
395 set: serviceSetYAML("dummy-service: {}"),
396 expect: map[string]interface{}{},
397}, {
398 about: "set some attributes",
399 initial: map[string]interface{}{
400 "title": "sir",
401 },
402 set: serviceSetYAML("dummy-service:\n skill-level: 9000\n username: admin001\n\n"),
403 expect: map[string]interface{}{
404 "title": "sir",
405 "username": "admin001",
406 "skill-level": int64(9000), // yaml int types are int64
407 },
408}, {
409 about: "remove an attribute by setting to nil",
410 initial: map[string]interface{}{
411 "title": "sir",
412 "username": "foo",
413 },
414 set: serviceSetYAML("dummy-service: {title: null}"),
415 expect: map[string]interface{}{
416 "username": "foo",
417 },
418}, {
419 about: "remove an attribute by setting to empty string",
420 initial: map[string]interface{}{
421 "title": "sir",
422 "username": "foo",
423 },
424 set: serviceSetYAML("dummy-service: {title: ''}"),
425 expect: map[string]interface{}{
426 "username": "foo",
427 },
428},
429}
430366
431func (s *ServiceSuite) TestSet(c *C) {367func (s *ServiceSuite) TestUpdateConfigSettings(c *C) {
432 sch := s.AddTestingCharm(c, "dummy")368 sch := s.AddTestingCharm(c, "dummy")
433 for i, t := range serviceSetTests {369 for i, t := range serviceUpdateConfigSettingsTests {
434 c.Logf("test %d. %s", i, t.about)370 c.Logf("test %d. %s", i, t.about)
435 svc, err := s.State.AddService("dummy-service", sch)371 svc, err := s.State.AddService("dummy-service", sch)
436 c.Assert(err, IsNil)372 c.Assert(err, IsNil)
437 if t.initial != nil {373 if t.initial != nil {
438 cfg, err := svc.Config()374 err := svc.UpdateConfigSettings(t.initial)
439 c.Assert(err, IsNil)
440 cfg.Update(t.initial)
441 _, err = cfg.Write()
442 c.Assert(err, IsNil)375 c.Assert(err, IsNil)
443 }376 }
444 err = t.set(svc)377 err = svc.UpdateConfigSettings(t.update)
445 if t.err != "" {378 if t.err != "" {
446 c.Assert(err, ErrorMatches, t.err)379 c.Assert(err, ErrorMatches, t.err)
447 } else {380 } else {
448 c.Assert(err, IsNil)381 c.Assert(err, IsNil)
449 cfg, err := svc.Config()382 settings, err := svc.ConfigSettings()
450 c.Assert(err, IsNil)383 c.Assert(err, IsNil)
451 c.Assert(cfg.Map(), DeepEquals, t.expect)384 expect := t.expect
385 if expect == nil {
386 expect = charm.Settings{}
387 }
388 c.Assert(settings, DeepEquals, expect)
452 }389 }
453 err = svc.Destroy()390 err = svc.Destroy()
454 c.Assert(err, IsNil)391 c.Assert(err, IsNil)
@@ -1127,25 +1064,6 @@
1127 c.Assert(err, ErrorMatches, `unit "mysql/0" not found`)1064 c.Assert(err, ErrorMatches, `unit "mysql/0" not found`)
1128}1065}
11291066
1130func (s *ServiceSuite) TestServiceConfig(c *C) {
1131 env, err := s.mysql.Config()
1132 c.Assert(err, IsNil)
1133 err = env.Read()
1134 c.Assert(err, IsNil)
1135 c.Assert(env.Map(), DeepEquals, map[string]interface{}{})
1136
1137 env.Update(map[string]interface{}{"spam": "eggs", "eggs": "spam"})
1138 env.Update(map[string]interface{}{"spam": "spam", "chaos": "emeralds"})
1139 _, err = env.Write()
1140 c.Assert(err, IsNil)
1141
1142 env, err = s.mysql.Config()
1143 c.Assert(err, IsNil)
1144 err = env.Read()
1145 c.Assert(err, IsNil)
1146 c.Assert(env.Map(), DeepEquals, map[string]interface{}{"spam": "spam", "eggs": "spam", "chaos": "emeralds"})
1147}
1148
1149func uint64p(val uint64) *uint64 {1067func uint64p(val uint64) *uint64 {
1150 return &val1068 return &val
1151}1069}
11521070
=== modified file 'state/statecmd/config_test.go'
--- state/statecmd/config_test.go 2013-05-02 15:55:42 +0000
+++ state/statecmd/config_test.go 2013-06-12 01:17:26 +0000
@@ -134,7 +134,9 @@
134 c.Assert(err, IsNil)134 c.Assert(err, IsNil)
135 }135 }
136 if t.config != nil {136 if t.config != nil {
137 err = svc.SetConfig(t.config)137 settings, err := ch.Config().ParseSettingsStrings(t.config)
138 c.Assert(err, IsNil)
139 err = svc.UpdateConfigSettings(settings)
138 c.Assert(err, IsNil)140 c.Assert(err, IsNil)
139 }141 }
140 expect := t.expect142 expect := t.expect
141143
=== modified file 'state/statecmd/deploy_test.go'
--- state/statecmd/deploy_test.go 2013-06-10 22:46:06 +0000
+++ state/statecmd/deploy_test.go 2013-06-12 01:17:26 +0000
@@ -219,9 +219,9 @@
219 s.AssertService(c, "dummy", curl, 1, 0)219 s.AssertService(c, "dummy", curl, 1, 0)
220 svc, err := s.State.Service("dummy")220 svc, err := s.State.Service("dummy")
221 c.Assert(err, IsNil)221 c.Assert(err, IsNil)
222 cfg, err := svc.Config()222 settings, err := svc.ConfigSettings()
223 c.Assert(err, IsNil)223 c.Assert(err, IsNil)
224 c.Assert(cfg.Map(), DeepEquals, map[string]interface{}{224 c.Assert(settings, DeepEquals, charm.Settings{
225 "skill-level": int64(1),225 "skill-level": int64(1),
226 })226 })
227}227}
@@ -239,9 +239,9 @@
239 s.AssertService(c, "dummy", curl, 1, 0)239 s.AssertService(c, "dummy", curl, 1, 0)
240 svc, err := s.State.Service("dummy")240 svc, err := s.State.Service("dummy")
241 c.Assert(err, IsNil)241 c.Assert(err, IsNil)
242 cfg, err := svc.Config()242 settings, err := svc.ConfigSettings()
243 c.Assert(err, IsNil)243 c.Assert(err, IsNil)
244 c.Assert(cfg.Map(), DeepEquals, map[string]interface{}{244 c.Assert(settings, DeepEquals, charm.Settings{
245 "skill-level": int64(9001),245 "skill-level": int64(9001),
246 })246 })
247}247}
248248
=== modified file 'state/statecmd/get.go'
--- state/statecmd/get.go 2013-05-02 15:55:42 +0000
+++ state/statecmd/get.go 2013-06-12 01:17:26 +0000
@@ -18,53 +18,48 @@
1818
19// ServiceGet returns the configuration for the named service.19// ServiceGet returns the configuration for the named service.
20func ServiceGet(st *state.State, p params.ServiceGet) (params.ServiceGetResults, error) {20func ServiceGet(st *state.State, p params.ServiceGet) (params.ServiceGetResults, error) {
21 svc, err := st.Service(p.ServiceName)21 service, err := st.Service(p.ServiceName)
22 if err != nil {22 if err != nil {
23 return params.ServiceGetResults{}, err23 return params.ServiceGetResults{}, err
24 }24 }
25 svcCfg, err := svc.Config()25 settings, err := service.ConfigSettings()
26 if err != nil {26 if err != nil {
27 return params.ServiceGetResults{}, err27 return params.ServiceGetResults{}, err
28 }28 }
29 charm, _, err := svc.Charm()29 charm, _, err := service.Charm()
30 if err != nil {30 if err != nil {
31 return params.ServiceGetResults{}, err31 return params.ServiceGetResults{}, err
32 }32 }
33 charmCfg := charm.Config().Options33 configInfo := describe(settings, charm.Config())
34
35 var constraints constraints.Value34 var constraints constraints.Value
36 if svc.IsPrincipal() {35 if service.IsPrincipal() {
37 constraints, err = svc.Constraints()36 constraints, err = service.Constraints()
38 if err != nil {37 if err != nil {
39 return params.ServiceGetResults{}, err38 return params.ServiceGetResults{}, err
40 }39 }
41 }40 }
42
43 return params.ServiceGetResults{41 return params.ServiceGetResults{
44 Service: p.ServiceName,42 Service: p.ServiceName,
45 Charm: charm.Meta().Name,43 Charm: charm.Meta().Name,
46 Config: merge(svcCfg.Map(), charmCfg),44 Config: configInfo,
47 Constraints: constraints,45 Constraints: constraints,
48 }, nil46 }, nil
49}47}
5048
51// merge returns the service settings merged with the charm49func describe(settings charm.Settings, config *charm.Config) map[string]interface{} {
52// schema, taking default values from the configuration
53// in the charm metadata.
54func merge(serviceCfg map[string]interface{}, charmCfg map[string]charm.Option) map[string]interface{} {
55 results := make(map[string]interface{})50 results := make(map[string]interface{})
56 for k, v := range charmCfg {51 for name, option := range config.Options {
57 m := map[string]interface{}{52 info := map[string]interface{}{
58 "description": v.Description,53 "description": option.Description,
59 "type": v.Type,54 "type": option.Type,
60 }55 }
61 if s, ok := serviceCfg[k]; ok {56 if value := settings[name]; value != nil {
62 m["value"] = s57 info["value"] = value
63 } else {58 } else {
64 m["value"] = v.Default59 info["value"] = option.Default
65 m["default"] = true60 info["default"] = true
66 }61 }
67 results[k] = m62 results[name] = info
68 }63 }
69 return results64 return results
70}65}
7166
=== modified file 'state/unit_test.go'
--- state/unit_test.go 2013-06-10 21:30:13 +0000
+++ state/unit_test.go 2013-06-12 01:17:26 +0000
@@ -59,7 +59,7 @@
59}59}
6060
61func (s *UnitSuite) TestConfigSettingsReflectService(c *C) {61func (s *UnitSuite) TestConfigSettingsReflectService(c *C) {
62 err := s.service.SetConfig(map[string]string{"blog-title": "no title"})62 err := s.service.UpdateConfigSettings(charm.Settings{"blog-title": "no title"})
63 c.Assert(err, IsNil)63 c.Assert(err, IsNil)
64 err = s.unit.SetCharmURL(s.charm.URL())64 err = s.unit.SetCharmURL(s.charm.URL())
65 c.Assert(err, IsNil)65 c.Assert(err, IsNil)
@@ -67,7 +67,7 @@
67 c.Assert(err, IsNil)67 c.Assert(err, IsNil)
68 c.Assert(settings, DeepEquals, charm.Settings{"blog-title": "no title"})68 c.Assert(settings, DeepEquals, charm.Settings{"blog-title": "no title"})
6969
70 err = s.service.SetConfig(map[string]string{"blog-title": "ironic title"})70 err = s.service.UpdateConfigSettings(charm.Settings{"blog-title": "ironic title"})
71 c.Assert(err, IsNil)71 c.Assert(err, IsNil)
72 settings, err = s.unit.ConfigSettings()72 settings, err = s.unit.ConfigSettings()
73 c.Assert(err, IsNil)73 c.Assert(err, IsNil)
@@ -127,18 +127,18 @@
127 assertChange()127 assertChange()
128128
129 // Update config a couple of times, check a single event.129 // Update config a couple of times, check a single event.
130 err = s.service.SetConfig(map[string]string{130 err = s.service.UpdateConfigSettings(charm.Settings{
131 "blog-title": "superhero paparazzi",131 "blog-title": "superhero paparazzi",
132 })132 })
133 c.Assert(err, IsNil)133 c.Assert(err, IsNil)
134 err = s.service.SetConfig(map[string]string{134 err = s.service.UpdateConfigSettings(charm.Settings{
135 "blog-title": "sauceror central",135 "blog-title": "sauceror central",
136 })136 })
137 c.Assert(err, IsNil)137 c.Assert(err, IsNil)
138 assertChange()138 assertChange()
139139
140 // Non-change is not reported.140 // Non-change is not reported.
141 err = s.service.SetConfig(map[string]string{141 err = s.service.UpdateConfigSettings(charm.Settings{
142 "blog-title": "sauceror central",142 "blog-title": "sauceror central",
143 })143 })
144 c.Assert(err, IsNil)144 c.Assert(err, IsNil)
@@ -151,8 +151,8 @@
151 assertNoChange()151 assertNoChange()
152152
153 // Change service config for new charm; nothing detected.153 // Change service config for new charm; nothing detected.
154 err = s.service.SetConfig(map[string]string{154 err = s.service.UpdateConfigSettings(charm.Settings{
155 "key": "42.0",155 "key": 42.0,
156 })156 })
157 c.Assert(err, IsNil)157 c.Assert(err, IsNil)
158 assertNoChange()158 assertNoChange()
159159
=== modified file 'worker/uniter/context_test.go'
--- worker/uniter/context_test.go 2013-06-10 21:04:37 +0000
+++ worker/uniter/context_test.go 2013-06-12 01:17:26 +0000
@@ -577,10 +577,9 @@
577 c.Assert(settings, DeepEquals, charm.Settings{"blog-title": "My Title"})577 c.Assert(settings, DeepEquals, charm.Settings{"blog-title": "My Title"})
578578
579 // Change remote config.579 // Change remote config.
580 node, err := s.service.Config()580 err = s.service.UpdateConfigSettings(charm.Settings{
581 c.Assert(err, IsNil)581 "blog-title": "Something Else",
582 node.Set("blog-title", "Something Else")582 })
583 _, err = node.Write()
584 c.Assert(err, IsNil)583 c.Assert(err, IsNil)
585584
586 // Local view is not changed.585 // Local view is not changed.
587586
=== modified file 'worker/uniter/filter_test.go'
--- worker/uniter/filter_test.go 2013-05-02 15:55:42 +0000
+++ worker/uniter/filter_test.go 2013-06-12 01:17:26 +0000
@@ -266,6 +266,9 @@
266 }266 }
267 assertNoChange()267 assertNoChange()
268268
269 // Set the charm URL to trigger config events.
270 err = f.SetCharm(s.wpcharm.URL())
271 c.Assert(err, IsNil)
269 assertChange := func() {272 assertChange := func() {
270 s.State.Sync()273 s.State.Sync()
271 select {274 select {
@@ -276,34 +279,23 @@
276 }279 }
277 assertNoChange()280 assertNoChange()
278 }281 }
279
280 // Set the charm URL to trigger config events.
281 err = f.SetCharm(s.wpcharm.URL())
282 c.Assert(err, IsNil)
283 assertChange()282 assertChange()
284283
285 // Make sure the charm URL is set now.
286 s.unit.Refresh()
287 curl, ok := s.unit.CharmURL()
288 c.Assert(ok, Equals, true)
289 c.Assert(curl, DeepEquals, s.wpcharm.URL())
290
291 // Change the config; new event received.284 // Change the config; new event received.
292 node, err := s.wordpress.Config()285 changeConfig := func(title interface{}) {
293 c.Assert(err, IsNil)286 err := s.wordpress.UpdateConfigSettings(charm.Settings{
294 node.Set("skill-level", 9001)287 "blog-title": title,
295 _, err = node.Write()288 })
296 c.Assert(err, IsNil)289 c.Assert(err, IsNil)
290 }
291 changeConfig("20,000 leagues in the cloud")
297 assertChange()292 assertChange()
298293
299 // Change the config a couple of times, then reset the events.294 // Change the config a few more times, then reset the events. We sync to
300 node.Set("title", "20,000 leagues in the cloud")295 // make sure the event has come into the filter before we tell it to discard
301 _, err = node.Write()296 // all received events.
302 c.Assert(err, IsNil)297 changeConfig(nil)
303 node.Set("outlook", "precipitous")298 changeConfig("the curious incident of the dog in the cloud")
304 _, err = node.Write()
305 c.Assert(err, IsNil)
306 // We make sure the event has come into the filter before we tell it to discard any received.
307 s.State.Sync()299 s.State.Sync()
308 f.DiscardConfigEvent()300 f.DiscardConfigEvent()
309 assertNoChange()301 assertNoChange()
@@ -318,12 +310,8 @@
318 assertNoChange()310 assertNoChange()
319311
320 // Further changes are still collapsed as appropriate.312 // Further changes are still collapsed as appropriate.
321 node.Set("skill-level", 123)313 changeConfig("forsooth")
322 _, err = node.Write()314 changeConfig("imagination failure")
323 c.Assert(err, IsNil)
324 node.Set("outlook", "expressive")
325 _, err = node.Write()
326 c.Assert(err, IsNil)
327 assertChange()315 assertChange()
328}316}
329317
330318
=== modified file 'worker/uniter/uniter_test.go'
--- worker/uniter/uniter_test.go 2013-06-05 02:44:14 +0000
+++ worker/uniter/uniter_test.go 2013-06-12 01:17:26 +0000
@@ -1320,10 +1320,7 @@
1320type changeConfig map[string]interface{}1320type changeConfig map[string]interface{}
13211321
1322func (s changeConfig) step(c *C, ctx *context) {1322func (s changeConfig) step(c *C, ctx *context) {
1323 node, err := ctx.svc.Config()1323 err := ctx.svc.UpdateConfigSettings(charm.Settings(s))
1324 c.Assert(err, IsNil)
1325 node.Update(s)
1326 _, err = node.Write()
1327 c.Assert(err, IsNil)1324 c.Assert(err, IsNil)
1328}1325}
13291326

Subscribers

People subscribed via source and target branches