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 <olivier.gayot@canonical.com>

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"
                 },

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

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

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

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': {

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):