Merge ~cgrabowski/maas:fix_lp1982866_3.1_backport into maas:3.1

[Christian Grabowski]

use BootResource's method for splitting base image to get a default value in get_boot_config_for_machine()
(cherry picked from commit d58366ec06df69f22273b72338e80e9459ee3f7c)

[MAAS Lander (maas-lander)]

UNIT TESTS
-b fix_lp1982866_3.1_backport lp:~cgrabowski/maas/+git/maas into -b 3.1 lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas-tester/753/consoleText
COMMIT: 446f3819632f3ea854ba22bca64e59c2739031f6

review: Needs Fixing

[MAAS Lander (maas-lander)]

UNIT TESTS
-b fix_lp1982866_3.1_backport lp:~cgrabowski/maas/+git/maas into -b 3.1 lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas-tester/756/consoleText
COMMIT: cd09b8b718cbbd68b07c4c9df9b3d7091e57c867

review: Needs Fixing

[Christian Grabowski (cgrabowski)]

jenkins: !test

[MAAS Lander (maas-lander)]

UNIT TESTS
-b fix_lp1982866_3.1_backport lp:~cgrabowski/maas/+git/maas into -b 3.1 lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: cd09b8b718cbbd68b07c4c9df9b3d7091e57c867

review: Approve

[Christian Grabowski (cgrabowski)]

self-approving backport

diff --git a/src/maasserver/forms/__init__.py b/src/maasserver/forms/__init__.py
index d88c443..2c0cf46 100644
--- a/src/maasserver/forms/__init__.py
+++ b/src/maasserver/forms/__init__.py
@@ -2493,7 +2493,7 @@ class BootResourceForm(MAASModelForm):
         return name
 
     def _get_base_image_info(self):
-        base_image = self.cleaned_data["base_image"]
+        base_image = self.cleaned_data.get("base_image")
         if not base_image:
             existing_resource = super().save(commit=False)
             existing_resource.name = self.cleaned_data["name"]
@@ -2522,9 +2522,22 @@ class BootResourceForm(MAASModelForm):
         try:
             base_osystem, base_version = self._get_base_image_info()
         except ValueError:
-            raise ValidationError(
-                "custom images require a valid base image name"
-            )
+            if not self.data.get("base_image"):
+                return "/".join(
+                    [
+                        val
+                        for val in Config.objects.get_configs(
+                            [
+                                "commissioning_osystem",
+                                "commissioning_distro_series",
+                            ]
+                        ).values()
+                    ]
+                )
+            else:
+                raise ValidationError(
+                    "a base image must follow the format: <osystem>/<series>"
+                )
         else:
             if base_osystem not in ("ubuntu", "centos", "rhel"):
                 raise ValidationError(
diff --git a/src/maasserver/forms/tests/test_bootresource.py b/src/maasserver/forms/tests/test_bootresource.py
index e8aaeec..4544a1d 100644
--- a/src/maasserver/forms/tests/test_bootresource.py
+++ b/src/maasserver/forms/tests/test_bootresource.py
@@ -10,7 +10,7 @@ from django.core.files.uploadedfile import SimpleUploadedFile
 
 from maasserver.enum import BOOT_RESOURCE_FILE_TYPE, BOOT_RESOURCE_TYPE
 from maasserver.forms import BootResourceForm
-from maasserver.models import BootResource, BootResourceFile
+from maasserver.models import BootResource, BootResourceFile, Config
 from maasserver.models.signals import bootresourcefiles, bootsources
 from maasserver.testing.architecture import make_usable_architecture
 from maasserver.testing.factory import factory
@@ -303,7 +303,7 @@ class TestBootResourceForm(MAASServerTestCase):
         image = form.save()
         self.assertEqual(image.base_image, data["base_image"])
 
-    def test_invalidates_nonexistent_custom_image_base_os(self):
+    def test_uses_commissioning_os_for_nonexistent_custom_image_base_os(self):
         name = "custom/%s" % factory.make_name("name")
         upload_type, filetype = self.pick_filetype()
         size = random.randint(1024, 2048)
@@ -315,10 +315,17 @@ class TestBootResourceForm(MAASServerTestCase):
             "title": factory.make_name("title"),
             "architecture": make_usable_architecture(self),
             "filetype": upload_type,
-            "base_image": factory.make_name("invalid"),
+            "base_image": "",
         }
         form = BootResourceForm(data=data, files={"content": uploaded_file})
-        self.assertFalse(form.is_valid())
+        form.save()
+        cfg = Config.objects.get_configs(
+            ["commissioning_osystem", "commissioning_distro_series"]
+        )
+        self.assertEqual(
+            f"{cfg['commissioning_osystem']}/{cfg['commissioning_distro_series']}",
+            form.instance.base_image,
+        )
 
     def test_invalidates_nonexistent_custom_image_base_os_no_prefix(self):
         name = factory.make_name("name")
