Merge lp:~fwereade/juju-core/manifest-charm-deployer into lp:~go-bot/juju-core/trunk

Proposed by William Reade
Status: Merged
Approved by: William Reade
Approved revision: no longer in the source branch.
Merged at revision: 2656
Proposed branch: lp:~fwereade/juju-core/manifest-charm-deployer
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 1382 lines (+875/-162)
10 files modified
worker/uniter/charm/charm_test.go (+12/-2)
worker/uniter/charm/converter.go (+152/-0)
worker/uniter/charm/converter_test.go (+130/-0)
worker/uniter/charm/export_test.go (+10/-0)
worker/uniter/charm/manifest_deployer.go (+1/-1)
worker/uniter/charm/manifest_deployer_test.go (+3/-9)
worker/uniter/modes.go (+19/-5)
worker/uniter/uniter.go (+20/-7)
worker/uniter/uniter_old_test.go (+226/-0)
worker/uniter/uniter_test.go (+302/-138)
To merge this branch: bzr merge lp:~fwereade/juju-core/manifest-charm-deployer
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+215460@code.launchpad.net

Commit message

uniter: hook up manifest deployer

New unit agents use a manifest deployer from the start. Pre-existing ones
use their git deployer only for long enough to recover from charm conflicts,
and use a manifest deployer for all subsequent upgrades.

The .git directory in the charm dir will persist until the next charm
upgrade completes, but can be safely removed at any time after the deployer
is changed.

https://codereview.appspot.com/86910043/

Description of the change

uniter: hook up manifest deployer

New unit agents use a manifest deployer from the start. Pre-existing ones
use their git deployer only for long enough to recover from charm conflicts,
and use a manifest deployer for all subsequent upgrades.

The .git directory in the charm dir will persist until the next charm
upgrade completes, but can be safely removed at any time after the deployer
is changed.

https://codereview.appspot.com/86910043/

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

Reviewers: mp+215460_code.launchpad.net,

Message:
Please take a look.

Description:
uniter: hook up manifest deployer

New unit agents use a manifest deployer from the start. Pre-existing
ones
use their git deployer only for long enough to recover from charm
conflicts,
and use a manifest deployer for all subsequent upgrades.

The .git directory in the charm dir will persist until the next charm
upgrade completes, but can be safely removed at any time after the
deployer
is changed.

https://code.launchpad.net/~fwereade/juju-core/manifest-charm-deployer/+merge/215460

(do not edit description out of merge proposal)

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

Affected files (+877, -162 lines):
   A [revision details]
   M worker/uniter/charm/charm_test.go
   A worker/uniter/charm/converter.go
   A worker/uniter/charm/converter_test.go
   M worker/uniter/charm/export_test.go
   M worker/uniter/charm/manifest_deployer.go
   M worker/uniter/charm/manifest_deployer_test.go
   M worker/uniter/modes.go
   M worker/uniter/uniter.go
   A worker/uniter/uniter_old_test.go
   M worker/uniter/uniter_test.go

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

I really enjoyed reviewing this, I wish you posted more code for us to
review. :)

Very well commented, and clearly robustly tested. LGTM

https://codereview.appspot.com/86910043/diff/1/worker/uniter/charm/converter_test.go
File worker/uniter/charm/converter_test.go (right):

https://codereview.appspot.com/86910043/diff/1/worker/uniter/charm/converter_test.go#newcode128
worker/uniter/charm/converter_test.go:128: ft.File{"common", "final",
0644}.Check(c, s.targetPath)
You don't seem to check what the status of "user" is here.

https://codereview.appspot.com/86910043/

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

Thanks :).

https://codereview.appspot.com/86910043/diff/1/worker/uniter/charm/converter_test.go
File worker/uniter/charm/converter_test.go (right):

https://codereview.appspot.com/86910043/diff/1/worker/uniter/charm/converter_test.go#newcode128
worker/uniter/charm/converter_test.go:128: ft.File{"common", "final",
0644}.Check(c, s.targetPath)
On 2014/04/16 10:36:16, jameinel wrote:
> You don't seem to check what the status of "user" is here.

preserveUser.Check(...)?

https://codereview.appspot.com/86910043/

Revision history for this message
Go Bot (go-bot) wrote :
Download full text (11.1 KiB)

The attempt to merge lp:~fwereade/juju-core/manifest-charm-deployer into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core 0.014s
ok launchpad.net/juju-core/agent 1.212s
ok launchpad.net/juju-core/agent/mongo 1.291s
ok launchpad.net/juju-core/agent/tools 0.186s
ok launchpad.net/juju-core/bzr 5.129s
ok launchpad.net/juju-core/cert 2.371s
ok launchpad.net/juju-core/charm 0.397s
? launchpad.net/juju-core/charm/hooks [no test files]
? launchpad.net/juju-core/charm/testing [no test files]
ok launchpad.net/juju-core/cloudinit 0.030s
ok launchpad.net/juju-core/cloudinit/sshinit 0.781s
ok launchpad.net/juju-core/cmd 0.147s
ok launchpad.net/juju-core/cmd/charm-admin 0.787s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/envcmd 0.155s
ok launchpad.net/juju-core/cmd/juju 229.451s
ok launchpad.net/juju-core/cmd/jujud 75.565s
ok launchpad.net/juju-core/cmd/plugins/juju-metadata 8.979s
? launchpad.net/juju-core/cmd/plugins/juju-restore [no test files]
ok launchpad.net/juju-core/cmd/plugins/local 0.213s
? launchpad.net/juju-core/cmd/plugins/local/juju-local [no test files]
ok launchpad.net/juju-core/constraints 0.020s
ok launchpad.net/juju-core/container 0.034s
ok launchpad.net/juju-core/container/factory 0.045s
ok launchpad.net/juju-core/container/kvm 0.153s
ok launchpad.net/juju-core/container/kvm/mock 0.042s
? launchpad.net/juju-core/container/kvm/testing [no test files]
ok launchpad.net/juju-core/container/lxc 4.270s
? launchpad.net/juju-core/container/lxc/mock [no test files]
? launchpad.net/juju-core/container/lxc/testing [no test files]
? launchpad.net/juju-core/container/testing [no test files]
ok launchpad.net/juju-core/downloader 5.219s
ok launchpad.net/juju-core/environs 2.347s
ok launchpad.net/juju-core/environs/bootstrap 12.658s
ok launchpad.net/juju-core/environs/cloudinit 0.511s
ok launchpad.net/juju-core/environs/config 2.636s
ok launchpad.net/juju-core/environs/configstore 0.033s
ok launchpad.net/juju-core/environs/filestorage 0.031s
ok launchpad.net/juju-core/environs/httpstorage 0.677s
ok launchpad.net/juju-core/environs/imagemetadata 0.407s
? launchpad.net/juju-core/environs/imagemetadata/testing [no test files]
ok launchpad.net/juju-core/environs/instances 0.041s
ok launchpad.net/juju-core/environs/jujutest 0.206s
ok launchpad.net/juju-core/environs/manual 12.909s
ok launchpad.net/juju-core/environs/simplestreams 0.306s
? launchpad.net/juju-core/environs/simplestreams/testing [no test files]
ok launchpad.net/juju-core/environs/sshstorage 0.958s
ok launchpad.net/juju-core/environs/storage 0.865s
ok launchpad.net/juju-core/environs/sync 50.234s
ok launchpad.net/juju-core/environs/testing 0.141s
ok launchpad.net/juju-core/environs/tools 4.602s
? launchpad.net/juju-core/environs/tools/testing [no test files]
ok launchpad.net/juju-core/errors 0.017s
ok launchpad.net/juju-core/instance 0.017s
? launchpad.net/juju-core/instance/testing [no test files]
ok launchpad.net/juju-core/juju 47....

Revision history for this message
Go Bot (go-bot) wrote :
Download full text (11.2 KiB)

