Merge ~ltrager/maas:remove_get_boot_sources_v1 into maas:master
- Git
- lp:~ltrager/maas
- remove_get_boot_sources_v1
- Merge into master
Proposed by
Lee Trager
Status: | Merged |
---|---|
Approved by: | Lee Trager |
Approved revision: | 4fd1831ecd69b9b74c0c6f76ca2f39ba6fb960d9 |
Merge reported by: | MAAS Lander |
Merged at revision: | not available |
Proposed branch: | ~ltrager/maas:remove_get_boot_sources_v1 |
Merge into: | maas:master |
Diff against target: |
316 lines (+8/-183) 5 files modified
src/maasserver/rpc/regionservice.py (+1/-14) src/maasserver/rpc/tests/test_regionservice_calls.py (+0/-21) src/provisioningserver/rackdservices/image_download_service.py (+4/-25) src/provisioningserver/rackdservices/tests/test_image_download_service.py (+2/-85) src/provisioningserver/rpc/region.py (+1/-38) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
MAAS Lander | Approve | ||
Björn Tillenius | Approve | ||
Review via email: mp+402296@code.launchpad.net |
Commit message
Remove the GetBootSources V1 RPC call.
Description of the change
To post a comment you must log in.
Revision history for this message
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b remove_
STATUS: SUCCESS
COMMIT: 4fd1831ecd69b9b
review:
Approve
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | diff --git a/src/maasserver/rpc/regionservice.py b/src/maasserver/rpc/regionservice.py | |||
2 | index 1ec0997..950b8d3 100644 | |||
3 | --- a/src/maasserver/rpc/regionservice.py | |||
4 | +++ b/src/maasserver/rpc/regionservice.py | |||
5 | @@ -1,4 +1,4 @@ | |||
7 | 1 | # Copyright 2014-2018 Canonical Ltd. This software is licensed under the | 1 | # Copyright 2014-2021 Canonical Ltd. This software is licensed under the |
8 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). |
9 | 3 | 3 | ||
10 | 4 | """RPC implementation for regions.""" | 4 | """RPC implementation for regions.""" |
11 | @@ -207,19 +207,6 @@ class Region(RPCProtocol): | |||
12 | 207 | 207 | ||
13 | 208 | @region.GetBootSources.responder | 208 | @region.GetBootSources.responder |
14 | 209 | def get_boot_sources(self, uuid): | 209 | def get_boot_sources(self, uuid): |
15 | 210 | """get_boot_sources() | ||
16 | 211 | |||
17 | 212 | Deprecated: get_boot_sources_v2() should be used instead. | ||
18 | 213 | |||
19 | 214 | Implementation of | ||
20 | 215 | :py:class:`~provisioningserver.rpc.region.GetBootSources`. | ||
21 | 216 | """ | ||
22 | 217 | d = deferToDatabase(get_simplestream_endpoint) | ||
23 | 218 | d.addCallback(lambda source: {"sources": [source]}) | ||
24 | 219 | return d | ||
25 | 220 | |||
26 | 221 | @region.GetBootSourcesV2.responder | ||
27 | 222 | def get_boot_sources_v2(self, uuid): | ||
28 | 223 | """get_boot_sources_v2() | 210 | """get_boot_sources_v2() |
29 | 224 | 211 | ||
30 | 225 | Implementation of | 212 | Implementation of |
31 | diff --git a/src/maasserver/rpc/tests/test_regionservice_calls.py b/src/maasserver/rpc/tests/test_regionservice_calls.py | |||
32 | index 2949f9e..604b168 100644 | |||
33 | --- a/src/maasserver/rpc/tests/test_regionservice_calls.py | |||
34 | +++ b/src/maasserver/rpc/tests/test_regionservice_calls.py | |||
35 | @@ -56,7 +56,6 @@ from provisioningserver.rpc.region import ( | |||
36 | 56 | GetArchiveMirrors, | 56 | GetArchiveMirrors, |
37 | 57 | GetBootConfig, | 57 | GetBootConfig, |
38 | 58 | GetBootSources, | 58 | GetBootSources, |
39 | 59 | GetBootSourcesV2, | ||
40 | 60 | GetControllerType, | 59 | GetControllerType, |
41 | 61 | GetDNSConfiguration, | 60 | GetDNSConfiguration, |
42 | 62 | GetProxies, | 61 | GetProxies, |
43 | @@ -341,26 +340,6 @@ class TestRegionProtocol_GetBootSources(MAASTransactionServerTestCase): | |||
44 | 341 | return d.addCallback(check) | 340 | return d.addCallback(check) |
45 | 342 | 341 | ||
46 | 343 | 342 | ||
47 | 344 | class TestRegionProtocol_GetBootSourcesV2(MAASTransactionServerTestCase): | ||
48 | 345 | def test_get_boot_sources_v2_is_registered(self): | ||
49 | 346 | protocol = Region() | ||
50 | 347 | responder = protocol.locateResponder(GetBootSourcesV2.commandName) | ||
51 | 348 | self.assertIsNotNone(responder) | ||
52 | 349 | |||
53 | 350 | @wait_for_reactor | ||
54 | 351 | def test_get_boot_sources_v2_returns_simplestreams_endpoint(self): | ||
55 | 352 | uuid = factory.make_name("uuid") | ||
56 | 353 | |||
57 | 354 | d = call_responder(Region(), GetBootSourcesV2, {"uuid": uuid}) | ||
58 | 355 | |||
59 | 356 | def check(response): | ||
60 | 357 | self.assertEqual( | ||
61 | 358 | {"sources": [get_simplestream_endpoint()]}, response | ||
62 | 359 | ) | ||
63 | 360 | |||
64 | 361 | return d.addCallback(check) | ||
65 | 362 | |||
66 | 363 | |||
67 | 364 | class TestRegionProtocol_GetArchiveMirrors(MAASTransactionServerTestCase): | 343 | class TestRegionProtocol_GetArchiveMirrors(MAASTransactionServerTestCase): |
68 | 365 | def test_get_archive_mirrors_is_registered(self): | 344 | def test_get_archive_mirrors_is_registered(self): |
69 | 366 | protocol = Region() | 345 | protocol = Region() |
70 | diff --git a/src/provisioningserver/rackdservices/image_download_service.py b/src/provisioningserver/rackdservices/image_download_service.py | |||
71 | index a86e879..87e8fb1 100644 | |||
72 | --- a/src/provisioningserver/rackdservices/image_download_service.py | |||
73 | +++ b/src/provisioningserver/rackdservices/image_download_service.py | |||
74 | @@ -1,4 +1,4 @@ | |||
76 | 1 | # Copyright 2014-2016 Canonical Ltd. This software is licensed under the | 1 | # Copyright 2014-2021 Canonical Ltd. This software is licensed under the |
77 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). |
78 | 3 | 3 | ||
79 | 4 | """Service to periodically refresh the boot images.""" | 4 | """Service to periodically refresh the boot images.""" |
80 | @@ -7,18 +7,13 @@ | |||
81 | 7 | from datetime import timedelta | 7 | from datetime import timedelta |
82 | 8 | 8 | ||
83 | 9 | from twisted.application.internet import TimerService | 9 | from twisted.application.internet import TimerService |
86 | 10 | from twisted.internet.defer import inlineCallbacks, returnValue | 10 | from twisted.internet.defer import inlineCallbacks |
85 | 11 | from twisted.protocols.amp import UnhandledCommand | ||
87 | 12 | 11 | ||
88 | 13 | from provisioningserver.boot import tftppath | 12 | from provisioningserver.boot import tftppath |
89 | 14 | from provisioningserver.logger import get_maas_logger, LegacyLogger | 13 | from provisioningserver.logger import get_maas_logger, LegacyLogger |
90 | 15 | from provisioningserver.rpc.boot_images import import_boot_images | 14 | from provisioningserver.rpc.boot_images import import_boot_images |
91 | 16 | from provisioningserver.rpc.exceptions import NoConnectionsAvailable | 15 | from provisioningserver.rpc.exceptions import NoConnectionsAvailable |
97 | 17 | from provisioningserver.rpc.region import ( | 16 | from provisioningserver.rpc.region import GetBootSources, GetProxies |
93 | 18 | GetBootSources, | ||
94 | 19 | GetBootSourcesV2, | ||
95 | 20 | GetProxies, | ||
96 | 21 | ) | ||
98 | 22 | from provisioningserver.utils.twisted import pause, retries | 17 | from provisioningserver.utils.twisted import pause, retries |
99 | 23 | 18 | ||
100 | 24 | maaslog = get_maas_logger("boot_image_download_service") | 19 | maaslog = get_maas_logger("boot_image_download_service") |
101 | @@ -58,22 +53,6 @@ class ImageDownloadService(TimerService): | |||
102 | 58 | return self.maybe_start_download().addErrback(download_failure) | 53 | return self.maybe_start_download().addErrback(download_failure) |
103 | 59 | 54 | ||
104 | 60 | @inlineCallbacks | 55 | @inlineCallbacks |
105 | 61 | def _get_boot_sources(self, client): | ||
106 | 62 | """Gets the boot sources from the region.""" | ||
107 | 63 | try: | ||
108 | 64 | sources = yield client(GetBootSourcesV2, uuid=client.localIdent) | ||
109 | 65 | except UnhandledCommand: | ||
110 | 66 | # Region has not been upgraded to support the new call, use the | ||
111 | 67 | # old call. The old call did not provide the new os selection | ||
112 | 68 | # parameter. Region does not support boot source selection by os, | ||
113 | 69 | # so its set too allow all operating systems. | ||
114 | 70 | sources = yield client(GetBootSources, uuid=client.localIdent) | ||
115 | 71 | for source in sources["sources"]: | ||
116 | 72 | for selection in source["selections"]: | ||
117 | 73 | selection["os"] = "*" | ||
118 | 74 | returnValue(sources) | ||
119 | 75 | |||
120 | 76 | @inlineCallbacks | ||
121 | 77 | def _start_download(self): | 56 | def _start_download(self): |
122 | 78 | client = None | 57 | client = None |
123 | 79 | # Retry a few times, since this service usually comes up before | 58 | # Retry a few times, since this service usually comes up before |
124 | @@ -91,7 +70,7 @@ class ImageDownloadService(TimerService): | |||
125 | 91 | return | 70 | return |
126 | 92 | 71 | ||
127 | 93 | # Get sources from region | 72 | # Get sources from region |
129 | 94 | sources = yield self._get_boot_sources(client) | 73 | sources = yield client(GetBootSources, uuid=client.localIdent) |
130 | 95 | # Get http proxy from region | 74 | # Get http proxy from region |
131 | 96 | proxies = yield client(GetProxies) | 75 | proxies = yield client(GetProxies) |
132 | 97 | 76 | ||
133 | diff --git a/src/provisioningserver/rackdservices/tests/test_image_download_service.py b/src/provisioningserver/rackdservices/tests/test_image_download_service.py | |||
134 | index e4e2462..63254a1 100644 | |||
135 | --- a/src/provisioningserver/rackdservices/tests/test_image_download_service.py | |||
136 | +++ b/src/provisioningserver/rackdservices/tests/test_image_download_service.py | |||
137 | @@ -1,24 +1,22 @@ | |||
139 | 1 | # Copyright 2014-2016 Canonical Ltd. This software is licensed under the | 1 | # Copyright 2014-2021 Canonical Ltd. This software is licensed under the |
140 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). |
141 | 3 | 3 | ||
142 | 4 | """Tests for provisioningserver.rackdservices.image_download_service""" | 4 | """Tests for provisioningserver.rackdservices.image_download_service""" |
143 | 5 | 5 | ||
144 | 6 | 6 | ||
145 | 7 | from datetime import timedelta | 7 | from datetime import timedelta |
147 | 8 | from unittest.mock import call, Mock, sentinel | 8 | from unittest.mock import Mock, sentinel |
148 | 9 | from urllib.parse import urlparse | 9 | from urllib.parse import urlparse |
149 | 10 | 10 | ||
150 | 11 | from fixtures import FakeLogger | 11 | from fixtures import FakeLogger |
151 | 12 | from twisted.application.internet import TimerService | 12 | from twisted.application.internet import TimerService |
152 | 13 | from twisted.internet import defer | 13 | from twisted.internet import defer |
153 | 14 | from twisted.internet.task import Clock | 14 | from twisted.internet.task import Clock |
154 | 15 | from twisted.protocols.amp import UnhandledCommand | ||
155 | 16 | 15 | ||
156 | 17 | from maastesting.factory import factory | 16 | from maastesting.factory import factory |
157 | 18 | from maastesting.matchers import ( | 17 | from maastesting.matchers import ( |
158 | 19 | get_mock_calls, | 18 | get_mock_calls, |
159 | 20 | MockCalledOnceWith, | 19 | MockCalledOnceWith, |
160 | 21 | MockCallsMatch, | ||
161 | 22 | MockNotCalled, | 20 | MockNotCalled, |
162 | 23 | ) | 21 | ) |
163 | 24 | from maastesting.testcase import MAASTestCase, MAASTwistedRunTest | 22 | from maastesting.testcase import MAASTestCase, MAASTwistedRunTest |
164 | @@ -30,7 +28,6 @@ from provisioningserver.rackdservices.image_download_service import ( | |||
165 | 30 | from provisioningserver.rpc import boot_images | 28 | from provisioningserver.rpc import boot_images |
166 | 31 | from provisioningserver.rpc.boot_images import _run_import | 29 | from provisioningserver.rpc.boot_images import _run_import |
167 | 32 | from provisioningserver.rpc.exceptions import NoConnectionsAvailable | 30 | from provisioningserver.rpc.exceptions import NoConnectionsAvailable |
168 | 33 | from provisioningserver.rpc.region import GetBootSources, GetBootSourcesV2 | ||
169 | 34 | 31 | ||
170 | 35 | 32 | ||
171 | 36 | class TestPeriodicImageDownloadService(MAASTestCase): | 33 | class TestPeriodicImageDownloadService(MAASTestCase): |
172 | @@ -196,83 +193,3 @@ class TestPeriodicImageDownloadService(MAASTestCase): | |||
173 | 196 | """, | 193 | """, |
174 | 197 | logger.output, | 194 | logger.output, |
175 | 198 | ) | 195 | ) |
176 | 199 | |||
177 | 200 | |||
178 | 201 | class TestGetBootSources(MAASTestCase): | ||
179 | 202 | |||
180 | 203 | run_tests_with = MAASTwistedRunTest.make_factory(timeout=5) | ||
181 | 204 | |||
182 | 205 | @defer.inlineCallbacks | ||
183 | 206 | def test_get_boot_sources_calls_get_boot_sources_v2_before_v1(self): | ||
184 | 207 | clock = Clock() | ||
185 | 208 | client_call = Mock() | ||
186 | 209 | client_call.side_effect = [ | ||
187 | 210 | defer.succeed(dict(sources=sentinel.sources)) | ||
188 | 211 | ] | ||
189 | 212 | client_call.localIdent = factory.make_UUID() | ||
190 | 213 | |||
191 | 214 | service = ImageDownloadService(sentinel.rpc, sentinel.tftp_root, clock) | ||
192 | 215 | sources = yield service._get_boot_sources(client_call) | ||
193 | 216 | self.assertEqual(sources.get("sources"), sentinel.sources) | ||
194 | 217 | self.assertThat( | ||
195 | 218 | client_call, | ||
196 | 219 | MockCalledOnceWith(GetBootSourcesV2, uuid=client_call.localIdent), | ||
197 | 220 | ) | ||
198 | 221 | |||
199 | 222 | @defer.inlineCallbacks | ||
200 | 223 | def test_get_boot_sources_calls_get_boot_sources_v1_on_v2_missing(self): | ||
201 | 224 | clock = Clock() | ||
202 | 225 | client_call = Mock() | ||
203 | 226 | client_call.side_effect = [ | ||
204 | 227 | defer.fail(UnhandledCommand()), | ||
205 | 228 | defer.succeed(dict(sources=[])), | ||
206 | 229 | ] | ||
207 | 230 | client_call.localIdent = factory.make_UUID() | ||
208 | 231 | |||
209 | 232 | service = ImageDownloadService(sentinel.rpc, sentinel.tftp_root, clock) | ||
210 | 233 | yield service._get_boot_sources(client_call) | ||
211 | 234 | self.assertThat( | ||
212 | 235 | client_call, | ||
213 | 236 | MockCallsMatch( | ||
214 | 237 | call(GetBootSourcesV2, uuid=client_call.localIdent), | ||
215 | 238 | call(GetBootSources, uuid=client_call.localIdent), | ||
216 | 239 | ), | ||
217 | 240 | ) | ||
218 | 241 | |||
219 | 242 | @defer.inlineCallbacks | ||
220 | 243 | def test_get_boot_sources_v1_sets_os_to_wildcard(self): | ||
221 | 244 | sources = [ | ||
222 | 245 | { | ||
223 | 246 | "path": factory.make_url(), | ||
224 | 247 | "selections": [ | ||
225 | 248 | { | ||
226 | 249 | "release": "trusty", | ||
227 | 250 | "arches": ["amd64"], | ||
228 | 251 | "subarches": ["generic"], | ||
229 | 252 | "labels": ["release"], | ||
230 | 253 | }, | ||
231 | 254 | { | ||
232 | 255 | "release": "precise", | ||
233 | 256 | "arches": ["amd64"], | ||
234 | 257 | "subarches": ["generic"], | ||
235 | 258 | "labels": ["release"], | ||
236 | 259 | }, | ||
237 | 260 | ], | ||
238 | 261 | } | ||
239 | 262 | ] | ||
240 | 263 | |||
241 | 264 | clock = Clock() | ||
242 | 265 | client_call = Mock() | ||
243 | 266 | client_call.side_effect = [ | ||
244 | 267 | defer.fail(UnhandledCommand()), | ||
245 | 268 | defer.succeed(dict(sources=sources)), | ||
246 | 269 | ] | ||
247 | 270 | |||
248 | 271 | service = ImageDownloadService(sentinel.rpc, sentinel.tftp_root, clock) | ||
249 | 272 | sources = yield service._get_boot_sources(client_call) | ||
250 | 273 | os_selections = [ | ||
251 | 274 | selection.get("os") | ||
252 | 275 | for source in sources["sources"] | ||
253 | 276 | for selection in source["selections"] | ||
254 | 277 | ] | ||
255 | 278 | self.assertEqual(["*", "*"], os_selections) | ||
256 | diff --git a/src/provisioningserver/rpc/region.py b/src/provisioningserver/rpc/region.py | |||
257 | index 6c1a741..990632a 100644 | |||
258 | --- a/src/provisioningserver/rpc/region.py | |||
259 | +++ b/src/provisioningserver/rpc/region.py | |||
260 | @@ -1,4 +1,4 @@ | |||
262 | 1 | # Copyright 2014-2016 Canonical Ltd. This software is licensed under the | 1 | # Copyright 2014-2021 Canonical Ltd. This software is licensed under the |
263 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). |
264 | 3 | 3 | ||
265 | 4 | """RPC declarations for the region. | 4 | """RPC declarations for the region. |
266 | @@ -13,7 +13,6 @@ __all__ = [ | |||
267 | 13 | "GetArchiveMirrors", | 13 | "GetArchiveMirrors", |
268 | 14 | "GetBootConfig", | 14 | "GetBootConfig", |
269 | 15 | "GetBootSources", | 15 | "GetBootSources", |
270 | 16 | "GetBootSourcesV2", | ||
271 | 17 | "GetControllerType", | 16 | "GetControllerType", |
272 | 18 | "GetDiscoveryState", | 17 | "GetDiscoveryState", |
273 | 19 | "GetDNSConfiguration", | 18 | "GetDNSConfiguration", |
274 | @@ -162,42 +161,6 @@ class GetBootConfig(amp.Command): | |||
275 | 162 | class GetBootSources(amp.Command): | 161 | class GetBootSources(amp.Command): |
276 | 163 | """Report boot sources and selections for the given cluster. | 162 | """Report boot sources and selections for the given cluster. |
277 | 164 | 163 | ||
278 | 165 | :since: 1.6 | ||
279 | 166 | :deprecated: 1.7 | ||
280 | 167 | """ | ||
281 | 168 | |||
282 | 169 | arguments = [ | ||
283 | 170 | # The cluster UUID. | ||
284 | 171 | (b"uuid", amp.Unicode()) | ||
285 | 172 | ] | ||
286 | 173 | response = [ | ||
287 | 174 | ( | ||
288 | 175 | b"sources", | ||
289 | 176 | AmpList( | ||
290 | 177 | [ | ||
291 | 178 | (b"url", amp.Unicode()), | ||
292 | 179 | (b"keyring_data", Bytes()), | ||
293 | 180 | ( | ||
294 | 181 | b"selections", | ||
295 | 182 | AmpList( | ||
296 | 183 | [ | ||
297 | 184 | (b"release", amp.Unicode()), | ||
298 | 185 | (b"arches", amp.ListOf(amp.Unicode())), | ||
299 | 186 | (b"subarches", amp.ListOf(amp.Unicode())), | ||
300 | 187 | (b"labels", amp.ListOf(amp.Unicode())), | ||
301 | 188 | ] | ||
302 | 189 | ), | ||
303 | 190 | ), | ||
304 | 191 | ] | ||
305 | 192 | ), | ||
306 | 193 | ) | ||
307 | 194 | ] | ||
308 | 195 | errors = [] | ||
309 | 196 | |||
310 | 197 | |||
311 | 198 | class GetBootSourcesV2(amp.Command): | ||
312 | 199 | """Report boot sources and selections for the given cluster. | ||
313 | 200 | |||
314 | 201 | Includes the new os field for the selections. | 164 | Includes the new os field for the selections. |
315 | 202 | 165 | ||
316 | 203 | :since: 1.7 | 166 | :since: 1.7 |
+1