Merge lp:~rvb/maas/tftp-bug-1317705 into lp:~maas-committers/maas/trunk

[Raphaël Badin]

Monkey patch the TFTP server to add error code 8.

Fix has landed upstream but this is taking forever to be SRUed so I think it's worth monkey patching this since the patch is simple.

[Gavin Panella (allenap)]

review: Approve

=== modified file 'src/provisioningserver/monkey.py'
--- src/provisioningserver/monkey.py	2015-06-08 06:17:54 +0000
+++ src/provisioningserver/monkey.py	2015-06-15 09:18:42 +0000
@@ -13,6 +13,7 @@
 
 __metaclass__ = type
 __all__ = [
+    "add_term_error_code_to_tftp",
     "force_simplestreams_to_use_urllib2",
 ]
 
@@ -38,3 +39,15 @@
         URL_READER=simplestreams.contentsource.Urllib2UrlReader,
         URL_READER_CLASSNAME="Urllib2UrlReader", urllib_error=urllib_error,
         urllib_request=urllib_request)
+
+
+def add_term_error_code_to_tftp():
+    """Add error code 8 to TFT server as introduced by RFC 2347.
+
+    Manually apply the fix to python-tx-tftp landed in
+    https://github.com/shylent/python-tx-tftp/pull/20
+    """
+    import tftp.datagram
+    if tftp.datagram.errors.get(8) is None:
+        tftp.datagram.errors[8] = (
+            "Terminate transfer due to option negotiation")
 
=== modified file 'src/provisioningserver/plugin.py'
--- src/provisioningserver/plugin.py	2015-06-08 06:17:54 +0000
+++ src/provisioningserver/plugin.py	2015-06-15 09:18:42 +0000
@@ -21,7 +21,10 @@
 import socket
 from socket import error as socket_error
 
-from provisioningserver.monkey import force_simplestreams_to_use_urllib2
+from provisioningserver.monkey import (
+    add_term_error_code_to_tftp,
+    force_simplestreams_to_use_urllib2,
+)
 from provisioningserver.utils.debug import (
     register_sigusr2_thread_dump_handler,
 )
@@ -233,6 +236,7 @@
         """Construct the MAAS Cluster service."""
         register_sigusr2_thread_dump_handler()
         force_simplestreams_to_use_urllib2()
+        add_term_error_code_to_tftp()
 
         from provisioningserver import services
         from provisioningserver.config import Config
 
=== modified file 'src/provisioningserver/tests/test_monkey.py'
--- src/provisioningserver/tests/test_monkey.py	2015-06-08 06:17:54 +0000
+++ src/provisioningserver/tests/test_monkey.py	2015-06-15 09:18:42 +0000
@@ -20,8 +20,12 @@
 
 from maastesting.testcase import MAASTestCase
 from mock import sentinel
-from provisioningserver.monkey import force_simplestreams_to_use_urllib2
+from provisioningserver.monkey import (
+    add_term_error_code_to_tftp,
+    force_simplestreams_to_use_urllib2,
+)
 from simplestreams import contentsource
+import tftp.datagram
 
 
 if sys.version_info > (3, 0):
@@ -62,3 +66,20 @@
         force_simplestreams_to_use_urllib2()
         self.assertEqual(
             self.value, getattr(contentsource, self.key))
+
+
+class TestAddTermErrorCodeToTFT(MAASTestCase):
+
+    def test_adds_error_code_8(self):
+        self.patch(tftp.datagram, 'errors', {})
+        add_term_error_code_to_tftp()
+        self.assertIn(8, tftp.datagram.errors)
+        self.assertEqual(
+            "Terminate transfer due to option negotiation",
+            tftp.datagram.errors.get(8))
+
+    def test_skips_adding_error_code_if_already_present(self):
+        self.patch(tftp.datagram, 'errors', {8: sentinel.error_8})
+        add_term_error_code_to_tftp()
+        self.assertEqual(
+            sentinel.error_8, tftp.datagram.errors.get(8))
 
=== modified file 'src/provisioningserver/tests/test_plugin.py'
--- src/provisioningserver/tests/test_plugin.py	2015-06-08 06:17:54 +0000
+++ src/provisioningserver/tests/test_plugin.py	2015-06-15 09:18:42 +0000
@@ -132,6 +132,15 @@
         service_maker.makeService(options)
         self.assertThat(mock_simplestreams_patch, MockCalledOnceWith())
 
+    def test_makeService_patches_tftp_service(self):
+        mock_tftp_patch = (
+            self.patch(plugin_module, 'add_term_error_code_to_tftp'))
+        options = Options()
+        options["config-file"] = self.write_config({})
+        service_maker = ProvisioningServiceMaker("Harry", "Hill")
+        service_maker.makeService(options)
+        self.assertThat(mock_tftp_patch, MockCalledOnceWith())
+
     def test_image_download_service(self):
         options = Options()
         options["config-file"] = self.write_config({})