Ubuntu system image

Merge lp:~sil2100/ubuntu-system-image/server-per_device_redirect into lp:ubuntu-system-image/server

  1. server-per_device_redirect
  2. Merge into server
Proposed by Łukasz Zemczak
Status: Merged
Merged at revision: 291
Proposed branch: lp:~sil2100/ubuntu-system-image/server-per_device_redirect
Merge into: lp:ubuntu-system-image/server
Diff against target: 317 lines (+246/-3)
2 files modified
lib/systemimage/tests/test_tree.py (+175/-0)
lib/systemimage/tree.py (+71/-3)
To merge this branch: bzr merge lp:~sil2100/ubuntu-system-image/server-per_device_redirect
Reviewer Review Type Date Requested Status
Barry Warsaw (community) Approve
Registry Administrators Pending
Review via email: mp+297904@code.launchpad.net

Commit message

Allow adding per-device channel redirects, allowing redirecting single devices out of a channel to a different one - similar to the normal channel redirects. This required some additional care-handling in multiple places.

Description of the change

Allow adding per-device channel redirects, allowing redirecting single devices out of a channel to a different one - similar to the normal channel redirects. This required some additional care-handling in multiple places.

To post a comment you must log in.
Revision history for this message
Barry Warsaw (barry) wrote :

LGTM, though I'd really like to do some real-world testing with a device to make sure that the redirect is doing what it's supposed to do on the phone.

review: Approve
Revision history for this message
Łukasz Zemczak (sil2100) wrote :

Yeah, agreed on that! Will not merge this just yet, will test it locally on a device here first, then I'll deploy in production and do some more testing on a completely detached set of channels.

Revision history for this message
Łukasz Zemczak (sil2100) wrote :

Ok, so I did as many tests as possible locally. Sadly I couldn't do an upgrade-test since I could not force ubuntu-download-manager to accept my local self-signed key (even when using the -self-signed-certs <DIR> parameter). But at least the channels.json modification parts are working and a fresh flash of a channel with the redirected device results in pulling in the right files and channel info (from the target channel).

