openstack
/
ironic
Code Issues Proposed changes

Enable cinder storage interface for generic hardware 

This patch enables cinder storage interface for generic hardware. It
also adds storage_interface field to node resource and driver resource
in API and bumps API version to 1.33 so that storage interface can be
set and shown via API.

Change-Id: I2c74f386291e588a25612f73de08e8367795acff
Partial-Bug: #1559691
This commit is contained in:
Hironori Shiina 2017-05-11 14:18:39 +09:00 committed by Julia Kreger
parent 54d5335edd
commit b90f7a15fb
11 changed files with 256 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
doc/source/contributor/webapi-version-history.rst
View File

@ -2,6 +2,11 @@
REST API Version History
========================
**1.33** (Pike)
Added ``storage_interface`` field to the node object to allow getting and
setting the interface.
**1.32** (Pike)
Added new endpoints for remote volume configuration:

15
ironic/api/controllers/v1/driver.py
View File

@ -64,6 +64,18 @@ _VENDOR_METHODS = {}
_RAID_PROPERTIES = {}
def hide_fields_in_newer_versions(obj):
"""This method hides fields that were added in newer API versions.
Certain fields were introduced at certain API versions.
These fields are only made available when the request's API version
matches or exceeds the versions when these fields were introduced.
"""
if not api_utils.allow_storage_interface():
obj.default_storage_interface = wsme.Unset
obj.enabled_storage_interfaces = wsme.Unset
class Driver(base.APIBase):
"""API representation of a driver."""
@ -91,6 +103,7 @@ class Driver(base.APIBase):
default_network_interface = wtypes.text
default_power_interface = wtypes.text
default_raid_interface = wtypes.text
default_storage_interface = wtypes.text
default_vendor_interface = wtypes.text
"""A list of enabled interfaces for a hardware type"""
@ -102,6 +115,7 @@ class Driver(base.APIBase):
enabled_network_interfaces = [wtypes.text]
enabled_power_interfaces = [wtypes.text]
enabled_raid_interfaces = [wtypes.text]
enabled_storage_interfaces = [wtypes.text]
enabled_vendor_interfaces = [wtypes.text]
@staticmethod
@ -172,6 +186,7 @@ class Driver(base.APIBase):
setattr(driver, 'default_%s_interface' % iface_type, None)
setattr(driver, 'enabled_%s_interfaces' % iface_type, None)
hide_fields_in_newer_versions(driver)
return driver
@classmethod

17
ironic/api/controllers/v1/node.py
View File

@ -153,6 +153,9 @@ def hide_fields_in_newer_versions(obj):
for field in api_utils.V31_FIELDS:
setattr(obj, field, wsme.Unset)
if not api_utils.allow_storage_interface():
obj.storage_interface = wsme.Unset
def update_state_in_older_versions(obj):
"""Change provision state names for API backwards compatibility.
@ -844,6 +847,9 @@ class Node(base.APIBase):
raid_interface = wsme.wsattr(wtypes.text)
"""The raid interface to be used for this node"""
storage_interface = wsme.wsattr(wtypes.text)
"""The storage interface to be used for this node"""
vendor_interface = wsme.wsattr(wtypes.text)
"""The vendor interface to be used for this node"""
@ -995,7 +1001,8 @@ class Node(base.APIBase):
boot_interface=None, console_interface=None,
deploy_interface=None, inspect_interface=None,
management_interface=None, power_interface=None,
raid_interface=None, vendor_interface=None)
raid_interface=None, vendor_interface=None,
storage_interface=None)
# NOTE(matty_dubs): The chassis_uuid getter() is based on the
# _chassis_uuid variable:
sample._chassis_uuid = 'edcad704-b2da-41d5-96d9-afd580ecfa12'
@ -1602,6 +1609,10 @@ class NodesController(rest.RestController):
if getattr(node, field) is not wsme.Unset:
raise exception.NotAcceptable()
if (not api_utils.allow_storage_interface() and
node.storage_interface is not wtypes.Unset):
raise exception.NotAcceptable()
# NOTE(deva): get_topic_for checks if node.driver is in the hash ring
# and raises NoValidHost if it is not.
# We need to ensure that node has a UUID before it can
@ -1666,6 +1677,10 @@ class NodesController(rest.RestController):
if api_utils.get_patch_values(patch, '/%s' % field):
raise exception.NotAcceptable()
s_interface = api_utils.get_patch_values(patch, '/storage_interface')
if s_interface and not api_utils.allow_storage_interface():
raise exception.NotAcceptable()
rpc_node = api_utils.get_rpc_node(node_ident)
remove_inst_uuid_patch = [{'op': 'remove', 'path': '/instance_uuid'}]

11
ironic/api/controllers/v1/utils.py
View File

@ -300,6 +300,8 @@ def check_allowed_fields(fields):
if not allow_dynamic_interfaces():
if set(V31_FIELDS).intersection(set(fields)):
raise exception.NotAcceptable()
if 'storage_interface' in fields and not allow_storage_interface():
raise exception.NotAcceptable()
def check_allowed_portgroup_fields(fields):
@ -557,6 +559,15 @@ def allow_volume():
return pecan.request.version.minor >= versions.MINOR_32_VOLUME
def allow_storage_interface():
"""Check if we should support storage_interface node field.
Version 1.33 of the API added support for storage interfaces.
"""
return (pecan.request.version.minor >=
versions.MINOR_33_STORAGE_INTERFACE)
def get_controller_reserved_names(cls):
"""Get reserved names for a given controller.

