Merge lp:~hazmat/pyjuju/formula-upgrade-removes-obsolete into lp:pyjuju

Proposed by Kapil Thangavelu
Status: Work in progress
Proposed branch: lp:~hazmat/pyjuju/formula-upgrade-removes-obsolete
Merge into: lp:pyjuju
Diff against target: 295 lines (+141/-9)
9 files modified
ensemble/agents/tests/test_unit.py (+50/-4)
ensemble/agents/unit.py (+19/-3)
ensemble/formula/base.py (+7/-0)
ensemble/formula/bundle.py (+7/-0)
ensemble/formula/directory.py (+21/-1)
ensemble/formula/tests/test_base.py (+4/-0)
ensemble/formula/tests/test_bundle.py (+7/-0)
ensemble/formula/tests/test_directory.py (+17/-1)
ensemble/unit/tests/test_lifecycle.py (+9/-0)
To merge this branch: bzr merge lp:~hazmat/pyjuju/formula-upgrade-removes-obsolete
Reviewer Review Type Date Requested Status
Gustavo Niemeyer Needs Information
Review via email: mp+66940@code.launchpad.net

Description of the change

On upgrade old formula files and directories not present in the new formula are removed. Also adds a dirent manifest for formulas. In retrospect this might have been achievable more simply with just removing the old formula dir while hook execution is stopped, and extracting the new one.. i had already implemented most of this before i realized that though..

To post a comment you must log in.
Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

[1]

25 + stale_file_path = os.path.join(formula_dir, "old-file")
26 + fh = open(stale_file_path, "w")
27 + fh.write("hello world")
28 + fh.close()

That's not a stale file, but a file created dynamically. I'm a bit concerned
about killing arbitrary files that the formula created, as it's basically saying
that the formula directory is necessarily read-only. Is that what we want?

review: Needs Information
Revision history for this message
Kapil Thangavelu (hazmat) wrote :

I talked with gustavo about this, and we settled on an explicit manifest of install files to be used as a delta, to avoid killing any formula state files. alternatively we'd have to specify a state dir for formulas like 'var'.

Unmerged revisions

265. By Kapil Thangavelu

stale formula files are removed on formula upgrade.

264. By Kapil Thangavelu

