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
=== modified file 'cmd/juju/deploy.go'
--- cmd/juju/deploy.go 2013-06-11 02:14:27 +0000
+++ cmd/juju/deploy.go 2013-06-11 02:14:27 +0000
@@ -6,15 +6,12 @@
6import (6import (
7 "errors"7 "errors"
8 "fmt"8 "fmt"
9 "io/ioutil"
10 "launchpad.net/gnuflag"9 "launchpad.net/gnuflag"
11 "launchpad.net/juju-core/charm"10 "launchpad.net/juju-core/charm"
12 "launchpad.net/juju-core/cmd"11 "launchpad.net/juju-core/cmd"
13 "launchpad.net/juju-core/constraints"12 "launchpad.net/juju-core/constraints"
14 "launchpad.net/juju-core/juju"13 "launchpad.net/juju-core/juju"
15 "launchpad.net/juju-core/state"14 "launchpad.net/juju-core/state"
16 "launchpad.net/juju-core/state/api/params"
17 "launchpad.net/juju-core/state/statecmd"
18 "os"15 "os"
19)16)
2017
@@ -123,22 +120,49 @@
123 if err != nil {120 if err != nil {
124 return err121 return err
125 }122 }
126 var configYAML []byte123 // TODO(fwereade) it's annoying to roundtrip the bytes through the client
124 // here, but it's the original behaviour and not convenient to change.
125 // PutCharm will always be required in some form for local charms; and we
126 // will need an EnsureStoreCharm method somewhere that gets the state.Charm
127 // for use in the following checks.
128 ch, err := conn.PutCharm(curl, repo, c.BumpRevision)
129 if err != nil {
130 return err
131 }
132 numUnits := c.NumUnits
133 if ch.Meta().Subordinate {
134 empty := constraints.Value{}
135 if c.Constraints != empty {
136 return errors.New("cannot specify constraints for subordinate service")
137 }
138 if numUnits == 1 && c.ForceMachineId == "" {
139 numUnits = 0
140 } else {
141 return errors.New("cannot specify units for subordinate service")
142 }
143 }
144 serviceName := c.ServiceName
145 if serviceName == "" {
146 serviceName = ch.Meta().Name
147 }
148 var settings charm.Settings
127 if c.Config.Path != "" {149 if c.Config.Path != "" {
128 configYAML, err = ioutil.ReadFile(c.Config.Path)150 configYAML, err := c.Config.Read(ctx)
151 if err != nil {
152 return err
153 }
154 settings, err = ch.Config().ParseSettingsYAML(configYAML, serviceName)
129 if err != nil {155 if err != nil {
130 return err156 return err
131 }157 }
132 }158 }
133 args := params.ServiceDeploy{159 _, err = conn.DeployService(juju.DeployServiceParams{
134 ServiceName: c.ServiceName,160 ServiceName: serviceName,
135 CharmUrl: c.CharmName,161 Charm: ch,
136 NumUnits: c.NumUnits,162 NumUnits: numUnits,
137 // BUG(lp:1162122): --config has no tests.163 ConfigSettings: settings,
138 ConfigYAML: string(configYAML),
139 Constraints: c.Constraints,164 Constraints: c.Constraints,
140 BumpRevision: c.BumpRevision,
141 ForceMachineId: c.ForceMachineId,165 ForceMachineId: c.ForceMachineId,
142 }166 })
143 return statecmd.ServiceDeploy(conn.State, args, conn, curl, repo)167 return err
144}168}
145169
=== modified file 'cmd/juju/deploy_test.go'
--- cmd/juju/deploy_test.go 2013-06-11 02:14:27 +0000
+++ cmd/juju/deploy_test.go 2013-06-11 02:14:27 +0000
@@ -5,7 +5,11 @@
55
6import (6import (
7 . "launchpad.net/gocheck"7 . "launchpad.net/gocheck"
8 "launchpad.net/juju-core/charm"
9 "launchpad.net/juju-core/constraints"
10 "launchpad.net/juju-core/errors"
8 "launchpad.net/juju-core/juju/testing"11 "launchpad.net/juju-core/juju/testing"
12 "launchpad.net/juju-core/state"
9 coretesting "launchpad.net/juju-core/testing"13 coretesting "launchpad.net/juju-core/testing"
10)14)
1115
@@ -55,3 +59,147 @@
55 c.Assert(err, ErrorMatches, t.err)59 c.Assert(err, ErrorMatches, t.err)
56 }60 }
57}61}
62
63func (s *DeploySuite) TestNoCharm(c *C) {
64 err := runDeploy(c, "local:unknown-123")
65 c.Assert(err, ErrorMatches, `cannot get charm: charm not found in ".*": local:precise/unknown-123`)
66}
67
68func (s *DeploySuite) TestCharmDir(c *C) {
69 coretesting.Charms.ClonedDirPath(s.SeriesPath, "dummy")
70 err := runDeploy(c, "local:dummy")
71 c.Assert(err, IsNil)
72 curl := charm.MustParseURL("local:precise/dummy-1")
73 s.AssertService(c, "dummy", curl, 1, 0)
74}
75
76func (s *DeploySuite) TestUpgradeCharmDir(c *C) {
77 dirPath := coretesting.Charms.ClonedDirPath(s.SeriesPath, "dummy")
78 err := runDeploy(c, "local:dummy", "-u")
79 c.Assert(err, IsNil)
80 curl := charm.MustParseURL("local:precise/dummy-2")
81 s.AssertService(c, "dummy", curl, 1, 0)
82 // Check the charm really was upgraded.
83 ch, err := charm.ReadDir(dirPath)
84 c.Assert(err, IsNil)
85 c.Assert(ch.Revision(), Equals, 2)
86}
87
88func (s *DeploySuite) TestCharmBundle(c *C) {
89 coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
90 err := runDeploy(c, "local:dummy", "some-service-name")
91 c.Assert(err, IsNil)
92 curl := charm.MustParseURL("local:precise/dummy-1")
93 s.AssertService(c, "some-service-name", curl, 1, 0)
94}
95
96func (s *DeploySuite) TestCannotUpgradeCharmBundle(c *C) {
97 coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
98 err := runDeploy(c, "local:dummy", "-u")
99 c.Assert(err, ErrorMatches, `cannot increment revision of charm "local:precise/dummy-1": not a directory`)
100 // Verify state not touched...
101 curl := charm.MustParseURL("local:precise/dummy-1")
102 _, err = s.State.Charm(curl)
103 c.Assert(err, ErrorMatches, `charm "local:precise/dummy-1" not found`)
104 _, err = s.State.Service("dummy")
105 c.Assert(err, ErrorMatches, `service "dummy" not found`)
106}
107
108func (s *DeploySuite) TestSubordinateCharm(c *C) {
109 coretesting.Charms.BundlePath(s.SeriesPath, "logging")
110 err := runDeploy(c, "local:logging")
111 c.Assert(err, IsNil)
112 curl := charm.MustParseURL("local:precise/logging-1")
113 s.AssertService(c, "logging", curl, 0, 0)
114}
115
116func (s *DeploySuite) TestConfig(c *C) {
117 coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
118 path := setupConfigfile(c, c.MkDir())
119 err := runDeploy(c, "local:dummy", "dummy-service", "--config", path)
120 c.Assert(err, IsNil)
121 service, err := s.State.Service("dummy-service")
122 c.Assert(err, IsNil)
123 settings, err := service.ConfigSettings()
124 c.Assert(err, IsNil)
125 c.Assert(settings, DeepEquals, charm.Settings{
126 "skill-level": int64(9000),
127 "username": "admin001",
128 })
129}
130
131func (s *DeploySuite) TestConfigError(c *C) {
132 coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
133 path := setupConfigfile(c, c.MkDir())
134 err := runDeploy(c, "local:dummy", "other-service", "--config", path)
135 c.Assert(err, ErrorMatches, `no settings found for "other-service"`)
136 _, err = s.State.Service("other-service")
137 c.Assert(errors.IsNotFoundError(err), Equals, true)
138}
139
140func (s *DeploySuite) TestConstraints(c *C) {
141 coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
142 err := runDeploy(c, "local:dummy", "--constraints", "mem=2G cpu-cores=2")
143 c.Assert(err, IsNil)
144 curl := charm.MustParseURL("local:precise/dummy-1")
145 service, _ := s.AssertService(c, "dummy", curl, 1, 0)
146 cons, err := service.Constraints()
147 c.Assert(err, IsNil)
148 c.Assert(cons, DeepEquals, constraints.MustParse("mem=2G cpu-cores=2"))
149}
150
151func (s *DeploySuite) TestSubordinateConstraints(c *C) {
152 coretesting.Charms.BundlePath(s.SeriesPath, "logging")
153 err := runDeploy(c, "local:logging", "--constraints", "mem=1G")
154 c.Assert(err, ErrorMatches, "cannot specify constraints for subordinate service")
155}
156
157func (s *DeploySuite) TestNumUnits(c *C) {
158 coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
159 err := runDeploy(c, "local:dummy", "-n", "13")
160 c.Assert(err, IsNil)
161 curl := charm.MustParseURL("local:precise/dummy-1")
162 s.AssertService(c, "dummy", curl, 13, 0)
163}
164
165func (s *DeploySuite) TestNumUnitsSubordinate(c *C) {
166 coretesting.Charms.BundlePath(s.SeriesPath, "logging")
167 err := runDeploy(c, "--num-units", "3", "local:logging")
168 c.Assert(err, ErrorMatches, `cannot specify units for subordinate service`)
169 _, err = s.State.Service("dummy")
170 c.Assert(err, ErrorMatches, `service "dummy" not found`)
171}
172
173func (s *DeploySuite) TestForceMachine(c *C) {
174 coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
175 machine, err := s.State.AddMachine("precise", state.JobHostUnits)
176 c.Assert(err, IsNil)
177 err = runDeploy(c, "--force-machine", machine.Id(), "local:dummy", "portlandia")
178 c.Assert(err, IsNil)
179 svc, err := s.State.Service("portlandia")
180 c.Assert(err, IsNil)
181 units, err := svc.AllUnits()
182 c.Assert(err, IsNil)
183 c.Assert(units, HasLen, 1)
184 mid, err := units[0].AssignedMachineId()
185 c.Assert(err, IsNil)
186 c.Assert(mid, Equals, machine.Id())
187}
188
189func (s *DeploySuite) TestForceMachineNotFound(c *C) {
190 coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
191 err := runDeploy(c, "--force-machine", "42", "local:dummy", "portlandia")
192 c.Assert(err, ErrorMatches, `cannot assign unit "portlandia/0" to machine: machine 42 not found`)
193 _, err = s.State.Service("dummy")
194 c.Assert(err, ErrorMatches, `service "dummy" not found`)
195}
196
197func (s *DeploySuite) TestForceMachineSubordinate(c *C) {
198 machine, err := s.State.AddMachine("precise", state.JobHostUnits)
199 c.Assert(err, IsNil)
200 coretesting.Charms.BundlePath(s.SeriesPath, "logging")
201 err = runDeploy(c, "--force-machine", machine.Id(), "local:logging")
202 c.Assert(err, ErrorMatches, `cannot specify units for subordinate service`)
203 _, err = s.State.Service("dummy")
204 c.Assert(err, ErrorMatches, `service "dummy" not found`)
205}
58206
=== modified file 'juju/conn.go'
--- juju/conn.go 2013-06-11 02:14:27 +0000
+++ juju/conn.go 2013-06-11 02:14:27 +0000
@@ -186,51 +186,55 @@
186// DeployServiceParams contains the arguments required to deploy the referenced186// DeployServiceParams contains the arguments required to deploy the referenced
187// charm.187// charm.
188type DeployServiceParams struct {188type DeployServiceParams struct {
189 ServiceName string189 ServiceName string
190 Charm *state.Charm190 Charm *state.Charm
191 NumUnits int191 ConfigSettings charm.Settings
192 // Config is used only by the API.192 Constraints constraints.Value
193 Config map[string]string193 NumUnits int
194 // ConfigYAML takes precedence over Config if both are provided.
195 ConfigYAML string
196 Constraints constraints.Value
197 // Use string for deploy-to machine to avoid ambiguity around machine 0.194 // Use string for deploy-to machine to avoid ambiguity around machine 0.
198 ForceMachineId string195 ForceMachineId string
199}196}
200197
201// DeployService takes a charm and various parameters and deploys it.198// DeployService takes a charm and various parameters and deploys it.
202func (conn *Conn) DeployService(args DeployServiceParams) (*state.Service, error) {199func (conn *Conn) DeployService(args DeployServiceParams) (*state.Service, error) {
203 var err error200 settings, err := args.Charm.Config().ValidateSettings(args.ConfigSettings)
204 var settings charm.Settings
205 config := args.Charm.Config()
206 if args.ConfigYAML != "" {
207 settings, err = config.ParseSettingsYAML([]byte(args.ConfigYAML), args.ServiceName)
208 } else if args.Config != nil {
209 settings, err = config.ParseSettingsStrings(args.Config)
210 }
211 if err != nil {201 if err != nil {
212 return nil, err202 return nil, err
213 }203 }
214 svc, err := conn.State.AddService(args.ServiceName, args.Charm)204 emptyCons := constraints.Value{}
205 if args.Charm.Meta().Subordinate {
206 if args.NumUnits != 0 || args.ForceMachineId != "" {
207 return nil, fmt.Errorf("subordinate service must be deployed without units")
208 }
209 if args.Constraints != emptyCons {
210 return nil, fmt.Errorf("subordinate service must be deployed without constraints")
211 }
212 }
213 // TODO(fwereade): transactional State.AddService including settings, constraints
214 // (, minimumUnitCount, initialMachineIds?).
215 service, err := conn.State.AddService(args.ServiceName, args.Charm)
215 if err != nil {216 if err != nil {
216 return nil, err217 return nil, err
217 }218 }
218 if len(settings) > 0 {219 if len(settings) > 0 {
219 if err := svc.UpdateConfigSettings(settings); err != nil {220 if err := service.UpdateConfigSettings(settings); err != nil {
220 return nil, err221 return nil, err
221 }222 }
222 }223 }
223 if args.Charm.Meta().Subordinate {224 if args.Charm.Meta().Subordinate {
224 return svc, nil225 return service, nil
225 }226 }
226 if err := svc.SetConstraints(args.Constraints); err != nil {227 if args.Constraints != emptyCons {
227 return nil, err228 if err := service.SetConstraints(args.Constraints); err != nil {
228 }229 return nil, err
229 _, err = conn.AddUnits(svc, args.NumUnits, args.ForceMachineId)230 }
230 if err != nil {231 }
231 return nil, err232 if args.NumUnits > 0 {
232 }233 if _, err := conn.AddUnits(service, args.NumUnits, args.ForceMachineId); err != nil {
233 return svc, nil234 return nil, err
235 }
236 }
237 return service, nil
234}238}
235239
236func (conn *Conn) addCharm(curl *charm.URL, ch charm.Charm) (*state.Charm, error) {240func (conn *Conn) addCharm(curl *charm.URL, ch charm.Charm) (*state.Charm, error) {
237241
=== modified file 'juju/conn_test.go'
--- juju/conn_test.go 2013-06-11 02:14:27 +0000
+++ juju/conn_test.go 2013-06-11 02:14:27 +0000
@@ -11,11 +11,13 @@
11 "launchpad.net/juju-core/environs"11 "launchpad.net/juju-core/environs"
12 "launchpad.net/juju-core/environs/config"12 "launchpad.net/juju-core/environs/config"
13 "launchpad.net/juju-core/environs/dummy"13 "launchpad.net/juju-core/environs/dummy"
14 "launchpad.net/juju-core/errors"
14 "launchpad.net/juju-core/juju"15 "launchpad.net/juju-core/juju"
15 "launchpad.net/juju-core/juju/testing"16 "launchpad.net/juju-core/juju/testing"
16 "launchpad.net/juju-core/state"17 "launchpad.net/juju-core/state"
17 coretesting "launchpad.net/juju-core/testing"18 coretesting "launchpad.net/juju-core/testing"
18 "launchpad.net/juju-core/utils"19 "launchpad.net/juju-core/utils"
20 "launchpad.net/juju-core/utils/set"
19 "os"21 "os"
20 "path/filepath"22 "path/filepath"
21 stdtesting "testing"23 stdtesting "testing"
@@ -402,63 +404,159 @@
402404
403}405}
404406
407// DeployLocalSuite uses a fresh copy of the same local dummy charm for each
408// test, because ServiceDeploy demands that a charm already exists in state,
409// and that's is the simplest way to get one in there.
405type DeployLocalSuite struct {410type DeployLocalSuite struct {
406 testing.JujuConnSuite411 testing.JujuConnSuite
407 repo *charm.LocalRepository412 repo charm.Repository
408 defaultSeries string413 charm *state.Charm
409 seriesPath string414 oldCacheDir string
410 charmUrl *charm.URL
411}415}
412416
413var _ = Suite(&DeployLocalSuite{})417var _ = Suite(&DeployLocalSuite{})
414418
419func (s *DeployLocalSuite) SetUpSuite(c *C) {
420 s.JujuConnSuite.SetUpSuite(c)
421 s.repo = &charm.LocalRepository{Path: coretesting.Charms.Path}
422 s.oldCacheDir, charm.CacheDir = charm.CacheDir, c.MkDir()
423}
424
425func (s *DeployLocalSuite) TearDownSuite(c *C) {
426 charm.CacheDir = s.oldCacheDir
427 s.JujuConnSuite.TearDownSuite(c)
428}
429
415func (s *DeployLocalSuite) SetUpTest(c *C) {430func (s *DeployLocalSuite) SetUpTest(c *C) {
416 s.JujuConnSuite.SetUpTest(c)431 s.JujuConnSuite.SetUpTest(c)
417 repoPath := c.MkDir()432 curl := charm.MustParseURL("local:series/dummy")
418 s.defaultSeries = "precise"433 charm, err := s.Conn.PutCharm(curl, s.repo, false)
419 s.repo = &charm.LocalRepository{Path: repoPath}434 c.Assert(err, IsNil)
420 s.seriesPath = filepath.Join(repoPath, s.defaultSeries)435 s.charm = charm
421 err := os.Mkdir(s.seriesPath, 0777)436}
422 c.Assert(err, IsNil)437
423 coretesting.Charms.BundlePath(s.seriesPath, "wordpress")438func (s *DeployLocalSuite) TestDeployMinimal(c *C) {
424 s.charmUrl, err = charm.InferURL("local:wordpress", s.defaultSeries)439 service, err := s.Conn.DeployService(juju.DeployServiceParams{
425 c.Assert(err, IsNil)440 ServiceName: "bob",
426}441 Charm: s.charm,
427442 })
428func (s *DeployLocalSuite) TestDeploy(c *C) {443 c.Assert(err, IsNil)
429 ch, err := s.Conn.PutCharm(s.charmUrl, s.repo, false)444 s.assertCharm(c, service, s.charm.URL())
430 c.Assert(err, IsNil)445 s.assertSettings(c, service, charm.Settings{})
431 cons := constraints.MustParse("mem=4G")446 s.assertConstraints(c, service, constraints.Value{})
432 args := juju.DeployServiceParams{447 s.assertMachines(c, service, constraints.Value{})
433 ServiceName: "bob",448}
434 Charm: ch,449
435 NumUnits: 3,450func (s *DeployLocalSuite) TestDeploySettings(c *C) {
436 Constraints: cons,451 service, err := s.Conn.DeployService(juju.DeployServiceParams{
437 ConfigYAML: "bob: {blog-title: aspidistra flagpole}",452 ServiceName: "bob",
438 }453 Charm: s.charm,
439 svc, err := s.Conn.DeployService(args)454 ConfigSettings: charm.Settings{
440 c.Assert(err, IsNil)455 "title": "banana cupcakes",
441 scons, err := svc.Constraints()456 "skill-level": 9901,
442 c.Assert(err, IsNil)457 },
443 c.Assert(scons, DeepEquals, cons)458 })
444 settings, err := svc.ConfigSettings()459 c.Assert(err, IsNil)
445 c.Assert(err, IsNil)460 s.assertSettings(c, service, charm.Settings{
446 c.Assert(settings, DeepEquals, charm.Settings{461 "title": "banana cupcakes",
447 "blog-title": "aspidistra flagpole",462 "skill-level": int64(9901),
448 })463 })
449464}
450 units, err := svc.AllUnits()465
451 c.Assert(err, IsNil)466func (s *DeployLocalSuite) TestDeploySettingsError(c *C) {
452 c.Assert(len(units), Equals, 3)467 _, err := s.Conn.DeployService(juju.DeployServiceParams{
468 ServiceName: "bob",
469 Charm: s.charm,
470 ConfigSettings: charm.Settings{
471 "skill-level": 99.01,
472 },
473 })
474 c.Assert(err, ErrorMatches, `option "skill-level" expected int, got 99.01`)
475 _, err = s.State.Service("bob")
476 c.Assert(errors.IsNotFoundError(err), Equals, true)
477}
478
479func (s *DeployLocalSuite) TestDeployConstraints(c *C) {
480 err := s.State.SetEnvironConstraints(constraints.MustParse("mem=2G"))
481 c.Assert(err, IsNil)
482 serviceCons := constraints.MustParse("cpu-cores=2")
483 service, err := s.Conn.DeployService(juju.DeployServiceParams{
484 ServiceName: "bob",
485 Charm: s.charm,
486 Constraints: serviceCons,
487 })
488 c.Assert(err, IsNil)
489 s.assertConstraints(c, service, serviceCons)
490}
491
492func (s *DeployLocalSuite) TestDeployNumUnits(c *C) {
493 err := s.State.SetEnvironConstraints(constraints.MustParse("mem=2G"))
494 c.Assert(err, IsNil)
495 serviceCons := constraints.MustParse("cpu-cores=2")
496 service, err := s.Conn.DeployService(juju.DeployServiceParams{
497 ServiceName: "bob",
498 Charm: s.charm,
499 Constraints: serviceCons,
500 NumUnits: 2,
501 })
502 c.Assert(err, IsNil)
503 s.assertConstraints(c, service, serviceCons)
504 s.assertMachines(c, service, constraints.MustParse("mem=2G cpu-cores=2"), "0", "1")
505}
506
507func (s *DeployLocalSuite) TestDeployForceMachineId(c *C) {
508 machine, err := s.State.AddMachine("series", state.JobHostUnits)
509 c.Assert(err, IsNil)
510 c.Assert(machine.Id(), Equals, "0")
511 err = s.State.SetEnvironConstraints(constraints.MustParse("mem=2G"))
512 c.Assert(err, IsNil)
513 serviceCons := constraints.MustParse("cpu-cores=2")
514 service, err := s.Conn.DeployService(juju.DeployServiceParams{
515 ServiceName: "bob",
516 Charm: s.charm,
517 Constraints: serviceCons,
518 NumUnits: 1,
519 ForceMachineId: "0",
520 })
521 c.Assert(err, IsNil)
522 s.assertConstraints(c, service, serviceCons)
523 s.assertMachines(c, service, constraints.Value{}, "0")
524}
525
526func (s *DeployLocalSuite) assertCharm(c *C, service *state.Service, expect *charm.URL) {
527 curl, force := service.CharmURL()
528 c.Assert(curl, DeepEquals, expect)
529 c.Assert(force, Equals, false)
530}
531
532func (s *DeployLocalSuite) assertSettings(c *C, service *state.Service, expect charm.Settings) {
533 settings, err := service.ConfigSettings()
534 c.Assert(err, IsNil)
535 c.Assert(settings, DeepEquals, expect)
536}
537
538func (s *DeployLocalSuite) assertConstraints(c *C, service *state.Service, expect constraints.Value) {
539 cons, err := service.Constraints()
540 c.Assert(err, IsNil)
541 c.Assert(cons, DeepEquals, expect)
542}
543
544func (s *DeployLocalSuite) assertMachines(c *C, service *state.Service, expectCons constraints.Value, expectIds ...string) {
545 units, err := service.AllUnits()
546 c.Assert(err, IsNil)
547 c.Assert(units, HasLen, len(expectIds))
548 unseenIds := set.NewStrings(expectIds...)
453 for _, unit := range units {549 for _, unit := range units {
454 mid, err := unit.AssignedMachineId()550 id, err := unit.AssignedMachineId()
455 c.Assert(err, IsNil)551 c.Assert(err, IsNil)
456 machine, err := s.State.Machine(mid)552 unseenIds.Remove(id)
457 c.Assert(err, IsNil)553 machine, err := s.State.Machine(id)
458 mcons, err := machine.Constraints()554 c.Assert(err, IsNil)
459 c.Assert(err, IsNil)555 cons, err := machine.Constraints()
460 c.Assert(mcons, DeepEquals, cons)556 c.Assert(err, IsNil)
557 c.Assert(cons, DeepEquals, expectCons)
461 }558 }
559 c.Assert(unseenIds, DeepEquals, set.NewStrings())
462}560}
463561
464type InitJujuHomeSuite struct {562type InitJujuHomeSuite struct {
465563
=== modified file 'state/api/params/params.go'
--- state/api/params/params.go 2013-06-04 13:00:57 +0000
+++ state/api/params/params.go 2013-06-11 02:14:27 +0000
@@ -94,7 +94,6 @@
94 Config map[string]string94 Config map[string]string
95 ConfigYAML string // Takes precedence over config if both are present.95 ConfigYAML string // Takes precedence over config if both are present.
96 Constraints constraints.Value96 Constraints constraints.Value
97 BumpRevision bool
98 ForceMachineId string97 ForceMachineId string
99}98}
10099
101100
=== modified file 'state/apiserver/client.go'
--- state/apiserver/client.go 2013-06-11 02:14:27 +0000
+++ state/apiserver/client.go 2013-06-11 02:14:27 +0000
@@ -4,6 +4,7 @@
4package apiserver4package apiserver
55
6import (6import (
7 "fmt"
7 "launchpad.net/juju-core/charm"8 "launchpad.net/juju-core/charm"
8 "launchpad.net/juju-core/juju"9 "launchpad.net/juju-core/juju"
9 "launchpad.net/juju-core/state/api"10 "launchpad.net/juju-core/state/api"
@@ -102,30 +103,45 @@
102103
103var CharmStore charm.Repository = charm.Store104var CharmStore charm.Repository = charm.Store
104105
105// ServiceDeploy fetches the charm from the charm store and deploys it. Local106// ServiceDeploy fetches the charm from the charm store and deploys it. Local
106// charms are not supported.107// charms are not supported.
107func (c *srvClient) ServiceDeploy(args params.ServiceDeploy) error {108func (c *srvClient) ServiceDeploy(args params.ServiceDeploy) error {
108 state := c.root.srv.state109 curl, err := charm.ParseURL(args.CharmUrl)
109 conf, err := state.EnvironConfig()110 if err != nil {
110 if err != nil {111 return err
111 return err112 }
112 }113 if curl.Schema != "cs" {
113 curl, err := charm.InferURL(args.CharmUrl, conf.DefaultSeries())114 return fmt.Errorf(`charm url has unsupported schema %q`, curl.Schema)
114 if err != nil {115 }
115 return err116 if curl.Revision < 0 {
116 }117 return fmt.Errorf("charm url must include revision")
117 conn, err := juju.NewConnFromState(state)118 }
118 if err != nil {119 conn, err := juju.NewConnFromState(c.root.srv.state)
119 return err120 if err != nil {
120 }121 return err
121 if args.NumUnits == 0 {122 }
122 args.NumUnits = 1123 ch, err := conn.PutCharm(curl, CharmStore, false)
123 }124 if err != nil {
124 serviceName := args.ServiceName125 return err
125 if serviceName == "" {126 }
126 serviceName = curl.Name127 var settings charm.Settings
127 }128 if len(args.ConfigYAML) > 0 {
128 return statecmd.ServiceDeploy(state, args, conn, curl, CharmStore)129 settings, err = ch.Config().ParseSettingsYAML([]byte(args.ConfigYAML), args.ServiceName)
130 } else if len(args.Config) > 0 {
131 settings, err = ch.Config().ParseSettingsStrings(args.Config)
132 }
133 if err != nil {
134 return err
135 }
136 _, err = conn.DeployService(juju.DeployServiceParams{
137 ServiceName: args.ServiceName,
138 Charm: ch,
139 NumUnits: args.NumUnits,
140 ConfigSettings: settings,
141 Constraints: args.Constraints,
142 ForceMachineId: args.ForceMachineId,
143 })
144 return err
129}145}
130146
131// AddServiceUnits adds a given number of units to a service.147// AddServiceUnits adds a given number of units to a service.
132148
=== modified file 'state/apiserver/client_test.go'
--- state/apiserver/client_test.go 2013-06-11 02:14:27 +0000
+++ state/apiserver/client_test.go 2013-06-11 02:14:27 +0000
@@ -4,6 +4,7 @@
4package apiserver_test4package apiserver_test
55
6import (6import (
7 "fmt"
7 . "launchpad.net/gocheck"8 . "launchpad.net/gocheck"
8 "launchpad.net/juju-core/charm"9 "launchpad.net/juju-core/charm"
9 "launchpad.net/juju-core/constraints"10 "launchpad.net/juju-core/constraints"
@@ -300,74 +301,130 @@
300 c.Assert(mode, Equals, state.ResolvedNoHooks)301 c.Assert(mode, Equals, state.ResolvedNoHooks)
301}302}
302303
303var serviceDeployTests = []struct {304func (s *clientSuite) TestClientServiceDeployCharmErrors(c *C) {
304 about string305 withMockCharmStore(func(store *coretesting.MockCharmStore) {
305 charmUrl string306 for url, expect := range map[string]string{
306 numUnits int307 // TODO(fwereade) make these errors consistent one day.
307 expectedNumUnits int308 "wordpress": `charm URL has invalid schema: "wordpress"`,
308 constraints constraints.Value309 "cs:wordpress": `charm URL without series: "cs:wordpress"`,
309}{{310 "cs:precise/wordpress": "charm url must include revision",
310 about: "Normal deploy",311 "cs:precise/wordpress-999999": `cannot get charm: charm not found in mock store: cs:precise/wordpress-999999`,
311 charmUrl: "local:series/wordpress",312 "local:precise/wordpress-999999": `charm url has unsupported schema "local"`,
312 expectedNumUnits: 1,313 } {
313 constraints: constraints.MustParse("mem=1G"),314 c.Logf("test %s", url)
314}, {315 err := s.APIState.Client().ServiceDeploy(
315 about: "Two units",316 url, "service", 1, "", constraints.Value{},
316 charmUrl: "local:series/wordpress",317 )
317 numUnits: 2,318 c.Check(err, ErrorMatches, expect)
318 expectedNumUnits: 2,319 _, err = s.State.Service("service")
319 constraints: constraints.MustParse("mem=4G"),
320},
321}
322
323func (s *clientSuite) TestClientServiceDeploy(c *C) {
324 s.setUpScenario(c)
325
326 for i, test := range serviceDeployTests {
327 c.Logf("test %d; %s", i, test.about)
328 parsedUrl := charm.MustParseURL(test.charmUrl)
329 localRepo, err := charm.InferRepository(parsedUrl, coretesting.Charms.Path)
330 c.Assert(err, IsNil)
331 withRepo(localRepo, func() {
332 serviceName := "mywordpress"
333 _, err = s.State.Service(serviceName)
334 c.Assert(errors.IsNotFoundError(err), Equals, true)320 c.Assert(errors.IsNotFoundError(err), Equals, true)
335 err = s.APIState.Client().ServiceDeploy(321 }
336 test.charmUrl, serviceName, test.numUnits, "mywordpress: {}", test.constraints,322 })
337 )323}
338 c.Assert(err, IsNil)324
339325func (s *clientSuite) TestClientServiceDeployPrincipal(c *C) {
340 service, err := s.State.Service(serviceName)326 // TODO(fwereade): test ForceMachineId directly on srvClient, when we
341 c.Assert(err, IsNil)327 // manage to extract it as a package and can thus do it conveniently.
342 defer removeServiceAndUnits(c, service)328 withMockCharmStore(func(store *coretesting.MockCharmStore) {
343 scons, err := service.Constraints()329 curl, bundle := addCharm(c, store, "dummy")
344 c.Assert(err, IsNil)330 mem4g := constraints.MustParse("mem=4G")
345 c.Assert(scons, DeepEquals, test.constraints)331 err := s.APIState.Client().ServiceDeploy(
346332 curl.String(), "service", 3, "", mem4g,
347 units, err := service.AllUnits()333 )
348 c.Assert(err, IsNil)334 c.Assert(err, IsNil)
349 c.Assert(units, HasLen, test.expectedNumUnits)335 service, err := s.State.Service("service")
350 for _, unit := range units {336 c.Assert(err, IsNil)
351 mid, err := unit.AssignedMachineId()337 charm, force, err := service.Charm()
352 c.Assert(err, IsNil)338 c.Assert(err, IsNil)
353 machine, err := s.State.Machine(mid)339 c.Assert(force, Equals, false)
354 c.Assert(err, IsNil)340 c.Assert(charm.URL(), DeepEquals, curl)
355 mcons, err := machine.Constraints()341 c.Assert(charm.Meta(), DeepEquals, bundle.Meta())
356 c.Assert(err, IsNil)342 c.Assert(charm.Config(), DeepEquals, bundle.Config())
357 c.Assert(mcons, DeepEquals, test.constraints)343
358 }344 cons, err := service.Constraints()
359 })345 c.Assert(err, IsNil)
360 }346 c.Assert(cons, DeepEquals, mem4g)
361}347 units, err := service.AllUnits()
362348 c.Assert(err, IsNil)
363func withRepo(repo charm.Repository, f func()) {349 for _, unit := range units {
364 // Monkey-patch server repository.350 mid, err := unit.AssignedMachineId()
365 originalServerCharmStore := apiserver.CharmStore351 c.Assert(err, IsNil)
366 apiserver.CharmStore = repo352 machine, err := s.State.Machine(mid)
367 defer func() {353 c.Assert(err, IsNil)
368 apiserver.CharmStore = originalServerCharmStore354 cons, err := machine.Constraints()
369 }()355 c.Assert(err, IsNil)
370 f()356 c.Assert(cons, DeepEquals, mem4g)
357 }
358 })
359}
360
361func (s *clientSuite) TestClientServiceDeploySubordinate(c *C) {
362 withMockCharmStore(func(store *coretesting.MockCharmStore) {
363 curl, bundle := addCharm(c, store, "logging")
364 err := s.APIState.Client().ServiceDeploy(
365 curl.String(), "service-name", 0, "", constraints.Value{},
366 )
367 service, err := s.State.Service("service-name")
368 c.Assert(err, IsNil)
369 charm, force, err := service.Charm()
370 c.Assert(err, IsNil)
371 c.Assert(force, Equals, false)
372 c.Assert(charm.URL(), DeepEquals, curl)
373 c.Assert(charm.Meta(), DeepEquals, bundle.Meta())
374 c.Assert(charm.Config(), DeepEquals, bundle.Config())
375
376 units, err := service.AllUnits()
377 c.Assert(err, IsNil)
378 c.Assert(units, HasLen, 0)
379 })
380}
381
382func (s *clientSuite) TestClientServiceDeployConfig(c *C) {
383 // TODO(fwereade): test Config/ConfigYAML handling directly on srvClient.
384 // Can't be done cleanly until it's extracted similarly to Machiner.
385 withMockCharmStore(func(store *coretesting.MockCharmStore) {
386 curl, _ := addCharm(c, store, "dummy")
387 err := s.APIState.Client().ServiceDeploy(
388 curl.String(), "service-name", 1, "service-name:\n username: fred", constraints.Value{},
389 )
390 c.Assert(err, IsNil)
391 service, err := s.State.Service("service-name")
392 c.Assert(err, IsNil)
393 settings, err := service.ConfigSettings()
394 c.Assert(err, IsNil)
395 c.Assert(settings, DeepEquals, charm.Settings{"username": "fred"})
396 })
397}
398
399func (s *clientSuite) TestClientServiceDeployConfigError(c *C) {
400 // TODO(fwereade): test Config/ConfigYAML handling directly on srvClient.
401 // Can't be done cleanly until it's extracted similarly to Machiner.
402 withMockCharmStore(func(store *coretesting.MockCharmStore) {
403 curl, _ := addCharm(c, store, "dummy")
404 err := s.APIState.Client().ServiceDeploy(
405 curl.String(), "service-name", 1, "service-name:\n skill-level: fred", constraints.Value{},
406 )
407 c.Assert(err, ErrorMatches, `option "skill-level" expected int, got "fred"`)
408 _, err = s.State.Service("service-name")
409 c.Assert(errors.IsNotFoundError(err), Equals, true)
410 })
411}
412
413func withMockCharmStore(f func(*coretesting.MockCharmStore)) {
414 mockStore := coretesting.NewMockCharmStore()
415 origStore := apiserver.CharmStore
416 apiserver.CharmStore = mockStore
417 defer func() { apiserver.CharmStore = origStore }()
418 f(mockStore)
419}
420
421func addCharm(c *C, store *coretesting.MockCharmStore, name string) (*charm.URL, charm.Charm) {
422 bundle := coretesting.Charms.Bundle(c.MkDir(), name)
423 scurl := fmt.Sprintf("cs:precise/%s-%d", name, bundle.Revision())
424 curl := charm.MustParseURL(scurl)
425 err := store.SetCharm(curl, bundle)
426 c.Assert(err, IsNil)
427 return curl, bundle
371}428}
372429
373func (s *clientSuite) TestSuccessfulAddRelation(c *C) {430func (s *clientSuite) TestSuccessfulAddRelation(c *C) {
374431
=== modified file 'state/apiserver/perm_test.go'
--- state/apiserver/perm_test.go 2013-06-11 02:14:27 +0000
+++ state/apiserver/perm_test.go 2013-06-11 02:14:27 +0000
@@ -5,11 +5,9 @@
55
6import (6import (
7 . "launchpad.net/gocheck"7 . "launchpad.net/gocheck"
8 "launchpad.net/juju-core/charm"
9 "launchpad.net/juju-core/constraints"8 "launchpad.net/juju-core/constraints"
10 "launchpad.net/juju-core/state"9 "launchpad.net/juju-core/state"
11 "launchpad.net/juju-core/state/api"10 "launchpad.net/juju-core/state/api"
12 "launchpad.net/juju-core/state/apiserver"
13 coretesting "launchpad.net/juju-core/testing"11 coretesting "launchpad.net/juju-core/testing"
14 "strings"12 "strings"
15)13)
@@ -288,23 +286,16 @@
288}286}
289287
290func opClientServiceDeploy(c *C, st *api.State, mst *state.State) (func(), error) {288func opClientServiceDeploy(c *C, st *api.State, mst *state.State) (func(), error) {
291 // We are cheating and using a local repo only.289 var err error
292290 withMockCharmStore(func(store *coretesting.MockCharmStore) {
293 // Set the CharmStore to the test repository.291 curl, _ := addCharm(c, store, "dummy")
294 serviceName := "mywordpress"292 err = st.Client().ServiceDeploy(curl.String(), "deploy-service", 1, "", constraints.Value{})
295 charmUrl := "local:series/wordpress"293 })
296 parsedUrl := charm.MustParseURL(charmUrl)
297 repo, err := charm.InferRepository(parsedUrl, coretesting.Charms.Path)
298 originalServerCharmStore := apiserver.CharmStore
299 apiserver.CharmStore = repo
300
301 err = st.Client().ServiceDeploy(charmUrl, serviceName, 1, "mywordpress: {}", constraints.Value{})
302 if err != nil {294 if err != nil {
303 return func() {}, err295 return func() {}, err
304 }296 }
305 return func() {297 return func() {
306 apiserver.CharmStore = originalServerCharmStore298 service, err := mst.Service("deploy-service")
307 service, err := mst.Service(serviceName)
308 c.Assert(err, IsNil)299 c.Assert(err, IsNil)
309 removeServiceAndUnits(c, service)300 removeServiceAndUnits(c, service)
310 }, nil301 }, nil
311302
=== removed file 'state/statecmd/deploy.go'
--- state/statecmd/deploy.go 2013-05-31 00:13:28 +0000
+++ state/statecmd/deploy.go 1970-01-01 00:00:00 +0000
@@ -1,63 +0,0 @@
1package statecmd
2
3import (
4 "errors"
5 "fmt"
6 "launchpad.net/juju-core/charm"
7 "launchpad.net/juju-core/constraints"
8 "launchpad.net/juju-core/juju"
9 "launchpad.net/juju-core/state"
10 "launchpad.net/juju-core/state/api/params"
11)
12
13// ServiceDeploy deploys a service to the environment from the given repository.
14// The connection, charm URL, and repository will be provided by the caller,
15// since these values already exist in the calling locations and can be
16// defaulted in different scenarios (i.e.: only the charmstore will be used
17// when called from the websocket API).
18func ServiceDeploy(st *state.State, args params.ServiceDeploy, conn *juju.Conn, curl *charm.URL, repo charm.Repository) error {
19 if args.ServiceName != "" && !state.IsServiceName(args.ServiceName) {
20 return fmt.Errorf("invalid service name %q", args.ServiceName)
21 }
22 if args.ForceMachineId != "" {
23 if !state.IsMachineId(args.ForceMachineId) {
24 return fmt.Errorf("invalid machine id %q", args.ForceMachineId)
25 }
26 if args.NumUnits > 1 {
27 return fmt.Errorf("force-machine cannot be used for multiple units")
28 }
29 }
30
31 charm, err := conn.PutCharm(curl, repo, args.BumpRevision)
32 if err != nil {
33 return err
34 }
35 if charm.Meta().Subordinate {
36 empty := constraints.Value{}
37 if args.Constraints != empty {
38 return state.ErrSubordinateConstraints
39 }
40 if args.ForceMachineId != "" {
41 return fmt.Errorf("subordinate service cannot specify force-machine")
42 }
43 } else {
44 if args.NumUnits < 1 {
45 return errors.New("must deploy at least one unit")
46 }
47 }
48 serviceName := args.ServiceName
49 if serviceName == "" {
50 serviceName = curl.Name
51 }
52 deployArgs := juju.DeployServiceParams{
53 Charm: charm,
54 ServiceName: serviceName,
55 NumUnits: args.NumUnits,
56 Config: args.Config,
57 ConfigYAML: args.ConfigYAML,
58 Constraints: args.Constraints,
59 ForceMachineId: args.ForceMachineId,
60 }
61 _, err = conn.DeployService(deployArgs)
62 return err
63}
640
=== removed file 'state/statecmd/deploy_test.go'
--- state/statecmd/deploy_test.go 2013-06-11 02:14:27 +0000
+++ state/statecmd/deploy_test.go 1970-01-01 00:00:00 +0000
@@ -1,273 +0,0 @@
1package statecmd_test
2
3import (
4 . "launchpad.net/gocheck"
5 "launchpad.net/juju-core/charm"
6 "launchpad.net/juju-core/constraints"
7 "launchpad.net/juju-core/juju/testing"
8 "launchpad.net/juju-core/state"
9 "launchpad.net/juju-core/state/api/params"
10 "launchpad.net/juju-core/state/statecmd"
11 coretesting "launchpad.net/juju-core/testing"
12 "os"
13)
14
15type DeploySuite struct {
16 testing.RepoSuite
17}
18
19var _ = Suite(&DeploySuite{})
20
21func (s *DeploySuite) runDeploy(c *C, args params.ServiceDeploy) error {
22 c.Logf("Running deploy: %+v", args)
23 conf, err := s.State.EnvironConfig()
24 c.Assert(err, IsNil)
25 curl, err := charm.InferURL(args.CharmUrl, conf.DefaultSeries())
26 c.Assert(err, IsNil)
27 repo, err := charm.InferRepository(curl, os.Getenv("JUJU_REPOSITORY"))
28 c.Assert(err, IsNil)
29 return statecmd.ServiceDeploy(s.State, args, s.Conn, curl, repo)
30}
31
32func (s *DeploySuite) TestCharmDir(c *C) {
33 coretesting.Charms.ClonedDirPath(s.SeriesPath, "dummy")
34 args := params.ServiceDeploy{
35 CharmUrl: "local:dummy",
36 NumUnits: 1,
37 }
38 err := s.runDeploy(c, args)
39 c.Assert(err, IsNil)
40 curl := charm.MustParseURL("local:precise/dummy-1")
41 // Note that this tests the automatic creation of a service name (dummy)
42 // from the charm URL. This functionality will be going away soon as
43 // ServiceName becomes a required argument.
44 s.AssertService(c, "dummy", curl, 1, 0)
45}
46
47func (s *DeploySuite) TestUpgradeCharmDir(c *C) {
48 dirPath := coretesting.Charms.ClonedDirPath(s.SeriesPath, "dummy")
49 args := params.ServiceDeploy{
50 CharmUrl: "local:dummy",
51 BumpRevision: true,
52 NumUnits: 1,
53 }
54 err := s.runDeploy(c, args)
55 c.Assert(err, IsNil)
56 curl := charm.MustParseURL("local:precise/dummy-2")
57 s.AssertService(c, "dummy", curl, 1, 0)
58 // Check the charm really was upgraded.
59 ch, err := charm.ReadDir(dirPath)
60 c.Assert(err, IsNil)
61 c.Assert(ch.Revision(), Equals, 2)
62}
63
64func (s *DeploySuite) TestCharmBundle(c *C) {
65 coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
66 args := params.ServiceDeploy{
67 CharmUrl: "local:dummy",
68 ServiceName: "some-service-name",
69 NumUnits: 1,
70 }
71 err := s.runDeploy(c, args)
72 c.Assert(err, IsNil)
73 curl := charm.MustParseURL("local:precise/dummy-1")
74 s.AssertService(c, "some-service-name", curl, 1, 0)
75}
76
77func (s *DeploySuite) TestForceMachine(c *C) {
78 coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
79 machine, err := s.State.AddMachine("precise", state.JobHostUnits)
80 c.Assert(err, IsNil)
81 args := params.ServiceDeploy{
82 CharmUrl: "local:dummy",
83 ServiceName: "portlandia",
84 ForceMachineId: machine.Id(),
85 NumUnits: 1,
86 }
87 err = s.runDeploy(c, args)
88 c.Assert(err, IsNil)
89 svc, err := s.State.Service("portlandia")
90 c.Assert(err, IsNil)
91 units, err := svc.AllUnits()
92 c.Assert(err, IsNil)
93 c.Assert(units, HasLen, 1)
94 mid, err := units[0].AssignedMachineId()
95 c.Assert(err, IsNil)
96 c.Assert(mid, Equals, machine.Id())
97}
98
99func (s *DeploySuite) TestForceMachineInvalid(c *C) {
100 coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
101 args := params.ServiceDeploy{
102 CharmUrl: "local:dummy",
103 ServiceName: "portlandia",
104 ForceMachineId: "42",
105 NumUnits: 1,
106 }
107 err := s.runDeploy(c, args)
108 c.Assert(err, ErrorMatches, `cannot assign unit "portlandia/0" to machine: machine 42 not found`)
109
110 args = params.ServiceDeploy{
111 CharmUrl: "local:dummy",
112 ServiceName: "portlandia",
113 ForceMachineId: "abc",
114 NumUnits: 1,
115 }
116
117 err = s.runDeploy(c, args)
118 c.Assert(err, ErrorMatches, `invalid machine id "abc"`)
119
120 machine, err := s.State.AddMachine("precise", state.JobHostUnits)
121 args = params.ServiceDeploy{
122 CharmUrl: "local:dummy",
123 ServiceName: "portlandia",
124 ForceMachineId: machine.Id(),
125 NumUnits: 5,
126 }
127 err = s.runDeploy(c, args)
128 c.Assert(err, ErrorMatches, `force-machine cannot be used for multiple units`)
129
130 coretesting.Charms.BundlePath(s.SeriesPath, "logging")
131 args = params.ServiceDeploy{
132 CharmUrl: "local:logging",
133 ForceMachineId: machine.Id(),
134 }
135 err = s.runDeploy(c, args)
136 c.Assert(err, ErrorMatches, `subordinate service cannot specify force-machine`)
137}
138
139func (s *DeploySuite) TestCannotUpgradeCharmBundle(c *C) {
140 coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
141 args := params.ServiceDeploy{
142 CharmUrl: "local:dummy",
143 BumpRevision: true,
144 }
145 err := s.runDeploy(c, args)
146 c.Assert(err, ErrorMatches, `cannot increment revision of charm "local:precise/dummy-1": not a directory`)
147 // Verify state not touched...
148 curl := charm.MustParseURL("local:precise/dummy-1")
149 _, err = s.State.Charm(curl)
150 c.Assert(err, ErrorMatches, `charm "local:precise/dummy-1" not found`)
151 _, err = s.State.Service("dummy")
152 c.Assert(err, ErrorMatches, `service "dummy" not found`)
153}
154
155func (s *DeploySuite) TestAddsPeerRelations(c *C) {
156 coretesting.Charms.BundlePath(s.SeriesPath, "riak")
157 args := params.ServiceDeploy{
158 CharmUrl: "local:riak",
159 NumUnits: 1,
160 }
161 err := s.runDeploy(c, args)
162 c.Assert(err, IsNil)
163 curl := charm.MustParseURL("local:precise/riak-7")
164 _, rels := s.AssertService(c, "riak", curl, 1, 1)
165 rel := rels[0]
166 ep, err := rel.Endpoint("riak")
167 c.Assert(err, IsNil)
168 c.Assert(ep.Name, Equals, "ring")
169 c.Assert(ep.Role, Equals, charm.RolePeer)
170 c.Assert(ep.Scope, Equals, charm.ScopeGlobal)
171}
172
173func (s *DeploySuite) TestNumUnits(c *C) {
174 coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
175 args := params.ServiceDeploy{
176 CharmUrl: "local:dummy",
177 NumUnits: 13,
178 }
179 err := s.runDeploy(c, args)
180 c.Assert(err, IsNil)
181 curl := charm.MustParseURL("local:precise/dummy-1")
182 s.AssertService(c, "dummy", curl, 13, 0)
183}
184
185func (s *DeploySuite) TestNumUnitsZero(c *C) {
186 coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
187 args := params.ServiceDeploy{
188 CharmUrl: "local:dummy",
189 NumUnits: 0,
190 }
191 err := s.runDeploy(c, args)
192 c.Assert(err, ErrorMatches, "must deploy at least one unit")
193}
194
195func (s *DeploySuite) TestSubordinateCharm(c *C) {
196 coretesting.Charms.BundlePath(s.SeriesPath, "logging")
197 args := params.ServiceDeploy{
198 CharmUrl: "local:logging",
199 }
200 err := s.runDeploy(c, args)
201 c.Assert(err, IsNil)
202 curl := charm.MustParseURL("local:precise/logging-1")
203 s.AssertService(c, "logging", curl, 0, 0)
204}
205
206func (s *DeploySuite) TestConfigMap(c *C) {
207 coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
208 args := params.ServiceDeploy{
209 CharmUrl: "local:dummy",
210 Config: map[string]string{
211 "skill-level": "1",
212 "username": "",
213 },
214 NumUnits: 1,
215 }
216 err := s.runDeploy(c, args)
217 c.Assert(err, IsNil)
218 curl := charm.MustParseURL("local:precise/dummy-1")
219 s.AssertService(c, "dummy", curl, 1, 0)
220 svc, err := s.State.Service("dummy")
221 c.Assert(err, IsNil)
222 settings, err := svc.ConfigSettings()
223 c.Assert(err, IsNil)
224 c.Assert(settings, DeepEquals, charm.Settings{
225 "skill-level": int64(1),
226 })
227}
228
229func (s *DeploySuite) TestConfigYAML(c *C) {
230 coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
231 args := params.ServiceDeploy{
232 CharmUrl: "local:dummy",
233 ConfigYAML: "dummy: {skill-level: 9001, username: null}",
234 NumUnits: 1,
235 }
236 err := s.runDeploy(c, args)
237 c.Assert(err, IsNil)
238 curl := charm.MustParseURL("local:precise/dummy-1")
239 s.AssertService(c, "dummy", curl, 1, 0)
240 svc, err := s.State.Service("dummy")
241 c.Assert(err, IsNil)
242 settings, err := svc.ConfigSettings()
243 c.Assert(err, IsNil)
244 c.Assert(settings, DeepEquals, charm.Settings{
245 "skill-level": int64(9001),
246 })
247}
248
249func (s *DeploySuite) TestConstraints(c *C) {
250 coretesting.Charms.BundlePath(s.SeriesPath, "dummy")
251 args := params.ServiceDeploy{
252 CharmUrl: "local:dummy",
253 Constraints: constraints.MustParse("mem=2G cpu-cores=2"),
254 NumUnits: 1,
255 }
256 err := s.runDeploy(c, args)
257 c.Assert(err, IsNil)
258 curl := charm.MustParseURL("local:precise/dummy-1")
259 service, _ := s.AssertService(c, "dummy", curl, 1, 0)
260 cons, err := service.Constraints()
261 c.Assert(err, IsNil)
262 c.Assert(cons, DeepEquals, constraints.MustParse("mem=2G cpu-cores=2"))
263}
264
265func (s *DeploySuite) TestSubordinateConstraints(c *C) {
266 coretesting.Charms.BundlePath(s.SeriesPath, "logging")
267 args := params.ServiceDeploy{
268 CharmUrl: "local:logging",
269 Constraints: constraints.MustParse("mem=1G"),
270 }
271 err := s.runDeploy(c, args)
272 c.Assert(err, Equals, state.ErrSubordinateConstraints)
273}
2740
=== modified file 'testing/charm.go'
--- testing/charm.go 2013-05-02 15:55:42 +0000
+++ testing/charm.go 2013-06-11 02:14:27 +0000
@@ -111,3 +111,62 @@
111 check(err)111 check(err)
112 return ch112 return ch
113}113}
114
115type MockCharmStore struct {
116 charms map[string]map[int]*charm.Bundle
117}
118
119func NewMockCharmStore() *MockCharmStore {
120 return &MockCharmStore{map[string]map[int]*charm.Bundle{}}
121}
122
123func (s *MockCharmStore) SetCharm(curl *charm.URL, bundle *charm.Bundle) error {
124 base := curl.WithRevision(-1).String()
125 if curl.Revision < 0 {
126 return fmt.Errorf("bad charm url revision")
127 }
128 if bundle == nil {
129 delete(s.charms[base], curl.Revision)
130 return nil
131 }
132 bundleRev := bundle.Revision()
133 bundleName := bundle.Meta().Name
134 if bundleName != curl.Name || bundleRev != curl.Revision {
135 return fmt.Errorf("charm url %s mismatch with bundle %s-%d", curl, bundleName, bundleRev)
136 }
137 if _, found := s.charms[base]; !found {
138 s.charms[base] = map[int]*charm.Bundle{}
139 }
140 s.charms[base][curl.Revision] = bundle
141 return nil
142}
143
144func (s *MockCharmStore) interpret(curl *charm.URL) (base string, rev int) {
145 base, rev = curl.WithRevision(-1).String(), curl.Revision
146 if rev == -1 {
147 for candidate := range s.charms[base] {
148 if candidate > rev {
149 rev = candidate
150 }
151 }
152 }
153 return
154}
155
156func (s *MockCharmStore) Get(curl *charm.URL) (charm.Charm, error) {
157 base, rev := s.interpret(curl)
158 charm, found := s.charms[base][rev]
159 if !found {
160 return nil, fmt.Errorf("charm not found in mock store: %s", curl)
161 }
162 return charm, nil
163}
164
165func (s *MockCharmStore) Latest(curl *charm.URL) (int, error) {
166 curl = curl.WithRevision(-1)
167 base, rev := s.interpret(curl)
168 if _, found := s.charms[base][rev]; !found {
169 return 0, fmt.Errorf("charm not found in mock store: %s", curl)
170 }
171 return rev, nil
172}

Subscribers

People subscribed via source and target branches