From ff46c48d605e05a6fce305915d25f31b571dcd76 Mon Sep 17 00:00:00 2001 From: Olivier Gayot Date: Tue, 14 Feb 2023 19:19:20 +0100 Subject: [PATCH] mirror: use apt->mirror-selection instead of relying on version We used to rely on a version key under apt autoinstall section to specify if we want the old implementation (i.e., a single candidate primary mirror) or the new implementation (i.e., multiple primary mirror candidates). Instead, we now introduce the apt->mirror-selection autoinstall section, and move the primary section under it. If the primary section if under apt, we want the old implementation. If the primary section is under apt->mirror-selection, we want the new implementation. Signed-off-by: Olivier Gayot --- autoinstall-schema.json | 36 +++++++++++- examples/autoinstall.yaml | 10 ++-- subiquity/models/mirror.py | 58 +++++++++++-------- subiquity/models/tests/test_mirror.py | 32 ++++------ subiquity/server/controllers/mirror.py | 29 +++++++++- .../server/controllers/tests/test_mirror.py | 13 +++++ 6 files changed, 121 insertions(+), 57 deletions(-) diff --git a/autoinstall-schema.json b/autoinstall-schema.json index 8ed8ba98..9fa31681 100644 --- a/autoinstall-schema.json +++ b/autoinstall-schema.json @@ -307,15 +307,45 @@ "apt": { "type": "object", "properties": { - "version": { - "type": "integer" - }, "preserve_sources_list": { "type": "boolean" }, "primary": { "type": "array" }, + "mirror-selection": { + "type": "object", + "properties": { + "primary": { + "type": "array", + "items": { + "anyOf": [ + { + "type": "string", + "const": "country-mirror" + }, + { + "type": "object", + "properties": { + "uri": { + "type": "string" + }, + "arches": { + "type": "array", + "items": { + "type": "string" + } + } + }, + "required": [ + "uri" + ] + } + ] + } + } + } + }, "geoip": { "type": "boolean" }, diff --git a/examples/autoinstall.yaml b/examples/autoinstall.yaml index 8a6f2a38..5fa2055b 100644 --- a/examples/autoinstall.yaml +++ b/examples/autoinstall.yaml @@ -16,11 +16,11 @@ network: dhcp6: yes debconf-selections: eek apt: - version: 2 - primary: - - uri: http://mymirror.local/repository/Apt/ubuntu/ - - country-mirror - - uri: http://archive.ubuntu.com/ubuntu + mirror-selection: + primary: + - uri: http://mymirror.local/repository/Apt/ubuntu/ + - country-mirror + - uri: http://archive.ubuntu.com/ubuntu disable_components: - non-free - restricted diff --git a/subiquity/models/mirror.py b/subiquity/models/mirror.py index 4c4874e7..bcfb2062 100644 --- a/subiquity/models/mirror.py +++ b/subiquity/models/mirror.py @@ -50,18 +50,21 @@ elected primary section --------------- - * the "primary section" is what corresponds to the whole 'apt->primary' - autoinstall section. Today we support two formats for this - section: - * the legacy format, inherited from curtin, where the whole section denotes - a single primary candidate. This format cannot be used to specify multiple + * the "primary section" contains the different candidates for mirror + selection. Today we support two different formats for this section, and the + position of the primary section is what determines which format is used. + * the legacy format, inherited from curtin, where the primary section is a + direct child of the 'apt' section. In this format, the whole section denotes + a single primary candidate so it cannot be used to specify multiple candidates. - * the more modern format where the primary section is split into multiple - "entries", each denoting a primary candidate. + * the more modern format where the primary section is a child of the + 'mirror-selection' key (which itself is a child of 'apt'). In this format, + the section is split into multiple "entries", each denoting a primary + candidate. primary entry ------------- - * represents a fragment of the 'apt->primary' autoinstall section. Each entry + * represents a fragment of the 'primary' autoinstall section. Each entry can be used as a primary candidate. * in the legacy format, the primary entry corresponds to the whole primary section. @@ -112,7 +115,7 @@ DEFAULT = { @attr.s(auto_attribs=True) class BasePrimaryEntry(abc.ABC): - """ Base class to represent an entry from the 'apt->primary' autoinstall + """ Base class to represent an entry from the 'primary' autoinstall section. A BasePrimaryEntry is expected to have a URI and therefore can be used as a primary candidate. """ parent: "MirrorModel" = attr.ib(kw_only=True) @@ -131,8 +134,8 @@ class BasePrimaryEntry(abc.ABC): @attr.s(auto_attribs=True) class PrimaryEntry(BasePrimaryEntry): """ Represents a single primary mirror candidate; which can be converted - to/from an entry of the 'apt->primary' autoinstall section in the modern - format. """ + to/from an entry of the 'apt->mirror-selection->primary' autoinstall + section. """ # Having uri set to None is only valid for a country mirror. uri: Optional[str] = None # When arches is None, it is assumed that the mirror is compatible with the @@ -247,21 +250,27 @@ class MirrorModel(object): return self._default_primary_entries() def load_autoinstall_data(self, data): - self.legacy_primary = data.pop("version", 1) < 2 if "disable_components" in data: self.disabled_components = set(data.pop("disable_components")) + + if "primary" in data and "mirror-selection" in data: + raise ValueError("apt->primary and apt->mirror-selection are" + " mutually exclusive.") + self.legacy_primary = "primary" in data + + primary_candidates = self.get_default_primary_candidates() if "primary" in data: - if self.legacy_primary: - # Legacy sections only support a single candidate - self.primary_candidates = \ - [LegacyPrimaryEntry(data.pop("primary"), parent=self)] - else: - self.primary_candidates = [] - for section in data.pop("primary"): + # Legacy sections only support a single candidate + primary_candidates = \ + [LegacyPrimaryEntry(data.pop("primary"), parent=self)] + if "mirror-selection" in data: + mirror_selection = data.pop("mirror-selection") + if "primary" in mirror_selection: + primary_candidates = [] + for section in mirror_selection["primary"]: entry = PrimaryEntry.from_config(section, parent=self) - self.primary_candidates.append(entry) - else: - self.primary_candidates = self.get_default_primary_candidates() + primary_candidates.append(entry) + self.primary_candidates = primary_candidates merge_config(self.config, data) @@ -368,7 +377,6 @@ class MirrorModel(object): def make_autoinstall(self): config = self._get_apt_config_common() - config["version"] = 1 if self.legacy_primary else 2 if self.legacy_primary: # Only one candidate is supported if self.primary_elected is not None: @@ -378,7 +386,7 @@ class MirrorModel(object): to_serialize = self.primary_candidates[0] config["primary"] = to_serialize.serialize_for_ai() else: - config["primary"] = \ - [c.serialize_for_ai() for c in self.primary_candidates] + primary = [c.serialize_for_ai() for c in self.primary_candidates] + config["mirror-selection"] = {"primary": primary} return config diff --git a/subiquity/models/tests/test_mirror.py b/subiquity/models/tests/test_mirror.py index 06c689b8..6776ba72 100644 --- a/subiquity/models/tests/test_mirror.py +++ b/subiquity/models/tests/test_mirror.py @@ -179,32 +179,22 @@ class TestMirrorModel(unittest.TestCase): model = MirrorModel() data = {'disable_components': ['non-free']} model.load_autoinstall_data(data) - self.assertTrue(model.legacy_primary) + self.assertFalse(model.legacy_primary) model.primary_candidates[0].stage() self.assertEqual(set(['non-free']), model.disabled_components) - - model = MirrorModel() - data = {"version": 1} - model.load_autoinstall_data(data) - self.assertTrue(model.legacy_primary) - self.assertEqual(model.primary_candidates, - [LegacyPrimaryEntry.new_from_default(parent=model)]) - - data = {"version": 2} - model.load_autoinstall_data(data) - self.assertFalse(model.legacy_primary) self.assertEqual(model.primary_candidates, model._default_primary_entries()) def test_from_autoinstall_modern(self): data = { - "version": 2, - "primary": [ - "country-mirror", - { - "uri": "http://mirror", - }, - ] + "mirror-selection": { + "primary": [ + "country-mirror", + { + "uri": "http://mirror", + }, + ], + } } model = MirrorModel() model.load_autoinstall_data(data) @@ -256,8 +246,7 @@ class TestMirrorModel(unittest.TestCase): ] cfg = self.model.make_autoinstall() self.assertEqual(cfg["disable_components"], ["non-free"]) - self.assertEqual(cfg["primary"], expected_primary) - self.assertEqual(cfg["version"], 2) + self.assertEqual(cfg["mirror-selection"]["primary"], expected_primary) def test_make_autoinstall_legacy_primary(self): primary = [{"arches": "amd64", "uri": "http://mirror"}] @@ -269,7 +258,6 @@ class TestMirrorModel(unittest.TestCase): cfg = self.model.make_autoinstall() self.assertEqual(cfg["disable_components"], ["non-free"]) self.assertEqual(cfg["primary"], primary) - self.assertEqual(cfg["version"], 1) def test_create_primary_candidate(self): self.model.legacy_primary = False diff --git a/subiquity/server/controllers/mirror.py b/subiquity/server/controllers/mirror.py index 171c2049..8af48603 100644 --- a/subiquity/server/controllers/mirror.py +++ b/subiquity/server/controllers/mirror.py @@ -62,9 +62,34 @@ class MirrorController(SubiquityController): autoinstall_schema = { # This is obviously incomplete. 'type': 'object', 'properties': { - 'version': {'type': 'integer'}, 'preserve_sources_list': {'type': 'boolean'}, - 'primary': {'type': 'array'}, + 'primary': {'type': 'array'}, # Legacy format defined by curtin. + 'mirror-selection': { + 'type': 'object', + 'properties': { + 'primary': { + 'type': 'array', + 'items': { + 'anyOf': [ + { + 'type': 'string', + 'const': 'country-mirror', + }, { + 'type': 'object', + 'properties': { + 'uri': {'type': 'string'}, + 'arches': { + 'type': 'array', + 'items': {'type': 'string'}, + }, + }, + 'required': ['uri'], + }, + ], + }, + }, + }, + }, 'geoip': {'type': 'boolean'}, 'sources': {'type': 'object'}, 'disable_components': { diff --git a/subiquity/server/controllers/tests/test_mirror.py b/subiquity/server/controllers/tests/test_mirror.py index a57c599b..1fff2d4f 100644 --- a/subiquity/server/controllers/tests/test_mirror.py +++ b/subiquity/server/controllers/tests/test_mirror.py @@ -57,8 +57,21 @@ class TestMirrorController(unittest.IsolatedAsyncioTestCase): self.controller.model.primary_candidates[0].elect() config = self.controller.make_autoinstall() self.assertIn("disable_components", config.keys()) + self.assertIn("mirror-selection", config.keys()) + self.assertIn("geoip", config.keys()) + self.assertNotIn("primary", config.keys()) + + def test_make_autoinstall_legacy(self): + self.controller.model = MirrorModel() + self.controller.model.legacy_primary = True + self.controller.model.primary_candidates = \ + self.controller.model.get_default_primary_candidates() + self.controller.model.primary_candidates[0].elect() + config = self.controller.make_autoinstall() + self.assertIn("disable_components", config.keys()) self.assertIn("primary", config.keys()) self.assertIn("geoip", config.keys()) + self.assertNotIn("mirror-selection", config.keys()) async def test_run_mirror_testing(self): def fake_mirror_check_success(output):