4
ironic/api/controllers/v1/versions.py
View File

@ -63,6 +63,7 @@ BASE_VERSION = 1
# v1.30: Add dynamic driver interactions.
# v1.31: Add dynamic interfaces fields to node.
# v1.32: Add volume support.
# v1.33: Add node storage interface
MINOR_0_JUNO = 0
MINOR_1_INITIAL_VERSION = 1
@ -97,11 +98,12 @@ MINOR_29_INJECT_NMI = 29
MINOR_30_DYNAMIC_DRIVERS = 30
MINOR_31_DYNAMIC_INTERFACES = 31
MINOR_32_VOLUME = 32
MINOR_33_STORAGE_INTERFACE = 33
# When adding another version, update MINOR_MAX_VERSION and also update
# doc/source/dev/webapi-version-history.rst with a detailed explanation of
# what the version has changed.
MINOR_MAX_VERSION = MINOR_32_VOLUME
MINOR_MAX_VERSION = MINOR_33_STORAGE_INTERFACE
# String representations of the minor and maximum versions
MIN_VERSION_STRING = '{}.{}'.format(BASE_VERSION, MINOR_1_INITIAL_VERSION)

7
ironic/drivers/generic.py
View File

@ -26,6 +26,8 @@ from ironic.drivers.modules.network import neutron
from ironic.drivers.modules.network import noop as noop_net
from ironic.drivers.modules import noop
from ironic.drivers.modules import pxe
from ironic.drivers.modules.storage import cinder
from ironic.drivers.modules.storage import noop as noop_storage
class GenericHardware(hardware_type.AbstractHardwareType):
@ -65,6 +67,11 @@ class GenericHardware(hardware_type.AbstractHardwareType):
# default. Hence, even if AgentRAID is enabled, NoRAID is the default.
return [noop.NoRAID, agent.AgentRAID]
@property
def supported_storage_interfaces(self):
"""List of supported storage interfaces."""
return [noop_storage.NoopStorage, cinder.CinderStorage]
class ManualManagementHardware(GenericHardware):
"""Hardware type that uses manual power and boot management.

47
ironic/tests/unit/api/v1/test_drivers.py
View File

@ -48,7 +48,7 @@ class TestListDrivers(base.BaseApiTest):
self.dbapi.register_conductor_hardware_interfaces(
c.id, self.d3, 'deploy', ['iscsi', 'direct'], 'direct')
def _test_drivers(self, use_dynamic, detail=False):
def _test_drivers(self, use_dynamic, detail=False, storage_if=False):
self.register_fake_conductors()
headers = {}
expected = [
@ -58,7 +58,10 @@ class TestListDrivers(base.BaseApiTest):
]
expected = sorted(expected, key=lambda d: d['name'])
if use_dynamic:
headers[api_base.Version.string] = '1.30'
if storage_if:
headers[api_base.Version.string] = '1.33'
else:
headers[api_base.Version.string] = '1.30'
path = '/drivers'
if detail:
@ -83,6 +86,12 @@ class TestListDrivers(base.BaseApiTest):
# as this case can't actually happen.
if detail:
self.assertIn('default_deploy_interface', d)
if storage_if:
self.assertIn('default_storage_interface', d)
self.assertIn('enabled_storage_interfaces', d)
else:
self.assertNotIn('default_storage_interface', d)
self.assertNotIn('enabled_storage_interfaces', d)
else:
# ensure we don't spill these fields into driver listing
# one should be enough
@ -94,7 +103,7 @@ class TestListDrivers(base.BaseApiTest):
def test_drivers_with_dynamic(self):
self._test_drivers(True)
def test_drivers_with_dynamic_detailed(self):
def _test_drivers_with_dynamic_detailed(self, storage_if=False):
with mock.patch.object(self.dbapi, 'list_hardware_type_interfaces',
autospec=True) as mock_hw:
mock_hw.return_value = [
@ -112,7 +121,13 @@ class TestListDrivers(base.BaseApiTest):
},
]
self._test_drivers(True, detail=True)
self._test_drivers(True, detail=True, storage_if=storage_if)
def test_drivers_with_dynamic_detailed(self):
self._test_drivers_with_dynamic_detailed()
def test_drivers_with_dynamic_detailed_storage_interface(self):
self._test_drivers_with_dynamic_detailed(storage_if=True)
def _test_drivers_type_filter(self, requested_type):
self.register_fake_conductors()
@ -163,7 +178,8 @@ class TestListDrivers(base.BaseApiTest):
self.assertEqual([], data['drivers'])
@mock.patch.object(rpcapi.ConductorAPI, 'get_driver_properties')
def _test_drivers_get_one_ok(self, use_dynamic, mock_driver_properties):
def _test_drivers_get_one_ok(self, use_dynamic, mock_driver_properties,
storage_if=False):
# get_driver_properties mock is required by validate_link()
self.register_fake_conductors()
@ -176,8 +192,14 @@ class TestListDrivers(base.BaseApiTest):
driver_type = 'classic'
hosts = [self.h1]
headers = {}
if storage_if:
headers[api_base.Version.string] = '1.33'
else:
headers[api_base.Version.string] = '1.30'
data = self.get_json('/drivers/%s' % driver,
headers={api_base.Version.string: '1.30'})
headers=headers)
self.assertEqual(driver, data['name'])
self.assertEqual(sorted(hosts), sorted(data['hosts']))
@ -186,8 +208,7 @@ class TestListDrivers(base.BaseApiTest):
if use_dynamic:
for iface in driver_base.ALL_INTERFACES:
# NOTE(jroll) we don't expose storage interface yet
if iface != 'storage':
if storage_if or iface != 'storage':
self.assertIn('default_%s_interface' % iface, data)
self.assertIn('enabled_%s_interfaces' % iface, data)
self.assertIsNotNone(data['default_deploy_interface'])
@ -204,7 +225,7 @@ class TestListDrivers(base.BaseApiTest):
def test_drivers_get_one_ok_classic(self):
self._test_drivers_get_one_ok(False)
def test_drivers_get_one_ok_dynamic(self):
def _test_drivers_get_one_ok_dynamic(self, storage_if=False):
with mock.patch.object(self.dbapi, 'list_hardware_type_interfaces',
autospec=True) as mock_hw:
mock_hw.return_value = [
@ -222,9 +243,15 @@ class TestListDrivers(base.BaseApiTest):
},
]
self._test_drivers_get_one_ok(True)
self._test_drivers_get_one_ok(True, storage_if=storage_if)
mock_hw.assert_called_once_with([self.d3])
def test_drivers_get_one_ok_dynamic(self):
self._test_drivers_get_one_ok_dynamic()
def test_drivers_get_one_ok_dynamic_storage_interface(self):
self._test_drivers_get_one_ok_dynamic(storage_if=True)
def test_driver_properties_hidden_in_lower_version(self):
self.register_fake_conductors()
data = self.get_json('/drivers/%s' % self.d1,

92
ironic/tests/unit/api/v1/test_nodes.py
View File

@ -116,6 +116,7 @@ class TestListNodes(test_api_base.BaseApiTest):
self.assertNotIn('resource_class', data['nodes'][0])
for field in api_utils.V31_FIELDS:
self.assertNotIn(field, data['nodes'][0])
self.assertNotIn('storage_interface', data['nodes'][0])
# never expose the chassis_id
self.assertNotIn('chassis_id', data['nodes'][0])
@ -149,6 +150,7 @@ class TestListNodes(test_api_base.BaseApiTest):
self.assertIn('resource_class', data)
for field in api_utils.V31_FIELDS:
self.assertIn(field, data)
self.assertIn('storage_interface', data)
# never expose the chassis_id
self.assertNotIn('chassis_id', data)
@ -168,6 +170,14 @@ class TestListNodes(test_api_base.BaseApiTest):
for field in api_utils.V31_FIELDS:
self.assertNotIn(field, data)
def test_node_storage_interface_hidden_in_lower_version(self):
node = obj_utils.create_test_node(self.context,
storage_interface='cinder')
data = self.get_json(
'/nodes/%s' % node.uuid,
headers={api_base.Version.string: '1.32'})
self.assertNotIn('storage_interface', data)
def test_get_one_custom_fields(self):
node = obj_utils.create_test_node(self.context,
chassis_id=self.chassis.id)
@ -267,6 +277,25 @@ class TestListNodes(test_api_base.BaseApiTest):
for field in api_utils.V31_FIELDS:
self.assertIn(field, response)
def test_get_storage_interface_fields_invalid_api_version(self):
node = obj_utils.create_test_node(self.context,
chassis_id=self.chassis.id)
fields = 'storage_interface'
response = self.get_json(
'/nodes/%s?fields=%s' % (node.uuid, fields),
headers={api_base.Version.string: str(api_v1.MIN_VER)},
expect_errors=True)
self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int)
def test_get_storage_interface_fields(self):
node = obj_utils.create_test_node(self.context,
chassis_id=self.chassis.id)
fields = 'storage_interface'
response = self.get_json(
'/nodes/%s?fields=%s' % (node.uuid, fields),
headers={api_base.Version.string: str(api_v1.MAX_VER)})
self.assertIn('storage_interface', response)
def test_detail(self):
node = obj_utils.create_test_node(self.context,
chassis_id=self.chassis.id)
@ -294,6 +323,7 @@ class TestListNodes(test_api_base.BaseApiTest):
self.assertIn('resource_class', data['nodes'][0])
for field in api_utils.V31_FIELDS:
self.assertIn(field, data['nodes'][0])
self.assertIn('storage_interface', data['nodes'][0])
# never expose the chassis_id
self.assertNotIn('chassis_id', data['nodes'][0])
@ -413,6 +443,17 @@ class TestListNodes(test_api_base.BaseApiTest):
headers={api_base.Version.string: "1.32"})
self.assertIn('volume', data)
def test_hide_fields_in_newer_versions_storage_interface(self):
node = obj_utils.create_test_node(self.context,
storage_interface='cinder')
data = self.get_json(
'/nodes/detail', headers={api_base.Version.string: '1.32'})
self.assertNotIn('storage_interface', data['nodes'][0])
new_data = self.get_json(
'/nodes/detail', headers={api_base.Version.string: '1.33'})
self.assertEqual(node.storage_interface,
new_data['nodes'][0]["storage_interface"])
def test_many(self):
nodes = []
for id in range(5):
@ -2013,6 +2054,35 @@ class TestPatch(test_api_base.BaseApiTest):
self.assertEqual(http_client.BAD_REQUEST, response.status_int)
self.assertEqual('application/json', response.content_type)
def test_update_storage_interface(self):
node = obj_utils.create_test_node(self.context,
uuid=uuidutils.generate_uuid())
self.mock_update_node.return_value = node
storage_interface = 'cinder'
headers = {api_base.Version.string: str(api_v1.MAX_VER)}
response = self.patch_json('/nodes/%s' % node.uuid,
[{'path': '/storage_interface',
'value': storage_interface,
'op': 'add'}],
headers=headers)
self.assertEqual('application/json', response.content_type)
self.assertEqual(http_client.OK, response.status_code)
def test_update_storage_interface_old_api(self):
node = obj_utils.create_test_node(self.context,
uuid=uuidutils.generate_uuid())
self.mock_update_node.return_value = node
storage_interface = 'cinder'
headers = {api_base.Version.string: '1.32'}
response = self.patch_json('/nodes/%s' % node.uuid,
[{'path': '/storage_interface',
'value': storage_interface,
'op': 'add'}],
headers=headers,
expect_errors=True)
self.assertEqual('application/json', response.content_type)
self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_code)
def _create_node_locally(node):
driver_factory.check_and_update_node_interfaces(node)
@ -2122,6 +2192,12 @@ class TestPost(test_api_base.BaseApiTest):
self.assertEqual('application/json', response.content_type)
self.assertTrue(response.json['error_message'])
def test_create_node_explicit_storage_interface(self):
headers = {api_base.Version.string: '1.33'}
result = self._test_create_node(headers=headers,
storage_interface='cinder')
self.assertEqual('cinder', result['storage_interface'])
def test_create_node_name_empty_invalid(self):
ndict = test_api_utils.post_get_test_node(name='')
response = self.post_json('/nodes', ndict,
@ -2508,6 +2584,22 @@ class TestPost(test_api_base.BaseApiTest):
self.assertEqual('application/json', response.content_type)
self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int)
def test_create_node_storage_interface_old_api_version(self):
headers = {api_base.Version.string: '1.32'}
ndict = test_api_utils.post_get_test_node(storage_interface='cinder')
response = self.post_json('/nodes', ndict, headers=headers,
expect_errors=True)
self.assertEqual('application/json', response.content_type)
self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int)
def test_create_node_invalid_storage_interface(self):
ndict = test_api_utils.post_get_test_node(storage_interface='foo')
response = self.post_json('/nodes', ndict, expect_errors=True,
headers={api_base.Version.string:
str(api_v1.MAX_VER)})
self.assertEqual('application/json', response.content_type)
self.assertEqual(http_client.BAD_REQUEST, response.status_int)
class TestDelete(test_api_base.BaseApiTest):

7
ironic/tests/unit/api/v1/test_utils.py
View File

@ -419,6 +419,13 @@ class TestApiUtils(base.TestCase):
mock_request.version.minor = 31
self.assertFalse(utils.allow_volume())
@mock.patch.object(pecan, 'request', spec_set=['version'])
def test_allow_storage_interface(self, mock_request):
mock_request.version.minor = 33
self.assertTrue(utils.allow_storage_interface())
mock_request.version.minor = 32
self.assertFalse(utils.allow_storage_interface())
class TestNodeIdent(base.TestCase):

60
ironic/tests/unit/drivers/test_ipmi.py
View File

@ -19,6 +19,8 @@ from ironic.drivers.modules import ipmitool
from ironic.drivers.modules import iscsi_deploy
from ironic.drivers.modules import noop
from ironic.drivers.modules import pxe
from ironic.drivers.modules.storage import cinder
from ironic.drivers.modules.storage import noop as noop_storage
from ironic.tests.unit.db import base as db_base
from ironic.tests.unit.objects import utils as obj_utils
@ -34,17 +36,36 @@ class IPMIHardwareTestCase(db_base.DbTestCase):
enabled_console_interfaces=['no-console'],
enabled_vendor_interfaces=['ipmitool', 'no-vendor'])
def _validate_interfaces(self, task, **kwargs):
self.assertIsInstance(
task.driver.management,
kwargs.get('management', ipmitool.IPMIManagement))
self.assertIsInstance(
task.driver.power,
kwargs.get('power', ipmitool.IPMIPower))
self.assertIsInstance(
task.driver.boot,
kwargs.get('boot', pxe.PXEBoot))
self.assertIsInstance(
task.driver.deploy,
kwargs.get('deploy', iscsi_deploy.ISCSIDeploy))
self.assertIsInstance(
task.driver.console,
kwargs.get('console', noop.NoConsole))
self.assertIsInstance(
task.driver.raid,
kwargs.get('raid', noop.NoRAID))
self.assertIsInstance(
task.driver.vendor,
kwargs.get('vendor', ipmitool.VendorPassthru))
self.assertIsInstance(
task.driver.storage,
kwargs.get('storage', noop_storage.NoopStorage))
def test_default_interfaces(self):
node = obj_utils.create_test_node(self.context, driver='ipmi')
with task_manager.acquire(self.context, node.id) as task:
self.assertIsInstance(task.driver.management,
ipmitool.IPMIManagement)
self.assertIsInstance(task.driver.power, ipmitool.IPMIPower)
self.assertIsInstance(task.driver.boot, pxe.PXEBoot)
self.assertIsInstance(task.driver.deploy, iscsi_deploy.ISCSIDeploy)
self.assertIsInstance(task.driver.console, noop.NoConsole)
self.assertIsInstance(task.driver.raid, noop.NoRAID)
self.assertIsInstance(task.driver.vendor, ipmitool.VendorPassthru)
self._validate_interfaces(task)
def test_override_with_shellinabox(self):
self.config(enabled_console_interfaces=['ipmitool-shellinabox',
@ -56,15 +77,20 @@ class IPMIHardwareTestCase(db_base.DbTestCase):
console_interface='ipmitool-shellinabox',
vendor_interface='no-vendor')
with task_manager.acquire(self.context, node.id) as task:
self.assertIsInstance(task.driver.management,
ipmitool.IPMIManagement)
self.assertIsInstance(task.driver.power, ipmitool.IPMIPower)
self.assertIsInstance(task.driver.boot, pxe.PXEBoot)
self.assertIsInstance(task.driver.deploy, agent.AgentDeploy)
self.assertIsInstance(task.driver.console,
ipmitool.IPMIShellinaboxConsole)
self.assertIsInstance(task.driver.raid, agent.AgentRAID)
self.assertIsInstance(task.driver.vendor, noop.NoVendor)
self._validate_interfaces(
task,
deploy=agent.AgentDeploy,
console=ipmitool.IPMIShellinaboxConsole,
raid=agent.AgentRAID,
vendor=noop.NoVendor)
def test_override_with_cinder_storage(self):
self.config(enabled_storage_interfaces=['noop', 'cinder'])
node = obj_utils.create_test_node(
self.context, driver='ipmi',
storage_interface='cinder')
with task_manager.acquire(self.context, node.id) as task:
self._validate_interfaces(task, storage=cinder.CinderStorage)
class IPMIClassicDriversTestCase(testtools.TestCase):

20
releasenotes/notes/node-storage-interface-api-1d6e217303bd53ff.yaml Normal file
View File

@ -0,0 +1,20 @@
---
features:
- |
Adds version 1.33 of the REST API, which exposes the ``storage_interface``
field of the node resource. This version also exposes
``default_storage_interface`` and ``enable_storage_interfaces`` fields
of the driver resource.
There are 2 available storage interfaces:
* ``noop``: This interface provides nothing regarding storage.
* ``cinder``: This interface enables a node to attach and detach volumes
by leveraging cinder API.
A storage interface can be set when creating or updating a node. Enabled
storage interfaces are defined via the
``[DEFAULT]/enabled_storage_interfaces`` configuration option. A default
interface for a created node can be specified with
``[DEFAULT]/default_storage_interface`` configuration option.