Merge lp:~ltrager/maas/lp1554636_2.0 into lp:maas/2.0
- lp1554636_2.0
- Merge into 2.0
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Lee Trager | ||||
Approved revision: | no longer in the source branch. | ||||
Merged at revision: | 5203 | ||||
Proposed branch: | lp:~ltrager/maas/lp1554636_2.0 | ||||
Merge into: | lp:maas/2.0 | ||||
Diff against target: |
256 lines (+178/-7) 2 files modified
src/provisioningserver/import_images/boot_resources.py (+23/-6) src/provisioningserver/import_images/tests/test_boot_resources.py (+155/-1) |
||||
To merge this branch: | bzr merge lp:~ltrager/maas/lp1554636_2.0 | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Lee Trager (community) | Approve | ||
Review via email: mp+309880@code.launchpad.net |
Commit message
Update iSCSI targets on every import (in case last time failed to update).
Description of the change
MAAS Lander (maas-lander) wrote : | # |
The attempt to merge lp:~ltrager/maas/lp1554636_2.0 into lp:maas/2.0 failed. Below is the output from the failed tests.
Hit:1 http://
Get:2 http://
Get:3 http://
Hit:4 http://
Get:5 http://
Get:6 http://
Fetched 964 kB in 2s (476 kB/s)
Reading package lists...
sudo DEBIAN_
--no-
Reading package lists...
Building dependency tree...
Reading state information...
authbind is already the newest version (2.1.1+nmu1).
build-essential is already the newest version (12.1ubuntu2).
debhelper is already the newest version (9.20160115ubun
distro-info is already the newest version (0.14build1).
freeipmi-tools is already the newest version (1.4.11-1ubuntu1).
git is already the newest version (1:2.7.4-0ubuntu1).
libjs-angularjs is already the newest version (1.2.28-1ubuntu2).
libjs-jquery is already the newest version (1.11.3+dfsg-4).
libjs-yui3-full is already the newest version (3.5.1-1ubuntu3).
libjs-yui3-min is already the newest version (3.5.1-1ubuntu3).
make is already the newest version (4.1-6).
postgresql is already the newest version (9.5+173).
pxelinux is already the newest version (3:6.03+
python-formencode is already the newest version (1.3.0-0ubuntu5).
python-lxml is already the newest version (3....
Preview Diff
1 | === modified file 'src/provisioningserver/import_images/boot_resources.py' |
2 | --- src/provisioningserver/import_images/boot_resources.py 2016-07-07 21:57:57 +0000 |
3 | +++ src/provisioningserver/import_images/boot_resources.py 2016-11-02 18:09:22 +0000 |
4 | @@ -235,6 +235,11 @@ |
5 | return BootSources.parse(StringIO(sources_yaml)) |
6 | |
7 | |
8 | +def update_iscsi_targets(snapshot_path): |
9 | + maaslog.info("Updating boot image iSCSI targets.") |
10 | + update_targets_conf(snapshot_path) |
11 | + |
12 | + |
13 | def import_images(sources): |
14 | """Import images. Callable from the command line. |
15 | |
16 | @@ -246,6 +251,9 @@ |
17 | maaslog.warning("Can't import: region did not provide a source.") |
18 | return False |
19 | |
20 | + with ClusterConfiguration.open() as config: |
21 | + storage = FilePath(config.tftp_root).parent().path |
22 | + |
23 | with tempdir('keyrings') as keyrings_path: |
24 | # XXX: Band-aid to ensure that the keyring_data is bytes. Future task: |
25 | # try to figure out why this sometimes happens. |
26 | @@ -261,15 +269,15 @@ |
27 | |
28 | image_descriptions = download_all_image_descriptions(sources) |
29 | if image_descriptions.is_empty(): |
30 | + update_iscsi_targets(os.path.join(storage, 'current')) |
31 | maaslog.warning( |
32 | "Finished importing boot images, the region does not have " |
33 | "any boot images available.") |
34 | return False |
35 | |
36 | - with ClusterConfiguration.open() as config: |
37 | - storage = FilePath(config.tftp_root).parent().path |
38 | meta_file_content = image_descriptions.dump_json() |
39 | if meta_contains(storage, meta_file_content): |
40 | + update_iscsi_targets(os.path.join(storage, 'current')) |
41 | maaslog.info( |
42 | "Finished importing boot images, the region does not " |
43 | "have any new images.") |
44 | @@ -277,8 +285,17 @@ |
45 | |
46 | product_mapping = map_products(image_descriptions) |
47 | |
48 | - snapshot_path = download_all_boot_resources( |
49 | - sources, storage, product_mapping) |
50 | + try: |
51 | + snapshot_path = download_all_boot_resources( |
52 | + sources, storage, product_mapping) |
53 | + except: |
54 | + update_iscsi_targets(os.path.join(storage, 'current')) |
55 | + # Cleanup snapshots and cache since download failed. |
56 | + maaslog.warning( |
57 | + "Unable to import boot images; cleaning up failed snapshot " |
58 | + "and cache.") |
59 | + cleanup_snapshots_and_cache(storage) |
60 | + raise |
61 | |
62 | maaslog.info("Writing boot image metadata and iSCSI targets.") |
63 | write_snapshot_metadata(snapshot_path, meta_file_content) |
64 | @@ -289,8 +306,8 @@ |
65 | |
66 | # If we got here, all went well. This is now truly the "current" snapshot. |
67 | update_current_symlink(storage, snapshot_path) |
68 | - maaslog.info("Updating boot image iSCSI targets.") |
69 | - update_targets_conf(snapshot_path) |
70 | + |
71 | + update_iscsi_targets(snapshot_path) |
72 | |
73 | # Now cleanup the old snapshots and cache. |
74 | maaslog.info('Cleaning up old snapshots and cache.') |
75 | |
76 | === modified file 'src/provisioningserver/import_images/tests/test_boot_resources.py' |
77 | --- src/provisioningserver/import_images/tests/test_boot_resources.py 2016-07-07 21:57:57 +0000 |
78 | +++ src/provisioningserver/import_images/tests/test_boot_resources.py 2016-11-02 18:09:22 +0000 |
79 | @@ -19,11 +19,15 @@ |
80 | Popen, |
81 | ) |
82 | from unittest import mock |
83 | -from unittest.mock import call |
84 | +from unittest.mock import ( |
85 | + call, |
86 | + MagicMock, |
87 | +) |
88 | |
89 | from maastesting.factory import factory |
90 | from maastesting.matchers import ( |
91 | MockAnyCall, |
92 | + MockCalledOnce, |
93 | MockCalledOnceWith, |
94 | MockCalledWith, |
95 | MockCallsMatch, |
96 | @@ -511,6 +515,7 @@ |
97 | BootImageMapping()) |
98 | self.patch_maaslog() |
99 | self.patch(boot_resources, 'RepoWriter') |
100 | + self.patch(boot_resources, 'update_iscsi_targets') |
101 | args = self.make_args(sources_file=sources_fixture.filename) |
102 | |
103 | boot_resources.main(args) |
104 | @@ -657,3 +662,152 @@ |
105 | boot_resources.import_images(sources) |
106 | self.assertThat( |
107 | fake_write_all_keyrings, MockCalledWith(mock.ANY, sources)) |
108 | + |
109 | + def test__returns_false_when_no_images(self): |
110 | + # Stop import_images() from actually doing anything. |
111 | + self.patch(boot_resources, 'maaslog') |
112 | + fake_download_all_image_descriptions = self.patch( |
113 | + boot_resources, 'download_all_image_descriptions') |
114 | + fake_download_all_image_descriptions.return_value = MagicMock() |
115 | + fake_update_iscsi_targets = self.patch( |
116 | + boot_resources, 'update_iscsi_targets') |
117 | + |
118 | + self.patch(boot_resources, 'write_all_keyrings') |
119 | + sources = [ |
120 | + { |
121 | + 'keyring_data': self.getUniqueString(), |
122 | + 'url': factory.make_name("something"), |
123 | + 'selections': [ |
124 | + { |
125 | + 'os': factory.make_name("os"), |
126 | + 'release': factory.make_name("release"), |
127 | + 'arches': [factory.make_name("arch")], |
128 | + 'subarches': [factory.make_name("subarch")], |
129 | + 'labels': [factory.make_name("label")], |
130 | + }, |
131 | + ], |
132 | + }, |
133 | + ], |
134 | + self.assertFalse(boot_resources.import_images(sources)) |
135 | + self.assertThat(fake_update_iscsi_targets, MockCalledOnce()) |
136 | + |
137 | + def test__returns_false_when_no_new_images(self): |
138 | + # Stop import_images() from actually doing anything. |
139 | + self.patch(boot_resources, 'maaslog') |
140 | + fake_download_all_image_descriptions = self.patch( |
141 | + boot_resources, 'download_all_image_descriptions') |
142 | + fake_image_descriptions = MagicMock() |
143 | + fake_image_descriptions.is_empty.return_value = False |
144 | + fake_download_all_image_descriptions.return_value = ( |
145 | + fake_image_descriptions) |
146 | + self.patch(boot_resources, 'meta_contains').return_value = True |
147 | + fake_update_iscsi_targets = self.patch( |
148 | + boot_resources, 'update_iscsi_targets') |
149 | + |
150 | + self.patch(boot_resources, 'write_all_keyrings') |
151 | + sources = [ |
152 | + { |
153 | + 'keyring_data': self.getUniqueString(), |
154 | + 'url': factory.make_name("something"), |
155 | + 'selections': [ |
156 | + { |
157 | + 'os': factory.make_name("os"), |
158 | + 'release': factory.make_name("release"), |
159 | + 'arches': [factory.make_name("arch")], |
160 | + 'subarches': [factory.make_name("subarch")], |
161 | + 'labels': [factory.make_name("label")], |
162 | + }, |
163 | + ], |
164 | + }, |
165 | + ], |
166 | + self.assertFalse(boot_resources.import_images(sources)) |
167 | + self.assertThat(fake_update_iscsi_targets, MockCalledOnce()) |
168 | + |
169 | + def test__cleans_up_on_failure(self): |
170 | + # Stop import_images() from actually doing anything. |
171 | + self.patch(boot_resources, 'maaslog') |
172 | + fake_download_all_image_descriptions = self.patch( |
173 | + boot_resources, 'download_all_image_descriptions') |
174 | + fake_image_descriptions = MagicMock() |
175 | + fake_image_descriptions.is_empty.return_value = False |
176 | + fake_download_all_image_descriptions.return_value = ( |
177 | + fake_image_descriptions) |
178 | + self.patch(boot_resources, 'meta_contains').return_value = False |
179 | + self.patch(boot_resources, 'map_products') |
180 | + self.patch( |
181 | + boot_resources, 'download_all_boot_resources' |
182 | + ).side_effect = Exception |
183 | + fake_update_iscsi_targets = self.patch( |
184 | + boot_resources, 'update_iscsi_targets') |
185 | + fake_cleanup_snapshots_and_cache = self.patch( |
186 | + boot_resources, 'cleanup_snapshots_and_cache') |
187 | + |
188 | + self.patch(boot_resources, 'write_all_keyrings') |
189 | + sources = [ |
190 | + { |
191 | + 'keyring_data': self.getUniqueString(), |
192 | + 'url': factory.make_name("something"), |
193 | + 'selections': [ |
194 | + { |
195 | + 'os': factory.make_name("os"), |
196 | + 'release': factory.make_name("release"), |
197 | + 'arches': [factory.make_name("arch")], |
198 | + 'subarches': [factory.make_name("subarch")], |
199 | + 'labels': [factory.make_name("label")], |
200 | + }, |
201 | + ], |
202 | + }, |
203 | + ], |
204 | + self.assertRaises( |
205 | + Exception, boot_resources.import_images, sources) |
206 | + self.assertThat(fake_update_iscsi_targets, MockCalledOnce()) |
207 | + self.assertThat(fake_cleanup_snapshots_and_cache, MockCalledOnce()) |
208 | + |
209 | + def test__runs_import_and_returns_true(self): |
210 | + # Stop import_images() from actually doing anything. |
211 | + self.patch(boot_resources, 'maaslog') |
212 | + fake_download_all_image_descriptions = self.patch( |
213 | + boot_resources, 'download_all_image_descriptions') |
214 | + fake_image_descriptions = MagicMock() |
215 | + fake_image_descriptions.is_empty.return_value = False |
216 | + fake_download_all_image_descriptions.return_value = ( |
217 | + fake_image_descriptions) |
218 | + self.patch(boot_resources, 'meta_contains').return_value = False |
219 | + self.patch(boot_resources, 'map_products') |
220 | + self.patch(boot_resources, 'download_all_boot_resources') |
221 | + fake_write_snapshot_metadata = self.patch( |
222 | + boot_resources, 'write_snapshot_metadata') |
223 | + fake_targets_conf = self.patch( |
224 | + boot_resources, 'write_targets_conf') |
225 | + fake_install_boot_loaders = self.patch( |
226 | + boot_resources, 'install_boot_loaders') |
227 | + fake_update_current_symlink = self.patch( |
228 | + boot_resources, 'update_current_symlink') |
229 | + fake_update_iscsi_targets = self.patch( |
230 | + boot_resources, 'update_iscsi_targets') |
231 | + fake_cleanup_snapshots_and_cache = self.patch( |
232 | + boot_resources, 'cleanup_snapshots_and_cache') |
233 | + |
234 | + self.patch(boot_resources, 'write_all_keyrings') |
235 | + sources = [ |
236 | + { |
237 | + 'keyring_data': self.getUniqueString(), |
238 | + 'url': factory.make_name("something"), |
239 | + 'selections': [ |
240 | + { |
241 | + 'os': factory.make_name("os"), |
242 | + 'release': factory.make_name("release"), |
243 | + 'arches': [factory.make_name("arch")], |
244 | + 'subarches': [factory.make_name("subarch")], |
245 | + 'labels': [factory.make_name("label")], |
246 | + }, |
247 | + ], |
248 | + }, |
249 | + ], |
250 | + self.assertTrue(boot_resources.import_images(sources)) |
251 | + self.assertThat(fake_write_snapshot_metadata, MockCalledOnce()) |
252 | + self.assertThat(fake_targets_conf, MockCalledOnce()) |
253 | + self.assertThat(fake_install_boot_loaders, MockCalledOnce()) |
254 | + self.assertThat(fake_update_current_symlink, MockCalledOnce()) |
255 | + self.assertThat(fake_update_iscsi_targets, MockCalledOnce()) |
256 | + self.assertThat(fake_cleanup_snapshots_and_cache, MockCalledOnce()) |
Reviewed in https:/ /code.launchpad .net/~ltrager/ maas/lp1554636/ +merge/ 309318