juju-core

Merge lp:~fwereade/juju-core/config-7-deploy-sensible-layering into lp:~juju/juju-core/trunk

  1. config-7-deploy-sensible-layering
  2. Merge into trunk
Proposed by William Reade
Status: Merged
Merged at revision: 1277
Proposed branch: lp:~fwereade/juju-core/config-7-deploy-sensible-layering
Merge into: lp:~juju/juju-core/trunk
Prerequisite: lp:~fwereade/juju-core/config-6-state-service-sane-methods
Diff against target: 1318 lines (+590/-530)
11 files modified
cmd/juju/deploy.go (+38/-14)
cmd/juju/deploy_test.go (+148/-0)
juju/conn.go (+32/-28)
juju/conn_test.go (+145/-47)
state/api/params/params.go (+0/-1)
state/apiserver/client.go (+38/-22)
state/apiserver/client_test.go (+124/-67)
state/apiserver/perm_test.go (+6/-15)
state/statecmd/deploy.go (+0/-63)
state/statecmd/deploy_test.go (+0/-273)
testing/charm.go (+59/-0)
To merge this branch: bzr merge lp:~fwereade/juju-core/config-7-deploy-sensible-layering
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+168581@code.launchpad.net

Description of the change

deploy refactoring

GUI deploy and CLI deploy are different enough that the common statecmd
caused more problems than it solved. Testing is noticeably improved.

juju.Conn.DeployService and juju.Conn.AddUnits are now probably ready to
move to some other place that just requires a state connection (and not
an environment as well); juju.Conn.PutCharm needs some love too, and
thought devoted to how we're going to put local charms over the API.

But, for now, the various bits all happen in the right place (*except* that
the CLI once again downloads store charms and uploads them itself, rather
than taking advantage of that functionality on the server side. This can
and will be fixed, but not this CL).

https://codereview.appspot.com/10166044/

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

Reviewers: mp+168581_code.launchpad.net,

Message:
Please take a look.

Description:
deploy refactoring

GUI deploy and CLI deploy are different enough that the common statecmd
caused more problems than it solved. Testing is noticeably improved.

juju.Conn.DeployService and juju.Conn.AddUnits are now probably ready to
move to some other place that just requires a state connection (and not
an environment as well); juju.Conn.PutCharm needs some love too, and
thought devoted to how we're going to put local charms over the API.

But, for now, the various bits all happen in the right place (*except*
that
the CLI once again downloads store charms and uploads them itself,
rather
than taking advantage of that functionality on the server side. This can
and will be fixed, but not this CL).

https://code.launchpad.net/~fwereade/juju-core/config-7-deploy-sensible-layering/+merge/168581

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

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M cmd/juju/deploy.go
   M cmd/juju/deploy_test.go
   M juju/conn.go
   M juju/conn_test.go
   M state/api/params/params.go
   M state/apiserver/client.go
   M state/apiserver/client_test.go
   M state/apiserver/perm_test.go
   D state/statecmd/deploy.go
   D state/statecmd/deploy_test.go
   M testing/charm.go

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

LGTM, with some thoughts and suggestions below.
very nice, and thanks in particular for the extra test coverage.

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

