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 | 235 | return BootSources.parse(StringIO(sources_yaml)) | 235 | return BootSources.parse(StringIO(sources_yaml)) |
6 | 236 | 236 | ||
7 | 237 | 237 | ||
8 | 238 | def update_iscsi_targets(snapshot_path): | ||
9 | 239 | maaslog.info("Updating boot image iSCSI targets.") | ||
10 | 240 | update_targets_conf(snapshot_path) | ||
11 | 241 | |||
12 | 242 | |||
13 | 238 | def import_images(sources): | 243 | def import_images(sources): |
14 | 239 | """Import images. Callable from the command line. | 244 | """Import images. Callable from the command line. |
15 | 240 | 245 | ||
16 | @@ -246,6 +251,9 @@ | |||
17 | 246 | maaslog.warning("Can't import: region did not provide a source.") | 251 | maaslog.warning("Can't import: region did not provide a source.") |
18 | 247 | return False | 252 | return False |
19 | 248 | 253 | ||
20 | 254 | with ClusterConfiguration.open() as config: | ||
21 | 255 | storage = FilePath(config.tftp_root).parent().path | ||
22 | 256 | |||
23 | 249 | with tempdir('keyrings') as keyrings_path: | 257 | with tempdir('keyrings') as keyrings_path: |
24 | 250 | # XXX: Band-aid to ensure that the keyring_data is bytes. Future task: | 258 | # XXX: Band-aid to ensure that the keyring_data is bytes. Future task: |
25 | 251 | # try to figure out why this sometimes happens. | 259 | # try to figure out why this sometimes happens. |
26 | @@ -261,15 +269,15 @@ | |||
27 | 261 | 269 | ||
28 | 262 | image_descriptions = download_all_image_descriptions(sources) | 270 | image_descriptions = download_all_image_descriptions(sources) |
29 | 263 | if image_descriptions.is_empty(): | 271 | if image_descriptions.is_empty(): |
30 | 272 | update_iscsi_targets(os.path.join(storage, 'current')) | ||
31 | 264 | maaslog.warning( | 273 | maaslog.warning( |
32 | 265 | "Finished importing boot images, the region does not have " | 274 | "Finished importing boot images, the region does not have " |
33 | 266 | "any boot images available.") | 275 | "any boot images available.") |
34 | 267 | return False | 276 | return False |
35 | 268 | 277 | ||
36 | 269 | with ClusterConfiguration.open() as config: | ||
37 | 270 | storage = FilePath(config.tftp_root).parent().path | ||
38 | 271 | meta_file_content = image_descriptions.dump_json() | 278 | meta_file_content = image_descriptions.dump_json() |
39 | 272 | if meta_contains(storage, meta_file_content): | 279 | if meta_contains(storage, meta_file_content): |
40 | 280 | update_iscsi_targets(os.path.join(storage, 'current')) | ||
41 | 273 | maaslog.info( | 281 | maaslog.info( |
42 | 274 | "Finished importing boot images, the region does not " | 282 | "Finished importing boot images, the region does not " |
43 | 275 | "have any new images.") | 283 | "have any new images.") |
44 | @@ -277,8 +285,17 @@ | |||
45 | 277 | 285 | ||
46 | 278 | product_mapping = map_products(image_descriptions) | 286 | product_mapping = map_products(image_descriptions) |
47 | 279 | 287 | ||
50 | 280 | snapshot_path = download_all_boot_resources( | 288 | try: |
51 | 281 | sources, storage, product_mapping) | 289 | snapshot_path = download_all_boot_resources( |
52 | 290 | sources, storage, product_mapping) | ||
53 | 291 | except: | ||
54 | 292 | update_iscsi_targets(os.path.join(storage, 'current')) | ||
55 | 293 | # Cleanup snapshots and cache since download failed. | ||
56 | 294 | maaslog.warning( | ||
57 | 295 | "Unable to import boot images; cleaning up failed snapshot " | ||
58 | 296 | "and cache.") | ||
59 | 297 | cleanup_snapshots_and_cache(storage) | ||
60 | 298 | raise | ||
61 | 282 | 299 | ||
62 | 283 | maaslog.info("Writing boot image metadata and iSCSI targets.") | 300 | maaslog.info("Writing boot image metadata and iSCSI targets.") |
63 | 284 | write_snapshot_metadata(snapshot_path, meta_file_content) | 301 | write_snapshot_metadata(snapshot_path, meta_file_content) |
64 | @@ -289,8 +306,8 @@ | |||
65 | 289 | 306 | ||
66 | 290 | # If we got here, all went well. This is now truly the "current" snapshot. | 307 | # If we got here, all went well. This is now truly the "current" snapshot. |
67 | 291 | update_current_symlink(storage, snapshot_path) | 308 | update_current_symlink(storage, snapshot_path) |
70 | 292 | maaslog.info("Updating boot image iSCSI targets.") | 309 | |
71 | 293 | update_targets_conf(snapshot_path) | 310 | update_iscsi_targets(snapshot_path) |
72 | 294 | 311 | ||
73 | 295 | # Now cleanup the old snapshots and cache. | 312 | # Now cleanup the old snapshots and cache. |
74 | 296 | maaslog.info('Cleaning up old snapshots and cache.') | 313 | maaslog.info('Cleaning up old snapshots and cache.') |
75 | 297 | 314 | ||
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 | 19 | Popen, | 19 | Popen, |
81 | 20 | ) | 20 | ) |
82 | 21 | from unittest import mock | 21 | from unittest import mock |
84 | 22 | from unittest.mock import call | 22 | from unittest.mock import ( |
85 | 23 | call, | ||
86 | 24 | MagicMock, | ||
87 | 25 | ) | ||
88 | 23 | 26 | ||
89 | 24 | from maastesting.factory import factory | 27 | from maastesting.factory import factory |
90 | 25 | from maastesting.matchers import ( | 28 | from maastesting.matchers import ( |
91 | 26 | MockAnyCall, | 29 | MockAnyCall, |
92 | 30 | MockCalledOnce, | ||
93 | 27 | MockCalledOnceWith, | 31 | MockCalledOnceWith, |
94 | 28 | MockCalledWith, | 32 | MockCalledWith, |
95 | 29 | MockCallsMatch, | 33 | MockCallsMatch, |
96 | @@ -511,6 +515,7 @@ | |||
97 | 511 | BootImageMapping()) | 515 | BootImageMapping()) |
98 | 512 | self.patch_maaslog() | 516 | self.patch_maaslog() |
99 | 513 | self.patch(boot_resources, 'RepoWriter') | 517 | self.patch(boot_resources, 'RepoWriter') |
100 | 518 | self.patch(boot_resources, 'update_iscsi_targets') | ||
101 | 514 | args = self.make_args(sources_file=sources_fixture.filename) | 519 | args = self.make_args(sources_file=sources_fixture.filename) |
102 | 515 | 520 | ||
103 | 516 | boot_resources.main(args) | 521 | boot_resources.main(args) |
104 | @@ -657,3 +662,152 @@ | |||
105 | 657 | boot_resources.import_images(sources) | 662 | boot_resources.import_images(sources) |
106 | 658 | self.assertThat( | 663 | self.assertThat( |
107 | 659 | fake_write_all_keyrings, MockCalledWith(mock.ANY, sources)) | 664 | fake_write_all_keyrings, MockCalledWith(mock.ANY, sources)) |
108 | 665 | |||
109 | 666 | def test__returns_false_when_no_images(self): | ||
110 | 667 | # Stop import_images() from actually doing anything. | ||
111 | 668 | self.patch(boot_resources, 'maaslog') | ||
112 | 669 | fake_download_all_image_descriptions = self.patch( | ||
113 | 670 | boot_resources, 'download_all_image_descriptions') | ||
114 | 671 | fake_download_all_image_descriptions.return_value = MagicMock() | ||
115 | 672 | fake_update_iscsi_targets = self.patch( | ||
116 | 673 | boot_resources, 'update_iscsi_targets') | ||
117 | 674 | |||
118 | 675 | self.patch(boot_resources, 'write_all_keyrings') | ||
119 | 676 | sources = [ | ||
120 | 677 | { | ||
121 | 678 | 'keyring_data': self.getUniqueString(), | ||
122 | 679 | 'url': factory.make_name("something"), | ||
123 | 680 | 'selections': [ | ||
124 | 681 | { | ||
125 | 682 | 'os': factory.make_name("os"), | ||
126 | 683 | 'release': factory.make_name("release"), | ||
127 | 684 | 'arches': [factory.make_name("arch")], | ||
128 | 685 | 'subarches': [factory.make_name("subarch")], | ||
129 | 686 | 'labels': [factory.make_name("label")], | ||
130 | 687 | }, | ||
131 | 688 | ], | ||
132 | 689 | }, | ||
133 | 690 | ], | ||
134 | 691 | self.assertFalse(boot_resources.import_images(sources)) | ||
135 | 692 | self.assertThat(fake_update_iscsi_targets, MockCalledOnce()) | ||
136 | 693 | |||
137 | 694 | def test__returns_false_when_no_new_images(self): | ||
138 | 695 | # Stop import_images() from actually doing anything. | ||
139 | 696 | self.patch(boot_resources, 'maaslog') | ||
140 | 697 | fake_download_all_image_descriptions = self.patch( | ||
141 | 698 | boot_resources, 'download_all_image_descriptions') | ||
142 | 699 | fake_image_descriptions = MagicMock() | ||
143 | 700 | fake_image_descriptions.is_empty.return_value = False | ||
144 | 701 | fake_download_all_image_descriptions.return_value = ( | ||
145 | 702 | fake_image_descriptions) | ||
146 | 703 | self.patch(boot_resources, 'meta_contains').return_value = True | ||
147 | 704 | fake_update_iscsi_targets = self.patch( | ||
148 | 705 | boot_resources, 'update_iscsi_targets') | ||
149 | 706 | |||
150 | 707 | self.patch(boot_resources, 'write_all_keyrings') | ||
151 | 708 | sources = [ | ||
152 | 709 | { | ||
153 | 710 | 'keyring_data': self.getUniqueString(), | ||
154 | 711 | 'url': factory.make_name("something"), | ||
155 | 712 | 'selections': [ | ||
156 | 713 | { | ||
157 | 714 | 'os': factory.make_name("os"), | ||
158 | 715 | 'release': factory.make_name("release"), | ||
159 | 716 | 'arches': [factory.make_name("arch")], | ||
160 | 717 | 'subarches': [factory.make_name("subarch")], | ||
161 | 718 | 'labels': [factory.make_name("label")], | ||
162 | 719 | }, | ||
163 | 720 | ], | ||
164 | 721 | }, | ||
165 | 722 | ], | ||
166 | 723 | self.assertFalse(boot_resources.import_images(sources)) | ||
167 | 724 | self.assertThat(fake_update_iscsi_targets, MockCalledOnce()) | ||
168 | 725 | |||
169 | 726 | def test__cleans_up_on_failure(self): | ||
170 | 727 | # Stop import_images() from actually doing anything. | ||
171 | 728 | self.patch(boot_resources, 'maaslog') | ||
172 | 729 | fake_download_all_image_descriptions = self.patch( | ||
173 | 730 | boot_resources, 'download_all_image_descriptions') | ||
174 | 731 | fake_image_descriptions = MagicMock() | ||
175 | 732 | fake_image_descriptions.is_empty.return_value = False | ||
176 | 733 | fake_download_all_image_descriptions.return_value = ( | ||
177 | 734 | fake_image_descriptions) | ||
178 | 735 | self.patch(boot_resources, 'meta_contains').return_value = False | ||
179 | 736 | self.patch(boot_resources, 'map_products') | ||
180 | 737 | self.patch( | ||
181 | 738 | boot_resources, 'download_all_boot_resources' | ||
182 | 739 | ).side_effect = Exception | ||
183 | 740 | fake_update_iscsi_targets = self.patch( | ||
184 | 741 | boot_resources, 'update_iscsi_targets') | ||
185 | 742 | fake_cleanup_snapshots_and_cache = self.patch( | ||
186 | 743 | boot_resources, 'cleanup_snapshots_and_cache') | ||
187 | 744 | |||
188 | 745 | self.patch(boot_resources, 'write_all_keyrings') | ||
189 | 746 | sources = [ | ||
190 | 747 | { | ||
191 | 748 | 'keyring_data': self.getUniqueString(), | ||
192 | 749 | 'url': factory.make_name("something"), | ||
193 | 750 | 'selections': [ | ||
194 | 751 | { | ||
195 | 752 | 'os': factory.make_name("os"), | ||
196 | 753 | 'release': factory.make_name("release"), | ||
197 | 754 | 'arches': [factory.make_name("arch")], | ||
198 | 755 | 'subarches': [factory.make_name("subarch")], | ||
199 | 756 | 'labels': [factory.make_name("label")], | ||
200 | 757 | }, | ||
201 | 758 | ], | ||
202 | 759 | }, | ||
203 | 760 | ], | ||
204 | 761 | self.assertRaises( | ||
205 | 762 | Exception, boot_resources.import_images, sources) | ||
206 | 763 | self.assertThat(fake_update_iscsi_targets, MockCalledOnce()) | ||
207 | 764 | self.assertThat(fake_cleanup_snapshots_and_cache, MockCalledOnce()) | ||
208 | 765 | |||
209 | 766 | def test__runs_import_and_returns_true(self): | ||
210 | 767 | # Stop import_images() from actually doing anything. | ||
211 | 768 | self.patch(boot_resources, 'maaslog') | ||
212 | 769 | fake_download_all_image_descriptions = self.patch( | ||
213 | 770 | boot_resources, 'download_all_image_descriptions') | ||
214 | 771 | fake_image_descriptions = MagicMock() | ||
215 | 772 | fake_image_descriptions.is_empty.return_value = False | ||
216 | 773 | fake_download_all_image_descriptions.return_value = ( | ||
217 | 774 | fake_image_descriptions) | ||
218 | 775 | self.patch(boot_resources, 'meta_contains').return_value = False | ||
219 | 776 | self.patch(boot_resources, 'map_products') | ||
220 | 777 | self.patch(boot_resources, 'download_all_boot_resources') | ||
221 | 778 | fake_write_snapshot_metadata = self.patch( | ||
222 | 779 | boot_resources, 'write_snapshot_metadata') | ||
223 | 780 | fake_targets_conf = self.patch( | ||
224 | 781 | boot_resources, 'write_targets_conf') | ||
225 | 782 | fake_install_boot_loaders = self.patch( | ||
226 | 783 | boot_resources, 'install_boot_loaders') | ||
227 | 784 | fake_update_current_symlink = self.patch( | ||
228 | 785 | boot_resources, 'update_current_symlink') | ||
229 | 786 | fake_update_iscsi_targets = self.patch( | ||
230 | 787 | boot_resources, 'update_iscsi_targets') | ||
231 | 788 | fake_cleanup_snapshots_and_cache = self.patch( | ||
232 | 789 | boot_resources, 'cleanup_snapshots_and_cache') | ||
233 | 790 | |||
234 | 791 | self.patch(boot_resources, 'write_all_keyrings') | ||
235 | 792 | sources = [ | ||
236 | 793 | { | ||
237 | 794 | 'keyring_data': self.getUniqueString(), | ||
238 | 795 | 'url': factory.make_name("something"), | ||
239 | 796 | 'selections': [ | ||
240 | 797 | { | ||
241 | 798 | 'os': factory.make_name("os"), | ||
242 | 799 | 'release': factory.make_name("release"), | ||
243 | 800 | 'arches': [factory.make_name("arch")], | ||
244 | 801 | 'subarches': [factory.make_name("subarch")], | ||
245 | 802 | 'labels': [factory.make_name("label")], | ||
246 | 803 | }, | ||
247 | 804 | ], | ||
248 | 805 | }, | ||
249 | 806 | ], | ||
250 | 807 | self.assertTrue(boot_resources.import_images(sources)) | ||
251 | 808 | self.assertThat(fake_write_snapshot_metadata, MockCalledOnce()) | ||
252 | 809 | self.assertThat(fake_targets_conf, MockCalledOnce()) | ||
253 | 810 | self.assertThat(fake_install_boot_loaders, MockCalledOnce()) | ||
254 | 811 | self.assertThat(fake_update_current_symlink, MockCalledOnce()) | ||
255 | 812 | self.assertThat(fake_update_iscsi_targets, MockCalledOnce()) | ||
256 | 813 | self.assertThat(fake_cleanup_snapshots_and_cache, MockCalledOnce()) |
Reviewed in https:/ /code.launchpad .net/~ltrager/ maas/lp1554636/ +merge/ 309318