Merge lp:~fwereade/juju-core/manifest-charm-deployer into lp:~go-bot/juju-core/trunk
- manifest-charm-deployer
- Merge into trunk
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 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Juju Engineering | Pending | ||
Review via email:
|
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.
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.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
William Reade (fwereade) wrote : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
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:/
File worker/
https:/
worker/
0644}.Check(c, s.targetPath)
You don't seem to check what the status of "user" is here.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
William Reade (fwereade) wrote : | # |
Thanks :).
https:/
File worker/
https:/
worker/
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.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Go Bot (go-bot) wrote : | # |
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.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
? launchpad.
? launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
? launchpad.
? launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
? launchpad.
ok launchpad.
? launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
? launchpad.
ok launchpad.
? launchpad.
? launchpad.
? launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
? launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
? launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
? launchpad.
ok launchpad.
ok launchpad.
? launchpad.
ok launchpad.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Go Bot (go-bot) wrote : | # |
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.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
? launchpad.
? launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
? launchpad.
? launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
? launchpad.
ok launchpad.
? launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
? launchpad.
ok launchpad.
? launchpad.
? launchpad.
? launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
? launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
? launchpad.
ok launchpad.
? launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
? launchpad.
ok launchpad.
ok launchpad.
? launchpad.
Preview Diff
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 | +} |
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): uniter/ charm/charm_ test.go uniter/ charm/converter .go uniter/ charm/converter _test.go uniter/ charm/export_ test.go uniter/ charm/manifest_ deployer. go uniter/ charm/manifest_ deployer_ test.go uniter/ modes.go uniter/ uniter. go uniter/ uniter_ old_test. go uniter/ uniter_ test.go
A [revision details]
M worker/
A worker/
A worker/
M worker/
M worker/
M worker/
M worker/
M worker/
A worker/
M worker/