Merge lp:~verterok/ubuntuone-client/survive-to-broken-vm-metadata into lp:ubuntuone-client

Proposed by Guillermo Gonzalez
Status: Merged
Approved by: Eric Casteleijn
Approved revision: 247
Merged at revision: not available
Proposed branch: lp:~verterok/ubuntuone-client/survive-to-broken-vm-metadata
Merge into: lp:ubuntuone-client
Diff against target: 340 lines
2 files modified
tests/syncdaemon/test_fsm.py (+285/-1)
ubuntuone/syncdaemon/filesystem_manager.py (+11/-2)
To merge this branch: bzr merge lp:~verterok/ubuntuone-client/survive-to-broken-vm-metadata
Reviewer Review Type Date Requested Status
Eric Casteleijn (community) Approve
dobey (community) Approve
Review via email: mp+13289@code.launchpad.net

Commit message

handle broken shares in filesystem manager index load/creation

To post a comment you must log in.
Revision history for this message
Guillermo Gonzalez (verterok) wrote :

handle broken shares in filesystem manager index load/creation

Revision history for this message
dobey (dobey) :
review: Approve
Revision history for this message
Eric Casteleijn (thisfred) wrote :

Looks great, thanks for adding those tests!

I get this:

===============================================================================
[ERROR]: tests.syncdaemon.test_localrescan.InotifyTests.test_man_in_the_middle

Traceback (most recent call last):
  File "/usr/lib/python2.6/dist-packages/twisted/internet/gtk2reactor.py", line 199, in _doReadOrWrite
    why = source.doRead()
  File "/home/eric/survive-to-broken-vm-metadata/ubuntuone/syncdaemon/event_queue.py", line 413, in doRead
    notifier.process_events()
  File "/usr/lib/pymodules/python2.6/pyinotify.py", line 1011, in process_events
    revent = self._sys_proc_fun(raw_event) # system processings
  File "/usr/lib/pymodules/python2.6/pyinotify.py", line 549, in __call__
    return meth(event)
  File "/usr/lib/pymodules/python2.6/pyinotify.py", line 627, in process_IN_CREATE
    return self.process_default(raw_event)
  File "/usr/lib/pymodules/python2.6/pyinotify.py", line 714, in process_default
    'path': watch_.path,
exceptions.AttributeError: 'NoneType' object has no attribute 'path'
-------------------------------------------------------------------------------

