From 1350ef8d1b17f3d1063789c46dec32fd5d0ea114 Mon Sep 17 00:00:00 2001 From: Olivier Gayot Date: Wed, 15 Feb 2023 10:55:56 +0100 Subject: [PATCH] mirror: do not raise from POST /mirror if no usable mirror Raising from the POST /mirror handler makes it difficult for the client to distinguish an internal error from the lack of usable mirror. We now return an enumeration with the following possible values: * ok * no-usable-mirror It is up to the client to either: 1. adjust the network settings and retry 2. give up on the install Signed-off-by: Olivier Gayot --- subiquity/common/apidef.py | 5 ++++- subiquity/common/types.py | 5 +++++ subiquity/server/controllers/mirror.py | 24 +++++++++++++++++------- 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/subiquity/common/apidef.py b/subiquity/common/apidef.py index 5b69b0bf..382a23da 100644 --- a/subiquity/common/apidef.py +++ b/subiquity/common/apidef.py @@ -45,6 +45,7 @@ from subiquity.common.types import ( NetworkStatus, MirrorGet, MirrorPost, + MirrorPostResponse, MirrorCheckResponse, ModifyPartitionV2, ReformatDisk, @@ -351,7 +352,9 @@ class API: class mirror: def GET() -> MirrorGet: ... - def POST(data: Payload[Optional[MirrorPost]]) -> None: ... + + def POST(data: Payload[Optional[MirrorPost]]) \ + -> MirrorPostResponse: ... class disable_components: def GET() -> List[str]: ... diff --git a/subiquity/common/types.py b/subiquity/common/types.py index 1d366fb3..094ad7f0 100644 --- a/subiquity/common/types.py +++ b/subiquity/common/types.py @@ -754,6 +754,11 @@ class MirrorPost: staged: Optional[str] = None +class MirrorPostResponse(enum.Enum): + OK = "ok" + NO_USABLE_MIRROR = "no-usable-mirror" + + @attr.s(auto_attribs=True) class MirrorGet: elected: Optional[str] diff --git a/subiquity/server/controllers/mirror.py b/subiquity/server/controllers/mirror.py index 8af48603..2ffb6368 100644 --- a/subiquity/server/controllers/mirror.py +++ b/subiquity/server/controllers/mirror.py @@ -29,6 +29,7 @@ from subiquity.common.types import ( MirrorCheckStatus, MirrorGet, MirrorPost, + MirrorPostResponse, ) from subiquity.server.apt import get_apt_configurer, AptConfigCheckError from subiquity.server.controller import SubiquityController @@ -291,15 +292,23 @@ class MirrorController(SubiquityController): candidates = [c.uri for c in compatibles if c.uri is not None] return MirrorGet(elected=elected, candidates=candidates, staged=staged) - async def POST(self, data: Optional[MirrorPost]) -> None: + async def POST(self, data: Optional[MirrorPost]) -> MirrorPostResponse: log.debug(data) if data is None: - # TODO If we want the ability to fallback to an offline install, we - # probably need to catch NoUsableMirrorError and inform the client - # somehow. - await self.find_and_elect_candidate_mirror(self.context) - await self.configured() - return + # If this call fails with NoUsableMirrorError, we do not + # automatically apply the fallback method. Instead, we let the + # client know that they need to adjust something. Disabling the + # network would be one way to do it. The client can also consider + # this a fatal error and give up on the install. + try: + await self.find_and_elect_candidate_mirror(self.context) + except NoUsableMirrorError: + log.warning("found no usable mirror, expecting the client to" + " give up or to adjust the settings and retry") + return MirrorPostResponse.NO_USABLE_MIRROR + else: + await self.configured() + return MirrorPostResponse.OK if data.candidates is not None: if not data.candidates: @@ -329,6 +338,7 @@ class MirrorController(SubiquityController): ensure_elected_in_candidates() await self.configured() + return MirrorPostResponse.OK async def disable_components_GET(self) -> List[str]: return sorted(self.model.disabled_components)