Cache negotiated api microversion for server

If we negotiate an API microversion in the ironiccclient, cache
this value so that we don't need to re-negotiate it again for the
next API call, if the user doesn't specify a version.

We cache the version for a particular ironic instance in a
temporary file, one per server, in a multi-user, multi-server
safe-way - deleting old files once they expire, so only the
latest is kept.

Depends-On: Icb29fdc92ecd54e388b7c16899070572458308da
Change-Id: I0232bc611789fb96491d855020042e23ba0c4fab
Blueprint: version-caching
This commit is contained in:
Michael Davies 2015-04-14 08:32:34 +09:30
parent 3f4671a0b7
commit 44e6949b0c
8 changed files with 342 additions and 30 deletions

View File

@ -0,0 +1,86 @@
#
# Copyright 2015 Rackspace, Inc
# All Rights Reserved
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
import os
import appdirs
import dogpile.cache
AUTHOR = 'openstack'
PROGNAME = 'python-ironicclient'
CACHE_DIR = appdirs.user_cache_dir(PROGNAME, AUTHOR)
CACHE_FILENAME = os.path.join(CACHE_DIR, 'ironic-api-version.dbm')
CACHE = None
DEFAULT_EXPIRY = 300 # seconds
def _get_cache():
"""Configure file caching."""
global CACHE
if CACHE is None:
# Ensure cache directory present
if not os.path.exists(CACHE_DIR):
os.makedirs(CACHE_DIR)
CACHE = dogpile.cache.make_region().configure(
'dogpile.cache.dbm',
expiration_time=DEFAULT_EXPIRY,
arguments={
"filename": CACHE_FILENAME,
}
)
return CACHE
def _build_key(host, port):
"""Build a key based upon the hostname or address supplied."""
return "%s:%s" % (host, port)
def save_data(host, port, data):
"""Save 'data' for a particular 'host' in the appropriate cache dir.
param host: The host that we need to save data for
param port: The port on the host that we need to save data for
param data: The data we want saved
"""
key = _build_key(host, port)
_get_cache().set(key, data)
def retrieve_data(host, port, expiry=None):
"""Retrieve the version stored for an ironic 'host', if it's not stale.
Check to see if there is valid cached data for the host/port
combination and return that if it isn't stale.
param host: The host that we need to retrieve data for
param port: The port on the host that we need to retrieve data for
param expiry: The age in seconds before cached data is deemed invalid
"""
# Ensure that a cache file exists first
if not os.path.isfile(CACHE_FILENAME):
return None
key = _build_key(host, port)
data = _get_cache().get(key, expiration_time=expiry)
if data == dogpile.cache.api.NO_VALUE:
return None
return data

View File