dynamically computed manifests for formulas

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'ensemble/agents/tests/test_unit.py'
2--- ensemble/agents/tests/test_unit.py 2011-05-26 17:01:24 +0000
3+++ ensemble/agents/tests/test_unit.py 2011-07-05 17:39:27 +0000
4@@ -1,6 +1,7 @@
5 import argparse
6 import logging
7 import os
8+import stat
9 import yaml
10
11 from twisted.internet.defer import (
12@@ -501,16 +502,51 @@
13 yield self.agent.workflow.fire_transition("stop")
14
15 @inlineCallbacks
16+ def test_upgrade_removes_stale_files(self):
17+ """A formula upgrade removes any old formula files."""
18+ self.agent.set_watch_enabled(False)
19+ yield self.agent.startService()
20+ yield self.mark_formula_upgrade()
21+
22+ # Add a stale entries which should get removed.
23+ formula_dir = os.path.join(self.agent.unit_directory, "formula")
24+
25+ stale_file_path = os.path.join(formula_dir, "old-file")
26+ fh = open(stale_file_path, "w")
27+ fh.write("hello world")
28+ fh.close()
29+
30+ stale_hook_path = os.path.join(formula_dir, "hooks", "stop")
31+ fh = open(stale_hook_path, "w")
32+ fh.write("#!/bin/bash\necho done")
33+ fh.close()
34+
35+ stale_dir = os.path.join(formula_dir, "empty")
36+ os.mkdir(stale_dir)
37+
38+ stale = [stale_file_path, stale_hook_path, stale_dir]
39+
40+ # Perform the upgrade and verify
41+ hook_done = self.wait_on_hook(
42+ "upgrade-formula", executor=self.agent.executor)
43+
44+ upgrade = FormulaUpgradeOperation(self.agent)
45+ value = yield upgrade.run()
46+ self.assertTrue(value)
47+ yield hook_done
48+
49+ for stale_path in stale:
50+ self.assertFalse(os.path.exists(stale_path))
51+
52+ @inlineCallbacks
53 def test_agent_upgrade(self):
54 """The agent can succesfully upgrade its formula."""
55 self.agent.set_watch_enabled(False)
56 yield self.agent.startService()
57-
58 yield self.mark_formula_upgrade()
59
60 hook_done = self.wait_on_hook(
61 "upgrade-formula", executor=self.agent.executor)
62- self.write_hook("upgrade-formula", "#!/bin/bash\nexit 0")
63 output = self.capture_logging("unit.upgrade", level=logging.DEBUG)
64
65 # Do the upgrade
66@@ -589,8 +625,19 @@
67 self.agent.set_watch_enabled(False)
68 yield self.agent.startService()
69
70- # Upload a new version of the unit's formula
71+ # Upload a new version of the unit's formula, with a bad
72+ # upgrade hook.
73 repository = self.increment_formula(self.formula)
74+ formula_dir = repository.find("mysql").path
75+ os.mkdir(os.path.join(formula_dir, "hooks"))
76+ upgrade_hook_path = os.path.join(
77+ formula_dir, "hooks", "upgrade-formula")
78+
79+ fp = open(upgrade_hook_path, "w")
80+ fp.write("#!/bin/bash\nexit 1")
81+ fp.close()
82+ os.chmod(upgrade_hook_path, stat.S_IRWXU)
83+
84 formula, formula_state = yield self.publish_formula(
85 repository.find("mysql").path)
86
87@@ -600,7 +647,6 @@
88
89 hook_done = self.wait_on_hook(
90 "upgrade-formula", executor=self.agent.executor)
91- self.write_hook("upgrade-formula", "#!/bin/bash\nexit 1")
92 output = self.capture_logging("unit.upgrade", level=logging.DEBUG)
93
94 # Do the upgrade
95
96=== modified file 'ensemble/agents/unit.py'
97--- ensemble/agents/unit.py 2011-05-24 18:07:03 +0000
98+++ ensemble/agents/unit.py 2011-07-05 17:39:27 +0000
99@@ -6,6 +6,7 @@
100 from twisted.internet.defer import inlineCallbacks, returnValue
101
102 from ensemble.errors import EnsembleError
103+from ensemble.formula import get_formula_from_path
104 from ensemble.state.service import ServiceStateManager
105 from ensemble.hooks.protocol import UnitSettingsFactory
106 from ensemble.hooks.executor import HookExecutor
107@@ -195,6 +196,21 @@
108 shutil.rmtree(self._formula_directory)
109 return result
110
111+ def _remove_stale(self, new_formula, formula_dir):
112+ new_entries = set(new_formula.manifest())
113+ old_entries = set(formula_dir.manifest())
114+
115+ stale_entries = list(old_entries - new_entries)
116+ stale_entries.sort(lambda x, y: cmp(len(x), len(y)))
117+ stale_entries.reverse()
118+
119+ for entry in stale_entries:
120+ entry_path = os.path.join(formula_dir.path, entry)
121+ if entry_path.endswith("/"):
122+ os.rmdir(entry_path)
123+ else:
124+ os.remove(entry_path)
125+
126 def run(self):
127 d = self._run()
128 d.addBoth(self._remove_tree)
129@@ -254,13 +270,13 @@
130 yield self._agent.unit_state.set_formula_id(service_formula_id)
131
132 # Extract formula
133+ formula_dir = os.path.join(self._agent.unit_directory, "formula")
134 self._log.debug("Extracting new formula.")
135- formula.extract_to(
136- os.path.join(self._agent.unit_directory, "formula"))
137+ self._remove_stale(formula, get_formula_from_path(formula_dir))
138+ formula.extract_to(formula_dir)
139
140 # Upgrade
141 self._log.debug("Invoking upgrade transition.")
142-
143 success = yield self._agent.workflow.fire_transition(
144 "upgrade_formula")
145
146
147=== modified file 'ensemble/formula/base.py'
148--- ensemble/formula/base.py 2010-08-18 15:23:10 +0000
149+++ ensemble/formula/base.py 2011-07-05 17:39:27 +0000
150@@ -33,3 +33,10 @@
151 if self._sha256 is None:
152 self._sha256 = self.compute_sha256()
153 return self._sha256
154+
155+ def manifest(self):
156+ """Return a file and directory manifest as a list of entries.
157+
158+ The entries are relative to the formula.
159+ """
160+ self._unsupported("manifest()")
161
162=== modified file 'ensemble/formula/bundle.py'
163--- ensemble/formula/bundle.py 2011-06-05 20:19:18 +0000
164+++ ensemble/formula/bundle.py 2011-07-05 17:39:27 +0000
165@@ -64,3 +64,10 @@
166 path"""
167 dn = tempfile.mkdtemp(prefix="tmp")
168 return self.extract_to(dn)
169+
170+ def manifest(self):
171+ """Return a file and directory manifest as a list of entries.
172+
173+ The entries are relative to the formula.
174+ """
175+ return ZipFile(self.path, "r").namelist()
176
177=== modified file 'ensemble/formula/directory.py'
178--- ensemble/formula/directory.py 2011-06-09 07:10:07 +0000
179+++ ensemble/formula/directory.py 2011-07-05 17:39:27 +0000
180@@ -1,6 +1,7 @@
181+import itertools
182 import os
183+import tempfile
184 import zipfile
185-import tempfile
186
187 from ensemble.formula.config import ConfigOptions
188 from ensemble.formula.metadata import MetaData
189@@ -77,3 +78,22 @@
190 Compute sha256, based on the bundle.
191 """
192 return self.as_bundle().compute_sha256()
193+
194+ def manifest(self):
195+ """Return a file and directory manifest as a list of entries.
196+
197+ The entries are relative to the formula.
198+ """
199+ entries = []
200+ for dirpath, dirnames, filenames in os.walk(self.path):
201+ relative_path = dirpath[len(self.path):]
202+ dirnames = ["%s/" % d for d in dirnames]
203+ for dirent in itertools.chain(dirnames, filenames):
204+ entry = "/".join((relative_path, dirent))
205+ if entry.startswith("/"):
206+ entry = entry[1:]
207+ if self._ignore(entry):
208+ continue
209+ entries.append(entry)
210+
211+ return entries
212
213=== modified file 'ensemble/formula/tests/test_base.py'
214--- ensemble/formula/tests/test_base.py 2010-09-09 18:34:47 +0000
215+++ ensemble/formula/tests/test_base.py 2011-07-05 17:39:27 +0000
216@@ -28,6 +28,10 @@
217 self.assertUnsupported(lambda: self.formula.compute_sha256(),
218 "compute_sha256()")
219
220+ def test_manifest_is_unsupported(self):
221+ self.assertUnsupported(lambda: self.formula.manifest(),
222+ "manifest()")
223+
224 def test_get_sha256_fails_if_not_preset(self):
225 """
226 get_sha256() will internally call compute_sha256() if
227
228=== modified file 'ensemble/formula/tests/test_bundle.py'
229--- ensemble/formula/tests/test_bundle.py 2011-05-06 04:45:17 +0000
230+++ ensemble/formula/tests/test_bundle.py 2011-07-05 17:39:27 +0000
231@@ -131,3 +131,10 @@
232 fn = os.path.split(f2.path)[1]
233 # verify that it used the expected prefix
234 self.assertStartsWith(fn, "tmp")
235+
236+ def test_manifest(self):
237+ """A formula manifest contains all directories and files contained."""
238+ bundle = FormulaBundle(self.filename)
239+ self.assertEqual(bundle.manifest(),
240+ ["config.yaml", "metadata.yaml", "src/",
241+ "src/hello.c", "empty/"])
242
243=== modified file 'ensemble/formula/tests/test_directory.py'
244--- ensemble/formula/tests/test_directory.py 2011-05-06 04:45:17 +0000
245+++ ensemble/formula/tests/test_directory.py 2011-07-05 17:39:27 +0000
246@@ -56,7 +56,11 @@
247 included = [info.filename for info in zf.infolist()]
248 self.assertEqual(
249 set(included),
250- set(("metadata.yaml", "empty/", "src/", "src/hello.c", "config.yaml")))
251+ set(("metadata.yaml",
252+ "empty/",
253+ "src/",
254+ "src/hello.c",
255+ "config.yaml")))
256
257 def test_as_bundle(self):
258 directory = FormulaDirectory(self.sample_dir1)
259@@ -131,3 +135,15 @@
260 self.assertEquals(directory.config.get_serialization_data(),
261 sample_yaml_data)
262
263+ def test_manifest(self):
264+ """A formula manifest contains all directories and files contained."""
265+ directory = FormulaDirectory(self.sample_dir1)
266+ self.assertEqual(directory.manifest(), ["metadata.yaml"])
267+ directory = FormulaDirectory(sample_directory)
268+ self.assertEqual(
269+ set(directory.manifest()),
270+ set(("metadata.yaml",
271+ "empty/",
272+ "src/",
273+ "src/hello.c",
274+ "config.yaml")))
275
276=== modified file 'ensemble/unit/tests/test_lifecycle.py'
277--- ensemble/unit/tests/test_lifecycle.py 2011-05-26 11:20:43 +0000
278+++ ensemble/unit/tests/test_lifecycle.py 2011-07-05 17:39:27 +0000
279@@ -62,7 +62,16 @@
280 self.unit_directory = os.path.join(self.ensemble_directory,
281 "units",
282 self.states["unit"].unit_name.replace("/", "-"))
283+
284 os.makedirs(os.path.join(self.unit_directory, "formula", "hooks"))
285+ fp = open(os.path.join(
286+ self.unit_directory, "formula", "metadata.yaml"), "w")
287+ fp.write(yaml.dump(
288+ {"ensemble": "formula",
289+ "revision": 1,
290+ "name": "mysql",
291+ "description": "db",
292+ "summary": "db"}))
293 os.makedirs(os.path.join(self.ensemble_directory, "state"))
294
295 def write_hook(self, name, text, no_exec=False):

Subscribers

People subscribed via source and target branches

to status/vote changes: