openstack
/
ironic-inspector
Code Issues Proposed changes

Refactor stable API to be /v1/introspection/<UUID> 

Also:
* Stop doing bulk-processing of discovery requests
  Seems like nobody is actually using it and it complicates testing.
* Rename discover.discover -> introspect
* Fix one missing retry_on_conflict (found during test refactor)
* Stop setting unused /extra/discovery_timestamp
* Refactor tests
* Keep old API around of compatibility

Change-Id: I4de50e666298aedcbeddcac27fe2d1ac74472cbb
Implements: blueprint v1-api-reform
This commit is contained in:
Dmitry Tantsur 2015-01-12 15:07:47 +01:00
parent 639c12b3de
commit 8da4fd576b
5 changed files with 239 additions and 228 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

46
ironic_discoverd/discover.py
View File

@ -14,7 +14,6 @@
"""Handling discovery request."""
import logging
import time
import eventlet
from ironicclient import exceptions
@ -31,34 +30,23 @@ VALID_STATES = {'enroll', 'managed', 'inspecting'}
VALID_POWER_STATES = {'power off'}
def discover(uuids):
"""Initiate discovery for given node uuids."""
if not uuids:
raise utils.DiscoveryFailed("No nodes to discover")
def introspect(uuid):
"""Initiate hardware properties introspection for a given node.
:param uuid: node uuid
:raises: DiscoveryFailed
"""
ironic = utils.get_client()
LOG.debug('Validating nodes %s', uuids)
nodes = []
for uuid in uuids:
try:
node = ironic.node.get(uuid)
except exceptions.NotFound:
LOG.error('Node %s cannot be found', uuid)
raise utils.DiscoveryFailed("Cannot find node %s" % uuid, code=404)
except exceptions.HttpError as exc:
LOG.exception('Cannot get node %s', uuid)
raise utils.DiscoveryFailed("Cannot get node %s: %s" % (uuid, exc))
_validate(ironic, node)
nodes.append(node)
try:
node = ironic.node.get(uuid)
except exceptions.NotFound:
LOG.error('Node %s cannot be found', uuid)
raise utils.DiscoveryFailed("Cannot find node %s" % uuid, code=404)
except exceptions.HttpError as exc:
LOG.exception('Cannot get node %s', uuid)
raise utils.DiscoveryFailed("Cannot get node %s: %s" % (uuid, exc))
LOG.info('Proceeding with discovery on node(s) %s',
[n.uuid for n in nodes])
for node in nodes:
eventlet.greenthread.spawn_n(_background_start_discover, ironic, node)
def _validate(ironic, node):
if (node.extra.get('ipmi_setup_credentials') and not
conf.getboolean('discoverd', 'enable_setting_ipmi_credentials')):
msg = 'IPMI credentials setup is disabled in configuration'
@ -91,12 +79,12 @@ def _validate(ironic, node):
raise utils.DiscoveryFailed(
'Failed validation of power interface for node %s' % node.uuid)
eventlet.greenthread.spawn_n(_background_start_discover, ironic, node)
def _background_start_discover(ironic, node):
patch = [{'op': 'add', 'path': '/extra/on_discovery', 'value': 'true'},
{'op': 'add', 'path': '/extra/discovery_timestamp',
'value': str(time.time())}]
ironic.node.update(node.uuid, patch)
patch = [{'op': 'add', 'path': '/extra/on_discovery', 'value': 'true'}]
utils.retry_on_conflict(ironic.node.update, node.uuid, patch)
# TODO(dtantsur): pagination
macs = [p.address for p in ironic.node.list_ports(node.uuid, limit=0)]

79
ironic_discoverd/main.py
View File

@ -14,6 +14,7 @@
import eventlet
eventlet.monkey_patch(thread=False)
import functools
import json
import logging
import sys
@ -34,50 +35,66 @@ app = Flask(__name__)
LOG = logging.getLogger('ironic_discoverd.main')
def check_auth():
if not conf.getboolean('discoverd', 'authenticate'):
return
if not request.headers.get('X-Auth-Token'):
LOG.error("No X-Auth-Token header, rejecting request")
raise utils.DiscoveryFailed('Authentication required', code=401)
try:
utils.get_keystone(token=request.headers['X-Auth-Token'])
except exceptions.Unauthorized:
LOG.error("Keystone denied access, rejecting request")
raise utils.DiscoveryFailed('Access denied', code=403)
# TODO(dtanstur): check for admin role
def convert_exceptions(func):
@functools.wraps(func)
def wrapper(*args, **kwargs):
try:
return func(*args, **kwargs)
except utils.DiscoveryFailed as exc:
return str(exc), exc.http_code
return wrapper
@app.route('/v1/continue', methods=['POST'])
@convert_exceptions
def post_continue():
data = request.get_json(force=True)
LOG.debug("/v1/continue got JSON %s", data)
try:
res = process.process(data)
except utils.DiscoveryFailed as exc:
LOG.debug('/v1/continue failed: %s', exc)
return str(exc), exc.http_code
else:
return json.dumps(res), 200, {'Content-Type': 'applications/json'}
res = process.process(data)
return json.dumps(res), 200, {'Content-Type': 'applications/json'}
@app.route('/v1/introspection/<uuid>')
@app.route('/v1/introspection/<uuid>', methods=['GET', 'POST'])
@convert_exceptions
def introspection(uuid):
# NOTE(dtantsur): in the future this method will also accept PUT
# to initiate introspection.
node_info = node_cache.get_node(uuid)
return flask_json.jsonify(finished=bool(node_info.finished_at),
error=node_info.error or None)
check_auth()
if request.method == 'POST':
discover.introspect(uuid)
return '', 202
else:
node_info = node_cache.get_node(uuid)
return flask_json.jsonify(finished=bool(node_info.finished_at),
error=node_info.error or None)
@app.route('/v1/discover', methods=['POST'])
@convert_exceptions
def post_discover():
if conf.getboolean('discoverd', 'authenticate'):
if not request.headers.get('X-Auth-Token'):
LOG.error("No X-Auth-Token header, rejecting request")
return 'Authentication required', 401
try:
utils.get_keystone(token=request.headers['X-Auth-Token'])
except exceptions.Unauthorized:
LOG.error("Keystone denied access, rejecting request")
return 'Access denied', 403
# TODO(dtanstur): check for admin role
check_auth()
data = request.get_json(force=True)
LOG.debug("/v1/discover got JSON %s", data)
try:
discover.discover(data)
except utils.DiscoveryFailed as exc:
LOG.debug('/v1/discover failed: %s', exc)
return str(exc), exc.http_code
else:
return "", 202
for uuid in data:
discover.introspect(uuid)
return "", 202
def periodic_update(period):

3
ironic_discoverd/test/base.py
View File