Merging this in and then proceeding with testing on completely new test-related channels.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/systemimage/tests/test_tree.py'
2--- lib/systemimage/tests/test_tree.py 2015-05-14 22:33:12 +0000
3+++ lib/systemimage/tests/test_tree.py 2016-06-20 12:44:28 +0000
4@@ -389,6 +389,103 @@
5 self.assertEqual(device.list_images(), target.list_images())
6
7 @unittest.skipUnless(HAS_TEST_KEYS, MISSING_KEYS_WARNING)
8+ def test_per_device_redirect(self):
9+ """ """
10+ test_tree = tree.Tree(self.config)
11+
12+ # Create some channels
13+ test_tree.create_channel("parent")
14+ test_tree.create_channel("redirect")
15+
16+ # Create the required devices
17+ test_tree.create_device("parent", "device")
18+ test_tree.create_device("parent", "other")
19+ test_tree.create_device("redirect", "other")
20+
21+ # Test standard failure cases
22+ self.assertRaises(KeyError,
23+ test_tree.create_per_device_channel_redirect,
24+ "device", "redirect1", "parent")
25+ self.assertRaises(KeyError,
26+ test_tree.create_per_device_channel_redirect,
27+ "other", "redirect", "parent")
28+ self.assertRaises(KeyError,
29+ test_tree.create_per_device_channel_redirect,
30+ "device", "redirect", "parent1")
31+ self.assertRaises(KeyError,
32+ test_tree.create_per_device_channel_redirect,
33+ "device2", "redirect", "parent")
34+
35+ # Create the device channel redirect
36+ test_tree.create_per_device_channel_redirect(
37+ "device", "redirect", "parent")
38+
39+ # Publish an image
40+
41+ # # First file
42+ first = os.path.join(self.config.publish_path, "parent/device/full")
43+ open(first, "w+").close()
44+ gpg.sign_file(self.config, "image-signing", first)
45+
46+ # # Second file
47+ second = os.path.join(self.config.publish_path,
48+ "parent/device/version-1234.tar.xz")
49+
50+ tools.generate_version_tarball(self.config, "parent", "test", "1234",
51+ second.replace(".xz", ""))
52+ tools.xz_compress(second.replace(".xz", ""))
53+ os.remove(second.replace(".xz", ""))
54+ gpg.sign_file(self.config, "image-signing", second)
55+
56+ with open(second.replace(".tar.xz", ".json"), "w+") as fd:
57+ metadata = {}
58+ metadata['channel.ini'] = {}
59+ metadata['channel.ini']['version_detail'] = "test"
60+ fd.write(json.dumps(metadata))
61+ gpg.sign_file(self.config, "image-signing",
62+ second.replace(".tar.xz", ".json"))
63+
64+ # # Adding the entry
65+ device = test_tree.get_device("parent", "device")
66+ device.create_image("full", 1234, "abc",
67+ ["parent/device/full",
68+ "parent/device/version-1234.tar.xz"])
69+ device.set_phased_percentage(1234, 50)
70+
71+ # # Get the target
72+ target = test_tree.get_device("redirect", "device")
73+
74+ # Confirm the fs layout
75+ self.assertTrue(os.path.exists(os.path.join(
76+ self.config.publish_path, "redirect")))
77+ self.assertFalse(os.path.exists(os.path.join(
78+ self.config.publish_path, "redirect", "device")))
79+ self.assertTrue(os.path.exists(os.path.join(
80+ self.config.publish_path, "parent", "device")))
81+ self.assertEqual(device.list_images(), target.list_images())
82+
83+ # Check the channels index
84+ channels = test_tree.list_channels()
85+ self.assertIn("other", channels['redirect']['devices'])
86+ self.assertEqual(
87+ channels['redirect']['devices']['other']['index'],
88+ "/redirect/other/index.json")
89+ self.assertIn("device", channels['redirect']['devices'])
90+ self.assertEqual(
91+ channels['redirect']['devices']['device']['index'],
92+ "/parent/device/index.json")
93+ self.assertIn(
94+ "redirect",
95+ channels['redirect']['devices']['device'])
96+
97+ # Try removing a per-channel redirect
98+ self.assertTrue(test_tree.remove_device("redirect", "device"))
99+
100+ # Confirm files are not removed
101+ self.assertTrue(os.path.exists(os.path.join(
102+ self.config.publish_path, "parent", "device", "index.json")))
103+
104+ @unittest.skipUnless(HAS_TEST_KEYS, MISSING_KEYS_WARNING)
105 def test_redirect_alias(self):
106 # LP: #1455119 - a channel which is both an alias and a redirect is
107 # culled from sync_aliases().
108@@ -412,6 +509,57 @@
109 self.assertFalse(mock.called)
110
111 @unittest.skipUnless(HAS_TEST_KEYS, MISSING_KEYS_WARNING)
112+ def test_cleanup_device_redirects(self):
113+ test_tree = tree.Tree(self.config)
114+
115+ # Create some channels and devices
116+ test_tree.create_channel("parent")
117+ test_tree.create_channel("redirect1")
118+ test_tree.create_channel("redirect2")
119+ test_tree.create_channel("ignore")
120+
121+ test_tree.create_device("parent", "device")
122+ test_tree.create_device("parent", "other")
123+ test_tree.create_device("parent", "ignore")
124+
125+ test_tree.create_device("redirect1", "other")
126+
127+ # Create per-device redirects
128+ test_tree.create_per_device_channel_redirect(
129+ "device", "redirect1", "parent")
130+ test_tree.create_per_device_channel_redirect(
131+ "other", "redirect2", "parent")
132+
133+ # Check if removal of an unrelated channel does nothing
134+ channels = test_tree.list_channels()
135+ devices_before1 = dict(channels['redirect1']['devices'])
136+ devices_before2 = dict(channels['redirect2']['devices'])
137+ test_tree.remove_channel("ignore")
138+ channels = test_tree.list_channels()
139+ self.assertEqual(devices_before1, channels['redirect1']['devices'])
140+ self.assertEqual(devices_before2, channels['redirect2']['devices'])
141+
142+ # Check if removal of an unrelated device does nothing
143+ channels = test_tree.list_channels()
144+ devices_before1 = dict(channels['redirect1']['devices'])
145+ devices_before2 = dict(channels['redirect2']['devices'])
146+ test_tree.remove_device("parent", "ignore")
147+ channels = test_tree.list_channels()
148+ self.assertEqual(devices_before1, channels['redirect1']['devices'])
149+ self.assertEqual(devices_before2, channels['redirect2']['devices'])
150+
151+ # Check cleanup after single device removal
152+ test_tree.remove_device("parent", "device")
153+ channels = test_tree.list_channels()
154+ self.assertNotIn("device", channels['redirect1']['devices'])
155+ self.assertIn("other", channels['redirect1']['devices'])
156+
157+ # Check cleanup after whole channel removal
158+ test_tree.remove_channel("parent")
159+ channels = test_tree.list_channels()
160+ self.assertNotIn("other", channels['redirect2']['devices'])
161+
162+ @unittest.skipUnless(HAS_TEST_KEYS, MISSING_KEYS_WARNING)
163 def test_rename(self):
164 test_tree = tree.Tree(self.config)
165
166@@ -472,6 +620,33 @@
167 {'devices': {'device': {'index': '/test/new/device/index.json'}}})
168
169 @unittest.skipUnless(HAS_TEST_KEYS, MISSING_KEYS_WARNING)
170+ def test_rename_with_redirects(self):
171+ test_tree = tree.Tree(self.config)
172+
173+ # Create some channels
174+ test_tree.create_channel("old")
175+ test_tree.create_channel("redirect")
176+
177+ # Create basic devices
178+ test_tree.create_device("old", "device")
179+ test_tree.create_device("redirect", "other")
180+
181+ # Create a redirect
182+ test_tree.create_per_device_channel_redirect(
183+ "device", "redirect", "old")
184+
185+ # Rename
186+ self.assertTrue(test_tree.rename_channel("old", "new"))
187+
188+ channels = test_tree.list_channels()
189+ self.assertIn("device", channels['redirect']['devices'])
190+ self.assertIn("other", channels['redirect']['devices'])
191+ device = channels['redirect']['devices']['device']
192+ self.assertIn("redirect", device)
193+ self.assertEqual(device['redirect'], "new")
194+ self.assertEqual(device['index'], "/new/device/index.json")
195+
196+ @unittest.skipUnless(HAS_TEST_KEYS, MISSING_KEYS_WARNING)
197 def test_index(self):
198 device = tree.Device(self.config, self.temp_directory)
199
200
201=== modified file 'lib/systemimage/tree.py'
202--- lib/systemimage/tree.py 2015-10-27 15:25:38 +0000
203+++ lib/systemimage/tree.py 2016-06-20 12:44:28 +0000
204@@ -306,6 +306,37 @@
205
206 return True
207
208+ def create_per_device_channel_redirect(self, device_name, channel_name,
209+ target_name):
210+ """
211+ Creates a device-specific channel redirect, redirecting that device
212+ to point to a different channel.
213+ """
214+
215+ with channels_json(self.config, self.indexpath, True) as channels:
216+ if channel_name not in channels:
217+ raise KeyError("Couldn't find channel: %s" % channel_name)
218+
219+ if device_name in channels[channel_name]['devices']:
220+ raise KeyError("Device already exists: %s" % device_name)
221+
222+ if target_name not in channels:
223+ raise KeyError("Couldn't find target channel: %s" %
224+ target_name)
225+
226+ if device_name not in channels[target_name]['devices']:
227+ raise KeyError("Couldn't find device on target channel: "
228+ "%s, %s" %
229+ (target_name, device_name))
230+
231+ channels[channel_name]['devices'][device_name] = \
232+ dict(channels[target_name]['devices'][device_name])
233+
234+ channels[channel_name]['devices'][device_name]['redirect'] = \
235+ target_name
236+
237+ return True
238+
239 def create_device(self, channel_name, device_name, keyring_path=None):
240 """
241 Creates a new device entry in the tree.
242@@ -514,6 +545,9 @@
243 shutil.rmtree(channel_path)
244 channels.pop(channel_name)
245
246+ # Remove all redirect device channels pointing at this channel
247+ self.cleanup_device_redirects(channel_name)
248+
249 return True
250
251 def remove_device(self, channel_name, device_name):
252@@ -528,14 +562,21 @@
253 if device_name not in channels[channel_name]['devices']:
254 raise KeyError("Couldn't find device: %s" % device_name)
255
256- device_path = os.path.join(self.path, channel_name, device_name)
257- if os.path.exists(device_path):
258- shutil.rmtree(device_path)
259+ # Do not remove the device files for per-device redirects
260+ device = channels[channel_name]['devices'][device_name]
261+ if "redirect" not in device:
262+ device_path = os.path.join(
263+ self.path, channel_name, device_name)
264+ if os.path.exists(device_path):
265+ shutil.rmtree(device_path)
266 channels[channel_name]['devices'].pop(device_name)
267
268 self.sync_aliases(channel_name)
269 self.sync_redirects(channel_name)
270
271+ # Remove all redirect channels pointing at this device
272+ self.cleanup_device_redirects(channel_name, device_name)
273+
274 return True
275
276 def rename_channel(self, old_name, new_name):
277@@ -581,6 +622,15 @@
278 .replace("/%s/" % old_name,
279 "/%s/" % new_name)
280
281+ # Handle any device-specific channel redirects
282+ for channel_name, channel in channels.items():
283+ for device_name, device in channel['devices'].items():
284+ if "redirect" in device and device['redirect'] == old_name:
285+ index_path = "/%s/%s/index.json" % (new_name,
286+ device_name)
287+ device['redirect'] = new_name
288+ device['index'] = index_path
289+
290 channels.pop(old_name)
291
292 return True
293@@ -804,6 +854,24 @@
294
295 return True
296
297+ def cleanup_device_redirects(self, channel_name,
298+ redirect_device_name=None):
299+ """
300+ Cleanup any dangling device-specific channel redirects.
301+ """
302+
303+ with channels_json(self.config, self.indexpath, True) as channels:
304+ for target_name, channel in channels.items():
305+ devices = dict(channel['devices'])
306+ for device_name, device in devices.items():
307+ if ("redirect" in device and
308+ device['redirect'] == channel_name and
309+ (not redirect_device_name or
310+ redirect_device_name == device_name)):
311+ channels[target_name]['devices'].pop(device_name)
312+
313+ return True
314+
315
316 class Device:
317 def __init__(self, config, path):

Subscribers

People subscribed via source and target branches