Which I *think* is unrelated, I've seen it before. Please deal with it as you deem appropriate...

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'tests/syncdaemon/test_fsm.py'
2--- tests/syncdaemon/test_fsm.py 2009-10-07 15:18:51 +0000
3+++ tests/syncdaemon/test_fsm.py 2009-10-13 16:10:21 +0000
4@@ -26,6 +26,7 @@
5
6 from contrib.testing.testcase import (
7 FakeVolumeManager,
8+ FakeMain,
9 )
10
11 from ubuntuone.syncdaemon.filesystem_manager import (
12@@ -34,7 +35,7 @@
13 InconsistencyError,
14 METADATA_VERSION,
15 )
16-from ubuntuone.syncdaemon.volume_manager import Share
17+from ubuntuone.syncdaemon.volume_manager import Share, allow_writes
18
19 TESTS_DIR = os.path.join(os.getcwd(), "tmp")
20
21@@ -2273,6 +2274,289 @@
22 self.assertEquals(can_write_parent, os.access(self.share_path, os.W_OK))
23
24
25+class RealVMTestCase(FSMTestCase):
26+
27+ def setUp(self):
28+ """ Setup the test """
29+ unittest.TestCase.setUp(self)
30+ try:
31+ os.mkdir(TESTS_DIR)
32+ except OSError:
33+ # already there, remove it to clean and create again
34+ self.rmtree(TESTS_DIR)
35+ os.mkdir(TESTS_DIR)
36+ self.shares_dir = os.path.join(TESTS_DIR, 'shares')
37+ os.makedirs(self.shares_dir)
38+ self.root_dir = os.path.join(TESTS_DIR, 'root')
39+ os.makedirs(self.root_dir)
40+ self.data_dir = os.path.join(TESTS_DIR, "data")
41+ self.partials_dir = os.path.join(TESTS_DIR, "partials")
42+ self.main = FakeMain(self.root_dir, self.shares_dir, self.data_dir, self.partials_dir)
43+ self.fsm = self.main.fs
44+ self.share = self.create_share('share', 'share_name',
45+ self.fsm, self.shares_dir)
46+ self.share_path = self.share.path
47+
48+ def tearDown(self):
49+ """ Clean up the tests. """
50+ self.main.shutdown()
51+ FSMTestCase.tearDown(self)
52+
53+
54+ @staticmethod
55+ def create_share(share_id, share_name, fsm, shares_dir,
56+ access_level='Modify'):
57+ with allow_writes(shares_dir):
58+ return FSMTestCase.create_share(share_id, share_name, fsm,
59+ shares_dir, access_level)
60+
61+ def test_old_metadata_None_missing_share(self):
62+ """test loading metadata v0. that points to a share that
63+ we don't have
64+ """
65+ # create some stuff
66+ path = os.path.join(self.share.path, 'path')
67+ open(path, "w").close()
68+ mdid = self.fsm.create(path, "share")
69+ self.fsm.set_node_id(path, "uuid")
70+ # create a path with the old layout
71+ other_share = self.create_share('share1', 'share1_name',
72+ self.fsm, self.shares_dir)
73+ share_mdid = self.fsm.create(other_share.path, "share1")
74+ self.fsm.set_node_id(other_share.path, "uuid1")
75+ os.makedirs(os.path.join(self.root_dir, 'Ubuntu One'))
76+ old_shares_path = os.path.join(self.root_dir, 'Ubuntu One',
77+ 'Shared With Me')
78+ old_path = os.path.join(old_shares_path, 'share1_name')
79+ os.symlink(self.shares_dir, old_shares_path)
80+
81+ # put the old path in the mdobj
82+ share_md = self.fsm.fs[share_mdid]
83+ share_md['path'] = old_path
84+ self.fsm.fs[share_mdid] = share_md
85+
86+ # break the node on purpose
87+ real_mdobj = self.fsm.fs[mdid]
88+ del real_mdobj["stat"]
89+ real_mdobj["path"] = unicode(real_mdobj["path"])
90+ real_mdobj["local_hash"] = None
91+ real_mdobj["server_hash"] = None
92+ self.fsm.fs[mdid] = real_mdobj
93+
94+ # delete the version that should have left the previous fsm
95+ version_file = os.path.join(self.data_dir, "metadata_version")
96+ os.remove(version_file)
97+
98+ # remove the share!
99+ del self.fsm.vm.shares[other_share.id]
100+
101+ # start up again, and check
102+ newfsm = FileSystemManager(self.data_dir, self.partials_dir, self.fsm.vm)
103+ md_version = open(version_file).read()
104+ self.assertEqual(md_version, "4")
105+ newmdobj = newfsm.get_by_path(path)
106+ self.assertEqual(newmdobj.mdid, mdid)
107+ self.assertEqual(newmdobj.stat, os.stat(path))
108+ self.assertEqual(newmdobj.local_hash, "")
109+ self.assertEqual(newmdobj.server_hash, "")
110+ self.assertTrue(isinstance(newmdobj.path, str))
111+ self.assertTrue(other_share.path not in newfsm._idx_path)
112+ self.assertFalse(old_path in self.fsm._idx_path)
113+ self.assertFalse(old_path in newfsm._idx_path)
114+ self.assertRaises(KeyError, newfsm.get_by_mdid, share_mdid)
115+
116+ def test_old_metadata_1_missing_share(self):
117+ """test loading metadata v1. that points to a share that
118+ we don't have
119+ """
120+ # create some stuff
121+ path1 = os.path.join(self.share.path, 'path1')
122+ path2 = os.path.join(self.share.path, 'path2')
123+ mdid1 = self.fsm.create(path1, "share")
124+ self.fsm.set_node_id(path1, "uuid1")
125+ mdid2 = self.fsm.create(path2, "share")
126+ self.fsm.set_node_id(path2, "uuid2")
127+
128+ # create a path with the old layout
129+ other_share = self.create_share('share1', 'share1_name',
130+ self.fsm, self.shares_dir)
131+ share_mdid = self.fsm.create(other_share.path, "share1")
132+ self.fsm.set_node_id(other_share.path, "uuid3")
133+ os.makedirs(os.path.join(self.root_dir, 'Ubuntu One'))
134+ old_shares_path = os.path.join(self.root_dir, 'Ubuntu One',
135+ 'Shared With Me')
136+ old_path = os.path.join(old_shares_path, 'share1_name')
137+ os.symlink(self.shares_dir, old_shares_path)
138+
139+ # put the old path in the mdobj
140+ share_md = self.fsm.fs[share_mdid]
141+ share_md['path'] = old_path
142+ self.fsm.fs[share_mdid] = share_md
143+
144+ # break the node on purpose, with unicode valid and not
145+ real_mdobj = self.fsm.fs[mdid1]
146+ real_mdobj["path"] = unicode(real_mdobj["path"])
147+ real_mdobj["local_hash"] = None
148+ real_mdobj["server_hash"] = None
149+ self.fsm.fs[mdid1] = real_mdobj
150+ real_mdobj = self.fsm.fs[mdid2]
151+ real_mdobj["path"] = "asdas\x00\xff\xffasd"
152+ self.fsm.fs[mdid2] = real_mdobj
153+
154+ # put the version file in 1
155+ version_file = os.path.join(self.data_dir, "metadata_version")
156+ with open(version_file, "w") as fh:
157+ fh.write("1")
158+
159+ # remove the share!
160+ del self.fsm.vm.shares[other_share.id]
161+
162+ # start up again, and check
163+ newfsm = FileSystemManager(self.data_dir, self.partials_dir, self.fsm.vm)
164+ version_file = os.path.join(self.data_dir, "metadata_version")
165+ md_version = open(version_file).read()
166+ self.assertEqual(md_version, METADATA_VERSION)
167+ # pylint: disable-msg=W0212
168+ self.assertEqual(1, len(newfsm._idx_node_id))
169+ self.assertEqual(2, len(newfsm._idx_path))
170+ self.assertEquals('uuid1', newfsm.get_by_mdid(mdid1).node_id)
171+ self.assertRaises(KeyError, newfsm.get_by_mdid, share_mdid)
172+
173+ def test_old_metadata_2_missing_share(self):
174+ """test loading metadata v2. that points to a share that
175+ we don't have
176+ """
177+ # create some stuff
178+ path = os.path.join(self.share.path, 'path')
179+ mdid = self.fsm.create(path, "share")
180+ self.fsm.set_node_id(path, "uuid")
181+ # create a path with the old layout
182+ other_share = self.create_share('share1', 'share1_name',
183+ self.fsm, self.shares_dir)
184+ share_mdid = self.fsm.create(other_share.path, "share1")
185+ self.fsm.set_node_id(other_share.path, "uuid3")
186+ os.makedirs(os.path.join(self.root_dir, 'Ubuntu One'))
187+ old_shares_path = os.path.join(self.root_dir, 'Ubuntu One',
188+ 'Shared With Me')
189+ old_path = os.path.join(old_shares_path, 'share1_name')
190+ os.symlink(self.shares_dir, old_shares_path)
191+
192+ # put the old path in the mdobj
193+ share_md = self.fsm.fs[share_mdid]
194+ share_md['path'] = old_path
195+ self.fsm.fs[share_mdid] = share_md
196+
197+ # break the node on purpose, with hashes in None
198+ real_mdobj = self.fsm.fs[mdid]
199+ real_mdobj["local_hash"] = None
200+ real_mdobj["server_hash"] = None
201+ self.fsm.fs[mdid] = real_mdobj
202+
203+ # put the version file in 1
204+ version_file = os.path.join(self.data_dir, "metadata_version")
205+ with open(version_file, "w") as fh:
206+ fh.write("2")
207+
208+ # remove the share!
209+ del self.fsm.vm.shares[other_share.id]
210+
211+ # start up again, and check
212+ newfsm = FileSystemManager(self.data_dir, self.partials_dir, self.fsm.vm)
213+ version_file = os.path.join(self.data_dir, "metadata_version")
214+ md_version = open(version_file).read()
215+ self.assertEqual(md_version, METADATA_VERSION)
216+ self.assertTrue(newfsm.get_by_mdid(mdid) is not None)
217+ # pylint: disable-msg=W0212
218+ self.assertEqual(1, len(newfsm._idx_node_id))
219+ self.assertEqual(2, len(newfsm._idx_path))
220+ self.assertRaises(KeyError, newfsm.get_by_mdid, share_mdid)
221+
222+ def test_old_metadata_3_missing_share(self):
223+ """test loading metadata v3. that points to a share that
224+ we don't have
225+ """
226+ # create a path with the old layout and metadata
227+ # the root
228+ root_mdid = self.fsm.get_by_path(self.root_dir).mdid
229+ self.fsm.set_node_id(self.root_dir, "uuid")
230+ # a share
231+ other_share = self.create_share('share1', 'share1_name',
232+ self.fsm, self.shares_dir)
233+ share_mdid = self.fsm.create(other_share.path, "share1")
234+ self.fsm.set_node_id(other_share.path, "uuid1")
235+ os.makedirs(os.path.join(self.root_dir, 'Ubuntu One'))
236+ old_shares_path = os.path.join(self.root_dir, 'Ubuntu One',
237+ 'Shared With Me')
238+ old_path = os.path.join(old_shares_path, 'share1_name')
239+ os.symlink(self.shares_dir, old_shares_path)
240+ old_root_path = os.path.join(os.path.dirname(self.root_dir),
241+ 'Ubuntu One', 'My Files')
242+
243+ # put the old path in the mdobjs
244+ share_md = self.fsm.fs[share_mdid]
245+ share_md['path'] = old_path
246+ self.fsm.fs[share_mdid] = share_md
247+ root_md = self.fsm.fs[root_mdid]
248+ root_md['path'] = old_root_path
249+ self.fsm.fs[root_mdid] = root_md
250+
251+ # put the version file in 1
252+ version_file = os.path.join(self.data_dir, "metadata_version")
253+ with open(version_file, "w") as fh:
254+ fh.write("3")
255+
256+ # remove the share!
257+ del self.fsm.vm.shares[other_share.id]
258+
259+ # start up again, and check
260+ newfsm = FileSystemManager(self.data_dir, self.partials_dir, self.fsm.vm)
261+ version_file = os.path.join(self.data_dir, "metadata_version")
262+ md_version = open(version_file).read()
263+ self.assertEqual(md_version, METADATA_VERSION)
264+ self.assertTrue(newfsm.get_by_mdid(root_mdid) is not None)
265+ # pylint: disable-msg=W0212
266+ self.assertEqual(1, len(newfsm._idx_node_id))
267+ self.assertEqual(1, len(newfsm._idx_path))
268+ self.assertRaises(KeyError, newfsm.get_by_mdid, share_mdid)
269+
270+ def test_metadata_missing_share(self):
271+ """test loading current metadata that points to a share
272+ that we don't have
273+ """
274+ md_version = open(os.path.join(self.data_dir, "metadata_version")).read()
275+ self.assertEqual(md_version, METADATA_VERSION)
276+ path = os.path.join(self.share.path, 'path')
277+ path1 = os.path.join(self.share.path, 'path1')
278+ other_share = self.create_share('share1', 'share1_name',
279+ self.fsm, self.shares_dir)
280+
281+ path2 = os.path.join(other_share.path, 'broken_path2')
282+ for p in [path, path1, path2]:
283+ open(p, "w").close()
284+ mdid = self.fsm.create(path, "share")
285+ self.fsm.set_node_id(path, "uuid")
286+ mdid1 = self.fsm.create(path1, "share")
287+ self.fsm.set_node_id(path1, "uuid1")
288+ mdid2 = self.fsm.create(path2, "share1")
289+ self.fsm.set_node_id(path2, "uuid2")
290+
291+ # remove the share!
292+ del self.fsm.vm.shares[other_share.id]
293+
294+ # start up again, and check
295+ newfsm = FileSystemManager(self.data_dir, self.partials_dir, self.fsm.vm)
296+ version_file = os.path.join(self.data_dir, "metadata_version")
297+ md_version = open(version_file).read()
298+ self.assertEqual(md_version, "4")
299+ self.assertTrue(newfsm.get_by_mdid(mdid) is not None)
300+ # pylint: disable-msg=W0212
301+ self.assertEqual(2, len(newfsm._idx_node_id))
302+ self.assertEqual(3, len(newfsm._idx_path))
303+ # check that the broken mdid's load the old metadata
304+ self.assertEquals('uuid', newfsm.get_by_mdid(mdid).node_id)
305+ self.assertEquals('uuid1', newfsm.get_by_mdid(mdid1).node_id)
306+ self.assertRaises(KeyError, newfsm.get_by_mdid, mdid2)
307+
308 def test_suite():
309 # pylint: disable-msg=C0111
310 return unittest.TestLoader().loadTestsFromName(__name__)
311
312=== modified file 'ubuntuone/syncdaemon/filesystem_manager.py'
313--- ubuntuone/syncdaemon/filesystem_manager.py 2009-09-28 17:34:38 +0000
314+++ ubuntuone/syncdaemon/filesystem_manager.py 2009-10-13 16:10:21 +0000
315@@ -262,7 +262,16 @@
316 # return False, in order to be filtered later
317 return False
318 else:
319- return mdid, mdobj
320+ # check if the share exists
321+ try:
322+ self._get_share(mdobj["share_id"])
323+ except KeyError, e:
324+ # oops, the share is gone!, invalidate this mdid
325+ log_warning('Share %s disappeared! deleting mdid: %s', mdobj['share_id'], mdid)
326+ del self.fs[mdid]
327+ return False
328+ else:
329+ return mdid, mdobj
330 safe_iteritems = itertools.imap(safeget_mdobj, self.fs.keys())
331 # filter all False values
332 return itertools.ifilter(None, safe_iteritems)
333@@ -907,7 +916,7 @@
334 """ returns the share with id: share_id. """
335 share = self.shares.get(share_id, None)
336 if share is None:
337- share = self.vm.shares.get(share_id)
338+ share = self.vm.shares[share_id]
339 self.shares[share_id] = share
340 return share
341

Subscribers

People subscribed via source and target branches