Merge ~ltrager/maas:lp1851622 into maas:master

[Lee Trager]

LP: #1851622 - Validate netplan usage before use in ephemeral environments.

Netplan is only supported in Bionic+. MAAS now prevents ephemeral deployments
and testing in Xenial. This also fixes a bug where MAAS attempted to apply
network configuration when commissioning a machine that had all of its
disks deleted.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp1851622 lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/6973/console
COMMIT: 21ccdd41d667e119f71e5b1eaac9a4756356f13f

review: Needs Fixing

75c3d57... by Lee Trager on 2019-11-13

Fix lint

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp1851622 lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/6974/console
COMMIT: 75c3d57e3ae9ffbb0470587587de3cb0c9418556

review: Needs Fixing

[Newell Jensen (newell-jensen)]

LGTM

review: Approve

3ac1df9... by Lee Trager on 2019-11-14

Fix broken tests

diff --git a/src/maasserver/api/machines.py b/src/maasserver/api/machines.py
index 4f71844..5444773 100644
--- a/src/maasserver/api/machines.py
+++ b/src/maasserver/api/machines.py
@@ -725,9 +725,7 @@ class MachineHandler(NodeHandler, OwnerDataMixin, PowerMixin):
             NodePermission.admin, machine
         ):
             raise PermissionDenied()
-        if options.install_kvm and (
-            machine.ephemeral_deployment or options.ephemeral_deploy
-        ):
+        if options.install_kvm and machine.ephemeral_deployment:
             raise MAASAPIBadRequest(
                 "Cannot install KVM host for ephemeral deployments."
             )
diff --git a/src/maasserver/api/tests/test_machine.py b/src/maasserver/api/tests/test_machine.py
index faea37a..8b62ea3 100644
--- a/src/maasserver/api/tests/test_machine.py
+++ b/src/maasserver/api/tests/test_machine.py
@@ -554,9 +554,8 @@ class TestMachineAPI(APITestCase.ForUser):
             distro_series=distro_series,
             osystem=osystem,
             architecture=make_usable_architecture(self),
+            ephemeral_deploy=True,
         )
