x/cloudbase-init
Code Issues Proposed changes

Refactor the CloudStack Metadata Service

Browse Source 
This patch changes the way that the service interacts with
the Password Server in order to ease the service testing
and to remove redundant code.

Change-Id: I855899f6163465dfc2974047efb0dfd290a617ef
Co-Authored-By: Stefan Caraiman <scaraiman@cloudbasesolutions.com>
Implements-Blueprint: cloudstack-password-port
This commit is contained in:
Alexandru Coman 2016-09-06 10:31:23 +03:00 committed by Stefan Caraiman
parent 7078b58038
commit e3b6c9662e
3 changed files with 97 additions and 96 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
cloudbaseinit/conf/cloudstack.py
View File

@ -31,6 +31,10 @@ class CloudStackOptions(conf_base.Options):
help="The base URL where the service looks for metadata",
deprecated_name="cloudstack_metadata_ip",
deprecated_group="DEFAULT"),
cfg.IntOpt(
"password_server_port", default=8080,
help="The port number used by the Password Server."
),
cfg.BoolOpt(
"https_allow_insecure", default=False,
help="Whether to disable the validation of HTTPS "

125
cloudbaseinit/metadata/services/cloudstack.py
View File

@ -27,25 +27,34 @@ from cloudbaseinit.utils import encoding
CONF = cloudbaseinit_conf.CONF
LOG = oslo_logging.getLogger(__name__)
BAD_REQUEST = b"bad_request"
SAVED_PASSWORD = b"saved_password"
BAD_REQUEST = "bad_request"
SAVED_PASSWORD = "saved_password"
TIMEOUT = 10
class CloudStack(base.BaseHTTPMetadataService):
"""Metadata service for Apache CloudStack.
Apache CloudStack is an open source software designed to deploy and
manage large networks of virtual machines, as a highly available,
highly scalable Infrastructure as a Service (IaaS) cloud computing
platform.
"""
def __init__(self):
# Note: The base url used by the current metadata service will be
# updated later by the `_test_api` method.
super(CloudStack, self).__init__(
# Note(alexcoman): The base url used by the current metadata
# service will be updated later by the `_test_api` method.
base_url=None,
https_allow_insecure=CONF.cloudstack.https_allow_insecure,
https_ca_bundle=CONF.cloudstack.https_ca_bundle)
self._osutils = osutils_factory.get_os_utils()
self._router_ip = None
self._metadata_host = None
def _get_path(self, resource, version="latest"):
@staticmethod
def _get_path(resource, version="latest"):
"""Get the relative path for the received resource."""
return posixpath.normpath(
posixpath.join(version, "meta-data", resource))
@ -68,7 +77,7 @@ class CloudStack(base.BaseHTTPMetadataService):
LOG.debug('Available services: %s', response)
netloc = urllib.parse.urlparse(metadata_url).netloc
self._router_ip = netloc.split(":")[0]
self._metadata_host = netloc.split(":")[0]
return True
def load(self):
@ -114,13 +123,37 @@ class CloudStack(base.BaseHTTPMetadataService):
ssh_keys.append(ssh_key)
return ssh_keys
def _password_client(self, body=None, headers=None, decode=True):
"""Client for the Password Server."""
port = CONF.cloudstack.password_server_port
with contextlib.closing(http_client.HTTPConnection(
self._metadata_host, port, timeout=TIMEOUT)) as connection:
try:
connection.request("GET", "/", body=body, headers=headers)
response = connection.getresponse()
except http_client.HTTPException as exc:
LOG.error("Request failed: %s", exc)
raise
content = response.read()
if decode:
content = encoding.get_as_string(content)
if response.status != 200:
raise http_client.HTTPException(
"%(status)s %(reason)s - %(message)r",
{"status": response.status, "reason": response.reason,
"message": content})
return content
def _get_password(self):
"""Get the password from the Password Server.
The Password Server can be found on the DHCP_SERVER on the port 8080.
.. note:
The Password Server can return the following values:
* `bad_request`: the Password Server did not recognise
* `bad_request`: the Password Server did not recognize
the request
* `saved_password`: the password was already deleted from
the Password Server
@ -132,45 +165,33 @@ class CloudStack(base.BaseHTTPMetadataService):
headers = {"DomU_Request": "send_my_password"}
password = None
with contextlib.closing(http_client.HTTPConnection(
self._router_ip, 8080, timeout=TIMEOUT)) as connection:
for _ in range(CONF.retry_count):
try:
connection.request("GET", "/", headers=headers)
response = connection.getresponse()
except http_client.HTTPException as exc:
LOG.exception(exc)
continue
for _ in range(CONF.retry_count):
try:
content = self._password_client(headers=headers).strip()
except http_client.HTTPConnection as exc:
LOG.error("Getting password failed: %s", exc)
continue
if response.status != 200:
LOG.warning("Getting password failed: %(status)s "
"%(reason)s - %(message)r",
{"status": response.status,
"reason": response.reason,
"message": response.read()})
continue
if not content:
LOG.warning("The Password Server did not have any "
"password for the current instance.")
continue
content = response.read().strip()
if not content:
LOG.warning("The Password Server did not have any "
"password for the current instance.")
continue
if content == BAD_REQUEST:
LOG.error("The Password Server did not recognise the "
"request.")
break
if content == SAVED_PASSWORD:
LOG.warning("For this instance the password was already "
"taken from the Password Server.")
break
LOG.info("The password server return a valid password "
"for the current instance.")
password = encoding.get_as_string(content)
if content == BAD_REQUEST:
LOG.error("The Password Server did not recognize the "
"request.")
break
if content == SAVED_PASSWORD:
LOG.warning("The password was already taken from the "
"Password Server for the current instance.")
break
LOG.info("The password server returned a valid password "
"for the current instance.")
password = content
break
return password
def _delete_password(self):
@ -182,21 +203,15 @@ class CloudStack(base.BaseHTTPMetadataService):
LOG.debug("Remove the password for this instance from the "
"Password Server.")
headers = {"DomU_Request": "saved_password"}
connection = http_client.HTTPConnection(self._router_ip, 8080,
timeout=TIMEOUT)
for _ in range(CONF.retry_count):
connection.request("GET", "/", headers=headers)
response = connection.getresponse()
if response.status != 200:
LOG.warning("Removing password failed: %(status)s "
"%(reason)s - %(message)r",
{"status": response.status,
"reason": response.reason,
"message": response.read()})
try:
content = self._password_client(headers=headers).strip()
except http_client.HTTPConnection as exc:
LOG.error("Removing password failed: %s", exc)
continue
content = response.read()
if content != BAD_REQUEST: # comparing bytes with bytes
if content != BAD_REQUEST:
LOG.info("The password was removed from the Password Server.")
break
else:

64
cloudbaseinit/tests/metadata/services/test_cloudstack.py
View File

@ -170,20 +170,15 @@ class CloudStackTest(unittest.TestCase):
response = self._service.get_public_keys()
self.assertEqual([], response)
@mock.patch('six.moves.http_client.HTTPConnection')
def test_get_password(self, mock_http_connection):
@mock.patch('cloudbaseinit.metadata.services.cloudstack.CloudStack'
'._password_client')
def test_get_password(self, mock_password_client):
headers = {"DomU_Request": "send_my_password"}
mock_connection = mock.Mock()
mock_http_connection.return_value = mock_connection
mock_response = mock_connection.getresponse()
mock_request = mock_connection.request
mock_response.status = 200
expected_password = b"password"
mock_response.read.side_effect = [expected_password]
self._service._router_ip = mock.sentinel.router_ip
expected_password = "password"
mock_password_client.return_value = expected_password
expected_output = [
"Try to get password from the Password Server.",
"The password server return a valid password "
"The password server returned a valid password "
"for the current instance."
]
@ -191,29 +186,22 @@ class CloudStackTest(unittest.TestCase):
'cloudstack') as snatcher:
password = self._service._get_password()
mock_http_connection.assert_called_once_with(
mock.sentinel.router_ip, 8080, timeout=cloudstack.TIMEOUT)
mock_request.assert_called_once_with("GET", "/", headers=headers)
self.assertEqual(expected_password.decode(), password)
mock_password_client.assert_called_once_with(headers=headers)
self.assertEqual(expected_password, password)
self.assertEqual(expected_output, snatcher.output)
@mock.patch('six.moves.http_client.HTTPConnection')
def test_get_password_fail(self, mock_http_connection):
mock_connection = mock.Mock()
mock_http_connection.return_value = mock_connection
mock_response = mock_connection.getresponse()
mock_request = mock_connection.request
mock_response.status = 200
mock_response.read.side_effect = [b"", cloudstack.BAD_REQUEST,
cloudstack.SAVED_PASSWORD]
@mock.patch('cloudbaseinit.metadata.services.cloudstack.CloudStack'
'._password_client')
def test_get_password_fail(self, mock_password_client):
mock_password_client.side_effect = ["", cloudstack.BAD_REQUEST,
cloudstack.SAVED_PASSWORD]
expected_output = [
["Try to get password from the Password Server.",
"For this instance the password was already taken from "
"the Password Server."],
"The password was already taken from the Password Server "
"for the current instance."],
["Try to get password from the Password Server.",
"The Password Server did not recognise the request."],
"The Password Server did not recognize the request."],
["Try to get password from the Password Server.",
"The Password Server did not have any password for the "
@ -225,21 +213,16 @@ class CloudStackTest(unittest.TestCase):
self.assertIsNone(self._service._get_password())
self.assertEqual(expected_output.pop(), snatcher.output)
self.assertEqual(3, mock_request.call_count)
self.assertEqual(3, mock_password_client.call_count)
@mock.patch('six.moves.http_client.HTTPConnection')
def test_delete_password(self, mock_http_connection):
mock_connection = mock.Mock()
mock_http_connection.return_value = mock_connection
mock_response = mock_connection.getresponse()
mock_request = mock_connection.request
mock_response.read.side_effect = [cloudstack.BAD_REQUEST,
cloudstack.SAVED_PASSWORD]
mock_response.status = 400
@mock.patch('cloudbaseinit.metadata.services.cloudstack.CloudStack'
'._password_client')
def test_delete_password(self, mock_password_client):
mock_password_client.side_effect = [cloudstack.BAD_REQUEST,
cloudstack.SAVED_PASSWORD]
expected_output = [
'Remove the password for this instance from the '
'Password Server.',
'Removing password failed',
'Fail to remove the password from the Password Server.',
'Remove the password for this instance from the '
@ -251,9 +234,8 @@ class CloudStackTest(unittest.TestCase):
with testutils.LogSnatcher('cloudbaseinit.metadata.services.'
'cloudstack') as snatcher:
self.assertIsNone(self._service._delete_password())
mock_response.status = 200
self.assertIsNone(self._service._delete_password())
self.assertEqual(2, mock_request.call_count)
self.assertEqual(2, mock_password_client.call_count)
for expected, output in zip(expected_output, snatcher.output):
self.assertTrue(output.startswith(expected))