The attempt to merge lp:~fwereade/juju-core/manifest-charm-deployer into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core 0.014s
ok launchpad.net/juju-core/agent 1.189s
ok launchpad.net/juju-core/agent/mongo 1.286s
ok launchpad.net/juju-core/agent/tools 0.180s
ok launchpad.net/juju-core/bzr 5.135s
ok launchpad.net/juju-core/cert 2.480s
ok launchpad.net/juju-core/charm 0.444s
? launchpad.net/juju-core/charm/hooks [no test files]
? launchpad.net/juju-core/charm/testing [no test files]
ok launchpad.net/juju-core/cloudinit 0.030s
ok launchpad.net/juju-core/cloudinit/sshinit 0.721s
ok launchpad.net/juju-core/cmd 0.170s
ok launchpad.net/juju-core/cmd/charm-admin 0.708s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/envcmd 0.170s
ok launchpad.net/juju-core/cmd/juju 226.973s
ok launchpad.net/juju-core/cmd/jujud 76.783s
ok launchpad.net/juju-core/cmd/plugins/juju-metadata 10.310s
? launchpad.net/juju-core/cmd/plugins/juju-restore [no test files]
ok launchpad.net/juju-core/cmd/plugins/local 0.156s
? launchpad.net/juju-core/cmd/plugins/local/juju-local [no test files]
ok launchpad.net/juju-core/constraints 0.021s
ok launchpad.net/juju-core/container 0.043s
ok launchpad.net/juju-core/container/factory 0.057s
ok launchpad.net/juju-core/container/kvm 0.183s
ok launchpad.net/juju-core/container/kvm/mock 0.047s
? launchpad.net/juju-core/container/kvm/testing [no test files]
ok launchpad.net/juju-core/container/lxc 4.291s
? launchpad.net/juju-core/container/lxc/mock [no test files]
? launchpad.net/juju-core/container/lxc/testing [no test files]
? launchpad.net/juju-core/container/testing [no test files]
ok launchpad.net/juju-core/downloader 5.235s
ok launchpad.net/juju-core/environs 2.314s
ok launchpad.net/juju-core/environs/bootstrap 11.946s
ok launchpad.net/juju-core/environs/cloudinit 0.460s
ok launchpad.net/juju-core/environs/config 1.848s
ok launchpad.net/juju-core/environs/configstore 0.033s
ok launchpad.net/juju-core/environs/filestorage 0.027s
ok launchpad.net/juju-core/environs/httpstorage 0.674s
ok launchpad.net/juju-core/environs/imagemetadata 0.470s
? launchpad.net/juju-core/environs/imagemetadata/testing [no test files]
ok launchpad.net/juju-core/environs/instances 0.046s
ok launchpad.net/juju-core/environs/jujutest 0.166s
ok launchpad.net/juju-core/environs/manual 10.231s
? launchpad.net/juju-core/environs/network [no test files]
ok launchpad.net/juju-core/environs/simplestreams 0.289s
? launchpad.net/juju-core/environs/simplestreams/testing [no test files]
ok launchpad.net/juju-core/environs/sshstorage 0.881s
ok launchpad.net/juju-core/environs/storage 0.954s
ok launchpad.net/juju-core/environs/sync 50.376s
ok launchpad.net/juju-core/environs/testing 0.151s
ok launchpad.net/juju-core/environs/tools 4.484s
? launchpad.net/juju-core/environs/tools/testing [no test files]
ok launchpad.net/juju-core/errors 0.021s
ok launchpad.net/juju-core/instance 0.021s
? launchpad.net/juju-core/instanc...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'worker/uniter/charm/charm_test.go'
2--- worker/uniter/charm/charm_test.go 2014-04-11 06:39:27 +0000
3+++ worker/uniter/charm/charm_test.go 2014-04-21 10:30:06 +0000
4@@ -64,7 +64,9 @@
5 func (br *bundleReader) AddCustomBundle(c *gc.C, url *corecharm.URL, customize func(path string)) charm.BundleInfo {
6 base := c.MkDir()
7 dirpath := coretesting.Charms.ClonedDirPath(base, "dummy")
8- customize(dirpath)
9+ if customize != nil {
10+ customize(dirpath)
11+ }
12 dir, err := corecharm.ReadDir(dirpath)
13 c.Assert(err, gc.IsNil)
14 err = dir.SetDiskRevision(url.Revision)
15@@ -107,5 +109,13 @@
16 }
17
18 func (b mockBundle) ExpandTo(dir string) error {
19- return b.expand(dir)
20+ if b.expand != nil {
21+ return b.expand(dir)
22+ }
23+ return nil
24+}
25+
26+func charmURL(revision int) *corecharm.URL {
27+ baseURL := corecharm.MustParseURL("cs:s/c")
28+ return baseURL.WithRevision(revision)
29 }
30
31=== added file 'worker/uniter/charm/converter.go'
32--- worker/uniter/charm/converter.go 1970-01-01 00:00:00 +0000
33+++ worker/uniter/charm/converter.go 2014-04-21 10:30:06 +0000
34@@ -0,0 +1,152 @@
35+// Copyright 2014 Canonical Ltd.
36+// Licensed under the AGPLv3, see LICENCE file for details.
37+
38+package charm
39+
40+import (
41+ "fmt"
42+ "os"
43+ "path/filepath"
44+
45+ "launchpad.net/juju-core/charm"
46+ "launchpad.net/juju-core/utils/set"
47+)
48+
49+// NewDeployer returns a Deployer of whatever kind is currently in use for the
50+// supplied paths, or a manifest deployer if none exists yet. It is a var so
51+// that it can be patched for uniter tests.
52+var NewDeployer = newDeployer
53+
54+func newDeployer(charmPath, dataPath string, bundles BundleReader) (Deployer, error) {
55+ gitDeployer := NewGitDeployer(charmPath, dataPath, bundles).(*gitDeployer)
56+ if exists, err := gitDeployer.current.Exists(); err != nil {
57+ return nil, err
58+ } else if exists {
59+ return gitDeployer, nil
60+ }
61+ return NewManifestDeployer(charmPath, dataPath, bundles), nil
62+}
63+
64+// FixDeployer ensures that the supplied Deployer address points to a manifest
65+// deployer. If a git deployer is passed into FixDeployer, it will be converted
66+// to a manifest deployer, and the git deployer data will be removed. The charm
67+// is assumed to be in a stable state; this should not be called if there is any
68+// chance the git deployer is partway through an upgrade, or in a conflicted state.
69+// It is a var so that it can be patched for uniter tests.
70+var FixDeployer = fixDeployer
71+
72+func fixDeployer(deployer *Deployer) error {
73+ if manifestDeployer, ok := (*deployer).(*manifestDeployer); ok {
74+ // This works around a race at the very end of this func, in which
75+ // the process could have been killed after removing the "current"
76+ // symlink but before removing the orphan repos from the data dir.
77+ collectGitOrphans(manifestDeployer.dataPath)
78+ return nil
79+ }
80+ gitDeployer, ok := (*deployer).(*gitDeployer)
81+ if !ok {
82+ return fmt.Errorf("cannot fix unknown deployer type: %T", *deployer)
83+ }
84+ logger.Infof("converting git-based deployer to manifest deployer")
85+ manifestDeployer := &manifestDeployer{
86+ charmPath: gitDeployer.target.Path(),
87+ dataPath: gitDeployer.dataPath,
88+ bundles: gitDeployer.bundles,
89+ }
90+
91+ // Ensure that the staged charm matches the deployed charm: it's possible
92+ // that the uniter was stopped after staging, but before deploying, a new
93+ // bundle.
94+ deployedURL, err := ReadCharmURL(manifestDeployer.CharmPath(charmURLPath))
95+ if err != nil && !os.IsNotExist(err) {
96+ return err
97+ }
98+
99+ // If we deployed something previously, we need to copy some state over.
100+ if deployedURL != nil {
101+ if err := ensureCurrentGitCharm(gitDeployer, deployedURL); err != nil {
102+ return err
103+ }
104+ // Now we know we've got the right stuff checked out in gitDeployer.current,
105+ // we can turn that into a manifest that will be used in future upgrades...
106+ // even if users desparate for space deleted the original bundle.
107+ manifest, err := gitManifest(gitDeployer.current.Path())
108+ if err != nil {
109+ return err
110+ }
111+ if err := manifestDeployer.storeManifest(deployedURL, manifest); err != nil {
112+ return err
113+ }
114+ }
115+
116+ // We're left with the staging repo and a symlink to it. We decide deployer
117+ // type by checking existence of the symlink's target, so we start off by
118+ // trashing the symlink itself; collectGitOrphans will then delete all the
119+ // original deployer's repos.
120+ if err := os.RemoveAll(gitDeployer.current.Path()); err != nil {
121+ return err
122+ }
123+ // Note potential race alluded to at the start of this func.
124+ collectGitOrphans(gitDeployer.dataPath)
125+
126+ // Phew. Done.
127+ *deployer = manifestDeployer
128+ return nil
129+}
130+
131+// ensureCurrentGitCharm checks out progressively earlier versions of the
132+// gitDeployer's current staging repo, until it finds one in which the
133+// content of charmURLPath matches the supplied charm URL.
134+func ensureCurrentGitCharm(gitDeployer *gitDeployer, expectURL *charm.URL) error {
135+ i := 1
136+ repo := gitDeployer.current
137+ for {
138+ stagedURL, err := gitDeployer.current.ReadCharmURL()
139+ if err != nil {
140+ return err
141+ }
142+ logger.Debugf("staged url: %s", stagedURL)
143+ if *stagedURL == *expectURL {
144+ return nil
145+ }
146+ if err := repo.cmd("checkout", fmt.Sprintf("master~%d", i)); err != nil {
147+ return err
148+ }
149+ i++
150+ }
151+}
152+
153+// gitManifest returns every file path in the supplied directory, *except* for:
154+// * paths below .git, because we don't need to track every file: we just
155+// want them all gone
156+// * charmURLPath, because we don't ever want to remove that: that's how
157+// the manifestDeployer keeps track of what version it's upgrading from.
158+// All paths are slash-separated, to match the bundle manifest format.
159+func gitManifest(linkPath string) (set.Strings, error) {
160+ dirPath, err := os.Readlink(linkPath)
161+ if err != nil {
162+ return set.NewStrings(), err
163+ }
164+ manifest := set.NewStrings()
165+ err = filepath.Walk(dirPath, func(path string, fileInfo os.FileInfo, err error) error {
166+ if err != nil {
167+ return err
168+ }
169+ relPath, err := filepath.Rel(dirPath, path)
170+ if err != nil {
171+ return err
172+ }
173+ switch relPath {
174+ case ".", charmURLPath:
175+ return nil
176+ case ".git":
177+ err = filepath.SkipDir
178+ }
179+ manifest.Add(filepath.ToSlash(relPath))
180+ return err
181+ })
182+ if err != nil {
183+ return set.NewStrings(), err
184+ }
185+ return manifest, nil
186+}
187
188=== added file 'worker/uniter/charm/converter_test.go'
189--- worker/uniter/charm/converter_test.go 1970-01-01 00:00:00 +0000
190+++ worker/uniter/charm/converter_test.go 2014-04-21 10:30:06 +0000
191@@ -0,0 +1,130 @@
192+// Copyright 2014 Canonical Ltd.
193+// Licensed under the AGPLv3, see LICENCE file for details.
194+
195+package charm_test
196+
197+import (
198+ jc "github.com/juju/testing/checkers"
199+ gc "launchpad.net/gocheck"
200+
201+ "launchpad.net/juju-core/testing"
202+ ft "launchpad.net/juju-core/testing/filetesting"
203+ "launchpad.net/juju-core/worker/uniter/charm"
204+)
205+
206+type ConverterSuite struct {
207+ testing.GitSuite
208+ targetPath string
209+ dataPath string
210+ bundles *bundleReader
211+}
212+
213+var _ = gc.Suite(&ConverterSuite{})
214+
215+func (s *ConverterSuite) SetUpTest(c *gc.C) {
216+ s.GitSuite.SetUpTest(c)
217+ s.targetPath = c.MkDir()
218+ s.dataPath = c.MkDir()
219+ s.bundles = &bundleReader{}
220+}
221+
222+func (s *ConverterSuite) TestNewDeployerCreatesManifestDeployer(c *gc.C) {
223+ deployer, err := charm.NewDeployer(s.targetPath, s.dataPath, s.bundles)
224+ c.Assert(err, gc.IsNil)
225+ c.Assert(deployer, jc.Satisfies, charm.IsManifestDeployer)
226+}
227+
228+func (s *ConverterSuite) TestNewDeployerCreatesGitDeployerOnceStaged(c *gc.C) {
229+ gitDeployer := charm.NewGitDeployer(s.targetPath, s.dataPath, s.bundles)
230+ info := s.bundles.AddBundle(c, charmURL(1), mockBundle{})
231+ err := gitDeployer.Stage(info, nil)
232+ c.Assert(err, gc.IsNil)
233+
234+ deployer, err := charm.NewDeployer(s.targetPath, s.dataPath, s.bundles)
235+ c.Assert(err, gc.IsNil)
236+ c.Assert(deployer, jc.Satisfies, charm.IsGitDeployer)
237+}
238+
239+func (s *ConverterSuite) TestConvertGitDeployerBeforeDeploy(c *gc.C) {
240+ gitDeployer := charm.NewGitDeployer(s.targetPath, s.dataPath, s.bundles)
241+ info := s.bundles.AddBundle(c, charmURL(1), mockBundle{})
242+ err := gitDeployer.Stage(info, nil)
243+ c.Assert(err, gc.IsNil)
244+
245+ deployer, err := charm.NewDeployer(s.targetPath, s.dataPath, s.bundles)
246+ c.Assert(err, gc.IsNil)
247+ err = charm.FixDeployer(&deployer)
248+ c.Assert(err, gc.IsNil)
249+ c.Assert(deployer, jc.Satisfies, charm.IsManifestDeployer)
250+ ft.Removed{"current"}.Check(c, s.dataPath)
251+
252+ err = deployer.Stage(info, nil)
253+ c.Assert(err, gc.IsNil)
254+ err = deployer.Deploy()
255+ c.Assert(err, gc.IsNil)
256+ ft.Removed{".git"}.Check(c, s.targetPath)
257+}
258+
259+func (s *ConverterSuite) TestConvertGitDeployerAfterDeploy(c *gc.C) {
260+ gitDeployer := charm.NewGitDeployer(s.targetPath, s.dataPath, s.bundles)
261+ info1 := s.bundles.AddBundle(c, charmURL(1), mockBundle{})
262+ err := gitDeployer.Stage(info1, nil)
263+ c.Assert(err, gc.IsNil)
264+ err = gitDeployer.Deploy()
265+ c.Assert(err, gc.IsNil)
266+
267+ deployer, err := charm.NewDeployer(s.targetPath, s.dataPath, s.bundles)
268+ c.Assert(err, gc.IsNil)
269+ err = charm.FixDeployer(&deployer)
270+ c.Assert(err, gc.IsNil)
271+ c.Assert(deployer, jc.Satisfies, charm.IsManifestDeployer)
272+ ft.Removed{"current"}.Check(c, s.dataPath)
273+ ft.Dir{"manifests", 0755}.Check(c, s.dataPath)
274+
275+ info2 := s.bundles.AddBundle(c, charmURL(2), mockBundle{})
276+ err = deployer.Stage(info2, nil)
277+ c.Assert(err, gc.IsNil)
278+ err = deployer.Deploy()
279+ c.Assert(err, gc.IsNil)
280+ ft.Removed{".git"}.Check(c, s.targetPath)
281+}
282+
283+func (s *ConverterSuite) TestPathological(c *gc.C) {
284+ initial := s.bundles.AddCustomBundle(c, charmURL(1), func(path string) {
285+ ft.File{"common", "initial", 0644}.Create(c, path)
286+ ft.File{"initial", "blah", 0644}.Create(c, path)
287+ })
288+ staged := s.bundles.AddCustomBundle(c, charmURL(2), func(path string) {
289+ ft.File{"common", "staged", 0644}.Create(c, path)
290+ ft.File{"user", "badwrong", 0644}.Create(c, path)
291+ })
292+ final := s.bundles.AddCustomBundle(c, charmURL(3), func(path string) {
293+ ft.File{"common", "final", 0644}.Create(c, path)
294+ ft.File{"final", "blah", 0644}.Create(c, path)
295+ })
296+
297+ gitDeployer := charm.NewGitDeployer(s.targetPath, s.dataPath, s.bundles)
298+ err := gitDeployer.Stage(initial, nil)
299+ c.Assert(err, gc.IsNil)
300+ err = gitDeployer.Deploy()
301+ c.Assert(err, gc.IsNil)
302+
303+ preserveUser := ft.File{"user", "preserve", 0644}.Create(c, s.targetPath)
304+ err = gitDeployer.Stage(staged, nil)
305+ c.Assert(err, gc.IsNil)
306+
307+ deployer, err := charm.NewDeployer(s.targetPath, s.dataPath, s.bundles)
308+ c.Assert(err, gc.IsNil)
309+ err = charm.FixDeployer(&deployer)
310+ c.Assert(err, gc.IsNil)
311+
312+ err = deployer.Stage(final, nil)
313+ c.Assert(err, gc.IsNil)
314+ err = deployer.Deploy()
315+ c.Assert(err, gc.IsNil)
316+ ft.Removed{".git"}.Check(c, s.targetPath)
317+ ft.Removed{"initial"}.Check(c, s.targetPath)
318+ ft.Removed{"staged"}.Check(c, s.targetPath)
319+ ft.File{"common", "final", 0644}.Check(c, s.targetPath)
320+ preserveUser.Check(c, s.targetPath)
321+}
322
323=== modified file 'worker/uniter/charm/export_test.go'
324--- worker/uniter/charm/export_test.go 2014-03-25 10:06:50 +0000
325+++ worker/uniter/charm/export_test.go 2014-04-21 10:30:06 +0000
326@@ -12,3 +12,13 @@
327 func GitDeployerCurrent(d Deployer) *GitDir {
328 return d.(*gitDeployer).current
329 }
330+
331+func IsGitDeployer(d Deployer) bool {
332+ _, ok := d.(*gitDeployer)
333+ return ok
334+}
335+
336+func IsManifestDeployer(d Deployer) bool {
337+ _, ok := d.(*manifestDeployer)
338+ return ok
339+}
340
341=== modified file 'worker/uniter/charm/manifest_deployer.go'
342--- worker/uniter/charm/manifest_deployer.go 2014-04-11 06:39:27 +0000
343+++ worker/uniter/charm/manifest_deployer.go 2014-04-21 10:30:06 +0000
344@@ -105,7 +105,6 @@
345 }
346
347 // Move the deploying file over the charm URL file, and we're done.
348- logger.Debugf("finishing deploy of charm %q", d.staged.url)
349 return d.finishDeploy()
350 }
351
352@@ -145,6 +144,7 @@
353
354 // finishDeploy persists the fact that we've finished deploying the staged bundle.
355 func (d *manifestDeployer) finishDeploy() error {
356+ logger.Debugf("finishing deploy of charm %q", d.staged.url)
357 oldPath := d.CharmPath(deployingURLPath)
358 newPath := d.CharmPath(charmURLPath)
359 return utils.ReplaceFile(oldPath, newPath)
360
361=== modified file 'worker/uniter/charm/manifest_deployer_test.go'
362--- worker/uniter/charm/manifest_deployer_test.go 2014-04-11 06:39:27 +0000
363+++ worker/uniter/charm/manifest_deployer_test.go 2014-04-21 10:30:06 +0000
364@@ -9,7 +9,6 @@
365
366 gc "launchpad.net/gocheck"
367
368- corecharm "launchpad.net/juju-core/charm"
369 ft "launchpad.net/juju-core/testing/filetesting"
370 "launchpad.net/juju-core/testing/testbase"
371 "launchpad.net/juju-core/utils/set"
372@@ -37,17 +36,12 @@
373 s.deployer = charm.NewManifestDeployer(s.targetPath, deployerPath, s.bundles)
374 }
375
376-func (s *ManifestDeployerSuite) charmURL(revision int) *corecharm.URL {
377- baseURL := corecharm.MustParseURL("cs:s/c")
378- return baseURL.WithRevision(revision)
379-}
380-
381 func (s *ManifestDeployerSuite) addMockCharm(c *gc.C, revision int, bundle charm.Bundle) charm.BundleInfo {
382- return s.bundles.AddBundle(c, s.charmURL(revision), bundle)
383+ return s.bundles.AddBundle(c, charmURL(revision), bundle)
384 }
385
386 func (s *ManifestDeployerSuite) addCharm(c *gc.C, revision int, content ...ft.Entry) charm.BundleInfo {
387- return s.bundles.AddCustomBundle(c, s.charmURL(revision), func(path string) {
388+ return s.bundles.AddCustomBundle(c, charmURL(revision), func(path string) {
389 ft.Entries(content).Create(c, path)
390 })
391 }
392@@ -65,7 +59,7 @@
393 func (s *ManifestDeployerSuite) assertCharm(c *gc.C, revision int, content ...ft.Entry) {
394 url, err := charm.ReadCharmURL(filepath.Join(s.targetPath, ".juju-charm"))
395 c.Assert(err, gc.IsNil)
396- c.Assert(url, gc.DeepEquals, s.charmURL(revision))
397+ c.Assert(url, gc.DeepEquals, charmURL(revision))
398 ft.Entries(content).Check(c, s.targetPath)
399 }
400
401
402=== modified file 'worker/uniter/modes.go'
403--- worker/uniter/modes.go 2014-04-12 05:53:58 +0000
404+++ worker/uniter/modes.go 2014-04-21 10:30:06 +0000
405@@ -203,6 +203,9 @@
406 if u.s.Op != Continue {
407 return nil, fmt.Errorf("insane uniter state: %#v", u.s)
408 }
409+ if err := u.fixDeployer(); err != nil {
410+ return nil, err
411+ }
412 if err = u.unit.SetStatus(params.StatusStarted, "", nil); err != nil {
413 return nil, err
414 }
415@@ -296,7 +299,7 @@
416
417 // ModeHookError is responsible for watching and responding to:
418 // * user resolution of hook errors
419-// * charm upgrade requests
420+// * forced charm upgrade requests
421 func ModeHookError(u *Uniter) (next Mode, err error) {
422 defer modeContext("ModeHookError", &err)()
423 if u.s.Op != RunHook || u.s.OpStep != Pending {
424@@ -359,6 +362,15 @@
425 select {
426 case <-u.tomb.Dying():
427 return nil, tomb.ErrDying
428+ case curl = <-u.f.UpgradeEvents():
429+ if err := u.deployer.NotifyRevert(); err != nil {
430+ return nil, err
431+ }
432+ // Now the git dir (if it is one) has been reverted, it's safe to
433+ // use a manifest deployer to deploy the new charm.
434+ if err := u.fixDeployer(); err != nil {
435+ return nil, err
436+ }
437 case <-u.f.ResolvedEvents():
438 err = u.deployer.NotifyResolved()
439 if e := u.f.ClearResolved(); e != nil {
440@@ -367,10 +379,12 @@
441 if err != nil {
442 return nil, err
443 }
444- case curl = <-u.f.UpgradeEvents():
445- if err := u.deployer.NotifyRevert(); err != nil {
446- return nil, err
447- }
448+ // We don't fixDeployer at this stage, because we have *no idea*
449+ // what (if anything) the user has done to the charm dir before
450+ // setting resolved. But the balance of probability is that the
451+ // dir is filled with git droppings, that will be considered user
452+ // files and hang around forever, so in this case we wait for the
453+ // upgrade to complete and fixDeployer in ModeAbide.
454 }
455 return ModeUpgrading(curl), nil
456 }
457
458=== modified file 'worker/uniter/uniter.go'
459--- worker/uniter/uniter.go 2014-04-17 12:53:23 +0000
460+++ worker/uniter/uniter.go 2014-04-21 10:30:06 +0000
461@@ -71,7 +71,7 @@
462 baseDir string
463 toolsDir string
464 relationsDir string
465- charm *charm.GitDir
466+ charmPath string
467 deployer charm.Deployer
468 s *State
469 sf *StateFile
470@@ -202,10 +202,13 @@
471
472 u.relationers = map[int]*Relationer{}
473 u.relationHooks = make(chan hook.Info)
474- u.charm = charm.NewGitDir(filepath.Join(u.baseDir, "charm"))
475+ u.charmPath = filepath.Join(u.baseDir, "charm")
476 deployerPath := filepath.Join(u.baseDir, "state", "deployer")
477 bundles := charm.NewBundlesDir(filepath.Join(u.baseDir, "state", "bundles"))
478- u.deployer = charm.NewGitDeployer(u.charm.Path(), deployerPath, bundles)
479+ u.deployer, err = charm.NewDeployer(u.charmPath, deployerPath, bundles)
480+ if err != nil {
481+ return fmt.Errorf("cannot create deployer: %v", err)
482+ }
483 u.sf = NewStateFile(filepath.Join(u.baseDir, "state", "uniter"))
484 u.rand = rand.New(rand.NewSource(time.Now().Unix()))
485
486@@ -410,7 +413,7 @@
487 }
488 defer srv.Close()
489
490- result, err := hctx.RunCommands(commands, u.charm.Path(), u.toolsDir, socketPath)
491+ result, err := hctx.RunCommands(commands, u.charmPath, u.toolsDir, socketPath)
492 if result != nil {
493 logger.Tracef("run commands: rc=%v\nstdout:\n%sstderr:\n%s", result.Code, result.Stdout, result.Stderr)
494 }
495@@ -480,7 +483,7 @@
496 }
497 logger.Infof("running %q hook", hookName)
498 ranHook := true
499- err = hctx.RunHook(hookName, u.charm.Path(), u.toolsDir, socketPath)
500+ err = hctx.RunHook(hookName, u.charmPath, u.toolsDir, socketPath)
501 if IsMissingHookError(err) {
502 ranHook = false
503 } else if err != nil {
504@@ -634,8 +637,8 @@
505 if rel.Life() != params.Alive {
506 continue
507 }
508- // Make sure we ignore relations not implemented by the unit's charm
509- ch, err := corecharm.ReadDir(u.charm.Path())
510+ // Make sure we ignore relations not implemented by the unit's charm.
511+ ch, err := corecharm.ReadDir(u.charmPath)
512 if err != nil {
513 return nil, err
514 }
515@@ -721,6 +724,16 @@
516 }
517 }
518
519+// fixDeployer replaces the uniter's git-based charm deployer with a manifest-
520+// based one, if necessary. It should not be called unless the existing charm
521+// deployment is known to be in a stable state.
522+func (u *Uniter) fixDeployer() error {
523+ if err := charm.FixDeployer(&u.deployer); err != nil {
524+ return fmt.Errorf("cannot convert git deployment to manifest deployment: %v", err)
525+ }
526+ return nil
527+}
528+
529 // updatePackageProxy updates the package proxy settings from the
530 // environment.
531 func (u *Uniter) updatePackageProxy(cfg *config.Config) {
532
533=== added file 'worker/uniter/uniter_old_test.go'
534--- worker/uniter/uniter_old_test.go 1970-01-01 00:00:00 +0000
535+++ worker/uniter/uniter_old_test.go 2014-04-21 10:30:06 +0000
536@@ -0,0 +1,226 @@
537+// Copyright 2012-2014 Canonical Ltd.
538+// Licensed under the AGPLv3, see LICENCE file for details.
539+
540+package uniter_test
541+
542+import (
543+ "io/ioutil"
544+ "os"
545+ "os/exec"
546+ "path/filepath"
547+ "strconv"
548+
549+ jc "github.com/juju/testing/checkers"
550+ gc "launchpad.net/gocheck"
551+
552+ "launchpad.net/juju-core/state"
553+ "launchpad.net/juju-core/state/api/params"
554+ ft "launchpad.net/juju-core/testing/filetesting"
555+)
556+
557+// These tests are copies of the old git-deployer-related tests, to test that
558+// the uniter with the manifest-deployer work patched out still works how it
559+// used to; thus demonstrating that the *other* tests that verify manifest
560+// deployer behaviour in the presence of an old git deployer are working against
561+// an accurate representation of the base state.
562+// The only actual behaviour change is that we no longer commit changes after
563+// each hook execution; this is reflected by checking that it's dirty in a couple
564+// of places where we once checked it was not.
565+
566+var upgradeGitConflictsTests = []uniterTest{
567+ // Upgrade scenarios - handling conflicts.
568+ ut(
569+ "upgrade: conflicting files",
570+ startGitUpgradeError{},
571+
572+ // NOTE: this is just dumbly committing the conflicts, but AFAICT this
573+ // is the only reasonable solution; if the user tells us it's resolved
574+ // we have to take their word for it.
575+ resolveError{state.ResolvedNoHooks},
576+ waitHooks{"upgrade-charm", "config-changed"},
577+ waitUnit{
578+ status: params.StatusStarted,
579+ charm: 1,
580+ },
581+ verifyGitCharm{revision: 1},
582+ ), ut(
583+ `upgrade: conflicting directories`,
584+ createCharm{
585+ customize: func(c *gc.C, ctx *context, path string) {
586+ err := os.Mkdir(filepath.Join(path, "data"), 0755)
587+ c.Assert(err, gc.IsNil)
588+ appendHook(c, path, "start", "echo DATA > data/newfile")
589+ },
590+ },
591+ serveCharm{},
592+ createUniter{},
593+ waitUnit{
594+ status: params.StatusStarted,
595+ },
596+ waitHooks{"install", "config-changed", "start"},
597+ verifyGitCharm{dirty: true},
598+
599+ createCharm{
600+ revision: 1,
601+ customize: func(c *gc.C, ctx *context, path string) {
602+ data := filepath.Join(path, "data")
603+ err := ioutil.WriteFile(data, []byte("<nelson>ha ha</nelson>"), 0644)
604+ c.Assert(err, gc.IsNil)
605+ },
606+ },
607+ serveCharm{},
608+ upgradeCharm{revision: 1},
609+ waitUnit{
610+ status: params.StatusError,
611+ info: "upgrade failed",
612+ charm: 1,
613+ },
614+ verifyWaiting{},
615+ verifyGitCharm{dirty: true},
616+
617+ resolveError{state.ResolvedNoHooks},
618+ waitHooks{"upgrade-charm", "config-changed"},
619+ waitUnit{
620+ status: params.StatusStarted,
621+ charm: 1,
622+ },
623+ verifyGitCharm{revision: 1},
624+ ), ut(
625+ "upgrade conflict resolved with forced upgrade",
626+ startGitUpgradeError{},
627+ createCharm{
628+ revision: 2,
629+ customize: func(c *gc.C, ctx *context, path string) {
630+ otherdata := filepath.Join(path, "otherdata")
631+ err := ioutil.WriteFile(otherdata, []byte("blah"), 0644)
632+ c.Assert(err, gc.IsNil)
633+ },
634+ },
635+ serveCharm{},
636+ upgradeCharm{revision: 2, forced: true},
637+ waitUnit{
638+ status: params.StatusStarted,
639+ charm: 2,
640+ },
641+ waitHooks{"upgrade-charm", "config-changed"},
642+ verifyGitCharm{revision: 2},
643+ custom{func(c *gc.C, ctx *context) {
644+ // otherdata should exist (in v2)
645+ otherdata, err := ioutil.ReadFile(filepath.Join(ctx.path, "charm", "otherdata"))
646+ c.Assert(err, gc.IsNil)
647+ c.Assert(string(otherdata), gc.Equals, "blah")
648+
649+ // ignore should not (only in v1)
650+ _, err = os.Stat(filepath.Join(ctx.path, "charm", "ignore"))
651+ c.Assert(err, jc.Satisfies, os.IsNotExist)
652+
653+ // data should contain what was written in the start hook
654+ data, err := ioutil.ReadFile(filepath.Join(ctx.path, "charm", "data"))
655+ c.Assert(err, gc.IsNil)
656+ c.Assert(string(data), gc.Equals, "STARTDATA\n")
657+ }},
658+ ), ut(
659+ "upgrade conflict service dying",
660+ startGitUpgradeError{},
661+ serviceDying,
662+ verifyWaiting{},
663+ resolveError{state.ResolvedNoHooks},
664+ waitHooks{"upgrade-charm", "config-changed", "stop"},
665+ waitUniterDead{},
666+ ), ut(
667+ "upgrade conflict unit dying",
668+ startGitUpgradeError{},
669+ unitDying,
670+ verifyWaiting{},
671+ resolveError{state.ResolvedNoHooks},
672+ waitHooks{"upgrade-charm", "config-changed", "stop"},
673+ waitUniterDead{},
674+ ), ut(
675+ "upgrade conflict unit dead",
676+ startGitUpgradeError{},
677+ unitDead,
678+ waitUniterDead{},
679+ waitHooks{},
680+ ),
681+}
682+
683+func (s *UniterSuite) TestUniterUpgradeGitConflicts(c *gc.C) {
684+ patchedTests := make([]uniterTest, len(upgradeGitConflictsTests))
685+ for i, test := range upgradeGitConflictsTests {
686+ patchedTests[i] = ut(test.summary, prepareGitUniter{test.steps})
687+ }
688+ s.runUniterTests(c, patchedTests)
689+}
690+
691+type verifyGitCharm struct {
692+ revision int
693+ dirty bool
694+}
695+
696+func (s verifyGitCharm) step(c *gc.C, ctx *context) {
697+ charmPath := filepath.Join(ctx.path, "charm")
698+ if !s.dirty {
699+ revisionPath := filepath.Join(charmPath, "revision")
700+ content, err := ioutil.ReadFile(revisionPath)
701+ c.Assert(err, gc.IsNil)
702+ c.Assert(string(content), gc.Equals, strconv.Itoa(s.revision))
703+ err = ctx.unit.Refresh()
704+ c.Assert(err, gc.IsNil)
705+ url, ok := ctx.unit.CharmURL()
706+ c.Assert(ok, gc.Equals, true)
707+ c.Assert(url, gc.DeepEquals, curl(s.revision))
708+ }
709+
710+ // Before we try to check the git status, make sure expected hooks are all
711+ // complete, to prevent the test and the uniter interfering with each other.
712+ step(c, ctx, waitHooks{})
713+ step(c, ctx, waitHooks{})
714+ cmd := exec.Command("git", "status")
715+ cmd.Dir = filepath.Join(ctx.path, "charm")
716+ out, err := cmd.CombinedOutput()
717+ c.Assert(err, gc.IsNil)
718+ cmp := gc.Matches
719+ if s.dirty {
720+ cmp = gc.Not(gc.Matches)
721+ }
722+ c.Assert(string(out), cmp, "(# )?On branch master\nnothing to commit.*\n")
723+}
724+
725+type startGitUpgradeError struct{}
726+
727+func (s startGitUpgradeError) step(c *gc.C, ctx *context) {
728+ steps := []stepper{
729+ createCharm{
730+ customize: func(c *gc.C, ctx *context, path string) {
731+ appendHook(c, path, "start", "echo STARTDATA > data")
732+ },
733+ },
734+ serveCharm{},
735+ createUniter{},
736+ waitUnit{
737+ status: params.StatusStarted,
738+ },
739+ waitHooks{"install", "config-changed", "start"},
740+ verifyGitCharm{dirty: true},
741+
742+ createCharm{
743+ revision: 1,
744+ customize: func(c *gc.C, ctx *context, path string) {
745+ ft.File{"data", "<nelson>ha ha</nelson>", 0644}.Create(c, path)
746+ ft.File{"ignore", "anything", 0644}.Create(c, path)
747+ },
748+ },
749+ serveCharm{},
750+ upgradeCharm{revision: 1},
751+ waitUnit{
752+ status: params.StatusError,
753+ info: "upgrade failed",
754+ charm: 1,
755+ },
756+ verifyWaiting{},
757+ verifyGitCharm{dirty: true},
758+ }
759+ for _, s_ := range steps {
760+ step(c, ctx, s_)
761+ }
762+}
763
764=== modified file 'worker/uniter/uniter_test.go'
765--- worker/uniter/uniter_test.go 2014-04-14 12:36:13 +0000
766+++ worker/uniter/uniter_test.go 2014-04-21 10:30:06 +0000
767@@ -17,12 +17,13 @@
768 stdtesting "testing"
769 "time"
770
771+ gt "github.com/juju/testing"
772 jc "github.com/juju/testing/checkers"
773 gc "launchpad.net/gocheck"
774 "launchpad.net/goyaml"
775
776 "launchpad.net/juju-core/agent/tools"
777- "launchpad.net/juju-core/charm"
778+ corecharm "launchpad.net/juju-core/charm"
779 "launchpad.net/juju-core/errors"
780 "launchpad.net/juju-core/instance"
781 "launchpad.net/juju-core/juju/osenv"
782@@ -38,6 +39,7 @@
783 "launchpad.net/juju-core/utils/fslock"
784 "launchpad.net/juju-core/worker"
785 "launchpad.net/juju-core/worker/uniter"
786+ "launchpad.net/juju-core/worker/uniter/charm"
787 )
788
789 // worstCase is used for timeouts when timing out
790@@ -248,7 +250,7 @@
791 serveCharm{},
792 writeFile{"charm", 0644},
793 createUniter{},
794- waitUniterDead{`ModeInstalling cs:quantal/wordpress-0: charm deployment failed: ".*charm" is not a directory`},
795+ waitUniterDead{`ModeInstalling cs:quantal/wordpress-0: open .*: not a directory`},
796 ), ut(
797 "charm cannot be downloaded",
798 createCharm{},
799@@ -629,8 +631,7 @@
800 },
801 waitHooks{"upgrade-charm", "config-changed"},
802 verifyRunning{},
803- ),
804- ut(
805+ ), ut(
806 // This test does an add-relation as quickly as possible
807 // after an upgrade-charm, in the hope that the scheduler will
808 // deliver the events in the wrong order. The observed
809@@ -658,6 +659,89 @@
810 s.runUniterTests(c, steadyUpgradeTests)
811 }
812
813+func (s *UniterSuite) TestUniterUpgradeOverwrite(c *gc.C) {
814+ makeTest := func(description string, content, extraChecks ft.Entries) uniterTest {
815+ return ut(description,
816+ createCharm{
817+ // This is the base charm which all upgrade tests start out running.
818+ customize: func(c *gc.C, ctx *context, path string) {
819+ ft.Entries{
820+ ft.Dir{"dir", 0755},
821+ ft.File{"file", "blah", 0644},
822+ ft.Symlink{"symlink", "file"},
823+ }.Create(c, path)
824+ // Note that it creates "dir/user-file" at runtime, which may be
825+ // preserved or removed depending on the test.
826+ script := "echo content > dir/user-file && chmod 755 dir/user-file"
827+ appendHook(c, path, "start", script)
828+ },
829+ },
830+ serveCharm{},
831+ createUniter{},
832+ waitUnit{
833+ status: params.StatusStarted,
834+ },
835+ waitHooks{"install", "config-changed", "start"},
836+
837+ createCharm{
838+ revision: 1,
839+ customize: func(c *gc.C, _ *context, path string) {
840+ content.Create(c, path)
841+ },
842+ },
843+ serveCharm{},
844+ upgradeCharm{revision: 1},
845+ waitUnit{
846+ status: params.StatusStarted,
847+ charm: 1,
848+ },
849+ waitHooks{"upgrade-charm", "config-changed"},
850+ verifyCharm{revision: 1},
851+ custom{func(c *gc.C, ctx *context) {
852+ path := filepath.Join(ctx.path, "charm")
853+ content.Check(c, path)
854+ extraChecks.Check(c, path)
855+ }},
856+ verifyRunning{},
857+ )
858+ }
859+
860+ s.runUniterTests(c, []uniterTest{
861+ makeTest(
862+ "files overwite files, dirs, symlinks",
863+ ft.Entries{
864+ ft.File{"file", "new", 0755},
865+ ft.File{"dir", "new", 0755},
866+ ft.File{"symlink", "new", 0755},
867+ },
868+ ft.Entries{
869+ ft.Removed{"dir/user-file"},
870+ },
871+ ), makeTest(
872+ "symlinks overwite files, dirs, symlinks",
873+ ft.Entries{
874+ ft.Symlink{"file", "new"},
875+ ft.Symlink{"dir", "new"},
876+ ft.Symlink{"symlink", "new"},
877+ },
878+ ft.Entries{
879+ ft.Removed{"dir/user-file"},
880+ },
881+ ), makeTest(
882+ "dirs overwite files, symlinks; merge dirs",
883+ ft.Entries{
884+ ft.Dir{"file", 0755},
885+ ft.Dir{"dir", 0755},
886+ ft.File{"dir/charm-file", "charm-content", 0644},
887+ ft.Dir{"symlink", 0755},
888+ },
889+ ft.Entries{
890+ ft.File{"dir/user-file", "content\n", 0755},
891+ },
892+ ),
893+ })
894+}
895+
896 var errorUpgradeTests = []uniterTest{
897 // Upgrade scenarios from error state.
898 ut(
899@@ -723,103 +807,160 @@
900 s.runUniterTests(c, errorUpgradeTests)
901 }
902
903+func (s *UniterSuite) TestUniterDeployerConversion(c *gc.C) {
904+ deployerConversionTests := []uniterTest{
905+ ut(
906+ "install normally, check not using git",
907+ quickStart{},
908+ verifyCharm{
909+ checkFiles: ft.Entries{ft.Removed{".git"}},
910+ },
911+ ), ut(
912+ "install with git, restart in steady state",
913+ prepareGitUniter{[]stepper{
914+ quickStart{},
915+ verifyGitCharm{},
916+ stopUniter{},
917+ }},
918+ startUniter{},
919+ waitHooks{"config-changed"},
920+
921+ // At this point, the deployer has been converted, but the
922+ // charm directory itself hasn't; the *next* deployment will
923+ // actually hit the charm directory and strip out the git
924+ // stuff.
925+ createCharm{revision: 1},
926+ upgradeCharm{revision: 1},
927+ waitHooks{"upgrade-charm", "config-changed"},
928+ waitUnit{
929+ status: params.StatusStarted,
930+ charm: 1,
931+ },
932+ verifyCharm{
933+ revision: 1,
934+ checkFiles: ft.Entries{ft.Removed{".git"}},
935+ },
936+ verifyRunning{},
937+ ), ut(
938+ "install with git, get conflicted, mark resolved",
939+ prepareGitUniter{[]stepper{
940+ startGitUpgradeError{},
941+ stopUniter{},
942+ }},
943+ startUniter{},
944+
945+ resolveError{state.ResolvedNoHooks},
946+ waitHooks{"upgrade-charm", "config-changed"},
947+ waitUnit{
948+ status: params.StatusStarted,
949+ charm: 1,
950+ },
951+ verifyCharm{revision: 1},
952+ verifyRunning{},
953+
954+ // Due to the uncertainties around marking upgrade conflicts resolved,
955+ // the charm directory again remains unconverted, although the deployer
956+ // should have been fixed. Again, we check this by running another
957+ // upgrade and verifying the .git dir is then removed.
958+ createCharm{revision: 2},
959+ upgradeCharm{revision: 2},
960+ waitHooks{"upgrade-charm", "config-changed"},
961+ waitUnit{
962+ status: params.StatusStarted,
963+ charm: 2,
964+ },
965+ verifyCharm{
966+ revision: 2,
967+ checkFiles: ft.Entries{ft.Removed{".git"}},
968+ },
969+ verifyRunning{},
970+ ), ut(
971+ "install with git, get conflicted, force an upgrade",
972+ prepareGitUniter{[]stepper{
973+ startGitUpgradeError{},
974+ stopUniter{},
975+ }},
976+ startUniter{},
977+
978+ createCharm{
979+ revision: 2,
980+ customize: func(c *gc.C, ctx *context, path string) {
981+ ft.File{"data", "OVERWRITE!", 0644}.Create(c, path)
982+ },
983+ },
984+ serveCharm{},
985+ upgradeCharm{revision: 2, forced: true},
986+ waitHooks{"upgrade-charm", "config-changed"},
987+ waitUnit{
988+ status: params.StatusStarted,
989+ charm: 2,
990+ },
991+
992+ // A forced upgrade allows us to swap out the git deployer *and*
993+ // the .git dir inside the charm immediately; check we did so.
994+ verifyCharm{
995+ revision: 2,
996+ checkFiles: ft.Entries{
997+ ft.Removed{".git"},
998+ ft.File{"data", "OVERWRITE!", 0644},
999+ },
1000+ },
1001+ verifyRunning{},
1002+ ),
1003+ }
1004+ s.runUniterTests(c, deployerConversionTests)
1005+}
1006+
1007 var upgradeConflictsTests = []uniterTest{
1008 // Upgrade scenarios - handling conflicts.
1009 ut(
1010- "upgrade: conflicting files",
1011- startUpgradeError{},
1012-
1013- // NOTE: this is just dumbly committing the conflicts, but AFAICT this
1014- // is the only reasonable solution; if the user tells us it's resolved
1015- // we have to take their word for it.
1016- resolveError{state.ResolvedNoHooks},
1017- waitHooks{"upgrade-charm", "config-changed"},
1018- waitUnit{
1019- status: params.StatusStarted,
1020- charm: 1,
1021- },
1022- verifyCharm{revision: 1},
1023- ), ut(
1024- `upgrade: conflicting directories`,
1025- createCharm{
1026- customize: func(c *gc.C, ctx *context, path string) {
1027- err := os.Mkdir(filepath.Join(path, "data"), 0755)
1028- c.Assert(err, gc.IsNil)
1029- appendHook(c, path, "start", "echo DATA > data/newfile")
1030- },
1031- },
1032- serveCharm{},
1033- createUniter{},
1034- waitUnit{
1035- status: params.StatusStarted,
1036- },
1037- waitHooks{"install", "config-changed", "start"},
1038- verifyCharm{dirty: true},
1039-
1040- createCharm{
1041- revision: 1,
1042- customize: func(c *gc.C, ctx *context, path string) {
1043- data := filepath.Join(path, "data")
1044- err := ioutil.WriteFile(data, []byte("<nelson>ha ha</nelson>"), 0644)
1045- c.Assert(err, gc.IsNil)
1046- },
1047- },
1048- serveCharm{},
1049- upgradeCharm{revision: 1},
1050- waitUnit{
1051- status: params.StatusError,
1052- info: "upgrade failed",
1053- charm: 1,
1054- },
1055- verifyWaiting{},
1056- verifyCharm{dirty: true},
1057-
1058- resolveError{state.ResolvedNoHooks},
1059- waitHooks{"upgrade-charm", "config-changed"},
1060- waitUnit{
1061- status: params.StatusStarted,
1062- charm: 1,
1063- },
1064- verifyCharm{revision: 1},
1065- ), ut(
1066- "upgrade conflict resolved with forced upgrade",
1067- startUpgradeError{},
1068- createCharm{
1069- revision: 2,
1070- customize: func(c *gc.C, ctx *context, path string) {
1071- otherdata := filepath.Join(path, "otherdata")
1072- err := ioutil.WriteFile(otherdata, []byte("blah"), 0644)
1073- c.Assert(err, gc.IsNil)
1074- },
1075- },
1076+ "upgrade: resolving doesn't help until underlying problem is fixed",
1077+ startUpgradeError{},
1078+ resolveError{state.ResolvedNoHooks},
1079+ waitUnit{
1080+ status: params.StatusError,
1081+ info: "upgrade failed",
1082+ charm: 1,
1083+ },
1084+ verifyWaiting{},
1085+ verifyCharm{attemptedRevision: 1},
1086+
1087+ fixUpgradeError{},
1088+ resolveError{state.ResolvedNoHooks},
1089+ waitHooks{"upgrade-charm", "config-changed"},
1090+ waitUnit{
1091+ status: params.StatusStarted,
1092+ charm: 1,
1093+ },
1094+ verifyCharm{revision: 1},
1095+ ), ut(
1096+ `upgrade: forced upgrade does work without explicit resolution if underlying problem was fixed`,
1097+ startUpgradeError{},
1098+ resolveError{state.ResolvedNoHooks},
1099+ waitUnit{
1100+ status: params.StatusError,
1101+ info: "upgrade failed",
1102+ charm: 1,
1103+ },
1104+ verifyWaiting{},
1105+ verifyCharm{attemptedRevision: 1},
1106+
1107+ fixUpgradeError{},
1108+ createCharm{revision: 2},
1109 serveCharm{},
1110 upgradeCharm{revision: 2, forced: true},
1111+ waitHooks{"upgrade-charm", "config-changed"},
1112 waitUnit{
1113 status: params.StatusStarted,
1114 charm: 2,
1115 },
1116- waitHooks{"upgrade-charm", "config-changed"},
1117 verifyCharm{revision: 2},
1118- custom{func(c *gc.C, ctx *context) {
1119- // otherdata should exist (in v2)
1120- otherdata, err := ioutil.ReadFile(filepath.Join(ctx.path, "charm", "otherdata"))
1121- c.Assert(err, gc.IsNil)
1122- c.Assert(string(otherdata), gc.Equals, "blah")
1123-
1124- // ignore should not (only in v1)
1125- _, err = os.Stat(filepath.Join(ctx.path, "charm", "ignore"))
1126- c.Assert(err, jc.Satisfies, os.IsNotExist)
1127-
1128- // data should contain what was written in the start hook
1129- data, err := ioutil.ReadFile(filepath.Join(ctx.path, "charm", "data"))
1130- c.Assert(err, gc.IsNil)
1131- c.Assert(string(data), gc.Equals, "STARTDATA\n")
1132- }},
1133 ), ut(
1134 "upgrade conflict service dying",
1135 startUpgradeError{},
1136 serviceDying,
1137 verifyWaiting{},
1138+ fixUpgradeError{},
1139 resolveError{state.ResolvedNoHooks},
1140 waitHooks{"upgrade-charm", "config-changed", "stop"},
1141 waitUniterDead{},
1142@@ -828,6 +969,7 @@
1143 startUpgradeError{},
1144 unitDying,
1145 verifyWaiting{},
1146+ fixUpgradeError{},
1147 resolveError{state.ResolvedNoHooks},
1148 waitHooks{"upgrade-charm", "config-changed", "stop"},
1149 waitUniterDead{},
1150@@ -837,6 +979,7 @@
1151 unitDead,
1152 waitUniterDead{},
1153 waitHooks{},
1154+ fixUpgradeError{},
1155 ),
1156 }
1157
1158@@ -1174,7 +1317,7 @@
1159
1160 // Create the subordinate service.
1161 dir := coretesting.Charms.ClonedDir(c.MkDir(), "logging")
1162- curl, err := charm.ParseURL("cs:quantal/logging")
1163+ curl, err := corecharm.ParseURL("cs:quantal/logging")
1164 c.Assert(err, gc.IsNil)
1165 curl = curl.WithRevision(dir.Revision())
1166 step(c, ctx, addCharm{dir, curl})
1167@@ -1217,8 +1360,7 @@
1168 s.step(c, ctx)
1169 }
1170
1171-type ensureStateWorker struct {
1172-}
1173+type ensureStateWorker struct{}
1174
1175 func (s ensureStateWorker) step(c *gc.C, ctx *context) {
1176 addresses, err := ctx.st.Addresses()
1177@@ -1257,7 +1399,7 @@
1178 if s.customize != nil {
1179 s.customize(c, ctx, base)
1180 }
1181- dir, err := charm.ReadDir(base)
1182+ dir, err := corecharm.ReadDir(base)
1183 c.Assert(err, gc.IsNil)
1184 err = dir.SetDiskRevision(s.revision)
1185 c.Assert(err, gc.IsNil)
1186@@ -1265,8 +1407,8 @@
1187 }
1188
1189 type addCharm struct {
1190- dir *charm.Dir
1191- curl *charm.URL
1192+ dir *corecharm.Dir
1193+ curl *corecharm.URL
1194 }
1195
1196 func (s addCharm) step(c *gc.C, ctx *context) {
1197@@ -1628,7 +1770,7 @@
1198 type changeConfig map[string]interface{}
1199
1200 func (s changeConfig) step(c *gc.C, ctx *context) {
1201- err := ctx.svc.UpdateConfigSettings(charm.Settings(s))
1202+ err := ctx.svc.UpdateConfigSettings(corecharm.Settings(s))
1203 c.Assert(err, gc.IsNil)
1204 }
1205
1206@@ -1646,35 +1788,26 @@
1207 }
1208
1209 type verifyCharm struct {
1210- revision int
1211- dirty bool
1212+ revision int
1213+ attemptedRevision int
1214+ checkFiles ft.Entries
1215 }
1216
1217 func (s verifyCharm) step(c *gc.C, ctx *context) {
1218- if !s.dirty {
1219- path := filepath.Join(ctx.path, "charm", "revision")
1220- content, err := ioutil.ReadFile(path)
1221- c.Assert(err, gc.IsNil)
1222- c.Assert(string(content), gc.Equals, strconv.Itoa(s.revision))
1223- err = ctx.unit.Refresh()
1224- c.Assert(err, gc.IsNil)
1225- url, ok := ctx.unit.CharmURL()
1226- c.Assert(ok, gc.Equals, true)
1227- c.Assert(url, gc.DeepEquals, curl(s.revision))
1228- }
1229-
1230- // Before we try to check the git status, make sure expected hooks are all
1231- // complete, to prevent the test and the uniter interfering with each other.
1232- step(c, ctx, waitHooks{})
1233- cmd := exec.Command("git", "status")
1234- cmd.Dir = filepath.Join(ctx.path, "charm")
1235- out, err := cmd.CombinedOutput()
1236- c.Assert(err, gc.IsNil)
1237- cmp := gc.Matches
1238- if s.dirty {
1239- cmp = gc.Not(gc.Matches)
1240- }
1241- c.Assert(string(out), cmp, "(# )?On branch master\nnothing to commit.*\n")
1242+ s.checkFiles.Check(c, filepath.Join(ctx.path, "charm"))
1243+ path := filepath.Join(ctx.path, "charm", "revision")
1244+ content, err := ioutil.ReadFile(path)
1245+ c.Assert(err, gc.IsNil)
1246+ c.Assert(string(content), gc.Equals, strconv.Itoa(s.revision))
1247+ checkRevision := s.revision
1248+ if s.attemptedRevision > checkRevision {
1249+ checkRevision = s.attemptedRevision
1250+ }
1251+ err = ctx.unit.Refresh()
1252+ c.Assert(err, gc.IsNil)
1253+ url, ok := ctx.unit.CharmURL()
1254+ c.Assert(ok, gc.Equals, true)
1255+ c.Assert(url, gc.DeepEquals, curl(checkRevision))
1256 }
1257
1258 type startUpgradeError struct{}
1259@@ -1683,7 +1816,7 @@
1260 steps := []stepper{
1261 createCharm{
1262 customize: func(c *gc.C, ctx *context, path string) {
1263- appendHook(c, path, "start", "echo STARTDATA > data")
1264+ appendHook(c, path, "start", "chmod 555 $CHARM_DIR")
1265 },
1266 },
1267 serveCharm{},
1268@@ -1692,19 +1825,9 @@
1269 status: params.StatusStarted,
1270 },
1271 waitHooks{"install", "config-changed", "start"},
1272- verifyCharm{dirty: true},
1273+ verifyCharm{},
1274
1275- createCharm{
1276- revision: 1,
1277- customize: func(c *gc.C, ctx *context, path string) {
1278- data := filepath.Join(path, "data")
1279- err := ioutil.WriteFile(data, []byte("<nelson>ha ha</nelson>"), 0644)
1280- c.Assert(err, gc.IsNil)
1281- ignore := filepath.Join(path, "ignore")
1282- err = ioutil.WriteFile(ignore, []byte("anything"), 0644)
1283- c.Assert(err, gc.IsNil)
1284- },
1285- },
1286+ createCharm{revision: 1},
1287 serveCharm{},
1288 upgradeCharm{revision: 1},
1289 waitUnit{
1290@@ -1713,13 +1836,21 @@
1291 charm: 1,
1292 },
1293 verifyWaiting{},
1294- verifyCharm{dirty: true},
1295+ verifyCharm{attemptedRevision: 1},
1296 }
1297 for _, s_ := range steps {
1298 step(c, ctx, s_)
1299 }
1300 }
1301
1302+type fixUpgradeError struct{}
1303+
1304+func (s fixUpgradeError) step(c *gc.C, ctx *context) {
1305+ charmPath := filepath.Join(ctx.path, "charm")
1306+ err := os.Chmod(charmPath, 0755)
1307+ c.Assert(err, gc.IsNil)
1308+}
1309+
1310 type addRelation struct {
1311 waitJoin bool
1312 }
1313@@ -1964,8 +2095,8 @@
1314 c.Assert(ctx.subordinate.Destroy(), gc.IsNil)
1315 }}
1316
1317-func curl(revision int) *charm.URL {
1318- return charm.MustParseURL("cs:quantal/wordpress").WithRevision(revision)
1319+func curl(revision int) *corecharm.URL {
1320+ return corecharm.MustParseURL("cs:quantal/wordpress").WithRevision(revision)
1321 }
1322
1323 func appendHook(c *gc.C, charm, name, data string) {
1324@@ -1982,10 +2113,10 @@
1325 f, err := os.Open(path)
1326 c.Assert(err, gc.IsNil)
1327 defer f.Close()
1328- meta, err := charm.ReadMeta(f)
1329+ meta, err := corecharm.ReadMeta(f)
1330 c.Assert(err, gc.IsNil)
1331
1332- replace := func(what map[string]charm.Relation) bool {
1333+ replace := func(what map[string]corecharm.Relation) bool {
1334 for relName, relation := range what {
1335 if relName == oldName {
1336 what[newName] = relation
1337@@ -2005,7 +2136,7 @@
1338 f, err = os.Open(path)
1339 c.Assert(err, gc.IsNil)
1340 defer f.Close()
1341- meta, err = charm.ReadMeta(f)
1342+ meta, err = corecharm.ReadMeta(f)
1343 c.Assert(err, gc.IsNil)
1344 }
1345
1346@@ -2153,3 +2284,36 @@
1347 time.Sleep(coretesting.ShortWait)
1348 c.Assert(verify.filename, jc.DoesNotExist)
1349 }
1350+
1351+// prepareGitUniter runs a sequence of uniter tests with the manifest deployer
1352+// replacement logic patched out, simulating the effect of running an older
1353+// version of juju that exclusively used a git deployer. This is useful both
1354+// for testing the new deployer-replacement code *and* for running the old
1355+// tests against the new, patched code to check that the tweaks made to
1356+// accommodate the manifest deployer do not change the original behaviour as
1357+// simulated by the patched-out code.
1358+type prepareGitUniter struct {
1359+ prepSteps []stepper
1360+}
1361+
1362+func (s prepareGitUniter) step(c *gc.C, ctx *context) {
1363+ c.Assert(ctx.uniter, gc.IsNil, gc.Commentf("please don't try to patch stuff while the uniter's running"))
1364+ newDeployer := func(charmPath, dataPath string, bundles charm.BundleReader) (charm.Deployer, error) {
1365+ return charm.NewGitDeployer(charmPath, dataPath, bundles), nil
1366+ }
1367+ restoreNewDeployer := gt.PatchValue(&charm.NewDeployer, newDeployer)
1368+ defer restoreNewDeployer()
1369+
1370+ fixDeployer := func(deployer *charm.Deployer) error {
1371+ return nil
1372+ }
1373+ restoreFixDeployer := gt.PatchValue(&charm.FixDeployer, fixDeployer)
1374+ defer restoreFixDeployer()
1375+
1376+ for _, prepStep := range s.prepSteps {
1377+ step(c, ctx, prepStep)
1378+ }
1379+ if ctx.uniter != nil {
1380+ step(c, ctx, stopUniter{})
1381+ }
1382+}

Subscribers

People subscribed via source and target branches

to status/vote changes: