Merge lp:~daniel-thewatkins/cloud-init/lp1464253 into lp:~cloud-init-dev/cloud-init/trunk

[Dan Watkins]

[Scott Moser (smoser)]

Changes are fine, but you have to fix the tests also.
currently failing.

[lp:~daniel-thewatkins/cloud-init/lp1464253]

1115. By Dan Watkins on 2015-06-16

Use wget to fetch CloudStack passwords.

Different versions of the CloudStack password server respond
differently; wget handles these nicely for us, so it's easier to just
use wget.

=== modified file 'cloudinit/sources/DataSourceCloudStack.py'
--- cloudinit/sources/DataSourceCloudStack.py	2015-02-23 09:36:36 +0000
+++ cloudinit/sources/DataSourceCloudStack.py	2015-06-16 16:35:32 +0000
@@ -29,8 +29,6 @@
 from socket import inet_ntoa
 from struct import pack
 
-from six.moves import http_client
-
 from cloudinit import ec2_utils as ec2
 from cloudinit import log as logging
 from cloudinit import url_helper as uhelp
@@ -47,35 +45,22 @@
     has documentation about the system.  This implementation is following that
     found at
     https://github.com/shankerbalan/cloudstack-scripts/blob/master/cloud-set-guest-password-debian
-
-    The CloudStack password server is, essentially, a broken HTTP
-    server. It requires us to provide a valid HTTP request (including a
-    DomU_Request header, which is the meat of the request), but just
-    writes the text of its response on to the socket, without a status
-    line or any HTTP headers.  This makes HTTP libraries sad, which
-    explains the screwiness of the implementation of this class.
-
-    This should be fixed in CloudStack by commit
-    a72f14ea9cb832faaac946b3cf9f56856b50142a in December 2014.
     """
 
     def __init__(self, virtual_router_address):
         self.virtual_router_address = virtual_router_address
 
     def _do_request(self, domu_request):
-        # We have to provide a valid HTTP request, but a valid HTTP
-        # response is not returned. This means that getresponse() chokes,
-        # so we use the socket directly to read off the response.
-        # Because we're reading off the socket directly, we can't re-use the
-        # connection.
-        conn = http_client.HTTPConnection(self.virtual_router_address, 8080)
-        try:
-            conn.request('GET', '', headers={'DomU_Request': domu_request})
-            conn.sock.settimeout(30)
-            output = conn.sock.recv(1024).decode('utf-8').strip()
-        finally:
-            conn.close()
-        return output
+        # The password server was in the past, a broken HTTP server, but is now
+        # fixed.  wget handles this seamlessly, so it's easier to shell out to
+        # that rather than write our own handling code.
+        output, _ = util.subp([
+            'wget', '--quiet', '--tries', '3', '--timeout', '20',
+            '--output-document', '-', '--header',
+            'DomU_Request: {0}'.format(domu_request),
+            '{0}:8080'.format(self.virtual_router_address)
+        ])
+        return output.strip()
 
     def get_password(self):
         password = self._do_request('send_my_password')

=== modified file 'tests/unittests/test_datasource/test_cloudstack.py'
--- tests/unittests/test_datasource/test_cloudstack.py	2015-02-20 10:30:36 +0000
+++ tests/unittests/test_datasource/test_cloudstack.py	2015-06-16 16:35:32 +0000
@@ -23,13 +23,11 @@
         self.patches.enter_context(mock.patch('{0}.uhelp'.format(mod_name)))
 
     def _set_password_server_response(self, response_string):
-        http_client = mock.MagicMock()
-        http_client.HTTPConnection.return_value.sock.recv.return_value = \
-            response_string.encode('utf-8')
+        subp = mock.MagicMock(return_value=(response_string, ''))
         self.patches.enter_context(
-            mock.patch('cloudinit.sources.DataSourceCloudStack.http_client',
-                       http_client))
-        return http_client
+            mock.patch('cloudinit.sources.DataSourceCloudStack.util.subp',
+                       subp))
+        return subp
 
     def test_empty_password_doesnt_create_config(self):
         self._set_password_server_response('')
@@ -55,26 +53,28 @@
         ds = DataSourceCloudStack({}, None, helpers.Paths({}))
         self.assertTrue(ds.get_data())
 
-    def assertRequestTypesSent(self, http_client, expected_request_types):
-        request_types = [
-            kwargs['headers']['DomU_Request']
-            for _, kwargs
-            in http_client.HTTPConnection.return_value.request.call_args_list]
+    def assertRequestTypesSent(self, subp, expected_request_types):
+        request_types = []
+        for call in subp.call_args_list:
+            args = call[0][0]
+            for arg in args:
+                if arg.startswith('DomU_Request'):
+                    request_types.append(arg.split()[1])
         self.assertEqual(expected_request_types, request_types)
 
     def test_valid_response_means_password_marked_as_saved(self):
         password = 'SekritSquirrel'
-        http_client = self._set_password_server_response(password)
+        subp = self._set_password_server_response(password)
         ds = DataSourceCloudStack({}, None, helpers.Paths({}))
         ds.get_data()
-        self.assertRequestTypesSent(http_client,
+        self.assertRequestTypesSent(subp,
                                     ['send_my_password', 'saved_password'])
 
     def _check_password_not_saved_for(self, response_string):
-        http_client = self._set_password_server_response(response_string)
+        subp = self._set_password_server_response(response_string)
         ds = DataSourceCloudStack({}, None, helpers.Paths({}))
         ds.get_data()
-        self.assertRequestTypesSent(http_client, ['send_my_password'])
+        self.assertRequestTypesSent(subp, ['send_my_password'])
 
     def test_password_not_saved_if_empty(self):
         self._check_password_not_saved_for('')