-        for bd in machine.blockdevice_set.all():
-            bd.delete()
         response = self.client.post(
             self.get_machine_uri(machine),
             {"op": "deploy", "install_kvm": True},
diff --git a/src/maasserver/models/node.py b/src/maasserver/models/node.py
index b679963..d087f37 100644
--- a/src/maasserver/models/node.py
+++ b/src/maasserver/models/node.py
@@ -16,7 +16,7 @@ from collections import defaultdict, namedtuple, OrderedDict
 import copy
 from datetime import datetime, timedelta
 from functools import partial
-from itertools import count
+from itertools import chain, count
 import json
 import logging
 from operator import attrgetter
@@ -1463,6 +1463,16 @@ class Node(CleanSave, TimestampedModel):
         # Devices should always local boot.
         if self.is_device:
             return False
+        # Only machines which are deploying can be deployed ephemerally. Allow
+        # ALLOCATED and READY as this is checked before transitioning in the
+        # API.
+        if self.status not in {
+            NODE_STATUS.DEPLOYING,
+            NODE_STATUS.DEPLOYED,
+            NODE_STATUS.ALLOCATED,
+            NODE_STATUS.READY,
+        }:
+            return False
         return self.is_diskless or self.ephemeral_deploy
 
     def retrieve_storage_layout_issues(
@@ -2216,21 +2226,55 @@ class Node(CleanSave, TimestampedModel):
         )
 
         # Create a new ScriptSet for this commissioning run.
-        script_set = ScriptSet.objects.create_commissioning_script_set(
+        commis_script_set = ScriptSet.objects.create_commissioning_script_set(
             self, commissioning_scripts, script_input
         )
-        self.current_commissioning_script_set = script_set
 
         # Create a new ScriptSet for any tests to be run after commissioning.
         try:
-            script_set = ScriptSet.objects.create_testing_script_set(
+            testing_script_set = ScriptSet.objects.create_testing_script_set(
                 self, testing_scripts, script_input
             )
-            self.current_testing_script_set = script_set
         except NoScriptsFound:
             # Commissioning can run without running tests after.
+            testing_script_set = []
             pass
 
+        config = Config.objects.get_configs(
+            [
+                "commissioning_osystem",
+                "commissioning_distro_series",
+                "default_osystem",
+                "default_distro_series",
+                "default_min_hwe_kernel",
+            ]
+        )
+
+        # Testing configured networking requires netplan which is only
+        # available in 18.04+
+        if config["commissioning_distro_series"] == "xenial":
+            apply_configured_networking_found = False
+            for script_result in chain(commis_script_set, testing_script_set):
+                if (
+                    script_result.script
+                    and script_result.script.apply_configured_networking
+                ):
+                    apply_configured_networking_found = True
+                    break
+            if apply_configured_networking_found:
+                commis_script_set.delete()
+                if testing_script_set:
+                    testing_script_set.delete()
+                raise ValidationError(
+                    "Unable to run script which configures custom networking "
+                    "when using Ubuntu Xenial 16.04 as the ephemeral "
+                    "operating system."
+                )
+
+        self.current_commissioning_script_set = commis_script_set
+        if testing_script_set:
+            self.current_testing_script_set = testing_script_set
+
         # Clear the current storage configuration if networking is not being
         # skipped during commissioning.
         if not self.skip_storage:
@@ -2247,15 +2291,7 @@ class Node(CleanSave, TimestampedModel):
         old_status = self.status
         self.status = NODE_STATUS.COMMISSIONING
         self.owner = user
-        config = Config.objects.get_configs(
-            [
-                "commissioning_osystem",
-                "commissioning_distro_series",
-                "default_osystem",
-                "default_distro_series",
-                "default_min_hwe_kernel",
-            ]
-        )
+
         # Set min_hwe_kernel to default_min_hwe_kernel.
         # This makes sure that the min_hwe_kernel is up to date
         # with what is stored in the settings.
@@ -2377,6 +2413,9 @@ class Node(CleanSave, TimestampedModel):
         script_set = ScriptSet.objects.create_testing_script_set(
             self, testing_scripts, script_input
         )
+        commissioning_distro_series = Config.objects.get_config(
+            "commissioning_distro_series"
+        )
         # Additional validation for when starting testing and not
         # commissioning + testing.
         for script_result in script_set:
@@ -2401,6 +2440,17 @@ class Node(CleanSave, TimestampedModel):
                 raise ValidationError(
                     "Unable to run destructive test while deployed!"
                 )
+            if (
+                commissioning_distro_series == "xenial"
+                and script_result.script
+                and script_result.script.apply_configured_networking
+            ):
+                script_set.delete()
+                raise ValidationError(
+                    "Unable to run script which configures custom networking "
+                    "when using Ubuntu Xenial 16.04 as the ephemeral "
+                    "operating system."
+                )
 
         self.current_testing_script_set = script_set
 
@@ -3612,13 +3662,13 @@ class Node(CleanSave, TimestampedModel):
 
     def set_ephemeral_deploy(self, on=False):
         """Set ephermal deploy on or off."""
+        # self.ephemeral_deployment returns True when the system is diskless.
+        self.ephemeral_deploy = on or self.is_diskless
         log.info(
             "{hostname}: Turning ephemeral deploy %s for node"
-            % ("off" if on is False else "on"),
+            % ("on" if self.ephemeral_deploy else "off"),
             hostname=self.hostname,
         )
-        self.ephemeral_deploy = on
-        self.save()
 
     def split_arch(self):
         """Return architecture and subarchitecture, as a tuple."""
@@ -4986,6 +5036,7 @@ class Node(CleanSave, TimestampedModel):
                     # If ephemeral is in purposes, this comes from Caringo OS.
                     # Set the ephemeral_deploy field on the node.
                     self.set_ephemeral_deploy(True)
+                    self.save()
                     return "ephemeral"
                 else:
                     return "xinstall"
@@ -5160,6 +5211,9 @@ class Node(CleanSave, TimestampedModel):
         bridge_stp=None,
         bridge_fd=None,
     ):
+        # Avoid circular imports
+        from maasserver.utils.osystems import release_a_newer_than_b
+
         if not user.has_perm(NodePermission.edit, self):
             # You can't start a node you don't own unless you're an admin.
             raise PermissionDenied()
@@ -5227,6 +5281,12 @@ class Node(CleanSave, TimestampedModel):
             raise ValidationError(
                 "Cannot install KVM host for ephemeral deployments."
             )
+        if self.ephemeral_deployment and not release_a_newer_than_b(
+            self.distro_series, "bionic"
+        ):
+            raise ValidationError(
+                "Ephemeral deployments only supported on Ubuntu Bionic 18.04+"
+            )
         self._register_request_event(
             user, event, action="start", comment=comment
         )
diff --git a/src/maasserver/models/tests/test_node.py b/src/maasserver/models/tests/test_node.py
index 2f0ec78..e98fa74 100644
--- a/src/maasserver/models/tests/test_node.py
+++ b/src/maasserver/models/tests/test_node.py
@@ -1096,33 +1096,62 @@ class TestNode(MAASServerTestCase):
         self.assertFalse(node.is_diskless)
 
     def test_no_ephemeral_deploy(self):
-        node = factory.make_Node(ephemeral_deploy=False)
+        node = factory.make_Node(
+            ephemeral_deploy=False, status=NODE_STATUS.DEPLOYING
+        )
         self.assertFalse(node.ephemeral_deploy)
 
     def test_ephemeral_deploy(self):
-        node = factory.make_Node(ephemeral_deploy=True)
+        node = factory.make_Node(
+            ephemeral_deploy=True, status=NODE_STATUS.DEPLOYING
+        )
         self.assertTrue(node.ephemeral_deploy)
 
     def test_ephemeral_deployment_checks_diskless(self):
-        node = factory.make_Node(with_boot_disk=True)
+        node = factory.make_Node(
+            with_boot_disk=True, status=NODE_STATUS.DEPLOYING
+        )
         self.assertFalse(node.ephemeral_deployment)
 
     def test_ephemeral_deployment_checks_not_diskless(self):
-        node = factory.make_Node(with_boot_disk=False)
+        node = factory.make_Node(
+            with_boot_disk=False, status=NODE_STATUS.DEPLOYING
+        )
         self.assertTrue(node.ephemeral_deployment)
 
     def test_ephemeral_deployment_checks_ephemeral_deploy(self):
-        node = factory.make_Node(ephemeral_deploy=True)
+        node = factory.make_Node(
+            ephemeral_deploy=True, status=NODE_STATUS.DEPLOYING
+        )
         self.assertTrue(node.ephemeral_deployment)
 
     def test_ephemeral_deployment_checks_no_ephemeral_deploy(self):
-        node = factory.make_Node(ephemeral_deploy=False)
+        node = factory.make_Node(
+            ephemeral_deploy=False, status=NODE_STATUS.DEPLOYING
+        )
         self.assertFalse(node.ephemeral_deployment)
 
     def test_ephemeral_deployment_checks_is_device(self):
-        device = factory.make_Device()
+        device = factory.make_Device(
+            ephemeral_deploy=True, status=NODE_STATUS.DEPLOYING
+        )
         self.assertFalse(device.ephemeral_deployment)
 
+    def test_ephemeral_deployment_no_if_not_deploying(self):
+        node = factory.make_Node(
+            status=factory.pick_choice(
+                NODE_STATUS_CHOICES,
+                but_not=[
+                    NODE_STATUS.DEPLOYING,
+                    NODE_STATUS.DEPLOYED,
+                    NODE_STATUS.ALLOCATED,
+                    NODE_STATUS.READY,
+                ],
+            ),
+            ephemeral_deploy=True,
+        )
+        self.assertFalse(node.ephemeral_deployment)
+
     def test_system_id_is_a_valid_znum(self):
         node = factory.make_Node()
         self.assertThat(
@@ -3456,6 +3485,39 @@ class TestNode(MAASServerTestCase):
         )
         self.assertEqual(events[1].type.name, EVENT_TYPES.COMMISSIONING)
 
+    def test_start_commissioning_fails_on_xenial_with_network_testing_c(self):
+        Config.objects.set_config("commissioning_distro_series", "xenial")
+        node = factory.make_Node(status=NODE_STATUS.NEW)
+        admin = factory.make_admin()
+        script = factory.make_Script(
+            script_type=SCRIPT_TYPE.COMMISSIONING,
+            apply_configured_networking=True,
+        )
+        self.assertRaises(
+            ValidationError,
+            node.start_commissioning,
+            user=admin,
+            commissioning_scripts=[script.name],
+        )
+        self.assertEquals(0, ScriptSet.objects.count())
+        self.assertEquals(0, ScriptResult.objects.count())
+
+    def test_start_commissioning_fails_on_xenial_with_network_testing_t(self):
+        Config.objects.set_config("commissioning_distro_series", "xenial")
+        node = factory.make_Node(status=NODE_STATUS.NEW)
+        admin = factory.make_admin()
+        script = factory.make_Script(
+            script_type=SCRIPT_TYPE.TESTING, apply_configured_networking=True
+        )
+        self.assertRaises(
+            ValidationError,
+            node.start_commissioning,
+            user=admin,
+            testing_scripts=[script.name],
+        )
+        self.assertEquals(0, ScriptSet.objects.count())
+        self.assertEquals(0, ScriptResult.objects.count())
+
     def test_abort_commissioning_reverts_to_sane_state_on_error(self):
         # If abort commissioning hits an error when trying to stop the
         # node, it will revert the node to the state it was in before
@@ -3920,6 +3982,21 @@ class TestNode(MAASServerTestCase):
         )
         self.assertFalse(ScriptSet.objects.filter(node=node).exists())
 
+    def test_start_testing_prevents_network_testing_with_xenial(self):
+        script = factory.make_Script(
+            script_type=SCRIPT_TYPE.TESTING, apply_configured_networking=True
+        )
+        admin = factory.make_admin()
+        node = factory.make_Node(status=NODE_STATUS.READY)
+        self.assertRaises(
+            ValidationError,
+            node.start_testing,
+            admin,
+            testing_scripts=[script.name],
+        )
+        self.assertEquals(0, ScriptSet.objects.count())
+        self.assertEquals(0, ScriptResult.objects.count())
+
     def test_save_logs_node_status_transition(self):
         self.disable_node_query()
         node = factory.make_Node(
@@ -5212,7 +5289,11 @@ class TestNode(MAASServerTestCase):
 
     def test_storage_layout_issues_is_valid_when_ephemeral_deployment(self):
         # A diskless node is one that it is ephemerally deployed.
-        node = factory.make_Node(with_boot_disk=False, osystem="ubuntu")
+        node = factory.make_Node(
+            with_boot_disk=False,
+            osystem="ubuntu",
+            status=NODE_STATUS.DEPLOYING,
+        )
         self.assertEqual([], node.storage_layout_issues())
 
     def test_storage_layout_issues_is_invalid_when_no_disks_non_ubuntu(self):
@@ -6397,6 +6478,11 @@ class NodeManagerTest(MAASServerTestCase):
         node.set_ephemeral_deploy(True)
         self.assertTrue(node.ephemeral_deploy)
 
+    def test_ephemeral_deploy_auto_on_when_diskless(self):
+        node = factory.make_Node(ephemeral_deploy=False, with_boot_disk=False)
+        node.set_ephemeral_deploy(False)
+        self.assertTrue(node.ephemeral_deploy)
+
     def test_ephemeral_deploy_off(self):
         node = factory.make_Node(ephemeral_deploy=True)
         node.set_ephemeral_deploy(False)
@@ -8013,6 +8099,8 @@ class TestNode_Start(MAASTransactionServerTestCase):
         power_state=POWER_STATE.OFF,
         network=None,
         with_boot_disk=True,
+        install_kvm=False,
+        ephemeral_deploy=False,
     ):
         if network is None:
             network = factory.make_ip4_or_6_network()
@@ -8041,6 +8129,8 @@ class TestNode_Start(MAASTransactionServerTestCase):
             cidr=cidr,
             osystem=osystem,
             distro_series=distro_series,
+            install_kvm=install_kvm,
+            ephemeral_deploy=ephemeral_deploy,
         )
         node.acquire(user)
         return node
