Merge lp:~mpontillo/maas/fix-curtin-custom-driver-preseed--bug-1667426 into lp:~maas-committers/maas/trunk

[Mike Pontillo]

Fix curtin config rendering for third-party drivers.

It seems that third-party driver functionality in MAAS has gone unused since the move to MAAS 2.x. We have a bug leftover from the Python 2 -> Python 3 conversion which caused the (binary format) APT keys in drivers.yaml to fail to be rendered.

This branch also makes the YAML template more robust in the following ways:

 - Gracefully handle missing keys by leaving commands out of the YAML rather than raising a difficult-to-debug exception during deployment.

 - Nodes will no longer fail to deploy if the kernel module specified in drivers.yaml fails to be inserted. If the deployment succeeds anyway, this is something that will either be automatically corrected post-deployment (because a DKMS package was installed), OR would likely be a known issue with the driver that can be corrected by the user post-deployment. So this has been changed from a failure condition to a warning. (As a side effect, this allows custom drivers which do not require a kernel module to be used, but this branch does not add tests for that.)

 - If keys are missing from drivers.yaml, preseed generation should no longer crash and time out deployment with no visible error. The deployment will simply proceed without the missing keys. (As a side effect, this should allow the third-party driver feature to install custom packages that do not necessarily need a custom repository to work, but this branch does not add tests for that.)

[Lee Trager (ltrager)]

LGTM!

review: Approve

[MAAS Lander (maas-lander)]

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

[MAAS Lander (maas-lander)]

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

[Mike Pontillo (mpontillo)]

Hm. Launchpad bug? Diff seems stale. Trying the Needs Review -> Work in Progress -> Needs Review transition again...

[Mike Pontillo (mpontillo)]

Oh. Just now noticed this; it WAS a Launchpad bug.

Fault 380: 'An unexpected error has occurred while updating a Launchpad branch. Please report a Launchpad bug and quote: OOPS-05b5986d0373f49ba74c975b35393199.

=== modified file 'contrib/preseeds_v2/curtin_userdata'
--- contrib/preseeds_v2/curtin_userdata	2016-12-07 09:25:19 +0000
+++ contrib/preseeds_v2/curtin_userdata	2017-02-24 03:57:27 +0000
@@ -6,21 +6,35 @@
   {{endfor}}
 {{if third_party_drivers and driver}}
 early_commands:
-  {{py: key_string = ''.join(['\\x%x' % x for x in map(ord, driver['key_binary'])])}}
+  {{py: key_string = ''.join(['\\x%x' % x for x in driver['key_binary']])}}
+  {{if driver['key_binary'] and driver['repository'] and driver['package']}}
   driver_00_get_key: /bin/echo -en '{{key_string}}' > /tmp/maas-{{driver['package']}}.gpg
   driver_01_add_key: ["apt-key", "add", "/tmp/maas-{{driver['package']}}.gpg"]
+  {{endif}}
+  {{if driver['repository']}}
   driver_02_add: ["add-apt-repository", "-y", "deb {{driver['repository']}} {{node.get_distro_series()}} main"]
+  {{endif}}
+  {{if driver['package']}}
   driver_03_update_install: ["sh", "-c", "apt-get update --quiet && apt-get --assume-yes install {{driver['package']}}"]
-  driver_04_load: ["sh", "-c", "depmod && modprobe {{driver['module']}}"]
+  {{endif}}
+  {{if driver['module']}}
+  driver_04_load: ["sh", "-c", "depmod && modprobe {{driver['module']}} || echo 'Warning: Failed to load module: {{driver['module']}}'"]
+  {{endif}}
 {{endif}}
 late_commands:
   maas: [wget, '--no-proxy', {{node_disable_pxe_url|escape.json}}, '--post-data', {{node_disable_pxe_data|escape.json}}, '-O', '/dev/null']
 {{if third_party_drivers and driver}}