@ -45,7 +45,8 @@ class NodeTest(BaseTest):
self.uuid = 'uuid'
self.bmc_address = '1.2.3.4'
self.macs = ['11:22:33:44:55:66', '66:55:44:33:22:11']
self.node = mock.Mock(driver_info={'ipmi_address': self.bmc_address},
self.node = mock.Mock(driver='pxe_ipmitool',
driver_info={'ipmi_address': self.bmc_address},
properties={'cpu_arch': 'i386', 'local_gb': 40},
uuid=self.uuid,
power_state='power on',

304
ironic_discoverd/test/test_discover.py
View File

@ -11,8 +11,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import time
import eventlet
from ironicclient import exceptions
import mock
@ -27,220 +25,220 @@ from ironic_discoverd import utils
@mock.patch.object(eventlet.greenthread, 'sleep', lambda _: None)
@mock.patch.object(eventlet.greenthread, 'spawn_n',
side_effect=lambda f, *a: f(*a) and None)
lambda f, *a: f(*a) and None)
@mock.patch.object(firewall, 'update_filters', autospec=True)
@mock.patch.object(node_cache, 'add_node', autospec=True)
@mock.patch.object(utils, 'get_client', autospec=True)
class TestDiscover(test_base.BaseTest):
class TestDiscover(test_base.NodeTest):
def setUp(self):
super(TestDiscover, self).setUp()
self.node1 = mock.Mock(driver='pxe_ssh',
uuid='uuid1',
driver_info={},
maintenance=True,
instance_uuid=None,
# allowed with maintenance=True
power_state='power on',
provision_state='foobar',
extra={})
self.node2 = mock.Mock(driver='pxe_ipmitool',
uuid='uuid2',
driver_info={'ipmi_address': '1.2.3.4'},
maintenance=False,
instance_uuid=None,
power_state=None,
provision_state=None,
extra={'on_discovery': True})
self.node3 = mock.Mock(driver='pxe_ipmitool',
uuid='uuid3',
driver_info={'ipmi_address': '1.2.3.5'},
maintenance=False,
instance_uuid=None,
power_state='POWER OFF',
provision_state='INSPECTING',
extra={'on_discovery': True})
self.node.power_state = 'power off'
self.node_compat = mock.Mock(driver='pxe_ssh',
uuid='uuid_compat',
driver_info={},
maintenance=True,
# allowed with maintenance=True
power_state='power on',
provision_state='foobar',
extra={'on_discovery': True})
self.ports = [mock.Mock(address=m) for m in self.macs]
self.patch = [{'op': 'add', 'path': '/extra/on_discovery',
'value': 'true'}]
@mock.patch.object(time, 'time', lambda: 42.0)
def test_ok(self, client_mock, add_mock, filters_mock, spawn_mock):
def test_ok(self, client_mock, add_mock, filters_mock):
cli = client_mock.return_value
cli.node.get.side_effect = [
self.node1,
self.node2,
self.node3,
]
ports = [
[mock.Mock(address='1-1'), mock.Mock(address='1-2')],
[mock.Mock(address='2-1'), mock.Mock(address='2-2')],
[],
]
cli.node.list_ports.side_effect = ports
# Failure to powering on does not cause total failure
cli.node.set_power_state.side_effect = [
None,
exceptions.Conflict(), # this is just retried
exceptions.InternalServerError(),
None]
# Failure to set boot device is also not fatal
cli.node.set_boot_device.side_effect = [
exceptions.Conflict(), # this is just retried
None,
exceptions.InternalServerError(),
None]
cli.node.get.return_value = self.node
cli.node.validate.return_value = mock.Mock(power={'result': True})
cli.node.list_ports.return_value = self.ports
discover.discover(['uuid1', 'uuid2', 'uuid3'])
discover.introspect(self.node.uuid)
self.assertEqual(3, cli.node.get.call_count)
self.assertEqual(3, cli.node.list_ports.call_count)
self.assertEqual(3, add_mock.call_count)
cli.node.list_ports.assert_any_call('uuid1', limit=0)
cli.node.list_ports.assert_any_call('uuid2', limit=0)
cli.node.list_ports.assert_any_call('uuid3', limit=0)
add_mock.assert_any_call(self.node1.uuid,
bmc_address=None,
mac=['1-1', '1-2'])
add_mock.assert_any_call(self.node2.uuid,
bmc_address='1.2.3.4',
mac=['2-1', '2-2'])
add_mock.assert_any_call(self.node3.uuid,
bmc_address='1.2.3.5',
mac=[])
cli.node.get.assert_called_once_with(self.uuid)
cli.node.validate.assert_called_once_with(self.uuid)
cli.node.list_ports.assert_called_once_with(self.uuid, limit=0)
cli.node.update.assert_called_once_with(self.uuid, self.patch)
add_mock.assert_called_once_with(self.uuid,
bmc_address=self.bmc_address,
mac=self.macs)
filters_mock.assert_called_with(cli)
self.assertEqual(2, filters_mock.call_count) # 1 node w/o ports
self.assertEqual(4, cli.node.set_boot_device.call_count)
self.assertEqual(4, cli.node.set_power_state.call_count)
cli.node.set_power_state.assert_called_with(mock.ANY, 'reboot')
patch = [{'op': 'add', 'path': '/extra/on_discovery', 'value': 'true'},
{'op': 'add', 'path': '/extra/discovery_timestamp',
'value': '42.0'}]
cli.node.update.assert_any_call('uuid1', patch)
cli.node.update.assert_any_call('uuid2', patch)
cli.node.update.assert_any_call('uuid3', patch)
self.assertEqual(3, cli.node.update.call_count)
spawn_mock.assert_called_with(discover._background_start_discover,
cli, mock.ANY)
self.assertEqual(3, spawn_mock.call_count)
cli.node.set_boot_device.assert_called_once_with(self.uuid,
'pxe',
persistent=False)
cli.node.set_power_state.assert_called_once_with(self.uuid,
'reboot')
def test_setup_ipmi_credentials(self, client_mock, add_mock, filters_mock,
spawn_mock):
def test_retries(self, client_mock, add_mock, filters_mock):
cli = client_mock.return_value
cli.node.get.return_value = self.node
cli.node.validate.side_effect = [exceptions.Conflict,
mock.Mock(power={'result': True})]
cli.node.list_ports.return_value = self.ports
cli.node.update.side_effect = [exceptions.Conflict,
exceptions.Conflict,
None]
cli.node.set_boot_device.side_effect = [exceptions.Conflict,
None]
cli.node.set_power_state.side_effect = [exceptions.Conflict,
None]
discover.introspect(self.node.uuid)
cli.node.get.assert_called_once_with(self.uuid)
cli.node.validate.assert_called_with(self.uuid)
cli.node.list_ports.assert_called_once_with(self.uuid, limit=0)
cli.node.update.assert_called_with(self.uuid, self.patch)
add_mock.assert_called_once_with(self.uuid,
bmc_address=self.bmc_address,
mac=self.macs)
filters_mock.assert_called_with(cli)
cli.node.set_boot_device.assert_called_with(self.uuid,
'pxe',
persistent=False)
cli.node.set_power_state.assert_called_with(self.uuid,
'reboot')
def test_juno_compat(self, client_mock, add_mock, filters_mock):
cli = client_mock.return_value
cli.node.get.return_value = self.node_compat
cli.node.validate.return_value = mock.Mock(power={'result': True})
cli.node.list_ports.return_value = self.ports
discover.introspect(self.node_compat.uuid)
cli.node.get.assert_called_once_with(self.node_compat.uuid)
cli.node.validate.assert_called_once_with(self.node_compat.uuid)
cli.node.list_ports.assert_called_once_with(self.node_compat.uuid,
limit=0)
cli.node.update.assert_called_once_with(self.node_compat.uuid,
self.patch)
add_mock.assert_called_once_with(self.node_compat.uuid,
bmc_address=None,
mac=self.macs)
filters_mock.assert_called_with(cli)
cli.node.set_boot_device.assert_called_once_with(self.node_compat.uuid,
'pxe',
persistent=False)
cli.node.set_power_state.assert_called_once_with(self.node_compat.uuid,
'reboot')
def test_no_macs(self, client_mock, add_mock, filters_mock):
cli = client_mock.return_value
cli.node.get.return_value = self.node
cli.node.list_ports.return_value = []
discover.introspect(self.node.uuid)
cli.node.list_ports.assert_called_once_with(self.uuid, limit=0)
cli.node.update.assert_called_once_with(self.uuid, self.patch)
add_mock.assert_called_once_with(self.uuid,
bmc_address=self.bmc_address,
mac=[])
self.assertFalse(filters_mock.called)
cli.node.set_boot_device.assert_called_once_with(self.uuid,
'pxe',
persistent=False)
cli.node.set_power_state.assert_called_once_with(self.uuid,
'reboot')
def test_setup_ipmi_credentials(self, client_mock, add_mock, filters_mock):
conf.CONF.set('discoverd', 'enable_setting_ipmi_credentials', 'true')
cli = client_mock.return_value
cli.node.get.return_value = self.node1
cli.node.list_ports.return_value = []
cli.node.get.return_value = self.node
cli.node.list_ports.return_value = self.ports
cli.node.validate.side_effect = Exception()
self.node1.extra['ipmi_setup_credentials'] = True
self.node.extra['ipmi_setup_credentials'] = True
discover.discover(['uuid1'])
discover.introspect(self.uuid)
cli.node.update.assert_called_once_with(self.uuid, self.patch)
add_mock.assert_called_once_with(self.uuid,
bmc_address=self.bmc_address,
mac=self.macs)
filters_mock.assert_called_with(cli)
self.assertFalse(cli.node.set_boot_device.called)
self.assertFalse(cli.node.set_power_state.called)
spawn_mock.assert_called_once_with(discover._background_start_discover,
cli, mock.ANY)
def test_setup_ipmi_credentials_disabled(self, client_mock, add_mock,
filters_mock, spawn_mock):
filters_mock):
cli = client_mock.return_value
cli.node.get.return_value = self.node1
cli.node.get.return_value = self.node
cli.node.list_ports.return_value = []
cli.node.validate.side_effect = Exception()
self.node1.extra['ipmi_setup_credentials'] = True
self.node.extra['ipmi_setup_credentials'] = True
self.assertRaisesRegexp(utils.DiscoveryFailed, 'disabled',
discover.discover, ['uuid1'])
discover.introspect, self.uuid)
def test_failed_to_get_node(self, client_mock, add_mock, filters_mock,
spawn_mock):
def test_failed_to_get_node(self, client_mock, add_mock, filters_mock):
cli = client_mock.return_value
cli.node.get.side_effect = [
self.node1,
exceptions.NotFound(),
]
cli.node.get.side_effect = exceptions.NotFound()
self.assertRaisesRegexp(utils.DiscoveryFailed,
'Cannot find node uuid2',
discover.discover, ['uuid1', 'uuid2'])
'Cannot find node',
discover.introspect, self.uuid)
cli.node.get.side_effect = [
exceptions.BadRequest(),
self.node1,
]
cli.node.get.side_effect = exceptions.BadRequest()
self.assertRaisesRegexp(utils.DiscoveryFailed,
'Cannot get node uuid1',
discover.discover, ['uuid1', 'uuid2'])
'Cannot get node',
discover.introspect, self.uuid)
self.assertEqual(3, cli.node.get.call_count)
self.assertEqual(0, cli.node.list_ports.call_count)
self.assertEqual(0, filters_mock.call_count)
self.assertEqual(0, cli.node.set_power_state.call_count)
self.assertEqual(0, cli.node.update.call_count)
self.assertFalse(add_mock.called)
def test_failed_to_validate_node(self, client_mock, add_mock, filters_mock,
spawn_mock):
def test_failed_to_validate_node(self, client_mock, add_mock,
filters_mock):
cli = client_mock.return_value
cli.node.get.side_effect = [
self.node1,
self.node2,
]
cli.node.validate.side_effect = [
mock.Mock(power={'result': True}),
mock.Mock(power={'result': False, 'reason': 'oops'}),
]
cli.node.get.return_value = self.node
cli.node.validate.return_value = mock.Mock(power={'result': False,
'reason': 'oops'})
self.assertRaisesRegexp(
utils.DiscoveryFailed,
'Failed validation of power interface for node uuid2',
discover.discover, ['uuid1', 'uuid2'])
'Failed validation of power interface for node',
discover.introspect, self.uuid)
self.assertEqual(2, cli.node.get.call_count)
self.assertEqual(2, cli.node.validate.call_count)
cli.node.validate.assert_called_once_with(self.uuid)
self.assertEqual(0, cli.node.list_ports.call_count)
self.assertEqual(0, filters_mock.call_count)
self.assertEqual(0, cli.node.set_power_state.call_count)
self.assertEqual(0, cli.node.update.call_count)
self.assertFalse(add_mock.called)
def test_no_uuids(self, client_mock, add_mock, filters_mock, spawn_mock):
self.assertRaisesRegexp(utils.DiscoveryFailed,
'No nodes to discover',
discover.discover, [])
self.assertFalse(client_mock.called)
self.assertFalse(add_mock.called)
def test_wrong_provision_state(self, client_mock, add_mock, filters_mock,
spawn_mock):
self.node2.provision_state = 'active'
def test_wrong_provision_state(self, client_mock, add_mock, filters_mock):
self.node.provision_state = 'active'
cli = client_mock.return_value
cli.node.get.side_effect = [
self.node1,
self.node2,
]
cli.node.get.return_value = self.node
self.assertRaisesRegexp(
utils.DiscoveryFailed,
'node uuid2 with provision state "active"',
discover.discover, ['uuid1', 'uuid2'])
'node uuid with provision state "active"',
discover.introspect, self.uuid)
self.assertEqual(2, cli.node.get.call_count)
self.assertEqual(0, cli.node.list_ports.call_count)
self.assertEqual(0, filters_mock.call_count)
self.assertEqual(0, cli.node.set_power_state.call_count)
self.assertEqual(0, cli.node.update.call_count)
self.assertFalse(add_mock.called)
def test_wrong_power_state(self, client_mock, add_mock, filters_mock,
spawn_mock):
self.node2.power_state = 'power on'
self.node2.maintenance = False
def test_wrong_power_state(self, client_mock, add_mock, filters_mock):
self.node.power_state = 'power on'
cli = client_mock.return_value
cli.node.get.side_effect = [
self.node1,
self.node2,
]
cli.node.get.return_value = self.node
self.assertRaisesRegexp(
utils.DiscoveryFailed,
'node uuid2 with power state "power on"',
discover.discover, ['uuid1', 'uuid2'])
'node uuid with power state "power on"',
discover.introspect, self.uuid)
self.assertEqual(2, cli.node.get.call_count)
self.assertEqual(0, cli.node.list_ports.call_count)
self.assertEqual(0, filters_mock.call_count)
self.assertEqual(0, cli.node.set_power_state.call_count)