@@ -485,7 +492,7 @@ class TestBootResourceForm(MAASServerTestCase):
         form = BootResourceForm(data={})
         self.assertFalse(form.is_valid(), form.errors)
         self.assertEqual(
-            {"name", "architecture", "filetype", "content", "base_image"},
+            {"name", "architecture", "filetype", "content"},
             form.errors.keys(),
         )
 
@@ -522,3 +529,31 @@ class TestBootResourceForm(MAASServerTestCase):
                 resource_set__resource=resource
             ).count(),
         )
+
+    def test_clean_base_image_sets_commissioning_osystem_and_distro_series_where_none_is_given(
+        self,
+    ):
+        architecture = make_usable_architecture(self)
+        upload_type, filetype = self.pick_filetype()
+        content = factory.make_string(1024).encode("utf-8")
+        upload_name = factory.make_name("filename")
+        uploaded_file = SimpleUploadedFile(content=content, name=upload_name)
+        data = {
+            "name": f"custom/{factory.make_name()}",
+            "architecture": architecture,
+            "filetype": upload_type,
+        }
+        form = BootResourceForm(data=data, files={"content": uploaded_file})
+        form.save()
+        cfg = Config.objects.get_configs(
+            ["commissioning_osystem", "commissioning_distro_series"]
+        )
+        self.assertEqual(
+            "/".join(
+                [
+                    cfg["commissioning_osystem"],
+                    cfg["commissioning_distro_series"],
+                ]
+            ),
+            form.instance.base_image,
+        )
diff --git a/src/maasserver/models/bootresource.py b/src/maasserver/models/bootresource.py
index 0c16c67..39b6a66 100644
--- a/src/maasserver/models/bootresource.py
+++ b/src/maasserver/models/bootresource.py
@@ -28,6 +28,7 @@ from maasserver.fields import JSONObjectField
 from maasserver.models.bootresourceset import BootResourceSet
 from maasserver.models.bootsourcecache import BootSourceCache
 from maasserver.models.cleansave import CleanSave
+from maasserver.models.config import Config
 from maasserver.models.timestampedmodel import now, TimestampedModel
 from maasserver.utils.orm import get_first, get_one
 from provisioningserver.drivers.osystem import OperatingSystemRegistry
@@ -582,6 +583,16 @@ class BootResource(CleanSave, TimestampedModel):
         return self.architecture.split("/")
 
     def split_base_image(self):
+        # handle older custom images that may not have a base image
+        if not self.base_image:
+            cfg = Config.objects.get_configs(
+                ["commissioning_osystem", "commissioning_distro_series"]
+            )
+            return (
+                cfg["commissioning_osystem"],
+                cfg["commissioning_distro_series"],
+            )
+
         return self.base_image.split("/")
 
     def get_next_version_name(self):
diff --git a/src/maasserver/models/tests/test_bootresource.py b/src/maasserver/models/tests/test_bootresource.py
index e4280b0..ba3dc71 100644
--- a/src/maasserver/models/tests/test_bootresource.py
+++ b/src/maasserver/models/tests/test_bootresource.py
@@ -21,6 +21,7 @@ from maasserver.models.bootresource import (
     BootResource,
     RTYPE_REQUIRING_OS_SERIES_NAME,
 )
+from maasserver.models.config import Config
 from maasserver.models.signals import bootsources
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import MAASServerTestCase
@@ -1155,3 +1156,14 @@ class TestBootResource(MAASServerTestCase):
         self.assertFalse(
             resource.supports_subarch(factory.make_name("subarch"))
         )
+
+    def test_split_base_image_handles_no_base_image(self):
+        resource = factory.make_BootResource()
+        resource.base_image = None
+        cfg = Config.objects.get_configs(
+            ["commissioning_osystem", "commissioning_distro_series"]
+        )
+        self.assertCountEqual(
+            (cfg["commissioning_osystem"], cfg["commissioning_distro_series"]),
+            resource.split_base_image(),
+        )
diff --git a/src/maasserver/rpc/boot.py b/src/maasserver/rpc/boot.py
index 12f14b4..22f4102 100644
--- a/src/maasserver/rpc/boot.py
+++ b/src/maasserver/rpc/boot.py
@@ -231,10 +231,7 @@ def get_boot_config_for_machine(machine, configs, purpose):
             install_image = None
             if osystem == "custom":
                 install_image = BootResource.objects.get(name=series)