+  {{if driver['key_binary'] and driver['repository'] and driver['package']}}
   driver_00_key_get: curtin in-target -- sh -c "/bin/echo -en '{{key_string}}' > /tmp/maas-{{driver['package']}}.gpg"
   driver_02_key_add: ["curtin", "in-target", "--", "apt-key", "add", "/tmp/maas-{{driver['package']}}.gpg"]
+  {{endif}}
+  {{if driver['repository']}}
   driver_03_add: ["curtin", "in-target", "--", "add-apt-repository", "-y", "deb {{driver['repository']}} {{node.get_distro_series()}} main"]
+  {{endif}}
   driver_04_update_install: ["curtin", "in-target", "--", "apt-get", "update", "--quiet"]
+  {{if driver['package']}}
   driver_05_install: ["curtin", "in-target", "--", "apt-get", "-y", "install", "{{driver['package']}}"]
+  {{endif}}
   driver_06_depmod: ["curtin", "in-target", "--", "depmod"]
   driver_07_update_initramfs: ["curtin", "in-target", "--", "update-initramfs", "-u"]
 {{endif}}

=== modified file 'src/maasserver/tests/test_preseed.py'
--- src/maasserver/tests/test_preseed.py	2017-01-28 00:51:47 +0000
+++ src/maasserver/tests/test_preseed.py	2017-02-24 03:57:27 +0000
@@ -77,6 +77,7 @@
     MAASServerTestCase,
     MAASTransactionServerTestCase,
 )
+from maasserver.third_party_drivers import DriversConfig
 from maasserver.utils import absolute_reverse
 from maasserver.utils.curtin import curtin_supports_webhook_events
 from maastesting.matchers import (
@@ -850,6 +851,54 @@
         self.assertThat(mock_supports_storage, MockCalledOnceWith())
 
 
+class TestRenderCurtinUserdataWithThirdPartyDrivers(
+        PreseedRPCMixin, BootImageHelperMixin, MAASServerTestCase):
+    """Ensures curtin configs for all third-party drivers can be rendered."""
+
+    # Try rendering each driver in drivers.yaml.
+    scenarios = [
+        (driver['comment'], {'driver': driver})
+        for driver in DriversConfig.load_from_cache()['drivers']
+    ]
+
+    def test_render_curtin_preseed_with_third_party_driver(self):
+        node = factory.make_Node_with_Interface_on_Subnet(
+            primary_rack=self.rpc_rack_controller)
+        Config.objects.set_config(
+            'enable_third_party_drivers', True)
+        self.configure_get_boot_images_for_node(node, 'xinstall')
+        get_third_party_driver = self.patch(
+            preseed_module, "get_third_party_driver")
+        get_third_party_driver.return_value = self.driver
+        curtin_config_text = get_curtin_config(node)
+        config = yaml.safe_load(curtin_config_text)
+        self.assertThat(
+            config['early_commands'], Contains('driver_00_get_key'))
+        self.assertThat(
+            config['early_commands'], Contains('driver_01_add_key'))
+        self.assertThat(
+            config['early_commands'], Contains('driver_02_add'))
+        self.assertThat(
+            config['early_commands'], Contains('driver_03_update_install'))
+        self.assertThat(
+            config['early_commands'], Contains('driver_04_load'))
+        self.assertThat(
+            config['late_commands'], Contains('driver_00_key_get'))
+        self.assertThat(
+            config['late_commands'], Contains('driver_02_key_add'))
+        self.assertThat(
+            config['late_commands'], Contains('driver_03_add'))
+        self.assertThat(
+            config['late_commands'], Contains('driver_04_update_install'))
+        self.assertThat(
+            config['late_commands'], Contains('driver_05_install'))
+        self.assertThat(
+            config['late_commands'], Contains('driver_06_depmod'))
+        self.assertThat(
+            config['late_commands'],
+            Contains('driver_07_update_initramfs'))
+
+
 class TestGetCurtinUserDataOS(
         PreseedRPCMixin, BootImageHelperMixin, MAASServerTestCase):
     """Tests for `get_curtin_userdata` using os specific scenarios."""