35
ironic_discoverd/test/test_main.py
View File

@ -35,44 +35,51 @@ class TestApi(test_base.BaseTest):
super(TestApi, self).setUp()
main.app.config['TESTING'] = True
self.app = main.app.test_client()
@mock.patch.object(discover, 'discover', autospec=True)
def test_discover_no_authentication(self, discover_mock):
conf.CONF.set('discoverd', 'authenticate', 'false')
res = self.app.post('/v1/discover', data='["uuid1"]')
@mock.patch.object(discover, 'introspect', autospec=True)
def test_introspect_no_authentication(self, discover_mock):
conf.CONF.set('discoverd', 'authenticate', 'false')
res = self.app.post('/v1/introspection/uuid1')
self.assertEqual(202, res.status_code)
discover_mock.assert_called_once_with(["uuid1"])
discover_mock.assert_called_once_with("uuid1")
@mock.patch.object(discover, 'discover', autospec=True)
def test_discover_failed(self, discover_mock):
conf.CONF.set('discoverd', 'authenticate', 'false')
@mock.patch.object(discover, 'introspect', autospec=True)
def test_intospect_failed(self, discover_mock):
discover_mock.side_effect = utils.DiscoveryFailed("boom")
res = self.app.post('/v1/discover', data='["uuid1"]')
res = self.app.post('/v1/introspection/uuid1')
self.assertEqual(400, res.status_code)
self.assertEqual(b"boom", res.data)
discover_mock.assert_called_once_with(["uuid1"])
discover_mock.assert_called_once_with("uuid1")
@mock.patch.object(discover, 'discover', autospec=True)
@mock.patch.object(discover, 'introspect', autospec=True)
def test_discover_missing_authentication(self, discover_mock):
conf.CONF.set('discoverd', 'authenticate', 'true')
res = self.app.post('/v1/discover', data='["uuid1"]')
res = self.app.post('/v1/introspection/uuid1')
self.assertEqual(401, res.status_code)
self.assertFalse(discover_mock.called)
@mock.patch.object(utils, 'get_keystone', autospec=True)
@mock.patch.object(discover, 'discover', autospec=True)
@mock.patch.object(discover, 'introspect', autospec=True)
def test_discover_failed_authentication(self, discover_mock,
keystone_mock):
conf.CONF.set('discoverd', 'authenticate', 'true')
keystone_mock.side_effect = keystone_exc.Unauthorized()
res = self.app.post('/v1/discover', data='["uuid1"]',
res = self.app.post('/v1/introspection/uuid1',
headers={'X-Auth-Token': 'token'})
self.assertEqual(403, res.status_code)
self.assertFalse(discover_mock.called)
keystone_mock.assert_called_once_with(token='token')
@mock.patch.object(discover, 'introspect', autospec=True)
def test_discover(self, discover_mock):
res = self.app.post('/v1/discover', data='["uuid1"]')
self.assertEqual(202, res.status_code)
discover_mock.assert_called_once_with("uuid1")
@mock.patch.object(process, 'process', autospec=True)
def test_continue(self, process_mock):
conf.CONF.set('discoverd', 'authenticate', 'true') # should be ignored
process_mock.return_value = [42]
res = self.app.post('/v1/continue', data='"JSON"')
self.assertEqual(200, res.status_code)