@ -27,6 +27,7 @@ from keystoneclient import adapter
import six
import six.moves.urllib.parse as urlparse
from ironicclient.common import filecache
from ironicclient import exc
@ -43,7 +44,7 @@ USER_AGENT = 'python-ironicclient'
CHUNKSIZE = 1024 * 64 # 64kB
API_VERSION = '/v1'
API_VERSION_SELECTED_STATES = ('user', 'negotiated', 'default')
API_VERSION_SELECTED_STATES = ('user', 'negotiated', 'cached', 'default')
DEFAULT_MAX_RETRIES = 5
@ -69,6 +70,14 @@ def _extract_error_json(body):
return error_json
def get_server(endpoint):
"""Extract and return the server & port that we're connecting to."""
if endpoint is None:
return None, None
parts = urlparse.urlparse(endpoint)
return parts.hostname, str(parts.port)
class VersionNegotiationMixin(object):
def negotiate_version(self, conn, resp):
"""Negotiate the server version
@ -117,7 +126,6 @@ class VersionNegotiationMixin(object):
% {'req': self.os_ironic_api_version,
'min': min_ver, 'max': max_ver}))
# TODO(deva): cache the negotiated version for this server
negotiated_ver = min(self.os_ironic_api_version, max_ver)
if negotiated_ver < min_ver:
negotiated_ver = min_ver
@ -127,6 +135,10 @@ class VersionNegotiationMixin(object):
self.os_ironic_api_version = negotiated_ver
LOG.debug('Negotiated API version is %s', negotiated_ver)
# Cache the negotiated version for this server
host, port = get_server(self.endpoint)
filecache.save_data(host=host, port=port, data=negotiated_ver)
return negotiated_ver
def _generic_parse_version_headers(self, accessor_func):
@ -444,11 +456,13 @@ class SessionClient(VersionNegotiationMixin, adapter.LegacyJsonAdapter):
api_version_select_state,
max_retries,
retry_interval,
endpoint,
**kwargs):
self.os_ironic_api_version = os_ironic_api_version
self.api_version_select_state = api_version_select_state
self.conflict_max_retries = max_retries
self.conflict_retry_interval = retry_interval
self.endpoint = endpoint
super(SessionClient, self).__init__(**kwargs)
@ -579,8 +593,8 @@ def _construct_http_client(endpoint,
api_version_select_state=api_version_select_state,
max_retries=max_retries,
retry_interval=retry_interval,
endpoint=endpoint,
**kwargs)
else:
if kwargs:
LOG.warn('The following arguments are being ignored when '

View File

@ -11,8 +11,11 @@
# under the License.
import fixtures
import mock
import ironicclient
from ironicclient.client import get_client
from ironicclient.common import filecache
from ironicclient import exc
from ironicclient.tests.unit import utils
from ironicclient.v1 import client as v1
@ -140,24 +143,36 @@ class ClientTest(utils.BaseTestCase):
self.assertEqual(ksclient().auth_ref, client.http_client.auth_ref)
def test_get_client_with_api_version(self):
self.useFixture(fixtures.MonkeyPatch(
'ironicclient.client._get_ksclient', fake_get_ksclient))
@mock.patch.object(filecache, 'retrieve_data', autospec=True)
@mock.patch.object(ironicclient.client, '_get_ksclient', autospec=True)
def _get_client_with_api_version(self, version, mock_ksclient,
mock_retrieve_data):
mock_ksclient.return_value = fake_get_ksclient()
kwargs = {
'os_tenant_name': 'TENANT_NAME',
'os_username': 'USERNAME',
'os_password': 'PASSWORD',
'os_auth_url': 'http://localhost:35357/v2.0',
'os_auth_token': '',
'os_ironic_api_version': 'latest',
'os_ironic_api_version': version,
}
client = get_client('1', **kwargs)
self.assertEqual(0, mock_retrieve_data.call_count)
self.assertEqual(version, client.http_client.os_ironic_api_version)
self.assertEqual('user', client.http_client.api_version_select_state)
self.assertEqual('latest', client.http_client.os_ironic_api_version)
def test_get_client_with_api_version_latest(self):
self._get_client_with_api_version("latest")
def test_get_client_default_version_set(self):
self.useFixture(fixtures.MonkeyPatch(
'ironicclient.client._get_ksclient', fake_get_ksclient))
def test_get_client_with_api_version_numeric(self):
self._get_client_with_api_version("1.4")
@mock.patch.object(filecache, 'retrieve_data', autospec=True)
@mock.patch.object(ironicclient.client, '_get_ksclient', autospec=True)
def test_get_client_default_version_set_no_cache(self, mock_ksclient,
mock_retrieve_data):
mock_ksclient.return_value = fake_get_ksclient()
mock_retrieve_data.return_value = None
kwargs = {
'os_tenant_name': 'TENANT_NAME',
'os_username': 'USERNAME',
@ -166,6 +181,32 @@ class ClientTest(utils.BaseTestCase):
'os_auth_token': '',
}
client = get_client('1', **kwargs)
mock_retrieve_data.assert_called_once_with(
host=utils.DEFAULT_TEST_HOST,
port=utils.DEFAULT_TEST_PORT)
self.assertEqual(v1.DEFAULT_VER,
client.http_client.os_ironic_api_version)
self.assertEqual('default',
client.http_client.api_version_select_state)
@mock.patch.object(filecache, 'retrieve_data', autospec=True)
@mock.patch.object(ironicclient.client, '_get_ksclient', autospec=True)
def test_get_client_default_version_set_cached(self, mock_ksclient,
mock_retrieve_data):
version = '1.3'
# Make sure we don't coincidentally succeed
self.assertNotEqual(v1.DEFAULT_VER, version)
mock_ksclient.return_value = fake_get_ksclient()
mock_retrieve_data.return_value = version
kwargs = {
'os_tenant_name': 'TENANT_NAME',
'os_username': 'USERNAME',
'os_password': 'PASSWORD',
'os_auth_url': 'http://localhost:35357/v2.0',
'os_auth_token': '',
}
client = get_client('1', **kwargs)
mock_retrieve_data.assert_called_once_with(host=mock.ANY,
port=mock.ANY)
self.assertEqual(version, client.http_client.os_ironic_api_version)
self.assertEqual('cached', client.http_client.api_version_select_state)

View File

@ -0,0 +1,113 @@
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
import os
import dogpile.cache
import mock
from ironicclient.common import filecache
from ironicclient.tests.unit import utils
class FileCacheTest(utils.BaseTestCase):
def test__build_key_ok(self):
result = filecache._build_key('localhost', '5000')
self.assertEqual('localhost:5000', result)
def test__build_key_none(self):
result = filecache._build_key(None, None)
self.assertEqual('None:None', result)
@mock.patch.object(os.path, 'exists', autospec=True)
@mock.patch.object(os, 'makedirs', autospec=True)
@mock.patch.object(dogpile.cache, 'make_region', autospec=True)
def test__get_cache_mkdir(self, mock_makeregion, mock_makedirs,
mock_exists):
cache_val = 6
filecache.CACHE = None
mock_exists.return_value = False
cache_region = mock.Mock(spec=dogpile.cache.region.CacheRegion)
cache_region.configure.return_value = cache_val
mock_makeregion.return_value = cache_region
self.assertEqual(cache_val, filecache._get_cache())
mock_exists.assert_called_once_with(filecache.CACHE_DIR)
mock_makedirs.assert_called_once_with(filecache.CACHE_DIR)
@mock.patch.object(os.path, 'exists', autospec=True)
@mock.patch.object(os, 'makedirs', autospec=True)
def test__get_cache_dir_already_exists(self, mock_makedirs, mock_exists):
cache_val = 5552368
mock_exists.return_value = True
filecache.CACHE = cache_val
self.assertEqual(cache_val, filecache._get_cache())
self.assertEqual(0, mock_exists.call_count)
self.assertEqual(0, mock_makedirs.call_count)
@mock.patch.object(filecache, 'CACHE', autospec=True)
def test_save_data_ok(self, mock_cache):
mock_cache.set = mock.Mock(spec=dogpile.cache.region.CacheRegion.set)
host = 'fred'
port = '1234'
hostport = '%s:%s' % (host, port)
data = 'some random data'
filecache.save_data(host, port, data)
mock_cache.set.assert_called_once_with(hostport, data)
@mock.patch.object(os.path, 'isfile', autospec=True)
@mock.patch.object(filecache, 'CACHE', autospec=True)
def test_retrieve_data_ok(self, mock_cache, mock_isfile):
s = 'spam'
mock_isfile.return_value = True
mock_cache.get = mock.Mock(spec=dogpile.cache.region.CacheRegion.get,
return_value=s)
host = 'fred'
port = '1234'
hostport = '%s:%s' % (host, port)
result = filecache.retrieve_data(host, port)
mock_cache.get.assert_called_once_with(hostport, expiration_time=None)
self.assertEqual(s, result)
@mock.patch.object(os.path, 'isfile', autospec=True)
@mock.patch.object(filecache, 'CACHE', autospec=True)
def test_retrieve_data_ok_with_expiry(self, mock_cache, mock_isfile):
s = 'spam'
mock_isfile.return_value = True
mock_cache.get = mock.Mock(spec=dogpile.cache.region.CacheRegion.get,
return_value=s)
host = 'fred'
port = '1234'
expiry = '987'
hostport = '%s:%s' % (host, port)
result = filecache.retrieve_data(host, port, expiry)
mock_cache.get.assert_called_once_with(hostport,
expiration_time=expiry)
self.assertEqual(s, result)
@mock.patch.object(os.path, 'isfile', autospec=True)
@mock.patch.object(filecache, 'CACHE', autospec=True)
def test_retrieve_data_not_found(self, mock_cache, mock_isfile):
mock_isfile.return_value = True
mock_cache.get = mock.Mock(spec=dogpile.cache.region.CacheRegion.get,
return_value=dogpile.cache.api.NO_VALUE)
host = 'fred'
port = '1234'
hostport = '%s:%s' % (host, port)
result = filecache.retrieve_data(host, port)
mock_cache.get.assert_called_once_with(hostport, expiration_time=None)
self.assertIsNone(result)
@mock.patch.object(os.path, 'isfile', autospec=True)
def test_retrieve_data_no_cache_file(self, mock_isfile):
mock_isfile.return_value = False
self.assertIsNone(filecache.retrieve_data(host='spam', port='eggs'))

View File

@ -18,6 +18,7 @@ import json
import mock
import six
from ironicclient.common import filecache
from ironicclient.common import http
from ironicclient import exc
from ironicclient.tests.unit import utils
@ -27,6 +28,9 @@ HTTP_CLASS = six.moves.http_client.HTTPConnection
HTTPS_CLASS = http.VerifiedHTTPSConnection
DEFAULT_TIMEOUT = 600
DEFAULT_HOST = 'localhost'
DEFAULT_PORT = '1234'
def _get_error_body(faultstring=None, debuginfo=None):
error_body = {
@ -48,6 +52,8 @@ def _session_client(**kwargs):
interface=None,
service_type='publicURL',
region_name='',
endpoint='http://%s:%s' % (DEFAULT_HOST,
DEFAULT_PORT),
**kwargs)
@ -58,9 +64,12 @@ class VersionNegotiationMixinTest(utils.BaseTestCase):
self.test_object = http.VersionNegotiationMixin()
self.test_object.os_ironic_api_version = '1.6'
self.test_object.api_version_select_state = 'default'
self.test_object.endpoint = "http://localhost:1234"
self.mock_mcu = mock.MagicMock()
self.test_object._make_connection_url = self.mock_mcu
self.response = utils.FakeResponse({}, status=406)
self.test_object.get_server = mock.MagicMock(
return_value=('localhost', '1234'))
def test__generic_parse_version_headers_has_headers(self):
response = {'X-OpenStack-Ironic-API-Minimum-Version': '1.1',
@ -76,40 +85,52 @@ class VersionNegotiationMixinTest(utils.BaseTestCase):
result = self.test_object._generic_parse_version_headers(response.get)
self.assertEqual(expected, result)
def test_negotiate_version_bad_state(self):
@mock.patch.object(filecache, 'save_data', autospec=True)
def test_negotiate_version_bad_state(self, mock_save_data):
# Test if bad api_version_select_state value
self.test_object.api_version_select_state = 'word of the day: augur'
self.assertRaises(
RuntimeError,
self.test_object.negotiate_version,
None, None)
self.assertEqual(0, mock_save_data.call_count)
@mock.patch.object(filecache, 'save_data', autospec=True)
@mock.patch.object(http.VersionNegotiationMixin, '_parse_version_headers',
autospec=True)
def test_negotiate_version_server_older(self, mock_pvh):
def test_negotiate_version_server_older(self, mock_pvh, mock_save_data):
# Test newer client and older server
mock_pvh.return_value = ('1.1', '1.5')
latest_ver = '1.5'
mock_pvh.return_value = ('1.1', latest_ver)
mock_conn = mock.MagicMock()
result = self.test_object.negotiate_version(mock_conn, self.response)
self.assertEqual('1.5', result)
self.assertEqual(latest_ver, result)
self.assertEqual(1, mock_pvh.call_count)
host, port = http.get_server(self.test_object.endpoint)
mock_save_data.assert_called_once_with(host=host, port=port,
data=latest_ver)
@mock.patch.object(filecache, 'save_data', autospec=True)
@mock.patch.object(http.VersionNegotiationMixin, '_parse_version_headers',
autospec=True)
def test_negotiate_version_server_newer(self, mock_pvh):
def test_negotiate_version_server_newer(self, mock_pvh, mock_save_data):
# Test newer server and older client
mock_pvh.return_value = ('1.1', '99.99')
mock_conn = mock.MagicMock()
result = self.test_object.negotiate_version(mock_conn, self.response)
self.assertEqual('1.6', result)
self.assertEqual(1, mock_pvh.call_count)
mock_save_data.assert_called_once_with(host=DEFAULT_HOST,
port=DEFAULT_PORT,
data='1.6')
@mock.patch.object(filecache, 'save_data', autospec=True)
@mock.patch.object(http.VersionNegotiationMixin, '_make_simple_request',
autospec=True)
@mock.patch.object(http.VersionNegotiationMixin, '_parse_version_headers',
autospec=True)
def test_negotiate_version_server_no_version_on_error(
self, mock_pvh, mock_msr):
self, mock_pvh, mock_msr, mock_save_data):
# Test older Ironic version which errored with no version number and
# have to retry with simple get
mock_pvh.side_effect = iter([(None, None), ('1.1', '1.2')])
@ -118,10 +139,13 @@ class VersionNegotiationMixinTest(utils.BaseTestCase):
self.assertEqual('1.2', result)
self.assertTrue(mock_msr.called)
self.assertEqual(2, mock_pvh.call_count)
self.assertEqual(1, mock_save_data.call_count)
@mock.patch.object(filecache, 'save_data', autospec=True)
@mock.patch.object(http.VersionNegotiationMixin, '_parse_version_headers',
autospec=True)
def test_negotiate_version_server_explicit_too_high(self, mock_pvh):
def test_negotiate_version_server_explicit_too_high(self, mock_pvh,
mock_save_data):
# requested version is not supported because it is too large
mock_pvh.return_value = ('1.1', '1.6')
mock_conn = mock.MagicMock()
@ -132,10 +156,13 @@ class VersionNegotiationMixinTest(utils.BaseTestCase):
self.test_object.negotiate_version,
mock_conn, self.response)
self.assertEqual(1, mock_pvh.call_count)
self.assertEqual(0, mock_save_data.call_count)
@mock.patch.object(filecache, 'save_data', autospec=True)
@mock.patch.object(http.VersionNegotiationMixin, '_parse_version_headers',
autospec=True)
def test_negotiate_version_server_explicit_not_supported(self, mock_pvh):
def test_negotiate_version_server_explicit_not_supported(self, mock_pvh,
mock_save_data):
# requested version is supported by the server but the server returned
# 406 because the requested operation is not supported with the
# requested version
@ -148,6 +175,13 @@ class VersionNegotiationMixinTest(utils.BaseTestCase):
self.test_object.negotiate_version,
mock_conn, self.response)
self.assertEqual(1, mock_pvh.call_count)
self.assertEqual(0, mock_save_data.call_count)
def test_get_server(self):
host = 'ironic-host'
port = '6385'
endpoint = 'http://%s:%s/ironic/v1/' % (host, port)
self.assertEqual((host, port), http.get_server(endpoint))
class HttpClientTest(utils.BaseTestCase):
@ -358,25 +392,30 @@ class HttpClientTest(utils.BaseTestCase):
result = client._parse_version_headers(fake_resp)
self.assertEqual(expected_result, result)
@mock.patch.object(filecache, 'save_data', autospec=True)
@mock.patch.object(http.HTTPClient, 'get_connection', autospec=True)
def test__http_request_client_fallback_fail(self, mock_getcon):
def test__http_request_client_fallback_fail(self, mock_getcon,
mock_save_data):
# Test when fallback to a supported version fails
host, port, latest_ver = 'localhost', '1234', '1.6'
error_body = _get_error_body()
fake_resp = utils.FakeResponse(
{'X-OpenStack-Ironic-API-Minimum-Version': '1.1',
'X-OpenStack-Ironic-API-Maximum-Version': '1.6',
'X-OpenStack-Ironic-API-Maximum-Version': latest_ver,
'content-type': 'text/plain',
},
six.StringIO(error_body),
version=1,
status=406)
client = http.HTTPClient('http://localhost/')
client = http.HTTPClient('http://%s:%s/' % (host, port))
mock_getcon.return_value = utils.FakeConnection(fake_resp)
self.assertRaises(
exc.UnsupportedVersion,
client._http_request,
'/v1/resources',
'GET')
mock_save_data.assert_called_once_with(host=host, data=latest_ver,
port=port)
@mock.patch.object(http.VersionNegotiationMixin, 'negotiate_version',
autospec=False)

View File

@ -25,6 +25,11 @@ import testtools
from ironicclient.common import http
DEFAULT_TEST_HOST = 'localhost'
DEFAULT_TEST_REGION = 'regionhost'
DEFAULT_TEST_PORT = '6385'
class BaseTestCase(testtools.TestCase):
def setUp(self):
@ -112,9 +117,11 @@ class FakeServiceCatalog(object):
def url_for(self, endpoint_type, service_type, attr=None,
filter_value=None):
if attr == 'region' and filter_value:
return 'http://regionhost:6385/v1/f14b41234'
return 'http://%s:%s/v1/f14b41234' % (DEFAULT_TEST_REGION,
DEFAULT_TEST_PORT)
else:
return 'http://localhost:6385/v1/f14b41234'
return 'http://%s:%s/v1/f14b41234' % (DEFAULT_TEST_HOST,
DEFAULT_TEST_PORT)
class FakeKeystone(object):

View File

@ -13,6 +13,7 @@
# License for the specific language governing permissions and limitations
# under the License.
from ironicclient.common import filecache
from ironicclient.common import http
from ironicclient.common.http import DEFAULT_VER
from ironicclient.v1 import chassis
@ -33,13 +34,22 @@ class Client(object):
def __init__(self, *args, **kwargs):
"""Initialize a new client for the Ironic v1 API."""
# set the default API version header string, if none specified
if not kwargs.get('os_ironic_api_version'):
kwargs['os_ironic_api_version'] = DEFAULT_VER
kwargs['api_version_select_state'] = "default"
else:
if kwargs.get('os_ironic_api_version'):
kwargs['api_version_select_state'] = "user"
else:
# If the user didn't specify a version, use a cached version if
# one has been stored
host, netport = http.get_server(args[0])
saved_version = filecache.retrieve_data(host=host, port=netport)
if saved_version:
kwargs['api_version_select_state'] = "cached"
kwargs['os_ironic_api_version'] = saved_version
else:
kwargs['api_version_select_state'] = "default"
kwargs['os_ironic_api_version'] = DEFAULT_VER
self.http_client = http._construct_http_client(*args, **kwargs)
self.chassis = chassis.ChassisManager(self.http_client)
self.node = node.NodeManager(self.http_client)
self.port = port.PortManager(self.http_client)

View File

@ -2,6 +2,8 @@
# of appearance. Changing the order has an impact on the overall integration
# process, which may cause wedges in the gate later.
anyjson>=0.3.3
appdirs>=1.3.0
dogpile.cache>=0.5.3
httplib2>=0.7.5
lxml>=2.3
oslo.i18n>=1.5.0 # Apache-2.0