@@ -8073,10 +8163,24 @@ class TestNode_Start(MAASTransactionServerTestCase):
     def test__raises_ValidationError_if_ephemeral_deploy_and_install_kvm(self):
         admin = factory.make_admin()
         node = self.make_acquired_node_with_interface(
-            admin, power_type="manual", with_boot_disk=False
+            admin,
+            power_type="manual",
+            with_boot_disk=False,
+            ephemeral_deploy=True,
+            install_kvm=True,
         )
-        node.install_kvm = True
-        node.save()
+        with ExpectedException(ValidationError):
+            node.start(admin)
+
+    def test__raises_ValidationError_if_ephemeral_deployment_less_bionic(self):
+        admin = factory.make_admin()
+        node = self.make_acquired_node_with_interface(
+            admin,
+            power_type="manual",
+            with_boot_disk=False,
+            ephemeral_deploy=True,
+        )
+        node.distro_series = "xenial"
         with ExpectedException(ValidationError):
             node.start(admin)
 
diff --git a/src/metadataserver/tests/test_vendor_data.py b/src/metadataserver/tests/test_vendor_data.py
index 24f1175..d57282f 100644
--- a/src/metadataserver/tests/test_vendor_data.py
+++ b/src/metadataserver/tests/test_vendor_data.py
@@ -384,7 +384,11 @@ class TestGenerateEphemeralDeploymentNetworkConfiguration(MAASServerTestCase):
         self.assertThat(dict(configuration), Equals({}))
 
     def test_yields_configuration_when_node_is_ephemeral_deployment(self):
-        node = factory.make_Node(with_boot_disk=False)
+        node = factory.make_Node(
+            with_boot_disk=False,
+            ephemeral_deploy=True,
+            status=NODE_STATUS.DEPLOYING,
+        )
         configuration = get_vendor_data(node, None)
         config = dict(configuration)
         self.assertThat(