-                if install_image.base_image is not None:
-                    # if the operating system is a custom ubuntu image,
-                    # use the base image to install
-                    osystem, series = install_image.base_image.split("/")
+                osystem, series = install_image.split_base_image()
 
             if install_image is None or osystem != "ubuntu":
                 # Use only the commissioning osystem and series, for operating
diff --git a/src/maasserver/rpc/tests/test_boot.py b/src/maasserver/rpc/tests/test_boot.py
index 517dc8e..a0cae0d 100644
--- a/src/maasserver/rpc/tests/test_boot.py
+++ b/src/maasserver/rpc/tests/test_boot.py
@@ -24,7 +24,11 @@ from maasserver.models.timestampedmodel import now
 from maasserver.node_status import get_node_timeout, MONITORED_STATUSES
 from maasserver.preseed import compose_enlistment_preseed_url
 from maasserver.rpc import boot as boot_module
-from maasserver.rpc.boot import event_log_pxe_request, get_boot_filenames
+from maasserver.rpc.boot import (
+    event_log_pxe_request,
+    get_boot_config_for_machine,
+    get_boot_filenames,
+)
 from maasserver.rpc.boot import get_config as orig_get_config
 from maasserver.rpc.boot import merge_kparams_with_extra
 from maasserver.testing.architecture import make_usable_architecture
@@ -1520,3 +1524,104 @@ class TestGetBootFilenames(MAASServerTestCase):
             initrd,
         )
         self.assertIsNone(boot_dbt)
+
+
+class TestGetBootConfigForMachine(MAASServerTestCase):
+    def test_get_boot_config_for_machine_builtin_image(self):
+        machine = factory.make_Machine(
+            status=NODE_STATUS.DEPLOYING,
+            osystem="ubuntu",
+            distro_series="focal",
+        )
+        configs = Config.objects.get_configs(
+            [
+                "commissioning_osystem",
+                "commissioning_distro_series",
+                "enable_third_party_drivers",
+                "default_min_hwe_kernel",
+                "default_osystem",
+                "default_distro_series",
+                "kernel_opts",
+                "use_rack_proxy",
+                "maas_internal_domain",
+                "remote_syslog",
+                "maas_syslog_port",
+            ]
+        )
+
+        osystem, series, config_arch = get_boot_config_for_machine(
+            machine, configs, "xinstall"
+        )
+
+        self.assertEqual(osystem, "ubuntu")
+        self.assertEqual(series, "focal")
+        self.assertEqual(config_arch, "generic")
+
+    def test_get_boot_config_for_machine_new_custom_image(self):
+        arch = make_usable_architecture(self)
+        boot_resource = factory.make_BootResource(
+            base_image="ubuntu/jammy", architecture=arch
+        )
+        machine = factory.make_Machine(
+            status=NODE_STATUS.DEPLOYING,
+            osystem="custom",
+            distro_series=boot_resource.name,
+        )
+        configs = Config.objects.get_configs(
+            [
+                "commissioning_osystem",
+                "commissioning_distro_series",
+                "enable_third_party_drivers",
+                "default_min_hwe_kernel",
+                "default_osystem",
+                "default_distro_series",
+                "kernel_opts",
+                "use_rack_proxy",
+                "maas_internal_domain",
+                "remote_syslog",
+                "maas_syslog_port",
+            ]
+        )
+
+        osystem, series, config_arch = get_boot_config_for_machine(
+            machine, configs, "xinstall"
+        )
+
+        self.assertEqual(osystem, "ubuntu")
+        self.assertEqual(series, "jammy")
+        self.assertEqual(config_arch, "generic")
+
+    def test_get_boot_config_for_machine_legacy_custom_image(self):
+        arch = make_usable_architecture(self)
+        boot_resource = factory.make_BootResource(
+            base_image="", architecture=arch
+        )
+        machine = factory.make_Machine(
+            status=NODE_STATUS.DEPLOYING,
+            osystem="custom",
+            distro_series=boot_resource.name,
+        )
+        configs = Config.objects.get_configs(
+            [
+                "commissioning_osystem",
+                "commissioning_distro_series",
+                "enable_third_party_drivers",
+                "default_min_hwe_kernel",
+                "default_osystem",
+                "default_distro_series",
+                "kernel_opts",
+                "use_rack_proxy",
+                "maas_internal_domain",
+                "remote_syslog",
+                "maas_syslog_port",
+            ]
+        )
+
+        osystem, series, config_arch = get_boot_config_for_machine(
+            machine, configs, "xinstall"
+        )
+
+        # legacy custom images should use the default commissioning image as a base image
+        self.assertEqual(osystem, configs["commissioning_osystem"])
+        self.assertEqual(series, configs["commissioning_distro_series"])
+        self.assertEqual(config_arch, "generic")