https://codereview.appspot.com/10166044/diff/1/cmd/juju/deploy.go#newcode141
cmd/juju/deploy.go:141: return errors.New("cannot specify units for
subordinate service")
s/units/unit count/ ?

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

https://codereview.appspot.com/10166044/diff/1/cmd/juju/deploy_test.go#newcode62
cmd/juju/deploy_test.go:62:
tyvm for the extra tests.

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

https://codereview.appspot.com/10166044/diff/1/juju/conn.go#newcode214
juju/conn.go:214: // (, minimumUnitCount, initialMachineIds?).
s/, //

https://codereview.appspot.com/10166044/diff/1/juju/conn.go#newcode227
juju/conn.go:227: if args.Constraints != emptyCons {
maybe we should define

var None Value

in the constraints package.

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

https://codereview.appspot.com/10166044/diff/1/state/apiserver/client_test.go#newcode305
state/apiserver/client_test.go:305: withMockCharmStore(func(store
*coretesting.MockCharmStore) {
tbh i think i'd prefer:

store, restore := newMockCharmStore()
defer restore()

and lose the extra indentation.

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

https://codereview.appspot.com/10166044/diff/1/state/apiserver/perm_test.go#newcode290
state/apiserver/perm_test.go:290: withMockCharmStore(func(store
*coretesting.MockCharmStore) {
let's just lose all this and try to deploy an invalid charm.
various examples of that technique below (e.g. opClientAddServiceUnits).

then we could move the mock charm store stuff into the client test
suite.

https://codereview.appspot.com/10166044/diff/1/testing/charm.go
File testing/charm.go (right):

https://codereview.appspot.com/10166044/diff/1/testing/charm.go#newcode115
testing/charm.go:115: type MockCharmStore struct {
doc comment please

https://codereview.appspot.com/10166044/diff/1/testing/charm.go#newcode123
testing/charm.go:123: func (s *MockCharmStore) SetCharm(curl *charm.URL,
bundle *charm.Bundle) error {
ditto

https://codereview.appspot.com/10166044/diff/1/testing/charm.go#newcode144
testing/charm.go:144: func (s *MockCharmStore) interpret(curl
*charm.URL) (base string, rev int) {
ditto (what does "interpret" mean in this context?)

https://codereview.appspot.com/10166044/diff/1/testing/charm.go#newcode156
testing/charm.go:156: func (s *MockCharmStore) Get(curl *charm.URL)
(charm.Charm, error) {
// Get implements charm.Repository.Get.
?

https://codereview.appspot.com/10166044/diff/1/testing/charm.go#newcode165
testing/charm.go:165: func (s *MockCharmStore) Latest(curl *charm.URL)
(int, error) {
// Latest implements charm.Repository.Latest.
?

https://codereview.appspot.com/10166044/

Revision history for this message
John A Meinel (jameinel) wrote :

LGTM overall

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

https://codereview.appspot.com/10166044/diff/1/state/apiserver/client_test.go#newcode305
state/apiserver/client_test.go:305: withMockCharmStore(func(store
*coretesting.MockCharmStore) {
On 2013/06/11 13:22:07, rog wrote:
> tbh i think i'd prefer:

> store, restore := newMockCharmStore()
> defer restore()

> and lose the extra indentation.

Given I don't think you'll ever want to have a test call
'withMockCharmStore' 2 times, I think I agree. We have the pattern:

defer MakeFakeHome().Restore()

elsewhere, but I guess you need the return parameter?

https://codereview.appspot.com/10166044/

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

LGTM with the comments of Roger and John.

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

https://codereview.appspot.com/10166044/diff/1/juju/conn.go#newcode227
juju/conn.go:227: if args.Constraints != emptyCons {
On 2013/06/11 13:22:07, rog wrote:
> maybe we should define

> var None Value

> in the constraints package.

+1

https://codereview.appspot.com/10166044/diff/1/juju/conn_test.go
File juju/conn_test.go (right):

https://codereview.appspot.com/10166044/diff/1/juju/conn_test.go#newcode408
juju/conn_test.go:408: // test, because ServiceDeploy demands that a
charm already exists in state,
s/ServiceDeploy/DeployService/

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

https://codereview.appspot.com/10166044/diff/1/state/apiserver/client_test.go#newcode305
state/apiserver/client_test.go:305: withMockCharmStore(func(store
*coretesting.MockCharmStore) {
On 2013/06/12 08:19:56, jameinel wrote:
> On 2013/06/11 13:22:07, rog wrote:
> > tbh i think i'd prefer:
> >
> > store, restore := newMockCharmStore()
> > defer restore()
> >
> > and lose the extra indentation.

> Given I don't think you'll ever want to have a test call
'withMockCharmStore' 2
> times, I think I agree. We have the pattern:

> defer MakeFakeHome().Restore()

> elsewhere, but I guess you need the return parameter?

store := coretesting.MockCharmStore{}
defer makeMockCharmStore(&store).restore()

https://codereview.appspot.com/10166044/diff/1/testing/charm.go
File testing/charm.go (right):

https://codereview.appspot.com/10166044/diff/1/testing/charm.go#newcode115
testing/charm.go:115: type MockCharmStore struct {
On 2013/06/11 13:22:07, rog wrote:
> doc comment please

+1

https://codereview.appspot.com/10166044/

Revision history for this message
William Reade (fwereade) wrote :
Download full text (5.4 KiB)

*** Submitted:

deploy refactoring

GUI deploy and CLI deploy are different enough that the common statecmd
caused more problems than it solved. Testing is noticeably improved.

juju.Conn.DeployService and juju.Conn.AddUnits are now probably ready to
move to some other place that just requires a state connection (and not
an environment as well); juju.Conn.PutCharm needs some love too, and
thought devoted to how we're going to put local charms over the API.

But, for now, the various bits all happen in the right place (*except*
that
the CLI once again downloads store charms and uploads them itself,
rather
than taking advantage of that functionality on the server side. This can
and will be fixed, but not this CL).

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

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

https://codereview.appspot.com/10166044/diff/1/cmd/juju/deploy.go#newcode141
cmd/juju/deploy.go:141: return errors.New("cannot specify units for
subordinate service")
On 2013/06/11 13:22:07, rog wrote:
> s/units/unit count/ ?

Done differently.

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

https://codereview.appspot.com/10166044/diff/1/juju/conn.go#newcode214
juju/conn.go:214: // (, minimumUnitCount, initialMachineIds?).
On 2013/06/11 13:22:07, rog wrote:
> s/, //

Done.

https://codereview.appspot.com/10166044/diff/1/juju/conn.go#newcode227
juju/conn.go:227: if args.Constraints != emptyCons {
On 2013/06/12 08:30:01, mue wrote:
> On 2013/06/11 13:22:07, rog wrote:
> > maybe we should define
> >
> > var None Value
> >
> > in the constraints package.

> +1

Yeah, it is a bit annoying, but I recoil at the possibility/likelihood
of someone modifying Empty. Any objections to
constraints.Value.IsEmpty() in a followup?

https://codereview.appspot.com/10166044/diff/1/juju/conn_test.go
File juju/conn_test.go (right):

https://codereview.appspot.com/10166044/diff/1/juju/conn_test.go#newcode408
juju/conn_test.go:408: // test, because ServiceDeploy demands that a
charm already exists in state,
On 2013/06/12 08:30:01, mue wrote:
> s/ServiceDeploy/DeployService/

Ha, thanks. Done.

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

https://codereview.appspot.com/10166044/diff/1/state/apiserver/client_test.go#newcode305
state/apiserver/client_test.go:305: withMockCharmStore(func(store
*coretesting.MockCharmStore) {
On 2013/06/12 08:30:01, mue wrote:
> On 2013/06/12 08:19:56, jameinel wrote:
> > On 2013/06/11 13:22:07, rog wrote:
> > > tbh i think i'd prefer:
> > >
> > > store, restore := newMockCharmStore()
> > > defer restore()
> > >
> > > and lose the extra indentation.
> >
> > Given I don't think you'll ever want to have a test call
 'withMockCharmStore'
> 2
> > times, I think I agree. We have the pattern:
> >
> > defer MakeFakeHome().Restore()
> >
> > elsewhere, but I guess you need the return parameter?

> store := coretesting.MockCharmStore{}
> defer makeMockCharmStore(&store).restore()

Was originally written to support a table-based test that wanted a fresh
Charm...

Read more...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'cmd/juju/deploy.go'
2--- cmd/juju/deploy.go 2013-06-11 02:14:27 +0000
3+++ cmd/juju/deploy.go 2013-06-11 02:14:27 +0000
4@@ -6,15 +6,12 @@
5 import (
6 "errors"
7 "fmt"
8- "io/ioutil"
9 "launchpad.net/gnuflag"
10 "launchpad.net/juju-core/charm"
11 "launchpad.net/juju-core/cmd"
12 "launchpad.net/juju-core/constraints"
13 "launchpad.net/juju-core/juju"
14 "launchpad.net/juju-core/state"
15- "launchpad.net/juju-core/state/api/params"
16- "launchpad.net/juju-core/state/statecmd"
17 "os"
18 )
19
20@@ -123,22 +120,49 @@
21 if err != nil {
22 return err
23 }
24- var configYAML []byte
25+ // TODO(fwereade) it's annoying to roundtrip the bytes through the client
26+ // here, but it's the original behaviour and not convenient to change.
27+ // PutCharm will always be required in some form for local charms; and we
28+ // will need an EnsureStoreCharm method somewhere that gets the state.Charm
29+ // for use in the following checks.
30+ ch, err := conn.PutCharm(curl, repo, c.BumpRevision)
31+ if err != nil {
32+ return err
33+ }
34+ numUnits := c.NumUnits
35+ if ch.Meta().Subordinate {
36+ empty := constraints.Value{}
37+ if c.Constraints != empty {
38+ return errors.New("cannot specify constraints for subordinate service")
39+ }
40+ if numUnits == 1 && c.ForceMachineId == "" {
41+ numUnits = 0
42+ } else {
43+ return errors.New("cannot specify units for subordinate service")
44+ }
45+ }
46+ serviceName := c.ServiceName
47+ if serviceName == "" {
48+ serviceName = ch.Meta().Name
49+ }
50+ var settings charm.Settings
51 if c.Config.Path != "" {
52- configYAML, err = ioutil.ReadFile(c.Config.Path)
53+ configYAML, err := c.Config.Read(ctx)
54+ if err != nil {
55+ return err
56+ }
57+ settings, err = ch.Config().ParseSettingsYAML(configYAML, serviceName)
58 if err != nil {
59 return err
60 }
61 }
62- args := params.ServiceDeploy{
63- ServiceName: c.ServiceName,
64- CharmUrl: c.CharmName,
65- NumUnits: c.NumUnits,
66- // BUG(lp:1162122): --config has no tests.
67- ConfigYAML: string(configYAML),
68+ _, err = conn.DeployService(juju.DeployServiceParams{
69+ ServiceName: serviceName,
70+ Charm: ch,
71+ NumUnits: numUnits,
72+ ConfigSettings: settings,
73 Constraints: c.Constraints,
74- BumpRevision: c.BumpRevision,
75 ForceMachineId: c.ForceMachineId,
76- }
77- return statecmd.ServiceDeploy(conn.State, args, conn, curl, repo)
78+ })
79+ return err
80 }
81
82=== modified file 'cmd/juju/deploy_test.go'
83--- cmd/juju/deploy_test.go 2013-06-11 02:14:27 +0000
84+++ cmd/juju/deploy_test.go 2013-06-11 02:14:27 +0000
85@@ -5,7 +5,11 @@
86
87 import (
88 . "launchpad.net/gocheck"
89+ "launchpad.net/juju-core/charm"
90+ "launchpad.net/juju-core/constraints"
91+ "launchpad.net/juju-core/errors"
92 "launchpad.net/juju-core/juju/testing"
93+ "launchpad.net/juju-core/state"
94 coretesting "launchpad.net/juju-core/testing"
95 )
96
97@@ -55,3 +59,147 @@
98 c.Assert(err, ErrorMatches, t.err)
99 }
100 }
101+
102+func (s *DeploySuite) TestNoCharm(c *C) {
103+ err := runDeploy(c, "local:unknown-123")
104+ c.Assert(err, ErrorMatches, `cannot get charm: charm not found in ".*": local:precise/unknown-123`)
105+}
106+
107+func (s *DeploySuite) TestCharmDir(c *C) {
108+ coretesting.Charms.ClonedDirPath(s.SeriesPath, "dummy")
109+ err := runDeploy(c, "local:dummy")
110+ c.Assert(err, IsNil)
111+ curl := charm.MustParseURL("local:precise/dummy-1")
112+ s.AssertService(c, "dummy", curl, 1, 0)
113+}
114+
115+func (s *DeploySuite) TestUpgradeCharmDir(c *C) {
116+ dirPath := coretesting.Charms.ClonedDirPath(s.SeriesPath, "dummy")
117+ err := runDeploy(c, "local:dummy", "-u")
118+ c.Assert(err, IsNil)
119+ curl := charm.MustParseURL("local:precise/dummy-2")
120+ s.AssertService(c, "dummy", curl, 1, 0)
121+ // Check the charm really was upgraded.
122+ ch, err := charm.ReadDir(dirPath)
123+ c.Assert(err, IsNil)
124+ c.Assert(ch.Revision(), Equals, 2)
125+}
126+
127+func (s *DeploySuite) TestCharmBundle(c *C) {
128+ coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
129+ err := runDeploy(c, "local:dummy", "some-service-name")
130+ c.Assert(err, IsNil)
131+ curl := charm.MustParseURL("local:precise/dummy-1")
132+ s.AssertService(c, "some-service-name", curl, 1, 0)
133+}
134+
135+func (s *DeploySuite) TestCannotUpgradeCharmBundle(c *C) {
136+ coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
137+ err := runDeploy(c, "local:dummy", "-u")
138+ c.Assert(err, ErrorMatches, `cannot increment revision of charm "local:precise/dummy-1": not a directory`)
139+ // Verify state not touched...
140+ curl := charm.MustParseURL("local:precise/dummy-1")
141+ _, err = s.State.Charm(curl)
142+ c.Assert(err, ErrorMatches, `charm "local:precise/dummy-1" not found`)
143+ _, err = s.State.Service("dummy")
144+ c.Assert(err, ErrorMatches, `service "dummy" not found`)
145+}
146+
147+func (s *DeploySuite) TestSubordinateCharm(c *C) {
148+ coretesting.Charms.BundlePath(s.SeriesPath, "logging")
149+ err := runDeploy(c, "local:logging")
150+ c.Assert(err, IsNil)
151+ curl := charm.MustParseURL("local:precise/logging-1")
152+ s.AssertService(c, "logging", curl, 0, 0)
153+}
154+
155+func (s *DeploySuite) TestConfig(c *C) {
156+ coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
157+ path := setupConfigfile(c, c.MkDir())
158+ err := runDeploy(c, "local:dummy", "dummy-service", "--config", path)
159+ c.Assert(err, IsNil)
160+ service, err := s.State.Service("dummy-service")
161+ c.Assert(err, IsNil)
162+ settings, err := service.ConfigSettings()
163+ c.Assert(err, IsNil)
164+ c.Assert(settings, DeepEquals, charm.Settings{
165+ "skill-level": int64(9000),
166+ "username": "admin001",
167+ })
168+}
169+
170+func (s *DeploySuite) TestConfigError(c *C) {
171+ coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
172+ path := setupConfigfile(c, c.MkDir())
173+ err := runDeploy(c, "local:dummy", "other-service", "--config", path)
174+ c.Assert(err, ErrorMatches, `no settings found for "other-service"`)
175+ _, err = s.State.Service("other-service")
176+ c.Assert(errors.IsNotFoundError(err), Equals, true)
177+}
178+
179+func (s *DeploySuite) TestConstraints(c *C) {
180+ coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
181+ err := runDeploy(c, "local:dummy", "--constraints", "mem=2G cpu-cores=2")
182+ c.Assert(err, IsNil)
183+ curl := charm.MustParseURL("local:precise/dummy-1")
184+ service, _ := s.AssertService(c, "dummy", curl, 1, 0)
185+ cons, err := service.Constraints()
186+ c.Assert(err, IsNil)
187+ c.Assert(cons, DeepEquals, constraints.MustParse("mem=2G cpu-cores=2"))
188+}
189+
190+func (s *DeploySuite) TestSubordinateConstraints(c *C) {
191+ coretesting.Charms.BundlePath(s.SeriesPath, "logging")
192+ err := runDeploy(c, "local:logging", "--constraints", "mem=1G")
193+ c.Assert(err, ErrorMatches, "cannot specify constraints for subordinate service")
194+}
195+
196+func (s *DeploySuite) TestNumUnits(c *C) {
197+ coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
198+ err := runDeploy(c, "local:dummy", "-n", "13")
199+ c.Assert(err, IsNil)
200+ curl := charm.MustParseURL("local:precise/dummy-1")
201+ s.AssertService(c, "dummy", curl, 13, 0)
202+}
203+
204+func (s *DeploySuite) TestNumUnitsSubordinate(c *C) {
205+ coretesting.Charms.BundlePath(s.SeriesPath, "logging")
206+ err := runDeploy(c, "--num-units", "3", "local:logging")
207+ c.Assert(err, ErrorMatches, `cannot specify units for subordinate service`)
208+ _, err = s.State.Service("dummy")
209+ c.Assert(err, ErrorMatches, `service "dummy" not found`)
210+}
211+
212+func (s *DeploySuite) TestForceMachine(c *C) {
213+ coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
214+ machine, err := s.State.AddMachine("precise", state.JobHostUnits)
215+ c.Assert(err, IsNil)
216+ err = runDeploy(c, "--force-machine", machine.Id(), "local:dummy", "portlandia")
217+ c.Assert(err, IsNil)
218+ svc, err := s.State.Service("portlandia")
219+ c.Assert(err, IsNil)
220+ units, err := svc.AllUnits()
221+ c.Assert(err, IsNil)
222+ c.Assert(units, HasLen, 1)
223+ mid, err := units[0].AssignedMachineId()
224+ c.Assert(err, IsNil)
225+ c.Assert(mid, Equals, machine.Id())
226+}
227+
228+func (s *DeploySuite) TestForceMachineNotFound(c *C) {
229+ coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
230+ err := runDeploy(c, "--force-machine", "42", "local:dummy", "portlandia")
231+ c.Assert(err, ErrorMatches, `cannot assign unit "portlandia/0" to machine: machine 42 not found`)
232+ _, err = s.State.Service("dummy")
233+ c.Assert(err, ErrorMatches, `service "dummy" not found`)
234+}
235+
236+func (s *DeploySuite) TestForceMachineSubordinate(c *C) {
237+ machine, err := s.State.AddMachine("precise", state.JobHostUnits)
238+ c.Assert(err, IsNil)
239+ coretesting.Charms.BundlePath(s.SeriesPath, "logging")
240+ err = runDeploy(c, "--force-machine", machine.Id(), "local:logging")
241+ c.Assert(err, ErrorMatches, `cannot specify units for subordinate service`)
242+ _, err = s.State.Service("dummy")
243+ c.Assert(err, ErrorMatches, `service "dummy" not found`)
244+}
245
246=== modified file 'juju/conn.go'
247--- juju/conn.go 2013-06-11 02:14:27 +0000
248+++ juju/conn.go 2013-06-11 02:14:27 +0000
249@@ -186,51 +186,55 @@
250 // DeployServiceParams contains the arguments required to deploy the referenced
251 // charm.
252 type DeployServiceParams struct {
253- ServiceName string
254- Charm *state.Charm
255- NumUnits int
256- // Config is used only by the API.
257- Config map[string]string
258- // ConfigYAML takes precedence over Config if both are provided.
259- ConfigYAML string
260- Constraints constraints.Value
261+ ServiceName string
262+ Charm *state.Charm
263+ ConfigSettings charm.Settings
264+ Constraints constraints.Value
265+ NumUnits int
266 // Use string for deploy-to machine to avoid ambiguity around machine 0.
267 ForceMachineId string
268 }
269
270 // DeployService takes a charm and various parameters and deploys it.
271 func (conn *Conn) DeployService(args DeployServiceParams) (*state.Service, error) {
272- var err error
273- var settings charm.Settings
274- config := args.Charm.Config()
275- if args.ConfigYAML != "" {
276- settings, err = config.ParseSettingsYAML([]byte(args.ConfigYAML), args.ServiceName)
277- } else if args.Config != nil {
278- settings, err = config.ParseSettingsStrings(args.Config)
279- }
280+ settings, err := args.Charm.Config().ValidateSettings(args.ConfigSettings)
281 if err != nil {
282 return nil, err
283 }
284- svc, err := conn.State.AddService(args.ServiceName, args.Charm)
285+ emptyCons := constraints.Value{}
286+ if args.Charm.Meta().Subordinate {
287+ if args.NumUnits != 0 || args.ForceMachineId != "" {
288+ return nil, fmt.Errorf("subordinate service must be deployed without units")
289+ }
290+ if args.Constraints != emptyCons {
291+ return nil, fmt.Errorf("subordinate service must be deployed without constraints")
292+ }
293+ }
294+ // TODO(fwereade): transactional State.AddService including settings, constraints
295+ // (, minimumUnitCount, initialMachineIds?).
296+ service, err := conn.State.AddService(args.ServiceName, args.Charm)
297 if err != nil {
298 return nil, err
299 }
300 if len(settings) > 0 {
301- if err := svc.UpdateConfigSettings(settings); err != nil {
302+ if err := service.UpdateConfigSettings(settings); err != nil {
303 return nil, err
304 }
305 }
306 if args.Charm.Meta().Subordinate {
307- return svc, nil
308- }
309- if err := svc.SetConstraints(args.Constraints); err != nil {
310- return nil, err
311- }
312- _, err = conn.AddUnits(svc, args.NumUnits, args.ForceMachineId)
313- if err != nil {
314- return nil, err
315- }
316- return svc, nil
317+ return service, nil
318+ }
319+ if args.Constraints != emptyCons {
320+ if err := service.SetConstraints(args.Constraints); err != nil {
321+ return nil, err
322+ }
323+ }
324+ if args.NumUnits > 0 {
325+ if _, err := conn.AddUnits(service, args.NumUnits, args.ForceMachineId); err != nil {
326+ return nil, err
327+ }
328+ }
329+ return service, nil
330 }
331
332 func (conn *Conn) addCharm(curl *charm.URL, ch charm.Charm) (*state.Charm, error) {
333
334=== modified file 'juju/conn_test.go'
335--- juju/conn_test.go 2013-06-11 02:14:27 +0000
336+++ juju/conn_test.go 2013-06-11 02:14:27 +0000
337@@ -11,11 +11,13 @@
338 "launchpad.net/juju-core/environs"
339 "launchpad.net/juju-core/environs/config"
340 "launchpad.net/juju-core/environs/dummy"
341+ "launchpad.net/juju-core/errors"
342 "launchpad.net/juju-core/juju"
343 "launchpad.net/juju-core/juju/testing"
344 "launchpad.net/juju-core/state"
345 coretesting "launchpad.net/juju-core/testing"
346 "launchpad.net/juju-core/utils"
347+ "launchpad.net/juju-core/utils/set"
348 "os"
349 "path/filepath"
350 stdtesting "testing"
351@@ -402,63 +404,159 @@
352
353 }
354
355+// DeployLocalSuite uses a fresh copy of the same local dummy charm for each
356+// test, because ServiceDeploy demands that a charm already exists in state,
357+// and that's is the simplest way to get one in there.
358 type DeployLocalSuite struct {
359 testing.JujuConnSuite
360- repo *charm.LocalRepository
361- defaultSeries string
362- seriesPath string
363- charmUrl *charm.URL
364+ repo charm.Repository
365+ charm *state.Charm
366+ oldCacheDir string
367 }
368
369 var _ = Suite(&DeployLocalSuite{})
370
371+func (s *DeployLocalSuite) SetUpSuite(c *C) {
372+ s.JujuConnSuite.SetUpSuite(c)
373+ s.repo = &charm.LocalRepository{Path: coretesting.Charms.Path}
374+ s.oldCacheDir, charm.CacheDir = charm.CacheDir, c.MkDir()
375+}
376+
377+func (s *DeployLocalSuite) TearDownSuite(c *C) {
378+ charm.CacheDir = s.oldCacheDir
379+ s.JujuConnSuite.TearDownSuite(c)
380+}
381+
382 func (s *DeployLocalSuite) SetUpTest(c *C) {
383 s.JujuConnSuite.SetUpTest(c)
384- repoPath := c.MkDir()
385- s.defaultSeries = "precise"
386- s.repo = &charm.LocalRepository{Path: repoPath}
387- s.seriesPath = filepath.Join(repoPath, s.defaultSeries)
388- err := os.Mkdir(s.seriesPath, 0777)
389- c.Assert(err, IsNil)
390- coretesting.Charms.BundlePath(s.seriesPath, "wordpress")
391- s.charmUrl, err = charm.InferURL("local:wordpress", s.defaultSeries)
392- c.Assert(err, IsNil)
393-}
394-
395-func (s *DeployLocalSuite) TestDeploy(c *C) {
396- ch, err := s.Conn.PutCharm(s.charmUrl, s.repo, false)
397- c.Assert(err, IsNil)
398- cons := constraints.MustParse("mem=4G")
399- args := juju.DeployServiceParams{
400- ServiceName: "bob",
401- Charm: ch,
402- NumUnits: 3,
403- Constraints: cons,
404- ConfigYAML: "bob: {blog-title: aspidistra flagpole}",
405- }
406- svc, err := s.Conn.DeployService(args)
407- c.Assert(err, IsNil)
408- scons, err := svc.Constraints()
409- c.Assert(err, IsNil)
410- c.Assert(scons, DeepEquals, cons)
411- settings, err := svc.ConfigSettings()
412- c.Assert(err, IsNil)
413- c.Assert(settings, DeepEquals, charm.Settings{
414- "blog-title": "aspidistra flagpole",
415- })
416-
417- units, err := svc.AllUnits()
418- c.Assert(err, IsNil)
419- c.Assert(len(units), Equals, 3)
420+ curl := charm.MustParseURL("local:series/dummy")
421+ charm, err := s.Conn.PutCharm(curl, s.repo, false)
422+ c.Assert(err, IsNil)
423+ s.charm = charm
424+}
425+
426+func (s *DeployLocalSuite) TestDeployMinimal(c *C) {
427+ service, err := s.Conn.DeployService(juju.DeployServiceParams{
428+ ServiceName: "bob",
429+ Charm: s.charm,
430+ })
431+ c.Assert(err, IsNil)
432+ s.assertCharm(c, service, s.charm.URL())
433+ s.assertSettings(c, service, charm.Settings{})
434+ s.assertConstraints(c, service, constraints.Value{})
435+ s.assertMachines(c, service, constraints.Value{})
436+}
437+
438+func (s *DeployLocalSuite) TestDeploySettings(c *C) {
439+ service, err := s.Conn.DeployService(juju.DeployServiceParams{
440+ ServiceName: "bob",
441+ Charm: s.charm,
442+ ConfigSettings: charm.Settings{
443+ "title": "banana cupcakes",
444+ "skill-level": 9901,
445+ },
446+ })
447+ c.Assert(err, IsNil)
448+ s.assertSettings(c, service, charm.Settings{
449+ "title": "banana cupcakes",
450+ "skill-level": int64(9901),
451+ })
452+}
453+
454+func (s *DeployLocalSuite) TestDeploySettingsError(c *C) {
455+ _, err := s.Conn.DeployService(juju.DeployServiceParams{
456+ ServiceName: "bob",
457+ Charm: s.charm,
458+ ConfigSettings: charm.Settings{
459+ "skill-level": 99.01,
460+ },
461+ })
462+ c.Assert(err, ErrorMatches, `option "skill-level" expected int, got 99.01`)
463+ _, err = s.State.Service("bob")
464+ c.Assert(errors.IsNotFoundError(err), Equals, true)
465+}
466+
467+func (s *DeployLocalSuite) TestDeployConstraints(c *C) {
468+ err := s.State.SetEnvironConstraints(constraints.MustParse("mem=2G"))
469+ c.Assert(err, IsNil)
470+ serviceCons := constraints.MustParse("cpu-cores=2")
471+ service, err := s.Conn.DeployService(juju.DeployServiceParams{
472+ ServiceName: "bob",
473+ Charm: s.charm,
474+ Constraints: serviceCons,
475+ })
476+ c.Assert(err, IsNil)
477+ s.assertConstraints(c, service, serviceCons)
478+}
479+
480+func (s *DeployLocalSuite) TestDeployNumUnits(c *C) {
481+ err := s.State.SetEnvironConstraints(constraints.MustParse("mem=2G"))
482+ c.Assert(err, IsNil)
483+ serviceCons := constraints.MustParse("cpu-cores=2")
484+ service, err := s.Conn.DeployService(juju.DeployServiceParams{
485+ ServiceName: "bob",
486+ Charm: s.charm,
487+ Constraints: serviceCons,
488+ NumUnits: 2,
489+ })
490+ c.Assert(err, IsNil)
491+ s.assertConstraints(c, service, serviceCons)
492+ s.assertMachines(c, service, constraints.MustParse("mem=2G cpu-cores=2"), "0", "1")
493+}
494+
495+func (s *DeployLocalSuite) TestDeployForceMachineId(c *C) {
496+ machine, err := s.State.AddMachine("series", state.JobHostUnits)
497+ c.Assert(err, IsNil)
498+ c.Assert(machine.Id(), Equals, "0")
499+ err = s.State.SetEnvironConstraints(constraints.MustParse("mem=2G"))
500+ c.Assert(err, IsNil)
501+ serviceCons := constraints.MustParse("cpu-cores=2")
502+ service, err := s.Conn.DeployService(juju.DeployServiceParams{
503+ ServiceName: "bob",
504+ Charm: s.charm,
505+ Constraints: serviceCons,
506+ NumUnits: 1,
507+ ForceMachineId: "0",
508+ })
509+ c.Assert(err, IsNil)
510+ s.assertConstraints(c, service, serviceCons)
511+ s.assertMachines(c, service, constraints.Value{}, "0")
512+}
513+
514+func (s *DeployLocalSuite) assertCharm(c *C, service *state.Service, expect *charm.URL) {
515+ curl, force := service.CharmURL()
516+ c.Assert(curl, DeepEquals, expect)
517+ c.Assert(force, Equals, false)
518+}
519+
520+func (s *DeployLocalSuite) assertSettings(c *C, service *state.Service, expect charm.Settings) {
521+ settings, err := service.ConfigSettings()
522+ c.Assert(err, IsNil)
523+ c.Assert(settings, DeepEquals, expect)
524+}
525+
526+func (s *DeployLocalSuite) assertConstraints(c *C, service *state.Service, expect constraints.Value) {
527+ cons, err := service.Constraints()
528+ c.Assert(err, IsNil)
529+ c.Assert(cons, DeepEquals, expect)
530+}
531+
532+func (s *DeployLocalSuite) assertMachines(c *C, service *state.Service, expectCons constraints.Value, expectIds ...string) {
533+ units, err := service.AllUnits()
534+ c.Assert(err, IsNil)
535+ c.Assert(units, HasLen, len(expectIds))
536+ unseenIds := set.NewStrings(expectIds...)
537 for _, unit := range units {
538- mid, err := unit.AssignedMachineId()
539- c.Assert(err, IsNil)
540- machine, err := s.State.Machine(mid)
541- c.Assert(err, IsNil)
542- mcons, err := machine.Constraints()
543- c.Assert(err, IsNil)
544- c.Assert(mcons, DeepEquals, cons)
545+ id, err := unit.AssignedMachineId()
546+ c.Assert(err, IsNil)
547+ unseenIds.Remove(id)
548+ machine, err := s.State.Machine(id)
549+ c.Assert(err, IsNil)
550+ cons, err := machine.Constraints()
551+ c.Assert(err, IsNil)
552+ c.Assert(cons, DeepEquals, expectCons)
553 }
554+ c.Assert(unseenIds, DeepEquals, set.NewStrings())
555 }
556
557 type InitJujuHomeSuite struct {
558
559=== modified file 'state/api/params/params.go'
560--- state/api/params/params.go 2013-06-04 13:00:57 +0000
561+++ state/api/params/params.go 2013-06-11 02:14:27 +0000
562@@ -94,7 +94,6 @@
563 Config map[string]string
564 ConfigYAML string // Takes precedence over config if both are present.
565 Constraints constraints.Value
566- BumpRevision bool
567 ForceMachineId string
568 }
569
570
571=== modified file 'state/apiserver/client.go'
572--- state/apiserver/client.go 2013-06-11 02:14:27 +0000
573+++ state/apiserver/client.go 2013-06-11 02:14:27 +0000
574@@ -4,6 +4,7 @@
575 package apiserver
576
577 import (
578+ "fmt"
579 "launchpad.net/juju-core/charm"
580 "launchpad.net/juju-core/juju"
581 "launchpad.net/juju-core/state/api"
582@@ -102,30 +103,45 @@
583
584 var CharmStore charm.Repository = charm.Store
585
586-// ServiceDeploy fetches the charm from the charm store and deploys it. Local
587+// ServiceDeploy fetches the charm from the charm store and deploys it. Local
588 // charms are not supported.
589 func (c *srvClient) ServiceDeploy(args params.ServiceDeploy) error {
590- state := c.root.srv.state
591- conf, err := state.EnvironConfig()
592- if err != nil {
593- return err
594- }
595- curl, err := charm.InferURL(args.CharmUrl, conf.DefaultSeries())
596- if err != nil {
597- return err
598- }
599- conn, err := juju.NewConnFromState(state)
600- if err != nil {
601- return err
602- }
603- if args.NumUnits == 0 {
604- args.NumUnits = 1
605- }
606- serviceName := args.ServiceName
607- if serviceName == "" {
608- serviceName = curl.Name
609- }
610- return statecmd.ServiceDeploy(state, args, conn, curl, CharmStore)
611+ curl, err := charm.ParseURL(args.CharmUrl)
612+ if err != nil {
613+ return err
614+ }
615+ if curl.Schema != "cs" {
616+ return fmt.Errorf(`charm url has unsupported schema %q`, curl.Schema)
617+ }
618+ if curl.Revision < 0 {
619+ return fmt.Errorf("charm url must include revision")
620+ }
621+ conn, err := juju.NewConnFromState(c.root.srv.state)
622+ if err != nil {
623+ return err
624+ }
625+ ch, err := conn.PutCharm(curl, CharmStore, false)
626+ if err != nil {
627+ return err
628+ }
629+ var settings charm.Settings
630+ if len(args.ConfigYAML) > 0 {
631+ settings, err = ch.Config().ParseSettingsYAML([]byte(args.ConfigYAML), args.ServiceName)
632+ } else if len(args.Config) > 0 {
633+ settings, err = ch.Config().ParseSettingsStrings(args.Config)
634+ }
635+ if err != nil {
636+ return err
637+ }
638+ _, err = conn.DeployService(juju.DeployServiceParams{
639+ ServiceName: args.ServiceName,
640+ Charm: ch,
641+ NumUnits: args.NumUnits,
642+ ConfigSettings: settings,
643+ Constraints: args.Constraints,
644+ ForceMachineId: args.ForceMachineId,
645+ })
646+ return err
647 }
648
649 // AddServiceUnits adds a given number of units to a service.
650
651=== modified file 'state/apiserver/client_test.go'
652--- state/apiserver/client_test.go 2013-06-11 02:14:27 +0000
653+++ state/apiserver/client_test.go 2013-06-11 02:14:27 +0000
654@@ -4,6 +4,7 @@
655 package apiserver_test
656
657 import (
658+ "fmt"
659 . "launchpad.net/gocheck"
660 "launchpad.net/juju-core/charm"
661 "launchpad.net/juju-core/constraints"
662@@ -300,74 +301,130 @@
663 c.Assert(mode, Equals, state.ResolvedNoHooks)
664 }
665
666-var serviceDeployTests = []struct {
667- about string
668- charmUrl string
669- numUnits int
670- expectedNumUnits int
671- constraints constraints.Value
672-}{{
673- about: "Normal deploy",
674- charmUrl: "local:series/wordpress",
675- expectedNumUnits: 1,
676- constraints: constraints.MustParse("mem=1G"),
677-}, {
678- about: "Two units",
679- charmUrl: "local:series/wordpress",
680- numUnits: 2,
681- expectedNumUnits: 2,
682- constraints: constraints.MustParse("mem=4G"),
683-},
684-}
685-
686-func (s *clientSuite) TestClientServiceDeploy(c *C) {
687- s.setUpScenario(c)
688-
689- for i, test := range serviceDeployTests {
690- c.Logf("test %d; %s", i, test.about)
691- parsedUrl := charm.MustParseURL(test.charmUrl)
692- localRepo, err := charm.InferRepository(parsedUrl, coretesting.Charms.Path)
693- c.Assert(err, IsNil)
694- withRepo(localRepo, func() {
695- serviceName := "mywordpress"
696- _, err = s.State.Service(serviceName)
697+func (s *clientSuite) TestClientServiceDeployCharmErrors(c *C) {
698+ withMockCharmStore(func(store *coretesting.MockCharmStore) {
699+ for url, expect := range map[string]string{
700+ // TODO(fwereade) make these errors consistent one day.
701+ "wordpress": `charm URL has invalid schema: "wordpress"`,
702+ "cs:wordpress": `charm URL without series: "cs:wordpress"`,
703+ "cs:precise/wordpress": "charm url must include revision",
704+ "cs:precise/wordpress-999999": `cannot get charm: charm not found in mock store: cs:precise/wordpress-999999`,
705+ "local:precise/wordpress-999999": `charm url has unsupported schema "local"`,
706+ } {
707+ c.Logf("test %s", url)
708+ err := s.APIState.Client().ServiceDeploy(
709+ url, "service", 1, "", constraints.Value{},
710+ )
711+ c.Check(err, ErrorMatches, expect)
712+ _, err = s.State.Service("service")
713 c.Assert(errors.IsNotFoundError(err), Equals, true)
714- err = s.APIState.Client().ServiceDeploy(
715- test.charmUrl, serviceName, test.numUnits, "mywordpress: {}", test.constraints,
716- )
717- c.Assert(err, IsNil)
718-
719- service, err := s.State.Service(serviceName)
720- c.Assert(err, IsNil)
721- defer removeServiceAndUnits(c, service)
722- scons, err := service.Constraints()
723- c.Assert(err, IsNil)
724- c.Assert(scons, DeepEquals, test.constraints)
725-
726- units, err := service.AllUnits()
727- c.Assert(err, IsNil)
728- c.Assert(units, HasLen, test.expectedNumUnits)
729- for _, unit := range units {
730- mid, err := unit.AssignedMachineId()
731- c.Assert(err, IsNil)
732- machine, err := s.State.Machine(mid)
733- c.Assert(err, IsNil)
734- mcons, err := machine.Constraints()
735- c.Assert(err, IsNil)
736- c.Assert(mcons, DeepEquals, test.constraints)
737- }
738- })
739- }
740-}
741-
742-func withRepo(repo charm.Repository, f func()) {
743- // Monkey-patch server repository.
744- originalServerCharmStore := apiserver.CharmStore
745- apiserver.CharmStore = repo
746- defer func() {
747- apiserver.CharmStore = originalServerCharmStore
748- }()
749- f()
750+ }
751+ })
752+}
753+
754+func (s *clientSuite) TestClientServiceDeployPrincipal(c *C) {
755+ // TODO(fwereade): test ForceMachineId directly on srvClient, when we
756+ // manage to extract it as a package and can thus do it conveniently.
757+ withMockCharmStore(func(store *coretesting.MockCharmStore) {
758+ curl, bundle := addCharm(c, store, "dummy")
759+ mem4g := constraints.MustParse("mem=4G")
760+ err := s.APIState.Client().ServiceDeploy(
761+ curl.String(), "service", 3, "", mem4g,
762+ )
763+ c.Assert(err, IsNil)
764+ service, err := s.State.Service("service")
765+ c.Assert(err, IsNil)
766+ charm, force, err := service.Charm()
767+ c.Assert(err, IsNil)
768+ c.Assert(force, Equals, false)
769+ c.Assert(charm.URL(), DeepEquals, curl)
770+ c.Assert(charm.Meta(), DeepEquals, bundle.Meta())
771+ c.Assert(charm.Config(), DeepEquals, bundle.Config())
772+
773+ cons, err := service.Constraints()
774+ c.Assert(err, IsNil)
775+ c.Assert(cons, DeepEquals, mem4g)
776+ units, err := service.AllUnits()
777+ c.Assert(err, IsNil)
778+ for _, unit := range units {
779+ mid, err := unit.AssignedMachineId()
780+ c.Assert(err, IsNil)
781+ machine, err := s.State.Machine(mid)
782+ c.Assert(err, IsNil)
783+ cons, err := machine.Constraints()
784+ c.Assert(err, IsNil)
785+ c.Assert(cons, DeepEquals, mem4g)
786+ }
787+ })
788+}
789+
790+func (s *clientSuite) TestClientServiceDeploySubordinate(c *C) {
791+ withMockCharmStore(func(store *coretesting.MockCharmStore) {
792+ curl, bundle := addCharm(c, store, "logging")
793+ err := s.APIState.Client().ServiceDeploy(
794+ curl.String(), "service-name", 0, "", constraints.Value{},
795+ )
796+ service, err := s.State.Service("service-name")
797+ c.Assert(err, IsNil)
798+ charm, force, err := service.Charm()
799+ c.Assert(err, IsNil)
800+ c.Assert(force, Equals, false)
801+ c.Assert(charm.URL(), DeepEquals, curl)
802+ c.Assert(charm.Meta(), DeepEquals, bundle.Meta())
803+ c.Assert(charm.Config(), DeepEquals, bundle.Config())
804+
805+ units, err := service.AllUnits()
806+ c.Assert(err, IsNil)
807+ c.Assert(units, HasLen, 0)
808+ })
809+}
810+
811+func (s *clientSuite) TestClientServiceDeployConfig(c *C) {
812+ // TODO(fwereade): test Config/ConfigYAML handling directly on srvClient.
813+ // Can't be done cleanly until it's extracted similarly to Machiner.
814+ withMockCharmStore(func(store *coretesting.MockCharmStore) {
815+ curl, _ := addCharm(c, store, "dummy")
816+ err := s.APIState.Client().ServiceDeploy(
817+ curl.String(), "service-name", 1, "service-name:\n username: fred", constraints.Value{},
818+ )
819+ c.Assert(err, IsNil)
820+ service, err := s.State.Service("service-name")
821+ c.Assert(err, IsNil)
822+ settings, err := service.ConfigSettings()
823+ c.Assert(err, IsNil)
824+ c.Assert(settings, DeepEquals, charm.Settings{"username": "fred"})
825+ })
826+}
827+
828+func (s *clientSuite) TestClientServiceDeployConfigError(c *C) {
829+ // TODO(fwereade): test Config/ConfigYAML handling directly on srvClient.
830+ // Can't be done cleanly until it's extracted similarly to Machiner.
831+ withMockCharmStore(func(store *coretesting.MockCharmStore) {
832+ curl, _ := addCharm(c, store, "dummy")
833+ err := s.APIState.Client().ServiceDeploy(
834+ curl.String(), "service-name", 1, "service-name:\n skill-level: fred", constraints.Value{},
835+ )
836+ c.Assert(err, ErrorMatches, `option "skill-level" expected int, got "fred"`)
837+ _, err = s.State.Service("service-name")
838+ c.Assert(errors.IsNotFoundError(err), Equals, true)
839+ })
840+}
841+
842+func withMockCharmStore(f func(*coretesting.MockCharmStore)) {
843+ mockStore := coretesting.NewMockCharmStore()
844+ origStore := apiserver.CharmStore
845+ apiserver.CharmStore = mockStore
846+ defer func() { apiserver.CharmStore = origStore }()
847+ f(mockStore)
848+}
849+
850+func addCharm(c *C, store *coretesting.MockCharmStore, name string) (*charm.URL, charm.Charm) {
851+ bundle := coretesting.Charms.Bundle(c.MkDir(), name)
852+ scurl := fmt.Sprintf("cs:precise/%s-%d", name, bundle.Revision())
853+ curl := charm.MustParseURL(scurl)
854+ err := store.SetCharm(curl, bundle)
855+ c.Assert(err, IsNil)
856+ return curl, bundle
857 }
858
859 func (s *clientSuite) TestSuccessfulAddRelation(c *C) {
860
861=== modified file 'state/apiserver/perm_test.go'
862--- state/apiserver/perm_test.go 2013-06-11 02:14:27 +0000
863+++ state/apiserver/perm_test.go 2013-06-11 02:14:27 +0000
864@@ -5,11 +5,9 @@
865
866 import (
867 . "launchpad.net/gocheck"
868- "launchpad.net/juju-core/charm"
869 "launchpad.net/juju-core/constraints"
870 "launchpad.net/juju-core/state"
871 "launchpad.net/juju-core/state/api"
872- "launchpad.net/juju-core/state/apiserver"
873 coretesting "launchpad.net/juju-core/testing"
874 "strings"
875 )
876@@ -288,23 +286,16 @@
877 }
878
879 func opClientServiceDeploy(c *C, st *api.State, mst *state.State) (func(), error) {
880- // We are cheating and using a local repo only.
881-
882- // Set the CharmStore to the test repository.
883- serviceName := "mywordpress"
884- charmUrl := "local:series/wordpress"
885- parsedUrl := charm.MustParseURL(charmUrl)
886- repo, err := charm.InferRepository(parsedUrl, coretesting.Charms.Path)
887- originalServerCharmStore := apiserver.CharmStore
888- apiserver.CharmStore = repo
889-
890- err = st.Client().ServiceDeploy(charmUrl, serviceName, 1, "mywordpress: {}", constraints.Value{})
891+ var err error
892+ withMockCharmStore(func(store *coretesting.MockCharmStore) {
893+ curl, _ := addCharm(c, store, "dummy")
894+ err = st.Client().ServiceDeploy(curl.String(), "deploy-service", 1, "", constraints.Value{})
895+ })
896 if err != nil {
897 return func() {}, err
898 }
899 return func() {
900- apiserver.CharmStore = originalServerCharmStore
901- service, err := mst.Service(serviceName)
902+ service, err := mst.Service("deploy-service")
903 c.Assert(err, IsNil)
904 removeServiceAndUnits(c, service)
905 }, nil
906
907=== removed file 'state/statecmd/deploy.go'
908--- state/statecmd/deploy.go 2013-05-31 00:13:28 +0000
909+++ state/statecmd/deploy.go 1970-01-01 00:00:00 +0000
910@@ -1,63 +0,0 @@
911-package statecmd
912-
913-import (
914- "errors"
915- "fmt"
916- "launchpad.net/juju-core/charm"
917- "launchpad.net/juju-core/constraints"
918- "launchpad.net/juju-core/juju"
919- "launchpad.net/juju-core/state"
920- "launchpad.net/juju-core/state/api/params"
921-)
922-
923-// ServiceDeploy deploys a service to the environment from the given repository.
924-// The connection, charm URL, and repository will be provided by the caller,
925-// since these values already exist in the calling locations and can be
926-// defaulted in different scenarios (i.e.: only the charmstore will be used
927-// when called from the websocket API).
928-func ServiceDeploy(st *state.State, args params.ServiceDeploy, conn *juju.Conn, curl *charm.URL, repo charm.Repository) error {
929- if args.ServiceName != "" && !state.IsServiceName(args.ServiceName) {
930- return fmt.Errorf("invalid service name %q", args.ServiceName)
931- }
932- if args.ForceMachineId != "" {
933- if !state.IsMachineId(args.ForceMachineId) {
934- return fmt.Errorf("invalid machine id %q", args.ForceMachineId)
935- }
936- if args.NumUnits > 1 {
937- return fmt.Errorf("force-machine cannot be used for multiple units")
938- }
939- }
940-
941- charm, err := conn.PutCharm(curl, repo, args.BumpRevision)
942- if err != nil {
943- return err
944- }
945- if charm.Meta().Subordinate {
946- empty := constraints.Value{}
947- if args.Constraints != empty {
948- return state.ErrSubordinateConstraints
949- }
950- if args.ForceMachineId != "" {
951- return fmt.Errorf("subordinate service cannot specify force-machine")
952- }
953- } else {
954- if args.NumUnits < 1 {
955- return errors.New("must deploy at least one unit")
956- }
957- }
958- serviceName := args.ServiceName
959- if serviceName == "" {
960- serviceName = curl.Name
961- }
962- deployArgs := juju.DeployServiceParams{
963- Charm: charm,
964- ServiceName: serviceName,
965- NumUnits: args.NumUnits,
966- Config: args.Config,
967- ConfigYAML: args.ConfigYAML,
968- Constraints: args.Constraints,
969- ForceMachineId: args.ForceMachineId,
970- }
971- _, err = conn.DeployService(deployArgs)
972- return err
973-}
974
975=== removed file 'state/statecmd/deploy_test.go'
976--- state/statecmd/deploy_test.go 2013-06-11 02:14:27 +0000
977+++ state/statecmd/deploy_test.go 1970-01-01 00:00:00 +0000
978@@ -1,273 +0,0 @@
979-package statecmd_test
980-
981-import (
982- . "launchpad.net/gocheck"
983- "launchpad.net/juju-core/charm"
984- "launchpad.net/juju-core/constraints"
985- "launchpad.net/juju-core/juju/testing"
986- "launchpad.net/juju-core/state"
987- "launchpad.net/juju-core/state/api/params"
988- "launchpad.net/juju-core/state/statecmd"
989- coretesting "launchpad.net/juju-core/testing"
990- "os"
991-)
992-
993-type DeploySuite struct {
994- testing.RepoSuite
995-}
996-
997-var _ = Suite(&DeploySuite{})
998-
999-func (s *DeploySuite) runDeploy(c *C, args params.ServiceDeploy) error {
1000- c.Logf("Running deploy: %+v", args)
1001- conf, err := s.State.EnvironConfig()
1002- c.Assert(err, IsNil)
1003- curl, err := charm.InferURL(args.CharmUrl, conf.DefaultSeries())
1004- c.Assert(err, IsNil)
1005- repo, err := charm.InferRepository(curl, os.Getenv("JUJU_REPOSITORY"))
1006- c.Assert(err, IsNil)
1007- return statecmd.ServiceDeploy(s.State, args, s.Conn, curl, repo)
1008-}
1009-
1010-func (s *DeploySuite) TestCharmDir(c *C) {
1011- coretesting.Charms.ClonedDirPath(s.SeriesPath, "dummy")
1012- args := params.ServiceDeploy{
1013- CharmUrl: "local:dummy",
1014- NumUnits: 1,
1015- }
1016- err := s.runDeploy(c, args)
1017- c.Assert(err, IsNil)
1018- curl := charm.MustParseURL("local:precise/dummy-1")
1019- // Note that this tests the automatic creation of a service name (dummy)
1020- // from the charm URL. This functionality will be going away soon as
1021- // ServiceName becomes a required argument.
1022- s.AssertService(c, "dummy", curl, 1, 0)
1023-}
1024-
1025-func (s *DeploySuite) TestUpgradeCharmDir(c *C) {
1026- dirPath := coretesting.Charms.ClonedDirPath(s.SeriesPath, "dummy")
1027- args := params.ServiceDeploy{
1028- CharmUrl: "local:dummy",
1029- BumpRevision: true,
1030- NumUnits: 1,
1031- }
1032- err := s.runDeploy(c, args)
1033- c.Assert(err, IsNil)
1034- curl := charm.MustParseURL("local:precise/dummy-2")
1035- s.AssertService(c, "dummy", curl, 1, 0)
1036- // Check the charm really was upgraded.
1037- ch, err := charm.ReadDir(dirPath)
1038- c.Assert(err, IsNil)
1039- c.Assert(ch.Revision(), Equals, 2)
1040-}
1041-
1042-func (s *DeploySuite) TestCharmBundle(c *C) {
1043- coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
1044- args := params.ServiceDeploy{
1045- CharmUrl: "local:dummy",
1046- ServiceName: "some-service-name",
1047- NumUnits: 1,
1048- }
1049- err := s.runDeploy(c, args)
1050- c.Assert(err, IsNil)
1051- curl := charm.MustParseURL("local:precise/dummy-1")
1052- s.AssertService(c, "some-service-name", curl, 1, 0)
1053-}
1054-
1055-func (s *DeploySuite) TestForceMachine(c *C) {
1056- coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
1057- machine, err := s.State.AddMachine("precise", state.JobHostUnits)
1058- c.Assert(err, IsNil)
1059- args := params.ServiceDeploy{
1060- CharmUrl: "local:dummy",
1061- ServiceName: "portlandia",
1062- ForceMachineId: machine.Id(),
1063- NumUnits: 1,
1064- }
1065- err = s.runDeploy(c, args)
1066- c.Assert(err, IsNil)
1067- svc, err := s.State.Service("portlandia")
1068- c.Assert(err, IsNil)
1069- units, err := svc.AllUnits()
1070- c.Assert(err, IsNil)
1071- c.Assert(units, HasLen, 1)
1072- mid, err := units[0].AssignedMachineId()
1073- c.Assert(err, IsNil)
1074- c.Assert(mid, Equals, machine.Id())
1075-}
1076-
1077-func (s *DeploySuite) TestForceMachineInvalid(c *C) {
1078- coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
1079- args := params.ServiceDeploy{
1080- CharmUrl: "local:dummy",
1081- ServiceName: "portlandia",
1082- ForceMachineId: "42",
1083- NumUnits: 1,
1084- }
1085- err := s.runDeploy(c, args)
1086- c.Assert(err, ErrorMatches, `cannot assign unit "portlandia/0" to machine: machine 42 not found`)
1087-
1088- args = params.ServiceDeploy{
1089- CharmUrl: "local:dummy",
1090- ServiceName: "portlandia",
1091- ForceMachineId: "abc",
1092- NumUnits: 1,
1093- }
1094-
1095- err = s.runDeploy(c, args)
1096- c.Assert(err, ErrorMatches, `invalid machine id "abc"`)
1097-
1098- machine, err := s.State.AddMachine("precise", state.JobHostUnits)
1099- args = params.ServiceDeploy{
1100- CharmUrl: "local:dummy",
1101- ServiceName: "portlandia",
1102- ForceMachineId: machine.Id(),
1103- NumUnits: 5,
1104- }
1105- err = s.runDeploy(c, args)
1106- c.Assert(err, ErrorMatches, `force-machine cannot be used for multiple units`)
1107-
1108- coretesting.Charms.BundlePath(s.SeriesPath, "logging")
1109- args = params.ServiceDeploy{
1110- CharmUrl: "local:logging",
1111- ForceMachineId: machine.Id(),
1112- }
1113- err = s.runDeploy(c, args)
1114- c.Assert(err, ErrorMatches, `subordinate service cannot specify force-machine`)
1115-}
1116-
1117-func (s *DeploySuite) TestCannotUpgradeCharmBundle(c *C) {
1118- coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
1119- args := params.ServiceDeploy{
1120- CharmUrl: "local:dummy",
1121- BumpRevision: true,
1122- }
1123- err := s.runDeploy(c, args)
1124- c.Assert(err, ErrorMatches, `cannot increment revision of charm "local:precise/dummy-1": not a directory`)
1125- // Verify state not touched...
1126- curl := charm.MustParseURL("local:precise/dummy-1")
1127- _, err = s.State.Charm(curl)
1128- c.Assert(err, ErrorMatches, `charm "local:precise/dummy-1" not found`)
1129- _, err = s.State.Service("dummy")
1130- c.Assert(err, ErrorMatches, `service "dummy" not found`)
1131-}
1132-
1133-func (s *DeploySuite) TestAddsPeerRelations(c *C) {
1134- coretesting.Charms.BundlePath(s.SeriesPath, "riak")
1135- args := params.ServiceDeploy{
1136- CharmUrl: "local:riak",
1137- NumUnits: 1,
1138- }
1139- err := s.runDeploy(c, args)
1140- c.Assert(err, IsNil)
1141- curl := charm.MustParseURL("local:precise/riak-7")
1142- _, rels := s.AssertService(c, "riak", curl, 1, 1)
1143- rel := rels[0]
1144- ep, err := rel.Endpoint("riak")
1145- c.Assert(err, IsNil)
1146- c.Assert(ep.Name, Equals, "ring")
1147- c.Assert(ep.Role, Equals, charm.RolePeer)
1148- c.Assert(ep.Scope, Equals, charm.ScopeGlobal)
1149-}
1150-
1151-func (s *DeploySuite) TestNumUnits(c *C) {
1152- coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
1153- args := params.ServiceDeploy{
1154- CharmUrl: "local:dummy",
1155- NumUnits: 13,
1156- }
1157- err := s.runDeploy(c, args)
1158- c.Assert(err, IsNil)
1159- curl := charm.MustParseURL("local:precise/dummy-1")
1160- s.AssertService(c, "dummy", curl, 13, 0)
1161-}
1162-
1163-func (s *DeploySuite) TestNumUnitsZero(c *C) {
1164- coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
1165- args := params.ServiceDeploy{
1166- CharmUrl: "local:dummy",
1167- NumUnits: 0,
1168- }
1169- err := s.runDeploy(c, args)
1170- c.Assert(err, ErrorMatches, "must deploy at least one unit")
1171-}
1172-
1173-func (s *DeploySuite) TestSubordinateCharm(c *C) {
1174- coretesting.Charms.BundlePath(s.SeriesPath, "logging")
1175- args := params.ServiceDeploy{
1176- CharmUrl: "local:logging",
1177- }
1178- err := s.runDeploy(c, args)
1179- c.Assert(err, IsNil)
1180- curl := charm.MustParseURL("local:precise/logging-1")
1181- s.AssertService(c, "logging", curl, 0, 0)
1182-}
1183-
1184-func (s *DeploySuite) TestConfigMap(c *C) {
1185- coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
1186- args := params.ServiceDeploy{
1187- CharmUrl: "local:dummy",
1188- Config: map[string]string{
1189- "skill-level": "1",
1190- "username": "",
1191- },
1192- NumUnits: 1,
1193- }
1194- err := s.runDeploy(c, args)
1195- c.Assert(err, IsNil)
1196- curl := charm.MustParseURL("local:precise/dummy-1")
1197- s.AssertService(c, "dummy", curl, 1, 0)
1198- svc, err := s.State.Service("dummy")
1199- c.Assert(err, IsNil)
1200- settings, err := svc.ConfigSettings()
1201- c.Assert(err, IsNil)
1202- c.Assert(settings, DeepEquals, charm.Settings{
1203- "skill-level": int64(1),
1204- })
1205-}
1206-
1207-func (s *DeploySuite) TestConfigYAML(c *C) {
1208- coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
1209- args := params.ServiceDeploy{
1210- CharmUrl: "local:dummy",
1211- ConfigYAML: "dummy: {skill-level: 9001, username: null}",
1212- NumUnits: 1,
1213- }
1214- err := s.runDeploy(c, args)
1215- c.Assert(err, IsNil)
1216- curl := charm.MustParseURL("local:precise/dummy-1")
1217- s.AssertService(c, "dummy", curl, 1, 0)
1218- svc, err := s.State.Service("dummy")
1219- c.Assert(err, IsNil)
1220- settings, err := svc.ConfigSettings()
1221- c.Assert(err, IsNil)
1222- c.Assert(settings, DeepEquals, charm.Settings{
1223- "skill-level": int64(9001),
1224- })
1225-}
1226-
1227-func (s *DeploySuite) TestConstraints(c *C) {
1228- coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
1229- args := params.ServiceDeploy{
1230- CharmUrl: "local:dummy",
1231- Constraints: constraints.MustParse("mem=2G cpu-cores=2"),
1232- NumUnits: 1,
1233- }
1234- err := s.runDeploy(c, args)
1235- c.Assert(err, IsNil)
1236- curl := charm.MustParseURL("local:precise/dummy-1")
1237- service, _ := s.AssertService(c, "dummy", curl, 1, 0)
1238- cons, err := service.Constraints()
1239- c.Assert(err, IsNil)
1240- c.Assert(cons, DeepEquals, constraints.MustParse("mem=2G cpu-cores=2"))
1241-}
1242-
1243-func (s *DeploySuite) TestSubordinateConstraints(c *C) {
1244- coretesting.Charms.BundlePath(s.SeriesPath, "logging")
1245- args := params.ServiceDeploy{
1246- CharmUrl: "local:logging",
1247- Constraints: constraints.MustParse("mem=1G"),
1248- }
1249- err := s.runDeploy(c, args)
1250- c.Assert(err, Equals, state.ErrSubordinateConstraints)
1251-}
1252
1253=== modified file 'testing/charm.go'
1254--- testing/charm.go 2013-05-02 15:55:42 +0000
1255+++ testing/charm.go 2013-06-11 02:14:27 +0000
1256@@ -111,3 +111,62 @@
1257 check(err)
1258 return ch
1259 }
1260+
1261+type MockCharmStore struct {
1262+ charms map[string]map[int]*charm.Bundle
1263+}
1264+
1265+func NewMockCharmStore() *MockCharmStore {
1266+ return &MockCharmStore{map[string]map[int]*charm.Bundle{}}
1267+}
1268+
1269+func (s *MockCharmStore) SetCharm(curl *charm.URL, bundle *charm.Bundle) error {
1270+ base := curl.WithRevision(-1).String()
1271+ if curl.Revision < 0 {
1272+ return fmt.Errorf("bad charm url revision")
1273+ }
1274+ if bundle == nil {
1275+ delete(s.charms[base], curl.Revision)
1276+ return nil
1277+ }
1278+ bundleRev := bundle.Revision()
1279+ bundleName := bundle.Meta().Name
1280+ if bundleName != curl.Name || bundleRev != curl.Revision {
1281+ return fmt.Errorf("charm url %s mismatch with bundle %s-%d", curl, bundleName, bundleRev)
1282+ }
1283+ if _, found := s.charms[base]; !found {
1284+ s.charms[base] = map[int]*charm.Bundle{}
1285+ }
1286+ s.charms[base][curl.Revision] = bundle
1287+ return nil
1288+}
1289+
1290+func (s *MockCharmStore) interpret(curl *charm.URL) (base string, rev int) {
1291+ base, rev = curl.WithRevision(-1).String(), curl.Revision
1292+ if rev == -1 {
1293+ for candidate := range s.charms[base] {
1294+ if candidate > rev {
1295+ rev = candidate
1296+ }
1297+ }
1298+ }
1299+ return
1300+}
1301+
1302+func (s *MockCharmStore) Get(curl *charm.URL) (charm.Charm, error) {
1303+ base, rev := s.interpret(curl)
1304+ charm, found := s.charms[base][rev]
1305+ if !found {
1306+ return nil, fmt.Errorf("charm not found in mock store: %s", curl)
1307+ }
1308+ return charm, nil
1309+}
1310+
1311+func (s *MockCharmStore) Latest(curl *charm.URL) (int, error) {
1312+ curl = curl.WithRevision(-1)
1313+ base, rev := s.interpret(curl)
1314+ if _, found := s.charms[base][rev]; !found {
1315+ return 0, fmt.Errorf("charm not found in mock store: %s", curl)
1316+ }
1317+ return rev, nil
1318+}

Subscribers

People subscribed via source and target branches